crypto: plug hardware optimized SHA256 into libsecp256k1's context #36188

furszy commented at 8:28 PM on September 7, 2026: member

Since https://github.com/bitcoin-core/secp256k1/pull//1777 , libsecp256k1 allows an externally provided SHA256 compression function. This PR plug ours in, so it runs on our SHA-NI/ARMv8/SSE4 hardware optimized implementation instead of the libsecp bare internal one.

The biggest gains are at the signing side, not at the verification side.

The first commits restructure how the libsecp context is encapsulated and initialized. ECC_Context now properly handles the libsecp context lifecycle and fully behaves as a singleton, getting decoupled from the key.h/cpp primitive (which is no longer accessed by upper layers just to init ECC), allowing us to introduce a verification context within the same object, and moving the singleton initialization to ecc_init.h which simplifies usage to a single entry point for all users.

Benchmarks at the bench introduction commit (43c9bdbf8ada1b581b5ab1f0a102517c243a60da) and at the tip:

On ARM64, arm_shani implementation

benchmark	before	after	change
`ECDSASign`	79.2 µs	69.3 µs	-13%
`SchnorrSign`	39.9 µs	37.1 µs	-7%
`ECDSAVerify`	27.4 µs	27.3 µs	-0%
`SchnorrVerify`	28.1 µs	27.8 µs	-1%
`EllSwiftCreate`	29.8 µs	28.8 µs	-3%
`BIP324_ECDH`	30.4 µs	30.1 µs	-1%

DrahtBot added the label Utils/log/libs on Sep 7, 2026

DrahtBot commented at 8:28 PM on September 7, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36188
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36188.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36188#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	fametrano, w0xlt
Concept ACK	fjahr, sedited, ismaelsadeeq, theStack, hebasto

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36122 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36122.svg"> (BIP460: CISA for Taproot key path spends by fjahr)
#36074 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36074.svg"> (scripted-diff: [test] Add util/check.h includes for assertions by maflcko)
#35511 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35511.svg"> (RFC: consensus: Make CAmount a class by hodlinator)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label CI failed on Sep 7, 2026

DrahtBot commented at 9:39 PM on September 7, 2026: contributor

🚧 At least one of the CI tasks failed. Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/34159538723/job/101858273499 LLM reason (✨ experimental): CI failed because the IWYU lint check detected missing/incorrect headers (generated “Failure generated from IWYU” for src/bench/ecc_sign_verify.cpp).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

furszy force-pushed on Sep 7, 2026

DrahtBot removed the label CI failed on Sep 8, 2026

w0xlt commented at 7:00 AM on September 8, 2026: contributor

Concept ACK

fjahr commented at 7:23 AM on September 8, 2026: contributor

Concept ACK

sedited commented at 7:30 AM on September 8, 2026: contributor

Finally! Concept ACK

ismaelsadeeq commented at 9:32 AM on September 8, 2026: member

Awesome Concept ACK

theStack commented at 4:55 PM on September 8, 2026: contributor

Splendid Concept ACK

fanquake added this to the milestone 33.0 on Sep 9, 2026

fanquake commented at 10:51 AM on September 9, 2026: member

cc @real-or-random

hebasto commented at 2:48 PM on September 10, 2026: member

Concept ACK.

DrahtBot added the label Needs rebase on Sep 24, 2026

in src/pubkey.cpp:242 in e0d46a42e0 outdated

 240 |      assert(sigbytes.size() == 64);
 241 |      secp256k1_xonly_pubkey pubkey;
 242 | -    if (!secp256k1_xonly_pubkey_parse(secp256k1_context_static, &pubkey, m_keydata.data())) return false;
 243 | -    return secp256k1_schnorrsig_verify(secp256k1_context_static, sigbytes.data(), msg.begin(), 32, &pubkey);
 244 | +    if (!secp256k1_xonly_pubkey_parse(GetSecp256k1VerifyContext(), &pubkey, m_keydata.data())) return false;
 245 | +    return secp256k1_schnorrsig_verify(GetSecp256k1VerifyContext(), sigbytes.data(), msg.begin(), 32, &pubkey);

theStack commented at 1:06 PM on September 24, 2026:

minimal-diff nit: from all the secp function calls in this file, this (_schnorrsig_verify) is the only one that involves hashing and thus benefits from the SHA256 override, i.e. it would be fine if the others kept using the static context

in src/ecc_context.h:34 in 99a29f69fd outdated

  30 | @@ -27,9 +31,11 @@ class ECC_Context
  31 |      ~ECC_Context();
  32 |  
  33 |      secp256k1_context* SignContext() const { return m_sign_ctx; }
  34 | +    const secp256k1_context* VerifyContext() const { return m_verify_ctx; }

theStack commented at 2:34 PM on September 30, 2026:

An alternative to this split-up would be to keep having only one "full" (i.e. non-static) context and use it for both signing and verifying (and any other API calls that either involve a generator point multiplication or hashing). It's true that some functions don't take use of the blinding data generated with _context_randomize, but it also doesn't slow things down or hurt in another way. The only challenge might be to find a good name for that context, since it's not only used for signing anymore (FullContext, DynamicContext, ...?).

furszy commented at 3:46 PM on September 30, 2026:

An alternative to this split-up would be to keep having only one "full" (i.e. non-static) context and use it for both signing and verifying (and any other API calls that either involve a generator point multiplication or hashing). It's true that some functions don't take use of the blinding data generated with _context_randomize, but it also doesn't slow things down or hurt in another way.

What if we ever want to re-seed the context rng? (secp256k1_context_randomize docs suggests doing it). By having two contexts, we avoid adding a mutex on the verification side and keep that context immutable and thread safe after construction.

theStack commented at 4:27 PM on September 30, 2026:

An alternative to this split-up would be to keep having only one "full" (i.e. non-static) context and use it for both signing and verifying (and any other API calls that either involve a generator point multiplication or hashing). It's true that some functions don't take use of the blinding data generated with _context_randomize, but it also doesn't slow things down or hurt in another way.

What if we ever want to re-seed the context rng? (secp256k1_context_randomize docs suggests doing it). By having two contexts, we avoid adding a mutex on the verification side and keep that context immutable and thread safe after construction.

Fair point, I didn't consider that there could be potential use-cases where the context needs any further modification after its initial setup in ECC_Start. Not sure how realistic plans of implementing re-seeding in Core currently are (and what the actual gain in security would really be by doing that), but I agree it's worth to take into account. No strong objection on the split-up, but wanted to bring the simpler option for discussion.

real-or-random commented at 1:52 PM on October 1, 2026:

Not sure how realistic plans of implementing re-seeding in Core currently are (and what the actual gain in security would really be by doing that),

I suspect there's a real gain when it comes to certain types of side-channel attack (e.g., power analysis). There were many discussions in the past, and I wrote a libsecp256k1 issue a while ago. The problem is just that nobody is working on it. ;)

The simplest thing we could do is to re-randomize the context after every operation that makes an ecmult_gen (pubkey generation and signing). This would be a ~100% hit in performance for these operations because rerandomizing also needs an ecmult_gen. 100% sounds huge but I'm not sure if anyone would really care here for these operations.

If people do care, the next simplest thing is to it only after (every n operations). For example, if we set n=10, then the performance hit is only 10%.

Anyway, this discussion belongs to a separate issue. But I agree that it's worth taking into account.

furszy commented at 5:47 PM on October 1, 2026:

Nice issue write-up.

I think we shouldn't over-think this now. Most of the work in this PR is about structuring the code properly so that we don't continue expanding the technical debt (the transform callback plug could have been done in 2 commits max otherwise, but they would be built on top of a dirty base). Once this lands, any future decision should be simple to do and isolated.

theStack commented at 2:38 PM on September 30, 2026: contributor

LGTM, just left two organizational comments. The MuSig and BIP352 modules could also take use of the plugged in compression function, either here or in a follow-up PR.

libsecp: generalize context setup and teardown

No behavior change.

First step toward an ECC_Context that owns the lifecycle of both
libsecp contexts, signing and verification.

ECC_Start receives the rng seed, so a verification context can be
created without one. Blinding is not needed for public data.

Also disallow copying ECC_Context, two copies would destroy the
same context.

06ce2eb353

libsecp: encapsulate signing context inside ECC_Context

No behavior change.

The signing context belongs to the ECC_Context that creates and
destroys it. Only one instance may exist at a time, and the
getter reads from it.

827ddba0d7

furszy force-pushed on Oct 15, 2026

furszy commented at 4:47 AM on October 15, 2026: member

conflicts solved, ready for review

DrahtBot added the label CI failed on Oct 15, 2026

DrahtBot commented at 5:47 AM on October 15, 2026: contributor

🚧 At least one of the CI tasks failed. Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/38682946486/job/116251332091 LLM reason (✨ experimental): CI failed the IWYU lint check: it added/fixed includes in eight files, leaving a diff that caused the check to exit with an error.

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot removed the label Needs rebase on Oct 15, 2026

init: decouple ECC init from primitives

No behavior change.

One entry point for ECC initialization, which will soon be moved to
the consensus library, and set up a verification context.

The goal is to decouple context handling from the primitive key.h/cpp
and pubkey.h/cpp, which shouldn't be accessed by the upper layers.

6e9abc6c17

libsecp: ECC_Context takes the blinding seed

No behavior change.

Last ECC_Context dependency on the RNG before the move to
the consensus library.

5a38d401bd

refactor: move ECC_Context to ecc_context.{h,cpp}

No behavior change.

ECC_Context now lives in the consensus library, so it can
hold the libsecp verification context next.

c33fd61b04

test: add ECC_Context lifecycle coverage cb2955f74a

bench: ECDSA and Schnorr signing and verification

Low-level signature creation and validation benchmarks.

7caaaca9d2

libsecp: plug our hardware optimized SHA256 into libsecp256k1 ctx

libsecp256k1 lets a context use an external SHA256 compression
function. With ours, ECDSA signing gets ~15% faster on CPUs with
SHA extensions, Schnorr signing ~3%.

3104f72514

libsecp: introduce verification context to ECC_Context

Verification uses secp256k1_context_static, which can't take our
optimized SHA256 compression function. Rather than reusing the signing
context, which blinds certain calculations, for no benefit to public
data, verification gets its own context, owned by ECC_Context along
with the signing one.

Around 1% faster Schnorr signature verification. ECDSA remains equal
as hashing is done outside libsecp.

4bd3fcefa6

scripted-diff: use verification context in pubkey.cpp

Every use of secp256k1_context_static in pubkey.cpp uses public
data, so all of them can use the verification  context, which
contains our optimized SHA256 compression plugged in.

-BEGIN VERIFY SCRIPT-
sed -i -e '/\/\*/! s/secp256k1_context_static/GetSecp256k1VerifyContext()/g' -e 's|^#include <hash.h>$|#include <ecc_context.h>\n#include <hash.h>|' src/pubkey.cpp
-END VERIFY SCRIPT-

285abdaded

fuzz: use libsecp verification context

Make fuzz tests use the verification context instead of the
static one provided by libsecp.

Also, introduces a simple unit test validating VerifySchnorr
provides the correct context to libsecp.

ecf779fa06

furszy force-pushed on Oct 15, 2026

DrahtBot removed the label CI failed on Oct 15, 2026

fametrano commented at 6:54 AM on October 25, 2026: none

tACK ecf779fa06cef78ca668715cba21b30a1fda8a06

On an Apple M5 (arm_shani), a differential harness compared a context using libsecp256k1's own compression with one using SHA256Transform, with both Core's arm_shani and portable transforms. It ran 20,000 random rounds of tagged hashes, BIP340 signing of 0- to 300-byte messages, ECDSA, ElligatorSwift XDH, MuSig2 and silent payments labels, plus the BIP340 vectors. All results matched, and a sample also matched the pure-Python code of a library I maintain. Unit tests and ctest pass.

Medians of three interleaved bench_bitcoin runs against 7caaaca9d2, the commit before the plug: ECDSASign -11%, SchnorrSign -3.7%, EllSwiftCreate -3.7%, BIP324_ECDH -1.9%, SchnorrVerify -1.2%. The ConnectBlock benches stay within noise. With Core's portable transform forced, hashing and signing run at libsecp256k1's own speed, so a CPU that falls back to it loses nothing per operation.

