Bump secp256k1 version to 0.22

[h4sh3d]

I tried to bump secp to version 0.22. Looks like there is breaking changes in no_aux signature generation.

I tracked the changes down to this change: https://github.com/rust-bitcoin/rust-secp256k1/commit/8294ea3f50f6de88f88b848c01eb60cab2a73241#diff-ca866735681590c6fc69b50b0a9100c0ea0c485b164e1dc508a14bf60496941b in src/schnorr.rs (last file in diff).

impl<C: Signing> Secp256k1<C> {
    fn sign_schnorr_helper(
        &self,
        msg: &Message,
        keypair: &KeyPair,
-       nonce_data: *const ffi::types::c_void,
+       nonce_data: *const ffi::types::c_uchar,
    ) -> Signature {
        unsafe {
            let mut sig = [0u8; constants::SCHNORR_SIGNATURE_SIZE];
            assert_eq!(
                1,
                ffi::secp256k1_schnorrsig_sign(
                    self.ctx,
                    sig.as_mut_c_ptr(),
                    msg.as_c_ptr(),
                    keypair.as_ptr(),
-                   ffi::secp256k1_nonce_function_bip340,
-                   nonce_data
+                   nonce_data,
                )
            );

            Signature(sig)
        }
    }

Function sign_schnorr_helper is then used in sign_schnorr_no_aux_rand:

    /// Create a schnorr signature without using any auxiliary random data.
    pub fn sign_schnorr_no_aux_rand(
        &self,
        msg: &Message,
        keypair: &KeyPair,
    ) -> Signature {
        self.sign_schnorr_helper(msg, keypair, ptr::null())
    }

---

I created a small test project to confirm that secp256k1 introduced that change at version 0.22.0.

[package]
name = "secp-tests"
version = "0.1.0"
edition = "2021"

[dependencies]
secp256k1-0213 = { package = "secp256k1", version = "=0.21.3", features = ["std", "global-context"] }
secp256k1-0220 = { package = "secp256k1", version = "=0.22.0", features = ["std", "global-context"] }

Two signature are created for the same key (secret key 1) and message (null array) as the failing test.

fn main() {
    let msg = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0];

    let secp = secp256k1_0213::SECP256K1;
    let key = secp256k1_0213::ONE_KEY;
    let keypair = secp256k1_0213::KeyPair::from_secret_key(&secp, key);
    let secp_msg = secp256k1_0213::Message::from_slice(&msg).unwrap();
    let sig_0213 = secp.sign_schnorr_no_aux_rand(&secp_msg, &keypair);
    println!("{:#?}", sig_0213);

    let secp = secp256k1_0220::SECP256K1;
    let key = secp256k1_0220::ONE_KEY;
    let keypair = secp256k1_0220::KeyPair::from_secret_key(&secp, key);
    let secp_msg = secp256k1_0220::Message::from_slice(&msg).unwrap();
    let sig_0220 = secp.sign_schnorr_no_aux_rand(&secp_msg, &keypair);
    println!("{:#?}", sig_0220);

    assert_eq!(sig_0213.as_ref(), sig_0220.as_ref());
}

It crashes and confirms what was suspected

Signature(ea6d5771020d414e9a05a6393dfb793b6697f1e6686eb4269a99d29abc0efc95064a4b0eccac29c0809d411bfa6a66a31c26fd1d5c48174ea2277c904def2744)
Signature(d2bcee6a047e765467f3ed7c3e8f55edcfa4a5fd37a9bcd064c1b5041599b187c3f9f2be0665d539e38eb75989b4bc3f6dd2d9d18c5c123613615d1731e0523e)
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `[234, 109, 87, 113, 2, 13, 65, 78, 154, 5, 166, 57, 61, 251, 121, 59, 102, 151, 241, 230, 104, 110, 180, 38, 154, 153, 210, 154, 188, 14, 252, 149, 6, 74, 75, 14, 204, 172, 41, 192, 128, 157, 65, 27, 250, 106, 102, 163, 28, 38, 253, 29, 92, 72, 23, 78, 162, 39, 124, 144, 77, 239, 39, 68]`,
 right: `[210, 188, 238, 106, 4, 126, 118, 84, 103, 243, 237, 124, 62, 143, 85, 237, 207, 164, 165, 253, 55, 169, 188, 208, 100, 193, 181, 4, 21, 153, 177, 135, 195, 249, 242, 190, 6, 101, 213, 57, 227, 142, 183, 89, 137, 180, 188, 63, 109, 210, 217, 209, 140, 92, 18, 54, 19, 97, 93, 23, 49, 224, 82, 62]`', src/main.rs:18:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I'll now investigate what changes are needed to update schnorr_fun and push a patch on this branch.

