Skip to content

fix: single in-flight identify per peer with event-driven fallback - #8955

Merged
wemeetagain merged 1 commit into
ChainSafe:cayman/libp2p3-againfrom
lodekeeper:fix/libp2p3-identify-minimal
Feb 24, 2026
Merged

fix: single in-flight identify per peer with event-driven fallback#8955
wemeetagain merged 1 commit into
ChainSafe:cayman/libp2p3-againfrom
lodekeeper:fix/libp2p3-identify-minimal

Conversation

@lodekeeper

Copy link
Copy Markdown
Contributor

Motivation

PR #8890 (libp2p v3) suffers from ~22% Unknown peer rate (vs ~4% on v2/unstable). Root cause: libp2p v3 enforces per-protocol stream limits (identify: maxOutboundStreams=1). When repeated STATUS messages trigger overlapping identify() calls for the same peer, v3 throws TooManyOutboundProtocolStreamsError which cascades into massive EOF failures (~5000 identify errors/2h on feat1 vs ~287 on unstable).

Description

Minimal fix — no retries, no backoff, no spray:

  • Single in-flight identify per peer: identifyInProgress map keyed by PeerIdStr → connection.id. Before calling identify(), checks if there's already one in-flight for the same connection. If so, skips.
  • Event-driven fallback: Listens to peer:identify events from libp2p (fired on successful identify or identify-push). Updates agentVersion/agentClient even if our explicit identify() failed earlier.
  • Reconnect race safety: Uses connection.id as epoch token. After await identify(), verifies the in-flight key still matches before writing results — a reconnect during the await clears the old entry, so stale results are discarded.
  • Cleanup on disconnect: Removes in-flight tracking when peer disconnects.

Changes

  • packages/beacon-node/src/network/peers/peerManager.ts: Added identifyInProgress map, onPeerIdentify event handler, dedup guard in onStatus, stale-result guard in identifyPeer(), cleanup in disconnect handler
  • packages/beacon-node/test/e2e/network/peers/peerManager.test.ts: 4 new tests (dedup, reconnect race, event-driven fallback, existing flow preserved)

Evidence

Loki log comparison (2h window):

  • feat1 (v3): ~5000 identify errors — 3794 EOF, 863 EOF-while-reading, 283 missing public key, 26 too-many-outbound-streams
  • unstable (v2): ~287 identify errors — 143 unexpected-end, 82 timeouts
# The overlapping call pattern (before fix):
STATUS #1 → identify() in-flight
STATUS #2 → identify() overlaps → TooManyOutboundProtocolStreamsError → EOF cascade

# After fix:
STATUS #1 → identify() in-flight, tracked in identifyInProgress
STATUS #2 → sees in-flight marker, skips
peer:identify event → updates agentVersion as safety net

Note: This is a replacement for the retry-based approach in #8954. That PR added retry machinery which masked the root cause rather than preventing overlapping calls.

AI Disclosure

This PR was authored with AI assistance (Claude Opus 4.6 via OpenClaw). All code was reviewed and tested by the AI agent.

Replace retry-based identify approach with minimal fix:
- Track one in-flight identify per peer using connection.id as epoch key
- Add peer:identify event listener as safety net for identify-push
- Dedup overlapping identify calls from repeated STATUS messages
- Guard against stale results on reconnect (connection id mismatch)
- Clean up in-flight tracking on disconnect

Root cause: libp2p v3 enforces per-protocol stream limits (identify
maxOutbound=1). Overlapping identify attempts cause
TooManyOutboundProtocolStreamsError + cascading EOF failures,
inflating Unknown peer percentage to ~22% (vs ~4% on v2).

This fix ensures exactly one identify attempt per peer/connection,
eliminating the overlapping-call pattern without retry spray.
@lodekeeper
lodekeeper requested a review from a team as a code owner February 24, 2026 15:41
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @lodekeeper, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical issue in libp2p v3 where repeated identify() calls for the same peer lead to TooManyOutboundProtocolStreamsError and subsequent EOF failures, significantly increasing the "Unknown peer" rate. The solution introduces a robust mechanism to manage peer identification, ensuring that only one identify operation is active per peer connection at any given time. It also incorporates an event-driven fallback to capture successful identification results and includes safeguards against race conditions during reconnects, thereby improving network stability and peer discovery accuracy.

Highlights

  • Prevented duplicate identify calls: Implemented a mechanism to ensure only one identify() call is in-flight per peer-connection pair, using an identifyInProgress map.
  • Introduced event-driven fallback: Enabled updating peer agent information from peer:identify events, even if an explicit identify() call failed.
  • Ensured reconnect safety: Utilized connection.id to prevent stale identify results from overwriting newer ones after a peer reconnects.
  • Cleaned up on disconnect: Added logic to remove in-flight identify tracking when a peer disconnects.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • packages/beacon-node/src/network/peers/peerManager.ts
    • Imported IdentifyResult type from @libp2p/interface.
    • Added identifyInProgress map to track active identify operations.
    • Registered and unregistered an event listener for peer:identify events.
    • Modified onStatus method to check for existing in-flight identify calls and initiate new ones with tracking.
    • Ensured identifyInProgress entries are cleared upon peer disconnection.
    • Implemented onPeerIdentify to process successful identify results from libp2p events.
    • Updated identifyPeer to incorporate connection-specific identifyKey for race condition prevention.
  • packages/beacon-node/test/e2e/network/peers/peerManager.test.ts
    • Added a test case to verify the deduplication of concurrent identify requests.
    • Included a test to confirm that stale identify results are correctly ignored after a peer reconnects.
    • Introduced a test to ensure agentVersion is updated via peer:identify events even when explicit identify() calls fail.
Activity
  • The pull request aims to fix a high "Unknown peer" rate and massive EOF failures observed in libp2p v3 (PR feat: libp2p v3 #8890) compared to v2/unstable.
  • It replaces a previous retry-based approach (fix(network): reduce Unknown peers via identify retries + peer:identify event #8954) that was deemed to mask the root cause rather than prevent overlapping calls.
  • Evidence for the fix's effectiveness is provided through Loki log comparisons, showing a significant reduction in identify errors.
  • The author disclosed the use of AI assistance (Claude Opus 4.6 via OpenClaw) in authoring the PR, with all code reviewed and tested by the AI agent.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request effectively addresses the issue of excessive identify errors by preventing overlapping calls for the same peer. The use of an identifyInProgress map with the connection ID as a key is a solid approach. The logic to handle race conditions during reconnects is well-thought-out, particularly the checks against the identifyKey before and after the async identify call. The event-driven fallback using the peer:identify event is also a great addition for robustness.

I've found one potential race condition in the onPeerIdentify event handler that could undermine the fix under specific circumstances. My review includes a comment with a detailed explanation and a suggested fix.

The new tests are comprehensive and cover the new logic well, including deduplication, reconnect races, and the event-driven fallback.

Overall, this is a great fix. Addressing the suggested change will make it even more robust.

@wemeetagain
wemeetagain merged commit 10ea50e into ChainSafe:cayman/libp2p3-again Feb 24, 2026
14 of 15 checks passed
nflaig added a commit that referenced this pull request Feb 25, 2026
wemeetagain pushed a commit that referenced this pull request Feb 25, 2026
Squash merged #8958 and
reverted #8955.

Deployed to `feat1`
lodekeeper pushed a commit to lodekeeper/lodestar that referenced this pull request Mar 13, 2026
Squash merged ChainSafe#8958 and
reverted ChainSafe#8955.

Deployed to `feat1`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants