Add error types (#17)

Cargo.toml

@@ -13,3 +13,6 @@ bitcoin = { version =  "0.31.2" }
 bitcoin_hashes = "0.14.0"
 hex = "0.4.3"
 miniscript = "11.0.0"
+
+[dev-dependencies]
+pretty_assertions = "1.4.0"

src/error.rs

@@ -0,0 +1,8 @@
+#[derive(Debug, PartialEq, Eq)]
+pub enum Bip322Error {
+  InvalidAddress, // for legacy addresses 1... (p2pkh) not supported, also any non taproot
+  Invalid,        // Address no key; pubkey not recovered, invalid signature
+  MalformedSignature, // wrong length, etc.
+  InvalidSigHash, // only sighash All and Default supported
+  NotKeyPathSpend, // only single key path spend supported
+}

src/lib.rs

@@ -7,6 +7,7 @@ use {
   bitcoin_hashes::{sha256, Hash},
 };
 
+mod error;
 mod sign;
 mod verify;
 
@@ -43,6 +44,8 @@ impl Wallet {
 
 const TAG: &str = "BIP0322-signed-message";
 
+type Result<T = (), E = error::Bip322Error> = std::result::Result<T, E>;
+
 // message_hash = sha256(sha256(tag) || sha256(tag) || message); see BIP340
 fn message_hash(message: &str) -> Vec<u8> {
   let mut tag_hash = sha256::Hash::hash(TAG.as_bytes()).to_byte_array().to_vec();
@@ -102,7 +105,7 @@ fn create_to_sign(to_spend: &Transaction) -> Psbt {
     }],
   };
 
-  let mut psbt = Psbt::from_unsigned_tx(to_sign).unwrap();
+  let mut psbt = Psbt::from_unsigned_tx(to_sign).unwrap(); // TODO
   psbt.inputs[0].witness_utxo = Some(TxOut {
     value: Amount::from_sat(0),
     script_pubkey: to_spend.output[0].script_pubkey.clone(),
@@ -113,7 +116,7 @@ fn create_to_sign(to_spend: &Transaction) -> Psbt {
 
 #[cfg(test)]
 mod tests {
-  use {super::*, std::str::FromStr};
+  use {super::*, error::Bip322Error, pretty_assertions::assert_eq, std::str::FromStr};
 
   /// From https://github.com/bitcoin/bips/blob/master/bip-0322.mediawiki#test-vectors
   /// and https://github.com/ACken2/bip322-js/blob/main/test/Verifier.test.ts
@@ -189,15 +192,16 @@ mod tests {
         &Address::from_str(TAPROOT_ADDRESS).unwrap().assume_checked(),
         "Hello World", 
         "AUHd69PrJQEv+oKTfZ8l+WROBHuy9HKrbFCJu7U1iK2iiEy1vMU5EfMtjc+VSHM7aU0SDbak5IUZRVno2P5mjSafAQ=="
-      )
+      ).is_ok()
     );
 
-    assert!(
-      !simple_verify(
+    assert_eq!(
+      simple_verify(
         &Address::from_str(TAPROOT_ADDRESS).unwrap().assume_checked(),
         "Hello World -- This should fail",
         "AUHd69PrJQEv+oKTfZ8l+WROBHuy9HKrbFCJu7U1iK2iiEy1vMU5EfMtjc+VSHM7aU0SDbak5IUZRVno2P5mjSafAQ=="
-      )
+      ),
+      Err(Bip322Error::Invalid)
     );
   }
 
@@ -229,7 +233,8 @@ mod tests {
         "Hello World",
         &wallet
       )
-    ));
+    )
+    .is_ok());
   }
 
   #[test]
@@ -244,6 +249,38 @@ mod tests {
         "Hello World",
         &wallet
       )
-    ));
+    )
+    .is_ok());
+  }
+
+  #[test]
+  fn invalid_address() {
+    assert_eq!(simple_verify(
+      &Address::from_str("3B5fQsEXEaV8v6U3ejYc8XaKXAkyQj2MjV").unwrap().assume_checked(), 
+      "",
+      "AkcwRAIgM2gBAQqvZX15ZiysmKmQpDrG83avLIT492QBzLnQIxYCIBaTpOaD20qRlEylyxFSeEA2ba9YOixpX8z46TSDtS40ASECx/EgAxlkQpQ9hYjgGu6EBCPMVPwVIVJqO4XCsMvViHI="), 
+    Err(Bip322Error::InvalidAddress)
+    )
+  }
+
+  #[test]
+  fn malformed_signature() {
+    assert_eq!(
+      simple_verify(
+        &Address::from_str(TAPROOT_ADDRESS).unwrap().assume_checked(),
+        "Hello World",
+        "invalid signature not in base64 encoding"
+      ),
+      Err(Bip322Error::MalformedSignature)
+    );
+
+    assert_eq!(
+      simple_verify(
+        &Address::from_str(TAPROOT_ADDRESS).unwrap().assume_checked(),
+        "Hello World", 
+        "AkcwRAIgM2gBAQqvZX15ZiysmKmQpDrG83avLIT492QBzLnQIxYCIBaTpOaD20qRlEylyxFSeEA2ba9YOixpX8z46TSDtS40ASECx/EgAxlkQpQ9hYjgGu6EBCPMVPwVIVJqO4XCsMvViH"
+      ),
+      Err(Bip322Error::MalformedSignature)
+    )
   }
 }

