From fb318efe5e9901fb396254cc4bc630f6a1e1f248 Mon Sep 17 00:00:00 2001 From: Chris Stewart Date: Thu, 21 Nov 2024 11:42:18 -0600 Subject: [PATCH] core: Fix bug where we weren't checking for valid hash types in `TaprootKeyPath.isValid()` (#5780) * core: Fix bug where we weren't checking for valid hash types in TaprootKeyPath.isValid() * Fix TaprootWitness generator to use valid taproot hash types --- .../crypto/TransactionSignatureChecker.scala | 15 +-------------- .../core/protocol/script/ScriptWitness.scala | 17 +++++++++++++++-- .../core/script/crypto/CryptoInterpreter.scala | 2 +- .../scala/org/bitcoins/crypto/HashType.scala | 14 ++++++++++++++ project/CommonSettings.scala | 1 + .../testkitcore/gen/CryptoGenerators.scala | 4 +++- 6 files changed, 35 insertions(+), 18 deletions(-) diff --git a/core/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureChecker.scala b/core/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureChecker.scala index ca24a4e206..769dc9f497 100644 --- a/core/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureChecker.scala +++ b/core/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureChecker.scala @@ -88,7 +88,7 @@ trait TransactionSignatureChecker { val hashType = schnorrSignature.hashTypeOpt.getOrElse(HashType.sigHashDefault) // bip341 restricts valid hash types: https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#common-signature-message - val validHashType = checkTaprootHashType(hashType) + val validHashType = HashType.checkTaprootHashType(hashType) if (!validHashType) { ScriptErrorSchnorrSigHashType } else { @@ -101,19 +101,6 @@ trait TransactionSignatureChecker { } } - // (hash_type <= 0x03 || (hash_type >= 0x81 && hash_type <= 0x83)) - private val validTaprootHashTypes: Vector[Byte] = Vector(0x00.toByte, - 0x01.toByte, - 0x02.toByte, - 0x03.toByte, - 0x81.toByte, - 0x82.toByte, - 0x83.toByte) - - def checkTaprootHashType(hashType: HashType): Boolean = { - validTaprootHashTypes.contains(hashType.byte) - } - /** Checks the signature of a scriptSig in the spending transaction against * the given scriptPubKey & explicitly given public key This is useful for * instances of non standard scriptSigs diff --git a/core/src/main/scala/org/bitcoins/core/protocol/script/ScriptWitness.scala b/core/src/main/scala/org/bitcoins/core/protocol/script/ScriptWitness.scala index ff3680a8cf..b0aec3b79d 100644 --- a/core/src/main/scala/org/bitcoins/core/protocol/script/ScriptWitness.scala +++ b/core/src/main/scala/org/bitcoins/core/protocol/script/ScriptWitness.scala @@ -331,13 +331,26 @@ object TaprootKeyPath extends Factory[TaprootKeyPath] { def isValid(stack: Vector[ByteVector]): Boolean = { val noAnnex = stack.length == 1 && (stack.head.length == 64 || stack.head.length == 65) && - SchnorrPublicKey.fromBytesT(stack.head.take(32)).isSuccess + SchnorrPublicKey + .fromBytesT(stack.head.take(32)) + .isSuccess && checkHashType(stack.head) val annex = stack.length == 2 && TaprootScriptPath.hasAnnex(stack) && (stack(1).length == 64 || stack(1).length == 65) && - SchnorrPublicKey.fromBytesT(stack(1).take(32)).isSuccess + SchnorrPublicKey + .fromBytesT(stack(1).take(32)) + .isSuccess && checkHashType(stack(1)) noAnnex || annex } + + private def checkHashType(bytes: ByteVector): Boolean = { + require(bytes.length == 64 || bytes.length == 65) + if (bytes.length == 64) true + else { + val h = HashType.fromByte(bytes(64)) + HashType.checkTaprootHashType(h) + } + } } /** Spending a taproot output via the script path */ diff --git a/core/src/main/scala/org/bitcoins/core/script/crypto/CryptoInterpreter.scala b/core/src/main/scala/org/bitcoins/core/script/crypto/CryptoInterpreter.scala index 2877533906..6bb34d7a67 100644 --- a/core/src/main/scala/org/bitcoins/core/script/crypto/CryptoInterpreter.scala +++ b/core/src/main/scala/org/bitcoins/core/script/crypto/CryptoInterpreter.scala @@ -226,7 +226,7 @@ sealed abstract class CryptoInterpreter { helperE match { case Right(helper) => val validHashType = - TransactionSignatureChecker.checkTaprootHashType(helper.hashType) + HashType.checkTaprootHashType(helper.hashType) if (validHashType) { val result = TransactionSignatureChecker.checkSigTapscript( txSignatureComponent = program.txSignatureComponent, diff --git a/crypto/src/main/scala/org/bitcoins/crypto/HashType.scala b/crypto/src/main/scala/org/bitcoins/crypto/HashType.scala index 4488a38e73..d88b6dda31 100644 --- a/crypto/src/main/scala/org/bitcoins/crypto/HashType.scala +++ b/crypto/src/main/scala/org/bitcoins/crypto/HashType.scala @@ -200,6 +200,20 @@ object HashType extends Factory[HashType] { def isDefinedHashtypeSignature(sig: ECDigitalSignature): Boolean = { sig.bytes.nonEmpty && hashTypeBytes.contains(sig.bytes.last) } + + // (hash_type <= 0x03 || (hash_type >= 0x81 && hash_type <= 0x83)) + val validTaprootHashTypes: Vector[HashType] = + Vector(0x00.toByte, + 0x01.toByte, + 0x02.toByte, + 0x03.toByte, + 0x81.toByte, + 0x82.toByte, + 0x83.toByte).map(HashType.fromByte) + + def checkTaprootHashType(hashType: HashType): Boolean = { + validTaprootHashTypes.contains(hashType) + } } case object SIGHASH_DEFAULT extends HashType { diff --git a/project/CommonSettings.scala b/project/CommonSettings.scala index aee9e3e663..cf9f491f36 100644 --- a/project/CommonSettings.scala +++ b/project/CommonSettings.scala @@ -169,6 +169,7 @@ object CommonSettings { lazy val testSettings: Seq[Setting[_]] = Seq( //show full stack trace (-oF) of failed tests and duration of tests (-oD) Test / testOptions += Tests.Argument(TestFrameworks.ScalaTest, "-oDF"), + //Test / testOptions += Tests.Argument(TestFrameworks.ScalaTest,"-S","-2582694989510866987"), Test / logBuffered := false, skip / publish := true ) ++ settings diff --git a/testkit-core/src/main/scala/org/bitcoins/testkitcore/gen/CryptoGenerators.scala b/testkit-core/src/main/scala/org/bitcoins/testkitcore/gen/CryptoGenerators.scala index 5ba5d31089..06d1a16091 100644 --- a/testkit-core/src/main/scala/org/bitcoins/testkitcore/gen/CryptoGenerators.scala +++ b/testkit-core/src/main/scala/org/bitcoins/testkitcore/gen/CryptoGenerators.scala @@ -244,12 +244,14 @@ sealed abstract class CryptoGenerators { } } + def taprootHashType: Gen[HashType] = Gen.oneOf(HashType.validTaprootHashTypes) + def schnorrDigitalSignatureHashType: Gen[SchnorrDigitalSignature] = { for { privKey <- privateKey hash <- CryptoGenerators.doubleSha256Digest sigNoHashType = privKey.schnorrSign(hash.bytes) - hashType <- hashType + hashType <- taprootHashType } yield { sigNoHashType.appendHashType(hashType) }