fix(p2p): warn when peer status is ahead

[dicethedev]

🗒️ Description / Motivation

• Logs a warning when a peer’s status response reports a head slot higher than the local head.
• Makes it easier for operators to identify when a node is behind its peers.
• Completes the status-response observability requested in Add warning message when a peer returns a higher slot in their status message #211.

What Changed

• Updated crates/net/p2p/src/req_resp/handlers.rs.
• Added a structured warning containing:
  • Peer ID
  • Peer head slot
  • Local head slot
  • Slot gap
• The warning is emitted before range synchronization begins.

Correctness / Behavior Guarantees

• Status responses at or behind the local head do not produce a warning.
• Existing BlocksByRange synchronization behavior is unchanged.
• The change only adds observability and does not alter peer selection, request ranges, or block processing.

Tests Added / Run

• No new tests were added because this is a logging-only change.
• Ran cargo fmt --all -- --check — clean.
• P2P lint and unit tests were started, but interrupted while compiling RocksDB.
• git diff --check — clean.

Related Issues / PRs

• Closes Add warning message when a peer returns a higher slot in their status message #211
• Related to Improve sync resilience: periodic pending retry, better backoff, peer status tracking #193
• Builds on feat(p2p): use BlocksByRange for long-range sync #351

✅ Verification Checklist

• Ran make fmt — clean
• Ran make lint (clippy with -D warnings) — clean
• Ran cargo test --workspace --release — all passing

[greptile-apps]

Greptile Summary

This PR adds structured warn! observability to handle_status_response so operators can see when a peer's reported head slot exceeds the local head, and how large the gap is. The lock file also contains several dependency version changes that are unrelated to the logging feature.

• handlers.rs: A warn! block with peer, peer_head_slot, local_head_slot, and slot_gap fields is inserted after the early-return guard; the arithmetic is safe and the existing sync logic is unchanged.
• Cargo.lock: Multiple windows-sys version downgrades, a spin upgrade, and a Plonky3 git-commit bump are included without explanation — these should be separated or justified.

Confidence Score: 4/5

The handler change is safe — it adds a log statement with no effect on sync logic — but the unexplained Cargo.lock dependency changes deserve a second look before merging.

The core handlers.rs change is a read-only observability addition that cannot affect correctness. The Cargo.lock modifications (windows-sys downgrades, spin upgrade, new Plonky3 commit) are unrelated to the stated scope of the PR and could have downstream build or runtime effects on non-Linux platforms or on code that depends on Plonky3 internals.

Cargo.lock — the unrelated dependency changes need to be verified as intentional; handlers.rs is otherwise straightforward.

Important Files Changed

Filename	Overview
crates/net/p2p/src/req_resp/handlers.rs	Adds a warn! log in handle_status_response when a peer's head slot exceeds our local head; arithmetic is safe and fields are accurate, but the warning fires on every status response while behind, which may be noisier than intended.
Cargo.lock	Contains unexplained changes unrelated to the stated feature: windows-sys version downgrades, a spin upgrade, and a new Plonky3 git commit hash — these should be reviewed separately or explained.

Sequence Diagram

sequenceDiagram
    participant Peer
    participant P2PServer
    participant Store
    participant RangeSyncState

    Peer->>P2PServer: Status response (head_slot)
    P2PServer->>Store: head_slot()
    Store-->>P2PServer: our_head_slot

    alt "peer head <= our head"
        P2PServer-->>Peer: return early, no action
    else "peer head > our head"
        P2PServer->>P2PServer: "gap = peer_head - our_head"
        Note over P2PServer: warn!(peer, peer_head_slot, local_head_slot, slot_gap) NEW
        P2PServer->>RangeSyncState: merge_peer() or new RangeSyncState
        P2PServer->>Peer: BlocksByRange request
        P2PServer->>P2PServer: info! Long-range sync started
    end

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
crates/net/p2p/src/req_resp/handlers.rs:174-180
**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.

### Issue 2 of 2
Cargo.lock:174-188
**Unrelated dependency changes included in a logging-only PR**

The lock file contains changes that go beyond what a logging-only addition would require: `windows-sys` is downgraded from `0.61.2` to `0.60.2` in four crates and to `0.52.0` in three others; `spin` is upgraded from `0.10.0` to `0.11.0`; and all Plonky3 packages are pinned to a different upstream git commit (`3f67d136` → `82cfad73`). If these were pulled in incidentally (e.g., a rebase or local `cargo update`), they should be separated into a dedicated dependency-update PR or documented here so reviewers can verify the changes are intentional and safe.

_{Reviews (1): Last reviewed commit: "fix(p2p): warn when peer status is ahead" | Re-trigger Greptile}

[greptile-apps]

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!

[greptile-apps]

Unrelated dependency changes included in a logging-only PR

The lock file contains changes that go beyond what a logging-only addition would require: windows-sys is downgraded from 0.61.2 to 0.60.2 in four crates and to 0.52.0 in three others; spin is upgraded from 0.10.0 to 0.11.0; and all Plonky3 packages are pinned to a different upstream git commit (3f67d136 → 82cfad73). If these were pulled in incidentally (e.g., a rebase or local cargo update), they should be separated into a dedicated dependency-update PR or documented here so reviewers can verify the changes are intentional and safe.

Prompt To Fix With AI

This is a comment left during a code review.
Path: Cargo.lock
Line: 174-188

Comment:
**Unrelated dependency changes included in a logging-only PR**

The lock file contains changes that go beyond what a logging-only addition would require: `windows-sys` is downgraded from `0.61.2` to `0.60.2` in four crates and to `0.52.0` in three others; `spin` is upgraded from `0.10.0` to `0.11.0`; and all Plonky3 packages are pinned to a different upstream git commit (`3f67d136` → `82cfad73`). If these were pulled in incidentally (e.g., a rebase or local `cargo update`), they should be separated into a dedicated dependency-update PR or documented here so reviewers can verify the changes are intentional and safe.

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!

[MegaRedHand]

Please revert lockfile changes

[MegaRedHand]

Please revert lockfile changes

[pablodeymo]

Cargo.lock