From 384ad311443d1dd5016d823bc9d83f0454cd2f5b Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Wed, 26 Jul 2023 14:00:28 +1000 Subject: [PATCH] Introduce specific errors for individual use cases This PR attempts to introduce a new convention for error handling. The aim of which is to make errors more informative to help debugging, to make the code easier to read, and to help stabalize the API. Each function that returns a `Result` returns is made to return an error that is either a struct or an enum in which _every_ variant is used. We then provide a general purpose error for the crate and conversion functions to it from all the other error types. --- examples/sign_verify_recovery.rs | 4 +- src/context.rs | 23 ++- src/ecdh.rs | 16 +- src/ecdsa/mod.rs | 48 +++-- src/ecdsa/recovery.rs | 34 ++-- src/ecdsa/serialized_signature.rs | 5 +- src/error.rs | 281 ++++++++++++++++++++++++++++++ src/key/error.rs | 85 +++++++++ src/{key.rs => key/mod.rs} | 179 +++++++++---------- src/lib.rs | 97 +++-------- src/macros.rs | 19 -- src/schnorr.rs | 40 +++-- 12 files changed, 571 insertions(+), 260 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 edcc7b3bc..42370b48d 100644 --- a/examples/sign_verify_recovery.rs +++ b/examples/sign_verify_recovery.rs @@ -4,6 +4,8 @@ extern crate secp256k1; use bitcoin_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,7 @@ 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) + Ok(secp.recover_ecdsa(&msg, &sig)?) } fn sign_recovery( diff --git a/src/context.rs b/src/context.rs index 61ab985ad..4d35fd728 100644 --- a/src/context.rs +++ b/src/context.rs @@ -8,7 +8,7 @@ use core::ptr::NonNull; pub use self::alloc_only::*; use crate::ffi::types::{c_uint, c_void, AlignedType}; use crate::ffi::{self, CPtr}; -use crate::{Error, Secp256k1}; +use crate::Secp256k1; #[cfg(all(feature = "global-context", feature = "std"))] /// Module implementing a singleton pattern for a global `Secp256k1` context. @@ -320,14 +320,25 @@ unsafe impl<'buf> PreallocatedContext<'buf> for AllPreallocated<'buf> {} unsafe impl<'buf> PreallocatedContext<'buf> for SignOnlyPreallocated<'buf> {} unsafe impl<'buf> PreallocatedContext<'buf> for VerifyOnlyPreallocated<'buf> {} +/// Not enough preallocated memory for the requested buffer size. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct NotEnoughMemoryError; + +crate::error::impl_simple_struct_error!( + NotEnoughMemoryError, + "not enough preallocated memory for the requested buffer size" +); + impl<'buf, C: Context + PreallocatedContext<'buf>> Secp256k1 { /// Lets you create a context with a preallocated buffer in a generic manner (sign/verify/all). - pub fn preallocated_gen_new(buf: &'buf mut [AlignedType]) -> Result, Error> { + pub fn preallocated_gen_new( + buf: &'buf mut [AlignedType], + ) -> Result, NotEnoughMemoryError> { #[cfg(target_arch = "wasm32")] ffi::types::sanity_checks_for_wasm(); if buf.len() < Self::preallocate_size_gen() { - return Err(Error::NotEnoughMemory); + return Err(NotEnoughMemoryError); } // Safe because buf is not null since it is not empty. let buf = unsafe { NonNull::new_unchecked(buf.as_mut_c_ptr() as *mut c_void) }; @@ -343,7 +354,7 @@ impl<'buf> Secp256k1> { /// Creates a new Secp256k1 context with all capabilities. pub fn preallocated_new( buf: &'buf mut [AlignedType], - ) -> Result>, Error> { + ) -> Result>, NotEnoughMemoryError> { Secp256k1::preallocated_gen_new(buf) } /// Uses the ffi `secp256k1_context_preallocated_size` to check the memory size needed for a context. @@ -378,7 +389,7 @@ impl<'buf> Secp256k1> { /// Creates a new Secp256k1 context that can only be used for signing. pub fn preallocated_signing_only( buf: &'buf mut [AlignedType], - ) -> Result>, Error> { + ) -> Result>, NotEnoughMemoryError> { Secp256k1::preallocated_gen_new(buf) } @@ -402,7 +413,7 @@ impl<'buf> Secp256k1> { /// Creates a new Secp256k1 context that can only be used for verification pub fn preallocated_verification_only( buf: &'buf mut [AlignedType], - ) -> Result>, Error> { + ) -> Result>, NotEnoughMemoryError> { Secp256k1::preallocated_gen_new(buf) } diff --git a/src/ecdh.rs b/src/ecdh.rs index 60990944a..ac6325b2c 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -8,9 +8,9 @@ use core::{ptr, str}; use secp256k1_sys::types::{c_int, c_uchar, c_void}; +use crate::constants; use crate::ffi::{self, CPtr}; use crate::key::{PublicKey, SecretKey}; -use crate::{constants, Error}; // The logic for displaying shared secrets relies on this (see `secret.rs`). const SHARED_SECRET_SIZE: usize = constants::SECRET_KEY_SIZE; @@ -65,25 +65,25 @@ 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 bytes.len() { SHARED_SECRET_SIZE => { let mut ret = [0u8; SHARED_SECRET_SIZE]; ret[..].copy_from_slice(bytes); Ok(SharedSecret(ret)) } - _ => Err(Error::InvalidSharedSecret), + _ => Err(SharedSecretError), } } } impl str::FromStr for SharedSecret { - type Err = Error; + type Err = SharedSecretError; fn from_str(s: &str) -> Result { let mut res = [0u8; SHARED_SECRET_SIZE]; match crate::from_hex(s, &mut res) { Ok(SHARED_SECRET_SIZE) => Ok(SharedSecret::from_bytes(res)), - _ => Err(Error::InvalidSharedSecret), + _ => Err(SharedSecretError), } } } @@ -183,6 +183,12 @@ impl<'de> ::serde::Deserialize<'de> for SharedSecret { } } +/// Share secret is invalid. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct SharedSecretError; + +crate::error::impl_simple_struct_error!(SharedSecretError, "shared secret is invalid"); + #[cfg(test)] #[allow(unused_imports)] mod tests { diff --git a/src/ecdsa/mod.rs b/src/ecdsa/mod.rs index 86c919586..c92dcd464 100644 --- a/src/ecdsa/mod.rs +++ b/src/ecdsa/mod.rs @@ -15,9 +15,7 @@ pub use self::serialized_signature::SerializedSignature; use crate::ffi::CPtr; #[cfg(feature = "global-context")] use crate::SECP256K1; -use crate::{ - ffi, from_hex, Error, Message, PublicKey, Secp256k1, SecretKey, Signing, Verification, -}; +use crate::{ffi, from_hex, Message, PublicKey, Secp256k1, SecretKey, Signing, Verification}; /// An ECDSA signature #[derive(Copy, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)] @@ -36,12 +34,12 @@ 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; 72]; match from_hex(s, &mut res) { Ok(x) => Signature::from_der(&res[0..x]), - _ => Err(Error::InvalidSignature), + _ => Err(SignatureError), } } } @@ -49,9 +47,9 @@ impl str::FromStr for Signature { 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); } unsafe { @@ -65,15 +63,15 @@ impl Signature { { Ok(Signature(ret)) } else { - Err(Error::InvalidSignature) + Err(SignatureError) } } } /// 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); } unsafe { @@ -86,7 +84,7 @@ impl Signature { { Ok(Signature(ret)) } else { - Err(Error::InvalidSignature) + Err(SignatureError) } } } @@ -95,9 +93,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); } unsafe { @@ -111,7 +109,7 @@ impl Signature { { Ok(Signature(ret)) } else { - Err(Error::InvalidSignature) + Err(SignatureError) } } } @@ -194,7 +192,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<(), SignatureError> { SECP256K1.verify_ecdsa(msg, self, pk) } } @@ -366,7 +364,7 @@ impl Secp256k1 { /// /// ```rust /// # #[cfg(feature = "rand-std")] { - /// # use secp256k1::{rand, Secp256k1, Message, Error}; + /// # use secp256k1::{ecdsa, rand, Secp256k1, Message}; /// # /// # let secp = Secp256k1::new(); /// # let (secret_key, public_key) = secp.generate_keypair(&mut rand::thread_rng()); @@ -376,7 +374,7 @@ impl Secp256k1 { /// assert_eq!(secp.verify_ecdsa(&message, &sig, &public_key), Ok(())); /// /// let message = Message::from_slice(&[0xcd; 32]).expect("32 bytes"); - /// assert_eq!(secp.verify_ecdsa(&message, &sig, &public_key), Err(Error::IncorrectSignature)); + /// assert_eq!(secp.verify_ecdsa(&message, &sig, &public_key), Err(ecdsa::SignatureError)); /// # } /// ``` #[inline] @@ -385,7 +383,7 @@ impl Secp256k1 { msg: &Message, sig: &Signature, pk: &PublicKey, - ) -> Result<(), Error> { + ) -> Result<(), SignatureError> { unsafe { if ffi::secp256k1_ecdsa_verify( self.ctx.as_ptr(), @@ -394,7 +392,7 @@ impl Secp256k1 { pk.as_c_ptr(), ) == 0 { - Err(Error::IncorrectSignature) + Err(SignatureError) } else { Ok(()) } @@ -429,3 +427,15 @@ pub(crate) fn der_length_check(sig: &ffi::Signature, max_len: usize) -> bool { } len <= max_len } + +/// Signature is invalid. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct SignatureError; + +crate::error::impl_simple_struct_error!(SignatureError, "signature is invalid"); + +/// Recovery ID is invalid. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct RecoveryIdError; + +crate::error::impl_simple_struct_error!(RecoveryIdError, "recover ID is invalid"); diff --git a/src/ecdsa/recovery.rs b/src/ecdsa/recovery.rs index ab337d016..c253452a5 100644 --- a/src/ecdsa/recovery.rs +++ b/src/ecdsa/recovery.rs @@ -7,10 +7,10 @@ use core::ptr; use self::super_ffi::CPtr; -use super::ffi as super_ffi; +use super::{ffi as super_ffi, RecoveryIdError, SignatureError}; use crate::ecdsa::Signature; 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 +23,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), + _ => Err(RecoveryIdError), } } @@ -39,16 +39,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); } let mut ret = ffi::RecoverableSignature::new(); unsafe { if data.len() != 64 { - Err(Error::InvalidSignature) + Err(SignatureError) } else if ffi::secp256k1_ecdsa_recoverable_signature_parse_compact( super_ffi::secp256k1_context_no_precomp, &mut ret, @@ -58,7 +61,7 @@ impl RecoverableSignature { { Ok(RecoverableSignature(ret)) } else { - Err(Error::InvalidSignature) + Err(SignatureError) } } } @@ -113,7 +116,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 +194,7 @@ impl Secp256k1 { &self, msg: &Message, sig: &RecoverableSignature, - ) -> Result { + ) -> Result { unsafe { let mut pk = super_ffi::PublicKey::new(); if ffi::secp256k1_ecdsa_recover( @@ -201,7 +204,7 @@ impl Secp256k1 { msg.as_c_ptr(), ) != 1 { - return Err(Error::InvalidSignature); + return Err(SignatureError); } Ok(key::PublicKey::from(pk)) } @@ -214,9 +217,9 @@ 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 +319,8 @@ mod tests { let msg = crate::random_32_bytes(&mut rand::thread_rng()); let msg = Message::from_slice(&msg).unwrap(); - assert_eq!(s.verify_ecdsa(&msg, &sig, &pk), Err(Error::IncorrectSignature)); + // TODO: Is this ok, or do we want IncorrectSignatureError as well? + assert_eq!(s.verify_ecdsa(&msg, &sig, &pk), Err(SignatureError)); let recovered_key = s.recover_ecdsa(&msg, &sigr).unwrap(); assert!(recovered_key != pk); @@ -366,7 +370,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)); // ...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 1b2a65a2d..a6ef85305 100644 --- a/src/ecdsa/serialized_signature.rs +++ b/src/ecdsa/serialized_signature.rs @@ -11,8 +11,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; @@ -98,7 +97,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/error.rs b/src/error.rs new file mode 100644 index 000000000..2fe7134b8 --- /dev/null +++ b/src/error.rs @@ -0,0 +1,281 @@ +// SPDX-License-Identifier: CC0-1.0 + +//! Error types and conversion functions. + +use core::fmt; + +use crate::context::NotEnoughMemoryError; +use crate::ecdh::SharedSecretError; +use crate::ecdsa::{self, RecoveryIdError}; +use crate::key::error::{ + ParityValueError, PublicKeyError, PublicKeySumError, SecretKeyError, TweakError, + XOnlyTweakError, +}; +use crate::{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`. +#[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) } + } + + impl From<$crate::NotEnoughMemoryError> for $error { + fn from(e: $crate::NotEnoughMemoryError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::MessageLengthError> for $error { + fn from(e: $crate::MessageLengthError) -> 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::RecoveryIdError> for $error { + fn from(e: $crate::RecoveryIdError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::SecretKeyError> for $error { + fn from(e: $crate::SecretKeyError) -> 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::TweakError> for $error { + fn from(e: $crate::TweakError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::ParityValueError> for $error { + fn from(e: $crate::ParityValueError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::XOnlyTweakError> for $error { + fn from(e: $crate::XOnlyTweakError) -> Self { Self::$variant(e.into()) } + } + + impl From<$crate::SharedSecretError> for $error { + fn from(e: $crate::SharedSecretError) -> 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, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum Error { + /// Not enough preallocated memory for the requested buffer size. + NotEnoughMemory(NotEnoughMemoryError), + /// Messages must be 32 bytes long. + MessageLength(MessageLengthError), + /// Schnorr signature is invalid. + SchnorrSignature(schnorr::SignatureError), + /// ECDSA signature is invalid. + EcdsaSignature(ecdsa::SignatureError), + /// Invalid recovery ID (ECDSA). + RecoveryId(RecoveryIdError), + /// Secret key is invalid. + SecretKey(SecretKeyError), + /// Public key is invalid. + PublicKey(PublicKeyError), + /// Public key summation is invalid. + PublicKeySum(PublicKeySumError), + /// Invalid key tweak. + Tweak(TweakError), + /// Invalid value for parity - must be 0 or 1. + ParityValue(ParityValueError), + /// X-only pubic key tweak failed. + XOnlyTweak(XOnlyTweakError), + /// Invalid shared secret. + SharedSecret(SharedSecretError), +} + +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 useful and does not + // contain redundant content. + match *self { + NotEnoughMemory(ref e) => write_err!(f, "not enough memory"; e), + MessageLength(ref e) => write_err!(f, "invalid message length"; e), + SchnorrSignature(ref e) => write_err!(f, "invalid schnorr sig"; e), + EcdsaSignature(ref e) => write_err!(f, "invalid ECDSA sig"; e), + RecoveryId(ref e) => write_err!(f, "invalid recovery ID (ECDSA)"; e), + SecretKey(ref e) => write_err!(f, "invalid secret key"; e), + PublicKey(ref e) => write_err!(f, "invalid public key"; e), + PublicKeySum(ref e) => write_err!(f, "invalid public key sum"; e), + Tweak(ref e) => write_err!(f, "invalid tweak"; e), + ParityValue(ref e) => write_err!(f, "invalid parity"; e), + XOnlyTweak(ref e) => write_err!(f, "x-only tweak error"; e), + SharedSecret(ref e) => write_err!(f, "invalid shared secret"; e), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + use Error::*; + + match *self { + NotEnoughMemory(ref e) => Some(e), + MessageLength(ref e) => Some(e), + SchnorrSignature(ref e) => Some(e), + EcdsaSignature(ref e) => Some(e), + RecoveryId(ref e) => Some(e), + SecretKey(ref e) => Some(e), + PublicKey(ref e) => Some(e), + PublicKeySum(ref e) => Some(e), + Tweak(ref e) => Some(e), + ParityValue(ref e) => Some(e), + XOnlyTweak(ref e) => Some(e), + SharedSecret(ref e) => Some(e), + } + } +} + +impl From for Error { + fn from(e: NotEnoughMemoryError) -> Self { Self::NotEnoughMemory(e) } +} + +impl From for Error { + fn from(e: MessageLengthError) -> Self { Self::MessageLength(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: RecoveryIdError) -> Self { Self::RecoveryId(e) } +} + +impl From for Error { + fn from(e: SecretKeyError) -> Self { Self::SecretKey(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: TweakError) -> Self { Self::Tweak(e) } +} + +impl From for Error { + fn from(e: ParityValueError) -> Self { Self::ParityValue(e) } +} + +impl From for Error { + fn from(e: XOnlyTweakError) -> Self { Self::XOnlyTweak(e) } +} + +impl From for Error { + fn from(e: SharedSecretError) -> Self { Self::SharedSecret(e) } +} + +macro_rules! impl_from_for_error_type { + ($error:ty, $variant:ident, $mod:ident::$inner:ident) => { + impl From<$mod::$inner> for $error { + fn from(e: $mod::$inner) -> Self { + use $error::*; + $variant(e) + } + } + }; + ($error:ty, $variant:ident, $inner:ident) => { + impl From<$inner> for $error { + fn from(e: $inner) -> Self { + use $error::*; + $variant(e) + } + } + }; +} +pub(crate) use impl_from_for_error_type; + +/// Implements `std::error::Error` and `fmt::Display` for a struct with no fields. +macro_rules! impl_simple_struct_error { + ($type:ty, $msg:literal) => { + $crate::error::impl_std_error!($type); + $crate::error::impl_display!($type, $msg); + }; +} +pub(crate) use impl_simple_struct_error; + +/// Impls `std::error::Error` for the specified type with appropriate attributes, +/// possibly returning source. +macro_rules! impl_std_error { + // No source available + ($type:ty) => { + #[cfg(feature = "std")] + impl std::error::Error for $type {} + }; + // Struct with $field as source + ($type:ty, $field:ident) => { + #[cfg(feature = "std")] + impl std::error::Error for $type { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.$field) } + } + }; +} +pub(crate) use impl_std_error; + +macro_rules! impl_display { + ($type:ty, $msg:literal) => { + impl core::fmt::Display for $type { + fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> { + f.write_str($msg) + } + } + }; +} +pub(crate) use impl_display; + +/// 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/key/error.rs b/src/key/error.rs new file mode 100644 index 000000000..fc3e257f2 --- /dev/null +++ b/src/key/error.rs @@ -0,0 +1,85 @@ +// 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(Clone, Copy, Debug, PartialEq, Eq)] +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), + } + } +} + +crate::error::impl_from_for_error_type!(XOnlyTweakError, Tweak, TweakError); +crate::error::impl_from_for_error_type!(XOnlyTweakError, PublicKey, PublicKeyError); +crate::error::impl_from_for_error_type!(XOnlyTweakError, ParityValue, ParityValueError); + +/// Secret key is invalid. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct SecretKeyError; + +crate::error::impl_simple_struct_error!(SecretKeyError, "secret key is invalid"); + +/// Public key is invalid. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct PublicKeyError; + +crate::error::impl_simple_struct_error!(PublicKeyError, "public key is invalid"); + +/// Public key summation is invalid. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct PublicKeySumError; + +crate::error::impl_simple_struct_error!(PublicKeySumError, "public key summation is invalid"); + +/// Invalid key tweak. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct TweakError; + +crate::error::impl_simple_struct_error!(TweakError, "invalid key tweak"); + +/// Invalid value for parity - must be 0 or 1. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +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) + } +} + +crate::error::impl_std_error!(ParityValueError); 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 2b7d8e9f9..f981edc6e 100644 --- a/src/key.rs +++ b/src/key/mod.rs @@ -12,13 +12,18 @@ use serde::ser::SerializeTuple; use crate::ffi::types::c_uint; use crate::ffi::{self, CPtr}; -use crate::Error::{self, InvalidPublicKey, InvalidPublicKeySum, InvalidSecretKey}; use crate::{constants, from_hex, schnorr, Message, Scalar, Secp256k1, Signing, Verification}; #[cfg(feature = "global-context")] use crate::{ecdsa, SECP256K1}; #[cfg(feature = "bitcoin_hashes")] use crate::{hashes, ThirtyTwoByteHash}; +pub mod error; +pub use self::error::{ + ParityValueError, PublicKeyError, PublicKeySumError, SecretKeyError, TweakError, + XOnlyTweakError, +}; + /// Secret 256-bit key used as `x` in an ECDSA signature. /// /// # Side channel attacks @@ -105,12 +110,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 from_hex(s, &mut res) { Ok(constants::SECRET_KEY_SIZE) => SecretKey::from_slice(&res), - _ => Err(Error::InvalidSecretKey), + _ => Err(SecretKeyError), } } } @@ -158,14 +163,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 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), } } } @@ -206,7 +211,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 { @@ -215,12 +220,12 @@ impl SecretKey { data.as_c_ptr(), ) == 0 { - return Err(InvalidSecretKey); + return Err(SecretKeyError); } } Ok(SecretKey(data)) } - Err(_) => Err(InvalidSecretKey), + Err(_) => Err(SecretKeyError), } } @@ -299,7 +304,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, @@ -307,7 +312,7 @@ impl SecretKey { tweak.as_c_ptr(), ) != 1 { - Err(Error::InvalidTweak) + Err(TweakError) } else { Ok(self) } @@ -320,7 +325,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, @@ -328,7 +333,7 @@ impl SecretKey { tweak.as_c_ptr(), ) != 1 { - Err(Error::InvalidTweak) + Err(TweakError) } else { Ok(self) } @@ -455,9 +460,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 { @@ -471,7 +476,7 @@ impl PublicKey { { Ok(PublicKey(pk)) } else { - Err(InvalidPublicKey) + Err(PublicKeyError) } } } @@ -571,14 +576,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) } } } @@ -593,14 +598,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) } } } @@ -624,7 +629,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]) } @@ -651,12 +656,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 { @@ -672,7 +677,7 @@ impl PublicKey { { Ok(PublicKey(ret)) } else { - Err(InvalidPublicKeySum) + Err(PublicKeySumError) } } } @@ -811,15 +816,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 { @@ -827,7 +832,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) } } } @@ -836,14 +841,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 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), } } @@ -851,10 +859,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) } @@ -925,7 +933,7 @@ impl KeyPair { mut self, secp: &Secp256k1, tweak: &Scalar, - ) -> Result { + ) -> Result { unsafe { let err = ffi::secp256k1_keypair_xonly_tweak_add( secp.ctx.as_ptr(), @@ -933,7 +941,7 @@ impl KeyPair { tweak.as_c_ptr(), ); if err != 1 { - return Err(Error::InvalidTweak); + return Err(TweakError); } Ok(self) @@ -998,7 +1006,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 { @@ -1110,13 +1118,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 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), } } } @@ -1159,12 +1167,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 { @@ -1177,7 +1185,7 @@ impl XOnlyPublicKey { { Ok(XOnlyPublicKey(pk)) } else { - Err(Error::InvalidPublicKey) + Err(PublicKeyError) } } } @@ -1228,7 +1236,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(); @@ -1239,7 +1247,7 @@ impl XOnlyPublicKey { tweak.as_c_ptr(), ); if err != 1 { - return Err(Error::InvalidTweak); + return Err(TweakError)?; } err = ffi::secp256k1_xonly_pubkey_from_pubkey( @@ -1249,7 +1257,7 @@ impl XOnlyPublicKey { &pubkey, ); if err == 0 { - return Err(Error::InvalidPublicKey); + return Err(PublicKeyError)?; } let parity = Parity::from_i32(pk_parity)?; @@ -1321,7 +1329,7 @@ impl XOnlyPublicKey { secp: &Secp256k1, msg: &Message, sig: &schnorr::Signature, - ) -> Result<(), Error> { + ) -> Result<(), schnorr::SignatureError> { secp.verify_schnorr(sig, msg, self) } } @@ -1350,7 +1358,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()) } @@ -1358,25 +1366,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) } } @@ -1405,27 +1413,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 { @@ -1536,8 +1523,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, from_hex, to_hex, Scalar}; #[cfg(not(secp256k1_fuzz))] @@ -1552,7 +1538,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()); @@ -1560,8 +1546,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, @@ -1604,13 +1590,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, @@ -1664,31 +1650,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] @@ -1696,22 +1679,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 14d65f224..f26685b63 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -161,6 +161,7 @@ mod key; pub mod constants; pub mod ecdh; pub mod ecdsa; +pub mod error; pub mod scalar; pub mod schnorr; #[cfg(feature = "serde")] @@ -180,11 +181,18 @@ pub use secp256k1_sys as ffi; #[cfg(feature = "serde")] pub use serde; -pub use crate::context::*; +pub use crate::context::*; // Includes NotEnoughMemoryError +pub use crate::ecdh::SharedSecretError; +pub use crate::ecdsa::RecoveryIdError; +pub use crate::error::Error; use crate::ffi::types::AlignedType; use crate::ffi::CPtr; #[cfg(feature = "bitcoin_hashes")] use crate::hashes::Hash; +pub use crate::key::error::{ + ParityValueError, PublicKeyError, PublicKeySumError, SecretKeyError, TweakError, + XOnlyTweakError, +}; pub use crate::key::{PublicKey, SecretKey, *}; pub use crate::scalar::Scalar; @@ -225,14 +233,14 @@ impl Message { /// the result of signing isn't a /// [secure signature](https://twitter.com/pwuille/status/1063582706288586752). #[inline] - pub fn from_slice(data: &[u8]) -> Result { + pub fn from_slice(data: &[u8]) -> Result { match data.len() { constants::MESSAGE_SIZE => { let mut ret = [0u8; constants::MESSAGE_SIZE]; ret[..].copy_from_slice(data); Ok(Message(ret)) } - _ => Err(Error::InvalidMessage), + len => Err(MessageLengthError(len)), } } @@ -278,73 +286,17 @@ 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), -} +/// Messages must be 32 bytes long. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct MessageLengthError(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), - } + 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), - } - } -} +crate::error::impl_std_error!(MessageLengthError); /// The secp256k1 engine, used to execute all signature operations. pub struct Secp256k1 { @@ -510,9 +462,8 @@ mod tests { #[cfg(target_arch = "wasm32")] use wasm_bindgen_test::wasm_bindgen_test as test; - #[allow(unused_imports)] // When building with no default features. use super::*; - use crate::{constants, ecdsa, from_hex, Error, Message}; + use crate::{constants, ecdsa, from_hex, Message}; #[cfg(feature = "alloc")] use crate::{ffi, PublicKey, Secp256k1, SecretKey}; @@ -830,27 +781,27 @@ mod tests { let msg = crate::random_32_bytes(&mut rand::thread_rng()); let msg = Message::from_slice(&msg).unwrap(); - assert_eq!(s.verify_ecdsa(&msg, &sig, &pk), Err(Error::IncorrectSignature)); + assert_eq!(s.verify_ecdsa(&msg, &sig, &pk), Err(ecdsa::SignatureError)); } #[test] fn test_bad_slice() { assert_eq!( ecdsa::Signature::from_der(&[0; constants::MAX_SIGNATURE_SIZE + 1]), - Err(Error::InvalidSignature) + Err(ecdsa::SignatureError) ); assert_eq!( ecdsa::Signature::from_der(&[0; constants::MAX_SIGNATURE_SIZE]), - Err(Error::InvalidSignature) + Err(ecdsa::SignatureError) ); assert_eq!( Message::from_slice(&[0; constants::MESSAGE_SIZE - 1]), - Err(Error::InvalidMessage) + Err(crate::MessageLengthError(31)) ); assert_eq!( Message::from_slice(&[0; constants::MESSAGE_SIZE + 1]), - Err(Error::InvalidMessage) + Err(MessageLengthError(33)) ); assert!(Message::from_slice(&[0; constants::MESSAGE_SIZE]).is_ok()); assert!(Message::from_slice(&[1; constants::MESSAGE_SIZE]).is_ok()); @@ -922,7 +873,7 @@ mod tests { let msg = Message::from_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(ecdsa::SignatureError)); // 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/schnorr.rs b/src/schnorr.rs index aa0270391..3f5c9d95c 100644 --- a/src/schnorr.rs +++ b/src/schnorr.rs @@ -12,9 +12,7 @@ use crate::ffi::{self, CPtr}; use crate::key::{KeyPair, XOnlyPublicKey}; #[cfg(feature = "global-context")] use crate::SECP256K1; -use crate::{ - constants, from_hex, impl_array_newtype, Error, Message, Secp256k1, Signing, Verification, -}; +use crate::{constants, from_hex, impl_array_newtype, Message, Secp256k1, Signing, Verification}; /// Represents a schnorr signature. #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -63,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 from_hex(s, &mut res) { Ok(constants::SCHNORR_SIGNATURE_SIZE) => Signature::from_slice(&res[0..constants::SCHNORR_SIGNATURE_SIZE]), - _ => Err(Error::InvalidSignature), + _ => Err(SignatureError), } } } @@ -77,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), } } @@ -95,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) } } @@ -168,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(), @@ -181,12 +179,18 @@ impl Secp256k1 { if ret == 1 { Ok(()) } else { - Err(Error::InvalidSignature) + Err(SignatureError) } } } } +/// Signature is invalid. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct SignatureError; + +crate::error::impl_simple_struct_error!(SignatureError, "signature is invalid"); + #[cfg(test)] #[allow(unused_imports)] mod tests { @@ -198,8 +202,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, from_hex, Message, Secp256k1, SecretKey}; #[cfg(all(not(secp256k1_fuzz), feature = "alloc"))] @@ -310,8 +314,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, @@ -351,26 +355,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]