Merge rust-bitcoin/rust-secp256k1#794: Followups to #716 (add musig2 API)

d611a4f25eba6bba9e0586e8234e0c184192468e musig: weaken/simplify warnings about nonce reuse (Andrew Poelstra)
8a43317781b1c48057d164561c9f16945bb26ead musig: add a bunch of unit tests (Andrew Poelstra)
40a8b654979a3e6cc307a7923b17dd4567302764 musig: explicitly panic when given an empty slice of pubkeys to aggregate (Andrew Poelstra)
ebdaec78597a00198e31895bbf8ba559a0095550 musig: clarify doc comment about aggregate nonce proxy (Andrew Poelstra)
dc04575e68a3e7181e1d022c263c24a4cdb3abd5 musig: a couple small improvements of byte array APIs (Andrew Poelstra)
c492c75f2d6a08ccae1761d8256b41cef8e0bd17 key: move pubkey_sort to method on Secp256k1; rename (Andrew Poelstra)
ec66003e68aa6dbfd25e1a72c095bd9586b776e9 musig: remove SessionSecretRand::new constructor (Andrew Poelstra)
6d938d30b790d7828579c3bfc1c0aa502998f3f3 musig: add missing Panics sections to docs (Andrew Poelstra)
00c8c75d1acc3e3b9d8d715e652778b5d8bba74c musig: remove outdated doc references to ZeroSession error (Andrew Poelstra)
3b0232a7ebb92d5c173ee0da8f44ef6c18514f11 musig: fix all the doctests (Andrew Poelstra)
4dd861fed4e5c2e2f41b42ddeeada4498233d33e stop using deprecated thread_rng (Andrew Poelstra)
9615ec8a8dcf192054e19a40bfc5ffe230ad03c3 context: whitelist new compiler warning (Andrew Poelstra)
7c56bcc187b7bcd2ce860fffc1d280398ba42108 clippy: whitelist a bunch of lints (Andrew Poelstra)
07922fd7ee4223f687ea1a4fe1fda75485327e10 musig: fix a couple FFI bindings (Andrew Poelstra)
f5f90af2455b50c484500f8e88f89410a8ed013c fmt: stop blacklisting secp256k1-sys; just fmt whole crate (Andrew Poelstra)

