diff --git a/README.md b/README.md index 9af1a35d..7529641c 100644 --- a/README.md +++ b/README.md @@ -526,6 +526,7 @@ This section will be updated soon. ## Known Limitations and Bugs +- Compilation with libraries is currently not supported. The best workaround is to compile using `forge build --libraries --build-info --build-info-path ` and then use `` using the `--buildcache` argument. - Currently only solidity is supported. - Only projects with `solc` version starting from `0.5.13` are supported due to the lack of generated storage layout in older versions (see [solc release 0.5.13](https://github.com/ethereum/solidity/releases/tag/v0.5.13)). - The RPC endpoints automatically parsed in `dv generate-config` are not guaranteed to be compatible. diff --git a/lib/bytecode_verification/compare_bytecodes.rs b/lib/bytecode_verification/compare_bytecodes.rs index 31ea708a..23156bf8 100644 --- a/lib/bytecode_verification/compare_bytecodes.rs +++ b/lib/bytecode_verification/compare_bytecodes.rs @@ -145,6 +145,16 @@ impl CompareBytecode { Self::ignore_immutables(&project_info.immutables, &mut relevant_indices); + // Ignore Library Address at the beginning of a Library: PUSH20 address + if project_info.is_library { + debug!("Skipping initial library bytes"); + for index in relevant_indices.iter_mut().take(21).skip(1) { + *index = false; + } + // Check for PUSH20 + assert_eq!(compiled_bytecode[0], 0x73); + } + // If no metadata is appended if Some(BytecodeHash::None) == project_info.cbor_metadata { let matched = @@ -260,6 +270,7 @@ impl CompareInitCode { ); } // println!("Relevant Indices: {:?}", relevant_indices.clone()); + // debug!("Initcode comparison: {:?} ?= {:?}", compiled_init_code.clone(), init_bytecode[..init_len].to_vec()); if !compare_relevant( &compiled_init_code, &init_bytecode[..init_len], @@ -325,6 +336,7 @@ mod tests { types: HashMap::new(), storage: vec![], absolute_path: None, + is_library: false, }; let compare_status = CompareBytecode::compare(&mut p, false, &onchain_code); assert!(!compare_status.matched); @@ -349,6 +361,7 @@ mod tests { types: HashMap::new(), storage: vec![], absolute_path: None, + is_library: false, }; let compare_status = CompareBytecode::compare(&mut p, false, &onchain_code); assert!(compare_status.matched); @@ -404,6 +417,7 @@ mod tests { types: HashMap::new(), storage: vec![], absolute_path: None, + is_library: false, }; let compare_status = CompareInitCode::compare(&mut p, &tx_init_code, false); diff --git a/lib/bytecode_verification/parse_json.rs b/lib/bytecode_verification/parse_json.rs index 5bffed55..fad1069e 100644 --- a/lib/bytecode_verification/parse_json.rs +++ b/lib/bytecode_verification/parse_json.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; use alloy::json_abi::Constructor; use clap::ValueEnum; use semver::Version; +use serde_json; use serde_json::Value; use std::path::Path; use std::process::Command; @@ -26,8 +27,8 @@ use alloy::json_abi::Event; use alloy::primitives::U256; use foundry_compilers::artifacts::Error as CompilerError; use foundry_compilers::artifacts::{ - BytecodeHash, BytecodeObject, Contract as ContractArt, DeployedBytecode, Node as EAstNode, - NodeType, SolcInput, SourceFile, + BytecodeHash, BytecodeObject, Contract as ContractArt, ContractDefinition, ContractKind, + DeployedBytecode, Node as EAstNode, NodeType, SolcInput, SourceFile, }; use foundry_compilers::buildinfo::BuildInfo as BInfo; use foundry_compilers::CompilerOutput; @@ -55,6 +56,7 @@ pub struct ProjectInfo { pub storage: Vec, pub types: HashMap, pub absolute_path: Option, + pub is_library: bool, } impl ProjectInfo { @@ -1317,21 +1319,32 @@ impl ProjectInfo { } } - /// Parses the AST for a contract definition. - fn contains_contract(node: &EAstNode, contract_name: &String) -> bool { + // Tries to figure out whether this is a library + + // Parses the AST for a contract definition. + // Assumes that it is one of the top nodes + fn find_contract_definition( + node: &EAstNode, + contract_name: &String, + ) -> Result { if node.node_type == NodeType::ContractDefinition { if let Some(name) = node.other.get("name") { if name == contract_name { - return true; + let serialized = serde_json::to_value(node)?; + return Ok(serde_json::from_value::(serialized)?); } } } for subnode in &node.nodes { - if Self::contains_contract(subnode, contract_name) { - return true; + if let Ok(contract_definition) = Self::find_contract_definition(subnode, contract_name) + { + return Ok(contract_definition); } } - false + Err(ValidationError::from(format!( + "Could not find contract definition for {}", + contract_name + ))) } // Parses the AST to find all associated contracts (libraries & parent contracts) @@ -1341,14 +1354,14 @@ impl ProjectInfo { exported_ids: &mut Vec, ) { for source in sources.values() { - if let Some(new_ast) = source.ast.clone() { + if let Some(new_ast) = &source.ast { for node in &new_ast.nodes { - if Self::contains_contract(node, contract_name) { - for (sub_contract, symbols) in new_ast.exported_symbols { + if Self::find_contract_definition(node, contract_name).is_ok() { + for (sub_contract, symbols) in &new_ast.exported_symbols { // TODO: what does it mean if there is more than 1 symbol per contract? if symbols.len() == 1 && !exported_ids.contains(&symbols[0]) { exported_ids.extend(symbols); - Self::find_exported_ids(sources, &sub_contract, exported_ids); + Self::find_exported_ids(sources, sub_contract, exported_ids); } } break; @@ -1532,11 +1545,18 @@ impl ProjectInfo { let mut types: HashMap = HashMap::new(); let mut exported_ids: Vec = vec![]; let mut absolute_path: Option = None; - for (file, source) in build_info.output.sources.clone() { - if let Some(new_ast) = source.ast.clone() { - for node in &new_ast.nodes { - if Self::contains_contract(node, contract_name) { - absolute_path = Some(new_ast.absolute_path.to_string()); + // TODO: Use relevant_ast instead of repeatedly searching + // let mut relevant_ast: Ast; + let mut contract_definition: Option = None; + for (file, source) in &build_info.output.sources { + if let Some(ast_ref) = &source.ast { + for node in &ast_ref.nodes { + if let Ok(tmp_contract_definition) = + Self::find_contract_definition(node, contract_name) + { + // relevant_ast = ast_ref.clone(); + absolute_path = Some(ast_ref.absolute_path.clone()); + contract_definition = Some(tmp_contract_definition); break; } } @@ -1544,6 +1564,15 @@ impl ProjectInfo { debug!("Empty AST found: {}", file.display()); } } + + if contract_definition.is_none() { + return Err(ValidationError::from(format!( + "Could not find the contract definition AST node of {}", + &contract_name + ))); + } + let contract_definition = contract_definition.unwrap(); + // get exported AST IDs of the current contract to prevent parsing storage slots of other contracts // in the project Self::find_exported_ids(&build_info.output.sources, contract_name, &mut exported_ids); @@ -1555,7 +1584,7 @@ impl ProjectInfo { } for source in build_info.output.sources.values() { // TODO: Error handle here, what though? - if let Some(new_ast) = source.ast.clone() { + if let Some(new_ast) = &source.ast { for node in &new_ast.nodes { Self::find_var_defs(node, &mut id_to_ast); } @@ -1584,6 +1613,10 @@ impl ProjectInfo { fs::remove_dir_all(&build_info_path)?; }; + let contract_kind = contract_definition.kind.clone(); + let is_library = contract_kind == ContractKind::Library; + debug!("Contract Kind: {contract_kind:?}, is_library: {is_library:?}"); + let pi = ProjectInfo { compiled_bytecode: compiled_bytecode_str, init_code: init_code_str, @@ -1604,6 +1637,7 @@ impl ProjectInfo { storage, types, absolute_path, + is_library, }; Ok(pi) diff --git a/lib/state/contract_state.rs b/lib/state/contract_state.rs index 749b117e..2f274c93 100644 --- a/lib/state/contract_state.rs +++ b/lib/state/contract_state.rs @@ -462,8 +462,8 @@ impl<'a> ContractState<'a> { .extend(self.get_critical_variable(&base, snapshot, table)?); // Check if we need to skip multiple slots if base_num_bytes > 32 { - current_slot = - current_slot.add(U256::from((current_offset + base_num_bytes + 31) / 32)); + current_slot = current_slot + .add(U256::from((current_offset + base_num_bytes).div_ceil(32))); current_offset = 0; // Check if we need to skip one slot } else if current_offset + base_num_bytes + base_num_bytes > 32 { diff --git a/tests/Contracts/script/Deploy_Lib.s.sol b/tests/Contracts/script/Deploy_Lib.s.sol new file mode 100644 index 00000000..917ee2c5 --- /dev/null +++ b/tests/Contracts/script/Deploy_Lib.s.sol @@ -0,0 +1,21 @@ +pragma solidity ^0.8.12; + +import "forge-std/Script.sol"; +import "../src/LibUser.sol"; + +contract S is Script { + uint256 x; + uint256 y; + + function run() external { + uint256 anvilDefaultKey = 0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80; + //uint256 ganacheDefaultKey = 0x0cc0c2de7e8c30525b4ca3b9e0b9703fb29569060d403261055481df7014f7fa; + vm.startBroadcast(anvilDefaultKey); + + LibUser libUser = new LibUser(); + for (uint256 i = 0; i < 5; i++) { + libUser.doSomething(); + } + vm.stopBroadcast(); + } +} diff --git a/tests/Contracts/src/Lib.sol b/tests/Contracts/src/Lib.sol new file mode 100644 index 00000000..568c1498 --- /dev/null +++ b/tests/Contracts/src/Lib.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +library Lib { + function doSomething(uint256 a, uint256 b) external pure returns (uint256) { + return a + b; + } +} diff --git a/tests/Contracts/src/LibUser.sol b/tests/Contracts/src/LibUser.sol new file mode 100644 index 00000000..885fb33a --- /dev/null +++ b/tests/Contracts/src/LibUser.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "../src/Lib.sol"; + +contract LibUser { + function doSomething() external { + Lib.doSomething(1, 2); + } +} diff --git a/tests/expected_dvfs/Lib.dvf.json b/tests/expected_dvfs/Lib.dvf.json new file mode 100644 index 00000000..c41fa165 --- /dev/null +++ b/tests/expected_dvfs/Lib.dvf.json @@ -0,0 +1,27 @@ +{ + "version": "0.9.1", + "id": "0xdb26661b8d95e1e02f9331fc20ac3e28635fa9895083c1366c7918a59963312d", + "contract_name": "Lib", + "address": "0x8627e5da250bd67817177c77ed8432e8528d2bc9", + "chain_id": 31337, + "deployment_block_num": 1, + "init_block_num": 4, + "deployment_tx": "0xc46bb378f0ae75fbf0c0c65d9536df3854c9147ebb0d8adec1cd65355cfe36bc", + "codehash": "0xacdec87e06ce0e6478a315ee1ceccbd69f4b0e9bb3ee92c4ea70b8a757818512", + "insecure": false, + "immutables": [], + "constructor_args": [], + "critical_storage_variables": [], + "critical_events": [], + "unvalidated_metadata": { + "author_name": "Author", + "description": "System Description", + "hardfork": [ + "paris", + "shanghai" + ], + "audit_report": "https://example.org/report.pdf", + "source_url": "https://github.com/source/code", + "security_contact": "security@example.org" + } +} \ No newline at end of file diff --git a/tests/test_end_to_end.rs b/tests/test_end_to_end.rs index ae5a85d8..974f7d50 100644 --- a/tests/test_end_to_end.rs +++ b/tests/test_end_to_end.rs @@ -786,6 +786,12 @@ mod tests { let mut testcases: Vec = vec![]; + testcases.push(TestCaseE2E { + script: String::from("script/Deploy_Lib.s.sol"), + contract: String::from("Lib"), + expected: String::from("tests/expected_dvfs/Lib.dvf.json"), + }); + testcases.push(TestCaseE2E { script: String::from("script/Deploy_0.s.sol"), contract: String::from("BytesMapping"), @@ -829,9 +835,14 @@ mod tests { contract: String::from("CrazyHiddenStruct"), expected: String::from("tests/expected_dvfs/CrazyHiddenStruct.dvf.json"), }); + for testcase in testcases { let url = format!("http://localhost:{}", port).to_string(); for client_type in LocalClientType::iterator() { + // Don't run this test with Geth as it requires a different setup + if testcase.contract == "Lib" && client_type == LocalClientType::Geth { + continue; + } let local_client = start_local_client(client_type.clone(), port); // forge script script/Deploy_0.s.sol --rpc-url "http://127.0.0.1:8546" --broadcast --slow @@ -861,7 +872,11 @@ mod tests { &config_file.path().to_string_lossy(), "init", "--address", - "0x5fbdb2315678afecb367f032d93f642f64180aa3", + if testcase.contract == "Lib" { + "0x8627e5da250bd67817177c77ed8432e8528d2bc9" + } else { + "0x5fbdb2315678afecb367f032d93f642f64180aa3" + }, "--chainid", &chain_id_str(client_type.clone()), "--project",