EDIT: explain ongoing investigation on deterministic schnorr signature breakage

[h4sh3d]

Nonce calculation is the real breaking change between 0.21.3 and 0.22.0.

master: https://github.com/rust-bitcoin/rust-secp256k1/blob/master/secp256k1-sys/depend/secp256k1/src/modules/schnorrsig/main_impl.h#L52-L97 0.21.3: https://github.com/rust-bitcoin/rust-secp256k1/blob/secp256k1-0.21.3/secp256k1-sys/depend/secp256k1/src/modules/schnorrsig/main_impl.h#L50-L92

Especially the treatment of null data where data is the aux_rand

    if (data != NULL) {
        // ...
    } else {
        /* Precomputed TaggedHash("BIP0340/aux", 0x0000...00); */
        static const unsigned char ZERO_MASK[32] = {
              84, 241, 105, 207, 201, 226, 229, 114,
             116, 128,  68,  31, 144, 186,  37, 196,
             136, 244,  97, 199,  11,  94, 165, 220,
             170, 247, 175, 105, 39,  10, 165,  20
        };
        for (i = 0; i < 32; i++) {
            masked_key[i] = key32[i] ^ ZERO_MASK[i];
        }
    }

Working on a patch now. It will probably be necessary to update some test vectors. This change has been introduced in rustsecp256k1_v0_5_0.

[h4sh3d]

Tests are now passing locally. I updated tree tests and added a new one.

[LLFourn]

Hey @h4sh3d thanks a lot for tracking this down. It's very important to figure out what's going on here. Is my understanding correct:

1. The BIP340 spec hasn't changed
2. libsecp256k1 changed its API somehow so that passing in a nulll as the aux trandomness leads to hashing a bunch of zeroes now rather than nothing at all. Do we know why they made this change?
3. We found this because output of deterministic signing algorithm is now different when no aux rand is provided.

If the above is the case we could consider simply dropping these tests or perhaps better just contriving the "aux" randomness so that it is 32 zeroes or whatever by using the same shim I made for the BIP340 spec tests.

[edit] this thing: https://github.com/LLFourn/secp256kfun/blob/f51990a0584affcc61fe4ddd52232d1cc7b34aac/schnorr_fun/tests/bip340.rs#L14 we could make it public with a warning no to use it unless you are just checking it has the same output as libsecp256k1.

[h4sh3d]

1. The BIP340 spec hasn't changed

No change in BIP340 (w.r.t. aux data) since that commit two years ago: https://github.com/bitcoin/bips/commit/cd19095fb039ee74dbb33225af21e83846e5a083 Aux definition changed from [..] or even an empty byte array of length 0. to [..] or even the constant array with 32 null bytes.

2. libsecp256k1 changed its API somehow so that passing in a nulll as the aux trandomness leads to hashing a bunch of zeroes now rather than nothing at all. Do we know why they made this change?

Yes, in that commit: https://github.com/bitcoin-core/secp256k1/commit/5324f8942dd322448fae6c9b225ecac2854fa7e2

3. We found this because output of deterministic signing algorithm is now different when no aux rand is provided.

Yes

[...] just contriving the "aux" randomness so that it is 32 zeroes or whatever by using the same shim I made for the BIP340 spec tests.

I can create in the schnorr module a deterministic nonce generator that implements BIP340 standard and use that one in tests.

EDIT: it looks like there is references here and there in docs to BIP340 and the deterministic nonce in secpfun impl. I don't know if having a second deterministic implementation is good or not.

[LLFourn]

So after thinking on this I think the right thing to do here is to leave the deterministic nonce generator as it is. Hashing stuff unnecessarily and xoring the key with a fixed value seems like a bad idea to me. So yes, please create a strict Bip340NoAux type that implements NonceGen hashes zeroes and xors it with the secret key. I think we can just put it in the against_c_lib.rs so that all changes are restricted to that file.