Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions crates/chain-state/src/in_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ impl<N: NodePrimitives> CanonicalInMemoryState<N> {
{
if self.inner.in_memory_state.blocks.read().get(&persisted_num_hash.hash).is_none() {
// do nothing
return
return;
}
}

Expand Down Expand Up @@ -542,7 +542,7 @@ impl<N: NodePrimitives> CanonicalInMemoryState<N> {
if let Some(tx) =
block_state.block_ref().recovered_block().body().transaction_by_hash(&hash)
{
return Some(tx.clone())
return Some(tx.clone());
}
}
None
Expand Down Expand Up @@ -869,6 +869,7 @@ mod tests {
use rand::Rng;
use reth_errors::ProviderResult;
use reth_ethereum_primitives::{EthPrimitives, Receipt};
use reth_execution_types::{FlatPreState, FlatWitnessRecord};
use reth_primitives_traits::{Account, Bytecode};
use reth_storage_api::{
AccountReader, BlockHashReader, BytecodeReader, HashedPostStateProvider,
Expand Down Expand Up @@ -1031,6 +1032,10 @@ mod tests {
) -> ProviderResult<Vec<Bytes>> {
Ok(Vec::default())
}

fn flat_witness(&self, _record: FlatWitnessRecord) -> ProviderResult<FlatPreState> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flat_witness is a new method added to the StateProofProvider. Analogous to the existing witness one. I'll ignore commenting on mock/test-utils impl of StateProofProvider, and only in the relevant ones.

Ok(FlatPreState::default())
}
}

#[test]
Expand Down
5 changes: 5 additions & 0 deletions crates/chain-state/src/memory_overlay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::ExecutedBlock;
use alloy_consensus::BlockHeader;
use alloy_primitives::{keccak256, Address, BlockNumber, Bytes, StorageKey, StorageValue, B256};
use reth_errors::ProviderResult;
use reth_execution_types::{FlatPreState, FlatWitnessRecord};
use reth_primitives_traits::{Account, Bytecode, NodePrimitives};
use reth_storage_api::{
AccountReader, BlockHashReader, BytecodeReader, HashedPostStateProvider, StateProofProvider,
Expand Down Expand Up @@ -203,6 +204,10 @@ impl<N: NodePrimitives> StateProofProvider for MemoryOverlayStateProviderRef<'_,
input.prepend_self(self.trie_input().clone());
self.historical.witness(input, target)
}

fn flat_witness(&self, record: FlatWitnessRecord) -> ProviderResult<FlatPreState> {
self.historical.flat_witness(record)
}
}

