[New Feature] Support bip45 nonsegwit legacy multisig p2sh

src/seedsigner/helpers/embit_utils.py

@@ -87,18 +87,16 @@ def get_multisig_address(descriptor: Descriptor, index: int = 0, is_change: bool
     else:
         branch_index = 0
 
-    if descriptor.is_segwit:
-        # Could be native segwit or nested segwit (descriptor.is_wrapped)
+    # Can derive p2wsh, p2sh-p2wsh, and legacy (non-segwit) p2sh
+    if descriptor.is_segwit or (descriptor.is_legacy and descriptor.is_basic_multisig):
         return descriptor.derive(index, branch_index=branch_index).script_pubkey().address(network=NETWORKS[embit_network])
 
-    elif descriptor.is_legacy:
-        # TODO: Not yet implemented!
-        raise Exception("Legacy P2PKH verification not yet implemented!")
-
     elif descriptor.is_taproot:
         # TODO: Not yet implemented!
         raise Exception("Taproot verification not yet implemented!")
 
+    raise Exception(f"{descriptor.script_pubkey().script_type()} address verification not yet implemented!")
+
 
 
 def get_embit_network_name(settings_name):

src/seedsigner/models/decode_qr.py

@@ -954,11 +954,12 @@ def add(self, segment, qr_type=QRType.BITCOIN_ADDRESS):
                     self.address_type = (SettingsConstants.LEGACY_P2PKH, SettingsConstants.TESTNET)
 
                 elif r == "3":
-                    # Nested Segwit Single Sig (P2WPKH in P2SH) or Multisig (P2WSH in P2SH); mainnet
+                    # Nested segwit single sig (p2sh-p2wpkh), nested segwit multisig (p2sh-p2wsh), or legacy multisig (p2sh); mainnet
+                    # TODO: Would be more correct to use a P2SH constant
                     self.address_type = (SettingsConstants.NESTED_SEGWIT, SettingsConstants.MAINNET)
 
                 elif r == "2":
-                    # Nested Segwit Single Sig (P2WPKH in P2SH) or Multisig (P2WSH in P2SH); testnet
+                    # Nested segwit single sig (p2sh-p2wpkh), nested segwit multisig (p2sh-p2wsh), or legacy multisig (p2sh); testnet / regtest
                     self.address_type = (SettingsConstants.NESTED_SEGWIT, SettingsConstants.TESTNET)
 
                 elif r == "bc1q":

src/seedsigner/models/psbt_parser.py

@@ -88,12 +88,17 @@ def _parse_inputs(self):
         for inp in self.psbt.inputs:
             if inp.witness_utxo:
                 self.input_amount += inp.witness_utxo.value
-                inp_policy = PSBTParser._get_policy(inp, inp.witness_utxo.script_pubkey, self.psbt.xpubs)
-                if self.policy == None:
-                    self.policy = inp_policy
-                else:
-                    if self.policy != inp_policy:
-                        raise RuntimeError("Mixed inputs in the transaction")
+                script_pubkey = inp.witness_utxo.script_pubkey
+            elif inp.non_witness_utxo:
+                self.input_amount += inp.utxo.value
+                script_pubkey = inp.script_pubkey
+
+            inp_policy = PSBTParser._get_policy(inp, script_pubkey, self.psbt.xpubs)
+            if self.policy == None:
+                self.policy = inp_policy
+            else:
+                if self.policy != inp_policy:
+                    raise RuntimeError("Mixed inputs in the transaction")
 
 
     def _parse_outputs(self):
@@ -118,29 +123,37 @@ def _parse_outputs(self):
 
                 # empty script by default
                 sc = script.Script(b"")
+
                 # multisig, we know witness script
                 if self.policy["type"] == "p2wsh":
                     sc = script.p2wsh(out.witness_script)
+
                 elif self.policy["type"] == "p2sh-p2wsh":
                     sc = script.p2sh(script.p2wsh(out.witness_script))
+
+                # Arbitrary p2sh; includes pre-segwit multisig (m/45')
+                elif self.policy["type"] == "p2sh":
+                    sc = script.p2sh(out.redeem_script)
 
                 # single-sig
                 elif "pkh" in self.policy["type"]:
                     my_pubkey = None
+
                     # should be one or zero for single-key addresses
                     if len(out.bip32_derivations.values()) > 0:
                         der = list(out.bip32_derivations.values())[0].derivation
                         my_pubkey = self.root.derive(der)
+
                     if self.policy["type"] == "p2wpkh" and my_pubkey is not None:
                         sc = script.p2wpkh(my_pubkey)
+
                     elif self.policy["type"] == "p2sh-p2wpkh" and my_pubkey is not None:
                         sc = script.p2sh(script.p2wpkh(my_pubkey))
 
                     if sc.data == self.psbt.tx.vout[i].script_pubkey.data:
                         is_change = True
 
                 elif "p2tr" in self.policy["type"]:
-                    print("TAPROOT output!")
                     my_pubkey = None
                     # should have one or zero derivations for single-key addresses
                     if len(out.taproot_bip32_derivations.values()) > 0:
@@ -236,15 +249,25 @@ def _get_policy(scope, scriptpubkey, xpubs):
             ):
                 script_type = "p2sh-p2wpkh"
         policy = {"type": script_type}
+
         # expected multisig
+        script = None
         if "p2wsh" in script_type and scope.witness_script is not None:
-            m, n, pubkeys = PSBTParser._parse_multisig(scope.witness_script)
+            script = scope.witness_script
+
+        elif "p2sh" in script_type and scope.redeem_script is not None:
+            script = scope.redeem_script
+
+        if script is not None:
+            m, n, pubkeys = PSBTParser._parse_multisig(script)
+
             # check pubkeys are derived from cosigners
             try:
                 cosigners = PSBTParser._get_cosigners(pubkeys, scope.bip32_derivations, xpubs)
                 policy.update({"m": m, "n": n, "cosigners": cosigners})
             except:
                 policy.update({"m": m, "n": n})
+
         return policy
 
 

src/seedsigner/views/seed_views.py

@@ -1586,28 +1586,28 @@ def run(self):
 
 
 class AddressVerificationSigTypeView(View):
-    def run(self):
-        sig_type_settings_entry = SettingsDefinition.get_settings_entry(SettingsConstants.SETTING__SIG_TYPES)
-        SINGLE_SIG = sig_type_settings_entry.get_selection_option_display_name_by_value(SettingsConstants.SINGLE_SIG)
-        MULTISIG = sig_type_settings_entry.get_selection_option_display_name_by_value(SettingsConstants.MULTISIG)
+    SINGLE_SIG = "Single Sig"
+    MULTISIG = "Multisig"
 
-        button_data = [SINGLE_SIG, MULTISIG]
-        selected_menu_num = seed_screens.AddressVerificationSigTypeScreen(
+    def run(self):
+        button_data = [self.SINGLE_SIG, self.MULTISIG]
+        selected_menu_num = self.run_screen(
+            seed_screens.AddressVerificationSigTypeScreen,
             title="Verify Address",
             text="Sig type can't be auto-detected from this address. Please specify:",
             button_data=button_data,
             is_bottom_list=True,
-        ).display()
+        )
 
         if selected_menu_num == RET_CODE__BACK_BUTTON:
             self.controller.unverified_address = None
             return Destination(BackStackView)
 
-        elif button_data[selected_menu_num] == SINGLE_SIG:
+        elif button_data[selected_menu_num] == self.SINGLE_SIG:
             sig_type = SettingsConstants.SINGLE_SIG
             destination = Destination(SeedSelectSeedView, view_args=dict(flow=Controller.FLOW__VERIFY_SINGLESIG_ADDR))
 
-        elif button_data[selected_menu_num] == MULTISIG:
+        elif button_data[selected_menu_num] == self.MULTISIG:
             sig_type = SettingsConstants.MULTISIG
             if self.controller.multisig_wallet_descriptor:
                 destination = Destination(SeedAddressVerificationView)
@@ -1708,7 +1708,8 @@ def run(self):
         # and resume displaying the screen. User won't even notice that the Screen is
         # being re-constructed.
         while True:
-            selected_menu_num = seed_screens.SeedAddressVerificationScreen(
+            selected_menu_num = self.run_screen(
+                seed_screens.SeedAddressVerificationScreen,
                 address=self.address,
                 derivation_path=self.derivation_path,
                 script_type=script_type_display,
@@ -1718,14 +1719,20 @@ def run(self):
                 threadsafe_counter=self.threadsafe_counter,
                 verified_index=self.verified_index,
                 button_data=button_data,
-            ).display()
+            )
 
             if self.verified_index.cur_count is not None:
                 break
 
             if selected_menu_num == RET_CODE__BACK_BUTTON:
                 break
 
