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.
f48b1bfa5d hash: add midstate initializer and use it for tagged hashes (w0xlt)
Pull request description:
Each tagged hash midstate function (e.g., `secp256k1_schnorrsig_sha256_tagged`) calls `secp256k1_sha256_initialize` before immediately overwriting every field it sets: `s[0]` through `s[7]` and `bytes`. The `buf[64]` member does not need initialization either, because `bytes` is set to 64, which means the buffer position (`bytes & 0x3F`) (`= bytes % 64`) is 0, so buf is always written before being read.
Remove the 11 redundant `secp256k1_sha256_initialize` calls across the `schnorrsig`, `ellswift`, and `musig` modules.
ACKs for top commit:
real-or-random:
utACK f48b1bfa5d
theStack:
Code-review ACK f48b1bfa5d
Tree-SHA512: 769beb96f3921cc3c180ed0d17484ffa0dc78041c889a8e56603679d8eaca5fe13e63759ada78f83d8e0ff7aae392e6bcbc1a9fe8b959105ea4a3d8ef51abf15
Introduce secp256k1_sha256_initialize_midstate() in the hash layer and use it at all tagged-hash midstate call sites across schnorrsig, musig, and ellswift.
Document the byte-counter contract at the declaration site in hash.h and add run_sha256_initialize_midstate_tests() to directly verify helper behavior against initialize_tagged.
Also switch the helper to take const uint32_t state[8] to reduce argument-order risk at call sites.
The secp256k1_ellswift_xdh function uses overflow = secp256k1_scalar_is_zero(&s) which overwrites the overflow flag from the preceding secp256k1_scalar_set_b32 call. This means secret keys >= the curve order are silently accepted (reduced mod n) instead of being rejected.
The fix changes = to |=, matching the correct pattern already used in secp256k1_ecdh (main_impl.h, line 51).
The ECDH module's test suite explicitly tests overflow rejection (passes secp256k1_group_order_bytes as a key and checks the function returns 0). The ellswift test suite has no corresponding test, which is why this went undetected.
No behavior changes.
Refactors the previously monolithic ElligatorSwift test into isolated,
independent test cases. Doing so allows the test suite to execute
these cases in parallel rather than sequentially.
Overall, seen 35-40% tests time reduction locally.
This is quite useful for the Debug build with no optimizations,
which is noticeably slow.
#### Local Debug-build Results (7 jobs):
- master: 138.0 seconds.
- this PR: 89.3 seconds.
(~1.55× speedup, ~35% reduction)
#### Local Release-build Results (7 jobs):
- master: 9.5 seconds.
- this PR: 5.9 seconds.
(~1.61× speedup, ~38% reduction)
2f4546ce56 test: add --log option to display tests execution (furszy)
95b9953ea4 test: Add option to display all available tests (furszy)
953f7b0088 test: support running specific tests/modules targets (furszy)
0302c1a3d7 test: add --help for command-line options (furszy)
9ec3bfe22d test: adapt modules to the new test infrastructure (furszy)
48789dafc2 test: introduce (mini) unit test framework (furszy)
9cce703863 refactor: move 'gettime_i64()' to tests_common.h (furszy)
Pull request description:
Early Note:
Don’t be scared by the PR’s line changes count — most of it’s just doc or part of the test framework API.
Context:
Currently, all tests run single-threaded sequentially and the library lacks the ability to specify which test (or group of tests) you would like to run. This is not only inconvenient as more tests are added but also time consuming during development and affects downstream projects that may want to parallelize the workload (such as Bitcoin-Core CI).
PR Goal:
Introduce a lightweight, extensible C89 unit test framework with no dynamic memory allocations, providing a structured way to register, execute, and report tests. The framework supports named command-line arguments in `-key=value` form, parallel test execution across multiple worker processes, granular test selection (selecting tests either by name or by module name), and time accumulation reports.
The introduced framework supports:
* `-help` or `-h`: display list of available commands along with their descriptions.
* `-jobs=<num>`: distribute tests across multiple worker processes (default: sequential if 0).
* `-target=<name>` or `-t=<name>`: run only specific tests by name; can be repeated to select multiple tests.
* `-target=<module name>`, `-t=<module>` Run all tests within a specific module (can be provided multiple times)
* `-seed=<hex>`: set a specific RNG seed (defaults to random if unspecified).
* `-iterations=<n>`: specify the number of iterations.
* `-list_tests`: display list of available tests and modules you can run.
* `-log=<0|1>`: enable or disable test execution logging (default: 0 = disabled).
Beyond these features, the idea is to also make future developments smoother, as adding new tests require only a single entry in the central test registry, and new command-line options can be introduced easily by extending the framework’s `parse_arg()` function.
Compatibility Note:
The framework continues accepting the two positional arguments previously supported (iterations and seed), ensuring existing workflows remain intact.
Testing Notes:
Have fun. You can quickly try it through `./tests -j=<workers_num>` for parallel execution or `./tests -t=<test_name>` to run a specific test (call `./tests -print_tests` to display all available tests and modules).
Extra Note:
I haven't checked the exhaustive tests file so far, but I will soon. For now, this only runs all tests declared in the `tests` binary.
Testing Results: (Current master branch vs PR in seconds)
* Raspberry Pi 5: master \~100 s → PR \~38 s (5 jobs)
* MacBook Pro M1: master \~30 s → PR \~10 s (6 jobs)
ACKs for top commit:
theStack:
re-ACK 2f4546ce56
real-or-random:
ACK 2f4546ce56
hebasto:
ACK 2f4546ce56.
Tree-SHA512: 85ca2cbb620b84b35b353d5d4cf093c388fc3851ca405eeb0e458f8fa72b60534bccd357c7edabf8fc9aa93d9ad0a6fbac3dd5c4d5f9dfdf4d8701a9834755b9
secp256k1_memclear has the side effect of undefining bytes for
valgrind checks. In some cases, we may want to zero bytes
but allow subsequent reads. So we split memclear into
memclear_explicit, which makes no guarantees about the content
of the buffer on return, and memzero_explicit, which guarantees
zero value on return.
Change the memset in partial_sign to use memzero_explicit.
Move the sha256_tag_test_internal function out of the musig module
into tests.c. This makes it available to other modules wishing to verify tagged
hashes without needing to duplicate the function.
Change the function signature to expect a const unsigned char and update
the tagged hash tests to use static const unsigned char character
arrays (where necessary).
Add a comment for each tag. This is done as a convenience for checking
the strings against the protocol specifications, where the tags are
normally specified as strings.
Update tests in the ellswift and schnorrsig modules to use the
sha256_tag_test_internal helper function.
This gives the caller more control about whether the state should
be cleaned (= should be considered secret). Moreover, it gives the
caller the possibility to clean a hash struct without finalizing it.
All of the invocations of secp256k1_memclear() operate on stack
memory and happen after the function is done with the memory object.
This commit replaces existing memset() invocations and also adds
secp256k1_memclear() to code locations where clearing was missing;
there is no guarantee that this commit covers all code locations
where clearing is necessary.
Co-Authored-By: isle2983 <isle2983@yahoo.com>
The previous code is correct and harmless to initialize an array with a
non-terminated character sequence using a string literal.
However, it requires exactly specifying the array size, which can be
cumbersome.
Also, GCC-15 may issue the -Wunterminated-string-initialization warning.
[1]
Fix both issues by using array initialization. This refactoring commit
does not change behavior.
[1] Example warning:
src/modules/schnorrsig/main_impl.h:48:46: error: initializer-string for array of 'unsigned char' is too long [-Werror=unterminated-string-initialization]
48 | static const unsigned char bip340_algo[13] = "BIP0340/nonce";
| ^~~~~~~~~~~~~~~
By providing an uppercase variant of these verification functions, it is
better visible that it is test code and surrounding `#ifdef VERIFY`
blocks can be removed (if there is no other code around that could
remain in production mode), as they don't serve their purpose any more.
At some places intentional blank lines are inserted for grouping and
better readadbility.
Now that the `VERIFY_CHECK` compiles to empty in non-VERIFY mode, blocks
that only consist of these macros don't need surrounding `#ifdef VERIFY`
conditions anymore.
At some places intentional blank lines are inserted for grouping and
better readadbility.
As suggested in issue #1381, this will make things simpler and
improve code readability, as we don't need to force omitting of
evaluations on a case-by-case basis anymore and hence can remove
lots of `#ifdef VERIFY`/`#endif` lines (see next commit). Plus,
VERIFY_CHECK behaves now identical in both non-VERIFY and coverage mode,
making the latter not special anymore and hopefully decreasing
maintenance burden. The idea of "side-effect safety" is given up.
Note that at two places in the ellswift module void-casts of return
values have to be inserted for non-VERIFY builds, in order to avoid
"variable ... set but not used [-Wunused-but-set-variable]"
warnings.
When configured with `--disable-module-ecdh --enable-module-recovery`, then
`./tests 64 81af32fd7ab8c9cbc2e62a689f642106` fails with
```
src/modules/ellswift/tests_impl.h:396: test condition failed: secp256k1_memcmp_var(share32_bad, share32a, 32) != 0
```
This tests verifies that changing the `party` bit of the
`secp256k1_ellswift_xdh` function results in a different share. However, that's
not the case when the secret keys of both parties are the same and this is
actually what happens in the observed test failure. The keys can be equal in
this test case because they are created by the `random_scalar_order_test`
function whose output is not uniformly random (it's biased towards 0).
This commit restores the assummption that the secret keys differ.
The scheme implemented is described below, and largely follows the paper
"SwiftEC: Shallue–van de Woestijne Indifferentiable Function To Elliptic Curves",
by Chavez-Saab, Rodriguez-Henriquez, and Tibouchi
(https://eprint.iacr.org/2022/759).
A new 64-byte public key format is introduced, with the property that *every*
64-byte array is an encoding for a non-infinite curve point. Each curve point
has roughly 2^256 distinct encodings. This permits disguising public keys as
uniformly random bytes.
The new API functions:
* secp256k1_ellswift_encode: convert a normal public key to an ellswift 64-byte
public key, using additional entropy to pick among the many possible
encodings.
* secp256k1_ellswift_decode: convert an ellswift 64-byte public key to a normal
public key.
* secp256k1_ellswift_create: a faster and safer equivalent to calling
secp256k1_ec_pubkey_create + secp256k1_ellswift_encode.
* secp256k1_ellswift_xdh: x-only ECDH directly on ellswift 64-byte public keys,
where the key encodings are fed to the hash function.
The scheme itself is documented in secp256k1_ellswift.h.
