From 2de10a15b512d81bfb5ce9de24272db1afad41c7 Mon Sep 17 00:00:00 2001 From: Roman Krasiuk Date: Thu, 25 Jan 2024 18:16:18 +0100 Subject: [PATCH] feat(trie): historical & sidechain state root (#6131) Co-authored-by: Matthias Seitz --- crates/blockchain-tree/src/blockchain_tree.rs | 259 +++++++----------- crates/blockchain-tree/src/chain.rs | 145 +++------- crates/blockchain-tree/src/config.rs | 3 +- crates/consensus/beacon/src/engine/mod.rs | 48 ++-- crates/interfaces/src/blockchain_tree/mod.rs | 42 ++- .../src/providers/state/historical.rs | 46 +++- crates/trie/src/hashed_cursor/post_state.rs | 12 +- crates/trie/src/state.rs | 72 ++++- crates/trie/src/trie.rs | 1 - 9 files changed, 297 insertions(+), 331 deletions(-) diff --git a/crates/blockchain-tree/src/blockchain_tree.rs b/crates/blockchain-tree/src/blockchain_tree.rs index 537f34164a..d2e93c6a0d 100644 --- a/crates/blockchain-tree/src/blockchain_tree.rs +++ b/crates/blockchain-tree/src/blockchain_tree.rs @@ -2,7 +2,6 @@ use crate::{ canonical_chain::CanonicalChain, - chain::BlockKind, metrics::{MakeCanonicalAction, MakeCanonicalDurationsRecorder, TreeMetrics}, state::{BlockChainId, TreeState}, AppendableChain, BlockIndices, BlockchainTreeConfig, BundleStateData, TreeExternals, @@ -11,7 +10,7 @@ use reth_db::{database::Database, DatabaseError}; use reth_interfaces::{ blockchain_tree::{ error::{BlockchainTreeError, CanonicalError, InsertBlockError, InsertBlockErrorKind}, - BlockStatus, BlockValidationKind, CanonicalOutcome, InsertPayloadOk, + BlockAttachment, BlockStatus, BlockValidationKind, CanonicalOutcome, InsertPayloadOk, }, consensus::{Consensus, ConsensusError}, executor::{BlockExecutionError, BlockValidationError}, @@ -132,7 +131,6 @@ impl BlockchainTree { /// Function will check: /// * if block is inside database returns [BlockStatus::Valid]. /// * if block is inside buffer returns [BlockStatus::Disconnected]. - /// * if block is part of a side chain returns [BlockStatus::Accepted]. /// * if block is part of the canonical returns [BlockStatus::Valid]. /// /// Returns an error if @@ -147,12 +145,12 @@ impl BlockchainTree { if block.number <= last_finalized_block { // check if block is canonical if self.is_block_hash_canonical(&block.hash)? { - return Ok(Some(BlockStatus::Valid)) + return Ok(Some(BlockStatus::Valid(BlockAttachment::Canonical))) } // check if block is inside database if self.externals.provider_factory.provider()?.block_number(block.hash)?.is_some() { - return Ok(Some(BlockStatus::Valid)) + return Ok(Some(BlockStatus::Valid(BlockAttachment::Canonical))) } return Err(BlockchainTreeError::PendingBlockIsFinalized { @@ -163,12 +161,12 @@ impl BlockchainTree { // check if block is part of canonical chain if self.is_block_hash_canonical(&block.hash)? { - return Ok(Some(BlockStatus::Valid)) + return Ok(Some(BlockStatus::Valid(BlockAttachment::Canonical))) } // is block inside chain - if let Some(status) = self.is_block_inside_chain(&block) { - return Ok(Some(status)) + if let Some(attachment) = self.is_block_inside_chain(&block) { + return Ok(Some(BlockStatus::Valid(attachment))) } // check if block is disconnected @@ -289,7 +287,7 @@ impl BlockchainTree { &mut self, block: SealedBlockWithSenders, block_validation_kind: BlockValidationKind, - ) -> Result { + ) -> Result { debug_assert!(self.validate_block(&block).is_ok(), "Block must be validated"); let parent = block.parent_num_hash(); @@ -301,11 +299,8 @@ impl BlockchainTree { } // if not found, check if the parent can be found inside canonical chain. - if self - .is_block_hash_canonical(&parent.hash) - .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))? - { - return self.try_append_canonical_chain(block, block_validation_kind) + if self.is_block_hash_canonical(&parent.hash)? { + return self.try_append_canonical_chain(block.clone(), block_validation_kind) } // this is another check to ensure that if the block points to a canonical block its block @@ -315,22 +310,17 @@ impl BlockchainTree { { // we found the parent block in canonical chain if canonical_parent_number != parent.number { - return Err(InsertBlockError::consensus_error( - ConsensusError::ParentBlockNumberMismatch { - parent_block_number: canonical_parent_number, - block_number: block.number, - }, - block.block, - )) + return Err(ConsensusError::ParentBlockNumberMismatch { + parent_block_number: canonical_parent_number, + block_number: block.number, + } + .into()) } } // if there is a parent inside the buffer, validate against it. if let Some(buffered_parent) = self.state.buffered_blocks.block(&parent.hash) { - self.externals - .consensus - .validate_header_against_parent(&block, buffered_parent) - .map_err(|err| InsertBlockError::consensus_error(err, block.block.clone()))?; + self.externals.consensus.validate_header_against_parent(&block, buffered_parent)?; } // insert block inside unconnected block buffer. Delaying its execution. @@ -341,10 +331,7 @@ impl BlockchainTree { // shouldn't happen right after insertion let lowest_ancestor = self.state.buffered_blocks.lowest_ancestor(&block.hash).ok_or_else(|| { - InsertBlockError::tree_error( - BlockchainTreeError::BlockBufferingFailed { block_hash: block.hash }, - block.block, - ) + BlockchainTreeError::BlockBufferingFailed { block_hash: block.hash } })?; Ok(BlockStatus::Disconnected { missing_ancestor: lowest_ancestor.parent_num_hash() }) @@ -360,91 +347,59 @@ impl BlockchainTree { &mut self, block: SealedBlockWithSenders, block_validation_kind: BlockValidationKind, - ) -> Result { + ) -> Result { let parent = block.parent_num_hash(); let block_num_hash = block.num_hash(); debug!(target: "blockchain_tree", head = ?block_num_hash.hash, ?parent, "Appending block to canonical chain"); - // create new chain that points to that block - //return self.fork_canonical_chain(block.clone()); - // TODO save pending block to database - // https://github.com/paradigmxyz/reth/issues/1713 - let (block_status, chain) = { - let provider = self - .externals - .provider_factory - .provider() - .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?; + let provider = self.externals.provider_factory.provider()?; - // Validate that the block is post merge - let parent_td = provider - .header_td(&block.parent_hash) - .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))? - .ok_or_else(|| { - InsertBlockError::tree_error( - BlockchainTreeError::CanonicalChain { block_hash: block.parent_hash }, - block.block.clone(), - ) - })?; + // Validate that the block is post merge + let parent_td = provider + .header_td(&block.parent_hash)? + .ok_or_else(|| BlockchainTreeError::CanonicalChain { block_hash: block.parent_hash })?; - // Pass the parent total difficulty to short-circuit unnecessary calculations. - if !self - .externals - .provider_factory - .chain_spec() - .fork(Hardfork::Paris) - .active_at_ttd(parent_td, U256::ZERO) - { - return Err(InsertBlockError::execution_error( - BlockValidationError::BlockPreMerge { hash: block.hash }.into(), - block.block, - )) - } + // Pass the parent total difficulty to short-circuit unnecessary calculations. + if !self + .externals + .provider_factory + .chain_spec() + .fork(Hardfork::Paris) + .active_at_ttd(parent_td, U256::ZERO) + { + return Err(BlockExecutionError::Validation(BlockValidationError::BlockPreMerge { + hash: block.hash, + }) + .into()) + } - let parent_header = provider - .header(&block.parent_hash) - .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))? - .ok_or_else(|| { - InsertBlockError::tree_error( - BlockchainTreeError::CanonicalChain { block_hash: block.parent_hash }, - block.block.clone(), - ) - })? - .seal(block.parent_hash); + let parent_header = provider + .header(&block.parent_hash)? + .ok_or_else(|| BlockchainTreeError::CanonicalChain { block_hash: block.parent_hash })? + .seal(block.parent_hash); - let canonical_chain = self.canonical_chain(); + let canonical_chain = self.canonical_chain(); - if block.parent_hash == canonical_chain.tip().hash { - let chain = AppendableChain::new_canonical_head_fork( - block, - &parent_header, - canonical_chain.inner(), - parent, - &self.externals, - block_validation_kind, - )?; - let status = if block_validation_kind.is_exhaustive() { - BlockStatus::Valid - } else { - BlockStatus::Accepted - }; - - (status, chain) - } else { - let chain = AppendableChain::new_canonical_fork( - block, - &parent_header, - canonical_chain.inner(), - parent, - &self.externals, - )?; - (BlockStatus::Accepted, chain) - } + let block_attachment = if block.parent_hash == canonical_chain.tip().hash { + BlockAttachment::Canonical + } else { + BlockAttachment::HistoricalFork }; + let chain = AppendableChain::new_canonical_fork( + block, + &parent_header, + canonical_chain.inner(), + parent, + &self.externals, + block_attachment, + block_validation_kind, + )?; + self.insert_chain(chain); self.try_connect_buffered_blocks(block_num_hash); - Ok(block_status) + + Ok(BlockStatus::Valid(block_attachment)) } /// Try inserting a block into the given side chain. @@ -456,7 +411,7 @@ impl BlockchainTree { block: SealedBlockWithSenders, chain_id: BlockChainId, block_validation_kind: BlockValidationKind, - ) -> Result { + ) -> Result { debug!(target: "blockchain_tree", "Inserting block into side chain"); let block_num_hash = block.num_hash(); // Create a new sidechain by forking the given chain, or append the block if the parent @@ -464,37 +419,25 @@ impl BlockchainTree { let block_hashes = self.all_chain_hashes(chain_id); // get canonical fork. - let canonical_fork = match self.canonical_fork(chain_id) { - None => { - return Err(InsertBlockError::tree_error( - BlockchainTreeError::BlockSideChainIdConsistency { chain_id: chain_id.into() }, - block.block, - )) - } - Some(fork) => fork, - }; + let canonical_fork = self.canonical_fork(chain_id).ok_or_else(|| { + BlockchainTreeError::BlockSideChainIdConsistency { chain_id: chain_id.into() } + })?; // get chain that block needs to join to. - let parent_chain = match self.state.chains.get_mut(&chain_id) { - Some(parent_chain) => parent_chain, - None => { - return Err(InsertBlockError::tree_error( - BlockchainTreeError::BlockSideChainIdConsistency { chain_id: chain_id.into() }, - block.block, - )) - } - }; + let parent_chain = self.state.chains.get_mut(&chain_id).ok_or_else(|| { + BlockchainTreeError::BlockSideChainIdConsistency { chain_id: chain_id.into() } + })?; let chain_tip = parent_chain.tip().hash(); let canonical_chain = self.state.block_indices.canonical_chain(); // append the block if it is continuing the side chain. - let status = if chain_tip == block.parent_hash { + let block_attachment = if chain_tip == block.parent_hash { // check if the chain extends the currently tracked canonical head - let block_kind = if canonical_fork.hash == canonical_chain.tip().hash { - BlockKind::ExtendsCanonicalHead + let block_attachment = if canonical_fork.hash == canonical_chain.tip().hash { + BlockAttachment::Canonical } else { - BlockKind::ForksHistoricalBlock + BlockAttachment::HistoricalFork }; debug!(target: "blockchain_tree", "Appending block to side chain"); @@ -506,19 +449,12 @@ impl BlockchainTree { canonical_chain.inner(), &self.externals, canonical_fork, - block_kind, + block_attachment, block_validation_kind, )?; self.block_indices_mut().insert_non_fork_block(block_number, block_hash, chain_id); - - if block_kind.extends_canonical_head() && block_validation_kind.is_exhaustive() { - // if the block can be traced back to the canonical head, we were able to fully - // validate it - Ok(BlockStatus::Valid) - } else { - Ok(BlockStatus::Accepted) - } + block_attachment } else { debug!(target: "blockchain_tree", ?canonical_fork, "Starting new fork from side chain"); // the block starts a new fork @@ -528,15 +464,16 @@ impl BlockchainTree { canonical_chain.inner(), canonical_fork, &self.externals, + block_validation_kind, )?; self.insert_chain(chain); - Ok(BlockStatus::Accepted) + BlockAttachment::HistoricalFork }; // After we inserted the block, we try to connect any buffered blocks self.try_connect_buffered_blocks(block_num_hash); - status + Ok(BlockStatus::Valid(block_attachment)) } /// Get all block hashes from a sidechain that are not part of the canonical chain. @@ -724,21 +661,21 @@ impl BlockchainTree { Ok(()) } - /// Check if block is found inside chain and if the chain extends the canonical chain. + /// Check if block is found inside chain and its attachment. /// - /// if it does extend the canonical chain, return `BlockStatus::Valid` - /// if it does not extend the canonical chain, return `BlockStatus::Accepted` + /// if it is canonical or extends the canonical chain, return [BlockAttachment::Canonical] + /// if it does not extend the canonical chain, return [BlockAttachment::HistoricalFork] #[track_caller] - fn is_block_inside_chain(&self, block: &BlockNumHash) -> Option { + fn is_block_inside_chain(&self, block: &BlockNumHash) -> Option { // check if block known and is already in the tree if let Some(chain_id) = self.block_indices().get_blocks_chain_id(&block.hash) { // find the canonical fork of this chain let canonical_fork = self.canonical_fork(chain_id).expect("Chain id is valid"); // if the block's chain extends canonical chain return if canonical_fork == self.block_indices().canonical_tip() { - Some(BlockStatus::Valid) + Some(BlockAttachment::Canonical) } else { - Some(BlockStatus::Accepted) + Some(BlockAttachment::HistoricalFork) } } None @@ -783,9 +720,10 @@ impl BlockchainTree { return Err(InsertBlockError::consensus_error(err, block.block)) } - Ok(InsertPayloadOk::Inserted( - self.try_insert_validated_block(block, block_validation_kind)?, - )) + let status = self + .try_insert_validated_block(block.clone(), block_validation_kind) + .map_err(|kind| InsertBlockError::new(block.block, kind))?; + Ok(InsertPayloadOk::Inserted(status)) } /// Finalize blocks up until and including `finalized_block`, and remove them from the tree. @@ -1587,7 +1525,7 @@ mod tests { assert_eq!( tree.insert_block(canonical_block_1.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::Inserted(BlockStatus::Valid) + InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical)) ); assert_eq!( @@ -1597,12 +1535,12 @@ mod tests { assert_eq!( tree.insert_block(canonical_block_2.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::Inserted(BlockStatus::Valid) + InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical)) ); assert_eq!( tree.insert_block(sidechain_block_1.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::Inserted(BlockStatus::Accepted) + InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::HistoricalFork)) ); assert_eq!( @@ -1617,7 +1555,7 @@ mod tests { assert_eq!( tree.insert_block(sidechain_block_2.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::Inserted(BlockStatus::Accepted) + InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::HistoricalFork)) ); assert_eq!( @@ -1627,7 +1565,7 @@ mod tests { assert_eq!( tree.insert_block(canonical_block_3.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::Inserted(BlockStatus::Accepted) + InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::HistoricalFork)) ); assert_eq!( @@ -1644,7 +1582,7 @@ mod tests { let genesis = data.genesis; // test pops execution results from vector, so order is from last to first. - let externals = setup_externals(vec![exec2.clone(), exec1.clone(), exec2, exec1]); + let externals = setup_externals(vec![exec2.clone(), exec2, exec1]); // last finalized block would be number 9. setup_genesis(&externals.provider_factory, genesis); @@ -1660,12 +1598,12 @@ mod tests { assert_eq!( tree.insert_block(block1.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::Inserted(BlockStatus::Valid) + InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical)) ); assert_eq!( tree.insert_block(block2.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::Inserted(BlockStatus::Valid) + InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical)) ); // we have one chain that has two blocks. @@ -1690,7 +1628,7 @@ mod tests { assert_eq!( tree.insert_block(block2a.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::Inserted(BlockStatus::Accepted) + InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::HistoricalFork)) ); // fork chain. @@ -1783,7 +1721,7 @@ mod tests { // insert block1 and buffered block2 is inserted assert_eq!( tree.insert_block(block1.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::Inserted(BlockStatus::Valid) + InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::Canonical)) ); // Buffered blocks: [] @@ -1806,13 +1744,13 @@ mod tests { // already inserted block will `InsertPayloadOk::AlreadySeen(_)` assert_eq!( tree.insert_block(block1.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::AlreadySeen(BlockStatus::Valid) + InsertPayloadOk::AlreadySeen(BlockStatus::Valid(BlockAttachment::Canonical)) ); // block two is already inserted. assert_eq!( tree.insert_block(block2.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::AlreadySeen(BlockStatus::Valid) + InsertPayloadOk::AlreadySeen(BlockStatus::Valid(BlockAttachment::Canonical)) ); // make block1 canonical @@ -1852,7 +1790,7 @@ mod tests { // reinsert two blocks that point to canonical chain assert_eq!( tree.insert_block(block1a.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::Inserted(BlockStatus::Accepted) + InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::HistoricalFork)) ); TreeTester::default() @@ -1867,7 +1805,7 @@ mod tests { assert_eq!( tree.insert_block(block2a.clone(), BlockValidationKind::Exhaustive).unwrap(), - InsertPayloadOk::Inserted(BlockStatus::Accepted) + InsertPayloadOk::Inserted(BlockStatus::Valid(BlockAttachment::HistoricalFork)) ); // Trie state: // b2 b2a (side chain) @@ -2069,7 +2007,10 @@ mod tests { // update canonical block to b2, this would make b2a be removed assert_eq!(tree.connect_buffered_blocks_to_canonical_hashes_and_finalize(12), Ok(())); - assert_eq!(tree.is_block_known(block2.num_hash()).unwrap(), Some(BlockStatus::Valid)); + assert_eq!( + tree.is_block_known(block2.num_hash()).unwrap(), + Some(BlockStatus::Valid(BlockAttachment::Canonical)) + ); // Trie state: // b2 (finalized) diff --git a/crates/blockchain-tree/src/chain.rs b/crates/blockchain-tree/src/chain.rs index 8e192a4b25..578dae720a 100644 --- a/crates/blockchain-tree/src/chain.rs +++ b/crates/blockchain-tree/src/chain.rs @@ -8,8 +8,8 @@ use crate::BundleStateDataRef; use reth_db::database::Database; use reth_interfaces::{ blockchain_tree::{ - error::{BlockchainTreeError, InsertBlockError}, - BlockValidationKind, + error::{BlockchainTreeError, InsertBlockErrorKind}, + BlockAttachment, BlockValidationKind, }, consensus::{Consensus, ConsensusError}, RethResult, @@ -59,18 +59,19 @@ impl AppendableChain { self.chain } - /// Create a new chain that forks off the canonical chain. + /// Create a new chain that forks off of the canonical chain. /// - /// if [BlockValidationKind::Exhaustive] is provides this will verify the state root of the - /// block extending the canonical chain. - pub fn new_canonical_head_fork( + /// if [BlockValidationKind::Exhaustive] is specified, the method will verify the state root of + /// the block. + pub fn new_canonical_fork( block: SealedBlockWithSenders, parent_header: &SealedHeader, canonical_block_hashes: &BTreeMap, canonical_fork: ForkBlock, externals: &TreeExternals, + block_attachment: BlockAttachment, block_validation_kind: BlockValidationKind, - ) -> Result + ) -> Result where DB: Database, EF: ExecutorFactory, @@ -90,47 +91,13 @@ impl AppendableChain { parent_header, state_provider, externals, - BlockKind::ExtendsCanonicalHead, + block_attachment, block_validation_kind, - ) - .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?; + )?; Ok(Self { chain: Chain::new(vec![block], bundle_state, trie_updates) }) } - /// Create a new chain that forks off of the canonical chain. - pub fn new_canonical_fork( - block: SealedBlockWithSenders, - parent_header: &SealedHeader, - canonical_block_hashes: &BTreeMap, - canonical_fork: ForkBlock, - externals: &TreeExternals, - ) -> Result - where - DB: Database, - EF: ExecutorFactory, - { - let state = BundleStateWithReceipts::default(); - let empty = BTreeMap::new(); - - let state_provider = BundleStateDataRef { - state: &state, - sidechain_block_hashes: &empty, - canonical_block_hashes, - canonical_fork, - }; - - let bundle_state = Self::validate_and_execute_sidechain( - block.clone(), - parent_header, - state_provider, - externals, - ) - .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?; - - Ok(Self { chain: Chain::new(vec![block], bundle_state, None) }) - } - /// Create a new chain that forks off of an existing sidechain. /// /// This differs from [AppendableChain::new_canonical_fork] in that this starts a new fork. @@ -141,18 +108,16 @@ impl AppendableChain { canonical_block_hashes: &BTreeMap, canonical_fork: ForkBlock, externals: &TreeExternals, - ) -> Result + block_validation_kind: BlockValidationKind, + ) -> Result where DB: Database, EF: ExecutorFactory, { let parent_number = block.number - 1; - let parent = self.blocks().get(&parent_number).ok_or_else(|| { - InsertBlockError::tree_error( - BlockchainTreeError::BlockNumberNotFoundInChain { block_number: parent_number }, - block.block.clone(), - ) - })?; + let parent = self.blocks().get(&parent_number).ok_or( + BlockchainTreeError::BlockNumberNotFoundInChain { block_number: parent_number }, + )?; let mut state = self.state().clone(); @@ -166,13 +131,14 @@ impl AppendableChain { canonical_block_hashes, canonical_fork, }; - let block_state = Self::validate_and_execute_sidechain( + let (block_state, _) = Self::validate_and_execute( block.clone(), parent, bundle_state_data, externals, - ) - .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?; + BlockAttachment::HistoricalFork, + block_validation_kind, + )?; // extending will also optimize few things, mostly related to selfdestruct and wiping of // storage. state.extend(block_state); @@ -193,15 +159,15 @@ impl AppendableChain { /// Note: State root validation is limited to blocks that extend the canonical chain and is /// optional, see [BlockValidationKind]. So this function takes two parameters to determine /// if the state can and should be validated. - /// - [BlockKind] represents if the block extends the canonical chain, and thus if the state - /// root __can__ be validated. + /// - [BlockAttachment] represents if the block extends the canonical chain, and thus we can + /// cache the trie state updates. /// - [BlockValidationKind] determines if the state root __should__ be validated. fn validate_and_execute( block: SealedBlockWithSenders, parent_block: &SealedHeader, bundle_state_data_provider: BSDP, externals: &TreeExternals, - block_kind: BlockKind, + block_attachment: BlockAttachment, block_validation_kind: BlockValidationKind, ) -> RethResult<(BundleStateWithReceipts, Option)> where @@ -226,9 +192,15 @@ impl AppendableChain { // check state root if the block extends the canonical chain __and__ if state root // validation was requested. - if block_kind.extends_canonical_head() && block_validation_kind.is_exhaustive() { + if block_validation_kind.is_exhaustive() { // check state root - let (state_root, trie_updates) = provider.state_root_with_updates(&bundle_state)?; + let (state_root, trie_updates) = if block_attachment.is_canonical() { + provider + .state_root_with_updates(&bundle_state) + .map(|(root, updates)| (root, Some(updates)))? + } else { + (provider.state_root(&bundle_state)?, None) + }; if block.state_root != state_root { return Err(ConsensusError::BodyStateRootDiff( GotExpected { got: state_root, expected: block.state_root }.into(), @@ -236,35 +208,12 @@ impl AppendableChain { .into()) } - Ok((bundle_state, Some(trie_updates))) + Ok((bundle_state, trie_updates)) } else { Ok((bundle_state, None)) } } - /// Validate and execute the given sidechain block, skipping state root validation. - fn validate_and_execute_sidechain( - block: SealedBlockWithSenders, - parent_block: &SealedHeader, - bundle_state_data_provider: BSDP, - externals: &TreeExternals, - ) -> RethResult - where - BSDP: BundleStateDataProvider, - DB: Database, - EF: ExecutorFactory, - { - let (state, _) = Self::validate_and_execute( - block, - parent_block, - bundle_state_data_provider, - externals, - BlockKind::ForksHistoricalBlock, - BlockValidationKind::SkipStateRootValidation, - )?; - Ok(state) - } - /// Validate and execute the given block, and append it to this chain. /// /// This expects that the block's ancestors can be traced back to the `canonical_fork` (the @@ -285,9 +234,9 @@ impl AppendableChain { canonical_block_hashes: &BTreeMap, externals: &TreeExternals, canonical_fork: ForkBlock, - block_kind: BlockKind, + block_attachment: BlockAttachment, block_validation_kind: BlockValidationKind, - ) -> Result<(), InsertBlockError> + ) -> Result<(), InsertBlockErrorKind> where DB: Database, EF: ExecutorFactory, @@ -306,36 +255,12 @@ impl AppendableChain { parent_block, bundle_state_data, externals, - block_kind, + block_attachment, block_validation_kind, - ) - .map_err(|err| InsertBlockError::new(block.block.clone(), err.into()))?; + )?; // extend the state. self.chain.append_block(block, block_state, trie_updates); Ok(()) } } - -/// Represents what kind of block is being executed and validated. -/// -/// This is required because the state root check can only be performed if the targeted block can be -/// traced back to the canonical __head__. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum BlockKind { - /// The `block` is a descendant of the canonical head: - /// - /// [`head..(block.parent)*,block`] - ExtendsCanonicalHead, - /// The block can be traced back to an ancestor of the canonical head: a historical block, but - /// this chain does __not__ include the canonical head. - ForksHistoricalBlock, -} - -impl BlockKind { - /// Returns `true` if the block is a descendant of the canonical head. - #[inline] - pub(crate) fn extends_canonical_head(&self) -> bool { - matches!(self, BlockKind::ExtendsCanonicalHead) - } -} diff --git a/crates/blockchain-tree/src/config.rs b/crates/blockchain-tree/src/config.rs index 89a861206d..733cb6a1a0 100644 --- a/crates/blockchain-tree/src/config.rs +++ b/crates/blockchain-tree/src/config.rs @@ -78,8 +78,7 @@ impl BlockchainTreeConfig { /// It is calculated as the maximum of `max_reorg_depth` (which is the number of blocks required /// for the deepest reorg possible according to the consensus protocol) and /// `num_of_additional_canonical_block_hashes` (which is the number of block hashes needed to - /// satisfy the `BLOCKHASH` opcode in the EVM. See [`crate::BundleStateDataRef`] and - /// [`crate::AppendableChain::new_canonical_head_fork`] where it's used). + /// satisfy the `BLOCKHASH` opcode in the EVM. See [`crate::BundleStateDataRef`]). pub fn num_of_canonical_hashes(&self) -> u64 { self.max_reorg_depth.max(self.num_of_additional_canonical_block_hashes) } diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index 89c39f0c1e..91c26861e8 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/crates/consensus/beacon/src/engine/mod.rs @@ -1263,14 +1263,19 @@ where let mut latest_valid_hash = None; let block = Arc::new(block); let status = match status { - InsertPayloadOk::Inserted(BlockStatus::Valid) => { + InsertPayloadOk::Inserted(BlockStatus::Valid(attachment)) => { latest_valid_hash = Some(block_hash); - self.listeners.notify(BeaconConsensusEngineEvent::CanonicalBlockAdded(block)); + let event = if attachment.is_canonical() { + BeaconConsensusEngineEvent::CanonicalBlockAdded(block) + } else { + BeaconConsensusEngineEvent::ForkBlockAdded(block) + }; + self.listeners.notify(event); PayloadStatusEnum::Valid } - InsertPayloadOk::Inserted(BlockStatus::Accepted) => { - self.listeners.notify(BeaconConsensusEngineEvent::ForkBlockAdded(block)); - PayloadStatusEnum::Accepted + InsertPayloadOk::AlreadySeen(BlockStatus::Valid(_)) => { + latest_valid_hash = Some(block_hash); + PayloadStatusEnum::Valid } InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) | InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { @@ -1284,11 +1289,6 @@ where // not known to be invalid, but we don't know anything else PayloadStatusEnum::Syncing } - InsertPayloadOk::AlreadySeen(BlockStatus::Valid) => { - latest_valid_hash = Some(block_hash); - PayloadStatusEnum::Valid - } - InsertPayloadOk::AlreadySeen(BlockStatus::Accepted) => PayloadStatusEnum::Accepted, }; Ok(PayloadStatus::new(status, latest_valid_hash)) } @@ -1351,20 +1351,14 @@ where /// /// ## [BlockStatus::Valid] /// - /// The block is connected to the current canonical head and is valid. - /// If the engine is still SYNCING, then we can try again to make the chain canonical. - /// - /// ## [BlockStatus::Accepted] - /// - /// All ancestors are known, but the block is not connected to the current canonical _head_. If - /// the block is an ancestor of the current forkchoice head, then we can try again to make the - /// chain canonical, which would trigger a reorg in this case since the new head is therefore - /// not connected to the current head. + /// The block is connected to the current canonical chain and is valid. + /// If the block is an ancestor of the current forkchoice head, then we can try again to make + /// the chain canonical. /// /// ## [BlockStatus::Disconnected] /// - /// The block is not connected to the canonical head, and we need to download the missing parent - /// first. + /// The block is not connected to the canonical chain, and we need to download the missing + /// parent first. /// /// ## Insert Error /// @@ -1386,12 +1380,10 @@ where { Ok(status) => { match status { - InsertPayloadOk::Inserted(BlockStatus::Valid) => { - // block is connected to the current canonical head and is valid. - self.try_make_sync_target_canonical(downloaded_num_hash); - } - InsertPayloadOk::Inserted(BlockStatus::Accepted) => { - // block is connected to the canonical chain, but not the current head + InsertPayloadOk::Inserted(BlockStatus::Valid(_)) => { + // block is connected to the canonical chain and is valid. + // if it's not connected to current canonical head, the state root + // has not been validated. self.try_make_sync_target_canonical(downloaded_num_hash); } InsertPayloadOk::Inserted(BlockStatus::Disconnected { @@ -1469,7 +1461,7 @@ where /// Attempt to form a new canonical chain based on the current sync target. /// /// This is invoked when we successfully __downloaded__ a new block from the network which - /// resulted in either [BlockStatus::Accepted] or [BlockStatus::Valid]. + /// resulted in [BlockStatus::Valid]. /// /// Note: This will not succeed if the sync target has changed since the block download request /// was issued and the new target is still disconnected and additional missing blocks are diff --git a/crates/interfaces/src/blockchain_tree/mod.rs b/crates/interfaces/src/blockchain_tree/mod.rs index 3d77fef03c..0eabb5c763 100644 --- a/crates/interfaces/src/blockchain_tree/mod.rs +++ b/crates/interfaces/src/blockchain_tree/mod.rs @@ -182,17 +182,16 @@ impl CanonicalOutcome { /// From Engine API spec, block inclusion can be valid, accepted or invalid. /// Invalid case is already covered by error, but we need to make distinction -/// between if it is valid (extends canonical chain) or just accepted (is side chain). -/// If we don't know the block parent we are returning Disconnected status -/// as we can't make a claim if block is valid or not. +/// between valid blocks that extend canonical chain and the ones that fork off +/// into side chains (see [BlockAttachment]). If we don't know the block +/// parent we are returning Disconnected status as we can't make a claim if +/// block is valid or not. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum BlockStatus { - /// If block validation is valid and block extends canonical chain. - /// In BlockchainTree sense it forks on canonical tip. - Valid, - /// If the block is valid, but it does not extend canonical chain. - /// (It is side chain) or hasn't been fully validated but ancestors of a payload are known. - Accepted, + /// If block is valid and block extends canonical chain. + /// In BlockchainTree terms, it forks off canonical tip. + Valid(BlockAttachment), + /// If block is valid and block forks off canonical chain. /// If blocks is not connected to canonical chain. Disconnected { /// The lowest ancestor block that is not connected to the canonical chain. @@ -200,6 +199,31 @@ pub enum BlockStatus { }, } +/// Represents what kind of block is being executed and validated. +/// +/// This is required to: +/// - differentiate whether trie state updates should be cached. +/// - inform other +/// This is required because the state root check can only be performed if the targeted block can be +/// traced back to the canonical __head__. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BlockAttachment { + /// The `block` is canonical or a descendant of the canonical head. + /// ([`head..(block.parent)*,block`]) + Canonical, + /// The block can be traced back to an ancestor of the canonical head: a historical block, but + /// this chain does __not__ include the canonical head. + HistoricalFork, +} + +impl BlockAttachment { + /// Returns `true` if the block is canonical or a descendant of the canonical head. + #[inline] + pub const fn is_canonical(&self) -> bool { + matches!(self, BlockAttachment::Canonical) + } +} + /// How a payload was inserted if it was valid. /// /// If the payload was valid, but has already been seen, [`InsertPayloadOk::AlreadySeen(_)`] is diff --git a/crates/storage/provider/src/providers/state/historical.rs b/crates/storage/provider/src/providers/state/historical.rs index 85460498d3..f2892ee7dc 100644 --- a/crates/storage/provider/src/providers/state/historical.rs +++ b/crates/storage/provider/src/providers/state/historical.rs @@ -12,9 +12,10 @@ use reth_db::{ }; use reth_interfaces::provider::ProviderResult; use reth_primitives::{ - trie::AccountProof, Account, Address, BlockNumber, Bytecode, StorageKey, StorageValue, B256, + constants::EPOCH_SLOTS, trie::AccountProof, Account, Address, BlockNumber, Bytecode, + StorageKey, StorageValue, B256, }; -use reth_trie::updates::TrieUpdates; +use reth_trie::{updates::TrieUpdates, HashedPostState}; /// State provider for a given block number which takes a tx reference. /// @@ -95,6 +96,31 @@ impl<'b, TX: DbTx> HistoricalStateProviderRef<'b, TX> { ) } + /// Retrieve revert hashed state for this history provider. + fn revert_state(&self) -> ProviderResult { + if !self.lowest_available_blocks.is_account_history_available(self.block_number) || + !self.lowest_available_blocks.is_storage_history_available(self.block_number) + { + return Err(ProviderError::StateAtBlockPruned(self.block_number)) + } + + let (tip, _) = self + .tx + .cursor_read::()? + .last()? + .ok_or(ProviderError::BestBlockNotFound)?; + + if tip.saturating_sub(self.block_number) > EPOCH_SLOTS { + tracing::warn!( + target: "provider::historical_sp", + target = self.block_number, + "Attempt to calculate state root for an old block might result in OOM, tread carefully" + ); + } + + Ok(HashedPostState::from_revert_range(self.tx, self.block_number..=tip)?) + } + fn history_info( &self, key: K, @@ -199,15 +225,23 @@ impl<'b, TX: DbTx> BlockHashReader for HistoricalStateProviderRef<'b, TX> { } impl<'b, TX: DbTx> StateRootProvider for HistoricalStateProviderRef<'b, TX> { - fn state_root(&self, _bundle_state: &BundleStateWithReceipts) -> ProviderResult { - Err(ProviderError::StateRootNotAvailableForHistoricalBlock) + fn state_root(&self, state: &BundleStateWithReceipts) -> ProviderResult { + let mut revert_state = self.revert_state()?; + revert_state.extend(state.hash_state_slow()); + revert_state.sort(); + revert_state.state_root(self.tx).map_err(|err| ProviderError::Database(err.into())) } fn state_root_with_updates( &self, - _bundle_state: &BundleStateWithReceipts, + state: &BundleStateWithReceipts, ) -> ProviderResult<(B256, TrieUpdates)> { - Err(ProviderError::StateRootNotAvailableForHistoricalBlock) + let mut revert_state = self.revert_state()?; + revert_state.extend(state.hash_state_slow()); + revert_state.sort(); + revert_state + .state_root_with_updates(self.tx) + .map_err(|err| ProviderError::Database(err.into())) } } diff --git a/crates/trie/src/hashed_cursor/post_state.rs b/crates/trie/src/hashed_cursor/post_state.rs index 81762aba6a..6a6aebb80b 100644 --- a/crates/trie/src/hashed_cursor/post_state.rs +++ b/crates/trie/src/hashed_cursor/post_state.rs @@ -270,7 +270,7 @@ where // If the storage has been wiped at any point storage.wiped && // and the current storage does not contain any non-zero values - storage.non_zero_valued_storage.is_empty() + storage.non_zero_valued_slots.is_empty() } None => self.cursor.seek_exact(key)?.is_none(), }; @@ -294,12 +294,11 @@ where if let Some(storage) = self.post_state.storages.get(&account) { debug_assert!(storage.sorted, "`HashedStorage` must be pre-sorted"); - post_state_entry = storage.non_zero_valued_storage.get(self.post_state_storage_index); + post_state_entry = storage.non_zero_valued_slots.get(self.post_state_storage_index); while post_state_entry.map(|(slot, _)| slot < &subkey).unwrap_or_default() { self.post_state_storage_index += 1; - post_state_entry = - storage.non_zero_valued_storage.get(self.post_state_storage_index); + post_state_entry = storage.non_zero_valued_slots.get(self.post_state_storage_index); } } @@ -374,11 +373,10 @@ where if let Some(storage) = self.post_state.storages.get(&account) { debug_assert!(storage.sorted, "`HashedStorage` must be pre-sorted"); - post_state_entry = storage.non_zero_valued_storage.get(self.post_state_storage_index); + post_state_entry = storage.non_zero_valued_slots.get(self.post_state_storage_index); while post_state_entry.map(|(slot, _)| slot <= last_slot).unwrap_or_default() { self.post_state_storage_index += 1; - post_state_entry = - storage.non_zero_valued_storage.get(self.post_state_storage_index); + post_state_entry = storage.non_zero_valued_slots.get(self.post_state_storage_index); } } diff --git a/crates/trie/src/state.rs b/crates/trie/src/state.rs index 552d6d5fed..620254edad 100644 --- a/crates/trie/src/state.rs +++ b/crates/trie/src/state.rs @@ -78,7 +78,7 @@ impl HashedPostState { /// NOTE: In order to have the resulting [HashedPostState] be a correct /// overlay of the plain state, the end of the range must be the current tip. pub fn from_revert_range( - tx: TX, + tx: &TX, range: RangeInclusive, ) -> Result { // A single map for aggregating state changes where each map value is a tuple @@ -119,7 +119,7 @@ impl HashedPostState { this.insert_account(hashed_address, account_change); } - // The `wiped`` flag indicates only whether previous storage entries should be looked + // The `wiped` flag indicates only whether previous storage entries should be looked // up in db or not. For reverts it's a noop since all wiped changes had been written as // storage reverts. let mut hashed_storage = HashedStorage::new(false); @@ -132,6 +132,40 @@ impl HashedPostState { Ok(this.sorted()) } + /// Extend this hashed post state with contents of another. + /// Entries in the second hashed post state take precedence. + pub fn extend(&mut self, other: Self) { + // Merge accounts and insert them into extended state. + let mut accounts: HashMap> = HashMap::from_iter( + self.accounts + .drain(..) + .map(|(hashed_address, account)| (hashed_address, Some(account))) + .chain( + self.destroyed_accounts.drain().map(|hashed_address| (hashed_address, None)), + ), + ); + for (hashed_address, account) in other.accounts { + accounts.insert(hashed_address, Some(account)); + } + for hashed_address in other.destroyed_accounts { + accounts.insert(hashed_address, None); + } + for (hashed_address, account) in accounts { + self.insert_account(hashed_address, account); + } + + for (hashed_address, storage) in other.storages { + match self.storages.entry(hashed_address) { + hash_map::Entry::Vacant(entry) => { + entry.insert(storage); + } + hash_map::Entry::Occupied(mut entry) => { + entry.get_mut().extend(storage); + } + } + } + } + /// Sort and return self. pub fn sorted(mut self) -> Self { self.sort(); @@ -204,7 +238,7 @@ impl HashedPostState { account_prefix_set.insert(Nibbles::unpack(hashed_address)); let storage_prefix_set_entry = storage_prefix_set.entry(*hashed_address).or_default(); - for (hashed_slot, _) in &hashed_storage.non_zero_valued_storage { + for (hashed_slot, _) in &hashed_storage.non_zero_valued_slots { storage_prefix_set_entry.insert(Nibbles::unpack(hashed_slot)); } for hashed_slot in &hashed_storage.zero_valued_slots { @@ -223,6 +257,7 @@ impl HashedPostState { &self, tx: &'a TX, ) -> StateRoot<&'a TX, HashedPostStateCursorFactory<'a, '_, TX>> { + assert!(self.sorted, "Hashed post state must be sorted for state root calculation"); let (account_prefix_set, storage_prefix_set) = self.construct_prefix_sets(); let hashed_cursor_factory = HashedPostStateCursorFactory::new(tx, self); StateRoot::from_tx(tx) @@ -277,10 +312,10 @@ impl HashedPostState { } /// The post state account storage with hashed slots. -#[derive(Debug, Clone, Eq, PartialEq)] +#[derive(Clone, Eq, PartialEq, Debug)] pub struct HashedStorage { /// Hashed storage slots with non-zero. - pub(crate) non_zero_valued_storage: Vec<(B256, U256)>, + pub(crate) non_zero_valued_slots: Vec<(B256, U256)>, /// Slots that have been zero valued. pub(crate) zero_valued_slots: AHashSet, /// Whether the storage was wiped or not. @@ -293,13 +328,32 @@ impl HashedStorage { /// Create new instance of [HashedStorage]. pub fn new(wiped: bool) -> Self { Self { - non_zero_valued_storage: Vec::new(), + non_zero_valued_slots: Vec::new(), zero_valued_slots: AHashSet::new(), wiped, sorted: true, // empty is sorted } } + /// Extend hashed storage with contents of other. + /// The entries in second hashed storage take precedence. + pub fn extend(&mut self, other: Self) { + let mut entries: HashMap = + HashMap::from_iter(self.non_zero_valued_slots.drain(..).chain( + self.zero_valued_slots.drain().map(|hashed_slot| (hashed_slot, U256::ZERO)), + )); + for (hashed_slot, value) in other.non_zero_valued_slots { + entries.insert(hashed_slot, value); + } + for hashed_slot in other.zero_valued_slots { + entries.insert(hashed_slot, U256::ZERO); + } + for (hashed_slot, value) in entries { + self.insert_slot(hashed_slot, value); + } + self.wiped |= other.wiped; + } + /// Returns `true` if the storage was wiped. pub fn wiped(&self) -> bool { self.wiped @@ -310,13 +364,13 @@ impl HashedStorage { self.zero_valued_slots .iter() .map(|slot| (*slot, U256::ZERO)) - .chain(self.non_zero_valued_storage.iter().cloned()) + .chain(self.non_zero_valued_slots.iter().cloned()) } /// Sorts the non zero value storage entries. pub fn sort_storage(&mut self) { if !self.sorted { - self.non_zero_valued_storage.sort_unstable_by_key(|(slot, _)| *slot); + self.non_zero_valued_slots.sort_unstable_by_key(|(slot, _)| *slot); self.sorted = true; } } @@ -327,7 +381,7 @@ impl HashedStorage { if value.is_zero() { self.zero_valued_slots.insert(slot); } else { - self.non_zero_valued_storage.push((slot, value)); + self.non_zero_valued_slots.push((slot, value)); self.sorted = false; } } diff --git a/crates/trie/src/trie.rs b/crates/trie/src/trie.rs index 917a8aa214..1d103a475c 100644 --- a/crates/trie/src/trie.rs +++ b/crates/trie/src/trie.rs @@ -1164,7 +1164,6 @@ mod tests { } #[test] - fn account_trie_around_extension_node_with_dbtrie() { let factory = create_test_provider_factory(); let tx = factory.provider_rw().unwrap();