-
Notifications
You must be signed in to change notification settings - Fork 299
Removing hdkey native dependency #128
Removing hdkey native dependency #128
Conversation
Leaving some notes as requested... PR pulls
It should now be possible to rewrite this line as an ethereumjs-wallet/src/hdkey.ts Line 3 in 2121d5a
In sum, this is +/- the same package but aligns the deps here with js-ethereum-cryptography's goal of standardizing the way crypto packages are managed across the eco-system's libraries. The only relevant Trail of Bits audit recommendation has been addressed (though technically the hdkey submodule at j-e-c is not the latest published version.) LGTM 👍 |
Thanks for this comprehensive write-up @cgewecke! @nebojsa94 Can you update the HDKey requires to imports (there is another one in the tests) as suggested by Chris? I think we are then ready with all the changes (the |
@holgerd77 I've updated the HDKey require to import in hdkey.ts, I wasn't able to locate the require in tests, can you point the line? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nebojsa94 ah, sorry, my bad, had a too rough look at my local editor search results.
This looks good now, thanks, will merge.
Hey @cgewecke ! Thanks for digging into this. As you mentioned, the hdkey version is outdated, but the same changes are applied locally. I'm working on getting those incorporated upstream here: cryptocoinjs/hdkey#41 |
This PR removes hdkey dependency and instead uses ethereum-cryptography package that doesn't require native dependency compiling.