Skip to content

Commit e159a65

Browse files
authored
fix(tracing): correctly handle selfdestructed accounts in post state (#377)
Closes #376 - Don't insert selfdestructed accounts in post state - Added tests to verify behavior for London and Cancun
1 parent ee2021d commit e159a65

File tree

2 files changed

+113
-1
lines changed

2 files changed

+113
-1
lines changed

src/tracing/builder/geth.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,6 @@ impl<'a> GethTraceBuilder<'a> {
316316
}
317317

318318
state_diff.pre.insert(addr, pre_state);
319-
state_diff.post.insert(addr, post_state);
320319

321320
// determine the change type
322321
let pre_change = if changed_acc.is_created() {
@@ -331,6 +330,11 @@ impl<'a> GethTraceBuilder<'a> {
331330
};
332331

333332
account_change_kinds.insert(addr, (pre_change, post_change));
333+
334+
// Don't insert selfdestructed accounts into post state
335+
if !changed_acc.is_selfdestructed() {
336+
state_diff.post.insert(addr, post_state);
337+
}
334338
}
335339

336340
// ensure we're only keeping changed entries

tests/it/geth.rs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,3 +701,111 @@ fn test_geth_calltracer_null_bytes_revert_reason_omitted() {
701701
"revertReason field should not be present in JSON when containing only null bytes"
702702
);
703703
}
704+
705+
#[test]
706+
fn test_geth_prestate_diff_selfdestruct_london() {
707+
test_geth_prestate_diff_selfdestruct(SpecId::LONDON);
708+
}
709+
710+
#[test]
711+
fn test_geth_prestate_diff_selfdestruct_cancun() {
712+
test_geth_prestate_diff_selfdestruct(SpecId::CANCUN);
713+
}
714+
715+
fn test_geth_prestate_diff_selfdestruct(spec_id: SpecId) {
716+
/*
717+
contract DummySelfDestruct {
718+
constructor() payable {}
719+
function close() public {
720+
selfdestruct(payable(msg.sender));
721+
}
722+
}
723+
*/
724+
725+
// simple contract that selfdestructs when a function is called
726+
let code = hex!("608080604052606b908160108239f3fe6004361015600c57600080fd5b6000803560e01c6343d726d614602157600080fd5b346032578060031936011260325733ff5b80fdfea2646970667358221220f393fc6be90126d52315ccd38ae6608ac4fd5bef4c59e119e280b2a2b149d0dc64736f6c63430008190033");
727+
728+
let deployer = alloy_primitives::address!("341348115259a8bf69f1f50101c227fced83bac6");
729+
let value = alloy_primitives::U256::from(69);
730+
731+
// Deploy the contract in a separate transaction first
732+
let mut context = Context::mainnet()
733+
.with_db(CacheDB::<EmptyDB>::default())
734+
.modify_db_chained(|db| {
735+
db.insert_account_info(
736+
deployer,
737+
revm::state::AccountInfo { balance: value, ..Default::default() },
738+
);
739+
})
740+
.modify_cfg_chained(|cfg| cfg.spec = spec_id);
741+
742+
context.modify_tx(|tx| tx.value = value);
743+
let mut evm = context.build_mainnet();
744+
let output = deploy_contract(&mut evm, code.into(), deployer, spec_id);
745+
let addr = output.created_address().unwrap();
746+
747+
// Create inspector with diff mode enabled
748+
let prestate_config = PreStateConfig {
749+
diff_mode: Some(true),
750+
disable_code: Some(false),
751+
disable_storage: Some(false),
752+
};
753+
let insp =
754+
TracingInspector::new(TracingInspectorConfig::from_geth_prestate_config(&prestate_config));
755+
756+
let db = evm.ctx().db().clone();
757+
let mut evm = evm.with_inspector(insp);
758+
759+
let res = evm
760+
.inspect_tx(TxEnv {
761+
caller: deployer,
762+
gas_limit: 1000000,
763+
kind: TransactTo::Call(addr),
764+
data: hex!("43d726d6").into(),
765+
nonce: 1,
766+
..Default::default()
767+
})
768+
.unwrap();
769+
770+
assert!(res.result.is_success(), "{res:#?}");
771+
772+
// Get the prestate diff traces
773+
let insp = evm.into_inspector();
774+
let frame = insp
775+
.with_transaction_gas_used(res.result.gas_used())
776+
.geth_builder()
777+
.geth_prestate_traces(&res, &prestate_config, db)
778+
.unwrap();
779+
780+
match frame {
781+
PreStateFrame::Diff(diff_mode) => {
782+
// In LONDON, selfdestruct actually destroys the account
783+
// In CANCUN+, due to EIP-6780, selfdestruct on an existing contract only transfers
784+
// funds and doesn't actually destroy it
785+
let is_post_cancun = spec_id >= SpecId::CANCUN;
786+
787+
if is_post_cancun {
788+
// In CANCUN+, the account is NOT selfdestructed (just balance transfer)
789+
// so it WILL be in post state with a changed balance
790+
assert!(
791+
diff_mode.post.contains_key(&addr),
792+
"In CANCUN+, non-selfdestructed account should be in post state"
793+
);
794+
} else {
795+
// In pre-CANCUN (e.g. LONDON), the account IS selfdestructed
796+
// so it should NOT be in the post state
797+
assert!(
798+
!diff_mode.post.contains_key(&addr),
799+
"Selfdestructed account should NOT be in post state (LONDON)"
800+
);
801+
802+
// The account should be in pre state since it existed before
803+
assert!(
804+
diff_mode.pre.contains_key(&addr),
805+
"Selfdestructed account should be in pre state (LONDON)"
806+
);
807+
}
808+
}
809+
_ => panic!("Expected Diff mode PreStateFrame"),
810+
}
811+
}

0 commit comments

Comments
 (0)