Skip to content

Commit 12b0abb

Browse files
apoelstraapoelstra
committed
make ffi::Type::new functions all unsafe, expand documentation
1 parent be9a78f commit 12b0abb

File tree

5 files changed

+120
-55
lines changed

5 files changed

+120
-55
lines changed

‎secp256k1-sys/src/lib.rs

Lines changed: 100 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,32 @@ impl_array_newtype!(PublicKey, c_uchar, 64);
106106
impl_raw_debug!(PublicKey);
107107

108108
impl PublicKey {
109-
/// Create a new (zeroed) public key usable for the FFI interface
110-
pub fn new() -> PublicKey { PublicKey([0; 64]) }
111-
}
109+
/// Creates an "uninitialized" FFI public key which is zeroed out
110+
///
111+
/// If you pass this to any FFI functions, except as an out-pointer,
112+
/// the result is likely to be an assertation failure and process
113+
/// termination.
114+
pub unsafe fn new() -> Self {
115+
Self::from_array_unchecked([0; 64])
116+
}
117+
118+
/// Create a new public key usable for the FFI interface from raw bytes
119+
///
120+
/// Does not check the validity of the underlying representation. If it is
121+
/// invalid the result may be assertation failures (and process aborts) from
122+
/// the underlying library. You should not use this method except with data
123+
/// that you obtained from the FFI interface of the same version of this
124+
/// library.
125+
pub unsafe fn from_array_unchecked(data: [c_uchar; 64]) -> Self {
126+
PublicKey(data)
127+
}
112128

