Closed
Conversation
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 file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[bitcoin-core/secp256k1#1824]: util: introduce and use
ARRAY_SIZEmacro[bitcoin-core/secp256k1#1777]: Make SHA256 compression runtime pluggable
[bitcoin-core/secp256k1#1834]: ecmult: Document and test ng=NULL in ecmult
Tips:
git show --remerge-diff <pr-branch>to show the conflict resolution in the merge commit.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.
Manual edit: Let me add that this PR has been automatically created by a GitHub Actions workflow. Perhaps we should create a separate bot account...