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

Building reth-primitives fails when consuming reth as a library target #8713

Closed
1 task done
jmcph4 opened this issue Jun 10, 2024 · 5 comments
Closed
1 task done

Building reth-primitives fails when consuming reth as a library target #8713

jmcph4 opened this issue Jun 10, 2024 · 5 comments
Labels
C-bug An unexpected or incorrect behavior S-needs-triage This issue needs to be labelled

Comments

@jmcph4
Copy link

jmcph4 commented Jun 10, 2024

Describe the bug

In bluealloy/revm#1143, the signature for validate_initial_tx_gas was changed to accept an additional parameter for the EOF initcodes. As of Reth 0.2.0-beta.9, when calculating intrinsic gas usage of a transaction, the pre-EOF interface is used (i.e., only four parameters are provided to validate_initial_tx_gas).

/// Calculates the Intrinsic Gas usage for a Transaction
///
/// Caution: This only checks past the Merge hardfork.
#[inline]
pub fn calculate_intrinsic_gas_after_merge(
input: &[u8],
kind: &TxKind,
access_list: &[(Address, Vec<U256>)],
is_shanghai: bool,
) -> u64 {
let spec_id = if is_shanghai { SpecId::SHANGHAI } else { SpecId::MERGE };
validate_initial_tx_gas(spec_id, input, kind.is_create(), access_list)
}

To achieve this, Reth patches the various revm-* crates:

reth/Cargo.toml

Lines 453 to 457 in 7b435e0

[patch.crates-io]
revm = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-interpreter = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-precompile = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-primitives = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }

When building reth-primitives directly, via cargo build -p reth-primitives, this works; however, breaks when relying on Reth as an upstream dependency as Git-based dependencies cannot be transitively patched.

This is fixed by not hiding Reth's dependency on a specific commit by patching:

diff --git a/Cargo.toml b/Cargo.toml
index b281d4e7d..4f159de81 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -325,8 +325,8 @@ reth-trie = { path = "crates/trie/trie" }
 reth-trie-parallel = { path = "crates/trie/parallel" }
 
 # revm
-revm = { version = "9.0.0", features = [ "std", "secp256k1", "blst", ], default-features = false }
-revm-primitives = { version = "4.0.0", features = [ "std", ], default-features = false }
+revm = { git = "https://github.com/bluealloy/revm", rev = "a28a543", features = [ "std", "secp256k1", "blst" ], default-features = false }
+revm-primitives = { git = "https://github.com/bluealloy/revm", rev = "a28a543", features = [ "std", ], default-features = false }
 revm-inspectors = { git = "https://github.com/paradigmxyz/evm-inspectors", rev = "53aa2b2" }
 
 # eth
@@ -455,9 +455,3 @@ proptest-derive = "0.4"
 serial_test = "3"
 similar-asserts = "1.5.0"
 test-fuzz = "5"
-
-[patch.crates-io]
-revm = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
-revm-interpreter = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
-revm-precompile = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
-revm-primitives = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }

Steps to reproduce

$ rustup override set nightly
$ cat src/main.rs
#![feature(async_closure)]
use std::sync::Arc;

use clap::Parser;
use reth::{
    builder::{InitState, NodeHandle, WithLaunchContext},
    cli::Cli,
    commands::node::NoArgs,
};
use reth_db::DatabaseEnv;
use reth_node_ethereum::EthereumNode;

fn main() -> eyre::Result<()> {
    Cli::<NoArgs>::parse().run(
        async move |builder: WithLaunchContext<Arc<DatabaseEnv>, InitState>, _args| {
            let NodeHandle {
                node,
                node_exit_future,
            } = builder.node(EthereumNode::default()).launch().await?;
            node.task_executor.spawn(Box::pin(async move {
                println!("Hello, world!");
            }));

            node_exit_future.await
        },
    )?;

    Ok(())
}
$ cat Cargo.toml
[package]
name = "foobar"
version = "0.1.0"
edition = "2021"

