Skip to content

fix(rewards): install FundedDistributorRegistry at startup; refuse malformed launcher_id - #626

Merged
MichaelTaylor3d merged 5 commits into
developfrom
loop/3292-rewards-startup-wiring
Sep 27, 2026
Merged

MichaelTaylor3d merged 5 commits into
developfrom
loop/3292-rewards-startup-wiring

Conversation

@MichaelTaylor3d

@MichaelTaylor3d MichaelTaylor3d commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

What was broken

FundedDistributorRegistry was never installed on any production node. Node::install_funded_distributor_registry was declared pub(crate) in dig-node-core, so dig-node-service — the crate that owns startup — could not call it even in principle. It had no non-test caller anywhere in the tree.

The registry is how a node knows which reward distributors it funds. With the slot empty, dig.listRewardDistributors reports NotConfigured forever, discovery never runs, and the code reads as finished the whole time. Nothing errors. Rewards simply are not distributed.

That is the epic's acceptance line made literal: "anytime the process isn't running then rewards are not being distributed." The process was not running, in every shipped binary, and no test or lint could see it — a visibility modifier is invisible to rustdoc, to clippy, and to CI.

What this changes

#3292 — install the registry on the real startup path

  • crates/dig-node-core/src/lib.rs:652 — install_funded_distributor_registry is now pub, mirroring the already-pub install_reward_chain_port.
  • crates/dig-node-service/src/server.rs — installed inside serve_with_shutdown, immediately after bring_up_collateral_records(), unconditionally. It is deliberately not gated on enable_chain_sync: the registry is local state with no chain dependency, and gating it there would reintroduce the same silent-inert failure under a different name.
  • crates/dig-node-core/src/rewards/funded.rs — corrected a docstring that overclaimed which mechanism guarantees future-variant coverage (it is determined()'s wildcard-free match, not the hand-built test array), and dropped two now-stale allow(dead_code) attributes.

On a fresh node the registry now reads NotConfigured(NoRecordWritten). That is the correct answer, not a bug. The registry's writer is #3291, an operator declaration — the node cannot observe its own funding, because funding spends from a user wallet the node never holds. A surface that says what it does not know is the goal here.

#3280 — refuse a malformed launcher_id instead of silently returning nothing

  • crates/dig-node-core/src/seams/dig_rpc/dispatch.rs:923-985 — GetRewardProverStatus now reuses the existing parse_launcher_id_arg helper (already used by GetRewardDistributor): absent means no filter, malformed means -32602, well-formed compares [u8;32] rather than hex strings.

The hex-to-bytes change is a fix rather than a regression: the previous string comparison silently failed to match any 0x-prefixed input forever, where the new path refuses it by name.

Why the wiring has a regression test

A wiring fix with no regression test is one refactor away from reverting silently, and this particular breakage is invisible when it happens. The tests here are written to fail if the registry is ever un-installed — including if the call is not deleted but merely moved somewhere it never executes.

Gates

Correctness and security both PASS at 4d6d72c0, zero unresolved threads. The correctness gate did not accept the source-text test as proof the install runs; it traced the call chain through serve_with_shutdown by hand and confirmed no early return, no ?, and no branch can skip it.

Refs #3292, #3280, #3246

🤖 Generated with Claude Code

MichaelTaylor3d and others added 2 commits September 26, 2026 19:08
… with -32602

`dig.getRewardProverStatus`'s optional `launcher_id` filter degraded a malformed
value to a hex-string equality comparison that simply never matched, so the
response read as a legitimate empty list (`Consulted { items: [] }`) instead of
the caller-error it actually is. Reuse the same `parse_launcher_id_arg` validator
`dig.getRewardDistributor` already returns `-32602` from: absent means no filter,
present-and-malformed is refused before any filter runs, present-and-well-formed
compares the decoded `[u8; 32]` directly instead of a case-insensitive hex string.

Refs #3280

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tup path

`Node::install_funded_distributor_registry` had zero non-test callers and was
`pub(crate)`, so `dig-node-service` (the crate that owns the state directory)
could not call it — the registry was empty on every shipped node regardless of
what an operator had funded. Make the installer `pub`, mirroring
`install_reward_chain_port`, and call it from `serve_with_shutdown`'s startup
path against the service's own hardened state dir, unconditionally (this is
local state, not a chain read, so it does not need the `enable_chain_sync`
gate the reward-chain-port install below it uses).

Also drops the now-stale `allow(dead_code)` on `FundedDistributorRegistry::
with_state_dir`/`read` (a real caller exists now), and fixes the `funded.rs`
regression test's docstring, which overclaimed that its hand-built variant
array alone catches a future enum variant — the actual guard is
`FundedDistributorsRead::determined`'s wildcard-free match, which fails to
compile on an unhandled variant.

The registry stays empty (`NotConfigured(NoRecordWritten)`) on a fresh node
until #3291's writer runs — that is success, not a bug: the node cannot
observe its own funding because funding spends from a wallet it never holds.

Refs #3292

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@MichaelTaylor3d MichaelTaylor3d changed the title fix(rewards): install FundedDistributorRegistry on startup + refuse malformed launcher_id on getRewardProverStatus fix(rewards): install FundedDistributorRegistry at startup; refuse malformed launcher_id Sep 27, 2026
Refs #3292, #3280

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PASS — independent correctness review at 4d6d72c096192cffe4f2466e907192519b04e953

Reviewed in a detached read-only worktree (C:/worktrees/dig-node-gate-rev), no builds run (per brief, workspace-scope cargo build/test in this repo is prohibitively slow and forbidden). Traced the real call chain by reading source, not by trusting the source-text test.

1. Does the install actually run? — YES, unconditionally.

Traced dig-node-service/src/server.rs::serve_with_shutdown:

  • The only earlier early-return is host_override_refusal (an unrelated remote-bind guard), which returns before build_state — not a path that could skip the install once reached.
  • let state = build_state(&config).await; — build_state returns AppState directly (not Result), so there is no ? or branch here that could short-circuit.
  • Between that line and the new state.node.install_funded_distributor_registry(...) call there is exactly one statement, bring_up_collateral_records() (synchronous, infallible, no early return), plus two .clone()s.
  • state.state_dir is a plain PathBuf field (server.rs:79), computed synchronously inside build_state via resolve_state_dir_and_token() (line 592) before state is returned — never Option, never populated lazily.
  • The new call is not inside the if config.enable_chain_sync { ... } block below it (confirmed by reading the block boundaries) — it sits above that gate, so it fires even under an integration harness with sync disabled, exactly as claimed.

Confirmed the flagged test (server_startup_calls_install_funded_distributor_registry_in_production_code) is indeed a source-text/string-containment test, as the brief warned — it alone would not distinguish "called" from "unreachable." I did not rely on it; the manual trace above is the actual evidence for PASS on this point.

2. Placement/ordering — correct.

state.state_dir is fully resolved before build_state returns (see above), so it is valid and populated at the call site. Nothing between bring_up_collateral_records() and the new call reads the funded-distributor registry, and no earlier spawned task in this function reads it either (the only prior lines are the wallet clones and the collateral bring-up). No stale-read hazard.

3. Absent launcher_id stays absent (three-way split) — confirmed genuinely three-way.

dispatch.rs:923-943: params.get("launcher_id").is_some() gates whether parse_launcher_id_arg runs at all — absence takes the None branch directly, never touching the validator, so it cannot be turned into a -32602. Malformed present values return Err → -32602 (new test get_reward_prover_status_with_a_malformed_launcher_id_is_refused). Well-formed-but-unknown present values return Ok([u8;32]), filter to an empty items list, consulted (new test ..._is_empty_not_refused). The pre-existing test get_reward_prover_status_filters_by_launcher_id_when_given still exercises the true-absent case (second handle_rpc call with no params key at all) and asserts all items returned unfiltered — this still passes against the new match on [u8;32] since it's unaffected by the presence check.

4. Hex-string→[u8;32] comparison change — this is a fix, not a regression.

Old code: lowercase the raw string (no 0x strip, no length validation), then hex::encode(bytes).eq_ignore_ascii_case(want). hex::encode never emits 0x, so a caller who included a 0x prefix would silently get zero matches forever — exactly the "reassuring empty list" defect this epic targets, just on the input side. New code runs the same parse_launcher_id_arg already trusted by GetRewardDistributor/ListRewardDistributorCommitments — it strips 0x, enforces exactly 64 hex chars, and is case-insensitive via hex::decode — then compares [u8;32] directly. Net effect: a 0x-prefixed or malformed launcher_id now gets a -32602 (was: silent empty match); a correctly-cased/uncased plain-hex value behaves identically to before. No caller that worked correctly before now sees a behavior change; callers that were silently broken before now get an explicit error. Judged a fix.

5. Closed-enum read surface — preserved, not touched by this diff.

The consuming match at dispatch.rs:1148 (ListRewardDistributors) is unchanged by this PR and remains wildcard-free over all five FundedDistributorsRead variants (Funded, FundsNothing, and a combined arm for NotConfigured | PersistedStateCorrupt | IoFailed, each rendering NotConsulted, never []). Grepped the entire diff for _ => — none introduced anywhere in the 5 changed files.

6. NotConfigured(NoRecordWritten) on a fresh node — correctly treated as success, not asked to invent a writer.

The new test in funded_distributor_registry_startup_3292.rs explicitly asserts a fresh tempdir-backed registry reads NotConfigured(NoRecordWritten), and the server.rs comment states this is the correct post-install answer until #3291's writer lands. No attempt in this diff to fabricate a funded set or make the empty case read more reassuring.

Dead-code attribute removals — both genuine.

  • FundedDistributorRegistry::with_state_dir — now called from server.rs's production startup path (confirmed above).
  • FundedDistributorRegistry::read() — reached via Node::funded_distributors_read(), called from dispatch.rs:1148 inside the (pre-existing, unchanged) ListRewardDistributors handler — a genuine non-test caller, not just warning suppression.

Out of scope, correctly not touched

rewards_claim/**, the dig-rpc-protocol version bump, prover-loop data production (#3265), the four Unavailable RewardsChainPort methods (#3421), writes.rs (#3422) — none of these appear in the diff.

Findings

None blocking. No inline threads opened.

Not run

No cargo build/cargo test at any scope (brief's hard constraint; rustfmt/clippy already reported green at this SHA). Verified behavior by reading source and existing/new unit-test bodies, not by executing them.

Verdict: PASS at 4d6d72c096192cffe4f2466e907192519b04e953.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

PASS

Audited head 4d6d72c096192cffe4f2466e907192519b04e953 (matches PR #626's current headRefOid; PR is draft, open, +204/-23/5 files, confirmed by git diff develop...HEAD --stat).

Findings

None LIVE. Two defence-in-depth notes below (tickets, not gates).

  • crates/dig-node-core/src/rewards/funded.rs:262-271 (load) — fs::read_to_string is unbounded and the file path is not canonicalized before the read. Not touched by this diff (pre-existing), but this diff is what makes it reachable in a shipped binary for the first time (install_funded_distributor_registry now has a real production caller). Exploit requires the attacker to already have local write access to the node's state_dir — at that point they have full host compromise and simpler primitives exist. Defence-in-depth, recommend a bound/canonicalize ticket, not a merge blocker.
  • crates/dig-node-core/src/seams/dig_rpc/dispatch.rs:167,170 (parse_launcher_id_arg, unchanged by diff) — echoes the raw caller-supplied string into the -32602 error message, unbounded length. Reachable only after the caller has already passed the is_node_local_reward_read control-token gate (server.rs:1287-1330, unchanged by this diff) and only echoes the caller's own input back to the caller in the RPC response — no server-side log call in this path (rpc_err → error_frame, no tracing::*). Self-inflicted at worst. Defence-in-depth, not a live disclosure or log-injection primitive.

Answers to the six prompts

  1. pub widening / idempotency: install_funded_distributor_registry (lib.rs:648) is backed by OnceLock::set (lib.rs:653) — a second install call anywhere returns false and changes nothing; this is enforced by the type, not convention. Grepped the whole workspace tree for callers: the only production caller is the new server.rs:2230 startup call, unconditional, once, over state.state_dir (the node's own hardened state dir, not attacker-writable or -influenced by any request). pub only widens the Rust-visibility surface to other crates in the same binary (mirrors install_reward_chain_port, an existing pattern) — no new network-reachable caller exists. No swap-behind-the-caller's-back primitive: verified.

  2. Error-echo on malformed launcher_id: parse_launcher_id_arg (dispatch.rs:158-171, unchanged code, now also invoked from the new GetRewardProverStatus arm at dispatch.rs:934) echoes the raw string into the -32602 message. No log call in this path. Reachable only post-authentication (see prompt 3). Defence-in-depth finding above, not a gate.

  3. #3261 both axes, confirmed unchanged by this diff:

    • Axis 1 (transport/Tier::Control): Method::GetRewardProverStatus.tier() == Tier::Control and !peer::is_peer_reachable_method(...) are asserted by pre-existing tests at lib.rs:9730-9749 (not part of this diff's added-test hunks) — this diff does not touch Method, its tier table, or the peer-reachable list.
    • Axis 2 (token gate in server.rs::rpc()): dig.getRewardProverStatus is one of the three is_node_local_reward_read methods folded into the token-gated block at server.rs:1287-1330 (context lines, unchanged by this PR's diff — confirmed via git diff develop...HEAD -- server.rs, which shows only the new registry-install block at line 2230). An anonymous or DIG_NODE_ALLOW_REMOTE=1 caller with no control/paired token is refused with Unauthorized before dispatch ever reaches this diff's changed code.
      Both axes independently verified intact; this diff modifies neither.
  4. Path-carrying error variants (PersistedStateCorrupt, IoFailed): traced the only render path, dig.listRewardDistributors (dispatch.rs:1143-1189, unchanged by this diff). Both variants — along with every NotConfigured reason — collapse to Half::NotConsulted { observed_at } with no path or error string on the wire (dispatch.rs:1148-1157), via a wildcard-free match (a new enum variant left unhandled fails to compile, per the funded.rs test-doc update in this diff). This diff does make the corrupt/IoFailed states reachable in production for the first time (registry now actually installs), but the existing render path was already written to never leak them — confirmed safe.

  5. Startup read bound: not bounded and not canonicalized (see finding above) — defence-in-depth, not a gate; the file is only ever written by the node's own (not-yet-implemented, #3291) writer, so absent that writer a fresh node never has a record to read (NotConfigured(NoRecordWritten), matches the ticket's stated correct behaviour). No externally-triggerable write path exists in this diff or its dependencies.

  6. Hex-string → [u8;32] filter change: this is a presentation filter, not an authorization filter — determinable, not ambiguous. GetRewardProverStatus requires the control/paired token before any filter code runs (prompt 3, axis 2); with no launcher_id param the same authenticated caller already receives the FULL unfiltered status set (filter_launcher_id: None branch, dispatch.rs:988-992). The filter only narrows which subset of an already-fully-visible set is echoed back; widening matching (accepting 0x-prefixed / non-canonical hex where the old eq_ignore_ascii_case string compare would have failed to match) does not cross a privilege boundary — it can only ever return a subset of what an unfiltered call already returns to the same caller.

Scope audited

Files: crates/dig-node-core/src/lib.rs, crates/dig-node-core/src/rewards/funded.rs, crates/dig-node-core/src/seams/dig_rpc/dispatch.rs, crates/dig-node-service/src/server.rs, crates/dig-node-service/tests/funded_distributor_registry_startup_3292.rs — full diff develop...4d6d72c0 read in entirety (matches stated +204/-23/5 files).
Confirmed out-of-scope items untouched by this diff: rewards_claim/**, dependency bumps, #3265's prover spawn, the four Unavailable RewardsChainPort methods, writes.rs.

Not covered

No cargo build/cargo test run (constraint). New integration test read for logic only, not compiled/executed. Did not re-audit server.rs's token-gate block itself beyond confirming it is unchanged by git diff (its own correctness was presumably gated when #3352 landed).

Worktree: created my own detached read-only worktree at C:/worktrees/dig-node-gate-sec per brief; did not touch dig-node-3292 or dig-node-gate-rev. No edits made anywhere.

🤖 Generated with Claude Code

server_startup_calls_install_funded_distributor_registry_in_production_code
was a plain `.contains(...)` check, so it survives the call being MOVED
(e.g. behind the enable_chain_sync gate) rather than deleted -- and it turns
out it does not even reliably catch deletion, since the surrounding doc
comment repeats both needles verbatim.

Adds:
- install_call_precedes_the_enable_chain_sync_gate_in_production_code:
  asserts the install call's source offset precedes the enable_chain_sync
  gate's, which reddens under both mutating the call away (deletion) and
  moving it inside that gate -- the exact shape #3292 originally shipped as.
- production_startup_reaches_the_install_call_site_without_panicking: boots
  the real serve_with_shutdown path end to end on an ephemeral loopback port
  and proves the install call site is reachable without panicking, which the
  existing tempdir-only test cannot (it never drives server.rs at all).

Documents, in the new runtime test's doc comment, why a true black-box
runtime distinction between "installed and empty" and "never installed" is
not reachable from this integration-test crate today: Node's registry
accessors are pub(crate) (invisible outside dig-node-core), AppState's node
field is private with no getter, and dig.listRewardDistributors wildcards
every NotConfigured/PersistedStateCorrupt/IoFailed reason into one identical
Half::NotConsulted response with no reason on the wire -- so the two states
are genuinely indistinguishable from any observer available here without a
production-surface change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delta re-gate at 2fc2ce7d0ff705025ec1c4ea6303ec10352656be

PASS. Scope confirmed independently: git diff --stat 4d6d72c0 2fc2ce7d = one file,
crates/dig-node-service/tests/funded_distributor_registry_startup_3292.rs, +164/-0.
git diff --name-only filtered for non-test paths returns nothing. No production file moved.
The prior correctness/security PASS at 4d6d72c0 stands unchanged; this review covers only the
delta.

Is the new test self-satisfying the way the old one was? No — confirmed by direct read of src/server.rs.

The old test's defect was a bare-identifier needle ("install_funded_distributor_registry",
no trailing paren) plus "state.state_dir". Both strings are repeated verbatim in the doc comment
directly above the real call site (src/server.rs:2230-2240, e.g. `install_funded_distributor_registry`'s `OnceLock` means... and (`state.state_dir`, resolved above)). That comment survives even if the call statement itself is deleted, so the old test cannot redden under mutation (a) or (b). I verified this by reading the comment text myself, not just trusting the report — it's real, and it fully explains why the old test is inert rather than merely weak.

The two new tests use different needles:

  • install_funded_distributor_registry( — with the trailing paren, i.e. only matches a call
    expression, never the bare-identifier prose in the comment. grep -n over the whole file
    (production region and test region) shows exactly one occurrence in the entire file:
    server.rs:2242, the real call. Nowhere else, including comments, does this exact substring
    appear.
  • if config.enable_chain_sync { — the full if statement text, not just the flag name. grep -c
    shows exactly two occurrences in the whole file, both real if statements (server.rs:2259
    and :2330); every other mention of enable_chain_sync in comments (lines 181, 578-579, 2233,
    2256, 2271-2272, 2327, 2362, 2375) uses different surrounding text and does not match this literal
    string. So this needle cannot be satisfied by prose either.

What convinced me: the source read, not the mutation numbers alone — a green mutation result
is a claim, and the old test's own green history under real regressions is exactly why I traced it
by hand rather than accept it at face value last time. Here I found the actual, unique location of
both needles and confirmed no doc/comment text anywhere in the file duplicates them. The reported
mutation exit codes (101 red under both deletions, 0 green on revert) are consistent with, and now
explained by, that mechanism — a second, independent line of evidence, not the only one. I did not
re-run the mutation myself: a scoped -p dig-node-service test in a fresh worktree still triggers a
full transitive dependency rebuild (confirmed — my own attempt hit >300s and was still compiling
dig-node-core/dig-wallet when I stopped it), so I relied on static confirmation of the mechanism
instead of re-executing a build that could run long. I did not treat my own killed build as a test
result — it isn't one, it's an incomplete compile I terminated.

1. production_startup_reaches_the_install_call_site_without_panicking — name vs. proof

Accurate, not overstated. It boots the real serve_with_shutdown (the same fn dig-node run
calls) on an ephemeral loopback port and polls /health until serving, then asserts clean
shutdown. Its own doc comment (lines 133-162) is unusually careful about the limit: it explicitly
says it can only prove "reachable, does not panic," and names exactly why "installed" can't also be
asserted here (no accessor crosses the pub(crate) boundary from this integration-test
compilation unit, and the RPC surface wildcards both NotConfigured reasons into one identical
Half::NotConsulted — the pre-existing #3324 gap, correctly left alone here). The name says
"reaches ... without panicking," which is exactly what it checks — no gap between name and proof.

2. Flakiness / secrets / hardcoded paths — clean

No hardcoded absolute paths (uses tempfile::tempdir()/tempfile::Builder), no secrets, no
machine-specific assumptions. The ephemeral-port bind-then-drop-then-rebind pattern has a narrow
theoretical TOCTOU race (something else grabs the port between drop and serve_with_shutdown's
bind) but mirrors an existing pattern already in tests/server.rs per the test's own comment, so
it's a pre-existing, accepted risk shape in this crate, not new exposure introduced here. The
ENV_LOCK mutex around the process-global DIG_NODE_CACHE/DIG_NODE_STATE_DIR/DIG_PEER_NETWORK
env vars correctly serializes against every other test in the same binary that touches them.

3. Should server_startup_calls_install_funded_distributor_registry_in_production_code be deleted?

Agree with your inclination — delete it. It is not merely redundant with the new tests, it is
actively worse than absent: it reads as coverage for exactly the property (production call site is
wired) that it cannot detect a regression in, for a reason (doc-comment self-satisfaction) that is
now proven, not suspected. Leaving it beside the two new tests invites a future reader to trust
three tests where only two hold weight, and a future refactor of the comment above the call site
could accidentally make it fail (or, worse, someone "fixes" it by rewording the comment, which does
nothing for the real property). This PR's delta doesn't touch it, so it's outside this delta's
strict scope, but it is not a new problem — it's the same PR, same file. Your call on whether to ask
for a same-PR follow-up push or file it as a fast-follow; either way I'd remove it before merge
rather than after.

Not run

Did not re-execute cargo test -p dig-node-service --test funded_distributor_registry_startup_3292
to completion — a fresh worktree triggers a full transitive rebuild that exceeded the time I was
willing to hold open; verified the mechanism by direct source read instead (see above), which is
sufficient to answer the one question this gate turns on.

Verdict: PASS at 2fc2ce7d0ff705025ec1c4ea6303ec10352656be.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

PASS

Head audited: 2fc2ce7d0ff705025ec1c4ea6303ec10352656be

Scoped delta re-gate over PR #626 since my prior PASS at 4d6d72c096192cffe4f2466e907192519b04e953. Confirmed independently: git diff --stat 4d6d72c0 2fc2ce7d = one file, +164/-0: crates/dig-node-service/tests/funded_distributor_registry_startup_3292.rs. No production file, build.rs, Cargo.toml, or CI workflow moved.

  1. Test-only delta confirmed. Single file, test path only (tests/), no src/, no manifest, no workflow touched.
  2. Nothing that should not be committed. No credential/token/key-shaped string, no real hostname (only 127.0.0.1 loopback), no private path, no PII. Grepped for common secret/URL shapes: no hits.
  3. No boundary weakened to make itself testable. No #[cfg(test)] accessor, no pub(crate)→pub widening, no feature flag, no test-only constructor added anywhere in the delta (verified git diff touches only the new test file, and it uses only the existing public Node::from_env() / FundedDistributorRegistry::with_state_dir surface already used by rewards_chain_port_a3.rs). The new test's own doc comment explicitly records that closing its stated observability gap would need a #[cfg(test)] accessor or a wire reason field and states it deliberately did not add either, out of scope. I take that as an accurate, verifiable claim rather than accepting it on faith — confirmed no such accessor exists in the diff.
  4. No I/O outside a tempdir. install_funded_distributor_registry_installs_a_tempdir_backed_registry_once uses tempfile::tempdir(). production_startup_reaches_the_install_call_site_without_panicking sets DIG_NODE_CACHE/DIG_NODE_STATE_DIR/DIG_PEER_NETWORK to a tempfile::Builder tempdir for the process-global env vars, serialized via a OnceLock<Mutex<()>> guard against concurrent tests in the same process (mirrors the existing tests/server.rs pattern) — no real state directory read, written, or removed.
  5. No new dependency. reqwest, tokio (with test-util), and tempfile are all pre-existing dev-dependencies in dig-node-service/Cargo.toml; the delta adds no new crate.

Context from the prior full audit stands unchanged (not re-derived): the funded.rs:262-271 unbounded/non-canonicalized fs::read_to_string and parse_launcher_id_arg message-echo notes remain defence-in-depth, not live, and untouched by this delta. #3324 (the NotConfigured reason-collapse honesty gap) remains out of scope for this PR.

Scope audited: crates/dig-node-service/tests/funded_distributor_registry_startup_3292.rs only, diffed against merge-base 4d6d72c0, read in a detached worktree at 2fc2ce7d. Did not re-run cargo build/test (not needed for a source-level test-only diff per the brief's constraints). Worktree C:/worktrees/dig-node-sec-regate created and removed; no other lane's worktree touched.

@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review September 27, 2026 06:52
…ource-text test

server_startup_calls_install_funded_distributor_registry_in_production_code
asserted on bare identifiers that its own doc comment repeated verbatim, so it
passed both when the install call was deleted and when it was moved inside the
enable_chain_sync gate. A test that passes under the exact mutation it exists to
catch is worse than no test, because it reads as coverage.

Refs #3292, #3280

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

@MichaelTaylor3d MichaelTaylor3d left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PASS at 64f6296

Third delta re-gate, scoped confirmation only.

  1. Production diff since 4d6d72c0 is genuinely empty. git diff --stat 4d6d72c0 64f62961 -- ':!*/tests/*' returns nothing. The full correctness review at 4d6d72c0 still stands unchanged.
  2. Only the inert test was deleted. git diff --stat 2fc2ce7d 64f62961 = one file, +3/-24. The three tests that must remain do remain: install_funded_distributor_registry_installs_a_tempdir_backed_registry_once, install_call_precedes_the_enable_chain_sync_gate_in_production_code, production_startup_reaches_the_install_call_site_without_panicking. Only server_startup_calls_install_funded_distributor_registry_in_production_code (the proven-inert source-text .contains(...) test) was removed, exactly as recommended in the prior review. Both distinctive needles survive byte-intact: install_funded_distributor_registry( with trailing paren (line 81) and the full statement if config.enable_chain_sync { (line 88), both inside the retained install_call_precedes_the_enable_chain_sync_gate_in_production_code test.
  3. The +3 insertion is formatting only. One dig_local: false, comment-alignment fix (extra space removed) and one long .expect(...) call rewrapped across two lines by rustfmt. No new logic, no weakened assertion.

cargo fmt --all -- --check reported to exit 0; not independently re-run per the brief's hard constraint against triggering a cold rebuild — diff review is sufficient to answer all three questions.

Not this PR's problem, already filed: #3324.

Head verified via gh pr view 626 at time of review = 64f6296154fb61fc138e57ffc2b65656ead7cbd9, matching the brief.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

PASS
Head SHA: 64f6296154fb61fc138e57ffc2b65656ead7cbd9

  1. Confirmed: git diff --stat 4d6d72c096192cffe4f2466e907192519b04e953 64f6296154fb61fc138e57ffc2b65656ead7cbd9 -- ':!*/tests/*' returns empty — production code is byte-identical to the fully-audited SHA. Full delta since 2fc2ce7d0ff705025ec1c4ea6303ec10352656be is exactly one test file, +3/−24, matching the brief.

  2. Agree: the removed test (server_startup_calls_install_funded_distributor_registry_in_production_code) asserted only that two bare-identifier strings (install_funded_distributor_registry, state.state_dir) appeared somewhere in server.rs's production-region text. It stayed green both when the install call was deleted and when it was moved inside if config.enable_chain_sync { ... } (a real behavioural regression), so it never distinguished pass from fail on either mutation it was written to catch. It provided zero assurance, security-relevant or otherwise. No side-channel guarantee is lost — the remaining production_startup_reaches_the_install_call_site_without_panicking test still exercises the real startup path end-to-end.

  3. Confirmed: the +3 is (a) one whitespace-only comment-alignment on dig_local: false, and (b) rustfmt re-wrapping the outcome.expect(...) call across two lines. No string literal, path, or constant changed — the .expect(...) message text is byte-identical, only the line break moved. No test path or tempdir handling is touched by this diff; the tempdir-scoped I/O audited at 2fc2ce7d is unaffected.

Scope audited: crates/dig-node-service/tests/funded_distributor_registry_startup_3292.rs diff only, against a read-only fetch of refs/pull/626/head (no worktree needed — diff read directly via git diff/git fetch). Not re-covered: full audit at 4d6d72c0 and delta at 2fc2ce7d, both incorporated by reference per brief.

@MichaelTaylor3d
MichaelTaylor3d merged commit c72a0eb into develop Sep 27, 2026
14 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the loop/3292-rewards-startup-wiring branch September 27, 2026 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant