fix(peerstore): bound the identify stream close

[gmelodie]

Summary

peerStore.identify tears its stream down in a finally with await noCancel stream.closeWithEOF() and no bound. closeWithEOF half-closes and then reads until the remote's EOF (libp2p/stream/lpstream.nim:337), so a remote that answers identify and then holds the stream open parks that read. noCancel clears the cancel callback, so the caller's deadline cannot end the wait either.

What ends the wait instead is the muxer idle timer, 5 minutes on mplex and yamux (libp2p/builders.nim:185, libp2p/muxers/yamux/yamux.nim:531). On QUIC nothing does: QuicStream sets timeout: 0.millis (libp2p/transports/quictransport.nim:68), so it runs no idle monitor, and the transport idle timeout is refreshed by every other stream on the connection.

Both callers run identify under a 30 second deadline that therefore does not hold:

• Dialer.finishUpgrade (libp2p/dialer.nim:289) holds the peer's dial lock, so every later dial to that peer queues behind it.
• Switch.upgrader (libp2p/switch.nim:242) holds one of the 32 ConcurrentUpgrades slots.

DefaultDialerTimeout and UpgradeTimeout are both 30 seconds, so the hold outlasts its budget by 10x on a muxed stream, and without limit on QUIC.

The fix is the retireStream pattern from libp2p/protocols/kademlia/message_sender.nim:83: the close gets its own bound, and the stream is reset when the remote does not answer it. noCancel wraps the whole closeWithEOF().withTimeout(...), so the inner close stays cancellable for the timer while the teardown still ignores the caller's cancellation.

Sibling of #2936, on the dial path instead of the Kademlia RPC path.

Affected Areas

• Peer Management / Discovery

Compatibility & Downstream Validation

• Nimbus: nimbus-eth2 unstable with vendor/nim-libp2p set to this branch. nimbus_beacon_node builds, and both make local-testnet-minimal and make local-testnet-mainnet report "The simulation completed successfully" (exit 0). The build also needs nim-libplum v0.6.2 in vendor/, which nimbus-eth2 does not vendor yet; that gap is independent of this PR.
• Waku: N/A
• Codex: N/A

Impact on Library Users

One new public constant, IdentifyCloseTimeout (5 seconds). No API, wire or config change.

A remote that answers identify and never closes the stream now costs 5 seconds instead of up to 5 minutes on mplex or yamux, and instead of no bound at all on QUIC. The dial then completes normally, because identify already stored its info.

Risk Assessment

• A healthy remote closes at once, so the bound is never reached in a normal identify. A remote that is more than 5 seconds late gets a reset, which costs nothing on this stream: both sides already sent everything.
• Security: such a remote holds a dial lock or an upgrade slot for 5 seconds instead of minutes.
• The noCancel stays. It comes from fix(peerstore): reset identify stream on cancellation #2459, which guarantees the teardown runs. Without it the finally dies on the first cancel and leaves the stream neither closed nor reset, which nothing cleans up on QUIC.
• Residual: the other teardown path, await noCancel stream.reset(), is still unbounded, because LPChannel.reset writes through noCancel s.conn.writeMsg (libp2p/muxers/mplex/lpchannel.nim:114). It wants its own PR.

References

• fix(kad): an admission probe no longer waits out the dialer timeout #2936, same defect class on the Kademlia RPC path.

Additional Notes

One regression test, A remote that never closes the identify stream frees the dial in tests/libp2p/test_dialer.nim. The remote answers identify and then holds the stream, and the test dials twice, so the second dial passes only if the first freed the dial lock. It fails at 15.0 s on the old code and passes with the fix (dialer suite 6 OK). Each dial is bounded with join().withTimeout(...), not wait(), because a wait() would cancel into the noCancel teardown and hang.

libp2p/services/identify_pusher.nim:98 holds the same unbounded close, but its caller detaches the send, so it stalls no dial. retireStream now exists twice and wants a shared home in LPStream. Both are follow-ups.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.23%. Comparing base (9357845) to head (ff15e18).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2940      +/-   ##
==========================================
+ Coverage   82.21%   82.23%   +0.01%     
==========================================
  Files         177      177              
  Lines       32372    32375       +3     
  Branches       12       11       -1     
==========================================
+ Hits        26616    26623       +7     
+ Misses       5756     5752       -4     

Files with missing lines	Coverage Δ
libp2p/peerstore.nim	97.61% <100.00%> (-0.28%)	⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[richard-ramos]

I created the following PR to verify if nimbus aint affected: status-im/nimbus-eth2#8885

[richard-ramos]

peerStore.identify closes its stream in a finally with await noCancel stream.closeWithEOF() and no bound. closeWithEOF half-closes and then reads until the remote's EOF (libp2p/stream/lpstream.nim:337), so a remote that answers identify and then holds the stream open parks that read for as long as the connection lives. noCancel clears the cancel callback, so the caller's deadline cannot end the wait either: chronos wait cancels the future and then waits for it to finish, and that cancellation lands on a future that ignores it.

I am not sure of this PR because of the following reasons:

• This behavior has been here for a while (8d5ea43e2b) , with the noCancel being added in fix(peerstore): reset identify stream on cancellation #2459 some months ago.

holds the stream open parks that read for as long as the connection lives.

This is not true, because streams get closed under inactivity (actually this is something i gotta check in quic)

---

Did you try removing the noCancel in await noCancel stream.closeWithEOF()?

[gmelodie]

streams get closed under inactivity

This is true for mplex and yamux, or, in claude's words: "the 5 minute channel idle timer resets the stream and ends that read".

On QUIC the stream has no idle timer at all. QuicStream is built with timeout: 0.millis (quictransport.nim:68), and Connection.initStream only starts the inactivity monitor when the timeout is > 0 (connection.nim:80), so nothing resets that stream when it goes quiet. Here's a claude-built example of what is happening in quic:

• A dials B over QUIC. The dial opens an identify stream and awaits the teardown of that stream under a 30 s deadline.
• B answers identify in full, then keeps its side of that one stream open and sends no EOF.
• A's closeWithEOF half-closes and waits for that EOF. noCancel means A's 30 s deadline cannot end the wait.
• On mplex or yamux a per-stream idle timer would reset the stream after 5 minutes and free the wait. QUIC streams have no such timer: QuicStream is built with timeout: 0.millis (quictransport.nim:68) and the monitor only starts above zero (connection.nim:80).
• QUIC has one idle timeout for the whole connection, not per stream. B keeps gossip, pings, or any other stream running on the same connection, so that timeout never fires.
• Result: A waits on the dead identify stream for as long as B keeps the connection alive, and A's dial lock for B (or one of the 32 upgrade slots) stays taken the whole time.

[richard-ramos]

Yeah this is something to be fixed for Quic. Streams being infinite when Idle is a bug.