From 9e40f9d56c0ac1eb99ce1d559391255541a847ae Mon Sep 17 00:00:00 2001 From: Edgar Luque Date: Thu, 19 Mar 2026 13:40:05 +0100 Subject: [PATCH 1/4] fix(l1): populate block hash cache with prior batch hashes during import When importing blocks in batches, the BLOCKHASH opcode could fail for blocks from the previous batch because those blocks weren't marked as canonical yet. The block_hash_cache only contained hashes for the current batch. Fix: before each batch, load the last 256 block hashes from the store into the cache so cross-batch BLOCKHASH lookups succeed. Closes #6385 --- crates/blockchain/blockchain.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 6b153dca0a..de9a322283 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -2303,8 +2303,20 @@ impl Blockchain { let chain_config: ChainConfig = self.storage.get_chain_config(); - // Cache block hashes for the full batch so we can access them during execution without having to store the blocks beforehand - let block_hash_cache = blocks.iter().map(|b| (b.header.number, b.hash())).collect(); + // Cache block hashes for the full batch so we can access them during + // execution without having to store the blocks beforehand. + // Also include the last 256 block hashes from the store so that + // BLOCKHASH can resolve references to blocks from previous batches + // (they may not be canonical yet during import). + let mut block_hash_cache: BTreeMap = + blocks.iter().map(|b| (b.header.number, b.hash())).collect(); + let first_number = first_block_header.number; + let lookback_start = first_number.saturating_sub(256); + for n in lookback_start..first_number { + if let Ok(Some(header)) = self.storage.get_block_header(n) { + block_hash_cache.entry(n).or_insert_with(|| header.hash()); + } + } let parent_header = self .storage From 026044193a95fd687f4d1f3edbcb78bff58fe24b Mon Sep 17 00:00:00 2001 From: Edgar Luque Date: Thu, 19 Mar 2026 13:45:09 +0100 Subject: [PATCH 2/4] fix(l1): populate block hash cache with prior batch hashes during import When importing blocks in batches, the BLOCKHASH opcode could fail for blocks from the previous batch because those blocks weren't marked as canonical yet. The block_hash_cache only contained hashes for the current batch. Fix: walk the parent chain from the first block's parent to cache the last 256 block hashes. Uses get_block_header_by_hash (works without canonical index) instead of get_block_header (requires canonical). Add regression test: batch_cross_batch_blockhash_regression builds 3 blocks where block 3 calls BLOCKHASH for block 2, then executes them in two separate batches. Without the fix, batch 2 fails with "Block hash not found for block number 2". Closes #6385 --- crates/blockchain/blockchain.rs | 30 +++-- test/tests/blockchain/batch_tests.rs | 164 +++++++++++++++++++++++++++ 2 files changed, 184 insertions(+), 10 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index de9a322283..77939d9ad9 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -2305,24 +2305,34 @@ impl Blockchain { // Cache block hashes for the full batch so we can access them during // execution without having to store the blocks beforehand. - // Also include the last 256 block hashes from the store so that - // BLOCKHASH can resolve references to blocks from previous batches - // (they may not be canonical yet during import). let mut block_hash_cache: BTreeMap = blocks.iter().map(|b| (b.header.number, b.hash())).collect(); - let first_number = first_block_header.number; - let lookback_start = first_number.saturating_sub(256); - for n in lookback_start..first_number { - if let Ok(Some(header)) = self.storage.get_block_header(n) { - block_hash_cache.entry(n).or_insert_with(|| header.hash()); - } - } let parent_header = self .storage .get_block_header_by_hash(first_block_header.parent_hash) .map_err(|e| (ChainError::StoreError(e), None))? .ok_or((ChainError::ParentNotFound, None))?; + + // Walk the parent chain to cache the last 256 block hashes so that + // BLOCKHASH can resolve references to blocks from previous batches + // (they may not be canonical yet during import). + { + let mut hash = parent_header.hash(); + let mut number = parent_header.number; + block_hash_cache.entry(number).or_insert(hash); + let lookback = first_block_header.number.saturating_sub(256); + while number > lookback { + match self.storage.get_block_header_by_hash(hash) { + Ok(Some(header)) => { + hash = header.parent_hash; + number = number.saturating_sub(1); + block_hash_cache.entry(number).or_insert(hash); + } + _ => break, + } + } + } let vm_db = StoreVmDatabase::new_with_block_hash_cache( self.storage.clone(), parent_header, diff --git a/test/tests/blockchain/batch_tests.rs b/test/tests/blockchain/batch_tests.rs index 3b3ed724b9..8d4ef1b515 100644 --- a/test/tests/blockchain/batch_tests.rs +++ b/test/tests/blockchain/batch_tests.rs @@ -247,6 +247,170 @@ async fn batch_selfdestruct_created_account_no_spurious_state() { ); } +/// Regression test for cross-batch BLOCKHASH resolution during import. +/// +/// When importing blocks in batches, a block in batch N+1 may execute +/// BLOCKHASH for a block that was in batch N. Without the fix, the hash +/// wouldn't be found because batch N's blocks aren't canonical yet and +/// the block_hash_cache only covers the current batch. +/// +/// This test builds 3 blocks: +/// - Block 1: deploy a contract that stores `blockhash(number - 1)` in storage +/// - Block 2: empty (just advances the chain) +/// - Block 3: call the contract (reads blockhash of block 2) +/// +/// Then executes blocks 1-2 in one batch, and block 3 in a second batch. +/// Block 3 needs `blockhash(2)` which is only in the first batch. +#[tokio::test] +async fn batch_cross_batch_blockhash_regression() { + let sk = test_secret_key(); + let sender = sender_from_key(&sk); + let signer: Signer = LocalSigner::new(sk).into(); + + let (store_a, chain_id) = setup_store(sender).await; + let blockchain_a = Blockchain::default_with_store(store_a.clone()); + let genesis_header = store_a.get_block_header(0).unwrap().unwrap(); + + // Deploy contract: BLOCKHASH(NUMBER - 1) -> SSTORE(0) + // Bytecode: NUMBER PUSH1 1 SWAP1 SUB BLOCKHASH PUSH1 0 SSTORE STOP + // 43 60 01 90 03 40 60 00 55 00 + let deploy_code = { + let runtime = vec![0x43, 0x60, 0x01, 0x90, 0x03, 0x40, 0x60, 0x00, 0x55, 0x00]; + let mut init = Vec::new(); + // PUSH PUSH1 0 RETURN + init.push(0x60 + 0); // PUSH1 + init.push(runtime.len() as u8); + init.push(0x60); // PUSH1 + init.push(0x00); // offset 0 + init.push(0x39); // CODECOPY — but we need the runtime in memory first + + // Simpler: use PUSH bytes directly + // PUSH10 PUSH1 0 MSTORE PUSH1 10 PUSH1 22 RETURN + // Actually let's just use a minimal init code: + // Copy runtime to memory and return it + let rt_len = runtime.len(); + init.clear(); + // PUSH1 rt_len PUSH1 (init_len) PUSH1 0 CODECOPY PUSH1 rt_len PUSH1 0 RETURN + let init_len = 12; // we'll verify this + init.push(0x60); + init.push(rt_len as u8); // PUSH1 rt_len + init.push(0x60); + init.push(init_len as u8); // PUSH1 init_code_len (offset of runtime in full code) + init.push(0x60); + init.push(0x00); // PUSH1 0 (dest offset in memory) + init.push(0x39); // CODECOPY + init.push(0x60); + init.push(rt_len as u8); // PUSH1 rt_len + init.push(0x60); + init.push(0x00); // PUSH1 0 + init.push(0xF3); // RETURN + assert_eq!(init.len(), init_len as usize); + init.extend_from_slice(&runtime); + Bytes::from(init) + }; + + let contract_address = calculate_create_address(sender, 0); + + // tx1: deploy the contract + let mut tx1 = Transaction::EIP1559Transaction(EIP1559Transaction { + chain_id, + nonce: 0, + max_priority_fee_per_gas: 0, + max_fee_per_gas: TEST_MAX_FEE_PER_GAS, + gas_limit: TEST_GAS_LIMIT, + to: TxKind::Create, + value: U256::zero(), + data: deploy_code, + ..Default::default() + }); + tx1.sign_inplace(&signer).await.unwrap(); + + blockchain_a + .add_transaction_to_pool(tx1) + .await + .expect("tx1 should enter pool"); + + // Build block 1 (deploys the contract) + let block1 = build_block(&store_a, &blockchain_a, &genesis_header).await; + assert!( + !block1.body.transactions.is_empty(), + "block1 must include tx" + ); + blockchain_a + .add_block(block1.clone()) + .expect("block1 valid"); + store_a + .forkchoice_update(vec![], 1, block1.hash(), None, None) + .await + .unwrap(); + blockchain_a + .remove_block_transactions_from_pool(&block1) + .unwrap(); + + // Build block 2 (empty, just advances the chain) + let block2 = build_block(&store_a, &blockchain_a, &block1.header).await; + blockchain_a + .add_block(block2.clone()) + .expect("block2 valid"); + store_a + .forkchoice_update(vec![], 2, block2.hash(), None, None) + .await + .unwrap(); + + // tx3: call the contract (triggers BLOCKHASH for block 2) + let mut tx3 = Transaction::EIP1559Transaction(EIP1559Transaction { + chain_id, + nonce: 1, + max_priority_fee_per_gas: 0, + max_fee_per_gas: TEST_MAX_FEE_PER_GAS, + gas_limit: TEST_GAS_LIMIT, + to: TxKind::Call(contract_address), + value: U256::zero(), + data: Bytes::new(), + ..Default::default() + }); + tx3.sign_inplace(&signer).await.unwrap(); + + blockchain_a + .add_transaction_to_pool(tx3) + .await + .expect("tx3 should enter pool"); + + // Build block 3 (calls contract, needs blockhash of block 2) + let block3 = build_block(&store_a, &blockchain_a, &block2.header).await; + assert!( + !block3.body.transactions.is_empty(), + "block3 must include tx" + ); + blockchain_a + .add_block(block3.clone()) + .expect("block3 valid"); + + // Now re-execute on a fresh store in TWO batches: + // Batch 1: blocks 1-2, Batch 2: block 3 + // Block 3 needs blockhash(2) which is only in batch 1. + let (store_b, _) = setup_store(sender).await; + let blockchain_b = Blockchain::default_with_store(store_b); + + let result1 = blockchain_b + .add_blocks_in_batch(vec![block1, block2], CancellationToken::new()) + .await; + assert!( + result1.is_ok(), + "batch 1 should succeed — got error: {:?}", + result1.err() + ); + + let result2 = blockchain_b + .add_blocks_in_batch(vec![block3], CancellationToken::new()) + .await; + assert!( + result2.is_ok(), + "batch 2 should succeed (needs blockhash from batch 1) — got error: {:?}", + result2.err() + ); +} + /// Simpler variant: a single block with a self-destructing contract, executed /// in batch. Ensures the basic batch path doesn't regress for single-block /// batches containing selfdestruct. From 727923e99c0a475054ef9ff40799c43632b141b9 Mon Sep 17 00:00:00 2001 From: Edgar Luque Date: Mon, 23 Mar 2026 13:18:51 +0100 Subject: [PATCH 3/4] fix(l1): address review findings in BLOCKHASH cache walk - Skip redundant DB lookup on first loop iteration - Split error handling to log storage failures - Remove unnecessary scope braces - Clean up dead code in test --- crates/blockchain/blockchain.rs | 28 ++++++++++++++++------------ test/tests/blockchain/batch_tests.rs | 19 ++++--------------- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/crates/blockchain/blockchain.rs b/crates/blockchain/blockchain.rs index 77939d9ad9..7ad455b3dd 100644 --- a/crates/blockchain/blockchain.rs +++ b/crates/blockchain/blockchain.rs @@ -2317,19 +2317,23 @@ impl Blockchain { // Walk the parent chain to cache the last 256 block hashes so that // BLOCKHASH can resolve references to blocks from previous batches // (they may not be canonical yet during import). - { - let mut hash = parent_header.hash(); - let mut number = parent_header.number; + block_hash_cache + .entry(parent_header.number) + .or_insert_with(|| parent_header.hash()); + let mut hash = parent_header.parent_hash; + let mut number = parent_header.number.saturating_sub(1); + let lookback = first_block_header.number.saturating_sub(256); + while number > lookback { block_hash_cache.entry(number).or_insert(hash); - let lookback = first_block_header.number.saturating_sub(256); - while number > lookback { - match self.storage.get_block_header_by_hash(hash) { - Ok(Some(header)) => { - hash = header.parent_hash; - number = number.saturating_sub(1); - block_hash_cache.entry(number).or_insert(hash); - } - _ => break, + match self.storage.get_block_header_by_hash(hash) { + Ok(Some(header)) => { + hash = header.parent_hash; + number = number.saturating_sub(1); + } + Ok(None) => break, + Err(e) => { + warn!("Failed to fetch block header by hash during BLOCKHASH cache walk: {e}"); + break; } } } diff --git a/test/tests/blockchain/batch_tests.rs b/test/tests/blockchain/batch_tests.rs index 8d4ef1b515..ff543d7c0c 100644 --- a/test/tests/blockchain/batch_tests.rs +++ b/test/tests/blockchain/batch_tests.rs @@ -276,22 +276,11 @@ async fn batch_cross_batch_blockhash_regression() { // 43 60 01 90 03 40 60 00 55 00 let deploy_code = { let runtime = vec![0x43, 0x60, 0x01, 0x90, 0x03, 0x40, 0x60, 0x00, 0x55, 0x00]; - let mut init = Vec::new(); - // PUSH PUSH1 0 RETURN - init.push(0x60 + 0); // PUSH1 - init.push(runtime.len() as u8); - init.push(0x60); // PUSH1 - init.push(0x00); // offset 0 - init.push(0x39); // CODECOPY — but we need the runtime in memory first - - // Simpler: use PUSH bytes directly - // PUSH10 PUSH1 0 MSTORE PUSH1 10 PUSH1 22 RETURN - // Actually let's just use a minimal init code: - // Copy runtime to memory and return it let rt_len = runtime.len(); - init.clear(); - // PUSH1 rt_len PUSH1 (init_len) PUSH1 0 CODECOPY PUSH1 rt_len PUSH1 0 RETURN - let init_len = 12; // we'll verify this + // Init code: CODECOPY runtime into memory, then RETURN it. + // PUSH1 rt_len PUSH1 init_len PUSH1 0 CODECOPY PUSH1 rt_len PUSH1 0 RETURN + let init_len = 12; + let mut init = Vec::new(); init.push(0x60); init.push(rt_len as u8); // PUSH1 rt_len init.push(0x60); From a6b09d753410c5c7e53cb5e37868e037c699e8e6 Mon Sep 17 00:00:00 2001 From: Edgar Date: Thu, 28 May 2026 14:52:26 +0200 Subject: [PATCH 4/4] style: collapse vec init+push into vec! literal in batch test Satisfies clippy::vec_init_then_push (-D warnings) without changing behaviour. The init bytecode layout is preserved via inline grouping. --- test/tests/blockchain/batch_tests.rs | 29 +++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/test/tests/blockchain/batch_tests.rs b/test/tests/blockchain/batch_tests.rs index ff543d7c0c..60067b3e68 100644 --- a/test/tests/blockchain/batch_tests.rs +++ b/test/tests/blockchain/batch_tests.rs @@ -280,19 +280,22 @@ async fn batch_cross_batch_blockhash_regression() { // Init code: CODECOPY runtime into memory, then RETURN it. // PUSH1 rt_len PUSH1 init_len PUSH1 0 CODECOPY PUSH1 rt_len PUSH1 0 RETURN let init_len = 12; - let mut init = Vec::new(); - init.push(0x60); - init.push(rt_len as u8); // PUSH1 rt_len - init.push(0x60); - init.push(init_len as u8); // PUSH1 init_code_len (offset of runtime in full code) - init.push(0x60); - init.push(0x00); // PUSH1 0 (dest offset in memory) - init.push(0x39); // CODECOPY - init.push(0x60); - init.push(rt_len as u8); // PUSH1 rt_len - init.push(0x60); - init.push(0x00); // PUSH1 0 - init.push(0xF3); // RETURN + // PUSH1 rt_len | PUSH1 init_code_len | PUSH1 0 | CODECOPY + // PUSH1 rt_len | PUSH1 0 | RETURN + let mut init = vec![ + 0x60, + rt_len as u8, + 0x60, + init_len as u8, + 0x60, + 0x00, + 0x39, + 0x60, + rt_len as u8, + 0x60, + 0x00, + 0xF3, + ]; assert_eq!(init.len(), init_len as usize); init.extend_from_slice(&runtime); Bytes::from(init)