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.

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

[New Feature] Support bip45 nonsegwit legacy multisig p2sh #540

Merged
merged 11 commits into from
Jun 24, 2024

Conversation

kdmukai
Copy link
Contributor

@kdmukai kdmukai commented Apr 1, 2024

Description

Adds p2sh nonsegwit legacy multisig support.

The intention is to at least support bip45 (m/45') p2sh multisig, which is the format used by Unchained. There may be some assumptions baked in here that might not fully support all arbitrary p2sh scripts.

  • Lightly mods embit_utils and psbt_parser to support p2sh.
  • All multisig use cases supported: sign p2sh PSBTs, Address Explorer via descriptor, Verify Address via descriptor.

Additional tests:

  • Enable and expand previously disabled p2sh test in test_embit_utils
  • Explicit new test for p2sh in test_psbt_parser.
  • Add p2sh Address Explorer flow test
  • Add p2sh multisig Verify Address flow test (first flow test for any kind of Verify Address scenario)
    • Misc View edits to support flow-based tests.

Misc:

  • Note that test_psbt_parser.py was using tab indents which have now been changed to spaces in this PR (which is why the otherwise unaltered p2tr test shows up in this diff).

To aid testing, here is the p2sh multisig descriptor used in the psbt_parser test:

legacy_multisig_p2sh_test_descriptor
sh(sortedmulti(2,[0f889044/45h]tpubD8NkS3Gngj7L4FJRYrwojKhsx2seBhrNrXVdvqaUyvtVe1YDCVcziZVa9g3KouXz7FN5CkGBkoC16nmNu2HcG9ubTdtCbSW8DEXSMHmmu62/<0;1>/*,[03cd0a2b/45h]tpubD8HkLLgkdJkVitn1i9CN4HpFKJdom48iKm9PyiXYz5hivn1cGz6H3VeS6ncmCEgamvzQA2Qofu2YSTwWzvuaYWbJDEnvTUtj5R96vACdV6L/<0;1>/*,[769f695c/45h]tpubD98hRDKvtATTM8hy5Vvt5ZrvDXwJvrUZm1p1mTKDmd7FqUHY9Wj2k4X1CvxjjtTf3JoChWqYbnWjfkRJ65GQnpVJKbbMfjnGzCwoBUXafyM/<0;1>/*))#uardwtq4

The descriptor from the test can also be imported into Sparrow for more interactive testing / sanity checking:

  • Save the descriptor to a txt file
  • Launch Sparrow w/-n regtest
  • File -> Import. Select Bitcoin Core Output Descriptor and select the txt file

Additional resources:


This pull request is categorized as a:

  • New feature

Checklist

  • I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

  • Yes

I have tested this PR on the following platforms/os:

@newtonick
Copy link
Collaborator

newtonick commented Jun 24, 2024

Did some testing on this. One issue I ran into is there does not appear to be a way to export a p2sh multisig xpub. At least I could not get it to work in Sparrow until I added:

            elif origin.components[0].index == 45: # P2SH Multisig
                ur_outputs.append(Output([SCRIPT_EXPRESSION_TAG_MAP[400]],self.ur_hdkey))

to src/seedsigner/models/encode_qr.py on line 514.

@newtonick
Copy link
Collaborator

The additions here LGTM. Tested with the only issue being xpub export. This can be added/corrected in a separate PR.

@newtonick newtonick merged commit c94725e into SeedSigner:dev Jun 24, 2024
1 check passed
@kdmukai kdmukai deleted the nonsegwit_p2sh_psbts branch September 2, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged Not Yet Released
Development

Successfully merging this pull request may close these issues.

2 participants