psbt: don't put xpubs and full paths into tx by def; only while signing

electrum/gui/qt/transaction_dialog.py

@@ -96,7 +96,7 @@ class TxDialog(QDialog, MessageBoxMixin):
 
         # if the wallet can populate the inputs with more info, do it now.
         # as a result, e.g. we might learn an imported address tx is segwit,
-        # in which case it's ok to display txid
+        # or that a beyond-gap-limit address is is_mine
         tx.add_info_from_wallet(self.wallet)
 
         self.setMinimumWidth(950)

electrum/keystore.py

@@ -349,14 +349,15 @@ class Xpub:
         """
         return self._root_fingerprint
 
-    def get_fp_and_derivation_to_be_used_in_partial_tx(self, der_suffix: Sequence[int]) -> Tuple[bytes, Sequence[int]]:
+    def get_fp_and_derivation_to_be_used_in_partial_tx(self, der_suffix: Sequence[int], *,
-        """Returns fingerprint and 'full' derivation path corresponding to a derivation suffix.
+                                                       only_der_suffix: bool = True) -> Tuple[bytes, Sequence[int]]:
-        The fingerprint is either the root fp or the intermediate fp, depending on what is available,
+        """Returns fingerprint and derivation path corresponding to a derivation suffix.
-        and the 'full' derivation path is adjusted accordingly.
+        The fingerprint is either the root fp or the intermediate fp, depending on what is available
+        and 'only_der_suffix', and the derivation path is adjusted accordingly.
         """
         fingerprint_hex = self.get_root_fingerprint()
         der_prefix_str = self.get_derivation_prefix()
-        if fingerprint_hex is not None and der_prefix_str is not None:
+        if not only_der_suffix and fingerprint_hex is not None and der_prefix_str is not None:
             # use root fp, and true full path
             fingerprint_bytes = bfh(fingerprint_hex)
             der_prefix_ints = convert_bip32_path_to_list_of_uint32(der_prefix_str)
@@ -367,9 +368,10 @@ class Xpub:
         der_full = der_prefix_ints + list(der_suffix)
         return fingerprint_bytes, der_full
 
-    def get_xpub_to_be_used_in_partial_tx(self) -> str:
+    def get_xpub_to_be_used_in_partial_tx(self, *, only_der_suffix: bool) -> str:
         assert self.xpub
-        fp_bytes, der_full = self.get_fp_and_derivation_to_be_used_in_partial_tx(der_suffix=[])
+        fp_bytes, der_full = self.get_fp_and_derivation_to_be_used_in_partial_tx(der_suffix=[],
+                                                                                 only_der_suffix=only_der_suffix)
         bip32node = BIP32Node.from_xkey(self.xpub)
         depth = len(der_full)
         child_number_int = der_full[-1] if len(der_full) >= 1 else 0
@@ -603,7 +605,8 @@ class Old_KeyStore(Deterministic_KeyStore):
         return self._root_fingerprint
 
     # TODO Old_KeyStore and Xpub could share a common baseclass?
-    def get_fp_and_derivation_to_be_used_in_partial_tx(self, der_suffix: Sequence[int]) -> Tuple[bytes, Sequence[int]]:
+    def get_fp_and_derivation_to_be_used_in_partial_tx(self, der_suffix: Sequence[int], *,
+                                                       only_der_suffix: bool = True) -> Tuple[bytes, Sequence[int]]:
         fingerprint_hex = self.get_root_fingerprint()
         der_prefix_str = self.get_derivation_prefix()
         fingerprint_bytes = bfh(fingerprint_hex)

electrum/plugins/coldcard/coldcard.py

@@ -575,7 +575,7 @@ class ColdcardPlugin(HW_PluginBase):
         xpubs = []
         derivs = set()
         for xpub, ks in zip(wallet.get_master_public_keys(), wallet.get_keystores()):
-            fp_bytes, der_full = ks.get_fp_and_derivation_to_be_used_in_partial_tx(der_suffix=[])
+            fp_bytes, der_full = ks.get_fp_and_derivation_to_be_used_in_partial_tx(der_suffix=[], only_der_suffix=False)
             fp_hex = fp_bytes.hex().upper()
             der_prefix_str = bip32.convert_bip32_intpath_to_strpath(der_full)
             xpubs.append( (fp_hex, xpub, der_prefix_str) )
@@ -620,7 +620,7 @@ class ColdcardPlugin(HW_PluginBase):
             xfp_paths = []
             for pubkey_hex in pubkey_deriv_info:
                 ks, der_suffix = pubkey_deriv_info[pubkey_hex]
-                fp_bytes, der_full = ks.get_fp_and_derivation_to_be_used_in_partial_tx(der_suffix)
+                fp_bytes, der_full = ks.get_fp_and_derivation_to_be_used_in_partial_tx(der_suffix, only_der_suffix=False)
                 xfp_int = xfp_int_from_xfp_bytes(fp_bytes)
                 xfp_paths.append([xfp_int] + list(der_full))
 

electrum/transaction.py

@@ -1731,19 +1731,46 @@ class PartialTransaction(Transaction):
         txin.witness = None
         self.invalidate_ser_cache()
 
-    def add_info_from_wallet(self, wallet: 'Abstract_Wallet') -> None:
+    def add_info_from_wallet(self, wallet: 'Abstract_Wallet', *,
+                             include_xpubs_and_full_paths: bool = False) -> None:
         if self.is_complete():
             return
-        from .keystore import Xpub
+        only_der_suffix = not include_xpubs_and_full_paths
-        for ks in wallet.get_keystores():
+        if include_xpubs_and_full_paths:
-            if isinstance(ks, Xpub):
+            from .keystore import Xpub
-                fp_bytes, der_full = ks.get_fp_and_derivation_to_be_used_in_partial_tx(der_suffix=[])
+            for ks in wallet.get_keystores():
-                bip32node = BIP32Node.from_xkey(ks.get_xpub_to_be_used_in_partial_tx())
+                if isinstance(ks, Xpub):
-                self.xpubs[bip32node] = (fp_bytes, der_full)
+                    fp_bytes, der_full = ks.get_fp_and_derivation_to_be_used_in_partial_tx(der_suffix=[],
+                                                                                           only_der_suffix=only_der_suffix)
+                    xpub = ks.get_xpub_to_be_used_in_partial_tx(only_der_suffix=only_der_suffix)
+                    bip32node = BIP32Node.from_xkey(xpub)
+                    self.xpubs[bip32node] = (fp_bytes, der_full)
         for txin in self.inputs():
-            wallet.add_input_info(txin)
+            wallet.add_input_info(txin, only_der_suffix=only_der_suffix)
         for txout in self.outputs():
-            wallet.add_output_info(txout)
+            wallet.add_output_info(txout, only_der_suffix=only_der_suffix)
+
+    def remove_xpubs_and_bip32_paths(self) -> None:
+        self.xpubs.clear()
+        for txin in self.inputs():
+            txin.bip32_paths.clear()
+        for txout in self.outputs():
+            txout.bip32_paths.clear()
+
+    def prepare_for_export_for_coinjoin(self) -> None:
+        """Removes all sensitive details."""
+        # globals
+        self.xpubs.clear()
+        self._unknown.clear()
+        # inputs
+        for txin in self.inputs():
+            txin.bip32_paths.clear()
+        # outputs
+        for txout in self.outputs():
+            txout.redeem_script = None
+            txout.witness_script = None
+            txout.bip32_paths.clear()
+            txout._unknown.clear()
 
     def remove_signatures(self):
         for txin in self.inputs():

electrum/wallet.py

@@ -1226,10 +1226,11 @@ class Abstract_Wallet(AddressSynchronizer):
         locktime = get_locktime_for_new_transaction(self.network)
         return PartialTransaction.from_io(inputs, outputs, locktime=locktime)
 
-    def _add_input_sig_info(self, txin: PartialTxInput, address: str) -> None:
+    def _add_input_sig_info(self, txin: PartialTxInput, address: str, *, only_der_suffix: bool = True) -> None:
         raise NotImplementedError()  # implemented by subclasses
 
-    def _add_txinout_derivation_info(self, txinout: Union[PartialTxInput, PartialTxOutput], address: str) -> None:
+    def _add_txinout_derivation_info(self, txinout: Union[PartialTxInput, PartialTxOutput],
+                                     address: str, *, only_der_suffix: bool = True) -> None:
         pass  # implemented by subclasses
 
     def _add_input_utxo_info(self, txin: PartialTxInput, address: str) -> None:
@@ -1255,7 +1256,7 @@ class Abstract_Wallet(AddressSynchronizer):
         """
         return False  # implemented by subclasses
 
-    def add_input_info(self, txin: PartialTxInput) -> None:
+    def add_input_info(self, txin: PartialTxInput, *, only_der_suffix: bool = True) -> None:
         address = self.get_txin_address(txin)
         if not self.is_mine(address):
             is_mine = self._learn_derivation_path_for_address_from_txinout(txin, address)
@@ -1277,7 +1278,7 @@ class Abstract_Wallet(AddressSynchronizer):
                 txin.witness_script = bfh(witness_script_hex) if witness_script_hex else None
             except UnknownTxinType:
                 pass
-        self._add_input_sig_info(txin, address)
+        self._add_input_sig_info(txin, address, only_der_suffix=only_der_suffix)
 
     def can_sign(self, tx: Transaction) -> bool:
         if not isinstance(tx, PartialTransaction):
@@ -1309,7 +1310,7 @@ class Abstract_Wallet(AddressSynchronizer):
                 tx = Transaction(raw_tx)
         return tx
 
-    def add_output_info(self, txout: PartialTxOutput) -> None:
+    def add_output_info(self, txout: PartialTxOutput, *, only_der_suffix: bool = True) -> None:
         address = txout.address
         if not self.is_mine(address):
             is_mine = self._learn_derivation_path_for_address_from_txinout(txout, address)
@@ -1320,7 +1321,7 @@ class Abstract_Wallet(AddressSynchronizer):
         txout.is_change = self.is_change(address)
         if isinstance(self, Multisig_Wallet):
             txout.num_sig = self.m
-        self._add_txinout_derivation_info(txout, address)
+        self._add_txinout_derivation_info(txout, address, only_der_suffix=only_der_suffix)
         if txout.redeem_script is None:
             try:
                 redeem_script_hex = self.get_redeem_script(address)
@@ -1339,14 +1340,21 @@ class Abstract_Wallet(AddressSynchronizer):
             return
         if not isinstance(tx, PartialTransaction):
             return
-        tx.add_info_from_wallet(self)
+        # add info to a temporary tx copy; including xpubs
+        # and full derivation paths as hw keystores might want them
+        tmp_tx = copy.deepcopy(tx)
+        tmp_tx.add_info_from_wallet(self, include_xpubs_and_full_paths=True)
         # sign. start with ready keystores.
         for k in sorted(self.get_keystores(), key=lambda ks: ks.ready_to_sign(), reverse=True):
             try:
-                if k.can_sign(tx):
+                if k.can_sign(tmp_tx):
-                    k.sign_transaction(tx, password)
+                    k.sign_transaction(tmp_tx, password)
             except UserCancelled:
                 continue
+        # remove sensitive info; then copy back details from temporary tx
+        tmp_tx.remove_xpubs_and_bip32_paths()
+        tx.combine_with_other_psbt(tmp_tx)
+        tx.add_info_from_wallet(self, include_xpubs_and_full_paths=False)
         return tx
 
     def try_detecting_internal_addresses_corruption(self):
@@ -1901,7 +1909,7 @@ class Imported_Wallet(Simple_Wallet):
     def get_txin_type(self, address):
         return self.db.get_imported_address(address).get('type', 'address')
 
-    def _add_input_sig_info(self, txin, address):
+    def _add_input_sig_info(self, txin, address, *, only_der_suffix=True):
         if not self.is_mine(address):
             return
         if txin.script_type in ('unknown', 'address'):
@@ -2013,17 +2021,18 @@ class Deterministic_Wallet(Abstract_Wallet):
         return {k.derive_pubkey(*der_suffix): (k, der_suffix)
                 for k in self.get_keystores()}
 
-    def _add_input_sig_info(self, txin, address):
+    def _add_input_sig_info(self, txin, address, *, only_der_suffix=True):
-        self._add_txinout_derivation_info(txin, address)
+        self._add_txinout_derivation_info(txin, address, only_der_suffix=only_der_suffix)
 
-    def _add_txinout_derivation_info(self, txinout, address):
+    def _add_txinout_derivation_info(self, txinout, address, *, only_der_suffix=True):
         if not self.is_mine(address):
             return
         pubkey_deriv_info = self.get_public_keys_with_deriv_info(address)
         txinout.pubkeys = sorted([bfh(pk) for pk in list(pubkey_deriv_info)])
         for pubkey_hex in pubkey_deriv_info:
             ks, der_suffix = pubkey_deriv_info[pubkey_hex]
-            fp_bytes, der_full = ks.get_fp_and_derivation_to_be_used_in_partial_tx(der_suffix)
+            fp_bytes, der_full = ks.get_fp_and_derivation_to_be_used_in_partial_tx(der_suffix,
+                                                                                   only_der_suffix=only_der_suffix)
             txinout.bip32_paths[bfh(pubkey_hex)] = (fp_bytes, der_full)
 
     def create_new_address(self, for_change=False):