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]

BLOCKING: drive()'s loop body (sleep -> run_cycle -> handle.record) emits no tracing:: call on this, the success/running path. Every tracing:: call this PR adds fires only when the loop does NOT run (NoOperatorWallet/Disabled/ChainSyncDisabled). handle()/ClaimLoopHandle also have no caller outside this file's own tests (confirmed via git grep). Net: nothing in the shipped binary lets an operator distinguish 'never ran a cycle' from 'ran and is healthy' -- the exact inert-but-green failure this epic exists to close, moved one layer in. Suggest one tracing::debug!/info! here logging cycles_driven/resulting ClaimLoopState after run_cycle, without leaking payout puzzle hash/amount beyond what's already visible in chain state.