Repository navigation
Kill missed mutants from the 2026-10-16 nightly - #1041
Conversation
addr, cfilter, cfheaders, cfcheckpt, reject, and alert decode as those messages. A zero fee and the max-money fee are accepted; a negative fee and one sat past the cap are not. Sixty-four invs stay cheap to encode, and a trailing verack byte stays unknown. Made-with: Cursor
A caller that asks for 10001 blocks gets the cap error before any block is mined. Made-with: Cursor
Drop compares that fold to the same result. A checkpoint replay does not open a challenger, a rewind keeps a base fork and drops a popped one, and an unlinked refill is not a competing chain. Made-with: Cursor
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
cache_body_count is what tip: perf prints as cache_bodies. A genesis branch of two blocks leaves both in the body window, so that count is 2. Made-with: Cursor
![rearden-grok[bot]](https://avatars.githubusercontent.com/u/730881?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
The chain-hub and header-walk edits drop comparisons that return the same value for every consistent input (equal chain work, a header tip that already has a header row, a chain-work prefix that truncate already shortens, a challenger fork that is already the rewind tip or base, and an anchor whose height is the header being priced). The new tests pin the surviving boundaries on accept, submitheader, chaintips, rewind, and header replies, and the 10000 generate cap is nblocks > 10_000 with consensus: nblocks too large (max 10000). The one behavior change that is not equivalent is the confirmed-set seed: it now reads every height before inserting any hash, so a disconnect during that scan is undone and has_block stays false for the rest of the chain until the scan finishes.
Issue counts by severity
- bugs: 1
- suggestions: 6
- nits: 0
| g.insert(hash); | ||
| } | ||
| } | ||
| for chunk in hashes.chunks(4096) { |
There was a problem hiding this comment.
[bug] spawn_confirmed_seed used to insert each 4096-hash batch as header_at_height walked the chain. It now keeps every hash in hashes and only then takes confirmed and inserts. Two reachable effects follow. Until that second loop runs, has_block is still only the immediate seed (genesis, tip-1, tip), so assign/getdata can request bodies the store already has for a chain taller than two blocks. If disconnect_to removes a hash in that window (reorg, invalidateblock, rewind), the later insert puts it back. confirmed is insert-only across reorgs, and a stale hit permanently suppresses getdata.
Suggestion: Insert each chunk inside the height walk, as before, so a hash removed after it was published is not replayed from a stale snapshot. Re-check the height against the live tip before insert if the full-vector shape has to stay.
There was a problem hiding this comment.
Agreed. The seed was keeping every hash until the walk finished, then inserting them all. Until that insert, has_block was only genesis, the block under the tip, and the tip, so a tall chain could be asked for bodies it already had. A rewind in that window could also put a disconnected block back, and has_block would keep suppressing getdata for it.
The fill now copies the height index that open already built, while it holds the same tip-commit lock a rewind holds while it drops hashes above the new tip. A block removed before the copy is not in that index, so it is not inserted again. The test builds a short chain, rewinds to genesis, runs that copy, and checks the old tip stays gone while genesis stays.
I did not bring back the old flush-every-4096 header walk. Open has already read the headers once. Copying that index makes has_block true for the whole chain without reading them again.
| } | ||
| } | ||
| // Equal work is the floor. A comparison here is the same value on | ||
| // both sides, so the missed `>` / `>=` mutants cannot be told apart. |
There was a problem hiding this comment.
[suggestion] The comment exists to narrate a missed > / >= mutant ("cannot be told apart") rather than an invariant a later edit needs. std::cmp::max already says equal work stays the floor.
Suggestion: Delete the comment, or replace it with one line only if max-versus-a-strict-compare is actually subtle here.
There was a problem hiding this comment.
Agreed. I deleted the comment. std::cmp::max already keeps equal work as the floor.
| return Some(*parent); | ||
| } | ||
| // Equality is handled below. Checking `>` first makes `>=` return | ||
| // nothing for the parent itself. |
There was a problem hiding this comment.
[suggestion] "Checking > first makes >= return nothing for the parent itself" narrates the mutant. The equality arm below is the behavior; the comment does not say why a caller would care.
Suggestion: Drop the comment. The want > / want < / parent return is already the pin, and header_along_off_tip is covered by the regtest walk.
There was a problem hiding this comment.
Agreed. I deleted the comment. When the wanted height is the parent, the function returns that parent. The too-high and too-low arms are the other two cases.
| return Ok(()); | ||
| } | ||
| // `truncate` is a no-op when the prefix is already at or below the tip, | ||
| // so a longer-than-tip cache and an exact one share this line. |
There was a problem hiding this comment.
[suggestion] "truncate is a no-op when the prefix is already at or below the tip, so a longer-than-tip cache and an exact one share this line" restates Vec::truncate and explains why two mutant operands collapsed. It does not record a constraint the next edit would otherwise miss.
Suggestion: Delete the comment.
There was a problem hiding this comment.
Agreed. I deleted the comment. truncate is the whole rule on that line.
| } | ||
|
|
||
| /// Work floor, header-only window, submitheader parents, chaintips rank, | ||
| /// and a reopened datadir. Each assertion is the side of a comparison the |
There was a problem hiding this comment.
[suggestion] The doc says this test covers "a reopened datadir" and that "each assertion is the side of a comparison the other operator gets wrong." Nothing in chain_edges_pin_work_headers_and_reopen reopens a datadir (that is reopened_chain_seeds_heights_between_genesis_and_the_tip), and the second sentence is mutant narration. The same narration is on latest_run_chain_surfaces ("operators the last mutants run left open").
Suggestion: Describe the observable edges the test actually hits (work floor, submitheader parent, chaintips rank, 288-header window) and drop the mutant wording.
There was a problem hiding this comment.
Agreed. The note on chain_edges_pin_work_headers_and_reopen now names the work floor, submitheader parents, chaintips rank, and the 288-header window. It no longer says this test reopens a datadir. That case stays in reopened_chain_seeds_heights_between_genesis_and_the_tip.
I also rewrote the latest_run_chain_surfaces note. It now lists the empty-store branch, a block the node already has, a held child that finishes the accept, rewind bounds, a stale body cache, and the body count on the tip line.
| frame(padded(name), payload).decode().payload().clone() | ||
| } | ||
|
|
||
| /// Peer frames for the commands whose decode arm a missed mutant can delete, |
There was a problem hiding this comment.
[suggestion] "whose decode arm a missed mutant can delete" and the verack note that "replacing that guard with true would return Verack" document the mutant, not the wire rule. The assertions already state the peer-visible result (unknown command, feefilter bounds, 64 invs not heavy).
Suggestion: Say which frames must stay distinct (Verack versus trailing payload, fee range, inv CPU cutoff) and delete the mutant clauses.
There was a problem hiding this comment.
Agreed. The test note now states the wire rules: addr, compact filters, reject, and alert stay distinct from an unknown command; feefilter accepts 0 through max money; a trailing byte is not verack; sixty-four invs stay off the heavy path. The assertion comment is just "A trailing byte is not verack."
| } | ||
| } | ||
| // `clear_path_above` already dropped path slots above `keep`. The | ||
| // ordered filter is what still removes a hash that never got a height. |
There was a problem hiding this comment.
[suggestion] "The ordered filter is what still removes a hash that never got a height" is not what the filter does. A queued hash with no hash_height is kept unless it is the dead tip (None => dead != Some(*h)). drop_queue_above drops an unheighted hash only when it is absent from keep_hashes. Reading the comment as the rule would drop the hash two_abandoned_candidates_both_stay_dead requires to stay queued.
Suggestion: Say that clear_path_above already removed slots above keep, and that an unheighted queued hash stays unless it is the abandoned tip.
There was a problem hiding this comment.
Agreed. The old sentence was wrong. An ordered hash with no height is kept unless it is the abandoned tip. The comment now says clear_path_above already dropped the path slots above keep, and that an unheighted hash stays unless it is that abandoned tip.
The background seed publishes a height only while that hash is still the header there, and it does so under the same tip-commit lock a reorg uses to remove the hash. Comments that described missed mutants now describe the behavior. Made-with: Cursor
Open already reads every header to build that index. The seed copies it under the tip-commit lock instead of reading each header again. Made-with: Cursor
2 participants
Summary
Kills the missed mutants from cargo-mutants run 38854276078 (head
4055146e). One pull request.generateaccepts ten thousand blocks and refuses one more before mining.No
#[mutants::skip].CHANGELOG.mdis unchanged. The unreleased note ischangelog.d/overnight-38854276078.md.Test plan
rbitcoin-nettests for the new pins, including the ten-thousand-block generate capcargo clippy --workspace --all-targets -- -D warningsbefore rebase, andcargo clippy -p rbitcoin-net --all-targets -- -D warningsafter rebase onto current mastercargo fmt --all -- --check,./scripts/ast-grep.sh,./scripts/ci-os-smoke.shqc,test,windows,macos,coverage