[dependencies]
clap = { version = "4.5", features = ["derive"] }
eyre = "0.6.12"
pretty_env_logger = "0.5.0"
reth = { git = "https://github.com/paradigmxyz/reth", rev = "7b435e0" }
reth-node-ethereum = { git = "https://github.com/paradigmxyz/reth", rev = "7b435e0" }
reth-db = { git = "https://github.com/paradigmxyz/reth", rev = "7b435e0" }
$ cargo run
error[E0061]: this function takes 5 arguments but 4 arguments were supplied
   --> /home/jmcph4/.cargo/git/checkouts/reth-36d3ea1d1152b20c/7b435e0/crates/primitives/src/revm/compat.rs:39:5
    |
39  |     validate_initial_tx_gas(spec_id, input, kind.is_create(), access_list)
    |     ^^^^^^^^^^^^^^^^^^^^^^^----------------------------------------------- an argument of type `&[alloy_primitives::Bytes]` is missing
    |
note: function defined here
   --> /home/jmcph4/.cargo/registry/src/index.crates.io-6f17d22bba15001f/revm-interpreter-5.0.0/src/gas/calc.rs:356:8
    |
356 | pub fn validate_initial_tx_gas(
    |        ^^^^^^^^^^^^^^^^^^^^^^^
help: provide the argument
    |
39  |     validate_initial_tx_gas(spec_id, input, kind.is_create(), access_list, /* &[alloy_primitives::Bytes] */)
    |                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

   Compiling hyper-util v0.1.5
error[E0063]: missing fields `eof_initcodes` and `eof_initcodes_hashed` in initializer of `TxEnv`
   --> /home/jmcph4/.cargo/git/checkouts/reth-36d3ea1d1152b20c/7b435e0/crates/primitives/src/revm/env.rs:180:14
    |
180 |     env.tx = TxEnv {
    |              ^^^^^ missing `eof_initcodes` and `eof_initcodes_hashed`

$ echo $?
101

Node logs

N/A

Platform(s)

Linux (x86)

What version/commit are you on?

0.2.0-beta.9 (7b435e0)

What database version are you on?

N/A

Which chain / network are you on?

N/A

What type of node are you running?

Archive (default)

What prune config do you use, if any?

N/A

If you've built Reth from source, provide the full command you used

cargo run

Code of Conduct

  • I agree to follow the Code of Conduct
@jmcph4 jmcph4 added C-bug An unexpected or incorrect behavior S-needs-triage This issue needs to be labelled labels Jun 10, 2024
@jmcph4
Copy link
Author

jmcph4 commented Jun 10, 2024

Propagating the patch locally (by copying it into the Cargo.toml of the downstream project) also seems to work.

@mattsse
Copy link
Collaborator

mattsse commented Jun 10, 2024

yeah, we currently use a revm patch, see

reth/Cargo.toml

Lines 459 to 463 in 40fc19f

[patch.crates-io]
revm = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-interpreter = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-precompile = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-primitives = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }

we'll get rid of this with a new revm release

@jmcph4
Copy link
Author

jmcph4 commented Jun 17, 2024

yeah, we currently use a revm patch, see

reth/Cargo.toml

Lines 459 to 463 in 40fc19f

[patch.crates-io]
revm = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-interpreter = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-precompile = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-primitives = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }

we'll get rid of this with a new revm release

This should be fixed prior to 1.0. Given that we're already at a release candidate and that no such revm release has occurred yet, I don't really think this should have been closed -- especially given that use as a library is an explicit design goal of Reth.

@undenuicap
Copy link

an empty project only with this fails :-(

[dependencies]
reth = { git = "https://github.com/paradigmxyz/reth", tag = "v1.0.0-rc.2" }

[patch.crates-io]
revm = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-interpreter = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-precompile = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }
revm-primitives = { git = "https://github.com/bluealloy/revm", rev = "a28a543" }

@shekhirin
Copy link
Collaborator

@undenuicap we will remove the patches soon, in the meantime please use this revision (from Cargo.toml)

[patch.crates-io]
revm = { git = "https://github.com/bluealloy/revm.git", rev = "41e2f7f" }
revm-interpreter = { git = "https://github.com/bluealloy/revm.git", rev = "41e2f7f" }
revm-precompile = { git = "https://github.com/bluealloy/revm.git", rev = "41e2f7f" }
revm-primitives = { git = "https://github.com/bluealloy/revm.git", rev = "41e2f7f" }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug An unexpected or incorrect behavior S-needs-triage This issue needs to be labelled
Projects
Archived in project
Development

No branches or pull requests

4 participants