Merge remote-tracking branch 'rust-bitcoin/master'

* rust-bitcoin/master:
  Fix no-std raw test, after removal of lang items
  Fix broken benchmarks
  Disable emscripten tests until they work again https://github.com/rust-lang/rust/issues/66916 https://github.com/rustwasm/team/issues/291
  Add constant of the prime of the curve field.
  Simplify callback logic to returning raw coordinates
  Removed no longer used dont_replace_c_symbols feature
  Fix wrong feature name external-symbols
  Fix missing return c_int in NonceFn

.travis.yml

@@ -1,7 +1,7 @@
 language: rust
-cache: 
+# cache:
-  directories:
+#   directories:
-  - cargo_web
+#   - cargo_web
 
 rust:
   - stable
@@ -45,12 +45,12 @@ script:
   - cargo run --example generate_keys --features=rand
   - if [ ${TRAVIS_RUST_VERSION} == "stable" ]; then cargo doc --verbose --features="rand,serde,recovery,endomorphism"; fi
   - if [ ${TRAVIS_RUST_VERSION} == "nightly" ]; then cargo test --verbose --benches --features=unstable; fi
-  - if [ ${TRAVIS_RUST_VERSION} == "nightly" -a "$TRAVIS_OS_NAME" = "linux" ]; then 
+  - if [ ${TRAVIS_RUST_VERSION} == "nightly" -a "$TRAVIS_OS_NAME" = "linux" ]; then
     cd no_std_test &&
     cargo run --release | grep -q "Verified Successfully";
     fi
-  - if [ ${TRAVIS_RUST_VERSION} == "stable" -a "$TRAVIS_OS_NAME" = "linux" ]; then 
+  - #if [ ${TRAVIS_RUST_VERSION} == "stable" -a "$TRAVIS_OS_NAME" = "linux" ]; then
-    CARGO_TARGET_DIR=cargo_web cargo install --verbose --force cargo-web && 
+    #CARGO_TARGET_DIR=cargo_web cargo install --verbose --force cargo-web &&
-    cargo web build --verbose --target=asmjs-unknown-emscripten && 
+    #cargo web build --verbose --target=asmjs-unknown-emscripten &&
-    cargo web test --verbose --target=asmjs-unknown-emscripten; 
+    #cargo web test --verbose --target=asmjs-unknown-emscripten;
-    fi
+    #fi

Cargo.toml

@@ -22,7 +22,7 @@ name = "secp256k1"
 path = "src/lib.rs"
 
 [features]
-unstable = []
+unstable = ["recovery", "rand-std"]
 default = ["std"]
 std = ["secp256k1-sys/std"]
 rand-std = ["rand/std"]

no_std_test/src/main.rs

@@ -14,13 +14,13 @@
 
 //! # secp256k1 no-std test.
 //! This binary is a short smallest rust code to produce a working binary *without libstd*.
-//! This gives us 2 things: 
+//! This gives us 2 things:
 //!     1. Test that the parts of the code that should work in a no-std enviroment actually work.
 //!     2. Test that we don't accidentally import libstd into `secp256k1`.
-//! 
+//!
 //! The first is tested using the following command `cargo run --release | grep -q "Verified Successfully"`.
 //! (Making sure that it successfully printed that. i.e. it didn't abort before that).
-//! 
+//!
 //! The second is tested by the fact that it compiles. if we accidentally link against libstd we should see the following error:
 //! `error[E0152]: duplicate lang item found`.
 //! Example:
@@ -33,11 +33,11 @@
 //!    |
 //!    = note: first defined in crate `panic_unwind` (which `std` depends on).
 //! ```
-//! 
+//!
-//! Notes: 
+//! Notes:
 //!     * Requires `panic=abort` and `--release` to not depend on libunwind(which is provided usually by libstd) https://github.com/rust-lang/rust/issues/47493
 //!     * Requires linking with `libc` for calling `printf`.
-//!     
+//!
 
 #![feature(lang_items)]
 #![feature(start)]
