Skip to content

monorepo: upgrade ethereum-crypography and reduce reliance on it#4030

Merged
acolytec3 merged 14 commits into
masterfrom
monorepo/reduce-reliance-ethereum-cryptography
Apr 25, 2025
Merged

monorepo: upgrade ethereum-crypography and reduce reliance on it#4030
acolytec3 merged 14 commits into
masterfrom
monorepo/reduce-reliance-ethereum-cryptography

Conversation

@gabrocheleau

Copy link
Copy Markdown
Contributor

This PR:

  • Upgrades ethereum-cryptography the latest release (3.2.0)
  • Refactors direct @noble/hashes and @noble/curves re-exports from ethereum-cryptography to directly use those rather than going through ethereum-cryptography.

I was evaluated the possibility of fully removing reliance on ethereum-cryptography, but this would require re-writing some helpers or wrapper functions (e.g. randomBytes, utf8ToBytes, byteToUtf8, equalsBytes and some others). We can consider doing that eventually if we feel strongly about it.

This PR still allows us to remove the ethereum-cryptography direct dependencies from several packages.

@codecov

codecov Bot commented Apr 25, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 95.86777% with 5 lines in your changes missing coverage. Please review.

Project coverage is 79.56%. Comparing base (f10cb80) to head (e490bb9).
Report is 1 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 84.33% <100.00%> (ø)
blockchain 89.32% <ø> (ø)
client 67.99% <100.00%> (?)
common 97.51% <ø> (ø)
devp2p 86.78% <100.00%> (ø)
evm 73.11% <75.00%> (ø)
mpt 90.05% <100.00%> (+0.30%) ⬆️
statemanager 69.06% <93.33%> (ø)
static 99.11% <90.90%> (ø)
tx 90.59% <100.00%> (ø)
util 89.36% <100.00%> (ø)
vm 55.50% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@acolytec3 acolytec3 merged commit 1f8f8c7 into master Apr 25, 2025
@acolytec3 acolytec3 deleted the monorepo/reduce-reliance-ethereum-cryptography branch April 25, 2025 20:45
@holgerd77

Copy link
Copy Markdown
Member

Urgent:

This change substantially increases bundle sizes by double-including Noble now several times, e.g. Block going up from 324KB to 442KB:

grafik

Before:

grafik

After:

grafik

(this is actually the reason why we just should not do these kind of last minute changes, because it increases the risk of something happening)

So, this needs urgently to be investigated.

In doubt (my preference) we should revert the whole PR, and put this aside.

@paulmillr

paulmillr commented Apr 26, 2025

Copy link
Copy Markdown
Contributor

I was evaluated the possibility of fully removing reliance on ethereum-cryptography, but this would require re-writing some helpers or wrapper functions (e.g. randomBytes, utf8ToBytes, byteToUtf8, equalsBytes and some others). We can consider doing that eventually if we feel strongly about it.

@gabrocheleau what's up with that methods?

randomBytes, utf8ToBytes, byteToUtf8, equalsBytes

All of them are present in either noble-hashes or noble-curves. hexToBytes in E-C is a wrapper around hashes hexToBytes which adds 0x prefix. curves have constant-time equalsBytes which may be slightly less performant but it's still ok.

https://github.com/ethereum/js-ethereum-cryptography/blob/31f980b2847545d33268f2510ba38a3836202a44/src/utils.ts#L10

@gabrocheleau

Copy link
Copy Markdown
Contributor Author

I was evaluated the possibility of fully removing reliance on ethereum-cryptography, but this would require re-writing some helpers or wrapper functions (e.g. randomBytes, utf8ToBytes, byteToUtf8, equalsBytes and some others). We can consider doing that eventually if we feel strongly about it.

@gabrocheleau what's up with that methods?

randomBytes, utf8ToBytes, byteToUtf8, equalsBytes

All of them are present in either noble-hashes or noble-curves. hexToBytes in E-C is a wrapper around hashes hexToBytes which adds 0x prefix. curves have constant-time equalsBytes which may be slightly less performant but it's still ok.

ethereum/js-ethereum-cryptography@31f980b/src/utils.ts#L10

Indeed, the wrapper-related logic is minimal, but we would still need to rewrite these wrappers (if we need them) in our library, which we felt was a bit too "last-minute" for the imminent release. Not saying we won't do it, just out of scope for this release. Also have to evaluate impact on bundle-size

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants