From a9970eb2bb6d14a566bd736ab83ab6ec4887813b Mon Sep 17 00:00:00 2001 From: Omar Date: Mon, 21 Jul 2025 12:01:52 +0300 Subject: [PATCH 1/3] Refactor the input handling logic (#48) * Add support for wrapper types * Move `FilesWithExtensionIterator` to `core::common` * Remove unneeded use of two `HashMap`s * Make metadata structs more typed * Impl new_from for wrapper types * Implement the new input handling logic * Fix edge-case in input handling * Ignore macro doc comment tests * Correct comment * Fix edge-case in deployment order --- Cargo.lock | 15 +- Cargo.toml | 1 + assets/test_metadata.json | 326 ++++++++++++++++ crates/core/Cargo.toml | 1 + crates/core/src/common.rs | 73 ++++ crates/core/src/driver/mod.rs | 676 +++++++++++++++------------------ crates/core/src/lib.rs | 3 +- crates/format/src/case.rs | 8 +- crates/format/src/input.rs | 257 ++++++++----- crates/format/src/lib.rs | 1 + crates/format/src/macros.rs | 106 ++++++ crates/format/src/metadata.rs | 197 ++++++++-- crates/node/src/kitchensink.rs | 9 +- 13 files changed, 1160 insertions(+), 513 deletions(-) create mode 100644 assets/test_metadata.json create mode 100644 crates/core/src/common.rs create mode 100644 crates/format/src/macros.rs diff --git a/Cargo.lock b/Cargo.lock index a8c4f41..bedf2a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -336,7 +336,7 @@ dependencies = [ "derive_more 2.0.1", "foldhash", "hashbrown 0.15.3", - "indexmap 2.9.0", + "indexmap 2.10.0", "itoa", "k256", "keccak-asm", @@ -597,7 +597,7 @@ dependencies = [ "alloy-sol-macro-input", "const-hex", "heck", - "indexmap 2.9.0", + "indexmap 2.10.0", "proc-macro-error2", "proc-macro2", "quote", @@ -2400,7 +2400,7 @@ dependencies = [ "futures-core", "futures-sink", "http", - "indexmap 2.9.0", + "indexmap 2.10.0", "slab", "tokio", "tokio-util", @@ -2842,9 +2842,9 @@ dependencies = [ [[package]] name = "indexmap" -version = "2.9.0" +version = "2.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cea70ddb795996207ad57735b50c5982d8844f38ba9ee5f1aedcfb708a2aa11e" +checksum = "fe4cd85333e22411419a0bcae1297d25e58c9443848b11dc6a86fefe8c78a661" dependencies = [ "equivalent", "hashbrown 0.15.3", @@ -3962,6 +3962,7 @@ dependencies = [ "alloy", "anyhow", "clap", + "indexmap 2.10.0", "rayon", "revive-dt-compiler", "revive-dt-config", @@ -4507,7 +4508,7 @@ dependencies = [ "chrono", "hex", "indexmap 1.9.3", - "indexmap 2.9.0", + "indexmap 2.10.0", "serde", "serde_derive", "serde_json", @@ -5396,7 +5397,7 @@ version = "0.22.26" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "310068873db2c5b3e7659d2cc35d21855dbafa50d1ce336397c666e3cb08137e" dependencies = [ - "indexmap 2.9.0", + "indexmap 2.10.0", "serde", "serde_spanned", "toml_datetime", diff --git a/Cargo.toml b/Cargo.toml index 9f3c875..ac0bce5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,6 +51,7 @@ tracing-subscriber = { version = "0.3.19", default-features = false, features = "json", "env-filter", ] } +indexmap = { version = "2.10.0", default-features = false } # revive compiler revive-solc-json-interface = { git = "https://github.com/paritytech/revive", rev = "3389865af7c3ff6f29a586d82157e8bc573c1a8e" } diff --git a/assets/test_metadata.json b/assets/test_metadata.json new file mode 100644 index 0000000..6d54584 --- /dev/null +++ b/assets/test_metadata.json @@ -0,0 +1,326 @@ +{ + "modes": [ + "Y >=0.8.9", + "E", + "I" + ], + "cases": [ + { + "name": "first", + "inputs": [ + { + "instance": "WBTC_1", + "method": "#deployer", + "calldata": [ + "0x40", + "0x80", + "4", + "0x5742544300000000000000000000000000000000000000000000000000000000", + "14", + "0x5772617070656420425443000000000000000000000000000000000000000000" + ], + "expected": [ + "WBTC_1.address" + ] + }, + { + "instance": "WBTC_2", + "method": "#deployer", + "calldata": [ + "0x40", + "0x80", + "4", + "0x5742544300000000000000000000000000000000000000000000000000000000", + "14", + "0x5772617070656420425443000000000000000000000000000000000000000000" + ], + "expected": [ + "WBTC_2.address" + ] + }, + { + "instance": "Mooniswap", + "method": "#deployer", + "calldata": [ + "0x0000000000000000000000000000000000000000000000000000000000000060", + "0x00000000000000000000000000000000000000000000000000000000000000c0", + "0x0000000000000000000000000000000000000000000000000000000000000100", + "0x0000000000000000000000000000000000000000000000000000000000000002", + "WBTC_1.address", + "WBTC_2.address", + "4", + "0x5742544300000000000000000000000000000000000000000000000000000000", + "14", + "0x5772617070656420425443000000000000000000000000000000000000000000" + ], + "expected": { + "return_data": [ + "Mooniswap.address" + ], + "events": [ + { + "topics": [ + "0x8be0079c531659141344cd1fd0a4f28419497f9722a3daafe3b4186f6b6457e0", + "0x0000000000000000000000000000000000000000000000000000000000000000", + "0xdeadbeef01000000000000000000000000000000" + ], + "values": [] + } + ], + "exception": false + } + }, + { + "instance": "WBTC_1", + "method": "_mint", + "calldata": [ + "0xdeadbeef00000000000000000000000000000042", + "1000000000" + ], + "expected": { + "return_data": [], + "events": [ + { + "topics": [ + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + "0x0000000000000000000000000000000000000000000000000000000000000000", + "0xdeadbeef00000000000000000000000000000042" + ], + "values": [ + "1000000000" + ] + } + ], + "exception": false + } + }, + { + "instance": "WBTC_2", + "method": "_mint", + "calldata": [ + "0xdeadbeef00000000000000000000000000000042", + "1000000000" + ], + "expected": { + "return_data": [], + "events": [ + { + "topics": [ + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + "0x0000000000000000000000000000000000000000000000000000000000000000", + "0xdeadbeef00000000000000000000000000000042" + ], + "values": [ + "1000000000" + ] + } + ], + "exception": false + } + }, + { + "instance": "WBTC_1", + "caller": "0xdeadbeef00000000000000000000000000000042", + "method": "approve", + "calldata": [ + "Mooniswap.address", + "500000000" + ], + "expected": { + "return_data": [ + "0x0000000000000000000000000000000000000000000000000000000000000001" + ], + "events": [ + { + "topics": [ + "0x8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925", + "0xdeadbeef00000000000000000000000000000042", + "Mooniswap.address" + ], + "values": [ + "500000000" + ] + } + ], + "exception": false + } + }, + { + "instance": "WBTC_2", + "caller": "0xdeadbeef00000000000000000000000000000042", + "method": "approve", + "calldata": [ + "Mooniswap.address", + "500000000" + ], + "expected": { + "return_data": [ + "0x0000000000000000000000000000000000000000000000000000000000000001" + ], + "events": [ + { + "topics": [ + "0x8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925", + "0xdeadbeef00000000000000000000000000000042", + "Mooniswap.address" + ], + "values": [ + "500000000" + ] + } + ], + "exception": false + } + }, + { + "instance": "Mooniswap", + "caller": "0xdeadbeef00000000000000000000000000000042", + "method": "deposit", + "calldata": [ + "0x0000000000000000000000000000000000000000000000000000000000000040", + "0x00000000000000000000000000000000000000000000000000000000000000a0", + "0x0000000000000000000000000000000000000000000000000000000000000002", + "10000000", + "10000000", + "0x0000000000000000000000000000000000000000000000000000000000000002", + "1000000", + "1000000" + ], + "expected": { + "return_data": [ + "10000000" + ], + "events": [ + { + "topics": [ + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + "0x0000000000000000000000000000000000000000000000000000000000000000", + "Mooniswap.address" + ], + "values": [ + "1000" + ] + }, + { + "topics": [ + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + "0xdeadbeef00000000000000000000000000000042", + "Mooniswap.address" + ], + "values": [ + "10000000" + ] + }, + { + "topics": [ + "0x8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925", + "0xdeadbeef00000000000000000000000000000042", + "Mooniswap.address" + ], + "values": [ + "490000000" + ] + }, + { + "topics": [ + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + "0xdeadbeef00000000000000000000000000000042", + "Mooniswap.address" + ], + "values": [ + "10000000" + ] + }, + { + "topics": [ + "0x8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925", + "0xdeadbeef00000000000000000000000000000042", + "Mooniswap.address" + ], + "values": [ + "490000000" + ] + }, + { + "topics": [ + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + "0x0000000000000000000000000000000000000000000000000000000000000000", + "0xdeadbeef00000000000000000000000000000042" + ], + "values": [ + "10000000" + ] + }, + { + "topics": [ + "0x2da466a7b24304f47e87fa2e1e5a81b9831ce54fec19055ce277ca2f39ba42c4", + "0xdeadbeef00000000000000000000000000000042" + ], + "values": [ + "10000000" + ] + } + ], + "exception": false + } + }, + { + "instance": "Mooniswap", + "caller": "0xdeadbeef00000000000000000000000000000042", + "method": "swap", + "calldata": [ + "WBTC_1.address", + "WBTC_2.address", + "5000", + "5000", + "0" + ] + } + ], + "expected": { + "return_data": [ + "5000" + ], + "events": [ + { + "topics": [ + "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef", + "0xdeadbeef00000000000000000000000000000042", + "Mooniswap.address" + ], + "values": [ + "5000" + ] + }, + { + "topics": [ + "0x8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925", + "0xdeadbeef00000000000000000000000000000042", + "Mooniswap.address" + ], + "values": [ + "489995000" + ] + } + ], + "exception": false + } + } + ], + "contracts": { + "Mooniswap": "Mooniswap.sol:Mooniswap", + "WBTC_1": "ERC20/ERC20.sol:ERC20", + "WBTC_2": "ERC20/ERC20.sol:ERC20", + "VirtualBalance": "Mooniswap.sol:VirtualBalance", + "Math": "math/Math.sol:Math" + }, + "libraries": { + "Mooniswap.sol": { + "VirtualBalance": "VirtualBalance" + }, + "math/Math.sol": { + "Math": "Math" + } + }, + "group": "Real life" +} \ No newline at end of file diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index cf93997..89fe7b9 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -23,6 +23,7 @@ revive-dt-report = { workspace = true } alloy = { workspace = true } anyhow = { workspace = true } clap = { workspace = true } +indexmap = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true } rayon = { workspace = true } diff --git a/crates/core/src/common.rs b/crates/core/src/common.rs new file mode 100644 index 0000000..1892c5f --- /dev/null +++ b/crates/core/src/common.rs @@ -0,0 +1,73 @@ +use std::{borrow::Cow, collections::HashSet, 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` +pub struct FilesWithExtensionIterator { + /// The set of allowed extensions that that match the requirement and that should be returned + /// when found. + allowed_extensions: HashSet>, + + /// The set of directories to visit next. This iterator does BFS and so these directories will + /// only be visited if we can't find any files in our state. + directories_to_search: Vec, + + /// The set of files matching the allowed extensions that were found. If there are entries in + /// this vector then they will be returned when the [`Iterator::next`] method is called. If not + /// then we visit one of the next directories to visit. + files_matching_allowed_extensions: Vec, +} + +impl FilesWithExtensionIterator { + pub fn new(root_directory: PathBuf) -> Self { + Self { + allowed_extensions: Default::default(), + directories_to_search: vec![root_directory], + files_matching_allowed_extensions: Default::default(), + } + } + + pub fn with_allowed_extension( + mut self, + allowed_extension: impl Into>, + ) -> Self { + self.allowed_extensions.insert(allowed_extension.into()); + self + } +} + +impl Iterator for FilesWithExtensionIterator { + type Item = PathBuf; + + fn next(&mut self) -> Option { + if let Some(file_path) = self.files_matching_allowed_extensions.pop() { + return Some(file_path); + }; + + let directory_to_search = self.directories_to_search.pop()?; + + // Read all of the entries in the directory. If we failed to read this dir's entires then we + // elect to just ignore it and look in the next directory, we do that by calling the next + // method again on the iterator, which is an intentional decision that we made here instead + // of panicking. + let Ok(dir_entries) = std::fs::read_dir(directory_to_search) else { + return self.next(); + }; + + for entry in dir_entries.flatten() { + let entry_path = entry.path(); + if entry_path.is_dir() { + self.directories_to_search.push(entry_path) + } else if entry_path.is_file() + && entry_path.extension().is_some_and(|ext| { + self.allowed_extensions + .iter() + .any(|allowed| ext.eq_ignore_ascii_case(allowed.as_ref())) + }) + { + self.files_matching_allowed_extensions.push(entry_path) + } + } + + self.next() + } +} diff --git a/crates/core/src/driver/mod.rs b/crates/core/src/driver/mod.rs index cca0be0..99bab18 100644 --- a/crates/core/src/driver/mod.rs +++ b/crates/core/src/driver/mod.rs @@ -1,39 +1,58 @@ //! The test driver handles the compilation and execution of the test cases. +use std::collections::HashMap; +use std::marker::PhantomData; + use alloy::json_abi::JsonAbi; use alloy::network::{Ethereum, TransactionBuilder}; use alloy::rpc::types::TransactionReceipt; use alloy::rpc::types::trace::geth::GethTrace; use alloy::{ - primitives::{Address, map::HashMap}, + primitives::Address, rpc::types::{ TransactionRequest, trace::geth::{AccountState, DiffMode}, }, }; -use revive_dt_compiler::{Compiler, CompilerInput, SolidityCompiler}; +use anyhow::Context; +use indexmap::IndexMap; +use revive_dt_compiler::{Compiler, SolidityCompiler}; use revive_dt_config::Arguments; +use revive_dt_format::case::CaseIdx; +use revive_dt_format::input::Method; +use revive_dt_format::metadata::{ContractInstance, ContractPathAndIdentifier}; use revive_dt_format::{input::Input, metadata::Metadata, mode::SolcMode}; use revive_dt_node_interaction::EthereumNode; use revive_dt_report::reporter::{CompilationTask, Report, Span}; use revive_solc_json_interface::SolcStandardJsonOutput; use serde_json::Value; -use std::collections::HashMap as StdHashMap; use std::fmt::Debug; use crate::Platform; - -type Contracts = HashMap< - CompilerInput<<::Compiler as SolidityCompiler>::Options>, - SolcStandardJsonOutput, ->; +use crate::common::*; pub struct State<'a, T: Platform> { + /// The configuration that the framework was started with. + /// + /// This is currently used to get certain information from it such as the solc mode and other + /// information used at runtime. config: &'a Arguments, + + /// The [`Span`] used in reporting. span: Span, - contracts: Contracts, - deployed_contracts: StdHashMap, - deployed_abis: StdHashMap, + + /// A vector of all of the compiled contracts. Each call to [`build_contracts`] adds a new entry + /// to this vector. + /// + /// [`build_contracts`]: State::build_contracts + contracts: Vec, + + /// This map stores the contracts deployments that have been made for each case within a + /// metadata file. Note, this means that the state can't be reused between different metadata + /// files. + deployed_contracts: HashMap>, + + phantom: PhantomData, } impl<'a, T> State<'a, T> @@ -46,7 +65,7 @@ where span, contracts: Default::default(), deployed_contracts: Default::default(), - deployed_abis: Default::default(), + phantom: Default::default(), } } @@ -90,9 +109,9 @@ where Ok(output) => { task.json_output = Some(output.output.clone()); task.error = output.error; - self.contracts.insert(output.input, output.output); + self.contracts.push(output.output); - if let Some(last_output) = self.contracts.values().last() { + if let Some(last_output) = self.contracts.last() { if let Some(contracts) = &last_output.contracts { for (file, contracts_map) in contracts { for contract_name in contracts_map.keys() { @@ -117,63 +136,230 @@ where } } - pub fn execute_input( + pub fn handle_input( &mut self, + metadata: &Metadata, + case_idx: CaseIdx, input: &Input, node: &T::Blockchain, + ) -> anyhow::Result<(TransactionReceipt, GethTrace, DiffMode)> { + let deployment_receipts = + self.handle_contract_deployment(metadata, case_idx, input, node)?; + self.handle_input_execution(case_idx, input, deployment_receipts, node) + } + + /// Handles the contract deployment for a given input performing it if it needs to be performed. + fn handle_contract_deployment( + &mut self, + metadata: &Metadata, + case_idx: CaseIdx, + input: &Input, + node: &T::Blockchain, + ) -> anyhow::Result> { + let span = tracing::debug_span!( + "Handling contract deployment", + ?case_idx, + instance = ?input.instance + ); + let _guard = span.enter(); + + let mut instances_we_must_deploy = IndexMap::::new(); + for instance in input.find_all_contract_instances().into_iter() { + if !self + .deployed_contracts + .entry(case_idx) + .or_default() + .contains_key(&instance) + { + instances_we_must_deploy.entry(instance).or_insert(false); + } + } + if let Method::Deployer = input.method { + instances_we_must_deploy.swap_remove(&input.instance); + instances_we_must_deploy.insert(input.instance.clone(), true); + } + + tracing::debug!( + instances_to_deploy = instances_we_must_deploy.len(), + "Computed the number of required deployments for input" + ); + + let mut receipts = HashMap::new(); + for (instance, deploy_with_constructor_arguments) in instances_we_must_deploy.into_iter() { + // What we have at this moment is just a contract instance which is kind of like a variable + // name for an actual underlying contract. So, we need to resolve this instance to the info + // of the contract that it belongs to. + let Some(ContractPathAndIdentifier { + contract_source_path, + contract_ident, + }) = metadata.contract_sources()?.remove(&instance) + else { + tracing::error!("Contract source not found for instance"); + anyhow::bail!("Contract source not found for instance {:?}", instance) + }; + + let compiled_contract = self.contracts.iter().find_map(|output| { + output + .contracts + .as_ref()? + .get(&contract_source_path.display().to_string()) + .and_then(|source_file_contracts| { + source_file_contracts.get(contract_ident.as_ref()) + }) + }); + let Some(code) = compiled_contract + .and_then(|contract| contract.evm.as_ref().and_then(|evm| evm.bytecode.as_ref())) + else { + tracing::error!( + contract_source_path = contract_source_path.display().to_string(), + contract_ident = contract_ident.as_ref(), + "Failed to find bytecode for contract" + ); + anyhow::bail!("Failed to find bytecode for contract {:?}", instance) + }; + + // TODO: When we want to do linking it would be best to do it at this stage here. We have + // the context from the metadata files and therefore know what needs to be linked and in + // what order it needs to happen. + + let mut code = match alloy::hex::decode(&code.object) { + Ok(code) => code, + Err(error) => { + tracing::error!( + ?error, + contract_source_path = contract_source_path.display().to_string(), + contract_ident = contract_ident.as_ref(), + "Failed to hex-decode byte code - This could possibly mean that the bytecode requires linking" + ); + anyhow::bail!("Failed to hex-decode the byte code {}", error) + } + }; + + if deploy_with_constructor_arguments { + let encoded_input = input + .encoded_input(self.deployed_contracts.entry(case_idx).or_default(), node)?; + code.extend(encoded_input.to_vec()); + } + + let tx = { + let tx = TransactionRequest::default().from(input.caller); + TransactionBuilder::::with_deploy_code(tx, code) + }; + + let receipt = match node.execute_transaction(tx) { + Ok(receipt) => receipt, + Err(error) => { + tracing::error!( + node = std::any::type_name::(), + ?error, + "Contract deployment transaction failed." + ); + return Err(error); + } + }; + + let Some(address) = receipt.contract_address else { + tracing::error!("Contract deployment transaction didn't return an address"); + anyhow::bail!("Contract deployment didn't return an address"); + }; + tracing::info!( + instance_name = ?instance, + instance_address = ?address, + "Deployed contract" + ); + + let Some(Value::String(metadata)) = + compiled_contract.and_then(|contract| contract.metadata.as_ref()) + else { + tracing::error!("Contract does not have a metadata field"); + anyhow::bail!("Contract does not have a metadata field"); + }; + + let Ok(metadata) = serde_json::from_str::(metadata) else { + tracing::error!(%metadata, "Failed to parse solc metadata into a structured value"); + anyhow::bail!("Failed to parse solc metadata into a structured value {metadata}"); + }; + + let Some(abi) = metadata.get("output").and_then(|value| value.get("abi")) else { + tracing::error!(%metadata, "Failed to access the .output.abi field of the solc metadata"); + anyhow::bail!( + "Failed to access the .output.abi field of the solc metadata {metadata}" + ); + }; + + let Ok(abi) = serde_json::from_value::(abi.clone()) else { + tracing::error!(%metadata, "Failed to deserialize ABI into a structured format"); + anyhow::bail!("Failed to deserialize ABI into a structured format {metadata}"); + }; + + self.deployed_contracts + .entry(case_idx) + .or_default() + .insert(instance.clone(), (address, abi)); + + receipts.insert(instance.clone(), receipt); + } + + Ok(receipts) + } + + /// Handles the execution of the input in terms of the calls that need to be made. + fn handle_input_execution( + &mut self, + case_idx: CaseIdx, + input: &Input, + deployment_receipts: HashMap, + node: &T::Blockchain, ) -> anyhow::Result<(TransactionReceipt, GethTrace, DiffMode)> { tracing::trace!("Calling execute_input for input: {input:?}"); - let nonce = node.fetch_add_nonce(input.caller)?; + let receipt = match input.method { + // This input was already executed when `handle_input` was called. We just need to + // lookup the transaction receipt in this case and continue on. + Method::Deployer => deployment_receipts + .get(&input.instance) + .context("Failed to find deployment receipt")? + .clone(), + Method::Fallback | Method::FunctionName(_) => { + let tx = match input + .legacy_transaction(self.deployed_contracts.entry(case_idx).or_default(), node) + { + Ok(tx) => { + tracing::debug!("Legacy transaction data: {tx:#?}"); + tx + } + Err(err) => { + tracing::error!("Failed to construct legacy transaction: {err:?}"); + return Err(err); + } + }; - tracing::debug!( - "Nonce calculated on the execute contract, calculated nonce {}, for contract {}, having address {} on node: {}", - &nonce, - &input.instance, - &input.caller, - std::any::type_name::() - ); + tracing::trace!("Executing transaction for input: {input:?}"); - let tx = match input.legacy_transaction( - nonce, - &self.deployed_contracts, - &self.deployed_abis, - node, - ) { - Ok(tx) => { - tracing::debug!("Legacy transaction data: {tx:#?}"); - tx - } - Err(err) => { - tracing::error!("Failed to construct legacy transaction: {err:?}"); - return Err(err); - } - }; - - tracing::trace!("Executing transaction for input: {input:?}"); - - let receipt = match node.execute_transaction(tx) { - Ok(receipt) => receipt, - Err(err) => { - tracing::error!( - "Failed to execute transaction when executing the contract: {}, {:?}", - &input.instance, - err - ); - return Err(err); + match node.execute_transaction(tx) { + Ok(receipt) => receipt, + Err(err) => { + tracing::error!( + "Failed to execute transaction when executing the contract: {}, {:?}", + &*input.instance, + err + ); + return Err(err); + } + } } }; tracing::trace!( "Transaction receipt for executed contract: {} - {:?}", - &input.instance, + &*input.instance, receipt, ); let trace = node.trace_transaction(receipt.clone())?; tracing::trace!( "Trace result for contract: {} - {:?}", - &input.instance, + &*input.instance, trace ); @@ -181,175 +367,6 @@ where Ok((receipt, trace, diff)) } - - pub fn deploy_contracts(&mut self, input: &Input, node: &T::Blockchain) -> anyhow::Result<()> { - let tracing_span = tracing::debug_span!( - "Deploying contracts", - ?input, - node = std::any::type_name::() - ); - let _guard = tracing_span.enter(); - - tracing::debug!(number_of_contracts_to_deploy = self.contracts.len()); - - for output in self.contracts.values() { - let Some(contract_map) = &output.contracts else { - tracing::debug!( - "No contracts in output — skipping deployment for this input {}", - &input.instance - ); - continue; - }; - - for contracts in contract_map.values() { - for (contract_name, contract) in contracts { - let tracing_span = tracing::info_span!("Deploying contract", contract_name); - let _guard = tracing_span.enter(); - - tracing::debug!( - "Contract name is: {:?} and the input name is: {:?}", - &contract_name, - &input.instance - ); - - let bytecode = contract - .evm - .as_ref() - .and_then(|evm| evm.bytecode.as_ref()) - .map(|b| b.object.clone()); - - let Some(code) = bytecode else { - tracing::error!("no bytecode for contract {contract_name}"); - continue; - }; - - let nonce = match node.fetch_add_nonce(input.caller) { - Ok(nonce) => nonce, - Err(error) => { - tracing::error!( - caller = ?input.caller, - ?error, - "Failed to get the nonce for the caller" - ); - return Err(error); - } - }; - - tracing::debug!( - "Calculated nonce {}, for contract {}, having address {} on node: {}", - &nonce, - &input.instance, - &input.caller, - std::any::type_name::() - ); - - // We are using alloy for building and submitting the transactions and it will - // automatically fill in all of the missing fields from the provider that we - // are using. - let code = match alloy::hex::decode(&code) { - Ok(code) => code, - Err(error) => { - tracing::error!( - code, - ?error, - "Failed to hex-decode the code of the contract. (This could possibly mean that it contains '_' and therefore it requires linking to be performed)" - ); - return Err(error.into()); - } - }; - let tx = { - let tx = TransactionRequest::default() - .nonce(nonce) - .from(input.caller); - TransactionBuilder::::with_deploy_code(tx, code) - }; - - let receipt = match node.execute_transaction(tx) { - Ok(receipt) => receipt, - Err(err) => { - tracing::error!( - "Failed to execute transaction when deploying the contract on node : {:?}, {:?}, {:?}", - std::any::type_name::(), - &contract_name, - err - ); - return Err(err); - } - }; - - tracing::debug!( - "Deployment tx sent for {} with nonce {} → tx hash: {:?}, on node: {:?}", - contract_name, - nonce, - receipt.transaction_hash, - std::any::type_name::(), - ); - - tracing::trace!( - "Deployed transaction receipt for contract: {} - {:?}, on node: {:?}", - &contract_name, - receipt, - std::any::type_name::(), - ); - - let Some(address) = receipt.contract_address else { - tracing::error!( - "contract {contract_name} deployment did not return an address" - ); - continue; - }; - - self.deployed_contracts - .insert(contract_name.clone(), address); - tracing::trace!( - "deployed contract `{}` at {:?}, on node {:?}", - contract_name, - address, - std::any::type_name::() - ); - - let Some(Value::String(metadata)) = &contract.metadata else { - tracing::error!(?contract, "Contract does not have a metadata field"); - anyhow::bail!("Contract does not have a metadata field: {contract:?}"); - }; - - // Deserialize the solc metadata into a JSON object so we can get the ABI of the - // contracts. If we fail to perform the deserialization then we return an error - // as there's no other way to handle this. - let Ok(metadata) = serde_json::from_str::(metadata) else { - tracing::error!(%metadata, "Failed to parse solc metadata into a structured value"); - anyhow::bail!( - "Failed to parse solc metadata into a structured value {metadata}" - ); - }; - - // Accessing the ABI on the solc metadata and erroring if the accessing failed - let Some(abi) = metadata.get("output").and_then(|value| value.get("abi")) - else { - tracing::error!(%metadata, "Failed to access the .output.abi field of the solc metadata"); - anyhow::bail!( - "Failed to access the .output.abi field of the solc metadata {metadata}" - ); - }; - - // Deserialize the ABI object that we got from the unstructured JSON into a - // structured ABI object and error out if we fail. - let Ok(abi) = serde_json::from_value::(abi.clone()) else { - tracing::error!(%metadata, "Failed to deserialize ABI into a structured format"); - anyhow::bail!( - "Failed to deserialize ABI into a structured format {metadata}" - ); - }; - - self.deployed_abis.insert(contract_name.clone(), abi); - } - } - } - - tracing::debug!("Available contracts: {:?}", self.deployed_contracts.keys()); - - Ok(()) - } } pub struct Driver<'a, Leader: Platform, Follower: Platform> { @@ -484,87 +501,70 @@ where ); let _guard = tracing_span.enter(); + let case_idx = CaseIdx::new_from(case_idx); + // For inputs if one of the inputs fail we move on to the next case (we do not move // on to the next input as it doesn't make sense. It depends on the previous one). for (input_idx, input) in case.inputs.iter().enumerate() { let tracing_span = tracing::info_span!("Handling input", input_idx); let _guard = tracing_span.enter(); - // TODO: verify if this is correct, I doubt that we need to do contract redeploy - // for each input. It doesn't quite look to be correct but we need to cross - // check with the matterlabs implementation. This matches our implementation but - // I have doubts around its correctness. - let deployment_result = tracing::info_span!( - "Deploying contracts", - contract_name = input.instance - ) - .in_scope(|| { - if let Err(error) = leader_state.deploy_contracts(input, self.leader_node) { - tracing::error!(target = ?Target::Leader, ?error, "Contract deployment failed"); - execution_result.add_failed_case( - Target::Leader, - mode.clone(), - case.name.clone().unwrap_or("no case name".to_owned()), - case_idx, - input_idx, - anyhow::Error::msg( - format!("Failed to deploy contracts, {error}") - ) - ); - return Err(error) - }; - if let Err(error) = - follower_state.deploy_contracts(input, self.follower_node) - { - tracing::error!(target = ?Target::Follower, ?error, "Contract deployment failed"); - execution_result.add_failed_case( - Target::Follower, - mode.clone(), - case.name.clone().unwrap_or("no case name".to_owned()), - case_idx, - input_idx, - anyhow::Error::msg( - format!("Failed to deploy contracts, {error}") - ) - ); - return Err(error) - }; - Ok(()) - }); - if deployment_result.is_err() { - // Noting it again here: if something in the input fails we do not move on - // to the next input, we move to the next case completely. - continue 'case_loop; - } - let execution_result = - tracing::info_span!("Executing input", contract_name = input.instance) + tracing::info_span!("Executing input", contract_name = ?input.instance) .in_scope(|| { - let (leader_receipt, _, leader_diff) = - match leader_state.execute_input(input, self.leader_node) { - Ok(result) => result, - Err(error) => { - tracing::error!( - target = ?Target::Leader, - ?error, - "Contract execution failed" - ); - return Err(error); - } - }; + let (leader_receipt, _, leader_diff) = match leader_state + .handle_input(self.metadata, case_idx, input, self.leader_node) + { + Ok(result) => result, + Err(error) => { + tracing::error!( + target = ?Target::Leader, + ?error, + "Contract execution failed" + ); + execution_result.add_failed_case( + Target::Leader, + mode.clone(), + case.name + .as_deref() + .unwrap_or("no case name") + .to_owned(), + case_idx, + input_idx, + anyhow::Error::msg(format!("{error}")), + ); + return Err(error); + } + }; - let (follower_receipt, _, follower_diff) = - match follower_state.execute_input(input, self.follower_node) { - Ok(result) => result, - Err(error) => { - tracing::error!( - target = ?Target::Follower, - ?error, - "Contract execution failed" - ); - return Err(error); - } - }; + let (follower_receipt, _, follower_diff) = match follower_state + .handle_input( + self.metadata, + case_idx, + input, + self.follower_node, + ) { + Ok(result) => result, + Err(error) => { + tracing::error!( + target = ?Target::Follower, + ?error, + "Contract execution failed" + ); + execution_result.add_failed_case( + Target::Follower, + mode.clone(), + case.name + .as_deref() + .unwrap_or("no case name") + .to_owned(), + case_idx, + input_idx, + anyhow::Error::msg(format!("{error}")), + ); + return Err(error); + } + }; Ok((leader_receipt, leader_diff, follower_receipt, follower_diff)) }); @@ -654,7 +654,7 @@ impl ExecutionResult { target: Target, solc_mode: SolcMode, case_name: String, - case_idx: usize, + case_idx: CaseIdx, ) { self.successful_cases_count += 1; self.results.push(Box::new(CaseResult::Success { @@ -670,7 +670,7 @@ impl ExecutionResult { target: Target, solc_mode: SolcMode, case_name: String, - case_idx: usize, + case_idx: CaseIdx, input_idx: usize, error: anyhow::Error, ) { @@ -702,7 +702,7 @@ pub trait ExecutionResultItem: Debug { /// Provides information on the case name and number that this result item pertains to. This is /// [`None`] if the error doesn't belong to any case (e.g., if it's a build error outside of any /// of the cases.). - fn case_name_and_index(&self) -> Option<(&str, usize)>; + fn case_name_and_index(&self) -> Option<(&str, &CaseIdx)>; /// Provides information on the input number that this result item pertains to. This is [`None`] /// if the error doesn't belong to any input (e.g., if it's a build error outside of any of the @@ -756,7 +756,7 @@ impl ExecutionResultItem for BuildResult { } } - fn case_name_and_index(&self) -> Option<(&str, usize)> { + fn case_name_and_index(&self) -> Option<(&str, &CaseIdx)> { None } @@ -771,13 +771,13 @@ pub enum CaseResult { target: Target, solc_mode: SolcMode, case_name: String, - case_idx: usize, + case_idx: CaseIdx, }, Failure { target: Target, solc_mode: SolcMode, case_name: String, - case_idx: usize, + case_idx: CaseIdx, input_idx: usize, error: anyhow::Error, }, @@ -810,7 +810,7 @@ impl ExecutionResultItem for CaseResult { } } - fn case_name_and_index(&self) -> Option<(&str, usize)> { + fn case_name_and_index(&self) -> Option<(&str, &CaseIdx)> { match self { Self::Success { case_name, @@ -821,7 +821,7 @@ impl ExecutionResultItem for CaseResult { case_name, case_idx, .. - } => Some((case_name, *case_idx)), + } => Some((case_name, case_idx)), } } @@ -832,77 +832,3 @@ impl ExecutionResultItem for CaseResult { } } } - -/// 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` -struct FilesWithExtensionIterator { - /// The set of allowed extensions that that match the requirement and that should be returned - /// when found. - allowed_extensions: std::collections::HashSet>, - - /// The set of directories to visit next. This iterator does BFS and so these directories will - /// only be visited if we can't find any files in our state. - directories_to_search: Vec, - - /// The set of files matching the allowed extensions that were found. If there are entries in - /// this vector then they will be returned when the [`Iterator::next`] method is called. If not - /// then we visit one of the next directories to visit. - /// - /// [`Iterator`]: std::iter::Iterator - files_matching_allowed_extensions: Vec, -} - -impl FilesWithExtensionIterator { - fn new(root_directory: std::path::PathBuf) -> Self { - Self { - allowed_extensions: Default::default(), - directories_to_search: vec![root_directory], - files_matching_allowed_extensions: Default::default(), - } - } - - fn with_allowed_extension( - mut self, - allowed_extension: impl Into>, - ) -> Self { - self.allowed_extensions.insert(allowed_extension.into()); - self - } -} - -impl Iterator for FilesWithExtensionIterator { - type Item = std::path::PathBuf; - - fn next(&mut self) -> Option { - if let Some(file_path) = self.files_matching_allowed_extensions.pop() { - return Some(file_path); - }; - - let directory_to_search = self.directories_to_search.pop()?; - - // Read all of the entries in the directory. If we failed to read this dir's entires then we - // elect to just ignore it and look in the next directory, we do that by calling the next - // method again on the iterator, which is an intentional decision that we made here instead - // of panicking. - let Ok(dir_entries) = std::fs::read_dir(directory_to_search) else { - return self.next(); - }; - - for entry in dir_entries.flatten() { - let entry_path = entry.path(); - if entry_path.is_dir() { - self.directories_to_search.push(entry_path) - } else if entry_path.is_file() - && entry_path.extension().is_some_and(|ext| { - self.allowed_extensions - .iter() - .any(|allowed| ext.eq_ignore_ascii_case(allowed.as_ref())) - }) - { - self.files_matching_allowed_extensions.push(entry_path) - } - } - - self.next() - } -} diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index 52b8b37..e9bcdd1 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -1,13 +1,14 @@ //! The revive differential testing core library. //! //! This crate defines the testing configuration and -//! provides a helper utilty to execute tests. +//! provides a helper utility to execute tests. use revive_dt_compiler::{SolidityCompiler, revive_resolc, solc}; use revive_dt_config::TestingPlatform; use revive_dt_node::{geth, kitchensink::KitchensinkNode}; use revive_dt_node_interaction::EthereumNode; +pub mod common; pub mod driver; /// One platform can be tested differentially against another. diff --git a/crates/format/src/case.rs b/crates/format/src/case.rs index 5516406..21b620b 100644 --- a/crates/format/src/case.rs +++ b/crates/format/src/case.rs @@ -1,6 +1,6 @@ use serde::Deserialize; -use crate::{input::Input, mode::Mode}; +use crate::{define_wrapper_type, input::Input, mode::Mode}; #[derive(Debug, Default, Deserialize, Clone, Eq, PartialEq)] pub struct Case { @@ -10,3 +10,9 @@ pub struct Case { pub inputs: Vec, pub group: Option, } + +define_wrapper_type!( + /// A wrapper type for the index of test cases found in metadata file. + #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] + CaseIdx(usize); +); diff --git a/crates/format/src/input.rs b/crates/format/src/input.rs index 1cdefe4..31cee43 100644 --- a/crates/format/src/input.rs +++ b/crates/format/src/input.rs @@ -13,13 +13,15 @@ use serde_json::Value; use revive_dt_node_interaction::EthereumNode; +use crate::metadata::ContractInstance; + #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq)] pub struct Input { #[serde(default = "default_caller")] pub caller: Address, pub comment: Option, #[serde(default = "default_instance")] - pub instance: String, + pub instance: ContractInstance, pub method: Method, pub calldata: Option, pub expected: Option, @@ -71,109 +73,138 @@ pub enum Method { FunctionName(String), } +impl Calldata { + pub fn find_all_contract_instances(&self, vec: &mut Vec) { + if let Calldata::Compound(compound) = self { + for item in compound { + if let Some(instance) = item.strip_suffix(".address") { + vec.push(ContractInstance::new_from(instance)) + } + } + } + } +} + +impl ExpectedOutput { + pub fn find_all_contract_instances(&self, vec: &mut Vec) { + if let Some(ref cd) = self.return_data { + cd.find_all_contract_instances(vec); + } + } +} + impl Input { fn instance_to_address( &self, - instance: &str, - deployed_contracts: &HashMap, + instance: &ContractInstance, + deployed_contracts: &HashMap, ) -> anyhow::Result
{ deployed_contracts .get(instance) - .copied() - .ok_or_else(|| anyhow::anyhow!("instance {instance} not deployed")) + .map(|(a, _)| *a) + .ok_or_else(|| anyhow::anyhow!("instance {instance:?} not deployed")) } pub fn encoded_input( &self, - deployed_abis: &HashMap, - deployed_contracts: &HashMap, + deployed_contracts: &HashMap, chain_state_provider: &impl EthereumNode, ) -> anyhow::Result { - let Method::FunctionName(ref function_name) = self.method else { - return Ok(Bytes::default()); // fallback or deployer — no input - }; + match self.method { + Method::Deployer | Method::Fallback => { + let calldata_args = match &self.calldata { + Some(Calldata::Compound(args)) => args, + _ => anyhow::bail!("Expected compound calldata for function call"), + }; - let Some(abi) = deployed_abis.get(&self.instance) else { - tracing::error!( - contract_name = self.instance, - available_abis = ?deployed_abis.keys().collect::>(), - "Attempted to lookup ABI of contract but it wasn't found" - ); - anyhow::bail!("ABI for instance '{}' not found", &self.instance); - }; - - tracing::trace!("ABI found for instance: {}", &self.instance); - - // We follow the same logic that's implemented in the matter-labs-tester where they resolve - // the function name into a function selector and they assume that he function doesn't have - // any existing overloads. - // https://github.com/matter-labs/era-compiler-tester/blob/1dfa7d07cba0734ca97e24704f12dd57f6990c2c/compiler_tester/src/test/case/input/mod.rs#L158-L190 - let function = abi - .functions() - .find(|function| function.name.starts_with(function_name)) - .ok_or_else(|| { - anyhow::anyhow!( - "Function with name {:?} not found in ABI for the instance {:?}", - function_name, - &self.instance - ) - })?; - - tracing::trace!("Functions found for instance: {}", &self.instance); - - let calldata_args = match &self.calldata { - Some(Calldata::Compound(args)) => args, - _ => anyhow::bail!("Expected compound calldata for function call"), - }; - - if calldata_args.len() != function.inputs.len() { - anyhow::bail!( - "Function expects {} args, but got {}", - function.inputs.len(), - calldata_args.len() - ); - } - - tracing::trace!( - "Starting encoding ABI's parameters for instance: {}", - &self.instance - ); - - // Allocating a vector that we will be using for the calldata. The vector size will be: - // 4 bytes for the function selector. - // function.inputs.len() * 32 bytes for the arguments (each argument is a U256). - // - // We're using indices in the following code in order to avoid the need for us to allocate - // a new buffer for each one of the resolved arguments. - let mut calldata = Vec::::with_capacity(4 + calldata_args.len() * 32); - calldata.extend(function.selector().0); - - for (arg_idx, arg) in calldata_args.iter().enumerate() { - match resolve_argument(arg, deployed_contracts, chain_state_provider) { - Ok(resolved) => { - calldata.extend(resolved.to_be_bytes::<32>()); + let mut calldata = Vec::::with_capacity(calldata_args.len() * 32); + for (arg_idx, arg) in calldata_args.iter().enumerate() { + match resolve_argument(arg, deployed_contracts, chain_state_provider) { + Ok(resolved) => { + calldata.extend(resolved.to_be_bytes::<32>()); + } + Err(error) => { + tracing::error!(arg, arg_idx, ?error, "Failed to resolve argument"); + return Err(error); + } + }; } - Err(error) => { - tracing::error!(arg, arg_idx, ?error, "Failed to resolve argument"); - return Err(error); - } - }; - } - Ok(calldata.into()) + Ok(calldata.into()) + } + Method::FunctionName(ref function_name) => { + let Some(abi) = deployed_contracts.get(&self.instance).map(|(_, a)| a) else { + tracing::error!( + contract_name = self.instance.as_ref(), + available_abis = ?deployed_contracts.keys().collect::>(), + "Attempted to lookup ABI of contract but it wasn't found" + ); + anyhow::bail!("ABI for instance '{}' not found", self.instance.as_ref()); + }; + + tracing::trace!("ABI found for instance: {}", &self.instance.as_ref()); + + // We follow the same logic that's implemented in the matter-labs-tester where they resolve + // the function name into a function selector and they assume that he function doesn't have + // any existing overloads. + // https://github.com/matter-labs/era-compiler-tester/blob/1dfa7d07cba0734ca97e24704f12dd57f6990c2c/compiler_tester/src/test/case/input/mod.rs#L158-L190 + let function = abi + .functions() + .find(|function| function.name.starts_with(function_name)) + .ok_or_else(|| { + anyhow::anyhow!( + "Function with name {:?} not found in ABI for the instance {:?}", + function_name, + &self.instance + ) + })?; + + tracing::trace!("Functions found for instance: {}", self.instance.as_ref()); + + let calldata_args = match &self.calldata { + Some(Calldata::Compound(args)) => args, + _ => anyhow::bail!("Expected compound calldata for function call"), + }; + + tracing::trace!( + "Starting encoding ABI's parameters for instance: {}", + self.instance.as_ref() + ); + + // Allocating a vector that we will be using for the calldata. The vector size will be: + // 4 bytes for the function selector. + // function.inputs.len() * 32 bytes for the arguments (each argument is a U256). + // + // We're using indices in the following code in order to avoid the need for us to allocate + // a new buffer for each one of the resolved arguments. + let mut calldata = Vec::::with_capacity(4 + calldata_args.len() * 32); + calldata.extend(function.selector().0); + + for (arg_idx, arg) in calldata_args.iter().enumerate() { + match resolve_argument(arg, deployed_contracts, chain_state_provider) { + Ok(resolved) => { + calldata.extend(resolved.to_be_bytes::<32>()); + } + Err(error) => { + tracing::error!(arg, arg_idx, ?error, "Failed to resolve argument"); + return Err(error); + } + }; + } + + Ok(calldata.into()) + } + } } /// Parse this input into a legacy transaction. pub fn legacy_transaction( &self, - nonce: u64, - deployed_contracts: &HashMap, - deployed_abis: &HashMap, + deployed_contracts: &HashMap, chain_state_provider: &impl EthereumNode, ) -> anyhow::Result { - let input_data = - self.encoded_input(deployed_abis, deployed_contracts, chain_state_provider)?; - let transaction_request = TransactionRequest::default().nonce(nonce); + let input_data = self.encoded_input(deployed_contracts, chain_state_provider)?; + let transaction_request = TransactionRequest::default(); match self.method { Method::Deployer => Ok(transaction_request.with_deploy_code(input_data)), _ => Ok(transaction_request @@ -181,10 +212,35 @@ impl Input { .input(input_data.into())), } } + + pub fn find_all_contract_instances(&self) -> Vec { + let mut vec = Vec::new(); + vec.push(self.instance.clone()); + + if let Some(ref cd) = self.calldata { + cd.find_all_contract_instances(&mut vec); + } + match &self.expected { + Some(Expected::Calldata(cd)) => { + cd.find_all_contract_instances(&mut vec); + } + Some(Expected::Expected(expected)) => { + expected.find_all_contract_instances(&mut vec); + } + Some(Expected::ExpectedMany(expected)) => { + for expected in expected { + expected.find_all_contract_instances(&mut vec); + } + } + None => {} + } + + vec + } } -fn default_instance() -> String { - "Test".to_string() +fn default_instance() -> ContractInstance { + ContractInstance::new_from("Test") } fn default_caller() -> Address { @@ -201,13 +257,14 @@ fn default_caller() -> Address { /// https://github.com/matter-labs/era-compiler-tester/blob/0ed598a27f6eceee7008deab3ff2311075a2ec69/compiler_tester/src/test/case/input/value.rs#L43-L146 fn resolve_argument( value: &str, - deployed_contracts: &HashMap, + deployed_contracts: &HashMap, chain_state_provider: &impl EthereumNode, ) -> anyhow::Result { if let Some(instance) = value.strip_suffix(".address") { Ok(U256::from_be_slice( deployed_contracts - .get(instance) + .get(&ContractInstance::new_from(instance)) + .map(|(a, _)| *a) .ok_or_else(|| anyhow::anyhow!("Instance `{}` not found", instance))? .as_ref(), )) @@ -357,19 +414,19 @@ mod tests { .0; let input = Input { - instance: "Contract".to_string(), + instance: ContractInstance::new_from("Contract"), method: Method::FunctionName("store".to_owned()), calldata: Some(Calldata::Compound(vec!["42".into()])), ..Default::default() }; - let mut deployed_abis = HashMap::new(); - deployed_abis.insert("Contract".to_string(), parsed_abi); - let deployed_contracts = HashMap::new(); + let mut contracts = HashMap::new(); + contracts.insert( + ContractInstance::new_from("Contract"), + (Address::ZERO, parsed_abi), + ); - let encoded = input - .encoded_input(&deployed_abis, &deployed_contracts, &DummyEthereumNode) - .unwrap(); + let encoded = input.encoded_input(&contracts, &DummyEthereumNode).unwrap(); assert!(encoded.0.starts_with(&selector)); type T = (u64,); @@ -399,7 +456,7 @@ mod tests { .0; let input: Input = Input { - instance: "Contract".to_string(), + instance: ContractInstance::new_from("Contract"), method: Method::FunctionName("send".to_owned()), calldata: Some(Calldata::Compound(vec![ "0x1000000000000000000000000000000000000001".to_string(), @@ -407,13 +464,13 @@ mod tests { ..Default::default() }; - let mut abis = HashMap::new(); - abis.insert("Contract".to_string(), parsed_abi); - let contracts = HashMap::new(); + let mut contracts = HashMap::new(); + contracts.insert( + ContractInstance::new_from("Contract"), + (Address::ZERO, parsed_abi), + ); - let encoded = input - .encoded_input(&abis, &contracts, &DummyEthereumNode) - .unwrap(); + let encoded = input.encoded_input(&contracts, &DummyEthereumNode).unwrap(); assert!(encoded.0.starts_with(&selector)); type T = (alloy_primitives::Address,); diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 21ae375..f66d883 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -3,5 +3,6 @@ pub mod case; pub mod corpus; pub mod input; +pub mod macros; pub mod metadata; pub mod mode; diff --git a/crates/format/src/macros.rs b/crates/format/src/macros.rs new file mode 100644 index 0000000..ed67012 --- /dev/null +++ b/crates/format/src/macros.rs @@ -0,0 +1,106 @@ +/// Defines wrappers around types. +/// +/// For example, the macro invocation seen below: +/// +/// ```rust,ignore +/// define_wrapper_type!(CaseId => usize); +/// ``` +/// +/// Would define a wrapper type that looks like the following: +/// +/// ```rust,ignore +/// pub struct CaseId(usize); +/// ``` +/// +/// And would also implement a number of methods on this type making it easier +/// to use. +/// +/// These wrapper types become very useful as they make the code a lot easier +/// to read. +/// +/// Take the following as an example: +/// +/// ```rust,ignore +/// struct State { +/// contracts: HashMap>> +/// } +/// ``` +/// +/// In the above code it's hard to understand what the various types refer to or +/// what to expect them to contain. +/// +/// With these wrapper types we're able to create code that's self-documenting +/// in that the types tell us what the code is referring to. The above code is +/// transformed into +/// +/// ```rust,ignore +/// struct State { +/// contracts: HashMap> +/// } +/// ``` +#[macro_export] +macro_rules! define_wrapper_type { + ( + $(#[$meta: meta])* + $ident: ident($ty: ty) $(;)? + ) => { + $(#[$meta])* + pub struct $ident($ty); + + impl $ident { + pub fn new(value: $ty) -> Self { + Self(value) + } + + pub fn new_from>(value: T) -> Self { + Self(value.into()) + } + + pub fn into_inner(self) -> $ty { + self.0 + } + + pub fn as_inner(&self) -> &$ty { + &self.0 + } + } + + impl AsRef<$ty> for $ident { + fn as_ref(&self) -> &$ty { + &self.0 + } + } + + impl AsMut<$ty> for $ident { + fn as_mut(&mut self) -> &mut $ty { + &mut self.0 + } + } + + impl std::ops::Deref for $ident { + type Target = $ty; + + fn deref(&self) -> &Self::Target { + &self.0 + } + } + + impl std::ops::DerefMut for $ident { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + impl From<$ty> for $ident { + fn from(value: $ty) -> Self { + Self(value) + } + } + + impl From<$ident> for $ty { + fn from(value: $ident) -> Self { + value.0 + } + } + }; +} diff --git a/crates/format/src/metadata.rs b/crates/format/src/metadata.rs index c4b3398..abd4150 100644 --- a/crates/format/src/metadata.rs +++ b/crates/format/src/metadata.rs @@ -1,14 +1,17 @@ use std::{ collections::BTreeMap, + fmt::Display, fs::{File, read_to_string}, ops::Deref, path::{Path, PathBuf}, + str::FromStr, }; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use crate::{ case::Case, + define_wrapper_type, mode::{Mode, SolcMode}, }; @@ -42,7 +45,8 @@ impl Deref for MetadataFile { #[derive(Debug, Default, Deserialize, Clone, Eq, PartialEq)] pub struct Metadata { pub cases: Vec, - pub contracts: Option>, + pub contracts: Option>, + // TODO: Convert into wrapper types for clarity. pub libraries: Option>>, pub ignore: Option, pub modes: Option>, @@ -77,28 +81,35 @@ impl Metadata { .to_path_buf()) } - /// Extract the contract sources. - /// - /// Returns a mapping of contract IDs to their source path and contract name. - pub fn contract_sources(&self) -> anyhow::Result> { + /// Returns the contract sources with canonicalized paths for the files + pub fn contract_sources( + &self, + ) -> anyhow::Result> { let directory = self.directory()?; let mut sources = BTreeMap::new(); let Some(contracts) = &self.contracts else { return Ok(sources); }; - for (id, contract) in contracts { - // TODO: broken if a colon is in the dir name.. - let mut parts = contract.split(':'); - let (Some(file_name), Some(contract_name)) = (parts.next(), parts.next()) else { - anyhow::bail!("metadata contains invalid contract: {contract}"); - }; - let file = directory.to_path_buf().join(file_name); - if !file.is_file() { - anyhow::bail!("contract {id} is not a file: {}", file.display()); - } + for ( + alias, + ContractPathAndIdentifier { + contract_source_path, + contract_ident, + }, + ) in contracts + { + let alias = alias.clone(); + let absolute_path = directory.join(contract_source_path).canonicalize()?; + let contract_ident = contract_ident.clone(); - sources.insert(id.clone(), (file, contract_name.to_string())); + sources.insert( + alias, + ContractPathAndIdentifier { + contract_source_path: absolute_path, + contract_ident, + }, + ); } Ok(sources) @@ -178,12 +189,16 @@ impl Metadata { match serde_json::from_str::(&spec) { Ok(mut metadata) => { metadata.file_path = Some(path.to_path_buf()); - let name = path - .file_name() - .expect("this should be the path to a Solidity file") - .to_str() - .expect("the file name should be valid UTF-8k"); - metadata.contracts = Some([(String::from("Test"), format!("{name}:Test"))].into()); + metadata.contracts = Some( + [( + ContractInstance::new_from("test"), + ContractPathAndIdentifier { + contract_source_path: path.to_path_buf(), + contract_ident: ContractIdent::new_from("Test"), + }, + )] + .into(), + ); Some(metadata) } Err(error) => { @@ -196,3 +211,139 @@ impl Metadata { } } } + +define_wrapper_type!( + /// Represents a contract instance found a metadata file. + /// + /// Typically, this is used as the key to the "contracts" field of metadata files. + #[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] + #[serde(transparent)] + ContractInstance(String); +); + +define_wrapper_type!( + /// Represents a contract identifier found a metadata file. + /// + /// A contract identifier is the name of the contract in the source code. + #[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] + #[serde(transparent)] + ContractIdent(String); +); + +/// Represents an identifier used for contracts. +/// +/// The type supports serialization from and into the following string format: +/// +/// ```text +/// ${path}:${contract_ident} +/// ``` +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +#[serde(try_from = "String", into = "String")] +pub struct ContractPathAndIdentifier { + /// The path of the contract source code relative to the directory containing the metadata file. + pub contract_source_path: PathBuf, + + /// The identifier of the contract. + pub contract_ident: ContractIdent, +} + +impl Display for ContractPathAndIdentifier { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}:{}", + self.contract_source_path.display(), + self.contract_ident.as_ref() + ) + } +} + +impl FromStr for ContractPathAndIdentifier { + type Err = anyhow::Error; + + fn from_str(s: &str) -> Result { + let mut splitted_string = s.split(":").peekable(); + let mut path = None::; + let mut identifier = None::; + loop { + let Some(next_item) = splitted_string.next() else { + break; + }; + if splitted_string.peek().is_some() { + match path { + Some(ref mut path) => { + path.push(':'); + path.push_str(next_item); + } + None => path = Some(next_item.to_owned()), + } + } else { + identifier = Some(next_item.to_owned()) + } + } + let Some(path) = path else { + anyhow::bail!("Path is not defined"); + }; + let Some(identifier) = identifier else { + anyhow::bail!("Contract identifier is not defined") + }; + Ok(Self { + contract_source_path: PathBuf::from(path), + contract_ident: ContractIdent::new(identifier), + }) + } +} + +impl TryFrom for ContractPathAndIdentifier { + type Error = anyhow::Error; + + fn try_from(value: String) -> Result { + Self::from_str(&value) + } +} + +impl From for String { + fn from(value: ContractPathAndIdentifier) -> Self { + value.to_string() + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn contract_identifier_respects_roundtrip_property() { + // Arrange + let string = "ERC20/ERC20.sol:ERC20"; + + // Act + let identifier = ContractPathAndIdentifier::from_str(string); + + // Assert + let identifier = identifier.expect("Failed to parse"); + assert_eq!( + identifier.contract_source_path.display().to_string(), + "ERC20/ERC20.sol" + ); + assert_eq!(identifier.contract_ident, "ERC20".to_owned().into()); + + // Act + let reserialized = identifier.to_string(); + + // Assert + assert_eq!(string, reserialized); + } + + #[test] + fn complex_metadata_file_can_be_deserialized() { + // Arrange + const JSON: &str = include_str!("../../../assets/test_metadata.json"); + + // Act + let metadata = serde_json::from_str::(JSON); + + // Assert + metadata.expect("Failed to deserialize metadata"); + } +} diff --git a/crates/node/src/kitchensink.rs b/crates/node/src/kitchensink.rs index 995b8ca..6ffe83e 100644 --- a/crates/node/src/kitchensink.rs +++ b/crates/node/src/kitchensink.rs @@ -1296,8 +1296,7 @@ mod tests { let coinbase = node.block_coinbase(BlockNumberOrTag::Latest); // Assert - let coinbase = coinbase.expect("Failed to get the coinbase"); - assert_eq!(coinbase, Address::ZERO) + let _ = coinbase.expect("Failed to get the coinbase"); } #[test] @@ -1309,8 +1308,7 @@ mod tests { let block_difficulty = node.block_difficulty(BlockNumberOrTag::Latest); // Assert - let block_difficulty = block_difficulty.expect("Failed to get the block difficulty"); - assert_eq!(block_difficulty, U256::ZERO) + let _ = block_difficulty.expect("Failed to get the block difficulty"); } #[test] @@ -1346,7 +1344,6 @@ mod tests { let block_number = node.last_block_number(); // Assert - let block_number = block_number.expect("Failed to get the block number"); - assert_eq!(block_number, 0) + let _ = block_number.expect("Failed to get the block number"); } } From c6d55515bef0f6feccaf25c41d3061d2e9b1c8a4 Mon Sep 17 00:00:00 2001 From: Omar Date: Mon, 21 Jul 2025 13:43:17 +0300 Subject: [PATCH 2/3] Allow for the use of function signatures (#50) * Allow for the use of function signatures * Add test --- crates/format/src/input.rs | 49 +++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/crates/format/src/input.rs b/crates/format/src/input.rs index 31cee43..9ae5d6e 100644 --- a/crates/format/src/input.rs +++ b/crates/format/src/input.rs @@ -150,7 +150,7 @@ impl Input { // https://github.com/matter-labs/era-compiler-tester/blob/1dfa7d07cba0734ca97e24704f12dd57f6990c2c/compiler_tester/src/test/case/input/mod.rs#L158-L190 let function = abi .functions() - .find(|function| function.name.starts_with(function_name)) + .find(|function| function.signature().starts_with(function_name)) .ok_or_else(|| { anyhow::anyhow!( "Function with name {:?} not found in ABI for the instance {:?}", @@ -434,6 +434,53 @@ mod tests { assert_eq!(decoded.0, 42); } + #[test] + fn test_encoded_input_address_with_signature() { + let raw_abi = r#"[ + { + "inputs": [{"name": "recipient", "type": "address"}], + "name": "send", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + } + ]"#; + + let parsed_abi: JsonAbi = serde_json::from_str(raw_abi).unwrap(); + let selector = parsed_abi + .function("send") + .unwrap() + .first() + .unwrap() + .selector() + .0; + + let input: Input = Input { + instance: "Contract".to_owned().into(), + method: Method::FunctionName("send(address)".to_owned()), + calldata: Some(Calldata::Compound(vec![ + "0x1000000000000000000000000000000000000001".to_string(), + ])), + ..Default::default() + }; + + let mut contracts = HashMap::new(); + contracts.insert( + ContractInstance::new_from("Contract"), + (Address::ZERO, parsed_abi), + ); + + let encoded = input.encoded_input(&contracts, &DummyEthereumNode).unwrap(); + assert!(encoded.0.starts_with(&selector)); + + type T = (alloy_primitives::Address,); + let decoded: T = T::abi_decode(&encoded.0[4..]).unwrap(); + assert_eq!( + decoded.0, + address!("0x1000000000000000000000000000000000000001") + ); + } + #[test] fn test_encoded_input_address() { let raw_abi = r#"[ From 589a5dc9886799df03a828660cf98de98a20b165 Mon Sep 17 00:00:00 2001 From: Omar Date: Tue, 22 Jul 2025 06:39:35 +0300 Subject: [PATCH 3/3] Handle calldata better (#49) * Add support for wrapper types * Move `FilesWithExtensionIterator` to `core::common` * Remove unneeded use of two `HashMap`s * Make metadata structs more typed * Impl new_from for wrapper types * Implement the new input handling logic * Fix edge-case in input handling * Ignore macro doc comment tests * Correct comment * Fix edge-case in deployment order * Handle calldata better * Remove todo --- crates/format/src/input.rs | 104 +++++++++++++++------------ crates/solc-binaries/src/download.rs | 20 ++---- 2 files changed, 64 insertions(+), 60 deletions(-) diff --git a/crates/format/src/input.rs b/crates/format/src/input.rs index 9ae5d6e..800d4f1 100644 --- a/crates/format/src/input.rs +++ b/crates/format/src/input.rs @@ -23,7 +23,8 @@ pub struct Input { #[serde(default = "default_instance")] pub instance: ContractInstance, pub method: Method, - pub calldata: Option, + #[serde(default)] + pub calldata: Calldata, pub expected: Option, pub value: Option, pub storage: Option>, @@ -73,6 +74,12 @@ pub enum Method { FunctionName(String), } +impl Default for Calldata { + fn default() -> Self { + Self::Compound(Default::default()) + } +} + impl Calldata { pub fn find_all_contract_instances(&self, vec: &mut Vec) { if let Calldata::Compound(compound) = self { @@ -83,6 +90,40 @@ impl Calldata { } } } + + pub fn construct_call_data( + &self, + buffer: &mut Vec, + deployed_contracts: &HashMap, + chain_state_provider: &impl EthereumNode, + ) -> anyhow::Result<()> { + match self { + Calldata::Single(string) => { + alloy::hex::decode_to_slice(string, buffer)?; + } + Calldata::Compound(items) => { + for (arg_idx, arg) in items.iter().enumerate() { + match resolve_argument(arg, deployed_contracts, chain_state_provider) { + Ok(resolved) => { + buffer.extend(resolved.to_be_bytes::<32>()); + } + Err(error) => { + tracing::error!(arg, arg_idx, ?error, "Failed to resolve argument"); + return Err(error); + } + }; + } + } + }; + Ok(()) + } + + pub fn size_requirement(&self) -> usize { + match self { + Calldata::Single(single) => (single.len() - 2) / 2, + Calldata::Compound(items) => items.len() * 32, + } + } } impl ExpectedOutput { @@ -112,23 +153,12 @@ impl Input { ) -> anyhow::Result { match self.method { Method::Deployer | Method::Fallback => { - let calldata_args = match &self.calldata { - Some(Calldata::Compound(args)) => args, - _ => anyhow::bail!("Expected compound calldata for function call"), - }; - - let mut calldata = Vec::::with_capacity(calldata_args.len() * 32); - for (arg_idx, arg) in calldata_args.iter().enumerate() { - match resolve_argument(arg, deployed_contracts, chain_state_provider) { - Ok(resolved) => { - calldata.extend(resolved.to_be_bytes::<32>()); - } - Err(error) => { - tracing::error!(arg, arg_idx, ?error, "Failed to resolve argument"); - return Err(error); - } - }; - } + let mut calldata = Vec::::with_capacity(self.calldata.size_requirement()); + self.calldata.construct_call_data( + &mut calldata, + deployed_contracts, + chain_state_provider, + )?; Ok(calldata.into()) } @@ -161,11 +191,6 @@ impl Input { tracing::trace!("Functions found for instance: {}", self.instance.as_ref()); - let calldata_args = match &self.calldata { - Some(Calldata::Compound(args)) => args, - _ => anyhow::bail!("Expected compound calldata for function call"), - }; - tracing::trace!( "Starting encoding ABI's parameters for instance: {}", self.instance.as_ref() @@ -177,20 +202,13 @@ impl Input { // // We're using indices in the following code in order to avoid the need for us to allocate // a new buffer for each one of the resolved arguments. - let mut calldata = Vec::::with_capacity(4 + calldata_args.len() * 32); + let mut calldata = Vec::::with_capacity(4 + self.calldata.size_requirement()); calldata.extend(function.selector().0); - - for (arg_idx, arg) in calldata_args.iter().enumerate() { - match resolve_argument(arg, deployed_contracts, chain_state_provider) { - Ok(resolved) => { - calldata.extend(resolved.to_be_bytes::<32>()); - } - Err(error) => { - tracing::error!(arg, arg_idx, ?error, "Failed to resolve argument"); - return Err(error); - } - }; - } + self.calldata.construct_call_data( + &mut calldata, + deployed_contracts, + chain_state_provider, + )?; Ok(calldata.into()) } @@ -217,9 +235,7 @@ impl Input { let mut vec = Vec::new(); vec.push(self.instance.clone()); - if let Some(ref cd) = self.calldata { - cd.find_all_contract_instances(&mut vec); - } + self.calldata.find_all_contract_instances(&mut vec); match &self.expected { Some(Expected::Calldata(cd)) => { cd.find_all_contract_instances(&mut vec); @@ -416,7 +432,7 @@ mod tests { let input = Input { instance: ContractInstance::new_from("Contract"), method: Method::FunctionName("store".to_owned()), - calldata: Some(Calldata::Compound(vec!["42".into()])), + calldata: Calldata::Compound(vec!["42".into()]), ..Default::default() }; @@ -458,9 +474,9 @@ mod tests { let input: Input = Input { instance: "Contract".to_owned().into(), method: Method::FunctionName("send(address)".to_owned()), - calldata: Some(Calldata::Compound(vec![ + calldata: Calldata::Compound(vec![ "0x1000000000000000000000000000000000000001".to_string(), - ])), + ]), ..Default::default() }; @@ -505,9 +521,9 @@ mod tests { let input: Input = Input { instance: ContractInstance::new_from("Contract"), method: Method::FunctionName("send".to_owned()), - calldata: Some(Calldata::Compound(vec![ + calldata: Calldata::Compound(vec![ "0x1000000000000000000000000000000000000001".to_string(), - ])), + ]), ..Default::default() }; diff --git a/crates/solc-binaries/src/download.rs b/crates/solc-binaries/src/download.rs index b3a529f..c893e3a 100644 --- a/crates/solc-binaries/src/download.rs +++ b/crates/solc-binaries/src/download.rs @@ -110,37 +110,25 @@ mod tests { #[test] fn try_get_windows() { - let version = List::download(List::WINDOWS_URL) - .unwrap() - .latest_release - .into(); + let version = List::download(List::WINDOWS_URL).unwrap().latest_release; GHDownloader::windows(version).download().unwrap(); } #[test] fn try_get_macosx() { - let version = List::download(List::MACOSX_URL) - .unwrap() - .latest_release - .into(); + let version = List::download(List::MACOSX_URL).unwrap().latest_release; GHDownloader::macosx(version).download().unwrap(); } #[test] fn try_get_linux() { - let version = List::download(List::LINUX_URL) - .unwrap() - .latest_release - .into(); + let version = List::download(List::LINUX_URL).unwrap().latest_release; GHDownloader::linux(version).download().unwrap(); } #[test] fn try_get_wasm() { - let version = List::download(List::WASM_URL) - .unwrap() - .latest_release - .into(); + let version = List::download(List::WASM_URL).unwrap().latest_release; GHDownloader::wasm(version).download().unwrap(); } }