BIP458: Schnorr (Incremental) Half Aggregation #1566

fjahr commented at 3:24 PM on July 2, 2024: contributor

This implements BIP458: https://github.com/bitcoin/bips/pull/2205

The original version of this code ported over the work from b-wagn in https://github.com/BlockstreamResearch/secp256k1-zkp/pull/261

fjahr force-pushed on Jul 2, 2024

real-or-random added the label feature on Jul 2, 2024

fjahr force-pushed on Jul 2, 2024

jjones3695 approved

bitcoin-core deleted a comment on Jul 16, 2024

fjahr force-pushed on Nov 18, 2024

fjahr commented at 4:17 PM on November 18, 2024: contributor

Rebased on the musig changes and added the compile options for cmake

fjahr force-pushed on Nov 18, 2024

fjahr force-pushed on Nov 26, 2024

fjahr force-pushed on Dec 16, 2025

fjahr commented at 10:53 PM on December 16, 2025: contributor

Rebased to take into account the latest changes to cmake, CI and test framework. Also squashed the commit since maintaining them was annoying and there wasn't really a good reason for them. It was more of a crutch for me while I was taking my first steps here.

fjahr force-pushed on Dec 16, 2025

in include/secp256k1_schnorrsig_halfagg.h:93 in ccc826d1ff outdated

  88 | + *  Returns:          1: correct signature.
  89 | + *                    0: incorrect signature.
  90 | + *  Args:           ctx: a secp256k1 context object.
  91 | + *  In:         pubkeys: Array of n many x-only public keys. Can only be NULL if n is 0.
  92 | + *               msgs32: Array of n many 32-byte messages. Can only be NULL if n is 0.
  93 | + *                    n: number of signatures to that have been aggregated.

Jonatanc05 commented at 1:58 PM on February 22, 2026:

 *                    n: number of signatures that have been aggregated.

fjahr commented at 9:51 AM on February 26, 2026:

Thanks, I will address it the next time I will push an update!

fjahr commented at 9:58 PM on June 28, 2026:

Fixed

Jonatanc05 commented at 4:55 PM on February 22, 2026: none

I think there's a typo

fjahr force-pushed on Jun 28, 2026

fjahr commented at 9:57 PM on June 28, 2026: contributor

Rebased and added the test vectors from the BIP draft: https://github.com/bitcoin/bips/pull/2205

fjahr force-pushed on Jun 28, 2026

fjahr force-pushed on Jul 7, 2026

fjahr renamed this:
~~Schnorr (Incremental) Half Aggregation~~
BIP458: Schnorr (Incremental) Half Aggregation
on Jul 7, 2026

fjahr force-pushed on Jul 11, 2026

fjahr force-pushed on Jul 12, 2026

fjahr force-pushed on Aug 15, 2026

in src/modules/schnorrsig_halfagg/main_impl.h:216 in 1e67cc39f2

 211 | +    secp256k1_ecmult(&lhs, &rhs, &secp256k1_scalar_zero, &s);
 212 | +
 213 | +    /* Check that lhs == rhs */
 214 | +    secp256k1_gej_neg(&lhs, &lhs);
 215 | +    secp256k1_gej_add_var(&lhs, &lhs, &rhs, NULL);
 216 | +    return secp256k1_gej_is_infinity(&lhs);

theStack commented at 5:42 PM on September 11, 2026:

could use the secp256k1_gej_eq_var helper here which does the some steps internally:

    return secp256k1_gej_eq_var(&lhs, &rhs);

fjahr commented at 1:22 PM on September 12, 2026:

Done, thanks

in src/modules/schnorrsig_halfagg/main_impl.h:22 in 1e67cc39f2

  17 | +    sha->s[4] = 0x1d4db069ul;
  18 | +    sha->s[5] = 0xb4d587e1ul;
  19 | +    sha->s[6] = 0x50451c2aul;
  20 | +    sha->s[7] = 0x10fb63e9ul;
  21 | +
  22 | +    sha->bytes = 64;

theStack commented at 5:46 PM on September 11, 2026:

