chore(wallet): checkpoint round-2 work for #1702

Co-Authored-By: Claude <noreply@anthropic.com>

Author: MichaelTaylor3d

src/wallet/approval.rs

@@ -276,18 +276,18 @@ mod tests {
         let pending = PendingApproval::new(vec![coin_spend(1234)], summary(), verified());
         assert_eq!(pending.summary().recipients[0].amount_mojos, 42);
 
-        let approval = pending.confirmed(SpendDecision::Approve).unwrap();
+        let approval = pending.decided(SpendDecision::Approve).unwrap();
         assert_eq!(approval.coin_spends(), &[coin_spend(1234)]);
         assert_eq!(approval.summary(), &summary());
     }
 
-    /// A decline is `PolicyDenied` — terminal — not an escalatable refusal a caller could re-prompt.
+    /// A decline is `UserDeclined` — terminal — not an escalatable refusal a caller could re-prompt.
     #[test]
     fn a_declined_ceremony_denies_outright_and_never_yields_an_approval() {
         let pending = PendingApproval::new(vec![coin_spend(1)], summary(), verified());
-        let err = denial(pending.confirmed(SpendDecision::Decline(Some("not mine".into()))));
+        let err = denial(pending.decided(SpendDecision::Decline(Some("not mine".into()))));
         assert!(
-            matches!(&err, AccountError::PolicyDenied(m) if m.contains("declined") && m.contains("not mine")),
+            matches!(&err, AccountError::UserDeclined(m) if m.contains("declined") && m.contains("not mine")),
             "{err:?}"
         );
     }
@@ -296,7 +296,7 @@ mod tests {
     #[test]
     fn a_reasonless_decline_is_still_denied() {
         let pending = PendingApproval::new(vec![coin_spend(1)], summary(), verified());
-        let err = denial(pending.confirmed(SpendDecision::Decline(None)));
-        assert!(matches!(err, AccountError::PolicyDenied(_)), "{err:?}");
+        let err = denial(pending.decided(SpendDecision::Decline(None)));
+        assert!(matches!(err, AccountError::UserDeclined(_)), "{err:?}");
     }
 }

src/wallet/authorizer.rs

@@ -142,18 +142,26 @@ mod tests {
         )
     }
 
