feat(rewards): wire the peer claim loop onto a cadence driver from real startup (#3268)

[MichaelTaylor3d]

Why

dig-node#594 shipped a complete, tested peer reward-claim engine that nothing constructs. At develop the module says so in its own words — rewards_claim/mod.rs carries a section headed "Not yet wired into node startup (Defect D — stated, not fixed here)" whose last line is "this module compiles, is fully tested against the fake chain port, and does nothing in a running node."

So no scheduler calls a cycle, the 86400s cadence never fires, and the anti-silence surface is unreadable. Meanwhile rewards_claim.enabled defaults to true — the node's own config asserts the subsystem is on while nothing can run. That is a surface stating something false about money, the same class as the three defects this epic already corrected.

This PR is the follow-through: the wiring half of DIG-Network/dig_ecosystem#3268.

What

A background cadence driver reachable from the node's real startup path, so rewards_claim.enabled = true comes to mean "a background task exists, drives a cycle every cadence_seconds + jitter, and its outcome is readable in-process as a named ClaimLoopState."

• crates/dig-node-service/src/rewards_claim/driver.rs (new) — the cadence driver plus the tested spawn seam. Shape mirrors self_heal's split rather than inventing one: a private injected-tick drive(...) that is falsifiable under a paused clock, and a pure spawn gate that is itself a tested unit, so a refactor cannot silently flip it to always- or never-spawn.
• crates/dig-node-service/src/server.rs — exactly ONE call in serve_with_shutdown, in the existing spawn block beside spawn_collateral_census(...) and self_heal::spawn_driver_if_service(), gated on enable_chain_sync for the reason those are: that flag already means "this node talks to the Chia network", and a harness sets it false precisely so nothing dials.
• crates/dig-node-service/src/rewards_claim/mod.rs — the now-false "Defect D / not yet wired" module-doc section replaced by what the wiring actually does.

own_payout_puzzle_hash is the CAT-wrapped hash (mirror::funding::dig_cat_puzzle_hash(operator_puzzle_hash)), derived the same public, no-unseal-required way spawn_mirror_passes derives its wallet material, and proven against the engine's own comparison site. This is a money-correctness choice, not plumbing: pick the raw inner hash instead and every distributor is refused as a payout mismatch — a condition this epic has already measured as rendering Nominal.

The only production chain adapter until DIG-Network/dig_ecosystem#3249 lands is UnavailableClaimChainPort, so every real cycle reports ChainSourceUnavailable and submits nothing. That is the honest state and is why landing this now is worth it: it makes the gap loud instead of silent.

Scope — the RPC half is deliberately absent

DIG-Network/dig_ecosystem#3268 carries a non-optional acceptance condition:

#3268 MUST NOT expose ClaimStatus over RPC until the status surface has been re-derived against #3249's real chain adapter.

Three gate passes over #594 found twelve defects, each pass finding new ones inside the previous pass's own remedies, so the ClaimStatus semantics are a reviewed hypothesis rather than a verified surface — and a surface whose job is to report "the peer is earning nothing and here is why" cannot be validated against a port that can only ever answer one way. This PR therefore makes the status readable in-process only. No RPC method, no dispatch-table row, no handler: rpc.rs and every reward RPC handler belong to #3269's lane.

The bar

"It compiles and is not wired up yet" is not available here — that argument is literally the defect being fixed. The headline test is the inverse of the obvious one: "scheduler running, zero cycles ever fired, nothing reported wrong" must FAIL. The driver exposes an observed cycle count and keeps ClaimLoopState::Idle honest; asserting that spawn returned would be the defect, not the evidence. Test effort goes to restart, clock movement and corrupt persisted state, because across six gate passes on this engine not one of the 25 defects was at the chain seam.

Refs DIG-Network/dig_ecosystem#3268, DIG-Network/dig_ecosystem#3251, DIG-Network/dig_ecosystem#3246.

Draft: the head commit is a salvage checkpoint pushed after a session cap and its build is unverified. CI is the compiler; this is not review-ready until checks are green.

🤖 Generated with Claude Code

[MichaelTaylor3d]

Security audit — PASS

Head SHA audited: 4cc49f2560a90aa5918997170992a8a9c29e171a
Base: develop (merge-base e9f07c41fdcf295d525344722572de92a6d8efe2)
Scope: crates/dig-node-service/src/rewards_claim/driver.rs (new), rewards_claim/mod.rs, server.rs (+11), Cargo.toml, Cargo.lock. Fresh clone, read-only.

Attack surfaces worked

1. Spend ceilings / persisted budget — run_claim_driver_in builds the engine with .with_persisted_fee_window(state_dir, cfg.cadence_seconds) and loads RewardsClaimConfig::load_from(state_dir) fresh on every call into run_claim_driver_in. That call happens exactly once per process lifetime (spawned once from server.rs), so a crash-restart re-reads the SAME persisted window file from disk rather than fabricating a new one in memory — the budget/window logic itself (fail-closed on corrupt/future-dated state) lives unchanged in engine.rs, out of this PR's scope per the brief, and this wiring does not bypass or re-derive it. No live defect found in the wiring's interaction with the ceiling.

2. own_payout_puzzle_hash / destination — driver.rs:214-216 computes mirror::funding::dig_cat_puzzle_hash(owner_inner_puzzle_hash) where owner_inner_puzzle_hash comes from dig_wallet::operator_wallet::operator_puzzle_hash(&dig_wallet::autoseed::default_paths()) (driver.rs:229-241). This is the node's own local operator wallet path, not attacker-reachable, and the CAT-wrapping choice is proven both directions by test (a_distributor_paying_this_nodes_derivation_is_claimable, plus the unwrapped-hash refusal case). No user key enters the node here — only the node's own operator wallet is touched, consistent with the custody boundary. default_paths() is a local filesystem convention, not redirectable by an unprivileged remote principal from what this diff shows.

3. Rate-limiter/budget keyed on attacker input — the driver introduces no new keying; the per-cycle budget shape is entirely engine.rs (unchanged, out of scope, already gated per the ticket's own history). Nothing in driver.rs lets an outside party influence which distributor is visited first or re-key the window.

4. Refusal/status surface as an info channel — confirmed IN-PROCESS ONLY: handle()/ClaimLoopHandle/driver::handle are referenced nowhere outside rewards_claim/driver.rs and rewards_claim/mod.rs's re-export (git grep across the tree). No rpc.rs reference to ClaimStatus or rewards_claim exists in this diff or at head. Nothing leaks the payout puzzle hash or wallet paths over the wire.

5. Startup order / privilege — the spawn call sits in the existing serve_with_shutdown spawn block beside spawn_collateral_census/self_heal, gated on config.enable_chain_sync, matching sibling calls. decide_claim_driver is a pure, fully-tested function (disabled_never_spawns, enabled_but_chain_sync_off_refuses_named, enabled_and_chain_sync_on_spawns) — no path lets it spawn before the gate evaluates. No new file writes/permissions introduced by this PR; persistence paths are inherited from the unchanged engine.rs/config.rs.

6. ring = "0.17" — verified against both develop and PR lockfiles: ring 0.17.14 (same registry, same checksum) was ALREADY resolved transitively (via rustls) before this PR; the PR only promotes it to a direct dependency — no new supply-chain source. OsJitter (driver.rs:178-193) draws from ring::rand::SystemRandom (real OS CSPRNG), returns 0 jitter only on a CSPRNG error (degrades to no-spread, never panics, never a seeded/global RNG). Jitter bound (cadence.rs) is operator-configured (jitter_seconds), not attacker-influenced remotely; a fleet operator setting it to 0 is a self-inflicted, local config choice, not an externally exploitable primitive.

Not a finding, noted for the record

git diff develop pr605 (raw tip-to-tip) shows crates/dig-node-core/src/{lib.rs,rewards/{mod,port}.rs} reverting #606's funder-ownership registry, because PR #605's branch was cut from e9f07c41 (before #606 merged) and never rebased. Checked whether this is a live hazard: #605 makes NO changes to those files relative to their common ancestor, gh pr view --json mergeable,mergeStateStatus reports MERGEABLE/CLEAN, and GitHub's merge/squash computes a real three-way merge (equivalent to a rebase), not a literal reapplication of the raw two-tip diff — so merging will NOT revert #606's work. Confirmed this is a stale-branch artifact of the raw diff view, not a mergeable hazard. Recommend rebasing before merge anyway for hygiene, but it is not a gate blocker.

Not covered

engine.rs, types.rs, port.rs, cadence.rs's prior history, and config.rs were read only as needed to trace the wiring seam — unchanged in this diff, out of scope per the brief (belongs to #3249's re-derivation pass if a defect is later found inside them).

Verdict: PASS.

[MichaelTaylor3d]

ADVERSARIAL GATE (third leg) — CHANGES-REQUIRED @ 4cc49f25

Read at head 4cc49f2560a90aa5918997170992a8a9c29e171a (driver.rs 1013 lines, mod.rs, server.rs:2216, with types.rs/engine.rs read-only for context). This leg attacks the SHAPE and the CLAIM, not style; the correctness and security gates run in parallel and I did not duplicate them.

BLOCKING (one finding, driver.rs only, no design latitude)

The honesty claim rests on a reader that does not exist in the binary. The module doc and the PR body both state that after this change enabled = true means "a task exists, drives a cycle every interval, and its outcome is readable as a named ClaimLoopState". In the shipped process, it is readable by nothing:

• handle() has zero callers outside driver.rs's own tests — no rewards_claim::handle in server.rs or anywhere else in the crate, and no RPC row (correctly forbidden until #3249).
• drive() — the loop around engine.run_cycle(t) / handle.record(...) — emits no tracing event at all.
• engine.rs logs only two fee-window persistence warnings (engine.rs:187, :198) — never the cycle's ClaimLoopState.

So all three tracing:: calls this PR adds (the NoOperatorWallet warn, the Disabled debug, the ChainSyncDisabled warn) fire only on paths where the loop does not run. On the path where it does run — the default path on every node, enabled = true with chain sync on — the subsystem produces exactly the observable output it produced before this PR: silence. Today that silence covers a permanent ChainSourceUnavailable; after #3249 it will equally cover Faulted, PersistedStateCorrupt and ClaimableButNotClaiming.

That is the anti-silence property this epic exists to enforce, missing at the outermost layer — the layer the previous six passes never reached, because until this PR there was no outermost layer to inspect.

Remedy (blocking, in this PR): in drive(), after handle.record(engine.status()), emit one tracing event per cycle naming the state and the cycle count — info! when the state is Nominal, warn! otherwise — with target: "rewards_claim", plus distributors_known and claims_submitted_this_cycle. Then the acceptance sentence is true of a running node rather than only of the test suite. No new state, no new type, one added statement; the existing start_paused tests are unaffected.

The five questions

1. Honesty: a real improvement, but the claim as written is overstated — and the fix is the blocking item above, not the default flip. "enabled = true and nothing is constructed" is the worse lie, because it is unfalsifiable, whereas "a loop runs and can only say ChainSourceUnavailable" becomes true the day #3249 lands. I reject the alternative of defaulting enabled = false: it makes the config truthful only by making the subsystem absent, so #3249 would have to ship a silent behavioural flip that every existing operator's on-disk false then overrides forever — the failure direction there is a network of nodes that never claim, each with a config file saying so in a place nobody re-reads. Land the wiring. But with no log and no RPC, a running-and-impotent loop today does create the new false impression: "claiming is handled" is asserted in the module doc and observable nowhere. The one-line remedy converts the claim from a doc statement into a measured one.
2. EmptyPort → Nominal is a real instance of the pattern, but it belongs to #3249, not here. A peer that discovered zero distributors will never be paid, and Nominal (types.rs:196, "a cycle completed, nothing above is true") is a reassuring reading of that; distributors_known = 0 rides on the same ClaimStatus, so the reading is underdetermined rather than laundered — a reader holding the struct can tell, a reader holding the state name cannot. Two reasons it does not block: types.rs/engine.rs are out of scope and held by a user condition, and with UnavailableClaimChainPort as the only production port a real node cannot reach this reading at all today. One hazard to record, though — the vault page's item 5: zero_cycles_before_the_interval_elapses_then_a_counted_number_after now asserts Nominal as the expected outcome of a zero-distributor cycle, pinning semantics a future pass must change. Follow-on with #3249: a NoDistributorsDiscovered variant, or an explicit comment on that assertion recording the reading as provisional.
3. The four zero-cycle truths are genuinely distinguishable and ClaimDriverRefusal does not move the conflation — but NoOperatorWallet is a process-lifetime latch with no retry. refusal() is None/Disabled/ChainSyncDisabled/NoOperatorWallet and cycles_driven() is monotonic from zero, so the four readings are pairwise distinct (the PR's own test asserts exactly that pairwise-distinctness); no path records nothing, and no latch pins a cycle state — record overwrites status wholesale every cycle, and set_refusal is only ever called on paths that immediately return. The residue: run_claim_driver reads the operator wallet once, and on absence sets NoOperatorWallet and returns forever. A node started before its operator wallet exists — a fresh install, or a key imported later — then never claims for the lifetime of a process that runs for weeks, and the single startup warn! saying so has long scrolled past. That is the prior ChainSourceUnavailable process-lifetime latch, relocated to the driver. Follow-on ticket, not blocking: re-check the operator wallet on the cadence rather than once, or clear the refusal and continue the loop.
4. The chain from server.rs:2216 down to run_cycle is now genuinely covered; the residue is small, and one part of it is money-shaped. the_production_body_drives_counted_cycles_from_a_written_config (driver.rs:928) and the_production_adapter_reports_chain_source_unavailable_by_name (:976) drive the real run_claim_driver_in body — config load, engine construction, drive, counted cycles, and the production port's named state — so the joint is no longer assumed. What remains untested is the five-line run_claim_driver wrapper: default_paths(), operator_puzzle_hash, state_dir(), UnavailableClaimChainPort. Four of those five are inconsequential; the derivation is not, and it diverges from its own cited precedent. spawn_mirror_passes (server.rs:2742-2745) deliberately prefers signer.owner_puzzle_hash() and uses operator_wallet::operator_puzzle_hash(&paths) only as a fallback, with a comment giving the reason: "the key a spend is built for and the address its bonds are observed under cannot be two different values". The driver takes that fallback unconditionally, while its doc describes it as "the same derivation spawn_mirror_passes falls back to" — accurate about the fallback, silent about the preference it skips. Consequence today is bounded (nothing submits, and a mismatch yields PayoutPuzzleHashMismatch, i.e. refusal, not loss), so follow-on, not blocking: either mirror the signer-first preference, or record in own_payout_puzzle_hash's doc why the fallback alone is the correct source for a claim.
5. The shape does not reintroduce the FeeWindowState hazard, but it introduces a milder cousin. The OnceLock holds no transient decision state — a Mutex<ClaimStatus> overwritten wholesale each cycle, a monotonic counter, and a set-once refusal — so nothing here can carry a stale fee window or budget across cycles, and the E0609 structural closure is untouched. The cousin: if the detached task ever panics inside run_cycle, the process-wide handle keeps its last recorded status and a frozen counter forever with refusal() == None, indistinguishable at a single read from "spawned, interval not yet elapsed". It is recoverable across two reads via cycles_driven() and ClaimStatus::last_attempt_at, so it is not a defect today, and the blocking log remedy also makes a dead task visible as an absence of periodic events. Worth one sentence in ClaimLoopHandle's doc once #3249 gives the handle a real reader.

Blocks vs follows on

• Blocks 4cc49f25: the per-cycle tracing event in drive() (the Q1/Q4 finding above). One statement, driver.rs only.
• Follows on, to file against #3249's re-derivation: zero-distributors-as-Nominal and the assertion pinning it (Q2); the NoOperatorWallet no-retry latch (Q3); the signer-vs-operator payout derivation divergence (Q4); the frozen-handle-after-panic note (Q5).

If the correctness gate reaches PASS I do not withdraw: my objection is not to the diff's logic but to its acceptance claim, which is unmeasurable in a running node as written. The remedy is additive and cannot fail the existing suite.

🤖 Generated with Claude Code

[MichaelTaylor3d]

loop-reviewer independent correctness gate — CHANGES-REQUIRED

Head reviewed: ac01370edc7e13310d89457a60342261c75d3e49 (rebased onto current develop by the orchestrator; confirmed content-identical to the originally-briefed 4cc49f2560a90aa5918997170992a8a9c29e171a via git diff 4cc49f25 ac01370e -- <the 4 changed files> = empty).

Method

Fresh clone (develop base + fetched PR head by SHA), read driver.rs in full plus the unchanged engine.rs/types.rs/config.rs/self_heal.rs/Cargo.lock it depends on or mirrors. No worktree reused. No code edited.

Verdict: CHANGES-REQUIRED — one blocking finding

Blocking: the wiring this PR claims ("reachable from node startup") has no observable effect on the shipped binary's output.

drive() (crates/dig-node-service/src/rewards_claim/driver.rs, the loop { sleep; run_cycle; handle.record } body) emits zero tracing:: calls on its running path. I grepped every tracing:: call site in the file at the current head: they are at the NoOperatorWallet refusal branch (run_claim_driver, ~line 232) and the Disabled/ChainSyncDisabled decision branches (spawn_claim_driver_if, ~lines 350/357) — i.e. every log line this PR adds fires only when the loop does NOT run. On the default/success path (operator wallet present, enabled=true, chain-sync enabled), the loop runs forever and produces no distinguishable trace output versus pre-PR #594 (where it also compiled but was never spawned). handle()/ClaimLoopHandle are exported (mod.rs:62) but I confirmed via git grep there is no caller of handle() and no reference to ClaimLoopHandle anywhere outside driver.rs itself (tests included) — so nothing in the shipped binary reads the in-process status this PR builds either.

Net effect: this PR is unfalsifiable from the outside. A future regression that silently stops cycles from firing (e.g. a panic in OsJitter, see low finding below) would look identical in production logs to a healthy node — the exact "inert but green" failure mode this epic (#3246) exists to close, just moved one layer in. I independently confirm this is real and blocking, not a style nit: the ticket's own acceptance bar is "a cycle observably fires from a real startup path," and today that's only true inside the unit-test harness, not the deployed binary.

Suggested fix (does not require touching engine.rs/types.rs, stays inside driver.rs): add one tracing::debug!/info! call inside drive()'s loop body after engine.run_cycle — e.g. logging cycles_driven and the resulting ClaimLoopState — so an operator (or an on-call human grepping logs) can distinguish "never ran" from "ran and is Nominal" from "ran and is refused" without needing the not-yet-built RPC surface. Do not log anything that could leak a payout puzzle hash or amount beyond what's already visible in existing chain state.

Non-blocking findings (recorded, not gating)

1. driver.rs, OsJitter::jitter_seconds (~line 180): bound + 1 overflows if jitter_seconds in the persisted config is ever u64::MAX (no upper-bound clamp in config.rs). Debug build panics; release wraps to % 0 which also panics — either way the detached claim-loop task dies permanently with no restart and no distinguishing log line (compounding the blocking finding above). Low severity: requires local state-dir file write to reach. Suggest checked_add/saturating_add fallback to 0 as a follow-up, not blocking this PR.
2. run_claim_driver: the NoOperatorWallet refusal is set once at startup and never re-evaluated; if an operator wallet is added post-startup without a node restart, the refusal stays permanently stale. Likely acceptable (matches the rest of this function's one-shot-at-startup shape) but worth a one-line doc comment noting the restart requirement.

Confirmed correct (per brief's specific scrutiny areas)

• Clock: next_interval_seconds / restart-skip / future-dated-completion handling is delegated to unchanged, already-tested engine.rs (CycleConditions.future_dated_clock) and re-exercised at the integration level by restart_with_a_recent_completion_skips_via_cadence_not_elapsed, restart_with_an_elapsed_completion_runs_a_cycle, a_future_dated_completion_fails_closed_not_underflowed. No underflow path found.
• Persisted file: run_claim_driver_in deliberately does NOT gate spawn on RewardsClaimConfig::load_from corruption — confirmed correct because engine.rs::run_cycle re-reads the config fresh every single cycle and fails closed (PersistedStateCorrupt) on every path, including post-restart. Comment in the diff explaining this matches the actual unchanged engine.rs behavior.
• Status surface: ClaimDriverRefusal::{Disabled, ChainSyncDisabled, NoOperatorWallet} are three distinct, non-latching truths (only spawn_claim_driver_if's pure decision sets them, and the driver's own Idle/Nominal/etc. is a separate independent field) — they do not collapse into one reassuring Idle, confirmed by direct read of ClaimLoopHandle::{status, refusal} and decide_claim_driver.
• Joint tests: the_production_body_drives_counted_cycles_from_a_written_config (driver.rs) and the_production_adapter_reports_chain_source_unavailable_by_name both assert on handle.cycles_driven() transitioning 0→1(→2), not merely "spawn returned" — confirmed by direct read, satisfies the bar set after the earlier sent-back revision.
• ring = "0.17": confirmed via develop's Cargo.lock (pre-PR) that ring 0.17.14 is already present transitively (via rustls) at the same version — no new dependency source introduced.
• EmptyPort → Nominal (independent read, not a change request — types.rs is out of scope for #605): ClaimLoopState::compute_state's priority ladder (unchanged, pre-existing #594 semantics) puts Nominal as the bottom-of-ladder fallback, so "zero distributors discovered" and "distributors discovered but none currently due" are indistinguishable in the status surface — both read Nominal. I don't think this is fully honest in the everyday-English sense ("nothing to claim right now" vs "this node has never once found a single distributor" are different facts an operator would want told apart), but it predates this PR and is explicitly out of this PR's blast radius. Recording as an observation for whoever eventually revisits types.rs, not a defect in #605.

Not run

Did not wait for Test + coverage/Analyze (rust) (still pending at time of this review, per parent orchestrator's instruction that CI is not this gate's blocking signal — this review is a static/independent correctness read of the diff, not a CI-completion gate).

---

Concur with the security leg (PASS) and independently reproduce the adversarial leg's blocking finding above via direct tracing:: call-site and handle()-caller greps at this head — not just taking it on report.

[MichaelTaylor3d]

Adversarial re-gate: PASS — head 5fce8477 (5fce847770455862f39d022f0a9a9285fb8e1375)

My earlier CHANGES-REQUIRED on this PR was: the honesty claim had no reader in the shipped binary —
handle() had no non-test caller, drive() emitted nothing, and all three tracing:: calls fired only on
paths where the loop does not run. That finding is remedied. Read from a fresh fetch of the blob at this
head (tree 5300289552…, byte-identical to 97ba3160; the amend was message-only, confirmed).

1. Is the reader real, or does it just move the silence?

Real. driver.rs:170 calls log_cycle(&status, handle.cycles_driven()) unconditionally inside drive's
loop body, after handle.record(status) — not inside a branch, not after a ?, and not dependent on
anything run_cycle did internally. The ChainUnavailable early returns inside run_cycle cannot skip it:
drive reads engine.status() after the call returns, so an early-returning cycle is logged exactly like
any other. Level choice is right: Nominal → info!, every other state → warn! (:184-208), which on
a money surface is correct — a peer that is earning nothing is not routine chatter.

The one thing that could still have moved the silence, and the reason I did not take the commit message on
trust: this module uses a custom tracing target "rewards_claim", and there is no other custom
target: anywhere in dig-node — so if the shipped subscriber's filter were crate-scoped, every one of these
records would be dropped and my original finding would simply have relocated into the filter. It is not:
logging.rs::init installs dig_logging 0.2, whose DEFAULT_DIRECTIVE is
"info,hyper=warn,rustls=warn,h2=warn,tower=warn" — a bare global info, which matches an arbitrary
target. So on today's default production path (enabled = true, chain sync on, UnavailableClaimChainPort)
an operator gets a recurring WARN naming ChainSourceUnavailable and the cycle count, once per
cadence + jitter. The tests prove the emission by capturing rendered subscriber output
(a_cycle_that_cannot_claim_warns_and_names_why asserts both WARN and ChainSourceUnavailable on the real
production adapter), not by asserting a call site exists.

2. Does any silent-inertness path remain?

No new one, and no reachable panic left in the changed code. With sanitized_schedule applied at
:381-384 before the engine is built, cadence ≤ 31d and jitter ≤ 31d, so next_interval_seconds cannot
saturate, Duration::from_secs cannot overflow, and OsJitter's bound.saturating_add(1) (:243) cannot
panic for any input the scheduler can now see. record and every handle accessor use
PoisonError::into_inner, so a poisoned mutex is not a second panic path.

Two residual paths, both out of scope here:

• A panic anywhere inside ClaimEngine::run_cycle (read-only, unchanged) still kills the detached task for
  the process lifetime — spawn_claim_driver drops the JoinHandle, there is no supervision, and
  cycles_driven() then freezes with no further warning. Pre-existing, not introduced by this PR.
• NoOperatorWallet warns once at startup and is then permanently inert — that is the
  operator-wallet-read-once latch already filed on DIG-Network/dig_ecosystem#3296. It has not become
  blocking: it emits a named warning naming the condition, which is what my block required.

config.rs reporting unclamped values does not matter for this PR: cadence_seconds / jitter_seconds
have no consumer outside this module, and the driver is the only scheduler that reads them. Sanitizing at the
read is the right seam.

3. Did three rounds of remedy introduce anything new?

One design consequence, not a defect, and it is visible. sanitized_schedule substitutes the documented
default rather than clamping to CLAIM_SCHEDULE_SECONDS_MAX, so an operator who writes
cadence_seconds = 60 days meaning "claim rarely" gets a 1-day cadence — over 31× more often than asked,
in the more-active direction. It is warned at WARN naming field, rejected value, substituted value and max
(:326-341), and at this head nothing is ever submitted (UnavailableClaimChainPort), so there is no spend
consequence today. Two notes for the follow-on, neither blocking:

• prefer clamping to CLAIM_SCHEDULE_SECONDS_MAX over substituting the default — clamping is the
  substitution closest to the operator's stated intent, and matters once #3249's real adapter can spend fees;
• the substitution warns once per process, while the per-cycle line carries the state but not the fact
  that the configured cadence was ignored — so an operator reading a mid-life log sees a cadence they did not
  configure with no nearby explanation.

I also checked the sanitized cadence is the one threaded into with_persisted_fee_window (:400), not the
raw config value — it is, so the fee window and the actual schedule agree. That was the easy way to get this
round wrong.

4. Is the merge argument sound?

One sentence: at this head, rewards_claim.enabled = true means that whenever chain sync is on and an
operator wallet exists, a detached task drives a claim cycle every cadence + jitter — with any
loop-disabling cadence or jitter rejected and warned — and every cycle writes a named, filterable record
to the node's log, today always a WARN naming ChainSourceUnavailable because no chain adapter exists
yet. That sentence is TRUE.

Follow-on only (no blockers): the clamp-vs-substitute choice and the once-per-process substitution warning →
add to DIG-Network/dig_ecosystem#3296; detached-task panic supervision is a separate, pre-existing item.

[MichaelTaylor3d]

RE-GATE (correctness leg) — PASS

Head reviewed: 5fce8477 (5fce847770455862f39d022f0a9a9285fb8e1375), base develop.
Re-pointed from 97ba3160 after a message-only force-push; I verified that independently rather than on report — commit.tree.sha is 530028955201a30aa90d9447a6bebd560f8f5934 on both commits, so the tree, the diff and every line cited below are byte-identical across the two SHAs.

The blocking thread is CLOSED

PRRT_kwDOTHG0ds6hLkkX ("the honesty claim has no reader in the shipped binary") — resolved, with the verification written into the thread. In short:

• driver.rs:171 — log_cycle(&status, handle.cycles_driven()) is the last statement of drive()'s loop body, unconditional, not inside any if/match/?. No path a cycle takes can skip it.
• The startup chain is unbroken and ungated by cfg: server.rs:2216 → spawn_claim_driver_if → spawn_claim_driver → run_claim_driver → run_claim_driver_in → drive → log_cycle.
• The historical hazard this thread named — run_cycle's ChainUnavailable early returns skipping the end-of-function assignment block — cannot make the new line lie: engine.rs:215 resets every per-cycle field at the top and assigns self.status.state immediately before every early return (:292, :315, :344, :410, :470). engine.status() is therefore always this cycle's state.
• The proof is a capture, not a call-site assertion: capture_logs() (:1162) installs a real tracing_subscriber::fmt over an in-memory writer; a_driven_cycle_emits_an_event_naming_its_state (:1177) asserts the buffer is EMPTY before the interval elapses, advances the paused clock, then asserts the rendered bytes contain Nominal, cycles_driven=1 and the rewards_claim target. Revert log_cycle's call and the first contains fails against an empty string — non-vacuous. a_cycle_that_cannot_claim_warns_and_names_why (:1231) repeats it through the real UnavailableClaimChainPort, which is today's production path, and asserts WARN.

The acceptance bar is met on its own terms: a cycle fires from the real startup path, and it is now paired with an operator-visible signal — warn! for every non-Nominal state, which with UnavailableClaimChainPort is every real cycle today.

The three new commits, reviewed as a fresh diff against ac01370e

e27f6d2d log_cycle (:161-208) — no finding. Reads &ClaimStatus and a u64; no arithmetic, no unwrap, no allocation on the hot path; info! only on Nominal, warn! otherwise. handle.record() precedes it so the logged cycles_driven always includes the cycle being reported. There is exactly one driver task per process (spawn_claim_driver_from_config is called once, server.rs:2216), so the record-then-load pair has no interleaving writer.

d1f9cac9 bound.saturating_add(1) (:230) — no finding. The draw stays in 0..=bound for every u64, including u64::MAX, and the bound == 0 guard above still short-circuits, so % 1 is unreachable rather than merely harmless.

97ba3160/5fce8477 sanitized_schedule (:305-345), applied at :381-384 — no blocking finding. I weighted the substitution semantics specifically:

• Can a substitution change behaviour the operator did not ask for and cannot see? It changes it, and it is visible: each branch emits warn! naming field, rejected, substituted and max before returning the default. Nothing is accepted silently, and the default is not accepted silently either.
• The sanitized cadence is what reaches with_persisted_fee_window(state_dir, cadence_seconds) at :400 as well as drive at :404-405. That is the correct pairing and worth stating, because the alternative is a live defect: had the engine kept the raw value while drive used the substituted one, the engine's cadence gate would have returned CadenceNotElapsed on every cycle forever — a loop that fires, logs, and never does any work. The two agree.
• A zero cadence is rejected (it would busy-loop the engine) and a zero jitter is deliberately preserved as legitimate — asserted by an_out_of_range_cadence_is_replaced_by_the_default_and_warned, which checks jitter == 0 survives untouched.
• No new overflow: with both fields bounded by 31 days, next_interval_seconds' sum cannot approach u64::MAX, so its saturation is now unreachable rather than load-bearing. No new panic path in non-test code; every unwrap/expect added is inside #[cfg(test)].
• an_out_of_range_jitter_is_replaced_and_the_loop_still_drives_cycles (:1342) is the strongest of the new tests: it persists jitter_seconds = u64::MAX to a real config file and drives the real production body (run_claim_driver_in, real OsJitter), then asserts cycles_driven() >= 1 within cadence + CLAIM_JITTER_SECONDS_DEFAULT. Without the sanitizer the interval saturates and the count stays 0 — the test fails for the right reason.
• ring = "0.17" enters as a real dependency (not dev), which is correct for OsJitter; tracing-subscriber = "0.3" and tempfile = "3" were already [dev-dependencies] (Cargo.toml:373, :393), so the log-capture harness adds nothing to the shipped binary.
• mod.rs's module doc no longer claims "does nothing in a running node" — doc and code now agree, and the "no wire surface" clause is still true (nothing added to any dispatch table).

Findings

Sev	Location	Finding
Low (non-blocking, resolved by me)	driver.rs:304 / :367	The new const's /// block was inserted directly beneath run_claim_driver_in's doc comment with no blank line, so Rust joins them into one doc comment: the const's rustdoc opens with prose about a function and its generic P, and run_claim_driver_in — the function that reads the operator's config and applies the sanitizer — is left with no doc comment at all. Doc-only, no observable effect, invisible to clippy. Fix is a blank line plus moving the block back above :367. The inline thread carries the full instruction.

Nothing blocking. an_unclamped_max_jitter_bound_does_not_panic_the_driver (:1281) asserts offset <= bound where bound == u64::MAX, which is tautological as a range check — it is still a valid panic test (the pre-fix bound + 1 panics before the assert is reached), so I am not filing it; noted only so nobody later mistakes it for a range proof.

Criteria checked, one by one

1. Reader on the default production path — yes, driver.rs:171 via server.rs:2216, proven by captured subscriber output.
2. log_cycle unskippable on every path a cycle takes, including run_cycle's early returns — yes, verified in engine.rs (out-of-scope file, read-only, read for this purpose).
3. Test vacuity, all four new tests — each fails with only its fix reverted; named above per test.
4. New panic / overflow paths — none in non-test code.
5. Substitution observability on a money path — warned, naming field, rejected value, substituted value and max.
6. Scope — the diff touches only driver.rs, mod.rs (doc + one pub use), server.rs (+11, one call and its comment), Cargo.toml (+6), Cargo.lock (+1). engine.rs / types.rs / port.rs / cadence.rs / config.rs are unchanged, as declared.
7. Readable-code bar — grep -cE '[^ ] {10,}[^ ]' driver.rs = 0; the 7 lines over 100 columns are all //////! prose, which rustfmt does not wrap; the file is valid UTF-8 (57,736 bytes, no cp1252 stragglers).
8. Not re-reported, per brief: the EmptyPort→Nominal fall-through, the operator-wallet read-once latch, and server.rs:2742-2745's payout-destination inconsistency — all on dig_ecosystem#3296. Also not treated as gaps: no ClaimStatus RPC (a user-written condition until #3249), no real chain adapter.

What I did NOT run

cargo test -p dig-node-service, cargo clippy --all-targets -- -D warnings and cargo build. No warm target/ for this crate exists outside another lane's worktree, and using one would mutate a shared checkout; a cold build of this dependency tree does not fit this gate's budget. The reported 21-driver-test / clean-clippy result is therefore unverified locally by me and rests on CI, which the dispatching orchestrator owns. Every claim above comes from reading the blobs at 5fce8477, not from an execution. If CI is red on this SHA, this PASS does not survive it.

Verdict: PASS at 5fce8477. Zero open review threads.

Same-identity review, so this is posted as a COMMENT, not an approval — GitHub returns 422 for --approve from the author identity.