diff --git a/crates/e2e-test-utils/src/testsuite/actions/fork.rs b/crates/e2e-test-utils/src/testsuite/actions/fork.rs index 6d0529c415..07ecf232fe 100644 --- a/crates/e2e-test-utils/src/testsuite/actions/fork.rs +++ b/crates/e2e-test-utils/src/testsuite/actions/fork.rs @@ -13,11 +13,20 @@ use reth_rpc_api::clients::EthApiClient; use std::marker::PhantomData; use tracing::debug; -/// Action to create a fork from a specified block number and produce blocks on top +/// Fork base target for fork creation +#[derive(Debug, Clone)] +pub enum ForkBase { + /// Block number + Number(u64), + /// Tagged block reference + Tag(String), +} + +/// Action to create a fork from a specified block and produce blocks on top #[derive(Debug)] pub struct CreateFork { - /// Block number to use as the base of the fork - pub fork_base_block: u64, + /// Fork base specification (either block number or tag) + pub fork_base: ForkBase, /// Number of blocks to produce on top of the fork base pub num_blocks: u64, /// Tracks engine type @@ -25,9 +34,18 @@ pub struct CreateFork { } impl CreateFork { - /// Create a new `CreateFork` action + /// Create a new `CreateFork` action from a block number pub fn new(fork_base_block: u64, num_blocks: u64) -> Self { - Self { fork_base_block, num_blocks, _phantom: Default::default() } + Self { + fork_base: ForkBase::Number(fork_base_block), + num_blocks, + _phantom: Default::default(), + } + } + + /// Create a new `CreateFork` action from a tagged block + pub fn new_from_tag(tag: impl Into, num_blocks: u64) -> Self { + Self { fork_base: ForkBase::Tag(tag.into()), num_blocks, _phantom: Default::default() } } } @@ -40,18 +58,34 @@ where { fn execute<'a>(&'a mut self, env: &'a mut Environment) -> BoxFuture<'a, Result<()>> { Box::pin(async move { - // store the fork base for later validation - env.current_fork_base = Some(self.fork_base_block); + // resolve the fork base and execute the appropriate sequence + match &self.fork_base { + ForkBase::Number(block_number) => { + // store the fork base for later validation + env.current_fork_base = Some(*block_number); - let mut sequence = Sequence::new(vec![ - Box::new(SetForkBase::new(self.fork_base_block)), - Box::new(ProduceBlocks::new(self.num_blocks)), - // Note: ValidateFork is not called here because fork blocks are not accessible - // via RPC until they are made canonical. Validation will be done automatically - // as part of MakeCanonical or ReorgTo actions. - ]); + let mut sequence = Sequence::new(vec![ + Box::new(SetForkBase::new(*block_number)), + Box::new(ProduceBlocks::new(self.num_blocks)), + ]); + sequence.execute(env).await + } + ForkBase::Tag(tag) => { + let block_info = + env.block_registry.get(tag).copied().ok_or_else(|| { + eyre::eyre!("Block tag '{}' not found in registry", tag) + })?; - sequence.execute(env).await + // store the fork base for later validation + env.current_fork_base = Some(block_info.number); + + let mut sequence = Sequence::new(vec![ + Box::new(SetForkBaseFromBlockInfo::new(block_info)), + Box::new(ProduceBlocks::new(self.num_blocks)), + ]); + sequence.execute(env).await + } + } }) } } @@ -63,6 +97,13 @@ pub struct SetForkBase { pub fork_base_block: u64, } +/// Sub-action to set the fork base block from existing block info +#[derive(Debug)] +pub struct SetForkBaseFromBlockInfo { + /// Complete block info to use as the base of the fork + pub fork_base_info: BlockInfo, +} + impl SetForkBase { /// Create a new `SetForkBase` action pub const fn new(fork_base_block: u64) -> Self { @@ -70,6 +111,13 @@ impl SetForkBase { } } +impl SetForkBaseFromBlockInfo { + /// Create a new `SetForkBaseFromBlockInfo` action + pub const fn new(fork_base_info: BlockInfo) -> Self { + Self { fork_base_info } + } +} + impl Action for SetForkBase where Engine: EngineTypes, @@ -117,6 +165,37 @@ where } } +impl Action for SetForkBaseFromBlockInfo +where + Engine: EngineTypes, +{ + fn execute<'a>(&'a mut self, env: &'a mut Environment) -> BoxFuture<'a, Result<()>> { + Box::pin(async move { + let block_info = self.fork_base_info; + + debug!( + "Set fork base from block info: block {} (hash: {})", + block_info.number, block_info.hash + ); + + // update environment to point to the fork base block + env.current_block_info = Some(block_info); + env.latest_header_time = block_info.timestamp; + + // update fork choice state to the fork base + env.latest_fork_choice_state = ForkchoiceState { + head_block_hash: block_info.hash, + safe_block_hash: block_info.hash, + finalized_block_hash: block_info.hash, + }; + + debug!("Set fork base to block {} (hash: {})", block_info.number, block_info.hash); + + Ok(()) + }) + } +} + /// Sub-action to validate that a fork was created correctly #[derive(Debug)] pub struct ValidateFork { diff --git a/crates/e2e-test-utils/src/testsuite/actions/mod.rs b/crates/e2e-test-utils/src/testsuite/actions/mod.rs index 8b77a96436..7d9971d4b5 100644 --- a/crates/e2e-test-utils/src/testsuite/actions/mod.rs +++ b/crates/e2e-test-utils/src/testsuite/actions/mod.rs @@ -12,7 +12,7 @@ pub mod fork; pub mod produce_blocks; pub mod reorg; -pub use fork::{CreateFork, SetForkBase, ValidateFork}; +pub use fork::{CreateFork, ForkBase, SetForkBase, SetForkBaseFromBlockInfo, ValidateFork}; pub use produce_blocks::{ AssertMineBlock, BroadcastLatestForkchoice, BroadcastNextNewPayload, CheckPayloadAccepted, GenerateNextPayload, GeneratePayloadAttributes, PickNextBlockProducer, ProduceBlocks, diff --git a/crates/engine/tree/src/tree/e2e_tests.rs b/crates/engine/tree/src/tree/e2e_tests.rs index 6b76e035bc..59298cfb0f 100644 --- a/crates/engine/tree/src/tree/e2e_tests.rs +++ b/crates/engine/tree/src/tree/e2e_tests.rs @@ -74,3 +74,38 @@ async fn test_engine_tree_fcu_reorg_with_all_blocks_e2e() -> Result<()> { Ok(()) } + +/// Test that verifies valid forks with an older canonical head. +/// +/// This test creates two competing fork chains starting from a common ancestor, +/// then switches between them using forkchoice updates, verifying that the engine +/// correctly handles chains where the canonical head is older than fork tips. +#[tokio::test] +async fn test_engine_tree_valid_forks_with_older_canonical_head_e2e() -> Result<()> { + reth_tracing::init_test_tracing(); + + let test = TestBuilder::new() + .with_setup(default_engine_tree_setup()) + // create base chain with 1 block (this will be our old head) + .with_action(ProduceBlocks::::new(1)) + .with_action(CaptureBlock::new("old_head")) + .with_action(MakeCanonical::new()) + // extend base chain with 5 more blocks to establish a fork point + .with_action(ProduceBlocks::::new(5)) + .with_action(CaptureBlock::new("fork_point")) + .with_action(MakeCanonical::new()) + // revert to old head to simulate scenario where canonical head is older + .with_action(ReorgTo::::new_from_tag("old_head")) + // create first competing chain (chain A) from fork point with 10 blocks + .with_action(CreateFork::::new_from_tag("fork_point", 10)) + .with_action(CaptureBlock::new("chain_a_tip")) + // create second competing chain (chain B) from same fork point with 10 blocks + .with_action(CreateFork::::new_from_tag("fork_point", 10)) + .with_action(CaptureBlock::new("chain_b_tip")) + // switch to chain B via forkchoice update - this should become canonical + .with_action(ReorgTo::::new_from_tag("chain_b_tip")); + + test.run::().await?; + + Ok(()) +} diff --git a/crates/engine/tree/src/tree/tests.rs b/crates/engine/tree/src/tree/tests.rs index 4213c9c659..7fa10ef904 100644 --- a/crates/engine/tree/src/tree/tests.rs +++ b/crates/engine/tree/src/tree/tests.rs @@ -1135,66 +1135,6 @@ async fn test_engine_tree_live_sync_fcu_extends_canon_chain() { test_harness.check_canon_head(main_last_hash); } -#[tokio::test] -async fn test_engine_tree_valid_forks_with_older_canonical_head() { - reth_tracing::init_test_tracing(); - - let chain_spec = MAINNET.clone(); - let mut test_harness = TestHarness::new(chain_spec.clone()); - - // create base chain and setup test harness with it - let base_chain: Vec<_> = test_harness.block_builder.get_executed_blocks(0..1).collect(); - test_harness = test_harness.with_blocks(base_chain.clone()); - - let old_head = base_chain.first().unwrap().recovered_block(); - - // extend base chain - let extension_chain = test_harness.block_builder.create_fork(old_head, 5); - let fork_block = extension_chain.last().unwrap().clone_sealed_block(); - - test_harness.setup_range_insertion_for_valid_chain(extension_chain.clone()); - test_harness.insert_chain(extension_chain).await; - - // fcu to old_head - test_harness.fcu_to(old_head.hash(), ForkchoiceStatus::Valid).await; - - // create two competing chains starting from fork_block - let chain_a = test_harness.block_builder.create_fork(&fork_block, 10); - let chain_b = test_harness.block_builder.create_fork(&fork_block, 10); - - // insert chain A blocks using newPayload - test_harness.setup_range_insertion_for_valid_chain(chain_a.clone()); - for block in &chain_a { - test_harness.send_new_payload(block.clone()).await; - } - - test_harness.check_canon_chain_insertion(chain_a.clone()).await; - - // insert chain B blocks using newPayload - test_harness.setup_range_insertion_for_valid_chain(chain_b.clone()); - for block in &chain_b { - test_harness.send_new_payload(block.clone()).await; - } - - test_harness.check_canon_chain_insertion(chain_b.clone()).await; - - // send FCU to make the tip of chain B the new head - let chain_b_tip_hash = chain_b.last().unwrap().hash(); - test_harness.send_fcu(chain_b_tip_hash, ForkchoiceStatus::Valid).await; - - // check for CanonicalChainCommitted event - test_harness.check_canon_commit(chain_b_tip_hash).await; - - // verify FCU was processed - test_harness.check_fcu(chain_b_tip_hash, ForkchoiceStatus::Valid).await; - - // verify the new canonical head - test_harness.check_canon_head(chain_b_tip_hash); - - // verify that chain A is now considered a fork - assert!(test_harness.tree.is_fork(chain_a.last().unwrap().sealed_header()).unwrap()); -} - #[tokio::test] async fn test_engine_tree_buffered_blocks_are_eventually_connected() { let chain_spec = MAINNET.clone();