Repository navigation
test: end-to-end hybrid transport tests, Python hybrid peer, misbehaving peers and fallback (#184 part 7) - #239
Conversation
|
@codex review |
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfd5a22aeb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
dfd5a22 to
d258d08
Compare
|
@codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d258d08268
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
4e84b79 to
4cb600a
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 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". |
4cb600a to
dfa3f7c
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? 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". |
Give the functional test framework an independent implementation of the hybrid post-quantum v2 handshake (doc/design/pq-transport.md), built on crypto/mlkem.py and the key recipe in crypto/bip324_cipher.py, and use it for end-to-end tests against the node. - v2_p2p.py: EncryptedP2PState(supports_v2_pq=True) negotiates in both roles. A responder offers an ML-KEM-1024 key from two independent 32-byte draws; an initiator holds its version packet until the responder's arrives, accepts a valid offer and switches; each side's first hybrid packet is a key confirmation, and the peer's must verify. parse_version_contents() mirrors the node's record grammar. Subclasses can override the offer and accept contents, the shared secret and the confirmation, and a peer can withhold its confirmation. - Framework peers stay legacy by default (supports_v2_pq=False). The unconditional assertion that the node's version contents are empty is gone: a legacy peer accepts a non-empty offer and answers with empty contents. Received and sent contents are recorded, so tests of switched-off nodes assert empty contents explicitly. - p2p.py: authenticate_handshake() also returns the bytes it produced (the held version packet, a confirmation), which the driver sends. The handshake completes only after the peer's confirmation verified and ours was sent; send_pq_confirmation() releases a withheld one. - test_node.py: add_p2p_connection() and add_outbound_p2p_connection() take supports_v2_pq, and the outbound helper gains wait_for_v2_handshake. - v2_p2p.py unit tests (feature_framework_unit_tests.py) replay all four vectors of src/test/data/pq_transport_vectors.json through two states, run every role pairing across a rekey, and check withheld and damaged confirmations. p2p_v2_pq.py covers two nodes in both directions (hybrid, the same session id, more than 224 packets each way), the Python peer in both roles (the node draws fresh keys and encapsulation randomness per connection), legacy and v1 peers under each switch setting and each pair of node settings, and the pending status while a peer withholds its confirmation. Work item G1 of #212 (#184 part 7). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Load shedding counts inbound offer attempts per second of MockableSteadyClock, which only unit tests could set. A functional test therefore had to fit more than 1,000 offers into one real second and then wait out the real quiet period. Slow CI machines (ASan, MSan, TSan, i686) could do neither reliably. Add two regtest-only test options, listed in -help-debug next to pq_fail_first_packet: - -test=pq_low_shed_threshold sets the connection manager's shedding threshold to TEST_PQ_SHED_THRESHOLD_PER_S (5 attempts per second), so six connections reach it. - -test=mock_steady_clock makes setmocktime set MockableSteadyClock to the same number of seconds as well; setmocktime 0 clears both. Without the option setmocktime leaves the steady clock alone, so the headers presync rate limit in other tests is unaffected. rpc_pqtransport.py checks that both options appear in -help-debug, are rejected off regtest and start without printing to stderr, and that the first one reports threshold_per_s 5. The next commit's p2p_v2_pq_misbehaving.py drives load shedding with both. For #212 (#184 part 7). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drive every way a hybrid negotiation can go wrong through a real node, using scripted variants of the Python hybrid peer, and check the fallback that follows repeated failures. p2p_v2_pq_misbehaving.py: - test_records: offers (the node initiates) and accepts (the node responds) of every shape: EK and CT one byte short and long, a bad-modulus EK, two own records (the first wins either way), unknown and reserved headers, junk after a record, zero, non-canonical, past-the-end, above-MAX_SIZE and truncated lengths, contents padded to exactly 4,000,013 bytes and one byte more, a damaged version tag, and version packets cut short before EOF. Each case checks that the connection is kept (with its transport_pq_status) or dropped at once, the exact log lines, and the exact per-direction counter deltas. - test_confirmation: a nonzero length, a bad tag and a non-decoy confirmation in both roles, a corrupted CT and a responder that derives another secret: each disconnects at once, long before -peertimeout. - test_decoys_and_partial_writes: decoys before the version packet (the first one authenticating the garbage), decoys after the confirmation, coalesced bursts and a handshake fragmented into 1- to 7-byte writes, in both roles. - test_unsolicited_accept: a switched-off responder ignores own-record bytes it never asked for. - test_load_shedding: with -test=pq_low_shed_threshold (5 attempts per second) and -test=mock_steady_clock, setmocktime places every attempt in a chosen second, so nothing depends on the machine's speed. Five attempts in each of two seconds are all offered; the sixth in one second starts shedding, is the first one shed, and since is that second. While shed, a hybrid initiator sees a legacy responder, an accept is ignored and outbound connections still negotiate. With the clock held, shedding outlasts the quiet period in real time (the one real-time check, which waiting longer only makes stricter). An attempt after nine quiet seconds is still shed; after ten, with no connection at all, the socket handler alone ends shedding. Every step checks exact counter deltas, and the log holds exactly one started and one stopped line. p2p_v2_pq_fallback.py, with addnode onetry for explicit outbound attempts (addconnection can't open manual connections): - test_three_failures: -test=pq_fail_first_packet on one node; three failed attempts enter the fallback set (fallback = 1, one warning line, the set's entry and ring), and the fourth connection is plain v2 with status fallback. - test_expiry_and_restart: past the window the negotiation is tried again with the next window at 4 h; a restart clears everything under a new instance_id. - test_malformed_offer_fallback: against a responder that always offers a bad-modulus key, the fourth connection completes, because a fallback connection neither parses nor validates offers. - test_inbound_and_success_exclusion: inbound failures, and outbound failures after a hybrid success with the endpoint, never lead to a fallback. - test_endpoint_separation: through a SOCKS5 proxy, hostnames in any letter case with or without the root dot, one hostname on two ports and one proxied address on two ports keep separate histories. This drives ConnectNode's name-proxy endpoint key through the real path. Mutating the node, each of these is killed: dropping the socket handler's shedding update, starting shedding at the threshold rather than above it, counting attempts across seconds, ending shedding a second early or late, never declining an offer, setmocktime leaving the steady clock alone, ignoring -test=pq_low_shed_threshold, keying name-proxy endpoints by the proxy, reporting v2 before the confirmation, parsing only up to the first own record, dropping the early confirmation length check, skipping the fallback lookup, the success protection or the window escalation, fixed KEM randomness, counting inbound failures, and parsing contents with the switch off. Work item G2 of #212 (#184 part 7). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Run every existing functional test with hybrid post-quantum v2 transport between all nodes as well, so the feature can't quietly break unrelated behavior. - Framework flag --v2pqtransport, which implies --v2transport: every node that knows the option (the build under test, or a release whose metadata says so) starts with -v2pqtransport=1. Plain --v2transport passes -v2pqtransport=0 explicitly, so the i686 job keeps covering plain v2 after the default changes (#222); --v1transport clears both. Only nodes that use v2 get the setting, because -v2pqtransport=1 with -v2transport=0 warns, and a test's own arguments come later and win. - Framework Python peers stay legacy BIP324 peers in this mode, so the suite pays no pure-Python KEM cost; links between nodes go hybrid. - Tests of the option itself set v2pqtransport_from_run_mode = False, so their nodes start with only their own arguments: rpc_pqtransport.py checks -v2pqtransport's defaults, qbit.conf and negation. - rpc_net.py: the exact getpeerinfo dictionary takes transport_pq and transport_pq_status from the run mode (v1, off, or legacy_peer for a framework peer when the node offers), and connections_pq counts both links between the two nodes when the run mode switches it on. - p2p_v2_misbehaving.py: random 5-byte version contents are redrawn only when they parse as an own record, and a deterministic 04 f0 00 00 00 case disconnects at once when the node offers (from getpqtransportinfo) and is ignored otherwise. The case dispatch no longer treats every unmatched type as SEND_NON_EMPTY_VERSION_PACKET. - CI: a pq-functional job in ci-pq.yml (a non-matrix job sharing the unit job's source guard, checkout and verification) runs the whole suite with --v2pqtransport from ci/test/00_setup_env_native_pq_functional.sh. It runs nightly through main's caller like every ci-pq job, and on a branch by named dispatch (jobs=pq-functional). It joins aarch64-unit, s390x-unit, macos-arm64-unit and macos-x86_64-unit in every job list, the outputs and NON_MATRIX_JOBS. Its timeout is twice a measured local run, rounded up: a cold build at -j 4 on 4 cores (475 s) plus the whole suite at -j 4 (727 s) is 1,202 s, so 45 minutes. The i686 env notes that it covers plain v2. ci/checks/test_ci_pq_contract.py covers the job, its selection, its source wiring and its environment. Work item G3 of #212 (#184 part 7). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
test_bad_confirmation released the initiator's held key confirmation, hybrid packet 0, and encrypted a damaged one after it as packet 1. The responder still expected packet 0, so all three cases failed on a length decrypted with the wrong cipher position, and the tag and ignore-bit checks were never reached: the test passed with either check removed. Damage the held confirmation through make_confirmation instead, so it is packet 0 as a real one is. Record which check failed, and require that each case fails its own: length, tag or not_decoy, the reasons the node logs. Dropping the tag or the ignore-bit check in receive_confirmation now fails the test. For #212 (#184 part 7). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ework On Windows the build links or copies test/functional into the build tree instead of symlinking it, so the v2 peer's unit test looked for src/test/data/pq_transport_vectors.json under the build directory and failed the Windows native job. Take the source tree from the build's config.ini when the file isn't beside the module's source. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
431aba5 to
1eb88a1
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! 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". |
Summary
Part 7 of #184 (#212): end-to-end tests of the hybrid v2 transport. Stacked on #236 (part 6), which provides
-v2pqtransportand the RPCs these tests use.test_framework/v2_p2p.pygains the ML-KEM-1024 hybrid handshake: offer, accept, key schedule and key confirmation. It uses part 2's independent Python ML-KEM-1024 and follows part 4's spec (doc/design/pq-transport.md).p2p_v2_pq.pycovers hybrid connections both ways between the node and the Python peer, plus legacy peers.-testoptions, listed in-help-debug:-test=pq_low_shed_thresholdsets the shedding threshold to 5 attempts per second, so six connections reach it.-test=mock_steady_clockmakessetmocktimeset the steady clock too (setmocktime 0clears both). Without the option,setmocktimeis unchanged, so the headers-presync rate limit in other tests isn't affected.p2p_v2_pq_misbehaving.py: malformed records, bad keys and ciphertexts, and a failed confirmation, each closing with the right outcome.p2p_v2_pq_fallback.py: the 3-strike fallback with its windows.test_runner.py --v2pqtransportruns every functional test with-v2pqtransport=1. Apq-functionaljob inci-pq.ymlruns it nightly and on a named dispatch, not on every PR. Itstimeout-minutes: 45is twice the measured cold 4-core build (475 s) plus the suite (727 s). G3 keeps part 3'smacos-x86_64-unitjob alongsidepq-functional.Testing
p2p_v2_pq.py,p2p_v2_pq_misbehaving.py,p2p_v2_pq_fallback.pyandrpc_pqtransport.pypass, with and without--v2pqtransport.taskset -c 0).--v2pqtransportat-j 4: 298 passed, 20 skipped (USDT, previous releases, IPC, assumeutxo prerequisites), 0 failed.net_pq_tests,net_tests,bip324_tests,mlkem_tests,rpc_tests,argsman_tests,getarg_tests.test/lint/check-doc.py, and the 10ci/checkssuites.pq-functionalhas not run on GitHub yet; check its first run's duration against the 45-minute limit.Target Branch
feature/pq-transport(Hybrid post-quantum v2 transport (#184) — integration #233), stacked on rpc, init: -v2pqtransport, getpqtransportinfo and hybrid peer fields (#184 part 6) #236.Risk / Review Notes
Notes:
pq-functionaljob, run nightly and on a named dispatch only.Part of #212 · Part of #184
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.