+            if selected_menu_num is None:
+                # Only happens in the test suite; the screen isn't actually executed so
+                # it returns before the brute force thread has completed.
+                time.sleep(0.1)
+                continue
+
             if button_data[selected_menu_num] == SKIP_10:
                 self.threadsafe_counter.increment(10)
 
@@ -1831,20 +1838,25 @@ def run(self):
 
 
 class LoadMultisigWalletDescriptorView(View):
+    SCAN = ("Scan Descriptor", SeedSignerIconConstants.QRCODE)
+    CANCEL = "Cancel"
+
     def run(self):
-        SCAN = ("Scan Descriptor", SeedSignerIconConstants.QRCODE)
-        CANCEL = "Cancel"
-        button_data = [SCAN, CANCEL]
-        selected_menu_num = seed_screens.LoadMultisigWalletDescriptorScreen(
+        button_data = [
+            self.SCAN,
+            self.CANCEL
+        ]
+        selected_menu_num = self.run_screen(
+            seed_screens.LoadMultisigWalletDescriptorScreen,
             button_data=button_data,
             show_back_button=False,
-        ).display()
+        )
 
-        if button_data[selected_menu_num] == SCAN:
+        if button_data[selected_menu_num] == self.SCAN:
             from seedsigner.views.scan_views import ScanWalletDescriptorView
             return Destination(ScanWalletDescriptorView)
 
-        elif button_data[selected_menu_num] == CANCEL:
+        elif button_data[selected_menu_num] == self.CANCEL:
             if self.controller.resume_main_flow == Controller.FLOW__PSBT:
                 return Destination(BackStackView)
             else:
@@ -1853,6 +1865,11 @@ def run(self):
 
 
 class MultisigWalletDescriptorView(View):
+    RETURN = "Return to PSBT"
+    VERIFY_ADDR = "Verify Addr"
+    ADDRESS_EXPLORER = "Address Explorer"
+    OK = "OK"
+
     def run(self):
         descriptor = self.controller.multisig_wallet_descriptor
 
@@ -1863,42 +1880,38 @@ def run(self):
 
         policy = descriptor.brief_policy.split("multisig")[0].strip()
 
-        RETURN = "Return to PSBT"
-        VERIFY = "Verify Addr"
-        EXPLORER = "Address Explorer"
-        OK = "OK"
-
-        button_data = [OK]
+        button_data = [self.OK]
         if self.controller.resume_main_flow:
             if self.controller.resume_main_flow == Controller.FLOW__PSBT:
-                button_data = [RETURN]
+                button_data = [self.RETURN]
             elif self.controller.resume_main_flow == Controller.FLOW__VERIFY_MULTISIG_ADDR and self.controller.unverified_address:
-                VERIFY += f""" {self.controller.unverified_address["address"][:7]}"""
-                button_data = [VERIFY]
+                verify_addr_display = f"""{self.VERIFY_ADDR} {self.controller.unverified_address["address"][:7]}"""
+                button_data = [verify_addr_display]
             elif self.controller.resume_main_flow == Controller.FLOW__ADDRESS_EXPLORER:
-                button_data = [EXPLORER]
+                button_data = [self.ADDRESS_EXPLORER]
 
-        selected_menu_num = seed_screens.MultisigWalletDescriptorScreen(
+        selected_menu_num = self.run_screen(
+            seed_screens.MultisigWalletDescriptorScreen,
             policy=policy,
             fingerprints=fingerprints,
             button_data=button_data,
-        ).display()
+        )
 
         if selected_menu_num == RET_CODE__BACK_BUTTON:
             self.controller.multisig_wallet_descriptor = None
             return Destination(BackStackView)
 
-        elif button_data[selected_menu_num] == RETURN:
+        elif button_data[selected_menu_num] == self.RETURN:
             # Jump straight back to PSBT change verification
             from seedsigner.views.psbt_views import PSBTChangeDetailsView
             self.controller.resume_main_flow = None
             return Destination(PSBTChangeDetailsView, view_args=dict(change_address_num=0))
 
-        elif button_data[selected_menu_num] == VERIFY:
+        elif button_data[selected_menu_num].startswith(self.VERIFY_ADDR):
             self.controller.resume_main_flow = None
             return Destination(SeedAddressVerificationView)
 
-        elif button_data[selected_menu_num] == EXPLORER:
+        elif button_data[selected_menu_num] == self.ADDRESS_EXPLORER:
             from seedsigner.views.tools_views import ToolsAddressExplorerAddressTypeView
             self.controller.resume_main_flow = None
             return Destination(ToolsAddressExplorerAddressTypeView)

