Merge #542: Use NonNull for Secp256k1 inner context field

082c3bd Use NonNull for Secp256k1 inner context field (Tobin C. Harding)

Pull request description:

  For raw pointers that can never be null Rust provides the `core::ptr::NonNull` type. Our `Secp256k1` type has an inner field that is a non-null pointer; use `NonNull` for it.

  Fix: #534

ACKs for top commit:
  apoelstra:
    ACK 082c3bd

Tree-SHA512: 80e6a931bc2efaaa5f9d11f7407b45960f6db669137fbb4f835dff3607b1459d6ea2bc039a649460c14008381a10d095e18df7e3b7b6b4c4b85a360e0127eef0

Author: apoelstra

src/context.rs

@@ -1,5 +1,6 @@
 use core::marker::PhantomData;
 use core::mem::ManuallyDrop;
+use core::ptr::NonNull;
 
 #[cfg(feature = "alloc")]
 #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
@@ -116,6 +117,7 @@ mod private {
 #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))]
 mod alloc_only {
     use core::marker::PhantomData;
+    use core::ptr::NonNull;
 
     use super::private;
     use crate::alloc::alloc;
@@ -209,7 +211,10 @@ mod alloc_only {
             #[allow(unused_mut)] // ctx is not mutated under some feature combinations.
             let mut ctx = Secp256k1 {
                 ctx: unsafe {
-                    ffi::secp256k1_context_preallocated_create(ptr as *mut c_void, C::FLAGS)
+                    NonNull::new_unchecked(ffi::secp256k1_context_preallocated_create(
+                        ptr as *mut c_void,
+                        C::FLAGS,
+                    ))
                 },
                 phantom: PhantomData,
             };
@@ -261,15 +266,18 @@ mod alloc_only {
 
     impl<C: Context> Clone for Secp256k1<C> {
         fn clone(&self) -> Secp256k1<C> {
-            let size = unsafe { ffi::secp256k1_context_preallocated_clone_size(self.ctx as _) };
+            let size = unsafe { ffi::secp256k1_context_preallocated_clone_size(self.ctx.as_ptr()) };
             let layout = alloc::Layout::from_size_align(size, ALIGN_TO).unwrap();
             let ptr = unsafe { alloc::alloc(layout) };
             if ptr.is_null() {
                 alloc::handle_alloc_error(layout);
             }
             Secp256k1 {
                 ctx: unsafe {
-                    ffi::secp256k1_context_preallocated_clone(self.ctx, ptr as *mut c_void)
+                    NonNull::new_unchecked(ffi::secp256k1_context_preallocated_clone(
+                        self.ctx.as_ptr(),
+                        ptr as *mut c_void,
+                    ))
                 },
                 phantom: PhantomData,
             }
@@ -321,10 +329,10 @@ impl<'buf, C: Context + 'buf> Secp256k1<C> {
         }
         Ok(Secp256k1 {
             ctx: unsafe {
-                ffi::secp256k1_context_preallocated_create(
+                NonNull::new_unchecked(ffi::secp256k1_context_preallocated_create(
                     buf.as_mut_c_ptr() as *mut c_void,
                     C::FLAGS,
-                )
+                ))
             },
             phantom: PhantomData,
         })
@@ -355,7 +363,7 @@ impl<'buf> Secp256k1<AllPreallocated<'buf>> {
     pub unsafe fn from_raw_all(
         raw_ctx: *mut ffi::Context,
     ) -> ManuallyDrop<Secp256k1<AllPreallocated<'buf>>> {
-        ManuallyDrop::new(Secp256k1 { ctx: raw_ctx, phantom: PhantomData })
+        ManuallyDrop::new(Secp256k1 { ctx: NonNull::new_unchecked(raw_ctx), phantom: PhantomData })
     }
 }
 
@@ -386,7 +394,7 @@ impl<'buf> Secp256k1<SignOnlyPreallocated<'buf>> {
     pub unsafe fn from_raw_signing_only(
         raw_ctx: *mut ffi::Context,
     ) -> ManuallyDrop<Secp256k1<SignOnlyPreallocated<'buf>>> {
-        ManuallyDrop::new(Secp256k1 { ctx: raw_ctx, phantom: PhantomData })
+        ManuallyDrop::new(Secp256k1 { ctx: NonNull::new_unchecked(raw_ctx), phantom: PhantomData })
     }
 }
 
@@ -417,6 +425,6 @@ impl<'buf> Secp256k1<VerifyOnlyPreallocated<'buf>> {
     pub unsafe fn from_raw_verification_only(
         raw_ctx: *mut ffi::Context,
     ) -> ManuallyDrop<Secp256k1<VerifyOnlyPreallocated<'buf>>> {
-        ManuallyDrop::new(Secp256k1 { ctx: raw_ctx, phantom: PhantomData })
+        ManuallyDrop::new(Secp256k1 { ctx: NonNull::new_unchecked(raw_ctx), phantom: PhantomData })
     }
 }

src/ecdsa/mod.rs

@@ -258,7 +258,7 @@ impl<C: Signing> Secp256k1<C> {
             // an invalid signature from a valid `Message` and `SecretKey`
             assert_eq!(
                 ffi::secp256k1_ecdsa_sign(
-                    self.ctx,
+                    self.ctx.as_ptr(),
                     &mut ret,
                     msg.as_c_ptr(),
                     sk.as_c_ptr(),
@@ -307,7 +307,7 @@ impl<C: Signing> Secp256k1<C> {
                 // an invalid signature from a valid `Message` and `SecretKey`
                 assert_eq!(
                     ffi::secp256k1_ecdsa_sign(
-                        self.ctx,
+                        self.ctx.as_ptr(),
                         &mut ret,
                         msg.as_c_ptr(),
                         sk.as_c_ptr(),
@@ -388,8 +388,12 @@ impl<C: Verification> Secp256k1<C> {
         pk: &PublicKey,
     ) -> Result<(), Error> {
         unsafe {
-            if ffi::secp256k1_ecdsa_verify(self.ctx, sig.as_c_ptr(), msg.as_c_ptr(), pk.as_c_ptr())
-                == 0
+            if ffi::secp256k1_ecdsa_verify(
+                self.ctx.as_ptr(),
+                sig.as_c_ptr(),
+                msg.as_c_ptr(),
+                pk.as_c_ptr(),
+            ) == 0
             {
                 Err(Error::IncorrectSignature)
             } else {

src/ecdsa/recovery.rs

@@ -158,7 +158,7 @@ impl<C: Signing> Secp256k1<C> {
             // an invalid signature from a valid `Message` and `SecretKey`
             assert_eq!(
                 ffi::secp256k1_ecdsa_sign_recoverable(
-                    self.ctx,
+                    self.ctx.as_ptr(),
                     &mut ret,
                     msg.as_c_ptr(),
                     sk.as_c_ptr(),
@@ -208,7 +208,12 @@ impl<C: Verification> Secp256k1<C> {
     ) -> Result<key::PublicKey, Error> {
         unsafe {
             let mut pk = super_ffi::PublicKey::new();
-            if ffi::secp256k1_ecdsa_recover(self.ctx, &mut pk, sig.as_c_ptr(), msg.as_c_ptr()) != 1
+            if ffi::secp256k1_ecdsa_recover(
+                self.ctx.as_ptr(),
+                &mut pk,
+                sig.as_c_ptr(),
+                msg.as_c_ptr(),
+            ) != 1
             {
                 return Err(Error::InvalidSignature);
             }

src/key.rs

@@ -459,7 +459,7 @@ impl PublicKey {
             let mut pk = ffi::PublicKey::new();
             // We can assume the return value because it's not possible to construct
             // an invalid `SecretKey` without transmute trickery or something.
-            let res = ffi::secp256k1_ec_pubkey_create(secp.ctx, &mut pk, sk.as_c_ptr());
+            let res = ffi::secp256k1_ec_pubkey_create(secp.ctx.as_ptr(), &mut pk, sk.as_c_ptr());
             debug_assert_eq!(res, 1);
             PublicKey(pk)
         }
@@ -575,7 +575,7 @@ impl PublicKey {
     #[must_use = "you forgot to use the negated public key"]
     pub fn negate<C: Verification>(mut self, secp: &Secp256k1<C>) -> PublicKey {
         unsafe {
-            let res = ffi::secp256k1_ec_pubkey_negate(secp.ctx, &mut self.0);
+            let res = ffi::secp256k1_ec_pubkey_negate(secp.ctx.as_ptr(), &mut self.0);
             debug_assert_eq!(res, 1);
         }
         self
@@ -593,7 +593,9 @@ impl PublicKey {
         tweak: &Scalar,
     ) -> Result<PublicKey, Error> {
         unsafe {
-            if ffi::secp256k1_ec_pubkey_tweak_add(secp.ctx, &mut self.0, tweak.as_c_ptr()) == 1 {
+            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)
@@ -613,7 +615,9 @@ impl PublicKey {
         other: &Scalar,
     ) -> Result<PublicKey, Error> {
         unsafe {
-            if ffi::secp256k1_ec_pubkey_tweak_mul(secp.ctx, &mut self.0, other.as_c_ptr()) == 1 {
+            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)
@@ -817,7 +821,7 @@ impl KeyPair {
     pub fn from_secret_key<C: Signing>(secp: &Secp256k1<C>, sk: &SecretKey) -> KeyPair {
         unsafe {
             let mut kp = ffi::KeyPair::new();
-            if ffi::secp256k1_keypair_create(secp.ctx, &mut kp, sk.as_c_ptr()) == 1 {
+            if ffi::secp256k1_keypair_create(secp.ctx.as_ptr(), &mut kp, sk.as_c_ptr()) == 1 {
                 KeyPair(kp)
             } else {
                 panic!("the provided secret key is invalid: it is corrupted or was not produced by Secp256k1 library")
@@ -842,7 +846,7 @@ impl KeyPair {
 
         unsafe {
             let mut kp = ffi::KeyPair::new();
-            if ffi::secp256k1_keypair_create(secp.ctx, &mut kp, data.as_c_ptr()) == 1 {
+            if ffi::secp256k1_keypair_create(secp.ctx.as_ptr(), &mut kp, data.as_c_ptr()) == 1 {
                 Ok(KeyPair(kp))
             } else {
                 Err(Error::InvalidSecretKey)
@@ -895,7 +899,9 @@ impl KeyPair {
         let mut data = crate::random_32_bytes(rng);
         unsafe {
             let mut keypair = ffi::KeyPair::new();
-            while ffi::secp256k1_keypair_create(secp.ctx, &mut keypair, data.as_c_ptr()) == 0 {
+            while ffi::secp256k1_keypair_create(secp.ctx.as_ptr(), &mut keypair, data.as_c_ptr())
+                == 0
+            {
                 data = crate::random_32_bytes(rng);
             }
             KeyPair(keypair)
@@ -946,8 +952,11 @@ impl KeyPair {
         tweak: &Scalar,
     ) -> Result<KeyPair, Error> {
         unsafe {
-            let err =
-                ffi::secp256k1_keypair_xonly_tweak_add(secp.ctx, &mut self.0, tweak.as_c_ptr());
+            let err = ffi::secp256k1_keypair_xonly_tweak_add(
+                secp.ctx.as_ptr(),
+                &mut self.0,
+                tweak.as_c_ptr(),
+            );
             if err != 1 {
                 return Err(Error::InvalidTweak);
             }
@@ -1243,7 +1252,7 @@ impl XOnlyPublicKey {
         unsafe {
             let mut pubkey = ffi::PublicKey::new();
             let mut err = ffi::secp256k1_xonly_pubkey_tweak_add(
-                secp.ctx,
+                secp.ctx.as_ptr(),
                 &mut pubkey,
                 self.as_c_ptr(),
                 tweak.as_c_ptr(),
@@ -1253,7 +1262,7 @@ impl XOnlyPublicKey {
             }
 
             err = ffi::secp256k1_xonly_pubkey_from_pubkey(
-                secp.ctx,
+                secp.ctx.as_ptr(),
                 &mut self.0,
                 &mut pk_parity,
                 &pubkey,
@@ -1306,7 +1315,7 @@ impl XOnlyPublicKey {
         let tweaked_ser = tweaked_key.serialize();
         unsafe {
             let err = ffi::secp256k1_xonly_pubkey_tweak_add_check(
-                secp.ctx,
+                secp.ctx.as_ptr(),
                 tweaked_ser.as_c_ptr(),
                 tweaked_parity.to_i32(),
                 &self.0,

src/lib.rs

@@ -180,6 +180,7 @@ pub mod schnorr;
 mod serde_util;
 
 use core::marker::PhantomData;
+use core::ptr::NonNull;
 use core::{fmt, mem, str};
 
 #[cfg(feature = "bitcoin-hashes")]
@@ -369,7 +370,7 @@ impl std::error::Error for Error {
 
 /// The secp256k1 engine, used to execute all signature operations.
 pub struct Secp256k1<C: Context> {
-    ctx: *mut ffi::Context,
+    ctx: NonNull<ffi::Context>,
     phantom: PhantomData<C>,
 }
 
@@ -387,9 +388,10 @@ impl<C: Context> Eq for Secp256k1<C> {}
 impl<C: Context> Drop for Secp256k1<C> {
     fn drop(&mut self) {
         unsafe {
-            let size = ffi::secp256k1_context_preallocated_clone_size(self.ctx);
-            ffi::secp256k1_context_preallocated_destroy(self.ctx);
-            C::deallocate(self.ctx as _, size);
+            let size = ffi::secp256k1_context_preallocated_clone_size(self.ctx.as_ptr());
+            ffi::secp256k1_context_preallocated_destroy(self.ctx.as_ptr());
+
+            C::deallocate(self.ctx.as_ptr() as _, size);
         }
     }
 }
@@ -405,7 +407,7 @@ impl<C: Context> Secp256k1<C> {
     /// shouldn't be needed with normal usage of the library. It enables
     /// extending the Secp256k1 with more cryptographic algorithms outside of
     /// this crate.
-    pub fn ctx(&self) -> &*mut ffi::Context { &self.ctx }
+    pub fn ctx(&self) -> NonNull<ffi::Context> { self.ctx }
 
     /// Returns the required memory for a preallocated context buffer in a generic manner(sign/verify/all).
     pub fn preallocate_size_gen() -> usize {
@@ -432,7 +434,7 @@ impl<C: Context> Secp256k1<C> {
     /// see comment in libsecp256k1 commit d2275795f by Gregory Maxwell.
     pub fn seeded_randomize(&mut self, seed: &[u8; 32]) {
         unsafe {
-            let err = ffi::secp256k1_context_randomize(self.ctx, seed.as_c_ptr());
+            let err = ffi::secp256k1_context_randomize(self.ctx.as_ptr(), seed.as_c_ptr());
             // This function cannot fail; it has an error return for future-proofing.
             // We do not expose this error since it is impossible to hit, and we have
             // precedent for not exposing impossible errors (for example in
@@ -556,11 +558,12 @@ mod tests {
         let ctx_sign = unsafe { ffi::secp256k1_context_create(SignOnlyPreallocated::FLAGS) };
         let ctx_vrfy = unsafe { ffi::secp256k1_context_create(VerifyOnlyPreallocated::FLAGS) };
 
-        let full: Secp256k1<AllPreallocated> = Secp256k1 { ctx: ctx_full, phantom: PhantomData };
+        let full: Secp256k1<AllPreallocated> =
+            Secp256k1 { ctx: unsafe { NonNull::new_unchecked(ctx_full) }, phantom: PhantomData };
         let sign: Secp256k1<SignOnlyPreallocated> =
-            Secp256k1 { ctx: ctx_sign, phantom: PhantomData };
+            Secp256k1 { ctx: unsafe { NonNull::new_unchecked(ctx_sign) }, phantom: PhantomData };
         let vrfy: Secp256k1<VerifyOnlyPreallocated> =
-            Secp256k1 { ctx: ctx_vrfy, phantom: PhantomData };
+            Secp256k1 { ctx: unsafe { NonNull::new_unchecked(ctx_vrfy) }, phantom: PhantomData };
 
         let (sk, pk) = full.generate_keypair(&mut rand::thread_rng());
         let msg = Message::from_slice(&[2u8; 32]).unwrap();
@@ -590,9 +593,9 @@ mod tests {
         let ctx_sign = Secp256k1::signing_only();
         let ctx_vrfy = Secp256k1::verification_only();
 
-        let mut full = unsafe { Secp256k1::from_raw_all(ctx_full.ctx) };
-        let mut sign = unsafe { Secp256k1::from_raw_signing_only(ctx_sign.ctx) };
-        let mut vrfy = unsafe { Secp256k1::from_raw_verification_only(ctx_vrfy.ctx) };
+        let mut full = unsafe { Secp256k1::from_raw_all(ctx_full.ctx.as_ptr()) };
+        let mut sign = unsafe { Secp256k1::from_raw_signing_only(ctx_sign.ctx.as_ptr()) };
+        let mut vrfy = unsafe { Secp256k1::from_raw_verification_only(ctx_vrfy.ctx.as_ptr()) };
 
         let (sk, pk) = full.generate_keypair(&mut rand::thread_rng());
         let msg = Message::from_slice(&[2u8; 32]).unwrap();

src/schnorr.rs

@@ -107,7 +107,7 @@ impl<C: Signing> Secp256k1<C> {
             assert_eq!(
                 1,
                 ffi::secp256k1_schnorrsig_sign(
-                    self.ctx,
+                    self.ctx.as_ptr(),
                     sig.as_mut_c_ptr(),
                     msg.as_c_ptr(),
                     keypair.as_c_ptr(),
@@ -168,7 +168,7 @@ impl<C: Verification> Secp256k1<C> {
     ) -> Result<(), Error> {
         unsafe {
             let ret = ffi::secp256k1_schnorrsig_verify(
-                self.ctx,
+                self.ctx.as_ptr(),
                 sig.as_c_ptr(),
                 msg.as_c_ptr(),
                 32,