feat(test): rewrite test_engine_tree_reorg_with_missing_ancestor_expecting_valid using e2e framework (#16761)

crates/e2e-test-utils/src/testsuite/actions/produce_blocks.rs

@@ -1,7 +1,7 @@
 //! Block production actions for the e2e testing framework.
 
 use crate::testsuite::{
-    actions::{validate_fcu_response, Action, Sequence},
+    actions::{expect_fcu_not_syncing_or_accepted, validate_fcu_response, Action, Sequence},
     BlockInfo, Environment,
 };
 use alloy_primitives::{Bytes, B256};
@@ -242,7 +242,9 @@ where
             debug!("FCU result: {:?}", fcu_result);
 
             // validate the FCU status before proceeding
-            validate_fcu_response(&fcu_result, "GenerateNextPayload")?;
+            // Note: In the context of GenerateNextPayload, Syncing usually means the engine
+            // doesn't have the requested head block, which should be an error
+            expect_fcu_not_syncing_or_accepted(&fcu_result, "GenerateNextPayload")?;
 
             let payload_id = if let Some(payload_id) = fcu_result.payload_id {
                 debug!("Received new payload ID: {:?}", payload_id);
@@ -269,7 +271,10 @@ where
                 debug!("Fresh FCU result: {:?}", fresh_fcu_result);
 
                 // validate the fresh FCU status
-                validate_fcu_response(&fresh_fcu_result, "GenerateNextPayload (fresh)")?;
+                expect_fcu_not_syncing_or_accepted(
+                    &fresh_fcu_result,
+                    "GenerateNextPayload (fresh)",
+                )?;
 
                 if let Some(payload_id) = fresh_fcu_result.payload_id {
                     payload_id

crates/engine/tree/src/tree/e2e_tests.rs

@@ -151,3 +151,37 @@ async fn test_engine_tree_valid_and_invalid_forks_with_older_canonical_head_e2e(
 
     Ok(())
 }
+
+/// Test that verifies engine tree behavior when handling invalid blocks.
+/// This test demonstrates that invalid blocks are correctly rejected and that
+/// attempts to build on top of them fail appropriately.
+#[tokio::test]
+async fn test_engine_tree_reorg_with_missing_ancestor_expecting_valid_e2e() -> Result<()> {
+    reth_tracing::init_test_tracing();
+
+    let test = TestBuilder::new()
+        .with_setup(default_engine_tree_setup())
+        // build main chain (blocks 1-6)
+        .with_action(ProduceBlocks::<EthEngineTypes>::new(6))
+        .with_action(MakeCanonical::new())
+        .with_action(CaptureBlock::new("main_chain_tip"))
+        // create a valid fork first
+        .with_action(CreateFork::<EthEngineTypes>::new_from_tag("main_chain_tip", 5))
+        .with_action(CaptureBlock::new("valid_fork_tip"))
+        // FCU to the valid fork should work
+        .with_action(ExpectFcuStatus::valid("valid_fork_tip"));
+
+    test.run::<EthereumNode>().await?;
+
+    // attempting to build invalid chains fails properly
+    let invalid_test = TestBuilder::new()
+        .with_setup(default_engine_tree_setup())
+        .with_action(ProduceBlocks::<EthEngineTypes>::new(3))
+        .with_action(MakeCanonical::new())
+        // This should fail when trying to build subsequent blocks on the invalid block
+        .with_action(ProduceInvalidBlocks::<EthEngineTypes>::with_invalid_at(2, 0));
+
+    assert!(invalid_test.run::<EthereumNode>().await.is_err());
+
+    Ok(())
+}

crates/engine/tree/src/tree/tests.rs

@@ -381,14 +381,6 @@ impl TestHarness {
         self.setup_range_insertion_for_chain(chain, None)
     }
 
-    fn setup_range_insertion_for_invalid_chain(
-        &mut self,
-        chain: Vec<RecoveredBlock<reth_ethereum_primitives::Block>>,
-        index: usize,
-    ) {
-        self.setup_range_insertion_for_chain(chain, Some(index))
-    }
-
     fn setup_range_insertion_for_chain(
         &mut self,
         chain: Vec<RecoveredBlock<reth_ethereum_primitives::Block>>,
@@ -1155,32 +1147,3 @@ async fn test_engine_tree_buffered_blocks_are_eventually_connected() {
     test_harness.check_canon_block_added(non_buffered_block_hash).await;
     test_harness.check_canon_block_added(buffered_block_hash).await;
 }
-
-#[tokio::test]
-async fn test_engine_tree_reorg_with_missing_ancestor_expecting_valid() {
-    reth_tracing::init_test_tracing();
-    let chain_spec = MAINNET.clone();
-    let mut test_harness = TestHarness::new(chain_spec.clone());
-
-    let base_chain: Vec<_> = test_harness.block_builder.get_executed_blocks(0..6).collect();
-    test_harness = test_harness.with_blocks(base_chain.clone());
-
-    // create a side chain with an invalid block
-    let side_chain =
-        test_harness.block_builder.create_fork(base_chain.last().unwrap().recovered_block(), 15);
-    let invalid_index = 9;
-
-    test_harness.setup_range_insertion_for_invalid_chain(side_chain.clone(), invalid_index);
-
-    for (index, block) in side_chain.iter().enumerate() {
-        test_harness.send_new_payload(block.clone()).await;
-
-        if index < side_chain.len() - invalid_index - 1 {
-            test_harness.send_fcu(block.hash(), ForkchoiceStatus::Valid).await;
-        }
-    }
-
-    // Try to do a forkchoice update to a block after the invalid one
-    let fork_tip_hash = side_chain.last().unwrap().hash();
-    test_harness.send_fcu(fork_tip_hash, ForkchoiceStatus::Invalid).await;
-}