Skip to content

Commit

Permalink
Split general error into specific
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tcharding committed Nov 6, 2023
1 parent ec02f3f commit f71b678
Show file tree
Hide file tree
Showing 15 changed files with 1,028 additions and 309 deletions.
4 changes: 3 additions & 1 deletion examples/sign_verify_recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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>(
Expand Down
32 changes: 26 additions & 6 deletions src/context.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -320,14 +321,33 @@ 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 {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
}

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) };
Expand All @@ -343,7 +363,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.
Expand Down Expand Up @@ -378,7 +398,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)
}

Expand All @@ -402,7 +422,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)
}

Expand Down
27 changes: 21 additions & 6 deletions src/ecdh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::ffi::{self, CPtr};
use crate::key::{PublicKey, SecretKey};
use crate::{constants, hex, Error};
use crate::{constants, hex};

// The logic for displaying shared secrets relies on this (see `secret.rs`).
const SHARED_SECRET_SIZE: usize = constants::SECRET_KEY_SIZE;
Expand Down Expand Up @@ -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 hex::from_hex(s, &mut res) {
Ok(SHARED_SECRET_SIZE) => Ok(SharedSecret::from_bytes(res)),
_ => Err(Error::InvalidSharedSecret),
_ => Err(SharedSecretError),
}
}
}
Expand Down Expand Up @@ -183,6 +183,21 @@ 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 {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
}

#[cfg(test)]
#[allow(unused_imports)]
mod tests {
Expand Down

0 comments on commit f71b678

Please sign in to comment.