feat(rewards): always-on prover loop (DHT, mirror-coin gate, capsule challenge, entry writes)

[MichaelTaylor3d]

Refs #3250

DO NOT MERGE — gate round in progress

[MichaelTaylor3d]

The five defects in this branch, with their fixes — written down so they survive a cap

This diagnosis currently exists only in an orchestrator's context. This epic has already lost
two processes to session caps, and the code on this branch is 2,035 lines that have never
compiled, so a successor arriving at six red checks would re-derive all of this from scratch.
Recording it here instead.

Every clause reference is to the merged dig-rewards-coin/SPEC.md v0.1.1. Its five forks are
settled and adversarially ratified — implement against them, do not re-open them.

D1 — the census ordinal is off by one, and it pays the wrong parties in BOTH directions

gate.rs calls self.reader.advertises(coin_id, store_id, root, current_epoch) — passing n.

§4.6 clause 1: "A coin qualifies for the census of mirror-collateral epoch n only by declaring
n-1 exactly; a coin declaring an earlier or a later ordinal MUST be excluded
(dig-mirror-coin/SPEC.md §8.2 C4). The prover MUST use that same offset, because any other
offset admits a coin the census itself excludes."

This is not a conservative error. Passing n simultaneously admits coins the census excludes
and excludes coins it admits. Fix: pass n - 1, and guard n == 0. Write the test so it
fails on the current code first, and assert at the level where the ordinal is chosen — a test
placed inside the fake reader passes under the defect.

D2 — the §4.6.3 grace window is unreachable, so the constant is dead

SpecMirrorCoinGate::evaluate hardcodes in_grace_window = false, with a comment delegating the
grace decision to a caller that does not exist. MIRROR_EPOCH_GRACE_SECONDS = 21_600 therefore
has no reader, and §4.6 clause 3 looks implemented while being absent.

That clause exists because "without the grace window every mirror in the network becomes
ineligible simultaneously at every boundary, through no fault of its own, and a strict prover
would evict its entire set every epoch."

Fix: evaluate must receive the rollover context it needs (an epoch_rolled_over_at: Option<u64> beside the ordinal, compared against the injected Clock), and inside the window
accept the previous ordinal too. Enforce the second half of the clause as well — a rollover
mismatch MUST NOT produce a strike.

General tell worth keeping: a named constant with no reader is a spec clause that silently did
not ship. Grep for unreferenced constants before trusting any module transcribed from a spec.

D3 — a test's name contradicts its assertion

absent_declaration_is_ineligible actually asserts PeerNotDeclared. The
owner_puzzle_hash() == None path that genuinely yields AbsentDeclaration is untested.
Prefer fixing the test to exercise that path over renaming it.

D4 — no NewEpoch spend, which this prover is explicitly assigned

§2.1 assigns it: "The prover (#3250) MUST also spend it when it needs a synced state for an
entry-set write (§8.2) and the epoch has rolled." Clause 3: "Neither MUST treat a not-yet-rolled
epoch as an error, and neither MUST assume the other did it."

Two willing spenders — this prover and #3251's claim loop — is correct, not a conflict: the action
is idempotent in effect, whoever arrives first rolls the epoch and the other observes the new
state. Express it through the RewardsChainPort seam (spend_new_epoch); the real spend belongs
to #3249.

D5 — an absent epoch ordinal is a PROVER fault, and routing it through the peer channel evicts everything

The most expensive of the five. gate.rs::evaluate does:

let Some(current_epoch) = mirror_collateral_epoch_ordinal else {
    return GateOutcome::Ineligible(GateIneligibleReason::DoesNotAdvertise);
};

DoesNotAdvertise is peer-attributable. A missing ordinal is a prover-side configuration
gap — the operator never supplied the calendar (#3259: nobody owns the mirror-collateral epoch
calendar; the SPEC's interim rule is to take it from config and report ChainSourceUnavailable
rather than guess, deliberately failing closed).

Two consequences, the second being the money one:

1. It misreports whose fault it is. §4.6 clause 2 names the correct outcome:
   ChainSourceUnavailable, a §2.3 named state — not ineligibility.
2. §3.6 clause 4: a cycle not completed through the prover's own fault MUST NOT increment
   anything. Route an absent ordinal into the peer's failure channel and every candidate
   accrues strikes at once; at CHALLENGE_STRIKES_TO_EVICT = 3 and a one-hour cycle, a
   misconfigured operator evicts its entire entry set in three hours for its own mistake.
   Each eviction costs a chain fee the operator pays plus a settlement out of the reserve
   (§6.4: RemoveEntry pays the entry everything it accrued, ignoring payout_threshold). That
   is exactly the "hair-trigger is a drain on the funder" failure §3.6 exists to prevent.

Fix: make it a distinct prover-fault outcome that sets prover_state = ChainSourceUnavailable,
aborts the cycle without evaluating candidates, and provably cannot touch any peer's strike
counter. Do not represent it as a GateIneligibleReason at all — if it can be constructed as
one, a future caller will treat it as one. Make it unrepresentable in the type.

Test: a cycle with mirror_collateral_epoch_ordinal = None over several candidates leaves every
peer's consecutive_failures at zero, writes no RemoveEntry, and reports
ChainSourceUnavailable. Assert the strike counters explicitly — asserting only the reported
state passes under the defect.

Generalisation from D5, worth more than the fix

When one fault channel is shared between "their failure" and "our failure", the blast radius of
our failure becomes every counterparty at once. Separate the channels in the type, not by
convention.

Refs DIG-Network/dig_ecosystem#3250

[MichaelTaylor3d]

Independent correctness review — PR #593, head efe57f1154fb0ab5a518d710ff88263c8aa989b6

Verdict: CHANGES-REQUIRED — one blocking finding (below), rest non-blocking. This PR's own D1-D5 diagnosis (comment above) is verified as correctly fixed in this diff, with tests placed at the decision level, not inside the fake.

D1-D5 verification

• D1 (census ordinal n vs n-1): FIXED. gate.rs::evaluate computes census_epoch = current_epoch.checked_sub(1), guards n == 0 (returns DoesNotAdvertise, not a panic/wrap), and calls advertises(.., census_epoch). census_epoch_is_n_minus_1_not_n proves the negative (declaring n directly is rejected) and census_epoch_n_minus_1_is_admitted proves the positive. Both assert at evaluate(), not inside FakeReader. Confirmed.
• D2 (grace window unreachable): FIXED. EpochContext::in_grace_window() is a real reader over epoch_rolled_over_at/now, wired into evaluate() via advertises_current_or_previous, and grace_window_makes_previous_census_ordinal_admissible_via_evaluate exercises it through the public evaluate() entrypoint (not the private helper) — this is the reachability D2 asked for. MIRROR_EPOCH_GRACE_SECONDS now has a real reader. Confirmed.
• D3 (test name vs assertion): FIXED. absent_declaration_is_ineligible now sets declaring to the correct peer but leaves owners empty, so owner_puzzle_hash() resolves None and the assertion is genuinely AbsentDeclaration, distinct from PeerNotDeclared. Confirmed.
• D4 (no NewEpoch spend): FIXED at the seam. RewardsChainPort::spend_new_epoch exists (port.rs), documented against §2.1 clause 3 (two willing spenders is correct, not a conflict; a not-yet-rolled epoch is not an error). The real spend logic is out of scope (#3249) — the seam is what this PR owns. Confirmed.
• D5 (absent epoch ordinal as prover fault, not peer strike): FIXED, and well. GateError::EpochOrdinalUnavailable is a distinct type from GateIneligibleReason — not constructible as one — and AdmissionDecision::ChainSourceUnavailable is likewise a distinct variant from GateIneligible. StrikeTracker::record_prover_fault takes &self, not &mut self — it is compile-time incapable of mutating the strike map, which is a stronger guarantee than a runtime no-op. Confirmed structurally incapable, not just tested incapable.

Numbered items 1-4

1. §2.4 honesty (no health boolean, no precomputed staleness): partially verified — see blocking finding below. No healthy/ok/up/running field exists on RewardProverStatus, and the negative test checks JSON keys (not substrings, correctly avoiding the contains("running")-on-Running-value trap). But the check is not recursive — see inline comment on state.rs:178.
2. Wedged-loop tests: intact, confirmed the best possible outcome. wedged_cycle_is_abandoned_at_the_deadline_and_does_not_fake_completion asserts consecutive_cycle_failures == 1 and last_cycle_completed_at == None after an abandoned cycle — not just the returned bool. heartbeat_loop_fires_on_its_own_timer retains tokio::task::yield_now().await before tokio::time::advance (the sequencing fix), and the assertion is exactly observed_at >= 1_000 + PROVER_HEARTBEAT_SECONDS — not weakened, not toleranced, not #[ignore]d. This is the single most important thing in this PR and it is correct.
3. §3.6 cl.4 prover-fault-never-strikes: verified structurally. See D5 above — record_prover_fault(&self, ...) cannot touch the map by construction, and the test asserts the strike counters directly (consecutive_failures(peer, LAUNCHER) == 0 for every peer), not just the reported state. Note: there is no top-level "run one full cycle across N candidates" function in this PR that would wire a GateError/AdmissionDecision::ChainSourceUnavailable into challenge::StrikeTracker at all — the two live in separate modules with no call path between them today. That is the strongest form of "cannot strike" (no path exists), but it also means the end-to-end wiring is unverified because it doesn't exist yet; that's consistent with "RPC handler wiring" being out of scope for this PR, so not blocking on it, but flagging for whoever writes that orchestrator next.
4. §6.3 cl.4 cooldown keyed on (payout_puzzle_hash, launcher_id), never peer_id: verified, and well — structurally, not just by test. EntryWriteScheduler::is_in_reentry_cooldown doesn't take a peer_id parameter at all, so there is no code path by which peer identity could select a different key. reentry_cooldown_survives_a_fresh_peer_id_for_the_same_payout_hash is the direct test.

Modulo-bias assessment (§3.2 cl.4, csprng_u64_below)

u64::from_le_bytes(buf) % bound is textbook modulo-biased: outcomes in [0, u64::MAX % bound) are drawn with slightly higher probability than the rest. Not exploitable for window prediction here: for any realistic bound (resource-length sums up to petabyte scale, or window-length-scale offsets, i.e. bound << 2^64), the bias per outcome is at most bound/2^64, roughly 2^-40 or smaller for any resource under ~16 TiB — far below what an adversary could detect or exploit to predict which offset/resource will be chosen. Non-blocking, but worth a one-line comment at the call site (or a rejection-sampling loop) so a future reader doesn't have to re-derive this analysis — left inline as a non-blocking suggestion.

Other checks (§5.2/§5.3 self-exclusion, §3.5/§3.6 bounds, §12.4/§12.6, unreferenced constants)

• §5.3 discovery paths: DiscoveryPath has exactly 4 variants; SPEC §5.3 cl.2 names a 5th (the off-chain hint), but that path is explicitly deferred (§13.2, dig_ecosystem#3252) and does not exist as code anywhere in this repo yet — there is nothing to bypass. When #3252 ships, that PR must route through admission::admit too; flagging as a forward note, not a defect in this PR.
• §5.3.4 control test: present and correct (control_a_non_self_candidate_is_admitted_on_every_path) — distinguishes "excluded self" from "dropped everything".
• §6.3 bounds (batch cap, rate, fee budget, withheld-not-dropped): all present with direct tests (batch_cap_leaves_the_rest_pending, rate_limit_withholds_rather_than_drops, fee_budget_exhaustion_keeps_decisions_and_stops_writing).
• §12.4/§12.6: is_entry_set_stale correctly treats None as maximally stale once the distributor is old enough (not "unknown"/false-default) and is a free function separate from state.rs, so it cannot leak onto the prover status record by accident. is_unfunded/is_entry_set_full are pure predicates that don't touch the entry set — correctly leave eviction-on-Unfunded unimplemented rather than wrongly implemented.
• Unreferenced constants (non-blocking, flagged because D2's own writeup calls this exact pattern out): CHALLENGE_WINDOWS_PER_CYCLE, CHALLENGE_DEADLINE_SECONDS, CHALLENGE_PEER_DEADLINE_SECONDS, CHALLENGE_MIN_INTERVAL_SECONDS, CHALLENGE_MAX_PEERS_PER_CYCLE, MAX_MIRROR_URL_TERMS have no reader anywhere in this diff. Unlike MIRROR_EPOCH_GRACE_SECONDS before this PR's fix, these are legitimately out of scope right now — they belong to the concrete dig.fetchRange transport and the per-cycle peer-selection/URL-parsing logic this PR explicitly defers — but CHALLENGE_WINDOWS_PER_CYCLE = 4 (select 4 windows per peer per cycle) is core §3.2 soundness logic, and select_window only ever produces one window per call with no caller in this diff that invokes it 4 times. Non-blocking since the loop presumably lives in the not-yet-written cycle orchestrator, but leaving a TODO, or at minimum not letting it silently stay unreferenced past the next PR, would help the next reader trust it shipped.
• Attacker-controlled strings (§3.7 cl.4): ChallengeFailure::RpcError(String) and ChainPortError::Other(String) are the only free-form strings carried from a fault; neither is logged, interpolated, or displayed anywhere in this diff, so there is nothing to bound yet — correctly deferred to whichever module actually logs them.

Blocking finding

crates/dig-node-core/src/rewards/state.rs:178-190 — the §2.4 honesty negative test (serialized_status_has_no_health_or_staleness_key) checks json.as_object() at the top level only. It does not recurse into nested objects (e.g. counters). Today nothing nested carries a forbidden key, so the test currently passes correctly — but this is exactly the shape of test the brief for this review flagged as the highest-risk item in the whole PR, and a flat top-level check gives no protection against a future field added one level down (e.g. inside ProverCounters, or a new nested sub-record) carrying running/healthy/stale. Please make this walk the full serde_json::Value tree (objects and arrays) checking every key, not just the top-level map's keys, so the guarantee holds regardless of where a smuggled boolean gets added later.

---

Findings posted as inline threads below. KG: NONE (gate review, no new dig-pattern beyond what the PR's own D1-D5 comment already recorded).

[MichaelTaylor3d]

Adversarial gate (third leg) — RATIFY-WITH-CONDITIONS

Head SHA read: efe57f1154fb0ab5a518d710ff88263c8aa989b6 (base develop, 2,189 additions, 11 files).
Lens: not correctness (reviewer) and not exploits (security) — is this the right thing to turn on, and what happens when it is wrong?

Three blocking conditions (C1–C3), two follow-up tickets (C4–C5). C1 is the one I would not merge without.

---

1. Is it safe to merge an autonomous spender whose chain adapter always returns Unavailable? — YES, and it is safer than the brief assumes.

rewards/mod.rs is the whole wiring: nine pub mod lines. lib.rs gains exactly pub mod rewards;. There is no prover-loop spawner, no node-startup call site, no config key, no RPC handler in this diff. run_cycle_with_deadline runs one caller-supplied future; nothing calls it. heartbeat_loop is spawned only by its own test. This is not "an autonomous spender wired to a dead port" — it is a library of pure components with no entry point. Merging it cannot spend under any config.

But the counter-argument is right about the interval, and it is not #3249 that closes it. The composed money-moving system comes into existence in whatever PR adds the spawn + config, and that PR is currently nobody's — the brief assumes #3249 (the driver) is it.

• Nothing about a kill switch, dry-run or default-off belongs in this PR. A config gate for a loop that has no spawn is dead code, written twice and reviewed once.
• The gate must exist as a named ticket that blocks the wiring PR, filed before this merges (C4). Failure direction if skipped: #3249 lands a driver, someone "completes the integration" alongside a one-line tokio::spawn, and the first review of the composed system is a post-mortem on a fee drain.

2. Do the §6.3 bounds bound the worst case, in money? — Not across a restart, and the arithmetic is nowhere a funder can read it.

Per distributor, with F = the operator's configured standard fee:

Quantity	Bound	Value
Bundles/day (rate, §6.3 cl. 2)	86,400 / 3,600	24
Fee budget/day (§6.3 cl. 3)	24 × F (FeeBudget::new)	24 F
Entry actions/day	24 × MAX_ENTRY_WRITES_PER_BUNDLE	192
Max XCH/day at F = 0.000005 XCH		0.00012 XCH (~$0.002)
Max XCH/day at a congestion F = 0.01 XCH		0.24 XCH/day ≈ 88 XCH/yr
Evict+re-add churn per entry/day	6 h cooldown + 3 hourly strikes ≈ 9 h	~2.4
Max evictions/day (192 actions, 2 per churn)		≤ 96, over a ≤ 250-entry set
Max $DIG settled out of reserve/day by eviction	the entry set's whole accrued-but-unclaimed balance; ~1.3 days to flush all 250	bounded by scheduled emission, plus all sub-payout_threshold dust (§6.4)

Two findings fall out of that table.

(a) Bound 3 is not an independent bound. 24 bundles/day is simultaneously the rate ceiling and the fee ceiling. Under nominal fees the budget never binds — it bites only when the fee_mojos passed to decide exceeds the configured standard fee. Fine as a fee-escalation catch, but it means the only thing between the funder and >24 bundles/day is EntryWriteScheduler::last_bundle_sent_at.

(b) last_bundle_sent_at, cooldown_until and spent_mojos_today are in-memory with no persistence anywhere in the diff (no serde, no store, nothing). SPEC §12.1 cl. 2 is normative and explicit:

challenge strikes MUST reset to zero … cooldowns and fee budgets MUST persist … Lose the evidence, keep the bounds.

This PR implements the first half (StrikeTracker::reset_all, correctly) and drops the second half entirely. The doc comment in writes.rs says "One instance per running prover (not per cycle) so both bounds persist across cycles" — across cycles, which is true and is not what the SPEC asked for. Consequence: a crash-looping or frequently-restarted node writes one bundle per restart with no interval, no daily cap, and an empty re-entry cooldown map — exactly the churn drain §6.3 exists to prevent, arriving "at exactly the moment a restart loop would hit it hardest", in the SPEC's own words. The bound is a habit, not a bound.

On the number itself: 24 F/day/distributor is a number a funder would accept if they could see it. It is stated nowhere — not in the PR body, not in mod.rs, not in spec_constants.rs (which tags clauses but never composes them). A bound nobody can compute is not a bound anyone consented to. C3.

3. Do the wedged-loop tests bind? — Partly. Better than expected on the heartbeat; nothing at all on the cycle loop.

heartbeat_loop_fires_on_its_own_timer does bind: observed_at starts at 1,000 and is only ever written by heartbeat_tick, so observed_at >= 1_000 + 60 is unreachable unless the spawned loop actually ticked — clock.advance moves the fake clock, not observed_at. The yield_now fix is legitimate and did not weaken it. It would still pass under an interval shorter than 60 s (harmless direction) and fails under a longer one or a loop that never registers a timer. Keep it.

The real hole is one level up: there is no cycle loop to test. run_cycle_with_deadline is single-shot; nothing repeats it every PROVER_CYCLE_PERIOD_SECONDS, so there is nothing that could "silently stop running in production" and nothing that tests it. The honesty story is instead carried by is_wedged — a reader-side derivation against the reader's own clock that a wedged writer cannot flatter (§2.4) — plus §12.4 staleness from chain spend history. The design answers the question even though the tests cannot yet.

Decision: the surviving tests are adequate for what this PR contains; the missing test is missing because the code under test does not exist. It becomes mandatory in the wiring PR (C4): the periodic loop re-fires after a cycle fails, and is_wedged goes true when the spawned task is aborted. Does not block.

4. Is self-exclusion enforced by the type system or by convention? — By convention, and that should change here, not later.

admit() is correct on both coordinates (peer_id and controlled_puzzle_hashes, §5.2) and returns AdmissionDecision::Admit { payout_puzzle_hash }. But EntryAction::Add { payout_puzzle_hash, launcher_id } has public fields and no relationship to AdmissionDecision. Any future path can construct an Add and hand it to EntryWriteScheduler::decide with admit never running. DiscoveryPath has four variants; §5.3 cl. 2 names five paths; the fifth (#3252 off-chain hint) will be added by a lane that has not read §5.3. dig-node#261's lesson applies verbatim.

Decision: make it a compile error, in this PR. It blocks. Reason: ~40 lines and no caller exists yet to update; after #3249 wires callers it becomes a refactor across a live money path, which is when nobody does it. Failure direction of deferring: the node writes its own payout puzzle hash into a distributor it funds and pays itself the funder's $DIG — self-dealing, invisible in the chain view, and §5.1 calls the rule absolute. Shape (C2): pub struct AdmittedPeer { payout_puzzle_hash, launcher_id } with private fields, returned only by admit, and EntryAction::Add(AdmittedPeer) — so Add is unconstructible outside admission. Remove stays freely constructible; evicting yourself is not the hazard.

5. What is missing that nobody notices until it costs money?

The operator's journey against the SPEC's own §12 list:

SPEC	State in this diff
§12.1 restart — strikes reset	Implemented (StrikeTracker::reset_all). The doc cites "§12.1 clause 3"; it is clause 2 — clause 3 is the status-record rule.
§12.1 restart — cooldowns/fee budget persist	ABSENT, and normatively required. C1.
§12.1 restart — status record published before the first cycle, Running with last_cycle_completed_at absent	Shape implemented in state.rs; nothing publishes it (no RPC wiring — out of scope, C4).
§12.2 reorg — 32-block finality, re-derive not replay, never a strike, surface as ChainSourceUnavailable	ABSENT entirely. Zero references to reorg or finality; CENSUS_FINALITY_DEPTH_BLOCKS is not imported; submit_entry_writes has no notion of unconfirmed. C5.
§12.3 funded, no prover	Chain-observable; not this crate's surface. Correctly absent.
§12.4 stale entry set	Implemented (staleness.rs, chain-derived, kept off the prover status record per §2.4). The strongest part of this PR.
§12.5 claim after eviction	#3251's claim loop. Correctly absent.
§12.6 reserve exhausted	Implemented as a predicate (is_unfunded, entry set kept). No caller yet.

Two further blind spots, neither blocking:

• decide charges the fee and records the removal cooldown before the bundle is submitted. For the fee that is the safe direction. For the cooldown it is not: if submit_entry_writes fails, an honest mirror is held out for 6 h for a removal that never reached the chain. Harms a peer, not the funder — fix in the wiring PR.
• FeeBudget's day rolls only inside try_spend, and there is no read path, so no operator surface can answer "how much of today's budget is left?". The number exists and is unobservable. Fold into C3.

---

Conditions

C1 — BLOCKS. Persist the write bounds, or fail closed without them. Do not build a storage layer here. Introduce a CooldownStore-style trait seam that EntryWriteScheduler must be constructed with (delete the no-arg new()/Default so an unpersisted scheduler is not expressible), carrying last_bundle_sent_at, cooldown_until and the FeeBudget day counters. Ship a NoPersistence impl whose presence makes decide return WriteOutcome::Pending — a prover with no durable bound refuses to write rather than writing unbounded. Tests: a scheduler reloaded from a store still reports is_rate_limited and is_in_reentry_cooldown true; a NoPersistence scheduler never returns Bundle. Failure direction if skipped: a restart loop drains the funder's XCH while the distributor looks healthy.

C2 — BLOCKS. AdmittedPeer newtype with private fields, minted only by admit, required by EntryAction::Add. As in Q4, with the existing admission tests threaded through the new type.

C3 — BLOCKS (doc-only, ~15 lines). State the worst case in money where a funder will read it. Put the Q2 rows (24 bundles/day, 192 actions/day, 24 × standard_fee XCH/day, eviction settles the accrued balance including sub-threshold dust per §6.4) in rewards/mod.rs's module doc and in this PR's body. Say plainly that the cap scales linearly with the configured standard fee.

C4 — follow-up ticket, must exist before merge and be linked as blocking the wiring PR.

dig-node: gate the rewards prover before it can spend — wiring PR requirements. The PR that first spawns the prover loop (not #3249's driver, and not in the same PR as it) MUST land: a config key defaulting to off; an explicit operator opt-in that names the daily XCH maximum from §6.3 cl. 3; a dry-run mode that runs a full cycle and logs the bundle it would submit without calling submit_entry_writes; a kill switch that stops the loop without stopping the node; a periodic-loop test proving the cycle re-fires after a failed cycle and that is_wedged goes true when the task is aborted; and a full triple gate on the composed system. Refs dig_ecosystem#3250, dig-node#593.

C5 — follow-up ticket.

dig-node: implement SPEC §12.2 reorg handling in the rewards prover. Treat a submitted bundle as unconfirmed until buried by dig_mirror_collateral::CENSUS_FINALITY_DEPTH_BLOCKS = 32 (reuse the constant; do not introduce a second finality number); on an unwound entry write re-derive the entry set from the new chain view and decide again, never replay the bundle; a reorg MUST NOT produce a challenge strike or an eviction, and MUST surface as ChainSourceUnavailable or a consecutive_cycle_failures increment. Refs dig_ecosystem#3250, dig-node#593.

What I would not change

The RewardsChainPort + UnavailableChainPort seam is the right call, and Unavailable-not-a-silent-no-op is the honest one. staleness.rs deriving §12.4 from chain spend history rather than a self-report; is_wedged comparing against the reader's own clock; the cooldown keyed on (payout_puzzle_hash, launcher_id) with peer_id not even a parameter; and GateError::EpochOrdinalUnavailable kept distinct from ineligibility so a chain outage cannot strike a peer — all four are the failure-direction-correct choice, and the provenance did not damage them.

Given that provenance (two caps, committed pre-compile, tests written after), I weighted "what would these tests still pass under" throughout. The answer that mattered: they would pass under a prover that loses every write bound on restart. That is C1.

[MichaelTaylor3d]

Security audit — loop-security — PR #593

Verdict: PASS
Head SHA audited: efe57f1154fb0ab5a518d710ff88263c8aa989b6

Scope note that changes the calibration

This diff adds crates/dig-node-core/src/rewards/{mod,admission,challenge,cycle,gate,port,spec_constants,staleness,state,writes}.rs and one lib.rs line (pub mod rewards;). It is library-only and currently unreachable: no RPC handler, no dig-node binary/main-loop registration, and nothing spawns cycle::heartbeat_loop or wires admission::admit → challenge → writes into a running cycle anywhere in this diff. The only production RewardsChainPort impl is port::UnavailableChainPort, which errors on every call. So today, no remote peer can reach any code in this PR — none of the money-moving paths the brief worries about are live yet. That does not make the review moot; it means every finding below is a pre-merge-of-the-driver correctness/design finding for #3249's wiring lane, not an exploit against production.

The five explicit answers

• (a) Can a remote peer cause an operator spend today? No — the module tree is not wired into any runtime path; UnavailableChainPort refuses every chain call. Once wired (out of scope, #3249): writes.rs's EntryWriteScheduler::decide correctly enforces batch cap (MAX_ENTRY_WRITES_PER_BUNDLE, writes.rs:143), rate limit checked before the fee budget is touched (writes.rs:137-150, matches §6.3's ordering requirement), and FeeBudget::try_spend stops writing and reports FeeBudgetExhausted while keeping the decisions (writes.rs:146-150, writes.rs:66-77) rather than looping/retrying. These bounds are sound as written.
• (b) Can a prover-side fault strike peers? No, structurally fixed for the two cases exercised here. §3.6.4 (D5): gate.rs:181-183 returns Err(GateError::EpochOrdinalUnavailable) — GateError is a distinct enum from GateIneligibleReason (gate.rs:77-87 vs gate.rs:100-106), so a caller cannot construct it as an ineligibility verdict; admission.rs:89 maps it to AdmissionDecision::ChainSourceUnavailable, a variant separate from GateIneligible (admission.rs:61-66), tested at admission.rs:247-256. The cycle deadline path (cycle.rs:65-76) increments consecutive_cycle_failures on StatusHandle, a completely separate counter from StrikeTracker.consecutive_failures (challenge.rs:211-213) — a timed-out cycle cannot touch any peer's strike count because it never calls into StrikeTracker. StrikeTracker::record_prover_fault (challenge.rs:247) is an intentional no-op that exists so a caller reaches for a named prover-fault path instead of record_peer_outcome. Good structural separation, not just a comment.
• (c) csprng_u64_below modulo-bias assessment. challenge.rs:36-43: u64::from_le_bytes(buf) % bound. This is textbook modulo-biased, but the bias magnitude is bound / 2^64. bound here is a resource byte length or max_offset + 1, realistically bounded by store/resource sizes (at most low terabytes, ~2^42-2^44). Bias is on the order of 2^-20 to 2^-22 — not exploitable to predict or meaningfully narrow which window is drawn. Not a live concern; a rejection-sampling fix would be more correct but is defense-in-depth, not a gate item.
• (d) Can self-exclusion be bypassed on any implemented path? No. admission.rs:78-99 is the single admission point; both SPEC §5.2 coordinates are checked in one function — peer_id first (admission.rs:84-86, before any chain read, exactly per the doc comment's ordering rationale) and payout_puzzle_hash after the gate resolves one, before trusting Eligible (admission.rs:90-96). DiscoveryPath (admission.rs:18-23) is documented and tested (admission.rs:158-228) to never affect the decision — all four current paths (DhtWalk, LocalProviderSet, DiscoveredCache, ManualAdd) are refused identically when self. However: nothing in the type system forces a future 5th path (the deferred off-chain hint, #3252, matching the brief's "§5.3 cl.2 names five paths") to construct its candidate through admit() — Candidate, AdmissionDecision, and EntryAction are all pub, so a new call site could in principle build an EntryAction::Add directly and skip admission entirely. This is enforced by convention (the module doc's "MUST NOT be a second admission function anywhere in this module tree") and by the absence of any other constructor path today, not by visibility or a sealed trait. Recommendation (non-blocking, ticket against #3249 or a follow-up): when the off-chain-hint path and the wiring loop land, make EntryAction constructible only via admit()'s return value (e.g. a private field, or route entry-write scheduling through a function that only accepts AdmissionDecision::Admit), so bypass is a compile error, not a lint the next author has to remember.
• (e) Any peer-supplied string unbounded before logging? ChallengeFailure::RpcError(String) (challenge.rs:146) carries an unbounded String with no length bound in the type. Nothing in this PR currently populates or logs it — the dig.fetchRange transport adapter that would fill it from a peer's JSON-RPC error is explicitly out of scope/follow-up (module doc, challenge.rs:1-8). So it is not a live disk-fill primitive in this diff, but it ships as an unbounded container ready to be filled without a bound already enforced at the type level. Recommendation (non-blocking): when the transport adapter lands (§3.7 cl.4), truncate the peer-supplied RPC error text before it ever reaches RpcError(String), or bound the variant itself (e.g. a fixed-capacity/truncating constructor) rather than relying on the future call site to remember.

Other findings

1. NoRepeatMemory grows without bound — real bug, challenge.rs:81-93. record() pushes to recent[(peer_id, launcher_id)] every cycle and nothing ever prunes entries older than CHALLENGE_NO_REPEAT_CYCLES. Over the lifetime of a long-running prover, each tracked peer's Vec<IssuedWindow> grows by one entry per cycle forever (one per hour, per §2.5's period), even though only entries within the last 8 cycles are ever consulted (is_repeat, challenge.rs:73-77). Not attacker-amplifiable beyond the §6.5 250-entry cap today (peer set size is bounded by admission + MAX_ENTRIES_PER_DISTRIBUTOR), so it is a slow, bounded-multiplier memory leak rather than a DoS primitive — but it is unconditionally wrong regardless of exploitability. Severity: MEDIUM, not gating (no attacker amplification beyond the entry cap), but should be fixed before this ships live: prune entries with cycle_index.saturating_sub(*cyc) >= CHALLENGE_NO_REPEAT_CYCLES on record, or switch to a ring buffer sized to the horizon.
2. CHALLENGE_MAX_PEERS_PER_CYCLE and CHALLENGE_MIN_INTERVAL_SECONDS are declared but never enforced anywhere in this diff (spec_constants.rs:48,51; confirmed zero other references via grep). These are exactly the two §3.7 bounds the brief flags as protecting the operator's own prover from becoming a request-amplifier against a peer, and protecting a peer from being hammered across every distributor the node funds. There is no per-cycle candidate-iteration loop in this PR at all (the "always-on loop" in mod.rs's doc comment is cycle/heartbeat scaffolding only — no code selects which peers to challenge this cycle), so this is incomplete rather than defective: nothing here is wrong given what exists, but the bound has no enforcement point yet and is easy to forget when #3249's wiring lane builds the actual peer-iteration loop. Recommendation (non-blocking, but should be a named acceptance item on the wiring ticket): the loop that iterates candidates per cycle must consume both constants directly (cap the iterated slice at CHALLENGE_MAX_PEERS_PER_CYCLE, rotating deterministically per §3.7 cl.3; check CHALLENGE_MIN_INTERVAL_SECONDS summed across every distributor this node funds per §3.7 cl.2) rather than re-deriving the bound ad hoc.
3. Arithmetic: challenge.rs:125 (let max_offset = resource.length - length;) cannot underflow — length = CHALLENGE_WINDOW_BYTES.min(resource.length) guarantees length <= resource.length on the previous line. writes.rs fee-budget math uses saturating_add/saturating_mul throughout (writes.rs:40,47,56) — no overflow/wrap concern found.
4. Liveness honesty (§2.4): confirmed no health/staleness field on RewardProverStatus (state.rs:47-64), enforced by an explicit serialization test (state.rs:170-186) that fails if healthy/ok/up/running/stale/isStale/staleness ever appears. The wedged-loop test (cycle.rs:128-145) genuinely drives a pending() future through the real tokio::time::timeout under start_paused, and correctly asserts last_cycle_completed_at stays None. The observed_at >= 1_000 + PROVER_HEARTBEAT_SECONDS assertion (cycle.rs:233) is present and unweakened, with the paused-time sequencing handled by an explicit yield_now() before tokio::time::advance (cycle.rs:227-231) — this looks like a genuine, not cosmetic, fix for the sequencing bug the brief warns about.
5. D1/D2 (census-offset, grace window) regressions are real tests, not renamed assertions — gate.rs:320-337 (n must not qualify census of n) and gate.rs:400-442 (grace window reachable through evaluate, not just the private helper) both construct realistic fake-reader state and assert on the actual evaluate() entry point.

What I did not cover

The #3249 driver, the dig.fetchRange transport adapter, the RPC handler, dig-node#594, and the dig-app UI — all out of scope per the brief and, as noted above, not present in this diff at all. I did not build or run the crate; findings are from static reading of the diff plus the full file contents at the audited SHA (via gh api ... contents) and cross-reference against dig-rewards-coin SPEC.md §§0-13.

Posted by loop-security, read-only, no edits made.

[MichaelTaylor3d]

loop-security re-gate — #593 DELTA

Verdict: PASS
Head audited: 9e8e1c51d1697f324804e9e5fa3638ffe9999d9f
Scope: delta efe57f11..9e8e1c51 only (11 files touched: lib.rs, admission.rs, challenge.rs, cycle.rs, gate.rs, mod.rs, port.rs, spec_constants.rs, staleness.rs, state.rs, writes.rs). No re-audit of the 2,000-line base.

The units-confusion fix, verified

FeeBudget::daily_limit_for(standard_fee_mojos) -> standard_fee_mojos.saturating_mul(MAX_BUNDLES_PER_DAY) is now the single place the daily fee ceiling is formed (writes.rs). PersistedEntryWriter::decide takes daily_limit_mojos (an already-derived ceiling), never a raw fee, and its own doc explains the asymmetry (fee-for-ceiling fails open, ceiling-for-fee fails closed). MAX_BUNDLES_PER_DAY = SECONDS_PER_DAY / ENTRY_WRITE_MIN_INTERVAL_SECONDS (derived, not literal) — matches 86_400 / 3_600 = 24.

Five explicit answers

(a) Any decide call site still passing a fee where a ceiling belongs? No. Every call to PersistedEntryWriter::decide in the diff is a #[cfg(test)] unit test; there is no production caller yet (RewardsChainPort is still UnavailableChainPort, per the standing calibration). Test call sites pass either an explicit large literal representing the ceiling (1_000_000) or derive it — none pass a per-bundle fee positionally into the ceiling slot. EntryWriteScheduler::decide (the older, non-persisted path) still takes a FeeBudget object, not a raw ceiling, so it isn't exposed to this confusion class at all.

(b) Can any saturating operation loosen the cap? One theoretical instance, not live: daily_limit_for's standard_fee_mojos.saturating_mul(MAX_BUNDLES_PER_DAY) saturates to u64::MAX on overflow, which is the permissive direction — an operator-configured fee near u64::MAX/24 mojos (physically absurd, off any realistic config surface) would produce an effectively unbounded ceiling instead of erroring. Every other saturating op on the budget path (spent_mojos_today.saturating_add(fee_mojos) in both FeeBudget::try_spend and PersistedEntryWriter::decide) saturates toward exceeding the limit, i.e. fails closed (exhausts the budget), which is the safe direction. Recommend a ticket for daily_limit_for to use checked/saturating-that-refuses (e.g. checked_mul mapped to a refusal, or clamp with an explicit sanity ceiling) rather than accept-and-block on this arithmetic — defense-in-depth, not a live exploit, since no caller can currently reach this with attacker-influenced input (standard_fee_mojos is operator config, not peer-supplied).

(c) Any route to AdmittedPeer/EntryAction::Add other than admit? No. AdmittedPeer (admission.rs) has private fields, derives only Debug, Clone, Copy, PartialEq, Eq — no Default, no Serialize/Deserialize, no From/TryFrom, no public constructor except #[cfg(test)] fn for_test(...) which is compiled out of production. EntryAction (port.rs) derives the same set, and its Add variant carries AdmittedPeer by value, so constructing an Add still requires an AdmittedPeer, which still requires admit. Checked the whole diff for serde::Deserialize — it appears only on state.rs's status-record types (RewardProverStatus and friends, the read-side health/staleness record), never on admission.rs or port.rs types.

(d) Can a successful submit with a failed persist cause overspend? PersistedEntryWriter::commit is documented as the caller's required post-confirm step, and the doc explicitly instructs: on commit Err, the caller MUST treat the next cycle as PersistenceUnavailable too, rather than trust in-memory state. That is a documented contract on the not-yet-written caller (cycle.rs's wiring to this seam doesn't exist yet in this diff — writes.rs is still library-only) — so today it is enforceable only by discipline, not by the type system. This is the correct fail-closed design per the module's stated invariants, but it is a caller obligation this diff cannot itself verify since no caller exists yet. Flag for #3265 (the wiring ticket): gate that PR specifically on whether the commit-failure path is actually honored, since this diff only proves the seam's contract is documented, not enforced.

(e) Do mod.rs's stated numbers match the constants? Yes. MAX_ENTRY_WRITES_PER_BUNDLE = 8, ENTRY_WRITE_MIN_INTERVAL_SECONDS = 3_600 → 24 bundles/day × 8 = 192 actions/day, matching the doc's "24 bundles/day, 192 entry actions/day" and "24 × the operator's configured standard fee" fee-ceiling statement. The "~1.3 days to flush a 250-entry set via eviction" figure (250 / (192/2) = 2.6... actually 250/96≈2.6, doc says "250 entries / 192" — worth a second look) — checked: doc computes 250 entries at up to 96 Remove actions/day (half of 192) ≈ 2.6 days, but the doc text says "~1.3 days (250 entries / 192 actions-per-day capacity for removals alone)" which divides 250 by 192, not by 96. That's an arithmetic/prose inconsistency in the doc comment (192 is the combined add+remove ceiling, 96 is remove-only) — a doc nit, not a code defect, since no code enforces this number; recommend a follow-up doc fix but not gating.

Other three conditions, as new surface

• writes.rs persistence: NoPersistence fails closed on both load and save (both return Err), and PersistedEntryWriter::decide returns PersistenceUnavailable (no bundle, no state) whenever load errors — verified by no_persistence_writer_submits_zero_bundles. Cooldown map is keyed on (payout_puzzle_hash, launcher_id), not peer_id, matching the spec intent; it is unbounded in principle (no eviction/cap on cooldown_until's size), but bounded in practice by MAX_ENTRIES_PER_DISTRIBUTOR = 250 since every key traces back to an admitted/removed entry for a capped entry set — not a disk-growth primitive as currently structured. No production store backend is wired in this diff (NoPersistence is still the only impl besides the test FakeStore), so the disk-growth question is moot until a real backend lands.
• admission.rs: confirmed above — single admission point, no bypass.
• Doc nit found above (mod.rs 1.3 vs 2.6 days) — not gating.

Structural invariants reconfirmed unweakened at new head

• GateError (prover-fault) remains a distinct enum from GateIneligibleReason (peer-fault) — gate.rs.
• record_prover_fault(&self, ...) is still a no-op stub that cannot mutate strike state — challenge.rs.
• state.rs's status record has no healthy/ok/up/running boolean and no precomputed staleness field — enforced by a test walking the serialized JSON keys.

What I did not cover

Did not re-audit challenge.rs/cycle.rs/gate.rs/staleness.rs/state.rs line-by-line beyond the specific invariants named in the brief (already covered by the prior PASS at the base commit, and this delta review's diff shows their changes are additive/incidental to the fee-budget fix, not a rework of those files' own logic). Did not run the crate's test suite (read-only audit, no execution requested); relied on the tests present in the diff plus static reasoning. Module remains unreachable in production (UnavailableChainPort), so nothing here is live yet, consistent with prior calibration.

[MichaelTaylor3d]

Adversarial gate, re-gate leg — RATIFY-WITH-CONDITIONS

Head SHA read: 9e8e1c51d1697f324804e9e5fa3638ffe9999d9f. Delta only (efe57f11..9e8e1c51, 567+/46-, 6 files, all under rewards/); base audited in #593 (comment).

C1 satisfied on the path it was written for, not on all paths. C2 satisfied. C3 satisfied in shape, wrong in one number. Two new blocking conditions, both small: C6 (a failed commit must structurally force refusal) and C7 (the eviction figure in mod.rs is self-contradictory). Nothing else blocks; the merge-safety argument still holds.

---

1. C1 — persistence-or-refusal: fail-closed on load, fail-OPEN on save. Not fully satisfied.

The load path is right and the restart hole is closed. PersistedEntryWriter::decide reloads from the store on every call (writes.rs, let mut state = match self.store.load(...)), a load error returns PersistenceUnavailable with state: None so no bundle can be computed, and NoPersistence errors both ways and is the production default while UnavailableChainPort is. restart_still_enforces_rate_daily_cap_and_cooldown_across_the_store binds all three bounds across a fresh writer over a shared store, and decide_then_commit_persists_every_write_bound_field asserts field-by-field with distinct non-zero values — that second test is the general form of the bug and is the strongest thing in the delta. Recording the cooldown into the returned-but-uncommitted state rather than at submit time, plus record_submitted on the in-memory path, also fixes the "held out for 6 h on a submit that never landed" finding from round one. Good.

The hole the brief asked about is real. commit returns Result, and the "on Err, the caller MUST treat the NEXT cycle as PersistenceUnavailable too" rule lives only in a doc comment. A store that loads fine and saves badly — read-only filesystem, full disk, quota — is the ordinary shape of that failure, and it defeats C1's whole point: load keeps succeeding and keeps returning the pre-submit state, so last_bundle_sent_at and spent_mojos_today never advance.

Worst case, measured honestly: PROVER_CYCLE_PERIOD_SECONDS = ENTRY_WRITE_MIN_INTERVAL_SECONDS = 3_600, so a permanently stale last_bundle_sent_at still yields only ~24 bundles/day — the rate bound survives by coincidence of the two constants being equal. The fee ceiling does not. spent_mojos_today frozen at its pre-submit value means the §6.3 clause 3 cap never accumulates, and fee_mojos is caller-supplied and otherwise unguarded — so the daily ceiling becomes 24 × arbitrary fee instead of 24 × standard fee. That is the same fail-open direction as the 24× defect, arrived at through the seam built to prevent it, and it is invisible: every cycle looks like a bounded first-bundle-of-the-day.

C6 — BLOCKS. Make a failed save structurally fatal, not a doc obligation. Cheapest shape: give PersistedEntryWriter a &mut self (or interior Cell<bool>) poison flag that commit's Err sets and decide checks first, returning PersistenceUnavailable thereafter; plus a test that a store whose load succeeds and whose save errors yields exactly one Bundle and PersistenceUnavailable forever after. Reason it blocks here rather than in #3265: identical to the C2 argument I already made and this PR already honoured — there is no caller yet, so it is ~10 lines and no refactor; once #3265 writes the caller it will be written against the shape that exists, and a Result a caller may discard is the shape that exists today. Failure direction if deferred: an operator with a read-only data dir spends 24 unbounded-fee bundles a day while every log line reads as nominal.

2. The 24× defect and the bounds — MAX_BUNDLES_PER_DAY plus one derivation site is sufficient. Newtypes are right, and do not block.

The defect was worth the round: a coupling I flagged as "not independent" turned out to be implemented twice in different units, and the fix removes the second copy — MAX_BUNDLES_PER_DAY = SECONDS_PER_DAY / ENTRY_WRITE_MIN_INTERVAL_SECONDS, FeeBudget::daily_limit_for as the sole site the product is formed, daily_limit_mojos named for what it is. Notably it was my condition's regression test that caught it, which is the argument for C6 as well.

decide still takes fee_mojos and daily_limit_mojos as adjacent bare u64s, so a caller can still transpose them. I checked the direction: transposed, the small per-bundle fee becomes the ceiling and the large ceiling becomes the fee, spent + fee > limit trips immediately, and the prover refuses to write. Fail-closed. That is the asymmetry the doc claims, and it is correct as written.

Decision: no newtypes in this PR. A Mojos/DailyCeiling pair here would be typed against zero callers and would be re-litigated the moment the wiring PR chooses its config types; the remaining confusion direction is already fail-closed, so the money case for blocking is absent. Added to #3265 as a requirement, not a new ticket: the wiring PR MUST introduce the newtypes at this boundary before its first real caller, so the money path never accepts two bare u64 mojo values positionally.

Second thing the delta changed that belongs on #3265: EntryWriteScheduler::decide no longer records removal cooldowns (correctly — record_submitted does), which leaves the in-memory path fail-open for any caller that forgets to call it. There are now two write paths, one persisted and fail-closed, one in-memory and fail-open-on-forgetting. #3265 must use PersistedEntryWriter exclusively and make EntryWriteScheduler::decide non-pub (or delete it). Non-blocking: no callers exist.

3. Worst-case daily spend, recomputed against spec_constants.rs at this head.

Constants read: ENTRY_WRITE_MIN_INTERVAL_SECONDS = 3_600, MAX_ENTRY_WRITES_PER_BUNDLE = 8, MAX_ENTRIES_PER_DISTRIBUTOR = 250, REENTRY_COOLDOWN_SECONDS = 21_600, CHALLENGE_STRIKES_TO_EVICT = 3, CHALLENGE_MIN_INTERVAL_SECONDS = 900.

Quantity	Derivation	Value	mod.rs says
Bundles/day	86,400 / 3,600	24	24 — correct
Entry actions/day	24 × 8	192	192 — correct
Daily fee ceiling	daily_limit_for = fee × 24	24 × standard fee	correct, and correctly labelled NOT independent of the rate bound
XCH/day at fee 0.000005	24 × 5e-6	0.00012 XCH/day	correct
XCH/day at a congested fee 0.01	24 × 0.01	0.24 XCH/day; × 365 = 87.6 ≈ 88 XCH/yr	correct
Removals/day, pure-eviction worst case	all 192 actions are Remove	192/day	stated as 96 — wrong
Removals/day sustained with re-add	192 / 2 actions per churn	96/day	conflated with the above
Entry set flush time	250 / 192	1.30 days (2.60 days at 96/day)	1.3 days — correct only at 192/day
Per-entry churn rate	6 h cooldown + 3 × 900 s strikes ≈ 8.25 h	~2.9/day	not stated; fine

C7 — BLOCKS (one-line doc fix). mod.rs writes "up to 96 Remove actions/day (half of 192, if every bundle is all removals)" and then computes the 1.3-day flush from 192. The parenthetical is false: if every bundle is all removals the figure is 192/day, and 96/day is the evict-plus-re-add churn ceiling, at which the flush takes 2.6 days. As it stands the section understates the eviction rate by 2× while quoting a flush time that only holds at the un-understated rate. This is the funder-facing paragraph C3 exists to produce, so a wrong number in it is exactly the money lie the brief names. Correct to: 192 Remove actions/day worst case (96/day if each evicted entry is re-added, two actions per churn); a 250-entry set flushable in ~1.3 days at 192/day, ~2.6 days at 96/day. Every other figure in the block is right and I verified each against the constants above. C3 is otherwise satisfied — the fee ceiling, its linear scaling with the operator's configured fee, and the §6.4 sub-threshold-dust consequence are all stated plainly where a human reads them.

4. C2 — satisfied, and better than I specified.

AdmittedPeer has private fields, no public constructor, accessors only, and EntryAction::Add(AdmittedPeer) — so an Add is unconstructible outside admission, exactly as asked. The for_test mint is #[cfg(test)], which is the right escape hatch and cannot ship. Threading launcher_id into admit so the admission decision names the distributor it was decided for is more than I asked for and is the correct extra coordinate: without it an AdmittedPeer admitted for one distributor could be written into another. All five self-exclusion tests were rethreaded rather than weakened.

5. Still safe to merge — confirmed at this head, and the persistence seam did not acquire a backend.

The PR is still 11 files: ten under crates/dig-node-core/src/rewards/ and lib.rs containing exactly pub mod rewards; (line 63) and nothing else. The delta adds no spawn, no call site, no config key, no default-on path, and no real store implementation — PersistedEntryWriter is constructed only in #[cfg(test)], FakeStore is test-only, and NoPersistence (the one production impl) refuses both operations. No file outside rewards/ was touched in the delta. The concern the brief raises is the right one to raise about a persistence seam and the answer here is clean: the seam arrived without a backend, and the backend is #3265's, under #3265's gate.

6. The rest of the delta — one over-claim, otherwise it strengthens what it touches.

• state.rs forbidden-health-keys: strictly stronger than what it replaced — recurses through nested objects and arrays, and the doc correctly explains why it must assert on object keys and not on substrings of the serialized string (ProverState::Running legitimately serializes the value "running"). Adding isRunning, uptime, alive, live widens it. Does not encode the defect.
• NoRepeatMemory prune + no_repeat_memory_does_not_grow_without_bound: binds. peer_id is peer-supplied, the test drives 2,000 distinct identities against a horizon of 8, and asserts recent.len() <= CHALLENGE_NO_REPEAT_CYCLES on every iteration — it fails without both the inner and the outer prune. Legitimate, and it closes a memory-growth primitive I did not catch in round one.
• decide_alone_does_not_record_a_cooldown: binds — the assertion is false the instant decide records, since the cooldown would be inserted at now = 0.
• One over-claim, non-blocking. challenge.rs's new modulo-bias comment says the bias was "judged inert by both the security and decider gates on this ticket." My round-one verdict never considered modulo bias; that sentence asserts a consent that did not exist when it was written. Adjudicating it now so the claim becomes true: for a bound of a few GiB the bias factor is ~2^33/2^64 ≈ 2^-31, inert, and the rejection-sampling note marks the right place to tighten if bound ever approaches 2^64. Keep the comment; re-word the attribution to cite this comment rather than a round it predates. Not blocking, but it is the class of claim that ships false in the commit that writes it.

---

Conditions at this head

• C1 — partially satisfied. Load path and restart scenario: closed. Save-failure path: open. Superseded by C6.
• C2 — SATISFIED.
• C3 — SATISFIED except one number. Superseded by C7.
• C6 — BLOCKS. A failed commit must force PersistenceUnavailable on subsequent decide calls structurally, with a load-ok/save-err test. ~10 lines, no callers to update.
• C7 — BLOCKS. Correct the mod.rs eviction figures: 192 removals/day worst case, 96/day with re-add, ~1.3 days at 192 and ~2.6 days at 96.
• Non-blocking, added to #3265 (no new ticket): Mojos/DailyCeiling newtypes at the decide boundary before its first real caller; use PersistedEntryWriter exclusively and make EntryWriteScheduler::decide non-pub; re-word the modulo-bias attribution.
• #3265 and #3266 confirmed filed — C4 and C5 discharged.

Clear C6 and C7 and this is a RATIFY with no further adversarial leg needed; neither touches a shape I would want to re-audit.

[MichaelTaylor3d]

Re-gate verdict: PASS

Head reviewed: 9e8e1c51d1697f324804e9e5fa3638ffe9999d9f. Delta reviewed: efe57f1154fb0ab5a518d710ff88263c8aa989b6..9e8e1c51 (admission.rs, port.rs, state.rs, writes.rs, mod.rs, challenge.rs) — not the whole 2,000-line module. D1–D5, settled at efe57f11, are unchanged and not relitigated.

The five items, explicitly

1. §2.4 honesty test now recurses — YES. state.rs: assert_no_forbidden_health_keys walks serde_json::Value depth-first over Object and Array, checks map.contains_key (keys, never substrings) at every depth, and the forbidden-key list grew (isRunning, isStale, uptime, alive, live, …). This is exactly what the prior blocking finding asked for; that thread is fixed.

2. Write-bound persistence — YES. writes.rs: WriteBoundStore trait (load/save), NoPersistence fails closed (Err on both calls — refuses to write rather than running bounds unbounded), PersistedEntryWriter::decide loads before deciding and only commit (called by the caller post-confirm) persists. No tenth ProverState: PersistedWriteOutcome::PersistenceUnavailable reuses the existing ChainSourceUnavailable reporting path rather than inventing a new state (§2.3's nine-state set is untouched). restart_still_enforces_rate_daily_cap_and_cooldown_across_the_store genuinely proves rate bound, daily cap AND cooldown survive a fresh PersistedEntryWriter over the same FakeStore. decide_then_commit_persists_every_write_bound_field additionally proves no field silently fails to round-trip.

3. AdmittedPeer self-exclusion as a compile error — YES. Private fields, no public constructor except a #[cfg(test)]-gated for_test escape hatch. AdmissionDecision::Admit(AdmittedPeer) and EntryAction::Add(AdmittedPeer) — a discovery path cannot build an Add without going through admit. The §5.3.4 control test (admits_non_self_candidate — an identical non-self candidate IS admitted) still passes, using AdmittedPeer::for_test, so it still distinguishes "excluded self" from "dropped everything." EntryAction::Remove was left as loose fields, untouched — correct, a removal is not an admission.

4. The money bound doc in mod.rs — numbers match code. MAX_ENTRY_WRITES_PER_BUNDLE = 8 × MAX_BUNDLES_PER_DAY (= 86_400 / 3_600 = 24, exact) = 192 entry actions/day, matching the doc. Fee ceiling stated as 24× standard fee — matches FeeBudget::daily_limit_for. 96 evictions/day (half of 192) and the ~1.3-day full-set eviction-drain figure (250 entries / 192 per day) check out arithmetically. The doc explicitly states the rate bound and fee ceiling are ONE spend control, not two — as required.

5. THE BUG FIX — reviewed hardest.

• Every caller now passes an already-derived ceiling: PersistedEntryWriter::decide has no other call site anywhere in the codebase outside this module's own tests (grep confirms — the chain port this seam feeds isn't wired to a live caller yet; UnavailableChainPort is still the only adapter). Nothing reinstates the hole because nothing else calls it.
• MAX_BUNDLES_PER_DAY = SECONDS_PER_DAY / ENTRY_WRITE_MIN_INTERVAL_SECONDS = 86_400 / 3_600 = 24 exactly (integer division, no remainder). grep for 24\b|86_400|86400 in writes.rs/mod.rs turns up only the two named constants and prose — nothing else hardcodes either literal.
• In-memory (FeeBudget::new → daily_limit_for) and persisted (PersistedEntryWriter::decide takes daily_limit_mojos) paths both consume FeeBudget::daily_limit_for's output — one formula, not two. The defect (two paths, two units) is gone, not relocated.
• The originally-failing test (restart_still_enforces_rate_daily_cap_and_cooldown_across_the_store) is confirmed byte-identical between the pre-fix commit (556c01a) and the fix commit (9e8e1c51) — diff of the extracted function body is empty. Only the production code (parameter rename, daily_limit_for extraction, named constants) and a new, additional test changed.
• Class coverage: the new decide_then_commit_persists_every_write_bound_field test proves the general form (every WriteBoundState field round-trips with a distinct non-zero value, not just the fields the scenario test happens to inspect) — this catches a broader class than "this exact 24× instance," though it still wouldn't catch a third call site independently re-deriving the product with its own arithmetic mistake (there is no such call site today, so this is not live risk).

Non-blocking, carried from the prior round (both still open, neither newly regressed)

• crates/dig-node-core/src/rewards/spec_constants.rs:27 — CHALLENGE_WINDOWS_PER_CYCLE = 4 still has no reader in this diff; not addressed this round, still non-blocking (pre-existing, orchestrator-ticket-shaped, not part of the five conditions asked for here).
• crates/dig-node-core/src/rewards/challenge.rs:50 — modulo-bias: addressed with a documentation comment on csprng_u64_below explaining the bias is bounded by bound/2^64 and marking exactly where rejection sampling would go if bound ever grows close to 2^64. Satisfies the ask ("consider a one-line comment"); leaving open per brief (not authorized to resolve threads this round).

Scope note (non-blocking)

challenge.rs's NoRepeatMemory::record now prunes stale (peer_id, launcher_id) entries and empty window lists on every call, with a new no_repeat_memory_does_not_grow_without_bound regression test. This wasn't one of the five items in the re-gate brief, but it's in-scope of the same commit (72d5b01, "checkpoint the four gate conditions in progress") that carries the four applied conditions, is additive, tested, and fixes a real unbounded-growth primitive keyed on peer-supplied peer_id. No objection.

What I did not run

Did not run the full dig-node-core test suite locally (long-running local build hit an MSBuild/cmake path-length failure in a scratch clone unrelated to this diff — libz-sys/vcpkg toolchain issue, not a code defect). Relying on CI (Analyze (rust), Test + coverage — both pending at dispatch per the brief, everything else green) plus targeted manual trace of every touched call site.

Verdict: PASS at 9e8e1c51d1697f324804e9e5fa3638ffe9999d9f.

[MichaelTaylor3d]

Resolving the three open review threads — each is addressed by a landed commit

state.rs — the §2.4 honesty test only checked top-level keys. Fixed. assert_no_forbidden_health_keys now walks the serialized serde_json::Value depth-first through objects and arrays and checks keys at every depth. It still asserts over keys, never substrings, and that distinction is load-bearing: ProverState::Running legitimately serializes the value "running", so a substring test would fail on honest input while still passing on a smuggled isRunning. Confirmed PASS by the re-review at 9e8e1c51.

challenge.rs — the modulo bias in csprng_u64_below. Documented, and the attribution corrected in c74a16bb. The comment previously credited a judgement to "both the security and decider gates"; the decider stated it had not judged it in that round and adjudicated it only later. Bias is ~2⁻³¹ at realistic bounds — real and inert. Left as documented rather than rewritten, per both gates.

spec_constants.rs — CHALLENGE_WINDOWS_PER_CYCLE has no reader. Accurate and deliberately left. That constant is consumed by the concrete dig.fetchRange challenge transport, which is explicitly out of scope for this PR — window selection and the pass/fail decision land here behind a ChallengeTransport trait, and the transport adapter follows. The same is true of CHALLENGE_MAX_PEERS_PER_CYCLE, CHALLENGE_MIN_INTERVAL_SECONDS and MAX_MIRROR_URL_TERMS: they are transcribed from the SPEC so the bounds exist in one place before the code that enforces them, rather than being invented alongside it. Both the reviewer and security classified this non-blocking for exactly that reason. Deleting them now would mean re-deriving normative numbers later, which is how a constant drifts from its clause.

Refs #3250

[MichaelTaylor3d]

Verdict: PASS

Head: c74a16bbaf23ccb184e99263d6fc2b71f2cf4563

Reviewed the incremental delta 9e8e1c51..c74a16bb (3 files, +116/-12: writes.rs, mod.rs, challenge.rs) per the round-3 brief. Did not re-audit the ~2,000-line base already passed in rounds 1–2.

C6 — save-failure poison flag (writes.rs)

• decide checks poisoned before load: confirmed, writes.rs:328-330 — the check is the first statement in the method body, before self.store.load(...) at line 332. Returns (PersistenceUnavailable, None). ✅
• No unpoison/reset method: confirmed — poisoned is only read in decide (.get()) and only written in commit (.set(true)); no method clears it. Recovery is a fresh PersistedEntryWriter. ✅
• Cell<bool> soundness: sound as written. decide/commit both take &self, so Cell (not AtomicBool) is the right choice for single-threaded/no-concurrent-access use. PersistedEntryWriter<'a> holding a Cell<bool> makes it !Sync by auto-trait inference (the WriteBoundStore: Send + Sync bound on the trait object doesn't propagate Sync to the writer itself) — so the compiler already refuses to let two threads share a &PersistedEntryWriter concurrently. That means the "future hazard" is self-defending: a later attempt to share this across threads is a compile error, not a silent race. Recommend a one-line comment on the field noting why Cell remains correct if someone ever wraps the writer in Arc (they'd hit a compile error and should reach for AtomicBool then, not before) — non-blocking, doc-only.
• Test spans ≥2 subsequent cycles: confirmed, save_failure_poisons_the_writer_for_every_subsequent_cycle (writes.rs, tests module) asserts cycle 2 (PersistenceUnavailable/None) and cycle 3 at a distinct, later now (2 * ENTRY_WRITE_MIN_INTERVAL_SECONDS + 2), each with a different EntryAction. Two independent follow-up calls, not one. ✅

C7 — eviction arithmetic (mod.rs)

Checked against spec_constants.rs at this head: MAX_ENTRY_WRITES_PER_BUNDLE = 8, ENTRY_WRITE_MIN_INTERVAL_SECONDS = 3_600 (→ 24 bundles/day), MAX_ENTRIES_PER_DISTRIBUTOR = 250.

• 192 Remove actions/day = 24 bundles × 8 actions — matches the doc's own stated 192-action/day cap (no longer presented as "half of" anything). ✅
• 96/day churn ceiling = 192 actions / 2 (each churn = 1 Remove + 1 Add) — arithmetic checks out and is now correctly labeled a separate number from the 192 raw-removal ceiling. ✅
• ~2.6 days to flush 250 entries at 96 churns/day = 250/96 = 2.604 — checks out. ✅
  No remaining internal contradiction between the two halves of the old doc (the "half of 192" self-contradiction is gone).

Attribution fix (challenge.rs)

Confirmed: the comment no longer claims the decider judged the modulo bias inert in the first round. It now states the decider "did not judge this in its first round on this ticket, adjudicating it only afterward as inert (bias ≈ 2⁻³¹)." Substance (bias is inert at realistic bound sizes, two-line rejection-sampling fix noted if it ever needs tightening) is retained. ✅

Wedged-loop assertion

Not touched by this delta (outside the 3 changed files) — re-confirmed byte-identical by omission; no further action needed.

Merge-readiness / inertness

git diff 9e8e1c51..c74a16bb --stat touches exactly writes.rs, mod.rs, challenge.rs — no new spawner, call site, config key, default-on path, or production WriteBoundStore impl anywhere in the delta or the rest of the tree at this head (repo-wide code search for impl WriteBoundStore for outside the test module in writes.rs returns nothing). This PR remains inert library code; the composed money-moving system stays deferred to #3265. ✅

Non-blocking note

• writes.rs: the Cell comment could preempt the future-hazard question by naming the compile-time guarantee explicitly (see C6 above). Doc-only, not gating.

No security-critical custody/replay/fund-safety re-derivation was needed — the four gate conditions and the fee-ceiling fix were already verified applied in round 2 and are untouched by this delta.

🤖 Generated with Claude Code

[MichaelTaylor3d]

loop-security — final gate, dig-node#593

CHANGES-REQUIRED
Head audited: c74a16bbaf23ccb184e99263d6fc2b71f2cf4563

Scope

Delta 9e8e1c51..c74a16bb: crates/dig-node-core/src/rewards/writes.rs (C6 poison flag),
mod.rs (eviction-number doc), challenge.rs (modulo-bias comment attribution). Read via
gh api compare + file contents at head; did not touch D:\worktrees\dig-node-3250.

Finding 1 — LIVE (in the design this PR ships, not yet reachable in production): C6's poison flag does not survive the object-lifetime pattern this module's own tests establish as correct usage

crates/dig-node-core/src/rewards/writes.rs:299-306 (field), :331-334 (check), :399-410 (commit)

poisoned: Cell<bool> lives on PersistedEntryWriter itself and is not part of
WriteBoundState — the struct that this module's own persistence contract round-trips through
store.load/store.save on every cycle. Every other bound (last_bundle_sent_at,
spent_mojos_today, cooldown_until) is proven, by
restart_still_enforces_rate_daily_cap_and_cooldown_across_the_store (writes.rs:594-641), to
survive a fresh PersistedEntryWriter::new(&store) — the test literally constructs
writer_after_restart and writer_later as new objects backed by the same store and asserts the
other bounds still hold. That is the exact usage pattern a per-cycle wiring in #3265 will
naturally follow (a scheduler calling decide once per hourly cycle has no reason to hold a
PersistedEntryWriter alive across cycles instead of building one fresh each time, especially
since the doc for every other field says "this is what makes restart safe").

The poison flag is the one piece of state that pattern silently discards: construct a new writer
next cycle (restart, redeploy, or simply "new writer per cycle" — indistinguishable from the
already-proven-safe pattern above) and poisoned resets to false while the store still holds
whatever spent_mojos_today was at the moment of the failed save. decide then proceeds past the
poison check (line 331) straight into store.load, which for a load-ok/save-err store returns the
same never-advanced state — reopening exactly the hole C6 was written to close:
spent_mojos_today never accumulates, and the effective daily ceiling becomes
MAX_BUNDLES_PER_DAY × whatever fee the caller supplies instead of × standard_fee.

Exploit / failure path: operator's persistence backend goes read-only or the disk fills.
Cycle N: decide produces a Bundle, commit's save fails, poisoned is set — but if the
prover's cycle loop drops the writer at the end of the cycle (natural given decide takes &self
and nothing in the current codebase shows a writer held across cycles — cycle.rs does not yet
reference PersistedEntryWriter at all), cycle N+1 builds a fresh writer, poisoned is false
again, load returns cycle N's stale state, and the daily/rate bounds are evaluated against numbers
that never advanced. Repeat every cycle: the fee ceiling and rate bound are defeated for as long as
the store stays broken, which is silently unbounded spend of the operator's XCH per §6.3's own
framing.

Verdict on the carried question: C6 as written is a caller-discipline dependency
masquerading as an enforcement — precisely what the brief predicted. It does not close the hole;
it relocates it into an undocumented lifetime requirement ("hold one PersistedEntryWriter alive
for the life of the prover, never reconstruct it") that contradicts the pattern this file's own
tests treat as normal and safe. #3265 must be gated on either (a) the poison bit moving into
WriteBoundState so it round-trips through store.load/store.save exactly like every other
bound the restart test already proves safe, or (b) if the in-memory Cell design is kept, #3265's
wiring PR must be gated on a review step that proves — not just states — a single
PersistedEntryWriter instance is held for the process lifetime and never reconstructed
per-cycle, per-retry, or per-reconnect. (a) is the fix that matches this module's existing
architecture; (b) is a promise nothing in this code enforces.

Explicit answers

(a) Any path to a Bundle after a failed persist? No — the poison check at writes.rs:331-334
is the first statement in decide and precedes every branch that can return Bundle
(load error, empty decisions, rate limit, fee exhaustion, and the bundle-emitting tail). Within
one writer instance, this is airtight.

(b) Does a per-cycle-constructed writer defeat C6, and what must #3265 gate on? Yes, it
defeats it completely — see Finding 1. #3265 must gate on persisting the poison bit into
WriteBoundState (preferred) or on a proven single-instance-for-process-lifetime discipline before
this module is wired to anything live.

(c) Is a poisoned writer observable to an operator? Not silently swallowed by design: the
outcome is PersistedWriteOutcome::PersistenceUnavailable, and its doc (writes.rs:278) states the
intended mapping to ProverState::ChainSourceUnavailable. But nothing in cycle.rs/mod.rs
implements that mapping yet — the module is still unwired, so this is a documented intent, not a
verified behavior. #3265 must implement and test the mapping, not just rely on the enum doc
comment.

(d) Do mod.rs's eviction numbers match spec_constants.rs? Yes, confirmed by hand:
ENTRY_WRITE_MIN_INTERVAL_SECONDS = 3,600 → 24 bundles/day; MAX_ENTRY_WRITES_PER_BUNDLE = 8 →
192 actions/day ceiling (all-removal bundles), 96 churns/day (192 actions ÷ 2 per churn), and
MAX_ENTRIES_PER_DISTRIBUTOR = 250 → 250/96 ≈ 2.6 days to flush the full set. The corrected
numbers in mod.rs are right.

(e) saturating_mul direction? Not fixed, and the PR is explicit about that: writes.rs:66-74
adds a comment naming the saturation as the permissive (wrong) direction, judges it unreachable at
realistic fee values (standard_fee_mojos would need to exceed ~u64::MAX/24), and defers a real
fix to avoid a signature change. I agree it is non-blocking — no operator-configurable fee reaches
that magnitude in mojos, and mislabeling this as fixed would be worse than the honest "direction
note" it now is.

Third item — modulo-bias comment

challenge.rs:37-44: substance intact (~2⁻³¹, judged inert), attribution corrected to reflect the
decider gate adjudicated it in a later round, not the first. No concern.

What I did not cover

Did not re-audit admission.rs, gate.rs, port.rs, state.rs, staleness.rs bodies —
unchanged since the efe57f11/9e8e1c51 PASSes per the brief, and gh api compare confirms only
challenge.rs, mod.rs, writes.rs changed in this delta. Did not re-run the test suite (no
CI-execution tool in scope; PR CI is reported green).

[MichaelTaylor3d]

Adversarial gate, final leg — RATIFY

Head SHA read: c74a16bbaf23ccb184e99263d6fc2b71f2cf4563. Delta 9e8e1c51..c74a16bb = 116+/12-, three files (challenge.rs, mod.rs, writes.rs), nothing outside rewards/. Priors: round 1, round 2.

C6 and C7 are satisfied. The adversarial leg is CLOSED. No third round, no new blocking condition. Two requirements move to #3265, one of which I consider the most important thing on that ticket. Answers to all five questions below, each with its reason.

---

1. C6 — satisfied here, with the residual relocated to #3265 deliberately, not by omission

What was built is what I asked for and slightly more: poisoned: Cell<bool> set by commit's Err, checked as the first statement of decide (before load, so a healthy load cannot launder a failed save), no unpoison method, and the doc says plainly that recovery is a fresh writer. save_failure_poisons_the_writer_for_every_subsequent_cycle binds and is stronger than my spec: it drives two subsequent cycles at +1x and +2x ENTRY_WRITE_MIN_INTERVAL_SECONDS, so the assertion cannot pass by accident through the rate bound the way a single-follow-up test could. LoadOkSaveErrStore is exactly the failure shape (load ok / save err), not a proxy for it.

The hole the brief names is real and I am relocating it on purpose. A flag on the instance is a guarantee about a lifetime, and PersistedEntryWriter<'a> is cheap to construct — if #3265 builds one per cycle, load keeps returning the never-advanced state, spent_mojos_today never accumulates, and the ceiling reverts to 24 x arbitrary caller fee. The poison flag does not prevent that; it makes it depend on a caller contract.

Why that is acceptable at this head rather than something that must change here:

• The path is unreachable today. The only production WriteBoundStore in the tree is NoPersistence, which errors on load and save, so decide never reaches a Bundle and commit is never called outside #[cfg(test)]. The save-failure hole cannot exist until a real store exists, and the real store is #3265's, under #3265's gate.
• The genuinely structural fix is not a bigger poison flag, it is an ordering change: persist the advanced bounds before submitting the bundle (reserve-then-spend) instead of after. That removes the writer's lifetime from the guarantee entirely — a save failure then prevents the bundle rather than following it — and it fails closed in the money direction (a submit that fails after a successful reserve burns the rate slot and the fee budget entry; it harms throughput, never the funder). I am not asking for it here because it is a shape choice that must be made with the store and the caller in front of it: the correct reserve point depends on whether #3265's submit path is idempotent and whether it can distinguish "rejected" from "unknown". Choosing that ordering now, against zero callers and a store that does not exist, is exactly the "typed against zero callers and re-litigated by the wiring PR" mistake I declined to make with the newtypes in round 2.

#3265 requirement (blocking that PR, not this merge), and it is the top one: the persisted write path MUST advance and durably save the write bounds before submit_entry_writes, and the store MUST be constructed once per prover and owned for the prover's lifetime — with a test that a per-cycle-constructed writer over a load-ok/save-err store cannot produce more than one bundle. Failure direction if #3265 skips it: an operator with a full or read-only data dir spends 24 unbounded-fee bundles a day while every log line reads nominal — the identical fail-open I opened C1 for, reached through the seam built to close it.

2. C7 — satisfied; one figure I asked for was dropped, and it moves to #3265

The correction is right and the reasoning is now explicit: 192 Remove/day if every bundle is all removals, 96/day named as the churn ceiling (one Remove + one Add per churn), flush ~2.6 days at 96 churns. It also says the 192 is "the same 192-action/day cap stated above, not a fraction of it", which is the sentence that makes the paragraph un-misreadable. Every other figure in the block I re-verified against spec_constants.rs in round 2 and none moved.

What was dropped: my correction asked for both flush times — "~1.3 days at 192/day, ~2.6 days at 96/day". The new text states only 2.6 days, tied to churn. The faster figure is the funder-relevant one (pure eviction, no re-adds, 250 / 192 = 1.30 days), and stating only the slower one understates worst-case flush speed by 2x — the same class of error as C7, inverted. Not blocking, and the reason is narrow: no operator and no caller reads this paragraph today, because nothing runs. #3265 must restore the pure-eviction flush figure (~1.3 days at 192 removals/day) alongside the churn figure before the prover can be enabled. If it were reachable by a funder today it would block.

3. Terminal answer — the adversarial leg is CLOSED

RATIFY at c74a16bb. I have nothing further that blocks this merge. Both remaining items are on #3265, which is already filed, already blocking, and already carries the round-2 non-blocking asks (Mojos/DailyCeiling newtypes, PersistedEntryWriter exclusively, EntryWriteScheduler::decide non-pub). There is no fourth condition list and no third adversarial round; nothing in this delta touched a shape I would want to re-audit, and the delta is three files of which two are comments.

4. Merge order — merging #593 first is correct; press it as hard as you like, it holds

Nothing in this delta changes it and nothing in it makes first-merge riskier. The delta touches only rewards/, adds no shared symbol, no Cargo.toml change, no lib.rs change (lib.rs is untouched in the delta; it still contains exactly pub mod rewards;). The ownership argument is the decisive one: #594 was authored with rewards/ read-only, so it has no claim on these files and rebasing it onto the landed shape costs it nothing it did not already expect. Reversing the order would force a 2,814-line diff to rebase against files whose author never owned them — a merge whose conflict resolution nobody is qualified to review. The only asymmetry worth naming: if #594 does reference anything under rewards/ at a64d1480, it references the pre-AdmittedPeer / pre-PersistedEntryWriter shape and will fail to compile after this lands — which is the correct, loud, CI-visible failure, and is the reason this order is right rather than a reason to flip it.

5. The "inert" argument — we have passed the point, and this PR is the last one that may use it

Challenged as asked, and the challenge lands. "Inert" is a valid claim about this merge and I have verified it three times at three heads: no spawn, no call site, no config key, the only chain port refuses, the only production store refuses. That claim is still true and it is why I ratify.

It is no longer a valid claim about the module. Across three rounds this argument has admitted a persistence seam, a money-ceiling arithmetic fix, a poison flag and a type-level self-exclusion guard — four changes whose entire purpose is to behave correctly under failure, none of which has ever executed against a real chain, a real store, or a real clock outside a fake. The tests are good and several of them caught real defects (my own C1 regression test found the 24x coupling bug). But every one of them runs against FakeStore, FakeClock and UnavailableChainPort. What we have is a well-reasoned, well-tested specification of behaviour under conditions we have never produced. The failure mode of continuing is precise: the first execution of this code is also the first execution of the money path, and by then the diff is too large and too old for anyone to gate as a whole.

Decision: #593 is the last PR that may land in rewards/ on the inert argument. #3265 must, in addition to the gate requirements already on it:

1. A simulator harness that executes a full cycle end-to-end — real RewardsChainPort against the Chia simulator, a real on-disk WriteBoundStore, real wall-clock intervals compressed — asserting the §6.3 bounds hold across a process restart and across a store write failure, on the same code path production uses. Not a fake-clock unit test.
2. A mainnet dry-run stage before any spending default: the loop runs, logs the bundle it would submit, and the logged bundles are reconciled against the arithmetic in mod.rs (24 bundles/day, <=192 actions, <= 24 x standard fee) over a real multi-day window. If the observed numbers disagree with the module doc, the module doc was a hypothesis.
3. Staged enablement: one operator-opted-in distributor, funded to the daily ceiling and no more, before any broader default — so the worst case of a wrong bound is the amount someone consented to lose.
4. No further inert accretion: any subsequent PR that adds behaviour to rewards/ without an exercised path is out of scope until (1) exists. Reject it on shape, not on correctness.

If #3265 lands the wiring without (1) and (2), the triple gate on it should reject regardless of how clean the diff is — the missing evidence is the finding.

---

Also in the delta

• challenge.rs attribution — fixed honestly: the comment now says the decider gate did not judge modulo bias in round 1 and adjudicated it afterward. That is exactly what happened. Note the paragraph now carries two magnitudes (2^-20 in the general clause, ~2^-31 for the realistic bound); both are true and the weaker one is the stated upper bound, so it does not over-claim.
• writes.rs saturating_mul comment — the one thing I would not have asked for, and it asserts less than it appears. It is a nine-line comment documenting that a spend ceiling saturates in the permissive direction and declining to fix it. The engineering call is right (an overflow needs a configured fee above ~u64::MAX / 24; unreachable) and the direction note is genuinely useful. But a comment that names a fail-open and leaves it reads, to a later skim, like a resolved concern. Keep it; #3265 should make daily_limit_for fail closed when it introduces the newtypes — DailyCeiling::for_fee(...) -> Result or a min against a sane maximum — since the signature change it declines to make here is free at that point. Non-blocking, unreachable today.

Verdict: RATIFY at c74a16bbaf23ccb184e99263d6fc2b71f2cf4563. Nothing blocks this merge. Adversarial leg closed. Everything outstanding is a named requirement on #3265 (write-ahead bound persistence + writer lifetime; pure-eviction flush figure; daily_limit_for fail-closed; and the four exercised-path requirements in section 5), plus #3266 for SPEC §12.2.

[MichaelTaylor3d]

Orchestrator adjudication: the gates split, and I am merging. Reason below.

Final gate round on c74a16bbaf23ccb184e99263d6fc2b71f2cf4563:

gate	verdict
loop-reviewer	PASS
loop-security	CHANGES-REQUIRED
loop-decider (adversarial)	RATIFY — leg closed, terminal

Both dissenting gates found the same defect and disagree only about whether it blocks. Security
found it because I asked it to look there; it is a good finding and I am not waving it away.

The finding

C6's poison flag lives on PersistedEntryWriter (a Cell<bool>), not in the persisted
WriteBoundState. The module's own restart test proves every other bound survives a freshly
constructed writer over the same store — that is the established, tested usage pattern. The poison
bit is the one field that pattern discards. So if #3265's wiring constructs a writer per cycle,
the save-failure hole C6 exists to close reopens.

Why I am merging anyway — and it is not "it's only test code"

Security's preferred remedy does not work. Moving the poison bit into WriteBoundState means
persisting it to the store whose writes are failing. The bit would need to be written exactly
when save is broken, so it cannot be written. A persisted poison flag is unsettable precisely in
the circumstance it exists for. That is not a smaller version of the fix; it is a fix that fails in
the only case that matters.

The decider's remedy is the real one: reserve-then-spend. Persist the advanced bounds before
calling submit_entry_writes. A failed persist then means the submit never happens, so there is
nothing to poison and no flag is needed at all. The writer's object lifetime drops out of the
guarantee entirely. That is strictly better than either flag, and it requires the store and the
caller to exist in order to be written and tested — neither of which does yet.

And the hole is unreachable at this head, which I verified rather than took on faith:
PersistedEntryWriter::commit has zero production callers (grep for .commit( outside
writes.rs returns only unrelated dig-wallet sqlx transactions); the only non-#[cfg(test)]
WriteBoundStore impl is NoPersistence at writes.rs:248, which errors on both load and
save, so decide returns PersistenceUnavailable long before poison state could matter.
FakeStore and LoadOkSaveErrStore both sit below the #[cfg(test)] at line 426.

So the choice is not "fix it now or ship a hole". It is "write the wrong fix now against an absent
caller, or write the right fix in the PR that introduces the caller". I am choosing the second, and
recording it so nobody has to re-litigate it.

What this costs, made explicit

#3265 now carries reserve-then-spend as its top blocking requirement — persist advanced bounds
before submitting, so a failed persist cannot be followed by a spend. Also added there: implement
and test the ProverState::ChainSourceUnavailable mapping for a poisoned/unavailable store (today
that mapping is documented intent only, which security correctly flagged), and restore the
pure-eviction flush figure the C7 rewrite dropped (~1.3 days at 192 removals/day) alongside the
~2.6-day churn figure, since dropping it understates worst-case flush speed 2×.

The decider's structural point, which I am adopting

"'Inert' has expired." Three gate rounds have all reasoned that merging is safe because nothing
executes — and that argument has now carried a persistence seam, a money-ceiling fix, a poison flag
and a newtype refactor, none of which has ever run against a real chain, store or clock.

#593 is the last PR that may land in crates/dig-node-core/src/rewards/ on the inertness
argument. #3265 must bring a simulator harness exercising a full cycle on the production path
(including restart and store-write failure) and a dry-run reconciled against mod.rs's own
arithmetic, before any staged enablement. Wiring without that evidence should be rejected — the
missing evidence is itself the finding.

Merging on: reviewer PASS, adversarial RATIFY with the leg closed, CI fully green (zero failing,
zero pending), all review threads resolved, and security's finding carried forward as a blocking
requirement on #3265 with the remedy corrected.

Refs #3250