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%

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.

0a0ab68160

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.

1253e64d63

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.

1fda5a0740

libsecp: ECC_Context takes the blinding seed

No behavior change.

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

fa2e2dc69f

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.

b44f0da9b2

test: add ECC_Context lifecycle coverage ee77338bbd

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.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36188.

Reviews

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

Type	Reviewers
Concept ACK	w0xlt, 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 (BIP460: CISA for Taproot key path spends by fjahr)
#36074 (scripted-diff: [test] Add util/check.h includes for assertions by maflcko)
#35675 (mining: add block template manager by ismaelsadeeq)
#35646 (RFC: Separate out runtime errors from BlockValidationState using util::Expected by yuvicc)
#35581 (node: add block template manager and track waitNext fee inflow by ismaelsadeeq)
#35301 (Silent Payments: Implement bip352 (take 2) by Eunovo)
#33922 (mining: add getMemoryLoad() and track template non-mempool memory footprint by Sjors)
#29700 (kernel, refactor: return error status on all fatal errors by ryanofsky)
#28690 (build: Introduce internal kernel library by sedited)

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. <sub>Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/34159538723/job/101858273499</sub> <sub>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).</sub>

<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>

bench: ECDSA and Schnorr signing and verification

Low-level signature creation and validation benchmarks.

f61a8a9594

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%.

5fb21ed986

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.

99a29f69fd

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-

e0d46a42e0

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.

1c216a069b

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

DrahtBot commented at 1:04 AM on September 24, 2026: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

in src/pubkey.cpp:242 in e0d46a42e0

 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

  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 also don't think we shouldn't over-think this now. Most of the work on 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.