From 051962a4abab1ea7b40c97bb59f3ba01e6b0a3af Mon Sep 17 00:00:00 2001 From: Hubert Bugaj Date: Mon, 24 Aug 2026 14:49:03 +0200 Subject: [PATCH] fix(eth): respect tombstone --- CHANGELOG.md | 2 + src/rpc/methods/eth.rs | 17 +-- src/rpc/methods/eth/trace/state_diff.rs | 120 +++++++++++++++++++--- src/rpc/methods/eth/trace/test_helpers.rs | 35 ++++++- src/rpc/methods/eth/utils.rs | 44 ++++---- src/state_manager/message_simulation.rs | 12 +-- 6 files changed, 168 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b208bf8abe..a6aee13fc22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ ### Fixed +- [#7537](https://github.com/ChainSafe/forest/pull/7537): Self-destructed EVM contracts no longer report stale state in the trace RPCs. `Forest.EthTraceCall` (`trace_call`) `stateDiff` and `Forest.EthDebugTraceTransaction` (`debug_traceTransaction`) `prestateTracer` now report a zero nonce and empty storage for them, matching `Filecoin.EthGetTransactionCount`, `Filecoin.EthGetCode`, `Filecoin.EthGetStorageAt` and Lotus. Previously a contract that self-destructed during the traced message showed no storage change at all. + - [#5795](https://github.com/ChainSafe/forest/issues/5795): `Filecoin.ChainNotify` now closes the subscription channel when a client falls too far behind instead of silently dropping head changes, matching Lotus, so clients can detect the gap and resubscribe. ## Forest v0.36.0 "bafy2bzacedpdckv7nsqfjwqnqwtgu7ipqbox4tfuuhwxhdox27uuznfyv3o2g" diff --git a/src/rpc/methods/eth.rs b/src/rpc/methods/eth.rs index dd0263d1995..b80e08ecd3f 100644 --- a/src/rpc/methods/eth.rs +++ b/src/rpc/methods/eth.rs @@ -53,7 +53,7 @@ use crate::rpc::{ state::ApiInvocResult, types::{ApiTipsetKey, EventEntry, MessageLookup}, }; -use crate::shim::actors::{EVMActorStateLoad as _, eam, evm, is_evm_actor, system}; +use crate::shim::actors::{eam, system}; use crate::shim::address::{Address as FilecoinAddress, Protocol}; use crate::shim::crypto::Signature; use crate::shim::econ::{BLOCK_GAS_LIMIT, TokenAmount}; @@ -2305,13 +2305,10 @@ async fn eth_get_storage_at( eth_address: EthAddress, position: EthBytes, ) -> Result { - // `GetStorageAtParams::new` validates and left-pads the position to a 32-byte big-endian key, - // matching the actor. Validate before touching state, as Lotus does. + // Validate and left-pad before touching state, as Lotus does. let position = GetStorageAtParams::new(position.0)?.0; let to_address = FilecoinAddress::try_from(ð_address)?; let TipsetState { state_root, .. } = ctx.state_manager.load_tipset_state(ts).await?; - // Read the slot straight from the EVM actor's storage KAMT (see `eth_storage_at`), instead of - // invoking its `GetStorageAt` method through the VM. let value = match ctx .state_manager .get_actor(&to_address, state_root) @@ -2369,15 +2366,7 @@ async fn eth_get_transaction_count( None => return Ok(EthUint64(0)), }; - if is_evm_actor(&actor.code) { - let evm_state = evm::State::load(ctx.db(), actor.code, actor.state)?; - if !evm_state.is_alive() { - return Ok(EthUint64(0)); - } - Ok(EthUint64(evm_state.nonce())) - } else { - Ok(EthUint64(actor.sequence)) - } + Ok(actor.eth_nonce(ctx.db())?) } pub enum EthMaxPriorityFeePerGas {} diff --git a/src/rpc/methods/eth/trace/state_diff.rs b/src/rpc/methods/eth/trace/state_diff.rs index 581e4cbe44b..edf9e2ef3a5 100644 --- a/src/rpc/methods/eth/trace/state_diff.rs +++ b/src/rpc/methods/eth/trace/state_diff.rs @@ -8,15 +8,14 @@ use super::super::EthBigInt; use super::super::types::{EthAddress, EthHash}; -use super::super::utils::{ActorStateEthExt as _, EvmStorageKamt, evm_kamt_config}; +use super::super::utils::{ActorStateEthExt as _, EvmStorageKamt, evm_kamt_config, live_evm_state}; use super::types::{AccountDiff, ChangedType, Delta, StateDiff}; use super::utils::{ZERO_HASH, u256_to_eth_hash}; use crate::prelude::*; use crate::shim::actors::evm::U256; -use crate::shim::actors::{EVMActorStateLoad as _, evm, is_evm_actor}; +use crate::shim::actors::is_evm_actor; use crate::shim::state_tree::{ActorState, StateTree}; use ahash::{HashMap, HashSet}; -use fvm_ipld_kamt::Kamt; use std::collections::BTreeMap; use tracing::debug; @@ -92,6 +91,8 @@ fn build_account_diff( /// - Account deleted (EVM → None): storage slots are `Delta::Removed` /// - Account modified (EVM → EVM): storage slots are `Delta::Changed` /// - Actor type changed (EVM ↔ non-EVM): treated as deletion + creation +/// - Contract self-destructed (EVM → tombstoned EVM): its storage reads as empty, so the slots are +/// `Delta::Changed` to zero fn diff_evm_storage_for_actors( store: &DB, pre_actor: Option<&ActorState>, @@ -178,18 +179,19 @@ fn diff_evm_storage_for_actors( } /// Extract all storage entries from an EVM actor's KAMT. -/// Returns empty map if actor is None, not an EVM actor, or state cannot be loaded. +/// Returns an empty map if the actor is absent, not an EVM actor, self-destructed, or its state +/// cannot be loaded. pub fn extract_evm_storage_entries( store: &DB, actor: Option<&ActorState>, ) -> HashMap<[u8; 32], U256> { - let actor = match actor { - Some(a) if is_evm_actor(&a.code) => a, - _ => return HashMap::default(), + let Some(actor) = actor else { + return HashMap::default(); }; - - let evm_state = match evm::State::load(store, actor.code, actor.state) { - Ok(state) => state, + let evm_state = match live_evm_state(store, actor) { + Ok(Some(state)) => state, + // Not an EVM actor, or a dead (self-destructed) contract: its storage reads as empty. + Ok(None) => return HashMap::default(), Err(e) => { debug!("failed to load EVM state for storage extraction: {e:#}"); return HashMap::default(); @@ -199,7 +201,7 @@ pub fn extract_evm_storage_entries( let storage_cid = evm_state.contract_state(); let config = evm_kamt_config(); - let kamt: EvmStorageKamt<&DB> = match Kamt::load_with_config(&storage_cid, store, config) { + let kamt = match EvmStorageKamt::load_with_config(&storage_cid, store, config) { Ok(k) => k, Err(e) => { debug!("failed to load storage KAMT: {e}"); @@ -252,6 +254,7 @@ mod tests { use crate::rpc::eth::EthUint64; use crate::rpc::eth::types::EthBytes; use crate::shim::address::Address as FilecoinAddress; + use crate::shim::econ::TokenAmount; use crate::shim::state_tree::StateTreeVersion; #[test] @@ -716,13 +719,104 @@ mod tests { #[test] fn test_actor_bytecode_evm_tombstoned() { let store = Arc::new(MemoryDB::default()); - let actor = create_tombstoned_evm_actor(&store, &[0x60, 0x80, 0x60, 0x40, 0x52]) - .expect("failed to create tombstoned EVM actor fixture"); + let actor = + create_tombstoned_evm_actor(&store, &[0x60, 0x80, 0x60, 0x40, 0x52], 0, Cid::default()) + .expect("failed to create tombstoned EVM actor fixture"); // A self-destructed contract reports no code even though its bytecode is present, // matching the EVM actor's GetBytecode. assert!(actor.eth_bytecode(store.as_ref()).unwrap().is_none()); } + /// Builds a storage KAMT holding `slot -> value` and returns its root. + fn storage_with_slot(store: &MemoryDB, slot: u64, value: u64) -> Cid { + let mut kamt = EvmStorageKamt::new_with_config(store, evm_kamt_config()); + kamt.set(U256::from(slot), U256::from(value)).unwrap(); + kamt.flush().unwrap() + } + + /// A self-destructed contract reports a zero nonce even though its state carries one, mirroring + /// Lotus's `itests/eth_bytecode_test.go` (nonce is 1 after deploy, zero after `destroy()`). + #[test] + fn test_actor_nonce_evm_tombstoned() { + let store = MemoryDB::default(); + // Actor sequence 0 but EVM nonce 7, so this also pins that the EVM nonce is what's read. + let alive = create_evm_actor_with_bytecode(&store, 0, 0, 7, Some(&[0x60])) + .expect("failed to create EVM actor fixture"); + assert_eq!(alive.eth_nonce(&store).unwrap().0, 7); + + let mut dead = create_tombstoned_evm_actor(&store, &[0x60], 7, Cid::default()) + .expect("failed to create tombstoned EVM actor fixture"); + // Nonzero sequence, so falling through to the non-EVM branch would not also yield 0. + dead.sequence = 9; + assert_eq!(dead.eth_nonce(&store).unwrap().0, 0); + } + + /// A self-destructed contract's storage reads as empty, matching the EVM actor's `System::load`. + #[test] + fn test_extract_evm_storage_entries_tombstoned() { + let store = MemoryDB::default(); + let contract_state = storage_with_slot(&store, 5, 42); + + // Sanity: the same KAMT is visible while the contract is alive. + let alive = create_evm_actor_with_storage(&store, contract_state) + .expect("failed to create EVM actor fixture"); + assert_eq!( + extract_evm_storage_entries(&store, Some(&alive)).len(), + 1, + "live contract must expose its slots" + ); + + let dead = create_tombstoned_evm_actor(&store, &[0x60], 0, contract_state) + .expect("failed to create tombstoned EVM actor fixture"); + assert!( + extract_evm_storage_entries(&store, Some(&dead)).is_empty(), + "self-destructed contract must read as empty storage" + ); + } + + /// An absent actor, or one whose EVM state cannot be loaded, reads as empty storage. + #[test] + fn test_extract_evm_storage_entries_absent_or_unloadable() { + let store = MemoryDB::default(); + assert!(extract_evm_storage_entries(&store, None).is_empty()); + + // EVM code CID, but the state block is not in the store. + let unloadable = ActorState::new( + get_evm_actor_code_cid().expect("bundled EVM actor code CID"), + Cid::default(), + TokenAmount::default(), + 0, + None, + ); + assert!( + extract_evm_storage_entries(&store, Some(&unloadable)).is_empty(), + "unloadable EVM state must read as empty storage" + ); + } + + /// Self-destructing clears the slots, so the diff reports them going to zero rather than + /// reporting no change at all. + #[test] + fn test_diff_evm_storage_reports_slots_cleared_on_selfdestruct() { + let store = MemoryDB::default(); + let contract_state = storage_with_slot(&store, 5, 42); + let pre = create_evm_actor_with_storage(&store, contract_state) + .expect("failed to create EVM actor fixture"); + let post = create_tombstoned_evm_actor(&store, &[0x60], 0, contract_state) + .expect("failed to create tombstoned EVM actor fixture"); + + let diff = diff_evm_storage_for_actors(&store, Some(&pre), Some(&post)).unwrap(); + let slot = u256_to_eth_hash(&U256::from(5u64)); + assert_eq!( + diff.get(&slot), + Some(&Delta::Changed(ChangedType { + from: u256_to_eth_hash(&U256::from(42u64)), + to: ZERO_HASH, + })), + "destroyed contract's slot must be reported as cleared" + ); + } + #[test] fn test_diff_entry_keys_both_empty() { let pre = HashMap::default(); diff --git a/src/rpc/methods/eth/trace/test_helpers.rs b/src/rpc/methods/eth/trace/test_helpers.rs index c31bb3f0a14..2556386b6c0 100644 --- a/src/rpc/methods/eth/trace/test_helpers.rs +++ b/src/rpc/methods/eth/trace/test_helpers.rs @@ -88,9 +88,34 @@ pub fn create_evm_actor_with_bytecode( )) } -/// Like [`create_evm_actor_with_bytecode`] but marks the actor self-destructed via a -/// tombstone, so it must report no bytecode even though the code block is present. -pub fn create_tombstoned_evm_actor(store: &MemoryDB, bytecode: &[u8]) -> Option { +/// An alive EVM actor whose storage KAMT root is `contract_state`. +pub fn create_evm_actor_with_storage(store: &MemoryDB, contract_state: Cid) -> Option { + let evm_state = fil_actor_evm_state::v17::State { + bytecode: Cid::default(), + bytecode_hash: fil_actor_evm_state::v17::BytecodeHash::EMPTY, + contract_state, + transient_data: None, + nonce: 0, + tombstone: None, + }; + Some(ActorState::new( + get_evm_actor_code_cid()?, + store.put_cbor_default(&evm_state).ok()?, + TokenAmount::from_atto(0), + 0, + None, + )) +} + +/// Like [`create_evm_actor_with_bytecode`] but marks the actor self-destructed via a tombstone, +/// so it must report no bytecode, a zero nonce and empty storage even though `bytecode`, +/// `evm_nonce` and `contract_state` are all populated. +pub fn create_tombstoned_evm_actor( + store: &MemoryDB, + bytecode: &[u8], + evm_nonce: u64, + contract_state: Cid, +) -> Option { use fvm_ipld_blockstore::Blockstore as _; use multihash_codetable::MultihashDigest as _; @@ -104,9 +129,9 @@ pub fn create_tombstoned_evm_actor(store: &MemoryDB, bytecode: &[u8]) -> Option< bytecode_hash: fil_actor_evm_state::v17::BytecodeHash::from( keccak_hash::keccak(bytecode).0, ), - contract_state: Cid::default(), + contract_state, transient_data: None, - nonce: 0, + nonce: evm_nonce, tombstone: Some(fil_actor_evm_state::v17::Tombstone { origin: 0, nonce: 0, diff --git a/src/rpc/methods/eth/utils.rs b/src/rpc/methods/eth/utils.rs index bafb1c1b499..d69bf3f2646 100644 --- a/src/rpc/methods/eth/utils.rs +++ b/src/rpc/methods/eth/utils.rs @@ -85,24 +85,29 @@ pub fn lookup_eth_address( Ok(Some(EthAddress::from_actor_id(id_addr))) } -/// The actor's EVM state, or `None` when it is not an EVM actor or is a dead (self-destructed) -/// contract, which reads as empty. -// -fn live_evm_state( - actor: &ActorState, - store: &DB, -) -> anyhow::Result> { +/// The actor's EVM state, or `None` when it is not an EVM actor. +fn evm_state(store: &DB, actor: &ActorState) -> anyhow::Result> { if !is_evm_actor(&actor.code) { return Ok(None); } - let state = - evm::State::load(store, actor.code, actor.state).context("failed to load EVM state")?; - Ok(state.is_alive().then_some(state)) + Ok(Some( + evm::State::load(store, actor.code, actor.state).context("failed to load EVM state")?, + )) +} + +/// As [`evm_state`], but also `None` for a dead (self-destructed) contract, which reads as empty. +// +pub(crate) fn live_evm_state( + store: &DB, + actor: &ActorState, +) -> anyhow::Result> { + Ok(evm_state(store, actor)?.filter(|state| state.is_alive())) } /// Extension trait for querying Ethereum-relevant state from a Filecoin actor. pub(crate) trait ActorStateEthExt { - /// Returns the effective nonce: EVM nonce for EVM actors, sequence otherwise. + /// Returns the effective nonce: EVM nonce for EVM actors (zero once self-destructed), + /// sequence otherwise. fn eth_nonce(&self, store: &DB) -> anyhow::Result; /// Returns the deployed bytecode of an EVM actor, or `None` for non-EVM or self-destructed actors. fn eth_bytecode(&self, store: &DB) -> anyhow::Result>; @@ -117,17 +122,16 @@ pub(crate) trait ActorStateEthExt { impl ActorStateEthExt for ActorState { fn eth_nonce(&self, store: &DB) -> anyhow::Result { - if is_evm_actor(&self.code) { - let evm_state = evm::State::load(store, self.code, self.state) - .context("failed to load EVM state for nonce")?; - Ok(EthUint64::from(evm_state.nonce())) - } else { - Ok(EthUint64::from(self.sequence)) - } + Ok(EthUint64(match evm_state(store, self)? { + Some(state) if state.is_alive() => state.nonce(), + // A dead contract's state still carries a nonce, but it reports zero. + Some(_) => 0, + None => self.sequence, + })) } fn eth_bytecode(&self, store: &DB) -> anyhow::Result> { - let Some(evm_state) = live_evm_state(self, store)? else { + let Some(evm_state) = live_evm_state(store, self)? else { return Ok(None); }; let bytecode = store @@ -143,7 +147,7 @@ impl ActorStateEthExt for ActorState { ) -> anyhow::Result<[u8; EVM_WORD_LENGTH]> { // Mirrors the EVM actor's `GetStorageAt`. // - let Some(evm_state) = live_evm_state(self, store)? else { + let Some(evm_state) = live_evm_state(store, self)? else { return Ok([0; EVM_WORD_LENGTH]); }; let kamt = diff --git a/src/state_manager/message_simulation.rs b/src/state_manager/message_simulation.rs index 6de69045381..4eca1fa7439 100644 --- a/src/state_manager/message_simulation.rs +++ b/src/state_manager/message_simulation.rs @@ -13,8 +13,9 @@ use std::time::Duration; use tracing::instrument; impl StateManager { + /// Blocking version of [`Self::call`], use with caution. #[instrument(skip(self))] - fn call_raw_blocking( + pub fn call_blocking( &self, msg: &Message, tipset: Option, @@ -141,15 +142,6 @@ impl StateManager { tokio::task::spawn_blocking(move || this.call_blocking(&message, tipset)).await? } - /// Blocking version of [`Self::call`], use with caution. - pub fn call_blocking( - &self, - message: &Message, - tipset: Option, - ) -> Result { - self.call_raw_blocking(message, tipset) - } - pub async fn apply_on_state_with_gas( &self, tipset: Option,