GridPlus / gridplus-sdk

SDK for communicating with the GridPlus Lattice1 hardware wallet
MIT License
48 stars 23 forks source link

Upgrade ethereumjs and cryptography libraries #427

Open paulmillr opened 2 years ago

paulmillr commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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.