src/seedsigner/views/tools_views.py

@@ -28,11 +28,11 @@ class ToolsMenuView(View):
     IMAGE = (" New seed", FontAwesomeIconConstants.CAMERA)
     DICE = ("New seed", FontAwesomeIconConstants.DICE)
     KEYBOARD = ("Calc 12th/24th word", FontAwesomeIconConstants.KEYBOARD)
-    EXPLORER = "Address Explorer"
-    ADDRESS = "Verify address"
+    ADDRESS_EXPLORER = "Address Explorer"
+    VERIFY_ADDRESS = "Verify address"
 
     def run(self):
-        button_data = [self.IMAGE, self.DICE, self.KEYBOARD, self.EXPLORER, self.ADDRESS]
+        button_data = [self.IMAGE, self.DICE, self.KEYBOARD, self.ADDRESS_EXPLORER, self.VERIFY_ADDRESS]
 
         selected_menu_num = self.run_screen(
             ButtonListScreen,
@@ -53,10 +53,10 @@ def run(self):
         elif button_data[selected_menu_num] == self.KEYBOARD:
             return Destination(ToolsCalcFinalWordNumWordsView)
 
-        elif button_data[selected_menu_num] == self.EXPLORER:
+        elif button_data[selected_menu_num] == self.ADDRESS_EXPLORER:
             return Destination(ToolsAddressExplorerSelectSourceView)
 
-        elif button_data[selected_menu_num] == self.ADDRESS:
+        elif button_data[selected_menu_num] == self.VERIFY_ADDRESS:
             from seedsigner.views.scan_views import ScanAddressView
             return Destination(ScanAddressView)
 

tests/test_embit_utils.py

@@ -266,9 +266,9 @@ def test_get_multisig_address():
     from embit.descriptor import Descriptor
 
     # jdlcdl: these vectors created with electrum & sparrow as a 2 of 3 multisig based on bip39-bip32-standard-path wallets
-    #    keystore1 = 0x00*16 = 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about'
-    #    keystore2 = 0x11*16 = 'baby mass dust captain baby mass dust captain baby mass dust casino'
-    #    keystore3 = 0x22*16 = 'captain baby mass dust captain baby mass dust captain baby mass dutch'
+    #    keystore1 = 0x00*16 = 73c5da0a = 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about'
+    #    keystore2 = 0x11*16 = 0be174ee = 'baby mass dust captain baby mass dust captain baby mass dust casino'
+    #    keystore3 = 0x22*16 = 8d55ff0d = 'captain baby mass dust captain baby mass dust captain baby mass dutch'
 
     vector_args_expected = { 
         # multisig native segwit on testnet, first payment and change addresses
@@ -279,8 +279,9 @@ def test_get_multisig_address():
         ("sh(wsh(sortedmulti(2,[73c5da0a/48h/1h/1h/0h/1h]tpubDFH9dgzveyD8yHQb8VrpG8FYAuwcLMHMje2CCcbBo1FpaGzYVtJeYYxcYgRqSTta5utUFts8nPPHs9C2bqoxrey5jia6Dwf9mpwrPq7YvcJ/{0,1}/*,[0be174ee/48h/1h/0h/1h]tpubDEsePyLPkbxbnj6XuKvWwdERHaKkikZxaGJ9sJqmM7okbZXgkNSFiGU6GX6qEes6kD8f9Z9FosYB9UEnBSgBEyEwwJhj4uUcFE1WE8VtKoh/{0,1}/*,[8d55ff0d/48h/1h/0h/1h]tpubDDxNVWk924RTT3vyGLHdSDoZ2JUVX7jUsPcwCQ9MrKHAtJrW5zECTF9rFHCvqu526E4PjHp61hBknts2c5aGexvX7hvCZ8TGPvQFdzxxy59/{0,1}/*)))#2ujlfp73", 0, False, "test"): "2MtgJH28mZWNWU7VRU4ba6ciFbRRGYWZDt3",
         ("sh(wsh(sortedmulti(2,[73c5da0a/48h/1h/1h/0h/1h]tpubDFH9dgzveyD8yHQb8VrpG8FYAuwcLMHMje2CCcbBo1FpaGzYVtJeYYxcYgRqSTta5utUFts8nPPHs9C2bqoxrey5jia6Dwf9mpwrPq7YvcJ/{0,1}/*,[0be174ee/48h/1h/0h/1h]tpubDEsePyLPkbxbnj6XuKvWwdERHaKkikZxaGJ9sJqmM7okbZXgkNSFiGU6GX6qEes6kD8f9Z9FosYB9UEnBSgBEyEwwJhj4uUcFE1WE8VtKoh/{0,1}/*,[8d55ff0d/48h/1h/0h/1h]tpubDDxNVWk924RTT3vyGLHdSDoZ2JUVX7jUsPcwCQ9MrKHAtJrW5zECTF9rFHCvqu526E4PjHp61hBknts2c5aGexvX7hvCZ8TGPvQFdzxxy59/{0,1}/*)))#2ujlfp73", 0, True, "test"): "2NAjjwUQqwD9XRGLeQ6TitSUyMHUz3cLiWm",
 
-        # legacy multisig p2sh on testnet, not supported
-        ("sh(sortedmulti(2,[8d55ff0d/45h]tpubDANogJ2yfnizHwX7fSi5kUVzybyuPXDhgHB2TR9TUvkSLZFW73cRq4STKFDpx7qjJJiisyq82tbu4CeiYtmKEmT1xoCq9P8BPvXV31HUh6d/{0,1}/*,[0be174ee/45h]tpubDBkeVF2tDNT1Pz7L47iJeBB6RokU12LX6x4E6Ph8T89hmjQfB77q1AMyGwL8qpREVGq9sCJEbWwmnemwNTxnpxGn1di7BGy8jx9wEi5Vahu/{0,1}/*,[73c5da0a/45h]tpubDBKsGC1UqBDNvx9aivFmxZNgeZTUnmsCFGhWrqkLzucUCDePvbWWm3n8tAaAwMmxBG2ihdKCG9fzBdUnMxKx5PrkiqSZFi6Vkv6msUs9ddN/{0,1}/*))#p5t8sa8c", 0, False, "test"): Exception,
+        # legacy multisig p2sh on testnet, first payment and change addresses
+        ("sh(sortedmulti(2,[8d55ff0d/45h]tpubDANogJ2yfnizHwX7fSi5kUVzybyuPXDhgHB2TR9TUvkSLZFW73cRq4STKFDpx7qjJJiisyq82tbu4CeiYtmKEmT1xoCq9P8BPvXV31HUh6d/{0,1}/*,[0be174ee/45h]tpubDBkeVF2tDNT1Pz7L47iJeBB6RokU12LX6x4E6Ph8T89hmjQfB77q1AMyGwL8qpREVGq9sCJEbWwmnemwNTxnpxGn1di7BGy8jx9wEi5Vahu/{0,1}/*,[73c5da0a/45h]tpubDBKsGC1UqBDNvx9aivFmxZNgeZTUnmsCFGhWrqkLzucUCDePvbWWm3n8tAaAwMmxBG2ihdKCG9fzBdUnMxKx5PrkiqSZFi6Vkv6msUs9ddN/{0,1}/*))#p5t8sa8c", 0, False, "test"): "2NBXci43Y2fagvrFYTg3QmXj2LCPU2oaRFH",
+        ("sh(sortedmulti(2,[8d55ff0d/45h]tpubDANogJ2yfnizHwX7fSi5kUVzybyuPXDhgHB2TR9TUvkSLZFW73cRq4STKFDpx7qjJJiisyq82tbu4CeiYtmKEmT1xoCq9P8BPvXV31HUh6d/{0,1}/*,[0be174ee/45h]tpubDBkeVF2tDNT1Pz7L47iJeBB6RokU12LX6x4E6Ph8T89hmjQfB77q1AMyGwL8qpREVGq9sCJEbWwmnemwNTxnpxGn1di7BGy8jx9wEi5Vahu/{0,1}/*,[73c5da0a/45h]tpubDBKsGC1UqBDNvx9aivFmxZNgeZTUnmsCFGhWrqkLzucUCDePvbWWm3n8tAaAwMmxBG2ihdKCG9fzBdUnMxKx5PrkiqSZFi6Vkv6msUs9ddN/{0,1}/*))#p5t8sa8c", 0, True, "test"): "2MuWQTq7hUGiX1HpXuPRnf7YTM42H5zoEwj",
 
         # multisig taproot on testnet, not supported
         # TODO: find what a multisig-taproot descriptor would look like and add a test so we can fall into the last condition exception.