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

refactor(l1, l2, levm): fix shared dependencies in workspace Cargo.toml #1680

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ilitteri
Copy link
Contributor

  • Fix shared dependencies in workspace Cargo.toml
  • Separate them in three blocks
    • ethrex libs
    • worspace libs (the ones shared between different crates)
    • non-shared libs
  • Update serde_json version
  • Update serde version
  • Update bytes version

1. ethrex deps
2. workspace deps
3. subcrate dep
@ilitteri ilitteri added levm Lambda EVM implementation L2 L1 labels Jan 10, 2025
@ilitteri ilitteri self-assigned this Jan 10, 2025
@ilitteri ilitteri requested a review from a team as a code owner January 10, 2025 14:07
Copy link

The amount of lines of code in the project has not changed.

@ilitteri ilitteri marked this pull request as draft January 10, 2025 14:26
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.

Look good, one comment regarding the l2 crate

crates/l2/prover/zkvm/interface/Cargo.toml Outdated Show resolved Hide resolved
@ilitteri ilitteri marked this pull request as ready for review January 10, 2025 15:03
Copy link

The amount of lines of code in the project has not changed.

1 similar comment
Copy link

The amount of lines of code in the project has not changed.

Copy link

The amount of lines of code in the project has not changed.

Copy link

Benchmark Results Comparison

PR Results

Benchmark Results: Factorial

Command Mean [s] Min [s] Max [s] Relative
revm_factorial 7.131 ± 0.037 7.028 7.160 1.00
levm_factorial 26.341 ± 0.184 26.119 26.634 3.69 ± 0.03

Benchmark Results: Fibonacci

Command Mean [s] Min [s] Max [s] Relative
revm_fibonacci 8.299 ± 1.681 6.999 10.581 1.00
levm_fibonacci 24.397 ± 0.804 23.863 26.603 2.94 ± 0.60

Main Results

Benchmark Results: Factorial

Command Mean [s] Min [s] Max [s] Relative
revm_factorial 7.089 ± 0.008 7.080 7.106 1.00
levm_factorial 26.577 ± 0.792 26.021 28.774 3.75 ± 0.11

Benchmark Results: Fibonacci

Command Mean [s] Min [s] Max [s] Relative
revm_fibonacci 7.254 ± 0.036 7.184 7.288 1.00
levm_fibonacci 23.918 ± 0.123 23.791 24.160 3.30 ± 0.02

anyhow = "1.0.86"
directories = "5.0.1"
tinyvec = "1.6.0"
revm = { version = "14.0.3", default-features = false }
Copy link
Collaborator

Choose a reason for hiding this comment

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

wait, why do we want this here at the top level? I think only one crate should have this as a dependency (vm), if there is more than one crate using this it means there's a problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L1 L2 levm Lambda EVM implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants