keystore: ignore fingerprint for pubkeys in psbt, try to match all keys

electrum/keystore.py

@@ -371,8 +371,9 @@ class MasterPublicKeyMixin(ABC):
             *,
             only_der_suffix=True,
     ) -> Union[Sequence[int], str, None]:
+        EXPECTED_DER_SUFFIX_LEN = 2
         def test_der_suffix_against_pubkey(der_suffix: Sequence[int], pubkey: bytes) -> bool:
-            if len(der_suffix) != 2:
+            if len(der_suffix) != EXPECTED_DER_SUFFIX_LEN:
                 return False
             try:
                 if pubkey != self.derive_pubkey(*der_suffix):
@@ -387,11 +388,11 @@ class MasterPublicKeyMixin(ABC):
         der_suffix = None
         full_path = None
         # 1. try fp against our root
-        my_root_fingerprint_hex = self.get_root_fingerprint()
-        my_der_prefix_str = self.get_derivation_prefix()
-        ks_der_prefix = convert_bip32_path_to_list_of_uint32(my_der_prefix_str) if my_der_prefix_str else None
-        if (my_root_fingerprint_hex is not None and ks_der_prefix is not None and
-                fp_found.hex() == my_root_fingerprint_hex):
+        ks_root_fingerprint_hex = self.get_root_fingerprint()
+        ks_der_prefix_str = self.get_derivation_prefix()
+        ks_der_prefix = convert_bip32_path_to_list_of_uint32(ks_der_prefix_str) if ks_der_prefix_str else None
+        if (ks_root_fingerprint_hex is not None and ks_der_prefix is not None and
+                fp_found.hex() == ks_root_fingerprint_hex):
             if path_found[:len(ks_der_prefix)] == ks_der_prefix:
                 der_suffix = path_found[len(ks_der_prefix):]
                 if not test_der_suffix_against_pubkey(der_suffix, pubkey):
@@ -402,10 +403,17 @@ class MasterPublicKeyMixin(ABC):
             der_suffix = path_found
             if not test_der_suffix_against_pubkey(der_suffix, pubkey):
                 der_suffix = None
-        # NOTE: problem: if we don't know our root fp, but tx contains root fp and full path,
-        #       we will miss the pubkey (false negative match). Though it might still work
-        #       within gap limit due to tx.add_info_from_wallet overwriting the fields.
-        #       Example: keystore has intermediate xprv without root fp; tx contains root fp and full path.
+        # 3. hack/bruteforce: ignore fp and check pubkey anyway
+        #    This is only to resolve the following scenario/problem:
+        #    problem: if we don't know our root fp, but tx contains root fp and full path,
+        #             we will miss the pubkey (false negative match). Though it might still work
+        #             within gap limit due to tx.add_info_from_wallet overwriting the fields.
+        #             Example: keystore has intermediate xprv without root fp; tx contains root fp and full path.
+        if der_suffix is None:
+            der_suffix = path_found[-EXPECTED_DER_SUFFIX_LEN:]
+            if not test_der_suffix_against_pubkey(der_suffix, pubkey):
+                der_suffix = None
+        # if all attempts/methods failed, we give up now:
         if der_suffix is None:
             return None
         if ks_der_prefix is not None:

electrum/tests/test_wallet_vertical.py

@@ -2045,6 +2045,27 @@ class TestWalletOfflineSigning(TestCaseForTestnet):
         self.assertEqual('ee76c0c6da87f0eb5ab4d1ae05d3942512dcd3c4c42518f9d3619e74400cfc1f', tx.txid())
         self.assertEqual('484e350beaa722a744bb3e2aa38de005baa8526d86536d6143e5814355acf775', tx.wtxid())
 
+    @mock.patch.object(wallet.Abstract_Wallet, 'save_db')
+    def test_signing_where_offline_ks_does_not_have_keyorigin_but_psbt_contains_it(self, mock_save_db):
+        # keystore has intermediate xprv without root fp; tx contains root fp and full path.
+        # tx has input with key beyond gap limit
+        wallet_offline = WalletIntegrityHelper.create_standard_wallet(
+            # bip39 seed: "brave scare company drastic consider confirm grow differ alter wide olympic utility"
+            # der: m/84'/1'/0'
+            keystore.from_xprv('vprv9KXDgRXYp3WCozCS3bMehASe2cJhY28DihCZ3KuyiTTjngopkfRC9QkH1SUREyCvnV7TSD6EgEHTTYa5yod7ZveBhVReEU1uDgfVASFqLNw'),
+            gap_limit=4,
+            config=self.config
+        )
+
+        tx = tx_from_any('70736274ff01005202000000017b748828553b1127b86674e71ad0cd4a2e5e8baeab8792a3c3263f7ea0ba86500000000000fdffffff01ad16010000000000160014d74b54300bc0d4b6e8f506fe540b47ce0da38b4a08f21c00000100bf0200000000010163a419b779be17167c54ff3acb1205e5347fbd72963f89fb1d66b5cf09f329c90000000000fdffffff011b17010000000000160014ed420532f0c33477b9b3fbb57431b4a1adce99c90247304402204e4ad4992fa8798e3b595d17c59961b905ca71c32dc3ba910ae14f139259ffbe02206ee2281f21499e46aa77f4bec2edce3674fea529d9dd340439365c2232bad35701210334080358ffdac08f83d6800a8e477e3512ad5c39ede553089db8c4bbe16f59aad7f11c00220602d137f257a96cbc58c7e60f2085cd65a311e242459e23d1efbed77dd8f372513818cc2bdaaa540000800100008000000080000000001e000000002202030671d324eeba0f85499a8749f783a4883103d23f5dedbe048391ff18c3da067818cc2bdaaa540000800100008000000080000000000100000000')
+        self.assertEqual('065b6e0a5731107641828337f5e000c9ddd94a12d074708643b0bca517374c6a', tx.txid())
+
+        # sign tx
+        tx = wallet_offline.sign_transaction(tx, password=None)
+        self.assertTrue(tx.is_complete())
+        self.assertEqual('020000000001017b748828553b1127b86674e71ad0cd4a2e5e8baeab8792a3c3263f7ea0ba86500000000000fdffffff01ad16010000000000160014d74b54300bc0d4b6e8f506fe540b47ce0da38b4a0247304402203098741bf4d4f956e96f2706a517a1c0a63f67a242a50d155fbc56ad0bbac8b102207e535391c03bdab641f3205762311c1e6648b3459681e53d68fa44e63604a7f6012102d137f257a96cbc58c7e60f2085cd65a311e242459e23d1efbed77dd8f372513808f21c00',
+                         str(tx))
+
     @mock.patch.object(wallet.Abstract_Wallet, 'save_db')
     def test_sending_offline_wif_online_addr_p2pkh(self, mock_save_db):  # compressed pubkey
         wallet_offline = WalletIntegrityHelper.create_imported_wallet(privkeys=True, config=self.config)