From 84026f9aeeec6623d0ab6b4884c9c38a6175d2c4 Mon Sep 17 00:00:00 2001 From: Omar Abdulla Date: Mon, 18 Aug 2025 06:34:26 +0300 Subject: [PATCH] Cache the compiler versions --- .gitignore | 4 +- Cargo.lock | 1 + Cargo.toml | 1 + crates/compiler/Cargo.toml | 1 + crates/compiler/src/lib.rs | 2 +- crates/compiler/src/revive_resolc.rs | 57 ++++--- crates/compiler/src/solc.rs | 87 +++++----- crates/compiler/tests/lib.rs | 1 - crates/core/src/cached_compiler.rs | 5 +- crates/core/src/driver/mod.rs | 5 +- crates/core/src/main.rs | 244 +++++++++++++++------------ crates/format/src/case.rs | 9 +- crates/format/src/mode.rs | 4 +- crates/node/src/geth.rs | 108 ++++++++---- crates/node/src/kitchensink.rs | 4 + crates/node/src/lib.rs | 3 + 16 files changed, 322 insertions(+), 214 deletions(-) diff --git a/.gitignore b/.gitignore index cf26eca..505fb8b 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,6 @@ node_modules # We do not want to commit any log files that we produce from running the code locally so this is # added to the .gitignore file. -*.log \ No newline at end of file +*.log + +profile.json.gz \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 20b1631..57dca8f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4482,6 +4482,7 @@ dependencies = [ "alloy", "alloy-primitives", "anyhow", + "dashmap", "foundry-compilers-artifacts", "revive-common", "revive-dt-common", diff --git a/Cargo.toml b/Cargo.toml index 598234d..c378619 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ anyhow = "1.0" bson = { version = "2.15.0" } cacache = { version = "13.1.0" } clap = { version = "4", features = ["derive"] } +dashmap = { version = "6.1.0" } foundry-compilers-artifacts = { version = "0.18.0" } futures = { version = "0.3.31" } hex = "0.4.3" diff --git a/crates/compiler/Cargo.toml b/crates/compiler/Cargo.toml index 9e10a10..c1c839f 100644 --- a/crates/compiler/Cargo.toml +++ b/crates/compiler/Cargo.toml @@ -18,6 +18,7 @@ revive-common = { workspace = true } alloy = { workspace = true } alloy-primitives = { workspace = true } anyhow = { workspace = true } +dashmap = { workspace = true } foundry-compilers-artifacts = { workspace = true } semver = { workspace = true } serde = { workspace = true } diff --git a/crates/compiler/src/lib.rs b/crates/compiler/src/lib.rs index 799124e..05d9868 100644 --- a/crates/compiler/src/lib.rs +++ b/crates/compiler/src/lib.rs @@ -47,7 +47,7 @@ pub trait SolidityCompiler { version: impl Into, ) -> impl Future>; - fn version(&self) -> anyhow::Result; + fn version(&self) -> impl Future>; /// Does the compiler support the provided mode and version settings? fn supports_mode( diff --git a/crates/compiler/src/revive_resolc.rs b/crates/compiler/src/revive_resolc.rs index 1041265..57c84b9 100644 --- a/crates/compiler/src/revive_resolc.rs +++ b/crates/compiler/src/revive_resolc.rs @@ -2,11 +2,12 @@ //! compiling contracts to PolkaVM (PVM) bytecode. use std::{ - os::unix::process::CommandExt, path::PathBuf, process::{Command, Stdio}, + sync::LazyLock, }; +use dashmap::DashMap; use revive_dt_common::types::VersionOrRequirement; use revive_dt_config::Arguments; use revive_solc_json_interface::{ @@ -223,29 +224,39 @@ impl SolidityCompiler for Resolc { Ok(PathBuf::from("resolc")) } - fn version(&self) -> anyhow::Result { - // Logic for parsing the resolc version from the following string: - // Solidity frontend for the revive compiler version 0.3.0+commit.b238913.llvm-18.1.8 + async fn version(&self) -> anyhow::Result { + /// This is a cache of the path of the compiler to the version number of the compiler. We + /// choose to cache the version in this way rather than through a field on the struct since + /// compiler objects are being created all the time from the path and the compiler object is + /// not reused over time. + static VERSION_CACHE: LazyLock> = LazyLock::new(Default::default); - let output = unsafe { - Command::new(self.resolc_path.as_path()) - .arg("--version") - .stdout(Stdio::piped()) - .pre_exec(|| Ok(())) - .spawn()? - .wait_with_output()? - .stdout - }; - let output = String::from_utf8_lossy(&output); - let version_string = output - .split("version ") - .nth(1) - .context("Version parsing failed")? - .split("+") - .next() - .context("Version parsing failed")?; + match VERSION_CACHE.entry(self.resolc_path.clone()) { + dashmap::Entry::Occupied(occupied_entry) => Ok(occupied_entry.get().clone()), + dashmap::Entry::Vacant(vacant_entry) => { + let output = Command::new(self.resolc_path.as_path()) + .arg("--version") + .stdout(Stdio::piped()) + .spawn()? + .wait_with_output()? + .stdout; - Version::parse(version_string).map_err(Into::into) + let output = String::from_utf8_lossy(&output); + let version_string = output + .split("version ") + .nth(1) + .context("Version parsing failed")? + .split("+") + .next() + .context("Version parsing failed")?; + + let version = Version::parse(version_string)?; + + vacant_entry.insert(version.clone()); + + Ok(version) + } + } } fn supports_mode( @@ -275,7 +286,7 @@ mod test { let compiler = Resolc::new(path); // Act - let version = compiler.version(); + let version = compiler.version().await; // Assert let _ = version.expect("Failed to get version"); diff --git a/crates/compiler/src/solc.rs b/crates/compiler/src/solc.rs index d7eefe9..a7d8501 100644 --- a/crates/compiler/src/solc.rs +++ b/crates/compiler/src/solc.rs @@ -2,11 +2,12 @@ //! compiling contracts to EVM bytecode. use std::{ - os::unix::process::CommandExt, path::PathBuf, process::{Command, Stdio}, + sync::LazyLock, }; +use dashmap::DashMap; use revive_dt_common::types::VersionOrRequirement; use revive_dt_config::Arguments; use revive_dt_solc_binaries::download_solc; @@ -48,7 +49,7 @@ impl SolidityCompiler for Solc { }: CompilerInput, _: Self::Options, ) -> anyhow::Result { - let compiler_supports_via_ir = self.version()? >= SOLC_VERSION_SUPPORTING_VIA_YUL_IR; + let compiler_supports_via_ir = self.version().await? >= SOLC_VERSION_SUPPORTING_VIA_YUL_IR; // Be careful to entirely omit the viaIR field if the compiler does not support it, // as it will error if you provide fields it does not know about. Because @@ -115,14 +116,11 @@ impl SolidityCompiler for Solc { }; let mut command = AsyncCommand::new(&self.solc_path); - unsafe { - command - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .arg("--standard-json") - .pre_exec(|| Ok(())) - }; + command + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .arg("--standard-json"); if let Some(ref base_path) = base_path { command.arg("--base-path").arg(base_path); @@ -213,33 +211,44 @@ impl SolidityCompiler for Solc { Ok(path) } - fn version(&self) -> anyhow::Result { - // The following is the parsing code for the version from the solc version strings which - // look like the following: - // ``` - // solc, the solidity compiler commandline interface - // Version: 0.8.30+commit.73712a01.Darwin.appleclang - // ``` + async fn version(&self) -> anyhow::Result { + /// This is a cache of the path of the compiler to the version number of the compiler. We + /// choose to cache the version in this way rather than through a field on the struct since + /// compiler objects are being created all the time from the path and the compiler object is + /// not reused over time. + static VERSION_CACHE: LazyLock> = LazyLock::new(Default::default); - let child = unsafe { - Command::new(self.solc_path.as_path()) - .arg("--version") - .stdout(Stdio::piped()) - .pre_exec(|| Ok(())) - .spawn() - }?; - let output = child.wait_with_output()?; - let output = String::from_utf8_lossy(&output.stdout); - let version_line = output - .split("Version: ") - .nth(1) - .context("Version parsing failed")?; - let version_string = version_line - .split("+") - .next() - .context("Version parsing failed")?; + match VERSION_CACHE.entry(self.solc_path.clone()) { + dashmap::Entry::Occupied(occupied_entry) => Ok(occupied_entry.get().clone()), + dashmap::Entry::Vacant(vacant_entry) => { + // The following is the parsing code for the version from the solc version strings + // which look like the following: + // ``` + // solc, the solidity compiler commandline interface + // Version: 0.8.30+commit.73712a01.Darwin.appleclang + // ``` + let child = Command::new(self.solc_path.as_path()) + .arg("--version") + .stdout(Stdio::piped()) + .spawn()?; + let output = child.wait_with_output()?; + let output = String::from_utf8_lossy(&output.stdout); + let version_line = output + .split("Version: ") + .nth(1) + .context("Version parsing failed")?; + let version_string = version_line + .split("+") + .next() + .context("Version parsing failed")?; - Version::parse(version_string).map_err(Into::into) + let version = Version::parse(version_string)?; + + vacant_entry.insert(version.clone()); + + Ok(version) + } + } } fn supports_mode( @@ -263,15 +272,13 @@ mod test { async fn compiler_version_can_be_obtained() { // Arrange let args = Arguments::default(); - println!("Getting compiler path"); let path = Solc::get_compiler_executable(&args, Version::new(0, 7, 6)) .await .unwrap(); - println!("Got compiler path"); let compiler = Solc::new(path); // Act - let version = compiler.version(); + let version = compiler.version().await; // Assert assert_eq!( @@ -284,15 +291,13 @@ mod test { async fn compiler_version_can_be_obtained1() { // Arrange let args = Arguments::default(); - println!("Getting compiler path"); let path = Solc::get_compiler_executable(&args, Version::new(0, 4, 21)) .await .unwrap(); - println!("Got compiler path"); let compiler = Solc::new(path); // Act - let version = compiler.version(); + let version = compiler.version().await; // Assert assert_eq!( diff --git a/crates/compiler/tests/lib.rs b/crates/compiler/tests/lib.rs index 733e2d3..80858f2 100644 --- a/crates/compiler/tests/lib.rs +++ b/crates/compiler/tests/lib.rs @@ -11,7 +11,6 @@ async fn contracts_can_be_compiled_with_solc() { let compiler_path = Solc::get_compiler_executable(&args, Version::new(0, 8, 30)) .await .unwrap(); - println!("About to assert"); // Act let output = Compiler::::new() diff --git a/crates/core/src/cached_compiler.rs b/crates/core/src/cached_compiler.rs index ee05428..b94b1f3 100644 --- a/crates/core/src/cached_compiler.rs +++ b/crates/core/src/cached_compiler.rs @@ -62,8 +62,9 @@ impl CachedCompiler { compiler_version_or_requirement, ) .await?; - let compiler_version = - ::new(compiler_path.clone()).version()?; + let compiler_version = ::new(compiler_path.clone()) + .version() + .await?; let cache_key = CacheKey { platform_key: P::config_id().to_string(), diff --git a/crates/core/src/driver/mod.rs b/crates/core/src/driver/mod.rs index 35ef782..22708fd 100644 --- a/crates/core/src/driver/mod.rs +++ b/crates/core/src/driver/mod.rs @@ -259,7 +259,10 @@ where tracer_config: GethDebugTracerConfig(serde_json::json! {{ "onlyTopCall": true, "withLog": false, - "withReturnData": false + "withStorage": false, + "withMemory": false, + "withStack": false, + "withReturnData": true }}), ..Default::default() }, diff --git a/crates/core/src/main.rs b/crates/core/src/main.rs index 2c7b5a5..c069ffe 100644 --- a/crates/core/src/main.rs +++ b/crates/core/src/main.rs @@ -1,8 +1,9 @@ mod cached_compiler; use std::{ - collections::HashMap, - path::{Path, PathBuf}, + collections::{BTreeMap, HashMap}, + io::{BufWriter, Write, stderr}, + path::Path, sync::{Arc, LazyLock}, time::Instant, }; @@ -13,8 +14,9 @@ use alloy::{ }; use anyhow::Context; use clap::Parser; -use futures::stream::futures_unordered::FuturesUnordered; +use futures::stream; use futures::{Stream, StreamExt}; +use indexmap::IndexMap; use revive_dt_node_interaction::EthereumNode; use temp_dir::TempDir; use tokio::sync::mpsc; @@ -33,6 +35,7 @@ use revive_dt_format::{ corpus::Corpus, input::{Input, Step}, metadata::{ContractPathAndIdent, Metadata, MetadataFile}, + mode::ParsedMode, }; use revive_dt_node::pool::NodePool; use revive_dt_report::reporter::{Report, Span}; @@ -42,13 +45,12 @@ use crate::cached_compiler::CachedCompiler; static TEMP_DIR: LazyLock = LazyLock::new(|| TempDir::new().unwrap()); /// this represents a single "test"; a mode, path and collection of cases. -#[derive(Clone)] -struct Test { - metadata: Metadata, - path: PathBuf, +struct Test<'a> { + metadata: &'a Metadata, + metadata_file_path: &'a Path, mode: Mode, case_idx: CaseIdx, - case: Case, + case: &'a Case, } /// This represents the results that we gather from running test cases. @@ -133,7 +135,7 @@ where L::Blockchain: revive_dt_node::Node + Send + Sync + 'static, F::Blockchain: revive_dt_node::Node + Send + Sync + 'static, { - let (report_tx, report_rx) = mpsc::unbounded_channel::<(Test, CaseResult)>(); + let (report_tx, report_rx) = mpsc::unbounded_channel::<(Test<'_>, CaseResult)>(); let tests = prepare_tests::(args, metadata_files); let driver_task = start_driver_task::(args, tests, span, report_tx).await?; @@ -144,71 +146,94 @@ where Ok(()) } -fn prepare_tests( +fn prepare_tests<'a, L, F>( args: &Arguments, - metadata_files: &[MetadataFile], -) -> impl Stream + metadata_files: &'a [MetadataFile], +) -> impl Stream> where L: Platform, F: Platform, L::Blockchain: revive_dt_node::Node + Send + Sync + 'static, F::Blockchain: revive_dt_node::Node + Send + Sync + 'static, { - metadata_files + let flattened_tests = metadata_files .iter() - .flat_map( - |MetadataFile { - path, - content: metadata, - }| { - metadata - .cases - .iter() - .enumerate() - .flat_map(move |(case_idx, case)| { - metadata - .solc_modes() - .into_iter() - .map(move |solc_mode| (path, metadata, case_idx, case, solc_mode)) - }) - }, - ) - .filter( - |(metadata_file_path, metadata, _, _, _)| match metadata.ignore { - Some(true) => { - tracing::warn!( - metadata_file_path = %metadata_file_path.display(), - "Ignoring metadata file" - ); - false + .flat_map(|metadata_file| { + metadata_file + .cases + .iter() + .enumerate() + .map(move |(case_idx, case)| (metadata_file, case_idx, case)) + }) + .flat_map(|(metadata_file, case_idx, case)| { + let modes = match (metadata_file.modes.as_ref(), case.modes.as_ref()) { + (Some(_), Some(modes)) | (None, Some(modes)) | (Some(modes), None) => { + ParsedMode::many_to_modes(modes.iter()).collect::>() } - Some(false) | None => true, + (None, None) => Mode::all().collect(), + }; + modes + .into_iter() + .map(move |mode| (metadata_file, case_idx, case, mode)) + }) + .fold( + IndexMap::<_, BTreeMap<_, Vec<_>>>::new(), + |mut map, (metadata_file, case_idx, case, mode)| { + let test = Test { + metadata: &metadata_file.content, + metadata_file_path: metadata_file.path.as_path(), + mode: mode.clone(), + case_idx: CaseIdx::new(case_idx), + case, + }; + map.entry(mode) + .or_default() + .entry(test.case_idx) + .or_default() + .push(test); + map }, - ) - .filter( - |(metadata_file_path, _, case_idx, case, _)| match case.ignore { - Some(true) => { - tracing::warn!( - metadata_file_path = %metadata_file_path.display(), - case_idx, - case_name = ?case.name, - "Ignoring case" - ); - false - } - Some(false) | None => true, - }, - ) - .filter(|(metadata_file_path, metadata, ..)| match metadata.required_evm_version { - Some(evm_version_requirement) => { + ); + + let filtered_tests = flattened_tests + .into_values() + .flatten() + .flat_map(|(_, value)| value.into_iter()) + // Filter the test out if the metadata file is ignored. + .filter(|test| { + if test.metadata.ignore.is_some_and(|ignore| ignore) { + tracing::warn!( + metadata_file_path = %test.metadata_file_path.display(), + "Ignoring test case since the metadata file is ignored" + ); + false + } else { + true + } + }) + // Filter the test case if the case is ignored. + .filter(|test| { + if test.case.ignore.is_some_and(|ignore| ignore) { + tracing::warn!( + metadata_file_path = %test.metadata_file_path.display(), + "Ignoring test case since the case file is ignored" + ); + false + } else { + true + } + }) + // Filtering based on the EVM version compatibility + .filter(|test| { + if let Some(evm_version_requirement) = test.metadata.required_evm_version { let is_allowed = evm_version_requirement - .matches(&::evm_version()) - && evm_version_requirement - .matches(&::evm_version()); + .matches(&::evm_version()) + && evm_version_requirement + .matches(&::evm_version()); if !is_allowed { tracing::warn!( - metadata_file_path = %metadata_file_path.display(), + metadata_file_path = %test.metadata_file_path.display(), leader_evm_version = %::evm_version(), follower_evm_version = %::evm_version(), version_requirement = %evm_version_requirement, @@ -217,37 +242,37 @@ where } is_allowed - } - None => true, - }) - .map(|(metadata_file_path, metadata, case_idx, case, solc_mode)| { - Test { - metadata: metadata.clone(), - path: metadata_file_path.to_path_buf(), - mode: solc_mode, - case_idx: case_idx.into(), - case: case.clone(), - } - }) - .map(async |test| test) - .collect::>() - .filter_map(async move |test| { - // Check that both compilers support this test, else we skip it - let is_supported = does_compiler_support_mode::(args, &test.mode).await.ok().unwrap_or(false) && - does_compiler_support_mode::(args, &test.mode).await.ok().unwrap_or(false); - - // We filter_map to avoid needing to clone `test`, but return it as-is. - if is_supported { - Some(test) } else { - tracing::warn!( - metadata_file_path = %test.path.display(), - case_idx = %test.case_idx, - case_name = ?test.case.name, - mode = %test.mode, - "Skipping test as one or both of the compilers don't support it" - ); - None + true + } + }); + + stream::iter(filtered_tests) + // Filter based on the compiler compatibility + .filter_map(|test| { + let args = args.clone(); + + async move { + let is_supported = does_compiler_support_mode::(&args, &test.mode) + .await + .ok() + .unwrap_or(false) + && does_compiler_support_mode::(&args, &test.mode) + .await + .ok() + .unwrap_or(false); + + if !is_supported { + tracing::warn!( + metadata_file_path = %test.metadata_file_path.display(), + case_idx = %test.case_idx, + case_name = ?test.case.name, + mode = %test.mode, + "Skipping test as one or both of the compilers don't support it" + ); + } + + is_supported.then_some(test) } }) } @@ -259,7 +284,7 @@ async fn does_compiler_support_mode( let compiler_version_or_requirement = mode.compiler_version_to_use(args.solc.clone()); let compiler_path = P::Compiler::get_compiler_executable(args, compiler_version_or_requirement).await?; - let compiler_version = P::Compiler::new(compiler_path.clone()).version()?; + let compiler_version = P::Compiler::new(compiler_path.clone()).version().await?; Ok(P::Compiler::supports_mode( &compiler_version, @@ -268,11 +293,11 @@ async fn does_compiler_support_mode( )) } -async fn start_driver_task( +async fn start_driver_task<'a, L, F>( args: &Arguments, - tests: impl Stream, + tests: impl Stream>, span: Span, - report_tx: mpsc::UnboundedSender<(Test, CaseResult)>, + report_tx: mpsc::UnboundedSender<(Test<'a>, CaseResult)>, ) -> anyhow::Result> where L: Platform, @@ -313,16 +338,16 @@ where let tracing_span = tracing::span!( Level::INFO, "Running driver", - metadata_file_path = %test.path.display(), + metadata_file_path = %test.metadata_file_path.display(), case_idx = ?test.case_idx, solc_mode = ?test.mode, ); let result = handle_case_driver::( - &test.path, - &test.metadata, + test.metadata_file_path, + test.metadata, test.case_idx, - &test.case, + test.case, test.mode.clone(), args, cached_compiler, @@ -341,7 +366,7 @@ where )) } -async fn start_reporter_task(mut report_rx: mpsc::UnboundedReceiver<(Test, CaseResult)>) { +async fn start_reporter_task(mut report_rx: mpsc::UnboundedReceiver<(Test<'_>, CaseResult)>) { let start = Instant::now(); const GREEN: &str = "\x1B[32m"; @@ -355,22 +380,25 @@ async fn start_reporter_task(mut report_rx: mpsc::UnboundedReceiver<(Test, CaseR let mut failures = vec![]; // Wait for reports to come from our test runner. When the channel closes, this ends. + let mut buf = BufWriter::new(stderr()); while let Some((test, case_result)) = report_rx.recv().await { let case_name = test.case.name.as_deref().unwrap_or("unnamed_case"); let case_idx = test.case_idx; - let test_path = test.path.display(); + let test_path = test.metadata_file_path.display(); let test_mode = test.mode.clone(); match case_result { Ok(_inputs) => { number_of_successes += 1; - eprintln!( + let _ = writeln!( + buf, "{GREEN}Case Succeeded:{COLOUR_RESET} {test_path} -> {case_name}:{case_idx} (mode: {test_mode})" ); } Err(err) => { number_of_failures += 1; - eprintln!( + let _ = writeln!( + buf, "{RED}Case Failed:{COLOUR_RESET} {test_path} -> {case_name}:{case_idx} (mode: {test_mode})" ); failures.push((test, err)); @@ -378,29 +406,31 @@ async fn start_reporter_task(mut report_rx: mpsc::UnboundedReceiver<(Test, CaseR } } - eprintln!(); + let _ = writeln!(buf,); let elapsed = start.elapsed(); // Now, log the failures with more complete errors at the bottom, like `cargo test` does, so // that we don't have to scroll through the entire output to find them. if !failures.is_empty() { - eprintln!("{BOLD}Failures:{BOLD_RESET}\n"); + let _ = writeln!(buf, "{BOLD}Failures:{BOLD_RESET}\n"); for failure in failures { let (test, err) = failure; let case_name = test.case.name.as_deref().unwrap_or("unnamed_case"); let case_idx = test.case_idx; - let test_path = test.path.display(); + let test_path = test.metadata_file_path.display(); let test_mode = test.mode.clone(); - eprintln!( + let _ = writeln!( + buf, "---- {RED}Case Failed:{COLOUR_RESET} {test_path} -> {case_name}:{case_idx} (mode: {test_mode}) ----\n\n{err}\n" ); } } // Summary at the end. - eprintln!( + let _ = writeln!( + buf, "{} cases: {GREEN}{number_of_successes}{COLOUR_RESET} cases succeeded, {RED}{number_of_failures}{COLOUR_RESET} cases failed in {} seconds", number_of_successes + number_of_failures, elapsed.as_secs() diff --git a/crates/format/src/case.rs b/crates/format/src/case.rs index b1bd234..1968ff4 100644 --- a/crates/format/src/case.rs +++ b/crates/format/src/case.rs @@ -1,6 +1,6 @@ use serde::{Deserialize, Serialize}; -use revive_dt_common::macros::define_wrapper_type; +use revive_dt_common::{macros::define_wrapper_type, types::Mode}; use crate::{ input::{Expected, Step}, @@ -60,6 +60,13 @@ impl Case { } }) } + + pub fn solc_modes(&self) -> Vec { + match &self.modes { + Some(modes) => ParsedMode::many_to_modes(modes.iter()).collect(), + None => Mode::all().collect(), + } + } } define_wrapper_type!( diff --git a/crates/format/src/mode.rs b/crates/format/src/mode.rs index 7e6dfc8..0476e4e 100644 --- a/crates/format/src/mode.rs +++ b/crates/format/src/mode.rs @@ -223,7 +223,7 @@ mod tests { for (actual, expected) in strings { let parsed = ParsedMode::from_str(actual) - .expect(format!("Failed to parse mode string '{actual}'").as_str()); + .unwrap_or_else(|_| panic!("Failed to parse mode string '{actual}'")); assert_eq!( expected, parsed.to_string(), @@ -249,7 +249,7 @@ mod tests { for (actual, expected) in strings { let parsed = ParsedMode::from_str(actual) - .expect(format!("Failed to parse mode string '{actual}'").as_str()); + .unwrap_or_else(|_| panic!("Failed to parse mode string '{actual}'")); let expected_set: HashSet<_> = expected.into_iter().map(|s| s.to_owned()).collect(); let actual_set: HashSet<_> = parsed.to_modes().map(|m| m.to_string()).collect(); diff --git a/crates/node/src/geth.rs b/crates/node/src/geth.rs index c034ba5..32d9513 100644 --- a/crates/node/src/geth.rs +++ b/crates/node/src/geth.rs @@ -21,9 +21,12 @@ use alloy::{ Address, BlockHash, BlockNumber, BlockTimestamp, FixedBytes, StorageKey, TxHash, U256, }, providers::{ - Provider, ProviderBuilder, + Identity, Provider, ProviderBuilder, RootProvider, ext::DebugApi, - fillers::{CachedNonceManager, ChainIdFiller, FillProvider, NonceFiller, TxFiller}, + fillers::{ + CachedNonceManager, ChainIdFiller, FillProvider, JoinFill, NonceFiller, TxFiller, + WalletFiller, + }, }, rpc::types::{ EIP1186AccountProofResponse, TransactionReceipt, TransactionRequest, @@ -33,6 +36,7 @@ use alloy::{ }; use anyhow::Context; use revive_common::EVMVersion; +use tokio::sync::RwLock; use tracing::{Instrument, Level}; use revive_dt_common::{fs::clear_directory, futures::poll}; @@ -52,6 +56,7 @@ static NODE_COUNT: AtomicU32 = AtomicU32::new(0); /// /// Prunes the child process and the base directory on drop. #[derive(Debug)] +#[allow(clippy::type_complexity)] pub struct GethNode { connection_string: String, base_directory: PathBuf, @@ -62,7 +67,24 @@ pub struct GethNode { handle: Option, start_timeout: u64, wallet: EthereumWallet, - nonce_manager: CachedNonceManager, + provider: Arc< + RwLock< + Option< + Arc< + FillProvider< + JoinFill< + JoinFill< + JoinFill, ChainIdFiller>, + NonceFiller, + >, + WalletFiller, + >, + RootProvider, + >, + >, + >, + >, + >, /// This vector stores [`File`] objects that we use for logging which we want to flush when the /// node object is dropped. We do not store them in a structured fashion at the moment (in /// separate fields) as the logic that we need to apply to them is all the same regardless of @@ -241,37 +263,36 @@ impl GethNode { self.logs_directory.join(Self::GETH_STDERR_LOG_FILE_NAME) } - fn provider( + async fn provider( &self, - ) -> impl Future< - Output = anyhow::Result< - FillProvider, impl Provider, Ethereum>, - >, - > + 'static { - let connection_string = self.connection_string(); - let wallet = self.wallet.clone(); + ) -> anyhow::Result, impl Provider, Ethereum>>> + { + let read_guard = self.provider.read().await; - // Note: We would like all providers to make use of the same nonce manager so that we have - // monotonically increasing nonces that are cached. The cached nonce manager uses Arc's in - // its implementation and therefore it means that when we clone it then it still references - // the same state. - let nonce_manager = self.nonce_manager.clone(); + match read_guard.as_ref() { + Some(provider) => Ok(provider.clone()), + None => { + drop(read_guard); + let mut write_guard = self.provider.write().await; - Box::pin(async move { - ProviderBuilder::new() - .disable_recommended_fillers() - .filler(FallbackGasFiller::new( - 25_000_000, - 1_000_000_000, - 1_000_000_000, - )) - .filler(ChainIdFiller::default()) - .filler(NonceFiller::new(nonce_manager)) - .wallet(wallet) - .connect(&connection_string) - .await - .map_err(Into::into) - }) + let provider = ProviderBuilder::new() + .disable_recommended_fillers() + .filler(FallbackGasFiller::new( + 25_000_000, + 1_000_000_000, + 1_000_000_000, + )) + .filler(ChainIdFiller::default()) + .filler(NonceFiller::new(CachedNonceManager::default())) + .wallet(self.wallet.clone()) + .connect(&self.connection_string) + .await + .map(Arc::new)?; + + *write_guard = Some(provider.clone()); + Ok(provider) + } + } } } @@ -281,8 +302,23 @@ impl EthereumNode for GethNode { &self, transaction: TransactionRequest, ) -> anyhow::Result { - let provider = Arc::new(self.provider().await?); - let transaction_hash = *provider.send_transaction(transaction).await?.tx_hash(); + let provider = self.provider().await?; + + let transaction = provider.fill(transaction).await?; + let transaction = transaction + .as_envelope() + .context("Filled transaction is not an envelope")?; + let transaction_hash = *transaction.tx_hash(); + + let rtn = provider.send_tx_envelope(transaction.clone()).await; + match rtn { + Ok(_) => {} + Err(error) => { + if !error.to_string().contains("already known") { + return Err(error.into()); + } + } + } // The following is a fix for the "transaction indexing is in progress" error that we // used to get. You can find more information on this in the following GH issue in geth @@ -523,13 +559,17 @@ impl Node for GethNode { handle: None, start_timeout: config.geth_start_timeout, wallet, + provider: Default::default(), // We know that we only need to be storing 2 files so we can specify that when creating // the vector. It's the stdout and stderr of the geth node. logs_file_to_flush: Vec::with_capacity(2), - nonce_manager: Default::default(), } } + fn id(&self) -> usize { + self.id as _ + } + #[tracing::instrument(level = "info", skip_all, fields(geth_node_id = self.id))] fn connection_string(&self) -> String { self.connection_string.clone() diff --git a/crates/node/src/kitchensink.rs b/crates/node/src/kitchensink.rs index f5a9e0a..a3f3149 100644 --- a/crates/node/src/kitchensink.rs +++ b/crates/node/src/kitchensink.rs @@ -582,6 +582,10 @@ impl Node for KitchensinkNode { } } + fn id(&self) -> usize { + self.id as _ + } + #[tracing::instrument(skip_all, fields(kitchensink_node_id = self.id))] fn connection_string(&self) -> String { self.rpc_url.clone() diff --git a/crates/node/src/lib.rs b/crates/node/src/lib.rs index 446b66a..d40fc18 100644 --- a/crates/node/src/lib.rs +++ b/crates/node/src/lib.rs @@ -18,6 +18,9 @@ pub trait Node: EthereumNode { /// Create a new uninitialized instance. fn new(config: &Arguments) -> Self; + /// Returns the identifier of the node. + fn id(&self) -> usize; + /// Spawns a node configured according to the genesis json. /// /// Blocking until it's ready to accept transactions.