From eaf5bc605b1375f15eb6ee4e72d4ab82425cb582 Mon Sep 17 00:00:00 2001
From: viquezclaudio <viquezclaudio@gmail.com>
Date: Tue, 18 Feb 2025 09:33:09 -0600
Subject: [PATCH] Fix issue when theres nothing new to learn from a peer

When processing epoch ids it could happen that by the time we requested the epoch ids
and the time we received those we no longer have anything new to learn from the peer,
so we need to emit the peer as good.
This caused the peer to hang within the macro-sync process, increasing the time to consensus
---
 consensus/src/sync/light/sync_requests.rs | 15 +++++++++++----
 consensus/src/sync/light/sync_stream.rs   | 23 ++++++++++++++++++++---
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/consensus/src/sync/light/sync_requests.rs b/consensus/src/sync/light/sync_requests.rs
index bf90bd2cc1..7d7d9733f4 100644
--- a/consensus/src/sync/light/sync_requests.rs
+++ b/consensus/src/sync/light/sync_requests.rs
@@ -24,7 +24,7 @@ use crate::{
             sync::{EpochIds, PeerMacroRequests},
             LightMacroSync,
         },
-        syncer::MacroSync,
+        syncer::{MacroSync, MacroSyncReturn},
     },
 };
 
@@ -180,7 +180,7 @@ impl<TNetwork: Network> LightMacroSync<TNetwork> {
     pub(crate) fn request_macro_headers(
         &mut self,
         mut epoch_ids: EpochIds<TNetwork::PeerId>,
-    ) -> Option<TNetwork::PeerId> {
+    ) -> Option<MacroSyncReturn<TNetwork::PeerId>> {
         // Read our current blockchain state.
         let (our_epoch_id, our_epoch_number, our_block_number) = {
             let blockchain = self.blockchain.read();
@@ -204,7 +204,7 @@ impl<TNetwork: Network> LightMacroSync<TNetwork> {
                     peer = %epoch_ids.sender,
                     "Peer is behind"
                 );
-                return Some(epoch_ids.sender);
+                return Some(MacroSyncReturn::Outdated(epoch_ids.sender));
             } else {
                 // Check that the epoch_id sent by the peer at our current epoch number corresponds to
                 // our accepted state. If it doesn't, the peer is on a "permanent" fork, so we ban it.
@@ -219,7 +219,7 @@ impl<TNetwork: Network> LightMacroSync<TNetwork> {
                         peer = %epoch_ids.sender,
                         "Peer is on a different chain"
                     );
-                    return Some(epoch_ids.sender);
+                    return Some(MacroSyncReturn::Outdated(epoch_ids.sender));
                 }
 
                 epoch_ids.ids = epoch_ids
@@ -236,6 +236,13 @@ impl<TNetwork: Network> LightMacroSync<TNetwork> {
             }
         }
 
+        // After discarding all epoch_ids & checkpoint prior to our current state
+        // It could happen that we don't have anything new to learn from this peer
+        if epoch_ids.ids.is_empty() && epoch_ids.checkpoint.is_none() {
+            log::debug!("Nothing new to learn from this peer, emit it as good");
+            return Some(MacroSyncReturn::Good(epoch_ids.sender));
+        }
+
         let mut peer_requests = PeerMacroRequests::new();
 
         log::trace!(%epoch_ids.sender,
diff --git a/consensus/src/sync/light/sync_stream.rs b/consensus/src/sync/light/sync_stream.rs
index 3d1b5fab44..94c522b54a 100644
--- a/consensus/src/sync/light/sync_stream.rs
+++ b/consensus/src/sync/light/sync_stream.rs
@@ -261,9 +261,26 @@ impl<TNetwork: Network> LightMacroSync<TNetwork> {
                 }
             }
 
-            // If the macro header process deems a peer useless, it is returned here and we emit it.
-            if let Some(agent) = self.request_macro_headers(epoch_ids) {
-                return Poll::Ready(Some(MacroSyncReturn::Outdated(agent)));
+            // Request the macro headers from the peer or emit it accordingly
+            if let Some(macro_sync_return) = self.request_macro_headers(epoch_ids) {
+                match macro_sync_return {
+                    MacroSyncReturn::Good(peer_id) => {
+                        // We are synced with this peer.
+                        debug!(?peer_id, "Finished macro syncing with peer");
+
+                        if self.blockchain.read().can_enforce_validity_window() {
+                            // We can enforce the validity window, so we are done.
+                            return Poll::Ready(Some(MacroSyncReturn::Good(peer_id)));
+                        } else {
+                            #[cfg(feature = "full")]
+                            self.start_validity_synchronization(peer_id);
+                        }
+                    }
+                    _ => {
+                        // Propagate any other sync result (outdated is the only one that can be emitted by the macro headers process)
+                        return Poll::Ready(Some(macro_sync_return));
+                    }
+                }
             }
         }