Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps: upgrade from ethereumjs-util #3943

Merged
merged 15 commits into from
Feb 24, 2024
Merged

Conversation

legobeat
Copy link
Contributor

@legobeat legobeat commented Feb 21, 2024

Explanation

Legacy ethereumjs-util is used primarily for two things: its re-exported bn.js instance and a handful of bytes/hex/string utility functions.

  • Remove the first category by importing BN directly from bn.js instead of via ethereumjs-util.
    • The version is the same. No user-facing changes.
  • Replace ethereumjs-util library functions with equivalent from @metamask/utils if already available
    • Mainly add0x/remove0x
  • Use ethereum-cryptography for crypto functions no longer available in @ethereumjs/util
    • chore: Add TextEncoder awareness to test runtime of controller-utils (needed for crypto hash function)
  • Use @ethereumjs/util@^8.1.0 for remaining ethereumjs-util imports

References

Changelog

@metamask/accounts-controller

  • CHANGED: Replace ethereumjs-util with @ethereumjs/util and ethereum-cryptography

@metamask/assets-controller

  • CHANGED: Replace ethereumjs-util with @ethereumjs/util and bn.js

@metamask/controller-utils

  • CHANGED: Replace ethereumjs-util with @ethereumjs/util

@metamask/gas-fee-controller

  • CHANGED: Replace ethereumjs-util with bn.js

@metamask/keyring-controller

  • CHANGED: Replace ethereumjs-util with @ethereumjs/util

@metamask/message-manager

  • CHANGED: Remove dependency ethereumjs-util

@metamask/signature-controller

  • CHANGED: Remove dependency ethereumjs-util

@metamask/transaction-controller

  • CHANGED: Replace ethereumjs-util with @ethereumjs/util and bn.js

@metamask/user-operation-controller

  • CHANGED: Replace ethereumjs-util with bn.js

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

Copy link

socket-security bot commented Feb 21, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@types/bn.js@5.1.5 None +1 3.57 MB types

🚮 Removed packages: npm/@types/bn.js@5.1.1

View full report↗︎

Copy link

socket-security bot commented Feb 21, 2024

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

View full report↗︎

@@ -41,7 +42,6 @@
"@metamask/message-manager": "^7.3.8",
"@metamask/utils": "^8.3.0",
"async-mutex": "^0.2.6",
"ethereumjs-util": "^7.0.10",
"ethereumjs-wallet": "^1.0.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Updating to @ethereumjs/wallet (and thereby a transitive dependency on ethereumjs-util) breaks Node.js 16 support, hence blocked by #3611

@legobeat legobeat marked this pull request as ready for review February 21, 2024 11:57
@legobeat legobeat requested a review from a team as a code owner February 21, 2024 11:57
@legobeat legobeat added the dependencies Pull requests that update a dependency file label Feb 21, 2024
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few things I noticed, but otherwise looks good.

package.json Outdated Show resolved Hide resolved
packages/controller-utils/src/util.ts Show resolved Hide resolved
packages/assets-controllers/package.json Outdated Show resolved Hide resolved
packages/gas-fee-controller/package.json Outdated Show resolved Hide resolved
packages/transaction-controller/package.json Show resolved Hide resolved
@legobeat legobeat requested review from mcmire and a team February 22, 2024 03:12
@legobeat legobeat force-pushed the deps-ethereumjs branch 2 times, most recently from ae7e3f3 to 69a88bb Compare February 22, 2024 12:28
TODO: see if safe to stop double-trimming leading as well as trailing
zeroes in getTokenSymbol and thereby replace toUtf8 with something
simpler.
…til and @metamask/utils

- test(keyring-controller): Update expected error message when importing invalid key
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@legobeat legobeat merged commit 40acc6c into MetaMask:main Feb 24, 2024
138 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants