Introduce specific errors for individual use cases

This patch attempts to introduce a new convention for error handling.

The aim 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.

Author: tcharding

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<C: Verification>(
     secp: &Secp256k1<C>,
     msg: &[u8],
@@ -15,7 +17,7 @@ fn recover<C: Verification>(
     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<C: Signing>(

src/context.rs

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: CC0-1.0
 
+use core::fmt;
 use core::marker::PhantomData;
 use core::mem::ManuallyDrop;
 use core::ptr::NonNull;
@@ -8,7 +9,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 +321,31 @@ 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(Debug, Clone, PartialEq, Eq)]
+#[non_exhaustive]
+#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive.
+pub struct NotEnoughMemoryError;
+
+impl fmt::Display for NotEnoughMemoryError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.write_str("not enough preallocated memory for the requested buffer size")
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for NotEnoughMemoryError {}
+
 impl<'buf, C: Context + PreallocatedContext<'buf>> Secp256k1<C> {
     /// 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<Secp256k1<C>, Error> {
+    pub fn preallocated_gen_new(
+        buf: &'buf mut [AlignedType],
+    ) -> Result<Secp256k1<C>, 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 +361,7 @@ impl<'buf> Secp256k1<AllPreallocated<'buf>> {
     /// Creates a new Secp256k1 context with all capabilities.
     pub fn preallocated_new(
         buf: &'buf mut [AlignedType],
-    ) -> Result<Secp256k1<AllPreallocated<'buf>>, Error> {
+    ) -> Result<Secp256k1<AllPreallocated<'buf>>, NotEnoughMemoryError> {
         Secp256k1::preallocated_gen_new(buf)
     }
     /// Uses the ffi `secp256k1_context_preallocated_size` to check the memory size needed for a context.
@@ -378,7 +396,7 @@ impl<'buf> Secp256k1<SignOnlyPreallocated<'buf>> {
     /// Creates a new Secp256k1 context that can only be used for signing.
     pub fn preallocated_signing_only(
         buf: &'buf mut [AlignedType],
-    ) -> Result<Secp256k1<SignOnlyPreallocated<'buf>>, Error> {
+    ) -> Result<Secp256k1<SignOnlyPreallocated<'buf>>, NotEnoughMemoryError> {
         Secp256k1::preallocated_gen_new(buf)
     }
 
@@ -402,7 +420,7 @@ impl<'buf> Secp256k1<VerifyOnlyPreallocated<'buf>> {
     /// Creates a new Secp256k1 context that can only be used for verification
     pub fn preallocated_verification_only(
         buf: &'buf mut [AlignedType],
-    ) -> Result<Secp256k1<VerifyOnlyPreallocated<'buf>>, Error> {
+    ) -> Result<Secp256k1<VerifyOnlyPreallocated<'buf>>, NotEnoughMemoryError> {
         Secp256k1::preallocated_gen_new(buf)
     }
 

src/ecdh.rs

@@ -4,13 +4,13 @@
 //!
 
 use core::borrow::Borrow;
-use core::{ptr, str};
+use core::{fmt, 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<SharedSecret, Error> {
+    pub fn from_slice(bytes: &[u8]) -> Result<SharedSecret, SharedSecretError> {
         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<Self, Self::Err> {
         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,19 @@ impl<'de> ::serde::Deserialize<'de> for SharedSecret {
     }
 }
 
+/// Share secret is invalid.
+#[derive(Debug, Clone, PartialEq, Eq)]
+#[non_exhaustive]
+#[allow(missing_copy_implementations)] // Don't implement Copy when we use non_exhaustive.
+pub struct SharedSecretError;
+
+impl fmt::Display for SharedSecretError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("shared secret is invalid") }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for SharedSecretError {}
+
 #[cfg(test)]
 #[allow(unused_imports)]
 mod tests {