src/verify.rs

@@ -1,7 +1,9 @@
 use {
-  crate::{create_to_sign, create_to_spend},
+  super::*,
+  crate::{create_to_sign, create_to_spend, error::Bip322Error},
   base64::{engine::general_purpose, Engine},
   bitcoin::{
+    address::AddressType,
     consensus::Decodable,
     secp256k1::{schnorr::Signature, Message, Secp256k1, XOnlyPublicKey},
     sighash::{self, SighashCache, TapSighashType},
@@ -10,45 +12,58 @@ use {
   std::io::Cursor,
 };
 
-pub fn simple_verify(address: &Address, message: &str, signature: &str) -> bool {
-  let to_spend = create_to_spend(address, message);
-  let to_sign = create_to_sign(&to_spend);
-
-  let mut cursor = Cursor::new(general_purpose::STANDARD.decode(signature).unwrap());
+fn extract_pub_key(address: &Address) -> Result<XOnlyPublicKey> {
+  if address
+    .address_type()
+    .is_some_and(|addr| addr != AddressType::P2tr)
+  {
+    return Err(Bip322Error::InvalidAddress);
+  }
 
-  let witness = match Witness::consensus_decode_from_finite_reader(&mut cursor) {
-    Ok(witness) => witness,
-    Err(_) => return false,
-  };
+  if let bitcoin::address::Payload::WitnessProgram(witness_program) = address.payload() {
+    if witness_program.version().to_num() != 1 {
+      return Err(Bip322Error::InvalidAddress);
+    }
 
-  let encoded_signature = &witness.to_vec()[0];
+    if witness_program.program().len() != 32 {
+      return Err(Bip322Error::NotKeyPathSpend);
+    }
 
-  let (signature, sighash_type) = if encoded_signature.len() == 65 {
-    (
-      Signature::from_slice(&encoded_signature.as_slice()[..64]).unwrap(),
-      TapSighashType::from_consensus_u8(encoded_signature[64]).unwrap(),
-    )
-  } else if encoded_signature.len() == 64 {
-    (
-      Signature::from_slice(encoded_signature.as_slice()).unwrap(),
-      TapSighashType::Default,
+    Ok(
+      XOnlyPublicKey::from_slice(witness_program.program().as_bytes())
+        .expect("should extract an xonly public key"),
     )
   } else {
-    return false;
+    Err(Bip322Error::InvalidAddress)
+  }
+}
+
+fn decode_and_verify(
+  encoded_signature: &Vec<u8>,
+  pub_key: &XOnlyPublicKey,
+  to_spend: Transaction,
+  to_sign: Transaction,
+) -> Result<()> {
+  let (signature, sighash_type) = match encoded_signature.len() {
+    65 => (
+      Signature::from_slice(&encoded_signature.as_slice()[..64])
+        .map_err(|_| Bip322Error::MalformedSignature)?,
+      TapSighashType::from_consensus_u8(encoded_signature[64])
+        .map_err(|_| Bip322Error::InvalidSigHash)?,
+    ),
+    64 => (
+      Signature::from_slice(encoded_signature.as_slice())
+        .map_err(|_| Bip322Error::MalformedSignature)?,
+      TapSighashType::Default,
+    ),
+    _ => return Err(Bip322Error::MalformedSignature),
   };
 
-  let pub_key =
-    if let bitcoin::address::Payload::WitnessProgram(witness_program) = address.payload() {
-      if witness_program.version().to_num() == 1 && witness_program.program().len() == 32 {
-        XOnlyPublicKey::from_slice(witness_program.program().as_bytes()).unwrap()
-      } else {
-        return false;
-      }
-    } else {
-      return false;
-    };
+  if !(sighash_type == TapSighashType::All || sighash_type == TapSighashType::Default) {
+    return Err(Bip322Error::InvalidSigHash);
+  }
 
-  let mut sighash_cache = SighashCache::new(to_sign.unsigned_tx);
+  let mut sighash_cache = SighashCache::new(to_sign);
 
   let sighash = sighash_cache
     .taproot_key_spend_signature_hash(
@@ -61,74 +76,58 @@ pub fn simple_verify(address: &Address, message: &str, signature: &str) -> bool
     )
     .expect("signature hash should compute");
 
-  let message = Message::from_digest_slice(sighash.as_ref()).unwrap();
+  let message =
+    Message::from_digest_slice(sighash.as_ref()).expect("should be cryptographically secure hash");
 
   Secp256k1::verification_only()
-    .verify_schnorr(&signature, &message, &pub_key)
-    .is_ok()
+    .verify_schnorr(&signature, &message, pub_key)
+    .map_err(|_| Bip322Error::Invalid)
 }
 
-pub fn full_verify(address: &Address, message: &str, to_sign: &str) -> bool {
+pub fn simple_verify(address: &Address, message: &str, signature: &str) -> Result<()> {
+  let pub_key = extract_pub_key(address)?;
   let to_spend = create_to_spend(address, message);
+  let to_sign = create_to_sign(&to_spend);
 
-  let mut cursor = Cursor::new(general_purpose::STANDARD.decode(to_sign).unwrap());
-  let to_sign_tx = match Transaction::consensus_decode_from_finite_reader(&mut cursor) {
-    Ok(to_sign) => to_sign,
-    Err(_) => return false,
-  };
+  let mut cursor = Cursor::new(
+    general_purpose::STANDARD
+      .decode(signature)
+      .map_err(|_| Bip322Error::MalformedSignature)?,
+  );
 
-  let to_spend_out_point = OutPoint {
-    txid: to_spend.txid(),
-    vout: 0,
+  let witness = match Witness::consensus_decode_from_finite_reader(&mut cursor) {
+    Ok(witness) => witness,
+    Err(_) => return Err(Bip322Error::MalformedSignature),
   };
 
-  if to_spend_out_point != to_sign_tx.input[0].previous_output {
-    return false;
-  }
+  let encoded_signature = &witness.to_vec()[0];
 
-  let encoded_signature = &to_sign_tx.input[0].witness.to_vec()[0];
+  decode_and_verify(encoded_signature, &pub_key, to_spend, to_sign.unsigned_tx)
+}
 
-  let (signature, sighash_type) = if encoded_signature.len() == 65 {
-    (
-      Signature::from_slice(&encoded_signature.as_slice()[..64]).unwrap(),
-      TapSighashType::from_consensus_u8(encoded_signature[64]).unwrap(),
-    )
-  } else if encoded_signature.len() == 64 {
-    (
-      Signature::from_slice(encoded_signature.as_slice()).unwrap(),
-      TapSighashType::Default,
-    )
-  } else {
-    return false;
-  };
+pub fn full_verify(address: &Address, message: &str, to_sign_base64: &str) -> Result<()> {
+  let pub_key = extract_pub_key(address)?;
+  let to_spend = create_to_spend(address, message);
 
-  let pub_key =
-    if let bitcoin::address::Payload::WitnessProgram(witness_program) = address.payload() {
-      if witness_program.version().to_num() == 1 && witness_program.program().len() == 32 {
-        XOnlyPublicKey::from_slice(witness_program.program().as_bytes()).unwrap()
-      } else {
-        return false;
-      }
-    } else {
-      return false;
-    };
+  let mut cursor = Cursor::new(
+    general_purpose::STANDARD
+      .decode(to_sign_base64)
+      .map_err(|_| Bip322Error::MalformedSignature)?,
+  );
 
-  let mut sighash_cache = SighashCache::new(to_sign_tx);
+  let to_sign = Transaction::consensus_decode_from_finite_reader(&mut cursor)
+    .map_err(|_| Bip322Error::MalformedSignature)?;
 
-  let sighash = sighash_cache
-    .taproot_key_spend_signature_hash(
-      0,
-      &sighash::Prevouts::All(&[TxOut {
-        value: Amount::from_sat(0),
-        script_pubkey: to_spend.output[0].clone().script_pubkey,
-      }]),
-      sighash_type,
-    )
-    .expect("signature hash should compute");
+  let to_spend_out_point = OutPoint {
+    txid: to_spend.txid(),
+    vout: 0,
+  };
 
-  let message = Message::from_digest_slice(sighash.as_ref()).unwrap();
+  if to_spend_out_point != to_sign.input[0].previous_output {
+    return Err(Bip322Error::Invalid);
+  }
 
-  Secp256k1::verification_only()
-    .verify_schnorr(&signature, &message, &pub_key)
-    .is_ok()
+  let encoded_signature = &to_sign.input[0].witness.to_vec()[0];
+
+  decode_and_verify(encoded_signature, &pub_key, to_spend, to_sign)
 }