fix: resend cached WHOAREYOU on any retry#235
Closed
bomanaps wants to merge 1 commit into
Closed
Conversation
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Author
|
I have read the CLA Document and I hereby sign the CLA |
Author
|
Closing as duplicate of #234 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description
The discv5 conformance test TestHandshakeResend fails on every consumer of this library because the responder issues a fresh WHOAREYOU on each retry instead of resending the original this PR makes the responder resend the cached challenge byte-for-byte when a session is mid-handshake, and relaxes the initiator's nonce check to accept any recently-sent outbound (bounded), with NodeSession tracking the set and NodeSessionManager cleaning up the index on teardown.
Verified locally: 7/7 green on the hive devp2p/discv5 suite against a Besu image built on this branch the 8th subtest (PingMultiIP) is excluded because it fails identically on every client locally due to a Docker cross-bridge SNAT artifact rather than any client bug.
Fixed Issue(s)
Note
Medium Risk
Changes handshake/nonce validation behavior and session nonce indexing, which can affect discovery connectivity and state cleanup if incorrect. Scope is contained to discv5 session management with added bounds and tests mitigating risk.
Overview
Improves discv5 handshake retry conformance by resending the earliest cached
WHOAREYOU(byte-for-byte) when an unauthorized packet arrives while a session is inWHOAREYOU_SENT, even if the retry uses a different per-packet nonce.Updates initiator-side validation to accept incoming
WHOAREYOUif its nonce matches any of the session’s bounded set of recent outbound nonces (instead of only the latest), addsNodeSessiontracking for those nonces, and updatesNodeSessionManagerteardown/index maintenance to clean up all tracked nonces.Extends unit tests to cover the new resend fallback behavior and the new
resendFirstPendingWhoAreYouhelper.Reviewed by Cursor Bugbot for commit 0d57674. Bugbot is set up for automated code reviews on this repo. Configure here.