Repository navigation
test: end a failed v2 handshake without resetting the connection - #254
Merged
Merged
Conversation
When the Python v2 peer fails the node's key confirmation, it writes out what it has sent and closes the connection, so that the node sees the peer's own confirmation fail (#246). But a close with the node's later bytes still unread on the peer's socket resets the connection, and on Windows the reset discards what the node has not read yet: the node then records closed_after_switch (reset) instead of first_packet_failed, and p2p_v2_pq_misbehaving.py fails. The node sends its version packet right after its confirmation, so this depends on timing. End the peer's stream with write_eof() instead and let the node close the connection, discarding what the node still sends until then. Sends after that are dropped, since a write after write_eof() raises. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
@codex review |
4 tasks done
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Reviewed at 6111007: 0 findings shared privately with maintainers. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
6 of 15 tasks
djh58
added a commit
that referenced
this pull request
Oct 27, 2026
* ci: check mainnet publication posture for 1.1.0 The "mainnet publication posture" job checks that the source being released keeps the mainnet launch posture: the AuxPoW chain ID, the genesis and ASERT launch data, the mainnet bootstrap, and the default-mainnet build. The "Classify release target" step only set mainnet_publish_gate_required for target branch 1.0.0, so pull requests, merge-queue runs and pushes into 1.1.0, the next mainnet release line, skipped the job and the Core Checks Gate never asked for its result. Set the output for 1.1.0 as well, and keep 1.0.0. The job's `if:` and the Core Checks Gate both read this output, so for source and release-policy changes into 1.1.0 the job now runs and a failure blocks the gate. Pin both branches in the existing workflow contract test, and update the release process, which names the branches the job runs for. Refs #222 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * build: bump version to v1.1.0-rc1 Set CLIENT_VERSION_MINOR to 1, CLIENT_VERSION_BUILD to 0 and CLIENT_VERSION_RC to 1 so the source identifies as v1.1.0-rc1, the first release candidate of v1.1.0, the release after v1.0.1 that adds the hybrid post-quantum v2 transport (#184). CLIENT_VERSION_IS_RELEASE stays true: 1.1.0 is a release line, so candidate binaries carry no commit suffix, as for v1.0.0 and the v1.0.1 candidates. CMake appends "-rc1" to CLIENT_VERSION_STRING, so every binary reports v1.1.0-rc1, the user agent becomes /qbit:1.1.0-rc1/, and the Windows installer metadata and the RPC docs manifest's project version derive from the same string. The Guix archive name comes from the v1.1.0-rc1 tag itself. Regenerate the seven manual pages with contrib/devtools/gen-manpages.py (help2man 1.49.3) from a build of this tree with the GUI, the wallet and every tool. Besides the version and date lines, qbitd.1 and qbit-qt.1 gain -v2pqtransport, which #184 added as a connection option. The other options #184 added (-mlkemportable and the regtest-only -test values) are debug options that only -help-debug lists, so the pages don't show them. util_tests/test_FormatSubVersion expected the "Test" client name to format as CLIENT_VERSION_STRING, which fails for every version with a release-candidate suffix: FormatSubVersion uses that string only for the qbit user agent and formats any other name numerically. Apply the same fix as the v1.0.1-rc1 preparation (#181), byte for byte, so that merging 1.x.x into 1.1.0 brings an identical change: expect the numeric form built from the version components. The qbit user-agent checks on the next lines still cover CLIENT_VERSION_STRING. Refs #222 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * doc: draft v1.1.0 release notes Add doc/release-notes-1.1.0.md in the structure of the v1.0.1 notes (#181). It covers every change merged to 1.1.0 since it branched from 1.x.x at 45e9e72: the hybrid post-quantum v2 transport, with the parts and fixes merged through #233 (#223 to #254), and #249. Behavior changes come first: the hybrid transport is on by default, how to switch it off, the numeric -v2pqtransport value, the single I2P session-creation limit and qbit-photon's OpenSSL configuration loading. A compatibility table states how v1.1.0 nodes connect with v1.0.0 and v1.0.1 nodes and over v1, in both directions, as p2p_v2_previous_release.py and rpc_pqtransport.py test it. Known limitations summarize the release fuzz campaign and the limits its report on #222 states, the basis of the load-shedding threshold, the portable ML-KEM code on AArch64 Linux, and the unsigned-platform waiver (#70). The only performance figures are the handshake benchmark results published in #240. Fold in and delete the 1.1.0 fragments doc/release-notes-223.md, -240.md and -249.md, and point the specification's and the operator guide's release-note links at the new notes' hybrid transport section. The v1.0.0 and v1.0.1 fragments are left alone. The notes say that v1.1.0 contains every change in v1.0.1. That holds once 1.x.x is merged into 1.1.0 after v1.0.1 ships; the short "Changes in v1.0.1" section then gains a link to the v1.0.1 notes. Refs #222 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * net: say how the load-shedding threshold was set The comment on DEFAULT_PQ_SHED_THRESHOLD_PER_S said that the release gate's flood measurement (#222, R3) would set its final value. R3 was settled with CPU benchmarks of the handshake instead, and the threshold stays at 1,000 offers per second, as the v1.1.0 release notes say. Say so, so the comment no longer points at a change that is not coming. Refs #222 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * doc: cite the new fuzz seeds in the v1.1.0 notes #255 merged into 1.1.0 after the notes were drafted. It adds generated seeds to the required fuzz corpus for the hybrid transport's targets, so the test section now cites it alongside the earlier seed changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes a timing-dependent failure from #246 that failed #233's Full Validation on
36868cc. The job wasWindows native, VS 2022, and the test wasp2p_v2_pq_misbehaving.py, in the case "the same responder failing the node's confirmation in turn" (run 40363459177).Cause.
socket recv error ... forcibly closed by the remote host (10054)andclosed_after_switch cause=resetinstead of thefirst_packet_failedthe test expects.Fix. The peer ends its stream with
write_eof()and lets the node close the connection, discarding what the node still sends until then. Sends after that are dropped, because a write afterwrite_eof()raises. Only this case reaches the code: it is the only one where the Python peer fails the node's confirmation.Tests run.
The socket behaviour, standalone. An asyncio peer answers a plain socket's first bytes and ends the connection while the second bytes are unread on its side. With
close()the other side read the answer, then a reset, in 3 of 3 runs. Withwrite_eof()it read the answer, then the end of the stream, in 3 of 3 runs.The functional tests:
p2p_v2_pq_misbehaving.pyfive times;p2p_v2_misbehaving,p2p_v2_encrypted,p2p_v2_transport,p2p_v2_pqandp2p_v2_pq_fallback;rpc_pqtransport,feature_framework_unit_testsandfeature_assumevalid;p2p_invalid_messages,p2p_pingandp2p_disconnect_ban(v1 and v2).All passed on Linux.
Not run: Windows, where the failure happened. Hybrid post-quantum v2 transport (#184) — integration #233's CI runs it once this merges. Since ci: stop validating feature/pq-transport by name #245, pull requests into
feature/pq-transportdon't run CI.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.