Skip to content

fix: resend active WHOAREYOU challenge - #309

Merged
jxs merged 3 commits into
sigp:masterfrom
mattsse:mattsse/fix-handshake-resend
Sep 21, 2026
Merged

jxs merged 3 commits into
sigp:masterfrom
mattsse:mattsse/fix-handshake-resend

Conversation

@mattsse

@mattsse mattsse commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Resends the currently active WHOAREYOU packet when another ordinary packet arrives from the same node and endpoint during an unfinished handshake.

This preserves the original request nonce and challenge data instead of dropping the retry or creating another outstanding challenge. It also adds a regression test for the geth/Hive HandshakeResend flow.

@ackintosh ackintosh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thank you @mattsse 🙏

@ackintosh

Copy link
Copy Markdown
Member

The clippy errors will be fixed by #310

@ackintosh

Copy link
Copy Markdown
Member

Note that the current spec says to respond with a new WHOAREYOU challenge.

go-ethereum has re-sent the existing challenge since ethereum/go-ethereum#31356, and I have filed ethereum/devp2p#278 to update the spec accordingly.

@jxs jxs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, some minor comment

Comment thread src/handler/tests.rs
@ackintosh

Copy link
Copy Markdown
Member

@mattsse Could you take a quick look at @jxs’s minor comment? 🙏 Also, the corresponding spec PR has been merged now, so we should be aligned there too.

@jxs jxs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

@jxs
jxs merged commit 2177af4 into sigp:master Sep 21, 2026
7 checks passed
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.

4 participants