diff --git a/crates/fbal/tests/builder/delegatecall.rs b/crates/fbal/tests/builder/delegatecall.rs new file mode 100644 index 00000000..5f0fa4f7 --- /dev/null +++ b/crates/fbal/tests/builder/delegatecall.rs @@ -0,0 +1,251 @@ +//! Tests for DELEGATECALL storage pattern tracking + +use std::collections::HashMap; + +use alloy_primitives::{B256, TxKind, U256}; +use alloy_sol_types::SolCall; +use op_revm::OpTransaction; +use revm::{ + context::TxEnv, + interpreter::instructions::utility::IntoAddress, + primitives::ONE_ETHER, + state::{AccountInfo, Bytecode}, +}; + +use super::{BASE_SEPOLIA_CHAIN_ID, Logic, Logic2, Proxy, execute_txns_build_access_list}; + +#[test] +/// Tests that DELEGATECALL storage changes are tracked on the calling contract (Proxy), +/// not the logic contract +fn test_delegatecall_storage_tracked_on_caller() { + let sender = U256::from(0xDEAD).into_address(); + let logic_addr = U256::from(0xBEEF).into_address(); + let proxy_addr = U256::from(0xCAFE).into_address(); + + let mut overrides = HashMap::new(); + overrides.insert(sender, AccountInfo::from_balance(U256::from(ONE_ETHER))); + + // Deploy logic contract first + overrides.insert( + logic_addr, + AccountInfo::default().with_code(Bytecode::new_raw(Logic::DEPLOYED_BYTECODE.clone())), + ); + + // Deploy proxy contract with logic as implementation + // We need to set up the proxy's storage slot 0 to point to logic + overrides.insert( + proxy_addr, + AccountInfo::default().with_code(Bytecode::new_raw(Proxy::DEPLOYED_BYTECODE.clone())), + ); + + // Call setValue(42) through the proxy + let tx = OpTransaction::builder() + .base( + TxEnv::builder() + .caller(sender) + .chain_id(Some(BASE_SEPOLIA_CHAIN_ID)) + .kind(TxKind::Call(proxy_addr)) + .data(Logic::setValueCall { v: U256::from(42) }.abi_encode().into()) + .nonce(0) + .gas_price(0) + .gas_priority_fee(None) + .max_fee_per_gas(0) + .gas_limit(200_000), + ) + .build_fill(); + + let access_list = execute_txns_build_access_list( + vec![tx], + Some(overrides), + Some(HashMap::from([(proxy_addr, HashMap::from([(U256::ZERO, logic_addr.into_word())]))])), + ); + + // Verify that proxy is in touched accounts + let proxy_changes = access_list + .account_changes + .iter() + .find(|ac| ac.address == proxy_addr) + .expect("Proxy should be in account changes"); + + // Verify storage change for slot 1 is on the PROXY, not the logic contract + // Slot 1 is where `value` is stored + let slot_1 = B256::from(U256::from(1)); + let has_slot_1_change = proxy_changes.storage_changes.iter().any(|sc| sc.slot == slot_1); + assert!(has_slot_1_change, "Proxy should have storage change for slot 1 (value)"); + + // Verify logic contract has NO storage changes (it's just providing code) + let logic_changes = access_list.account_changes.iter().find(|ac| ac.address == logic_addr); + if let Some(logic) = logic_changes { + assert!(logic.storage_changes.is_empty(), "Logic contract should have no storage changes"); + } +} + +#[test] +/// Tests that DELEGATECALL storage reads are tracked on the calling contract +fn test_delegatecall_read_tracked_on_caller() { + let sender = U256::from(0xDEAD).into_address(); + let logic_addr = U256::from(0xBEEF).into_address(); + let proxy_addr = U256::from(0xCAFE).into_address(); + + let mut overrides = HashMap::new(); + overrides.insert(sender, AccountInfo::from_balance(U256::from(ONE_ETHER))); + overrides.insert( + logic_addr, + AccountInfo::default().with_code(Bytecode::new_raw(Logic::DEPLOYED_BYTECODE.clone())), + ); + overrides.insert( + proxy_addr, + AccountInfo::default().with_code(Bytecode::new_raw(Proxy::DEPLOYED_BYTECODE.clone())), + ); + + // First set a value, then read it + let set_tx = OpTransaction::builder() + .base( + TxEnv::builder() + .caller(sender) + .chain_id(Some(BASE_SEPOLIA_CHAIN_ID)) + .kind(TxKind::Call(proxy_addr)) + .data(Logic::setValueCall { v: U256::from(42) }.abi_encode().into()) + .nonce(0) + .gas_price(0) + .gas_priority_fee(None) + .max_fee_per_gas(0) + .gas_limit(200_000), + ) + .build_fill(); + + let get_tx = OpTransaction::builder() + .base( + TxEnv::builder() + .caller(sender) + .chain_id(Some(BASE_SEPOLIA_CHAIN_ID)) + .kind(TxKind::Call(proxy_addr)) + .data(Logic::getValueCall {}.abi_encode().into()) + .nonce(1) + .gas_price(0) + .gas_priority_fee(None) + .max_fee_per_gas(0) + .gas_limit(200_000), + ) + .build_fill(); + + let access_list = execute_txns_build_access_list( + vec![set_tx, get_tx], + Some(overrides), + Some(HashMap::from([(proxy_addr, HashMap::from([(U256::ZERO, logic_addr.into_word())]))])), + ); + + // Verify proxy has storage reads recorded + let proxy_changes = access_list + .account_changes + .iter() + .find(|ac| ac.address == proxy_addr) + .expect("Proxy should be in account changes"); + + // Slot 1 should have been read (for getValue) + let slot_1 = B256::from(U256::from(1)); + let has_slot_1_read = proxy_changes.storage_reads.iter().any(|sr| *sr == slot_1); + assert!(has_slot_1_read, "Proxy should have storage read for slot 1"); + + // Verify both addresses are in touched accounts + assert!( + access_list.account_changes.iter().any(|ac| ac.address == proxy_addr), + "Proxy should be in touched accounts" + ); + assert!( + access_list.account_changes.iter().any(|ac| ac.address == logic_addr), + "Logic should be in touched accounts" + ); +} + +#[test] +/// Tests chained DELEGATECALL: Proxy -> Logic2 -> Logic +/// Storage changes should still be tracked on the original Proxy +fn test_delegatecall_chain() { + let sender = U256::from(0xDEAD).into_address(); + let logic_addr = U256::from(0xBEEF).into_address(); + let logic2_addr = U256::from(0xFACE).into_address(); + let proxy_addr = U256::from(0xCAFE).into_address(); + + let mut overrides = HashMap::new(); + overrides.insert(sender, AccountInfo::from_balance(U256::from(ONE_ETHER))); + overrides.insert( + logic_addr, + AccountInfo::default().with_code(Bytecode::new_raw(Logic::DEPLOYED_BYTECODE.clone())), + ); + overrides.insert( + logic2_addr, + AccountInfo::default().with_code(Bytecode::new_raw(Logic2::DEPLOYED_BYTECODE.clone())), + ); + overrides.insert( + proxy_addr, + AccountInfo::default().with_code(Bytecode::new_raw(Proxy::DEPLOYED_BYTECODE.clone())), + ); + + // Storage overrides: + // - Proxy slot 0 = logic2_addr (implementation) + // - Proxy slot 3 = logic_addr (nextLogic) - Because Logic2's chainedDelegatecall + // runs in Proxy's context, it reads nextLogic from Proxy's storage slot 3 + let storage_overrides = HashMap::from([( + proxy_addr, + HashMap::from([ + (U256::ZERO, logic2_addr.into_word()), + (U256::from(3), logic_addr.into_word()), + ]), + )]); + + // Call chainedDelegatecall with setValue(99) encoded + let inner_call = Logic::setValueCall { v: U256::from(99) }.abi_encode(); + let tx = OpTransaction::builder() + .base( + TxEnv::builder() + .caller(sender) + .chain_id(Some(BASE_SEPOLIA_CHAIN_ID)) + .kind(TxKind::Call(proxy_addr)) + .data( + Logic2::chainedDelegatecallCall { data: inner_call.into() }.abi_encode().into(), + ) + .nonce(0) + .gas_price(0) + .gas_priority_fee(None) + .max_fee_per_gas(0) + .gas_limit(300_000), + ) + .build_fill(); + + let access_list = + execute_txns_build_access_list(vec![tx], Some(overrides), Some(storage_overrides)); + + // Verify all three addresses are in touched accounts + assert!( + access_list.account_changes.iter().any(|ac| ac.address == proxy_addr), + "Proxy should be in touched accounts" + ); + assert!( + access_list.account_changes.iter().any(|ac| ac.address == logic2_addr), + "Logic2 should be in touched accounts" + ); + assert!( + access_list.account_changes.iter().any(|ac| ac.address == logic_addr), + "Logic should be in touched accounts" + ); + + // Storage changes should be on the PROXY (the original caller) + let proxy_changes = access_list + .account_changes + .iter() + .find(|ac| ac.address == proxy_addr) + .expect("Proxy should have account changes"); + + let slot_1 = B256::from(U256::from(1)); + let has_value_change = proxy_changes.storage_changes.iter().any(|sc| sc.slot == slot_1); + assert!(has_value_change, "Proxy should have storage change for slot 1 (value)"); + + // Logic contracts should have no storage changes + if let Some(logic2) = access_list.account_changes.iter().find(|ac| ac.address == logic2_addr) { + assert!(logic2.storage_changes.is_empty(), "Logic2 should have no storage changes"); + } + if let Some(logic) = access_list.account_changes.iter().find(|ac| ac.address == logic_addr) { + assert!(logic.storage_changes.is_empty(), "Logic should have no storage changes"); + } +} diff --git a/crates/fbal/tests/builder/deployment.rs b/crates/fbal/tests/builder/deployment.rs index f934fe45..d8eb04f9 100644 --- a/crates/fbal/tests/builder/deployment.rs +++ b/crates/fbal/tests/builder/deployment.rs @@ -55,7 +55,7 @@ fn test_create_deployment_tracked() { ) .build_fill(); - let access_list = execute_txns_build_access_list(vec![tx], Some(overrides)); + let access_list = execute_txns_build_access_list(vec![tx], Some(overrides), None); // Verify factory is in the access list let factory_entry = access_list.account_changes.iter().find(|ac| ac.address() == factory); @@ -123,7 +123,7 @@ fn test_create2_deployment_tracked() { ) .build_fill(); - let access_list = execute_txns_build_access_list(vec![tx], Some(overrides)); + let access_list = execute_txns_build_access_list(vec![tx], Some(overrides), None); // Verify factory is in the access list let factory_entry = access_list.account_changes.iter().find(|ac| ac.address() == factory); @@ -188,7 +188,7 @@ fn test_create_and_immediate_call() { ) .build_fill(); - let access_list = execute_txns_build_access_list(vec![tx], Some(overrides)); + let access_list = execute_txns_build_access_list(vec![tx], Some(overrides), None); // Verify factory is in the access list let factory_entry = access_list.account_changes.iter().find(|ac| ac.address() == factory); diff --git a/crates/fbal/tests/builder/main.rs b/crates/fbal/tests/builder/main.rs index cf90291f..73cfcab7 100644 --- a/crates/fbal/tests/builder/main.rs +++ b/crates/fbal/tests/builder/main.rs @@ -7,7 +7,7 @@ use alloy_eip7928::{ AccountChanges, BalanceChange, CodeChange, EMPTY_BLOCK_ACCESS_LIST_HASH, NonceChange, SlotChanges, StorageChange, }; -use alloy_primitives::{Address, B256}; +use alloy_primitives::{Address, B256, U256}; use alloy_sol_macro::sol; use base_fbal::{FlashblockAccessList, TouchedAccountsInspector}; use op_revm::OpTransaction; @@ -22,6 +22,7 @@ use revm::{ state::AccountInfo, }; +mod delegatecall; mod deployment; mod storage; mod transfers; @@ -53,11 +54,30 @@ sol!( ) ); +sol!( + #[sol(rpc)] + Proxy, + concat!(env!("CARGO_MANIFEST_DIR"), "/../test-utils/contracts/out/Proxy.sol/Proxy.json") +); + +sol!( + #[sol(rpc)] + Logic, + concat!(env!("CARGO_MANIFEST_DIR"), "/../test-utils/contracts/out/Proxy.sol/Logic.json") +); + +sol!( + #[sol(rpc)] + Logic2, + concat!(env!("CARGO_MANIFEST_DIR"), "/../test-utils/contracts/out/Proxy.sol/Logic2.json") +); + const BASE_SEPOLIA_CHAIN_ID: u64 = 84532; fn execute_txns_build_access_list( txs: Vec>, acc_overrides: Option>, + storage_overrides: Option>>, ) -> FlashblockAccessList { let chain_spec = Arc::new(OpChainSpec::from_genesis( serde_json::from_str(include_str!("../../../test-utils/assets/genesis.json")).unwrap(), @@ -70,6 +90,13 @@ fn execute_txns_build_access_list( db.insert_account_info(address, info); } } + if let Some(storage) = storage_overrides { + for (address, slots) in storage { + for (slot, value) in slots { + db.insert_account_storage(address, slot, U256::from_be_bytes(value.0)).unwrap(); + } + } + } let mut access_list = FlashblockAccessList { min_tx_index: 0, diff --git a/crates/fbal/tests/builder/storage.rs b/crates/fbal/tests/builder/storage.rs index 4f7ed78a..de4c9821 100644 --- a/crates/fbal/tests/builder/storage.rs +++ b/crates/fbal/tests/builder/storage.rs @@ -41,7 +41,7 @@ fn test_sload_zero_value() { ) .build_fill(); - let access_list = execute_txns_build_access_list(vec![tx], Some(overrides)); + let access_list = execute_txns_build_access_list(vec![tx], Some(overrides), None); dbg!(access_list); } @@ -96,7 +96,7 @@ fn test_update_one_value() { .build_fill(), ); - let access_list = execute_txns_build_access_list(txs, Some(overrides)); + let access_list = execute_txns_build_access_list(txs, Some(overrides), None); dbg!(access_list); } @@ -129,7 +129,7 @@ fn test_multi_sload_same_slot() { ) .build_fill(); - let access_list = execute_txns_build_access_list(vec![tx], Some(overrides)); + let access_list = execute_txns_build_access_list(vec![tx], Some(overrides), None); // TODO: dedup storage_reads dbg!(access_list); } @@ -170,6 +170,6 @@ fn test_multi_sstore() { ) .build_fill(); - let access_list = execute_txns_build_access_list(vec![tx], Some(overrides)); + let access_list = execute_txns_build_access_list(vec![tx], Some(overrides), None); dbg!(access_list); } diff --git a/crates/fbal/tests/builder/transfers.rs b/crates/fbal/tests/builder/transfers.rs index 175e42d0..9113f54d 100644 --- a/crates/fbal/tests/builder/transfers.rs +++ b/crates/fbal/tests/builder/transfers.rs @@ -17,7 +17,7 @@ fn test_precompiles() { let base_tx = TxEnv::builder().chain_id(Some(BASE_SEPOLIA_CHAIN_ID)).gas_limit(50_000).gas_price(0); let tx = OpTransaction::builder().base(base_tx).build_fill(); - let access_list = execute_txns_build_access_list(vec![tx], None); + let access_list = execute_txns_build_access_list(vec![tx], None, None); dbg!(access_list); } @@ -43,7 +43,7 @@ fn test_single_transfer() { ) .build_fill(); - let access_list = execute_txns_build_access_list(vec![tx], Some(overrides)); + let access_list = execute_txns_build_access_list(vec![tx], Some(overrides), None); dbg!(access_list); } @@ -71,7 +71,7 @@ fn test_gas_included_in_balance_change() { ) .build_fill(); - let access_list = execute_txns_build_access_list(vec![tx], Some(overrides)); + let access_list = execute_txns_build_access_list(vec![tx], Some(overrides), None); dbg!(access_list); } @@ -103,6 +103,6 @@ fn test_multiple_transfers() { txs.push(tx); } - let access_list = execute_txns_build_access_list(txs, Some(overrides)); + let access_list = execute_txns_build_access_list(txs, Some(overrides), None); dbg!(access_list); } diff --git a/crates/test-utils/contracts/src/Proxy.sol b/crates/test-utils/contracts/src/Proxy.sol new file mode 100644 index 00000000..9985d7c8 --- /dev/null +++ b/crates/test-utils/contracts/src/Proxy.sol @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +contract Proxy { + // Storage layout must match Logic contract + address public implementation; // slot 0 + uint256 public value; // slot 1 + uint256 public value2; // slot 2 + + constructor(address _impl) { + implementation = _impl; + } + + fallback() external payable { + address impl = implementation; + assembly { + calldatacopy(0, 0, calldatasize()) + let result := delegatecall(gas(), impl, 0, calldatasize(), 0, 0) + returndatacopy(0, 0, returndatasize()) + switch result + case 0 { revert(0, returndatasize()) } + default { return(0, returndatasize()) } + } + } + + receive() external payable {} +} + +contract Logic { + // Storage layout must match Proxy contract + address public implementation; // slot 0 - unused but must be here + uint256 public value; // slot 1 + uint256 public value2; // slot 2 + + function setValue(uint256 v) public { + value = v; + } + + function setValue2(uint256 v) public { + value2 = v; + } + + function getValue() public view returns (uint256) { + return value; + } + + function setBoth(uint256 v1, uint256 v2) public { + value = v1; + value2 = v2; + } +} + +contract Logic2 { + address public implementation; + uint256 public value; + uint256 public value2; + address public nextLogic; + + function setNextLogic(address _next) public { + nextLogic = _next; + } + + function chainedDelegatecall(bytes memory data) public returns (bytes memory) { + (bool success, bytes memory result) = nextLogic.delegatecall(data); + require(success, "Chained delegatecall failed"); + return result; + } +}