Pull request description:

  This PR needs to be merged before the next release because the existing code has one instance of UB when passing an empty array to the aggregate nonce function. (Ok, there's a rust panic in our alignment code so maybe no bad pointers make it across the C boundary and we're ok. But it's near-UB.)

  This PR is the first one I created using jujutsu. One thing I notice is that the tool encourages you to produce way more commits than you would with git. Most of these are small. Let me know if you want me to squash any.

ACKs for top commit:
  jlest01:
    ACK rust-bitcoin/rust-secp256k1@d611a4f
  jonasnick:
    ACK d611a4f25eba6bba9e0586e8234e0c184192468e modulo my comments on the PR (secret dependent branches) and that I only looked at the musig/libsecp-relevant bits.

Tree-SHA512: a504912639bcb6296bd6fdf7a0533464ce9e9064d1c2bf06bf142b7749b4aaf75ed4bae10f9912b92191c64a453bcf56bbd001e3c99ab383e02de4676c7c6a69

Author: william2332-limf

Cargo.toml

@@ -73,3 +73,13 @@ required-features = ["rand", "std"]
 [workspace]
 members = ["secp256k1-sys"]
 exclude = ["no_std_test"]
+
+[lints.clippy]
+# Exclude lints we don't think are valuable.
+large_enum_variant = "allow" # docs say "measure before paying attention to this"; why is it on by default??
+similar_names = "allow" # Too many (subjectively) false positives.
+uninlined_format_args = "allow" # This is a subjective style choice.
+indexing_slicing = "allow" # Too many false positives ... would be cool though
+match_bool = "allow" # Adds extra indentation and LOC.
+match_same_arms = "allow" # Collapses things that are conceptually unrelated to each other.
+must_use_candidate = "allow" # Useful for audit but many false positives.

examples/musig.rs

@@ -4,11 +4,11 @@ use secp256k1::musig::{
     new_nonce_pair, AggregatedNonce, KeyAggCache, PartialSignature, PublicNonce, Session,
     SessionSecretRand,
 };
-use secp256k1::{pubkey_sort, Keypair, Message, PublicKey, Scalar, Secp256k1, SecretKey};
+use secp256k1::{Keypair, Message, PublicKey, Scalar, Secp256k1, SecretKey};
 
 fn main() {
     let secp = Secp256k1::new();
-    let mut rng = rand::thread_rng();
+    let mut rng = rand::rng();
 
     let (seckey1, pubkey1) = secp.generate_keypair(&mut rng);
 
@@ -19,7 +19,7 @@ fn main() {
     let mut pubkeys_ref: Vec<&PublicKey> = pubkeys.iter().collect();
     let pubkeys_ref = pubkeys_ref.as_mut_slice();
 
-    pubkey_sort(&secp, pubkeys_ref);
+    secp.musig_sort_pubkeys(pubkeys_ref);
 
     let mut musig_key_agg_cache = KeyAggCache::new(&secp, pubkeys_ref);
 

rustfmt.toml

@@ -1,8 +1,3 @@
-# Eventually this shoud be: ignore = []
-ignore = [
-       "secp256k1-sys"
-]
-
 hard_tabs = false
 tab_spaces = 4
 newline_style = "Auto"

secp256k1-sys/Cargo.toml

@@ -35,3 +35,13 @@ alloc = []
 
 [lints.rust]
 unexpected_cfgs = { level = "deny", check-cfg = ['cfg(bench)', 'cfg(secp256k1_fuzz)', 'cfg(rust_secp_no_symbol_renaming)'] }
+
+[lints.clippy]
+# Exclude lints we don't think are valuable.
+large_enum_variant = "allow" # docs say "measure before paying attention to this"; why is it on by default??
+similar_names = "allow" # Too many (subjectively) false positives.
+uninlined_format_args = "allow" # This is a subjective style choice.
+indexing_slicing = "allow" # Too many false positives ... would be cool though
+match_bool = "allow" # Adds extra indentation and LOC.
+match_same_arms = "allow" # Collapses things that are conceptually unrelated to each other.
+must_use_candidate = "allow" # Useful for audit but many false positives.

secp256k1-sys/build.rs

@@ -16,21 +16,22 @@ use std::env;
 fn main() {
     // Actual build
     let mut base_config = cc::Build::new();
-    base_config.include("depend/secp256k1/")
-               .include("depend/secp256k1/include")
-               .include("depend/secp256k1/src")
-               .flag_if_supported("-Wno-unused-function") // some ecmult stuff is defined but not used upstream
-               .flag_if_supported("-Wno-unused-parameter") // patching out printf causes this warning
-               .define("SECP256K1_API", Some(""))
-               .define("ENABLE_MODULE_ECDH", Some("1"))
-               .define("ENABLE_MODULE_SCHNORRSIG", Some("1"))
-               .define("ENABLE_MODULE_EXTRAKEYS", Some("1"))
-               .define("ENABLE_MODULE_ELLSWIFT", Some("1"))
-               .define("ENABLE_MODULE_MUSIG", Some("1"))
-               // upstream sometimes introduces calls to printf, which we cannot compile
-               // with WASM due to its lack of libc. printf is never necessary and we can
-               // just #define it away.
-               .define("printf(...)", Some(""));
+    base_config
+        .include("depend/secp256k1/")
+        .include("depend/secp256k1/include")
+        .include("depend/secp256k1/src")
+        .flag_if_supported("-Wno-unused-function") // some ecmult stuff is defined but not used upstream
+        .flag_if_supported("-Wno-unused-parameter") // patching out printf causes this warning
+        .define("SECP256K1_API", Some(""))
+        .define("ENABLE_MODULE_ECDH", Some("1"))
+        .define("ENABLE_MODULE_SCHNORRSIG", Some("1"))
+        .define("ENABLE_MODULE_EXTRAKEYS", Some("1"))
+        .define("ENABLE_MODULE_ELLSWIFT", Some("1"))
+        .define("ENABLE_MODULE_MUSIG", Some("1"))
+        // upstream sometimes introduces calls to printf, which we cannot compile
+        // with WASM due to its lack of libc. printf is never necessary and we can
+        // just #define it away.
+        .define("printf(...)", Some(""));
 
     if cfg!(feature = "lowmemory") {
         base_config.define("ECMULT_WINDOW_SIZE", Some("4")); // A low-enough value to consume negligible memory
@@ -45,15 +46,15 @@ fn main() {
 
     // WASM headers and size/align defines.
     if env::var("CARGO_CFG_TARGET_ARCH").unwrap() == "wasm32" {
-        base_config.include("wasm/wasm-sysroot")
-                   .file("wasm/wasm.c");
+        base_config.include("wasm/wasm-sysroot").file("wasm/wasm.c");
     }
 
     // secp256k1
-    base_config.file("depend/secp256k1/contrib/lax_der_parsing.c")
-               .file("depend/secp256k1/src/precomputed_ecmult_gen.c")
-               .file("depend/secp256k1/src/precomputed_ecmult.c")
-               .file("depend/secp256k1/src/secp256k1.c");
+    base_config
+        .file("depend/secp256k1/contrib/lax_der_parsing.c")
+        .file("depend/secp256k1/src/precomputed_ecmult_gen.c")
+        .file("depend/secp256k1/src/precomputed_ecmult.c")
+        .file("depend/secp256k1/src/secp256k1.c");
 
     if base_config.try_compile("libsecp256k1.a").is_err() {
         // Some embedded platforms may not have, eg, string.h available, so if the build fails
@@ -63,4 +64,3 @@ fn main() {
         base_config.compile("libsecp256k1.a");
     }
 }
-