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

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

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 */

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,

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 {

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

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)
     }