Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimise FFI in the public API #507

Merged
merged 1 commit into from
Nov 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions secp256k1-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -711,10 +711,12 @@ unsafe fn strlen(mut str_ptr: *const c_char) -> usize {
}


/// A trait for producing pointers that will always be valid in C. (assuming NULL pointer is a valid no-op)
/// Rust doesn't promise what pointers does it give to ZST (<https://doc.rust-lang.org/nomicon/exotic-sizes.html#zero-sized-types-zsts>)
/// In case the type is empty this trait will give a NULL pointer, which should be handled in C.
/// A trait for producing pointers that will always be valid in C (assuming NULL pointer is a valid
/// no-op).
///
/// Rust does not guarantee pointers to Zero Sized Types
/// (<https://doc.rust-lang.org/nomicon/exotic-sizes.html#zero-sized-types-zsts>). In case the type
/// is empty this trait will return a NULL pointer, which should be handled in C.
pub trait CPtr {
type Target;
fn as_c_ptr(&self) -> *const Self::Target;
Expand Down
2 changes: 1 addition & 1 deletion src/ecdh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ pub fn shared_secret_point(point: &PublicKey, scalar: &SecretKey) -> [u8; 64] {
ffi::secp256k1_ecdh(
ffi::secp256k1_context_no_precomp,
xy.as_mut_ptr(),
point.as_ptr(),
point.as_c_ptr(),
scalar.as_ptr(),
Some(c_callback),
ptr::null_mut(),
Expand Down
12 changes: 7 additions & 5 deletions src/ecdsa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,16 @@ impl Signature {

/// Obtains a raw pointer suitable for use with FFI functions
#[inline]
#[deprecated(since = "0.25.0", note = "Use Self::as_c_ptr if you need to access the FFI layer")]
pub fn as_ptr(&self) -> *const ffi::Signature {
&self.0
self.as_c_ptr()
}

/// Obtains a raw mutable pointer suitable for use with FFI functions
#[inline]
#[deprecated(since = "0.25.0", note = "Use Self::as_mut_c_ptr if you need to access the FFI layer")]
pub fn as_mut_ptr(&mut self) -> *mut ffi::Signature {
&mut self.0
self.as_mut_c_ptr()
}

#[inline]
Expand Down Expand Up @@ -199,11 +201,11 @@ impl CPtr for Signature {
type Target = ffi::Signature;

fn as_c_ptr(&self) -> *const Self::Target {
self.as_ptr()
&self.0
}

fn as_mut_c_ptr(&mut self) -> *mut Self::Target {
self.as_mut_ptr()
&mut self.0
}
}

Expand Down Expand Up @@ -315,7 +317,7 @@ impl<C: Signing> Secp256k1<C> {

counter += 1;
extra_entropy[..4].copy_from_slice(&counter.to_le_bytes());
entropy_p = extra_entropy.as_ptr().cast::<ffi::types::c_void>();
entropy_p = extra_entropy.as_c_ptr().cast::<ffi::types::c_void>();

// When fuzzing, these checks will usually spinloop forever, so just short-circuit them.
#[cfg(fuzzing)]
Expand Down
10 changes: 6 additions & 4 deletions src/ecdsa/recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,16 @@ impl RecoverableSignature {

/// Obtains a raw pointer suitable for use with FFI functions.
#[inline]
#[deprecated(since = "0.25.0", note = "Use Self::as_c_ptr if you need to access the FFI layer")]
pub fn as_ptr(&self) -> *const ffi::RecoverableSignature {
&self.0
self.as_c_ptr()
}

/// Obtains a raw mutable pointer suitable for use with FFI functions.
#[inline]
#[deprecated(since = "0.25.0", note = "Use Self::as_mut_c_ptr if you need to access the FFI layer")]
pub fn as_mut_ptr(&mut self) -> *mut ffi::RecoverableSignature {
&mut self.0
self.as_mut_c_ptr()
}

#[inline]
Expand Down Expand Up @@ -133,11 +135,11 @@ impl RecoverableSignature {
impl CPtr for RecoverableSignature {
type Target = ffi::RecoverableSignature;
fn as_c_ptr(&self) -> *const Self::Target {
self.as_ptr()
&self.0
}

fn as_mut_c_ptr(&mut self) -> *mut Self::Target {
self.as_mut_ptr()
&mut self.0
}
}

Expand Down
54 changes: 39 additions & 15 deletions src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ impl SecretKey {
let ret = ffi::secp256k1_keypair_sec(
ffi::secp256k1_context_no_precomp,
sk.as_mut_c_ptr(),
keypair.as_ptr()
keypair.as_c_ptr()
);
debug_assert_eq!(ret, 1);
}
Expand Down Expand Up @@ -423,14 +423,16 @@ impl<'de> serde::Deserialize<'de> for SecretKey {
impl PublicKey {
/// Obtains a raw const pointer suitable for use with FFI functions.
#[inline]
#[deprecated(since = "0.25.0", note = "Use Self::as_c_ptr if you need to access the FFI layer")]
pub fn as_ptr(&self) -> *const ffi::PublicKey {
&self.0
self.as_c_ptr()
}

/// Obtains a raw mutable pointer suitable for use with FFI functions.
#[inline]
#[deprecated(since = "0.25.0", note = "Use Self::as_mut_c_ptr if you need to access the FFI layer")]
pub fn as_mut_ptr(&mut self) -> *mut ffi::PublicKey {
&mut self.0
self.as_mut_c_ptr()
}

/// Creates a new public key from a [`SecretKey`].
Expand Down Expand Up @@ -507,7 +509,7 @@ impl PublicKey {
let ret = ffi::secp256k1_keypair_pub(
ffi::secp256k1_context_no_precomp,
&mut pk,
keypair.as_ptr()
keypair.as_c_ptr()
);
debug_assert_eq!(ret, 1);
PublicKey(pk)
Expand Down Expand Up @@ -733,7 +735,7 @@ impl PublicKey {
ffi::secp256k1_context_no_precomp,
&mut xonly_pk,
&mut pk_parity,
self.as_ptr(),
self.as_c_ptr(),
);
debug_assert_eq!(ret, 1);
let parity = Parity::from_i32(pk_parity).expect("should not panic, pk_parity is 0 or 1");
Expand All @@ -743,19 +745,26 @@ impl PublicKey {
}
}

/// This trait enables interaction with the FFI layer and even though it is part of the public API
/// normal users should never need to directly interact with FFI types.
impl CPtr for PublicKey {
type Target = ffi::PublicKey;

/// Obtains a const pointer suitable for use with FFI functions.
fn as_c_ptr(&self) -> *const Self::Target {
self.as_ptr()
&self.0
}

/// Obtains a mutable pointer suitable for use with FFI functions.
fn as_mut_c_ptr(&mut self) -> *mut Self::Target {
self.as_mut_ptr()
&mut self.0
}
}


/// Creates a new public key from a FFI public key
/// Creates a new public key from a FFI public key.
///
/// Note, normal users should never need to interact directly with FFI types.
impl From<ffi::PublicKey> for PublicKey {
#[inline]
fn from(pk: ffi::PublicKey) -> PublicKey {
Expand Down Expand Up @@ -845,14 +854,16 @@ impl_display_secret!(KeyPair);
impl KeyPair {
/// Obtains a raw const pointer suitable for use with FFI functions.
#[inline]
#[deprecated(since = "0.25.0", note = "Use Self::as_c_ptr if you need to access the FFI layer")]
pub fn as_ptr(&self) -> *const ffi::KeyPair {
&self.0
self.as_c_ptr()
}

/// Obtains a raw mutable pointer suitable for use with FFI functions.
#[inline]
#[deprecated(since = "0.25.0", note = "Use Self::as_mut_c_ptr if you need to access the FFI layer")]
pub fn as_mut_ptr(&mut self) -> *mut ffi::KeyPair {
&mut self.0
self.as_mut_c_ptr()
}

/// Creates a [`KeyPair`] directly from a Secp256k1 secret key.
Expand Down Expand Up @@ -1152,6 +1163,17 @@ impl<'de> serde::Deserialize<'de> for KeyPair {
}
}

impl CPtr for KeyPair {
type Target = ffi::KeyPair;
fn as_c_ptr(&self) -> *const Self::Target {
&self.0
}

fn as_mut_c_ptr(&mut self) -> *mut Self::Target {
&mut self.0
}
}

/// An x-only public key, used for verification of Schnorr signatures and serialized according to BIP-340.
///
/// # Serde support
Expand Down Expand Up @@ -1210,14 +1232,16 @@ impl str::FromStr for XOnlyPublicKey {
impl XOnlyPublicKey {
/// Obtains a raw const pointer suitable for use with FFI functions.
#[inline]
#[deprecated(since = "0.25.0", note = "Use Self::as_c_ptr if you need to access the FFI layer")]
pub fn as_ptr(&self) -> *const ffi::XOnlyPublicKey {
&self.0
self.as_c_ptr()
}

/// Obtains a raw mutable pointer suitable for use with FFI functions.
#[inline]
#[deprecated(since = "0.25.0", note = "Use Self::as_mut_c_ptr if you need to access the FFI layer")]
pub fn as_mut_ptr(&mut self) -> *mut ffi::XOnlyPublicKey {
&mut self.0
self.as_mut_c_ptr()
}

/// Returns the [`XOnlyPublicKey`] (and it's [`Parity`]) for `keypair`.
Expand All @@ -1230,7 +1254,7 @@ impl XOnlyPublicKey {
ffi::secp256k1_context_no_precomp,
&mut xonly_pk,
&mut pk_parity,
keypair.as_ptr(),
keypair.as_c_ptr(),
);
debug_assert_eq!(ret, 1);
let parity = Parity::from_i32(pk_parity).expect("should not panic, pk_parity is 0 or 1");
Expand Down Expand Up @@ -1570,11 +1594,11 @@ impl<'de> serde::Deserialize<'de> for Parity {
impl CPtr for XOnlyPublicKey {
type Target = ffi::XOnlyPublicKey;
fn as_c_ptr(&self) -> *const Self::Target {
self.as_ptr()
&self.0
}

fn as_mut_c_ptr(&mut self) -> *mut Self::Target {
self.as_mut_ptr()
&mut self.0
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/schnorr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ impl<C: Signing> Secp256k1<C> {
self.ctx,
sig.as_mut_c_ptr(),
msg.as_c_ptr(),
keypair.as_ptr(),
keypair.as_c_ptr(),
nonce_data,
)
);
Expand Down