From 90b2dd4cfe33bdbfc9ecb557f71504ad62336079 Mon Sep 17 00:00:00 2001 From: Omar Date: Mon, 11 Aug 2025 00:57:41 +0300 Subject: [PATCH 1/6] Make metadata serializable (#132) --- crates/format/src/case.rs | 4 ++-- crates/format/src/input.rs | 14 +++++++------- crates/format/src/metadata.rs | 3 +-- crates/format/src/mode.rs | 19 ++++++++++++++++++- 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/crates/format/src/case.rs b/crates/format/src/case.rs index 4e1f958..59572f0 100644 --- a/crates/format/src/case.rs +++ b/crates/format/src/case.rs @@ -1,4 +1,4 @@ -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use revive_dt_common::macros::define_wrapper_type; @@ -7,7 +7,7 @@ use crate::{ mode::Mode, }; -#[derive(Debug, Default, Deserialize, Clone, Eq, PartialEq)] +#[derive(Debug, Default, Serialize, Deserialize, Clone, Eq, PartialEq)] pub struct Case { pub name: Option, pub comment: Option, diff --git a/crates/format/src/input.rs b/crates/format/src/input.rs index b1dd067..810ec31 100644 --- a/crates/format/src/input.rs +++ b/crates/format/src/input.rs @@ -17,7 +17,7 @@ use revive_dt_common::macros::define_wrapper_type; use crate::traits::ResolverApi; use crate::{metadata::ContractInstance, traits::ResolutionContext}; -#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq)] +#[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] pub struct Input { #[serde(default = "Input::default_caller")] pub caller: Address, @@ -33,7 +33,7 @@ pub struct Input { pub variable_assignments: Option, } -#[derive(Clone, Debug, Deserialize, Eq, PartialEq)] +#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)] #[serde(untagged)] pub enum Expected { Calldata(Calldata), @@ -41,7 +41,7 @@ pub enum Expected { ExpectedMany(Vec), } -#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq)] +#[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] pub struct ExpectedOutput { pub compiler_version: Option, pub return_data: Option, @@ -50,7 +50,7 @@ pub struct ExpectedOutput { pub exception: bool, } -#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq)] +#[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] pub struct Event { pub address: Option, pub topics: Vec, @@ -108,7 +108,7 @@ pub struct Event { /// [`Single`]: Calldata::Single /// [`Compound`]: Calldata::Compound /// [reverse polish notation]: https://en.wikipedia.org/wiki/Reverse_Polish_notation -#[derive(Clone, Debug, Deserialize, Eq, PartialEq)] +#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)] #[serde(untagged)] pub enum Calldata { Single(Bytes), @@ -142,7 +142,7 @@ enum Operation { } /// Specify how the contract is called. -#[derive(Debug, Default, Deserialize, Clone, Eq, PartialEq)] +#[derive(Debug, Default, Serialize, Deserialize, Clone, Eq, PartialEq)] pub enum Method { /// Initiate a deploy transaction, calling contracts constructor. /// @@ -167,7 +167,7 @@ define_wrapper_type!( pub struct EtherValue(U256); ); -#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq)] +#[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] pub struct VariableAssignments { /// A vector of the variable names to assign to the return data. /// diff --git a/crates/format/src/metadata.rs b/crates/format/src/metadata.rs index aad74fc..1255187 100644 --- a/crates/format/src/metadata.rs +++ b/crates/format/src/metadata.rs @@ -43,12 +43,11 @@ impl Deref for MetadataFile { } } -#[derive(Debug, Default, Deserialize, Clone, Eq, PartialEq)] +#[derive(Debug, Default, Serialize, Deserialize, Clone, Eq, PartialEq)] pub struct Metadata { pub targets: Option>, pub cases: Vec, pub contracts: Option>, - // TODO: Convert into wrapper types for clarity. pub libraries: Option>>, pub ignore: Option, pub modes: Option>, diff --git a/crates/format/src/mode.rs b/crates/format/src/mode.rs index 69b55b2..8b1f4c0 100644 --- a/crates/format/src/mode.rs +++ b/crates/format/src/mode.rs @@ -16,6 +16,7 @@ pub struct SolcMode { pub solc_version: Option, solc_optimize: Option, pub llvm_optimizer_settings: Vec, + mode_string: String, } impl SolcMode { @@ -29,7 +30,10 @@ impl SolcMode { /// - A solc `SemVer version requirement` string /// - One or more `-OX` where X is a supposed to be an LLVM opt mode pub fn parse_from_mode_string(mode_string: &str) -> Option { - let mut result = Self::default(); + let mut result = Self { + mode_string: mode_string.to_string(), + ..Default::default() + }; let mut parts = mode_string.trim().split(" "); @@ -104,3 +108,16 @@ impl<'de> Deserialize<'de> for Mode { Ok(Self::Unknown(mode_string)) } } + +impl Serialize for Mode { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + let string = match self { + Mode::Solidity(solc_mode) => &solc_mode.mode_string, + Mode::Unknown(string) => string, + }; + string.serialize(serializer) + } +} From f7fbe094ece9f3ef265a309c01c939943500ea02 Mon Sep 17 00:00:00 2001 From: Omar Date: Mon, 11 Aug 2025 15:11:16 +0300 Subject: [PATCH 2/6] Balance assertions (#133) * Make metadata serializable * Refactor tests to use steps * Add a balance assertion test step * Test balance deserialization * Box the test steps * Permit size difference in step output --- assets/test_metadata.json | 4 + .../files_with_extension_iterator.rs | 10 +- crates/core/src/driver/mod.rs | 160 +++++++++++++++--- crates/core/src/main.rs | 9 +- crates/format/src/case.rs | 24 ++- crates/format/src/input.rs | 27 +++ crates/node-interaction/src/lib.rs | 4 + crates/node/src/geth.rs | 9 + crates/node/src/kitchensink.rs | 9 + 9 files changed, 216 insertions(+), 40 deletions(-) diff --git a/assets/test_metadata.json b/assets/test_metadata.json index 6d54584..7fca971 100644 --- a/assets/test_metadata.json +++ b/assets/test_metadata.json @@ -8,6 +8,10 @@ { "name": "first", "inputs": [ + { + "address": "0xdeadbeef00000000000000000000000000000042", + "expected_balance": "1233" + }, { "instance": "WBTC_1", "method": "#deployer", diff --git a/crates/common/src/iterators/files_with_extension_iterator.rs b/crates/common/src/iterators/files_with_extension_iterator.rs index 1892c5f..ac81103 100644 --- a/crates/common/src/iterators/files_with_extension_iterator.rs +++ b/crates/common/src/iterators/files_with_extension_iterator.rs @@ -1,4 +1,8 @@ -use std::{borrow::Cow, collections::HashSet, path::PathBuf}; +use std::{ + borrow::Cow, + collections::HashSet, + path::{Path, PathBuf}, +}; /// An iterator that finds files of a certain extension in the provided directory. You can think of /// this a glob pattern similar to: `${path}/**/*.md` @@ -18,10 +22,10 @@ pub struct FilesWithExtensionIterator { } impl FilesWithExtensionIterator { - pub fn new(root_directory: PathBuf) -> Self { + pub fn new(root_directory: impl AsRef) -> Self { Self { allowed_extensions: Default::default(), - directories_to_search: vec![root_directory], + directories_to_search: vec![root_directory.as_ref().to_path_buf()], files_matching_allowed_extensions: Default::default(), } } diff --git a/crates/core/src/driver/mod.rs b/crates/core/src/driver/mod.rs index c070d38..32b13c5 100644 --- a/crates/core/src/driver/mod.rs +++ b/crates/core/src/driver/mod.rs @@ -26,9 +26,11 @@ use revive_dt_format::traits::{ResolutionContext, ResolverApi}; use semver::Version; use revive_dt_format::case::{Case, CaseIdx}; -use revive_dt_format::input::{Calldata, EtherValue, Expected, ExpectedOutput, Method}; +use revive_dt_format::input::{ + BalanceAssertion, Calldata, EtherValue, Expected, ExpectedOutput, Input, Method, +}; use revive_dt_format::metadata::{ContractInstance, ContractPathAndIdent}; -use revive_dt_format::{input::Input, metadata::Metadata}; +use revive_dt_format::{input::Step, metadata::Metadata}; use revive_dt_node::Node; use revive_dt_node_interaction::EthereumNode; use tracing::Instrument; @@ -70,6 +72,27 @@ where } } + pub async fn handle_step( + &mut self, + metadata: &Metadata, + case_idx: CaseIdx, + step: &Step, + node: &T::Blockchain, + ) -> anyhow::Result { + match step { + Step::FunctionCall(input) => { + let (receipt, geth_trace, diff_mode) = + self.handle_input(metadata, case_idx, input, node).await?; + Ok(StepOutput::FunctionCall(receipt, geth_trace, diff_mode)) + } + Step::BalanceAssertion(balance_assertion) => { + self.handle_balance_assertion(metadata, case_idx, balance_assertion, node) + .await?; + Ok(StepOutput::BalanceAssertion) + } + } + } + pub async fn handle_input( &mut self, metadata: &Metadata, @@ -78,7 +101,7 @@ where node: &T::Blockchain, ) -> anyhow::Result<(TransactionReceipt, GethTrace, DiffMode)> { let deployment_receipts = self - .handle_contract_deployment(metadata, case_idx, input, node) + .handle_input_contract_deployment(metadata, case_idx, input, node) .await?; let execution_receipt = self .handle_input_execution(input, deployment_receipts, node) @@ -93,8 +116,21 @@ where .await } + pub async fn handle_balance_assertion( + &mut self, + metadata: &Metadata, + _: CaseIdx, + balance_assertion: &BalanceAssertion, + node: &T::Blockchain, + ) -> anyhow::Result<()> { + self.handle_balance_assertion_contract_deployment(metadata, balance_assertion, node) + .await?; + + Ok(()) + } + /// Handles the contract deployment for a given input performing it if it needs to be performed. - async fn handle_contract_deployment( + async fn handle_input_contract_deployment( &mut self, metadata: &Metadata, case_idx: CaseIdx, @@ -462,6 +498,65 @@ where Ok((execution_receipt, trace, diff)) } + pub async fn handle_balance_assertion_contract_deployment( + &mut self, + metadata: &Metadata, + balance_assertion: &BalanceAssertion, + node: &T::Blockchain, + ) -> anyhow::Result<()> { + let Some(instance) = balance_assertion + .address + .strip_prefix(".address") + .map(ContractInstance::new) + else { + return Ok(()); + }; + self.get_or_deploy_contract_instance( + &instance, + metadata, + Input::default_caller(), + None, + None, + node, + ) + .await?; + Ok(()) + } + + pub async fn handle_balance_assertion_execution( + &mut self, + BalanceAssertion { + address: address_string, + expected_balance: amount, + }: &BalanceAssertion, + node: &T::Blockchain, + ) -> anyhow::Result<()> { + let address = Address::from_slice( + Calldata::new_compound([address_string]) + .calldata(node, self.default_resolution_context()) + .await? + .get(12..32) + .expect("Can't fail"), + ); + + let balance = node.balance_of(address).await?; + + let expected = *amount; + let actual = balance; + if expected != actual { + tracing::error!(%expected, %actual, %address, "Balance assertion failed"); + anyhow::bail!( + "Balance assertion failed - Expected {} but got {} for {} resolved to {}", + expected, + actual, + address_string, + address, + ) + } + + Ok(()) + } + /// Gets the information of a deployed contract or library from the state. If it's found to not /// be deployed then it will be deployed. /// @@ -651,38 +746,53 @@ where return Ok(0); } - let mut inputs_executed = 0; - for (input_idx, input) in self.case.inputs_iterator().enumerate() { - let tracing_span = tracing::info_span!("Handling input", input_idx); + let mut steps_executed = 0; + for (step_idx, step) in self.case.steps_iterator().enumerate() { + let tracing_span = tracing::info_span!("Handling input", step_idx); - let (leader_receipt, _, leader_diff) = self + let leader_step_output = self .leader_state - .handle_input(self.metadata, self.case_idx, &input, self.leader_node) + .handle_step(self.metadata, self.case_idx, &step, self.leader_node) .instrument(tracing_span.clone()) .await?; - let (follower_receipt, _, follower_diff) = self + let follower_step_output = self .follower_state - .handle_input(self.metadata, self.case_idx, &input, self.follower_node) + .handle_step(self.metadata, self.case_idx, &step, self.follower_node) .instrument(tracing_span) .await?; + match (leader_step_output, follower_step_output) { + ( + StepOutput::FunctionCall(leader_receipt, _, leader_diff), + StepOutput::FunctionCall(follower_receipt, _, follower_diff), + ) => { + if leader_diff == follower_diff { + tracing::debug!("State diffs match between leader and follower."); + } else { + tracing::debug!("State diffs mismatch between leader and follower."); + Self::trace_diff_mode("Leader", &leader_diff); + Self::trace_diff_mode("Follower", &follower_diff); + } - if leader_diff == follower_diff { - tracing::debug!("State diffs match between leader and follower."); - } else { - tracing::debug!("State diffs mismatch between leader and follower."); - Self::trace_diff_mode("Leader", &leader_diff); - Self::trace_diff_mode("Follower", &follower_diff); + if leader_receipt.logs() != follower_receipt.logs() { + tracing::debug!("Log/event mismatch between leader and follower."); + tracing::trace!("Leader logs: {:?}", leader_receipt.logs()); + tracing::trace!("Follower logs: {:?}", follower_receipt.logs()); + } + } + (StepOutput::BalanceAssertion, StepOutput::BalanceAssertion) => {} + _ => unreachable!("The two step outputs can not be of a different kind"), } - if leader_receipt.logs() != follower_receipt.logs() { - tracing::debug!("Log/event mismatch between leader and follower."); - tracing::trace!("Leader logs: {:?}", leader_receipt.logs()); - tracing::trace!("Follower logs: {:?}", follower_receipt.logs()); - } - - inputs_executed += 1; + steps_executed += 1; } - Ok(inputs_executed) + Ok(steps_executed) } } + +#[derive(Clone, Debug)] +#[allow(clippy::large_enum_variant)] +pub enum StepOutput { + FunctionCall(TransactionReceipt, GethTrace, DiffMode), + BalanceAssertion, +} diff --git a/crates/core/src/main.rs b/crates/core/src/main.rs index 4221b8d..70512a5 100644 --- a/crates/core/src/main.rs +++ b/crates/core/src/main.rs @@ -32,7 +32,7 @@ use revive_dt_core::{ use revive_dt_format::{ case::{Case, CaseIdx}, corpus::Corpus, - input::Input, + input::{Input, Step}, metadata::{ContractInstance, ContractPathAndIdent, Metadata, MetadataFile}, mode::SolcMode, }; @@ -446,9 +446,12 @@ where // doing the deployments from different accounts and therefore we're not slowed down by // the nonce. let deployer_address = case - .inputs + .steps .iter() - .map(|input| input.caller) + .filter_map(|step| match step { + Step::FunctionCall(input) => Some(input.caller), + Step::BalanceAssertion(..) => None, + }) .next() .unwrap_or(Input::default_caller()); let leader_tx = TransactionBuilder::::with_deploy_code( diff --git a/crates/format/src/case.rs b/crates/format/src/case.rs index 59572f0..26e803e 100644 --- a/crates/format/src/case.rs +++ b/crates/format/src/case.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; use revive_dt_common::macros::define_wrapper_type; use crate::{ - input::{Expected, Input}, + input::{Expected, Step}, mode::Mode, }; @@ -12,21 +12,27 @@ pub struct Case { pub name: Option, pub comment: Option, pub modes: Option>, - pub inputs: Vec, + #[serde(rename = "inputs")] + pub steps: Vec, pub group: Option, pub expected: Option, pub ignore: Option, } impl Case { - pub fn inputs_iterator(&self) -> impl Iterator { - let inputs_len = self.inputs.len(); - self.inputs + #[allow(irrefutable_let_patterns)] + pub fn steps_iterator(&self) -> impl Iterator { + let steps_len = self.steps.len(); + self.steps .clone() .into_iter() .enumerate() - .map(move |(idx, mut input)| { - if idx + 1 == inputs_len { + .map(move |(idx, mut step)| { + let Step::FunctionCall(ref mut input) = step else { + return step; + }; + + if idx + 1 == steps_len { if input.expected.is_none() { input.expected = self.expected.clone(); } @@ -36,9 +42,9 @@ impl Case { // the case? What are we supposed to do with that final expected field on the // case? - input + step } else { - input + step } }) } diff --git a/crates/format/src/input.rs b/crates/format/src/input.rs index 810ec31..cc96341 100644 --- a/crates/format/src/input.rs +++ b/crates/format/src/input.rs @@ -17,6 +17,19 @@ use revive_dt_common::macros::define_wrapper_type; use crate::traits::ResolverApi; use crate::{metadata::ContractInstance, traits::ResolutionContext}; +/// A test step. +/// +/// A test step can be anything. It could be an invocation to a function, an assertion, or any other +/// action that needs to be run or executed on the nodes used in the tests. +#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)] +#[serde(untagged)] +pub enum Step { + /// A function call or an invocation to some function on some smart contract. + FunctionCall(Box), + /// A step for performing a balance assertion on some account or contract. + BalanceAssertion(Box), +} + #[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] pub struct Input { #[serde(default = "Input::default_caller")] @@ -33,6 +46,20 @@ pub struct Input { pub variable_assignments: Option, } +#[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] +pub struct BalanceAssertion { + /// The address that the balance assertion should be done on. + /// + /// This is a string which will be resolved into an address when being processed. Therefore, + /// this could be a normal hex address, a variable such as `Test.address`, or perhaps even a + /// full on variable like `$VARIABLE:Uniswap`. It follows the same resolution rules that are + /// followed in the calldata. + pub address: String, + + /// The amount of balance to assert that the account or contract has. + pub expected_balance: U256, +} + #[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)] #[serde(untagged)] pub enum Expected { diff --git a/crates/node-interaction/src/lib.rs b/crates/node-interaction/src/lib.rs index 791ba4b..b052c7a 100644 --- a/crates/node-interaction/src/lib.rs +++ b/crates/node-interaction/src/lib.rs @@ -1,5 +1,6 @@ //! This crate implements all node interactions. +use alloy::primitives::{Address, U256}; use alloy::rpc::types::trace::geth::{DiffMode, GethDebugTracingOptions, GethTrace}; use alloy::rpc::types::{TransactionReceipt, TransactionRequest}; use anyhow::Result; @@ -21,4 +22,7 @@ pub trait EthereumNode { /// Returns the state diff of the transaction hash in the [TransactionReceipt]. fn state_diff(&self, receipt: &TransactionReceipt) -> impl Future>; + + /// Returns the balance of the provided [`Address`] back. + fn balance_of(&self, address: Address) -> impl Future>; } diff --git a/crates/node/src/geth.rs b/crates/node/src/geth.rs index 5ce7921..b3618ba 100644 --- a/crates/node/src/geth.rs +++ b/crates/node/src/geth.rs @@ -371,6 +371,15 @@ impl EthereumNode for GethNode { _ => anyhow::bail!("expected a diff mode trace"), } } + + #[tracing::instrument(skip_all, fields(geth_node_id = self.id))] + async fn balance_of(&self, address: Address) -> anyhow::Result { + self.provider() + .await? + .get_balance(address) + .await + .map_err(Into::into) + } } impl ResolverApi for GethNode { diff --git a/crates/node/src/kitchensink.rs b/crates/node/src/kitchensink.rs index 2300e05..805b71f 100644 --- a/crates/node/src/kitchensink.rs +++ b/crates/node/src/kitchensink.rs @@ -428,6 +428,15 @@ impl EthereumNode for KitchensinkNode { _ => anyhow::bail!("expected a diff mode trace"), } } + + #[tracing::instrument(skip_all, fields(kitchensink_node_id = self.id))] + async fn balance_of(&self, address: Address) -> anyhow::Result { + self.provider() + .await? + .get_balance(address) + .await + .map_err(Into::into) + } } impl ResolverApi for KitchensinkNode { From 67d767ffde29910f55761926d23072737e376a4b Mon Sep 17 00:00:00 2001 From: Omar Date: Mon, 11 Aug 2025 16:17:19 +0300 Subject: [PATCH 3/6] Implement storage empty assertion (#134) --- assets/test_metadata.json | 8 +++ crates/core/src/driver/mod.rs | 85 ++++++++++++++++++++++++++++++ crates/core/src/main.rs | 1 + crates/format/src/input.rs | 16 ++++++ crates/node-interaction/src/lib.rs | 11 +++- crates/node/src/geth.rs | 20 ++++++- crates/node/src/kitchensink.rs | 18 ++++++- 7 files changed, 153 insertions(+), 6 deletions(-) diff --git a/assets/test_metadata.json b/assets/test_metadata.json index 7fca971..127e808 100644 --- a/assets/test_metadata.json +++ b/assets/test_metadata.json @@ -12,6 +12,14 @@ "address": "0xdeadbeef00000000000000000000000000000042", "expected_balance": "1233" }, + { + "address": "0xdeadbeef00000000000000000000000000000042", + "is_storage_empty": true + }, + { + "address": "0xdeadbeef00000000000000000000000000000042", + "is_storage_empty": false + }, { "instance": "WBTC_1", "method": "#deployer", diff --git a/crates/core/src/driver/mod.rs b/crates/core/src/driver/mod.rs index 32b13c5..ae96a08 100644 --- a/crates/core/src/driver/mod.rs +++ b/crates/core/src/driver/mod.rs @@ -4,6 +4,7 @@ use std::collections::HashMap; use std::marker::PhantomData; use std::path::PathBuf; +use alloy::consensus::EMPTY_ROOT_HASH; use alloy::hex; use alloy::json_abi::JsonAbi; use alloy::network::{Ethereum, TransactionBuilder}; @@ -28,6 +29,7 @@ use semver::Version; use revive_dt_format::case::{Case, CaseIdx}; use revive_dt_format::input::{ BalanceAssertion, Calldata, EtherValue, Expected, ExpectedOutput, Input, Method, + StorageEmptyAssertion, }; use revive_dt_format::metadata::{ContractInstance, ContractPathAndIdent}; use revive_dt_format::{input::Step, metadata::Metadata}; @@ -90,6 +92,11 @@ where .await?; Ok(StepOutput::BalanceAssertion) } + Step::StorageEmptyAssertion(storage_empty) => { + self.handle_storage_empty(metadata, case_idx, storage_empty, node) + .await?; + Ok(StepOutput::StorageEmptyAssertion) + } } } @@ -125,7 +132,22 @@ where ) -> anyhow::Result<()> { self.handle_balance_assertion_contract_deployment(metadata, balance_assertion, node) .await?; + self.handle_balance_assertion_execution(balance_assertion, node) + .await?; + Ok(()) + } + pub async fn handle_storage_empty( + &mut self, + metadata: &Metadata, + _: CaseIdx, + storage_empty: &StorageEmptyAssertion, + node: &T::Blockchain, + ) -> anyhow::Result<()> { + self.handle_storage_empty_assertion_contract_deployment(metadata, storage_empty, node) + .await?; + self.handle_storage_empty_assertion_execution(storage_empty, node) + .await?; Ok(()) } @@ -557,6 +579,67 @@ where Ok(()) } + pub async fn handle_storage_empty_assertion_contract_deployment( + &mut self, + metadata: &Metadata, + storage_empty_assertion: &StorageEmptyAssertion, + node: &T::Blockchain, + ) -> anyhow::Result<()> { + let Some(instance) = storage_empty_assertion + .address + .strip_prefix(".address") + .map(ContractInstance::new) + else { + return Ok(()); + }; + self.get_or_deploy_contract_instance( + &instance, + metadata, + Input::default_caller(), + None, + None, + node, + ) + .await?; + Ok(()) + } + + pub async fn handle_storage_empty_assertion_execution( + &mut self, + StorageEmptyAssertion { + address: address_string, + is_storage_empty, + }: &StorageEmptyAssertion, + node: &T::Blockchain, + ) -> anyhow::Result<()> { + let address = Address::from_slice( + Calldata::new_compound([address_string]) + .calldata(node, self.default_resolution_context()) + .await? + .get(12..32) + .expect("Can't fail"), + ); + + let storage = node.latest_state_proof(address, Default::default()).await?; + let is_empty = storage.storage_hash == EMPTY_ROOT_HASH; + + let expected = is_storage_empty; + let actual = is_empty; + + if *expected != actual { + tracing::error!(%expected, %actual, %address, "Storage Empty Assertion failed"); + anyhow::bail!( + "Storage Empty Assertion failed - Expected {} but got {} for {} resolved to {}", + expected, + actual, + address_string, + address, + ) + }; + + Ok(()) + } + /// Gets the information of a deployed contract or library from the state. If it's found to not /// be deployed then it will be deployed. /// @@ -780,6 +863,7 @@ where } } (StepOutput::BalanceAssertion, StepOutput::BalanceAssertion) => {} + (StepOutput::StorageEmptyAssertion, StepOutput::StorageEmptyAssertion) => {} _ => unreachable!("The two step outputs can not be of a different kind"), } @@ -795,4 +879,5 @@ where pub enum StepOutput { FunctionCall(TransactionReceipt, GethTrace, DiffMode), BalanceAssertion, + StorageEmptyAssertion, } diff --git a/crates/core/src/main.rs b/crates/core/src/main.rs index 70512a5..7336542 100644 --- a/crates/core/src/main.rs +++ b/crates/core/src/main.rs @@ -451,6 +451,7 @@ where .filter_map(|step| match step { Step::FunctionCall(input) => Some(input.caller), Step::BalanceAssertion(..) => None, + Step::StorageEmptyAssertion(..) => None, }) .next() .unwrap_or(Input::default_caller()); diff --git a/crates/format/src/input.rs b/crates/format/src/input.rs index cc96341..d742f3a 100644 --- a/crates/format/src/input.rs +++ b/crates/format/src/input.rs @@ -28,6 +28,8 @@ pub enum Step { FunctionCall(Box), /// A step for performing a balance assertion on some account or contract. BalanceAssertion(Box), + /// A step for asserting that the storage of some contract or account is empty. + StorageEmptyAssertion(Box), } #[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] @@ -60,6 +62,20 @@ pub struct BalanceAssertion { pub expected_balance: U256, } +#[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] +pub struct StorageEmptyAssertion { + /// The address that the balance assertion should be done on. + /// + /// This is a string which will be resolved into an address when being processed. Therefore, + /// this could be a normal hex address, a variable such as `Test.address`, or perhaps even a + /// full on variable like `$VARIABLE:Uniswap`. It follows the same resolution rules that are + /// followed in the calldata. + pub address: String, + + /// A boolean of whether the storage of the address is empty or not. + pub is_storage_empty: bool, +} + #[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)] #[serde(untagged)] pub enum Expected { diff --git a/crates/node-interaction/src/lib.rs b/crates/node-interaction/src/lib.rs index b052c7a..a6e3b38 100644 --- a/crates/node-interaction/src/lib.rs +++ b/crates/node-interaction/src/lib.rs @@ -1,8 +1,8 @@ //! This crate implements all node interactions. -use alloy::primitives::{Address, U256}; +use alloy::primitives::{Address, StorageKey, U256}; use alloy::rpc::types::trace::geth::{DiffMode, GethDebugTracingOptions, GethTrace}; -use alloy::rpc::types::{TransactionReceipt, TransactionRequest}; +use alloy::rpc::types::{EIP1186AccountProofResponse, TransactionReceipt, TransactionRequest}; use anyhow::Result; /// An interface for all interactions with Ethereum compatible nodes. @@ -25,4 +25,11 @@ pub trait EthereumNode { /// Returns the balance of the provided [`Address`] back. fn balance_of(&self, address: Address) -> impl Future>; + + /// Returns the latest storage proof of the provided [`Address`] + fn latest_state_proof( + &self, + address: Address, + keys: Vec, + ) -> impl Future>; } diff --git a/crates/node/src/geth.rs b/crates/node/src/geth.rs index b3618ba..38fba17 100644 --- a/crates/node/src/geth.rs +++ b/crates/node/src/geth.rs @@ -17,14 +17,16 @@ use alloy::{ eips::BlockNumberOrTag, genesis::{Genesis, GenesisAccount}, network::{Ethereum, EthereumWallet, NetworkWallet}, - primitives::{Address, BlockHash, BlockNumber, BlockTimestamp, FixedBytes, TxHash, U256}, + primitives::{ + Address, BlockHash, BlockNumber, BlockTimestamp, FixedBytes, StorageKey, TxHash, U256, + }, providers::{ Provider, ProviderBuilder, ext::DebugApi, fillers::{CachedNonceManager, ChainIdFiller, FillProvider, NonceFiller, TxFiller}, }, rpc::types::{ - TransactionReceipt, TransactionRequest, + EIP1186AccountProofResponse, TransactionReceipt, TransactionRequest, trace::geth::{DiffMode, GethDebugTracingOptions, PreStateConfig, PreStateFrame}, }, signers::local::PrivateKeySigner, @@ -380,6 +382,20 @@ impl EthereumNode for GethNode { .await .map_err(Into::into) } + + #[tracing::instrument(skip_all, fields(geth_node_id = self.id))] + async fn latest_state_proof( + &self, + address: Address, + keys: Vec, + ) -> anyhow::Result { + self.provider() + .await? + .get_proof(address, keys) + .latest() + .await + .map_err(Into::into) + } } impl ResolverApi for GethNode { diff --git a/crates/node/src/kitchensink.rs b/crates/node/src/kitchensink.rs index 805b71f..4ef398c 100644 --- a/crates/node/src/kitchensink.rs +++ b/crates/node/src/kitchensink.rs @@ -17,7 +17,7 @@ use alloy::{ }, primitives::{ Address, B64, B256, BlockHash, BlockNumber, BlockTimestamp, Bloom, Bytes, FixedBytes, - TxHash, U256, + StorageKey, TxHash, U256, }, providers::{ Provider, ProviderBuilder, @@ -25,7 +25,7 @@ use alloy::{ fillers::{CachedNonceManager, ChainIdFiller, FillProvider, NonceFiller, TxFiller}, }, rpc::types::{ - TransactionReceipt, + EIP1186AccountProofResponse, TransactionReceipt, eth::{Block, Header, Transaction}, trace::geth::{DiffMode, GethDebugTracingOptions, PreStateConfig, PreStateFrame}, }, @@ -437,6 +437,20 @@ impl EthereumNode for KitchensinkNode { .await .map_err(Into::into) } + + #[tracing::instrument(skip_all, fields(kitchensink_node_id = self.id))] + async fn latest_state_proof( + &self, + address: Address, + keys: Vec, + ) -> anyhow::Result { + self.provider() + .await? + .get_proof(address, keys) + .latest() + .await + .map_err(Into::into) + } } impl ResolverApi for KitchensinkNode { From f67a9bf643180718eb01a9aae7aa0dc5bb49ec61 Mon Sep 17 00:00:00 2001 From: Omar Date: Tue, 12 Aug 2025 11:55:21 +0300 Subject: [PATCH 4/6] Refactor/ignore null values (#135) * Skip serialization of null values * Add support for comments in various steps --- crates/format/src/case.rs | 6 ++++++ crates/format/src/input.rs | 17 +++++++++++++++++ crates/format/src/metadata.rs | 6 ++++++ 3 files changed, 29 insertions(+) diff --git a/crates/format/src/case.rs b/crates/format/src/case.rs index 26e803e..ccb9394 100644 --- a/crates/format/src/case.rs +++ b/crates/format/src/case.rs @@ -9,13 +9,19 @@ use crate::{ #[derive(Debug, Default, Serialize, Deserialize, Clone, Eq, PartialEq)] pub struct Case { + #[serde(skip_serializing_if = "Option::is_none")] pub name: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub comment: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub modes: Option>, #[serde(rename = "inputs")] pub steps: Vec, + #[serde(skip_serializing_if = "Option::is_none")] pub group: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub expected: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub ignore: Option, } diff --git a/crates/format/src/input.rs b/crates/format/src/input.rs index d742f3a..1b80e97 100644 --- a/crates/format/src/input.rs +++ b/crates/format/src/input.rs @@ -36,20 +36,29 @@ pub enum Step { pub struct Input { #[serde(default = "Input::default_caller")] pub caller: Address, + #[serde(skip_serializing_if = "Option::is_none")] pub comment: Option, #[serde(default = "Input::default_instance")] pub instance: ContractInstance, pub method: Method, #[serde(default)] pub calldata: Calldata, + #[serde(skip_serializing_if = "Option::is_none")] pub expected: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub value: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub storage: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub variable_assignments: Option, } #[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] pub struct BalanceAssertion { + /// An optional comment on the balance assertion. + #[serde(skip_serializing_if = "Option::is_none")] + pub comment: Option, + /// The address that the balance assertion should be done on. /// /// This is a string which will be resolved into an address when being processed. Therefore, @@ -64,6 +73,10 @@ pub struct BalanceAssertion { #[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] pub struct StorageEmptyAssertion { + /// An optional comment on the storage empty assertion. + #[serde(skip_serializing_if = "Option::is_none")] + pub comment: Option, + /// The address that the balance assertion should be done on. /// /// This is a string which will be resolved into an address when being processed. Therefore, @@ -86,8 +99,11 @@ pub enum Expected { #[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] pub struct ExpectedOutput { + #[serde(skip_serializing_if = "Option::is_none")] pub compiler_version: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub return_data: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub events: Option>, #[serde(default)] pub exception: bool, @@ -95,6 +111,7 @@ pub struct ExpectedOutput { #[derive(Clone, Debug, Default, Serialize, Deserialize, Eq, PartialEq)] pub struct Event { + #[serde(skip_serializing_if = "Option::is_none")] pub address: Option, pub topics: Vec, pub values: Calldata, diff --git a/crates/format/src/metadata.rs b/crates/format/src/metadata.rs index 1255187..99debe3 100644 --- a/crates/format/src/metadata.rs +++ b/crates/format/src/metadata.rs @@ -45,12 +45,18 @@ impl Deref for MetadataFile { #[derive(Debug, Default, Serialize, Deserialize, Clone, Eq, PartialEq)] pub struct Metadata { + #[serde(skip_serializing_if = "Option::is_none")] pub targets: Option>, pub cases: Vec, + #[serde(skip_serializing_if = "Option::is_none")] pub contracts: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub libraries: Option>>, + #[serde(skip_serializing_if = "Option::is_none")] pub ignore: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub modes: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub file_path: Option, } From 9b40c9b9e3dae8753fc361b183f852cacabae1e6 Mon Sep 17 00:00:00 2001 From: Omar Date: Tue, 12 Aug 2025 13:19:59 +0300 Subject: [PATCH 5/6] Add an EVM version filter (#136) * Add an EVM version filter * Update naming --- Cargo.lock | 2 + Cargo.toml | 2 +- crates/core/src/driver/mod.rs | 2 + crates/core/src/main.rs | 21 ++++++ crates/format/Cargo.toml | 2 + crates/format/src/metadata.rs | 133 +++++++++++++++++++++++++++++++++ crates/node/Cargo.toml | 1 + crates/node/src/geth.rs | 5 ++ crates/node/src/kitchensink.rs | 5 ++ crates/node/src/lib.rs | 4 + 10 files changed, 176 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 02d801f..3d855c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4095,6 +4095,7 @@ dependencies = [ "alloy-primitives", "alloy-sol-types", "anyhow", + "revive-common", "revive-dt-common", "semver 1.0.26", "serde", @@ -4109,6 +4110,7 @@ version = "0.1.0" dependencies = [ "alloy", "anyhow", + "revive-common", "revive-dt-common", "revive-dt-config", "revive-dt-format", diff --git a/Cargo.toml b/Cargo.toml index 92bb064..2441471 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ authors = ["Parity Technologies "] license = "MIT/Apache-2.0" edition = "2024" repository = "https://github.com/paritytech/revive-differential-testing.git" -rust-version = "1.85.0" +rust-version = "1.87.0" [workspace.dependencies] revive-dt-common = { version = "0.1.0", path = "crates/common" } diff --git a/crates/core/src/driver/mod.rs b/crates/core/src/driver/mod.rs index ae96a08..435bd39 100644 --- a/crates/core/src/driver/mod.rs +++ b/crates/core/src/driver/mod.rs @@ -550,6 +550,7 @@ where BalanceAssertion { address: address_string, expected_balance: amount, + .. }: &BalanceAssertion, node: &T::Blockchain, ) -> anyhow::Result<()> { @@ -609,6 +610,7 @@ where StorageEmptyAssertion { address: address_string, is_storage_empty, + .. }: &StorageEmptyAssertion, node: &T::Blockchain, ) -> anyhow::Result<()> { diff --git a/crates/core/src/main.rs b/crates/core/src/main.rs index 7336542..81a688f 100644 --- a/crates/core/src/main.rs +++ b/crates/core/src/main.rs @@ -177,6 +177,27 @@ where Some(false) | None => true, }, ) + .filter(|(metadata_file_path, metadata, ..)| match metadata.required_evm_version { + Some(evm_version_requirement) => { + let is_allowed = evm_version_requirement + .matches(&::evm_version()) + && evm_version_requirement + .matches(&::evm_version()); + + if !is_allowed { + tracing::warn!( + metadata_file_path = %metadata_file_path.display(), + leader_evm_version = %::evm_version(), + follower_evm_version = %::evm_version(), + version_requirement = %evm_version_requirement, + "Skipped test since the EVM version requirement was not fulfilled." + ); + } + + is_allowed + } + None => true, + }) .collect::>(); let metadata_case_status = Arc::new(RwLock::new(test_cases.iter().fold( diff --git a/crates/format/Cargo.toml b/crates/format/Cargo.toml index 48d754b..0f50758 100644 --- a/crates/format/Cargo.toml +++ b/crates/format/Cargo.toml @@ -11,6 +11,8 @@ rust-version.workspace = true [dependencies] revive-dt-common = { workspace = true } +revive-common = { workspace = true } + alloy = { workspace = true } alloy-primitives = { workspace = true } alloy-sol-types = { workspace = true } diff --git a/crates/format/src/metadata.rs b/crates/format/src/metadata.rs index 99debe3..ad4fac9 100644 --- a/crates/format/src/metadata.rs +++ b/crates/format/src/metadata.rs @@ -1,4 +1,5 @@ use std::{ + cmp::Ordering, collections::BTreeMap, fmt::Display, fs::{File, read_to_string}, @@ -9,6 +10,7 @@ use std::{ use serde::{Deserialize, Serialize}; +use revive_common::EVMVersion; use revive_dt_common::{iterators::FilesWithExtensionIterator, macros::define_wrapper_type}; use crate::{ @@ -58,6 +60,12 @@ pub struct Metadata { pub modes: Option>, #[serde(skip_serializing_if = "Option::is_none")] pub file_path: Option, + + /// This field specifies an EVM version requirement that the test case has + /// where the test might be run of the evm version of the nodes match the + /// evm version specified here. + #[serde(skip_serializing_if = "Option::is_none")] + pub required_evm_version: Option, } impl Metadata { @@ -348,6 +356,131 @@ impl From for String { } } +/// An EVM version requirement that the test case has. This gets serialized and +/// deserialized from and into [`String`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +#[serde(try_from = "String", into = "String")] +pub struct EvmVersionRequirement { + ordering: Ordering, + or_equal: bool, + evm_version: EVMVersion, +} + +impl EvmVersionRequirement { + pub fn new_greater_than_or_equals(version: EVMVersion) -> Self { + Self { + ordering: Ordering::Greater, + or_equal: true, + evm_version: version, + } + } + + pub fn new_greater_than(version: EVMVersion) -> Self { + Self { + ordering: Ordering::Greater, + or_equal: false, + evm_version: version, + } + } + + pub fn new_equals(version: EVMVersion) -> Self { + Self { + ordering: Ordering::Equal, + or_equal: false, + evm_version: version, + } + } + + pub fn new_less_than(version: EVMVersion) -> Self { + Self { + ordering: Ordering::Less, + or_equal: false, + evm_version: version, + } + } + + pub fn new_less_than_or_equals(version: EVMVersion) -> Self { + Self { + ordering: Ordering::Less, + or_equal: true, + evm_version: version, + } + } + + pub fn matches(&self, other: &EVMVersion) -> bool { + let ordering = other.cmp(&self.evm_version); + ordering == self.ordering || (self.or_equal && matches!(ordering, Ordering::Equal)) + } +} + +impl Display for EvmVersionRequirement { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let Self { + ordering, + or_equal, + evm_version, + } = self; + match ordering { + Ordering::Less => write!(f, "<")?, + Ordering::Equal => write!(f, "=")?, + Ordering::Greater => write!(f, ">")?, + } + if *or_equal && !matches!(ordering, Ordering::Equal) { + write!(f, "=")?; + } + write!(f, "{evm_version}") + } +} + +impl FromStr for EvmVersionRequirement { + type Err = anyhow::Error; + + fn from_str(s: &str) -> Result { + match s.as_bytes() { + [b'>', b'=', remaining @ ..] => Ok(Self { + ordering: Ordering::Greater, + or_equal: true, + evm_version: str::from_utf8(remaining)?.try_into()?, + }), + [b'>', remaining @ ..] => Ok(Self { + ordering: Ordering::Greater, + or_equal: false, + evm_version: str::from_utf8(remaining)?.try_into()?, + }), + [b'<', b'=', remaining @ ..] => Ok(Self { + ordering: Ordering::Less, + or_equal: true, + evm_version: str::from_utf8(remaining)?.try_into()?, + }), + [b'<', remaining @ ..] => Ok(Self { + ordering: Ordering::Less, + or_equal: false, + evm_version: str::from_utf8(remaining)?.try_into()?, + }), + [b'=', remaining @ ..] => Ok(Self { + ordering: Ordering::Equal, + or_equal: false, + evm_version: str::from_utf8(remaining)?.try_into()?, + }), + _ => anyhow::bail!("Invalid EVM version requirement {s}"), + } + } +} + +impl TryFrom for EvmVersionRequirement { + type Error = anyhow::Error; + + fn try_from(value: String) -> Result { + value.parse() + } +} + +impl From for String { + fn from(value: EvmVersionRequirement) -> Self { + value.to_string() + } +} + #[cfg(test)] mod test { use super::*; diff --git a/crates/node/Cargo.toml b/crates/node/Cargo.toml index a930312..318e1a2 100644 --- a/crates/node/Cargo.toml +++ b/crates/node/Cargo.toml @@ -14,6 +14,7 @@ alloy = { workspace = true } tracing = { workspace = true } tokio = { workspace = true } +revive-common = { workspace = true } revive-dt-common = { workspace = true } revive-dt-config = { workspace = true } revive-dt-format = { workspace = true } diff --git a/crates/node/src/geth.rs b/crates/node/src/geth.rs index 38fba17..9d91040 100644 --- a/crates/node/src/geth.rs +++ b/crates/node/src/geth.rs @@ -32,6 +32,7 @@ use alloy::{ signers::local::PrivateKeySigner, }; use anyhow::Context; +use revive_common::EVMVersion; use tracing::{Instrument, Level}; use revive_dt_common::{fs::clear_directory, futures::poll}; @@ -579,6 +580,10 @@ impl Node for GethNode { Some(targets) => targets.iter().any(|str| str.as_str() == "evm"), } } + + fn evm_version() -> EVMVersion { + EVMVersion::Cancun + } } impl Drop for GethNode { diff --git a/crates/node/src/kitchensink.rs b/crates/node/src/kitchensink.rs index 4ef398c..c5dd254 100644 --- a/crates/node/src/kitchensink.rs +++ b/crates/node/src/kitchensink.rs @@ -32,6 +32,7 @@ use alloy::{ signers::local::PrivateKeySigner, }; use anyhow::Context; +use revive_common::EVMVersion; use revive_dt_common::fs::clear_directory; use revive_dt_format::traits::ResolverApi; use serde::{Deserialize, Serialize}; @@ -638,6 +639,10 @@ impl Node for KitchensinkNode { Some(targets) => targets.iter().any(|str| str.as_str() == "pvm"), } } + + fn evm_version() -> EVMVersion { + EVMVersion::Cancun + } } impl Drop for KitchensinkNode { diff --git a/crates/node/src/lib.rs b/crates/node/src/lib.rs index 1232e97..446b66a 100644 --- a/crates/node/src/lib.rs +++ b/crates/node/src/lib.rs @@ -1,5 +1,6 @@ //! This crate implements the testing nodes. +use revive_common::EVMVersion; use revive_dt_config::Arguments; use revive_dt_node_interaction::EthereumNode; @@ -36,4 +37,7 @@ pub trait Node: EthereumNode { /// Given a list of targets from the metadata file, this function determines if the metadata /// file can be ran on this node or not. fn matches_target(&self, targets: Option<&[String]>) -> bool; + + /// Returns the EVM version of the node. + fn evm_version() -> EVMVersion; } From 46aea0890d55088bfbdcaecf467ff2b70d1898f8 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Wed, 13 Aug 2025 14:10:26 +0100 Subject: [PATCH 6/6] Split reporter and case runner, use channels to pass test reports (#137) * Use channels to send data to reporting thread and avoid hangs / mutex / duration. Limit max concurrent tasks to avoid too many open files * More appropriate name for dirver/reporter task fns * Back to parallelise individual cases, report individual cases, address grumbles * newline before 'Failures' title in report --- crates/config/src/lib.rs | 20 ++- crates/core/src/main.rs | 361 ++++++++++++++++++++------------------- 2 files changed, 203 insertions(+), 178 deletions(-) diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 67ed625..b7871fb 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -96,10 +96,19 @@ pub struct Arguments { #[arg(long, default_value = "1")] pub number_of_nodes: usize, - /// Determines the amount of threads that will will be used. - #[arg(long, default_value = "12")] + /// Determines the amount of tokio worker threads that will will be used. + #[arg( + long, + default_value_t = std::thread::available_parallelism() + .map(|n| n.get()) + .unwrap_or(1) + )] pub number_of_threads: usize, + /// Determines the amount of concurrent tasks that will be spawned to run tests. Defaults to 10 x the number of nodes. + #[arg(long)] + pub number_concurrent_tasks: Option, + /// Extract problems back to the test corpus. #[arg(short, long = "extract-problems")] pub extract_problems: bool, @@ -134,6 +143,13 @@ impl Arguments { panic!("should have a workdir configured") } + /// Return the number of concurrent tasks to run. This is provided via the + /// `--number-concurrent-tasks` argument, and otherwise defaults to --number-of-nodes * 20. + pub fn number_of_concurrent_tasks(&self) -> usize { + self.number_concurrent_tasks + .unwrap_or(20 * self.number_of_nodes) + } + /// Try to parse `self.account` into a [PrivateKeySigner], /// panicing on error. pub fn wallet(&self) -> EthereumWallet { diff --git a/crates/core/src/main.rs b/crates/core/src/main.rs index 81a688f..55ec7d3 100644 --- a/crates/core/src/main.rs +++ b/crates/core/src/main.rs @@ -1,6 +1,6 @@ use std::{ collections::HashMap, - path::Path, + path::{Path, PathBuf}, sync::{Arc, LazyLock}, time::Instant, }; @@ -18,7 +18,7 @@ use revive_dt_common::iterators::FilesWithExtensionIterator; use revive_dt_node_interaction::EthereumNode; use semver::Version; use temp_dir::TempDir; -use tokio::sync::{Mutex, RwLock}; +use tokio::sync::{Mutex, RwLock, mpsc}; use tracing::{Instrument, Level}; use tracing_subscriber::{EnvFilter, FmtSubscriber}; @@ -41,15 +41,28 @@ use revive_dt_report::reporter::{Report, Span}; static TEMP_DIR: LazyLock = LazyLock::new(|| TempDir::new().unwrap()); -type CompilationCache<'a> = Arc< +type CompilationCache = Arc< RwLock< HashMap< - (&'a Path, SolcMode, TestingPlatform), + (PathBuf, SolcMode, TestingPlatform), Arc>>>, >, >, >; +/// this represents a single "test"; a mode, path and collection of cases. +#[derive(Clone)] +struct Test { + metadata: Metadata, + path: PathBuf, + mode: SolcMode, + case_idx: usize, + case: Case, +} + +/// This represents the results that we gather from running test cases. +type CaseResult = Result; + fn main() -> anyhow::Result<()> { let args = init_cli()?; @@ -120,7 +133,7 @@ fn collect_corpora(args: &Arguments) -> anyhow::Result( args: &Arguments, - tests: &[MetadataFile], + metadata_files: &[MetadataFile], span: Span, ) -> anyhow::Result<()> where @@ -129,10 +142,25 @@ where L::Blockchain: revive_dt_node::Node + Send + Sync + 'static, F::Blockchain: revive_dt_node::Node + Send + Sync + 'static, { - let leader_nodes = NodePool::::new(args)?; - let follower_nodes = NodePool::::new(args)?; + let (report_tx, report_rx) = mpsc::unbounded_channel::<(Test, CaseResult)>(); - let test_cases = tests + let tests = prepare_tests::(metadata_files); + let driver_task = start_driver_task::(args, tests, span, report_tx)?; + let status_reporter_task = start_reporter_task(report_rx); + + tokio::join!(status_reporter_task, driver_task); + + Ok(()) +} + +fn prepare_tests(metadata_files: &[MetadataFile]) -> impl Iterator +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 .iter() .flat_map( |MetadataFile { @@ -198,188 +226,159 @@ where } None => true, }) - .collect::>(); - - let metadata_case_status = Arc::new(RwLock::new(test_cases.iter().fold( - HashMap::<_, HashMap<_, _>>::new(), - |mut map, (path, _, case_idx, case, solc_mode)| { - map.entry((path.to_path_buf(), solc_mode.clone())) - .or_default() - .insert((CaseIdx::new(*case_idx), case.name.clone()), None::); - map - }, - ))); - let status_reporter_task = { - let metadata_case_status = metadata_case_status.clone(); - let start = Instant::now(); - async move { - const GREEN: &str = "\x1B[32m"; - const RED: &str = "\x1B[31m"; - const RESET: &str = "\x1B[0m"; - - let mut entries_to_delete = Vec::new(); - let mut number_of_successes = 0; - let mut number_of_failures = 0; - loop { - let metadata_case_status_read = metadata_case_status.read().await; - if metadata_case_status_read.is_empty() { - break; - } - - for ((metadata_file_path, solc_mode), case_status) in - metadata_case_status_read.iter() - { - if case_status.values().any(|value| value.is_none()) { - continue; - } - - let contains_failures = case_status - .values() - .any(|value| value.is_some_and(|value| !value)); - - if !contains_failures { - eprintln!( - "{}Succeeded:{} {} - {:?}", - GREEN, - RESET, - metadata_file_path.display(), - solc_mode - ) - } else { - eprintln!( - "{}Failed:{} {} - {:?}", - RED, - RESET, - metadata_file_path.display(), - solc_mode - ) - }; - - number_of_successes += case_status - .values() - .filter(|value| value.is_some_and(|value| value)) - .count(); - number_of_failures += case_status - .values() - .filter(|value| value.is_some_and(|value| !value)) - .count(); - - let mut case_status = case_status - .iter() - .map(|((case_idx, case_name), case_status)| { - (case_idx.into_inner(), case_name, case_status.unwrap()) - }) - .collect::>(); - case_status.sort_by(|a, b| a.0.cmp(&b.0)); - for (case_idx, case_name, case_status) in case_status.into_iter() { - if case_status { - eprintln!( - " {GREEN}Case Succeeded:{RESET} {} - Case Idx: {case_idx}", - case_name - .as_ref() - .map(|string| string.as_str()) - .unwrap_or("Unnamed case") - ) - } else { - eprintln!( - " {RED}Case Failed:{RESET} {} - Case Idx: {case_idx}", - case_name - .as_ref() - .map(|string| string.as_str()) - .unwrap_or("Unnamed case") - ) - }; - } - eprintln!(); - - entries_to_delete.push((metadata_file_path.clone(), solc_mode.clone())); - } - - drop(metadata_case_status_read); - let mut metadata_case_status_write = metadata_case_status.write().await; - for entry in entries_to_delete.drain(..) { - metadata_case_status_write.remove(&entry); - } - - tokio::time::sleep(std::time::Duration::from_secs(3)).await; + .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: case.clone(), } + }) +} - let elapsed = start.elapsed(); - eprintln!( - "{GREEN}{}{RESET} cases succeeded, {RED}{}{RESET} cases failed in {} seconds", - number_of_successes, - number_of_failures, - elapsed.as_secs() - ); - } - }; - +fn start_driver_task( + args: &Arguments, + tests: impl Iterator, + span: Span, + report_tx: mpsc::UnboundedSender<(Test, CaseResult)>, +) -> anyhow::Result> +where + L: Platform, + F: Platform, + L::Blockchain: revive_dt_node::Node + Send + Sync + 'static, + F::Blockchain: revive_dt_node::Node + Send + Sync + 'static, +{ + let leader_nodes = Arc::new(NodePool::::new(args)?); + let follower_nodes = Arc::new(NodePool::::new(args)?); let compilation_cache = Arc::new(RwLock::new(HashMap::new())); - let driver_task = futures::stream::iter(test_cases).for_each_concurrent( - None, - |(metadata_file_path, metadata, case_idx, case, solc_mode)| { + let number_concurrent_tasks = args.number_of_concurrent_tasks(); + + Ok(futures::stream::iter(tests).for_each_concurrent( + // We want to limit the concurrent tasks here because: + // + // 1. We don't want to overwhelm the nodes with too many requests, leading to responses timing out. + // 2. We don't want to open too many files at once, leading to the OS running out of file descriptors. + // + // By default, we allow maximum of 10 ongoing requests per node in order to limit (1), and assume that + // this number will automatically be low enough to address (2). The user can override this. + Some(number_concurrent_tasks), + move |test| { + let leader_nodes = leader_nodes.clone(); + let follower_nodes = follower_nodes.clone(); let compilation_cache = compilation_cache.clone(); - let leader_node = leader_nodes.round_robbin(); - let follower_node = follower_nodes.round_robbin(); - let tracing_span = tracing::span!( - Level::INFO, - "Running driver", - metadata_file_path = %metadata_file_path.display(), - case_idx = case_idx, - solc_mode = ?solc_mode, - ); - let metadata_case_status = metadata_case_status.clone(); + let report_tx = report_tx.clone(); + async move { + let leader_node = leader_nodes.round_robbin(); + let follower_node = follower_nodes.round_robbin(); + + let tracing_span = tracing::span!( + Level::INFO, + "Running driver", + metadata_file_path = %test.path.display(), + case_idx = ?test.case_idx, + solc_mode = ?test.mode, + ); + let result = handle_case_driver::( - metadata_file_path.as_path(), - metadata, - case_idx.into(), - case, - solc_mode.clone(), + &test.path, + &test.metadata, + test.case_idx.into(), + &test.case, + test.mode.clone(), args, compilation_cache.clone(), leader_node, follower_node, span, ) + .instrument(tracing_span) .await; - let mut metadata_case_status = metadata_case_status.write().await; - match result { - Ok(inputs_executed) => { - tracing::info!(inputs_executed, "Execution succeeded"); - metadata_case_status - .entry((metadata_file_path.clone(), solc_mode)) - .or_default() - .insert((CaseIdx::new(case_idx), case.name.clone()), Some(true)); - } - Err(error) => { - metadata_case_status - .entry((metadata_file_path.clone(), solc_mode)) - .or_default() - .insert((CaseIdx::new(case_idx), case.name.clone()), Some(false)); - tracing::error!(%error, "Execution failed") - } - } - tracing::info!("Execution completed"); + + report_tx + .send((test, result)) + .expect("Failed to send report"); } - .instrument(tracing_span) }, + )) +} + +async fn start_reporter_task(mut report_rx: mpsc::UnboundedReceiver<(Test, CaseResult)>) { + let start = Instant::now(); + + const GREEN: &str = "\x1B[32m"; + const RED: &str = "\x1B[31m"; + const COLOUR_RESET: &str = "\x1B[0m"; + const BOLD: &str = "\x1B[1m"; + const BOLD_RESET: &str = "\x1B[22m"; + + let mut number_of_successes = 0; + let mut number_of_failures = 0; + let mut failures = vec![]; + + // Wait for reports to come from our test runner. When the channel closes, this ends. + 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_mode = test.mode.clone(); + + match case_result { + Ok(_inputs) => { + number_of_successes += 1; + eprintln!( + "{GREEN}Case Succeeded:{COLOUR_RESET} {test_path} -> {case_name}:{case_idx} (mode: {test_mode:?})" + ); + } + Err(err) => { + number_of_failures += 1; + eprintln!( + "{RED}Case Failed:{COLOUR_RESET} {test_path} -> {case_name}:{case_idx} (mode: {test_mode:?})" + ); + failures.push((test, err)); + } + } + } + + eprintln!(); + 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"); + + 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_mode = test.mode.clone(); + + eprintln!( + "---- {RED}Case Failed:{COLOUR_RESET} {test_path} -> {case_name}:{case_idx} (mode: {test_mode:?}) ----\n\n{err}\n" + ); + } + } + + // Summary at the end. + eprintln!( + "{} 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() ); - - tokio::join!(status_reporter_task, driver_task); - - Ok(()) } #[allow(clippy::too_many_arguments)] -async fn handle_case_driver<'a, L, F>( - metadata_file_path: &'a Path, - metadata: &'a Metadata, +async fn handle_case_driver( + metadata_file_path: &Path, + metadata: &Metadata, case_idx: CaseIdx, case: &Case, mode: SolcMode, config: &Arguments, - compilation_cache: CompilationCache<'a>, + compilation_cache: CompilationCache, leader_node: &L::Blockchain, follower_node: &F::Blockchain, _: Span, @@ -520,11 +519,9 @@ where ); let Some(leader_library_address) = leader_receipt.contract_address else { - tracing::error!("Contract deployment transaction didn't return an address"); anyhow::bail!("Contract deployment didn't return an address"); }; let Some(follower_library_address) = follower_receipt.contract_address else { - tracing::error!("Contract deployment transaction didn't return an address"); anyhow::bail!("Contract deployment didn't return an address"); }; @@ -554,8 +551,16 @@ where .any(|(code, _)| !code.chars().all(|char| char.is_ascii_hexdigit())); let (leader_compiled_contracts, follower_compiled_contracts) = if metadata_file_contains_libraries && compiled_contracts_require_linking { - let leader_key = (metadata_file_path, mode.clone(), L::config_id()); - let follower_key = (metadata_file_path, mode.clone(), L::config_id()); + let leader_key = ( + metadata_file_path.to_path_buf(), + mode.clone(), + L::config_id(), + ); + let follower_key = ( + metadata_file_path.to_path_buf(), + mode.clone(), + F::config_id(), + ); { let mut cache = compilation_cache.write().await; cache.remove(&leader_key); @@ -609,15 +614,19 @@ where driver.execute().await } -async fn get_or_build_contracts<'a, P: Platform>( - metadata: &'a Metadata, - metadata_file_path: &'a Path, +async fn get_or_build_contracts( + metadata: &Metadata, + metadata_file_path: &Path, mode: SolcMode, config: &Arguments, - compilation_cache: CompilationCache<'a>, + compilation_cache: CompilationCache, deployed_libraries: &HashMap, ) -> anyhow::Result> { - let key = (metadata_file_path, mode.clone(), P::config_id()); + let key = ( + metadata_file_path.to_path_buf(), + mode.clone(), + P::config_id(), + ); if let Some(compilation_artifact) = compilation_cache.read().await.get(&key).cloned() { let mut compilation_artifact = compilation_artifact.lock().await; match *compilation_artifact {