diff --git a/crates/blockchain-tree-api/src/lib.rs b/crates/blockchain-tree-api/src/lib.rs index b1f1240bb5b77..d5c049fa1447e 100644 --- a/crates/blockchain-tree-api/src/lib.rs +++ b/crates/blockchain-tree-api/src/lib.rs @@ -198,6 +198,36 @@ impl CanonicalOutcome { } } +/// Block inclusion can be valid, accepted, or invalid. Invalid blocks are returned as an error +/// variant. +/// +/// If we don't know the block's parent, we return `Disconnected`, as we can't claim that the block +/// is valid or not. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum BlockStatusTwo { + /// The block is valid and block extends canonical chain. + Valid, + /// The block may be valid and has an unknown missing ancestor. + Disconnected { + /// Current canonical head. + head: BlockNumHash, + /// The lowest ancestor block that is not connected to the canonical chain. + missing_ancestor: BlockNumHash, + }, +} + +/// How a payload was inserted if it was valid. +/// +/// If the payload was valid, but has already been seen, [`InsertPayloadOkTwo::AlreadySeen(_)`] is +/// returned, otherwise [`InsertPayloadOkTwo::Inserted(_)`] is returned. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum InsertPayloadOkTwo { + /// The payload was valid, but we have already seen it. + AlreadySeen(BlockStatusTwo), + /// The payload was valid and inserted into the tree. + Inserted(BlockStatusTwo), +} + /// 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 valid blocks that extend canonical chain and the ones that fork off diff --git a/crates/engine/tree/src/tree/mod.rs b/crates/engine/tree/src/tree/mod.rs index d790f421c588f..660fbe31e990f 100644 --- a/crates/engine/tree/src/tree/mod.rs +++ b/crates/engine/tree/src/tree/mod.rs @@ -10,9 +10,8 @@ use reth_beacon_consensus::{ }; use reth_blockchain_tree::{ error::{InsertBlockErrorKindTwo, InsertBlockErrorTwo, InsertBlockFatalError}, - BlockAttachment, BlockBuffer, BlockStatus, + BlockBuffer, BlockStatusTwo, InsertPayloadOkTwo, }; -use reth_blockchain_tree_api::InsertPayloadOk; use reth_chain_state::{ CanonicalInMemoryState, ExecutedBlock, MemoryOverlayStateProvider, NewCanonicalChain, }; @@ -665,17 +664,19 @@ where match self.insert_block_without_senders(block) { Ok(status) => { let status = match status { - InsertPayloadOk::Inserted(BlockStatus::Valid(_)) => { + InsertPayloadOkTwo::Inserted(BlockStatusTwo::Valid) => { latest_valid_hash = Some(block_hash); self.try_connect_buffered_blocks(num_hash); PayloadStatusEnum::Valid } - InsertPayloadOk::AlreadySeen(BlockStatus::Valid(_)) => { + InsertPayloadOkTwo::AlreadySeen(BlockStatusTwo::Valid) => { latest_valid_hash = Some(block_hash); PayloadStatusEnum::Valid } - InsertPayloadOk::Inserted(BlockStatus::Disconnected { .. }) | - InsertPayloadOk::AlreadySeen(BlockStatus::Disconnected { .. }) => { + InsertPayloadOkTwo::Inserted(BlockStatusTwo::Disconnected { .. }) | + InsertPayloadOkTwo::AlreadySeen(BlockStatusTwo::Disconnected { + .. + }) => { // not known to be invalid, but we don't know anything else PayloadStatusEnum::Syncing } @@ -1404,12 +1405,7 @@ where Ok(res) => { debug!(target: "engine", child =?child_num_hash, ?res, "connected buffered block"); if self.is_sync_target_head(child_num_hash.hash) && - matches!( - res, - InsertPayloadOk::Inserted(BlockStatus::Valid( - BlockAttachment::Canonical - )) - ) + matches!(res, InsertPayloadOkTwo::Inserted(BlockStatusTwo::Valid)) { self.make_canonical(child_num_hash.hash); } @@ -1640,7 +1636,7 @@ where // try to append the block match self.insert_block(block) { - Ok(InsertPayloadOk::Inserted(BlockStatus::Valid(_))) => { + Ok(InsertPayloadOkTwo::Inserted(BlockStatusTwo::Valid)) => { if self.is_sync_target_head(block_num_hash.hash) { trace!(target: "engine", "appended downloaded sync target block"); // we just inserted the current sync target block, we can try to make it @@ -1652,12 +1648,15 @@ where trace!(target: "engine", "appended downloaded block"); self.try_connect_buffered_blocks(block_num_hash); } - Ok(InsertPayloadOk::Inserted(BlockStatus::Disconnected { head, missing_ancestor })) => { + Ok(InsertPayloadOkTwo::Inserted(BlockStatusTwo::Disconnected { + head, + missing_ancestor, + })) => { // block is not connected to the canonical head, we need to download // its missing branch first return self.on_disconnected_downloaded_block(block_num_hash, missing_ancestor, head) } - Ok(InsertPayloadOk::AlreadySeen(_)) => { + Ok(InsertPayloadOkTwo::AlreadySeen(_)) => { trace!(target: "engine", "downloaded block already executed"); } Err(err) => { @@ -1673,7 +1672,7 @@ where 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(InsertBlockErrorTwo::sender_recovery_error(block)), @@ -1683,7 +1682,7 @@ where fn insert_block( &mut self, block: SealedBlockWithSenders, - ) -> Result { + ) -> Result { self.insert_block_inner(block.clone()) .map_err(|kind| InsertBlockErrorTwo::new(block.block, kind)) } @@ -1691,10 +1690,9 @@ where fn insert_block_inner( &mut self, block: SealedBlockWithSenders, - ) -> Result { + ) -> Result { if self.block_by_hash(block.hash())?.is_some() { - let attachment = BlockAttachment::Canonical; // TODO: remove or revise attachment - return Ok(InsertPayloadOk::AlreadySeen(BlockStatus::Valid(attachment))) + return Ok(InsertPayloadOkTwo::AlreadySeen(BlockStatusTwo::Valid)) } let start = Instant::now(); @@ -1714,7 +1712,7 @@ where self.state.buffer.insert_block(block); - return Ok(InsertPayloadOk::Inserted(BlockStatus::Disconnected { + return Ok(InsertPayloadOkTwo::Inserted(BlockStatusTwo::Disconnected { head: self.state.tree_state.current_canonical_head, missing_ancestor, })) @@ -1791,9 +1789,7 @@ where }; self.emit_event(EngineApiEvent::BeaconConsensus(engine_event)); - let attachment = BlockAttachment::Canonical; // TODO: remove or revise attachment - - Ok(InsertPayloadOk::Inserted(BlockStatus::Valid(attachment))) + Ok(InsertPayloadOkTwo::Inserted(BlockStatusTwo::Valid)) } /// Handles an error that occurred while inserting a block. @@ -2176,7 +2172,7 @@ mod tests { fn insert_block( &mut self, block: SealedBlockWithSenders, - ) -> Result { + ) -> Result { let execution_outcome = self.block_builder.get_execution_outcome(block.clone()); self.extend_execution_outcome([execution_outcome]); self.tree.provider.add_state_root(block.state_root); @@ -2524,7 +2520,7 @@ mod tests { let outcome = test_harness.tree.insert_block_without_senders(sealed.clone()).unwrap(); assert_eq!( outcome, - InsertPayloadOk::Inserted(BlockStatus::Disconnected { + InsertPayloadOkTwo::Inserted(BlockStatusTwo::Disconnected { head: test_harness.tree.state.tree_state.current_canonical_head, missing_ancestor: sealed.parent_num_hash() })