ethereumjs / ethereumjs-monorepo

Monorepo for the Ethereum VM TypeScript Implementation
2.6k stars 760 forks source link

Additional Cryptography Primitives Usage / Devp2p #3246

Open acolytec3 opened 10 months ago

acolytec3 commented 10 months ago

Starting with #3192 and continuing in #3245, we have begun incorporating cryptographic primitives from @polkadot/wasm-crypto where applicable within our codebase in the name of improving performance (since the wasm versions of these primitives are 2-10x faster on average).

Outstanding items include

paulmillr commented 10 months ago

@acolytec3 @holgerd77 I've mentioned it several times - if hashing is the bottleneck, we can use the unrolled implementation of sha3, present in unrolled-nbl-hashes-sha3. It can also be audited.

Switching to the package gives 70-97% of the gained speed benefit: details, source code.

noble noble unrolled wasm
1k-3-32-ran 194 131 117
1k-5-32-ran 237 164 149
1k-9-32-ran 259 189 168
1k-1k-32-ran 272 192 175
1k-1k-32-mir 286 206 189
checkp: 5000 3901 5224 5373

wasm-crypto is basically 256 kilobytes of obfuscated, unauditable code. See bundle-polkadot-wasm-crypto.js. There are no reproducible deterministic builds, etc. How would you know it doesn't steal private keys?

holgerd77 commented 10 months ago

@paulmillr thanks for the summary!

This is not so much about a specific solution (WASM!) but the work we have done in e.g. #3192 and follow-up PRs is about generally getting more flexible about what crypto to use depending on the use case. So it is basically easier to swap the crypto libraries in a structured way now.

For the client we have decided to go WASM by default, since this is substantially faster for the different crypto primitives and the client both does not handle private keys and is mainly used for testnet/devnet runs, and there we benefit strongly from faster execution times and at the same time security requirements are not high.

If the client package is started to get adopted in more serious production cases in the future we might need to revise this decision. The unrolled version of the hashing package might definitely be an option to consider, thanks for the pointer.

holgerd77 commented 2 months ago

Have renamed this with some more devp2p focus, since that might be the part still worth to tackle.

acolytec3 commented 1 month ago

Going to close since we're moving away from wasm deps where possible rather than looking to expand.

holgerd77 commented 1 month ago

Hmm, this is mainly targeted at the client, there we have all WASM in and still look for speed and security is not that much of a factor (and once we modularize we can still keep a fast less-secure WASM version), so I would tend to not close, these devp2p switches might bring somewhat significant improvements and reduce CPU load. Will reopen for now.