test(l1): 1024 reorg test

.github/workflows/pr-main_l1.yaml

@@ -191,7 +191,7 @@ jobs:
             ethrex_flags: ""
           - name: "Engine withdrawal tests"
             simulation: ethereum/engine
-            test_pattern: "engine-withdrawals/Corrupted Block Hash Payload|Empty Withdrawals|engine-withdrawals test loader|GetPayloadBodies|GetPayloadV2 Block Value|Max Initcode Size|Sync after 2 blocks - Withdrawals on Genesis|Withdraw many accounts|Withdraw to a single account|Withdraw to two accounts|Withdraw zero amount|Withdraw many accounts|Withdrawals Fork on Block 1 - 1 Block Re-Org|Withdrawals Fork on Block 1 - 8 Block Re-Org NewPayload|Withdrawals Fork on Block 2|Withdrawals Fork on Block 3|Withdrawals Fork on Block 8 - 10 Block Re-Org NewPayload|Withdrawals Fork on Canonical Block 8 / Side Block 7 - 10 Block Re-Org [^S]|Withdrawals Fork on Canonical Block 8 / Side Block 9 - 10 Block Re-Org [^S]"
+            test_pattern: "engine-withdrawals/Corrupted Block Hash Payload|Empty Withdrawals|engine-withdrawals test loader|GetPayloadBodies|GetPayloadV2 Block Value|Max Initcode Size|Sync after 2 blocks - Withdrawals on Genesis|Withdraw many accounts|Withdraw to a single account|Withdraw to two accounts|Withdraw zero amount|Withdraw many accounts|Withdrawals Fork on Block 1 - 1 Block Re-Org|Withdrawals Fork on Block 1 - 8 Block Re-Org |Withdrawals Fork on Block 2|Withdrawals Fork on Block 3|Withdrawals Fork on Block 8 - 10 Block Re-Org |Withdrawals Fork on Canonical Block 8 / Side Block 7 - 10 Block Re-Org |Withdrawals Fork on Canonical Block 8 / Side Block 9 - 10 Block Re-Org"
           - name: "Sync full"
             simulation: ethereum/sync
             test_pattern: ""

crates/networking/p2p/sync.rs

@@ -336,96 +336,160 @@ impl Syncer {
             current_head, sync_head
         );
 
