Skip to content

fix(p2p): warn when peer status is ahead #432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dicethedev wants to merge 1 commit into
base: main
Choose a base branch
from
+34 −25
Open

fix(p2p): warn when peer status is ahead #432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 26 additions & 25 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions crates/net/p2p/src/req_resp/handlers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,14 @@ async fn handle_status_response(server: &mut P2PServer, status: Status, peer: Pe
return;
}
let gap = status.head.slot - our_head_slot;
warn!(
%peer,
peer_head_slot = status.head.slot,
local_head_slot = our_head_slot,
slot_gap = gap,
"Peer status head is ahead of local head"
);
Comment on lines +174 to +180

@greptile-apps greptile-apps Bot Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Warning fires on every status response, not just once at sync start

The warn! is placed unconditionally after the early-return guard, so it fires on every call to handle_status_response where the peer is ahead — including calls that arrive while a range sync is already in progress (the Some(state) => state.merge_peer(...) branch on line 186). With multiple peers and periodic status exchanges, a node that is 1 000 slots behind could emit dozens of identical warnings before catching up, making the warn level noisy and harder to distinguish from genuine problems. Consider gating the warn! on server.range_sync_state.is_none() so it only fires when the first sync session is initiated, or downgrading to debug! when sync is already in progress.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: crates/net/p2p/src/req_resp/handlers.rs
Line: 174-180

Comment:
**Warning fires on every status response, not just once at sync start**

The `warn!` is placed unconditionally after the early-return guard, so it fires on every call to `handle_status_response` where the peer is ahead — including calls that arrive while a range sync is already in progress (the `Some(state) => state.merge_peer(...)` branch on line 186). With multiple peers and periodic status exchanges, a node that is 1 000 slots behind could emit dozens of identical warnings before catching up, making the `warn` level noisy and harder to distinguish from genuine problems. Consider gating the `warn!` on `server.range_sync_state.is_none()` so it only fires when the first sync session is initiated, or downgrading to `debug!` when sync is already in progress.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


let start_slot = our_head_slot.saturating_add(1);
let end_exclusive = start_slot.saturating_add(gap.min(MAX_SYNC_RANGE));

Expand Down