Skip to content

Commit 2038b5e

Browse files
committed
Merge rust-bitcoin#798: More Musig2 followups
480370c ci: disable broken WASM and cross tests (Andrew Poelstra) 98bfb48 musig: make zero-check in SessionSecretRand::assume_unique constant time (Andrew Poelstra) d318169 musig: rename `musig_sort_pubkeys` to just `sort_pubkeys` (Andrew Poelstra) fdae0fd typo: leak -> lead (Andrew Poelstra) Pull request description: Addresses review comments from rust-bitcoin#794, and also disables a couple broken CI jobs. ACKs for top commit: jonasnick: ACK 480370c Tree-SHA512: 33e4849389fcefb9916a1dcb5234497a42aa9759f81b4954cac85002a26f1d763ddc5b5c38c62795be339b84ffdb3544c6b3e7408ef8d95b67289fd697d5f1e1
2 parents 2d6c4db + 480370c commit 2038b5e

File tree

5 files changed

+28
-19
lines changed

5 files changed

+28
-19
lines changed

.github/workflows/cross.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ jobs:
1717
# - i686-pc-windows-msvc # not supported by cross
1818
- i686-unknown-linux-gnu
1919
# - x86_64-apple-darwin # proprietary apple stuff
20-
- x86_64-pc-windows-gnu
20+
# - x86_64-pc-windows-gnu # seems to be broken 2025-06
2121
# - x86_64-pc-windows-msvc # not supported by cross
2222
- x86_64-unknown-linux-gnu
2323
############ Tier 2 with Host Tools

.github/workflows/rust.yml

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -253,19 +253,19 @@ jobs:
253253
- name: "Run test on i686"
254254
run: cargo test --target i686-unknown-linux-gnu
255255

256-
WASM:
257-
name: WASM - stable toolchain
258-
runs-on: ubuntu-latest
259-
strategy:
260-
fail-fast: false
261-
# Note we do not use the recent lock file for wasm testing.
262-
steps:
263-
- name: "Checkout repo"
264-
uses: actions/checkout@v4
265-
- name: "Select toolchain"
266-
uses: dtolnay/rust-toolchain@stable
267-
- name: "Run wasm script"
268-
run: ./contrib/wasm.sh
256+
# WASM:
257+
# name: WASM - stable toolchain
258+
# runs-on: ubuntu-latest
259+
# strategy:
260+
# fail-fast: false
261+
# # Note we do not use the recent lock file for wasm testing.
262+
# steps:
263+
# - name: "Checkout repo"
264+
# uses: actions/checkout@v4
265+
# - name: "Select toolchain"
266+
# uses: dtolnay/rust-toolchain@stable
267+
# - name: "Run wasm script"
268+
# run: ./contrib/wasm.sh
269269

270270
NoStd: # 1 job, run no-std test from script.
271271
name: no-std - nightly toolchain

examples/musig.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ fn main() {
1919
let mut pubkeys_ref: Vec<&PublicKey> = pubkeys.iter().collect();
2020
let pubkeys_ref = pubkeys_ref.as_mut_slice();
2121

22-
secp.musig_sort_pubkeys(pubkeys_ref);
22+
secp.sort_pubkeys(pubkeys_ref);
2323

2424
let mut musig_key_agg_cache = KeyAggCache::new(&secp, pubkeys_ref);
2525

src/key.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1620,6 +1620,8 @@ impl<'de> serde::Deserialize<'de> for XOnlyPublicKey {
16201620
impl<C: Verification> Secp256k1<C> {
16211621
/// Sort public keys using lexicographic (of compressed serialization) order.
16221622
///
1623+
/// This is the canonical way to sort public keys for use with Musig2.
1624+
///
16231625
/// Example:
16241626
///
16251627
/// ```rust
@@ -1636,10 +1638,10 @@ impl<C: Verification> Secp256k1<C> {
16361638
/// # let mut pubkeys_ref: Vec<&PublicKey> = pubkeys.iter().collect();
16371639
/// # let pubkeys_ref = pubkeys_ref.as_mut_slice();
16381640
/// #
1639-
/// # secp.musig_sort_pubkeys(pubkeys_ref);
1641+
/// # secp.sort_pubkeys(pubkeys_ref);
16401642
/// # }
16411643
/// ```
1642-
pub fn musig_sort_pubkeys(&self, pubkeys: &mut [&PublicKey]) {
1644+
pub fn sort_pubkeys(&self, pubkeys: &mut [&PublicKey]) {
16431645
let cx = self.ctx().as_ptr();
16441646
unsafe {
16451647
let mut pubkeys_ref = core::slice::from_raw_parts(

src/musig.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,13 @@ impl SessionSecretRand {
6565
/// a random number generator, or if that is not available, the output of a
6666
/// stable monotonic counter.
6767
pub fn assume_unique_per_nonce_gen(inner: [u8; 32]) -> Self {
68-
assert_ne!(inner, [0; 32], "session secrets may not be all zero");
68+
// See SecretKey::eq for this "constant-time" algorithm for comparison against zero.
69+
let inner_or = inner.iter().fold(0, |accum, x| accum | *x);
70+
assert!(
71+
unsafe { core::ptr::read_volatile(&inner_or) != 0 },
72+
"session secrets may not be all zero",
73+
);
74+
6975
SessionSecretRand(inner)
7076
}
7177

@@ -337,6 +343,7 @@ impl KeyAggCache {
337343
/// ensures the same resulting `agg_pk` for the same multiset of pubkeys.
338344
/// This is useful to do before aggregating pubkeys, such that the order of pubkeys
339345
/// does not affect the combined public key.
346+
/// To do this, call [`Secp256k1::sort_pubkeys`].
340347
///
341348
/// # Returns
342349
///
@@ -660,7 +667,7 @@ impl SecretNonce {
660667
///
661668
/// # Warning:
662669
///
663-
/// Storing and re-creating this structure may leak to nonce reuse, which will leak
670+
/// Storing and re-creating this structure may lead to nonce reuse, which will leak
664671
/// your secret key in two signing sessions, even if neither session is completed.
665672
/// These functions should be avoided if possible and used with care.
666673
///

0 commit comments

Comments
 (0)