From 7a65b3921e715cd0b0028fa03dc1567cc15bb9c8 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sun, 30 Aug 2026 13:03:38 -0700 Subject: [PATCH 1/7] chore(mirror): open lane for the production broadcaster (#424) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2bb32fe..409ab7b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -830,3 +830,5 @@ This project adheres to [Semantic Versioning](https://semver.org) and - Fix rustup component install syntax + + From e9d0ec051d1a91e076fc76add83a8d7889882d9e Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sun, 30 Aug 2026 13:13:26 -0700 Subject: [PATCH 2/7] feat(mirror): wire a production broadcaster for the mirror lifecycle `production_broadcaster()` was a literal `None`, so `sign_and_broadcast` refused before signing and both the spend-record writer (#411) and the mirror signer (#410) were inert. `ChainTransport::broadcaster()` is the narrow counterpart to the existing public `chain_source()`: it returns a broadcaster built on the ONE shared client, so `shared_client` stays `pub(crate)` and no caller gains the client itself. The seam keeps its single-derivation role and gains a three-way answer, so an unreachable chain is no longer reported as a switched-off flag. On a default install no broadcaster is constructed and no chain is dialed. Refs #424 --- SPEC.md | 21 +- .../dig-node-service/src/mirror/lifecycle.rs | 271 ++++++++++++------ crates/dig-node-service/src/server.rs | 16 +- crates/dig-wallet/src/sage/chain.rs | 22 ++ 4 files changed, 232 insertions(+), 98 deletions(-) diff --git a/SPEC.md b/SPEC.md index 4511e7c5..a1c08889 100644 --- a/SPEC.md +++ b/SPEC.md @@ -8119,12 +8119,21 @@ address the wallet already tracks. > no confirmation is observed, and nothing reconciles an `unresolved` or `failed` one. Both are > tracked as . > -> **A mirror spend is BUILT and not SENT.** The node wires no production broadcaster for this -> lifecycle, so a planned reclaim refuses by name before it signs and no mirror spend reaches the -> mempool — the create half refuses separately, for the coin selector -> (). The refusal is reported rather than -> silent, and the capability the node announces is derived from the same seam, so it cannot claim a -> power it does not have. Tracked as . +> **A mirror spend is SENT only when the operator has enabled live broadcast.** The lifecycle +> builds its own `Broadcaster` on this node's ONE shared chain client, and it is built only when +> `DIG_WALLET_ENABLE_LIVE_BROADCAST` is on. On a default install — the flag defaults off — no +> broadcaster is constructed and no chain is dialed for one, so a planned reclaim refuses by name +> before it signs and no mirror spend reaches the mempool. The refusal is reported rather than +> silent, and the capability the node announces is derived from the SAME seam the money path is +> handed, so it cannot claim a power it does not have: `Available` holds exactly when a broadcaster +> is handed over. +> +> The broadcaster is built per pass rather than once at bring-up, because the shared client does not +> cache a failure — a node that started with no network broadcasts as soon as its network returns, +> and a node that cannot reach a chain reports that distinctly from a switched-off flag. +> +> Nothing is attached to the served `WalletBackend`: the broadcaster is scoped to the mirror +> lifecycle, signs only from the §16.4 operator wallet, and never acts on a user's behalf (§908). **Every spend is audited, structurally — exactly ONE entry per signature, and it cannot lie about the spend.** The signer takes the `SpendJournal` (§23.3) and opens the record itself, returning the diff --git a/crates/dig-node-service/src/mirror/lifecycle.rs b/crates/dig-node-service/src/mirror/lifecycle.rs index 7c47be97..fc377c31 100644 --- a/crates/dig-node-service/src/mirror/lifecycle.rs +++ b/crates/dig-node-service/src/mirror/lifecycle.rs @@ -37,13 +37,12 @@ //! all — so a node whose XCH is exhausted can still recover $DIG it has locked. That is §25.4.4, and //! it is the invariant that matters most, because its failure mode is collateral locked forever. //! -//! The spend is complete; the WIRING is not. This node attaches no production [`Broadcaster`] yet -//! ([`production_broadcaster`] is `None`, dig-node#424), so a planned reclaim refuses by name before -//! it signs, exactly as a create refuses for dig-node#421. Both refusals are reported, both name -//! their missing piece, and neither is a guess dressed as a spend. The reported -//! [`SpendCapability`] is DERIVED from the same seam, so the node cannot announce a power it does -//! not have: while the broadcaster is absent the capability is -//! [`SpendCapability::BroadcasterUnwired`] and never `Available`. +//! The spend is complete and, since dig-node#424, so is the WIRING: [`production_broadcaster`] +//! builds a [`Broadcaster`] on this node's ONE shared chain client whenever the operator has +//! enabled live broadcast. On a default install it builds nothing and dials nothing, so a planned +//! reclaim still refuses by name before it signs. Every refusal is reported, names its missing +//! piece, and is never a guess dressed as a spend. The reported [`SpendCapability`] is DERIVED from +//! the same seam, so the node cannot announce a power it does not have. //! //! **Creates select their own collateral, from the OPERATOR wallet.** `dig_mirror_coin::create` //! takes its `Cat` inputs from its caller, and [`super::funding`] supplies them: it scans the chain @@ -83,6 +82,7 @@ use dig_mirror_coin::MirrorCoin; use dig_node_core::Node; use dig_wallet::autoseed::WalletPaths; use dig_wallet::operator_wallet::OperatorWallet; +use dig_wallet::sage::chain::ChainTransport; use dig_wallet::sage::rpc::WalletBackend; use dig_wallet::sage::spend::Broadcaster; @@ -523,14 +523,18 @@ pub enum SpendCapability { WalletUnavailable, /// A wallet opened but `DIG_WALLET_ENABLE_LIVE_BROADCAST` is off, the money-safe default. BroadcastDisabled, - /// Everything the OPERATOR controls is in place — a wallet opened and live broadcast is on — but - /// this build wires no production [`Broadcaster`] (dig-node#424), so a spend can be built and - /// signed for and still reach nothing. + /// Everything the OPERATOR controls is in place — a wallet opened and live broadcast is on — + /// but this node could not reach a chain to build a broadcaster on, so a spend could be built + /// and signed for and still reach nothing. /// /// Distinguished from [`Self::BroadcastDisabled`] because the two ask different things of the - /// reader: one is a switch they can flip, and this one is not. Reporting a missing wiring as a - /// disabled flag sends an operator to set a flag that is already set. - BroadcasterUnwired, + /// reader: one is a switch they can flip, and this one is not. Reporting an unreachable chain + /// as a disabled flag sends an operator to set a flag that is already set. + /// + /// It replaced `BroadcasterUnwired`, which said "this build wires no broadcaster" and became + /// false when dig-node#424 wired one. A variant no branch can produce reports a state the node + /// can no longer be in. + ChainUnreachable, } impl SpendCapability { @@ -540,14 +544,80 @@ impl SpendCapability { } } +/// What this build can do about SENDING a signed mirror spend — the ONE seam. +/// +/// Read by both the reported capability ([`open_signer`]) and the effects the scheduler builds, +/// precisely so the two cannot disagree. The alternative — a capability computed from the +/// environment and a broadcaster passed separately at the construction site — is what let this node +/// log "this node may create and reclaim collateral" while every reclaim refused: two answers to one +/// question, and only one of them on the path the money takes. +pub enum MirrorBroadcast { + /// A broadcaster built on this node's one shared chain client. + Wired(Arc), + /// `DIG_WALLET_ENABLE_LIVE_BROADCAST` is off — the money-safe default. NOTHING was built and + /// nothing was dialed. + Disabled, + /// Live broadcast is on, but no chain could be reached to build a broadcaster on. + ChainUnreachable, +} + +impl MirrorBroadcast { + /// The broadcaster to hand a pass, borrowed for as long as this value lives. + /// + /// `None` on every non-`Wired` outcome, so a pass refuses by name in `sign_and_broadcast` + /// rather than sending into something that is not there. + pub fn broadcaster(&self) -> Option<&dyn Broadcaster> { + match self { + Self::Wired(b) => Some(b.as_ref()), + Self::Disabled | Self::ChainUnreachable => None, + } + } +} + +/// Build the mirror lifecycle's [`Broadcaster`], or say why there is none (dig-node#424). +/// +/// # `live_broadcast` is checked FIRST, and that placement is the property +/// +/// On a default install nothing is constructed and no chain is dialed — the broadcaster does not +/// merely go unused, it never exists, so there is nothing for a later edit to attach. Checking the +/// flag downstream instead would leave a fully-built production broadcaster sitting in scope on +/// every default install, which is one line away from sending. +/// +/// # Called per pass rather than built once +/// +/// [`ChainTransport::broadcaster`](dig_wallet::sage::chain::ChainTransport::broadcaster) does not +/// cache a failure, so a node that starts offline can broadcast the moment its network returns. +/// Building this once at bring-up would silently convert that into a node that never broadcasts +/// again for the rest of its life. +pub async fn production_broadcaster( + chain: &ChainTransport, + live_broadcast: bool, +) -> MirrorBroadcast { + if !live_broadcast { + return MirrorBroadcast::Disabled; + } + match chain.broadcaster().await { + Ok(b) => MirrorBroadcast::Wired(b), + Err(e) => { + tracing::warn!( + target: "mirror", + error = %e, + "live broadcast is enabled but no chain could be reached to build a broadcaster on; mirror spends will be refused by name until one can be" + ); + MirrorBroadcast::ChainUnreachable + } + } +} + /// Open the operator wallet, or say why the lifecycle cannot spend. /// /// [`OperatorWallet::open`] returns `None` for BOTH §16.4 `Locked` and `Orphaned`, which is the /// behaviour this wants: neither state has a key it would be correct to substitute, so the honest /// outcome is the same in both — no signer, and a lifecycle that observes without spending. -pub fn open_signer( +pub async fn open_signer( paths: &WalletPaths, live_broadcast: bool, + chain: &ChainTransport, ) -> (Option, SpendCapability) { let Some(wallet) = OperatorWallet::open(paths, dig_constants::DIG_MAINNET.genesis_challenge()) else { @@ -564,38 +634,23 @@ pub fn open_signer( // broadcaster before it signs. ( Some(MirrorSigner::new(wallet)), - spend_capability(production_broadcaster().is_some()), + spend_capability(&production_broadcaster(chain, live_broadcast).await), ) } -/// What an OPENED wallet with live broadcast on can actually do, given whether a broadcaster exists. +/// What an OPENED wallet with live broadcast on can actually do, given the seam's own answer. /// -/// Separated from [`open_signer`] so the decision is reachable from a test on both branches: the -/// wired branch cannot be exercised through `open_signer` on a build where -/// [`production_broadcaster`] is `None`, and an untestable branch is how the previous version came -/// to report `Available` on a node that could not send. -pub fn spend_capability(broadcaster_wired: bool) -> SpendCapability { - if broadcaster_wired { - SpendCapability::Available - } else { - SpendCapability::BroadcasterUnwired +/// Separated from [`open_signer`] so the decision is reachable from a test on every branch. An +/// untestable branch is how the previous version came to report `Available` on a node that could +/// not send. +pub fn spend_capability(broadcast: &MirrorBroadcast) -> SpendCapability { + match broadcast { + MirrorBroadcast::Wired(_) => SpendCapability::Available, + MirrorBroadcast::Disabled => SpendCapability::BroadcastDisabled, + MirrorBroadcast::ChainUnreachable => SpendCapability::ChainUnreachable, } } -/// The [`Broadcaster`] this build attaches to the mirror lifecycle — `None` until dig-node#424. -/// -/// ONE seam, read by both the reported capability ([`open_signer`]) and the effects the scheduler -/// builds, precisely so the two cannot disagree. The alternative — a capability computed from the -/// environment and a broadcaster passed separately at the construction site — is what let this node -/// log "this node may create and reclaim collateral" while every reclaim refused: two answers to one -/// question, and only one of them on the path the money takes. -/// -/// `&'static` because the honest answer is a property of the build rather than of a request, and -/// because a static coerces into the shorter lifetime `NodeMirrorEffects` borrows for. -pub fn production_broadcaster() -> Option<&'static dyn Broadcaster> { - None -} - /// Publish what a pass observed, so `control.mirror.bondStates` can answer from it. /// /// A TRANSLATION of [`PassReport`], never a recomputation: the states and the locked total are @@ -707,82 +762,124 @@ mod tests { ] } - /// `Available` and a wired broadcaster are the SAME fact, so the node cannot announce a power a - /// spend does not have. + /// A DEFAULT INSTALL DOES NOT SPEND, and the seam itself is what guarantees it. /// - /// This is the assertion the earlier wiring had no room for. `open_signer` reported `Available` - /// from the environment while the scheduler built its effects with a hard-coded `None`, so the - /// bring-up log said "this node may create and reclaim collateral" and every reclaim refused — - /// two answers to one question, with only one of them on the path the money takes. Deriving both - /// from [`production_broadcaster`] makes disagreement inexpressible, and this test fails the - /// moment someone reintroduces a second source: report `Available` with no broadcaster wired, or - /// wire one while still reporting `BroadcasterUnwired`, and the implication below breaks. + /// `enable_live_broadcast` defaults false (`config.rs:176`/`:242`). This asserts the property at + /// its strongest available point: with live broadcast off the seam does not merely decline to + /// USE a broadcaster, it never BUILDS one and never dials — so there is no production + /// broadcaster in scope for a later edit to attach, and `NodeMirrorEffects` is handed `None`. /// - /// Written as an implication over the seam rather than as `assert_eq!(capability, Unwired)` - /// because the second spelling becomes a FALSE failure the day dig-node#424 lands — a test that - /// has to be deleted to ship the fix is not guarding the property, it is guarding the gap. - #[test] - fn an_opened_wallet_with_broadcast_enabled_still_may_not_spend_with_no_broadcaster_wired() { + /// The fixture is a real [`ChainTransport`], not a double, and it is the honest control: it is + /// perfectly capable of building a broadcaster (the test below builds one through the same + /// value), so a pass here means the FLAG stopped it rather than the fixture being unable to + /// produce one. A transport that could never broadcast would make this test pass for the wrong + /// reason. + /// + /// This is the assertion that protects every existing user, so it is deliberately written to be + /// the hardest one here to delete without noticing. + #[tokio::test] + async fn a_default_install_builds_no_broadcaster_and_cannot_spend() { use dig_wallet::autoseed::BootstrapState; + let chain = ChainTransport::new(); + + let seam = production_broadcaster(&chain, false).await; + assert!( + matches!(seam, MirrorBroadcast::Disabled), + "with live broadcast off the seam must report the switched-off flag, never a built \ + broadcaster and never a chain error" + ); + assert!( + seam.broadcaster().is_none(), + "a default install must hand the pass NO broadcaster, so sign_and_broadcast refuses \ + before it signs" + ); + assert_eq!( + spend_capability(&seam), + SpendCapability::BroadcastDisabled, + "and it must SAY so, rather than announcing a power it will not use" + ); + assert!(!spend_capability(&seam).may_spend()); + + // And the same through the production entry point, on a real opened wallet — so this holds + // for the path a node actually takes at bring-up, not only for the seam in isolation. let dir = tempfile::tempdir().expect("a temp dir"); let paths = WalletPaths::resolve(dir.path().join("seed")); - - // A REAL operator wallet, minted into the temp layout. Without one this test would take the - // `WalletUnavailable` path and pass while never reaching the decision under test — which is - // exactly how the first version of it went green against the very regression it names. let state = crate::wallet_bootstrap::ensure_wallet_seed_at(&paths) .expect("the autoseed bootstrap yields a state"); assert!( matches!(state, BootstrapState::Created | BootstrapState::Opened), - "the fixture must actually OPEN a wallet, or the assertions below are vacuous: {state:?}" + "the fixture must actually OPEN a wallet, or the assertion below is vacuous: {state:?}" ); - // Live broadcast ON — the operator has already done everything they can do. - let (signer, capability) = open_signer(&paths, true); - + let (signer, capability) = open_signer(&paths, false, &chain).await; assert!( - signer.is_some(), - "an opened wallet yields a signer; the observation half needs its puzzle hash" - ); - assert_eq!( - capability, - spend_capability(production_broadcaster().is_some()), - "open_signer must REPORT what the pass will actually be handed, never re-decide it" + signer.is_none(), + "a default install yields no signer, so nothing can be signed even if a broadcaster \ + were somehow attached" ); + assert_eq!(capability, SpendCapability::BroadcastDisabled); + assert!(!capability.may_spend()); + } - if production_broadcaster().is_none() { + /// The announced capability and the spend path are ONE derivation, not two that happen to agree. + /// + /// This is the assertion the earlier wiring had no room for. `open_signer` reported `Available` + /// from the environment while the scheduler built its effects with a hard-coded `None`, so the + /// bring-up log said "this node may create and reclaim collateral" and every reclaim refused — + /// two answers to one question, with only one of them on the path the money takes. + /// + /// Written as an equivalence over EVERY seam outcome rather than as a pair of literal + /// assertions: it says `Available` holds exactly when a broadcaster is handed over, so it fails + /// both ways round — announce a power with nothing to send through, or send through something + /// while announcing less. A test that only checked today's values would pass against a second + /// source that happened to agree at the moment it was written. + #[tokio::test] + async fn the_announced_capability_holds_exactly_when_a_broadcaster_is_handed_over() { + let chain = ChainTransport::new(); + + for seam in [ + production_broadcaster(&chain, false).await, + MirrorBroadcast::ChainUnreachable, + MirrorBroadcast::Wired(std::sync::Arc::new( + dig_wallet::sage::spend::MockBroadcaster::default(), + )), + ] { assert_eq!( - capability, - SpendCapability::BroadcasterUnwired, - "with the seam empty the honest answer is the missing wiring, not a switched-off \ - flag the operator has already switched on" - ); - assert!( - !capability.may_spend(), - "and a node that cannot broadcast must not announce that it may create and reclaim" + spend_capability(&seam).may_spend(), + seam.broadcaster().is_some(), + "the capability the node ANNOUNCES and the broadcaster the money path is HANDED \ + must be the same fact; two answers to one question is the regression this seam \ + exists to make inexpressible" ); } } - /// Both branches of the capability decision, including the one this build cannot reach. + /// Every branch of the capability decision, including the ones a given build cannot reach. /// - /// Separate from the test above because that one can only exercise the branch the current - /// [`production_broadcaster`] selects. A branch no fixture can take reads as covered while - /// never having run once, so the wired branch is asserted directly. + /// A branch no fixture can take reads as covered while never having run once, so each is + /// asserted directly. #[test] - fn the_capability_decision_says_available_only_for_a_wired_broadcaster() { + fn the_capability_decision_names_each_seam_outcome() { assert_eq!( - spend_capability(true), + spend_capability(&MirrorBroadcast::Wired(std::sync::Arc::new( + dig_wallet::sage::spend::MockBroadcaster::default() + ))), SpendCapability::Available, "a wired broadcaster is what Available means" ); assert_eq!( - spend_capability(false), - SpendCapability::BroadcasterUnwired, - "and its absence is a different answer, not the same one" + spend_capability(&MirrorBroadcast::Disabled), + SpendCapability::BroadcastDisabled, + "a switched-off flag is a switch the operator can flip" + ); + assert_eq!( + spend_capability(&MirrorBroadcast::ChainUnreachable), + SpendCapability::ChainUnreachable, + "and an unreachable chain is NOT that, so it must not be reported as one" ); - assert!(!spend_capability(false).may_spend()); + assert!(!spend_capability(&MirrorBroadcast::Disabled).may_spend()); + assert!(!spend_capability(&MirrorBroadcast::ChainUnreachable).may_spend()); } /// What a real installation LOOKS like, assembled independently of the needles. diff --git a/crates/dig-node-service/src/server.rs b/crates/dig-node-service/src/server.rs index 8e7670d3..8d45d1e7 100644 --- a/crates/dig-node-service/src/server.rs +++ b/crates/dig-node-service/src/server.rs @@ -2688,7 +2688,7 @@ fn spawn_mirror_passes( tokio::spawn(async move { let paths = dig_wallet::autoseed::default_paths(); - let (signer, capability) = lifecycle::open_signer(&paths, live_broadcast); + let (signer, capability) = lifecycle::open_signer(&paths, live_broadcast, &chain).await; // The owner puzzle hash comes from the SIGNER when there is one, so the key a spend is built // for and the address its bonds are observed under cannot be two different values. Without a @@ -2724,11 +2724,11 @@ fn spawn_mirror_passes( ), // Deliberately NOT phrased as a flag to set: the operator has already set // DIG_WALLET_ENABLE_LIVE_BROADCAST to reach this arm at all. - SpendCapability::BroadcasterUnwired => tracing::info!( + SpendCapability::ChainUnreachable => tracing::warn!( target: "mirror", "the mirror lifecycle OBSERVES only: the wallet opened and live broadcast is on, \ - but this build wires no broadcaster (dig-node#424), so a reclaim is planned and \ - reported and no spend is sent" + but no chain could be reached to broadcast through, so a reclaim is planned \ + and reported and no spend is sent" ), } @@ -2772,6 +2772,12 @@ fn spawn_mirror_passes( match chain.chain_source(tokio::runtime::Handle::current()).await { Ok(source) => { + // Re-read per pass, deliberately: `ChainTransport::broadcaster` does not + // cache a failure, so a node that started offline can broadcast the moment + // its network returns. Holding one built at bring-up would silently make + // that node one that never broadcasts again. + let broadcast = + lifecycle::production_broadcaster(&chain, live_broadcast).await; let runtime = tokio::runtime::Handle::current(); let signer_ref = signer.as_ref(); let ctx = PassContext { @@ -2803,7 +2809,7 @@ fn spawn_mirror_passes( // The SAME seam `open_signer` derived the reported capability from, so // what this node says it can do and what a spend can actually reach // cannot be two different answers (dig-node#424). - lifecycle::production_broadcaster(), + broadcast.broadcaster(), runtime, ); let mut pass = diff --git a/crates/dig-wallet/src/sage/chain.rs b/crates/dig-wallet/src/sage/chain.rs index 28683ec8..a2c47d69 100644 --- a/crates/dig-wallet/src/sage/chain.rs +++ b/crates/dig-wallet/src/sage/chain.rs @@ -252,6 +252,28 @@ impl ChainTransport { self.client().await } + /// A [`Broadcaster`](super::spend::Broadcaster) that pushes through the ONE shared client. + /// + /// The narrow counterpart to [`Self::chain_source`], and it exists for the same reason: a + /// consumer that needs to SEND a signed bundle needs the ability to push, not the client + /// itself. Handing out [`Self::shared_client`] would hand out every other power the client has, + /// and building a second client is what gave a live node two independent sets of full-node + /// sessions with two notions of the peak (dig_ecosystem#2761). + /// + /// It is deliberately NOT an `impl Broadcaster for ChainTransport`. An unused one sat in this + /// file once and made a one-line `.with_broadcaster(chain.clone())` compile, pass every test, + /// and silently enable node-custodied sending on a default install. A caller must ask for a + /// broadcaster by name, and asking is a visible line in a diff. + /// + /// # Errors + /// + /// The lazy client build — this node could not reach a chain at all. Not cached, so a later + /// call tries again; a node that starts offline can broadcast the moment its network returns. + pub async fn broadcaster(&self) -> Result> { + let client = self.shared_client().await?; + Ok(Arc::new(super::spend::ChiaQueryBroadcaster::new(client))) + } + /// This transport's chain reads presented as the canonical /// [`ChainSource`](chia_query::provider_registry::interface::ChainSource) — the trait every DIG consumer of /// chain state depends on. From 8a5659c315f7d502dfd0d04778cb7ef07fcebb19 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sun, 30 Aug 2026 13:16:44 -0700 Subject: [PATCH 3/7] chore(mirror): fmt, fix the remaining open_signer test call site, bump to 0.178.0 --- crates/dig-node-service/src/mirror/lifecycle.rs | 7 ++++--- crates/dig-node-service/src/server.rs | 3 +-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/dig-node-service/src/mirror/lifecycle.rs b/crates/dig-node-service/src/mirror/lifecycle.rs index fc377c31..3ef37475 100644 --- a/crates/dig-node-service/src/mirror/lifecycle.rs +++ b/crates/dig-node-service/src/mirror/lifecycle.rs @@ -734,12 +734,13 @@ mod tests { /// an operator fixes them differently: an absent wallet needs a seed, a disabled broadcast needs /// an environment variable. A test that only checked `signer.is_none()` would pass against an /// implementation that reported either reason for both. - #[test] - fn a_disabled_broadcast_is_reported_differently_from_an_unopenable_wallet() { + #[tokio::test] + async fn a_disabled_broadcast_is_reported_differently_from_an_unopenable_wallet() { let empty = tempfile::tempdir().expect("a temp dir"); let paths = WalletPaths::resolve(empty.path().join("seed")); + let chain = ChainTransport::new(); - let (signer, capability) = open_signer(&paths, true); + let (signer, capability) = open_signer(&paths, true, &chain).await; assert!(signer.is_none(), "no seed exists, so nothing may sign"); assert_eq!( capability, diff --git a/crates/dig-node-service/src/server.rs b/crates/dig-node-service/src/server.rs index 8d45d1e7..2fc26293 100644 --- a/crates/dig-node-service/src/server.rs +++ b/crates/dig-node-service/src/server.rs @@ -2776,8 +2776,7 @@ fn spawn_mirror_passes( // cache a failure, so a node that started offline can broadcast the moment // its network returns. Holding one built at bring-up would silently make // that node one that never broadcasts again. - let broadcast = - lifecycle::production_broadcaster(&chain, live_broadcast).await; + let broadcast = lifecycle::production_broadcaster(&chain, live_broadcast).await; let runtime = tokio::runtime::Handle::current(); let signer_ref = signer.as_ref(); let ctx = PassContext { From 2ca77cf65363b4f288369d194aec9d444c6174bf Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sun, 30 Aug 2026 13:23:03 -0700 Subject: [PATCH 4/7] chore: drop the WIP lane marker from CHANGELOG --- CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 409ab7b3..d2bb32fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -830,5 +830,3 @@ This project adheres to [Semantic Versioning](https://semver.org) and - Fix rustup component install syntax - - From f919982b13db4b8b7a3ebb79fc4c2c8e9ae5c621 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sun, 30 Aug 2026 15:03:43 -0700 Subject: [PATCH 5/7] chore: bump to 0.179.0 after rebasing onto 3d15118 PR #431 took 0.178.0 on main, so this branch's original bump to 0.178.0 was dropped by the rebase as 'already upstream' -- leaving no effective version increase. 0.179.0 is the first number past what main holds. Cargo.lock regenerated; CI runs --locked. --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 612d3e61..de9fe7e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3031,7 +3031,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.178.0" +version = "0.179.0" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index 408c5693..696fd3f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,7 @@ edition = "2021" # the ROOT manifest (`[workspace.package].version`), so it MUST be set here for a # release to fire (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) # keep their own independent versions — only the released binary tracks the workspace version. -version = "0.178.0" +version = "0.179.0" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over From 1bbb94b2fd8afa711451ad1b0dcf23b20866ad70 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sun, 30 Aug 2026 15:28:09 -0700 Subject: [PATCH 6/7] fix(mirror): a PENDING mempool ack is not an accepted broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from the adversarial gate on #434, all downstream of an operator having already enabled live broadcast. MONEY. `ChiaQueryBroadcaster::broadcast` read `TxStatus::success`, which `chia_query`'s `ack_to_tx_status` sets true for BOTH status 1 (SUCCESS) and status 2 (PENDING). PENDING means the full node did not admit the bundle to its mempool — held for an unknown parent, or a fee below the floor — so the broadcaster returned `Ok(())` for a submission that never happened, contradicting the `Broadcaster` contract in the same file. The journal then recorded it and the intra-pass reservation stranded the funding coin against a spend the network was not holding. Fixed locally by reading the status NAME the crate already produces, through a pure `accepted_by_mempool`. That consumes a distinction `chia-query` makes rather than re-deriving one, so it is not a rival implementation. The crate's own conflation is filed as DIG-Network/chia-query#48 and is a release-first cascade, not this PR. HONESTY. `ChainUnreachable` promised a reachability claim nothing makes: the client build succeeds with zero peers because the default config's coinset fallback tier makes the peer requirement optional. Renamed to `ChainClientUnavailable` and documented for what it observes — a failed CONSTRUCTION. `Available` now says plainly that it means a broadcaster was built, not that a chain answers; unreachability is discovered at the broadcast, by name. The three-way shape and the per-pass re-read are kept unchanged, so a node that starts offline still recovers. TESTS. The default-install test claimed a control no test exercised. Its fixture is now actually driven through `production_broadcaster(.., true)`, which both makes that test non-vacuous and pins what `Wired` means. MESSAGES. The refusal named a closed ticket instead of the flag an operator can reach; it now names both reasons it can fire. Four string literals had lost their line continuations. Co-Authored-By: Claude --- .../dig-node-service/src/mirror/lifecycle.rs | 153 ++++++++++++++---- crates/dig-node-service/src/server.rs | 6 +- crates/dig-wallet/src/sage/spend.rs | 97 ++++++++++- 3 files changed, 213 insertions(+), 43 deletions(-) diff --git a/crates/dig-node-service/src/mirror/lifecycle.rs b/crates/dig-node-service/src/mirror/lifecycle.rs index 3ef37475..df72ebdf 100644 --- a/crates/dig-node-service/src/mirror/lifecycle.rs +++ b/crates/dig-node-service/src/mirror/lifecycle.rs @@ -217,15 +217,21 @@ impl<'a, S: ChainSource> NodeMirrorEffects<'a, S> { let signer = self .signer .ok_or_else(|| PassError::Wallet("no operator wallet is available to sign".into()))?; - // Named the way `create` names dig-node#421: at the point this refusal is reachable, the - // operator has ALREADY set `DIG_WALLET_ENABLE_LIVE_BROADCAST` — `open_signer` yields no - // signer without it, and the signer is checked first — so blaming that flag would tell a - // person to set the flag they just set. What is missing is the wiring, and the wiring has a - // ticket. + // Names the two reasons a broadcaster can be absent NOW that one is wired (dig-node#424). + // + // The message used to say the build wires no broadcaster at all. That was true when it was + // written and became false in the commit that wired one — and worse than merely stale: it + // sent an operator to a CLOSED ticket instead of to the switch they can actually reach. + // + // Only two states reach here, and they are exactly `MirrorBroadcast`'s two non-`Wired` + // outcomes: the flag is off, or the shared chain client could not be built. Both are named, + // because this refusal cannot see which of the two produced the `None` it was handed. let broadcaster = self.broadcaster.ok_or_else(|| { PassError::Wallet( - "this node has no production broadcaster wired for the mirror lifecycle \ - (dig-node#424), so the spend was built and then NOT sent; nothing was signed" + "this node has no broadcaster for the mirror lifecycle, so the spend was built and \ + then NOT sent; nothing was signed. Either DIG_WALLET_ENABLE_LIVE_BROADCAST is off \ + (the money-safe default — set it to 1 to allow automated mirror spends), or it is \ + on and this node could not build a chain client to send through" .into(), ) })?; @@ -248,7 +254,8 @@ impl<'a, S: ChainSource> NodeMirrorEffects<'a, S> { tracing::warn!( target: "mirror", operation = spends.operation().as_str(), - "broadcast a mirror spend whose created coin this node cannot derive; the audit entry names no target coin rather than naming a guessed one" + "broadcast a mirror spend whose created coin this node cannot derive; the \ + audit entry names no target coin rather than naming a guessed one" ); } // Recorded UNCONDITIONALLY, and the two facts are recorded independently. The @@ -524,17 +531,27 @@ pub enum SpendCapability { /// A wallet opened but `DIG_WALLET_ENABLE_LIVE_BROADCAST` is off, the money-safe default. BroadcastDisabled, /// Everything the OPERATOR controls is in place — a wallet opened and live broadcast is on — - /// but this node could not reach a chain to build a broadcaster on, so a spend could be built - /// and signed for and still reach nothing. + /// but this node could not BUILD the shared chain client a broadcaster is made from, so a spend + /// could be built and signed for and still reach nothing. /// /// Distinguished from [`Self::BroadcastDisabled`] because the two ask different things of the - /// reader: one is a switch they can flip, and this one is not. Reporting an unreachable chain + /// reader: one is a switch they can flip, and this one is not. Reporting a failed client build /// as a disabled flag sends an operator to set a flag that is already set. /// - /// It replaced `BroadcasterUnwired`, which said "this build wires no broadcaster" and became - /// false when dig-node#424 wired one. A variant no branch can produce reports a state the node - /// can no longer be in. - ChainUnreachable, + /// # It is named for a CONSTRUCTION failure because that is the only thing it observes + /// + /// It was called `ChainClientUnavailable`, which promised more than it can deliver. The client build + /// is not a reachability test: `ChiaQueryConfig`'s default enables the coinset fallback tier, + /// which makes the peer tier `PeerRequirement::Optional`, so the build SUCCEEDS on a node with + /// zero peers and no network at all. A name that says "unreachable" would be read as a chain + /// probe by everyone who met it, and nothing here probes anything. + /// + /// So the honest reading of the pair is: [`Self::Available`] means a broadcaster was + /// constructed, NOT that a chain answers. A node whose network dies mid-life keeps announcing + /// `Available`, and it is the broadcast itself that discovers the truth and refuses by name. + /// That is deliberate rather than tolerated — the alternative is a reachability round-trip at + /// bring-up, which buys a one-shot answer that is stale by the first pass. + ChainClientUnavailable, } impl SpendCapability { @@ -553,12 +570,15 @@ impl SpendCapability { /// question, and only one of them on the path the money takes. pub enum MirrorBroadcast { /// A broadcaster built on this node's one shared chain client. + /// + /// Means the client was CONSTRUCTED, not that a chain answers — see + /// [`SpendCapability::ChainClientUnavailable`] for why the two cannot be told apart here. Wired(Arc), /// `DIG_WALLET_ENABLE_LIVE_BROADCAST` is off — the money-safe default. NOTHING was built and /// nothing was dialed. Disabled, - /// Live broadcast is on, but no chain could be reached to build a broadcaster on. - ChainUnreachable, + /// Live broadcast is on, but the shared chain client could not be built. + ChainClientUnavailable, } impl MirrorBroadcast { @@ -569,7 +589,7 @@ impl MirrorBroadcast { pub fn broadcaster(&self) -> Option<&dyn Broadcaster> { match self { Self::Wired(b) => Some(b.as_ref()), - Self::Disabled | Self::ChainUnreachable => None, + Self::Disabled | Self::ChainClientUnavailable => None, } } } @@ -588,7 +608,24 @@ impl MirrorBroadcast { /// [`ChainTransport::broadcaster`](dig_wallet::sage::chain::ChainTransport::broadcaster) does not /// cache a failure, so a node that starts offline can broadcast the moment its network returns. /// Building this once at bring-up would silently convert that into a node that never broadcasts -/// again for the rest of its life. +/// again for the rest of its life. The per-pass re-read is that retry, and it is the reason this is +/// a function of the transport rather than a value computed once. +/// +/// # What `Wired` actually asserts, and what the error arm actually observes +/// +/// `Wired` means the shared chain client was CONSTRUCTED. It is not a reachability claim: the +/// default `ChiaQueryConfig` enables the coinset fallback tier, so the build succeeds on a node +/// holding no peers and reaching no network. A node that is offline therefore reports `Wired` and +/// `Available`, and the refusal it deserves arrives at the broadcast, by name, from +/// [`Broadcaster::broadcast`] — which is also the only place that can honestly know. +/// +/// So [`MirrorBroadcast::ChainClientUnavailable`] is narrow by construction, and where it is +/// reachable depends on the caller. From [`open_signer`] at bring-up it is the first thing to need +/// a client, and a build failure there produces it. From the scheduler it cannot fire at all: that +/// call site sits inside the `Ok` arm of `ChainTransport::chain_source`, which has already built +/// the client, and a SUCCESSFUL build is cached. It is kept rather than deleted because it is the +/// honest error arm of a fallible call with a live producer — not because a branch might one day +/// want it. pub async fn production_broadcaster( chain: &ChainTransport, live_broadcast: bool, @@ -602,9 +639,10 @@ pub async fn production_broadcaster( tracing::warn!( target: "mirror", error = %e, - "live broadcast is enabled but no chain could be reached to build a broadcaster on; mirror spends will be refused by name until one can be" + "live broadcast is enabled but this node could not build the shared chain client a \ + broadcaster is made from; mirror spends will be refused by name until it can" ); - MirrorBroadcast::ChainUnreachable + MirrorBroadcast::ChainClientUnavailable } } } @@ -647,7 +685,7 @@ pub fn spend_capability(broadcast: &MirrorBroadcast) -> SpendCapability { match broadcast { MirrorBroadcast::Wired(_) => SpendCapability::Available, MirrorBroadcast::Disabled => SpendCapability::BroadcastDisabled, - MirrorBroadcast::ChainUnreachable => SpendCapability::ChainUnreachable, + MirrorBroadcast::ChainClientUnavailable => SpendCapability::ChainClientUnavailable, } } @@ -771,10 +809,16 @@ mod tests { /// broadcaster in scope for a later edit to attach, and `NodeMirrorEffects` is handed `None`. /// /// The fixture is a real [`ChainTransport`], not a double, and it is the honest control: it is - /// perfectly capable of building a broadcaster (the test below builds one through the same - /// value), so a pass here means the FLAG stopped it rather than the fixture being unable to - /// produce one. A transport that could never broadcast would make this test pass for the wrong - /// reason. + /// perfectly capable of building a broadcaster, so a pass here means the FLAG stopped it rather + /// than the fixture being unable to produce one. A transport that could never broadcast would + /// make this test pass for the wrong reason. + /// + /// That control is not asserted here, because a claim about the fixture is worth nothing until + /// something exercises it: it is + /// [`the_same_fixture_does_build_a_broadcaster_when_the_flag_is_on`], which drives this exact + /// value through `production_broadcaster(.., true)`. Until that test existed this doc claimed a + /// control no test performed — the two other assertions in this file build their broadcaster + /// with `MirrorBroadcast::Wired(MockBroadcaster)` directly and never touch a transport. /// /// This is the assertion that protects every existing user, so it is deliberately written to be /// the hardest one here to delete without noticing. @@ -823,6 +867,49 @@ mod tests { assert!(!capability.may_spend()); } + /// The control the default-install test relies on, actually exercised. + /// + /// It does two jobs, and the second is the one worth having. + /// + /// First, it makes + /// [`a_default_install_builds_no_broadcaster_and_cannot_spend`] non-vacuous: the SAME fixture + /// value, with the flag flipped and nothing else changed, DOES produce a broadcaster. So the + /// default-install refusal is attributable to the flag rather than to a fixture that could + /// never have built one either way. Flipping one input and holding the rest is the whole point; + /// a second, differently-built transport would prove nothing about the first. + /// + /// Second, it pins what `Wired` MEANS. This test runs with no network and no peers, and it + /// asserts `Wired` anyway — because the client build is not a reachability probe (the default + /// config's coinset fallback tier makes the peer requirement optional). Anyone who later + /// "fixes" `production_broadcaster` to report an offline node as + /// [`MirrorBroadcast::ChainClientUnavailable`] fails here, and should: that would be a + /// reachability claim the build cannot make, and it would trade a truthful `Available` for a + /// permanently-stuck refusal on a node whose network recovers. + #[tokio::test] + async fn the_same_fixture_does_build_a_broadcaster_when_the_flag_is_on() { + let chain = ChainTransport::new(); + + // The one input that differs from the default-install test is `live_broadcast`. + let seam = production_broadcaster(&chain, true).await; + + assert!( + matches!(seam, MirrorBroadcast::Wired(_)), + "a real ChainTransport must build a broadcaster once the flag is on, or the \ + default-install test is passing because the fixture cannot broadcast at all rather \ + than because the flag stopped it" + ); + assert!( + seam.broadcaster().is_some(), + "and the built broadcaster must actually be handed over" + ); + assert_eq!( + spend_capability(&seam), + SpendCapability::Available, + "Available means a broadcaster was CONSTRUCTED on the shared chain client — this node \ + has no network here, and that is deliberately not what the capability reports" + ); + } + /// The announced capability and the spend path are ONE derivation, not two that happen to agree. /// /// This is the assertion the earlier wiring had no room for. `open_signer` reported `Available` @@ -841,7 +928,7 @@ mod tests { for seam in [ production_broadcaster(&chain, false).await, - MirrorBroadcast::ChainUnreachable, + MirrorBroadcast::ChainClientUnavailable, MirrorBroadcast::Wired(std::sync::Arc::new( dig_wallet::sage::spend::MockBroadcaster::default(), )), @@ -875,12 +962,12 @@ mod tests { "a switched-off flag is a switch the operator can flip" ); assert_eq!( - spend_capability(&MirrorBroadcast::ChainUnreachable), - SpendCapability::ChainUnreachable, + spend_capability(&MirrorBroadcast::ChainClientUnavailable), + SpendCapability::ChainClientUnavailable, "and an unreachable chain is NOT that, so it must not be reported as one" ); assert!(!spend_capability(&MirrorBroadcast::Disabled).may_spend()); - assert!(!spend_capability(&MirrorBroadcast::ChainUnreachable).may_spend()); + assert!(!spend_capability(&MirrorBroadcast::ChainClientUnavailable).may_spend()); } /// What a real installation LOOKS like, assembled independently of the needles. @@ -955,7 +1042,8 @@ mod tests { for forbidden in forbidden_installations() { assert!( !source.contains(forbidden), - "{name} calls {forbidden} — the operator signer must stay inside the mirror lifecycle, never installed on the shared WalletBackend" + "{name} calls {forbidden} — the operator signer must stay inside the mirror \ + lifecycle, never installed on the shared WalletBackend" ); } } @@ -998,7 +1086,8 @@ mod tests { for (name, source) in guarded_sources() { assert!( source.contains("WalletBackend"), - "{name} must be the file that could install a signer; an empty or wrong include makes the guard pass forever" + "{name} must be the file that could install a signer; an empty or wrong include \ + makes the guard pass forever" ); } } diff --git a/crates/dig-node-service/src/server.rs b/crates/dig-node-service/src/server.rs index 2fc26293..ffecc72c 100644 --- a/crates/dig-node-service/src/server.rs +++ b/crates/dig-node-service/src/server.rs @@ -2724,11 +2724,11 @@ fn spawn_mirror_passes( ), // Deliberately NOT phrased as a flag to set: the operator has already set // DIG_WALLET_ENABLE_LIVE_BROADCAST to reach this arm at all. - SpendCapability::ChainUnreachable => tracing::warn!( + SpendCapability::ChainClientUnavailable => tracing::warn!( target: "mirror", "the mirror lifecycle OBSERVES only: the wallet opened and live broadcast is on, \ - but no chain could be reached to broadcast through, so a reclaim is planned \ - and reported and no spend is sent" + but this node could not build the shared chain client a broadcaster is made from, \ + so a reclaim is planned and reported and no spend is sent" ), } diff --git a/crates/dig-wallet/src/sage/spend.rs b/crates/dig-wallet/src/sage/spend.rs index b39b88be..799e1b9c 100644 --- a/crates/dig-wallet/src/sage/spend.rs +++ b/crates/dig-wallet/src/sage/spend.rs @@ -253,6 +253,37 @@ pub(crate) fn to_query_bundle(bundle: &SpendBundle) -> Result Result<()> { + if status.status == "SUCCESS" { + return Ok(()); + } + Err(Error::api(format!( + "the network did not admit the transaction to its mempool (ack: {}); nothing is pending on \ + chain for this bundle", + status.status + ))) +} + #[async_trait] impl Broadcaster for ChiaQueryBroadcaster { async fn broadcast(&self, bundle: &SpendBundle) -> Result<()> { @@ -262,14 +293,8 @@ impl Broadcaster for ChiaQueryBroadcaster { .push_tx(&wire) .await .map_err(|e| Error::internal(format!("broadcast (push_tx) failed: {e}")))?; - // Fail closed: a non-success mempool status is an error, not a silent no-op. - if !status.success { - return Err(Error::api(format!( - "the network rejected the transaction: {}", - status.status - ))); - } - Ok(()) + // Fail closed, and on ADMISSION rather than on `success` — see `accepted_by_mempool`. + accepted_by_mempool(&status) } } @@ -970,6 +995,62 @@ mod tests { use chia_sdk_test::Simulator; use chia_wallet_sdk::types::TESTNET11_CONSTANTS; + /// The exact ack `chia_query` produces for each `TransactionAck` status byte. + /// + /// Built from `chia_query::peer::translate::ack_to_tx_status`'s own mapping rather than from + /// what this module would like it to be, so a change to that mapping shows up here as a + /// failure instead of being silently accommodated. + fn ack(status: &str, success: bool) -> chia_query::TxStatus { + chia_query::TxStatus { + status: status.to_string(), + success, + } + } + + /// A PENDING ack is a REFUSAL, and it is the one ack that distinguishes this check from the + /// obvious wrong one. + /// + /// `chia_query` reports status 2 as `TxStatus { status: "PENDING", success: true }` — so a + /// broadcaster that reads `success` returns `Ok(())` for a bundle the full node never admitted + /// to its mempool. Every other ack shape agrees between the two readings; this fixture is the + /// only input that tells them apart, which is why it is written first and named for it. + #[test] + fn a_pending_ack_is_not_an_accepted_broadcast() { + let pending = ack("PENDING", true); + assert!( + pending.success, + "the fixture must carry the conflation it exists to catch: chia_query really does set \ + success=true on PENDING, and a fixture with success=false would pass against the \ + defect" + ); + + let err = accepted_by_mempool(&pending) + .expect_err("a bundle the mempool did not admit must not report as broadcast"); + assert!( + err.to_string().contains("PENDING"), + "the refusal must NAME the ack it saw, or an operator cannot tell a held bundle from a \ + rejected one: {err}" + ); + } + + /// SUCCESS, and only SUCCESS, is admission. + #[test] + fn only_a_success_ack_reports_an_accepted_broadcast() { + accepted_by_mempool(&ack("SUCCESS", true)).expect("status 1 is mempool admission"); + + for refused in [ + ack("PENDING", true), + ack("FAILED", false), + ack("UNKNOWN", false), + ] { + let name = refused.status.clone(); + assert!( + accepted_by_mempool(&refused).is_err(), + "{name} is not mempool admission and must not return Ok" + ); + } + } + /// A signer whose single key owns `alice`'s simulator coin, using the testnet11 agg-sig /// domain (the domain the simulator validates against). fn signer_for(sk: SecretKey) -> WalletSigner { From 7be00916ae5a4ae39a20fbc4c309dbaa7a636f92 Mon Sep 17 00:00:00 2001 From: Michael Taylor Date: Sun, 30 Aug 2026 15:32:26 -0700 Subject: [PATCH 7/7] docs(mirror): name the PENDING harm at the reservation, and correct two stale comments The reservation comment's 'extended only on a broadcast that REACHED the mempool' was false at the previous head and is what the PENDING fix makes true; it now says so and names the durable consequence. chain.rs no longer claims the transport is reachable only as a SignedBundlePusher, which stopped being true when broadcaster() was added. Co-Authored-By: Claude --- crates/dig-node-service/src/mirror/lifecycle.rs | 9 +++++++++ crates/dig-wallet/src/sage/chain.rs | 6 +++++- crates/dig-wallet/src/sage/spend.rs | 12 +++++++++++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/crates/dig-node-service/src/mirror/lifecycle.rs b/crates/dig-node-service/src/mirror/lifecycle.rs index df72ebdf..97be9629 100644 --- a/crates/dig-node-service/src/mirror/lifecycle.rs +++ b/crates/dig-node-service/src/mirror/lifecycle.rs @@ -282,6 +282,15 @@ impl<'a, S: ChainSource> NodeMirrorEffects<'a, S> { // strand a coin every time a broadcast failed, and the failure path below already // records that the money stayed put. // + // That sentence is load-bearing and was FALSE until the broadcaster stopped reading + // `TxStatus::success`, which is true for a PENDING ack — a bundle the full node + // explicitly did not admit. `Ok` here therefore meant "reached the mempool OR was + // held for an unknown parent", and the held case took this branch: it extended the + // reservation, wrote a durable `Submitted` entry below, and every later pass then + // excluded those funding coins forever via `committed_funding_coin_ids`. Nothing + // reconciles that, so the coins were stranded against a spend nobody was holding. + // `dig_wallet::sage::spend::accepted_by_mempool` is what makes the sentence true. + // // Reclaims feed it too, and that is deliberate: `funding_coin_ids` is read from the // bundle, so this set holds exactly what the journal holds, and a set that // disagreed with the record it mirrors would be a second answer to "what is in diff --git a/crates/dig-wallet/src/sage/chain.rs b/crates/dig-wallet/src/sage/chain.rs index a2c47d69..1fac3fc7 100644 --- a/crates/dig-wallet/src/sage/chain.rs +++ b/crates/dig-wallet/src/sage/chain.rs @@ -604,7 +604,11 @@ impl ChainFallback for ChainTransport { // `submit_transaction` for the node's custodied key, which is the decision // `DIG_WALLET_ENABLE_LIVE_BROADCAST` owns. An unused `impl Broadcaster for ChainTransport` sat here // and made a one-line `.with_broadcaster(chain.clone())` compile, pass every test, and silently -// enable node-custodied sending on a default install. The transport is reachable only as a +// enable node-custodied sending on a default install. +// +// A `Broadcaster` is reachable from the transport, but only by NAME, through +// [`ChainTransport::broadcaster`] — a visible line in a diff that a caller has to write on purpose. +// The `impl` is what made it ambient; asking for one is not. Its other route out is as a // `SignedBundlePusher`, whose contract is a bundle somebody already signed. /// Decode a hex-encoded, already-signed spend bundle. diff --git a/crates/dig-wallet/src/sage/spend.rs b/crates/dig-wallet/src/sage/spend.rs index 799e1b9c..bffb2d0e 100644 --- a/crates/dig-wallet/src/sage/spend.rs +++ b/crates/dig-wallet/src/sage/spend.rs @@ -269,10 +269,20 @@ pub(crate) fn to_query_bundle(bundle: &SpendBundle) -> Result Result<()> { if status.status == "SUCCESS" { return Ok(());