From 16412116a715443704b3e33da851b9a70d550a0c Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Fri, 21 Apr 2023 11:54:34 +0200 Subject: [PATCH] perf(tree): check block before inserting (#2324) --- .../src/blockchain_tree/block_indices.rs | 3 +- .../src/blockchain_tree/mod.rs | 77 +++++++++++-------- .../src/blockchain_tree/shareable.rs | 10 +++ 3 files changed, 59 insertions(+), 31 deletions(-) diff --git a/crates/blockchain-tree/src/blockchain_tree/block_indices.rs b/crates/blockchain-tree/src/blockchain_tree/block_indices.rs index fb98a1d4ab..8637a3092e 100644 --- a/crates/blockchain-tree/src/blockchain_tree/block_indices.rs +++ b/crates/blockchain-tree/src/blockchain_tree/block_indices.rs @@ -274,8 +274,9 @@ impl BlockIndices { } /// Used for finalization of block. + /// /// Return list of chains for removal that depend on finalized canonical chain. - pub fn finalize_canonical_blocks( + pub(crate) fn finalize_canonical_blocks( &mut self, finalized_block: BlockNumber, num_of_additional_canonical_hashes_to_retain: u64, diff --git a/crates/blockchain-tree/src/blockchain_tree/mod.rs b/crates/blockchain-tree/src/blockchain_tree/mod.rs index 844dc2396c..2138be4148 100644 --- a/crates/blockchain-tree/src/blockchain_tree/mod.rs +++ b/crates/blockchain-tree/src/blockchain_tree/mod.rs @@ -142,6 +142,34 @@ impl BlockchainTree }) } + /// Ensures that block matches range requirements depending on the current state of the tree. + /// - Ensures that the block's height is not lagging behind the currently tracked last + /// finalized block. + /// - Ensures that the distance of from the currently tracked last finalized block and the + /// given block's number is not larger than the configured maximum block range + pub(crate) fn ensure_block_is_in_range(&self, block: &SealedBlock) -> Result<(), ExecError> { + // check if block number is inside pending block slide + let last_finalized_block = self.block_indices.last_finalized_block(); + if block.number <= last_finalized_block { + return Err(ExecError::PendingBlockIsFinalized { + block_number: block.number, + block_hash: block.hash(), + last_finalized: last_finalized_block, + }) + } + + // we will not even try to insert blocks that are too far in the future. + if block.number > last_finalized_block + self.config.max_blocks_in_chain() { + return Err(ExecError::PendingBlockIsInFuture { + block_number: block.number, + block_hash: block.hash(), + last_finalized: last_finalized_block, + }) + } + + Ok(()) + } + /// Expose internal indices of the BlockchainTree. pub fn block_indices(&self) -> &BlockIndices { &self.block_indices @@ -230,7 +258,7 @@ impl BlockchainTree let canonical_block_hashes = self.block_indices.canonical_chain(); // append the block if it is continuing the chain. - if chain_tip == block.parent_hash { + return if chain_tip == block.parent_hash { let block_hash = block.hash(); let block_number = block.number; parent_chain.append_block( @@ -242,7 +270,7 @@ impl BlockchainTree )?; self.block_indices.insert_non_fork_block(block_number, block_hash, chain_id); - return Ok(BlockStatus::Valid) + Ok(BlockStatus::Valid) } else { let chain = parent_chain.new_chain_fork( block, @@ -252,7 +280,7 @@ impl BlockchainTree &self.externals, )?; self.insert_chain(chain); - return Ok(BlockStatus::Accepted) + Ok(BlockStatus::Accepted) } } // if not found, check if the parent can be found inside canonical chain. @@ -379,15 +407,15 @@ impl BlockchainTree /// Insert a block (with senders recovered) in the tree. /// - /// Returns `true` if: + /// Returns the [BlockStatus] on success: /// /// - The block is already part of a sidechain in the tree, or /// - The block is already part of the canonical chain, or /// - The parent is part of a sidechain in the tree, and we can fork at this block, or /// - The parent is part of the canonical chain, and we can fork at this block /// - /// Otherwise `false` is returned, indicating that neither the block nor its parent is part of - /// the chain or any sidechains. + /// Otherwise, and error is returned, indicating that neither the block nor its parent is part + /// of the chain or any sidechains. /// /// This means that if the block becomes canonical, we need to fetch the missing blocks over /// P2P. @@ -400,35 +428,24 @@ impl BlockchainTree &mut self, block: SealedBlockWithSenders, ) -> Result { - // check if block number is inside pending block slide - let last_finalized_block = self.block_indices.last_finalized_block(); - if block.number <= last_finalized_block { - return Err(ExecError::PendingBlockIsFinalized { - block_number: block.number, - block_hash: block.hash(), - last_finalized: last_finalized_block, - } - .into()) - } - - // we will not even try to insert blocks that are too far in future. - if block.number > last_finalized_block + self.config.max_blocks_in_chain() { - return Err(ExecError::PendingBlockIsInFuture { - block_number: block.number, - block_hash: block.hash(), - last_finalized: last_finalized_block, - } - .into()) - } + self.ensure_block_is_in_range(&block.block)?; + self.insert_in_range_block_with_senders(block) + } + /// Same as [BlockchainTree::insert_block_with_senders] but expects that the block is in range, + /// See [BlockchainTree::ensure_block_is_in_range]. + pub(crate) fn insert_in_range_block_with_senders( + &mut self, + block: SealedBlockWithSenders, + ) -> Result { // check if block known and is already inside Tree if let Some(chain_id) = self.block_indices.get_blocks_chain_id(&block.hash()) { let canonical_fork = self.canonical_fork(chain_id).expect("Chain id is valid"); - // if block chain extends canonical chain - if canonical_fork == self.block_indices.canonical_tip() { - return Ok(BlockStatus::Valid) + // if blockchain extends canonical chain + return if canonical_fork == self.block_indices.canonical_tip() { + Ok(BlockStatus::Valid) } else { - return Ok(BlockStatus::Accepted) + Ok(BlockStatus::Accepted) } } diff --git a/crates/blockchain-tree/src/blockchain_tree/shareable.rs b/crates/blockchain-tree/src/blockchain_tree/shareable.rs index 5de2cbaae0..d91bd12a92 100644 --- a/crates/blockchain-tree/src/blockchain_tree/shareable.rs +++ b/crates/blockchain-tree/src/blockchain_tree/shareable.rs @@ -36,6 +36,16 @@ impl ShareableBlockchainTree BlockchainTreeEngine for ShareableBlockchainTree { + /// Recover senders and call [`BlockchainTreeEngine::insert_block_with_senders`]. + fn insert_block(&self, block: SealedBlock) -> Result { + let mut tree = self.tree.write(); + tree.ensure_block_is_in_range(&block)?; + let block = block + .seal_with_senders() + .ok_or(reth_interfaces::executor::Error::SenderRecoveryError)?; + tree.insert_in_range_block_with_senders(block) + } + fn insert_block_with_senders( &self, block: SealedBlockWithSenders,