CI / x86_64: Linux (Debian stable) (clang, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[BENCH:no BUILD:distcheck CTIMETESTS:no WITH_VALGRIND:no]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[BENCH:no BUILD:distcheck CTIMETESTS:no WITH_VALGRIND:no]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / i686: Linux (Debian stable) (clang --target=i686-pc-linux-gnu -isystem /usr/i686-linux-gnu/include, map[env_vars:map[]]) (push) Has been cancelled
CI / MSan (clang, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -fsanitize-memory-param-retval -g CTIMETESTS:no]]) (push) Has been cancelled
CI / MSan (clang, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -g -O3 CTIMETESTS:yes ECMULTGENKB:2 ECMULTWINDOW:2]]) (push) Has been cancelled
CI / MSan (clang-snapshot, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -fsanitize-memory-param-retval -g CTIMETESTS:no]]) (push) Has been cancelled
CI / MSan (clang-snapshot, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -g -O3 CTIMETESTS:yes ECMULTGENKB:2 ECMULTWINDOW:2]]) (push) Has been cancelled
Four small divergences from the reference implementation and its API
contract, none of which changes any signature: the differential harness
(240 signing + 120 deterministic-signing cases against the Python
reference) produces byte-identical output before and after.
Length prefixes that do not fit
-------------------------------
secp256k1_frost_sha256_write_prefixed asserted, via VERIFY_CHECK, that
the length fits into its prefix. VERIFY_CHECK compiles away in release
builds, so a length that does not fit was silently truncated modulo
2^(8*prefix_size) instead of being rejected, yielding a nonce that does
not follow the spec. The reference raises OverflowError instead.
Only the 4-byte extra_in prefix of nonce_hash is affected, and only where
size_t is wider than 32 bits, so this needs an extra_in of 4 GiB to
trigger. It is nevertheless a silent deviation, so write_prefixed now
returns 0 without writing anything, and the failure is propagated:
secp256k1_frost_nonce_function and secp256k1_frost_det_nonce_function
return 0, and secp256k1_frost_nonce_gen returns 0 after wiping
session_secrand32 and the nonces. Checking the shifted-out bits (which
the loop already computes) rather than comparing extra_in_len against a
32-bit bound avoids a comparison that is always true on 32-bit platforms.
The bound is now documented on the extra_in_len parameter.
Identifiers equal to UINT32_MAX
-------------------------------
BIP 445 derive_interpolating_value accepts every identifier in
0 <= id < 2^32, but secp256k1_frost_ids_are_valid rejected UINT32_MAX
because the mapping to the polynomial x-coordinate, id + 1, overflows in
uint32_t arithmetic. The +1 is now added in scalar arithmetic, where it
cannot overflow, and the identifier restriction is gone. The denominator
never needed the +1 at all, since
x_j - x_i = (id_j + 1) - (my_id + 1) = id_j - my_id
so it is computed directly from the identifiers.
This was unreachable through the public API -- validate_session_params
already bounds identifiers by n_participants, which is at most
SECP256K1_FROST_MAX_PARTICIPANTS = 128 -- but it made an internal helper
diverge from the algorithm it implements. frost_large_id_test covers it
by reconstructing the constant term of a random degree-2 polynomial from
shares held by identifiers 0, UINT32_MAX - 1 and UINT32_MAX.
Zero-length messages
--------------------
secp256k1_frost_session_init and secp256k1_frost_deterministic_sign
required a non-NULL msg, so an empty message -- which the reference
represents as the byte string b"" -- could only be passed as a pointer
that is never dereferenced. Both now accept NULL when msglen is 0,
matching secp256k1_schnorrsig_sign_custom and the msg parameter of
secp256k1_frost_nonce_gen. secp256k1_sha256_write guards both of its
memcpy calls on a non-zero length, so it is never reached with a NULL
pointer.
NonceGen keeps its distinction between a NULL msg and a zero-length msg:
there the BIP really does distinguish msg = None (hashed as the single
byte 0x00) from msg = b"" (hashed as 0x01 followed by an eight-byte zero
length), and the API expresses that as NULL versus non-NULL.
frost_empty_msg_test runs a signing round over a zero-length message
passed both ways and checks that the two session objects are identical.
The two API tests that relied on a NULL msg always being rejected now
pass an explicit non-zero msglen; previously they passed a random msglen
that could be 0.
Header documentation
--------------------
The parameter tables of eleven doc comments had names that did not line
up with their block's continuation column. All parameter tables are now
aligned consistently, with wrapped text two columns past the colon.
Verification
------------
- gcc and clang, -std=c89 -pedantic-errors -Werror, with and without
-DVERIFY: clean
- tests (multiple seeds), noverify_tests and frost_example: pass
- ctime_tests under MemorySanitizer: exits 0 with halt_on_error=1
- vectors.h still reproduces exactly from the spec's JSON vectors
- 240 signing + 120 deterministic-signing differential cases against
the BIP 445 Python reference: byte-identical to the previous commit
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The module's BIP 445 logic itself is unchanged and was independently
validated against the pinned spec commit bb5396f (BIP v0.10.0), both via
the checked-in test vectors and via differential testing against the
Python reference over 360 randomised configurations (n up to 128,
shuffled non-contiguous signer ids, mixed xonly/plain tweak chains,
variable-length messages, pubshares present and absent). Every change
below is structural: the differential harness produces byte-identical
pubnonces, aggnonces, partial signatures and final signatures before and
after.
Two classes of problem prevented the module from passing CI.
1. Constant-time violations (ctime_tests)
-----------------------------------------
The CI matrix enables FROST in rows that also run
"valgrind --error-exitcode=42 ./ctime_tests" -- WITH_VALGRIND and
CTIMETESTS both default to 'yes'. With the module enabled that job
reported 639 "conditional jump depends on uninitialised value" errors,
all originating from two sites:
- secp256k1_frost_derive_coefficient returned
!overflow && !secp256k1_scalar_is_zero(out)
where the short-circuiting && branches on `overflow`, which is
derived from the threshold secret key. The caller declassifies the
return value, but the branch has already happened inside the callee.
Replaced with a bitwise &, matching the existing idiom in
secp256k1_scalar_set_b32_seckey (src/scalar_impl.h).
- secp256k1_frost_sign_internal performs the self-verification
recommended by BIP 445, which runs the *variable-time*
secp256k1_ecmult over the partial signature s. nonce_pts and pk were
already declassified ahead of that call; s was not. Since s is the
public output of the function, declassifying it before the
self-verification is both correct and sufficient.
secp256k1_frost_deterministic_sign carried three more instances of the
same class, invisible until now because ctime_tests did not exercise
that path at all:
- the `if (!valid)` check on secp256k1_scalar_set_b32_seckey lacked the
declassify that the identical checks in secp256k1_frost_nonce_gen and
secp256k1_frost_sign_internal already have;
- secp256k1_frost_det_nonce_function used the same short-circuiting &&,
here over the secret nonces;
- the branch on that function's result was not declassified.
The && in det_nonce_function is rewritten via two int locals rather than
a bare bitwise &: clang's -Wbitwise-instead-of-logical fires when both
operands are `!f(...)` expressions, which would break the -Werror clang
builds.
ctime_tests now also covers secp256k1_frost_deterministic_sign, so that
path stays checked from here on.
None of these leak anything of value in practice -- they reveal only
negligible-probability events (a hash overflowing the curve order, a zero
nonce) or whether a secret share is a valid secret key -- but they
violate the project's declassification discipline and fail the ctime
test.
2. C90 conformance (-Werror -pedantic-errors)
---------------------------------------------
The project targets C90 (CMAKE_C_STANDARD 90, -std=c89 -pedantic) and CI
passes WERROR_CFLAGS='-Werror -pedantic-errors'. Compiling src/tests.c
with those flags produced 62 errors in three groups:
- 40x "ISO C forbids empty initializer braces before C2X" in the
generated vectors.h; empty {} initializers are C23-only. Fixed in
tools/test_vectors_frost_generate.py so it survives regeneration:
hexstr_to_intarray now emits "0" for an empty byte string (all six
of its call sites wrap the result in braces), and init_group's
`counted` helper emits "{ 0 }" for an empty group. In every affected
slot the paired count/length field is 0, so the padding element is
never read.
- 1x "comma at end of enumerator list" (C99+), also in the generator.
- 21x "initializer element is not computable at load time" across 11
lines of tests_impl.h. C90 requires constant expressions in
initializers for automatic aggregates, so
const secp256k1_frost_pubnonce *ptrs[2] = { &a, &b };
is invalid. Rewritten as a declaration plus assignments, the style
the musig tests already use, which is why the pre-existing tree was
green.
vectors.h is regenerated from the spec's JSON vectors. Its hex payload is
byte-identical (verified by hashing every 0xNN token) and the file still
reproduces exactly from tools/test_vectors_frost_generate.py.
Verification
------------
- gcc and clang, -std=c89 -pedantic-errors -Werror, with and without
-DVERIFY: clean (was 62 errors)
- ctime_tests under MemorySanitizer: 0 reports (was 639); exits 0 with
halt_on_error=1
- tests, noverify_tests and frost_example: pass
- vectors.h regenerates identically from the pinned spec vectors
- 240 signing + 120 deterministic-signing differential cases against
the BIP 445 Python reference: byte-identical to the pre-fix build
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI / x86_64: Linux (Debian stable) (clang, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[BENCH:no BUILD:distcheck CTIMETESTS:no WITH_VALGRIND:no]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (clang-snapshot, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[BENCH:no BUILD:distcheck CTIMETESTS:no WITH_VALGRIND:no]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[CFLAGS:-O1 ECDH:yes ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes]]) (push) Has been cancelled
CI / x86_64: Linux (Debian stable) (gcc-snapshot, map[env_vars:map[ELLSWIFT:yes EXTRAKEYS:yes MUSIG:yes RECOVERY:yes SCHNORRSIG:yes WIDEMUL:int128]]) (push) Has been cancelled
CI / i686: Linux (Debian stable) (clang --target=i686-pc-linux-gnu -isystem /usr/i686-linux-gnu/include, map[env_vars:map[]]) (push) Has been cancelled
CI / MSan (clang, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -fsanitize-memory-param-retval -g CTIMETESTS:no]]) (push) Has been cancelled
CI / MSan (clang, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -g -O3 CTIMETESTS:yes ECMULTGENKB:2 ECMULTWINDOW:2]]) (push) Has been cancelled
CI / MSan (clang-snapshot, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -fsanitize-memory-param-retval -g CTIMETESTS:no]]) (push) Has been cancelled
CI / MSan (clang-snapshot, map[env_vars:map[CFLAGS:-fsanitize=memory -fsanitize-recover=memory -g -O3 CTIMETESTS:yes ECMULTGENKB:2 ECMULTWINDOW:2]]) (push) Has been cancelled
72867fd682 tests: Port hash context tests to zkp modules (mllwchrry)
Pull request description:
Ports the hash context tests from bitcoin-core/secp256k1#1777 to the zkp-specific modules.
ACKs for top commit:
apoelstra:
ACK 72867fd682279ff2c79cf13f4f8d8484048d2527; successfully ran local tests
Tree-SHA512: 1714a4c5fd793b32a0f00a92670a48709e4d01917bb81739428601c6e88e89f871cdc1294024e6df7272d6c16e681309d9b3448adcd07ce37bfa2be2dc3f4172
598e22dcde whitelist: document the degenerate W = -P_i destination (DarkWindman)
7ea12c2dca whitelist: document that the parsed key count is untrusted (DarkWindman)
e8c3396597 whitelist: honour the documented parse initialization guarantee (DarkWindman)
1de3864ff9 bppp: check for overflow in generator allocation (DarkWindman)
Pull request description:
Small defensive fixes + docs for whitelist/bppp:
- bppp: overflow check in `generators_create` allocation.
- whitelist: signature_parse always leaves `sig` in a valid-or-canonically-invalid state; guard serialize against untrusted `n_keys`.
- whitelist: document that `n_keys` is attacker-controlled and must not be used to size the arrays passed to `_verify`.
- whitelist: document the accepted degenerate `W = -P_i` destination.
No API/format changes. New tests cover theallocation overflow and the parse-failure guarantee.
ACKs for top commit:
apoelstra:
ACK 598e22dcde2152003c6a60f04196cc5fe2e2a090; successfully ran local tests
mllwchrry:
ACK 598e22d.
Tree-SHA512: 4c218775a4c2e53091f3ac11e5d93caf17083216afa167fb5c1a1cc38c0a7a2cb1e0564cf9e63c5391d72aba9fc42d90a64b9a5f288ae763f0bf9b4b20f3b2e1
The s-values produced by secp256k1_surjection_genrand previously
depended only on their indices and the difference between
input_blinding_key and output_blinding_key. Calls with the same
difference therefore reused s-values even when their proof inputs
differed.
For proofs with the same used-input selection and honest input index,
the same generated s-value was used as the signing nonce. Reusing this
nonce across different proof messages allowed recovery of the
blinding-key difference. The remaining repeated s-values also revealed
the honest input index.
This commit affects proof generation only; verification is unchanged.
Now that we have a function `_ecmult_gen_ge`, it makes sense to rename
the existing function `_ecmult_gen` to `_ecmult_gen_gej` for
consistency, to signal that the result is a Jacobian group element.
This diff was created by applying
```
$ sed -i s/secp256k1_ecmult_gen\(/secp256k1_ecmult_gen_gej\(/g $(git ls-files)
```
Scalar multiplication with the generator point frequently involves a
conversion to affine coordinates and clearing out the temporary Jacobian
group element object after to avoid leaking secret key material, i.e.
executing the following three steps:
- secp256k1_ecmult_gen(ctx, &rj, ...)
- secp256k1_ge_set_gej(&r, &rj)
- secp256k1_gej_clear(&rj)
This commit introduces a corresponding helper to deduplicate code
and mitigate the risk that last step is forgotten (which can easily
happen and is not detected by tests).
The idea came up during a conversation with furszy, see
https://github.com/bitcoin-core/secp256k1/pull/1765#issuecomment-4482838033
5698e66c64 Add exhaustive test for ECDH module (Sebastian Falbesoner)
Pull request description:
This PR adds an exhaustive test for the ECDH module, looping over all key combinations and verifying the commutativity property (ECDH(i\*G, j) == ECDH(j\*G, i)) and checking against a recalculated ECDH result (by manually invoking the default ECDH hash function on the precalculated group element `group[i * j]`'s coordinates). The existing test coverage is already solid (including Wycheproof test vectors), but I figured it likely wouldn't hurt to add this as well.
ACKs for top commit:
sipa:
ACK 5698e66c64
real-or-random:
utACK 5698e66c64
Tree-SHA512: e80b8508ee61e3bf5230951393a08c8937f19d2d16220ff2b02fe69b039195a1545809d3ea420dfed1f192c3711f403c63e86c3af2bb2fc4ab1254388ba50287
af1fdd1215 tests: compare full MuSig aggregate nonce (w0xlt)
Pull request description:
This PR has been created by a GitHub Actions workflow without human involvement.
[bitcoin-core/secp256k1#1854]: tests: compare full MuSig aggregate nonce
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.
ACKs for top commit:
mllwchrry:
ACK 8363a2d
Tree-SHA512: 55406814de269c612159922ae521dd874887107ee9bd26cff81e2e2cca28ade74a0609e5c48e25a7d9913698899da146aef8b3eebf96e5ad70933a884d75a655
8479eafa57 musig: always clear out secret key in `secp256k1_musig_nonce_gen_counter` (Sebastian Falbesoner)
3cca6451a2 ci: Bump GCC snapshot major version to 17 (Hennadii Stepanov)
285cb788e9 ci: Replace `ilammy/msvc-dev-cmd` with manual MSVC setup (Hennadii Stepanov)
Pull request description:
This PR has been created by a GitHub Actions workflow without human involvement.
[bitcoin-core/secp256k1#1846]: ci: Replace `ilammy/msvc-dev-cmd` with manual MSVC setup
[bitcoin-core/secp256k1#1848]: ci: Bump GCC snapshot major version to 17
[bitcoin-core/secp256k1#1849]: musig: always clear out secret key in `secp256k1_musig_nonce_gen_counter`
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.
ACKs for top commit:
mllwchrry:
ACK b11340b. This sync PR can be merged as-is.
DarkWindman:
ACK b11340b3ce
Tree-SHA512: 4b311e9bfa21f00b4780202c08af9d00380f5b3df40704641764d706cfc3408615b6206c7c82ca915b56c27a354bdf24680674269a28eefe7d4e93adc06cbaa5
Even though `secp256k1_musig_nonce_gen_internal` can currently only fail
if the API is misused (invalid `keypair` or `keyagg_cache` parameters),
clear out the buffer holding secret key data as well in this case to
follow best practices.
The issue was found and reported by l0rinc using GPT 5.5 (Thanks!).
4359f050cc surjection: Remove test that reads out of bounds (Tim Ruffing)
78999f3a9a surjection: Fix leading whitespace (Tim Ruffing)
229e1f127a surjection: Fix read of uninitialized value in tests (Tim Ruffing)
c0a26a9c1b ci: enable surjectionproof module in CI configs (copilot-swe-agent[bot])
Pull request description:
The CI workflow enabled the ZKP experimental module set but omitted surjectionproof, so that module was never exercised in those jobs. This change wires surjectionproof into the same CI paths as the other enabled ZKP modules.
- **CI module plumbing (`ci/ci.sh`)**
- Added `SURJECTIONPROOF` to the exported/printed CI env set.
- Passed `SURJECTIONPROOF` into configure via `--enable-module-surjectionproof`.
- **Workflow env defaults and matrices (`.github/workflows/ci.yml`)**
- Added a global default: `SURJECTIONPROOF: 'no'`.
- Added `SURJECTIONPROOF: 'yes'` in every job/matrix env block that already enables the ZKP module group (`RANGEPROOF`, `WHITELIST`, `GENERATOR`, etc.).
- **Representative wiring change**
```sh
--enable-module-rangeproof="$RANGEPROOF" \
--enable-module-surjectionproof="$SURJECTIONPROOF" \
--enable-module-whitelist="$WHITELIST"
```
<!-- START COPILOT ORIGINAL PROMPT -->
<details>
<summary>Original prompt</summary>
>
> ----
>
> *This section details on the original issue you should resolve*
>
> <issue_title>ci: surjectionproof module is never enabled</issue_title>
> <issue_description>There's not even a SURJECTIONPROOF env variable in the GitHub Actions workflow... This should be enabled everywhere where the other ZKP modules are enabled.
>
> It may be better to enable all modules by default, but that's a change that could potentially benefit even upstream, so maybe let's first just fix this by enabling surjectionproof.</issue_description>
>
> <agent_instructions>Only add enabling of the surjectionproof module (no enabling of all modules by default)</agent_instructions>
>
> ## Comments on the Issue (you are @copilot in this section)
>
> <comments>
> </comments>
>
</details>
<!-- START COPILOT CODING AGENT SUFFIX -->
- FixesBlockstreamResearch/secp256k1-zkp#334
ACKs for top commit:
mllwchrry:
ACK 4359f05
Tree-SHA512: 13d776a76f73db6e74ca253099583da3d2600f424ea1f6bcbc9cd12544721477225a4190ffd34f4f1a77ab3b1d1e21d1f6d83409aa969bafe66a7978bef1b2d2
4d92a083bc sha256: speed up writes using multi-block compression (furszy)
0753f8b909 Add API to override SHA256 compression at runtime (furszy)
fdb6a91a5e 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: f68bef06d9
ACKs for top commit:
real-or-random:
ACK 4d92a083bc
w0xlt:
ACK 4d92a083bc
theStack:
ACK 4d92a083bc
Tree-SHA512: 058e2e82071f1ca77254b684458292c621e60d65bbcc5500574429717e7db75bc9f3221129fafd11eb5d33e666a5efec5e9844460d3b194ef3b6b16f2df28fb9
921b9711ea 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](9702969978/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 921b9711ea
real-or-random:
utACK 921b9711ea
Tree-SHA512: 44b6bf0132cf00fade526a3fc04e03dc896d04874123614c032206b61f97c81f94d139b6cc0c108eceaa699251580c19420d230b3150607303ca2cb7ab9a0bcb
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.
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.
