Skip to content

Commit

Permalink
Fix issue when theres nothing new to learn from a peer
Browse files Browse the repository at this point in the history
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
  • Loading branch information
viquezclaudio committed Feb 18, 2025
1 parent 49ce59b commit 00cbbc5
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 7 deletions.
15 changes: 11 additions & 4 deletions consensus/src/sync/light/sync_requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::{
sync::{EpochIds, PeerMacroRequests},
LightMacroSync,
},
syncer::MacroSync,
syncer::{MacroSync, MacroSyncReturn},
},
};

Expand Down Expand Up @@ -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();
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions consensus/src/sync/light/sync_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,9 @@ 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) {
return Poll::Ready(Some(macro_sync_return));
}
}

Expand Down

0 comments on commit 00cbbc5

Please sign in to comment.