fix(rewards-claim): derive both cadences from one raw config value and inject the clock - #617
Conversation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…clock seam - GateCadenceSeconds / FeeWindowCadenceSeconds newtypes on ClaimEngine::with_persisted_fee_window so transposing the two cadence values at the driver.rs call site is a compile error, not a silent money defect (#3336). - run_claim_driver_in_with_clock: the production body with the clock parameterized instead of hardcoded to unix_now_seconds, so the timing behaviour is testable under tokio::time::advance. run_claim_driver_in stays a thin wrapper over it passing the real clock. Refs #3336 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…_in_with_clock Adds the_production_body_tracks_the_clamped_gate_and_the_raw_fee_window, proving the F2 gate-vs-fee-window distinction through the actual production body (run_claim_driver_in_with_clock) rather than a hand-assembled drive() call, so a transposition of the two cadence arguments at with_persisted_fee_window's call site (#3336) is caught end-to-end. Refs #3336 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
36ef55f to
46d803a
Compare
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
Independent correctness review at head 46d803a3f081b652f569d383454380edef68ec64.
Verdict: PASS
Answers to the five required questions:
-
Does the newtype make positional transposition impossible? Yes for the case the ticket describes (two adjacent bare
u64s at the call site) —GateCadenceSecondsandFeeWindowCadenceSecondsare distinct types, so passing one where the other is expected is a compile error; a caller cannot silently swap the positions anymore. It does not make a value swap impossible —GateCadenceSeconds(cfg.cadence_seconds)/FeeWindowCadenceSeconds(cadence_seconds)type-checks fine (both tuple fields arepub, both wrapu64). The PR's own doc comments on both types say this explicitly, so this isn't a hidden gap — it's a stated limitation. -
Is there a behavioural test that fails if the values are swapped inside the correctly-typed wrappers? Yes —
the_production_body_tracks_the_clamped_gate_and_the_raw_fee_window(driver.rs, new in this diff) drives the actual production call site (run_claim_driver_in_with_clock→ the onewith_persisted_fee_windowcall indriver.rs) through 3 ticks and asserts both halves of the defect: (a) the gate must open on the clamped cadence (cycle count == 2 at tick 2, not stalled — this is the #3306 no-op-gate regression check), and (b) the fee window must NOT roll until the raw cadence has elapsed (tick 2 must show the samefee_window_start_unixas tick 1). Reverting either wrapped value to the other's source (GateCadenceSeconds(cfg.cadence_seconds)orFeeWindowCadenceSeconds(cadence_seconds)) flips one of these two assertions. This is a real, non-vacuous proof test — it exercises production code, not a hand-rolled harness only.There's also a pre-existing hand-assembled version of the same property (
the_gate_tracks_the_clamped_cadence_while_the_fee_window_tracks_the_raw_one, referenced in the new test's doc comment) that predates this PR and still exists; the new test's value is that it goes through the actual driver call site instead of reconstructingClaimEngineby hand. -
Is
run_claim_driver_instill a faithful thin wrapper? Yes. Diff showsrun_claim_driver_innow does nothing but callrun_claim_driver_in_with_clock(..., unix_now_seconds)— the exact same real-clock function it inlined before. No behavioural change for the only production caller (run_claim_driver). -
Other call sites of
with_persisted_fee_windowsilently baked to the wrong value? Checked every call site in the diff (all in driver.rs and engine.rs test modules) plus a repo-wide code search (with_persisted_fee_windowmatches only these two files) — all updated consistently toGateCadenceSeconds(cadence)/FeeWindowCadenceSeconds(cadence), and in every test both values happen to be the same constant (CADENCE_SECONDS) so none of those tests could have distinguished a swap anyway — that's fine since the new dedicated test does distinguish it. -
Anything beyond the type change alters claim/fee behaviour? No. The only semantic addition is the clock-injection seam (
run_claim_driver_in_with_clock) used solely to make the new test possible; production path is untouched (see Q3). No other logic changed.
CI: all required checks green (build, clippy, rustfmt, CodeQL, Analyze x2, Test+coverage, packaging). No pending/stale contexts.
Threads: none opened — no defects found; nothing to gate on. 0 open, 0 resolved (none needed).
KG: NONE — this PR closes a previously-logged defect (#3336) with the remedy already specified by the ticket; no new pattern surfaced.
Gate leg 3 of 3 (adversarial) — CHANGES-REQUIREDHead judged: The finding: the new test is green under the very defect it names
— is false: under the transposition that tick still records, and the count is still 2. Worse, the window half is masked too. Trace the test with the value-transposed call site (
The whole test passes under the transposition. The gate's early return is exactly what hides the window defect from it. Checking the other wrong configurations: clamped-for-both is caught (rolls at tick 2); raw-for-both also passes undetected. The test catches one of three wrong wirings and not the one it was written for. Answers to the five questions
What would clear this legEither (a) re-shape to the single named-field cadence value (preferred — closes the value swap structurally, and is subtractive), or (b) keep the newtypes and make the test actually falsify the transposition: assert Verdict: CHANGES-REQUIRED at |
loop-security verdict — GATE LEG 2/3PASS Head SHA judged: Q1 — can the guard be forged?Partially, but not in a way that reintroduces the #3336 defect today. What it does guarantee, and is the only thing #3336 actually needed: the two arguments to I audited every call site in the diff ( Q2 — worst raw
|
…Cadences value
The positional-newtype shape (GateCadenceSeconds/FeeWindowCadenceSeconds as two
adjacent with_persisted_fee_window arguments) closed the positional swap but left
the value swap open -- GateCadenceSeconds(raw)/FeeWindowCadenceSeconds(clamped)
still compiled and still passed the existing test (dig_ecosystem#3336 rework).
Bundle both cadences into one ClaimCadences { gate_clamped, fee_window_raw }
value instead: there is no longer a pair of adjacent arguments for a swap to
target at the call site. Every production and test call site updated.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ept by its clamp DIG-Network/dig_ecosystem#3336: `ClaimCadences { gate_clamped: u64, fee_window_raw: u64 }` closed the positional swap between the two cadence values but left the value swap open -- both fields were plain u64, so nothing stopped a caller writing the RAW configured cadence into `gate_clamped` and the CLAMPED one into `fee_window_raw`. `ClampedGateCadence`'s inner field is now private with `ClampedGateCadence::clamp(FeeWindowCadenceSeconds) -> Self` as its only constructor, so a raw value has no path into the gate slot without going through the clamp. `FeeWindowCadenceSeconds` wraps the raw side so the two are no longer interchangeable plain integers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
CHANGES-REQUIRED — head 68f4101d5e9a1d8f5ae4cef94450acea96360adb
Re-gate leg 1 of 3 (correctness), fresh context, read from GitHub at this head only. Prior verdicts on 46d803a3 treated as void.
Verdict in one line
The reshape genuinely closes two of the three swap hazards structurally, and the third — the one that costs money — is closable in this same PR with a provably behaviour-identical one-value collapse. On a money surface guarded by exactly one test, that residual is worth closing rather than documenting.
What the diff actually proves (state of each hazard at this head)
| hazard | status |
|---|---|
| positional transposition at driver.rs:511-517 | CLOSED structurally — named fields + distinct types; a swap is E0308. |
| RAW value reaching the gate | CLOSED structurally — ClampedGateCadence::clamp re-applies min(CLAIM_SCHEDULE_SECONDS_MAX), so gate_clamped is clamped whichever source is fed. This is stronger than the PR body claims. |
CLAMPED value reaching fee_window_raw |
OPEN — compiles; halves the window; guarded by one test. See the inline thread at driver.rs:515. |
The five questions, answered
1. Is the_production_body_tracks_the_clamped_gate_and_the_raw_fee_window above the gate decision, and is the pairing sound?
Yes, it is above the decision: it drives run_claim_driver_in_with_clock, the real production body, drive calls now() exactly once per iteration (driver.rs:168), the injected clock advances one clamped interval per tick, and the engine's gate at engine.rs:412-418 and window-roll at engine.rs:421-429 are both reached through it. The arithmetic checks out: tick 2 gives now - last_completed = 2_678_400 >= gate 2_678_400 (gate opens) and now - window_start = 2_678_400 < 5_184_000 (window holds); tick 3 gives 5_356_800 >= 5_184_000 (rolls).
The pairing is sound at this head but weaker than its own comment states: assert_ne!(state, CadenceNotElapsed) is satisfied by any other state, including PersistedStateCorrupt, whose early return (engine.rs:389-392) also sits above the roll block. Nothing makes tick 2 corrupt or future-dated today, so it holds — but it is one added fail-closed guard away from vacuous. Inline thread at driver.rs:1473.
2. Other call sites of with_persisted_fee_window with a wrong source baked in?
No. Repo-wide the symbol appears only in rewards_claim/driver.rs and rewards_claim/engine.rs. One production call site (driver.rs:511). Every other call is a test; driver.rs:1092 and driver.rs:1405+ pass effective/configured correctly and deliberately; all engine.rs tests pass CADENCE_SECONDS for both, which is intentional for engine-level properties and is not a wrong-source bake-in.
3. Does ClampedGateCadence::clamp clamp correctly, and is the field genuinely private?
Yes on both. clamp is raw.0.min(CLAIM_SCHEDULE_SECONDS_MAX) against the same 31 * 24 * 60 * 60 bound sanitized_schedule uses (driver.rs:394-407); CLAIM_SCHEDULE_SECONDS_MAX was widened to pub(crate) only, not pub. The field is a private tuple field; derives are Clone, Copy, Debug, PartialEq, Eq only — no Default, no From, no Deref, no AsRef, no serde. seconds() is a private reader. clamp is the only constructor, and grep confirms no ClampedGateCadence(...) literal anywhere outside clamp itself — including in the child mod tests, which could bypass it (child modules see private fields) but does not.
4. Anything changing claim/fee behaviour beyond the type reshape?
No. run_claim_driver_in becomes a thin wrapper passing unix_now_seconds into run_claim_driver_in_with_clock; the production body is otherwise character-identical. The extra clamp applied to an already-sanitized cadence_seconds is a no-op. The .max(CLAIM_CADENCE_FLOOR_SECONDS) flooring on both fields is unchanged. The only visibility widening is CLAIM_SCHEDULE_SECONDS_MAX to pub(crate), needed by clamp.
5. Is the PR body accurate relative to what the diff proves?
Partly, and it now understates the diff — it still describes the pre-reshape framing ("two adjacent bare u64 args"), says "Work in progress", and does not record that the gate half is now structurally closed or that the window half is not. Two smaller issues: the title still says "newtype ... args", which no longer describes the shape (one struct, one of whose fields is constructor-pinned); and Fixes #3336 resolves against dig-node, not dig_ecosystem — a cross-repo close needs DIG-Network/dig_ecosystem#3336, otherwise it is either a no-op or points at the wrong issue. Refs #3336 below it has the same ambiguity. Non-blocking, but fix before ready-for-review.
Test-vacuity gate
- Property: "the production call site threads the CLAMPED cadence to the gate and the RAW cadence to the fee window."
- Nearest wrong implementation:
fee_window_raw: FeeWindowCadenceSeconds(cadence_seconds). Input that fails it: a 60-day config (5_184_000) — the window rolls at tick 2 instead of tick 3. Onlythe_production_body_tracks_the_clamped_gate_and_the_raw_fee_windowgoes red; the sibling money test stays green. Non-vacuous, but a guard of ONE. - Second nearest wrong implementation:
gate_clampedbuilt from the raw source — no longer producible,clampneutralises it. Correctly, no test is needed for it.
What I did NOT run
No build, no cargo test, no cargo clippy, no coverage run, and no mutation of my own — read-only from git objects at this head, per brief; I did not read or touch D:\worktrees\dig-node-3336. I did not re-derive the mutation measurements in the brief; I sanity-checked them against the code and they reconcile (including the Some(5356800) vs Some(2678400) 2x figure). Someone must confirm the suite is green at this head before merge — note this PR is still a DRAFT and its body says DO NOT MERGE.
Threads
2 inline threads posted, both blocking-or-above-note, both OPEN by design: driver.rs:515 (blocking) and driver.rs:1473 (medium). No pre-existing threads existed; none resolved. Re-review on the next head.
ADVERSARIAL GATE (leg 3/3) — CHANGES-REQUIRED at
|
loop-security (L2) — RE-GATE leg 2 of 3 · PASSHead audited: No LIVE vulnerability. One defence-in-depth finding, ticket-worthy, not a gate. Q1 — Can the guard be forged? No, for the gate slot. Verified structurally, not by comment.
The stronger property, which the PR's own doc understates: Residual, named honestly: privacy in Rust is module-scoped, so a future edit inside
Q2 — Worst raw
|
…value Closes the remaining half of the #3336 transposition hole structurally rather than by test. `ClaimCadences` now has private fields and a single constructor, `ClaimCadences::from_raw`, which derives `gate_clamped` from the same raw value it stores as `fee_window_raw`. The production call site passes ONE value, so there is no longer a pair to transpose or a `pub` field to write the wrong local into. Behaviour-identical: `CLAIM_CADENCE_FLOOR_SECONDS` is 60 and `config.rs` floors any smaller configured cadence at load, so `sanitized_schedule`'s zero branch is unreachable from this path and its output equals `min(cadence, MAX)` -- exactly what the clamp computes. Also renames `FeeWindowCadenceSeconds` to `RawConfiguredCadence` (it is the clamp's input type, not only the window's), and replaces the tick-1/tick-2 `assert_ne!(state, CadenceNotElapsed)` exclusions with exact-state assertions -- the exclusion was equally satisfied by PersistedStateCorrupt and ChainSourceUnavailable, whose early returns also sit above the window-roll block, so it would have gone vacuous the moment either fired. Refs DIG-Network/dig_ecosystem#3336 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…er assertions Convert literal backslash-n escapes with trailing spaces back to proper Rust string line continuations (trailing backslash at end of line). The strings render as single sentences without artificial newlines or multi-space runs. Fixes continuation_guard. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…arded half The #3336 doc comments asserted a type-level guarantee the code does not deliver. `ClaimCadences::from_raw` does close the pairing mutation -- the gate and the fee window cannot disagree with each other, because `from_raw` derives `gate_clamped` from the same `RawConfiguredCadence` it stores as `fee_window_raw`, and it is the only constructor. It does NOT close the value mutation: `from_raw(RawConfiguredCadence(cadence_seconds))`, the already-clamped local instead of `cfg.cadence_seconds`, has the same type and compiles. Measured consequence: the fee window halves -- the tick-2 assertion fails with left Some(5356800), right Some(2678400). That mutation is caught by exactly one test, `the_production_body_tracks_the_clamped_gate_and_the_raw_fee_window`. The other money test stays green under it, because it hand-assembles `drive` and never traverses the production call site. The old docs told the next reader the value swap was type-closed -- the exact sentence that would be cited to delete the one guarding test. Also corrects the stale "transposing the two arguments" description: the call takes ONE argument, so that mutation cannot be written at all; and drops the claim that the surviving mutation also breaks the gate (clamping an already-clamped value is the identity, so it does not). Docs only -- no behaviour, no type, and no assertion changed. Refs DIG-Network/dig_ecosystem#3336 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adversarial gate leg 3/3 — CHANGES-REQUIRED @
|
loop-security (re-gate leg 2/3) — PASSHead audited: Scope: the only two files in the PR at head — Prior PASSes on this PR (incl. 1. Can the guard be forged? — No in-crate path yields a
|
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
PASS — correctness re-gate leg 1/3 at 97450a0b52b10020cb3ce3a0194a8ed31492652c
Judged this head only. Read from git objects via gh api at the SHA; no worktree touched; nothing run locally (see "what I did not run").
The five questions
1. Is the derivation behaviour-identical to the code it replaced? YES, for every input production can produce — verified independently.
config.rs:52—CLAIM_CADENCE_FLOOR_SECONDS = 60.RewardsClaimConfig::load_fromhas exactly three exits (config.rs:213-260): missing file →default()(cadence_seconds = CLAIM_CADENCE_SECONDS_DEFAULT = 86_400); unreadable / unparsable / F14 →poisoned()(..Self::default(), so also 86_400); parsed → floored up to 60 atconfig.rs:232-240. So every value ofcfg.cadence_secondsreachingrun_claim_driver_in_with_clockis>= 60.sanitized_schedule(driver.rs:364-411) isif c == 0 { DEFAULT } else if c > MAX { MAX } else { c }. Forc >= 1that is exactlyc.min(MAX), which is exactlyClampedGateCadence::clamp(engine.rs:88-90).- The engine-side
.max(CLAIM_CADENCE_FLOOR_SECONDS)on both fields is unchanged from the old two-u64body.
Conclusion: the gate value the engine now receives (cfg.cadence_seconds.min(MAX).max(60)) is identical to what the old call passed (sanitized_schedule(..).0.max(60)) for all reachable inputs. The zero branch is unreachable, and even if the floor were removed the divergence (gate 60 vs 86_400, while drive still sleeps on the substituted 86_400) fails in the safe direction — the gate becomes strictly more permissive, so it cannot restore the #3306 silent-non-claiming defect, and fee-window sizing is unchanged in that case (0.max(60) == 60 under both shapes). Not a behaviour change on the money path. Non-gating note posted at engine.rs:87 about the doc sentence that asserts the two transformations are "the same bound" without naming that precondition.
2. RawConfiguredCadence.0 is pub(crate) — does that reopen anything? NO.
What private ClaimCadences fields close is the pairing: gate and window sizing off two different numbers. from_raw is the only constructor (engine.rs:57-66); ClaimCadences derives only Clone, Copy, Debug, PartialEq, Eq (no Default, no Deserialize), has no setters, and no field access outside engine.rs; ClampedGateCadence::clamp is now module-private and its only caller is from_raw. So no in-crate path can produce a ClaimCadences whose gate is not the clamp of its own window value. A pub(crate) raw field lets in-crate code label an arbitrary u64 as raw — precisely the hole the doc says is test-guarded only, not a new one. Repo-wide, with_persisted_fee_window is named only in driver.rs and engine.rs, so the pub → pub(crate) narrowing breaks no caller (mod engine is private to rewards_claim, and ClaimCadences is not re-exported in mod.rs:57-67).
3. Are the rewritten docs TRUE? Yes, with one over-broad sentence and one stale comment, both non-gating.
Checked every surviving guarantee claim:
- "
from_rawis the only constructor, both fields are private (so a struct literal is not an alternative path from outside this module)" — true, and correctly scoped. - "derives
gate_clampedby clamping the veryRawConfiguredCadenceit stores asfee_window_raw" — true. - "NOT type-enforced … WHICH
u64the call site labels raw … Exactly ONE test catches that … Do not delete it" — true and confirmed: of the four tests that reach the production body, three go throughrun_claim_driver_in(real wall clock, so they cannot observe per-cyclenow()values) and onlythe_production_body_tracks_the_clamped_gate_and_the_raw_fee_windowinjects a clock. This is the strongest sentence in the diff and it points at the test rather than licensing its removal. - "the gate is NOT affected (clamping an already-clamped value is the identity)" — true.
run_claim_driver_in_with_clock's seam rationale (tokio::time::advancecannot moveSystemTime::now(); the wrapper passes the real clock so no production caller changes) — true;driver.rs:442-449is a pure forwarding call.- Over-broad (non-gating, posted inline):
engine.rs:87— "the same boundsanitized_scheduleapplies at the config read, so the gate tracks the schedule the driver really sleeps on".clampreproduces onlysanitized_schedule's clamp arm, not its zero-substitution arm; the sentence is true only becauseconfig.rsfloors the cadence at 60 first. - Stale (non-gating, posted inline):
driver.rs:502— the F2 comment still describes "two DIFFERENT cadence values" and namescadence_secondsas the gate argument, but the call below passes one argument and the clamped local no longer reaches it. Semantics correct, mechanism superseded.
No surviving sentence claims a type guarantees the raw-label choice, and none could be cited to justify deleting the single guarding test.
4. Do the two rewritten assertion messages still describe their assertions? Yes — and the premise that no assertion logic changed is wrong: it changed, in the strengthening direction.
In the ungated window (68f4101d..97450a0b) four assertions flipped from assert_ne!(state, CadenceNotElapsed) to assert_eq!(state, Nominal) — two in the_gate_tracks_the_clamped_cadence_while_the_fee_window_tracks_the_raw_one, two in the new production-body test. Nominal is a strict subset of != CadenceNotElapsed, so each is strictly stronger than before; it removes exactly the vacuity the prior round's MEDIUM thread named (the exclusion was equally satisfied by PersistedStateCorrupt / ChainSourceUnavailable). The new messages say precisely that, and the after_tick_2 message correctly states it is meaningful only paired with the state assertion above it. No assertion was weakened; no assert_eq compares something other than what its message names.
5. Anything altering claim/fee behaviour beyond the reshape and docs? No.
The whole diff is two files. Behaviour-bearing changes: the one-argument call site (Q1 — identical), CLAIM_SCHEDULE_SECONDS_MAX const → pub(crate) const (visibility only), with_persisted_fee_window pub → pub(crate) (Q2 — no caller), and the new run_claim_driver_in_with_clock seam whose production wrapper forwards unix_now_seconds unchanged. Everything else is test call-site mechanics, the four strengthened assertions, the new test, and doc prose. No change to run_cycle, the gate comparison, the window-roll condition, fee accounting, or persistence.
Test-vacuity gate on the one guarding test
Property: at the production call site, the fee window is sized by the RAW configured cadence while the gate is sized by the CLAMPED one. Nearest wrong implementation that still compiles: from_raw(RawConfiguredCadence(cadence_seconds)). I re-derived the arithmetic from source rather than taking the measurement on trust: the window opens at t = 2_678_400 (tick 1); under the mutation fee_window_seconds = 2_678_400, so at tick 2 (now = 5_356_800) now - start = 2_678_400 >= 2_678_400 → the window rolls and after_tick_2 reads Some(5_356_800) against after_tick_1 = Some(2_678_400) — exactly the reported failure. Under correct code 2_678_400 < 5_184_000 → no roll at tick 2, and at tick 3 5_356_800 >= 5_184_000 → it rolls, so the tick-3 assert_ne also distinguishes "never rolls at all". drive calls now() exactly once per iteration (driver.rs:167-169), immediately after the sleep, so the injected one-interval-per-call clock models production faithfully; write_config sets jitter_seconds: 0 through the real save_to, so the scheduler's interval equals effective_cadence and the tick counts are not coincidental.
dig-constants
No new cross-repo constant. CLAIM_CADENCE_FLOOR_SECONDS and CLAIM_SCHEDULE_SECONDS_MAX are node-local scheduling bounds no second repo must match; DIG_ASSET_ID is already consumed from dig_mirror_coin. Nothing here belongs in dig-constants, and nothing here should be using it.
Threads
Both prior-round threads (the pub-field value-swap BLOCKER and the MEDIUM vacuity note) are addressed at this head and resolved with the fixing SHA. Two non-gating doc notes posted and resolved by me. Zero open threads.
What I did not run
Nothing: no build, no test, no clippy, no fmt. I did not read or write D:\worktrees\dig-node-3336. The COUNTS I rely on are the lane's (cargo test -p dig-node-service --lib rewards_claim → 92 passed / 0 failed; fmt --check 0; clippy -D warnings 0) plus the lane's revert-proof measurement, whose arithmetic I re-derived from source rather than re-executing. CI on this SHA is the independent leg for the greens.
…e clamp precondition Doc comments only; no code, assertion, type or visibility changed. The test count is unchanged (92 passed, 0 failed). - engine.rs: delete the `with_persisted_fee_window` "F2 (money): takes TWO cadence values, deliberately not one" block. The method takes ONE `ClaimCadences`; those two names are private fields, not parameters, and "deliberately not one" instructed the next author to restore the two-`u64` signature this PR makes unwritable -- twenty lines above the #3336 section that contradicts it. - engine.rs: `ClampedGateCadence::clamp` no longer claims flat equivalence with `sanitized_schedule`. It reproduces only that function's clamp arm, not its zero-substitution arm; they agree because a zero cannot reach it, and that precondition is now named. - driver.rs: the surviving production-call-site comment describes the one-argument call it sits above, and names the single test that catches passing the clamped local instead of the raw config value. - driver.rs: the sibling money test now records that it stays GREEN under that mutation, so its doc cannot be cited to delete the test that does catch it. Refs DIG-Network/dig_ecosystem#3336 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adversarial gate leg 3/3 — PASS @
|
… exit count Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Summary
ClaimEngine::with_persisted_fee_windowneeds two cadences that must differ: the schedule-CLAMPED one the restart-safety gate is measured against, and the RAW configured one that sizes the persisted fee-budget window. Getting them the wrong way round is a money defect — the clamped value in the window slot halves the window, doubling the number of fee-budget windows (and so the fee ceiling) a long-cadence operator configured.This PR replaces the two bare
u64arguments with oneClaimCadencesderived from a singleRawConfiguredCadence, and adds an injectable-clock seam so the production body itself can be tested.What is type-enforced, and what is only test-guarded
Both halves are not closed the same way. Measured on this branch:
TYPE-ENFORCED (cannot compile):
ClaimCadences::from_rawis the only constructor, both fields are private, and it derivesgate_clampedby clamping the veryRawConfiguredCadenceit stores asfee_window_raw. There is no pairing in which the window sizes off one number and the gate off another.TEST-GUARDED ONLY (compiles fine):
u64the call site labels raw.ClaimCadences::from_raw(RawConfiguredCadence(cadence_seconds))— the already-clamped local instead ofcfg.cadence_seconds— has the same type and compiles.left: Some(5356800),right: Some(2678400)— exactly 2x the fee-budget windows the operator sized. The gate is unaffected (clamping an already-clamped value is the identity).the_production_body_tracks_the_clamped_gate_and_the_raw_fee_window(fails atdriver.rs:1467). The other money test,the_gate_tracks_the_clamped_cadence_while_the_fee_window_tracks_the_raw_one, stays GREEN under this mutation, because it hand-assemblesdriveand never traverses the production call site — its own doc comment says so.A reader must not conclude that both halves are type-enforced. That misreading is how the value mutation gets reopened. The positional transposition is structurally gone -- the call takes one argument -- so what is left to get wrong is passing the clamped local instead of the raw config value, which compiles and is caught by exactly one named test (
the_production_body_tracks_the_clamped_gate_and_the_raw_fee_window). Every doc comment on the path now says that, and none of them claims a type stands behind it.Changes
engine.rs:RawConfiguredCadence(the one input),ClampedGateCadence(private field,clampthe only producer),ClaimCadences::from_raw(the only constructor).with_persisted_fee_windowtakes oneClaimCadencesand stayspub(crate)—ClaimCadencesis crate-private, so apubmethod taking it would be uncallable from outside anyway, and no out-of-crate caller exists.driver.rs:run_claim_driver_in_with_clock— the production body withdrive's clock as a parameter.tokio::time::advancemoves only the tokio virtual clock, notSystemTime::now(), so before this seam nothing that depends on the VALUEnow()returns each cycle was observable through the production body.run_claim_driver_instays a thin wrapper passing the real clock; no production caller changes.Verification
cargo fmt --all -- --check— cleancargo clippy -p dig-node-service— cleancargo test -p dig-node-service --lib rewards_claim— greenRefs DIG-Network/dig_ecosystem#3336
🤖 Generated with Claude Code