Skip to content

feat: Add a wrapper for arkworks for EIP196#2305

Merged
rakita merged 44 commits intobluealloy:mainfrom
kevaundray:kw/add-arkworks-lib
Mar 25, 2025
Merged

feat: Add a wrapper for arkworks for EIP196#2305
rakita merged 44 commits intobluealloy:mainfrom
kevaundray:kw/add-arkworks-lib

Conversation

@kevaundray
Copy link
Contributor

This PR supercedes and is based off of #2266 .

It uses the same code that #2266, however it replaces the matter-labs implementaton for EIP9162 with an arkworks implementation. This arkworks implementation was benchmarked in #2285 -- the code in the arkworks.rs file in that PR was essentially copy and pasted.

- remove bn as optional
- if default-features and matter-labs-eip1962 feature is enabled, then we choose matter-labs
- remove conditional configs in other  parts of code since bn is always available
@kevaundray kevaundray changed the title feat: Add arkworks library feat: Add a wrapper for arkworks for EIP196 Mar 25, 2025
@kevaundray
Copy link
Contributor Author

42aaa8c makes the substrate-bn implementation optional, I can also split this off into a separate PR

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 25, 2025

CodSpeed Performance Report

Merging #2305 will improve performances by ×2.9

Comparing kevaundray:kw/add-arkworks-lib (d68d720) with main (4003576)

Summary

⚡ 2 improvements
✅ 6 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
precompile bench | bn128 mul precompile 650.8 µs 343.2 µs +89.61%
precompile bench | ecpairing precompile 13.9 ms 4.9 ms ×2.9

@kevaundray kevaundray marked this pull request as ready for review March 25, 2025 18:02
Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, amazing work @kevaundray!

@rakita rakita merged commit efe528e into bluealloy:main Mar 25, 2025
28 checks passed
@github-actions github-actions bot mentioned this pull request Mar 25, 2025
rakita pushed a commit that referenced this pull request Mar 26, 2025
* add read_scalar, point_add and point_mul into the wrapper

* modify bn128.rs to use new api methods

* preserve previous semantics

* initial commit to add matter-labs wrapper

* feature gate matter-labs impl -- make substrate impl still default

* update revm and precompile cargo.toml file

* use cfg_if

* make bn an optional dependency

* cfg else -> else_if so that there is no silent fallback

* follow same cfg_if pattern as other precompiles

* fix optimism

* add back `self` import

* Push empty commit to trigger CI

* cargo fmt

* clippy fix

* make `bn` the default with revm and revme

* fix typo

* Update crates/revm/Cargo.toml

* Update crates/revm/Cargo.toml

* Update crates/precompile/src/lib.rs

* Update crates/optimism/Cargo.toml

* revert Cargo.toml formatting

* multi:

- remove bn as optional
- if default-features and matter-labs-eip1962 feature is enabled, then we choose matter-labs
- remove conditional configs in other  parts of code since bn is always available

* revert crates/optimism/src/precompiles.rs

* revert crates/optimism/src/evm.rs

* revert unnecessary changes in crates/precompile/src/lib.rs

* revert Cargo.toml changes

* revert automatic Cargo.toml formatting

* revert formatting on secp256k1 in Cargo.toml

* revert c-kzg formatting in Cargo.toml

* revert dev key in Cargo.toml

* revert c-kzg feature in Cargo.toml

* Update crates/precompile/Cargo.toml

* Apply suggestions from code review

* initial commit of arkworks file -- replace matter-labs

* add arkworks depedency

* grep replace matter-labs with arkworks

* make substrate-bn impl optional

* propagate arkworks/std

* add comment for substrate-bn impl

* Update crates/precompile/src/bn128/arkworks.rs

---------
rakita added a commit that referenced this pull request Mar 26, 2025
* temp

* feat: Add JournalInner

* fix(ci): Still run statetests from eth/tests repo (#2306)

* feat: Add a wrapper for arkworks for EIP196 (#2305)

* add read_scalar, point_add and point_mul into the wrapper

* modify bn128.rs to use new api methods

* preserve previous semantics

* initial commit to add matter-labs wrapper

* feature gate matter-labs impl -- make substrate impl still default

* update revm and precompile cargo.toml file

* use cfg_if

* make bn an optional dependency

* cfg else -> else_if so that there is no silent fallback

* follow same cfg_if pattern as other precompiles

* fix optimism

* add back `self` import

* Push empty commit to trigger CI

* cargo fmt

* clippy fix

* make `bn` the default with revm and revme

* fix typo

* Update crates/revm/Cargo.toml

* Update crates/revm/Cargo.toml

* Update crates/precompile/src/lib.rs

* Update crates/optimism/Cargo.toml

* revert Cargo.toml formatting

* multi:

- remove bn as optional
- if default-features and matter-labs-eip1962 feature is enabled, then we choose matter-labs
- remove conditional configs in other  parts of code since bn is always available

* revert crates/optimism/src/precompiles.rs

* revert crates/optimism/src/evm.rs

* revert unnecessary changes in crates/precompile/src/lib.rs

* revert Cargo.toml changes

* revert automatic Cargo.toml formatting

* revert formatting on secp256k1 in Cargo.toml

* revert c-kzg formatting in Cargo.toml

* revert dev key in Cargo.toml

* revert c-kzg feature in Cargo.toml

* Update crates/precompile/Cargo.toml

* Apply suggestions from code review

* initial commit of arkworks file -- replace matter-labs

* add arkworks depedency

* grep replace matter-labs with arkworks

* make substrate-bn impl optional

* propagate arkworks/std

* add comment for substrate-bn impl

* Update crates/precompile/src/bn128/arkworks.rs

---------

* feat: Add criterion to revme bench command (#2295)

* temp code preview

* add criterion to benchmarks

* added args to fn run() calls in bins/revme/

* modifed the code inside the benchmark tests

* left only .replay function inside bench_functions and added minor revisions to the rest of the code

* edited comment annotation in burntpix.rs file

* added comments and set different default value for warm_up_time

* clear all Journal fields

* doc

* formating change

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
Co-authored-by: nemnem7 <115541090+nemnem7@users.noreply.github.com>
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