diff --git a/Cargo.lock b/Cargo.lock index 7049ef89..97d6adcd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3031,7 +3031,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.175.1" +version = "0.176.0" dependencies = [ "async-trait", "axum", @@ -3334,7 +3334,7 @@ dependencies = [ [[package]] name = "dig-wallet" -version = "0.41.0" +version = "0.42.0" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index 84de6bc7..56887f88 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.175.1" +version = "0.176.0" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over diff --git a/SPEC.md b/SPEC.md index 9922f4ad..81a3d597 100644 --- a/SPEC.md +++ b/SPEC.md @@ -5849,8 +5849,11 @@ one live backend. - **No runtime signer load.** `current_signer` resolves ONLY the bring-up-injected signer (`with_signer`), which no shipped build attaches — it exists for the simulator/test path. A shipped node - therefore has no signer at all, and every method that needs one reports the wallet locked - (dig_ecosystem#1701, §908). + therefore has no signer at all, and every method that needs one refuses (dig_ecosystem#1701, §908). + A refusal MUST name the state the node is actually in and MUST NOT report the wallet locked merely + because no signer resolved: the two are independent here, so an UNLOCKED wallet would be told to + unlock, and node-managed unlock was removed (§18.24) so no unlock would help. The tip path + (§18.23) states the three observable cases separately. - **No custody dispatch.** `wallet.*` and `auth.*` reach no handler; the wallet gate refuses the prefixes outright (§7.12) and neither appears in discovery. The attached `WalletCustody` is a read (§18.20) that contributes PUBLIC addresses to the subscription set and to the push guard. @@ -5899,8 +5902,20 @@ payment does (`Cat::spend_all` CAT-wraps it). (the only money-moving step). A crash at any point leaves ≤1 reserved entry for that key; on restart the engine (re-loaded from the ledger file) treats the key as already tipped and SKIPS — erring toward under-tipping, never a double-spend. A definitively PRE-broadcast failure (`TipSpendOutcome::NotExecutable` - — locked wallet / not-yet-synced / insufficient $DIG) rolls the reservation back (retryable); an + — no signing key / not-yet-synced / insufficient $DIG) rolls the reservation back (retryable); an AMBIGUOUS broadcast error keeps it as `Failed` (never retried that day). +- **A signer-absence refusal names its own state (#410).** When no signing key resolves, the + `NotExecutable` reason MUST be exactly one of the three published `crate::sage::tipping::refusal` + constants, chosen by what the backend can OBSERVE: no custody view attached at all + (`NO_SIGNER_CONFIGURED`), a custody view holding an enrolled wallet whose sealed seed this node + cannot open (`WALLET_ENROLLED_BUT_UNOPENABLE`), or a custody view holding no wallet + (`NO_WALLET_ENROLLED`). The three MUST be distinct strings and none MUST assert that the wallet is + locked, because the signer is absent on the shipped node whether or not any wallet is locked, and + §18.24 removed the unlock such a message would send the reader after. `Orphaned` + (`crate::autoseed::BootstrapState::Orphaned`) is deliberately NOT among them: it is decided at + bootstrap from paths the backend does not hold, so reporting it here would be a guess of the same + kind this clause forbids. The refusal is PRE-broadcast in every case — no bundle is built, signed + or sent. - **Fail-closed on unreadable persisted state.** Load distinguishes an ABSENT file (a genuine first run: config → DEFAULT-ON, ledger → empty) from a file that is PRESENT but unreadable/unparseable (locked / corrupt / truncated / forward-incompatible). A present-but-unreadable **ledger** POISONS the diff --git a/crates/dig-wallet/Cargo.toml b/crates/dig-wallet/Cargo.toml index c6df5435..3a2f8004 100644 --- a/crates/dig-wallet/Cargo.toml +++ b/crates/dig-wallet/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "dig-wallet" -version = "0.41.0" +version = "0.42.0" edition = "2021" license = "GPL-2.0-only" description = "DIG Browser built-in Chia wallet sidecar: a local axum server (using digstore-chain + chia-wallet-sdk over coinset.org) that serves a Sage-mirroring wallet UI. Native Rust so BLS signing works; the browser opens it at 127.0.0.1." diff --git a/crates/dig-wallet/src/sage/rpc.rs b/crates/dig-wallet/src/sage/rpc.rs index d069f0d9..79d4a11c 100644 --- a/crates/dig-wallet/src/sage/rpc.rs +++ b/crates/dig-wallet/src/sage/rpc.rs @@ -2902,6 +2902,26 @@ impl WalletBackend { None } + /// Why [`Self::current_signer`] returned `None`, as one of the three published + /// [`super::tipping::refusal`] strings (#410). + /// + /// The states are distinguished by what this backend can actually SEE — whether a custody view + /// is attached at all, and whether that view holds any enrolled wallet — never by guessing. The + /// single reason these replace, `"wallet is locked"`, was false in the state a shipped node is + /// permanently in: `with_signer` has no non-test caller, so the signer is absent on an unlocked + /// wallet just as surely as on a locked one, and telling the user to unlock sent them after a + /// remedy that does not exist. + /// + /// Only meaningful when the signer is genuinely absent; callers reach it from the `else` arm. + fn signer_absence_reason(&self) -> &'static str { + use super::tipping::refusal; + match self.custody.as_ref() { + None => refusal::NO_SIGNER_CONFIGURED, + Some(custody) if custody.any_wallet() => refusal::WALLET_ENROLLED_BUT_UNOPENABLE, + Some(_) => refusal::NO_WALLET_ENROLLED, + } + } + /// Record `signer`'s public keys so the push guard still recognises the node's own coins after /// the signer is gone (see [`Self::custodied_public_keys`]). /// @@ -3064,7 +3084,7 @@ impl WalletBackend { /// tips never enables live broadcast for the whole wallet surface. /// /// Fail-closed contract (see [`super::tipping::TipSpender`]): definitively PRE-broadcast - /// conditions (locked wallet / no lineage / insufficient $DIG / build or validation failure) + /// conditions (no signing key / no lineage / insufficient $DIG / build or validation failure) /// return [`TipSpendOutcome::NotExecutable`] (retryable — no money moved); the ONLY money-moving /// step is `broadcaster.broadcast`, whose error propagates as `Err` (ambiguous — the engine keeps /// the reservation and does not retry that day). @@ -3077,10 +3097,11 @@ impl WalletBackend { confirmer: Option<&dyn super::spend::Confirmer>, ) -> Result { use super::tipping::TipSpendOutcome; - // Signer (node custody). A locked wallet is retryable, not a spend failure. + // Signer (node custody). Absence is retryable, not a spend failure — but WHY it is absent + // is three different situations, and saying the wrong one sends the user somewhere useless. let Some(signer) = self.current_signer() else { return Ok(TipSpendOutcome::NotExecutable { - reason: "wallet is locked".into(), + reason: self.signer_absence_reason().into(), }); }; // CAT-send needs a lineage source to resolve input coins; absent ⇒ not-yet-synced. @@ -5002,6 +5023,149 @@ mod tests { ); } + // ---- #410: the tip refusal names the state it is actually in --------------------------- + + /// A scratch config dir unique to this process AND thread, so parallel tests never share a + /// custody manifest. + fn refusal_scratch_dir(tag: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!( + "dig-wallet-tip-refusal-{tag}-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&dir); + dir + } + + /// Ask `be` for a tip and return the `NotExecutable` reason, asserting on the way that the + /// broadcaster was never reached — the refusal must be definitively PRE-broadcast. + async fn tip_refusal_reason(be: &WalletBackend) -> String { + let bc = crate::sage::spend::MockBroadcaster::default(); + let outcome = be + .build_and_broadcast_dig_tip(Bytes32::from([9u8; 32]), 1_000, 0, &bc, None) + .await + .expect("a signer-absence refusal is an Ok(NotExecutable), never an Err"); + assert!( + bc.sent.lock().unwrap().is_empty(), + "the refusal must happen before anything is broadcast" + ); + match outcome { + super::super::tipping::TipSpendOutcome::NotExecutable { reason } => reason, + other => panic!("expected a refusal, got {other:?}"), + } + } + + /// The three signer-absence reasons must be DISTINCT strings, or the split is decorative: + /// collapsing them back to one shared sentence would leave every equality assertion below + /// still passing. + #[test] + fn the_three_signer_absence_reasons_are_pairwise_distinct() { + use super::super::tipping::refusal; + let all = [ + refusal::NO_SIGNER_CONFIGURED, + refusal::WALLET_ENROLLED_BUT_UNOPENABLE, + refusal::NO_WALLET_ENROLLED, + ]; + for (i, a) in all.iter().enumerate() { + for b in all.iter().skip(i + 1) { + assert_ne!(a, b, "each signer-absence state needs its own sentence"); + } + } + } + + /// The defect this ticket exists for: a backend with no custody view refused with + /// `"wallet is locked"`, which is false — nothing is locked, nothing was ever configured. + /// + /// The negative assertion is on the SUBSTRING `"lock"`, not on the whole sentence, because the + /// harm was never the exact wording: any sentence containing `lock`/`unlock` sends the reader + /// after a remedy that does not exist on this node (SPEC §18.24 removed node-managed unlock). + #[tokio::test] + async fn with_no_custody_the_tip_refusal_says_unconfigured_and_never_mentions_a_lock() { + let be = backend_with(vec![], true).await; + assert!( + be.current_signer().is_none(), + "the fixture must genuinely have no signer, or this asserts nothing" + ); + + let reason = tip_refusal_reason(&be).await; + assert_eq!(reason, super::super::tipping::refusal::NO_SIGNER_CONFIGURED); + assert!( + !reason.to_ascii_lowercase().contains("lock"), + "an unlocked wallet must never be told it is locked; got {reason:?}" + ); + } + + /// A wallet IS enrolled and its seed cannot be opened. This is the one state the old sentence + /// was nearly right about — and it still must not claim a lock the user can open, so it names + /// the sealed seed instead. + #[tokio::test] + async fn an_enrolled_wallet_refuses_the_tip_as_an_unopenable_seed() { + let dir = refusal_scratch_dir("enrolled"); + WalletCustody::enroll_for_tests(&dir, "tip-refusal-fixture", &[BlsPair::new(410).pk]); + let custody = WalletCustody::open(dir.clone()); + assert!( + custody.any_wallet(), + "the fixture must really enrol a wallet, or it is the empty-custody case in disguise" + ); + + let be = backend_with(vec![], true).await.with_custody(custody); + let reason = tip_refusal_reason(&be).await; + assert_eq!( + reason, + super::super::tipping::refusal::WALLET_ENROLLED_BUT_UNOPENABLE + ); + + let _ = std::fs::remove_dir_all(&dir); + } + + /// Custody attached, nothing enrolled — a different situation from both of the above, and the + /// only one of the three a user fixes by creating a wallet. + #[tokio::test] + async fn custody_holding_no_wallet_refuses_the_tip_as_nothing_enrolled() { + let dir = refusal_scratch_dir("empty"); + std::fs::create_dir_all(&dir).expect("create the scratch dir"); + let custody = WalletCustody::open(dir.clone()); + assert!( + !custody.any_wallet(), + "the fixture must really be empty, or it is the enrolled case in disguise" + ); + + let be = backend_with(vec![], true).await.with_custody(custody); + assert_eq!( + tip_refusal_reason(&be).await, + super::super::tipping::refusal::NO_WALLET_ENROLLED + ); + + let _ = std::fs::remove_dir_all(&dir); + } + + /// The control that keeps the three assertions above from being true of everything: a backend + /// that CAN sign gets past the signer guard entirely, and refuses for a different reason. + /// Without this, a `signer_absence_reason` wired in unconditionally would pass every test here. + #[tokio::test] + async fn a_backend_that_can_sign_never_refuses_for_signer_absence() { + use super::super::tipping::refusal; + let be = backend_with(vec![], true).await.with_signer(Arc::new( + crate::sage::spend::WalletSigner::new(vec![], Bytes32::from([7u8; 32])), + )); + assert!( + be.current_signer().is_some(), + "the control needs a backend that really can sign" + ); + + let reason = tip_refusal_reason(&be).await; + for absent in [ + refusal::NO_SIGNER_CONFIGURED, + refusal::WALLET_ENROLLED_BUT_UNOPENABLE, + refusal::NO_WALLET_ENROLLED, + ] { + assert_ne!( + reason, absent, + "a signing backend reached the signer-absence branch" + ); + } + } + async fn backend_with(coins: Vec, synced: bool) -> WalletBackend { let db = WalletDb::open_in_memory().await.unwrap(); db.upsert_coins(&coins).await.unwrap(); diff --git a/crates/dig-wallet/src/sage/tipping.rs b/crates/dig-wallet/src/sage/tipping.rs index e51d765a..afd110f7 100644 --- a/crates/dig-wallet/src/sage/tipping.rs +++ b/crates/dig-wallet/src/sage/tipping.rs @@ -282,14 +282,49 @@ pub enum TipSpendOutcome { /// asynchronous, and the reservation blocks a same-day retry either way). confirmed: bool, }, - /// The wallet cannot currently build/broadcast the tip (locked / not-yet-synced / no lineage / - /// insufficient $DIG). Definitively PRE-broadcast — no money moved; the caller may retry later. + /// The wallet cannot currently build/broadcast the tip (no signing key / not-yet-synced / no + /// lineage / insufficient $DIG). Definitively PRE-broadcast — no money moved; the caller may + /// retry later. NotExecutable { - /// A human-readable reason. + /// A human-readable reason. When the refusal is signer-absence, it is exactly one of + /// [`NO_SIGNER_CONFIGURED`], [`WALLET_ENROLLED_BUT_UNOPENABLE`] or [`NO_WALLET_ENROLLED`]. reason: String, }, } +/// Why a tip refusal happened when no signing key could be resolved (#410). +/// +/// These are the exact `TipSpendOutcome::NotExecutable::reason` strings for the three signer-absence +/// states a [`super::rpc::WalletBackend`] can actually OBSERVE, published as constants so a caller +/// (and a test) can match a refusal by equality rather than by reading prose. +/// +/// They exist because the single reason they replace — `"wallet is locked"` — was false in the state +/// a shipped node is always in. Nothing attaches a signer to the served backend (`with_signer` has no +/// non-test caller), so a user with a perfectly unlocked wallet was told to unlock it, would try, and +/// would get nowhere. Each string below therefore describes a state the user can check, and none of +/// them asks for an unlock that would not help. +/// +/// A fourth state, `Orphaned` (a sealed seed whose device key is gone, +/// [`crate::autoseed::BootstrapState::Orphaned`]), is deliberately NOT represented: it is decided at +/// bootstrap from paths the backend does not hold, and [`super::custody::CustodyState`] has no +/// variant for it. Minting a reason the backend cannot distinguish would reintroduce exactly the +/// defect this fixes. +pub mod refusal { + /// No signing key and no custody view at all — this backend was built without either, so it + /// could never spend. There is no wallet state for the user to change. + pub const NO_SIGNER_CONFIGURED: &str = + "no signing key is configured on this node, so it cannot sign a tip"; + + /// A wallet IS enrolled on this device, and the node cannot open its sealed seed. Node-managed + /// unlock was removed (SPEC §18.24), so this is not a lock the user can open from here. + pub const WALLET_ENROLLED_BUT_UNOPENABLE: &str = + "a wallet is enrolled on this device but this node cannot open its sealed seed, so it cannot sign a tip"; + + /// Custody is attached and holds no wallet — nothing is enrolled to sign with. + pub const NO_WALLET_ENROLLED: &str = + "no wallet is enrolled on this device, so it cannot sign a tip"; +} + // ───────────────────────────────────────────────────────────────────────────── // Seams (traits) — injected so the money-safety logic is testable without a chain // ─────────────────────────────────────────────────────────────────────────────