Uh oh!
There was an error while loading. Please reload this page.
build(deps)!: unify dig-node's three Chia diallers onto chia-query's light client - #339
Conversation
dig-node holds three independent Chia peer-dialling stacks, each with its own TLS sessions to its own chosen full nodes and its own notion of the peak, with nothing reconciling them. Unify them onto the one crate that already owns this, chia_query::peer::light_client. Lane-opening stub so the work survives a session cap. Co-Authored-By: Claude <noreply@anthropic.com>
…ond one A node with live broadcast enabled held TWO independent chia_query peer pools: `build_live_wallet` called `ChiaQuery::new` itself, so the live sender and the wallet's chain reads dialled two independently-chosen sets of full nodes, each with its own notion of the peak and nothing able to reconcile them. The wiring now takes the client from `ChainTransport`. The second pool is unexpressible rather than merely unused: `build_live_wallet` has no way to construct one. Building the transport before the live wiring is what makes that possible, so the two are reordered. This does NOT reduce the plurality NC-12 depends on. The quorum corroborator and the corroborated-read sample keep drawing their own independently-dialled peers; what collapses is two pools serving the SAME role. Refs: DIG-Network/dig_ecosystem#2761 Co-Authored-By: Claude <noreply@anthropic.com>
…field The previous shape stored the client on `LiveWallet` purely so a test could compare pointers. Nothing in production read it, so `-D warnings` failed the Clippy gate on `field is never read` — and a field that exists only to be asserted on is dead code wearing a test's clothes. The property is unchanged and still distinguishing: seed the transport with a known client, then watch its reference count. A wiring that built its own pool leaves the count untouched. Dropping the wiring must return the count to its starting value, so the assertion cannot pass on references some other holder contributed. Co-Authored-By: Claude <noreply@anthropic.com>
…ing chia-query dig-node-core carried a `chia-peer`-backed subscribing light client, meant to register as a `ChainSourceProvider` into a `chia_query::ProviderRegistry`. Nothing ever constructed it. `ProviderRegistry` appears in this crate only inside that module's own import, one signature and its tests; there is no production registry, no caller, and no path from a running node to any of it. So it was a third Chia dialling stack on paper only -- and an expensive one. It was the SOLE consumer of `chia-query` in this crate, and the reason for the `chia-query = "=0.5.1"` exact pin, which the manifest itself recorded as what holds dig-node-core off the chia 0.36 line. Removing it collapses the two `chia-query` lines the process resolved to (0.5.1 via this crate, 0.6.2 via dig-wallet) down to one. Verified from the resolved lock, not the caret declarations: `chia-query` now appears once at 0.6.2 and `chia-peer` not at all. `dig-chainsource-interface` still resolves to two lines (0.1.0, 0.2.0). That is NOT this crate's doing and does not change here: 0.1.0 arrives via dig-merkle -> dig-store, independently of anything removed. The node's real Chia peer tier is dig-wallet's `ChainTransport`, which is untouched -- this deletes the unreachable twin and leaves the one that actually serves reads standing. If the subscribing light client is wanted again its home is `chia_query::peer::light_client`, which `chia-peer` is now a deprecated re-export facade over. BREAKING CHANGE: `dig_node_core::seams::chia_peer::light_client` is removed, along with `connect_light_client`, `register_light_client_provider`, `submit_spend`, `confirmation_depth`, `ChiaPeerSubscriptions` and `CHIA_PEER_INDEPENDENCE_GROUP`. None had a caller. Refs: DIG-Network/dig_ecosystem#2761 Refs: DIG-Network/dig_ecosystem#3152 Co-Authored-By: Claude <noreply@anthropic.com>
Root 0.147.0 -> 0.148.0, dig-node-core 0.55.0 -> 0.56.0, dig-wallet 0.32.0 -> 0.33.0. Minor rather than patch on all three: dig-node-core drops a public module, and on 0.x a breaking change is a minor bump. Also corrects the chia-query comment in dig-wallet, which told readers the pin was deliberately split from dig-node-core's `=0.5.1` and must not be aligned. That pin no longer exists, so the note now describes state that cannot occur -- and a stale warning against touching something is worse than no note, because it stops the next reader looking. Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3d
commented
Aug 24, 2026
loop-security — audit IN PROGRESS (interim, not a verdict)Auditing head Interim findings so far (each still being pressed): 1. The deleted stack was genuinely unreachable — refuted nothing, confirmed the claim. 2. §908 direction of travel is correct, not merely neutral. The removal DELETES a Still open and being attacked: the independent-dial count before/after (NC-12 plurality), |
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
Correctness gate - CHANGES-REQUIRED
Head reviewed: efc87f2 (resolved from the remote, not from the dispatch).
One gating finding, and it is a doc/contract one - the code change itself is correct and I could not break it. Everything the PR claims about the dialling collapse, the plurality, the lock and the revert-proof, I re-measured independently and it holds.
What I verified myself
1. NC-12 plurality survived - independent dial-holders counted by hand.
| dial holder | before (251b4c1) | after (efc87f2) |
|---|---|---|
ChainTransport pool (chain.rs:131, lazy, one per transport) | 1 | 1 |
live-broadcast pool (service.rs:310, flag-on) | 1 | 0 - unexpressible |
chia-peer light client | 0 (never constructed) | removed |
DialedPeerSample::mainnet() (chain.rs:116) | independent | independent, untouched |
ChiaQuorumCorroborator::mainnet() (service.rs:244) | independent | independent, untouched |
Production ChiaQuery::new call sites: 2 -> 1. The remaining occurrences at head are sage/fallback.rs:636 and sage/service.rs:561, both inside #[cfg(test)] modules. Neither the corroborator nor the peer sample takes a ChiaQuery, so nothing routes their reads through the shared client - the two collapsed pools served the same role, and the voices are intact.
2. The deleted stack was genuinely dead, not a rival.git grep at head for ProviderRegistry, connect_light_client, register_light_client_provider, ChiaPeerSubscriptions, CHIA_PEER_INDEPENDENCE_GROUP returns zero production references - only CHANGELOG.md, DEVELOPMENT_LOG.md and SPEC.md prose (see the finding). No trait-object or #[cfg] construction path exists: the only impl lived in the deleted file and nothing named its type. control.rs:3890the_light_client_chain_reads_survive_the_custody_carve_out is a different sense of "light client" (the control-surface wallet reads) and is unrelated.
3. Mutation re-run as written - it fires, and for the right reason. Own worktree, build_live_wallet restored to calling ChiaQuery::new(ChiaQueryConfig::default()) itself:
test ...the_live_wiring_reuses_the_transports_client_instead_of_opening_a_second_pool ... FAILED
panicked at crates/dig-wallet/src/sage/service.rs:590
test result: FAILED. 1 passed; 1 failed; 0 ignored; 607 filtered out
It failed on the sharing assertion at line 590, not on the fixture precondition (before == 2) and not on a compile error - so the revert-proof is not firing for the wrong reason. Restored: 2 passed; 0 failed. The drop-and-recount half is real: the second assertion requires the extra references to vanish with live, so a count inflated by any other holder fails it. Asserting that two pools merely agreed would have passed against the coincidence this ticket exists to remove.
4. Lock collapse holds, read from Cargo.lock and not the carets.
chia-query: 0.5.1 + 0.6.2 -> one line at 0.6.2chia-peer: 0.1.3 -> absent- dig_ecosystem#2227 is genuinely closed by this - the deliberate two-line split is gone.
dig-chainsource-interfacestill resolves to two lines (0.1.0, 0.2.0); attribution confirmed - the 0.1.0 dependents in the lock are exactlydig-merkleanddig-store, independent of everything removed here. Correctly reported, correctly not chased.
5. The two removed Closes keywords - confirmed, and the highest-value catch in the lane. The body closes nothing and says why for both #308 and #249. Merging the original form would have auto-closed two tickets whose work is unfinished. This must not regress on a later body edit.
The two claims to record, both confirmed:
- dig_ecosystem#2790 gap-2 is vacuously satisfied. "No path constructs its own peer fabric outside the registry" holds because there is no registry -
ProviderRegistryhad no production construction site before this PR and none after. A clause that passes because the thing it governs never occurs is vacuous, not satisfied. - dig-node#249 prescribed remedy is not executable as written. It says to inject "the node’s provider registry"; no such object exists in dig-node-core. Re-scoping onto
ChainTransport::peer_tier().peak_heightis the correct reading.
dig-constants check (both questions, per HARD RULE)
- Does anything here BELONG in dig-constants? No. The diff introduces no shared constant, port, address, asset id or version pin.
max_peers: 0is a test-local fixture value with no second consumer. - Should anything here be USING dig-constants? No. The only literals are
ChiaQueryConfigfields, owned by the chia-query defaults; importing them would move ownership away from the crate that defines the type.
Out of scope, agreed
The chia 0.26 -> 0.36.1 uplift (blocked on digstore-chain / dig_ecosystem#3149) and the dig-nat / dig-peer-selector deferral (two chia-bls lines in a crate still on 0.26) were both correctly not attempted. Blast radius via grep plus cargo tree -i per resolved-lock version is the sanctioned fallback and is declared in the body; the analysis is sound.
Verdict
CHANGES-REQUIRED, on the single doc-coherence finding below. It is cheap - a section deletion and a paragraph - and it is the one thing that leaves the ecosystem half-migrated: this crate’s own normative SPEC still specifies the seam the code no longer has.
(Recorded as a comment review: GitHub returns 422 for any verdict-bearing review event from the PR author identity, which the loop shares. The open inline thread is what bars the merge.)
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
MichaelTaylor3d
commented
Aug 24, 2026
loop-security: PASSAudited head: Read entirely from git objects ( Direct answer to the gating question: NO. A single dishonest peer gains no capability it did not have before this PR. Reasoning and the measured counts follow. Independent Chia dial stacks, measured before and afterProduction
Independent dial stacks 5 to 4. Peer sessions on a live-broadcast node 10 to 5. Every stack carrying an NC-12 agreement property is byte-identical between the two revs — verified by comparing blob SHAs, not by reading the diff: Why the pool that WAS removed carried no agreement propertyThe chia-query pool is not a voting mechanism. So: no read previously required two independent sources and now requires one. The second pool was an unreconciled duplicate, not a witness. The one real delta, and why it is not a gain for the attackerPre-PR an attacker needed a slot in pool A to lie about coin reads and a slot in pool B to lie about confirmation/lineage; post-PR one slot in pool A reaches both. That correlation is genuinely new. It does not amount to a gain, for two measured reasons:
The two costs move in opposite directions and neither dominates. I could not construct a scenario where one dishonest peer does something it previously could not. The five named attack surfaces1. The deleted stack was genuinely unreachable — independently confirmed, not taken on trust.
This is a dead-code deletion, not a capability deletion on the chain-read path. 2. Caveat, non-gating: 3. Trust-flag hygiene — clean, and nothing inherits a grant. Both pools used 4. Failure direction — the money path REFUSES. Correct direction, at file:line.
Honest qualifier, so nobody misreads the unification: "its peers disagree" has no branch on this path, before or after. The shared pool does not vote; disagreement detection lives in 5. IPv6-first (CLAUDE.md 5.2) — not implicated. The diff adds no dial path, no dial-back, and no address selection. The surviving dial is the pre-existing chia-query DNS-introducer discovery ( Section 908 — the custody boundary holds, and moves in the safe direction
Dependencies — purely subtractive, no pin loosened
Non-gating findings (report, do not hold the merge)S1 — Not a vulnerability, and it was already false at base (nothing constructed the client then either), so it is not a regression this PR introduces — but the PR is already editing the module doc directly above it, so section 4.2 says fix it here. S2 — the unified "one notion of the peak" is a THIRD-PARTY notion (pre-existing, outside the diff). This does not weaken the PR — both pools already read the peak from that same oracle, which is why they would rarely have disagreed, and the real benefit (halving the dials, removing a second unregistered trust domain) is unaffected. It matters because the PR narrative is "two notions of the peak becomes one"; the one is the coinset notion. Recommend recording it on eco#2790 rather than acting here — it is squarely that ticket "one peak measurement" goal. S3 — the money-path chain reads remain single-peer and uncorroborated. Round-robin, one peer per read, no agreement threshold on the The two claims I was asked to confirmCONFIRMED — the eco#2790 gap-2 clause is VACUOUSLY satisfied, and must not read as discharged. The clause is "no path constructs its own peer fabric outside the registry". After this PR One precision worth putting on #2790: its 2026-08-23 update widened gap 2 to "two chia-query lines means two provider registries". This PR does close that widening — CONFIRMED — the dig-node#249 remedy is uncarryable as written. It prescribes "Inject the node provider registry into Areas checked, and why each is clear
Verdict: PASS. No live security defect in this diff. S1 is a section-4.2 completeness defect for the correctness gate to rule on; S2 and S3 are pre-existing, outside the diff, and belong on eco#2790. |
…ired SPEC.md §4.1a specified the `chia-peer` light-client provider in the present tense, marked "Boundary (LOCKED, #1314)", instructing a reimplementer to build a `ProviderRegistry` the crate no longer has -- and contradicting the module doc added alongside the deletion. A normative sentence that describes absent code is worse than a missing one: it manufactures false defect reports and has already caused wrong fixes in this ecosystem. Deleted rather than softened. DEVELOPMENT_LOG.md carried the same `=0.5.1` blocker text removed from both Cargo.toml copies. Rewritten rather than deleted, because the durable lesson is the opposite of what it said: the pin was never load-bearing, and its articulate comment is exactly what made it look like it was. A blocker is verified against the resolved lock and a reference search, never against the note explaining it. Co-Authored-By: Claude <noreply@anthropic.com>
… fmt `cargo fmt` reflowed the backslash continuations into one long literal and baked the source indentation into the string, so the sentence a human reads when the guard fires carried two 14-space runs mid-clause. `concat!` keeps each piece a separate literal, which fmt cannot join. Co-Authored-By: Claude <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
DRAFT — DO NOT MERGE. Gate round has not run.
Closes nothing — deliberately. This PR was opened against #308 and #249 and does
NOT satisfy either; using the keyword would auto-close two tickets whose work is
unfinished.
This PR removes the
=0.5.1pin that stood in its way, so build(deps)!: uplift dig-wallet onto chia-wallet-sdk 0.34 / chia 0.36.1 #308 becomes a straightbump once
digstore-chainpublishes on 0.36.1.remedy does not apply as written.dig-node reports the chain tip from two independent measurements — make it one, without merging the connections #249 says to inject the node's provider registry
into
ChainTransport. There is no production provider registry: the onlyProviderRegistryin dig-node-core lived in the dead module this PR deletes. The twomeasurements (
dign wallet peak, a coinset-first oracle read;sync-status, thereplica's applied height) are still two, and still agree only by luck. dig-node reports the chain tip from two independent measurements — make it one, without merging the connections #249 needs
re-scoping onto what actually exists — commented there.
Epics (a submodule PR cannot close a super-repo issue — these close manually):
What this does
dig-node held three Chia dialling stacks. This removes one and merges another,
leaving one owner — without reducing the number of independently-held peer sessions
NC-12 depends on.
1. A third stack that was never wired at all.
dig-node-corecarried achia-peer-backed subscribing light client, meant to register as aChainSourceProviderinto achia_query::ProviderRegistry. Nothing constructed it.ProviderRegistryoccurs in that crate only in the module's own import, one signatureand its tests — no production registry, no caller, no path to it from a running node.
It was also the SOLE consumer of
chia-queryindig-node-core, and the reason forthe
chia-query = "=0.5.1"exact pin the manifest itself named as what held the crateoff the chia 0.36 line. Deleted.
2. A second full peer pool on the live-send path.
build_live_walletcalledChiaQuery::newitself, so a node withDIG_WALLET_ENABLE_LIVE_BROADCASTon held twoindependent five-peer pools — two sets of TLS sessions to two independently-chosen sets
of full nodes, each with its own notion of the peak. The wiring now takes the client
from
ChainTransport. The second pool is unexpressible, not merely unused:build_live_wallethas no way to construct one.Held connections: before → after
ChainTransportpool (serves reads, peak,chia_peer_count)chia-peerlight clientNC-12 plurality survives: still 5. What collapsed is two pools serving the SAME
role. The
ChiaQuorumCorroboratorandDialedPeerSamplekeep drawing their ownindependently-dialled peers, which is the agreement mechanism — unifying the owner must
not collapse the voices, and it does not.
Version lines — verified from the resolved lock, not the carets
chia-queryresolved to two lines (0.5.1 via dig-node-core, 0.6.2 via dig-wallet).It now resolves once, at 0.6.2, and
chia-peeris absent entirely. That isdig_ecosystem#2227's deliberate split, closed.
dig-chainsource-interfacestill resolves to two lines (0.1.0, 0.2.0). Not caused bythis PR and not fixed by it: 0.1.0 arrives via
dig-merkle → dig-store, independentlyof everything removed here. Reported, not chased — it is a different repo.
What is NOT here, and why
The chia 0.26 to 0.36.1 move is not in this PR — and that is now a deliberate scoping
call, not a blocker.
digstore-chain 0.28.0/digstore-core 0.28.0published while this PR was in the gate,on
chia-* ^0.36.1/chia-wallet-sdk ^0.34, so the original blocker is gone. I measuredthe move rather than assuming it, and it is a migration, not a manifest change:
digstore-host,-remote,-stage,-crypto,-compilerare not, so this cannot become version deps — all seven have tomove to the post-PR#50 rev together.
chia = "0.26"cannot be bumped: thechiaumbrella crate has NO 0.36 line. Its 55published versions step straight from 0.26 to
0.44.0 … 0.47.0. Everychia::*importmust be rewritten onto individual facade crates — 68 call sites across 20 files, in 7
sub-namespaces, with
chia::bls::SecretKey(11 sites) being exactly where a partialrewrite produces an internally-split crate.
dig-chat-protocol and dig-node-service, which must move as one set.
Expanding this PR to carry that would trade a green, complete change for a much larger one.
Measured and filed on #308 instead, with the corrected target set (
chia-*0.36.1,chia-wallet-sdk/-sdk-test/-sdk-utils0.34 — not 0.36, which is minor-incompatiblewith what
chia-query 0.15requires).What this PR contributes to it: the
=0.5.1pin and thechia-peerline that forced itare gone, and chia-query is down to one resolved line. That was the part that could not be
deferred; the remainder is now one coherent migration with no shim in it.
Blast radius checked
ProviderRegistry,connect_light_client,register_light_client_provider,submit_spend,confirmation_depth,ChiaPeerSubscriptions,CHIA_PEER_INDEPENDENCE_GROUP,dig_chainsource_interface— grepped across all crates;every one had zero references outside the deleted module and its own
mod.rsre-export.Inverse dependency (
cargo tree -i) run per resolved-lock version forchia-peer,chia-query@0.5.1,chia-query@0.6.2,dig-chainsource-interface@0.1.0and@0.2.0—that is what surfaced the
dig-merkleholder above, which the caret declarations do notshow.
Risk note: this removes a public module from
dig-node-core(BREAKING CHANGEfooter, minor bump on 0.x). No caller existed in this workspace; an out-of-workspace
consumer of
dig_node_core::seams::chia_peer::light_clientwould break, and there isnone — dig-node-core is not published to crates.io.
Evidence
build_live_wallet's ownChiaQuery::newfails it on the sharing assertion (not on the fixture precondition andnot on a compile error). It asserts reference-count sharing, then drops the wiring and
requires the count to return — so it cannot pass on references another holder
contributed. Asserting the two pools merely agreed would have passed against the
coincidence this ticket exists to remove.
cargo clippy --workspace --all-targets --all-featuresclean;cargo fmt --checkclean.Not proven: no running node was exercised. The held-connection reduction is argued
from construction (the second pool is unexpressible), not observed on a live host. The
default install is unaffected either way, because
enable_live_broadcastis default-OFF.