From 4e72db6f871a0f48fbd5353d74444c007df348fa Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 6 Nov 2023 17:43:10 +1100 Subject: [PATCH] Split general error into specific Currently we have a large general error type. Create specific error types for each function as needed so that a function returns ever variant available in its return type. --- examples/sign_verify_recovery.rs | 5 +- src/ecdh.rs | 20 +- src/ecdsa/mod.rs | 131 +++++++++-- src/ecdsa/recovery.rs | 53 +++-- src/ecdsa/serialized_signature.rs | 9 +- src/ellswift.rs | 23 +- src/error.rs | 378 ++++++++++++++++++++++++++++++ src/hex.rs | 149 +++++++++++- src/key/error.rs | 143 +++++++++++ src/{key.rs => key/mod.rs} | 184 +++++++-------- src/lib.rs | 151 ++++-------- src/macros.rs | 19 -- src/scalar.rs | 12 +- src/schnorr.rs | 47 ++-- 14 files changed, 1012 insertions(+), 312 deletions(-) create mode 100644 src/error.rs create mode 100644 src/key/error.rs rename src/{key.rs => key/mod.rs} (94%) diff --git a/examples/sign_verify_recovery.rs b/examples/sign_verify_recovery.rs index 2ab43ea06..80530cd33 100644 --- a/examples/sign_verify_recovery.rs +++ b/examples/sign_verify_recovery.rs @@ -4,6 +4,8 @@ extern crate secp256k1; use hashes::{sha256, Hash}; use secp256k1::{ecdsa, Error, Message, PublicKey, Secp256k1, SecretKey, Signing, Verification}; +// Notice that we provide a general error type for this crate and conversion +// functions to it from all the other error types so `?` works as expected. fn recover( secp: &Secp256k1, msg: &[u8], @@ -15,7 +17,8 @@ fn recover( let id = ecdsa::RecoveryId::from_i32(recovery_id as i32)?; let sig = ecdsa::RecoverableSignature::from_compact(&sig, id)?; - secp.recover_ecdsa(&msg, &sig) + let pk = secp.recover_ecdsa(&msg, &sig)?; + Ok(pk) } fn sign_recovery( diff --git a/src/ecdh.rs b/src/ecdh.rs index 9679f722d..b3044cc31 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -11,7 +11,13 @@ use secp256k1_sys::types::{c_int, c_uchar, c_void}; use crate::ffi::{self, CPtr}; use crate::key::{PublicKey, SecretKey}; -use crate::{constants, hex, Error}; +use crate::{constants, hex}; + +#[rustfmt::skip] // Keep public re-exports separate. +pub use crate::{ + error::InvalidSliceLengthError, + hex::FromHexError, +}; // The logic for displaying shared secrets relies on this (see `secret.rs`). const SHARED_SECRET_SIZE: usize = constants::SECRET_KEY_SIZE; @@ -66,22 +72,20 @@ impl SharedSecret { /// Creates a shared secret from `bytes` slice. #[inline] - pub fn from_slice(bytes: &[u8]) -> Result { + pub fn from_slice(bytes: &[u8]) -> Result { match <[u8; SHARED_SECRET_SIZE]>::try_from(bytes) { Ok(bytes) => Ok(SharedSecret(bytes)), - Err(_) => Err(Error::InvalidSharedSecret), + Err(_) => + Err(InvalidSliceLengthError { got: bytes.len(), expected: SHARED_SECRET_SIZE }), } } } impl str::FromStr for SharedSecret { - type Err = Error; + type Err = FromHexError; fn from_str(s: &str) -> Result { let mut res = [0u8; SHARED_SECRET_SIZE]; - match hex::from_hex(s, &mut res) { - Ok(SHARED_SECRET_SIZE) => Ok(SharedSecret::from_bytes(res)), - _ => Err(Error::InvalidSharedSecret), - } + hex::from_hex(s, &mut res).map(|_| SharedSecret::from_bytes(res)) } } diff --git a/src/ecdsa/mod.rs b/src/ecdsa/mod.rs index 45bf85f50..42c5e6e86 100644 --- a/src/ecdsa/mod.rs +++ b/src/ecdsa/mod.rs @@ -10,12 +10,14 @@ pub mod serialized_signature; use core::{fmt, ptr, str}; #[cfg(feature = "recovery")] -pub use self::recovery::{RecoverableSignature, RecoveryId}; +pub use self::recovery::{InvalidRecoveryIdError, RecoverableSignature, RecoveryId}; pub use self::serialized_signature::SerializedSignature; +use crate::error::{write_err, InvalidSliceLengthError, SysError}; use crate::ffi::CPtr; +use crate::hex::{self, FromHexError}; #[cfg(feature = "global-context")] use crate::SECP256K1; -use crate::{ffi, hex, Error, Message, PublicKey, Secp256k1, SecretKey, Signing, Verification}; +use crate::{ffi, Message, PublicKey, Secp256k1, SecretKey, Signing, Verification}; /// An ECDSA signature #[derive(Copy, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)] @@ -34,22 +36,21 @@ impl fmt::Display for Signature { } impl str::FromStr for Signature { - type Err = Error; + type Err = SignatureParseError; fn from_str(s: &str) -> Result { let mut res = [0u8; 72]; - match hex::from_hex(s, &mut res) { - Ok(x) => Signature::from_der(&res[0..x]), - _ => Err(Error::InvalidSignature), - } + let len = hex::from_hex(s, &mut res)?; + let sig = Signature::from_der(&res[0..len])?; + Ok(sig) } } impl Signature { #[inline] /// Converts a DER-encoded byte slice to a signature - pub fn from_der(data: &[u8]) -> Result { + pub fn from_der(data: &[u8]) -> Result { if data.is_empty() { - return Err(Error::InvalidSignature); + return Err(SignatureError::EmptySlice); } unsafe { @@ -63,15 +64,15 @@ impl Signature { { Ok(Signature(ret)) } else { - Err(Error::InvalidSignature) + Err(SignatureError::Sys(SysError {})) } } } /// Converts a 64-byte compact-encoded byte slice to a signature - pub fn from_compact(data: &[u8]) -> Result { + pub fn from_compact(data: &[u8]) -> Result { if data.len() != 64 { - return Err(Error::InvalidSignature); + return Err(SignatureError::invalid_length(data.len())); } unsafe { @@ -84,7 +85,7 @@ impl Signature { { Ok(Signature(ret)) } else { - Err(Error::InvalidSignature) + Err(SignatureError::Sys(SysError {})) } } } @@ -93,9 +94,9 @@ impl Signature { /// only useful for validating signatures in the Bitcoin blockchain from before /// 2016. It should never be used in new applications. This library does not /// support serializing to this "format" - pub fn from_der_lax(data: &[u8]) -> Result { + pub fn from_der_lax(data: &[u8]) -> Result { if data.is_empty() { - return Err(Error::InvalidSignature); + return Err(SignatureError::EmptySlice); } unsafe { @@ -109,7 +110,7 @@ impl Signature { { Ok(Signature(ret)) } else { - Err(Error::InvalidSignature) + Err(SignatureError::Sys(SysError {})) } } } @@ -192,7 +193,7 @@ impl Signature { /// The signature must be normalized or verification will fail (see [`Signature::normalize_s`]). #[inline] #[cfg(feature = "global-context")] - pub fn verify(&self, msg: &Message, pk: &PublicKey) -> Result<(), Error> { + pub fn verify(&self, msg: &Message, pk: &PublicKey) -> Result<(), SysError> { SECP256K1.verify_ecdsa(msg, self, pk) } } @@ -364,7 +365,7 @@ impl Secp256k1 { /// /// ```rust /// # #[cfg(feature = "rand-std")] { - /// # use secp256k1::{rand, Secp256k1, Message, Error}; + /// # use secp256k1::{ecdsa, rand, Secp256k1, Message, SysError}; /// # /// # let secp = Secp256k1::new(); /// # let (secret_key, public_key) = secp.generate_keypair(&mut rand::thread_rng()); @@ -374,7 +375,7 @@ impl Secp256k1 { /// assert_eq!(secp.verify_ecdsa(&message, &sig, &public_key), Ok(())); /// /// let message = Message::from_digest_slice(&[0xcd; 32]).expect("32 bytes"); - /// assert_eq!(secp.verify_ecdsa(&message, &sig, &public_key), Err(Error::IncorrectSignature)); + /// assert!(matches!(secp.verify_ecdsa(&message, &sig, &public_key), Err(SysError))); /// # } /// ``` #[inline] @@ -383,7 +384,7 @@ impl Secp256k1 { msg: &Message, sig: &Signature, pk: &PublicKey, - ) -> Result<(), Error> { + ) -> Result<(), SysError> { unsafe { if ffi::secp256k1_ecdsa_verify( self.ctx.as_ptr(), @@ -392,7 +393,7 @@ impl Secp256k1 { pk.as_c_ptr(), ) == 0 { - Err(Error::IncorrectSignature) + Err(SysError {}) } else { Ok(()) } @@ -427,3 +428,91 @@ pub(crate) fn der_length_check(sig: &ffi::Signature, max_len: usize) -> bool { } len <= max_len } + +/// Signature is invalid. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum SignatureError { + /// Tried to create signature from an empty slice. + EmptySlice, + /// Tried to create signature from an invalid length slice. + InvalidSliceLength(InvalidSliceLengthError), + /// FFI call failed. + Sys(SysError), +} + +impl SignatureError { + fn invalid_length(len: usize) -> Self { + InvalidSliceLengthError { got: len, expected: 64 }.into() + } +} + +impl fmt::Display for SignatureError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use SignatureError::*; + + match *self { + EmptySlice => write!(f, "tried to create signature from an empty slice"), + InvalidSliceLength(ref e) => + write_err!(f, "tried to create signature from an invalid length slice"; e), + Sys(ref e) => write_err!(f, "sys error"; e), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for SignatureError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use SignatureError::*; + + match *self { + EmptySlice => None, + InvalidSliceLength(ref e) => Some(e), + Sys(ref e) => Some(e), + } + } +} + +impl From for SignatureError { + fn from(e: InvalidSliceLengthError) -> Self { Self::InvalidSliceLength(e) } +} + +/// Signature string is invalid. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SignatureParseError { + /// Invalid hex string. + Hex(FromHexError), + /// Invalid signature. + Sig(SignatureError), +} + +impl fmt::Display for SignatureParseError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use SignatureParseError::*; + + match *self { + Hex(ref e) => write_err!(f, "error decoding hex"; e), + Sig(ref e) => write_err!(f, "invalid signature"; e), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for SignatureParseError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use SignatureParseError::*; + + match *self { + Hex(ref e) => Some(e), + Sig(ref e) => Some(e), + } + } +} + +impl From for SignatureParseError { + fn from(e: FromHexError) -> Self { Self::Hex(e) } +} + +impl From for SignatureParseError { + fn from(e: SignatureError) -> Self { Self::Sig(e) } +} diff --git a/src/ecdsa/recovery.rs b/src/ecdsa/recovery.rs index e0a5a97c8..871386531 100644 --- a/src/ecdsa/recovery.rs +++ b/src/ecdsa/recovery.rs @@ -4,13 +4,14 @@ //! signature. //! -use core::ptr; +use core::{fmt, ptr}; use self::super_ffi::CPtr; -use super::ffi as super_ffi; +use super::{ffi as super_ffi, SignatureError}; use crate::ecdsa::Signature; +use crate::error::SysError; use crate::ffi::recovery as ffi; -use crate::{key, Error, Message, Secp256k1, Signing, Verification}; +use crate::{key, Message, Secp256k1, Signing, Verification}; /// A tag used for recovering the public key from a compact signature. #[derive(Copy, Clone, PartialEq, Eq, Debug)] @@ -23,10 +24,10 @@ pub struct RecoverableSignature(ffi::RecoverableSignature); impl RecoveryId { #[inline] /// Allows library users to create valid recovery IDs from i32. - pub fn from_i32(id: i32) -> Result { + pub fn from_i32(id: i32) -> Result { match id { 0..=3 => Ok(RecoveryId(id)), - _ => Err(Error::InvalidRecoveryId), + other => Err(InvalidRecoveryIdError(other)), } } @@ -39,16 +40,19 @@ impl RecoverableSignature { #[inline] /// Converts a compact-encoded byte slice to a signature. This /// representation is nonstandard and defined by the libsecp256k1 library. - pub fn from_compact(data: &[u8], recid: RecoveryId) -> Result { + pub fn from_compact( + data: &[u8], + recid: RecoveryId, + ) -> Result { if data.is_empty() { - return Err(Error::InvalidSignature); + return Err(SignatureError::EmptySlice); } let mut ret = ffi::RecoverableSignature::new(); unsafe { if data.len() != 64 { - Err(Error::InvalidSignature) + Err(SignatureError::invalid_length(data.len())) } else if ffi::secp256k1_ecdsa_recoverable_signature_parse_compact( super_ffi::secp256k1_context_no_precomp, &mut ret, @@ -58,7 +62,7 @@ impl RecoverableSignature { { Ok(RecoverableSignature(ret)) } else { - Err(Error::InvalidSignature) + Err(SignatureError::Sys(SysError {})) } } } @@ -113,7 +117,7 @@ impl RecoverableSignature { /// verify-capable context. #[inline] #[cfg(feature = "global-context")] - pub fn recover(&self, msg: &Message) -> Result { + pub fn recover(&self, msg: &Message) -> Result { crate::SECP256K1.recover_ecdsa(msg, self) } } @@ -191,7 +195,7 @@ impl Secp256k1 { &self, msg: &Message, sig: &RecoverableSignature, - ) -> Result { + ) -> Result { unsafe { let mut pk = super_ffi::PublicKey::new(); if ffi::secp256k1_ecdsa_recover( @@ -201,22 +205,39 @@ impl Secp256k1 { msg.as_c_ptr(), ) != 1 { - return Err(Error::InvalidSignature); + return Err(SignatureError::Sys(SysError {})); } Ok(key::PublicKey::from(pk)) } } } +/// Recovery ID is invalid. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct InvalidRecoveryIdError(pub i32); + +impl fmt::Display for InvalidRecoveryIdError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "recovery ID is invalid: {}", self.0) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for InvalidRecoveryIdError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + #[cfg(test)] #[allow(unused_imports)] mod tests { #[cfg(target_arch = "wasm32")] use wasm_bindgen_test::wasm_bindgen_test as test; - use super::{RecoverableSignature, RecoveryId}; + use super::*; use crate::constants::ONE; - use crate::{Error, Message, Secp256k1, SecretKey}; + use crate::{Message, Secp256k1, SecretKey}; #[test] #[cfg(feature = "rand-std")] @@ -316,7 +337,7 @@ mod tests { let msg = crate::random_32_bytes(&mut rand::thread_rng()); let msg = Message::from_digest_slice(&msg).unwrap(); - assert_eq!(s.verify_ecdsa(&msg, &sig, &pk), Err(Error::IncorrectSignature)); + assert_eq!(s.verify_ecdsa(&msg, &sig, &pk), Err(SysError {})); let recovered_key = s.recover_ecdsa(&msg, &sigr).unwrap(); assert!(recovered_key != pk); @@ -366,7 +387,7 @@ mod tests { // Zero is not a valid sig let sig = RecoverableSignature::from_compact(&[0; 64], RecoveryId(0)).unwrap(); - assert_eq!(s.recover_ecdsa(&msg, &sig), Err(Error::InvalidSignature)); + assert_eq!(s.recover_ecdsa(&msg, &sig), Err(SignatureError::Sys(SysError {}))); // ...but 111..111 is let sig = RecoverableSignature::from_compact(&[1; 64], RecoveryId(0)).unwrap(); assert!(s.recover_ecdsa(&msg, &sig).is_ok()); diff --git a/src/ecdsa/serialized_signature.rs b/src/ecdsa/serialized_signature.rs index 8cf2c0294..808f59649 100644 --- a/src/ecdsa/serialized_signature.rs +++ b/src/ecdsa/serialized_signature.rs @@ -13,8 +13,7 @@ use core::{fmt, ops}; pub use into_iter::IntoIter; -use super::Signature; -use crate::Error; +use super::{Signature, SignatureError}; pub(crate) const MAX_LEN: usize = 72; @@ -123,13 +122,13 @@ impl<'a> From<&'a Signature> for SerializedSignature { } impl TryFrom for Signature { - type Error = Error; + type Error = SignatureError; fn try_from(value: SerializedSignature) -> Result { value.to_signature() } } impl<'a> TryFrom<&'a SerializedSignature> for Signature { - type Error = Error; + type Error = SignatureError; fn try_from(value: &'a SerializedSignature) -> Result { value.to_signature() @@ -164,7 +163,7 @@ impl SerializedSignature { /// Convert the serialized signature into the Signature struct. /// (This DER deserializes it) #[inline] - pub fn to_signature(&self) -> Result { Signature::from_der(self) } + pub fn to_signature(&self) -> Result { Signature::from_der(self) } /// Create a SerializedSignature from a Signature. /// (this DER serializes it) diff --git a/src/ellswift.rs b/src/ellswift.rs index e95eaa1a5..a18957f7d 100644 --- a/src/ellswift.rs +++ b/src/ellswift.rs @@ -42,7 +42,7 @@ use core::str::FromStr; use ffi::CPtr; use secp256k1_sys::types::{c_int, c_uchar, c_void}; -use crate::{constants, ffi, hex, Error, PublicKey, Secp256k1, SecretKey, Verification}; +use crate::{constants, ffi, hex, PublicKey, Secp256k1, SecretKey, Verification}; unsafe extern "C" fn hash_callback( output: *mut c_uchar, @@ -288,14 +288,14 @@ impl ElligatorSwiftParty { } impl FromStr for ElligatorSwift { - type Err = Error; + type Err = ParseError; fn from_str(hex: &str) -> Result { let mut ser = [0u8; 64]; let parsed = hex::from_hex(hex, &mut ser); match parsed { Ok(64) => Ok(ElligatorSwift::from_array(ser)), - _ => Err(Error::InvalidEllSwift), + _ => Err(ParseError), } } } @@ -320,6 +320,23 @@ impl ffi::CPtr for ElligatorSwift { fn as_c_ptr(&self) -> *const Self::Target { self.0.as_c_ptr() } } +/// Error converting from a hex string. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct ParseError; + +impl core::fmt::Display for ParseError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + write!(f, "error converting hex to ellswift") + } +} + +#[cfg(feature = "std")] +impl std::error::Error for ParseError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + #[cfg(test)] mod tests { use core::str::FromStr; diff --git a/src/error.rs b/src/error.rs new file mode 100644 index 000000000..5da56948d --- /dev/null +++ b/src/error.rs @@ -0,0 +1,378 @@ +// SPDX-License-Identifier: CC0-1.0 + +//! Error types and conversion functions. + +use core::fmt; + +use crate::context::NotEnoughMemoryError; +use crate::hex::{FromHexError, ToHexError}; +use crate::key::error::{ + ParityValueError, PublicKeyError, PublicKeySumError, SecretKeyError, TweakError, + XOnlyTweakError, +}; +use crate::{ecdh, ecdsa, ellswift, scalar, schnorr, MessageLengthError}; + +/// Implements `From for $error` for all the errors in this crate. +/// +/// Either pass in the variant to use or have a variant `Secp256k1` on `$error`. +/// +/// # Examples +/// +/// ``` +/// # #[cfg(feature = "rand-std")] { +/// use secp256k1::{PublicKey, SecretKey, Secp256k1}; +/// +/// // Use the a general `secp256k1::Error`. +/// +/// /// Foo error. +/// pub struct FooError; +/// +/// // A custom error enum in your application. +/// pub enum Error { +/// Foo(FooError), +/// Secp256k1(secp256k1::Error), +/// } +/// secp256k1::impl_from_for_all_crate_errors_for!(Error); +/// +/// impl From for Error { +/// fn from(e: FooError) -> Self { Self::Foo(e) } +/// } +/// +/// /// Some useful function. +/// pub fn foo() -> Result<(), FooError> { +/// // Do some stuff. +/// Err(FooError) +/// } +/// +/// // Call any secp256k1 function and convert to a single general error variant. +/// fn bar() -> Result<(), Error> { +/// let secp = Secp256k1::new(); +/// let key_data = [0_u8; 32]; // Dummy data. +/// let _ = SecretKey::from_slice(&key_data)?; +/// let _ = PublicKey::from_slice(&key_data)?; +/// let _ = foo()?; +/// Ok(()) +/// } +/// # } +/// ``` +// To find all errors in this crate use (note: -v 'git grep' to remove this comment): +// +// git grep -e 'impl std::error' | grep -v 'git grep' | cut -d ' ' -f 4 | sort +// +#[macro_export] +macro_rules! impl_from_for_all_crate_errors_for { + ($error:ty) => { + $crate::impl_from_for_all_crate_errors_for!($error, Secp256k1); + }; + ($error:ty, $variant:ident) => { + impl From<$crate::Error> for $error { + fn from(e: $crate::Error) -> Self { Self::$variant(e) } + } + + #[cfg(feature = "recovery")] + impl From<$crate::ecdsa::InvalidRecoveryIdError> for $error { + fn from(e: $crate::ecdsa::InvalidRecoveryIdError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::MessageLengthError> for $error { + fn from(e: $crate::MessageLengthError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::NotEnoughMemoryError> for $error { + fn from(e: $crate::NotEnoughMemoryError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::scalar::OutOfRangeError> for $error { + fn from(e: $crate::scalar::OutOfRangeError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::ParityValueError> for $error { + fn from(e: $crate::ParityValueError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::PublicKeyError> for $error { + fn from(e: $crate::PublicKeyError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::PublicKeySumError> for $error { + fn from(e: $crate::PublicKeySumError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::SecretKeyError> for $error { + fn from(e: $crate::SecretKeyError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::schnorr::SignatureError> for $error { + fn from(e: $crate::schnorr::SignatureError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::ecdsa::SignatureError> for $error { + fn from(e: $crate::ecdsa::SignatureError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::ecdsa::SignatureParseError> for $error { + fn from(e: $crate::ecdsa::SignatureParseError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::SysError> for $error { + fn from(e: $crate::SysError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::FromHexError> for $error { + fn from(e: $crate::FromHexError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::ToHexError> for $error { + fn from(e: $crate::ToHexError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::TweakError> for $error { + fn from(e: $crate::TweakError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::XOnlyTweakError> for $error { + fn from(e: $crate::XOnlyTweakError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::ellswift::ParseError> for $error { + fn from(e: $crate::ellswift::ParseError) -> Self { Self::$variant(e.into()) } + } + }; +} + +/// This is a general purpose error type that can be used to wrap all the errors in this crate. +/// +/// Every error types in this crate can be converted (using `?`) to this type. We also support +/// converting from any of the inner error types to this type, irrespective of the level of nesting. +#[derive(Debug, Clone, PartialEq, Eq)] +#[allow(missing_copy_implementations)] // For forward compatibility (combined with non_exhaustive). +#[non_exhaustive] +pub enum Error { + /// Error decoding from hex string. + FromHex(FromHexError), + /// Invalid recovery ID (ECDSA). + #[cfg(feature = "recovery")] + RecoveryId(ecdsa::InvalidRecoveryIdError), + /// Messages must be 32 bytes long. + MessageLength(MessageLengthError), + /// Not enough preallocated memory for the requested buffer size. + NotEnoughMemory(NotEnoughMemoryError), + /// Value of scalar is invalid - larger than the curve order. + InvalidScalar(scalar::OutOfRangeError), + /// Invalid value for parity - must be 0 or 1. + ParityValue(ParityValueError), + /// Public key is invalid. + PublicKey(PublicKeyError), + /// Public key summation is invalid. + PublicKeySum(PublicKeySumError), + /// Secret key is invalid. + SecretKey(SecretKeyError), + /// Schnorr signature is invalid. + SchnorrSignature(schnorr::SignatureError), + /// ECDSA signature is invalid. + EcdsaSignature(ecdsa::SignatureError), + /// ECDSA signature string invalid. + EcdsaSignatureParse(ecdsa::SignatureParseError), + /// Error calling into the FFI layer. + Sys(SysError), + /// Error encoding as hex string. + ToHex(ToHexError), + /// Invalid key tweak. + Tweak(TweakError), + /// X-only pubic key tweak failed. + XOnlyTweak(XOnlyTweakError), + /// Error converting hex string to ellswift. + Ellswift(ellswift::ParseError), + /// Invalid slice length. + InvalidSliceLength(InvalidSliceLengthError), +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use Error::*; + + // TODO: Check what gets out put in std and no-std builds an verify it is + // useful and does not contain redundant content. + match *self { + FromHex(ref e) => write_err!(f, "from hex"; e), + #[cfg(feature = "recovery")] + RecoveryId(ref e) => write_err!(f, "invalid recovery ID (ECDSA)"; e), + MessageLength(ref e) => write_err!(f, "invalid message length"; e), + NotEnoughMemory(ref e) => write_err!(f, "not enough memory"; e), + InvalidScalar(ref e) => write_err!(f, ""; e), + ParityValue(ref e) => write_err!(f, "invalid parity"; e), + PublicKey(ref e) => write_err!(f, "invalid public key"; e), + PublicKeySum(ref e) => write_err!(f, "invalid public key sum"; e), + SecretKey(ref e) => write_err!(f, "invalid secret key"; e), + SchnorrSignature(ref e) => write_err!(f, "invalid schnorr sig"; e), + EcdsaSignature(ref e) => write_err!(f, "invalid ECDSA sig"; e), + EcdsaSignatureParse(ref e) => write_err!(f, "invalid ECDSA sig string"; e), + Sys(ref e) => write_err!(f, "sys"; e), + ToHex(ref e) => write_err!(f, "to hex"; e), + Tweak(ref e) => write_err!(f, "invalid tweak"; e), + XOnlyTweak(ref e) => write_err!(f, "x-only tweak error"; e), + Ellswift(ref e) => write_err!(f, "ellswift error"; e), + InvalidSliceLength(ref e) => write_err!(f, "invalid slice"; e), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use Error::*; + + match *self { + FromHex(ref e) => Some(e), + #[cfg(feature = "recovery")] + RecoveryId(ref e) => Some(e), + MessageLength(ref e) => Some(e), + NotEnoughMemory(ref e) => Some(e), + InvalidScalar(ref e) => Some(e), + ParityValue(ref e) => Some(e), + PublicKey(ref e) => Some(e), + PublicKeySum(ref e) => Some(e), + SecretKey(ref e) => Some(e), + SchnorrSignature(ref e) => Some(e), + EcdsaSignature(ref e) => Some(e), + EcdsaSignatureParse(ref e) => Some(e), + Sys(ref e) => Some(e), + ToHex(ref e) => Some(e), + Tweak(ref e) => Some(e), + XOnlyTweak(ref e) => Some(e), + Ellswift(ref e) => Some(e), + InvalidSliceLength(ref e) => Some(e), + } + } +} + +impl From for Error { + fn from(e: FromHexError) -> Self { Self::FromHex(e) } +} + +#[cfg(feature = "recovery")] +impl From for Error { + fn from(e: ecdsa::InvalidRecoveryIdError) -> Self { Self::RecoveryId(e) } +} + +impl From for Error { + fn from(e: MessageLengthError) -> Self { Self::MessageLength(e) } +} + +impl From for Error { + fn from(e: NotEnoughMemoryError) -> Self { Self::NotEnoughMemory(e) } +} + +impl From for Error { + fn from(e: scalar::OutOfRangeError) -> Self { Self::InvalidScalar(e) } +} + +impl From for Error { + fn from(e: ParityValueError) -> Self { Self::ParityValue(e) } +} + +impl From for Error { + fn from(e: PublicKeyError) -> Self { Self::PublicKey(e) } +} + +impl From for Error { + fn from(e: PublicKeySumError) -> Self { Self::PublicKeySum(e) } +} + +impl From for Error { + fn from(e: SecretKeyError) -> Self { Self::SecretKey(e) } +} + +impl From for Error { + fn from(e: schnorr::SignatureError) -> Self { Self::SchnorrSignature(e) } +} + +impl From for Error { + fn from(e: ecdsa::SignatureError) -> Self { Self::EcdsaSignature(e) } +} + +impl From for Error { + fn from(e: ecdsa::SignatureParseError) -> Self { Self::EcdsaSignatureParse(e) } +} + +impl From for Error { + fn from(e: SysError) -> Self { Self::Sys(e) } +} + +impl From for Error { + fn from(e: ToHexError) -> Self { Self::ToHex(e) } +} + +impl From for Error { + fn from(e: TweakError) -> Self { Self::Tweak(e) } +} + +impl From for Error { + fn from(e: XOnlyTweakError) -> Self { Self::XOnlyTweak(e) } +} + +impl From for Error { + fn from(e: ellswift::ParseError) -> Self { Self::Ellswift(e) } +} + +impl From for Error { + fn from(e: ecdh::InvalidSliceLengthError) -> Self { Self::InvalidSliceLength(e) } +} + +/// Error parsing a slice. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct InvalidSliceLengthError { + pub(crate) got: usize, + pub(crate) expected: usize, +} + +impl core::fmt::Display for InvalidSliceLengthError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + write!(f, "invalid slice length {}, expected {}", self.got, self.expected) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for InvalidSliceLengthError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + +/// Error calling into the FFI layer. +// TODO: Do we want to include the error code returned for C function calls? +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct SysError {} + +impl core::fmt::Display for SysError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + f.write_str("FFI call failed") + } +} + +#[cfg(feature = "std")] +impl std::error::Error for SysError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + +/// Formats error. If `std` feature is OFF appends error source (delimited by `: `). We do this +/// because `e.source()` is only available in std builds, without this macro the error source is +/// lost for no-std builds. +macro_rules! write_err { + ($writer:expr, $string:literal $(, $args:expr),*; $source:expr) => { + { + #[cfg(feature = "std")] + { + let _ = &$source; // Prevents clippy warnings. + write!($writer, $string $(, $args)*) + } + #[cfg(not(feature = "std"))] + { + write!($writer, concat!($string, ": {}") $(, $args)*, $source) + } + } + } +} +pub(crate) use write_err; diff --git a/src/hex.rs b/src/hex.rs index 8cf889369..08f5a99e6 100644 --- a/src/hex.rs +++ b/src/hex.rs @@ -4,12 +4,21 @@ use core::str; +use crate::error::write_err; + /// Utility function used to parse hex into a target u8 buffer. Returns /// the number of bytes converted or an error if it encounters an invalid /// character or unexpected end of string. -pub(crate) fn from_hex(hex: &str, target: &mut [u8]) -> Result { - if hex.len() % 2 == 1 || hex.len() > target.len() * 2 { - return Err(()); +pub(crate) fn from_hex(hex: &str, target: &mut [u8]) -> Result { + if hex.len() % 2 == 1 { + return Err(FromHexError::UnevenLength(UnevenLengthError { len: hex.len() })); + } + + if hex.len() > target.len() * 2 { + return Err(FromHexError::BufferTooSmall(BufferTooSmallError { + hex: hex.len(), + buffer: target.len(), + })); } let mut b = 0; @@ -20,7 +29,7 @@ pub(crate) fn from_hex(hex: &str, target: &mut [u8]) -> Result { b'A'..=b'F' => b |= c - b'A' + 10, b'a'..=b'f' => b |= c - b'a' + 10, b'0'..=b'9' => b |= c - b'0', - _ => return Err(()), + byte => return Err(FromHexError::InvalidByte(InvalidByteError { invalid: byte })), } if (idx & 1) == 1 { target[idx / 2] = b; @@ -35,10 +44,10 @@ pub(crate) fn from_hex(hex: &str, target: &mut [u8]) -> Result { /// a reference to the target buffer as an str. Returns an error if the target /// buffer isn't big enough. #[inline] -pub(crate) fn to_hex<'a>(src: &[u8], target: &'a mut [u8]) -> Result<&'a str, ()> { +pub(crate) fn to_hex<'a>(src: &[u8], target: &'a mut [u8]) -> Result<&'a str, ToHexError> { let hex_len = src.len() * 2; if target.len() < hex_len { - return Err(()); + return Err(ToHexError { hex: hex_len, buffer: target.len() }); } const HEX_TABLE: [u8; 16] = *b"0123456789abcdef"; @@ -52,3 +61,131 @@ pub(crate) fn to_hex<'a>(src: &[u8], target: &'a mut [u8]) -> Result<&'a str, () debug_assert!(str::from_utf8(result).is_ok()); return unsafe { Ok(str::from_utf8_unchecked(result)) }; } + +/// Error converting from a hex string. +#[derive(Debug, Clone, PartialEq, Eq)] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +#[non_exhaustive] +pub enum FromHexError { + /// Hex string length uneven. + UnevenLength(UnevenLengthError), + /// Target data buffer too small to decode hex. + BufferTooSmall(BufferTooSmallError), + /// Byte is not valid hex ASCII. + InvalidByte(InvalidByteError), +} + +impl core::fmt::Display for FromHexError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + use FromHexError::*; + + match *self { + UnevenLength(ref e) => write_err!(f, "uneven length, converting from hex"; e), + BufferTooSmall(ref e) => write_err!(f, "buffer too small, converting from hex"; e), + InvalidByte(ref e) => write_err!(f, "invalid byte, convening from hex"; e), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for FromHexError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use FromHexError::*; + + match *self { + UnevenLength(ref e) => Some(e), + BufferTooSmall(ref e) => Some(e), + InvalidByte(ref e) => Some(e), + } + } +} + +/// Hex string length uneven. +#[derive(Debug, Clone, PartialEq, Eq)] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +#[non_exhaustive] +pub struct UnevenLengthError { + len: usize, +} + +impl core::fmt::Display for UnevenLengthError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + write!(f, "hex string uneven: {}", self.len) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for UnevenLengthError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + +/// Target data buffer too small to decode hex. +#[derive(Debug, Clone, PartialEq, Eq)] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +#[non_exhaustive] +pub struct BufferTooSmallError { + /// Length of the hex string. + hex: usize, + /// Size of the target data buffer. + buffer: usize, +} + +impl core::fmt::Display for BufferTooSmallError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + write!( + f, + "buffer too small to decode hex (hex length: {}, buffer size: {})", + self.hex, self.buffer + ) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for BufferTooSmallError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + +/// Byte is not valid hex ASCII. +#[derive(Debug, Clone, PartialEq, Eq)] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +#[non_exhaustive] +pub struct InvalidByteError { + invalid: u8, +} + +impl core::fmt::Display for InvalidByteError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + write!(f, "byte is not valid hex ASCII: {:x}", self.invalid) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for InvalidByteError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + +/// Buffer too small to encode hex data. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct ToHexError { + /// Required length of the encoded hex string. + hex: usize, + /// Size of the buffer (must be equal or larger that hex length). + buffer: usize, +} + +impl core::fmt::Display for ToHexError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + write!( + f, + "buffer too small to encode hex (required: {}, buffer: {})", + self.hex, self.buffer + ) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for ToHexError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} diff --git a/src/key/error.rs b/src/key/error.rs new file mode 100644 index 000000000..46bca89ba --- /dev/null +++ b/src/key/error.rs @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: CC0-1.0 + +//! Error types for the `key` module. + +use core::fmt; + +use crate::error::write_err; + +/// X-only public key tweak is invalid. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum XOnlyTweakError { + /// Invalid tweak. + Tweak(TweakError), + /// Invalid public key. + PublicKey(PublicKeyError), + /// Invalid parity value. + ParityValue(ParityValueError), +} + +impl fmt::Display for XOnlyTweakError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use XOnlyTweakError::*; + + // TODO: Check what gets out put in std and no-std builds an verify it useful and does not + // contain redundant content. + match *self { + Tweak(ref e) => write_err!(f, "invalid tweak"; e), + PublicKey(ref e) => write_err!(f, "invalid public key"; e), + ParityValue(ref e) => write_err!(f, "invalid parity value"; e), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for XOnlyTweakError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use XOnlyTweakError::*; + + match *self { + Tweak(ref e) => Some(e), + PublicKey(ref e) => Some(e), + ParityValue(ref e) => Some(e), + } + } +} + +impl From for XOnlyTweakError { + fn from(e: TweakError) -> Self { Self::Tweak(e) } +} + +impl From for XOnlyTweakError { + fn from(e: PublicKeyError) -> Self { Self::PublicKey(e) } +} + +impl From for XOnlyTweakError { + fn from(e: ParityValueError) -> Self { Self::ParityValue(e) } +} + +/// Secret key is invalid. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct SecretKeyError; + +impl core::fmt::Display for SecretKeyError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + f.write_str("secret key is invalid") + } +} + +#[cfg(feature = "std")] +impl std::error::Error for SecretKeyError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + +/// Public key is invalid. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct PublicKeyError; + +impl core::fmt::Display for PublicKeyError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + f.write_str("public key is invalid") + } +} + +#[cfg(feature = "std")] +impl std::error::Error for PublicKeyError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + +/// Public key summation is invalid. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct PublicKeySumError; + +impl core::fmt::Display for PublicKeySumError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + f.write_str("public key summation is invalid") + } +} + +#[cfg(feature = "std")] +impl std::error::Error for PublicKeySumError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + +/// Invalid key tweak. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct TweakError; + +impl core::fmt::Display for TweakError { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + f.write_str("invalid key tweak") + } +} + +#[cfg(feature = "std")] +impl std::error::Error for TweakError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + +/// Invalid value for parity - must be 0 or 1. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct ParityValueError(pub i32); + +impl fmt::Display for ParityValueError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "invalid value {} for parity - must be 0 or 1", self.0) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for ParityValueError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} diff --git a/src/key.rs b/src/key/mod.rs similarity index 94% rename from src/key.rs rename to src/key/mod.rs index 8cc56deb5..1fd4df438 100644 --- a/src/key.rs +++ b/src/key/mod.rs @@ -3,6 +3,14 @@ //! Public and secret keys. //! +pub mod error; + +#[rustfmt::skip] +pub use self::error::{ + ParityValueError, PublicKeyError, PublicKeySumError, SecretKeyError, TweakError, + XOnlyTweakError, +}; + use core::convert::TryFrom; use core::ops::{self, BitXor}; use core::{fmt, ptr, str}; @@ -11,9 +19,9 @@ use core::{fmt, ptr, str}; use serde::ser::SerializeTuple; use crate::ellswift::ElligatorSwift; +use crate::error::SysError; use crate::ffi::types::c_uint; use crate::ffi::{self, CPtr}; -use crate::Error::{self, InvalidPublicKey, InvalidPublicKeySum, InvalidSecretKey}; #[cfg(feature = "global-context")] use crate::SECP256K1; use crate::{constants, ecdsa, hex, schnorr, Message, Scalar, Secp256k1, Signing, Verification}; @@ -109,12 +117,12 @@ impl ffi::CPtr for SecretKey { } impl str::FromStr for SecretKey { - type Err = Error; + type Err = SecretKeyError; fn from_str(s: &str) -> Result { let mut res = [0u8; constants::SECRET_KEY_SIZE]; match hex::from_hex(s, &mut res) { Ok(constants::SECRET_KEY_SIZE) => SecretKey::from_slice(&res), - _ => Err(Error::InvalidSecretKey), + _ => Err(SecretKeyError), } } } @@ -162,14 +170,14 @@ impl fmt::Display for PublicKey { } impl str::FromStr for PublicKey { - type Err = Error; + type Err = PublicKeyError; fn from_str(s: &str) -> Result { let mut res = [0u8; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE]; match hex::from_hex(s, &mut res) { Ok(constants::PUBLIC_KEY_SIZE) => PublicKey::from_slice(&res[0..constants::PUBLIC_KEY_SIZE]), Ok(constants::UNCOMPRESSED_PUBLIC_KEY_SIZE) => PublicKey::from_slice(&res), - _ => Err(Error::InvalidPublicKey), + _ => Err(PublicKeyError), } } } @@ -210,7 +218,7 @@ impl SecretKey { /// let sk = SecretKey::from_slice(&[0xcd; 32]).expect("32 bytes, within curve order"); /// ``` #[inline] - pub fn from_slice(data: &[u8]) -> Result { + pub fn from_slice(data: &[u8]) -> Result { match <[u8; constants::SECRET_KEY_SIZE]>::try_from(data) { Ok(data) => { unsafe { @@ -219,12 +227,12 @@ impl SecretKey { data.as_c_ptr(), ) == 0 { - return Err(InvalidSecretKey); + return Err(SecretKeyError); } } Ok(SecretKey(data)) } - Err(_) => Err(InvalidSecretKey), + Err(_) => Err(SecretKeyError), } } @@ -303,7 +311,7 @@ impl SecretKey { /// /// Returns an error if the resulting key would be invalid. #[inline] - pub fn add_tweak(mut self, tweak: &Scalar) -> Result { + pub fn add_tweak(mut self, tweak: &Scalar) -> Result { unsafe { if ffi::secp256k1_ec_seckey_tweak_add( ffi::secp256k1_context_no_precomp, @@ -311,7 +319,7 @@ impl SecretKey { tweak.as_c_ptr(), ) != 1 { - Err(Error::InvalidTweak) + Err(TweakError) } else { Ok(self) } @@ -324,7 +332,7 @@ impl SecretKey { /// /// Returns an error if the resulting key would be invalid. #[inline] - pub fn mul_tweak(mut self, tweak: &Scalar) -> Result { + pub fn mul_tweak(mut self, tweak: &Scalar) -> Result { unsafe { if ffi::secp256k1_ec_seckey_tweak_mul( ffi::secp256k1_context_no_precomp, @@ -332,7 +340,7 @@ impl SecretKey { tweak.as_c_ptr(), ) != 1 { - Err(Error::InvalidTweak) + Err(TweakError) } else { Ok(self) } @@ -462,9 +470,9 @@ impl PublicKey { /// Creates a public key directly from a slice. #[inline] - pub fn from_slice(data: &[u8]) -> Result { + pub fn from_slice(data: &[u8]) -> Result { if data.is_empty() { - return Err(Error::InvalidPublicKey); + return Err(PublicKeyError); } unsafe { @@ -478,7 +486,7 @@ impl PublicKey { { Ok(PublicKey(pk)) } else { - Err(InvalidPublicKey) + Err(PublicKeyError) } } } @@ -578,14 +586,14 @@ impl PublicKey { mut self, secp: &Secp256k1, tweak: &Scalar, - ) -> Result { + ) -> Result { unsafe { if ffi::secp256k1_ec_pubkey_tweak_add(secp.ctx.as_ptr(), &mut self.0, tweak.as_c_ptr()) == 1 { Ok(self) } else { - Err(Error::InvalidTweak) + Err(TweakError) } } } @@ -600,14 +608,14 @@ impl PublicKey { mut self, secp: &Secp256k1, other: &Scalar, - ) -> Result { + ) -> Result { unsafe { if ffi::secp256k1_ec_pubkey_tweak_mul(secp.ctx.as_ptr(), &mut self.0, other.as_c_ptr()) == 1 { Ok(self) } else { - Err(Error::InvalidTweak) + Err(TweakError) } } } @@ -631,7 +639,7 @@ impl PublicKey { /// let sum = pk1.combine(&pk2).expect("It's improbable to fail for 2 random public keys"); /// # } /// ``` - pub fn combine(&self, other: &PublicKey) -> Result { + pub fn combine(&self, other: &PublicKey) -> Result { PublicKey::combine_keys(&[self, other]) } @@ -658,12 +666,12 @@ impl PublicKey { /// let sum = PublicKey::combine_keys(&[&pk1, &pk2, &pk3]).expect("It's improbable to fail for 3 random public keys"); /// # } /// ``` - pub fn combine_keys(keys: &[&PublicKey]) -> Result { + pub fn combine_keys(keys: &[&PublicKey]) -> Result { use core::i32::MAX; use core::mem::transmute; if keys.is_empty() || keys.len() > MAX as usize { - return Err(InvalidPublicKeySum); + return Err(PublicKeySumError); } unsafe { @@ -679,7 +687,7 @@ impl PublicKey { { Ok(PublicKey(ret)) } else { - Err(InvalidPublicKeySum) + Err(PublicKeySumError) } } } @@ -710,7 +718,7 @@ impl PublicKey { secp: &Secp256k1, msg: &Message, sig: &ecdsa::Signature, - ) -> Result<(), Error> { + ) -> Result<(), SysError> { secp.verify_ecdsa(msg, sig, self) } } @@ -828,15 +836,15 @@ impl Keypair { /// /// # Errors /// - /// [`Error::InvalidSecretKey`] if the provided data has an incorrect length, exceeds Secp256k1 + /// [`SecretKeyError`] if the provided data has an incorrect length, exceeds Secp256k1 /// field `p` value or the corresponding public key is not even. #[inline] pub fn from_seckey_slice( secp: &Secp256k1, data: &[u8], - ) -> Result { + ) -> Result { if data.is_empty() || data.len() != constants::SECRET_KEY_SIZE { - return Err(Error::InvalidSecretKey); + return Err(SecretKeyError); } unsafe { @@ -844,7 +852,7 @@ impl Keypair { if ffi::secp256k1_keypair_create(secp.ctx.as_ptr(), &mut kp, data.as_c_ptr()) == 1 { Ok(Keypair(kp)) } else { - Err(Error::InvalidSecretKey) + Err(SecretKeyError) } } } @@ -853,14 +861,17 @@ impl Keypair { /// /// # Errors /// - /// [`Error::InvalidSecretKey`] if corresponding public key for the provided secret key is not even. + /// [`SecretKeyError`] if corresponding public key for the provided secret key is not even. #[inline] - pub fn from_seckey_str(secp: &Secp256k1, s: &str) -> Result { + pub fn from_seckey_str( + secp: &Secp256k1, + s: &str, + ) -> Result { let mut res = [0u8; constants::SECRET_KEY_SIZE]; match hex::from_hex(s, &mut res) { Ok(constants::SECRET_KEY_SIZE) => - Keypair::from_seckey_slice(secp, &res[0..constants::SECRET_KEY_SIZE]), - _ => Err(Error::InvalidPublicKey), + Ok(Keypair::from_seckey_slice(secp, &res[0..constants::SECRET_KEY_SIZE])?), + _ => Err(SecretKeyError), } } @@ -868,10 +879,10 @@ impl Keypair { /// /// # Errors /// - /// [`Error::InvalidSecretKey`] if corresponding public key for the provided secret key is not even. + /// [`SecretKeyError`] if corresponding public key for the provided secret key is not even. #[inline] #[cfg(feature = "global-context")] - pub fn from_seckey_str_global(s: &str) -> Result { + pub fn from_seckey_str_global(s: &str) -> Result { Keypair::from_seckey_str(SECP256K1, s) } @@ -942,7 +953,7 @@ impl Keypair { mut self, secp: &Secp256k1, tweak: &Scalar, - ) -> Result { + ) -> Result { unsafe { let err = ffi::secp256k1_keypair_xonly_tweak_add( secp.ctx.as_ptr(), @@ -950,7 +961,7 @@ impl Keypair { tweak.as_c_ptr(), ); if err != 1 { - return Err(Error::InvalidTweak); + return Err(TweakError); } Ok(self) @@ -1015,7 +1026,7 @@ impl<'a> From<&'a Keypair> for PublicKey { } impl str::FromStr for Keypair { - type Err = Error; + type Err = SecretKeyError; #[allow(unused_variables, unreachable_code)] // When built with no default features. fn from_str(s: &str) -> Result { @@ -1128,13 +1139,13 @@ impl fmt::Display for XOnlyPublicKey { } impl str::FromStr for XOnlyPublicKey { - type Err = Error; + type Err = PublicKeyError; fn from_str(s: &str) -> Result { let mut res = [0u8; constants::SCHNORR_PUBLIC_KEY_SIZE]; match hex::from_hex(s, &mut res) { Ok(constants::SCHNORR_PUBLIC_KEY_SIZE) => XOnlyPublicKey::from_slice(&res[0..constants::SCHNORR_PUBLIC_KEY_SIZE]), - _ => Err(Error::InvalidPublicKey), + _ => Err(PublicKeyError), } } } @@ -1177,12 +1188,12 @@ impl XOnlyPublicKey { /// /// # Errors /// - /// Returns [`Error::InvalidPublicKey`] if the length of the data slice is not 32 bytes or the + /// Returns [`PublicKeyError`] if the length of the data slice is not 32 bytes or the /// slice does not represent a valid Secp256k1 point x coordinate. #[inline] - pub fn from_slice(data: &[u8]) -> Result { + pub fn from_slice(data: &[u8]) -> Result { if data.is_empty() || data.len() != constants::SCHNORR_PUBLIC_KEY_SIZE { - return Err(Error::InvalidPublicKey); + return Err(PublicKeyError); } unsafe { @@ -1195,7 +1206,7 @@ impl XOnlyPublicKey { { Ok(XOnlyPublicKey(pk)) } else { - Err(Error::InvalidPublicKey) + Err(PublicKeyError) } } } @@ -1246,7 +1257,7 @@ impl XOnlyPublicKey { mut self, secp: &Secp256k1, tweak: &Scalar, - ) -> Result<(XOnlyPublicKey, Parity), Error> { + ) -> Result<(XOnlyPublicKey, Parity), XOnlyTweakError> { let mut pk_parity = 0; unsafe { let mut pubkey = ffi::PublicKey::new(); @@ -1257,7 +1268,7 @@ impl XOnlyPublicKey { tweak.as_c_ptr(), ); if err != 1 { - return Err(Error::InvalidTweak); + return Err(TweakError)?; } err = ffi::secp256k1_xonly_pubkey_from_pubkey( @@ -1267,7 +1278,7 @@ impl XOnlyPublicKey { &pubkey, ); if err == 0 { - return Err(Error::InvalidPublicKey); + return Err(PublicKeyError)?; } let parity = Parity::from_i32(pk_parity)?; @@ -1339,7 +1350,7 @@ impl XOnlyPublicKey { secp: &Secp256k1, msg: &Message, sig: &schnorr::Signature, - ) -> Result<(), Error> { + ) -> Result<(), schnorr::SignatureError> { secp.verify_schnorr(sig, msg, self) } } @@ -1368,7 +1379,7 @@ impl Parity { /// /// The only allowed values are `0` meaning even parity and `1` meaning odd. /// Other values result in error being returned. - pub fn from_u8(parity: u8) -> Result { + pub fn from_u8(parity: u8) -> Result { Parity::from_i32(parity.into()) } @@ -1376,25 +1387,25 @@ impl Parity { /// /// The only allowed values are `0` meaning even parity and `1` meaning odd. /// Other values result in error being returned. - pub fn from_i32(parity: i32) -> Result { + pub fn from_i32(parity: i32) -> Result { match parity { 0 => Ok(Parity::Even), 1 => Ok(Parity::Odd), - _ => Err(InvalidParityValue(parity)), + _ => Err(ParityValueError(parity)), } } } /// `Even` for `0`, `Odd` for `1`, error for anything else impl TryFrom for Parity { - type Error = InvalidParityValue; + type Error = ParityValueError; fn try_from(parity: i32) -> Result { Self::from_i32(parity) } } /// `Even` for `0`, `Odd` for `1`, error for anything else impl TryFrom for Parity { - type Error = InvalidParityValue; + type Error = ParityValueError; fn try_from(parity: u8) -> Result { Self::from_u8(parity) } } @@ -1423,27 +1434,6 @@ impl BitXor for Parity { } } -/// Error returned when conversion from an integer to `Parity` fails. -// -// Note that we don't allow inspecting the value because we may change the type. -// Yes, this comment is intentionally NOT doc comment. -// Too many derives for compatibility with current Error type. -#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, Ord, PartialOrd)] -pub struct InvalidParityValue(i32); - -impl fmt::Display for InvalidParityValue { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "invalid value {} for Parity - must be 0 or 1", self.0) - } -} - -#[cfg(feature = "std")] -impl std::error::Error for InvalidParityValue {} - -impl From for Error { - fn from(error: InvalidParityValue) -> Self { Error::InvalidParityValue(error) } -} - /// The parity is serialized as `u8` - `0` for even, `1` for odd. #[cfg(feature = "serde")] impl serde::Serialize for Parity { @@ -1554,8 +1544,7 @@ mod test { #[cfg(target_arch = "wasm32")] use wasm_bindgen_test::wasm_bindgen_test as test; - use super::{Keypair, Parity, PublicKey, Secp256k1, SecretKey, XOnlyPublicKey, *}; - use crate::Error::{InvalidPublicKey, InvalidSecretKey}; + use super::*; use crate::{constants, hex, Scalar}; #[cfg(not(secp256k1_fuzz))] @@ -1570,7 +1559,7 @@ mod test { #[test] fn skey_from_slice() { let sk = SecretKey::from_slice(&[1; 31]); - assert_eq!(sk, Err(InvalidSecretKey)); + assert_eq!(sk, Err(SecretKeyError)); let sk = SecretKey::from_slice(&[1; 32]); assert!(sk.is_ok()); @@ -1578,8 +1567,8 @@ mod test { #[test] fn pubkey_from_slice() { - assert_eq!(PublicKey::from_slice(&[]), Err(InvalidPublicKey)); - assert_eq!(PublicKey::from_slice(&[1, 2, 3]), Err(InvalidPublicKey)); + assert_eq!(PublicKey::from_slice(&[]), Err(PublicKeyError)); + assert_eq!(PublicKey::from_slice(&[1, 2, 3]), Err(PublicKeyError)); let uncompressed = PublicKey::from_slice(&[ 4, 54, 57, 149, 239, 162, 148, 175, 246, 254, 239, 75, 154, 152, 10, 82, 234, 224, 85, @@ -1622,13 +1611,13 @@ mod test { #[rustfmt::skip] fn invalid_secret_key() { // Zero - assert_eq!(SecretKey::from_slice(&[0; 32]), Err(InvalidSecretKey)); + assert_eq!(SecretKey::from_slice(&[0; 32]), Err(SecretKeyError)); assert_eq!( SecretKey::from_str("0000000000000000000000000000000000000000000000000000000000000000"), - Err(InvalidSecretKey) + Err(SecretKeyError) ); // -1 - assert_eq!(SecretKey::from_slice(&[0xff; 32]), Err(InvalidSecretKey)); + assert_eq!(SecretKey::from_slice(&[0xff; 32]), Err(SecretKeyError)); // Top of range assert!(SecretKey::from_slice(&[ 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, @@ -1682,31 +1671,28 @@ mod test { // Bad sizes assert_eq!( PublicKey::from_slice(&[0; constants::PUBLIC_KEY_SIZE - 1]), - Err(InvalidPublicKey) + Err(PublicKeyError) ); assert_eq!( PublicKey::from_slice(&[0; constants::PUBLIC_KEY_SIZE + 1]), - Err(InvalidPublicKey) + Err(PublicKeyError) ); assert_eq!( PublicKey::from_slice(&[0; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE - 1]), - Err(InvalidPublicKey) + Err(PublicKeyError) ); assert_eq!( PublicKey::from_slice(&[0; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE + 1]), - Err(InvalidPublicKey) + Err(PublicKeyError) ); // Bad parse assert_eq!( PublicKey::from_slice(&[0xff; constants::UNCOMPRESSED_PUBLIC_KEY_SIZE]), - Err(InvalidPublicKey) - ); - assert_eq!( - PublicKey::from_slice(&[0x55; constants::PUBLIC_KEY_SIZE]), - Err(InvalidPublicKey) + Err(PublicKeyError) ); - assert_eq!(PublicKey::from_slice(&[]), Err(InvalidPublicKey)); + assert_eq!(PublicKey::from_slice(&[0x55; constants::PUBLIC_KEY_SIZE]), Err(PublicKeyError)); + assert_eq!(PublicKey::from_slice(&[]), Err(PublicKeyError)); } #[test] @@ -1714,22 +1700,16 @@ mod test { // Bad sizes assert_eq!( SecretKey::from_slice(&[0; constants::SECRET_KEY_SIZE - 1]), - Err(InvalidSecretKey) + Err(SecretKeyError) ); assert_eq!( SecretKey::from_slice(&[0; constants::SECRET_KEY_SIZE + 1]), - Err(InvalidSecretKey) + Err(SecretKeyError) ); // Bad parse - assert_eq!( - SecretKey::from_slice(&[0xff; constants::SECRET_KEY_SIZE]), - Err(InvalidSecretKey) - ); - assert_eq!( - SecretKey::from_slice(&[0x00; constants::SECRET_KEY_SIZE]), - Err(InvalidSecretKey) - ); - assert_eq!(SecretKey::from_slice(&[]), Err(InvalidSecretKey)); + assert_eq!(SecretKey::from_slice(&[0xff; constants::SECRET_KEY_SIZE]), Err(SecretKeyError)); + assert_eq!(SecretKey::from_slice(&[0x00; constants::SECRET_KEY_SIZE]), Err(SecretKeyError)); + assert_eq!(SecretKey::from_slice(&[]), Err(SecretKeyError)); } #[test] diff --git a/src/lib.rs b/src/lib.rs index 2aadc81ad..4823b4530 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -166,6 +166,7 @@ pub mod constants; pub mod ecdh; pub mod ecdsa; pub mod ellswift; +pub mod error; pub mod scalar; pub mod schnorr; #[cfg(feature = "serde")] @@ -175,22 +176,33 @@ use core::marker::PhantomData; use core::ptr::NonNull; use core::{fmt, mem}; -#[cfg(feature = "global-context")] -pub use context::global::SECP256K1; #[cfg(feature = "hashes")] use hashes::Hash; + +use crate::ffi::types::AlignedType; +use crate::ffi::CPtr; + +#[rustfmt::skip] // Keep public re-exports separate. +pub use crate::{ + context::*, // Includes NotEnoughMemoryError + hex::{FromHexError, ToHexError}, + key::{PublicKey, SecretKey, *}, + scalar::Scalar, + key::error::{ + ParityValueError, PublicKeyError, PublicKeySumError, SecretKeyError, TweakError, + XOnlyTweakError, + }, + error::{Error, SysError, InvalidSliceLengthError}, +}; + +#[cfg(feature = "global-context")] +pub use context::global::SECP256K1; #[cfg(feature = "rand")] pub use rand; pub use secp256k1_sys as ffi; #[cfg(feature = "serde")] pub use serde; -pub use crate::context::*; -use crate::ffi::types::AlignedType; -use crate::ffi::CPtr; -pub use crate::key::{PublicKey, SecretKey, *}; -pub use crate::scalar::Scalar; - /// Trait describing something that promises to be a 32-byte random number; in particular, /// it has negligible probability of being zero or overflowing the group order. Such objects /// may be converted to `Message`s without any error paths. @@ -229,7 +241,7 @@ impl Message { /// [secure signature](https://twitter.com/pwuille/status/1063582706288586752). #[inline] #[deprecated(since = "0.28.0", note = "use from_digest_slice instead")] - pub fn from_slice(digest: &[u8]) -> Result { + pub fn from_slice(digest: &[u8]) -> Result { Message::from_digest_slice(digest) } @@ -258,14 +270,14 @@ impl Message { /// /// [secure signature]: https://twitter.com/pwuille/status/1063582706288586752 #[inline] - pub fn from_digest_slice(digest: &[u8]) -> Result { + pub fn from_digest_slice(digest: &[u8]) -> Result { match digest.len() { constants::MESSAGE_SIZE => { let mut ret = [0u8; constants::MESSAGE_SIZE]; ret[..].copy_from_slice(digest); Ok(Message(ret)) } - _ => Err(Error::InvalidMessage), + len => Err(MessageLengthError(len)), } } @@ -311,76 +323,21 @@ impl fmt::Display for Message { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::LowerHex::fmt(self, f) } } -/// The main error type for this library. -#[derive(Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Debug)] -pub enum Error { - /// Signature failed verification. - IncorrectSignature, - /// Bad sized message ("messages" are actually fixed-sized digests [`constants::MESSAGE_SIZE`]). - InvalidMessage, - /// Bad public key. - InvalidPublicKey, - /// Bad signature. - InvalidSignature, - /// Bad secret key. - InvalidSecretKey, - /// Bad shared secret. - InvalidSharedSecret, - /// Bad recovery id. - InvalidRecoveryId, - /// Tried to add/multiply by an invalid tweak. - InvalidTweak, - /// Didn't pass enough memory to context creation with preallocated memory. - NotEnoughMemory, - /// Bad set of public keys. - InvalidPublicKeySum, - /// The only valid parity values are 0 or 1. - InvalidParityValue(key::InvalidParityValue), - /// Bad EllSwift value - InvalidEllSwift, -} +/// Messages must be 32 bytes long. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct MessageLengthError(pub usize); -impl fmt::Display for Error { +impl fmt::Display for MessageLengthError { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { - use Error::*; - - match *self { - IncorrectSignature => f.write_str("signature failed verification"), - InvalidMessage => f.write_str("message was not 32 bytes (do you need to hash?)"), - InvalidPublicKey => f.write_str("malformed public key"), - InvalidSignature => f.write_str("malformed signature"), - InvalidSecretKey => f.write_str("malformed or out-of-range secret key"), - InvalidSharedSecret => f.write_str("malformed or out-of-range shared secret"), - InvalidRecoveryId => f.write_str("bad recovery id"), - InvalidTweak => f.write_str("bad tweak"), - NotEnoughMemory => f.write_str("not enough memory allocated"), - InvalidPublicKeySum => f.write_str( - "the sum of public keys was invalid or the input vector lengths was less than 1", - ), - InvalidParityValue(e) => write_err!(f, "couldn't create parity"; e), - InvalidEllSwift => f.write_str("malformed EllSwift value"), - } + write!(f, "messages must be 32 bytes long, got: {}", self.0) } } #[cfg(feature = "std")] -impl std::error::Error for Error { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Error::IncorrectSignature => None, - Error::InvalidMessage => None, - Error::InvalidPublicKey => None, - Error::InvalidSignature => None, - Error::InvalidSecretKey => None, - Error::InvalidSharedSecret => None, - Error::InvalidRecoveryId => None, - Error::InvalidTweak => None, - Error::NotEnoughMemory => None, - Error::InvalidPublicKeySum => None, - Error::InvalidParityValue(error) => Some(error), - Error::InvalidEllSwift => None, - } - } +impl std::error::Error for MessageLengthError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } } /// The secp256k1 engine, used to execute all signature operations. @@ -499,6 +456,10 @@ mod tests { use wasm_bindgen_test::wasm_bindgen_test as test; use super::*; + use crate::error::SysError; + use crate::{constants, ecdsa, hex, Message}; + #[cfg(feature = "alloc")] + use crate::{ffi, PublicKey, Secp256k1, SecretKey}; macro_rules! hex { ($hex:expr) => {{ @@ -814,60 +775,32 @@ mod tests { let msg = crate::random_32_bytes(&mut rand::thread_rng()); let msg = Message::from_digest_slice(&msg).unwrap(); - assert_eq!(s.verify_ecdsa(&msg, &sig, &pk), Err(Error::IncorrectSignature)); + assert_eq!(s.verify_ecdsa(&msg, &sig, &pk), Err(SysError {})); } #[test] fn test_bad_slice() { assert_eq!( ecdsa::Signature::from_der(&[0; constants::MAX_SIGNATURE_SIZE + 1]), - Err(Error::InvalidSignature) + Err(ecdsa::SignatureError::Sys(SysError {})) ); assert_eq!( ecdsa::Signature::from_der(&[0; constants::MAX_SIGNATURE_SIZE]), - Err(Error::InvalidSignature) + Err(ecdsa::SignatureError::Sys(SysError {})) ); assert_eq!( Message::from_digest_slice(&[0; constants::MESSAGE_SIZE - 1]), - Err(Error::InvalidMessage) + Err(MessageLengthError(31)) ); assert_eq!( Message::from_digest_slice(&[0; constants::MESSAGE_SIZE + 1]), - Err(Error::InvalidMessage) + Err(MessageLengthError(33)) ); assert!(Message::from_digest_slice(&[0; constants::MESSAGE_SIZE]).is_ok()); assert!(Message::from_digest_slice(&[1; constants::MESSAGE_SIZE]).is_ok()); } - #[test] - #[cfg(feature = "rand-std")] - fn test_hex() { - use rand::RngCore; - - let mut rng = rand::thread_rng(); - const AMOUNT: usize = 1024; - for i in 0..AMOUNT { - // 255 isn't a valid utf8 character. - let mut hex_buf = [255u8; AMOUNT * 2]; - let mut src_buf = [0u8; AMOUNT]; - let mut result_buf = [0u8; AMOUNT]; - let src = &mut src_buf[0..i]; - rng.fill_bytes(src); - - let hex = hex::to_hex(src, &mut hex_buf).unwrap(); - assert_eq!(hex::from_hex(hex, &mut result_buf).unwrap(), i); - assert_eq!(src, &result_buf[..i]); - } - - assert!(hex::to_hex(&[1; 2], &mut [0u8; 3]).is_err()); - assert!(hex::to_hex(&[1; 2], &mut [0u8; 4]).is_ok()); - assert!(hex::from_hex("deadbeaf", &mut [0u8; 3]).is_err()); - assert!(hex::from_hex("deadbeaf", &mut [0u8; 4]).is_ok()); - assert!(hex::from_hex("a", &mut [0u8; 4]).is_err()); - assert!(hex::from_hex("ag", &mut [0u8; 4]).is_err()); - } - #[test] #[cfg(not(secp256k1_fuzz))] // fuzz-sigs have fixed size/format #[cfg(any(feature = "alloc", feature = "std"))] @@ -904,7 +837,7 @@ mod tests { let msg = Message::from_digest_slice(&msg[..]).unwrap(); // without normalization we expect this will fail - assert_eq!(secp.verify_ecdsa(&msg, &sig, &pk), Err(Error::IncorrectSignature)); + assert_eq!(secp.verify_ecdsa(&msg, &sig, &pk), Err(SysError {})); // after normalization it should pass sig.normalize_s(); assert_eq!(secp.verify_ecdsa(&msg, &sig, &pk), Ok(())); diff --git a/src/macros.rs b/src/macros.rs index 4111b3f0c..007e7f3a7 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -70,25 +70,6 @@ macro_rules! impl_non_secure_erase { }; } -/// Formats error. If `std` feature is OFF appends error source (delimited by `: `). We do this -/// because `e.source()` is only available in std builds, without this macro the error source is -/// lost for no-std builds. -macro_rules! write_err { - ($writer:expr, $string:literal $(, $args:expr),*; $source:expr) => { - { - #[cfg(feature = "std")] - { - let _ = &$source; // Prevents clippy warnings. - write!($writer, $string $(, $args)*) - } - #[cfg(not(feature = "std"))] - { - write!($writer, concat!($string, ": {}") $(, $args)*, $source) - } - } - } -} - /// Implements fast unstable comparison methods for `$ty`. macro_rules! impl_fast_comparisons { ($ty:ident) => { diff --git a/src/scalar.rs b/src/scalar.rs index 7ce54524b..3dca27444 100644 --- a/src/scalar.rs +++ b/src/scalar.rs @@ -123,12 +123,10 @@ impl From for Scalar { } /// Error returned when the value of scalar is invalid - larger than the curve order. -// Intentionally doesn't implement `Copy` to improve forward compatibility. -// Same reason for `non_exhaustive`. -#[allow(missing_copy_implementations)] -#[derive(Debug, Clone, Eq, PartialEq, Hash)] +#[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] -pub struct OutOfRangeError {} +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct OutOfRangeError; impl fmt::Display for OutOfRangeError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -137,4 +135,6 @@ impl fmt::Display for OutOfRangeError { } #[cfg(feature = "std")] -impl std::error::Error for OutOfRangeError {} +impl std::error::Error for OutOfRangeError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} diff --git a/src/schnorr.rs b/src/schnorr.rs index b7d496dc7..ab34b5dd9 100644 --- a/src/schnorr.rs +++ b/src/schnorr.rs @@ -12,7 +12,7 @@ use crate::ffi::{self, CPtr}; use crate::key::{Keypair, XOnlyPublicKey}; #[cfg(feature = "global-context")] use crate::SECP256K1; -use crate::{constants, hex, impl_array_newtype, Error, Message, Secp256k1, Signing, Verification}; +use crate::{constants, hex, impl_array_newtype, Message, Secp256k1, Signing, Verification}; /// Represents a schnorr signature. #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -61,13 +61,13 @@ impl fmt::Display for Signature { } impl str::FromStr for Signature { - type Err = Error; + type Err = SignatureError; fn from_str(s: &str) -> Result { let mut res = [0u8; constants::SCHNORR_SIGNATURE_SIZE]; match hex::from_hex(s, &mut res) { Ok(constants::SCHNORR_SIGNATURE_SIZE) => Signature::from_slice(&res[0..constants::SCHNORR_SIGNATURE_SIZE]), - _ => Err(Error::InvalidSignature), + _ => Err(SignatureError), } } } @@ -75,14 +75,14 @@ impl str::FromStr for Signature { impl Signature { /// Creates a `Signature` directly from a slice. #[inline] - pub fn from_slice(data: &[u8]) -> Result { + pub fn from_slice(data: &[u8]) -> Result { match data.len() { constants::SCHNORR_SIGNATURE_SIZE => { let mut ret = [0u8; constants::SCHNORR_SIGNATURE_SIZE]; ret[..].copy_from_slice(data); Ok(Signature(ret)) } - _ => Err(Error::InvalidSignature), + _ => Err(SignatureError), } } @@ -93,7 +93,7 @@ impl Signature { /// Verifies a schnorr signature for `msg` using `pk` and the global [`SECP256K1`] context. #[inline] #[cfg(feature = "global-context")] - pub fn verify(&self, msg: &Message, pk: &XOnlyPublicKey) -> Result<(), Error> { + pub fn verify(&self, msg: &Message, pk: &XOnlyPublicKey) -> Result<(), SignatureError> { SECP256K1.verify_schnorr(self, msg, pk) } } @@ -166,7 +166,7 @@ impl Secp256k1 { sig: &Signature, msg: &Message, pubkey: &XOnlyPublicKey, - ) -> Result<(), Error> { + ) -> Result<(), SignatureError> { unsafe { let ret = ffi::secp256k1_schnorrsig_verify( self.ctx.as_ptr(), @@ -179,12 +179,27 @@ impl Secp256k1 { if ret == 1 { Ok(()) } else { - Err(Error::InvalidSignature) + Err(SignatureError) } } } } +/// Signature is invalid. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive. +pub struct SignatureError; + +impl fmt::Display for SignatureError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("signature is invalid") } +} + +#[cfg(feature = "std")] +impl std::error::Error for SignatureError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + #[cfg(test)] #[allow(unused_imports)] mod tests { @@ -196,8 +211,8 @@ mod tests { use wasm_bindgen_test::wasm_bindgen_test as test; use super::*; + use crate::key::error::PublicKeyError; use crate::schnorr::{Keypair, Signature, XOnlyPublicKey}; - use crate::Error::InvalidPublicKey; use crate::{constants, hex, Message, Secp256k1, SecretKey}; #[cfg(all(not(secp256k1_fuzz), feature = "alloc"))] @@ -308,8 +323,8 @@ mod tests { #[test] fn test_pubkey_from_slice() { - assert_eq!(XOnlyPublicKey::from_slice(&[]), Err(InvalidPublicKey)); - assert_eq!(XOnlyPublicKey::from_slice(&[1, 2, 3]), Err(InvalidPublicKey)); + assert_eq!(XOnlyPublicKey::from_slice(&[]), Err(PublicKeyError)); + assert_eq!(XOnlyPublicKey::from_slice(&[1, 2, 3]), Err(PublicKeyError)); let pk = XOnlyPublicKey::from_slice(&[ 0xB3, 0x3C, 0xC9, 0xED, 0xC0, 0x96, 0xD0, 0xA8, 0x34, 0x16, 0x96, 0x4B, 0xD3, 0xC6, 0x24, 0x7B, 0x8F, 0xEC, 0xD2, 0x56, 0xE4, 0xEF, 0xA7, 0x87, 0x0D, 0x2C, 0x85, 0x4B, @@ -349,26 +364,26 @@ mod tests { // Bad sizes assert_eq!( XOnlyPublicKey::from_slice(&[0; constants::SCHNORR_PUBLIC_KEY_SIZE - 1]), - Err(InvalidPublicKey) + Err(PublicKeyError) ); assert_eq!( XOnlyPublicKey::from_slice(&[0; constants::SCHNORR_PUBLIC_KEY_SIZE + 1]), - Err(InvalidPublicKey) + Err(PublicKeyError) ); // Bad parse assert_eq!( XOnlyPublicKey::from_slice(&[0xff; constants::SCHNORR_PUBLIC_KEY_SIZE]), - Err(InvalidPublicKey) + Err(PublicKeyError) ); // In fuzzing mode restrictions on public key validity are much more // relaxed, thus the invalid check below is expected to fail. #[cfg(not(secp256k1_fuzz))] assert_eq!( XOnlyPublicKey::from_slice(&[0x55; constants::SCHNORR_PUBLIC_KEY_SIZE]), - Err(InvalidPublicKey) + Err(PublicKeyError) ); - assert_eq!(XOnlyPublicKey::from_slice(&[]), Err(InvalidPublicKey)); + assert_eq!(XOnlyPublicKey::from_slice(&[]), Err(PublicKeyError)); } #[test]