Uh oh!
There was an error while loading. Please reload this page.
feat(wallet): draw corroboration peers with exclusion instead of dial-and-discard - #226
Conversation
MichaelTaylor3d
commented
Aug 14, 2026
CORRECTNESS GATE: PASSHead reviewed: All six questions answered in favour of the change; all three mutation claims reproduced and confirmed. No money, custody, or privileged-action surface is made to lie. §908 untouched — nothing on this path signs. The six questions1. Does excluding 2. Mutation claims — reproduced, all three, in the warm worktree. Baseline: 3/3 green.
Each of the three behaviours therefore has an independent killer. 3. Is 4. The 5. 6. Test non-vacuity beyond the mutations. The tests assert on the ordered set of addresses reached, plus an explicit Also confirmed
Off-path findings — LOGGED, NOT GATING (§2.6 phase 1)
Required checksGreen at review time: Rustfmt, Clippy, Release-script tests, Commitlint (both), Check version increment, CodeQL (actions / javascript-typescript / rust). Still IN_PROGRESS: DisclosureMutation probing was performed in the lane's own worktree |
WIP toward #2904. The pool fix landed on the 0.26-generation line as 0.6.2, so this is a patch bump within one chia generation. Co-Authored-By: Claude <noreply@anthropic.com>
…iscard ChiaQuorumCorroborator::probe drew through connect_random_peer, which tries the priority addresses -- TRUSTED_FULLNODE, then 127.0.0.1 -- ahead of discovery on EVERY call. On a host with any co-resident process on 8444 the round was handed the same local address on every attempt, burned its whole MAX_PROBE_ATTEMPTS budget re-drawing one peer, and returned Verdict::Insufficient -- so corroboration silently failed on exactly the machines normal users run. The round now draws through connect_random_peer_excluding, passing the addresses it already holds, and admits PeerOrigin::Discovered draws only: a priority peer is preferred for speed, never counted as an independent voice. Sample assembly is extracted as assemble_distinct_sample with the dial as a seam, because neither rule is observable through a real dial. Refs #2904 Co-Authored-By: Claude <noreply@anthropic.com>
Minor: probe behaviour changes -- priority draws are no longer counted as independent voices. dig-wallet 0.22.0 -> 0.23.0 for the same reason. Co-Authored-By: Claude <noreply@anthropic.com>
6f1100c to
bb43d0dCompareMichaelTaylor3d
commented
Aug 14, 2026
Rebased onto The gate PASS on The one conflict was the predicted one, on the version-bump commit: root
Note for whoever merges: I rebased before the two in-flight checks ( Still DRAFT, as it should be until the checks return. dig-node's cron cuts a real stable release from Context: v0.118.1 is being released now (run |
MichaelTaylor3d
commented
Aug 14, 2026
PRE-MERGE SECURITY GATE — PASSHead audited: Diff audited in full: No shared checkout was mutated. All reads were The four questions1. Can an adversary influence which peers end up in the corroboration set? — No, and the change strictly REMOVES an attacker capability. The exclusion set is The capability actually removed: under chia-query 0.6.1 the priority addresses ( 2. Does the quorum still require genuine independence? — Yes, and independence is tightened.
Downstream independence bounds are unchanged: 3. Does rotation survive? — Yes.
4. Failure direction — closed, on every path.
No path in this diff produces a written peak from a smaller or attacker-chosen set. The money-lie class (a wrong balance reported as settled) is not reachable through the change: the failure mode is "the replica is not written", i.e. the honest-unknown state. Other areas checked
Log-and-defer (NOT blocking — pre-existing, non-regressive, fails closed)D1 — the exclusion only records a COMPLETED draw, so a silent co-resident peer is still re-offered on every attempt.
Scenario: an unprivileged co-resident process binds Why it does not gate: verified identical in the base. At D2 — a round that reached NOBODY is logged as a credibility-band split.
Neither is on the money/custody path; per 2.6 both are logged, not gated. Not covered
Verdict: PASS. No security defect introduced by this diff. Peer selection under the new path is at least as independent as before, and strictly more so on a host with a co-resident node; rotation is preserved; every failure direction is closed. |
Uh oh!
There was an error while loading. Please reload this page.
DO NOT MERGE — gate round has not returned. dig-node's
nightly-release.ymlstable job is guarded by
event_name == 'schedule', somainpublishes a realstable release unattended at midnight UTC (#698). This stays DRAFT until a gate
verdict is posted.
Closes DIG-Network/dig_ecosystem#2904
The defect being removed
ChiaQuorumCorroborator::probedrew throughchia_query::peer::connect::connect_random_peer,which tries the priority addresses —
TRUSTED_FULLNODE, then127.0.0.1— ahead of discovery onevery call. Un-excluded, a host with any co-resident process on
8444was handed the same localaddress on every attempt: a probe round burned its whole
MAX_PROBE_ATTEMPTSbudget re-drawing onepeer and returned
Verdict::Insufficient, which writes nothing. Corroboration therefore failedsilently on exactly the machines normal users run.
The fix has existed in chia-query since 0.10.0 and was backported to the 0.26-generation line as
0.6.2. This PR is the consumer adoption — a contract crate is done only when its consumers
adopt it (CLAUDE.md §1.3b).
What changed
crates/dig-wallet/Cargo.toml:chia-query"0.6.1"→"0.6.2".dig-node-core's=0.5.1is deliberately left alone — the two-line lock is documented at
crates/dig-wallet/Cargo.toml:66-72(#2227).
probedraws throughconnect_random_peer_excluding, passing the addresses the round already holds.PeerOrigin::Discovereddraws are admitted as opinions. A priority peer is preferred forspeed; it is not an independent voice, because a co-resident process is precisely a source a local
attacker can supply. Priority draws are ruled out of the round while still being excluded from
re-drawing.
assemble_distinct_sample, with the dial as a seam — neither rule isobservable through a real dial.
independent_peer_count()was deliberately NOT adopted. Its only candidate call site issage/chain.rs:157, whosepeer_countis documented atsage/quorum.rs:195as the transport'sheld pool — "measured, not targeted". Swapping in the independent count would silently change a
documented surface's meaning and would edit a file this PR proves needs no edit. Logged as a
follow-up instead.
Blast radius checked
Via ripgrep + direct read (§2.0 fallback bound 2 — a full
gitnexus analyzedirties tracked files,#2910; stated rather than hidden):
ChiaQuorumCorroborator::probe— exactly one caller, theCorroboratorimpl atsync_supervisor.rs:2090. Nothing outside this file calls it.ChiaQuorumCorroboratoris constructed in two places (sage/service.rs:241,tests.rs:1869),both via
::mainnet(), whose signature is unchanged.assemble_distinct_sample+Draware new and private to this module.connect_random_peercall remains, atsync_supervisor.rs:1865inChiaPeerSessionFactory.It is correct un-excluded: it wants ONE session peer and counts no opinions, so "try the local node
first" is the point there. Its stale doc line is corrected in this PR.
config.trustedis set anywhere on this path. The discovery session keepstrust_for(DialSource::Discovery). Priority orders discovery; it never confers authority.Risk: contained/LOW — one private function, one caller, no public signature changed.
How verified
cargo check -p dig-wallet --lockedclean with zero source edits on the pin bump alone — thisis the load-bearing verification that 0.6.2 is the same chia generation as 0.6.1. The earlier
attempt's 7-file blast radius was an artifact of the abandoned 0.10.0 generation jump, not of this
fix. The six other
chia_queryfiles (chain.rs,fallback.rs,service.rs,spend.rs,tipping.rs,sync_supervisor/tests.rs) are untouched, so there is no collision with PR fix(wallet): scope the chain-fallback CAT read to its asset id #224.cargo test -p dig-wallet --locked: 549 + 22 passed, 0 failed.cargo clippy -p dig-wallet --all-targets --locked: clean.cargo fmtapplied.cargo metadata --lockedexit 0.Each new test proven load-bearing by mutation
All three assert on the set of addresses reached, never a count or a success flag, and the
co-resident dialler counts its calls so a round that finished quickly is distinguishable from one
that made progress.
dial(ruled_out.clone())→dial(Vec::new())(exclusions never handed to the dialler)PeerOrigin::Discoveredfilter →if falseruled_out.contains(&addr)re-admission guard →if falseThe third mutation isolates the third test, so the defensive guard is not redundant with the
exclusion ask — it is what keeps a dialler that ignores its exclusions from turning one opinion into
a "quorum".
Note on the reused work
The parked branch
loop/2904-chia-query-0-10-adoption@305cea1(closed PR #225) was cherry-pickedrather than rewritten. Its tests did not compile (
E0373/E0507— theFnseam needsasync moveplus a reborrowed dialler), confirming they had never been gated. Fixed here.