Skip to content

Commit b7a14e2

Browse files
authored
fix(l1): improve witness generation spec conformance (#6877)
**Motivation** Currently we are failing several [witness generation tests](https://eth-act.github.io/eest-execution-witness-dashboard/#/group/execution-witness). **Description** Fixes the ordering by sorting, and the over-inclusion by: - Disabling both BAL and speculative warming when an execution witness is requested - While disabling BAL warming isn't strictly needed, it's the simpler solution. There are several edge-cases around accounts created within the block that have to be handled carefully. - Fixing over-accessing in OOG scenarios (call that runs out of gas right after finding out the target is a eip-7702 delegation, or paying the cold access cost, etc) Also adds a test runner for the execution witness tests. You can run the tests with: ``` cd tooling/ef_tests/blockchain && make test-stateless cd tooling/ef_tests/engine && make test ``` There are also hive [tests](https://github.com/eth-act/eest-execution-witness-dashboard) which take significantly longer to execute.
1 parent b32352f commit b7a14e2

16 files changed

Lines changed: 673 additions & 162 deletions

File tree

crates/blockchain/blockchain.rs

Lines changed: 78 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -551,32 +551,27 @@ impl Blockchain {
551551
vm.db.store = caching_store.clone();
552552

553553
let cancelled = AtomicBool::new(false);
554-
let bal_parallel_exec_enabled = self.options.bal_parallel_exec_enabled;
554+
// Witness collection also forces sequential execution: parallel lanes
555+
// re-read in-block-created state (e.g. a code deployed by an earlier
556+
// tx) from the logged store, while sequential execution serves it from
557+
// VM caches — recording accesses the canonical execution never makes.
558+
let bal_parallel_exec_enabled = self.options.bal_parallel_exec_enabled && !collect_witness;
555559

556560
// Synthesize BAL updates pre-scope so the merkleizer thread can start
557561
// trie work immediately, in parallel with execution.
558562
// `--no-bal-parallel-trie` opts out: leave `optimistic_updates = None` so
559563
// the merkleizer takes the streaming branch (fed by the EVM-side
560564
// `bal_to_account_updates` send over the channel below).
565+
// Witness collection forces the streaming branch too: the sequential
566+
// executor (see `bal_parallel_exec_enabled` below) streams per-tx
567+
// updates over the channel, which only the streaming merkleizer
568+
// consumes — the synthesized path would leave the receiver dropped.
561569
let optimistic_updates: Option<FxHashMap<Address, BalSynthesisItem>> =
562-
if self.options.bal_parallel_trie_enabled {
570+
if self.options.bal_parallel_trie_enabled && !collect_witness {
563571
bal.map(synthesize_bal_updates)
564572
} else {
565573
None
566574
};
567-
let optimistic_witness: Option<Vec<AccountUpdate>> = if collect_witness {
568-
optimistic_updates.as_ref().map(|m| {
569-
m.iter()
570-
.map(|(addr, item)| AccountUpdate {
571-
address: *addr,
572-
added_storage: item.added_storage.clone(),
573-
..Default::default()
574-
})
575-
.collect()
576-
})
577-
} else {
578-
None
579-
};
580575

581576
// Synchronously warm all BAL storage slots before the executor thread starts.
582577
//
@@ -607,8 +602,14 @@ impl Blockchain {
607602
// Gated by `--no-bal-prefetch`: when the operator disables BAL-driven
608603
// prefetching, skip the synchronous storage warm too. The warmer thread
609604
// below already honors the same toggle.
605+
// Witness collection records every read that reaches the store-backed
606+
// logger beneath the shared cache. The warmer's speculative reads would
607+
// be recorded as state accesses the canonical execution never makes,
608+
// polluting the witness (e.g. `engine_newPayloadWithWitnessV5`), so
609+
// warming is skipped entirely when a witness is being collected.
610610
#[cfg(all(feature = "rayon", not(feature = "eip-8025")))]
611611
if self.options.bal_prefetch_enabled
612+
&& !collect_witness
612613
&& let Some(bal) = bal
613614
{
614615
let slots = LEVM::bal_storage_slots(bal);
@@ -625,53 +626,59 @@ impl Blockchain {
625626
#[cfg(all(feature = "rayon", not(feature = "eip-8025")))]
626627
let bal_prefetch_enabled = self.options.bal_prefetch_enabled;
627628
#[cfg(all(feature = "rayon", not(feature = "eip-8025")))]
628-
let warm_handle = std::thread::Builder::new()
629-
.name("block_executor_warmer".to_string())
630-
.spawn_scoped(s, move || {
631-
// Warming uses the same caching store, sharing cached state with execution.
632-
// Precompile cache lives inside CachingDatabase, shared automatically.
633-
let start = Instant::now();
634-
if let Some(bal) = bal {
635-
if bal_prefetch_enabled {
636-
// Amsterdam+: BAL-based precise prefetching (no tx re-execution).
637-
if let Err(e) =
638-
LEVM::warm_block_from_bal(bal, caching_store, cancelled_ref)
639-
{
640-
debug!("BAL warming failed (non-fatal): {e}");
641-
}
642-
} else if !bal_parallel_exec_enabled {
643-
// --no-bal-prefetch combined with --no-bal-parallel-exec:
644-
// mirror the pre-Amsterdam setup where a parallel speculative
645-
// warmer races ahead of the serial executor. With parallel
646-
// exec still on, we skip warming instead — two parallel passes
647-
// over the same txs would just fight for cores.
648-
if let Err(e) = LEVM::warm_block(
649-
block,
650-
caching_store,
651-
vm_type,
652-
&NativeCrypto,
653-
cancelled_ref,
654-
) {
655-
debug!("Block warming failed (non-fatal): {e}");
629+
let warm_handle = (!collect_witness)
630+
.then(|| {
631+
std::thread::Builder::new()
632+
.name("block_executor_warmer".to_string())
633+
.spawn_scoped(s, move || {
634+
// Warming uses the same caching store, sharing cached state with execution.
635+
// Precompile cache lives inside CachingDatabase, shared automatically.
636+
let start = Instant::now();
637+
if let Some(bal) = bal {
638+
if bal_prefetch_enabled {
639+
// Amsterdam+: BAL-based precise prefetching (no tx re-execution).
640+
if let Err(e) = LEVM::warm_block_from_bal(
641+
bal,
642+
caching_store,
643+
cancelled_ref,
644+
) {
645+
debug!("BAL warming failed (non-fatal): {e}");
646+
}
647+
} else if !bal_parallel_exec_enabled {
648+
// --no-bal-prefetch combined with --no-bal-parallel-exec:
649+
// mirror the pre-Amsterdam setup where a parallel speculative
650+
// warmer races ahead of the serial executor. With parallel
651+
// exec still on, we skip warming instead — two parallel passes
652+
// over the same txs would just fight for cores.
653+
if let Err(e) = LEVM::warm_block(
654+
block,
655+
caching_store,
656+
vm_type,
657+
&NativeCrypto,
658+
cancelled_ref,
659+
) {
660+
debug!("Block warming failed (non-fatal): {e}");
661+
}
662+
}
663+
} else {
664+
// Pre-Amsterdam / P2P sync: speculative tx re-execution
665+
if let Err(e) = LEVM::warm_block(
666+
block,
667+
caching_store,
668+
vm_type,
669+
&NativeCrypto,
670+
cancelled_ref,
671+
) {
672+
debug!("Block warming failed (non-fatal): {e}");
673+
}
656674
}
657-
}
658-
} else {
659-
// Pre-Amsterdam / P2P sync: speculative tx re-execution
660-
if let Err(e) = LEVM::warm_block(
661-
block,
662-
caching_store,
663-
vm_type,
664-
&NativeCrypto,
665-
cancelled_ref,
666-
) {
667-
debug!("Block warming failed (non-fatal): {e}");
668-
}
669-
}
670-
start.elapsed()
675+
start.elapsed()
676+
})
677+
.map_err(|e| {
678+
ChainError::Custom(format!("Failed to spawn warmer thread: {e}"))
679+
})
671680
})
672-
.map_err(|e| {
673-
ChainError::Custom(format!("Failed to spawn warmer thread: {e}"))
674-
})?;
681+
.transpose()?;
675682
let max_queue_length_ref = &mut max_queue_length;
676683
// Channel is needed whenever the merkleizer takes the streaming
677684
// branch OR LEVM falls into the sequential path:
@@ -817,9 +824,13 @@ impl Blockchain {
817824
});
818825
#[cfg(all(feature = "rayon", not(feature = "eip-8025")))]
819826
let warmer_duration = warm_handle
820-
.join()
821-
.inspect_err(|e| warn!("Warming thread error: {e:?}"))
822-
.ok()
827+
.map(|handle| {
828+
handle
829+
.join()
830+
.inspect_err(|e| warn!("Warming thread error: {e:?}"))
831+
.ok()
832+
.unwrap_or(Duration::ZERO)
833+
})
823834
.unwrap_or(Duration::ZERO);
824835
#[cfg(any(not(feature = "rayon"), feature = "eip-8025"))]
825836
let warmer_duration = Duration::ZERO;
@@ -829,8 +840,10 @@ impl Blockchain {
829840
merkleization_result?;
830841
let (execution_result, produced_bal, exec_end_instant) = execution_result?;
831842

832-
// Synthesized witness wins when BAL is present; streaming witness wins otherwise.
833-
let accumulated_updates = optimistic_witness.or(streaming_witness);
843+
// Witness collection forces the streaming merkleizer (synthesized
844+
// updates are disabled above), so the streaming witness is the only
845+
// possible source of accumulated updates.
846+
let accumulated_updates = streaming_witness;
834847

835848
let exec_merkle_end_instant = Instant::now();
836849

crates/common/types/block_execution_witness.rs

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use crate::{
1313
use ethereum_types::{Address, H256, U256};
1414
use ethrex_crypto::{Crypto, NativeCrypto};
1515
use ethrex_rlp::error::RLPDecodeError;
16+
use ethrex_rlp::structs::{Decoder, Encoder};
1617
use ethrex_rlp::{decode::RLPDecode, encode::RLPEncode};
1718
use ethrex_trie::{EMPTY_TRIE_HASH, Nibbles, Node, NodeRef, Trie, TrieError};
1819
use rkyv::with::{Identity, MapKV};
@@ -124,19 +125,80 @@ impl TryFrom<ExecutionWitness> for RpcExecutionWitness {
124125
for node in value.storage_trie_roots.values() {
125126
node.encode_subtrie(&mut nodes)?;
126127
}
128+
// Canonical witness ordering (EIP-8025, geth `ExtWitness`): state nodes
129+
// and codes sorted lexicographically and deduplicated (identical
130+
// storage subtries would otherwise emit identical nodes twice); headers
131+
// ascending by block number and deduplicated (the same ancestor header
132+
// can be referenced by more than one block).
133+
nodes.sort();
134+
nodes.dedup();
135+
let mut codes = value.codes;
136+
codes.sort();
137+
codes.dedup();
138+
let mut headers = value.block_headers_bytes;
139+
headers.sort_by_cached_key(|bytes| {
140+
// Undecodable headers sort last; consumers reject them on decode anyway.
141+
BlockHeader::decode(bytes)
142+
.map(|header| header.number)
143+
.unwrap_or(u64::MAX)
144+
});
145+
// Identical headers share a block number, so they are now adjacent.
146+
headers.dedup();
127147
Ok(Self {
128148
state: nodes.into_iter().map(Bytes::from).collect(),
129149
keys: Vec::new(),
130-
codes: value.codes.into_iter().map(Bytes::from).collect(),
131-
headers: value
132-
.block_headers_bytes
133-
.into_iter()
134-
.map(Bytes::from)
135-
.collect(),
150+
codes: codes.into_iter().map(Bytes::from).collect(),
151+
headers: headers.into_iter().map(Bytes::from).collect(),
136152
})
137153
}
138154
}
139155

156+
/// Geth's `ExtWitness` wire shape, returned by
157+
/// `engine_newPayloadWithWitness*`: an RLP list of
158+
/// `(headers, codes, state, keys)`, with headers ascending by block number
159+
/// and code/state byte lists sorted lexicographically. Geth currently emits
160+
/// empty `keys`, but the trailing field is part of the RLP shape.
161+
/// Reference:
162+
/// https://github.com/ethereum/go-ethereum/blob/4daaaadfc4706b0a49d4dfde3559de7be968c28a/core/stateless/encoding.go#L30-L52
163+
#[derive(Debug, Clone)]
164+
pub struct ExtWitness {
165+
pub headers: Vec<BlockHeader>,
166+
pub codes: Vec<Bytes>,
167+
pub state: Vec<Bytes>,
168+
pub keys: Vec<Bytes>,
169+
}
170+
171+
impl RLPEncode for ExtWitness {
172+
fn encode(&self, buf: &mut dyn bytes::BufMut) {
173+
Encoder::new(buf)
174+
.encode_field(&self.headers)
175+
.encode_field(&self.codes)
176+
.encode_field(&self.state)
177+
.encode_field(&self.keys)
178+
.finish();
179+
}
180+
}
181+
182+
impl RLPDecode for ExtWitness {
183+
fn decode_unfinished(rlp: &[u8]) -> Result<(Self, &[u8]), RLPDecodeError> {
184+
let decoder = Decoder::new(rlp)?;
185+
let (headers, decoder) = decoder.decode_field("headers")?;
186+
let (codes, decoder) = decoder.decode_field("codes")?;
187+
let (state, decoder) = decoder.decode_field("state")?;
188+
let (keys, decoder) = decoder.decode_field("keys")?;
189+
let remaining = decoder.finish()?;
190+
Ok((
191+
ExtWitness {
192+
headers,
193+
codes,
194+
state,
195+
keys,
196+
},
197+
remaining,
198+
))
199+
}
200+
}
201+
140202
impl RpcExecutionWitness {
141203
/// Convert an RPC execution witness into the internal [`ExecutionWitness`]
142204
/// format by rebuilding trie structures from the flat node list.

crates/networking/rpc/engine/payload.rs

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@ use bytes::Bytes;
22
use ethrex_blockchain::error::ChainError;
33
use ethrex_blockchain::payload::PayloadBuildResult;
44
use ethrex_common::types::block_access_list::BlockAccessList;
5-
use ethrex_common::types::block_execution_witness::{ExecutionWitness, RpcExecutionWitness};
5+
use ethrex_common::types::block_execution_witness::{
6+
ExecutionWitness, ExtWitness, RpcExecutionWitness,
7+
};
68
use ethrex_common::types::payload::PayloadBundle;
79
use ethrex_common::types::requests::{EncodedRequests, compute_requests_hash};
810
use ethrex_common::types::{Block, BlockBody, BlockHash, BlockHeader, BlockNumber, Fork};
911
use ethrex_common::{H256, U256};
1012
use ethrex_p2p::sync::SyncMode;
11-
use ethrex_rlp::{decode::RLPDecode, error::RLPDecodeError, structs::Encoder};
13+
use ethrex_rlp::{decode::RLPDecode, encode::RLPEncode, error::RLPDecodeError};
1214
use serde_json::Value;
1315
use tokio::sync::oneshot;
1416
use tracing::{debug, error, info, warn};
@@ -1342,13 +1344,9 @@ fn encode_witness_for_engine_rpc(witness: ExecutionWitness) -> Result<Bytes, Rpc
13421344
/// Encodes the witness in geth's opaque `engine_newPayloadWithWitness*` shape.
13431345
///
13441346
/// Format: geth returns `rlp.EncodeToBytes(proofs)` from `newPayload`, and
1345-
/// `stateless.Witness::EncodeRLP` delegates to `ExtWitness`.
1346-
/// `ExtWitness` is an RLP list of `(headers, codes, state, keys)`, with
1347-
/// headers ordered by block number and code/state byte lists sorted
1348-
/// lexicographically. Geth currently emits empty keys, but the trailing field
1349-
/// is part of the RLP shape.
1350-
/// Reference:
1351-
/// https://github.com/ethereum/go-ethereum/blob/4daaaadfc4706b0a49d4dfde3559de7be968c28a/core/stateless/encoding.go#L30-L52
1347+
/// `stateless.Witness::EncodeRLP` delegates to [`ExtWitness`] — see its docs
1348+
/// for the shape and geth references.
1349+
/// Additional references:
13521350
/// https://github.com/ethereum/go-ethereum/blob/4daaaadfc4706b0a49d4dfde3559de7be968c28a/core/stateless/encoding.go#L92-L98
13531351
/// https://github.com/ethereum/go-ethereum/blob/4daaaadfc4706b0a49d4dfde3559de7be968c28a/eth/catalyst/api.go#L915-L920
13541352
fn encode_rpc_witness_for_engine_rpc(rpc_witness: RpcExecutionWitness) -> Result<Bytes, RpcErr> {
@@ -1365,14 +1363,13 @@ fn encode_rpc_witness_for_engine_rpc(rpc_witness: RpcExecutionWitness) -> Result
13651363
state.sort_by(|a, b| a.as_ref().cmp(b.as_ref()));
13661364
let mut keys = rpc_witness.keys;
13671365
keys.sort_by(|a, b| a.as_ref().cmp(b.as_ref()));
1368-
let mut encoded = Vec::new();
1369-
Encoder::new(&mut encoded)
1370-
.encode_field(&headers)
1371-
.encode_field(&codes)
1372-
.encode_field(&state)
1373-
.encode_field(&keys)
1374-
.finish();
1375-
Ok(Bytes::from(encoded))
1366+
let ext_witness = ExtWitness {
1367+
headers,
1368+
codes,
1369+
state,
1370+
keys,
1371+
};
1372+
Ok(Bytes::from(ext_witness.encode_to_vec()))
13761373
}
13771374

13781375
fn parse_get_payload_request(params: &Option<Vec<Value>>) -> Result<u64, RpcErr> {

crates/vm/backends/levm/mod.rs

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2690,10 +2690,11 @@ pub fn generic_system_contract_levm(
26902690
..Default::default()
26912691
};
26922692

2693-
// Invariant relied upon below: with a zero gas price a system call charges no
2694-
// gas to the SYSTEM_ADDRESS sender and pays no fee to the coinbase, so the only
2695-
// state change left to undo afterwards is the sender's nonce bump. If this ever
2696-
// becomes non-zero, the post-call cleanup must be revisited.
2693+
// Invariant: with a zero gas price (and `is_system_call` making the hook
2694+
// skip the sender path entirely) a system call leaves no SYSTEM_ADDRESS
2695+
// state behind — no nonce bump, no balance change, not even a read. If
2696+
// this ever becomes non-zero, the hook's system-call branches must be
2697+
// revisited.
26972698
debug_assert!(
26982699
env.gas_price.is_zero() && env.base_fee_per_gas.is_zero(),
26992700
"system calls must run with a zero gas price"
@@ -2734,15 +2735,7 @@ pub fn generic_system_contract_levm(
27342735
recorder.exit_system_call();
27352736
}
27362737

2737-
let report = result?;
2738-
2739-
// Undo the sender nonce bump: it's the only state change a system call leaves
2740-
// behind given that the gas price is set to zero.
2741-
if let Some(account) = db.current_accounts_state.get_mut(&system_address) {
2742-
account.info.nonce = account.info.nonce.saturating_sub(1);
2743-
}
2744-
2745-
Ok(report)
2738+
result
27462739
}
27472740

27482741
#[allow(unreachable_code)]

0 commit comments

Comments
 (0)