diff --git a/crates/blockchain-tree/src/blockchain_tree.rs b/crates/blockchain-tree/src/blockchain_tree.rs index 93a0b077d8..9cd869c11e 100644 --- a/crates/blockchain-tree/src/blockchain_tree.rs +++ b/crates/blockchain-tree/src/blockchain_tree.rs @@ -8,7 +8,7 @@ use reth_db::{cursor::DbCursorRO, database::Database, tables, transaction::DbTx} use reth_interfaces::{ blockchain_tree::{ error::{BlockchainTreeError, InsertBlockError, InsertBlockErrorKind}, - BlockStatus, CanonicalOutcome, + BlockStatus, CanonicalOutcome, InsertPayloadOk, }, consensus::{Consensus, ConsensusError}, executor::{BlockExecutionError, BlockValidationError}, @@ -595,7 +595,7 @@ impl BlockchainTree pub fn insert_block_without_senders( &mut self, block: SealedBlock, - ) -> Result { + ) -> Result { match block.try_seal_with_senders() { Ok(block) => self.insert_block(block), Err(block) => Err(InsertBlockError::sender_recovery_error(block)), @@ -683,10 +683,10 @@ impl BlockchainTree pub fn insert_block( &mut self, block: SealedBlockWithSenders, - ) -> Result { + ) -> Result { // check if we already have this block match self.is_block_known(block.num_hash()) { - Ok(Some(status)) => return Ok(status), + Ok(Some(status)) => return Ok(InsertPayloadOk::AlreadySeen(status)), Err(err) => return Err(InsertBlockError::new(block.block, err)), _ => {} } @@ -696,7 +696,7 @@ impl BlockchainTree return Err(InsertBlockError::consensus_error(err, block.block)) } - self.try_insert_validated_block(block) + Ok(InsertPayloadOk::Inserted(self.try_insert_validated_block(block)?)) } /// Finalize blocks up until and including `finalized_block`, and remove them from the tree. @@ -1220,7 +1220,9 @@ mod tests { // block 2 parent is not known, block2 is buffered. assert_eq!( tree.insert_block(block2.clone()).unwrap(), - BlockStatus::Disconnected { missing_parent: block2.parent_num_hash() } + InsertPayloadOk::Inserted(BlockStatus::Disconnected { + missing_parent: block2.parent_num_hash() + }) ); // Buffered block: [block2] @@ -1248,7 +1250,10 @@ mod tests { assert_eq!(tree.is_block_known(old_block).unwrap_err().as_tree_error(), Some(err)); // insert block1 and buffered block2 is inserted - assert_eq!(tree.insert_block(block1.clone()).unwrap(), BlockStatus::Valid); + assert_eq!( + tree.insert_block(block1.clone()).unwrap(), + InsertPayloadOk::Inserted(BlockStatus::Valid) + ); // Buffered blocks: [] // Trie state: @@ -1267,11 +1272,17 @@ mod tests { .with_pending_blocks((block1.number, HashSet::from([block1.hash]))) .assert(&tree); - // already inserted block will return true. - assert_eq!(tree.insert_block(block1.clone()).unwrap(), BlockStatus::Valid); + // already inserted block will `InsertPayloadOk::AlreadySeen(_)` + assert_eq!( + tree.insert_block(block1.clone()).unwrap(), + InsertPayloadOk::AlreadySeen(BlockStatus::Valid) + ); // block two is already inserted. - assert_eq!(tree.insert_block(block2.clone()).unwrap(), BlockStatus::Valid); + assert_eq!( + tree.insert_block(block2.clone()).unwrap(), + InsertPayloadOk::AlreadySeen(BlockStatus::Valid) + ); // make block1 canonical assert!(tree.make_canonical(&block1.hash()).is_ok()); @@ -1308,7 +1319,10 @@ mod tests { block2a.hash = block2a_hash; // reinsert two blocks that point to canonical chain - assert_eq!(tree.insert_block(block1a.clone()).unwrap(), BlockStatus::Accepted); + assert_eq!( + tree.insert_block(block1a.clone()).unwrap(), + InsertPayloadOk::Inserted(BlockStatus::Accepted) + ); TreeTester::default() .with_chain_num(1) @@ -1320,7 +1334,10 @@ mod tests { .with_pending_blocks((block2.number + 1, HashSet::from([]))) .assert(&tree); - assert_eq!(tree.insert_block(block2a.clone()).unwrap(), BlockStatus::Accepted); + assert_eq!( + tree.insert_block(block2a.clone()).unwrap(), + InsertPayloadOk::Inserted(BlockStatus::Accepted) + ); // Trie state: // b2 b2a (side chain) // | / @@ -1504,7 +1521,9 @@ mod tests { assert_eq!( tree.insert_block(block2b.clone()).unwrap(), - BlockStatus::Disconnected { missing_parent: block2b.parent_num_hash() } + InsertPayloadOk::Inserted(BlockStatus::Disconnected { + missing_parent: block2b.parent_num_hash() + }) ); TreeTester::default() diff --git a/crates/blockchain-tree/src/shareable.rs b/crates/blockchain-tree/src/shareable.rs index aa7f9215e0..75ced6dc90 100644 --- a/crates/blockchain-tree/src/shareable.rs +++ b/crates/blockchain-tree/src/shareable.rs @@ -4,8 +4,8 @@ use parking_lot::RwLock; use reth_db::database::Database; use reth_interfaces::{ blockchain_tree::{ - error::InsertBlockError, BlockStatus, BlockchainTreeEngine, BlockchainTreeViewer, - CanonicalOutcome, + error::InsertBlockError, BlockchainTreeEngine, BlockchainTreeViewer, CanonicalOutcome, + InsertPayloadOk, }, consensus::Consensus, Error, @@ -44,7 +44,10 @@ impl BlockchainTreeEngine self.tree.write().buffer_block(block) } - fn insert_block(&self, block: SealedBlockWithSenders) -> Result { + fn insert_block( + &self, + block: SealedBlockWithSenders, + ) -> Result { trace!(target: "blockchain_tree", hash=?block.hash, number=block.number, parent_hash=?block.parent_hash, "Inserting block"); self.tree.write().insert_block(block) } diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index a43729dabb..b31dea4092 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/crates/consensus/beacon/src/engine/mod.rs @@ -60,6 +60,7 @@ pub(crate) mod sync; use crate::engine::forkchoice::{ForkchoiceStateHash, ForkchoiceStateTracker}; pub use event::BeaconConsensusEngineEvent; +use reth_interfaces::blockchain_tree::InsertPayloadOk; /// The maximum number of invalid headers that can be tracked by the engine. const MAX_INVALID_HEADERS: u32 = 512u32; @@ -893,16 +894,17 @@ where let mut latest_valid_hash = None; let block = Arc::new(block); let status = match status { - BlockStatus::Valid => { + InsertPayloadOk::Inserted(BlockStatus::Valid) => { latest_valid_hash = Some(block_hash); self.listeners.notify(BeaconConsensusEngineEvent::CanonicalBlockAdded(block)); PayloadStatusEnum::Valid } - BlockStatus::Accepted => { + InsertPayloadOk::Inserted(BlockStatus::Accepted) => { self.listeners.notify(BeaconConsensusEngineEvent::ForkBlockAdded(block)); PayloadStatusEnum::Accepted } - BlockStatus::Disconnected { .. } => { + InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) | + InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { // check if the block's parent is already marked as invalid if let Some(status) = self.check_invalid_ancestor_with_head(block.parent_hash, block.hash) @@ -913,6 +915,11 @@ 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)) } @@ -1014,18 +1021,19 @@ where match self.blockchain.insert_block_without_senders(block) { Ok(status) => { match status { - BlockStatus::Valid => { + InsertPayloadOk::Inserted(BlockStatus::Valid) => { // block is connected to the current canonical head and is valid. self.try_make_sync_target_canonical(num_hash); } - BlockStatus::Accepted => { + InsertPayloadOk::Inserted(BlockStatus::Accepted) => { // block is connected to the canonical chain, but not the current head self.try_make_sync_target_canonical(num_hash); } - BlockStatus::Disconnected { missing_parent } => { + InsertPayloadOk::Inserted(BlockStatus::Disconnected { missing_parent }) => { // continue downloading the missing parent self.sync.download_full_block(missing_parent.hash); } + _ => (), } } Err(err) => { diff --git a/crates/interfaces/src/blockchain_tree/mod.rs b/crates/interfaces/src/blockchain_tree/mod.rs index 6a63de4775..4e9e3b4771 100644 --- a/crates/interfaces/src/blockchain_tree/mod.rs +++ b/crates/interfaces/src/blockchain_tree/mod.rs @@ -21,7 +21,7 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync { fn insert_block_without_senders( &self, block: SealedBlock, - ) -> Result { + ) -> Result { match block.try_seal_with_senders() { Ok(block) => self.insert_block(block), Err(block) => Err(InsertBlockError::sender_recovery_error(block)), @@ -43,7 +43,10 @@ pub trait BlockchainTreeEngine: BlockchainTreeViewer + Send + Sync { fn buffer_block(&self, block: SealedBlockWithSenders) -> Result<(), InsertBlockError>; /// Insert block with senders - fn insert_block(&self, block: SealedBlockWithSenders) -> Result; + fn insert_block( + &self, + block: SealedBlockWithSenders, + ) -> Result; /// Finalize blocks up until and including `finalized_block`, and remove them from the tree. fn finalize_block(&self, finalized_block: BlockNumber); @@ -135,6 +138,18 @@ pub enum BlockStatus { }, } +/// How a payload was inserted if it was valid. +/// +/// If the payload was valid, but has already been seen, [`InsertPayloadOk::AlreadySeen(_)`] is +/// returned, otherwise [`InsertPayloadOk::Inserted(_)`] is returned. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum InsertPayloadOk { + /// The payload was valid, but we have already seen it. + AlreadySeen(BlockStatus), + /// The payload was valid and inserted into the tree. + Inserted(BlockStatus), +} + /// Allows read only functionality on the blockchain tree. /// /// Tree contains all blocks that are not canonical that can potentially be included diff --git a/crates/storage/provider/src/providers/mod.rs b/crates/storage/provider/src/providers/mod.rs index a027bea8ae..e45080d2bd 100644 --- a/crates/storage/provider/src/providers/mod.rs +++ b/crates/storage/provider/src/providers/mod.rs @@ -7,7 +7,7 @@ use crate::{ }; use reth_db::{database::Database, models::StoredBlockBodyIndices}; use reth_interfaces::{ - blockchain_tree::{BlockStatus, BlockchainTreeEngine, BlockchainTreeViewer}, + blockchain_tree::{BlockchainTreeEngine, BlockchainTreeViewer}, consensus::ForkchoiceState, Error, Result, }; @@ -37,7 +37,9 @@ mod state; use crate::{providers::chain_info::ChainInfoTracker, traits::BlockSource}; pub use database::*; pub use post_state_provider::PostStateProvider; -use reth_interfaces::blockchain_tree::{error::InsertBlockError, CanonicalOutcome}; +use reth_interfaces::blockchain_tree::{ + error::InsertBlockError, CanonicalOutcome, InsertPayloadOk, +}; /// The main type for interacting with the blockchain. /// @@ -537,7 +539,7 @@ where fn insert_block( &self, block: SealedBlockWithSenders, - ) -> std::result::Result { + ) -> std::result::Result { self.tree.insert_block(block) }