impl<N: NodePrimitives> HashedPostStateProvider for MemoryOverlayStateProviderRef<'_, N> {
Expand Down
19 changes: 12 additions & 7 deletions crates/engine/tree/src/tree/cached_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ use reth_errors::ProviderResult;
use reth_metrics::Metrics;
use reth_primitives_traits::{Account, Bytecode};
use reth_provider::{
AccountReader, BlockHashReader, BytecodeReader, HashedPostStateProvider, StateProofProvider,
StateProvider, StateRootProvider, StorageRootProvider,
AccountReader, BlockHashReader, BytecodeReader, FlatPreState, FlatWitnessRecord,
HashedPostStateProvider, StateProofProvider, StateProvider, StateRootProvider,
StorageRootProvider,
};
use reth_revm::db::BundleState;
use reth_trie::{
Expand Down Expand Up @@ -119,7 +120,7 @@ impl<S: AccountReader> AccountReader for CachedStateProvider<S> {
fn basic_account(&self, address: &Address) -> ProviderResult<Option<Account>> {
if let Some(res) = self.caches.account_cache.get(address) {
self.metrics.account_cache_hits.increment(1);
return Ok(res)
return Ok(res);
}

self.metrics.account_cache_misses.increment(1);
Expand Down Expand Up @@ -170,7 +171,7 @@ impl<S: BytecodeReader> BytecodeReader for CachedStateProvider<S> {
fn bytecode_by_hash(&self, code_hash: &B256) -> ProviderResult<Option<Bytecode>> {
if let Some(res) = self.caches.code_cache.get(code_hash) {
self.metrics.code_cache_hits.increment(1);
return Ok(res)
return Ok(res);
}

self.metrics.code_cache_misses.increment(1);
Expand Down Expand Up @@ -230,6 +231,10 @@ impl<S: StateProofProvider> StateProofProvider for CachedStateProvider<S> {
) -> ProviderResult<Vec<alloy_primitives::Bytes>> {
self.state_provider.witness(input, target)
}

fn flat_witness(&self, record: FlatWitnessRecord) -> ProviderResult<FlatPreState> {
self.state_provider.flat_witness(record)
}
}

impl<S: StorageRootProvider> StorageRootProvider for CachedStateProvider<S> {
Expand Down Expand Up @@ -410,7 +415,7 @@ impl ExecutionCache {
// If the account was not modified, as in not changed and not destroyed, then we have
// nothing to do w.r.t. this particular account and can move on
if account.status.is_not_modified() {
continue
continue;
}

// If the account was destroyed, invalidate from the account / storage caches
Expand All @@ -419,15 +424,15 @@ impl ExecutionCache {
self.account_cache.invalidate(addr);

invalidated_accounts.insert(addr);
continue
continue;
}

// If we have an account that was modified, but it has a `None` account info, some wild
// error has occurred because this state should be unrepresentable. An account with
// `None` current info, should be destroyed.
let Some(ref account_info) = account.info else {
trace!(target: "engine::caching", ?account, "Account with None account info found in state updates");
return Err(())
return Err(());
};

// Now we iterate over all storage and make updates to the cached storage values
Expand Down
9 changes: 7 additions & 2 deletions crates/engine/tree/src/tree/instrumented_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ use reth_errors::ProviderResult;
use reth_metrics::Metrics;
use reth_primitives_traits::{Account, Bytecode};
use reth_provider::{
AccountReader, BlockHashReader, BytecodeReader, HashedPostStateProvider, StateProofProvider,
StateProvider, StateRootProvider, StorageRootProvider,
AccountReader, BlockHashReader, BytecodeReader, FlatPreState, FlatWitnessRecord,
HashedPostStateProvider, StateProofProvider, StateProvider, StateRootProvider,
StorageRootProvider,
};
use reth_trie::{
updates::TrieUpdates, AccountProof, HashedPostState, HashedStorage, MultiProof,
Expand Down Expand Up @@ -251,6 +252,10 @@ impl<S: StateProofProvider> StateProofProvider for InstrumentedStateProvider<S>
) -> ProviderResult<Vec<alloy_primitives::Bytes>> {
self.state_provider.witness(input, target)
}

fn flat_witness(&self, record: FlatWitnessRecord) -> ProviderResult<FlatPreState> {
self.state_provider.flat_witness(record)
}
}

impl<S: StorageRootProvider> StorageRootProvider for InstrumentedStateProvider<S> {
Expand Down
3 changes: 3 additions & 0 deletions crates/evm/execution-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ pub use execute::*;
mod execution_outcome;
pub use execution_outcome::*;

pub mod witness;
pub use witness::*;

/// Bincode-compatible serde implementations for commonly used types for (EVM) block execution.
///
/// `bincode` crate doesn't work with optionally serializable serde fields, but some of the
Expand Down
63 changes: 63 additions & 0 deletions crates/evm/execution-types/src/witness.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
//! Witness recording types for EVM execution.
use alloy_primitives::{keccak256, Address, B256, U256};
use revm::{
database::{AccountStatus, DbAccount, State},
primitives::{HashMap, HashSet},
state::Bytecode,
};

/// Records pre-state data for witness generation.
#[derive(Debug, Clone, Default)]
pub struct FlatPreState {
/// Accounts accessed during execution.
pub accounts: HashMap<Address, DbAccount>,
/// Bytecode accessed during execution.
pub contracts: HashMap<B256, Bytecode>,
/// The set of addresses that have been self-destructed in the execution.
pub destructed_addresses: HashSet<Address>,
}
Comment on lines +10 to +19
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is basically the "state witness" used for the execution-only STF. It is the most important field of the new FlatExecutionWitness.

The need for destructed_addresses is a rabbit hole that I'll explain later. But for now feel safe to not pollute understanding.


/// Records pre-state accesses that occurred during execution.
#[derive(Debug, Clone, Default)]
pub struct FlatWitnessRecord {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FlatWitnessRecord is analogous to WitnessRecord.
You can refresh how this worked for WitnessRecord in the blockchain_test.rs file, but the TL:DR is that when we execute a block, we can pass a closure that returns a State<DB> that we can use to capture which state was accessed.

I'll create a comment in that file later in the review.

/// Accounts accessed during execution.
pub accounts: HashMap<Address, AccessedAccount>,
/// Bytecode accessed during execution.
pub contracts: HashMap<B256, Option<Bytecode>>,
}

/// Represents an accessed account during execution.
#[derive(Debug, Clone)]
pub enum AccessedAccount {
/// Indicates if the account was destroyed during execution.
Destroyed,
/// Storage keys accessed during execution.
StorageKeys(HashSet<U256>),
}
Comment on lines +32 to +37
Copy link
Collaborator Author

@jsign jsign Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good time to explain a quirk about how things work today in Reth regarding witness generation.

When the State<DB> is passed in the closure after the block execution. State<DB> basically has the in-memory cache of all the state accessed/written during the execution.

The way this is used for the usual ExecutionWitness is the same as for FlatExecutionWitness, but there's a catch.

If an account was destroyed during execution, State<DB> set the account as None, since it was destroyed. A priori that is okay to give that signal, but unfortunately that means that it wiped out all the storage slots captured during the execution.

This means that for destroyed accounts, we lost information on which storage slots were accessed before the account was SELFDESTROYed.

This has somewhat nasty implications for witness generation, since now we can't know which storage slots for this contract we must put in the witness pre-state.

When I realized this through a failing EEST test, I was surprised since wasn't obvious to me how to solve it. But.. in theory this should be already solved in the ExecutionWitness, so... how is this solved today then?

The answer is that Reth includes all storage slots of destroyed accounts. If that feels surprising, see here.

In that link, we see the get_proof_targets() used when building the MPT proof for the usual ExecutionWitness. For wiped (i.e., destroyed) storage tries, it includes all storage slots! And not only the storage slots used during execution, which means that probably the witness might be bloated.

Now, here's an important practical detail why this isn't a problem today:

  • Pre-cancun, SELFDESTROY indeed deletes the storage trie of a contract, thus situation can arise (i.e., witness bloating due to including all storage slots)
  • Post-cancun, there was an EIP that if you SELFDESTROY an account that exists, it won't destroy the storage slots anymore.

This means that for post-Cancun blocks, the wiped flag won't happen since the storage trie isn't really destroyed thus the bloating won't happen.

"In short", this "include all storage slots" is a bloating that can only happen for pre-Cancun blocks. While doesn't sound like an "urgent" problem, I think will be a problem whenever we want to standarize the witness generation (if the standard also covers older forks)... since clearly adding more storage slots as needed isn't required.

Sorry for the length, but I wanted to explain this correctly. This situation of SELFDESTRUCT having to add all the storage slots, means a decent amount of complexity in the Provider traits implementations, that we'll see later... but after you understand the problem, you can understand the "why" of those.

I've been thinking if there's a reason why they though would be needed to include all the slots, but the only reason I can think of is to workaround the problem that State<DB> lost information of accessed storage slots in the case the account is destroyed. For destroying a storage trie, we don't really need all the values since the account will be nuked from the account trie, so we don't need to know their values. I think at some point would be nice to ask Roman or someone else about this.

To be honest, I think for correct witness generation, ideally we want the State<DB> to properly register the pre-state more cleanly, insetad of only registering the addresses and storage slots (since State contains post-state values) and having to use Providers after to get the pre-state.

For this PR, I didn't want to change revm, etc -- but I think doing proper pre-state capturing in revm is probably the best thing to do in the long run. But before doing that, probably is worth discussing with them.


impl FlatWitnessRecord {
/// Records the accessed state after execution.
pub fn record_executed_state<DB>(&mut self, statedb: &State<DB>) {
self.contracts = statedb
.cache
.contracts
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's another thing that I believe is a problem today (both for FlatExecutionWitness and to the existing ExecutionWitness).

As I mentioned before, statedb.cache contains all hte state that was read/written during block execution. As said before, that's nice since we can know which accounts and storage slots where touched so we can get the pre-state.

But I think that statedb.cache.contracts also has newly created contracts, which we shouldn't include in the witness.

If that sounds suprising, see here (this is master code). I honestly, don't know why new contracts needs to be in the witness. New bytecode will be generated during the re-execution, so there's no need to include it.

In any case, even if we remove that .chain(...), I checked and new contracts will also be in statedb.cache.contracts (i.e., revm only goes to the database if doesn't have any contract bytecode cached there, which also includes new contracts (which makes sense)).

So... this also means that unless I'm not mistaken, there is probably extra bloating today of bytecodes in the execution witness. I'm thinking of creating some EEST test to triple-check this, but I feel confident about it (plus, that last link comment I think sounds wrong).

.values()
.map(|code| (keccak256(code.original_bytes()), Some(code.clone())))
.collect();

for (address, account) in &statedb.cache.accounts {
let account = match account.status {
AccountStatus::Destroyed => AccessedAccount::Destroyed,
_ => {
let storage_keys = account
.account
.as_ref()
.map_or_else(HashSet::default, |a| a.storage.keys().copied().collect());
AccessedAccount::StorageKeys(storage_keys)
}
};
self.accounts.insert(*address, account);
}
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two above comments are probably teh only "hairy" part of what was discovered while doing this PR -- the rest should feel more natural.

1 change: 1 addition & 0 deletions crates/revm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ reth-primitives-traits.workspace = true
reth-storage-errors.workspace = true
reth-storage-api.workspace = true
reth-trie = { workspace = true, optional = true }
reth-execution-types.workspace = true

# alloy
alloy-primitives.workspace = true
Expand Down
5 changes: 5 additions & 0 deletions crates/revm/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use alloc::vec::Vec;
use alloy_primitives::{
keccak256, map::HashMap, Address, BlockNumber, Bytes, StorageKey, B256, U256,
};
use reth_execution_types::{FlatPreState, FlatWitnessRecord};
use reth_primitives_traits::{Account, Bytecode};
use reth_storage_api::{
AccountReader, BlockHashReader, BytecodeReader, HashedPostStateProvider, StateProofProvider,
Expand Down Expand Up @@ -142,6 +143,10 @@ impl StateProofProvider for StateProviderTest {
fn witness(&self, _input: TrieInput, _target: HashedPostState) -> ProviderResult<Vec<Bytes>> {
unimplemented!("witness generation is not supported")
}

fn flat_witness(&self, record: FlatWitnessRecord) -> ProviderResult<FlatPreState> {
unimplemented!("flat witness generation is not supported")
}
}

impl HashedPostStateProvider for StateProviderTest {
Expand Down
1 change: 1 addition & 0 deletions crates/rpc/rpc-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ reth-engine-primitives.workspace = true
reth-network-peers.workspace = true
reth-trie-common.workspace = true
reth-chain-state.workspace = true
reth-stateless.workspace = true

# ethereum
alloy-eips.workspace = true
Expand Down
21 changes: 21 additions & 0 deletions crates/rpc/rpc-api/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use alloy_rpc_types_trace::geth::{
BlockTraceResult, GethDebugTracingCallOptions, GethDebugTracingOptions, GethTrace, TraceResult,
};
use jsonrpsee::{core::RpcResult, proc_macros::rpc};
use reth_stateless::flat_witness::FlatExecutionWitness;
use reth_trie_common::{updates::TrieUpdates, HashedPostState};

/// Debug rpc interface.
Expand Down Expand Up @@ -144,6 +145,16 @@ pub trait DebugApi<TxReq: RpcObject> {
async fn debug_execution_witness(&self, block: BlockNumberOrTag)
-> RpcResult<ExecutionWitness>;

/// The `debug_flatExecutionWitness` method allows for re-execution of a block with the purpose of
/// generating an execution-only witness.
///
/// The first argument is the block number or tag.
#[method(name = "flatExecutionWitness")]
async fn debug_flat_execution_witness(
&self,
block: BlockNumberOrTag,
) -> RpcResult<FlatExecutionWitness>;

/// The `debug_executionWitnessByBlockHash` method allows for re-execution of a block with the
/// purpose of generating an execution witness. The witness comprises of a map of all hashed
/// trie nodes to their preimages that were required during the execution of the block,
Expand All @@ -156,6 +167,16 @@ pub trait DebugApi<TxReq: RpcObject> {
hash: B256,
) -> RpcResult<ExecutionWitness>;

/// The `debug_executionWitnessByBlockHash` method allows for re-execution of a block with the
/// purpose of generating an execution-only witness.
///
/// The first argument is the block hash.
#[method(name = "flatExecutionWitnessByBlockHash")]
async fn debug_flat_execution_witness_by_block_hash(
&self,
hash: B256,
) -> RpcResult<FlatExecutionWitness>;

/// Sets the logging backtrace location. When a backtrace location is set and a log message is
/// emitted at that location, the stack of the goroutine executing the log statement will
/// be printed to stderr.
Expand Down
5 changes: 5 additions & 0 deletions crates/rpc/rpc-eth-types/src/cache/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use alloy_primitives::{Address, B256, U256};
use reth_errors::ProviderResult;
use reth_execution_types::{FlatPreState, FlatWitnessRecord};
use reth_revm::{database::StateProviderDatabase, DatabaseRef};
use reth_storage_api::{BytecodeReader, HashedPostStateProvider, StateProvider};
use reth_trie::{HashedStorage, MultiProofTargets};
Expand Down Expand Up @@ -105,6 +106,10 @@ impl reth_storage_api::StateProofProvider for StateProviderTraitObjWrapper<'_> {
) -> reth_errors::ProviderResult<Vec<alloy_primitives::Bytes>> {
self.0.witness(input, target)
}

fn flat_witness(&self, record: FlatWitnessRecord) -> ProviderResult<FlatPreState> {
self.0.flat_witness(record)
}
}

impl reth_storage_api::AccountReader for StateProviderTraitObjWrapper<'_> {
Expand Down
1 change: 1 addition & 0 deletions crates/rpc/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ reth-consensus.workspace = true
reth-consensus-common.workspace = true
reth-node-api.workspace = true
reth-trie-common.workspace = true
reth-stateless.workspace = true

# ethereum
alloy-evm = { workspace = true, features = ["overrides"] }
Expand Down
Loading
Loading