bitcoinjs / bitcoinjs-lib

A javascript Bitcoin library for node.js and browsers.
MIT License
5.68k stars 2.1k forks source link

Taproot M out of N script broadcasting tx error: operation not valid with the current stack size #1961

Closed fess-v closed 1 year ago

fess-v commented 1 year ago

Hello! Trying to make a 1 out of 2 or 2 out of 3 multi-sig using Taproot scripts. <pk1> OP_CHECKSIG <pk2> OP_CHECKSIGADD <pk3> OP_CHECKSIGADD 2 OP_EQUAL - 2 out of 3 <pk1> OP_CHECKSIG <pk2> OP_CHECKSIGADD 1 OP_EQUAL 1 out of 2 After signing with the required amount of owners, trying to broadcast this transaction and getting: non-mandatory-script-verify-flag (Operation not valid with the current stack size)

Is it possible to add somehow an empty witness (or a dummy input) so such scripts work fine and I don't get the stack size error? Also tried to find an example in this repo of a 2 out of 3 taproot multi-sig, but only a 3 out of 3 exists.

P.S. works like that with any taproot multisig with M out of N owners, where M is less than N. All N out of N scripts work as expected.

fess-v commented 1 year ago

Tried after signing to add a dummy signature object without a real signature (the same one which was from the signed owner), so after finalizing owner signatures will be sorted and valid ones pass OP_CHECKSIG and OP_CHECKSIGADD

psbt.data.inputs.forEach(i => {
        i.tapScriptSig?.push({
          signature: i.tapScriptSig[0].signature,
          pubkey: Buffer.from(otherPublicKey),
          leafHash: i.tapScriptSig[0].leafHash
        })
})

psbt.finalizeAllInputs();

But this gives: non-mandatory-script-verify-flag (Invalid Schnorr signature), so the Shnorr signature becomes invalid What is the best way to add a dummy parameter in the correct order to the stack? so it doesn't break other signatures verification too

fess-v commented 1 year ago

export const toXOnly = (pubKey: Buffer) =>
  pubKey.length === 32 ? pubKey : pubKey.slice(1, 33);

psbt.data.inputs.forEach(i => {
        i.tapScriptSig?.push({
          signature: Buffer.from([]),
          pubkey: toXOnly(Buffer.from(otherPublicKey, 'hex'),
          leafHash: i.tapScriptSig[0].leafHash
        })
})

psbt.finalizeAllInputs();

Just solved this problem with the code above, transaction worked fine So basically need to send an empty signature for each owner who didn't sign that input

murchandamus commented 1 year ago

Yeah, exactly. OP_CHECKSIG and OP_CHECKSIGADD will consume one item from the stack, therefore the witness stack needs to have an equal count of signatures and dummies as public keys that get tested.

motorina0 commented 1 year ago

@fess-v can you please add a test here so others can find the example in the future.

fess-v commented 1 year ago

Sure, will do that @motorina0

motorina0 commented 1 year ago

My understanding was that the best way to do N-of-M with taproot is to have a leaf for each N-of-N combination. So instead of having one leaf with: <pk1> OP_CHECKSIG <pk2> OP_CHECKSIGADD <pk3> OP_CHECKSIGADD 2 OP_EQUAL there could be 3 leafs with: <pk1> OP_CHECKSIG <pk2> OP_CHECKSIGADD 2 OP_EQUAL <pk1> OP_CHECKSIG <pk3> OP_CHECKSIGADD 2 OP_EQUAL <pk2> OP_CHECKSIG <pk3> OP_CHECKSIGADD 2 OP_EQUAL

The advantages are that:

@murchandamus is that correct?

fess-v commented 1 year ago

@motorina0 That's true, but it's not appropriate in some cases, especially for our product it is not possible to make the flow like that

murchandamus commented 1 year ago

@motorina0: That’s true if you can use MuSig2 in the keypath with the two most likely keys, and only need the two backup pairs as leaves. Then they can both be at depth 1 and it’s cheaper than a 2-of-3 OP_CHECKSIG(ADD) construction. If you need all three pairs as leaves, only one can be at depth 1, the other two would be at depth 2. The other two would then be more expensive.

I wrote more about that here: https://murch.one/posts/2-of-3-using-p2tr/