113-
impl Default for PublicKey {
114-
fn default() -> Self {
115-
PublicKey::new()
129+
/// Returns the underlying FFI opaque representation of the public key
130+
///
131+
/// You should not use this unless you really know what you are doing. It is
132+
/// essentially only useful for extending the FFI interface itself.
133+
pub fn underlying_bytes(self) -> [c_uchar; 64] {
134+
self.0
116135
}
117136
}
118137

@@ -129,13 +148,32 @@ impl_array_newtype!(Signature, c_uchar, 64);
129148
impl_raw_debug!(Signature);
130149

131150
impl Signature {
132-
/// Create a new (zeroed) signature usable for the FFI interface
133-
pub fn new() -> Signature { Signature([0; 64]) }
134-
}
151+
/// Creates an "uninitialized" FFI signature which is zeroed out
152+
///
153+
/// If you pass this to any FFI functions, except as an out-pointer,
154+
/// the result is likely to be an assertation failure and process
155+
/// termination.
156+
pub unsafe fn new() -> Self {
157+
Self::from_array_unchecked([0; 64])
158+
}
135159

136-
impl Default for Signature {
137-
fn default() -> Self {
138-
Signature::new()
160+
/// Create a new signature usable for the FFI interface from raw bytes
161+
///
162+
/// Does not check the validity of the underlying representation. If it is
163+
/// invalid the result may be assertation failures (and process aborts) from
164+
/// the underlying library. You should not use this method except with data
165+
/// that you obtained from the FFI interface of the same version of this
166+
/// library.
167+
pub unsafe fn from_array_unchecked(data: [c_uchar; 64]) -> Self {
168+
Signature(data)
169+
}
170+
171+
/// Returns the underlying FFI opaque representation of the signature
172+
///
173+
/// You should not use this unless you really know what you are doing. It is
174+
/// essentially only useful for extending the FFI interface itself.
175+
pub fn underlying_bytes(self) -> [c_uchar; 64] {
176+
self.0
139177
}
140178
}
141179

@@ -145,11 +183,33 @@ impl_array_newtype!(XOnlyPublicKey, c_uchar, 64);
145183
impl_raw_debug!(XOnlyPublicKey);
146184

147185
impl XOnlyPublicKey {
148-
/// Create a new (zeroed) x-only public key usable for the FFI interface
149-
pub fn new() -> XOnlyPublicKey { XOnlyPublicKey([0; 64]) }
150-
pub fn from_array(data: [c_uchar; 64]) -> XOnlyPublicKey {
186+
/// Creates an "uninitialized" FFI x-only public key which is zeroed out
187+
///
188+
/// If you pass this to any FFI functions, except as an out-pointer,
189+
/// the result is likely to be an assertation failure and process
190+
/// termination.
191+
pub unsafe fn new() -> Self {
192+
Self::from_array_unchecked([0; 64])
193+
}
194+
195+
/// Create a new x-only public key usable for the FFI interface from raw bytes
196+
///
197+
/// Does not check the validity of the underlying representation. If it is
198+
/// invalid the result may be assertation failures (and process aborts) from
199+
/// the underlying library. You should not use this method except with data
200+
/// that you obtained from the FFI interface of the same version of this
201+
/// library.
202+
pub unsafe fn from_array_unchecked(data: [c_uchar; 64]) -> Self {
151203
XOnlyPublicKey(data)
152204
}
205+
206+
/// Returns the underlying FFI opaque representation of the x-only public key
207+
///
208+
/// You should not use this unless you really know what you are doing. It is
209+
/// essentially only useful for extending the FFI interface itself.
210+
pub fn underlying_bytes(self) -> [c_uchar; 64] {
211+
self.0
212+
}
153213
}
154214

155215
impl hash::Hash for XOnlyPublicKey {
@@ -158,23 +218,39 @@ impl hash::Hash for XOnlyPublicKey {
158218
}
159219
}
160220

161-
impl Default for XOnlyPublicKey {
162-
fn default() -> Self {
163-
XOnlyPublicKey::new()
164-
}
165-
}
166-
167221
#[repr(C)]
168222
pub struct KeyPair([c_uchar; 96]);
169223
impl_array_newtype!(KeyPair, c_uchar, 96);
170224
impl_raw_debug!(KeyPair);
171225

172226
impl KeyPair {
173-
/// Create a new (zeroed) key pair usable for the FFI interface
174-
pub fn new() -> KeyPair { KeyPair([0; 96]) }
175-
pub fn from_array(data: [c_uchar; 96]) -> KeyPair {
227+
/// Creates an "uninitialized" FFI keypair which is zeroed out
228+
///
229+
/// If you pass this to any FFI functions, except as an out-pointer,
230+
/// the result is likely to be an assertation failure and process
231+
/// termination.
232+
pub unsafe fn new() -> Self {
233+
Self::from_array_unchecked([0; 96])
234+
}
235+
236+
/// Create a new keypair usable for the FFI interface from raw bytes
237+
///
238+
/// Does not check the validity of the underlying representation. If it is
239+
/// invalid the result may be assertation failures (and process aborts) from
240+
/// the underlying library. You should not use this method except with data
241+
/// that you obtained from the FFI interface of the same version of this
242+
/// library.
243+
pub unsafe fn from_array_unchecked(data: [c_uchar; 96]) -> Self {
176244
KeyPair(data)
177245
}
246+
247+
/// Returns the underlying FFI opaque representation of the x-only public key
248+
///
249+
/// You should not use this unless you really know what you are doing. It is
250+
/// essentially only useful for extending the FFI interface itself.
251+
pub fn underlying_bytes(self) -> [c_uchar; 96] {
252+
self.0
253+
}
178254
}
179255

180256
impl hash::Hash for KeyPair {
@@ -183,12 +259,6 @@ impl hash::Hash for KeyPair {
183259
}
184260
}
185261

186-
impl Default for KeyPair {
187-
fn default() -> Self {
188-
KeyPair::new()
189-
}
190-
}
191-
192262
#[cfg(not(feature = "fuzztarget"))]
193263
extern "C" {
194264
/// Default ECDH hash function

‎src/key.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,23 +233,23 @@ impl PublicKey {
233233
pub fn from_secret_key<C: Signing>(secp: &Secp256k1<C>,
234234
sk: &SecretKey)
235235
-> PublicKey {
236-
let mut pk = ffi::PublicKey::new();
237236
unsafe {
237+
let mut pk = ffi::PublicKey::new();
238238
// We can assume the return value because it's not possible to construct
239239
// an invalid `SecretKey` without transmute trickery or something
240240
let res = ffi::secp256k1_ec_pubkey_create(secp.ctx, &mut pk, sk.as_c_ptr());
241241
debug_assert_eq!(res, 1);
242+
PublicKey(pk)
242243
}
243-
PublicKey(pk)
244244
}
245245

246246
/// Creates a public key directly from a slice
247247
#[inline]
248248
pub fn from_slice(data: &[u8]) -> Result<PublicKey, Error> {
249249
if data.is_empty() {return Err(Error::InvalidPublicKey);}
250250

251-
let mut pk = ffi::PublicKey::new();
252251
unsafe {
252+
let mut pk = ffi::PublicKey::new();
253253
if ffi::secp256k1_ec_pubkey_parse(
254254
ffi::secp256k1_context_no_precomp,
255255
&mut pk,

‎src/lib.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,8 @@ impl Signature {
269269
pub fn from_der(data: &[u8]) -> Result<Signature, Error> {
270270
if data.is_empty() {return Err(Error::InvalidSignature);}
271271

272-
let mut ret = ffi::Signature::new();
273-
274272
unsafe {
273+
let mut ret = ffi::Signature::new();
275274
if ffi::secp256k1_ecdsa_signature_parse_der(
276275
ffi::secp256k1_context_no_precomp,
277276
&mut ret,
@@ -288,12 +287,12 @@ impl Signature {
288287

289288
/// Converts a 64-byte compact-encoded byte slice to a signature
290289
pub fn from_compact(data: &[u8]) -> Result<Signature, Error> {
291-
let mut ret = ffi::Signature::new();
292290
if data.len() != 64 {
293291
return Err(Error::InvalidSignature)
294292
}
295293

296294
unsafe {
295+
let mut ret = ffi::Signature::new();
297296
if ffi::secp256k1_ecdsa_signature_parse_compact(
298297
ffi::secp256k1_context_no_precomp,
299298
&mut ret,
@@ -661,16 +660,15 @@ impl<C: Signing> Secp256k1<C> {
661660
pub fn sign(&self, msg: &Message, sk: &key::SecretKey)
662661
-> Signature {
663662

664-
let mut ret = ffi::Signature::new();
665663
unsafe {
664+
let mut ret = ffi::Signature::new();
666665
// We can assume the return value because it's not possible to construct
667666
// an invalid signature from a valid `Message` and `SecretKey`
668667
assert_eq!(ffi::secp256k1_ecdsa_sign(self.ctx, &mut ret, msg.as_c_ptr(),
669668
sk.as_c_ptr(), ffi::secp256k1_nonce_function_rfc6979,
670669
ptr::null()), 1);
670+
Signature::from(ret)
671671
}
672-
673-
Signature::from(ret)
674672
}
675673

676674
/// Generates a random keypair. Convenience function for `key::SecretKey::new`

‎src/recovery.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,16 +112,16 @@ impl RecoverableSignature {
112112
/// for verification
113113
#[inline]
114114
pub fn to_standard(&self) -> Signature {
115-
let mut ret = super_ffi::Signature::new();
116115
unsafe {
116+
let mut ret = super_ffi::Signature::new();
117117
let err = ffi::secp256k1_ecdsa_recoverable_signature_convert(
118118
super_ffi::secp256k1_context_no_precomp,
119119
&mut ret,
120120
self.as_c_ptr(),
121121
);
122122
assert!(err == 1);
123+
Signature(ret)
123124
}
124-
Signature(ret)
125125
}
126126
}
127127

@@ -178,15 +178,14 @@ impl<C: Verification> Secp256k1<C> {
178178
pub fn recover(&self, msg: &Message, sig: &RecoverableSignature)
179179
-> Result<key::PublicKey, Error> {
180180

181-
let mut pk = super_ffi::PublicKey::new();
182-
183181
unsafe {
182+
let mut pk = super_ffi::PublicKey::new();
184183
if ffi::secp256k1_ecdsa_recover(self.ctx, &mut pk,
185184
sig.as_c_ptr(), msg.as_c_ptr()) != 1 {
186185
return Err(Error::InvalidSignature);
187186
}
188-
};
189-
Ok(key::PublicKey::from(pk))
187+
Ok(key::PublicKey::from(pk))
188+
}
190189
}
191190
}
192191

‎src/schnorrsig.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ impl KeyPair {
123123
return Err(InvalidPublicKey);
124124
}
125125

126-
let mut kp = ffi::KeyPair::new();
127126
unsafe {
127+
let mut kp = ffi::KeyPair::new();
128128
if ffi::secp256k1_keypair_create(secp.ctx, &mut kp, data.as_c_ptr()) == 1 {
129129
Ok(KeyPair(kp))
130130
} else {
@@ -155,13 +155,13 @@ impl KeyPair {
155155
ret
156156
};
157157
let mut data = random_32_bytes();
158-
let mut keypair = ffi::KeyPair::new();
159158
unsafe {
159+
let mut keypair = ffi::KeyPair::new();
160160
while ffi::secp256k1_keypair_create(secp.ctx, &mut keypair, data.as_c_ptr()) == 0 {
161161
data = random_32_bytes();
162162
}
163+
KeyPair(keypair)
163164
}
164-
KeyPair(keypair)
165165
}
166166

167167
/// Tweak a keypair by adding the given tweak to the secret key and updating the
@@ -210,18 +210,18 @@ impl PublicKey {
210210
/// Creates a new Schnorr public key from a Schnorr key pair
211211
#[inline]
212212
pub fn from_keypair<C: Signing>(secp: &Secp256k1<C>, keypair: &KeyPair) -> PublicKey {
213-
let mut xonly_pk = ffi::XOnlyPublicKey::new();
214213
let mut pk_parity = 0;
215214
unsafe {
215+
let mut xonly_pk = ffi::XOnlyPublicKey::new();
216216
let ret = ffi::secp256k1_keypair_xonly_pub(
217217
secp.ctx,
218218
&mut xonly_pk,
219219
&mut pk_parity,
220220
keypair.as_ptr(),
221221
);
222222
debug_assert_eq!(ret, 1);
223+
PublicKey(xonly_pk)
223224
}
224-
PublicKey(xonly_pk)
225225
}
226226

227227
/// Creates a Schnorr public key directly from a slice
@@ -231,8 +231,8 @@ impl PublicKey {
231231
return Err(InvalidPublicKey);
232232
}
233233

234-
let mut pk = ffi::XOnlyPublicKey::new();
235234
unsafe {
235+
let mut pk = ffi::XOnlyPublicKey::new();
236236
if ffi::secp256k1_xonly_pubkey_parse(
237237
ffi::secp256k1_context_no_precomp,
238238
&mut pk,
@@ -326,9 +326,8 @@ impl From<ffi::XOnlyPublicKey> for PublicKey {
326326

327327
impl From<::key::PublicKey> for PublicKey {
328328
fn from(src: ::key::PublicKey) -> PublicKey {
329-
let mut pk = ffi::XOnlyPublicKey::new();
330-
331329
unsafe {
330+
let mut pk = ffi::XOnlyPublicKey::new();
332331
assert_eq!(
333332
1,
334333
ffi::secp256k1_xonly_pubkey_from_pubkey(
@@ -338,9 +337,8 @@ impl From<::key::PublicKey> for PublicKey {
338337
src.as_c_ptr(),
339338
)
340339
);
340+
PublicKey(pk)
341341
}
342-
343-
PublicKey(pk)
344342
}
345343
}
346344

0 commit comments

Comments
 (0)