-        loop {
-            debug!("Sync Log 1: In Full Sync");
-            debug!(
-                "Sync Log 3: State current headers len {}",
-                block_sync_state.current_headers.len()
-            );
-            debug!(
-                "Sync Log 4: State current blocks len {}",
-                block_sync_state.current_blocks.len()
-            );
+        info!("Sync Log 1: In Full Sync");
+        debug!(
+            "Sync Log 3: State current headers len {}",
+            block_sync_state.current_headers.len()
+        );
+        debug!(
+            "Sync Log 4: State current blocks len {}",
+            block_sync_state.current_blocks.len()
+        );
 
-            debug!("Requesting Block Headers from {current_head}");
+        debug!("Requesting Block Headers from NewToOld from sync_head {sync_head}");
 
-            let Some(mut block_headers) = self
-                .peers
-                .request_block_headers_from_hash(current_head, BlockRequestOrder::OldToNew)
-                .await
-            else {
-                warn!("Sync failed to find target block header, aborting");
-                debug!("Sync Log 8: Sync failed to find target block header, aborting");
-                return Ok(());
+        let requested_header =
+            if let Some(sync_head_block) = store.get_pending_block(sync_head).await? {
+                sync_head_block.header.parent_hash
+            } else {
+                sync_head
             };
 
-            debug!("Sync Log 9: Received {} block headers", block_headers.len());
+        let Some(mut block_headers) = self
+            .peers
+            .request_block_headers_from_hash(requested_header, BlockRequestOrder::NewToOld)
+            .await
+        else {
+            // sync_head or sync_head parent was not found
+            warn!("Sync failed to find target block header, aborting");
+            debug!("Sync Log 8: Sync failed to find target block header, aborting");
+            return Ok(());
+        };
 
-            let (first_block_hash, first_block_number, first_block_parent_hash) =
-                match block_headers.first() {
-                    Some(header) => (header.hash(), header.number, header.parent_hash),
-                    None => continue,
-                };
-            let (last_block_hash, last_block_number) = match block_headers.last() {
-                Some(header) => (header.hash(), header.number),
-                None => continue,
-            };
-            // TODO(#2126): This is just a temporary solution to avoid a bug where the sync would get stuck
-            // on a loop when the target head is not found, i.e. on a reorg with a side-chain.
-            if first_block_hash == last_block_hash
-                && first_block_hash == current_head
-                && current_head != sync_head
+        debug!("Sync Log 9: Received {} block headers", block_headers.len());
+
+        let mut found_common_ancestor = false;
+        for i in 0..block_headers.len() {
+            if store
+                .get_block_by_hash(block_headers[i].hash())
+                .await?
+                .is_some()
             {
-                // There is no path to the sync head this goes back until it find a common ancerstor
-                warn!("Sync failed to find target block header, going back to the previous parent");
-                current_head = first_block_parent_hash;
-                continue;
+                block_headers.drain(i..);
+                found_common_ancestor = true;
+                break;
             }
+        }
 
-            debug!(
-                "Received {} block headers| First Number: {} Last Number: {}",
-                block_headers.len(),
-                first_block_number,
-                last_block_number
-            );
+        if found_common_ancestor {
+            block_headers.reverse();
+            block_sync_state
+                .process_incoming_headers(
+                    block_headers,
+                    sync_head,
+                    true, // sync_head_found is true because of the NewToOld headers request
+                    self.blockchain.clone(),
+                    self.peers.clone(),
+                    self.cancel_token.clone(),
+                )
+                .await?;
+            Ok(())
+        } else {
+            // if found_common_ancestor is false that means we are more than 1024 blocks behind so, for now, we go back to syncing as it follows.
+            // TODO: Have full syncing always be from NewToOld
+            loop {
+                info!("Sync Log 1: In Full Sync");
+                debug!(
+                    "Sync Log 3: State current headers len {}",
+                    block_sync_state.current_headers.len()
+                );
+                debug!(
+                    "Sync Log 4: State current blocks len {}",
+                    block_sync_state.current_blocks.len()
+                );
 
-            // Filter out everything after the sync_head
-            let mut sync_head_found = false;
-            if let Some(index) = block_headers
-                .iter()
-                .position(|header| header.hash() == sync_head)
-            {
-                sync_head_found = true;
-                block_headers.drain(index + 1..);
-            }
+                debug!("Requesting Block Headers from OldToNew from current_head {current_head}");
 
-            // Update current fetch head
-            current_head = last_block_hash;
+                let Some(mut block_headers) = self
+                    .peers
+                    .request_block_headers_from_hash(current_head, BlockRequestOrder::OldToNew)
+                    .await
+                else {
+                    warn!("Sync failed to find target block header, aborting");
+                    debug!("Sync Log 8: Sync failed to find target block header, aborting");
+                    return Ok(());
+                };
 
-            // Discard the first header as we already have it
-            block_headers.remove(0);
-            if !block_headers.is_empty() {
-                let mut finished = false;
-                while !finished {
-                    (finished, sync_head_found) = block_sync_state
-                        .process_incoming_headers(
-                            block_headers.clone(),
-                            sync_head,
-                            sync_head_found,
-                            self.blockchain.clone(),
-                            self.peers.clone(),
-                            self.cancel_token.clone(),
-                        )
-                        .await?;
-                    block_headers.clear();
+                debug!("Sync Log 9: Received {} block headers", block_headers.len());
+
+                let (first_block_hash, first_block_number, first_block_parent_hash) =
+                    match block_headers.first() {
+                        Some(header) => (header.hash(), header.number, header.parent_hash),
+                        None => continue,
+                    };
+                let (last_block_hash, last_block_number) = match block_headers.last() {
+                    Some(header) => (header.hash(), header.number),
+                    None => continue,
+                };
+                // TODO(#2126): This is just a temporary solution to avoid a bug where the sync would get stuck
+                // on a loop when the target head is not found, i.e. on a reorg with a side-chain.
+                if first_block_hash == last_block_hash
+                    && first_block_hash == current_head
+                    && current_head != sync_head
+                {
+                    // There is no path to the sync head this goes back until it find a common ancerstor
+                    warn!(
+                        "Sync failed to find target block header, going back to the previous parent"
+                    );
+                    current_head = first_block_parent_hash;
+                    continue;
                 }
-            }
 
-            if sync_head_found {
-                break;
-            };
+                debug!(
+                    "Received {} block headers| First Number: {} Last Number: {}",
+                    block_headers.len(),
+                    first_block_number,
+                    last_block_number
+                );
+
+                // Filter out everything after the sync_head
+                let mut sync_head_found = false;
+                if let Some(index) = block_headers
+                    .iter()
+                    .position(|header| header.hash() == sync_head)
+                {
+                    sync_head_found = true;
+                    block_headers.drain(index + 1..);
+                }
+
+                // Update current fetch head
+                current_head = last_block_hash;
+
+                // Discard the first header as we already have it
+                block_headers.remove(0);
+                if !block_headers.is_empty() {
+                    let mut finished = false;
+                    while !finished {
+                        (finished, sync_head_found) = block_sync_state
+                            .process_incoming_headers(
+                                block_headers.clone(),
+                                sync_head,
+                                sync_head_found,
+                                self.blockchain.clone(),
+                                self.peers.clone(),
+                                self.cancel_token.clone(),
+                            )
+                            .await?;
+                        block_headers.clear();
+                    }
+                }
+
+                if sync_head_found {
+                    break;
+                };
+            }
+            Ok(())
         }
-        Ok(())
     }
 
     /// Executes the given blocks and stores them
@@ -677,7 +741,7 @@ impl FullBlockSyncState {
         if let Err((err, batch_failure)) = Syncer::add_blocks(
             blockchain.clone(),
             block_batch,
-            sync_head_found,
+            sync_head_found || finished,
             cancel_token.clone(),
         )
         .await