From d86517ae4fa70fbdcc792518c3fce88396ac9e4c Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 2 Jun 2023 14:48:22 +1000 Subject: [PATCH 1/4] taproot: Use error variants locally Add 'use Error::*' locally to make the code more terse. --- bitcoin/src/crypto/taproot.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/bitcoin/src/crypto/taproot.rs b/bitcoin/src/crypto/taproot.rs index ffb34c63..96067bee 100644 --- a/bitcoin/src/crypto/taproot.rs +++ b/bitcoin/src/crypto/taproot.rs @@ -73,12 +73,13 @@ pub enum Error { impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use Error::*; + match *self { - Error::InvalidSighashType(hash_ty) => - write!(f, "invalid signature hash type {}", hash_ty), - Error::Secp256k1(ref e) => + InvalidSighashType(hash_ty) => write!(f, "invalid signature hash type {}", hash_ty), + Secp256k1(ref e) => write_err!(f, "taproot signature has correct len but is malformed"; e), - Error::InvalidSignatureSize(sz) => write!(f, "invalid taproot signature size: {}", sz), + InvalidSignatureSize(sz) => write!(f, "invalid taproot signature size: {}", sz), } } } @@ -86,7 +87,7 @@ impl fmt::Display for Error { #[cfg(feature = "std")] impl std::error::Error for Error { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - use self::Error::*; + use Error::*; match self { Secp256k1(e) => Some(e), From 13d5c0536bc1c592475b8d652b7e34812db220a0 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 2 Jun 2023 14:32:06 +1000 Subject: [PATCH 2/4] Remove explicit error conversion We provide a `From` impl so we do not need to explicitly convert the error return, just use `?`. --- bitcoin/src/crypto/taproot.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bitcoin/src/crypto/taproot.rs b/bitcoin/src/crypto/taproot.rs index 96067bee..11c39c6f 100644 --- a/bitcoin/src/crypto/taproot.rs +++ b/bitcoin/src/crypto/taproot.rs @@ -31,7 +31,7 @@ impl Signature { 64 => { // default type let sig = - secp256k1::schnorr::Signature::from_slice(sl).map_err(Error::Secp256k1)?; + secp256k1::schnorr::Signature::from_slice(sl)?; Ok(Signature { sig, hash_ty: TapSighashType::Default }) } 65 => { @@ -39,7 +39,7 @@ impl Signature { let hash_ty = TapSighashType::from_consensus_u8(*hash_ty) .map_err(|_| Error::InvalidSighashType(*hash_ty))?; let sig = - secp256k1::schnorr::Signature::from_slice(sig).map_err(Error::Secp256k1)?; + secp256k1::schnorr::Signature::from_slice(sig)?; Ok(Signature { sig, hash_ty }) } len => Err(Error::InvalidSignatureSize(len)), From 29678cb82b6dde3f8e32b38bd9e7b960c65c60f3 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 2 Jun 2023 14:23:37 +1000 Subject: [PATCH 3/4] Correctly document InvalidSighashType variant The rustdoc on the `taproot::Error::InvalidSighashType` is wrong, fix it. --- bitcoin/src/crypto/taproot.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bitcoin/src/crypto/taproot.rs b/bitcoin/src/crypto/taproot.rs index 11c39c6f..357f39b3 100644 --- a/bitcoin/src/crypto/taproot.rs +++ b/bitcoin/src/crypto/taproot.rs @@ -63,7 +63,7 @@ impl Signature { #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] #[non_exhaustive] pub enum Error { - /// Base58 encoding error + /// Invalid signature hash type. InvalidSighashType(u8), /// Signature has valid size but does not parse correctly Secp256k1(secp256k1::Error), From 202d1cd5816a3b07f0fecd8690e61c9d0f472577 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Fri, 2 Jun 2023 15:01:10 +1000 Subject: [PATCH 4/4] Rename taproot::Error to SigFromSliceError This error type is only used in the `from_slice` function. Use prefix `Sig` because `taproot::FromSliceError` does not fully express how the error came about. Use specific identifier for the error, this aids usage but also prevents us later adding "random" other variants into this error and using it in other functions. --- bitcoin/src/crypto/taproot.rs | 30 +++++++++++++++--------------- bitcoin/src/psbt/error.rs | 2 +- bitcoin/src/psbt/serialize.rs | 8 +++++--- bitcoin/src/taproot.rs | 2 +- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/bitcoin/src/crypto/taproot.rs b/bitcoin/src/crypto/taproot.rs index 357f39b3..9a15cce1 100644 --- a/bitcoin/src/crypto/taproot.rs +++ b/bitcoin/src/crypto/taproot.rs @@ -26,23 +26,21 @@ pub struct Signature { impl Signature { /// Deserialize from slice - pub fn from_slice(sl: &[u8]) -> Result { + pub fn from_slice(sl: &[u8]) -> Result { match sl.len() { 64 => { // default type - let sig = - secp256k1::schnorr::Signature::from_slice(sl)?; + let sig = secp256k1::schnorr::Signature::from_slice(sl)?; Ok(Signature { sig, hash_ty: TapSighashType::Default }) } 65 => { let (hash_ty, sig) = sl.split_last().expect("Slice len checked == 65"); let hash_ty = TapSighashType::from_consensus_u8(*hash_ty) - .map_err(|_| Error::InvalidSighashType(*hash_ty))?; - let sig = - secp256k1::schnorr::Signature::from_slice(sig)?; + .map_err(|_| SigFromSliceError::InvalidSighashType(*hash_ty))?; + let sig = secp256k1::schnorr::Signature::from_slice(sig)?; Ok(Signature { sig, hash_ty }) } - len => Err(Error::InvalidSignatureSize(len)), + len => Err(SigFromSliceError::InvalidSignatureSize(len)), } } @@ -59,10 +57,12 @@ impl Signature { } } -/// A taproot sig related error. +/// An error constructing a [`taproot::Signature`] from a byte slice. +/// +/// [`taproot::Signature`]: crate::crypto::taproot::Signature #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] #[non_exhaustive] -pub enum Error { +pub enum SigFromSliceError { /// Invalid signature hash type. InvalidSighashType(u8), /// Signature has valid size but does not parse correctly @@ -71,9 +71,9 @@ pub enum Error { InvalidSignatureSize(usize), } -impl fmt::Display for Error { +impl fmt::Display for SigFromSliceError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - use Error::*; + use SigFromSliceError::*; match *self { InvalidSighashType(hash_ty) => write!(f, "invalid signature hash type {}", hash_ty), @@ -85,9 +85,9 @@ impl fmt::Display for Error { } #[cfg(feature = "std")] -impl std::error::Error for Error { +impl std::error::Error for SigFromSliceError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - use Error::*; + use SigFromSliceError::*; match self { Secp256k1(e) => Some(e), @@ -96,6 +96,6 @@ impl std::error::Error for Error { } } -impl From for Error { - fn from(e: secp256k1::Error) -> Error { Error::Secp256k1(e) } +impl From for SigFromSliceError { + fn from(e: secp256k1::Error) -> Self { SigFromSliceError::Secp256k1(e) } } diff --git a/bitcoin/src/psbt/error.rs b/bitcoin/src/psbt/error.rs index f08940ae..a32beb85 100644 --- a/bitcoin/src/psbt/error.rs +++ b/bitcoin/src/psbt/error.rs @@ -85,7 +85,7 @@ pub enum Error { /// Parsing error indicating invalid ECDSA signatures InvalidEcdsaSignature(crate::crypto::ecdsa::Error), /// Parsing error indicating invalid taproot signatures - InvalidTaprootSignature(crate::crypto::taproot::Error), + InvalidTaprootSignature(crate::crypto::taproot::SigFromSliceError), /// Parsing error indicating invalid control block InvalidControlBlock, /// Parsing error indicating invalid leaf version diff --git a/bitcoin/src/psbt/serialize.rs b/bitcoin/src/psbt/serialize.rs index 85501cf8..e515f31a 100644 --- a/bitcoin/src/psbt/serialize.rs +++ b/bitcoin/src/psbt/serialize.rs @@ -258,10 +258,12 @@ impl Serialize for taproot::Signature { impl Deserialize for taproot::Signature { fn deserialize(bytes: &[u8]) -> Result { + use taproot::SigFromSliceError::*; + taproot::Signature::from_slice(bytes).map_err(|e| match e { - taproot::Error::InvalidSighashType(flag) => Error::NonStandardSighashType(flag as u32), - taproot::Error::InvalidSignatureSize(_) => Error::InvalidTaprootSignature(e), - taproot::Error::Secp256k1(..) => Error::InvalidTaprootSignature(e), + InvalidSighashType(flag) => Error::NonStandardSighashType(flag as u32), + InvalidSignatureSize(_) => Error::InvalidTaprootSignature(e), + Secp256k1(..) => Error::InvalidTaprootSignature(e), }) } } diff --git a/bitcoin/src/taproot.rs b/bitcoin/src/taproot.rs index 2cdce7ec..36184204 100644 --- a/bitcoin/src/taproot.rs +++ b/bitcoin/src/taproot.rs @@ -17,7 +17,7 @@ use secp256k1::{self, Scalar, Secp256k1}; use crate::consensus::Encodable; use crate::crypto::key::{TapTweak, TweakedPublicKey, UntweakedPublicKey, XOnlyPublicKey}; // Re-export these so downstream only has to use one `taproot` module. -pub use crate::crypto::taproot::{Error, Signature}; +pub use crate::crypto::taproot::{SigFromSliceError, Signature}; use crate::prelude::*; use crate::{io, Script, ScriptBuf};