Skip to content

test: pin the #579 fix that nothing was pinning, in both directions - #595

Open
Zyrtnin wants to merge 1 commit into
mainfrom
fix/reserve-interval-scan-both-directions
Open

test: pin the #579 fix that nothing was pinning, in both directions#595
Zyrtnin wants to merge 1 commit into
mainfrom
fix/reserve-interval-scan-both-directions

Conversation

@Zyrtnin

@Zyrtnin Zyrtnin commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

A security panel reported that one of the seven #579 sites had no test that fails when reverted. Verified — and worse than reported: switching claim_executor._check_value_cap back to the Bitcoin interval passed the entire suite (10,993 tests), its own module, and the scanner written for exactly this defect class.

Two independent reasons it was invisible

The scanner was line-based. At that site the field and the interval sit on two different lines of one call, so no single line contained both. It is now an AST scan per call, which is line-agnostic — and which also removes the reason the old pattern had to exclude the correct spelling by hand, since rxd_block_interval_s only contains block_interval_s as text, never as an identifier.

No test could express the defect. #579 is inert while every fixture tags the burial in BLOCKS: normalize_to is then the identity and the interval argument is never read, so the wrong argument and the right one produce identical output. No test in that module supplied a SECONDS-tagged burial.

A test whose fixture cannot represent the bug passes for a reason unrelated to the code being correct. So the new one makes the two intervals differ (600 vs 300) and picks an amount between the two ceilings — 2000 photons, legitimate at the real ceiling of 3000, refused at the wrong one of 1500. Equal values hide conflations; this is the smallest fixture in which the two readings disagree. A companion test asserts the fixture still straddles, because if those ceilings ever coincide the main test proves nothing while still passing.

The scan is now symmetric

A Bitcoin reserve converted with the Radiant interval is the same defect, and was covered by nothing. I found that by accident: a restore during this verification rewrote btc_claim_reorg_depth to use rxd_block_interval_s, and the whole suite passed.

The field-to-chain map is now read off MarginPolicy by prefix instead of a hand-typed tuple of two names. A hand-kept list of what a guard covers is the same artifact that produced the bug being guarded — #511 added rxd_claim_inclusion and it was left off a validation list, fail-open. Deriving it means a new rxd_/btc_ reserve is covered the day it is added.

Verification

Both directions planted, each caught (3 failures and 1). The restores were confirmed with git diff rather than by eye — the bad restore mentioned above was a string replace that silently matched an earlier, correct call site, leaving the planted defect live while reporting success. That is a failure mode worth naming: a plant/restore cycle can report success and leave the tree wrong in two places at once.

Full suite: 10,997 passed, 192 skipped, 1 xfailed. No production code changes — this is coverage for a fix already on main.

🤖 Generated with Claude Code

A security panel reported that one of the seven #579 sites had no test that
fails when reverted. Verified, and it was worse than reported: switching
claim_executor._check_value_cap back to the Bitcoin interval passed the ENTIRE
suite - 10,993 tests, its own module, and the scanner written for exactly this
defect class.

TWO INDEPENDENT REASONS IT WAS INVISIBLE.

The scanner was line-based. At that site the field and the interval sit on two
different lines of one call, so no single line contained both. It is now an AST
scan per CALL, which is line-agnostic - and which also removes the reason the
old pattern had to exclude the correct spelling by hand, since
`rxd_block_interval_s` only CONTAINS `block_interval_s` as text, never as an
identifier.

And no test could express the defect. #579 is inert while every fixture tags the
burial in BLOCKS: normalize_to is then the identity and the interval argument is
never read, so the wrong argument and the right one produce identical output. No
test in that module supplied a SECONDS-tagged burial. A test whose fixture
cannot represent the bug passes for a reason unrelated to the code being
correct, which is why the new one makes the two intervals DIFFER (600 vs 300)
and picks an amount BETWEEN the two ceilings - 2000 photons, legitimate at the
real ceiling of 3000, refused at the wrong one of 1500. Equal values hide
conflations; this is the smallest fixture in which the two readings disagree.
A companion test asserts the fixture itself still straddles, because if those
ceilings ever coincide the main test proves nothing while still passing.

THE SCAN IS NOW SYMMETRIC. A Bitcoin reserve converted with the RADIANT interval
is the same defect and was covered by nothing. I found that by accident: a
restore during this verification rewrote btc_claim_reorg_depth to use
rxd_block_interval_s, and the whole suite passed. Both directions are now
scanned.

The field-to-chain map is read off MarginPolicy by prefix instead of a
hand-typed tuple of two names. A hand-kept list of what a guard covers is the
same artifact that produced the bug being guarded - #511 added
rxd_claim_inclusion and it was left off a validation list, fail-open. Deriving
it means a new rxd_/btc_ reserve is covered the day it is added.

Verified by planting both directions, each caught (3 failures and 1). The
restores were confirmed with git diff rather than by eye: the earlier bad
restore above was a string replace that silently matched an EARLIER, correct
call site, leaving the planted defect live while reporting success.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant