"signature not empty on failed checksig" in a 2-of-3 mutlsign with P2TR

[readygo67]

Based on btcd/txscript/sign_test.go TestRawTxInTapscriptSignature() case. I modify its script to 2/3 multisign as below.
it will report "signature not empty on failed checksig" in execution.
Any idea to fix this issue?

func TestRawTxInTapscriptSignatureScriptPath_2_3(t *testing.T) {
	privKey1, err := btcec.NewPrivateKey()
	require.NoError(t, err)
	privKey2, err := btcec.NewPrivateKey()
	require.NoError(t, err)
	privKey3, err := btcec.NewPrivateKey()
	require.NoError(t, err)

	pubKey1 := privKey1.PubKey()
	pubKey2 := privKey2.PubKey()
	pubKey3 := privKey3.PubKey()
	internalKey := pubKey1
       
       // 
	builder := NewScriptBuilder()
	builder.AddData(schnorr.SerializePubKey(pubKey1))
	builder.AddOp(OP_CHECKSIG)
	builder.AddData(schnorr.SerializePubKey(pubKey2))
	builder.AddOp(OP_CHECKSIGADD)
	builder.AddData(schnorr.SerializePubKey(pubKey3))
	builder.AddOp(OP_CHECKSIGADD)
	builder.AddInt64(2)
	builder.AddOp(OP_NUMEQUAL)
	pkScript, err := builder.Script()
	require.NoError(t, err)

	tapLeaf := NewBaseTapLeaf(pkScript)
	tapScriptTree := AssembleTaprootScriptTree(tapLeaf)

	ctrlBlock := tapScriptTree.LeafMerkleProofs[0].ToControlBlock(
		internalKey,
	)

	tapScriptRootHash := tapScriptTree.RootNode.TapHash()
	outputKey := ComputeTaprootOutputKey(
		internalKey, tapScriptRootHash[:],
	)
	p2trScript, err := PayToTaprootScript(outputKey)
	require.NoError(t, err)

	// We'll reuse this simple transaction for the tests below. It ends up
	// spending from a bip86 P2TR output.
	testTx := wire.NewMsgTx(2)
	testTx.AddTxIn(&wire.TxIn{
		PreviousOutPoint: wire.OutPoint{
			Index: 1,
		},
	})
	txOut := &wire.TxOut{
		Value: 1e8, PkScript: p2trScript,
	}
	testTx.AddTxOut(txOut)

	prevFetcher := NewCannedPrevOutputFetcher(
		txOut.PkScript, txOut.Value,
	)
	sigHashes := NewTxSigHashes(testTx, prevFetcher)

	sig1, err := RawTxInTapscriptSignature(
		testTx, sigHashes, 0, txOut.Value,
		txOut.PkScript, tapLeaf, SigHashDefault,
		privKey1,
	)
	require.NoError(t, err)

	sig2, err := RawTxInTapscriptSignature(
		testTx, sigHashes, 0, txOut.Value,
		txOut.PkScript, tapLeaf, SigHashDefault,
		privKey2,
	)
	require.NoError(t, err)

	sig3, err := RawTxInTapscriptSignature(
		testTx, sigHashes, 0, txOut.Value,
		txOut.PkScript, tapLeaf, SigHashDefault,
		privKey3,
	)
	require.NoError(t, err)

	// If this isn't sighash default, then a sighash should
	// be applied. Otherwise, it should be a normal sig.
	expectedLen := schnorr.SignatureSize
	require.Len(t, sig1, expectedLen)
	require.Len(t, sig2, expectedLen)
	require.Len(t, sig3, expectedLen)

	// Now that we have the sig, we'll make a valid witness
	// including the control block.
	ctrlBlockBytes, err := ctrlBlock.ToBytes()
	fmt.Printf("control block: %x\n", ctrlBlockBytes)
	require.NoError(t, err)
	txCopy := testTx.Copy()
	txCopy.TxIn[0].Witness = wire.TxWitness{
		sig1, sig2, sig3, pkScript, ctrlBlockBytes,
	}

	// Finally, ensure that the signature produced is valid.
	vm, err := NewEngine(
		txOut.PkScript, txCopy, 0, StandardVerifyFlags,
		nil, sigHashes, txOut.Value, prevFetcher,
	)
	require.NoError(t, err)
	require.NoError(t, vm.Execute())

}

[guggero]

A OP_CHECKSIGADD based multisig doesn't work the same way as the previous OP_CHECKMULTISIG opcode worked.
OP_CHECKMULTISIG does allow you to either specify 2 or 3 signatures (at least I think that's the case, could be wrong). But because you now have an explicit 2 OP_NUMEQUAL in your code, you can't specify 3 signatures. Because 2 is not equal to 3.
So you have to either use OP_GREATERTHANOREQUAL or use 3 as the number of required signatures (turning it into a 3-of-3 instead of a 2-of-3).

[readygo67]

	builder := NewScriptBuilder()
	builder.AddData(schnorr.SerializePubKey(pubKey1))
	builder.AddOp(OP_CHECKSIG)
	builder.AddData(schnorr.SerializePubKey(pubKey2))
	builder.AddOp(OP_CHECKSIGADD)
	builder.AddData(schnorr.SerializePubKey(pubKey3))
	builder.AddOp(OP_CHECKSIGADD)
	builder.AddInt64(3)
	builder.AddOp(OP_NUMEQUAL)
	pkScript, err := builder.Script()
	require.NoError(t, err)

and

	builder := NewScriptBuilder()
	builder.AddData(schnorr.SerializePubKey(pubKey1))
	builder.AddOp(OP_CHECKSIG)
	builder.AddData(schnorr.SerializePubKey(pubKey2))
	builder.AddOp(OP_CHECKSIGADD)
	builder.AddData(schnorr.SerializePubKey(pubKey3))
	builder.AddOp(OP_CHECKSIGADD)
	builder.AddInt64(3)
	builder.AddOp(OP_GREATERTHANOREQUAL)
	pkScript, err := builder.Script()
	require.NoError(t, err)

both meet same issue.

[guggero]

The stack is worked on from the back toward the front.
For me this works:

	builder := NewScriptBuilder()
	builder.AddData(schnorr.SerializePubKey(pubKey1))
	builder.AddOp(OP_CHECKSIG)
	builder.AddData(schnorr.SerializePubKey(pubKey2))
	builder.AddOp(OP_CHECKSIGADD)
	builder.AddData(schnorr.SerializePubKey(pubKey3))
	builder.AddOp(OP_CHECKSIGADD)
	builder.AddInt64(2)
	builder.AddOp(OP_GREATERTHANOREQUAL)

...

	txCopy.TxIn[0].Witness = wire.TxWitness{
		sig3, sig2, sig1, pkScript, ctrlBlockBytes,
	}

[readygo67]

@guggero thanks for clarification. I make 2 mistakes

1. OP_GREATERTHANOREQUAL rather than OP_NUMEQUAL
2. the stack is worked on from the back toward the front