Non-blocking: ECCContextCreate goes from 13.5 µs to 176 µs. Almost all of it is the libsecp256k1 self-test of the plugged function, run once for each of the two contexts: 88 µs each here, about 0.5 ms each with the portable transform. Creating the verify context with secp256k1_context_clone before randomizing the sign context keeps the plugged function and skips the second test. From reading the code, not measuring, this matters mainly for the secp256k1_ecdsa_signature_parse_der_lax fuzz target, which builds a context for every input that parses. Happy to share the scripts as a gist.

DrahtBot requested review from ismaelsadeeq on Oct 25, 2026

DrahtBot requested review from sedited on Oct 25, 2026

DrahtBot requested review from theStack on Oct 25, 2026

DrahtBot requested review from hebasto on Oct 25, 2026

DrahtBot requested review from fjahr on Oct 25, 2026

in src/ecc_context.cpp:50 in ecf779fa06

  45 | +{
  46 | +    assert(g_ecc_context == nullptr);
  47 | +    m_sign_ctx = ECC_Start(rng_seed32);
  48 | +    m_verify_ctx = ECC_Start(/*rng_seed32=*/{});
  49 | +    g_ecc_context = this;
  50 | +}

fametrano commented at 7:40 AM on October 25, 2026:

Cloning the signing context before randomizing it would skip the second SHA256 self-test.

secp256k1_context_clone copies the context, compression function included, and runs no self-test. Done before secp256k1_context_randomize, the verify context ends up in the same state as now and shares no blinding. In medians of three bench_bitcoin runs on an Apple M5, ECCContextCreate drops from 169 µs to 89 µs. The parse_der_lax fuzz target builds a context for every input that parses, so it saves the same time per input. Unit tests pass.

/** Create an elliptic curve context that uses our SHA256 implementation */
static secp256k1_context* ECC_Start() {
    secp256k1_context *ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
    assert(ctx != nullptr);

    SHA256AutoDetect();
    secp256k1_context_set_sha256_compression(ctx, SHA256Transform);

    return ctx;
}

ECC_Context::ECC_Context(std::span<const unsigned char> rng_seed32)
{
    assert(g_ecc_context == nullptr);
    assert(rng_seed32.empty() || rng_seed32.size() == 32);
    m_sign_ctx = ECC_Start();
    // Clone before randomizing: the copy keeps SHA256Transform without testing it again,
    // and does not share the blinding of the signing context.
    m_verify_ctx = secp256k1_context_clone(m_sign_ctx);
    assert(m_verify_ctx != nullptr);
    if (!rng_seed32.empty()) {
        // Pass in a random blinding seed to the signing context.
        bool ret = secp256k1_context_randomize(m_sign_ctx, rng_seed32.data());
        assert(ret);
    }
    g_ecc_context = this;
}

furszy commented at 3:42 PM on October 27, 2026:

We can actually drop the ECC context creation from the parse_der_lax fuzz target, nothing there requires it. Other than that, the concept of the change seems good to me, the only thing I would try to do is decoupling the ECC function calls out of the ECC_Context constructor, so it stays clean.

in src/ecc_context.cpp:32 in ecf779fa06

  27 | +    assert(rng_seed32.empty() || rng_seed32.size() == 32);
  28 | +
  29 | +    secp256k1_context *ctx = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
  30 | +    assert(ctx != nullptr);
  31 | +
  32 | +    SHA256AutoDetect();

w0xlt commented at 11:34 PM on October 27, 2026:

This call appears redundant. Both bitcoind.cpp and NodeImpl::baseInitialize() construct a kernel::Context, which already runs SHA256AutoDetect() before ECC initialization.

furszy commented at 2:03 PM on October 28, 2026:

This call appears redundant.

Not for benchmarks, unit and fuzz tests. It doesn't harm to keep it.

w0xlt commented at 6:23 PM on October 28, 2026:

bench_bitcoin already calls it in main() at ecf779fa06, and tests using BasicTestingSetup initialize it through kernel::Context. It is redundant for those callers, although the extra overhead is negligible during normal startup.

DrahtBot requested review from w0xlt on Oct 27, 2026

w0xlt commented at 6:24 PM on October 28, 2026: contributor

ACK ecf779fa06cef78ca668715cba21b30a1fda8a06