SP: improve sp spend without sp outputs and change detection

Convert validate_bip376_spend to validate_silent_payment_inputs
Use accounts to validate sp spend material and mark output.is_change
Introduce reusable sp_derive_path function
Fix change detection issue raised by previous test hygiene commit
- Signing was restored for test_sp_spend_labeled_change_detected
Fix psbt exception in consider_outputs with sp change
- Add apply_label_to_spend_key primitive
- Match spend public key to labeled spend public key for change

New tests:
sp spend to non sp output
sp spend rejection tests - unknown account, incorrect sp spend derivation path
sp change output with foreign keys is allowed but displays warning
Changed test:
sp spend to label change - fix label spend key derivation, use sp input

Author: macgyver13

shared/auth.py

@@ -432,9 +432,12 @@ async def interact(self):
             args = self.psbt.consider_inputs(cosign_xfp=ccc_c_xfp)
 
             # Silent Payments: Validate and pre-process Silent Payments outputs to make preview useful
-            if self.psbt.has_silent_payment_outputs():
-                with stash.SensitiveValues() as sv:
-                    if not self.psbt.process_silent_payments(sv):
+            with stash.SensitiveValues() as sv:
+                if self.psbt.has_silent_payment_inputs():
+                    self.psbt.validate_silent_payment_inputs(sv)
+
+                if self.psbt.has_silent_payment_outputs():
+                    if not self.psbt.process_silent_payment_outputs(sv):
                         # Coverage incomplete: shares computed but waiting on other signers
                         # Skip normal approval flow — prompt user to contribute shares, then save
                         del args

shared/export.py

@@ -13,7 +13,7 @@
 from charcodes import KEY_NFC, KEY_CANCEL, KEY_QR
 from ownership import OWNERSHIP
 from exceptions import QRTooBigError
-from silentpayments import encode_silent_payment_address
+from silentpayments import encode_silent_payment_address, sp_derive_path
 
 async def export_by_qr(body, label, type_code, force_bbqr=False):
     # render as QR and show on-screen
@@ -446,7 +446,7 @@ def generate_generic_export(account_num=0):
                 rv[name]['first'] = chain.address(node, fmt)
 
         # Silent Payments: scan-priv + spend-pub packed as a single bech32m "spscan" string
-        sp_deriv = "m/352h/%dh/%dh" % (chain.b44_cointype, account_num)
+        sp_deriv   = sp_derive_path(chain.b44_cointype, account_num)
         sp_node    = sv.derive_path(sp_deriv)
         scan_node  = sv.derive_path(sp_deriv + "/1h/0")
         spend_node = sv.derive_path(sp_deriv + "/0h/0")

shared/psbt.py

@@ -48,7 +48,7 @@
     AF_P2WSH, AF_P2WSH_P2SH, AF_P2SH, AF_P2TR, AF_P2WPKH, AF_CLASSIC, AF_P2WPKH_P2SH,
     AFC_SEGWIT, AF_BARE_PK
 )
-from silentpayments import SilentPaymentsMixin, compute_silent_payment_spending_privkey, validate_bip376_spend
+from silentpayments import SilentPaymentsMixin, compute_silent_payment_spending_privkey
 
 psbt_tmp256 = bytearray(256)
 
