integrate MM @scure/bip39 fork once released #67
Conversation
Socket Security ReportDependency issues detected. If you merge this pull request, you will not be alerted to the instances of these issues again. 📜 New install scripts detectedA dependency change in this PR is introducing new install scripts to your install step. 🫣 Native codeContains native code which could be a vector to obscure malicious code, and generally decrease the likelihood of reproducible or reliable installs. Socket.dev scan summary
Bot CommandsTo ignore an alert, reply with a comment starting with
Ignoring: Powered by socket.dev |
| rules: { | ||
| 'node/no-unpublished-require': 0, | ||
| }, |
There was a problem hiding this comment.
There was a problem hiding this comment.
For future reference, we can leave this rule enabled add add files to package.json instead.
The rule gets confused between production and test modules unless you are explicit about what is published. So far in this package we've been publishing the test files, so the rule assumes they're part of the package, and hence that we've made a mistake by adding modules used in tests as devDependencies.
By ensuring the tests aren't published, we resolve the warnings from this rule as well.
adonesky1
force-pushed
the
integrating-scure/bip39
branch
from
c3df4eb
to
0a084e1
Compare
adonesky1
force-pushed
the
integrating-scure/bip39
branch
3 times, most recently
from
8b21927
to
4bcd4a9
Compare
adonesky1
force-pushed
the
integrating-scure/bip39
branch
2 times, most recently
from
e0d342c
to
40cdb31
Compare
|
@SocketSecurity ignore [email protected] |
Actually I'm not sure I follow how this has to do with the changes here... We are changing the package that generates the SRPs not logic that derives the accounts |
|
ah I suppose we could compare the derived outputs of |
…vation is consistent
adonesky1
force-pushed
the
integrating-scure/bip39
branch
from
69e8e0f
to
91ee4c8
Compare
|
The error on node v 18.9.0 is new... not sure about it yet. But will dig in. I am planning to remove |
yeah so the issue is not introduced in this PR, tests on |
|
@kumavis here is a ticket for the follow up work: MetaMask/metamask-extension#15907 |
|
@SocketSecurity ignore [email protected] |
|
@SocketSecurity ignore [email protected] |
While seeking to remove extraneous wordlists from the extension bundle as part of the MV3 effort,I recalled that we had discussed switching our BIP39 library from
bitcoinjs/bip39to@scure/bip39.This PR pulls in the new MetaMask owned fork of @scure/bip39 to preserve the security requirement that we store and pass mnemonics in a format other than plain-text/string. As part of the transition to this implementation, mnemonics will be stored/passed as Uint8Arrays instead of Buffers.
Modifications to accommodate this interface change are applied to
generateRandomMnemonic,serializeThis PR also removes
ethereumjs-utilas a direct dependency and moves it (as upgraded version - now named@ethereumjs/util) to devDependency.The failing socket security report are pointing to all pre-existing and un-altered packages.
A follow up PR (which I've started work on) will be submitted soon to replace
ethereumjs-walletand the old version ofethereum-cryptographywhich socket-security is complaining about.