Add an experimental batch module

[siv2r]

Overview

This PR adds support for batch verifying Schnorr signatures and tweaked x-only public key checks. It is based on the work of @jonasnick in #760.

Batch Verification

This implementation does not strictly follow the BIP340 batch verification spec. The API design is loosely based on this suggestion: #760 (comment). Prior development discussion of this PR can be found in siv2r#2.

Speed Up

• batch verifying Schnorr signatures is 20% faster - graph here
• batch verifying tweak pubkey checks is 50% faster - graph here

Fixes #1087

[siv2r]

This implementation does not strictly follow the BIP340 batch verification spec.

For example,

• the random numbers (or randomizers) aren’t generated by passing a seed (hash of all inputs) to CSPRNG
• allows mixing of schnorrsig and tweak checks
• uses the tag “BIP0340/batch” for initializing the sha256 obj (not in the BIP340 specs)

Alternative Design Options

• batch module design
  • the current code has: (new batch module)
    • create, destroy, and verify APIs in the batch module
    • batch_add_* APIs in schnorrsig and extrakeys modules (with #ifdef ENABLE_MODULE_BATCH)
  • alternate option 1: (new batch module)
    • place the batch_add_* APIs in the batch module (with schnorrsig and extrakeys header guards)
  • alternate option 2: (no module)
    • place create, destroy, and verify in src/secp256k1.c
    • place batch_add_* APIs in schnorrsig and extrakeys modules
  • relevant discussions:
    • Schnorr Batch Verification Interface siv2r/secp256k1#2 (comment)
    • Schnorr Batch Verification Interface siv2r/secp256k1#2 (comment)
• Delayed randomizer generation
  • the current code generates a randomizer just after a user enters input (in _batch_add_*)
  • we could instead generate them (in batch_verify) after the user enters all their input
  • Pros: follows bip340 specs. Cons: consumes more memory.
  • more info: General batch verification API context #1087 (comment) and here
• Use BLAKE256 instead of SHA256 for generating randomizers (to improve speed).
• In _batch_create, we could:
  • Get memory size instead of max_terms.
  • Provide pre-determined sizes (small, medium, and large) instead of max_terms.
    • Better test coverage?

[siv2r]

Questions

• Streaming batch API (this PR) vs Single call batch API?
• Is transparent verification required?
  • the current code implements transparent verification (inside batch_add_* APIs)
  • without transparent verification, the user needs to check for space in the batch before calling _batch_add APIs:

  if (batch_enough_space_for_schnorrsig) {
    batch_add_schnorrsig()
  }

• On an empty batch, Should _batch_verify return 0 or 1?
  • the current code returns 1
    • simple implementation
  • if we want to return 0
    • needs an extra param in batch to avoid ANDing the result with its initial value
    • provides better security?
• xonly_pubkey_tweak_add_check recommends ctx to be initialized for verification, but it can work even if ctx is initialized as none.
  • Since batch_add_tweak_xonlypub_check is based on tweak_add_check, should it also recommend that ctx be initialized for verification?
  • Can it recommend that ctx be initialized for none instead?
• A better name for secp256k1_batch_usable?
  • the current name is confusing
  • here, “usable” means if the batch can be used by the batch_add_* functions

[siv2r]

In extrakeys/bench_impl.h, cast pointers to (void *) before freeing to avoid MSVC warning.

[sipa]

batch_verify_1 shouldn't be slower than non-batch verify. Is it possible to revert to using non-batch validation logic for this case?

[siv2r]

Not possible with the current design.

The non-batch validation (secp256k1_schnorrsig_verify) logic looks something like this:

• calc rj using secp256k1_ecmult: Rj = sG - eP
• convert rj (gej) to r (ge)
• check if the r.x = sig[0:32] and r.y = even

one schnorrsig occupies two points in the batch, and one tweak check occupies one point in the batch. If a batch contains two points, there is no guarantee that they are from a schnorrsig (R, P). It could be from two tweak checks. So, we can't use the r.y = even check.

Hence, I tried implementing a slightly modified schnorrsig_verify logic (not implement in this PR):

• calc neg_rj using secp256k1_ecmult: neg_Rj = -s*G + batch.scalars[1]*batch.points[1]
• check if neg_rj + batch.points[0] == inf using _gej_add_var
  • batch.scalars[0] = 1 always. So, we don't need to use ecmult again

This gives somewhat better benchmarks than before:

Benchmark                          ,    Min(us)    ,    Avg(us)    ,    Max(us)    

schnorrsig_sign                    ,    49.1       ,    50.1       ,    53.4    
schnorrsig_verify                  ,    86.6       ,    87.2       ,    88.4    
schnorrsig_batch_verify_1          ,    94.7       ,    95.0       ,    95.2 

But schnorrsig_batch_verify_1 is still slower than schnorrsig_verify.

[jonasnick]

This implementation does not strictly follow the BIP340 batch verification spec. [...] the random numbers (or randomizers) aren’t generated by passing a seed (hash of all inputs) to CSPRNG

The security argument for batch verification should relatively easily translate to the approach in this PR.

Is transparent verification required?

I initially wasn't a fan of transparent verification because developers generally want to know how long a certain function call takes and don't want to be surprised by some batch_add calls taking much longer than others. But I changed my mind on this.

The batch verification API right now is used as follows:

for (i = 0; i < N_SIGS; i++) {
    if(!batch_usable(batch) || !batch_add_schnorrsig(batch, sig[i], msg[i], sizeof(msg[i]), &pk)) {
        return 0;
    }
}
if(!batch_verify(ctx, batch)) {
    return 0;
}

Without TV, then one option is to have the user create a batch that is large enough. Of course that's not alway possible because the batch can get larger than the available memory. Moreover, this would require adding an API that, given a number of schnorrsigs and tweaks (and more in the future) returns the size of the required batch. That seems way more complicated than TV.

If there's no TV and the batch is not guaranteed to be large enough, then users need to essentially reimplement something like transparent verification:

for (i = 0; i < N_SIGS; i++) {
    if(!schnorrsig_batch_has_space(batch)) {
        if(!batch_verify(batch)) {
            return 0;
        }
    }
    if (!batch_add_schnorrsig(batch, sig[i], msg[i], sizeof(msg[i]), &pk)) {
        return 0;
    }
}
if(!batch_verify(ctx, batch)) {
    return 0;
}

1. This is more code compared to having TV built into batch_add.
2. *_batch_has_space is specific to whatever you're trying to add to the batch, i.e., we would also have to add tweak_has_space, for example.
3. The batch_usable function in the current implementation (with TV) only allows for earlier aborts and is not essential (unless you want to determine if a batch_add failed because its input is obviously malformed or because the previous batch_add triggered a batch_verify that failed).
4. If users don't want to use TV (for whatever reason), then they don't have to - even in the current implementation. This requires counting the terms that have been added to the batch and verifying before it's full (perhaps we can make this simpler).

xonly_pubkey_tweak_add_check recommends ctx to be initialized for verification, but it can work even if ctx is initialized as none. [...] Since batch_add_tweak_xonlypub_check is based on tweak_add_check, should it also recommend that ctx be initialized for verification?

I don't think so. #1126 removes "initialized for verification" from the API docs of xonly_pubkey_tweak_add_check. For consistency it would be better if you'd remove "(can be initialized for none)" from the API doc.

[real-or-random]

I haven't had a closer look yet but I agree with @jonasnick's comments about TV.

[fjahr]

@siv2r Are you still working on this topic? Do you plan to address @jonasnick 's comments?

[siv2r]

@fjahr, this PR needs review from other contributors regarding the (batch API) design decision made here. The #1134 (review) comment showed its support for the "Transparent Verification" feature, which was implemented.

IIRC, two documentation changes are required (at the time of writing):

• remove "initialized for verification" from the batch_add_tweak_xonlypub_check doc.
• document that batch_verify_1 is slower than schnorrsig_verify (see #1134 (comment)).

I avoided making these changes immediately to keep the commit history clean for reviewers. I would be happy to work on any required changes after it gets enough review.

[fjahr]

I have opened a draft PR for using this in bitcoin core: bitcoin/bitcoin#29491

Here is a rebased branch of the code that I am using: https://github.com/fjahr/secp256k1/tree/pr1134-rebase-2024

I hope this can create some new interest and motivate people to review here as well.

[fjahr]

Getting an unused variable warning about strauss_ret here when building this as part of bitcoin core, probably because the check is removed with optimizations?

[real-or-random]

The macro VERIFY_CHECK(code) is defined to be the empty string in production builds, that's why you need to suppress the warning using (void)strauss_ret;.

[fjahr]

Getting another warning here when compiling together with Bitcoin Core:

src/modules/batch/tests_impl.h:68:19: warning: mixing declarations and code is a C99 extension [-Wdeclaration-after-statement]
    unsigned char aux_rand16[32];
                  ^