@@ -1030,7 +1030,7 @@ def determine_my_signing_key(self, my_idx, addr_or_pubkey, my_xfp, psbt, parsed_
 
         elif self.af == AF_P2TR:
             if self.is_sp_spend:
-                validate_bip376_spend(self, addr_or_pubkey, my_xfp, psbt)
+                pass  # Silent Payments inputs are handled in validate_silent_payment_inputs
             elif len(parsed_subpaths) == 1:
                 # keyspend without a script path
                 assert self.taproot_merkle_root is None, "merkle_root should not be defined for simple keyspend"
@@ -2006,7 +2006,8 @@ def consider_outputs(self, len_pths, hard_p, prefix_pths, idx_max, cosign_xfp=No
                 self.num_change_outputs += 1
                 total_change += txo.nValue
 
-                if validate_inp_pths:
+                # Silent Payments: SP change is validated by _detect_sp_change_outputs, skip additional checks here
+                if validate_inp_pths and not output.sp_v0_info:
                     # Enforce some policy on change outputs:
                     # - need to "look like" they are going to same wallet as inputs came from
                     # - range limit last two path components (numerically)
@@ -2797,7 +2798,11 @@ def sign_it(self, alternate_secret=None, my_xfp=None):
             # Double-check the change outputs are right. This is slow, but critical because
             # it detects bad actors, not bugs or mistakes.
             # - equivalent check already done for p2sh outputs when we re-built the redeem script
-            change_outs = [n for n,o in enumerate(self.outputs) if o.is_change]
+
+            # SP change is verified at preview time by _detect_sp_change_outputs;
+            # the subpath/check_pubkey_at_path machinery below doesn't apply to it.
+            change_outs = [n for n, o in enumerate(self.outputs)
+                           if o.is_change and not o.sp_v0_info]
             if change_outs:
                 dis.fullscreen('Change Check...')
 
@@ -2840,8 +2845,10 @@ def sign_it(self, alternate_secret=None, my_xfp=None):
                               "BIP-32 path doesn't match actual address.")
 
             # Silent Payment Processing
+            if self.has_silent_payment_inputs():
+                self.validate_silent_payment_inputs(sv)
             if self.has_silent_payment_outputs():
-                if not self.process_silent_payments(sv):
+                if not self.process_silent_payment_outputs(sv):
                     # Silent Payments: must not sign if output scripts not set for all signers
                     # Defensive re-check - ApproveTransaction::interact should handle this case before reaching signing
                     raise FatalPSBTIssue("Silent Payments: Signing cannot proceed until all signers contribute their shares")

shared/silentpayments.py

@@ -7,9 +7,11 @@
 import chains
 import ngu
 import stash
+from glob import settings
 from dleq import generate_dleq_proof, verify_dleq_proof
-from exceptions import FatalPSBTIssue
+from exceptions import FatalPSBTIssue, FraudulentChangeOutput
 from precomp_tag_hash import (
+    BIP352_LABEL_TAG_H,
     BIP352_SHARED_SECRET_TAG_H,
     BIP352_INPUTS_TAG_H,
     TAP_TWEAK_H,
@@ -46,41 +48,34 @@ def encode_silent_payment_address(scan_key, spend_key, version=0):
     return ngu.codecs.bip352_encode(hrp, scan_key, spend_key, version)
 
 
-def validate_bip376_spend(input, output_xonly, my_xfp=None, parent=None):
-    """Validate silent payment spend using PSBT input data
-
-    TODO: replace with test vectors once available
-    
-    Raises FatalPSBTIssue if any SP spend validation checks fail
-    """
-    B_spend_coords, val_coords = input.sp_spend_bip32_derivation
-    xfp_path = input.parse_xfp_path(val_coords)
-    if my_xfp is not None:
-        xfp_path = input.handle_zero_xfp(xfp_path, my_xfp, parent)
-
-    sp_path = xfp_path[1:]
-    if len(sp_path) != 5:
-        raise FatalPSBTIssue("SP spend path must have 5 components")
-    if sp_path[0] != (352 | 0x80000000):
-        raise FatalPSBTIssue("SP spend key purpose must use 352h path")
-
-    expected_coin = chains.current_chain().b44_cointype | 0x80000000
-    if sp_path[1] != expected_coin:
-        raise FatalPSBTIssue("SP spend path coin type does not match network")
-    if not (sp_path[2] & 0x80000000):
-        raise FatalPSBTIssue("SP spend path account must be hardened")
-    if sp_path[3] != 0x80000000:
-        raise FatalPSBTIssue("SP spend path key type must be 0h")
-
-    B_spend = input.get(B_spend_coords)
-    if _compute_silent_payment_spending_xonly(B_spend, input.sp_tweak) != output_xonly:
-        raise FatalPSBTIssue("SP_TWEAK does not match UTXO output key")
+def sp_derive_path(coin_type, account):
+    """BIP-352 base derivation path for a given coin type and account number"""
+    return "m/352h/%dh/%dh" % (coin_type, account)
 
 
 # -----------------------------------------------------------------------------
 # Silent Payments Cryptographic Primitives
 # -----------------------------------------------------------------------------
 
+def _apply_label_to_spend_key(B_spend_bytes, b_scan_bytes, label):
+    """
+    Apply BIP-352 label tweak to spend key
+
+    Formula: B_m = B_spend + hash_BIP0352/Label(b_scan || m)*G
+
+    Args:
+        B_spend_bytes: Base spend public key (33-byte compressed)
+        b_scan_bytes: Scan private key (32-byte scalar)
+        label: Label integer (0 for change)
+
+    Returns:
+        bytes: Labeled spend public key (33-byte compressed)
+    """
+    msg = b_scan_bytes + label.to_bytes(4, 'big')
+    label_tweak_bytes = ngu.hash.sha256t(BIP352_LABEL_TAG_H, msg, True)
+    label_tweak_point = ngu.secp256k1.ec_pubkey_tweak_mul(G, label_tweak_bytes)
+    return ngu.secp256k1.ec_pubkey_combine([B_spend_bytes, label_tweak_point])
+
 
 def _combine_pubkeys(pubkeys):
     """
@@ -294,21 +289,19 @@ class SilentPaymentsMixin:
     This class assumes it is mixed into psbtObject and has access to psbt as self
     """
 
-    def process_silent_payments(self, sv):
+    def process_silent_payment_outputs(self, sv):
         """
         Core SP workflow: validate, compute shares, compute scripts
 
         Notes:
-        - This function is intended to be called during the preview phase, before signing
+        - This function is intended to be called during the preview phase and again during signing
         - Single-signer should be able to generate output and preview immediately
         - Multi-signer should generate shares and prompt user to collect all shares if not complete
 
         Returns:
             bool: True if output scripts were computed and are ready for preview/signing
-                  False if we generated shares but are waiting on others, or if we don't have necessary info to proceed
+                  False if we generated shares but are waiting on others
         """
-        if not self.has_silent_payment_outputs():
-            return False
         self._validate_psbt_structure()
         self._validate_input_eligibility()
         self._validate_ecdh_coverage()
@@ -319,7 +312,7 @@ def process_silent_payments(self, sv):
         if self._is_ecdh_coverage_complete():
             # Computes scripts, or validates existing ones against recomputed values
             self._compute_silent_payment_output_scripts()
-            self._detect_sp_change_outputs()
+            self._detect_sp_change_outputs(sv)
             return self._all_sp_outputs_have_scripts()
         return False
 
@@ -341,6 +334,63 @@ def render_silent_payment_output_string(self, output):
 
         return " - silent payments address -\n%s\n" % encode_silent_payment_address(scan_key, spend_key)
 
+    def validate_silent_payment_inputs(self, sv):
+        """
+        Validate inputs spending silent payments (BIP-376)
+
+        Raises FatalPSBTIssue on any failure.
+        """
+        coin_type = chains.current_chain().b44_cointype
+        expected_coin = coin_type | 0x80000000
+
+        candidate_accounts = self._get_candidate_sp_accounts()
+
+        for i, inp in enumerate(self.inputs):
+            if not inp.is_sp_spend:
+                continue
+
+            # Validate path shape and extract B_spend for both our and foreign inputs
+            B_spend_coords, val_coords = inp.sp_spend_bip32_derivation
+            xfp_path = inp.parse_xfp_path(val_coords)
+            xfp_path = inp.handle_zero_xfp(xfp_path, self.my_xfp, self)
+
+            sp_path = xfp_path[1:]
+            if len(sp_path) != 5:
+                raise FatalPSBTIssue("Input #%d: SP spend path must have 5 components" % i)
+            if sp_path[0] != (352 | 0x80000000):
+                raise FatalPSBTIssue("Input #%d: SP spend key purpose must use 352h path" % i)
+            if sp_path[1] != expected_coin:
+                raise FatalPSBTIssue("Input #%d: SP spend path coin type does not match network" % i)
+            if not (sp_path[2] & 0x80000000):
+                raise FatalPSBTIssue("Input #%d: SP spend path account must be hardened" % i)
+            if sp_path[3] != 0x80000000:
+                raise FatalPSBTIssue("Input #%d: SP spend path key type must be 0h" % i)
+            if sp_path[4] != 0:
+                raise FatalPSBTIssue("Input #%d: SP spend path index must be 0" % i)
+
+            B_spend = inp.get(B_spend_coords)
+
+            # For our inputs, validate account is known and B_spend matches derivation from this device
+            if xfp_path[0] == self.my_xfp:
+                account = sp_path[2] & ~0x80000000
+                if account not in candidate_accounts:
+                    raise FatalPSBTIssue(
+                        "Input #%d: SP spend account not recognized on this device" % i)
+
+                spend_node = sv.derive_path(
+                    sp_derive_path(coin_type, account) + "/0h/0", register=False)
+                derived = spend_node.pubkey()
+                del spend_node
+                if derived != B_spend:
+                    raise FatalPSBTIssue("Input #%d: SP spend pubkey mismatch" % i)
+
+            spk = inp.utxo_spk
+            if not spk or not _is_p2tr(spk):
+                raise FatalPSBTIssue("Input #%d: SP spend UTXO is not P2TR" % i)
+            utxo_xonly = spk[2:34]
+            if _compute_silent_payment_spending_xonly(B_spend, inp.sp_tweak) != utxo_xonly:
+                raise FatalPSBTIssue("Input #%d: SP_TWEAK does not match UTXO output key" % i)
+
     # -----------------------------------------------------------------------------
     # Input Helper Functions
     # -----------------------------------------------------------------------------
@@ -613,9 +663,6 @@ def _compute_and_store_ecdh_shares(self, sv):
         """
         Compute ECDH shares and DLEQ proofs for our inputs, store in PSBT fields
 
-        Notes:
-            Sets self.sp_all_inputs_ours for callers
-
         Returns:
             bool: True if shares were computed
                   False if we have no signable inputs
@@ -644,14 +691,12 @@ def _compute_and_store_ecdh_shares(self, sv):
                 raise FatalPSBTIssue("No eligible inputs for ECDH computation")
             return False
 
-        self.sp_all_inputs_ours = not has_foreign
-
         scan_keys = self._get_silent_payment_scan_keys()
         if not scan_keys:
             return False
 
         for scan_key in scan_keys:
-            if self.sp_all_inputs_ours:
+            if not has_foreign:
                 # Single-signer: combine all input private keys, one global ECDH share and DLEQ proofs
                 combined_sk = _sum_privkeys(sk for _, sk in input_material)
                 ecdh_share = _compute_ecdh_share(combined_sk, scan_key)
@@ -714,6 +759,10 @@ def _compute_silent_payment_output_scripts(self):
     # Utility Functions
     # -----------------------------------------------------------------------------
 
+    def has_silent_payment_inputs(self):
+        """Check if PSBT contains any silent payment inputs"""
+        return any(inp.is_sp_spend for inp in self.inputs)
+
     def has_silent_payment_outputs(self):
         """
         Check if PSBT contains any silent payment outputs
@@ -733,17 +782,59 @@ def _all_sp_outputs_have_scripts(self):
                 return False
         return True
 
-    def _detect_sp_change_outputs(self):
+    def _detect_sp_change_outputs(self, sv):
         """
-        Mark SP outputs as change if sp_v0_label is present and equals 0 (BIP-352 change convention)
+        Mark SP outputs as change if they match our accounts
 
-        No return value; modifies self.outputs 'is_change' in place
+        Raises FraudulentChangeOutput if all SP-eligible inputs are ours (single-signer);
+        multi-signer, a non-matching label=0 output is skipped (may belong to a co-signer).
         """
-        for outp in self.outputs:
+        has_foreign = False
+        for inp in self.inputs:
+            if not self._is_input_eligible(inp):
+                continue
+            if inp.sp_idxs:
+                if self._derive_input_privkey(inp, sv) is None:
+                    has_foreign = True
+                    break
+            elif self._pubkey_from_input(inp):
+                has_foreign = True
+                break
+
+        coin_type = chains.current_chain().b44_cointype
+        candidate_accounts = self._get_candidate_sp_accounts()
+
+        for out_idx, outp in enumerate(self.outputs):
             if not outp.sp_v0_info or not outp.sp_v0_label:
                 continue
-            if int.from_bytes(outp.sp_v0_label, "little") == 0:
-                outp.is_change = True
+            if int.from_bytes(outp.sp_v0_label, "little") != 0:
+                continue
+
+            scan_pub  = outp.sp_v0_info[:33]
+            spend_pub = outp.sp_v0_info[33:66]
+
+            matched = False
+            for account in candidate_accounts:
+                base = sp_derive_path(coin_type, account)
+                scan_node = sv.derive_path(base + "/1h/0", register=False)
+                spend_node = sv.derive_path(base + "/0h/0", register=False)
+                label_spend_pub = _apply_label_to_spend_key(spend_node.pubkey(), scan_node.privkey(), 0)
+                matched = (scan_node.pubkey() == scan_pub) and (label_spend_pub == spend_pub)
+                del scan_node, spend_node
+                if matched:
+                    outp.is_change = True
+                    break
+
+            if not matched and not has_foreign:
+                raise FraudulentChangeOutput(out_idx, "SP change output keys do not match this device")
+
+    def _get_candidate_sp_accounts(self):
+        """Get all known account numbers from device settings, always include account 0"""
+        seen = {0}
+        for _af, acct_num in settings.get('accts', []):
+            if acct_num:
+                seen.add(acct_num)
+        return sorted(seen)
 
     def _get_outpoints(self):
         """

testing/devtest/unit_silentpayments.py

@@ -458,8 +458,6 @@ def _make_sp_output(scan_key):
         (MY_XFP, 44, 0, 0, 1): TEST_PRIVKEY2,
     }
 )
-assert psbt.sp_all_inputs_ours is True
-
 # Foreign input
 psbt = MockPSBT()
 inp_ours = _make_eligible_input(pk_ours, TEST_DERIV_OURS, b"\x01" * 32, b"\x00" * 4)
@@ -469,8 +467,6 @@ def _make_sp_output(scan_key):
 outp.sp_v0_info = TEST_SCAN_KEY + TEST_SPEND_KEY
 psbt.outputs = [outp]
 psbt._compute_and_store_ecdh_shares({TEST_DERIV_OURS: TEST_PRIVKEY})
-assert psbt.sp_all_inputs_ours is False
-
 # No signable inputs
 psbt = MockPSBT()
 inp = _make_eligible_input(b"\x02" + b"\xbb" * 32, (FOREIGN_XFP, 44, 0, 0, 0), b"\x01" * 32, b"\x00" * 4)