Skip to content

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

@socket-security
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↗︎

@socket-security
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↗︎

"@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.

@legobeat legobeat requested review from a team and mcmire 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!

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