From 99e4e00345017a70eadc4e1d06353c56b23bb15c Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 15 Mar 2022 17:49:45 -0700 Subject: [PATCH] txscript: add more detailed taproot errors --- txscript/engine.go | 2 +- txscript/error.go | 51 +++++++++++++++++++++++++++++++++++++++++ txscript/error_test.go | 10 ++++++++ txscript/opcode.go | 10 ++++---- txscript/sigvalidate.go | 7 +++--- txscript/standard.go | 3 +-- txscript/taproot.go | 23 +++++++++++++------ 7 files changed, 87 insertions(+), 19 deletions(-) diff --git a/txscript/engine.go b/txscript/engine.go index 3c8f207636..7dfd092eae 100644 --- a/txscript/engine.go +++ b/txscript/engine.go @@ -178,7 +178,7 @@ func (t *taprootExecutionCtx) tallysigOp() error { t.sigOpsBudget -= sigOpsDelta if t.sigOpsBudget < 0 { - return fmt.Errorf("max sig ops exceeded") + return scriptError(ErrTaprootMaxSigOps, "") } return nil diff --git a/txscript/error.go b/txscript/error.go index 9170895425..072778a268 100644 --- a/txscript/error.go +++ b/txscript/error.go @@ -367,6 +367,47 @@ const ( // bytes. ErrDiscourageUpgradeablePubKeyType + // ErrTaprootSigInvalid is returned when an invalid taproot key spend + // signature is encountered. + ErrTaprootSigInvalid + + // ErrTaprootMerkleProofInvalid is returned when the revealed script + // merkle proof for a taproot spend is found to be invalid. + ErrTaprootMerkleProofInvalid + + // ErrTaprootOutputKeyParityMismatch is returned when the control block + // proof is valid, but the parity of the y-coordinate of the derived + // key doesn't match the value encoded in the control block. + ErrTaprootOutputKeyParityMismatch + + // ErrControlBlockTooSmall is returned when a parsed control block is + // less than 33 bytes. + ErrControlBlockTooSmall + + // ErrControlBlockTooLarge is returned when the control block is larger + // than the largest possible proof for a merkle script tree. + ErrControlBlockTooLarge + + // ErrControlBlockInvalidLength is returned when the control block, + // without the public key isn't a multiple of 32. + ErrControlBlockInvalidLength + + // ErrWitnessHasNoAnnex is returned when a caller attempts to extract + // an annex, but the witness has no annex present. + ErrWitnessHasNoAnnex + + // ErrInvalidTaprootSigLen is returned when taproot signature isn't 64 + // or 65 bytes. + ErrInvalidTaprootSigLen + + // ErrTaprootPubkeyIsEmpty is returned when a signature checking op + // code encounters an empty public key. + ErrTaprootPubkeyIsEmpty + + // ErrTaprootMaxSigOps is returned when the number of allotted sig ops + // is exceeded during taproot execution. + ErrTaprootMaxSigOps + // numErrorCodes is the maximum error code number used in tests. This // entry MUST be the last entry in the enum. numErrorCodes @@ -443,6 +484,16 @@ var errorCodeStrings = map[ErrorCode]string{ ErrDiscourageUpgradeableTaprootVersion: "ErrDiscourageUpgradeableTaprootVersion", ErrTapscriptCheckMultisig: "ErrTapscriptCheckMultisig", ErrDiscourageUpgradeablePubKeyType: "ErrDiscourageUpgradeablePubKeyType", + ErrTaprootSigInvalid: "ErrTaprootSigInvalid", + ErrTaprootMerkleProofInvalid: "ErrTaprootMerkleProofInvalid", + ErrTaprootOutputKeyParityMismatch: "ErrTaprootOutputKeyParityMismatch", + ErrControlBlockTooSmall: "ErrControlBlockTooSmall", + ErrControlBlockTooLarge: "ErrControlBlockTooLarge", + ErrControlBlockInvalidLength: "ErrControlBlockInvalidLength", + ErrWitnessHasNoAnnex: "ErrWitnessHasNoAnnex", + ErrInvalidTaprootSigLen: "ErrInvalidTaprootSigLen", + ErrTaprootPubkeyIsEmpty: "ErrTaprootPubkeyIsEmpty", + ErrTaprootMaxSigOps: "ErrTaprootMaxSigOps", } // String returns the ErrorCode as a human-readable name. diff --git a/txscript/error_test.go b/txscript/error_test.go index 8a9cef0926..accdf11a8c 100644 --- a/txscript/error_test.go +++ b/txscript/error_test.go @@ -86,6 +86,16 @@ func TestErrorCodeStringer(t *testing.T) { {ErrTapscriptCheckMultisig, "ErrTapscriptCheckMultisig"}, {ErrDiscourageUpgradableWitnessProgram, "ErrDiscourageUpgradableWitnessProgram"}, {ErrDiscourageUpgradeablePubKeyType, "ErrDiscourageUpgradeablePubKeyType"}, + {ErrTaprootSigInvalid, "ErrTaprootSigInvalid"}, + {ErrTaprootMerkleProofInvalid, "ErrTaprootMerkleProofInvalid"}, + {ErrTaprootOutputKeyParityMismatch, "ErrTaprootOutputKeyParityMismatch"}, + {ErrControlBlockTooSmall, "ErrControlBlockTooSmall"}, + {ErrControlBlockTooLarge, "ErrControlBlockTooLarge"}, + {ErrControlBlockInvalidLength, "ErrControlBlockInvalidLength"}, + {ErrWitnessHasNoAnnex, "ErrWitnessHasNoAnnex"}, + {ErrInvalidTaprootSigLen, "ErrInvalidTaprootSigLen"}, + {ErrTaprootPubkeyIsEmpty, "ErrTaprootPubkeyIsEmpty"}, + {ErrTaprootMaxSigOps, "ErrTaprootMaxSigOps"}, {0xffff, "Unknown ErrorCode (65535)"}, } diff --git a/txscript/opcode.go b/txscript/opcode.go index 9819ee6510..6e0434423e 100644 --- a/txscript/opcode.go +++ b/txscript/opcode.go @@ -2038,14 +2038,14 @@ func opcodeCheckSig(op *opcode, data []byte, vm *Engine) error { } } - // Empty public keys immeidately cause execution to fail. + // Empty public keys immediately cause execution to fail. if len(pkBytes) == 0 { - return fmt.Errorf("nil pub key") + return scriptError(ErrTaprootPubkeyIsEmpty, "") } // If this is tapscript execution, and the signature was // actually an empty vector, then we push on an empty vector - // and continue execution from ther, but only if the pubkey + // and continue execution from there, but only if the pubkey // isn't empty. if len(fullSigBytes) == 0 { vm.dstack.PushByteArray([]byte{}) @@ -2143,9 +2143,9 @@ func opcodeCheckSigAdd(op *opcode, data []byte, vm *Engine) error { } } - // Empty public keys immeidately cause execution to fail. + // Empty public keys immediately cause execution to fail. if len(pubKeyBytes) == 0 { - return fmt.Errorf("nil pubkey") + return scriptError(ErrTaprootPubkeyIsEmpty, "") } // If the signature is empty, then we'll just push the value N back diff --git a/txscript/sigvalidate.go b/txscript/sigvalidate.go index 8d5a8eb590..0bd00c326d 100644 --- a/txscript/sigvalidate.go +++ b/txscript/sigvalidate.go @@ -291,8 +291,8 @@ func parseTaprootSigAndPubKey(pkBytes, rawSig []byte, // Otherwise, this is an invalid signature, so we need to bail out. default: - // TODO(roasbeef): do proper error here - return nil, nil, 0, fmt.Errorf("invalid sig len: %v", len(rawSig)) + str := fmt.Sprintf("invalid sig len: %v", len(rawSig)) + return nil, nil, 0, scriptError(ErrInvalidTaprootSigLen, str) } return pubKey, sig, sigHashType, nil @@ -402,8 +402,7 @@ func newBaseTapscriptSigVerifier(pkBytes, rawSig []byte, // If the public key is zero bytes, then this is invalid, and will fail // immediately. case 0: - // TODO(roasbeef): better erro - return nil, fmt.Errorf("pubkey is zero bytes") + return nil, scriptError(ErrTaprootPubkeyIsEmpty, "") // If the public key is 32 byte as we expect, then we'll parse things // as normal. diff --git a/txscript/standard.go b/txscript/standard.go index 2ad658304b..aa7a7970d7 100644 --- a/txscript/standard.go +++ b/txscript/standard.go @@ -482,8 +482,7 @@ func isAnnexedWitness(witness wire.TxWitness) bool { // witness doesn't contain an annex, then an error is returned. func extractAnnex(witness [][]byte) ([]byte, error) { if !isAnnexedWitness(witness) { - // TODO(roasbeef): make into actual type - return nil, fmt.Errorf("no witness annex") + return nil, scriptError(ErrWitnessHasNoAnnex, "") } lastElement := witness[len(witness)-1] diff --git a/txscript/taproot.go b/txscript/taproot.go index f07e4d015a..2e452f92d7 100644 --- a/txscript/taproot.go +++ b/txscript/taproot.go @@ -89,8 +89,7 @@ func VerifyTaprootKeySpend(witnessProgram []byte, rawSig []byte, tx *wire.MsgTx, return nil } - // TODO(roasbeef): add proper error - return fmt.Errorf("invalid sig") + return scriptError(ErrTaprootSigInvalid, "") } // ControlBlock houses the structured witness input for a taproot spend. This @@ -189,18 +188,24 @@ func ParseControlBlock(ctrlBlock []byte) (*ControlBlock, error) { // The control block must minimally have 33 bytes for the internal // public key and script leaf version. case len(ctrlBlock) < ControlBlockBaseSize: - return nil, fmt.Errorf("invalid control block size") + str := fmt.Sprintf("min size is %v bytes, control block "+ + "is %v bytes", ControlBlockBaseSize, len(ctrlBlock)) + return nil, scriptError(ErrControlBlockTooSmall, str) // The control block can't be larger than a proof for the largest // possible tapscript merkle tree with 2^128 leaves. case len(ctrlBlock) > ControlBlockMaxSize: - return nil, fmt.Errorf("invalid max block size") + str := fmt.Sprintf("max size is %v, control block is %v bytes", + ControlBlockMaxSize, len(ctrlBlock)) + return nil, scriptError(ErrControlBlockTooLarge, str) // Ignoring the fixed sized portion, we expect the total number of // remaining bytes to be a multiple of the node size, which is 32 // bytes. case (len(ctrlBlock)-ControlBlockBaseSize)%ControlBlockNodeSize != 0: - return nil, fmt.Errorf("invalid max block size") + str := fmt.Sprintf("control block proof is not a multiple "+ + "of 32: %v", len(ctrlBlock)-ControlBlockBaseSize) + return nil, scriptError(ErrControlBlockInvalidLength, str) } // With the basic sanity checking complete, we can now parse the @@ -347,7 +352,8 @@ func VerifyTaprootLeafCommitment(controlBlock *ControlBlock, // program passed in. expectedWitnessProgram := schnorr.SerializePubKey(taprootKey) if !bytes.Equal(expectedWitnessProgram, taprootWitnessProgram) { - return fmt.Errorf("invalid witness commitment") + + return scriptError(ErrTaprootMerkleProofInvalid, "") } // Finally, we'll verify that the parity of the y coordinate of the @@ -355,7 +361,10 @@ func VerifyTaprootLeafCommitment(controlBlock *ControlBlock, derivedYIsOdd := (taprootKey.SerializeCompressed()[0] == secp.PubKeyFormatCompressedOdd) if controlBlock.OutputKeyYIsOdd != derivedYIsOdd { - return fmt.Errorf("invalid witness commitment") + str := fmt.Sprintf("control block y is odd: %v, derived "+ + "parity is odd: %v", controlBlock.OutputKeyYIsOdd, + derivedYIsOdd) + return scriptError(ErrTaprootOutputKeyParityMismatch, str) } // Otherwise, if we reach here, the commitment opening is valid and