From 2ef6f7ba63d92256ed53069932b848cd069c0d8f Mon Sep 17 00:00:00 2001 From: Omar Abdulla Date: Thu, 17 Jul 2025 15:31:18 +0300 Subject: [PATCH] Make metadata structs more typed --- assets/test_metadata.json | 326 ++++++++++++++++++++++++++++++++++ crates/core/src/common.rs | 97 ---------- crates/format/src/lib.rs | 1 + crates/format/src/macros.rs | 102 +++++++++++ crates/format/src/metadata.rs | 197 +++++++++++++++++--- 5 files changed, 603 insertions(+), 120 deletions(-) create mode 100644 assets/test_metadata.json create mode 100644 crates/format/src/macros.rs 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/src/common.rs b/crates/core/src/common.rs index 4bf8a24..1892c5f 100644 --- a/crates/core/src/common.rs +++ b/crates/core/src/common.rs @@ -1,102 +1,5 @@ use std::{borrow::Cow, collections::HashSet, path::PathBuf}; -/// Defines wrappers around types. -/// -/// For example, the macro invocation seen below: -/// -/// ```rust,no_run -/// define_wrapper_type!(CaseId => usize); -/// ``` -/// -/// Would define a wrapper type that looks like the following: -/// -/// ```rust,no_run -/// 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,no_run -/// 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,no_run -/// 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 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 - } - } - }; -} - -define_wrapper_type!( - /// Represents the ID of one of the cases in a metadata file. - #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] - CaseId => usize -); - /// 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 { 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..f3c02a6 --- /dev/null +++ b/crates/format/src/macros.rs @@ -0,0 +1,102 @@ +/// Defines wrappers around types. +/// +/// For example, the macro invocation seen below: +/// +/// ```rust,no_run +/// define_wrapper_type!(CaseId => usize); +/// ``` +/// +/// Would define a wrapper type that looks like the following: +/// +/// ```rust,no_run +/// 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,no_run +/// 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,no_run +/// 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 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..eb122d3 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( + [( + ContractAlias::new("test".into()), + ContractPathAndIdentifier { + contract_source_path: path.to_path_buf(), + contract_ident: ContractIdent::new("Test".into()), + }, + )] + .into(), + ); Some(metadata) } Err(error) => { @@ -196,3 +211,139 @@ impl Metadata { } } } + +define_wrapper_type!( + /// Represents a contract alias found a metadata file. + /// + /// Typically, this is used as the key to the "contracts" field of metadata files. + #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] + #[serde(transparent)] + ContractAlias => 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, 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"); + } +}