Skip to content

fix(autonat-v2): apply timeout to verification requests - #3658

Merged
tabcat merged 3 commits into
libp2p:mainfrom
kediyalua:main
Oct 11, 2026
Merged

tabcat merged 3 commits into
libp2p:mainfrom
kediyalua:main

Conversation

@kediyalua

Copy link
Copy Markdown
Contributor

Title

fix(autonat-v2): apply timeout to verification requests

Description

AutoNATv2Client.verifyExternalAddresses() creates a combined abort signal for
every queued verification job:

const signal = anySignal([options.signal, AbortSignal.timeout(this.timeout)])

...but then passes the original options object to askPeerToVerify() instead
of that signal, so the only thing the combined signal is used for is
signal.clear() in the finally block. As a result the timeout option
(TIMEOUT = 30s by default) never applied to the client side of the AutoNAT v2
exchange: connection.newStream(), messages.write() and messages.read()
could all block forever if the remote accepted the stream but never answered.

This looks like an oversight rather than a design decision - the surrounding
anySignal(...) / signal.clear() scaffolding only makes sense if the signal is
actually used, and the rest of the module does use its timeout signals
(handleDialBackStream() applies AbortSignal.timeout(this.timeout) to its
reads/writes, findRandomPeers() passes its combined signal to
randomWalk.walk() and openConnection()).

The fix forwards the combined signal to askPeerToVerify():

await this.askPeerToVerify(connection, segment, nonce, {
  ...options,
  signal
})

setMaxListeners(Infinity, signal) is added for consistency with
kad-dht/src/query/manager.ts - the signal is now listened to by every stream
operation performed by the job.

A regression test is included: it configures the service with the existing 100ms
timeout and uses a remote that accepts the dial-request stream but never sends
a DialResponse. Before the fix the stream stays open forever and the test
fails; after the fix the stream is aborted shortly after the timeout elapses.

Fixes https://github.com/libp2p/js-libp2p/issues/ISSUE_NUMBER

Notes & open questions

  • While writing the test I noticed the existing
    should time out when verifying an observed address case asserts
    addressManager.addObservedAddr was not called, but the AutoNAT v2 client only
    ever calls confirmObservedAddr / removeObservedAddr, so that assertion is
    trivially true and the case does not actually exercise any timeout behaviour.
    I left it untouched to keep this PR focused - happy to fix or remove it in a
    follow-up if you prefer.
  • Separately, findRandomPeers() builds an anySignal that is never
    clear()ed, which leaves its 10s AbortSignal.timeout() timer alive. Not
    addressed here; say the word and I'll include it.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

Signed-off-by: kediyalua <kediyalua@outlook.com>
@kediyalua
kediyalua requested a review from a team as a code owner October 10, 2026 10:37
Wait for the queue to go idle instead of a fixed delay, assert the stream is aborted and the nonce cleaned up, use the short timeout only in this test and remove the vacuous timeout test.
An unresponsive remote is an expected network condition.
@tabcat
tabcat merged commit 974e63f into libp2p:main Oct 11, 2026
60 of 62 checks passed
@tabcat tabcat mentioned this pull request Oct 11, 2026
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