Skip to content

Commit

Permalink
chore(tree): remove BlockAttachment usage
Browse files Browse the repository at this point in the history
  • Loading branch information
@Rjected
Rjected committed Aug 22, 2024
1 parent 7d8196e commit 8a407c2
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 26 deletions.
30 changes: 30 additions & 0 deletions crates/blockchain-tree-api/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 BlockStatus2 {
/// 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, [`InsertPayloadOk2::AlreadySeen(_)`] is
/// returned, otherwise [`InsertPayloadOk2::Inserted(_)`] is returned.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum InsertPayloadOk2 {
/// The payload was valid, but we have already seen it.
AlreadySeen(BlockStatus2),
/// The payload was valid and inserted into the tree.
Inserted(BlockStatus2),
}

/// 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
Expand Down
48 changes: 22 additions & 26 deletions crates/engine/tree/src/tree/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand All @@ -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) => {
Expand All @@ -1673,7 +1672,7 @@ where
fn insert_block_without_senders(
&mut self,
block: SealedBlock,
) -> Result<InsertPayloadOk, InsertBlockErrorTwo> {
) -> Result<InsertPayloadOkTwo, InsertBlockErrorTwo> {
match block.try_seal_with_senders() {
Ok(block) => self.insert_block(block),
Err(block) => Err(InsertBlockErrorTwo::sender_recovery_error(block)),
Expand All @@ -1683,18 +1682,17 @@ where
fn insert_block(
&mut self,
block: SealedBlockWithSenders,
) -> Result<InsertPayloadOk, InsertBlockErrorTwo> {
) -> Result<InsertPayloadOkTwo, InsertBlockErrorTwo> {
self.insert_block_inner(block.clone())
.map_err(|kind| InsertBlockErrorTwo::new(block.block, kind))
}

fn insert_block_inner(
&mut self,
block: SealedBlockWithSenders,
) -> Result<InsertPayloadOk, InsertBlockErrorKindTwo> {
) -> Result<InsertPayloadOkTwo, InsertBlockErrorKindTwo> {
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();
Expand All @@ -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,
}))
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -2176,7 +2172,7 @@ mod tests {
fn insert_block(
&mut self,
block: SealedBlockWithSenders,
) -> Result<InsertPayloadOk, InsertBlockErrorTwo> {
) -> Result<InsertPayloadOkTwo, InsertBlockErrorTwo> {
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);
Expand Down Expand Up @@ -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()
})
Expand Down

0 comments on commit 8a407c2

Please sign in to comment.