Merge pull request #567 from 3rdIteration/add-p2pkh-signing

Add Legacy P2PKH Signing

src/seedsigner/helpers/embit_utils.py

@@ -31,20 +31,24 @@ def get_standard_derivation_path(network: str = SettingsConstants.MAINNET, walle
         raise Exception("Unexpected network")
 
     if wallet_type == SettingsConstants.SINGLE_SIG:
-        if script_type == SettingsConstants.NATIVE_SEGWIT:
+        if script_type == SettingsConstants.LEGACY_P2PKH:
-            return f"m/84'/{network_path}/0'"
+            return f"m/44'/{network_path}/0'"
         elif script_type == SettingsConstants.NESTED_SEGWIT:
             return f"m/49'/{network_path}/0'"
+        elif script_type == SettingsConstants.NATIVE_SEGWIT:
+            return f"m/84'/{network_path}/0'"
         elif script_type == SettingsConstants.TAPROOT:
             return f"m/86'/{network_path}/0'"
         else:
             raise Exception("Unexpected script type")
 
     elif wallet_type == SettingsConstants.MULTISIG:
-        if script_type == SettingsConstants.NATIVE_SEGWIT:
+        if script_type == SettingsConstants.LEGACY_P2PKH:
-            return f"m/48'/{network_path}/0'/2'"
+            return f"m/45'" #BIP45
         elif script_type == SettingsConstants.NESTED_SEGWIT:
             return f"m/48'/{network_path}/0'/1'"
+        elif script_type == SettingsConstants.NATIVE_SEGWIT:
+            return f"m/48'/{network_path}/0'/2'"
         elif script_type == SettingsConstants.TAPROOT:
             raise Exception("Taproot multisig/musig not yet supported")
         else:
@@ -68,14 +72,14 @@ def get_single_sig_address(xpub: HDKey, script_type: str = SettingsConstants.NAT
     else:
         pubkey = xpub.derive([0,index]).key
 
-    if script_type == SettingsConstants.NATIVE_SEGWIT:
+    if script_type == SettingsConstants.LEGACY_P2PKH:
-        return embit.script.p2wpkh(pubkey).address(network=NETWORKS[embit_network])
+        return embit.script.p2pkh(pubkey).address(network=NETWORKS[embit_network])
 
     elif script_type == SettingsConstants.NESTED_SEGWIT:
         return embit.script.p2sh(embit.script.p2wpkh(pubkey)).address(network=NETWORKS[embit_network])
 
-    elif script_type == SettingsConstants.LEGACY_P2PKH:
+    elif script_type == SettingsConstants.NATIVE_SEGWIT:
-        return embit.script.p2pkh(pubkey).address(network=NETWORKS[embit_network])
+        return embit.script.p2wpkh(pubkey).address(network=NETWORKS[embit_network])
 
     elif script_type == SettingsConstants.TAPROOT:
         return embit.script.p2tr(pubkey).address(network=NETWORKS[embit_network])
@@ -130,8 +134,9 @@ def parse_derivation_path(derivation_path: str) -> dict:
 
     lookups = {
         "script_types": {
-            "84h": SettingsConstants.NATIVE_SEGWIT,
+            "44h": SettingsConstants.LEGACY_P2PKH,
             "49h": SettingsConstants.NESTED_SEGWIT,
+            "84h": SettingsConstants.NATIVE_SEGWIT,
             "86h": SettingsConstants.TAPROOT,
         },
         "networks": {

src/seedsigner/models/encode_qr.py

@@ -365,6 +365,10 @@ class UrXpubQrEncoder(BaseFountainQrEncoder, BaseXpubQrEncoder):
                         ur_outputs.append(Output([SCRIPT_EXPRESSION_TAG_MAP[400], SCRIPT_EXPRESSION_TAG_MAP[401]],self.ur_hdkey))
             elif origin.components[0].index == 86: # P2TR
                 ur_outputs.append(Output([SCRIPT_EXPRESSION_TAG_MAP[409]],self.ur_hdkey))
+            elif origin.components[0].index == 44: # P2PKH
+                ur_outputs.append(Output([SCRIPT_EXPRESSION_TAG_MAP[403]],self.ur_hdkey))
+            elif origin.components[0].index == 45: # P2SH 
+                ur_outputs.append(Output([SCRIPT_EXPRESSION_TAG_MAP[400]],self.ur_hdkey))
         
         # If empty, add all script types
         if len(ur_outputs) == 0:
@@ -373,6 +377,7 @@ class UrXpubQrEncoder(BaseFountainQrEncoder, BaseXpubQrEncoder):
             ur_outputs.append(Output([SCRIPT_EXPRESSION_TAG_MAP[401]],self.ur_hdkey))
             ur_outputs.append(Output([SCRIPT_EXPRESSION_TAG_MAP[400], SCRIPT_EXPRESSION_TAG_MAP[401]],self.ur_hdkey))
             ur_outputs.append(Output([SCRIPT_EXPRESSION_TAG_MAP[403]],self.ur_hdkey))
+            ur_outputs.append(Output([SCRIPT_EXPRESSION_TAG_MAP[400]],self.ur_hdkey))
         
         ur_account = Account(self.root.my_fingerprint, ur_outputs)
 

src/seedsigner/models/psbt_parser.py

@@ -108,7 +108,6 @@ class PSBTParser():
                 if self.policy != inp_policy:
                     raise RuntimeError("Mixed inputs in the transaction")
 
-
     def _parse_outputs(self):
         self.spend_amount = 0
         self.change_amount = 0
@@ -132,6 +131,10 @@ class PSBTParser():
                 # empty script by default
                 sc = script.Script(b"")
 
+                # if older multisig, just use existing script
+                if self.policy["type"] == "p2sh":
+                    sc = script.p2sh(out.redeem_script)
+
                 # multisig, we know witness script
                 if self.policy["type"] == "p2wsh":
                     sc = script.p2wsh(out.witness_script)
@@ -152,12 +155,15 @@ class PSBTParser():
                         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:
+                    if self.policy["type"] == "p2pkh" and my_pubkey is not None:
-                        sc = script.p2wpkh(my_pubkey)
+                        sc = script.p2pkh(my_pubkey)
 
                     elif self.policy["type"] == "p2sh-p2wpkh" and my_pubkey is not None:
                         sc = script.p2sh(script.p2wpkh(my_pubkey))
 
+                    elif self.policy["type"] == "p2wpkh" and my_pubkey is not None:
+                        sc = script.p2wpkh(my_pubkey)
+
                     if sc.data == self.psbt.tx.vout[i].script_pubkey.data:
                         is_change = True
 

src/seedsigner/models/settings_definition.py

@@ -111,14 +111,15 @@ class SettingsConstants:
         (MULTISIG, "Multisig"),
     ]
 
-    LEGACY_P2PKH = "leg"  # Intentionally excluded from ALL_SCRIPT_TYPES
+    LEGACY_P2PKH = "leg"
     NATIVE_SEGWIT = "nat"
     NESTED_SEGWIT = "nes"
     TAPROOT = "tr"
     CUSTOM_DERIVATION = "cus"
     ALL_SCRIPT_TYPES = [
         (NATIVE_SEGWIT, "Native Segwit"),
-        (NESTED_SEGWIT, "Nested Segwit (legacy)"),
+        (NESTED_SEGWIT, "Nested Segwit"),
+        (LEGACY_P2PKH, "Legacy"),
         (TAPROOT, "Taproot"),
         (CUSTOM_DERIVATION, "Custom Derivation"),
     ]

src/seedsigner/views/seed_views.py

@@ -1635,6 +1635,11 @@ class AddressVerificationStartView(View):
 
 
     def run(self):
+        if self.controller.unverified_address["script_type"] == SettingsConstants.LEGACY_P2PKH:
+            # Legacy P2PKH addresses are always singlesig
+            sig_type = SettingsConstants.SINGLE_SIG
+            destination = Destination(SeedSelectSeedView, skip_current_view=True)
+
         if self.controller.unverified_address["script_type"] == SettingsConstants.NESTED_SEGWIT:
             # No way to differentiate single sig from multisig
             return Destination(AddressVerificationSigTypeView, skip_current_view=True)
@@ -1658,10 +1663,6 @@ class AddressVerificationStartView(View):
             # TODO: add Taproot support
             return Destination(NotYetImplementedView)
 
-        elif self.controller.unverified_address["script_type"] == SettingsConstants.LEGACY_P2PKH:
-            # TODO: detect single sig vs multisig or have to prompt?
-            return Destination(NotYetImplementedView)
-
         derivation_path = embit_utils.get_standard_derivation_path(
             network=self.controller.unverified_address["network"],
             wallet_type=sig_type,

tests/test_embit_utils.py

@@ -27,6 +27,10 @@ def test_get_standard_derivation_path():
         (SC.TESTNET, SC.SINGLE_SIG, SC.TAPROOT): "m/86'/1'/0'",
         (SC.REGTEST, SC.SINGLE_SIG, SC.TAPROOT): "m/86'/1'/0'",
 
+        (SC.MAINNET, SC.SINGLE_SIG, SC.LEGACY_P2PKH): "m/44'/0'/0'",
+        (SC.TESTNET, SC.SINGLE_SIG, SC.LEGACY_P2PKH): "m/44'/1'/0'",
+        (SC.REGTEST, SC.SINGLE_SIG, SC.LEGACY_P2PKH): "m/44'/1'/0'",
+
 
         # multi sig
         (SC.MAINNET, SC.MULTISIG, SC.NATIVE_SEGWIT): "m/48'/0'/0'/2'",
@@ -41,6 +45,8 @@ def test_get_standard_derivation_path():
         (SC.TESTNET, SC.MULTISIG, SC.TAPROOT): Exception,
         (SC.REGTEST, SC.MULTISIG, SC.TAPROOT): Exception,
 
+        (SC.MAINNET, SC.MULTISIG, SC.LEGACY_P2PKH): "m/45'",
+
         # intentionally fall into exceptions
         (SC.MAINNET, SC.SINGLE_SIG, 'invalid'): Exception,
         (SC.MAINNET, SC.MULTISIG, 'invalid'): Exception,
@@ -155,6 +161,10 @@ def test_get_xpub():
         (vector_seeds[4], "m/49'/1'/0'", "test"): 
              bip32.HDKey.from_string("upub5EFU65HtV5TeiSHmZZm7FUffBGy8UKeqp7vw43jYbvZPpoVsgU93oac7Wk3u6moKegAEWtGNF8DehrnHtv21XXEMYRUocHqguyjknFHYfgY").to_base58(version=b'\x04\x35\x87\xcf'),
 
+        # https://github.com/satoshilabs/slips/blob/master/slip-0132.md#bitcoin-test-vectors
+        (vector_seeds[4], "m/44'/0'/0'", "main"): 
+             bip32.HDKey.from_string("xpub6BosfCnifzxcFwrSzQiqu2DBVTshkCXacvNsWGYJVVhhawA7d4R5WSWGFNbi8Aw6ZRc1brxMyWMzG3DSSSSoekkudhUd9yLb6qx39T9nMdj").to_base58(version=b'\x04\x88\xb2\x1e'),
+
         # https://github.com/bitcoin/bips/blob/master/bip-0086.mediawiki#test-vectors
         (vector_seeds[4], "m/86'/0'/0'", "main"): "xpub6BgBgsespWvERF3LHQu6CnqdvfEvtMcQjYrcRzx53QJjSxarj2afYWcLteoGVky7D3UKDP9QyrLprQ3VCECoY49yfdDEHGCtMMj92pReUsQ",
 
@@ -234,6 +244,14 @@ def test_get_single_sig_address():
         (HDKey.from_string("tpubDC5FSnBiZDMmhiuCmWAYsLwgLYrrT9rAqvTySfuCCrgsWz8wxMXUS9Tb9iVMvcRbvFcAHGkMD5Kx8koh4GquNGNTfohfk7pgjhaPCdXpoba"), "leg", 0, True, "test"):
             "mi8nhzZgGZQthq6DQHbru9crMDerUdTKva",
 
+        # https://github.com/satoshilabs/slips/blob/master/slip-0132.md#bitcoin-test-vectors (first payment address p2pkh on mainnet)
+        (HDKey.from_string("xpub6BosfCnifzxcFwrSzQiqu2DBVTshkCXacvNsWGYJVVhhawA7d4R5WSWGFNbi8Aw6ZRc1brxMyWMzG3DSSSSoekkudhUd9yLb6qx39T9nMdj"), "leg", 0, False, "main"):
+            "1LqBGSKuX5yYUonjxT5qGfpUsXKYYWeabA",
+
+        # 3rdIteration: derived via electrum m/44'/0'/0 (first change address p2pkh on mainnet)
+        (HDKey.from_string("xpub6BosfCnifzxcFwrSzQiqu2DBVTshkCXacvNsWGYJVVhhawA7d4R5WSWGFNbi8Aw6ZRc1brxMyWMzG3DSSSSoekkudhUd9yLb6qx39T9nMdj"), "leg", 0, True, "main"):
+            "1J3J6EvPrv8q6AC3VCjWV45Uf3nssNMRtH",
+
         # jdlcdl: nonsense script_type falls off end of function returning None.  TODO: Would it be preferred to "else: raise ValueError"?
         (HDKey.from_string("tpubDC5FSnBiZDMmhiuCmWAYsLwgLYrrT9rAqvTySfuCCrgsWz8wxMXUS9Tb9iVMvcRbvFcAHGkMD5Kx8koh4GquNGNTfohfk7pgjhaPCdXpoba"), "NONSENSE", 0, True, "test"):
             "None",
@@ -362,6 +380,13 @@ def test_parse_derivation_path():
         (SC.TESTNET, SC.TAPROOT, True): "m/86'/1'/0'/1/5",
         (SC.REGTEST, SC.TAPROOT, True): "m/86'/1'/0'/1/5",
 
+        (SC.MAINNET, SC.LEGACY_P2PKH, False): "m/44'/0'/0'/0/5",
+        (SC.TESTNET, SC.LEGACY_P2PKH, False): "m/44'/1'/0'/0/5",
+        (SC.REGTEST, SC.LEGACY_P2PKH, False): "m/44'/1'/0'/0/5",
+        (SC.MAINNET, SC.LEGACY_P2PKH, True): "m/44'/0'/0'/1/5",
+        (SC.TESTNET, SC.LEGACY_P2PKH, True): "m/44'/1'/0'/1/5",
+        (SC.REGTEST, SC.LEGACY_P2PKH, True): "m/44'/1'/0'/1/5",
+
         # Try a typical custom derivation path (Unchained vault keys)
         (SC.MAINNET, SC.CUSTOM_DERIVATION, False): "m/45'/0'/0'/0/5",
         (SC.TESTNET, SC.CUSTOM_DERIVATION, False): "m/45'/1'/0'/0/5",