+    /// A handle for a session that is still unlocked.
+    ///
+    /// Every `WalletOps` here is built through this, so the residency argument is never accidentally a
+    /// revoked one — the revoked case is exercised deliberately, by name, below.
+    fn live() -> Arc<Residency> {
+        Arc::new(Residency::new())
+    }
+
     #[test]
     fn wallet_key_matches_the_canonical_derivation_at_the_profile_index() {
         let s = seed();
-        let ops = WalletOps::new(s.clone(), ProfileIx(5));
+        let ops = WalletOps::new(s.clone(), ProfileIx(5), live());
         let expected = WalletKey::from_seed_at(&s.master_seed()[..], ProfileIx(5));
         assert_eq!(ops.wallet_key().secret_key(), expected.secret_key());
     }
 
     #[test]
     fn public_passthroughs_match_the_wallet_key_without_exposing_it() {
         let s = seed();
-        let ops = WalletOps::new(s, ProfileIx(2));
+        let ops = WalletOps::new(s, ProfileIx(2), live());
         // The public read-only surface exposes exactly the wallet key's public identifiers.
         assert_eq!(ops.public_key(), ops.wallet_key().public_key());
         assert_eq!(ops.puzzle_hash(), ops.wallet_key().puzzle_hash());
@@ -163,8 +171,8 @@ mod tests {
     #[test]
     fn distinct_profiles_derive_distinct_wallet_keys() {
         let s = seed();
-        let k0 = WalletOps::new(s.clone(), ProfileIx::ROOT).wallet_key();
-        let k1 = WalletOps::new(s, ProfileIx(1)).wallet_key();
+        let k0 = WalletOps::new(s.clone(), ProfileIx::ROOT, live()).wallet_key();
+        let k1 = WalletOps::new(s, ProfileIx(1), live()).wallet_key();
         assert_ne!(k0.secret_key(), k1.secret_key());
     }
 
@@ -186,7 +194,7 @@ mod tests {
         use chia_wallet_sdk::driver::{SpendContext, StandardLayer};
         use chia_wallet_sdk::types::Conditions;
 
-        let ops = WalletOps::new(seed(), ProfileIx::ROOT);
+        let ops = WalletOps::new(seed(), ProfileIx::ROOT, live());
         let wallet_ph = ops.puzzle_hash();
 
         let mut ctx = SpendContext::new();
@@ -226,7 +234,6 @@ mod tests {
 
         let bundle = ops
             .money_signer(Network::Mainnet)
-            .unwrap()
             .sign_approved(approval)
             .expect("an approved, wallet-owned send must sign");
         assert_eq!(

src/wallet/autosend.rs

@@ -194,7 +194,7 @@ mod tests {
         assert_eq!(policy.period_cap_mojos, 0);
         assert_eq!(policy.period_seconds, DEFAULT_PERIOD_SECONDS);
         for op_class in SpendOpClass::CONFIGURABLE {
-            let limits = policy.limits_for(op_class).unwrap();
+            let limits = policy.configured_limits(op_class).unwrap();
             assert!(!limits.enabled, "{op_class:?} must default to disabled");
             assert_eq!(
                 limits.per_tx_limit_mojos, 0,
@@ -324,6 +324,7 @@ mod tests {
             small_send: OpClassLimits::default(),
             period_seconds: 3_600,
             period_cap_mojos: 1_000,
+            max_confirmations_per_period: 8,
         };
         let json = serde_json::to_string(&policy).unwrap();
         assert_eq!(

src/wallet/enforcer.rs

@@ -601,6 +601,9 @@ mod tests {
             small_send: OpClassLimits::enabled_up_to(u64::MAX),
             period_seconds: DEFAULT_PERIOD_SECONDS,
             period_cap_mojos: u64::MAX,
+            // Permissive here too, so a test that escalates repeatedly fails on the rule it is
+            // about rather than on the prompt ceiling. The ceiling has its own tests.
+            max_confirmations_per_period: u32::MAX,
         }
     }
 
@@ -667,6 +670,43 @@ mod tests {
     }
 
     /// The same for a `Result<SpendApproval>`, which `PendingApproval::confirmed` returns.
+    /// Run the ceremony to `decision` through the real consent seam.
+    ///
+    /// A fixed-answer [`AuthProvider`] stands in for the harness, so these tests exercise
+    /// [`PendingApproval::confirm_with`] — the only public route from a pending approval to a
+    /// signature — rather than the crate-private tail it delegates to. A host cannot skip this seam,
+    /// so neither does the test.
+    async fn confirmed(
+        pending: PendingApproval,
+        decision: crate::auth::provider::SpendDecision,
+    ) -> Result<SpendApproval> {
+        use crate::auth::factors::AuthFactors;
+        use crate::auth::provider::{AuthProvider, SpendConfirmRequest, UnlockRequest};
+        use crate::id::AccountId;
+
+        use crate::auth::provider::SpendDecision;
+
+        struct Fixed(SpendDecision);
+
+        #[async_trait::async_trait]
+        impl AuthProvider for Fixed {
+            async fn collect_factors(&self, _: UnlockRequest) -> Result<AuthFactors> {
+                unreachable!("a spend ceremony never collects unlock factors")
+            }
+            async fn confirm_spend(&self, _: SpendConfirmRequest) -> Result<SpendDecision> {
+                Ok(self.0.clone())
+            }
+        }
+
+        pending
+            .confirm_with(
+                &Fixed(decision),
+                AccountId::new("ceremony-fixture"),
+                ProfileIx::ROOT,
+            )
+            .await
+    }
+
     fn denial(result: Result<SpendApproval>) -> AccountError {
         match result {
             Ok(_) => panic!("expected a denial, got a signable approval"),
@@ -1150,8 +1190,8 @@ mod tests {
 
     /// A `Confirm`-tier spend reaches the confirm path and, once confirmed, becomes signable — the
     /// whole ceremony, end to end, on a spend the gate will never auto-approve.
-    #[test]
-    fn a_confirm_tier_spend_reaches_the_ceremony_and_a_confirmation_makes_it_signable() {
+    #[tokio::test]
+    async fn a_confirm_tier_spend_reaches_the_ceremony_and_a_confirmation_makes_it_signable() {
         let gate = gate_with(hot_custody(), permissive_auto_send());
         let coin_spends = pays_third_party(CUSTODY_AUTO_SEND_CEILING + 1);
 
@@ -1163,8 +1203,8 @@ mod tests {
             "the user must be shown the spend's real value"
         );
 
-        let approved = escalated
-            .confirmed(crate::auth::provider::SpendDecision::Approve)
+        let approved = confirmed(escalated, crate::auth::provider::SpendDecision::Approve)
+            .await
             .expect("a confirmed spend becomes signable");
         assert_eq!(
             approved.coin_spends(),
@@ -1180,16 +1220,22 @@ mod tests {
 
     /// A declined ceremony yields no approval and charges nothing — a refusal must never cost the
     /// user their allowance.
-    #[test]
-    fn a_declined_ceremony_yields_no_approval_and_charges_nothing() {
+    #[tokio::test]
+    async fn a_declined_ceremony_yields_no_approval_and_charges_nothing() {
         let gate = gate_with(hot_custody(), permissive_auto_send());
         let escalated = pending(gate.authorize_op(
             &pays_third_party(CUSTODY_AUTO_SEND_CEILING + 1),
             SpendOpClass::Tip,
         ));
 
-        let err = denial(escalated.confirmed(crate::auth::provider::SpendDecision::Decline(None)));
-        assert!(matches!(err, AccountError::PolicyDenied(_)), "{err}");
+        let err = denial(
+            confirmed(
+                escalated,
+                crate::auth::provider::SpendDecision::Decline(None),
+            )
+            .await,
+        );
+        assert!(matches!(err, AccountError::UserDeclined(_)), "{err}");
         assert_eq!(ledger_total(&gate), 0);
     }
 
@@ -1446,24 +1492,89 @@ mod tests {
         approval(gate.authorize_op(&pays_third_party(1_000), SpendOpClass::Tip));
     }
 
-    /// The layered invariant this gate deliberately does NOT hold alone: an un-hinted output is
-    /// change, excluded from the summary's recipients, so no amount limit here can see it. The
-    /// composition is what protects the wallet — the policy gate approves this spend (its whole
-    /// visible effect is a 1 mojo fee) and the money signer still refuses to sign, because the change
-    /// output pays a puzzle hash the wallet does not own.
+    /// **The value the gate charges is the value that LEAVES — the author cannot shrink it by
+    /// omitting a memo.**
+    ///
+    /// This is the #1702 exploit, and the fixture is built so that the *only* thing that can refuse it
+    /// is the rule under test. The destination is `ProfileIx(1)` of the spender's OWN seed: an address
+    /// `owns_puzzle_hash` accepts, inside the signer's `0..address_gap` window. So the downstream
+    /// "change must be wallet-owned" check — the thing that refused the previous version of this test,
+    /// where the destination was a stranger — is silent here by construction. Aim the same 999 mojos at
+    /// a stranger and it is impossible to tell which layer refused.
+    ///
+    /// Before the fix, `analyze` filed the un-hinted output as CHANGE, `recipients` was empty, the
+    /// summary read "no recipients, fee 1", the ledger was charged **1**, and the signature authorized
+    /// **999**. The human would have been shown a 1 mojo fee and asked to approve a spend of the coin.
     #[test]
-    fn refuses_to_sign_unhinted_value_leaving_the_wallet_even_when_the_policy_approves() {
-        use crate::wallet::money_signer::{LocalMoneySigner, MoneySigner};
-        use dig_wallet_backend::types::Network;
+    fn an_unhinted_output_to_an_owned_derivation_is_counted_not_hidden() {
+        let attacker_owned = WalletKey::from_seed_at(&SPENDER_SEED, ProfileIx(1)).puzzle_hash();
+        assert_ne!(
+            attacker_owned,
+            spender().puzzle_hash(),
+            "the fixture must pay a DIFFERENT derivation, or it is testing genuine change"
+        );
+
+        let mut ctx = SpendContext::new();
+        StandardLayer::new(spender().public_key())
+            .spend(
+                &mut ctx,
+                Coin::new(Bytes32::new([1u8; 32]), spender().puzzle_hash(), 1_000),
+                // No memo: `analyze` files this as change, so a hinted-recipient sum cannot see it.
+                Conditions::new()
+                    .create_coin(attacker_owned, 999, Memos::None)
+                    .reserve_fee(1),
+            )
+            .unwrap();
+        let coin_spends = ctx.take();
+
+        let gate = gate_with(
+            hot_custody(),
+            AutoSendPolicy {
+                enabled: true,
+                rebalance: OpClassLimits::enabled_up_to(10),
+                period_cap_mojos: 10,
+                ..AutoSendPolicy::default()
+            },
+        );
+
+        // Escalated, not approved: 1_000 is far past the 10 mojo per-transaction bound.
+        let escalated = pending(gate.authorize_op(&coin_spends, SpendOpClass::Rebalance));
+        let summary = escalated.summary();
+        assert_eq!(
+            summary.recipients.len(),
+            1,
+            "the un-hinted output must appear in the line the human confirms: {summary}"
+        );
+        assert_eq!(summary.recipients[0].amount_mojos, 999);
+        assert_eq!(
+            summary.native_total_mojos(),
+            1_000,
+            "the whole coin leaves, so the whole coin is what is weighed"
+        );
+        assert_eq!(
+            ledger_total(&gate),
+            0,
+            "nothing was auto-approved, so nothing was charged"
+        );
+    }
 
+    /// The truthful control for the test above: **genuine change is still free.**
+    ///
+    /// Identical spend, one field changed — the output pays the exact puzzle hash of the coin being
+    /// spent. Value demonstrably has not moved, so it is excluded, the total is the fee alone, and the
+    /// spend auto-approves under the same 10 mojo bound that escalated the exploit.
+    ///
+    /// Without this control the test above would also pass on an implementation that counted every
+    /// output unconditionally — which would make every real send unspendable while looking strict.
+    #[test]
+    fn change_returning_to_the_spent_coins_own_puzzle_hash_is_not_counted() {
         let mut ctx = SpendContext::new();
         StandardLayer::new(spender().public_key())
             .spend(
                 &mut ctx,
                 Coin::new(Bytes32::new([1u8; 32]), spender().puzzle_hash(), 1_000),
-                // Un-hinted: `analyze` files this as CHANGE, so it never reaches `recipients`.
                 Conditions::new()
-                    .create_coin(third_party().puzzle_hash(), 999, Memos::None)
+                    .create_coin(spender().puzzle_hash(), 999, Memos::None)
                     .reserve_fee(1),
             )
             .unwrap();
@@ -1481,20 +1592,58 @@ mod tests {
         let approved = approval(gate.authorize_op(&coin_spends, SpendOpClass::Rebalance));
         assert!(
             approved.summary().recipients.is_empty(),
-            "an un-hinted output is invisible to the summary"
+            "returning change to the coin's own puzzle hash moves no value"
         );
-        assert_eq!(approved.summary().fee, 1, "only the fee is visible");
+        assert_eq!(approved.summary().fee, 1);
+        assert_eq!(ledger_total(&gate), 1, "only the fee is charged");
+    }
 
-        let signer = LocalMoneySigner::new_canonical(
-            SPENDER_SEED.to_vec(),
-            ProfileIx::ROOT.0,
-            Network::Mainnet,
-        )
-        .unwrap();
-        let err = signer.sign_approved(approved).unwrap_err();
-        assert!(
-            matches!(err, AccountError::Spend(_)),
-            "the signer must refuse un-hinted value leaving the wallet: {err}"
+    /// **Change paid to a FRESH derivation of the same wallet is counted. That is intended.**
+    ///
+    /// This is the deliberate cost of the rule above, recorded as behaviour rather than discovered as a
+    /// bug (`SPEC.md` §6.1.1). This layer holds no key: it cannot tell a fresh derivation of the user's
+    /// own wallet from a stranger's address, and the only way it could would be to accept "any owned
+    /// derivation" as change — which is exactly the exfiltration target the exploit above uses.
+    ///
+    /// So the rule over-counts, and a legitimate send whose change goes to a fresh address escalates to
+    /// the human instead of auto-sending. Over-counting asks a person; under-counting signs. Only one
+    /// of those is a custody failure.
+    #[test]
+    fn change_to_a_fresh_derivation_is_deliberately_overcounted_and_escalates() {
+        let fresh_change = WalletKey::from_seed_at(&SPENDER_SEED, ProfileIx(9)).puzzle_hash();
+
+        let mut ctx = SpendContext::new();
+        let recipient = third_party().puzzle_hash();
+        let hint = ctx.hint(recipient).unwrap();
+        StandardLayer::new(spender().public_key())
+            .spend(
+                &mut ctx,
+                Coin::new(Bytes32::new([1u8; 32]), spender().puzzle_hash(), 1_000),
+                Conditions::new()
+                    .create_coin(recipient, 5, hint)
+                    .create_coin(fresh_change, 994, Memos::None)
+                    .reserve_fee(1),
+            )
+            .unwrap();
+        let coin_spends = ctx.take();
+
+        let gate = gate_with(
+            hot_custody(),
+            AutoSendPolicy {
+                enabled: true,
+                rebalance: OpClassLimits::enabled_up_to(10),
+                period_cap_mojos: 1_000,
+                ..AutoSendPolicy::default()
+            },
+        );
+
+        // The genuine payment is 5 mojos, well inside the 10 mojo bound. It escalates anyway, because
+        // the 994 of change to an address this layer cannot vouch for is counted as leaving.
+        let escalated = pending(gate.authorize_op(&coin_spends, SpendOpClass::Rebalance));
+        assert_eq!(
+            escalated.summary().native_total_mojos(),
+            1_000,
+            "the fresh-derivation change is counted, by design"
         );
     }