Use aux rand for ecdsa adaptor encrypt

[Tibo-lg]

@nkohen pointed out to me that the latest version of the ECDSA adaptor signatures use auxiliary random data to generate the adaptor signature similarly to what is done for Schnorr signatures in BIP340.

Unfortunately I realized that when porting my code from the old version to the new one I just used null values for the nonce data and function, which makes it impossible to pass auxiliary random data at the moment. In this PR I mimicked what I had done in rust-bitcoin/rust-secp256k1#237 when adding Schnorr signatures to rust-secp256k1. Again unfortunately, while I think that this is the best way to implement things, it is also a breaking change AFAICT since now the simple EcdsaAdaptorSignature::encrypt function requires compilation with rand-std feature.

Sorry for missing that! Let me know what you think would be the best way to fix it.

[jonasnick]

Not a big deal since ECDSA signers don't expect to get the same kind of synthetic nonces as BIP-340 signers do. Your fix looks good to me.

But I think we should mention somewhere what this auxiliary randomness is because otherwise it seems rather confusing to someone who has never seen this as it's also not clear where to look upw hat this means. Perhaps somehere, e.g. in the doc for encrypt, we could say This function derives a nonce using a similar process as described in BIP-340. Therefore, the nonce derivation process can be strengthened against side channel attacks by providing auxiliary randomness.

[apoelstra]

ack 065fd19

I like this approach. Agree with Jonas that we could probably document this better so that nostd users know that it's perfectly safe to use encrypt_no_aux_rand.

[thomaseizinger]

This can forward to encrypt_with_aux_rand instead.

[Tibo-lg]

Good point fixed

[thomaseizinger]

If we have such kind of helper functions, I would prefer them to be defined further down from where they are used. Makes it easier to scan through the code if the public API is defined first and followed by code that is only an implementation detail.

In this case though, I don't think we need a helper function at all. Instead, I think it is clearer if we prefer forwarding to other public functions (see other comment) plus for the no_aux usecase, just duplicate the invocation. DRY has a price in the form of indirection and in this case, I don't think it is worth it :)

[Tibo-lg]

Agreed, updated as suggested. Another solution I thought of would be to have a single function encrypt_no_std that takes an Option<&[u8; 32]> as pasted below but I don't know if the match would have some negative performance impact.

    pub fn encrypt_no_std<C: Signing>(
        secp: &Secp256k1<C>,
        msg: &Message,
        sk: &SecretKey,
        enckey: &PublicKey,
        aux_rand: Option<&[u8; 32]>,
    ) -> EcdsaAdaptorSignature {
        let mut adaptor_sig = ffi::EcdsaAdaptorSignature::new();

        let aux_rand_ptr = match aux_rand {
            Some(aux) => aux.as_c_ptr() as *mut ffi::types::c_void,
            None => ptr::null_mut(),
        };

        unsafe {
            debug_assert!(
                ffi::secp256k1_ecdsa_adaptor_encrypt(
                    *secp.ctx(),
                    &mut adaptor_sig,
                    sk.as_c_ptr(),
                    enckey.as_c_ptr(),
                    msg.as_c_ptr(),
                    ffi::secp256k1_nonce_function_ecdsa_adaptor,
                    aux_rand_ptr,
                ) == 1
            );
        };
        EcdsaAdaptorSignature(adaptor_sig)
    }

[Tibo-lg]

@jonasnick @apoelstra thanks for the review. I updated the docs adapting a bit the suggestion I hope it's fine.

[thomaseizinger]

utACK 5a68fb3

[jonasnick]

ACK 5a68fb3