From 6867af4ebf012476248c57258422dfd0d379fe5b Mon Sep 17 00:00:00 2001 From: Wolfgang Welz Date: Tue, 23 Jan 2024 11:43:23 +0100 Subject: [PATCH] Fix OP block header validation (#74) * verify op block loaded from DB * improve errors * Simplify BatcherDb (#81) * fix warnings --------- Co-authored-by: Tim Carstens --- host/src/bin/op-derive.rs | 27 ++---- lib/src/optimism/batcher_db.rs | 163 ++++++++++++++++++--------------- lib/src/optimism/config.rs | 5 +- lib/src/optimism/mod.rs | 55 +++++++++-- 4 files changed, 148 insertions(+), 102 deletions(-) diff --git a/host/src/bin/op-derive.rs b/host/src/bin/op-derive.rs index 7c1011f8..c426351e 100644 --- a/host/src/bin/op-derive.rs +++ b/host/src/bin/op-derive.rs @@ -137,7 +137,7 @@ async fn main() -> Result<()> { let output_mem = DeriveMachine::new(&OPTIMISM_CHAIN_SPEC, derive_input.clone()) .context("Could not create derive machine")? .derive() - .unwrap(); + .context("could not derive")?; assert_eq!(output, output_mem); } @@ -354,6 +354,10 @@ impl RpcDb { } impl BatcherDb for RpcDb { + fn validate(&self) -> Result<()> { + Ok(()) + } + fn get_full_op_block(&mut self, block_no: u64) -> Result> { let mut provider = new_provider( op_cache_path(&self.cache, block_no), @@ -390,7 +394,7 @@ impl BatcherDb for RpcDb { Ok(header) } - fn get_full_eth_block(&mut self, block_no: u64) -> Result> { + fn get_full_eth_block(&mut self, block_no: u64) -> Result<&BlockInput> { let query = BlockQuery { block_no }; let mut provider = new_provider( eth_cache_path(&self.cache, block_no), @@ -430,23 +434,8 @@ impl BatcherDb for RpcDb { receipts, } }; - self.mem_db.full_eth_block.insert(block_no, block.clone()); + self.mem_db.full_eth_block.insert(block_no, block); provider.save()?; - Ok(block) - } - - fn get_eth_block_header(&mut self, block_no: u64) -> Result
{ - let mut provider = new_provider( - eth_cache_path(&self.cache, block_no), - self.eth_rpc_url.clone(), - )?; - let header: Header = provider - .get_partial_block(&BlockQuery { block_no })? - .try_into()?; - self.mem_db - .eth_block_header - .insert(block_no, header.clone()); - provider.save()?; - Ok(header) + self.mem_db.get_full_eth_block(block_no) } } diff --git a/lib/src/optimism/batcher_db.rs b/lib/src/optimism/batcher_db.rs index ad571310..d88b2297 100644 --- a/lib/src/optimism/batcher_db.rs +++ b/lib/src/optimism/batcher_db.rs @@ -14,7 +14,7 @@ use std::collections::HashMap; -use anyhow::{ensure, Result}; +use anyhow::{ensure, Context, Result}; use serde::{Deserialize, Serialize}; use zeth_primitives::{ block::Header, @@ -40,10 +40,10 @@ pub struct BlockInput { } pub trait BatcherDb { + fn validate(&self) -> Result<()>; fn get_full_op_block(&mut self, block_no: u64) -> Result>; fn get_op_block_header(&mut self, block_no: u64) -> Result
; - fn get_full_eth_block(&mut self, block_no: u64) -> Result>; - fn get_eth_block_header(&mut self, block_no: u64) -> Result
; + fn get_full_eth_block(&mut self, block_no: u64) -> Result<&BlockInput>; } #[derive(Debug, Clone, Deserialize, Serialize)] @@ -72,92 +72,107 @@ impl Default for MemDb { } impl BatcherDb for MemDb { - fn get_full_op_block(&mut self, block_no: u64) -> Result> { - let op_block = self.full_op_block.remove(&block_no).unwrap(); - assert_eq!(block_no, op_block.block_header.number); - - // Validate tx list - { - let mut tx_trie = MptNode::default(); - for (tx_no, tx) in op_block.transactions.iter().enumerate() { - let trie_key = tx_no.to_rlp(); - tx_trie.insert_rlp(&trie_key, tx)?; + fn validate(&self) -> Result<()> { + for (block_no, op_block) in &self.full_op_block { + ensure!( + *block_no == op_block.block_header.number, + "Block number mismatch" + ); + + // Validate tx list + { + let mut tx_trie = MptNode::default(); + for (tx_no, tx) in op_block.transactions.iter().enumerate() { + let trie_key = tx_no.to_rlp(); + tx_trie.insert_rlp(&trie_key, tx)?; + } + ensure!( + tx_trie.hash() == op_block.block_header.transactions_root, + "Invalid op block transaction data!" + ); } + + // Validate receipts ensure!( - tx_trie.hash() == op_block.block_header.transactions_root, - "Invalid op block transaction data!" + op_block.receipts.is_none(), + "Op blocks should not contain receipts" ); } - // Validate receipts - ensure!( - op_block.receipts.is_none(), - "Op blocks should not contain receipts" - ); + for (block_no, op_block) in &self.op_block_header { + ensure!(*block_no == op_block.number, "Block number mismatch"); + } - Ok(op_block) - } + for (block_no, eth_block) in &self.full_eth_block { + ensure!( + *block_no == eth_block.block_header.number, + "Block number mismatch" + ); - fn get_op_block_header(&mut self, block_no: u64) -> Result
{ - let op_block = self.op_block_header.remove(&block_no).unwrap(); - assert_eq!(block_no, op_block.number); + // Validate tx list + { + let mut tx_trie = MptNode::default(); + for (tx_no, tx) in eth_block.transactions.iter().enumerate() { + let trie_key = tx_no.to_rlp(); + tx_trie.insert_rlp(&trie_key, tx)?; + } + ensure!( + tx_trie.hash() == eth_block.block_header.transactions_root, + "Invalid eth block transaction data!" + ); + } - Ok(op_block) + // Validate receipts + if eth_block.receipts.is_some() { + let mut receipt_trie = MptNode::default(); + for (tx_no, receipt) in eth_block.receipts.as_ref().unwrap().iter().enumerate() { + let trie_key = tx_no.to_rlp(); + receipt_trie.insert_rlp(&trie_key, receipt)?; + } + ensure!( + receipt_trie.hash() == eth_block.block_header.receipts_root, + "Invalid eth block receipt data!" + ); + } else { + let can_contain_deposits = deposits::can_contain( + &OPTIMISM_CHAIN_SPEC.deposit_contract, + ð_block.block_header.logs_bloom, + ); + let can_contain_config = system_config::can_contain( + &OPTIMISM_CHAIN_SPEC.system_config_contract, + ð_block.block_header.logs_bloom, + ); + ensure!( + !can_contain_deposits, + "Eth block has no receipts, but bloom filter indicates it has deposits" + ); + ensure!( + !can_contain_config, + "Eth block has no receipts, but bloom filter indicates it has config updates" + ); + } + } + + Ok(()) } - fn get_full_eth_block(&mut self, block_no: u64) -> Result> { - let eth_block = self.full_eth_block.remove(&block_no).unwrap(); - assert_eq!(block_no, eth_block.block_header.number); + fn get_full_op_block(&mut self, block_no: u64) -> Result> { + let op_block = self.full_op_block.remove(&block_no).unwrap(); - // Validate tx list - { - let mut tx_trie = MptNode::default(); - for (tx_no, tx) in eth_block.transactions.iter().enumerate() { - let trie_key = tx_no.to_rlp(); - tx_trie.insert_rlp(&trie_key, tx)?; - } - ensure!( - tx_trie.hash() == eth_block.block_header.transactions_root, - "Invalid eth block transaction data!" - ); - } + Ok(op_block) + } - // Validate receipts - if eth_block.receipts.is_some() { - let mut receipt_trie = MptNode::default(); - for (tx_no, receipt) in eth_block.receipts.as_ref().unwrap().iter().enumerate() { - let trie_key = tx_no.to_rlp(); - receipt_trie.insert_rlp(&trie_key, receipt)?; - } - ensure!( - receipt_trie.hash() == eth_block.block_header.receipts_root, - "Invalid eth block receipt data!" - ); - } else { - let can_contain_deposits = deposits::can_contain( - &OPTIMISM_CHAIN_SPEC.deposit_contract, - ð_block.block_header.logs_bloom, - ); - let can_contain_config = system_config::can_contain( - &OPTIMISM_CHAIN_SPEC.system_config_contract, - ð_block.block_header.logs_bloom, - ); - ensure!( - !can_contain_deposits, - "Eth block has no receipts, but bloom filter indicates it has deposits" - ); - ensure!( - !can_contain_config, - "Eth block has no receipts, but bloom filter indicates it has config updates" - ); - } + fn get_op_block_header(&mut self, block_no: u64) -> Result
{ + let op_block = self + .op_block_header + .remove(&block_no) + .context("not or no longer in db")?; - Ok(eth_block) + Ok(op_block) } - fn get_eth_block_header(&mut self, block_no: u64) -> Result
{ - let eth_block = self.eth_block_header.remove(&block_no).unwrap(); - assert_eq!(block_no, eth_block.number); + fn get_full_eth_block(&mut self, block_no: u64) -> Result<&BlockInput> { + let eth_block = self.full_eth_block.get(&block_no).unwrap(); Ok(eth_block) } diff --git a/lib/src/optimism/config.rs b/lib/src/optimism/config.rs index 16ec5bdc..e1f8f57b 100644 --- a/lib/src/optimism/config.rs +++ b/lib/src/optimism/config.rs @@ -23,10 +23,12 @@ use super::system_config::SystemConfig; pub struct ChainConfig { /// The initial system config value pub system_config: SystemConfig, - // The L1 attributes depositor address + /// The L1 attributes depositor address pub l1_attributes_depositor: Address, /// The L1 attributes contract pub l1_attributes_contract: Address, + /// The L2 address accumulating any transaction priority fee + pub sequencer_fee_vault: Address, /// The batch inbox address pub batch_inbox: Address, /// The deposit contract address @@ -57,6 +59,7 @@ impl ChainConfig { }, l1_attributes_depositor: address!("deaddeaddeaddeaddeaddeaddeaddeaddead0001"), l1_attributes_contract: address!("4200000000000000000000000000000000000015"), + sequencer_fee_vault: address!("4200000000000000000000000000000000000011"), batch_inbox: address!("ff00000000000000000000000000000000000010"), deposit_contract: address!("bEb5Fc579115071764c7423A4f12eDde41f106Ed"), system_config_contract: address!("229047fed2591dbec1eF1118d64F7aF3dB9EB290"), diff --git a/lib/src/optimism/mod.rs b/lib/src/optimism/mod.rs index e36a18ba..8ff52536 100644 --- a/lib/src/optimism/mod.rs +++ b/lib/src/optimism/mod.rs @@ -105,6 +105,8 @@ pub struct DeriveMachine { impl DeriveMachine { /// Creates a new instance of DeriveMachine. pub fn new(chain_config: &ChainConfig, mut derive_input: DeriveInput) -> Result { + derive_input.db.validate()?; + let op_block_no = derive_input.op_head_block_no; // read system config from op_head (seq_no/epoch_no..etc) @@ -171,7 +173,7 @@ impl DeriveMachine { hash: set_l1_block_values.hash, }, }, - ð_head, + eth_head, )? }; @@ -208,7 +210,7 @@ impl DeriveMachine { .context("block not found")?; self.op_batcher - .process_l1_block(ð_block) + .process_l1_block(eth_block) .context("failed to create batcher transactions")?; } process_next_eth_block = true; @@ -247,23 +249,54 @@ impl DeriveMachine { Vec::new() }; - // Obtain new Op head + // obtain verified op block header let new_op_head = { // load the next op block header let new_op_head = self .derive_input .db .get_op_block_header(self.op_block_no) - .context("block not found")?; + .context("op block not found")?; - // Verify new op head has the expected parent + // Verify that the op block header loaded from the DB matches the payload + // attributes of the batch. ensure!( new_op_head.parent_hash == self.op_batcher.state.safe_head.hash, "Invalid op block parent hash" ); + ensure!( + new_op_head.beneficiary == self.op_batcher.config.sequencer_fee_vault, + "Invalid op block beneficiary" + ); + ensure!( + new_op_head.gas_limit == self.op_batcher.config.system_config.gas_limit, + "Invalid op block gas limit" + ); + ensure!( + new_op_head.timestamp == U256::from(op_batch.0.timestamp), + "Invalid op block timestamp" + ); + ensure!( + new_op_head.extra_data.is_empty(), + "Invalid op block extra data" + ); - // Verify that the new op head transactions are consistent with the batch - // transactions + // verify that the new op head mix hash matches the mix hash of the L1 block + { + let l1_epoch_header = self + .derive_input + .db + .get_full_eth_block(op_batch.0.epoch_num) + .context("eth block not found")? + .block_header + .clone(); + ensure!( + new_op_head.mix_hash == l1_epoch_header.mix_hash, + "Invalid op block mix hash" + ); + } + + // verify that the new op head transactions match the batch transactions { // From the spec: // The first transaction MUST be a L1 attributes deposited transaction, @@ -279,12 +312,18 @@ impl DeriveMachine { let trie_key = tx_no.to_rlp(); tx_trie.insert(&trie_key, tx)?; } + ensure!( tx_trie.hash() == new_op_head.transactions_root, - "Invalid op block transaction data! Transaction trie root does not match" + "Invalid op block transactions" ); } + ensure!( + new_op_head.withdrawals_root.is_none(), + "Invalid op block withdrawals" + ); + new_op_head };