could use secp256k1_sha256_initialize_midstate here (introduced in PR #1825, commit https://github.com/bitcoin-core/secp256k1/commit/f48b1bfa5d40a4d7303b196017d2e298520d1066), see other modules that use BIP340 tagged hashes

fjahr commented at 1:21 PM on September 12, 2026:

Done, thanks

in src/modules/schnorrsig_halfagg/main_impl.h:34 in 1e67cc39f2

  29 | +    secp256k1_scalar s;
  30 | +    int overflow;
  31 | +    const secp256k1_hash_ctx *hash_ctx;
  32 | +
  33 | +    VERIFY_CHECK(ctx != NULL);
  34 | +    hash_ctx = secp256k1_get_hash_context(ctx);

theStack commented at 6:00 PM on September 11, 2026:

this function was removed in #1908, so there is a (silent) merge conflict

fjahr commented at 1:21 PM on September 12, 2026:

Ah, good catch, fixed and rebased.

in src/CMakeLists.txt:18 in 1e67cc39f2 outdated

  11 | @@ -12,6 +12,18 @@ if(SECP256K1_ENABLE_MODULE_ELLSWIFT)
  12 |    set_property(TARGET secp256k1 APPEND PROPERTY PUBLIC_HEADER ${PROJECT_SOURCE_DIR}/include/secp256k1_ellswift.h)
  13 |  endif()
  14 |  
  15 | +if(SECP256K1_ENABLE_MODULE_SCHNORRSIG_HALFAGG)
  16 | +  if(NOT SECP256K1_EXPERIMENTAL)
  17 | +    message(FATAL_ERROR "Schnorrsig half-aggregation is experimental. Use -DSECP256K1_EXPERIMENTAL=ON to allow.")
  18 | +  endif()

theStack commented at 6:03 PM on September 11, 2026:

is the "experimental" category still used for new modules? (cc @real-or-random @sipa) I haven't seen it for the most recent ones (ellswift, musig, silentpayments), but IIUC the schnorrsig had it back then.

fjahr commented at 7:16 PM on September 11, 2026:

Yeah, I myself argued that it might be possible to remove the category at some IRC meeting or the in-person meeting probably about a year ago. This was about the batch module specifically that still had it and it made sense for me to remove it there because it was already in BIP340 and the code would only be merged if it was in a usable state anyway, the bar is just that high already. I think @sipa said the category should be kept around if we want to introduce a module where the API may still change, at least that's the only argument I remember from the conversation. I already considered removing the category here and in fullagg for the same reasons as batch but then reconsidered: Maybe we would want to set it for some code that is specifically added for a soft fork before the soft fork is actually activated? (There are other use cases outside of consensus for halfagg and fullagg but realistically those will not get these PRs merged here.) I also saw that the Schnorr module was also merged as experimental but I did not go the extra mile to ask if this was actually because of the consensus angle, but I did the lazy thing and made no change here :)

However, I just now did some more digging and found this comment from @sipa that clarifies the intention for BIP340 at the time: #817 (comment) The whole issue was actually just closed after we discussed it as a group, btw.

I am currently basically 50/50 because I think the arguments for keeping Schnorr experimental from the comment very much apply to this pull here as is. But on the other hand I think the pull has a very low chance of getting merged while these arguments still apply because of the high standard here and if we agree on that we can remove the status already. Well, there is a chance that an alternative, newly discovered use case becomes strong enough for an earlier merge so maybe 51/49 for keeping it experimental for now and potentially removing it before the merge if doesn't seem needed anymore.

That's a long way of saying, I am happy to remove or keep it around.

fjahr force-pushed on Sep 12, 2026

fjahr commented at 1:23 PM on September 12, 2026: contributor

Addressed the feedback from @theStack and rebased, happy to remove the experimental status if reviewers think it doesn't apply here.

in src/modules/schnorrsig_halfagg/main_impl.h:165 in 11e6a9127c

 160 | +            return 0;
 161 | +        }
 162 | +        secp256k1_fe_get_b32(pk_ser, &pp.x);
 163 | +
 164 | +        /* Step 1: z_i = TaggedHash(...) */
 165 | +        /* 1.a) Write into hash r_i, pk_i, m_i, r_i */

theStack commented at 6:05 PM on September 14, 2026:

        /* 1.a) Write into hash r_i, pk_i, m_i */

fjahr commented at 7:35 PM on September 14, 2026:

Good catch, fixed here and found the same error in inc_aggregate, fixed there as well.

in src/modules/schnorrsig_halfagg/main_impl.h:205 in 11e6a9127c outdated

 200 | +     * no ecmult_gen context, so aggverify works with secp256k1_context_static. */
 201 | +    secp256k1_scalar_set_b32(&s, &aggsig[n*32], &overflow);
 202 | +    if (overflow) {
 203 | +        return 0;
 204 | +    }
 205 | +    secp256k1_ecmult(&lhs, &rhs, &secp256k1_scalar_zero, &s);

theStack commented at 6:13 PM on September 14, 2026:

not applicable yet, but just leaving a note already that in the future this could be replaced with ecmult_gen_var_gej, if #1883 gets in. for the agg_verify benchmark (with N=3), I'm seeing a ~5% speedup on my machine (95.5us vs. 90.5us): https://github.com/theStack/secp256k1/commit/cbe15b8f2ceaec0c0262d86c427617e0e65eb01d

fjahr commented at 7:34 PM on September 14, 2026:

Nice, I will keep an eye on that!

fjahr force-pushed on Sep 14, 2026

Experimental: Add BIP 458 half-aggregation for Schnorr signatures

Code is based on the secp256k1-zkp implementation. Test vectors based on
BIP 458: https://github.com/bitcoin/bips/blob/master/bip-0458.mediawiki

Co-authored-by: Benedikt <crypto@benedikt-wagner.dev>

345d9ff4ca

halfagg: Add benchmarks 13b4e8c78e

halfagg: Add example a206a10d8f

halfagg: Add docs 3361972502

fjahr force-pushed on Sep 20, 2026

fjahr commented at 10:18 AM on September 20, 2026: contributor

Changed the tag to include the BIP number