Skip to content

[WIP] support new multisig template in wallet for Solver, signing, and signature combining#13449

Closed
instagibbs wants to merge 1 commit into
bitcoin:masterfrom
instagibbs:largemultisig
Closed

[WIP] support new multisig template in wallet for Solver, signing, and signature combining#13449
instagibbs wants to merge 1 commit into
bitcoin:masterfrom
instagibbs:largemultisig

Conversation

@instagibbs

@instagibbs instagibbs commented Jun 12, 2018

Copy link
Copy Markdown
Member

…ature combining

Currently the wallet can only sign/understand up to n-of-16 checkmultisig templates. Some efforts are being taken to expand this to n-of-20: #13348 . Rather than try to increase the number of possible signatures marginally this template is based on https://bitcoin.stackexchange.com/a/51653 which allows signing/combining of up to n-of-67, constrained by the non-push opcode limit.

+/* Script of the form:
+ *
+ * <pubkey1> OP_CHECKSIG
+ * OP_SWAP <pubkey2> OP_CHECKSIG OP_ADD
+ * OP_SWAP <pubkey3> OP_CHECKSIG OP_ADD
+ * ...
+ * OP_SWAP <pubkey67> OP_CHECKSIG OP_ADD
+ * <m> OP_NUMEQUAL
+ */

Wallet code currently disallows import of any redeem/witnessscript beyond 520 bytes(n-of-15), so no safety is lost due to the possible mixture of segwit/non-segwit scripts.

Looking for concept ACKs before I go write tests and attempt code de-duplication, but manual testing worked.

@laanwj laanwj changed the title [WIP] support new multisig template in wallet for Solver, signing, and sign… [WIP] support new multisig template in wallet for Solver, signing, and signature combining Jun 12, 2018
@laanwj laanwj added the Wallet label Jun 12, 2018
@sipa

sipa commented Jun 12, 2018

Copy link
Copy Markdown
Member

Concept ACK, though a significant part of the sign/combine logic is being changed in #13425.

One possibility is only supporting these large multisig using PSBT, and not through the existing partial raw transactions interface. That should be very simple to write, but more restrictive.

Otherwise, the code for extracting existing signatures from a partial raw transaction into a key/sig map will need to be combined with this.

@DrahtBot

DrahtBot commented Jun 14, 2018

Copy link
Copy Markdown
Contributor
Note to reviewers: This pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot

DrahtBot commented Jul 7, 2018

Copy link
Copy Markdown
Contributor
Needs rebase

@instagibbs

Copy link
Copy Markdown
Member Author

I'll rebase once #13557 is merged, making it PSBT-only.

@luke-jr

luke-jr commented Jul 30, 2018

Copy link
Copy Markdown
Member

Is there a reason for OP_NUMEQUAL rather than OP_EQUAL per BIP 19?

@instagibbs

Copy link
Copy Markdown
Member Author

@luke-jr had no idea there was a BIP. I used a stack overflow answer. Will investigate.

@sipa

sipa commented Aug 4, 2018

Copy link
Copy Markdown
Member

@luke-jr Interesting! Reading that BIP, I believe the n-of-n template suggested there is incorrect. In case all signatures are valid, it will leave the stack empty, which is interpreted as failure.

@instagibbs

Copy link
Copy Markdown
Member Author

Closing in favor of Future Work with descriptor language stuff

@instagibbs instagibbs closed this Aug 31, 2018
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants