diff --git a/.github/workflows/tests-evm.yml b/.github/workflows/tests-evm.yml index bcc95f3bcf44c..952f6d12039c0 100644 --- a/.github/workflows/tests-evm.yml +++ b/.github/workflows/tests-evm.yml @@ -107,6 +107,10 @@ jobs: # but still want to have debug assertions. RUSTFLAGS: "-C debug-assertions" RUST_BACKTRACE: 1 + strategy: + matrix: + platform: + ["test:pvm", "test:evm"] steps: - name: Checkout uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 @@ -120,7 +124,7 @@ jobs: uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 with: repository: paritytech/evm-test-suite - ref: 4dc2658b3c944d65e4624fad87e7532c9253448e + ref: 9359438a13e8ab68f73320724f8783e170ecc193 path: evm-test-suite - uses: denoland/setup-deno@v2 @@ -139,11 +143,8 @@ jobs: export ETH_RPC_PATH=$(readlink -f ../target/release/eth-rpc) echo $REVIVE_DEV_NODE_PATH $ETH_RPC_PATH - echo "== Running pvm tests ==j" - START_REVIVE_DEV_NODE=true START_ETH_RPC=true deno task test:pvm - - echo "== Running evm tests ==" - START_REVIVE_DEV_NODE=true START_ETH_RPC=true deno task test:evm + echo "== Running tests ==" + START_REVIVE_DEV_NODE=true START_ETH_RPC=true deno task ${{ matrix.platform }} confirm-required-test-evm-jobs-passed: runs-on: ubuntu-latest diff --git a/prdoc/pr_10244.prdoc b/prdoc/pr_10244.prdoc new file mode 100644 index 0000000000000..111019be7a6e9 --- /dev/null +++ b/prdoc/pr_10244.prdoc @@ -0,0 +1,7 @@ +title: '[pallet-revive] add tracing for selfdestruct' +doc: +- audience: Runtime Dev + description: Add tracing for selfdestruct +crates: +- name: pallet-revive + bump: patch diff --git a/substrate/frame/revive/src/evm/api/debug_rpc_types.rs b/substrate/frame/revive/src/evm/api/debug_rpc_types.rs index 27fd38f2589dc..866454e3e8fbc 100644 --- a/substrate/frame/revive/src/evm/api/debug_rpc_types.rs +++ b/substrate/frame/revive/src/evm/api/debug_rpc_types.rs @@ -163,6 +163,8 @@ pub enum CallType { Create, /// A create2 call. Create2, + /// A selfdestruct call. + Selfdestruct, } /// A Trace diff --git a/substrate/frame/revive/src/evm/tracing/call_tracing.rs b/substrate/frame/revive/src/evm/tracing/call_tracing.rs index 9392dcb8fb3e5..d26f0c9581124 100644 --- a/substrate/frame/revive/src/evm/tracing/call_tracing.rs +++ b/substrate/frame/revive/src/evm/tracing/call_tracing.rs @@ -25,7 +25,10 @@ use sp_core::{H160, H256, U256}; /// A Tracer that reports logs and nested call traces transactions. #[derive(Default, Debug, Clone, PartialEq)] -pub struct CallTracer { +pub struct CallTracer +where + Gas: core::fmt::Debug, +{ /// Map Weight to Gas equivalent. gas_mapper: GasMapper, /// Store all in-progress CallTrace instances. @@ -38,7 +41,7 @@ pub struct CallTracer { config: CallTracerConfig, } -impl CallTracer { +impl CallTracer { /// Create a new [`CallTracer`] instance. pub fn new(config: CallTracerConfig, gas_mapper: GasMapper) -> Self { Self { @@ -56,11 +59,30 @@ impl CallTracer { } } -impl Gas> Tracing for CallTracer { +impl Gas> Tracing + for CallTracer +{ fn instantiate_code(&mut self, code: &Code, salt: Option<&[u8; 32]>) { self.code_with_salt = Some((code.clone(), salt.is_some())); } + fn terminate( + &mut self, + contract_address: H160, + beneficiary_address: H160, + gas_left: Weight, + value: U256, + ) { + self.traces.last_mut().unwrap().calls.push(CallTrace { + from: contract_address, + to: beneficiary_address, + call_type: CallType::Selfdestruct, + gas: (self.gas_mapper)(gas_left), + value: Some(value), + ..Default::default() + }); + } + fn enter_child_span( &mut self, from: H160, @@ -80,13 +102,14 @@ impl Gas> Tracing for CallTracer ( + Some((Code::Upload(code), salt)) => ( if salt { CallType::Create2 } else { CallType::Create }, - v.into_iter().chain(input.to_vec().into_iter()).collect::>(), + code.into_iter().chain(input.to_vec().into_iter()).collect::>(), ), - Some((Code::Existing(v), salt)) => ( + Some((Code::Existing(code_hash), salt)) => ( if salt { CallType::Create2 } else { CallType::Create }, - v.to_fixed_bytes() + code_hash + .to_fixed_bytes() .into_iter() .chain(input.to_vec().into_iter()) .collect::>(), diff --git a/substrate/frame/revive/src/evm/tracing/prestate_tracing.rs b/substrate/frame/revive/src/evm/tracing/prestate_tracing.rs index 738252d4fef72..80fd61e6d3d6d 100644 --- a/substrate/frame/revive/src/evm/tracing/prestate_tracing.rs +++ b/substrate/frame/revive/src/evm/tracing/prestate_tracing.rs @@ -40,6 +40,9 @@ pub struct PrestateTracer { /// List of created contracts addresses. created_addrs: BTreeSet, + /// List of destructed contracts addresses. + destructed_addrs: BTreeSet, + // pre / post state trace: (BTreeMap, BTreeMap), @@ -89,6 +92,11 @@ where } } + // collect destructed contracts info in post state + for addr in &self.destructed_addrs { + Self::update_prestate_info(post.entry(*addr).or_default(), addr, None); + } + // clean up the storage that are in pre but not in post these are just read pre.iter_mut().for_each(|(addr, info)| { if let Some(post_info) = post.get(addr) { @@ -98,6 +106,14 @@ where } }); + // If the address was created and destructed we do not trace it + post.retain(|addr, _| { + if self.created_addrs.contains(addr) && self.destructed_addrs.contains(addr) { + return false + } + true + }); + pre.retain(|addr, pre_info| { if is_empty(&pre_info) { return false @@ -221,6 +237,19 @@ where self.create_code = Some(code.clone()); } + fn terminate( + &mut self, + contract_address: H160, + beneficiary_address: H160, + _gas_left: Weight, + _value: U256, + ) { + self.destructed_addrs.insert(contract_address); + self.trace.0.entry(beneficiary_address).or_insert_with_key(|addr| { + Self::prestate_info(addr, Pallet::::evm_balance(addr), None) + }); + } + fn enter_child_span( &mut self, from: H160, diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index 2e5e05a08e040..8748e04b2d5a7 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -1728,6 +1728,15 @@ where } fn terminate_if_same_tx(&mut self, beneficiary: &H160) -> Result { + if_tracing(|tracer| { + let addr = T::AddressMapper::to_address(self.account_id()); + tracer.terminate( + addr, + *beneficiary, + self.top_frame().nested_gas.gas_left(), + crate::Pallet::::evm_balance(&addr), + ); + }); let (account_id, contract_address, contract_info) = { let frame = self.top_frame_mut(); if frame.entry_point == ExportedFunction::Constructor { diff --git a/substrate/frame/revive/src/tests/pvm.rs b/substrate/frame/revive/src/tests/pvm.rs index fc163113453b4..dabe8930d7b85 100644 --- a/substrate/frame/revive/src/tests/pvm.rs +++ b/substrate/frame/revive/src/tests/pvm.rs @@ -5189,3 +5189,164 @@ fn existential_deposit_shall_not_charged_twice() { assert_eq!(get_balance(&callee_account), Contracts::min_balance()); }); } + +#[test] +fn self_destruct_by_syscall_tracing_works() { + use crate::{ + evm::{PrestateTrace, PrestateTracer, PrestateTracerConfig, Tracer}, + Trace, + }; + + let (binary, _code_hash) = compile_module("self_destruct_by_syscall").unwrap(); + + struct TestCase { + description: &'static str, + create_tracer: Box Tracer>, + expected_trace_fn: Box) -> Trace>, + } + + let test_cases = vec![ + TestCase { + description: "CallTracer", + create_tracer: Box::new(|| { + Tracer::CallTracer(CallTracer::new(Default::default(), |_| U256::zero())) + }), + expected_trace_fn: Box::new(|addr, _binary| { + Trace::Call(CallTrace { + from: ALICE_ADDR, + to: addr, + call_type: CallType::Call, + value: Some(U256::zero()), + calls: vec![CallTrace { + from: addr, + to: DJANGO_ADDR, + call_type: CallType::Selfdestruct, + value: Some(Pallet::::convert_native_to_evm(100_000u64)), + ..Default::default() + }], + ..Default::default() + }) + }), + }, + TestCase { + description: "PrestateTracer (diff mode)", + create_tracer: Box::new(|| { + Tracer::PrestateTracer(PrestateTracer::new(PrestateTracerConfig { + diff_mode: true, + disable_storage: false, + disable_code: false, + })) + }), + expected_trace_fn: Box::new(|addr, binary| { + use alloy_core::hex; + + let json = r#"{ + "pre": { + "{{DJANGO_ADDR}}": { + "balance": "{{DJANGO_BALANCE}}" + }, + "{{CONTRACT_ADDR}}": { + "balance": "{{CONTRACT_BALANCE}}", + "nonce": 1, + "code": "{{CONTRACT_CODE}}" + } + }, + "post": { + "{{DJANGO_ADDR}}": { + "balance": "{{DJANGO_BALANCE_POST}}" + }, + "{{CONTRACT_ADDR}}": { + "balance": "0x0" + } + } + }"#; + + let django_balance = Pallet::::evm_balance(&DJANGO_ADDR); + let contract_balance = Pallet::::evm_balance(&addr); + let django_balance_post = contract_balance - 50_000_000u64; + + let json = json + .replace("{{DJANGO_ADDR}}", &format!("{:#x}", DJANGO_ADDR)) + .replace("{{CONTRACT_ADDR}}", &format!("{:#x}", addr)) + .replace("{{DJANGO_BALANCE}}", &format!("{:#x}", django_balance)) + .replace("{{CONTRACT_BALANCE}}", &format!("{:#x}", contract_balance)) + .replace("{{DJANGO_BALANCE_POST}}", &format!("{:#x}", django_balance_post)) + .replace("{{CONTRACT_CODE}}", &format!("0x{}", hex::encode(&binary))); + + let expected: PrestateTrace = serde_json::from_str(&json).unwrap(); + Trace::Prestate(expected) + }), + }, + TestCase { + description: "PrestateTracer (prestate mode)", + create_tracer: Box::new(|| { + Tracer::PrestateTracer(PrestateTracer::new(PrestateTracerConfig { + diff_mode: false, + disable_storage: false, + disable_code: false, + })) + }), + expected_trace_fn: Box::new(|addr, binary| { + use alloy_core::hex; + + let json = r#"{ + "{{ALICE_ADDR}}": { + "balance": "{{ALICE_BALANCE}}", + "nonce": 1 + }, + "{{CONTRACT_ADDR}}": { + "balance": "{{CONTRACT_BALANCE}}", + "nonce": 1, + "code": "{{CONTRACT_CODE}}" + }, + "{{DJANGO_ADDR}}": { + "balance": "{{DJANGO_BALANCE}}" + } + }"#; + + let alice_balance = Pallet::::evm_balance(&ALICE_ADDR); + let contract_balance = Pallet::::evm_balance(&addr); + let django_balance = Pallet::::evm_balance(&DJANGO_ADDR); + + let json = json + .replace("{{ALICE_ADDR}}", &format!("{:#x}", ALICE_ADDR)) + .replace("{{CONTRACT_ADDR}}", &format!("{:#x}", addr)) + .replace("{{DJANGO_ADDR}}", &format!("{:#x}", DJANGO_ADDR)) + .replace("{{ALICE_BALANCE}}", &format!("{:#x}", alice_balance)) + .replace("{{CONTRACT_BALANCE}}", &format!("{:#x}", contract_balance)) + .replace("{{DJANGO_BALANCE}}", &format!("{:#x}", django_balance)) + .replace("{{CONTRACT_CODE}}", &format!("0x{}", hex::encode(&binary))); + + let expected: PrestateTrace = serde_json::from_str(&json).unwrap(); + Trace::Prestate(expected) + }), + }, + ]; + + for TestCase { description, create_tracer, expected_trace_fn } in test_cases { + ExtBuilder::default().existential_deposit(50).build().execute_with(|| { + let _ = ::Currency::set_balance(&ALICE, 1_000_000); + + let Contract { addr, .. } = builder::bare_instantiate(Code::Upload(binary.clone())) + .native_value(100_000) + .build_and_unwrap_contract(); + + get_contract(&addr); + + let expected_trace = expected_trace_fn(addr, binary.clone()); + let mut tracer = create_tracer(); + trace(tracer.as_tracing(), || { + builder::call(addr).build().unwrap(); + }); + + let trace = tracer.collect_trace(); + + let trace_wrapped = trace.map(|t| match t { + crate::evm::Trace::Call(ct) => Trace::Call(ct), + crate::evm::Trace::Prestate(pt) => Trace::Prestate(pt), + }); + + assert_eq!(trace_wrapped, Some(expected_trace), "Trace mismatch for: {}", description); + }); + } +} diff --git a/substrate/frame/revive/src/tracing.rs b/substrate/frame/revive/src/tracing.rs index 1641ad1e33e6d..cbb400c5504e9 100644 --- a/substrate/frame/revive/src/tracing.rs +++ b/substrate/frame/revive/src/tracing.rs @@ -58,6 +58,16 @@ pub trait Tracing { ) { } + /// Called when a contract calls terminates (selfdestructs) + fn terminate( + &mut self, + _contract_address: H160, + _beneficiary_address: H160, + _gas_left: Weight, + _value: U256, + ) { + } + /// Record the next code and salt to be instantiated. fn instantiate_code(&mut self, _code: &Code, _salt: Option<&[u8; 32]>) {}