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

Upgrade ethereumjs and cryptography libraries #427

Open
paulmillr opened this issue Jun 29, 2022 · 4 comments
Open

Upgrade ethereumjs and cryptography libraries #427

paulmillr opened this issue Jun 29, 2022 · 4 comments

Comments

@paulmillr
Copy link

js-sha3 and secp256k1 should be replaced by ethereum-cryptography package: https://github.com/ethereum/js-ethereum-cryptography. Check out the blog post about it: https://medium.com/nomic-labs-blog/a-safer-smaller-and-faster-ethereum-cryptography-stack-5eeb47f62d79

Also ethereumjs team did a great job on the new releases, which should be out any way now. The releases switch from bn.js to native bigints and massively decrease dependency burden.

@paulmillr
Copy link
Author

paulmillr commented Jul 25, 2022

GridPlus is one of packages that is used by metamask. It would be great to update gridplus when metamask would get new cryptography.

Should I make a pull request? @alex-miller-0 @douglance @ledbetterljoshua

All of those can be replaced by ethereum-cryptography:

    "aes-js": "^3.1.1",
    "bech32": "^2.0.0",
    "bignumber.js": "^9.0.1",
    "bs58check": "^2.1.2",
    "elliptic": "6.5.4",
    "hash.js": "^1.1.7",
    "js-sha3": "^0.8.0",
    "secp256k1": "4.0.2",

@alex-miller-0
Copy link
Contributor

@paulmillr thanks for following up - a PR would be appreciated!

One issue to watch out for would be Uint8Array vs Buffer types. It looks like ethereum-cryptography uses u8 arrays exclusively, but I believe some of the currently used packages (e.g. elliptic) use Buffer types and we probably don't want to mix those up. @douglance went through a big refresh of this codebase so he would probably know better than me where we are using Buffers.

Overall I would prefer to have a single crypto library so I support this. Even better if MetaMask is already importing it.

@paulmillr
Copy link
Author

if you want to keep buffers around, I can do Buffer.from(ui8a) in all calls, that wrapper shouldn't be a noticeable perf hit. They are basically the same.

We've chosen to get rid of buffers in the eth-crypto because ui8a are native to browsers while Buffers require a third party dependency.

@alex-miller-0
Copy link
Contributor

We want to get rid of buffers too. I was just pointing out that we still use them and don't want to mix types. I think the best path forward would be converting everything to u8a but that will require some work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants