Skip to content

fix(rewards): split the three conditions behind REWARD_CHAIN_UNAVAILABLE - #618

Merged
MichaelTaylor3d merged 5 commits into
developfrom
fix/3342-reward-chain-error-taxonomy
Sep 17, 2026
Merged

MichaelTaylor3d merged 5 commits into
developfrom
fix/3342-reward-chain-error-taxonomy

Conversation

@MichaelTaylor3d

@MichaelTaylor3d MichaelTaylor3d commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

The defect: one wire shape stood for three conditions

ChainPortError::Unavailable — machine code REWARD_CHAIN_UNAVAILABLE, numeric -32032 — was returned for all three of:

  1. No chain-read adapter installed on this Node at all. The only case where "no chain-read adapter is wired yet" is true.
  2. An installed adapter could not reach the chain — a genuine outage.
  3. The chain ANSWERED and holds no distributor at that launcher idread_distributor_guarded returning Ok(None), at chain_port.rs build_report.

Case 3 is the money defect. dig.getRewardDistributor and dig.listRewardDistributorCommitments are what a funder reads before deciding whether to claw back. With one shape for cases 2 and 3, that funder cannot tell "you have nothing there" from "we cannot see the chain" — so the clawback decision becomes a guess. This is the same shape as ClaimOutcome naming six ways a peer was legitimately unpaid and none to say something broke.

And the message named a cause that was false at the site that emitted it: on a default install (enable_chain_sync defaults true) an adapter is installed and is answering. The traced path is dispatch.rsnode.reward_chain_port() (a populated OnceLock, dig-node-core/src/lib.rs:608, installed at server.rs:2159) → RealRewardsChainPort::distributor_reportbuild_report. A diagnostic that misnames its own cause sends the next reader to the wrong subsystem.

The remedy

  • Ok(None) gets its own ChainPortError::NotADistributor and its own machine code REWARD_NOT_A_DISTRIBUTOR, so absence and outage can never share a wire shape again.
  • The false clause is not reworded — it is confined to reward_chain_port_absent_response, the one site where it is true. grep -rn "adapter is wired" crates/ returns exactly one hit. Reusing the Unavailable arm for the unset-OnceLock case would merely have relocated the false message.
  • Nothing enters dig-rpc-protocol: these machine codes are local &str consts by design, matching REWARD_INVALID_WITHDRAWAL_SHARE and REWARD_ZERO_IDENTITY beside them. An org-wide code search finds no consumer of REWARD_CHAIN_UNAVAILABLE outside this repo.

Mutation proof — the guard is pinned, not merely asserted

Reading proves a guard correct; only a revert going red proves it held. The two commits on this branch are that revert, in TDD order:

head build_report arm an_answering_chain_with_no_distributor_is_an_absence_not_an_outage suite
a0740145 .ok_or(ChainPortError::Unavailable)? FAILED, 3 retries — assertion left != right failed: ... got Err(Unavailable) 3431
60ba02f7 .ok_or(ChainPortError::NotADistributor)? PASS 3431 run, 3431 passed, 4 skipped

Suite size is 3431 at both heads — no test was deleted or relaxed to reach green.

What this does NOT fix

The epic's exit condition needs two more things, neither in scope here:

  • #3347 — the claim loop holds a second, different port trait (rewards_claim::port::ClaimChainPort) whose only production adapter is UnavailableClaimChainPort, so every claim cycle reports ChainSourceUnavailable. Sequenced behind #3336 / PR fix(rewards-claim): derive both cadences from one raw config value and inject the clock #617, which owns driver.rs.
  • #3339 / dig-account#58 — no distributor has ever been minted on mainnet, minting having been blocked on the money-path signing seam. The read path was never broken. This PR makes the answer legible; that one makes there be something to answer about.

Refs #3342
Refs #3246

🤖 Generated with Claude Code

MichaelTaylor3d and others added 2 commits September 16, 2026 20:34
Refs #3342

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…an absence

`build_report` maps `read_distributor_guarded`'s `Ok(None)` -- the chain
answering "no distributor at this launcher" -- onto the same
`ChainPortError::Unavailable` an unreachable chain produces. A funder
deciding whether to claw back cannot tell "you have nothing there" from
"we cannot see the chain", and one wire shape for both makes that call a
guess on a money surface.

This test is RED at this commit, deliberately: it is the acceptance
evidence for the split that follows, written before the fix so the fix
has something to turn green.

Refs #3342

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@MichaelTaylor3d MichaelTaylor3d changed the title fix(rewards): distinguish "not a distributor" from "chain unreachable" and delete the false adapter-wiring cause fix(rewards): split the three conditions behind REWARD_CHAIN_UNAVAILABLE Sep 17, 2026
`ChainPortError::Unavailable` stood for three conditions a caller must
tell apart:

1. no chain-read adapter installed on this Node at all;
2. an installed adapter could not reach the chain -- a real outage;
3. the chain ANSWERED and holds no distributor at that launcher id
   (`read_distributor_guarded` -> `Ok(None)`).

Case 3 is the money-surface defect: an absence rendered as an outage. A
funder deciding whether to claw back cannot distinguish "you have
nothing there" from "we cannot see the chain", and one wire shape for
both makes that call a guess.

`Ok(None)` now gets its own `ChainPortError::NotADistributor` and its
own machine code, so the two can never share a shape again.

The message "no chain-read adapter is wired yet" was also false at the
site that emitted it -- on a default install an adapter IS wired and IS
answering. It is not reworded; it is confined to
`reward_chain_port_absent_response`, the one case where it is true, and
the `Unavailable` arm now names its real cause. A diagnostic that
misnames its own cause sends the next reader to the wrong subsystem.

Turns green the red test added in the previous commit.

Refs #3342
Refs #3246

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Adversarial leg (loop-decider) — CHANGES-REQUIRED at 60ba02f718bd2aa6faae5c049b40c3e352c73f5c

Port-level split is correct and proven (red at a0740145, green here, 3431 tests both heads — not re-derived). Three holes survive.

H1 — the wire is unguarded; the re-collapse survives the whole suite

No test at this head asserts REWARD_NOT_A_DISTRIBUTOR (or any REWARD_* string) on a JSON-RPC response: dispatch.rs has zero REWARD_ occurrences past line 130, tests/reward_methods_tier_guard.rs and tests/rewards_chain_port_a3.rs have none. dig-app consumes data.code, not ChainPortError. This two-line edit re-collapses absence onto outage on the wire and leaves every test green:

-        ChainPortError::NotADistributor => json!({"jsonrpc":"2.0","id":id,"error":{
-            "code": CONTROL_ERROR,
-            "message": "no reward distributor exists at this launcher id on chain",
-            "data": { "code": REWARD_NOT_A_DISTRIBUTOR_MACHINE, "origin": "control" }
-        }}),
+        ChainPortError::NotADistributor => json!({"jsonrpc":"2.0","id":id,"error":{
+            "code": CONTROL_ERROR,
+            "message": "no reward distributor exists at this launcher id on chain",
+            "data": { "code": REWARD_CHAIN_UNAVAILABLE_MACHINE, "origin": "control" }
+        }}),
-const REWARD_NOT_A_DISTRIBUTOR_MACHINE: &str = "REWARD_NOT_A_DISTRIBUTOR";

(the const deletion only silences dead-code.) The port test is also assert_ne!(.., Unavailable) — it passes if NotADistributor is ever re-mapped to Other. Required: one dispatch-level test that drives reward_chain_port_error_response(&id, &ChainPortError::NotADistributor) and asserts data.code == "REWARD_NOT_A_DISTRIBUTOR" AND != "REWARD_CHAIN_UNAVAILABLE"; flip the port test to assert_eq!(result, Err(ChainPortError::NotADistributor)).

