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

Taproot script-spend-path with sighash ALL|ANYONECANPAY #2046

Closed
pxr64 opened this issue Feb 24, 2024 · 7 comments · Fixed by #2104
Closed

Taproot script-spend-path with sighash ALL|ANYONECANPAY #2046

pxr64 opened this issue Feb 24, 2024 · 7 comments · Fixed by #2104
Labels

Comments

@pxr64
Copy link

pxr64 commented Feb 24, 2024

Hello,

I need help with the following issue, i've created a taproot multisign spend script

function getMultiSignLock(pk1: string, pk2: string) {
  const opsMultiSig = [
    toXOnly(Buffer.from(pk1, "hex")), // lender
    opcodes.OP_CHECKSIG,
    toXOnly(Buffer.from(pk2, "hex")), // oracle
    opcodes.OP_CHECKSIGADD,
    opcodes.OP_2,
    opcodes.OP_NUMEQUAL,
  ];
  const multiSignLockScript = script.compile(opsMultiSig);
  return multiSignLockScript;
}

Everything works fine unless i want to add a ALL|ANYONECANPAY Sighash.

Am I missing someting?

@junderw
Copy link
Member

junderw commented Feb 24, 2024

How does it not work fine?

You need to explain the actual issue. What error are you getting? How are you constructing the transaction? Etc.

@kevinfaveri
Copy link

kevinfaveri commented Apr 16, 2024

Hmm I'm actually facing something similar @junderw.

You are probably aware of runes, right. In runes, you create a tapscript commitment like this:

const outputScript = bitcoin.script.compile([
    internalPubkey,
    bitcoin.opcodes.OP_CHECKSIG,
    bitcoin.opcodes.OP_FALSE,
    bitcoin.opcodes.OP_IF,
    runeCommitId,
    bitcoin.opcodes.OP_ENDIF
  ])

and then the p2tr is created like this:

 const scriptTree = {
    output: outputScript,
    redeemVersion: 192
  }

  const tapscriptCommitment = bitcoin.payments.p2tr({
    internalPubkey,
    scriptTree,
    redeem: scriptTree,
    network: bitcoinjs_network,
  })

payment.address, as you imagine, is where I pay to.
i then go on to setup an input like this:

tx.addInput({
    hash: paymentUtxo.txid,
    index: paymentUtxo.vout,
    witnessUtxo: {
      script: tapscriptCommitment.output!,
      value: paymentUtxo.value
    },
    tapLeafScript: [
      {
        leafVersion: tapscriptCommitment.redeemVersion!,
        script: tapscriptCommitment.redeem?.output!,
        controlBlock:
          tapscriptCommitment.witness?.[tapscriptCommitment.witness.length - 1]!
      }
    ],
    sequence: 0xfffffffe
  })

Then, I sign with the private key / public key that was used to create the p2tr commitment above, using a custom finalizer when signing this specific input above:

export const customFinalizer = (tapscriptCommitment: bitcoin.Payment) => {
  return (_inputIndex: number, input: any) => {
    const witness = [input.tapScriptSig[0].signature]
      .concat(tapscriptCommitment.redeem!.output)
      .concat(
        tapscriptCommitment.witness![tapscriptCommitment.witness!.length - 1]
      )

    return {
      finalScriptWitness: witnessStackToScriptWitness(witness)
    }
  }
}

And it works perfectly fine: https://mempool.space/testnet/tx/9549245bdd67cc80e1b5a1940e8ec88356689096cb7b97b53ee01e003baecaf0

However, the real problem happens if I try specifying any other sigHash, like SIGHASH_ALL, for example, to set in stone the changes that could be made to the transaction. However, the signing + finalization does not generate a healthy signature, as RPC throws with when I try to broadcast it:

{"result":null,"error":{"code":-26,"message":"mandatory-script-verify-flag-failed (Invalid Schnorr signature)"},"id":null}

To be clear, this is the only input in the transaction that gets broadcasted.

(The result is the same using the customFinalizer or not. With sigHash, works. Without sigHash, doesn't work. Altho without the custom finalizer it does not set the transaction correctly, finalized, as taproot, so don't take chain benefits of using it in this case, as can be seen on the example I gave above, which didnt use the customFinalizer).

I thought it could be related to #2004 but it seems related to key spend path only? Not script path, which would be this case?

Would love you if you could take a minute to educate me what part of the chess piece I'm missing.

And my fear would be that someone (since I'm not specifying sigHash in the transaction) could snipe this transaction the mempool to change outputs / transaction format, which shouldn't be possible, I want to explicitly forbid.

@kevinfaveri
Copy link

2 - Also can you please explain what not specifying any sigHash in the input means when using bitcoin.js? It uses DEFAULT? And what default translates to? Translates to using ALL in the background? Or NONE? Or just relying the transaction without adding anything?

@junderw
Copy link
Member

junderw commented Apr 16, 2024

Can you add a test to the taproot integrations that crashes, and make a PR? I'll look at that and make any fixes necessary once I understand the issue and have a test case I can run locally near-instantly.

@kevinfaveri
Copy link

Ser

@jasonandjay jasonandjay added the bug label May 1, 2024
@baryon
Copy link
Contributor

baryon commented Jun 5, 2024

Let me answer my own question. I made a small code modification and now it can be accepted by the Bitcoin node. I'm not entirely sure if this aligns with the original design. Please review it. If this is indeed a bug, I can submit a patch request or let the author fix it. @junderw

I modified the following code:

Transaction.SIGHASH_DEFAULT,

Changed Transaction.SIGHASH_DEFAULT to sighashType, and now it's working fine.


I seem to be encountering the same issue. I'm using bitcoin.Transaction.SIGHASH_SINGLE | bitcoin.Transaction.SIGHASH_ANYONECANPAY to construct UTXO inputs and signing them with a private key. The transaction passes the signature check and can be fully constructed.

Here's how the input is structured. If necessary, I can provide a minimal reproducible code snippet.

      const input = {
        hash: noteUtxo.txId,
        index: noteUtxo.outputIndex,
        sequence: MAX_SEQUENCE,
        witnessUtxo: {
          script: p2note.noteP2TR.output!,
          value: noteUtxo.satoshis,
        },
        tapLeafScript: [tapLeafScript], 
        // sighashType: bitcoin.Transaction.SIGHASH_DEFAULT, //Here Error
        sighashType:
          bitcoin.Transaction.SIGHASH_SINGLE |
          bitcoin.Transaction.SIGHASH_ANYONECANPAY,
      };
      psbt.addInput(input);

Note that if you set sighashType to bitcoin.Transaction.SIGHASH_DEFAULT, you'll encounter an error at psbt.addInput(input);: "Cannot add duplicate data to sighashType".

psbt.signInput(0, privateKey, [bitcoin.Transaction.SIGHASH_SINGLE | bitcoin.Transaction.SIGHASH_ANYONECANPAY]);
psbt.validateSignaturesOfInput(0, schnorrValidator);

However, when broadcasting to testnet4, I get an error:

{
"code": 16,
"message": "mandatory-script-verify-flag-failed (Invalid Schnorr signature)"
}

Is there a quick fix for this?

@junderw
Copy link
Member

junderw commented Jun 9, 2024

Released as 6.1.6

Please check if the issue is resolved, but it seems like this is the cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants