Skip to content

Commit 9d5fcba

Browse files
committed
Merge #816: secp256k1-sys: remove CPtr impl for &[T] and associated weirdness
debae90 secp256k1-sys: remove CPtr impl for &[T] (Andrew Poelstra) 2cef561 key.rs: clean up some pointer mangling in sort_pubkeys() (Andrew Poelstra) 44bcb89 ellswift: remove unused `data` argument from `shared_secret` (Andrew Poelstra) 2139aff secp256k1-sys: fix type signature in secp256k1_ec_pubkey_sort (Andrew Poelstra) Pull request description: Fixes the signature of secp256k1_ec_pubkey_sort to use `*mut` rather than `*const`. Separately, our call to `secp256k1_ec_pubkey_sort`, while correctly starting from a `&mut [PublicKey]`, briefly constructed a `&[ffi::PublicKey]` slice before obtaining a `*mut` pointer from this. This was unsound for completely unnecessary reasons, and only compiled because we have an impl of the `CPtr` trait for `&[T]`, which provides a `as_mut_c_ptr()` method which basically cannot be safely used. Removing that `&[T]` impl revealed another case where it was (unnecessarily) being used: in `EllSwift::shared_secret`, where we use it to obtain a `*mut` pointer from a `&[u8]`, which we pass across the FFI boundary which simply drops it. ACKs for top commit: tcharding: ACK debae90 Tree-SHA512: 2245883cbd0695dcf495a075d530155c6f496cf7fe78c2560b567888dc5fb5f3160970c8bbd44abd67e240bccf41555fc71fd3f74daac11d446fc2b4783028ed
2 parents 5e622c3 + debae90 commit 9d5fcba

File tree

3 files changed

+8
-30
lines changed

3 files changed

+8
-30
lines changed

secp256k1-sys/src/lib.rs

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -915,7 +915,7 @@ extern "C" {
915915
#[cfg_attr(not(rust_secp_no_symbol_renaming), link_name = "rustsecp256k1_v0_11_ec_pubkey_sort")]
916916
pub fn secp256k1_ec_pubkey_sort(
917917
ctx: *const Context,
918-
pubkeys: *const *const PublicKey,
918+
pubkeys: *mut *const PublicKey,
919919
n_pubkeys: size_t,
920920
) -> c_int;
921921
}
@@ -1388,25 +1388,6 @@ impl<T> CPtr for [T] {
13881388
}
13891389
}
13901390

1391-
impl<T> CPtr for &[T] {
1392-
type Target = T;
1393-
fn as_c_ptr(&self) -> *const Self::Target {
1394-
if self.is_empty() {
1395-
ptr::null()
1396-
} else {
1397-
self.as_ptr()
1398-
}
1399-
}
1400-
1401-
fn as_mut_c_ptr(&mut self) -> *mut Self::Target {
1402-
if self.is_empty() {
1403-
ptr::null_mut()
1404-
} else {
1405-
self.as_ptr() as *mut Self::Target
1406-
}
1407-
}
1408-
}
1409-
14101391
impl CPtr for [u8; 32] {
14111392
type Target = u8;
14121393
fn as_c_ptr(&self) -> *const Self::Target { self.as_ptr() }

src/ellswift.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,8 @@ impl ElligatorSwift {
167167
/// let alice_es = ElligatorSwift::from_seckey(&secp, alice_sk, None);
168168
/// let bob_es = ElligatorSwift::from_seckey(&secp, bob_sk, None);
169169
///
170-
/// let alice_shared_secret = ElligatorSwift::shared_secret(alice_es, bob_es, alice_sk, Party::Initiator, None);
171-
/// let bob_shared_secret = ElligatorSwift::shared_secret(alice_es, bob_es, bob_sk, Party::Responder, None);
170+
/// let alice_shared_secret = ElligatorSwift::shared_secret(alice_es, bob_es, alice_sk, Party::Initiator);
171+
/// let bob_shared_secret = ElligatorSwift::shared_secret(alice_es, bob_es, bob_sk, Party::Responder);
172172
///
173173
/// assert_eq!(alice_shared_secret, bob_shared_secret);
174174
/// # }
@@ -178,7 +178,6 @@ impl ElligatorSwift {
178178
ellswift_b: ElligatorSwift,
179179
secret_key: SecretKey,
180180
party: impl Into<Party>,
181-
data: Option<&[u8]>,
182181
) -> ElligatorSwiftSharedSecret {
183182
let mut shared_secret = [0u8; 32];
184183
let p: Party = party.into();
@@ -191,7 +190,7 @@ impl ElligatorSwift {
191190
secret_key.as_c_ptr(),
192191
p.to_ffi_int(),
193192
ffi::secp256k1_ellswift_xdh_hash_function_bip324,
194-
data.as_c_ptr() as *mut c_void,
193+
ptr::null_mut(),
195194
);
196195
debug_assert_eq!(ret, 1);
197196
}
@@ -631,7 +630,7 @@ mod tests {
631630
let sec_key = SecretKey::from_byte_array(my_secret).unwrap();
632631
let initiator = if initiator == 0 { Party::Responder } else { Party::Initiator };
633632

634-
let shared = ElligatorSwift::shared_secret(el_a, el_b, sec_key, initiator, None);
633+
let shared = ElligatorSwift::shared_secret(el_a, el_b, sec_key, initiator);
635634

636635
assert_eq!(shared.0, shared_secret);
637636
}

src/key.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,11 +1644,9 @@ impl<C: Verification> Secp256k1<C> {
16441644
pub fn sort_pubkeys(&self, pubkeys: &mut [&PublicKey]) {
16451645
let cx = self.ctx().as_ptr();
16461646
unsafe {
1647-
let mut pubkeys_ref = core::slice::from_raw_parts(
1648-
pubkeys.as_c_ptr() as *mut *const ffi::PublicKey,
1649-
pubkeys.len(),
1650-
);
1651-
if secp256k1_ec_pubkey_sort(cx, pubkeys_ref.as_mut_c_ptr(), pubkeys_ref.len()) == 0 {
1647+
// SAFETY: `PublicKey` has repr(transparent) so we can convert to `ffi::PublicKey`
1648+
let pubkeys_ptr = pubkeys.as_mut_c_ptr() as *mut *const ffi::PublicKey;
1649+
if secp256k1_ec_pubkey_sort(cx, pubkeys_ptr, pubkeys.len()) == 0 {
16521650
unreachable!("Invalid public keys for sorting function")
16531651
}
16541652
}

0 commit comments

Comments
 (0)