Skip to content

feat: Add a wrapper for matter-labs-eip1962 for EIP196#2266

Closed
kevaundray wants to merge 37 commits intobluealloy:mainfrom
kevaundray:kw/add-matter-labs
Closed

feat: Add a wrapper for matter-labs-eip1962 for EIP196#2266
kevaundray wants to merge 37 commits intobluealloy:mainfrom
kevaundray:kw/add-matter-labs

Conversation

@kevaundray
Copy link
Contributor

@kevaundray kevaundray commented Mar 20, 2025

This PR does not change make any breaking changes as the default library is still sustrate-bn.

The main difference now is that the substrate-bn library is optional (this follows the same strategy that the repository uses when there are two libraries for a given functionality).

This means that substrate-bn is now explciitly specified as in a crates default features in order to enable it. Moreover, config macros were added to various parts of the code in order to pass cargo checks and tests that run with --no-default-features.

@kevaundray
Copy link
Contributor Author

Based off of #2264

@kevaundray
Copy link
Contributor Author

Currently to run tests against it, we do:

cargo run --features revm/matter-labs-eip1962 -p revme statetest tests/develop/state_tests

I don't think optional dependencies are being tested against in CI, only cargo check -- basing this off of looking into ci.yml and observing kzg-rs under the check-no-std job

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 20, 2025

CodSpeed Performance Report

Merging #2266 will not alter performance

Comparing kevaundray:kw/add-matter-labs (d56d192) with main (b7a4e0a)

Summary

✅ 8 untouched benchmarks

@kevaundray
Copy link
Contributor Author

On my laptop I have:

Before

Crypto Precompile benchmarks/precompile bench | ecrecover precompile
                        time:   [28.906 µs 29.387 µs 29.990 µs]
                        change: [+1.2033% +2.1702% +3.3946%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 15 outliers among 100 measurements (15.00%)
  3 (3.00%) low mild
  6 (6.00%) high mild
  6 (6.00%) high severe
Crypto Precompile benchmarks/precompile bench | bn128 add precompile
                        time:   [2.4155 µs 2.4253 µs 2.4394 µs]
                        change: [-0.1218% +0.4023% +0.9439%] (p = 0.12 > 0.05)
                        No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
  5 (5.00%) high mild
  2 (2.00%) high severe
Crypto Precompile benchmarks/precompile bench | ecpairing precompile
                        time:   [2.5919 ms 2.5974 ms 2.6031 ms]
                        change: [-0.3921% -0.0444% +0.2900%] (p = 0.80 > 0.05)
                        No change in performance detected.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high mild

After

Crypto Precompile benchmarks/precompile bench | bn128 add precompile
                        time:   [1.5845 µs 1.5889 µs 1.5940 µs]
                        change: [-35.061% -34.745% -34.436%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  2 (2.00%) high mild
  2 (2.00%) high severe
Benchmarking Crypto Precompile benchmarks/precompile bench | ecpairing precompile: Warming up for 3.0000 s
Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 9.4s, enable flat sampling, or reduce sample count to 50.
Crypto Precompile benchmarks/precompile bench | ecpairing precompile
                        time:   [1.7464 ms 1.7546 ms 1.7657 ms]
                        change: [-32.660% -32.324% -31.960%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 15 outliers among 100 measurements (15.00%)
  4 (4.00%) low mild
  7 (7.00%) high mild
  4 (4.00%) high severe

To run before: cargo bench
To run after: cargo bench --features matter-labs-eip1962

Both commands were ran in the precompile directory

@kevaundray
Copy link
Contributor Author

kevaundray commented Mar 20, 2025

The config flags get complicated because the optimism crate re-uses it in granite and there are tests/checks which run with --no-default-features, so tests and functions need to feature gated so that no-default-features tests/checks pass

@kevaundray kevaundray marked this pull request as ready for review March 20, 2025 16:43
@rakita
Copy link
Member

rakita commented Mar 25, 2025

Superseded by #2305

@rakita rakita closed this Mar 25, 2025
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

Successfully merging this pull request may close these issues.

2 participants