H2 — the diff's own file still renders the absence as an outage, and it blinds the next real outage

crates/dig-node-service/src/rewards/chain_port.rs:113-123: any Err — now including NotADistributor — flips report_degraded to true and logs "distributor_report failed; reward-distributor reads for this launcher stay refused until the chain source recovers". That is Defect 2 of #3342 (a message naming a false cause) surviving one function above the fix. Worse: the latch is adapter-wide, so the exact probe in the ticket (0x1111…) sets degraded=true; a genuine chain outage that follows emits no warn until some Ok resets it. An absence must not enter the degraded latch. Required: match Err(ChainPortError::NotADistributor) alongside Ok(_) (or exclude it from the latch) and keep the warn for outages only.

H3 — PR body is false at this head (Q6, this epic's own defect class, second occurrence on this PR)

Body says "Landed so far: the failing test only … The error split … is not yet in this branch" with the split and dispatch items unchecked. Both are in 60ba02f7. Also "the false clause is deleted, not reworded" — it is relocated verbatim into reward_chain_port_absent_response (defensible, the body even argues it; say "relocated to the one true site"). Required: rewrite the status block to this head; define or drop "A2–A5".

Answers 1–6

  1. Producer guarded at the PORT, forgeable at the WIRE — see H1 edit.
  2. Taxonomy is three-of-four. read_distributor returns Ok(None) only when coin_record(launcher_id) is None; a launcher coin that EXISTS but is not a distributor launch (malformed("launcher solution is not a reward-distributor launch")) maps Malformed → Other(String). So NotADistributor actually means "no coin at this id"; "a coin, but not a distributor" still wears the generic Other shape. Rename or document the variant honestly (NoCoinAtLauncherId, or route that Malformed arm to NotADistributor). Not blocking, but the name overclaims.
  3. Consumers: gh search code REWARD_CHAIN_UNAVAILABLE --owner DIG-Network → only dig-node and two contextOS wiki pages (docs, not consumers); no dig-app/dig-chrome-extension hit. An unaware consumer sees -32032 with an unknown data.code and takes its generic control-error path; a retry-on-UNAVAILABLE loop STOPS retrying on absence — correct, since absence is terminal. Not worse. Note dig.listRewardDistributors (dispatch.rs:1140-1150) fails the WHOLE list on one identity's NotADistributor — pre-existing shape, now legible.
  4. Not subtractive: +1 variant, +1 const, +1 fn. The subtractive alternative (Result<Option<DistributorReport>> / result: null) leaks an absence as a null success and changes the trait — worse. The addition is the minimum; the body should stop calling it subtractive.
  5. Worth merging after H1–H3. CorroboratedChainSource::coin_record returns Ok(None) only when peers AGREE the coin does not exist (corroborated_source.rs:243-246); "too few / disagree" is Err → Unavailable. So a real-but-not-yet-visible distributor is already Unavailable, not NotADistributor — no fourth shape needed there. Once distributors exist the variant stays right.
  6. See H3.

Fixes are confined to chain_port.rs, dispatch.rs and the PR body — no driver.rs/engine.rs.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

IN PROGRESS — not the verdict

Traced reachability of dig.getRewardDistributor / dig.listRewardDistributorCommitments end to end:

  • reward_methods_tier_guard.rs + peer.rs::is_peer_reachable_method fail-closed correctly on the
    mTLS peer surface (another DIG node cannot reach these methods). That claim in the PR's doc
    comments is TRUE.
  • BUT dig-node-service/src/server.rs's POST / handler (rpc()) — the same wire surface the file
    header says matches rpc.dig.net and which keys anonymous callers by IP — dispatches ANY method
    that isn't in control::is_control_method, wallet_authz::requires_authorization, the 3 cache.*
    names, or the 2 chat.* names STRAIGHT to handle_rpc_as with no token check at all.
    dig.getRewardDistributor / dig.listRewardDistributorCommitments are in none of those sets.
    Grepped wallet_authz.rs and control.rs for "Reward" — zero hits.
  • dig_rpc_protocol::Tier::Control is never read at runtime anywhere in dig-node-core or
    dig-node-service — only asserted in unit tests. It gates NOTHING at the HTTP layer.
  • Default bind is loopback-only, but the file's OWN comments elsewhere (the cache.* gate) already
    document that a loopback bind is not a trust boundary — a same-host process or a DNS-rebound
    browser page can POST to dig.local/127.0.0.1 with no credential. DIG_NODE_ALLOW_REMOTE=1 widens
    this to the network.

Net: the "CONTROL plane: loopback admin / in-process FFI ONLY" comment this PR adds to
dispatch.rs:997-999 is FALSE as applied to the real HTTP entry point. Still checking whether this
gap is new or pre-existing, and whether this PR's specific change (splitting NotADistributor out)
turns a previously uninformative response into a genuine oracle over that reachable surface.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

CHANGES-REQUIRED
Head SHA audited: 60ba02f

Finding 1 (LIVE) — the split turns a pre-existing reachability gap into a real enumeration oracle

crates/dig-node-core/src/seams/dig_rpc/dispatch.rs:997-999 (and the mirrored comment on
ChainPortError::NotADistributor in crates/dig-node-core/src/rewards/port.rs:149-154) asserts
dig.getRewardDistributor / dig.listRewardDistributorCommitments are "CONTROL plane: loopback
admin / in-process FFI ONLY". That is true for exactly one surface and false for the one that
matters here.

What is actually true, traced end to end:

  • crates/dig-node-core/tests/reward_methods_tier_guard.rs + peer.rs::is_peer_reachable_method
    correctly fail-closed on the mTLS peer surface — another DIG node cannot reach these methods.
    That part of the claim holds.
  • crates/dig-node-service/src/server.rs::rpc() (POST /) — the file's own header says this is the
    same wire surface rpc.dig.net speaks, and it keys anonymous callers by connection IP
    (requestor_for, RequestorId::Anonymous) — dispatches any method not in
    control::is_control_method, wallet_authz::requires_authorization, the three cache.* names, or
    the two chat.* names straight to handle_rpc_as with no token check at all
    (server.rs:1319-1323, the fallthrough after every named gate). dig.getRewardDistributor /
    dig.listRewardDistributorCommitments are in none of those sets — confirmed by grepping
    wallet_authz.rs and control.rs for "Reward" (zero hits).
  • dig_rpc_protocol::Tier::Control is read at runtime nowhere in dig-node-core or
    dig-node-service — it is only asserted in unit tests (lib.rs:9677, 10045, 10212). Tier ==
    Control enforces nothing at the HTTP boundary.
  • Default bind is loopback-only, but this file already treats loopback as NOT a trust boundary for
    exactly this shape of risk — the cache.* gate at server.rs:1320-1332 exists precisely because
    a cross-site page can POST to dig.local (DNS-rebinding / local-service attack) and a loopback
    address does not prove operator authorization. That reasoning was never applied to the reward
    methods. DIG_NODE_ALLOW_REMOTE=1 widens the same gap to the network.

Exploit: any co-resident process, or any web page that DNS-rebinds to the node's loopback port
(the exact threat model this file already defends against for cache.*), POSTs a JSON-RPC request
for dig.getRewardDistributor with a probe launcher_id and no token. Before this PR every
non-answer (no adapter, unreachable chain, no distributor at that id) came back as the identical
REWARD_CHAIN_UNAVAILABLE — uninformative to probe. After this PR, NotADistributor answers
REWARD_NOT_A_DISTRIBUTOR distinctly: the caller now gets a definitive, chain-backed answer to
"does a reward distributor exist at launcher id X" from an untrusted, unauthenticated caller. That
is exactly the oracle Q1 asks about, and it IS reachable by more than "whoever can reach the method"
the PR's own reasoning assumes.

This is LIVE, not defense-in-depth: the underlying HTTP-surface gap is pre-existing and out of this
PR's diff, but this PR is what converts it from harmless (uninformative in every case) to a working
oracle. Recommend gating dig.getRewardDistributor / dig.listRewardDistributorCommitments (and
probably the other Reward-prefixed methods, which have the same gap) at the server.rs HTTP layer
the same way cache.* is gated — control token or paired token — before this ships, or at minimum
opening a same-priority follow-up ticket and holding this PR until it lands, since the PR itself is
what makes the gap exploitable for information disclosure.

Answers to the five questions

  1. Enumeration oracle? Yes, see Finding 1. It is reachable beyond the claimed loopback admin/FFI-only
    audience, via the anonymous POST / JSON-RPC surface dig-node-service exposes.
  2. Genuinely control-plane only? The mTLS peer-surface guard (is_peer_reachable_method +
    reward_methods_tier_guard.rs) is real, correctly implemented, and cannot be forged short of
    renaming the method away from containing "Reward" (the test's own doc calls this out as the trap
    it is designed to catch). But "control-plane only" as a whole claim is false: the HTTP surface
    never checks Tier or any token for these two methods. Producer-guarded on one channel, ungated on
    the other that actually carries anonymous traffic.
  3. reward_chain_port_absent_response new path: it is byte-identical (message + data.code) to what
    the "no port installed" case already returned before this PR, so it discloses nothing new about
    enable_chain_sync. Not exploitable, not a regression. The one behavior change is that a
    port-installed-but-Unavailable case now gets different wording ("the chain source could not
    answer" vs "no chain-read adapter is wired yet") while keeping the same data.code
    (REWARD_CHAIN_UNAVAILABLE) — a caller reading only data.code (the documented contract) still
    cannot distinguish "no adapter" from "adapter down"; only a caller scraping the free-text message
    could, which is not a stated contract and is a much smaller leak than Finding 1. Not gating on
    this.
  4. Interpolation: no new arm interpolates attacker-controlled data — NotADistributor's message is a
    fixed string, launcher_id never appears in it. Clean.
  5. Numeric code stability: confirmed all arms (existing + new) share CONTROL_ERROR (-32032); only
    data.code varies (REWARD_CHAIN_UNAVAILABLE, REWARD_NOT_A_DISTRIBUTOR,
    REWARD_INVALID_WITHDRAWAL_SHARE, REWARD_ZERO_IDENTITY, generic CONTROL_ERROR). A caller reading
    data.code cannot be confidently wrong about which condition fired.

Scope audited

Files: crates/dig-node-core/src/rewards/port.rs, crates/dig-node-core/src/seams/dig_rpc/dispatch.rs,
crates/dig-node-service/src/rewards/chain_port.rs (the diff), plus for reachability verification:
crates/dig-node-core/src/peer.rs, crates/dig-node-core/tests/reward_methods_tier_guard.rs,
crates/dig-node-core/src/lib.rs (handle_rpc/handle_rpc_as + reward tier tests),
crates/dig-node-service/src/server.rs (full rpc() handler), crates/dig-node-service/src/control.rs,
crates/dig-node-service/src/wallet_authz.rs.

Not covered

  • Did not audit dig-node-service's WS transport (ws_dispatch) for the same gap — likely shares the
    identical fallthrough given comments referencing WS parity elsewhere, but not traced line by line;
    worth the same check.
  • Did not check whether the upstream relay/passthrough fallback (for -32601 method-not-found) could
    leak anything, since these methods are always resolved locally and never reach that path — not a
    concern here.
  • Did not run the test suite locally; relying on the stated CI result (13 green, 3431/3431).

Shared checkout

Used git fetch origin pull/618/head:pr-618-audit inside the shared submodule checkout at
modules/apps/dig-node (read-only ref creation, no working-tree mutation there), then did all file
reads from an isolated worktree at D:\worktrees\dig-node-pr618-audit (branch pr-618-audit, same SHA).
Left the shared checkout's HEAD/working tree untouched. The worktree and the local branch ref are
harmless to leave in place, but can be removed with
git worktree remove /d/worktrees/dig-node-pr618-audit and git branch -D pr-618-audit (run from
the submodule checkout) if the orchestrator wants them gone.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Orchestrator decision on the three gate verdicts — with reasons, not just rulings

Not merging at 60ba02f7. Two of the findings are must-fix and are in progress on this branch; one I am downgrading, with my reasoning stated so it can be argued with.

H1 — wire distinction unpinned: ACCEPTED, blocking

The gate produced a two-line edit that re-collapses the split while all 3431 tests stay green. That is decisive: the guard is forgeable, so it does not hold. The port-level test is also too loose — assert_ne!(.., Unavailable) passes if the variant is later re-mapped to Other. Both are being fixed: a dispatch-level test asserting data.code by value, and the port test tightened to assert_eq!(Err(NotADistributor)). Each will be proven by an independent mutation, red then green.

H2 — the absence arms the degradation latch: ACCEPTED, blocking, and the most valuable finding here

chain_port.rs:113-123 sets the adapter-wide report_degraded latch on every Err, now including NotADistributor. So one probe for a launcher id that simply is not a distributor silences the warning for the next genuine outage. That is worse than the defect this PR set out to fix — it converts a legibility improvement into a telemetry regression on a money surface, which is exactly the shape this epic keeps producing. Fixing it, with a test proving a real Unavailable still warns after a NotADistributor.

Security finding — reachability gap: REAL, but DOWNGRADED as an exploit, and filed separately

The gate is right on the mechanism and I am not disputing it: the reward read methods are documented "loopback admin / in-process FFI ONLY" at dispatch.rs:997-999 and port.rs:149-154, the peer-surface guard (is_peer_reachable_method + reward_methods_tier_guard.rs) is real and fails closed, and server.rs::rpc() — anonymous POST / — reaches them anyway because they are in none of the gated sets, with Tier::Control never read at runtime.

Where I differ is on calling it an enumeration oracle created by this PR:

  1. The disclosed fact is public on-chain state. Anyone with a chain source can determine whether a launcher_id is a distributor directly. This node confirms something already world-readable; it discloses nothing about this node — not which distributors it funds, not its configuration.
  2. The chain read already happened before this PR. The old code performed the same build_report read and then discarded the answer into one uninformative error. So fix(rewards): split the three conditions behind REWARD_CHAIN_UNAVAILABLE #618 adds no new resource cost and no new amplification vector — only a more truthful rendering of a read that was already being done.

So the exploit delta from this PR is disclosure of public data that was already free to obtain. That is not a merge blocker.

What is genuinely wrong is the gap between the documented tier and the enforced one — a method whose doc says "loopback admin ONLY" while answering anonymously is a false statement in the code about the code, and the next reader will build on it. Same class as the defect this PR fixes, which is why it deserves its own ticket rather than being absorbed here. Filed at priority:1-high with both remedies spelled out: gate it at server.rs like cache.*, or correct the docs — pick one, not half of each. The WS transport's parity with the gap and Tier::Control being asserted only in tests are called out in the same ticket.

H3 — PR body stale: already fixed

The body was rewritten before this verdict landed and now states the defect, the mutation proof table, and what the PR does not fix. The gate is right that "deleted, not reworded" was imprecise — the sentence is relocated to reward_chain_port_absent_response, the one site where it is true, and the body now says so. The word "subtractive" is gone: this remedy is additive, and the gate's argument for why the subtractive alternative (an Option/null result) is worse — it leaks absence as success — is correct and settles it.

Q2, carried forward

NotADistributor currently means "no coin record at this id"; a launcher coin that exists but is not a distributor launch maps Malformed → Other. Non-blocking, but the name overpromises. Noted on #3342 for a follow-up rather than widened here.

Re-gate will run on the new head. A verdict on a replaced head covers nothing.

@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.

Verdict: CHANGES-REQUIRED

Head: 60ba02f718bd2aa6faae5c049b40c3e352c73f5c

What's solid

  • Q2 (call sites): all three let Some(port) = node.reward_chain_port() else { .. } arms (GetRewardDistributor, ListRewardDistributorCommitments, ListRewardDistributors) now route through reward_chain_port_absent_response, not the Unavailable arm of reward_chain_port_error_response. Verified by grep against crates/dig-node-core/src/seams/dig_rpc/dispatch.rs lines 1013, 1055, 1141.
  • Q3 (single "wired" site): grep -rn "adapter is wired" crates/ returns exactly the doc comment (line 111) and the message string (line 119) inside reward_chain_port_absent_response — one function, one site.
  • Q4/Q5 (no weakened assertions): diffed chain_port.rs — the two neighbouring tests (parent_spend_gap_is_reported_as_unavailable_not_as_a_distributor_identity_verdict, a_failing_chain_source_reports_a_named_unavailable_never_an_ok_default) still assert Unavailable; only their doc comments changed, and correctly — the old doc justified ParentSpendUnavailable => Unavailable by appeal to Ok(None)'s (now-defunct) sharing of that shape; the new doc justifies it on its own merit (a chain source that never reached the parent spend can't say anything about identity at all). No contains/assertion was widened or deleted — confirmed no - lines in the diff touch any assert/assert_eq/assert_ne line.
  • Q6 (no wire crate touched): dig-rpc-protocol is a separate repo/submodule, not present at all in this diff or in dig-node's own tree; the new machine codes are local &str consts in dispatch.rs, exactly as designed.
  • Port-level mutation proof (already measured, reconfirmed by the doc/diff read): an_answering_chain_with_no_distributor_is_an_absence_not_an_outage is a real red/green pair across a074014560ba02f7.

The gap — Q1, unanswered by any test

Nothing pins the WIRE-level distinction. dig-app (and any RPC caller) reads data.code on the JSON-RPC error body, not the Rust ChainPortError enum. I checked every test that exercises reward_chain_port_error_response/the dispatch handlers for the two reward-distributor methods:

  • reward_distributor_methods_chain_unavailable_is_a_distinct_error_never_a_zero_or_empty (crates/dig-node-core/src/lib.rs:10324) only calls test_node(None)no port installed — so it exercises reward_chain_port_absent_response (the "no adapter" case), not an installed port answering Err(ChainPortError::NotADistributor). It asserts REWARD_CHAIN_UNAVAILABLE != REWARD_INVALID_WITHDRAWAL_SHARE, but never compares against REWARD_NOT_A_DISTRIBUTOR.
  • grep -rn "NotADistributor" crates/dig-node-core/src/lib.rs returns zero hits. No test in this file installs a FakeRewardsChainPort returning Err(ChainPortError::NotADistributor) and checks the resulting data.code.
  • I mechanically confirmed the hole: collapsing reward_chain_port_error_response's NotADistributor arm's data.code onto REWARD_CHAIN_UNAVAILABLE_MACHINE (same string as the Unavailable arm) is a one-line, exhaustive-match-safe change — it compiles — and no test in the repo's dispatch/lib-level suite would go red for it. The only test that would catch a REGRESSION at the port level (an_answering_chain_with_no_distributor_is_an_absence_not_an_outage) checks ChainPortError::Unavailable directly, not the wire shape, so it stays green even if the wire collapses the two data.codes back together.

This means the actual deliverable this PR claims — "a funder can tell 'nothing there' from 'chain unreachable' over the wire" — is unproven. The port-level split is real and tested; the wire-level split (the only thing dig-app or any external caller can see) rides on doc comments and code review, not a test.

Ask: add a dispatch-level test — install a FakeRewardsChainPort returning Err(ChainPortError::NotADistributor) for a dig.getRewardDistributor/dig.listRewardDistributorCommitments call, assert resp["error"]["data"]["code"] == "REWARD_NOT_A_DISTRIBUTOR", and assert it != the REWARD_CHAIN_UNAVAILABLE value from the neighbouring Unavailable-port test. The fix must not just add a Unavailable-vs-NotADistributor equality check inside a Rust match (that's the port-level test that already exists) — it must round-trip through handle_rpc/JSON exactly like reward_distributor_methods_chain_unavailable_is_a_distinct_error_never_a_zero_or_empty does, because that's the only thing a caller ever observes.

Everything else in this diff — the split itself, the absent-adapter response, the doc corrections — is correct and I have no other blocking finding.

@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.

(duplicate submission of review 5235037742 above -- ignore this one, verdict and findings are in the first review)

@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.

Inline finding on the wire-level test gap (Q1) -- see thread.

Comment thread crates/dig-node-core/src/seams/dig_rpc/dispatch.rs
MichaelTaylor3d and others added 2 commits September 17, 2026 05:03
… latch

Two pre-merge gate findings, both proven by mutation.

H1 -- the wire-level distinction was unpinned. No test asserted any
REWARD_* string on a JSON-RPC response body, so pointing the
`NotADistributor` arm's `data.code` at `REWARD_CHAIN_UNAVAILABLE_MACHINE`
re-collapsed the split, compiled clean (the match stays exhaustive) and
left all 3431 tests green. The port-level test was also too loose:
`assert_ne!(.., Unavailable)` passes if the variant is later re-mapped to
`Other`. Now a dispatch test installs a port answering
`Err(NotADistributor)` and asserts `data.code` by value for both methods,
and the port test asserts the variant by name.

H2 -- an absence armed the degradation latch. Every `Err`, including
`NotADistributor`, set the adapter-wide `report_degraded` latch and
warned that reads "stay refused until the chain source recovers". The
warn fires only on a false->true transition, so one probe for a launcher
id that simply is not a distributor silenced the warning for the next
GENUINE outage. That is worse than the defect this branch set out to fix:
it trades a misleading error for missing telemetry on a money surface.
`NotADistributor` is now excluded from both the latch and the warn; `Ok`
still clears it, matching a real recovery.

Refs #3342
Refs #3246

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review September 17, 2026 12:54
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Fixed and proven at 50cb8990, not merely asserted.

reward_distributor_methods_pin_the_not_a_distributor_wire_code_distinct_from_unavailable (lib.rs:10370) now installs a FakeRewardsChainPort that answers Err(ChainPortError::NotADistributor) and asserts data.code by value for BOTH dig.getRewardDistributor and dig.listRewardDistributorCommitments — plus that only the absent-port case contains "adapter is wired". Your grep now returns hits.

The exact re-collapse you identified was executed as a mutation and the test went RED:

test tests::reward_distributor_methods_pin_the_not_a_distributor_wire_code_distinct_from_unavailable ... FAILED
panicked at crates\dig-node-core\src\lib.rs:10430:13:
assertion `left == right` failed: dig.getRewardDistributor
  left: String("REWARD_CHAIN_UNAVAILABLE")
 right: String("REWARD_NOT_A_DISTRIBUTOR")
test result: FAILED. 0 passed; 1 failed; 0 ignored; 1236 filtered out

Reverted, GREEN again (test result: ok. 1 passed), and Test + coverage is green on 50cb8990 across all 3431 tests. You were right that the mutation compiles clean — the match stays exhaustive — which is exactly why only a red test catches it.

The port-level test was also tightened per your finding, from assert_ne!(.., Unavailable) to assert_eq!(result, Err(ChainPortError::NotADistributor)), so a future re-map to Other fails too.

Resolving: the defect this thread names is fixed, and the fix is pinned by a test proven to fail without it.

@MichaelTaylor3d
MichaelTaylor3d merged commit a2d5c38 into develop Sep 17, 2026
14 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the fix/3342-reward-chain-error-taxonomy branch September 17, 2026 12:56
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