Upstream PRs 1824, 1777, 1834, 1837, 1839, 1841 by DarkWindman · Pull Request #357 · BlockstreamResearch/secp256k1-zkp

DarkWindman · 2026-04-03T16:00:47Z

[https://github.com/bitcoin-core/secp256k1/pull/1824]: util: introduce and use ARRAY_SIZE macro
[https://github.com/bitcoin-core/secp256k1/pull/1777]: Make SHA256 compression runtime pluggable
[https://github.com/bitcoin-core/secp256k1/pull/1834]: ecmult: Document and test ng=NULL in ecmult
[https://github.com/bitcoin-core/secp256k1/pull/1837]: tests: Fix function pointer initialization C89 error in ellswift tests
[https://github.com/bitcoin-core/secp256k1/pull/1839]: ecdsa: VERIFY_CHECK result of _fe_set_b32_limit
[https://github.com/bitcoin-core/secp256k1/pull/1841]: gha: Bump deprecated GHA workflow dependencies

Tips:

Use git show --remerge-diff <pr-branch> to show the conflict resolution in the merge commit.
Use git read-tree --reset -u <pr-branch> to replay these resolutions during the conflict resolution stage when recreating the PR branch locally.
Be aware that this may discard your index as well as the uncommitted changes and untracked files in your worktree.

The macro definition matches the one used in Linux, see e.g. https://github.com/torvalds/linux/blob/9702969978695d9a699a1f34771580cdbb153b33/include/linux/array_size.h#L11 (without the additional check rejecting pointers, as we would need GNU C for that, see e.g. https://stackoverflow.com/a/19455169)

This is purely a mechanical change with no behavior change. It introduces a secp256k1_hash_ctx struct inside secp256k1_context and propagates it to all SHA256-related operations. This sets up the ability to provide a hardware-optimized SHA256 compression function at runtime in a follow-up commit.

This introduces `secp256k1_context_set_sha256_compression()`, which allows users to provide their own SHA256 block-compression function at runtime. This is useful in setups where the fastest implementation can only be determined dynamically based on the available CPU features, and rebuilding the library is not possible. The callback is installed on the `secp256k1_context` and is then used by all operations that compute SHA256 hashes. As part of the setup, the library performs sanity checks to ensure that the supplied function is equivalent to the default transform. Passing NULL to the callback setter restores the built-in implementation.

Multiple 64-byte blocks can now be compressed directly from the input buffer, without copying them into the internal buffer.

…E` macro 921b971 util: introduce and use `ARRAY_SIZE` macro (Sebastian Falbesoner) Pull request description: This PR is another tiny improvement found while working on #1765, with the goal to avoid code repetition. The `ARRAY_SIZE` macro definition is pretty wide-spread in C projects and e.g. matches the one [used in the Linux Kernel](https://github.com/torvalds/linux/blob/9702969978695d9a699a1f34771580cdbb153b33/include/linux/array_size.h#L11) (without the additional check to reject pointers, as we would need GNU C for that, see e.g. https://stackoverflow.com/a/19455169; not sure if a useful counterpart exists that only relies on C89). Replacement instances were identified via `$ git grep sizeof.*/.*sizeof`. ACKs for top commit: w0xlt: ACK 921b971 real-or-random: utACK 921b971 Tree-SHA512: 44b6bf0132cf00fade526a3fc04e03dc896d04874123614c032206b61f97c81f94d139b6cc0c108eceaa699251580c19420d230b3150607303ca2cb7ab9a0bcb

…uggable 4d92a08 sha256: speed up writes using multi-block compression (furszy) 0753f8b Add API to override SHA256 compression at runtime (furszy) fdb6a91 Introduce hash context to support pluggable SHA256 compression (furszy) Pull request description: Tackling the long-standing request #702. Right now we ship our own SHA256 implementation, a standard baseline version that does not take advantage of any hardware-optimized instruction, and it cannot be accessed by the embedding application - it is for internal usage only. This means embedding applications often have to implement or include a different version for their use cases, wasting space on constrained environments, and in performance-sensitive setups it forces them to use a slower path than what the platform provides. Many projects already rely on tuned SHA-NI / ARMv8 / or other hardware-optimized code, so always using the baseline implementation we ship within the library is not ideal. These changes allow users to supply their own SHA256 compression function at runtime, while preserving the existing default behavior for everyone else. This is primarily intended for environments where the available SHA256 implementation is detected dynamically and recompiling the library with a different implementation is not feasible (equivalent build-time functionality will come in a follow-up PR). It introduces a new API: ```C89 secp256k1_context_set_sha256_transform_callback(ctx, fn_transform) ``` This function installs the optimized SHA256 compression into the `secp256k1_context`, which is then used by all internal computations. Important: The provided function is verified to be output-equivalent to the original one. As a quick example, using this functionality in Bitcoin-Core will be very straightforward: furszy/bitcoin-core@f68bef0 ACKs for top commit: real-or-random: ACK 4d92a08 w0xlt: ACK 4d92a08 theStack: ACK 4d92a08 Tree-SHA512: 058e2e82071f1ca77254b684458292c621e60d65bbcc5500574429717e7db75bc9f3221129fafd11eb5d33e666a5efec5e9844460d3b194ef3b6b16f2df28fb9

…in ecmult 3a40363 eckey: Call ecmult with NULL instead of zero scalar (Tim Ruffing) 7e68c0c ecmult: Document and test ng=NULL in ecmult (Tim Ruffing) Pull request description: ACKs for top commit: theStack: re-ACK 3a40363 Tree-SHA512: 954928d4dfa120845c6e899c1a69ad0408072809551d42735eac491b8bc41249eb25d7c57cfa4f44763167620b5cb78639b5c396c0a342c47b0afc48a088c755

This also avoids a spurious "-Wmaybe-uninitialized" warning emitted by gcc 16 (snapshot) when compiling with -DDETERMINISTIC.

…lization C89 error in ellswift tests b84635e tests: Fix C89 function pointer initialization in ellswift tests (mllwchrry) Pull request description: Fixes a C89 pedantic compliance error in `src/modules/ellswift/tests_impl.h` where function pointer array initialization is not allowed at declaration time. This error was exposed while I was testing the improved test coverage in CI. The initial plan was to simplify the configuration of modules in CI by enabling all modules by default and testing the disabling of each module independently. Error: src/modules/ellswift/tests_impl.h:442:110: error: initializer element is not computable at load time [-Wpedantic]. The error occurred when running the `x86_64_debian` GitHub Actions CI job, which uses GCC 16 (snapshot) with strict flags (-std=c89 -pedantic -pedantic-errors -Werror). See this action run for reference: https://github.com/mllwchrry/secp256k1/actions/runs/23301905657/job/67769464566. The fix uses `if/else` to assign function pointers after declaration, matching the pattern already used in the same file. While this is a minor C89 compliance issue, it blocks the potential CI simplification. ACKs for top commit: real-or-random: utACK b84635e theStack: ACK b84635e Tree-SHA512: 61e42afe9c3a215f817b1bf475ea66c103b4af6c598a9d7ee9e1a97789ac6f4e025260b4cd0e2ec219bd73706c7aa2799c58ab904916d9594626cf3c07e4b983

@mllwchrry

…set_b32_limit 43fca0f ecdsa: VERIFY_CHECK result of _fe_set_b32_limit (Tim Ruffing) Pull request description: This also avoids a spurious `-Wmaybe-uninitialized` warning emitted by gcc 16 (snapshot) when compiling with `-DDETERMINISTIC`. Alternative to #1838 by @mllwchrry who tried very a similar thing as this PR but couldn't convince the compiler. (The GCC snapshot is very annoying: a simple `VERIFY_CHECK(secp256k1_fe_set_b32_limit(&xr, c))` doesn't do the trick. I found this variant here with a local store rather by accident.) ACKs for top commit: mllwchrry: ACK 43fca0f theStack: utACK 43fca0f Tree-SHA512: 2550043e953675db7614f98bbdffb706721834967ef36f7c905f7cbfeee5d88189a9acfcd64865ef822bb0e3272d228440bdfb1124228afe083e025056e53212

Deprecation notice: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ Changelogs (no entries relevant to us): * https://github.com/docker/setup-buildx-action/releases/tag/v4.0.0 * https://github.com/docker/build-push-action/releases/tag/v7.0.0 * https://github.com/actions/cache#v5

…dependencies c5cd9d6 gha: Bump deprecated GHA workflow dependencies (Tim Ruffing) Pull request description: Deprecation notice: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/ Changelogs (no entries relevant to us): * https://github.com/docker/setup-buildx-action/releases/tag/v4.0.0 * https://github.com/docker/build-push-action/releases/tag/v7.0.0 * https://github.com/actions/cache#v5 ACKs for top commit: hebasto: ACK c5cd9d6. Tree-SHA512: d21509503a617344f5f93a87fe229774e5ab5322ae2f322b38c7fdaef54ab3386a5eaa9901cb1b15f32c7891d2587aab709c70df9f5aaedc7524710496981311

real-or-random · 2026-04-20T07:00:21Z

@mllwchrry Do you want to review this?

mllwchrry

The PR is good overall. A few style nits and one missed place for #1834.
Claude also caught a hard-to-catch bug in the #1777 port: secp256k1_ecdsa_adaptor_encrypt falls back to a static-context wrapper for nonce generation instead of dispatching to the _impl function with the actual context. This way a custom SHA256 compression function set on the context is silently ignored for adaptor signature nonce generation. We discussed it internally, and the fix will be included in this PR.

…p256k1#1824 to zkp-specific code

mllwchrry · 2026-04-21T10:23:56Z

Claude also caught a hard-to-catch bug in the #1777 port: secp256k1_ecdsa_adaptor_encrypt falls back to a static-context wrapper for nonce generation instead of dispatching to the _impl function with the actual context. This way a custom SHA256 compression function set on the context is silently ignored for adaptor signature nonce generation. We discussed it internally, and the fix will be included in this PR.

After the investigation, it turned out that the fix is more involved than expected. The nonce generation for the dleq proof inside secp256k1_dleq_prove also goes through the same static context wrapper, so both call sites need to be addressed.
Our fix plan is to restructure where the nonce is computed: moving it out of dleq_impl.h into main_impl.h and passing a pre-computed scalar to secp256k1_dleq_prove. This is required since we decided to apply the approach used for schnorrsig from #1777. The idea is to modify the internal nonce_function_ecdsa_adaptor_impl function to take hash context and call it if noncefp is NULL, keeping the public API wrapper unchanged and uncalled. The complication is that dleq_impl.h is included at the top of main_impl.h, so nonce_function_ecdsa_adaptor_impl isn't defined yet when dleq_impl.h is processed. That's why we decided to move nonce computation to main_impl.h. If you have any other thoughts on this, please share.

@real-or-random Would it make sense to merge this PR as-is once style fixes are applied and address the bug in another PR?

real-or-random · 2026-04-21T10:36:41Z

@real-or-random Would it make sense to merge this PR as-is once style fixes are applied and address the bug in another PR?

My understanding is that the bug is just that the externally-provided SHA256 callback is not used. While this classifies as a bug, it's just about performance and not about correctness. If this understanding is right, then yes, I think it's okay to fix this in a follow-up PR.

mllwchrry · 2026-04-21T16:15:30Z

ACK baac08d

theStack and others added 16 commits February 17, 2026 00:21

sha256: speed up writes using multi-block compression

4d92a08

Multiple 64-byte blocks can now be compressed directly from the input buffer, without copying them into the internal buffer.

ecmult: Document and test ng=NULL in ecmult

7e68c0c

eckey: Call ecmult with NULL instead of zero scalar

3a40363

tests: Fix C89 function pointer initialization in ellswift tests

b84635e

ecdsa: VERIFY_CHECK result of _fe_set_b32_limit

43fca0f

This also avoids a spurious "-Wmaybe-uninitialized" warning emitted by gcc 16 (snapshot) when compiling with -DDETERMINISTIC.

Merge branch 'master' into sync-7262adb4

f7e7e6b

real-or-random mentioned this pull request Apr 20, 2026

Upstream PRs 1824, 1777, 1834, 1837, 1839, 1841 #356

Closed

mllwchrry reviewed Apr 20, 2026

View reviewed changes

Comment thread src/modules/ecdsa_s2c/tests_impl.h Outdated

Comment thread src/modules/rangeproof/main_impl.h Outdated

Comment thread src/modules/rangeproof/main_impl.h Outdated

Comment thread src/eccommit_impl.h Outdated

Comment thread src/modules/bppp/main_impl.h Outdated

modules, tests: Port bitcoin-core/secp256k1#1777 and bitcoin-core/sec…

baac08d

…p256k1#1824 to zkp-specific code

DarkWindman force-pushed the sync-7262adb4 branch from fa8a68c to baac08d Compare April 21, 2026 11:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upstream PRs 1824, 1777, 1834, 1837, 1839, 1841#357

Upstream PRs 1824, 1777, 1834, 1837, 1839, 1841#357
DarkWindman wants to merge 17 commits intoBlockstreamResearch:masterfrom
mllwchrry:sync-7262adb4

DarkWindman commented Apr 3, 2026

Uh oh!

real-or-random commented Apr 20, 2026

Uh oh!

mllwchrry left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mllwchrry commented Apr 21, 2026

Uh oh!

real-or-random commented Apr 21, 2026

Uh oh!

mllwchrry commented Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

DarkWindman commented Apr 3, 2026

Uh oh!

real-or-random commented Apr 20, 2026

Uh oh!

mllwchrry left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mllwchrry commented Apr 21, 2026

Uh oh!

real-or-random commented Apr 21, 2026

Uh oh!

mllwchrry commented Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants