Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build(core): fix use of c-kzg flag and cleanup dependencies #1927

Merged
merged 9 commits into from
Feb 13, 2025

Conversation

mpaulucci
Copy link
Collaborator

@mpaulucci mpaulucci commented Feb 12, 2025

Motivation
We were having to add dependencies with default-features = false everywhere, which also didn't scale, since if one dependency adds the flag, it is added everywhere. We need to disable certain flags to be able to prove with ZK.

Description

  • Ordered crates alphabetically
  • Just kept cmd/ethrex as the default when running cargo build.
  • Removed c-kzg as a default flag in our crates. Added them to the commands that require them: ethrex, ef_tests
  • Changed dependencies to use the workspace version.

@mpaulucci mpaulucci requested a review from a team as a code owner February 12, 2025 13:46
Copy link

github-actions bot commented Feb 12, 2025

Total lines added: 0
Total lines removed: 0
Total lines changed: 0

Copy link

Summary: 13759/16775 (82.02%)
Prague: 2373/2373 (100.00%)
Cancun: 3579/3579 (100.00%)
Shanghai: 221/221 (100.00%)
Paris: 62/62 (100.00%)
London: 39/39 (100.00%)
Berlin: 2/35 (5.71%)
Istanbul: 34/35 (97.14%)
Constantinople: 1645/2406 (68.37%)
Petersburg: 2121/2400 (88.38%)
Byzantium: 2034/2330 (87.30%)
Homestead: 641/1324 (48.41%)
Frontier: 157/742 (21.16%)
SpuriousDragon: 451/579 (77.89%)
Tangerine: 400/650 (61.54%)

@@ -1,30 +1,30 @@
[workspace]
members = [
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

reorder alphabetically

Copy link

github-actions bot commented Feb 12, 2025

Benchmark Results Comparison

PR Results

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Factorial 239.6 ± 0.7 238.6 240.7 1.00
levm_Factorial 914.6 ± 16.0 901.2 954.3 3.82 ± 0.07

Benchmark Results: Factorial - Recursive

Command Mean [s] Min [s] Max [s] Relative
revm_FactorialRecursive 1.479 ± 0.090 1.370 1.626 1.00
levm_FactorialRecursive 15.668 ± 0.031 15.636 15.746 10.59 ± 0.64

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Fibonacci 211.8 ± 0.8 209.9 212.8 1.00
levm_Fibonacci 915.4 ± 18.6 892.4 949.6 4.32 ± 0.09

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ManyHashes 8.9 ± 0.1 8.8 9.0 1.00
levm_ManyHashes 18.3 ± 0.2 18.1 18.6 2.06 ± 0.03

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
revm_BubbleSort 3.250 ± 0.041 3.223 3.362 1.00
levm_BubbleSort 6.122 ± 0.026 6.096 6.174 1.88 ± 0.02

Benchmark Results: ERC20 - Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Transfer 260.8 ± 8.8 250.9 280.2 1.00
levm_ERC20Transfer 541.8 ± 2.8 536.7 547.3 2.08 ± 0.07

Benchmark Results: ERC20 - Mint

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Mint 142.2 ± 0.5 141.5 142.8 1.00
levm_ERC20Mint 355.3 ± 4.9 350.0 365.4 2.50 ± 0.04

Benchmark Results: ERC20 - Approval

Command Mean [s] Min [s] Max [s] Relative
revm_ERC20Approval 1.064 ± 0.008 1.054 1.080 1.00
levm_ERC20Approval 2.034 ± 0.007 2.018 2.043 1.91 ± 0.02

Main Results

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Factorial 234.6 ± 1.3 232.9 237.3 1.00
levm_Factorial 913.0 ± 10.3 903.9 936.1 3.89 ± 0.05

Benchmark Results: Factorial - Recursive

Command Mean [s] Min [s] Max [s] Relative
revm_FactorialRecursive 1.457 ± 0.095 1.357 1.602 1.00
levm_FactorialRecursive 15.810 ± 0.047 15.748 15.889 10.85 ± 0.71

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Fibonacci 213.0 ± 3.2 210.8 221.7 1.00
levm_Fibonacci 927.5 ± 25.2 903.3 971.1 4.35 ± 0.13

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ManyHashes 9.2 ± 0.1 9.1 9.3 1.00
levm_ManyHashes 19.0 ± 0.3 18.5 19.6 2.07 ± 0.04

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
revm_BubbleSort 3.257 ± 0.048 3.210 3.379 1.00
levm_BubbleSort 6.186 ± 0.024 6.146 6.215 1.90 ± 0.03

Benchmark Results: ERC20 - Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Transfer 253.1 ± 1.4 251.6 256.1 1.00
levm_ERC20Transfer 549.0 ± 8.7 541.9 571.4 2.17 ± 0.04

Benchmark Results: ERC20 - Mint

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Mint 143.0 ± 1.1 141.7 145.5 1.00
levm_ERC20Mint 356.9 ± 1.6 354.5 359.6 2.50 ± 0.02

Benchmark Results: ERC20 - Approval

Command Mean [s] Min [s] Max [s] Relative
revm_ERC20Approval 1.061 ± 0.007 1.051 1.072 1.00
levm_ERC20Approval 2.071 ± 0.029 2.048 2.127 1.95 ± 0.03

@mpaulucci mpaulucci marked this pull request as draft February 12, 2025 17:55
@mpaulucci mpaulucci changed the title build(core): only cargo build ethrex by default. build(core): fix use of c-kzg flag and cleanup dependencies Feb 12, 2025
@mpaulucci mpaulucci marked this pull request as ready for review February 12, 2025 18:57
Copy link
Contributor

@xqft xqft left a comment

Choose a reason for hiding this comment

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

nice

Copy link
Contributor

@fborello-lambda fborello-lambda left a comment

Choose a reason for hiding this comment

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

Tested it locally 🚀 .

@mpaulucci mpaulucci enabled auto-merge February 13, 2025 15:03
@mpaulucci mpaulucci added this pull request to the merge queue Feb 13, 2025
Merged via the queue into main with commit 3834cb2 Feb 13, 2025
44 checks passed
@mpaulucci mpaulucci deleted the only-build-ethrex-default branch February 13, 2025 15:14
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.

3 participants