Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Legacy P2PKH Signing #567

Merged
merged 9 commits into from
Jul 19, 2024
Merged

Add Legacy P2PKH Signing #567

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4cd5522
Add Legacy P2PKH Signing
3rdIteration Jul 7, 2024
300a44f
Remove not-implemented error
3rdIteration Jul 7, 2024
451639f
Add Legacy P2PKH Signing
3rdIteration Jul 7, 2024
b88d0cd
Add Legacy tests to embit_utils CI test
3rdIteration Jul 8, 2024
284e894
Merge branch 'add-p2pkh-signing' of https://github.com/3rdIteration/s…
3rdIteration Jul 8, 2024
05f5f0e
Fix Multisix Xpub Export not working correctly for P2PKH Legacy Wallets
3rdIteration Jul 10, 2024
4f6456b
Simplify Script Type Descriptions
3rdIteration Jul 12, 2024
909d7f5
Remove duplicate UTXO addition
3rdIteration Jul 19, 2024
4456ce4
Merge branch 'dev' into add-p2pkh-signing
newtonick Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions src/seedsigner/helpers/embit_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,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:
return f"m/84'/{network_path}/0'"
if script_type == SettingsConstants.LEGACY_P2PKH:
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:
return f"m/48'/{network_path}/0'/2'"
if script_type == SettingsConstants.LEGACY_P2PKH:
Copy link

@jdlcdl jdlcdl Jul 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??? Does this belong?

Wasn't sure, but now I see that "legacy" is not just meant as p2pkh.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The native segwit block got moved down, basically just trying to be mostly consistent in terms of the code logic reflecting the BIP numbers (So older types with smaller BIP numbers start fist and then iterate through the BIPs in order of their release, so legacy first, Taproot last)

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:
Expand All @@ -67,14 +71,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:
return embit.script.p2wpkh(pubkey).address(network=NETWORKS[embit_network])
if script_type == SettingsConstants.LEGACY_P2PKH:
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:
return embit.script.p2pkh(pubkey).address(network=NETWORKS[embit_network])
elif script_type == SettingsConstants.NATIVE_SEGWIT:
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])
Expand Down Expand Up @@ -129,8 +133,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": {
Expand Down
5 changes: 5 additions & 0 deletions src/seedsigner/models/encode_qr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,10 @@ def derivation_to_keypath(path: str) -> list:
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:
Expand All @@ -380,6 +384,7 @@ def derivation_to_keypath(path: str) -> list:
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)

Expand Down
21 changes: 19 additions & 2 deletions src/seedsigner/models/psbt_parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,15 @@ def _parse_inputs(self):
if self.policy != inp_policy:
raise RuntimeError("Mixed inputs in the transaction")

if inp.non_witness_utxo:
self.input_amount += inp.utxo.value
inp_policy = PSBTParser._get_policy(inp, 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")

3rdIteration marked this conversation as resolved.
Show resolved Hide resolved

def _parse_outputs(self):
self.spend_amount = 0
Expand All @@ -124,6 +133,10 @@ def _parse_outputs(self):
# 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)
Expand All @@ -144,12 +157,15 @@ def _parse_outputs(self):
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)
if self.policy["type"] == "p2pkh" and my_pubkey is not None:
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

Expand Down Expand Up @@ -258,6 +274,7 @@ def _get_policy(scope, scriptpubkey, xpubs):
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)

Expand Down
5 changes: 3 additions & 2 deletions src/seedsigner/models/settings_definition.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,15 @@ def map_network_to_embit(cls, network) -> str:
(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"),
]
Expand Down
9 changes: 5 additions & 4 deletions src/seedsigner/views/seed_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1544,6 +1544,11 @@ def __init__(self, address: str, script_type: str, network: str):


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)
Expand All @@ -1567,10 +1572,6 @@ def run(self):
# 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,
Expand Down
25 changes: 25 additions & 0 deletions tests/test_embit_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'",
Expand All @@ -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,
Expand Down Expand Up @@ -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",

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down