Both maint changes carry over unchanged. The hop-limit helpers follow the
module rename from src/protocol/link.rs to src/proto/link.rs, applied by
rename detection and verified at the renamed path rather than assumed; the old
path is not recreated. The rekey config validation and its jitter-constant
import apply as written, the node module exposing the constant on this branch
as well.
The only conflict was the unreleased changelog, where master's own Changed
entries and the incoming hop-limit entry were both correct for their branch.
Resolved as the union, with the incoming entry appended to master's Changed
list.
Config validation said nothing about the rekey block, so two settings that
disable rekey in appearance and hammer it in practice were accepted silently.
after_messages of zero makes the message-count arm true on every poll, since
the trigger tests the counter against it with a greater-or-equal. It reads like
a way to switch the arm off and does the opposite.
after_secs at or below the per-session jitter is the same trap on the timer
arm. Each session offsets the interval by a random value drawn from plus or
minus the jitter bound, so a smaller interval saturates to zero on a negative
draw and rekeys on sight, for roughly half of sessions and not the other half.
The rule is expressed against the jitter constant rather than its current
value, so it tracks if the bound ever moves.
Both are checked whether or not rekey is enabled, so turning it on later cannot
surface a configuration error at a surprising moment. Neither has an upper
bound: a very large value is the established way to disable one arm of the
trigger and stays legal.
The forwarding path was corrected to full IP semantics: local delivery is not
hop-limit gated, and forwarding decrements first and drops at zero. Two helpers
on SessionDatagram were left implementing the old rule. decrement_ttl still
checked before decrementing, and can_forward still answered true at a hop limit
of one, where the decrement leaves zero and the datagram is dropped. Neither is
called anywhere today, which is precisely why they are worth correcting rather
than leaving as an invitation to reintroduce the defect.
The TtlExhausted reject doc described behaviour that no longer exists. The
reject is charged for a transit arrival at one as well as at zero, and is never
charged for a datagram addressed to this node, whose delivery is decided ahead
of the test.
Both helpers are pinned by tests at the boundary, which fail against the
previous implementations.
Carries the address-to-link map's corrected doc comment. The only
conflict was the adjacent line naming the discovery config keys, which
this line renamed to node.lookup.* and the maintenance line did not; kept
this branch's names.
The doc said the map "enables dispatching incoming packets to the right
connection before authentication completes." It does not: find_link_by_addr
has no callers outside its own tests on any branch, and encrypted frames
dispatch by session index. Its live readers are the msg1 admission fast
path and the duplicate-inbound-handshake check.
The comment mattered because the map has a trap that invites exactly the
misuse it was advertising. An outbound dial registers the literal
configured address string, which may be a hostname, while an inbound
packet carries the resolved form; TransportAddr compares byte-wise, so
the two never match and a lookup keyed on an inbound address returns "no
such link" for every hostname-configured peer rather than failing. The
entry is also single-valued per key, so an inbound handshake overwrites
an outbound dial's entry for the same address.
Both current readers survive this because each compares a key written in
the same form it reads. That is a property of those two call sites and
not of the map, so the comment now says so, and says not to key a
peer-identity question on it.
Documentation only; no behaviour changes.
The send-counter arm of the rekey trigger had no test that could fail for
the reason it existed. The integration suites never reached it: they
inject after_messages = 65536 against a ping-driven workload, so every
rekey they observe is time-triggered. The unit test that looked like the
guard reproduced the trigger's OR predicate as a local closure and
asserted against its own copy, so deleting the arm from the real
predicate left it green.
That is the coverage hole that let the data-plane drop across a
message-count-triggered rekey survive: the defect is precisely a rekey of
a young session, which only the counter arm can produce.
Replaces the existing at-threshold check with a boundary test driving
poll_rekey, holding the time arm off so the counter is provably what
decides. The silence-below half is the part that is new, and it earns its
place by measurement: widening the bound to `p.counter >= 1` reds exactly
one test in the library suite, this one. Deletion, narrowing to `>` and
inversion are caught here too, though the time arm's negative tests
already catch a bound removed altogether.
Removes the closure-based test rather than repairing it. Its one
assertion over real code, that a fresh session entry's jitter lies within
the symmetric bound, is already covered over 100 samples elsewhere.
Note this leaves the deployed maintenance line untested on the same arm:
its trigger is inline in an async method that reads the clock directly,
with no pure function to drive, so the same test cannot be written there
without the sans-IO seam this line already has.
The three rekey integration suites (rekey, rekey-accept-off,
rekey-outbound-only) are dropped from both runners. Their coverage now
lives in fast, deterministic in-process tests:
- rekey timing and choreography (trigger, K-bit cutover, drain, jitter,
guards) in the sans-IO poll_rekey tests (proto/fsp, proto/fmp);
- data-plane continuity across a real cutover in
rekey_cutover_preserves_data_plane (a real IK rekey over loopback);
- the accept-off dual-init regression and the udp.outbound_only rekey
loop in the should_admit_msg1 and dual-init characterization tests.
Drop them from both runners in lockstep so the parity guard stays green,
and record the retirement in the deliberately-not-run block with a pointer
to the standalone runner. The rekey-test.sh script stays on disk.
Drives a real IK rekey handshake to K-bit cutover between two loopback
nodes and asserts an encrypted datagram sent after the cutover decodes on
the new session, with no spurious peer teardown. The rekey is forced
deterministically: rekey.after_messages = 1 crosses the initiator's
trigger on the first sent datagram, and both sessions are backdated past
the responder's 30s rekey-acceptance gate, so no wall clock is read.
The cutover is proven to actually occur (the live session index changes),
which is what makes the continuity assertion meaningful rather than
vacuous. Adds a make_test_node_with_config loopback helper for setting the
rekey thresholds. The rekey timing and choreography decisions themselves
are already covered exhaustively by the sans-IO poll_rekey tests.
The inbound max_peers early-gate in handle_msg1 (silent-drop a Msg1 from
a net-new identity at saturation, send no Msg2, admit no peer) is
unit-tested over a real UDP socket by
handle_msg1_silent_drops_at_cap_for_new_peer in src/node/tests/unit.rs:
it saturates a node, sends a Msg1 from a fresh identity, and polls the
sender socket to assert no Msg2 comes back — the same wire-observable
discriminator the Docker suite's tcpdump used — with a sibling test for
the existing-peer bypass. That is a deterministic superset of the Docker
packet-capture assertion.
Drop it from both runners in lockstep (its *_SUITES array, run function,
default-flow loop, --only dispatch arm, and the GitHub matrix leg with
its steps) so the parity guard stays green, and record the retirement in
the deliberately-not-run block with a pointer to the standalone runner.
The admission-cap-test.sh script stays on disk and runs by hand.
The ACL admission decision is exhaustively unit-tested per npub over real
loaded allow/deny files (src/node/acl.rs test module: allow-match-wins,
allowlist-miss falls through, deny-only, deny-all, allow_all override,
deny-after-allowlist-miss), and the inbound and outbound handshake-
admission paths are covered in-process over the loopback transport
(src/node/tests/acl.rs: inbound msg1 denial, outbound denial, reload).
The Docker suite's only unique coverage was admission over a real UDP
transport, which every other real-transport suite already exercises, and
its operator-facing log assertion.
Drop it from both runners in lockstep (its *_SUITES array, run function,
default-flow call, --only dispatch arm, and the GitHub matrix leg with
its steps) so the parity guard stays green, and record the retirement in
the deliberately-not-run block with a pointer to the standalone runner.
The testing/acl-allowlist/ suite stays on disk and runs by hand.
smoke-10 was a no-stressor 10-node tree-convergence sanity check (netem
off, no ping). Its subject, spanning-tree convergence and root election,
is now covered in-process, faster and deterministically, by the loopback
spanning-tree harness (src/node/tests/spanning_tree.rs: ring, star,
chain, 100-node and disconnected-component convergence) plus end-to-end
datagram delivery (src/node/tests/forwarding.rs). Real-UDP convergence
smoke still runs via static-mesh and the other scenarios' baseline
assertions, so no Docker coverage is lost.
Drop it from both runners in lockstep (the CHAOS_SUITES list and the
GitHub chaos matrix) so the parity guard stays green, delete the scenario
YAML, and update the chaos README.
Bring up the retirement of the six cost-based parent-selection chaos
scenarios (cost-reeval, cost-avoidance, cost-stability, depth-vs-cost,
mixed-technology, bottleneck-parent), which tested a decision the Docker
harness could not exercise reliably.
master already carries the equivalent sans-IO coverage in
src/proto/stp/tests (effective-depth cost selection, hysteresis, cost
degradation) and its transport-drop tests, so this keeps master's src
unchanged and takes only the scenario removals and the CI-list, README and
scenario-comment updates from maint.
The cost-selection chaos scenarios (cost-reeval, cost-avoidance,
cost-stability, depth-vs-cost, mixed-technology, bottleneck-parent) tested
TreeState::evaluate_parent's decision logic through a Docker mesh that could
not exercise it reliably: the tree roots at whichever node holds the smallest
NodeAddr, MMP link costs take several measurement windows to settle, and the
parent hold-down plus hysteresis timing all confound the outcome. A
deterministic link-cost flap still produced zero periodic parent switches in a
full run.
Replace those six scenarios with deterministic unit tests in src/tree/tests.rs
that drive evaluate_parent directly: cheaper-link selection at equal depth,
switch-on-cost-change, hysteresis suppressing a marginal change while allowing
a significant one, and the depth-versus-cost effective-depth tradeoff. Each is
constructed so that breaking the cost or hysteresis logic makes it fail.
The congestion kernel-drop signal (SO_RXQ_OVFL) cannot be provoked
deterministically in Docker: a fresh daemon reader keeps up with
container-speed traffic, so the socket receive queue never overflows (an
unshaped run with a 4 KB buffer and heavy traffic recorded zero drops on every
node). Extract the drop-detection edge -- read the cumulative counter, fire an
event only on the transition into a new drop burst -- into
TransportDropState::observe_drops and unit-test it directly. congestion-stress
keeps its ECN and MMP congestion-signal assertions, which do need the real
shaped bottleneck queue.
Remove the retired scenarios from both CI runners and update the chaos README.
gateway-lan pinned 172.20.1.0/24 and fd02::/64, so two concurrent local CI
runs collided on "Pool overlaps" at network creation. The IPv4 subnet has no
consumer -- the whole gateway LAN path is IPv6 -- so drop it and let docker
auto-assign. The IPv6 side cannot float, because the LAN clients' resolv.conf
pins the gateway's address as their nameserver and that must be a literal
known before they start, so claim a free /64 per run and thread the prefix
through the compose address pins, a generated resolv.conf, and the test via a
single exported variable.
The claim and the external network ship as a harness-only compose overlay
applied by run_gateway; the base compose keeps a normal gateway-lan network,
so the GitHub matrix and any standalone bring-up are unaffected. With no claim
every address renders exactly as before.
The boot check piped `systemctl is-system-running --wait` into `grep -qE
'running|degraded'`, but the script runs `set -o pipefail` and is-system-running
exits non-zero for `degraded`. So the pipeline failed on a degraded system even
though grep matched, and the wait looped to its timeout. The older distros reach
`running` (exit 0) and passed; debian:trixie and ubuntu:26.04 reach `degraded`
because a unit that cannot run in a container (systemd-modules-load) fails, so
they timed out at every ceiling -- which is why the earlier timeout increase did
nothing.
Capture the state string and test it directly instead of trusting the pipeline
exit, so a degraded-but-booted system is accepted as intended. Validated: all
five distros pass, including the two that previously failed.
cost-avoidance and mixed-technology decide a parent by measured link cost
(etx * (1 + srtt_ms/100)). Under the local CI's 4-way-parallel chaos the fast
fiber link's measured srtt spikes from host scheduling and can exceed the
Bluetooth link's, flipping the choice and reddening a run that proves nothing
about the daemon. The old margin was ~0.4 cost units, which a ~24ms one-sided
scheduling blip closes.
Widen the Bluetooth delay to 150-250ms so the margin dwarfs any plausible
one-sided spike. The link still establishes well within the handshake budget
(a ~500ms RTT against a 30s stale-handshake window), so the losing candidate
is still a real, established peer rather than an absent one.
Add a netem mutation exclude_edges option and use it in mixed-technology to
pin the two links n08 chooses between, so a random degradation can't flip the
asserted comparison either. An unknown excluded edge is a hard error, since a
silent no-op would reintroduce the flakiness it exists to remove.
Validated by running both scenarios repeatedly under heavier-than-CI
concurrent load: the parent choice holds every time.
Two concurrent ci-local runs could not both run chaos. Each child's subnet came
from its position in the suite list, so both runs walked 10.30.0 through
10.30.12 and requested identical ranges; whichever reached the daemon first won
and the other died with a pool overlap. A run index or hashed offset would only
make that unlikely, and it is precisely the failure being removed.
The simulator now claims its range: attempt-create on a candidate, advance on
docker's own overlap error, fail loudly on anything else. Docker's address pool
becomes the arbiter, so an overlap is impossible rather than improbable. The
claim lives in the sim rather than in ci-local because only the process that
creates the network can advance on conflict, and because it fixes the bare
chaos.sh path too, which a ci-local-only fix would have left broken.
The generated compose now declares the network external over the claimed one,
and teardown releases the range from both paths, including the setup-failed
path where a run that fell over after claiming still holds one.
Ordering matters here and is not obvious: node IPs derive from the subnet
during topology generation, and traffic shaping keys its filters on those
addresses, so the claim happens before the topology exists. Claiming later
would give a network on one range and filters on another, which does not fail
at bring-up and instead leaves the shaping matching nothing.
--subnet survives as an explicit pin for when a known range is wanted, and now
fails loudly if that range is taken rather than silently overlapping. ci-local
no longer passes it.
Validated by running two instances of the same scenario deliberately
concurrently: they claimed 10.30.1.0/24 and 10.30.0.0/24, both exited 0 with
their assertions passing, and both released their networks. The negative
control holds too: with a squatter on a pinned range the run aborts with a
message naming the range rather than proceeding.
With the root pinned, n08's test subject is assertable for the first time: its
two candidates sit at equal depth with fiber-grade default netem on one side and
Bluetooth on the other, which is exactly the preference this scenario exists to
test, and it takes the fiber parent in four of four runs. Encoded as written
rather than calibrated from the runs, since "should pick n03" is the spec and
the runs merely confirm it is met.
n06 is deliberately absent, and its documented criterion is corrected rather
than left standing. The header called n03 a fiber parent for n06; the netem
policies in the same file say that link is WiFi and the other candidate is
Bluetooth, so n06 chooses between two impaired links. Its only fiber-grade link
reaches a node at depth 3 that is never a good parent. There was never a reason
to expect it to prefer n03, and the two-of-four split observed is correct
behaviour: in a run where it took the Bluetooth peer, the fiber-labelled
candidate measured a link cost of 3 against the other's 1, so the daemon chose
the cheaper effective depth as it should.
Validated by replaying the assertion against all four archived runs, then
breaking it three ways to confirm it can fail, then one live run for the wiring.
One of the breaks expects n06 to take n03 and reports that it chose n02, which
is the direct demonstration of why n06 is not encoded.
cost-reeval records that its subject now fires in two runs of three, against
zero of five when it was structurally impossible, and why that rate is still not
one an assertion can rest on.
congestion-stress said the transport drop-detection path "is exercised by
nothing". Widening the scan beyond this one scenario finds it firing in six
others, so the path is live and what is dead is this scenario's ability to
provoke it. Records the arithmetic that explains why: at the 1 Mbps cap the
receive queue needs tens of milliseconds of reader stall to overflow, and the
ingress policer discards the same traffic a layer below the socket.
mixed-technology said the implementation does not do what its criteria say.
It does. The criteria were conditioned on a tree rooted at n01 while the mesh
rooted at n09, under which n08's choice was settled by depth before link
technology could matter. The archived parent distributions are relabelled as
describing the old root so nobody calibrates against them.
cost-reeval now records why it had no assertion about its own subject: its
designated subject was the root, so the parent switch it exists to observe
could not occur. The historical logs show the switch on n04 in 13 of 14 runs
when the root still wandered, and only on n01 in the five runs before the fix.
Each header now names what remains to be done and where it is tracked, rather
than leaving a disproved conclusion in place for the next reader to inherit.
For a value-returning helper the trailing echo is the return mechanism, not a
log call. The hazard is the same either way -- it fixes the exit status at zero
-- but calling it a log call misdescribes four of the sites the gate now finds.
Found while fixing count_log_pattern: the gate reported that function clean
even though it ends in echo and both callers consume its status. Its call-site
patterns only matched direct forms -- if fn, while fn, fn ||, fn && -- so a
status consumed through command substitution was invisible, because the line
begins with the variable rather than the function name.
That is not an exotic form. It is how a shell function returns a value, and it
is precisely the shape of the swallowed-failure family this gate exists to
catch, so the gate was blind to a large part of its own stated class.
Three patterns added for the assignment, if-guarded and test-expression forms.
The extension finds four real instances, all value-returning helpers whose
trailing echo made their exit status unconditionally zero, and each now carries
an explicit return 0. Also corrects the finding message, which described the
trailing command as a log call; for these it is the return mechanism, and the
hazard is that it fixes the status either way.
Validated by breaking what it guards: a probe reintroducing the defect shape
behind a command substitution is reported and exits 1, where before the
extension it would have passed.
Every chaos scenario draws n01 at the top of its topology, and until now that
held in three of thirteen. The mesh roots itself at the numerically smallest
NodeAddr, which is a hash of the node's public key and bears no relation to the
node numbering, so which node ended up as root was effectively arbitrary.
The consequences were not cosmetic. cost-reeval rooted at n04, its own
designated test subject, so that node had no parent and the periodic parent
switch the scenario exists to observe could not occur at all. mixed-technology
rooted at n09, which put its two documented parent criteria out of reach and
made correct cost-based selection look like a defect.
Identities are still derived from the mesh name exactly as before and are still
deterministic. What changes is which node id holds which one: they are now
assigned in NodeAddr order, so n01 holds the smallest and is the root. Verified
against a model of the daemon's own derivation that reproduces the previously
observed root for every scenario and n01's address byte for byte; all twelve
pinned scenarios now root at n01.
smoke-10 deliberately opts out via pin_root: false so that root election from an
arbitrary key distribution stays exercised somewhere. Its assertion is a
convergence floor and is root-agnostic, which is why it is the cheapest home for
that. The new key is rejected when non-boolean, and a near-miss spelling is
rejected as unknown; both checked.
Not yet established: the trees themselves change, so the parent-dependent
assertions in bottleneck-parent, cost-avoidance and cost-stability need
re-deriving against live runs. Those are held until the in-flight CI finishes,
because a chaos run rebuilds the shared fips-test image that run is using.
Four sites where a check could report a verdict it had not established, or
detect a failure and then not turn it red.
assert_no_panic in both NAT suites read `docker logs ... || true`, so a
container that could not be read produced empty output, matched no panic
pattern, and returned success. The assertion's failure mode was
indistinguishable from its success condition. It now reports that absence of
panics is not established.
deb-install's apt capture discarded the exit status, so a failed docker exec
gave an empty capture that matched neither error pattern and reached the pass
branch. The status is now kept and checked before the output is inspected, with
the output still printed on failure.
wait_for_systemd printed a warning and returned success on timeout, so every
check after it read a system that may not have started its units. It now returns
non-zero and the caller abandons that distro leg rather than testing an
unstarted system.
interop's copy of count_log_pattern carried the same defect fixed in rekey: a
node whose logs could not be read contributed zero to eight expect-zero
assertions. Fixed the same way, and its consumer now reports the unreadable case
rather than comparing a sentinel against zero.
Each validated by breaking what it guards and by confirming the healthy path is
unchanged: an absent container now fails each check where it previously passed,
a readable panic-free container still passes, a real panic is still caught, and
a genuinely successful install still passes.
count_log_pattern summed a per-node `docker logs | grep -c ... || true`, so a
node the harness could not read contributed zero. The six assert_zero_count
callers are negative health assertions -- no panics, no ERROR lines, no AEAD
decrypt failures, no rekey msg2 failures -- and a contributed zero reads as
clean, so one unreadable node silently weakened the assertion and all of them
voided it. This is the family the harness-fallback issue was raised to high
priority for, and it is the one remaining audit residual that can make a green
rekey run lie about protocol code.
The read now fails the count rather than degrading it, printing a sentinel that
names the container. Both callers split the declaration from the assignment,
because `local c=$(fn)` takes local's exit status and discards the function's --
which is how the original defect stayed invisible.
Validated by breaking what it guards rather than by observing green: against
absent containers the old reader returns 0 and assert_zero_count passes
vacuously, while the new one returns rc=1 and reports a failure. Checked the
other direction too, since a fix that reds a legitimately clean run is no use: a
genuine zero across readable nodes still returns 0, and the sum is unchanged at
2+1+0 for a shimmed three-node read.
A bash function returns its last command's status, so one ending in a log
call returns 0 whatever it did, and a caller written as `if func` or
`func || fail` has a gate that cannot fire. This tree found two in one
day: run_chaos ended in record, making every chaos row unconditionally
green since 2026-03-09, and build_fips_for_e2e ended in log, letting five
end-to-end legs test the previous commit's binary and report green. Both
were repaired one at a time. This is the rule that catches the next one.
What it enforces is a contract, not a bug hunt: a function whose status a
caller tests must end in an explicit return rather than leaving its
success value to whatever the last log call produced. That distinction is
deliberate and worth stating, because all three instances found in the
tree were benign — each failure path already returned early, so the
trailing echo reported a real success. Reading the function is the only
way to know that, and the next edit that puts an unguarded command before
the final log converts the benign shape into the defect with nothing to
notice. An explicit return costs a line and makes the class unreachable.
The three are given one here.
Scoped on the call sites rather than the definitions, the same way
check-log-strings.py scopes on what a grep reads rather than what its file
mentions: 315 functions scanned, 42 end in a log call, and only the ones
whose status something consumes are reported. A function that ends by
reporting and is called for its output is idiomatic and is left alone.
Without that scoping the finding list would be 42 long and would stop
being read.
Validated by construction rather than by a green run: injecting a copy of
build_fips_for_e2e's exact shape — unguarded docker cp, then a log as the
last statement, with a `|| { ...; return 1; }` caller — makes the checker
report it and exit 1.
Wired into both runners beside the parity and log-string checks.
mmp_focused_pane_indicator checked that the unfocused Link MMP title is
not cyan with assert_ne! over fg_at. fg_at is find(..)? mapped to the
cell's foreground, so it returns None for a title that was never drawn,
and None != Some(Cyan). The assertion therefore passed just as happily
when the pane was missing entirely as when it was present and unstyled.
Assert presence first, so the colour check means "not highlighted"
rather than "not there".
Demonstrated rather than argued. Renaming the pane title in mmp.rs so
"Link MMP" is never rendered leaves the original assertion passing, and
makes the new one fail with the message it was given. Both files restored
after.
This is the only instance of the shape: it is the sole assert_ne! over
fg_at or find in the fipstop tree, and the only assert_ne! in snapshots.rs
at all.
Quartet clean: fmt, build, clippy --all-targets -D warnings, test --lib
at 1376 passed / 0 failed / 7 ignored.
The UDP recv microbenchmark carried a bare #[ignore] while its sibling in
the link module carries a self-describing one. Match it, so the reason
appears wherever the test is listed rather than only in the doc comment.
Record why [profile.ci] retries = 2 is not applied locally. The two gates
disagree on purpose: the hosted runner retries a flaky test twice, the
local sweep fails on the first failure. It exists for shared-runner packet
loss, a property of that environment rather than of the code, and applying
it locally would suppress a real local flake — a failure that only
reproduces under load is a robustness bug to fix, not to retry past.
Keeping the local sweep strict is what makes it the sharper gate. An
undocumented asymmetry is indistinguishable from an oversight, which is
why this is written down rather than left to be rediscovered.
The cost is stated rather than hidden: a test that fails once and passes
on retry is reported green with no separate signal, so a genuine
intermittent failure can be absorbed. If that starts mattering the fix is
to surface retried-but-passed tests, not to drop the retries.
Quartet clean: fmt, build --workspace, clippy --all-targets -D warnings,
and test --lib at 1376 passed / 0 failed / 7 ignored.
Two suites could report a clean pass for work that did not run.
stun-faults skips Phase 2 when tc netem is unavailable, announcing it
several hundred lines above the result and then printing a bare
"stun-faults-test passed". The verdict line now carries the count and
names each skipped phase, and a run in which every phase was skipped
fails outright, since it tested nothing.
deb-install defines a skip() helper and a SKIP counter, prints "N
skipped" in its summary, and calls skip() from nowhere, so the number can
only ever be 0. Its exit tested FAIL alone, meaning a skip could not have
failed the run even once something did set the counter. Gate on SKIP too.
That changes no current outcome, which is the reason to do it now rather
than later: the machinery and the reported number already existed, so the
first skip path added would have printed "N skipped" beside a zero exit
and read as coverage.
Verified by driving the verdict logic at zero, one and three skips: clean
pass, pass with the skip named, and a non-zero exit when nothing ran.
ecn-ab-test.sh was a -test.sh with no path to a failing result: it
asserts nothing, applies no threshold, and nothing invokes it. It also
could not have worked. It read a fixed sim-results/ecn-ab-on/ path while
the runner has written timestamped directories since 2026-03-20, and not
one ecn-ab result directory exists on disk, so it has found neither input
for months and the "+10.2% recv throughput" figure the README carried is
not reproducible from anything available.
Rename it to ecn-ab-compare.sh, fix the path resolution to glob the
timestamped directories, and stop swallowing a failed simulation with
|| true so a broken run cannot feed the comparison.
Deliberately not adding the threshold assertion that would make it
gateable. That needs a calibration corpus, and none exists precisely
because the tool has never produced a kept result; a number invented now
would assert a guess. The prerequisite is a calibration run set and a
decision about what ECN is expected to deliver, which is a protocol
question. Written in the script header rather than left implied.
Add the exclusion block to ci-local.sh naming every suite and scenario
neither runner runs, with the reason for each: interop, boringtun,
iperf-test, this comparison tool, mesh-lab, and the four chaos scenarios
outside CHAOS_SUITES. Until now "not in the suite list" was
indistinguishable from "forgotten".
wait_until_connected decides whether every static suite proceeds or
gives up, so a regression in it turns those suites' verdicts into noise.
Its unit tests existed and were invoked by nothing.
Wire them into ci-local.sh beside the parity and log-string checks,
above the mode branches so --only and --test-only are gated too, and add
the matching step to the ci-parity job in ci.yml. Putting them in one
runner only would create exactly the drift check-ci-parity.sh exists to
catch, and neither placement is visible to that checker since this is not
a matrix suite.
Verified by breaking what it guards rather than by observing green:
setting wait_until_connected's default near-converged slack to 0 fails
case4, records FAIL wait-converge, and exits the sweep 1. Restored after.
They pass, which nothing had established before — 13 assertions, all
green. They also take about 45 seconds rather than the "a few seconds"
the header claimed, and nothing had contradicted that claim because no
runner had ever invoked it. Header corrected with the measurement.
The orphan sweep this called for is done and there are now no
zero-reference *-test.sh files under testing/. It also settles a
disagreement recorded between the audit and the task file: ecn-ab-test.sh
is invoked by nothing, its one reference being a README description
rather than a driver, so the audit was right and the earlier sweep wrong
to call it a documented manual tool. interop-test.sh does have a driver
in interop-stress.sh and iperf-test.sh one in iperf-compare-refs.sh, so
those two were correctly classified.
The baseline floors added for this scenario were calibrated from archived
runs, and those runs are all the 10-node 120-second variant ci-local
invokes as "churn-mixed --nodes 10 --duration 120". The file itself
defaults to 20 nodes and 600 seconds, and --nodes sed-patches num_nodes
in a copy before the scenario loads, so the gating run and a bare
chaos.sh run are different scenarios. Nothing at the site said so.
The floors hold for both, but only because the gating run is the smaller
one. Retuning them against a bare 20-node run, which clears them easily
at 20 answering, 1 root, 19 parented and 69 sessions, would put them past
what the 10-node run reaches and turn CI red while a manual run stayed
green. Say that where someone retuning them will read it.
Confirmed by running the gating invocation directly: 10 answered, 2
roots, 8 parented, 18 sessions, all inside the ranges the floors were
built from. churn-mixed is the only scenario CI overrides this way; the
other twelve run their files as written.
Every section of ping-test.sh is guarded by the profile name, so an
unrecognised profile fell through all of them and the script exited 0
having run no assertion. A typo in the caller's argument produced a green
run that tested nothing. Give it an else branch that names the valid
profiles and exits 2.
No existing leg changes behaviour: the hosted runner gates the ping step
on matrix.type == 'static', which only static-mesh and static-chain
carry, and ci-local passes only the two names in STATIC_SUITES. Worth
noting while confirming that, though: ping-test.sh accepts a mesh-public
profile that neither runner ever passes.
Also record why the convergence waits are `|| true` — they are settling
delays rather than assertions, and the directed-pair pings are what
decides the run — and add admission-cap to the suite list in the
ci-local header, which ran it without listing it.
Five scenarios named by the audit carried no assertions at all, so a run
in which the mesh never formed exited 0 and reported green. Add a
baseline assertion covering how many nodes answered, how many distinct
roots they agreed on, how many took a parent, and optionally how many
sessions were established, and apply it to those five plus the two cost
scenarios left unasserted by the previous commit.
For ethernet-only, ethernet-mesh, tcp-mesh, smoke-10, mixed-technology
and depth-vs-cost the values are not calibrated: one root and N-1
parented nodes is what a spanning tree is, and all provably-completed
archived runs of each show exactly that. churn-mixed is different and
says so at the site. A scenario that stops and starts nodes on purpose
does not hold a single tree, and its six completed runs end with two or
three roots and seven or eight of ten nodes parented, so its floors sit
one step outside the observed range. That leaves them catching a mesh
that collapsed rather than one that churned, which is the most a sample
of six supports.
This gives Ethernet transport its only assertion anywhere in CI. The
three tests in src/node/tests/ethernet.rs are ignored for requiring
CAP_NET_RAW and neither runner passes --ignored, so until now nothing
exercised that transport with a verdict attached.
The floor is deliberately weak and deliberately not a substitute: it says
the mesh formed, not that it formed the tree the scenario describes.
Where those differ the scenario now says so in its own comments.
Four scenarios exist to test cost-based parent selection and each named
its expected outcome in a comment that nothing read. Add a tree_parents
assertion mapping a node to the parent it must have in the final tree
snapshot, and encode it for the two scenarios whose stated outcome the
implementation meets: cost-avoidance (n04 takes the fiber n03) and
bottleneck-parent (n06 takes the fiber n03, n09 keeps its only parent
n05). Both hold in all six provably-completed archived runs.
The other two are left unasserted on purpose, and each for a different
reason worth keeping distinct.
mixed-technology says n06 and n08 should both pick the fiber parent n03.
They do not. Across the five completed archived runs n06 picks n10 three
times and n08 picks n04 four times. Encoding the criterion as written
would red the scenario most runs, and encoding the observed behaviour
would bless something nobody specified, so the disagreement is recorded
at the site as an open question. n06 preferring n10 may well be correct
and the comment stale, since n10 reaches it by fiber too.
depth-vs-cost is different: its validation line does not name an outcome
at all, saying only that the choice "reflects the actual cost tradeoff",
which either answer satisfies. The corpus shows both occurring under one
seed, three runs to two. That scenario needs a protocol decision about
which parent is correct before it can have an assertion.
Compare parents by address resolved from the snapshot's own
my_node_addr, and fail rather than skip when a node is absent from the
snapshot or still claims to be its own root. Those two states are the
common ones in the older corpus and both produce the same "no match" a
wrong parent does, so only separating them keeps a harness problem from
reading as a routing verdict.
The scenario listed four success criteria "verified via post-run
congestion snapshot". Nothing read the snapshot, so the scenario could
not fail on any of them.
Add a congestion_signals assertion taking a floor on the number of nodes
reporting each counter, and encode three of the four. The floors are one
node each because that is what the criteria say; tightening them to the
counts recently observed would assert something nobody wrote down.
The fourth criterion is not encoded, and that is the finding rather than
an omission. No node has reported a non-zero kernel_drop_events in any of
the 182 archived runs, so asserting it would red the scenario
permanently. It is recorded at the site as an unmet criterion and a
coverage gap: the transport drop-detection path the scenario names as its
second signal is exercised by nothing.
Two things about the corpus are worth carrying, both recorded in the
scenario. Only six archived runs carry a status.txt and are therefore
provably completed; all six meet the three encoded criteria with five to
nine nodes reporting each signal. The 176 older runs meet none of them,
and they are not invalid samples: each reached teardown far enough to
write an analysis.txt, and ECN landed before all but one of them. What
changed on 2026-07-22 is not established, since neither the scenario nor
netem.py, traffic.py or control.py has been touched.
Count the nodes reporting a signal rather than the magnitude of any one
counter, since a single node with a large count would satisfy a magnitude
test while proving the signal never propagated. A missing snapshot fails
rather than reading as an absence of congestion.
The simulation exit ladder reported panics and failed assertions but said
nothing about ERROR-level log lines, so a run in which every node errored
on every line still exited 0.
Add a max_errors assertion and apply it to every scenario by default
rather than having each one opt in. This is a floor on what a green run
means rather than a property of an individual scenario, and a scenario
that has to ask for the floor is one that can forget to.
The default ceiling of 0 is what the archived corpus supports: across
2416 result directories no node log contains an ERROR-level line, while
the WARN counter extracted by the same code path ranges from 0 to 1135,
so the counter is known to discriminate rather than merely known to read
zero. A scenario that legitimately induces errors raises the ceiling in
its own YAML and says there why.
Reject rust_log: off alongside it, since that is the one level that would
leave the ceiling counting zero whatever the mesh did.
The scenario's success criterion existed only as a comment: count
"Parent switched" in n04's log, expect at most 5. Nothing read it, and
the scenario declared no assertions at all, so its exit code could only
report that the mesh came up and nothing crashed.
Add a max_parent_switches assertion and wire that criterion to it.
The assertion takes an optional node scope, which is the part that
matters. The existing sibling counts mesh-wide, and a criterion written
about one node's log is a different quantity from the sum over every
node: across archived runs of this scenario n04 is 1 or 2 while the
mesh-wide total ranges 3 to 6. Asserting the sum against a per-node
threshold would check something other than what was specified, and at
this threshold would also have failed several runs that were fine.
Counting nothing must not read as stability. A node id absent from the
topology fails explicitly rather than matching zero log lines and
sailing under the ceiling, and a scenario that declares a parent-switch
assertion while setting a log level that suppresses the events it counts
is rejected at load rather than passing vacuously after a full run. The
parse rejects a mistyped key, a missing, negative, boolean or
non-integer ceiling, and a node written empty or as a non-string -- that
last one because YAML renders a bare `node:` as null, which would
silently revert to the mesh-wide count.
The scenario comment now records what the threshold is worth against
the 11 completed archived runs rather than a single sample: 5 is well
above anything observed, the daemon's own parent hold-down caps
switches near 6 per run, and nearly every switch lands during initial
tree formation before any mutation fires. So this catches only
near-pathological reparenting. Tightening to 3 would make it a real
detector, but that changes the stated criterion rather than fixing it,
so it is left as a decision.
Verified against a live run: the assertion evaluates n04 at 2 against
mesh-wide 4 and passes, and with the ceiling temporarily set to 0 the
scenario exits 3, the assertion-failure code, which it previously could
not reach.
The mapping-reclaimed check expects zero mappings, and read the count
with `r.get('data',{}).get('mappings',[])`. An error response carries
no data field, so it parsed to zero and satisfied the check without the
gateway having answered at all. The expected value and the failure
value were the same number, which is the shape that lets an assertion
pass without asking anything.
Require the key to exist and be a list, and exit non-zero otherwise, so
the existing fallback turns an unanswered or malformed response into a
failure. The gateway always emits the key on success, including for an
empty set, so a genuine zero still reads as zero; confirmed against the
running gateway, where the reclamation check passes.
The earlier show_mappings poll shares the parse and is left alone: it
waits for a positive two, which no error response can produce. The
hazard was already documented there, and this is the call site that was
exposed to it.
Case (a) sends an unallowed inbound request that the ruleset should
drop, and accepted any non-zero curl status as proof. A drop produces
no RST, so curl can only hit its deadline and exit 28; connection
refused, an unroutable address or a missing listener all fail too, with
different codes, and the check counted those as a blocked connection.
Require 28 exactly, so the assertion distinguishes silently dropped
from failed for some other reason. Confirmed against the harness: the
real run returns 28.
The baseline check matched `counter packets`, which any counter rule
satisfies whatever its verdict, including one that accepts. Require the
rendered drop rule instead.
The drop-counter read had the same weakness plus a worse one: it took
field three of the first line mentioning a counter, which is the packet
count only when the line begins with `counter`. On a rule such as
`tcp dport 9 counter packets 42 bytes 3000 drop` it printed the port
number. That shape is reachable, because the shipped ruleset includes
the operator drop-in directory ahead of the trailing default deny and a
drop-in may add its own counted drop. Extract by position within the
matched text, and take the last match rather than the first, since case
(a) falls through to the default deny that the ruleset emits as the
final rule.
Note in place at the baseline check that it still only proves a counted
drop rule exists somewhere in the table, not that it is the trailing
one; asserting rule position is a larger change than this warrants.
The sample output in the README is updated to the new wording.
The four rejection checks were independent greps over the whole log:
two npubs, a context and a decision. Together they proved only that
node-a mentioned each npub somewhere, rejected somebody on an inbound
handshake, and rejected somebody by denylist. Nothing tied a rejection
to a named peer, and node-a lists both denied peers as auto_connect
peers, so it logs their npubs on the outbound connect path whether or
not any rejection ever happened. The actual message was never matched.
Replace them with a helper that requires every given string on ONE
line, and assert per denied peer: the real message, that peer's npub,
and the denylist decision together. Strings match in any order by
chaining fixed-string greps over the surviving lines, so the assertion
does not depend on how the log formatter orders a message and its
fields. A grep over empty input yields the empty string rather than
anything a caller could mistake for a match, so an unreadable container
times out and fails instead of passing.
Two limits are written at the call sites rather than left implied. The
decision conjunct discriminates nothing, since the two allowing
variants return early and denylist is the only value that can reach
that warning. And node-a authorizes before dialing, so it emits a
fully formed rejection line for each denied peer on the outbound path;
these assertions are satisfiable without the inbound check running at
all, and it is the peer-count assertions that would catch that.
Verified against the running harness: all three find a real matching
line. The defect shape they replace was exercised offline first, with
the three strings spread across three lines, where the old checks
passed and the new ones fail.
The traversal clock cached a Unix millisecond value and an Instant at
first use, then served every later call by advancing the cached value
with the monotonic elapsed time. A monotonic clock does not tick while
the host is suspended, so once a machine had slept the returned value
trailed real time by the sleep duration for the rest of the process
lifetime, and never re-synced.
Almost everything that clock feeds is an absolute timestamp. The NIP-40
expiration tags on adverts and traversal signals, and the issuedAt and
expiresAt fields of offers and answers, are all computed as now plus a
TTL; the freshness and cache-pruning paths compare it against a
peer-authored, signed created_at. Once the host had slept longer than
signal_ttl_secs, every offer was published already expired, relays
dropped it, and the initiator timed out waiting for an answer with
traversal broken until a restart.
Read the wall clock on every call instead. This also removes a
mismatch inside the traversal failure-state map, which was written
here from the cached clock but written and read from the node
lifecycle with the real one.
Not platform-specific: monotonic clocks exclude suspended time on
Linux and Windows as well, so this affected any host that suspends.
A laptop is simply where a process lives long enough across a sleep
to notice.
The interval-shaped consumers hold up under a clock step. A forward
step, which is what a resume produces, saturates the punch start delay
to zero, and the attempt's own bounds are monotonic deadlines that are
unaffected. A backward step lengthens that delay and can cost one punch
attempt, which retries. Early eviction from the replay window cannot
admit a replay under the shipped defaults, because the freshness window
a replayed offer must also satisfy is strictly narrower than the replay
window itself.
The added test pins the contract and fires on a host that has genuinely
suspended, but it is not a regression guard for this defect: nothing
reachable from a unit test can simulate a suspend, so on a machine that
has not slept the old implementation passes it too. Its comment says
so rather than leaving a false sense of coverage.
Reported in https://github.com/jmcorgan/fips/issues/128
wait_for_peers_exact fell back to 0 when it could not read a container's peer
count. Two of its six call sites are the ACL denial checks, which expect
exactly 0 connected peers — so a failed docker exec satisfied them on the
first iteration and the isolation property the suite exists to prove was
never observed. Confirmed against a container that does not exist: the old
reader returns "0" and the expect-zero comparison passes.
The reader now returns the empty string when the daemon does not answer,
which no numeric comparison can satisfy, and the timeout message says which
of the two happened — never answered, or answered the wrong number. Same
shape as admission-cap-test.sh's read_peer_count.
A test that greps the daemon's log for a message the daemon stopped emitting
does not fail. It stops observing, and an assertion built on it — especially
one expecting a count of zero — then passes because nothing can be seen
rather than because nothing happened. Several findings have come from that
one class, so it is now checked mechanically.
testing/check-log-strings.py extracts the strings test code matches against
daemon log text and requires each to exist in src/. It reads four shapes:
python `"..." in line` tests, the first argument of the shell log helpers,
bash associative-array pattern tables, and greps whose input is daemon log
output. Patterns carry regex syntax, so each is reduced to the longest
literal run of every alternation branch, with escaping resolved in the same
pass — `\.` is a literal dot and `.` is a wildcard, and conflating them would
let a pattern match text that is not there.
Scoping is by what a grep READS, not by what its file mentions. A suite greps
its own analyzer output, fipsctl JSON, Tor's log and ping output, and none of
those have to correspond to a string in src/; scoping on the file produced 34
false positives. Strings that legitimately do not come from src/ are named in
ALLOWED with a reason, so the exceptions are reviewable rather than invisible.
It found six dead strings, three of which were not previously known: a second
copy of "Excessive decrypt failures" in the interop pattern table, a dead
"Handshake error" branch inside an alternation whose other branches still
matched, and "bootstrap failed" in the NAT suite. All six are corrected to
what the daemon emits, or dropped where the surviving branch already covers
the case. Verified by breaking it two ways and confirming it reports.
Also here, since it is the same failure shape one layer down: the shared log
library returned an empty string for a container whose logs it could not
read, so analysing a missing container reported no panics, no errors and no
sessions, and exited 0 — indistinguishable from a clean run. It now reports
which containers could not be read and raises, matching the fix already
landed in the chaos copy.
Two local CI runs on one host both asked docker for 172.20.0.0/24 and the
second lost its whole static family to "Pool overlaps". Docker honours a
fixed subnet request verbatim, so the only robust fix is to stop making one:
fips-net now requests no subnet and docker assigns from its own pool, which
cannot hand the same range to two runs.
That means node addresses are not known before `up`, so peers address each
other by container hostname instead. The generator emits node-<id>, or the
topology's docker_host where the compose hostname differs — only the gateway
profile, whose services are gw-*. External peers keep the address the
topology gives them, since it is not ours to assign. The resolv.conf mount
stays: dnsmasq is what forwards these names to docker's resolver and .fips
to the daemon, so removing it would take out every .fips assertion.
generated-configs is now per-run as well. A shared directory let two runs
overwrite each other's node configs, which the subnet collision had been
hiding by killing runs before that window opened. The generator, the compose
bind mounts and env_file, the six scripts that read it, and teardown all
follow FIPS_CI_NAME_SUFFIX; unset, every path renders as before. Teardown
keeps the directory after a failed run, where it is the evidence of what the
failing nodes were configured with.
Three things this exposed that were wrong independently:
admission-cap built its tcpdump patterns from the topology file's docker_ip
literals. Floating the subnet makes those match nothing, which would have
left its expect-zero "no Msg2 leaked" assertion passing because it could no
longer see anything at all. It now reads addresses from the running
containers. Restarting the denied peers together also made them swap
addresses, so each peer's counts were really the pair's total; they are
restarted one at a time now, and a check fails the suite outright if two
denied peers ever share an address, because per-peer attribution is
impossible once they do.
Attribute lookups in the generator used a fixed ten-line window and read the
next node's fields when a node omitted an attribute. An external node
followed by an internal one was classified as internal, which under hostname
peering would emit a name that resolves nowhere. Lookups are bounded to the
node's own block; generated output is byte-identical for all eight
topologies.
The rekey outbound-only variant used to rewrite peer addresses to hostnames
to set up its scenario. The generator now does that everywhere, so the
rewrite matched nothing and was silently doing no work. It asserts the
premise instead, and fails if a numeric address ever reappears.
Verified by running three instances of this compose at once — tcp-chain plus
two independent meshes — which drew 10.128.2/3/4.0/24 with no overlap while
both meshes passed ping-test 20/20 over the real .fips path. tcp-chain is
run by neither CI runner, so it was checked by hand: chain peer counts 1/2/1
and multi-hop .fips reachable both directions over TCP.
gateway-lan still pins its own IPv4 and fd02:: ranges and is unchanged here,
so the gateway profile is not yet concurrency-safe.
Four of these are places the parity guard could stop covering something without saying so, which is the failure mode it exists to prevent. Removing the deb-install suite array left it green, because the dispatch cross-check compared against a hardcoded name rather than the array it was meant to read. An arm-shaped line at an unexpected indent, or one written in quotes, was dropped silently by the arm pattern; unexpected indents are now a hard error rather than a quiet skip, and quoted arms parse. A matrix leg with a type of chaos or deb-install but no scenario key raised a Python traceback instead of reporting a problem, and a leg carrying a scenario but no suite key was skipped entirely even though scenario is now the identity for those legs.
The e2e builder no longer folds docker create's stderr into the container id. Docker prints warnings on success as well as failure, so a platform-mismatch warning would have become part of the id and made every later reference to that container fail, turning a working extraction into a spurious red.
Also corrects the comment about registering a new chaos assertion type, which named only one of the two key sets that actually need it.
The guard that keeps "local green" and "GitHub green" meaning the same thing was invoked by nothing. Its only entry point was an exec behind an explicit --check-parity flag, which by exec-ing could not be combined with an actual run, and no workflow step called it at all. A guard nobody runs is a guard that does not exist, which is how the mixed-profile divergence on next survived unnoticed.
It now runs as the first stage of every local run and as its own job on GitHub. Locally it reports through the same result-recording path as every other stage, so a divergence sets the run's exit status instead of scrolling past. It is deliberately placed above the mode branches, so a divergence also fails --only, --test-only and --build-only runs: whichever subset was asked for, the claim that a local result means what a GitHub result means is what has broken. The GitHub side installs pyyaml explicitly rather than assuming the runner image carries it.
--check-parity keeps working exactly as before. Three comment blocks that described the old folded comparison are corrected here, in the workflow, the local runner and the testing README; they were spread across three files and only two mention the guard by name, so the third is best found by searching for what it claims rather than for the script.
The guard collapsed all thirteen chaos legs to the token "chaos" and all five deb-install legs to "deb-install", so sixteen of the thirty-four integration legs were invisible to it. Deleting twelve chaos legs and four deb-install legs from a copy of the workflow left it still reporting "CI parity OK". The fold existed because the guard compared the cosmetic suite name, which really does differ between the runners; both sides have carried a directly comparable scenario field all along, so the fix is to compare that instead, along with the chaos flags.
The local suite set is now discovered by sweeping ci-local.sh for suite arrays rather than reading a hardcoded list of variable names, and the deb-install distro list is read from the suite's own script. A suite dispatched with no backing array is invisible to either approach, so every run_suite arm is now checked to have one and the guard names any that does not. That is not hypothetical: next dispatches its mixed-profile suite without an array, and the guard could only report it as missing from the local runner without being able to say why.
The guard also now refuses to run rather than failing obscurely when python3 or pyyaml is absent, since a parity check that cannot run must not look like one that passed. Verified by breaking each thing it guards: legs deleted from the workflow, drifted chaos flags, a fabricated suite array, and a dispatch arm whose array was removed. That last case initially did not fire because the arm pattern was pinned to the wrong indentation, which the test caught.
The scenario loader read every section and member through raw.get with a default, so a mistyped key was silently ignored and the block it belonged to stayed default-constructed while the YAML still looked correct on the page. Writing "assertion:" for "assertions:" disarmed a scenario's only assertions and left it unable to return a non-zero exit code; "link_flap:" for "link_flaps:" turned off the churn the scenario existed to inject, and a calm mesh then ran under the name of a chaos test.
Keys are now checked at load time, at the top level and inside every section with a fixed schema, including the netem policy bodies, the {min, max} ranges and the assertion bodies themselves. The error names the offending key, the section it appeared in, and the keys that section does understand. Three mappings are deliberately exempt because their keys are names the author chooses rather than a schema, and two sub-trees are passed through whole; all five are listed at the key sets with the reason.
Adding an assertion type now means registering it, so the next person to add one gets a rejection rather than a silent no-op. That coupling is the point and it is written down where they will see it. Verified against the seventeen scenarios in the tree, all of which still load, and against fixtures for each failure mode: a top-level typo, a nested typo, a typo inside an assertion body, and a section left empty, which previously raised an unrelated AttributeError from the next line of the loader.
The e2e scenarios build fips and fips-gateway in a builder image and extract them with two docker cp calls whose stderr went to /dev/null and whose exit status was never tested. Because build_fips_for_e2e ended in a log call it returned 0 on every path, so the caller's guard could not fire. A failed extraction left the previous run's binaries in the cache, they satisfied the caller's executable check, and the five e2e legs then exercised the previous commit's code and reported green.
The extraction now deletes the cached binaries before it starts, which is what makes the stale case impossible rather than merely detected, and it checks each docker cp independently, reports the failure with docker's own message, verifies each extracted file is non-empty and executable, and returns an explicit status. This mirrors what the deb-install suite already does.
Confirmed by inducing the failure rather than by observing a green run. With both extractions broken and a stale binary in place, the old code logged "Cached fips (26936 bytes)" and went on to build the runtime image around it; the new code reports both docker errors, deletes the stale binaries and fails the suite. Breaking either extraction on its own is caught separately, and an unmodified run passes 7 of 7.
The chaos simulation could not report a failure on this line either: scenarios collided on globally-scoped container names and a shared config directory, the runner swallowed whatever they raised, and the harness discarded their exit codes before anything read them. The five commits scope the names and directories per scenario, give an aborted run its own exit code and let that code reach the summary, stop a failed scenario harvesting whichever containers happen to hold its names, and keep the daemon's reason for a failure instead of throwing it away.
Testing only, and the merge is textual: every chaos file matches maint byte for byte, and the ci-local divergence on this line does not touch the changed regions.
Compose commands run with their output captured and with check set, so a failure raised a CalledProcessError and nothing ever looked at what the daemon had said. That exception reports the argv and the exit status and nothing else, so every scenario that died bringing its containers up left a runner log saying only that docker compose returned non-zero exit status 1, and the reason was captured and then discarded. In the run that prompted this work twelve scenarios failed that way and why they failed can no longer be established at all. Log the captured stderr and stdout before the error goes anywhere, keeping the tail of each so a verbose failure cannot fill the log file. A command that times out raises before its exit status is ever read, so that path records whatever it had emitted too, rather than leaving the same hole one shape over. Reproduced against both of the shapes this is meant to catch: an unusable network prefix now records the daemon's parse error, and a name already held by another container now records the conflict and names the container holding it. The success path logs nothing new, and a completed scenario's runner log is unchanged line for line.
The raising behaviour is deliberately untouched, since the abort flag and the exit code that came with it depend on it. The check is applied here rather than by subprocess so that a caller who passed check false gets the same record: both such callers are the compose down in teardown, which today can leave a mesh behind without saying so.
The same omission was costing the veth path its reasons. Failing ip commands had their stderr logged at debug, which the runner does not emit unless asked for verbose output, so a failed pair creation reached the log as nothing more than the names of the two interfaces it could not create. Raise that to a warning. The deletes issued to clear a stale pair are expected to fail and still say nothing.
Teardown runs from a finally, so it also runs after a failed setup, and it was guarded only on the topology and the compose file, both of which are set well before any container exists. A scenario that died bringing its containers up therefore ran the whole harvest anyway: final snapshots, docker logs, the analysis, the assertions and the metadata, all addressing containers by names that are global to the host. What it left behind was not an empty directory an investigator would notice but a full and plausible one, in the recorded case ten node logs and an analysis reporting two promotions and two parent switches, every byte of it from a different scenario's mesh. Gate the harvest on whether the containers ever started, and leave the compose down outside that gate so a partly successful start still gets cleaned up.
Record the outcome in a status file in every result directory, naming the run as completed, interrupted, aborted, setup-failed or teardown-failed, alongside the scenario, the seed and the container names it used. With the harvest gated, the presence of an analysis file is now itself proof that the scenario's own mesh existed, and the status file says which of the several ways a run can end applies. A run cut short by a signal keeps the existing exit codes rather than gaining one of its own: what it collected before stopping is real and still worth reporting, the status file records that it was truncated, and every wrapper already reports a Ctrl-C of its own.
Log collection never looked at the status of docker logs. It concatenated stdout and stderr unconditionally, so collecting from containers that were not there wrote the daemon's "No such container" reply into each node log and analysed the result as a mesh with no panics, no errors and no sessions, which reads exactly like a clean run and exits zero. Check the return code, and treat a harvest that cannot read every container, or that reads none, as a failed teardown. A teardown that raises now also reports on the same footing as a run that never started, instead of escaping past the exit codes as a bare traceback and being read as a malformed command line.
The simulation caught every exception a scenario raised, logged it, and exited 0, so a scenario that died during setup was recorded as a pass. Local CI discarded the exit code in any case: run_chaos ends in a call whose own last statement is an echo, so it returned 0 whatever the scenario did, and the parallel launcher's wait therefore always succeeded and always recorded a pass. Between them no chaos failure of any kind could turn a local run red, and none could since the script was written.
Carry the abort out of the run and give it its own exit code, ordered ahead of the two content codes so a run that never produced a mesh cannot report that mesh's panic and assertion counts. Return the scenario's status from run_chaos so the launcher sees it. The documented exit codes were stale before this change and are rewritten in full, including that the argument parser rejects a malformed command line with the same code that reports panics.
Expect scenarios that have been reporting green to start reporting red. In the last recorded local run twelve of the thirteen scenarios never got past starting their containers, and all thirteen were counted as passes; those that now execute have not run for months, and their assertions have never been evaluated against a mesh of their own.
Ethernet edges get a veth pair created in the host namespace and then moved into the containers, named from the node ids alone. ethernet-mesh and ethernet-only both connect n01 to n04 and always run together, so one tore down the other's live link. Hash the scenario suffix to four hex characters and insert those after the vh prefix; 15 characters is far too few to carry the suffix itself.
The names are now run-specific and never regenerated, so the delete-before-create no longer reclaims an interface an earlier run abandoned. ci-cleanup.sh takes over, deriving the names from the suffixes a run's teardown hands it. It is the first thing that script removes that is neither a docker object nor labelled, so an unscoped reap can sever a running bare simulation's links; testing/README.md and the --reap help say so.
Chaos scenarios run four at a time under local CI, but every one of them claimed the container names fips-node-nNN and wrote its generated configs and compose file to the same generated-configs/sim directory. Container names are global in Docker and are not scoped by the compose project, so concurrent scenarios collided on both, and a scenario could start containers from a compose file another had overwritten.
Thread the existing FIPS_CI_NAME_SUFFIX into the simulation. run_chaos narrows the run-wide suffix to the scenario, and the sim reads it once when the topology is built, applying it to the container names and to the config directory basename. The compose template renders the name through the topology accessor instead of duplicating the literal, so one expression produces every chaos container name.
The suffix is empty when the variable is unset, so a bare chaos.sh run and the hosted CI jobs render byte-identical names and paths. Verified by rendering every scenario's compose file before and after with the variable unset and diffing.
Both branches carry the same correction, authored separately because the
sans-IO relocation moved the logic into a core that returns an outcome rather
than acting inline. Git resolved the file move as a rename and offered to merge
the two versions textually, which would have been wrong in both files.
Resolved by keeping master's re-expression wholesale for the handler and its
tests, and taking the changelog entry from maint. Verified rather than assumed:
the resulting tree is byte identical to the reviewed and CI-green master commit
across every code path, and the merge contributes nothing but the fifteen line
changelog entry. Since the code is unchanged from a tree that already passed the
full local suite, that result still stands and no re-run is owed.
Nothing was lost from maint's side. Its tests are written against a file layout
that does not exist here, and master carries equivalent coverage written against
its own shape, including the multi-node chain and the cache warming cases that
the maint version does not have.
The same correction as on the maintenance line, re-expressed rather than merged,
because the sans-IO relocation moved this logic into a synchronous core that
returns an outcome instead of acting inline.
Delivery to the addressed node is no longer gated on the hop limit, and the
decrement happens before the drop decision, so a datagram that would leave with
zero is not sent. The reachable radius does not change: the old behavior refused
to deliver at zero but was willing to send at zero, and the new behavior is the
mirror of that, so the two cancel.
The shell above the core needed changing too, and that was the real work. It
carried two hop-limit predicates of its own, both justified by comments citing the
ordering that existed before the refactor. The first gated coordinate cache
warming, which this fix requires to be unconditional; left alone it would have
suppressed warming twice over, once for a datagram addressed here and now
delivered, and once for a transit datagram dropped for hop limit whose plaintext
coordinates are still perfectly usable. The second gated next hop resolution,
which carries a cache touch side effect, and now mirrors the core's
would-leave-zero rule instead of the old one. Neither divergence would have
surfaced as a test failure.
That last point is worth stating plainly, because it nearly went the other way.
Restoring the warming gate on top of the corrected core passes the entire suite
without a single failure. The tests could not see the seam between the core rule
and the shell's copy of it. So this adds coverage that can: a three node chain
pinning the decrement across a real hop, which is the only thing exercising the
shell predicates composed with the core, and two warming tests at a zero hop limit
covering both the delivered and the dropped case. Each was checked by putting the
specific defect it targets back and confirming the test fails.
One existing test that claimed to cover hop limit behavior asserted nothing
whatsoever. It asserts now, but those assertions do not discriminate this change,
since the case it exercises behaves identically under both rules. They guard
against the test going vacuous again, not against this defect, and the comment
says so.
The forwarding path tested the hop limit before testing whether the datagram was
addressed to this node, and decremented only on the forwarding branch. Two
consequences followed. A datagram addressed here that arrived with a zero hop
limit was dropped rather than delivered. And a forwarder receiving a transit
datagram with one hop left transmitted it with zero for the next hop to discard,
wasting a transmission on every expiring datagram.
Delivery to the addressed node is no longer gated on the hop limit, and the
decrement now happens before the drop decision, so a datagram that would leave
with zero is not sent. Saturating subtraction folds the two arrivals that cannot
be transmitted, already exhausted and last hop, into a single test.
The reachable radius does not change, which is worth stating because it is easy
to assume otherwise. The old behavior refused to deliver at zero but was willing
to send at zero, and the new behavior is the mirror of that, so the two cancel: a
path of h links still delivers for any source value of h or more. What this
actually buys is the wasted transmission, the exhaustion counter charging at the
node that makes the decision rather than the one after it, conformance with the
specified semantics, and one real gain during a rolling upgrade, where an
unupgraded forwarder feeding an upgraded destination delivers one hop further
than either version does on its own.
Coordinate cache warming moved ahead of both decisions, so a transit datagram
later dropped for hop limit still contributes its plaintext coordinates. The only
arrivals this newly warms from are those with a zero hop limit, and every insert
they can make is already achievable with a value of one, so it grants nothing
that was not already available and adds no memory growth the cap does not bound.
Four tests added: delivery at zero to this node, the transit drop at one, the
transit pass at two, and the one per hop decrement across a three node chain,
which brackets the emitted value from both sides since it is only observable
through what the next hop does. One existing test was renamed because its
destination was this node, so it never exercised the transit path its name
claimed, and it asserted nothing at all.
maint carries only its 0.4.2-dev version bump; master keeps its own
0.5.0-dev development version. Recorded as a linkage merge so later
forward-merges of real fixes land cleanly.
Bump the version to 0.4.2-dev and update the status badge and prose. The
0.4.x patch line continues; v0.4.1 remains the shipped release it points at.
Follows the shape of the v0.4.0 rollover commit. No maint reset was needed
this time: v0.4.1 was cut from maint, so the tag is already on this line and
it simply continues.
Carries the v0.4.1 content up the one-way branch flow. The release itself
belongs to maint, so the parts of this merge that identify a version are
resolved in master's favor and the parts that describe the project's state
are taken from maint.
Kept master's: Cargo.toml and Cargo.lock at 0.5.0-dev, the README status
badge, and the "FIPS is at v0.5.0-dev on the master branch" line. A patch
release consumes no minor version and master's development line is
unaffected by it.
Took from maint: the bloom antipoison FPR default change and the
duplicate-definition fix it rests on, the two docs that describe them, the
new v0.4.1 release notes, the correction to the v0.4.0 release date, and the
[0.4.1] changelog section, which slots below master's own [Unreleased] and
above [0.4.0]. The root RELEASE-NOTES.md mirror moves to v0.4.1 because it
tracks the latest shipped release, which also clears the stale provisional
date it had been carrying.
One line needed splitting rather than choosing: the README said "v0.4.0 has
shipped" inside the paragraph that identifies master as 0.5.0-dev. The
version identity stays master's and the shipped-release pointer moves to
v0.4.1.
Quartet green on the result: 1645 tests passed, clippy clean with -D warnings.
Drop the -dev suffix from the package version and refresh the lockfile.
The version is single-sourced from CARGO_PKG_VERSION, so build.rs and
src/version.rs need no change.
Also fix the archived v0.4.0 release notes, which still read
"2026-06-21 (provisional)" while the changelog records the actual
release date of 2026-06-27. The date was confirmed in the changelog at
the time but not in the two release-notes files that carry it, so the
shipped notes have been showing a wrong date and the word provisional
since that release. The root mirror is regenerated for v0.4.1, so only
the archived copy needed correcting.
Back-fill the changelog for the six user-facing changes on this line
since v0.4.0: the antipoison FPR cap default, the bloom probe and
secp256k1 context performance work, the coordinate-cache and path-MTU
fixes, and the removed parent_switched counter. The Unreleased section
was empty, so all six entries are new.
Write the v0.4.1 release notes and mirror them to the root copy. The
notes lead with the FPR cap change and state plainly that this is the
second raise of that default in two releases, that it buys headroom
rather than fixing the underlying fixed-filter constraint, and that the
structural remedy is the v2 filter work. They also flag the rolling
upgrade window, where an upgraded and a not-yet-upgraded node can
disagree about mesh size, and point consumers of parent_switched at
parent_switches.
Bump the README status badge and the roadmap prose together so the two
agree.
The inbound FilterAnnounce cap at 0.10 rejects aggregates that are
legitimately near their operating ceiling, before the network reaches
the fixed-filter capacity limit. Raise the default to 0.20, which
corresponds to fill 0.7248 at k=5, about 2,114 entries on the 1 KB
filter (Swamidass-Baldi).
Also remove the duplicate default definitions in BloomConfig. Each
default was written twice, once in impl Default and once in the serde
default function, with nothing enforcing that they agree, so a config
file that omits the key took a different path from one that sets it.
impl Default now delegates to the serde default functions, leaving a
single source of truth.
The isolation loop and the failure-log dump both built container names
without the run suffix, so under a suffixed run they addressed containers
that do not exist.
The consequence was worse than the visible failure. Two of the three
checks in that loop assert a ping FAILS, and a ping into a non-existent
container fails for the wrong reason - so both passed vacuously, and the
security assertion the loop exists for was silently a no-op. Only the
third check, which expects a ping to succeed, noticed anything was wrong.
Add a guard that fails loudly when the container is absent, so a future
naming slip cannot quietly turn these back into no-ops rather than
surfacing as one confusing failure among two false passes.
These sites were missed because the suite passes when run by hand: with no
suffix set the unsuffixed names are correct, and the defect only appears
under the automation that sets one. Verified this time with the suffix
set, which is the condition that matters: 18 of 18, container names
resolving to real containers.
Making the sidecar network name unique per run exposed a latent problem
rather than causing a new one. With a fixed name, compose found any
existing network and reused it, which quietly masked the fact that the
address range was hardcoded. With a unique name there is nothing to
reuse, so the range is requested every time and the run fails outright if
anything already holds it.
Claim the range instead of assuming it. The suite now tries to create its
network on a candidate range and, if the daemon reports an overlap, moves
to the next candidate. The daemon's own address pool becomes the arbiter,
so two concurrent runs cannot land on the same range at all - as opposed
to a range derived from the run identifier, which only makes a collision
unlikely, and a collision is the precise failure being avoided.
The node addresses are derived from whichever range is claimed rather than
written alongside it, including the gateway address the isolation check
probes. Moved to a range that nothing in the tree claims and that sits
outside the daemon's default pool.
Only an overlap is worth advancing on. Any other creation failure is real
and is reported with the daemon's own message, rather than being retried
sixty-four times and buried - a lesson from a failure elsewhere in the
harness this week that was undiagnosable because its error output was
discarded.
All three nodes now attach to the pre-created network as external, so
teardown removes it explicitly.
Verified: three concurrent claims take three distinct ranges; a
non-overlap error fails fast rather than looping; the suite passes 18/18;
and the claimed gateway address is confirmed pingable when not firewalled,
so the isolation assertions still mean something.
The suite red-ed with an empty final peer_count while every substantive
assertion passed: both denied peers showed inbound handshake attempts and
zero outbound responses, which is the gate this suite exists to verify.
Two pre-existing defects combined. The final count was sampled once, in
the same second the load driver issues its last round of peer restarts, so
the read can land on a daemon busy with those restarts and return nothing.
And the fallback meant to cover that could never fire, because the shell
applies it to the last stage of the pipeline, which succeeds on empty
input - so an unanswered query was indistinguishable from a genuine zero,
and reported as a cap violation.
Poll the final read for a short window instead, and give the two cases
distinct output. A real regression still fails, just after the retry
window, since the loop exits early only on the expected value. Extract the
read the two phases share rather than leaving them to drift.
Two runs on one host destroyed each other's containers, producing
mid-test "No such container" failures that look like real defects. The
automated builder runs a full local CI on the same box every few minutes,
so the machine is contended almost always and this has red-ed both a hand
run and an automated gate.
Two independent causes are fixed here. Container names were hardcoded, and
docker names are global rather than scoped by compose project, so two runs
collided on the same name; every name now takes an optional suffix that
the harness sets from the run id. And the cleanup sweep matched a label
shared by every run, so one run's teardown force-removed another's
containers; resources now also carry a per-run label and the sweep can be
narrowed to it.
A third hazard turned up that was not in the original report: the sidecar
suite passes explicit compose project names, which override the run-scoped
project and put it outside the shared prefix entirely. Its project names,
network, and derived container references are now scoped too.
Both are default-off. With the suffix unset, names render exactly as they
do today and a bare compose invocation is unchanged, which is what keeps
the hosted CI and the documentation correct. A cleanup run with no run id
still reaps everything, which is what a manual "clear the box" wants.
The literal-name sweep was not sufficient: six scripts build container
names dynamically from node labels, and two suites create their own
containers outside compose. Those are handled at their construction sites.
Verified: syntax check on all modified scripts; compose validation on
every modified file with the suffix both set and unset; and a synthetic
two-run reproduction that shows the old cleanup destroying a bystander run
and the new one leaving it alone.
Known gap: subnets are still hardcoded, so two concurrent full runs will
still collide on address-pool overlap. That fix reaches into topology
configs, chaos scenarios, diagrams and production source, so it is left
for its own change rather than half-done here.
The lifecycle view's doc described the shell as implementing it over a
`connections` map that no longer exists. The establish view named that map
too, and was doubly wrong: both of its snapshot builders read only
`peers`. Rewrite both to describe the maps actually read.
Several comments in the node module carried shorthand from private
working notes - bare parenthetical labels, and one reference to an
internal tracker item - none of which mean anything to a reader of this
repo. Rewrite them so each clause states its own reasoning inline. The
paragraph above the tracker reference already carried the rationale, so
that one is simply dropped.
Comment-only; no behavior change.
The rekey suites' Phase 4 polled until the initiator-cutover count
reached its value at phase entry plus two, while Phase 6 asserts an
absolute minimum of four. On an unloaded host the ping phases can
outrun the jittered first rekey cycle, so Phase 4 can begin with a
single cutover on the books, release at three, and leave Phase 6 to
lose a sub-second race against the remaining first-cycle cutovers.
A rekey-outbound-only run failed exactly this way: five counted
cutover lines by t=32.6s of container life, with the Phase-6 count
snapshot at t~32.4s seeing three.
Floor the Phase-4 wait target at four so a released wait guarantees
the Phase-6 assertion — the docker-logs count is monotone over a
container's lifetime. A genuine rekey stall still times out the
bounded wait and fails Phase 6 with the real count.
Also drop the second-cycle framing from the comment, the phase
banner, and the assertion description: with rekey timers resetting at
each cutover a second cycle cannot begin inside this test's window,
so the counted events are first-cycle cutovers spread across the
topology's links, emitted once or twice per completed link rekey
depending on which side's promotion path logs them.
The per-peer control machine has absorbed every field the pending
connection carried. What remained was a struct holding two Noise
handles beside a duplicate copy of bookkeeping nobody read. Replace it
with a small carrier for the two handles and delete the type.
Presence of that carrier, not the state of the handles inside it, is
what marks a machine as mid-handshake. The distinction is essential
rather than stylistic: a failed handshake drops its initiation handle
and is deliberately retained so the stale sweep can reclaim it, and a
completed one has its session taken before disposal. Deriving presence
from the handles would make both invisible to the sweep, the
connection count, and the peering budget at once, leaking the slot
permanently. A test drives an empty carrier past every presence
predicate and then detaches it, so a future edit cannot quietly couple
the two.
The remote startup epoch now comes from the surviving carrier, which
the handshake operations already wrote at the same two points with the
same value. The paired writes onto the pending connection's own
bookkeeping had no readers left and are gone.
The handshake-phase surface leaves the public API: it was public by
accident rather than design, and the machine behind it is crate
internal. Callers outside the crate that need a view of pending
handshakes go through the operator queries, which are unchanged.
ConnectionState::inbound_with_transport loses its last non-test caller
with the inbound seed and is marked test-only.
The pending connection and the per-peer control machine have carried
duplicate copies of the handshake-phase fields since the machine gained
its own connection state. Read them from the machine and drop the
connection's projections.
The peer identity is the sharp one. The connection learned it from msg1
and the machine's carrier did not, so the two views genuinely disagreed
for inbound connections until the Noise operations moved onto the
machine and began recording each result on both. That is now in place,
so every reader can take the machine's copy unchanged: the establish
snapshot, the promotion sweep for competing connections, the dial and
path in-progress checks, the peering observations and per-peer in-flight
budget, and the stale-connection sweep's retry address.
Also repointed: our_index, their_index, transport_id, started_at, the
stored handshake message bytes, and the idle-timeout check. Two
duplicate index writes on the connection are dropped, both immediately
preceded by the machine-side write of the same value. Reads that
previously came off a connection detached just before its machine was
disposed now capture the machine's value first.
Test seeding follows the establish paths: the seed builders write
our_index to the carrier, which promotion now reads. A new test pins
the inbound identity learn on the carrier and the retry address a
failed inbound connection reports to the sweep, so a silent regression
to a blank identity cannot pass.
ConnectionState::duration loses its last non-test caller with the
connection accessor and is marked test-only.
These three had no counterpart on the control machine, so readers still
reached them through the pending connection. Add machine-side
accessors and repoint every reader, then drop the connection's.
The peer address needed its writes lifted, not just its reads
repointed: the machine's copy was never written. It is now written at
each of the points the connection's copy was — the inbound seed, the
dial, message-2 completion, and the two paths that seed a machine from
a pre-built connection — so promotion and the resend path read a value
with the same provenance at the same time as before.
Link and direction need no lift. Both machine constructors already
seed them from the same arguments the connection is built with, so an
outbound machine carries outbound state and an inbound one inbound,
and the machine's link always equals the connection's. The handshake
operations' direction guards read the machine's copy for the same
reason.
The stale-connection sweep's teardown log and its resend path both now
take the transport and address from the machine, and each keeps its own
check that a pending connection is still attached rather than relying
on the caller to have established it.
Add a test pinning link, direction, and address on the two shapes that
seed a carrier independently — the dial and an accepted message 1. The
cross-connection winner reads both values from a carrier one of those
two already seeded.
The pending connection drove its own Noise handshake while the control
machine held it, so the crypto and the state it produces lived on the
carrier that is going away. Move the six operations onto the machine:
starting and completing an initiation, processing an inbound
initiation, taking the session, testing for one, and dropping the
handle on failure. Bodies are unchanged apart from reaching the
handles through the attached connection.
Each operation now records its results on both carriers. The learned
identity, the remote epoch, and the activity stamp are written to the
machine's own bookkeeping at the same point and with the same value as
they are written to the connection's. The connection's copies still
have readers until those reads are repointed, so both have to be
written; the machine's copy of the learned identity was previously
never populated for an inbound connection, which is why an inbound
pending row showed no expected peer.
Driving the handshake from the machine means the machine has to exist
before the crypto runs. On the inbound path it is built above the
message-1 processing and still kept local, so a rejected message
leaves no registry entry and allocates no index. On the outbound path
it already existed from the dial, so it simply takes the connection
before the index allocation, and both failure arms unwind it as they
did.
Completing message 2 no longer mirrors the activity stamp separately,
since the completion itself now writes both carriers at the point the
mirror was approximating.
Three tests cover what the compiler cannot. A connection whose
handshake failed holds neither Noise handle yet must stay visible to
the stale-connection sweep, or every failed connection would leak and
hold a peering-budget slot forever. A message 1 rejected by the crypto
or by the ACL must leave no machine, no index, and the same rejection
count as before. A dial whose message-1 preparation fails must unwind
the machine registered at dial time; that test drives the
index-allocation failure rather than a crypto failure, which is the
arm this change actually widens, since the allocation now happens with
the connection already attached.
`Node::connections()` yielded the pending connection itself, so its
consumers reached the handshake-phase fields through that value. The
pending connection is being folded into the control machine, and the
fields will move off it, so yield the machine (keyed by its link) and
let each consumer reach what it needs from there.
Membership is unchanged: the iterator still selects machines that
carry a pending connection, which is the predicate `connection_count`
and the stale-connection sweep already use. Every consumer takes the
same value from the same place, one hop further out, and no field
changes carrier here.
The method is now internal to the crate. It yields the control
machine, which is not part of the published surface, and nothing
outside the crate iterates pending connections — operator views of
the handshake phase go through the query surface instead.
`promote_connection` detached the pending connection and then
interleaved reads off that detached value with three separate lookups
of the same control machine. Gather the machine-side fields
(`their_index`, `transport_id`, link stats) in the single `get_mut`
that takes the connection, so the machine is borrowed once.
Behaviour is unchanged. The connection is still taken before anything
is validated, so a rejected promotion leaves the machine with no
pending connection, and the prelude only gathers options: the checks
below it still report the first missing field in the same order
(`our_index`, `their_index`, `transport_id`, `source_addr`).
Link stats move to the prelude, ahead of those checks. The value is
the same either way: they live on the surviving carrier rather than
the detached connection, and nothing between the two points touches
the machine map. The lookup could not fail at the old site either,
since the function had already reached it through a successful lookup
on the same key, so dropping the defaulting arm changes nothing.
Add a test covering the error order and the detach, driving promotion
with connections missing each required field in turn plus every later
one, so an implementation that validated during the prelude would
report the wrong field. Test seeding grows a variant that lets the
caller shape the seed.
Tests built a free-standing PeerConnection, mutated it, and handed it to
Node::add_connection by value. None of those sites survives the removal of
PeerConnection, so converting them afterwards would mean one enormous commit
that cannot be reviewed honestly. Convert them now, while the struct still
exists and the conversion can be validated against a green tree.
Adds a cfg(test) Node::seed_handshake_machine plus a HandshakeSeed builder,
and rewrites make_completed_connection (now seed_completed_connection) and
the twenty inline builders onto it. add_connection keeps its body and loses
its test callers.
The builder's carrier seeding is a verbatim copy of add_connection's: the two
conditional writes for their_index and transport_id, then set_leg, through the
same entry().or_insert_with() so an existing leg-less machine keeps its
constructor-side fields. Nothing else reaches the carrier -- our_index,
source_addr, post-construction started_at and the stored handshake bytes stay
leg-only. Seeding more than that would let these tests observe a carrier
richer than production's and keep passing even if a later production
write-lift were missed.
The Noise exchange now runs on the already-seeded leg rather than before the
hand-over. That is neutral because the only read of expected_identity is
guarded by is_outbound, and no crypto method allocates a session index.
Also adds a compile-time check that PeerAction is Clone + Eq, which is what
keeps a runtime handle from being smuggled into an action payload.
The session index was carried three ways: on the leg, on the peer machine's own
connection-state copy, and on a separate machine-owned shadow field. Populate the
machine's connection copy on the outbound path (it was written only on the inbound
and cross-connection-swap paths before), then delete the shadow field and route its
readers, the decrypt-registration lifecycle, and the rekey cutover through the
single carrier. The shadow and the connection copy diverged only on paths that are
unreachable or dormant in production (a rekey cutover with no newly allocated index,
the swap winner's later reads, and the never-dispatched timeout/disconnect events),
so the merge is byte-identical for every reachable path. Rewrote the outbound-promote
index tests and the dial-time comment for the single-carrier contract.
The connection leg's their_index, transport_id and link_stats duplicated the peer
machine's own connection-state copy. Route every production reader through the
machine copy: promotion, the stale-connection reaper, the outbound msg1 resend, and
the in-use / connecting-path checks. Seed the inbound machine's transport_id from the
msg1 packet (the same source the leg used), matching the outbound dial seed, so the
hard-required promote read never sees a missing transport. link_stats is a
never-mutated zero seed, so its leg accessors are removed outright; the leg
their_index/transport_id getters/setters remain only for the test-only connection
builder. Also delete the write-only next_resend_at_ms field from the connection
state (the resend deadline is carried by the machine-armed retransmit timer, not this
field). Byte-identical.
The connection leg's stored msg1/msg2 handshake-resend buffers duplicated the
peer machine's own connection-state copy. Write the machine copy at the outbound
msg1-prep and the two inbound authorize sites, then source the retransmit
resend-bytes reader, send_stored_msg1, and the pending tier of find_stored_msg2
from the machine. The post-promote active-peer msg2 copy is written from the same
wire bytes, so the pending tier now matching after promotion is value-identical.
Byte-identical resend behavior.
The expected_peer connection-row field read the peer identity off the leg;
source it from the machine's own connection-state copy instead, matching the
started_at/last_activity treatment. The machine carrier holds the same identity
as the leg for every connection that appears in the view: outbound legs seed
both from the dialed identity, and inbound legs never rest in the connections
view (the machine takes the leg at promotion), so their machine copy is
unobserved. The leg accessor stays for the crypto-extraction path and retry
decisions. Byte-identical.
The connection leg's started_at and last_activity duplicated the peer
machine's own connection-state copy. Make the machine copy the sole live
telemetry and timeout carrier: re-stamp it with the leg's provenance when
the leg is attached at handshake start (the msg1-prep clock, not the earlier
dial-time constructor value) and mirror the completion touch, then delete the
leg's last_activity/touch accessors and source the connection-row projection,
show_connections, and the timeout readers from the machine. Byte-identical
for all normal paths.
The connection leg's HandshakeState field duplicated the peer machine's
handshake phase. Delete it and derive the displayed handshake state string
from the machine's PeerState, looked up by link id (mirroring the resend
count). Move the failure signal onto the machine: a send_failed flag that
preserves retransmit eligibility (the machine stays in its handshake phase),
alongside the existing Failed state. The leg's crypto self-gates now guard on
Noise handle presence instead of the deleted phase field, and mark_failed
only drops the handle. Telemetry strings, wire bytes, index allocation, and
the stale-connection reaping are byte-identical for all normal paths.
Johnathan Corgan