@@ -52,10 +52,10 @@ use core::fmt::{self, write, Write};
 use core::intrinsics;
 use core::panic::PanicInfo;
 
+use secp256k1::ecdh::SharedSecret;
 use secp256k1::rand::{self, RngCore};
 use secp256k1::serde::Serialize;
 use secp256k1::*;
-use secp256k1::ecdh::SharedSecret;
 
 use serde_cbor::de;
 use serde_cbor::ser::SliceWrite;
@@ -105,30 +105,17 @@ fn start(_argc: isize, _argv: *const *const u8) -> isize {
 
     let _ = SharedSecret::new(&public_key, &secret_key);
     let mut x_arr = [0u8; 32];
-    let y_arr = unsafe { SharedSecret::new_with_hash_no_panic(&public_key, &secret_key, |x,y| {
+    let y_arr = SharedSecret::new_with_hash(&public_key, &secret_key, |x,y| {
         x_arr = x;
         y.into()
-    })}.unwrap();
+    });
     assert_ne!(x_arr, [0u8; 32]);
     assert_ne!(&y_arr[..], &[0u8; 32][..]);
-    
 
     unsafe { libc::printf("Verified Successfully!\n\0".as_ptr() as _) };
     0
 }
 
-// These functions are used by the compiler, but not
-// for a bare-bones hello world. These are normally
-// provided by libstd.
-#[lang = "eh_personality"]
-#[no_mangle]
-pub extern "C" fn rust_eh_personality() {}
-
-// This function may be needed based on the compilation target.
-#[lang = "eh_unwind_resume"]
-#[no_mangle]
-pub extern "C" fn rust_eh_unwind_resume() {}
-
 const MAX_PRINT: usize = 511;
 struct Print {
     loc: usize,

secp256k1-sys/build.rs

@@ -63,7 +63,6 @@ fn main() {
     } else {
         base_config.define("ECMULT_WINDOW_SIZE", Some("15")); // This is the default in the configure file (`auto`)
     }
-    #[cfg(not(feature = "dont_replace_c_symbols"))]
     base_config.define("USE_EXTERNAL_DEFAULT_CALLBACKS", Some("1"));
     #[cfg(feature = "endomorphism")]
     base_config.define("USE_ENDOMORPHISM", Some("1"));

secp256k1-sys/src/lib.rs

@@ -63,7 +63,7 @@ pub type NonceFn = unsafe extern "C" fn(nonce32: *mut c_uchar,
                                         algo16: *const c_uchar,
                                         data: *mut c_void,
                                         attempt: c_uint,
-);
+) -> c_int;
 
 /// Hash function to use to post-process an ECDH point to get
 /// a shared secret.
@@ -295,7 +295,7 @@ extern "C" {
 // Returns: a newly created context object.
 //  In:      flags: which parts of the context to initialize.
 #[no_mangle]
-#[cfg(all(feature = "std", not(feature = "external_symbols")))]
+#[cfg(all(feature = "std", not(feature = "external-symbols")))]
 pub unsafe extern "C" fn rustsecp256k1_v0_1_1_context_create(flags: c_uint) -> *mut Context {
     use std::mem;
     assert!(mem::align_of::<usize>() >= mem::align_of::<u8>());
@@ -312,7 +312,7 @@ pub unsafe extern "C" fn rustsecp256k1_v0_1_1_context_create(flags: c_uint) -> *
     secp256k1_context_preallocated_create(ptr as *mut c_void, flags)
 }
 
-#[cfg(all(feature = "std", not(feature = "external_symbols")))]
+#[cfg(all(feature = "std", not(feature = "external-symbols")))]
 pub unsafe fn secp256k1_context_create(flags: c_uint) -> *mut Context {
     rustsecp256k1_v0_1_1_context_create(flags)
 }
@@ -324,7 +324,7 @@ pub unsafe fn secp256k1_context_create(flags: c_uint) -> *mut Context {
 /// The pointer shouldn't be used after passing to this function, consider it as passing it to `free()`.
 ///
 #[no_mangle]
-#[cfg(all(feature = "std", not(feature = "external_symbols")))]
+#[cfg(all(feature = "std", not(feature = "external-symbols")))]
 pub unsafe extern "C" fn rustsecp256k1_v0_1_1_context_destroy(ctx: *mut Context) {
     secp256k1_context_preallocated_destroy(ctx);
     let ctx: *mut usize = ctx as *mut usize;
@@ -335,7 +335,7 @@ pub unsafe extern "C" fn rustsecp256k1_v0_1_1_context_destroy(ctx: *mut Context)
     let _ = Box::from_raw(slice as *mut [usize]);
 }
 
-#[cfg(all(feature = "std", not(feature = "external_symbols")))]
+#[cfg(all(feature = "std", not(feature = "external-symbols")))]
 pub unsafe fn secp256k1_context_destroy(ctx: *mut Context) {
     rustsecp256k1_v0_1_1_context_destroy(ctx)
 }
@@ -360,7 +360,7 @@ pub unsafe fn secp256k1_context_destroy(ctx: *mut Context) {
 /// See also secp256k1_default_error_callback_fn.
 ///
 #[no_mangle]
-#[cfg(not(feature = "external_symbols"))]
+#[cfg(not(feature = "external-symbols"))]
 pub unsafe extern "C" fn rustsecp256k1_v0_1_1_default_illegal_callback_fn(message: *const c_char, _data: *mut c_void) {
     use core::str;
     let msg_slice = slice::from_raw_parts(message as *const u8, strlen(message));
@@ -383,7 +383,7 @@ pub unsafe extern "C" fn rustsecp256k1_v0_1_1_default_illegal_callback_fn(messag
 /// See also secp256k1_default_illegal_callback_fn.
 ///
 #[no_mangle]
-#[cfg(not(feature = "external_symbols"))]
+#[cfg(not(feature = "external-symbols"))]
 pub unsafe extern "C" fn rustsecp256k1_v0_1_1_default_error_callback_fn(message: *const c_char, _data: *mut c_void) {
     use core::str;
     let msg_slice = slice::from_raw_parts(message as *const u8, strlen(message));
@@ -491,13 +491,6 @@ mod fuzz_dummy {
         1
     }
 
-    // TODO secp256k1_context_set_illegal_callback
-    // TODO secp256k1_context_set_error_callback
-    // (Actually, I don't really want these exposed; if either of these
-    // are ever triggered it indicates a bug in rust-secp256k1, since
-    // one goal is to use Rust's type system to eliminate all possible
-    // bad inputs.)
-
     // Pubkeys
     /// Parse 33/65 byte pubkey into PublicKey, losing compressed information
     pub unsafe fn secp256k1_ec_pubkey_parse(cx: *const Context, pk: *mut PublicKey,

src/constants.rs

@@ -34,6 +34,14 @@ pub const MAX_SIGNATURE_SIZE: usize = 72;
 /// The maximum size of a compact signature
 pub const COMPACT_SIGNATURE_SIZE: usize = 64;
 
+/// The Prime for the secp256k1 field element.
+pub const FIELD_SIZE: [u8; 32] = [
+    0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+    0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+    0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+    0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, 0xfc, 0x2f
+];
+
 /// The order of the secp256k1 curve
 pub const CURVE_ORDER: [u8; 32] = [
     0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,

src/ecdh.rs

@@ -22,7 +22,6 @@ use core::ops::{FnMut, Deref};
 use key::{SecretKey, PublicKey};
 use ffi::{self, CPtr};
 use secp256k1_sys::types::{c_int, c_uchar, c_void};
-use Error;
 
 /// A tag used for recovering the public key from a compact signature
 #[derive(Copy, Clone)]
@@ -89,39 +88,12 @@ impl Deref for SharedSecret {
 }
 
 
-unsafe fn callback_logic<F>(output: *mut c_uchar, x: *const c_uchar, y: *const c_uchar, data: *mut c_void) -> c_int 
+unsafe extern "C" fn c_callback(output: *mut c_uchar, x: *const c_uchar, y: *const c_uchar, _data: *mut c_void) -> c_int {
-    where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret {
+    ptr::copy_nonoverlapping(x, output, 32);
-    let callback: &mut F = &mut *(data as *mut F);
+    ptr::copy_nonoverlapping(y, output.offset(32), 32);
-
+    1
-    let mut x_arr = [0; 32];
-    let mut y_arr = [0; 32];
-    ptr::copy_nonoverlapping(x, x_arr.as_mut_ptr(), 32);
-    ptr::copy_nonoverlapping(y, y_arr.as_mut_ptr(), 32);
-
-    let secret = callback(x_arr, y_arr);
-    ptr::copy_nonoverlapping(secret.as_ptr(), output as *mut u8, secret.len());
-
-    secret.len() as c_int
 }
 
-#[cfg(feature = "std")]
-unsafe extern "C" fn hash_callback_catch_unwind<F>(output: *mut c_uchar, x: *const c_uchar, y: *const c_uchar, data: *mut c_void) -> c_int
-    where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret {
-
-    let res = ::std::panic::catch_unwind(||callback_logic::<F>(output, x, y, data));
-    if let Ok(len) = res {
-        len
-    } else {
-        -1
-    }
-}
-
-unsafe extern "C" fn hash_callback_unsafe<F>(output: *mut c_uchar, x: *const c_uchar, y: *const c_uchar, data: *mut c_void) -> c_int
-    where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret {
-    callback_logic::<F>(output, x, y, data)
-}
-
-
 impl SharedSecret {
     /// Creates a new shared secret from a pubkey and secret key
     #[inline]
@@ -137,35 +109,17 @@ impl SharedSecret {
                 ptr::null_mut(),
             )
         };
-        debug_assert_eq!(res, 1); // The default `secp256k1_ecdh_hash_function_default` should always return 1.
+        // The default `secp256k1_ecdh_hash_function_default` should always return 1.
+        // and the scalar was verified to be valid(0 > scalar > group_order) via the type system
+        debug_assert_eq!(res, 1);
         ss.set_len(32); // The default hash function is SHA256, which is 32 bytes long.
         ss
     }
 
-    fn new_with_callback_internal<F>(point: &PublicKey, scalar: &SecretKey, mut closure: F, callback: ffi::EcdhHashFn) -> Result<SharedSecret, Error>
-        where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret {
-        let mut ss = SharedSecret::empty();
-
-        let res =  unsafe {
-            ffi::secp256k1_ecdh(
-                ffi::secp256k1_context_no_precomp,
-                ss.get_data_mut_ptr(),
-                point.as_ptr(),
-                scalar.as_ptr(),
-                callback,
-                &mut closure as *mut F as *mut c_void,
-            )
-        };
-        if res == -1 {
-            return Err(Error::CallbackPanicked);
-        }
-        debug_assert!(res >= 16); // 128 bit is the minimum for a secure hash function and the minimum we let users.
-        ss.set_len(res as usize);
-        Ok(ss)
-
-    }
 
     /// Creates a new shared secret from a pubkey and secret key with applied custom hash function
+    /// The custom hash function must be in the form of `fn(x: [u8;32], y: [u8;32]) -> SharedSecret`
+    /// `SharedSecret` can be easily created via the `From` impl from arrays.
     /// # Examples
     /// ```
     /// # use secp256k1::ecdh::SharedSecret;
@@ -182,43 +136,29 @@ impl SharedSecret {
     /// });
     ///
     /// ```
-    #[cfg(feature = "std")]
+    pub fn new_with_hash<F>(point: &PublicKey, scalar: &SecretKey, mut hash_function: F) -> SharedSecret
-    pub fn new_with_hash<F>(point: &PublicKey, scalar: &SecretKey, hash_function: F) -> Result<SharedSecret, Error>
         where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret {
-            Self::new_with_callback_internal(point, scalar, hash_function, hash_callback_catch_unwind::<F>)
+        let mut xy = [0u8; 64];
-    }
 
-    /// Creates a new shared secret from a pubkey and secret key with applied custom hash function
+        let res = unsafe {
-    /// Note that this function is the same as [`new_with_hash`]
+            ffi::secp256k1_ecdh(
-    ///
+                ffi::secp256k1_context_no_precomp,
-    /// # Safety
+                xy.as_mut_ptr(),
-    /// The function doesn't wrap the callback with [`catch_unwind`]
+                point.as_ptr(),
-    /// so if the callback panics it will panic through an FFI boundray which is [`Undefined Behavior`]
+                scalar.as_ptr(),
-    /// If possible you should use [`new_with_hash`] which does wrap the callback with [`catch_unwind`] so is safe to use.
+                c_callback,
-    ///
+                ptr::null_mut(),
-    /// [`catch_unwind`]: https://doc.rust-lang.org/std/panic/fn.catch_unwind.html
+            )
-    /// [`Undefined Behavior`]: https://doc.rust-lang.org/nomicon/ffi.html#ffi-and-panics
+        };
-    /// [`new_with_hash`]: #method.new_with_hash
+        // Our callback *always* returns 1.
-    /// # Examples
+        // and the scalar was verified to be valid(0 > scalar > group_order) via the type system
-    /// ```
+        debug_assert_eq!(res, 1);
-    /// # use secp256k1::ecdh::SharedSecret;
+
-    /// # use secp256k1::{Secp256k1, PublicKey, SecretKey};
+        let mut x = [0u8; 32];
-    /// # fn sha2(_a: &[u8], _b: &[u8]) -> [u8; 32] {[0u8; 32]}
+        let mut y = [0u8; 32];
-    /// # let secp = Secp256k1::signing_only();
+        x.copy_from_slice(&xy[..32]);
-    /// # let secret_key = SecretKey::from_slice(&[3u8; 32]).unwrap();
+        y.copy_from_slice(&xy[32..]);
-    /// # let secret_key2 = SecretKey::from_slice(&[7u8; 32]).unwrap();
+        hash_function(x, y)
-    /// # let public_key = PublicKey::from_secret_key(&secp, &secret_key2);
-    //
-    /// let secret = unsafe { SharedSecret::new_with_hash_no_panic(&public_key, &secret_key, |x,y| {
-    ///     let hash: [u8; 32] = sha2(&x,&y);
-    ///     hash.into()
-    /// })};
-    ///
-    ///
-    /// ```
-    pub unsafe fn new_with_hash_no_panic<F>(point: &PublicKey, scalar: &SecretKey, hash_function: F) -> Result<SharedSecret, Error>
-        where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret {
-        Self::new_with_callback_internal(point, scalar, hash_function, hash_callback_unsafe::<F>)
     }
 }
 
@@ -248,9 +188,9 @@ mod tests {
         let (sk1, pk1) = s.generate_keypair(&mut thread_rng());
         let (sk2, pk2) = s.generate_keypair(&mut thread_rng());
         
-        let sec1 = SharedSecret::new_with_hash(&pk1, &sk2, |x,_| x.into()).unwrap();
+        let sec1 = SharedSecret::new_with_hash(&pk1, &sk2, |x,_| x.into());
-        let sec2 = SharedSecret::new_with_hash(&pk2, &sk1, |x,_| x.into()).unwrap();
+        let sec2 = SharedSecret::new_with_hash(&pk2, &sk1, |x,_| x.into());
-        let sec_odd = SharedSecret::new_with_hash(&pk1, &sk1, |x,_| x.into()).unwrap();
+        let sec_odd = SharedSecret::new_with_hash(&pk1, &sk1, |x,_| x.into());
         assert_eq!(sec1, sec2);
         assert_ne!(sec_odd, sec2);
     }
@@ -262,32 +202,29 @@ mod tests {
         let expect_result: [u8; 64] = [123; 64];
         let mut x_out = [0u8; 32];
         let mut y_out = [0u8; 32];
-        let result = SharedSecret::new_with_hash(&pk1, &sk1, | x, y | {
+        let result = SharedSecret::new_with_hash(&pk1, &sk1, |x, y| {
             x_out = x;
             y_out = y;
             expect_result.into()
-        }).unwrap();
+        });
-        let result_unsafe = unsafe {SharedSecret::new_with_hash_no_panic(&pk1, &sk1, | x, y | {
-            x_out = x;
-            y_out = y;
-            expect_result.into()
-        }).unwrap()};
         assert_eq!(&expect_result[..], &result[..]);
-        assert_eq!(result, result_unsafe);
         assert_ne!(x_out, [0u8; 32]);
         assert_ne!(y_out, [0u8; 32]);
     }
 
     #[test]
-    fn ecdh_with_hash_callback_panic() {
+    fn test_c_callback() {
-        let s = Secp256k1::signing_only();
+        let x = [5u8; 32];
-        let (sk1, pk1) = s.generate_keypair(&mut thread_rng());
+        let y = [7u8; 32];
-        let mut res = [0u8; 48];
+        let mut output = [0u8; 64];
-        let result = SharedSecret::new_with_hash(&pk1, &sk1, | x, _ | {
+        let res = unsafe { super::c_callback(output.as_mut_ptr(), x.as_ptr(), y.as_ptr(), ::ptr::null_mut()) };
-            res.copy_from_slice(&x); // res.len() != x.len(). this will panic.
+        assert_eq!(res, 1);
-            res.into()
+        let mut new_x = [0u8; 32];
-        });
+        let mut new_y = [0u8; 32];
-        assert_eq!(result, Err(Error::CallbackPanicked));
+        new_x.copy_from_slice(&output[..32]);
+        new_y.copy_from_slice(&output[32..]);
+        assert_eq!(x, new_x);
+        assert_eq!(y, new_y);
     }
 }
 
@@ -304,7 +241,6 @@ mod benches {
         let s = Secp256k1::signing_only();
         let (sk, pk) = s.generate_keypair(&mut thread_rng());
 
-        let s = Secp256k1::new();
         bh.iter( || {
             let res = SharedSecret::new(&pk, &sk);
             black_box(res);

src/lib.rs

@@ -496,8 +496,6 @@ pub enum Error {
     InvalidTweak,
     /// Didn't pass enough memory to context creation with preallocated memory
     NotEnoughMemory,
-    /// The callback has panicked.
-    CallbackPanicked,
 }
 
 impl Error {
@@ -511,7 +509,6 @@ impl Error {
             Error::InvalidRecoveryId => "secp: bad recovery id",
             Error::InvalidTweak => "secp: bad tweak",
             Error::NotEnoughMemory => "secp: not enough memory allocated",
-            Error::CallbackPanicked => "secp: a callback passed has panicked",
         }
     }
 }
@@ -734,7 +731,6 @@ mod tests {
 
 
     #[test]
-    #[cfg(not(feature = "dont_replace_c_symbols"))]
     fn test_manual_create_destroy() {
         let ctx_full = unsafe { ffi::secp256k1_context_create(AllPreallocated::FLAGS) };
         let ctx_sign = unsafe { ffi::secp256k1_context_create(SignOnlyPreallocated::FLAGS) };

src/recovery.rs

@@ -363,6 +363,10 @@ mod tests {
 
 #[cfg(all(test, feature = "unstable"))]
 mod benches {
+    use rand::{thread_rng, RngCore};
+    use test::{Bencher, black_box};
+    use super::{Message, Secp256k1};
+
     #[bench]
     pub fn bench_recover(bh: &mut Bencher) {
         let s = Secp256k1::new();