Skip to content

fix: resend active WHOAREYOU challenge - #309

Open
mattsse wants to merge 2 commits into
sigp:masterfrom
mattsse:mattsse/fix-handshake-resend
Open

fix: resend active WHOAREYOU challenge#309
mattsse wants to merge 2 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
challenge.header.kind,
PacketKind::WhoAreYou { .. }
));
assert_eq!(challenge.header.message_nonce, first_nonce);

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.

since we're here we might as well confirm the id_nonce in the WHOAREYOU header matches

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.

3 participants