Make SHA256 compression pluggable

furszy commented at 11:18 pm on November 27, 2025: member

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.

This PR gives our users a way to bring their own SHA256 compression, either at build time or at runtime, while keeping the default behavior untouched for everyone else.

Compile-time: External SHA256 module

The built-in transform is moved out of hash_impl.hinto a small sha module with its own public header (secp256k1_sha.h). The idea is to stop treating the compression step as a private, hidden detail, and instead make it part of a public interface that users can swap out if they want to. Both build systems gain optional support for pointing libsecp to an external implementation:

Autotools: --with-external-sha256=<path>
CMake: SECP256K1_EXTERNAL_SHA256_PATH

Note: Due to our current limitation of disabling C++ builds (see this), the provided external implementation must be written in C. (A way to overcome this would be to compile and link this externally instead of doing it within the library).

Runtime: Context callback

For setups where we detect the available SHA256 compression function at runtime and cannot re-compile the sources, or when the function is not written in bare C89, there’s a new API:

0secp256k1_context_set_sha256_transform_callback(ctx, fn_transform)

This lets users plug-in their hardware-optimized implementation into the secp256k1_context, which is required in all functions that compute sha256 hashes. During the initial setting, as a sanity check, this mechanism verifies that the provided compression function is equivalent to the default one.

As a quick example, the changes required to implement this in Bitcoin-Core at runtime are very straightforward: https://github.com/furszy/bitcoin-core/commit/f68bef06d95a589859f98fc898dd80ab2e35eb39

Implementing this at compile time in Bitcoin Core is possible, but it would require changing the C version we build against (which is not recommended). For example, we cannot link the SHA-NI implementation we have in Core (even if rewritten in C89) because it depends on the arm_neon library, which requires C99.

furszy force-pushed on Nov 28, 2025

fjahr commented at 4:16 pm on December 1, 2025: contributor

I guess this is more of a draft for initial review but I will spell it out anyway: It’s currently missing some CI coverage for building with an external library as well as some docs. Curious what people think in terms of docs for something like this: Should there be extensive guidance in which context this is safe to use or would users be left on their own to try it out? Or are we assuming all users that go to this length are competent enough to judge if it’s a good idea to bring their own sha256 or use the systems one?

furszy commented at 7:03 pm on December 1, 2025: member

Thanks for the feedback fjahr!

I guess this is more of a draft for initial review but I will spell it out anyway: It’s currently missing some CI coverage for building with an external library as well as some docs.

Yeah, can create a CI job testing the compile-time pluggable compression function very easily. Thanks for reminding that.

And about the missing docs; yeah. I didn’t add it because we currently don’t have much documentation outside configure.ac / CMakeLists.txt. Happy to create a file for it.

Curious what people think in terms of docs for something like this: Should there be extensive guidance in which context this is safe to use or would users be left on their own to try it out? Or are we assuming all users that go to this length are competent enough to judge if it’s a good idea to bring their own sha256 or use the systems one?

It’s not that we’re letting people plug in whatever they want. Both introduced features have guardrails:

Compile-time: we run all current tests against the provided implementation, plus a runtime self-test.
Runtime: besides the runtime self-test, have introduced an “equivalence check” that hashes known inputs and compares them against the library’s internal implementation before accepting the external function.

So you can bring your own compression function, but it still has to prove it behaves exactly like ours bit-for-bit.

Also, the target user here is someone who’s actually written a hardware-optimized SHA256. It’s not like they stumbled into this by accident.

in include/secp256k1.h:96 in 7fefa9c851

92@@ -92,6 +93,7 @@ typedef struct secp256k1_ecdsa_signature {
93  * the message, the algorithm, the key and the attempt.
94  */
95 typedef int (*secp256k1_nonce_function)(
96+    const secp256k1_context *ctx,

real-or-random commented at 8:11 am on December 8, 2025:

Argh, that’s a breaking change. I didn’t see that coming.

But I think it can be avoided. If the user wants to pass a custom nonce function, they anyway have no way of calling our internal function (whether it uses a custom SHA256 transform or not).

So we can keep the struct here and add the context arg only to our built-in nonce function. Our ECDSA signing function (and Schnorr signing, and ECDH, etc.) will need to special-case the built-in nonce function because it has a different function signature. Not elegant but it will do the job.

~~Maybe we can come up with something nicer if we (ever) add a modern ECDSA module and deprecate the ECDSA stuff in the main secp256k1.h file.~~ edit: Probably no because this affects not just ECDSA.

furszy commented at 9:23 pm on December 15, 2025:

So we can keep the struct here and add the context arg only to our built-in nonce function. Our ECDSA signing function (and Schnorr signing, and ECDH, etc.) will need to special-case the built-in nonce function because it has a different function signature. Not elegant but it will do the job.

Done. Spent quite a bit of time thinking about this, mainly because the so-called “default” function (the one exposed in the public header here) is not the one will be used when a custom sha256 transform is provided.., which somewhat defeats the purpose of calling it “default”. Still, after a few rounds of thought and discussions with theStack and sipa, I think this is fine. It’s expected that a “different” function is used when the user provides own sha transform.

in src/hash.h:17 in 7fefa9c851

 9@@ -10,13 +10,20 @@
10 #include <stdlib.h>
11 #include <stdint.h>
12 
13+typedef void (*sha256_transform_callback)(uint32_t *state, const unsigned char *block, size_t rounds);
14+
15+/* Validate user-supplied SHA-256 transform by comparing its output against
16+ * the library's linked implementation */
17+static int secp256k1_sha256_check_transform(sha256_transform_callback fn_transform);

real-or-random commented at 8:22 am on December 8, 2025:

Do we need this function given that there’s already secp256k1_selftest_sha256(void), which is run when a context is created? The existing one seems a bit simpler than your function and it’s already there, so using that will make the diff smaller (but it will need to be modified to obtain an optional context). And perhaps the existing one is a bit slower than yours. I’d say either function is fine but keeping both seems overkill.

(The existing selftest is currently in selftest.h. That’s already wrong; it should have been put in a selftest_impl.h. But your approach is better: the SHA256 test function should be exposed from the internal hash module and then called from selftest.)

edit: Okay, I see now. It’s annoying than I thought. We expose secp256k1_selftest(void) which doesn’t get a context object… The reason it’s exposed is that you can use it when using the static context.

So I believe it makes sense to check the SHA256 at two places:

In secp256k1_selftest(void) because this will catch bugs in the built-in SHA256. This is particularly important with this PR given that it makes possible overriding the built-in implementation it at build time.
When setting a custom SHA256 transform.

This means we may perform two checks in the worst case, but that won’t be the end of the world. You get the overhead only at library initialization time. But the question remains: How should the test look like? And I still think what I said above is true: It’s okay to have a single secp256k1_sha256_selftest function in the hash module, and this can be called from the selftest module and from secp256k1_context_set_sha256_transform_callback.

furszy commented at 3:54 pm on December 16, 2025:

So I believe it makes sense to check the SHA256 at two places:

In secp256k1_selftest(void) because this will catch bugs in the built-in SHA256. This is particularly important with this PR given that it makes possible overriding the built-in implementation it at build time.

When setting a custom SHA256 transform.

This means we may perform two checks in the worst case, but that won’t be the end of the world. You get the overhead only at library initialization time. But the question remains: How should the test look like? And I still think what I said above is true: It’s okay to have a single secp256k1_sha256_selftest function in the hash module, and this can be called from the selftest module and from secp256k1_context_set_sha256_transform_callback.

Yeah, I did start from that idea originally (that’s essentially why we also call to secp256k1_selftest_sha256 inside secp256k1_context_set_sha256_transform first), but I ended up feeling that the existing selftest is a bit too weak for what we want to validate here.

It only checks a single 64-byte message, so it exercises exactly one compression round and never goes through the padding path or the buffering across multiple writes. Which felt insufficient to me. Validating the transform against the internal implementation also seemed like a reasonable extra safety net to ensure parity without adding more hardcoded values.

Structurally, I agree that having a single secp256k1_sha256_selftest living in the hash module, makes sense.

The thing now is that neither test alone fully covers both aspects: we still want a check against hardcoded outputs, but we also want to verify more realistic usage patterns. That’s why I thought having both was useful. That said, we could get similar coverage by extending the existing selftest with more hardcoded values too.. (this comes from a nice talk I had with @theStack). So.. probably we could re-use the existing one for now, and extend its coverage in a follow-up. Happy to hear your thoughts

furszy commented at 3:56 pm on December 17, 2025:

I missed to update this one. The latest push uses only secp256k1_sha256_selftest. Calling it twice as suggested.

in src/hash.h:28 in 7fefa9c851

24 } secp256k1_sha256;
25 
26-static void secp256k1_sha256_initialize(secp256k1_sha256 *hash);
27+static void secp256k1_sha256_initialize(secp256k1_sha256 *hash, sha256_transform_callback fn_transform);
28 static void secp256k1_sha256_write(secp256k1_sha256 *hash, const unsigned char *data, size_t size);
29 static void secp256k1_sha256_finalize(secp256k1_sha256 *hash, unsigned char *out32);

real-or-random commented at 8:31 am on December 8, 2025:

A potential problem with storing the callback in the secp256k1_sha256 struct is that you could have this series of events:

User sets callback to cb1.
User calls API function that calls secp256k1_sha256_initialize, which outputs some opaque object that contains the secp256k1_sha256 struct.
User sets callback go cb2.
Users passes the opaque object to another API function, which calls secp256k1_sha256_write, which calls cb1. UB.

(I believe) this cannot happen in the current code because there’s no pair of API functions that behaves in this way, but it’s a potential footgun for the future.

My suggestion is just passing the context object to every internal function that needs the SHA256 callback. (Another angle: State is annoying. We have state already in the context, so let’s try to keep it there.)

furszy commented at 9:05 pm on December 16, 2025:

My suggestion is just passing the context object to every internal function that needs the SHA256 callback.

I did started there, but didn’t really like where it was leading me. Mainly because going down that path will be significantly more invasive than the current changes: It requires changing every secp256k1_sha256_write call to receive the function arg, every hmac, tagged hash etc. It also includes changing secp256k1_sha256_finalize! which will also require the function arg because we call write during finalization too. Overall, I think the end result would be quite ugly for what we gain here (see below for more rationale).

A potential problem with storing the callback in the secp256k1_sha256 struct is that you could have this series of events:

User sets callback to cb1.

User calls API function that calls secp256k1_sha256_initialize, which outputs some opaque object that contains the secp256k1_sha256 struct.

User sets callback go cb2.

Users passes the opaque object to another API function, which calls secp256k1_sha256_write, which calls cb1. UB.

(I believe) this cannot happen in the current code because there’s no pair of API functions that behaves in this way, but it’s a potential footgun for the future.

I’m not fully convinced it’s a realistic scenario. Changing the sha256 transform callback in the middle of an ongoing procedure doesn’t really make sense in any intended usage. If someone does that, they should already be well outside the model we expect the API to be used under. To me, this is similar to deleting the callback function from another thread while a libsecp operation is executing.

Also, exposing an in-use sha256 object across API boundaries is something we should simply never do right?. That feels really ugly. These structs are only valid under the assumption that they were initialized via secp256k1_sha256_initialize; otherwise, the internal fields would already be invalid, independently of the transform callback. And we cannot ensure that the struct has been initialized properly (or not corrupted) if we share it externally. I mean, if we ever want to share an existing sha256, I would say we should share the state and buffer directly as bytes and then re-initialize the struct on the receiving side.

So overall, I think it worth documenting the expectation clearly for sure: like sha256 structs must not be shared externally, and the provided sha256 callback, must be provided during app’s init, and must remain active for the entire app lifecycle once provided (because it doesn’t make sense to change it). But I’m not sure this is a problem we need to design around. It would add other complications without a clear benefit. But let me know what you think.

furszy commented at 7:04 pm on December 17, 2025:

Maybe I should give the “pass the function to every write call” patch-set another try and compare the resulting changes. I did it early on so maybe it is not as bad as I remember it. Let me see..

real-or-random commented at 8:35 pm on December 17, 2025:

Also, exposing an in-use sha256 object across API boundaries is something we should simply never do right?. That feels really ugly.

It doesn’t feel ugly to me. Of course, we wouldn’t share it standalone but inside some other object. For example, like we expose an secp256k1_scalar inside an opaque secp256k1_musig_keyagg_cache.

And I don’t think the use case would be unrealistic. Having a “hash builder” is common, and it could very well be part of a high-level function. Just imagine an advanced schnorrsig signing object that lets you append to the message multiple times before finalizing the signatures (e.g., for cases in which you want to sign a long message whose parts you receive step by step).

real-or-random commented at 8:39 pm on December 17, 2025:

Maybe I should give the “pass the function to every write call” patch-set another try and compare the resulting changes. I did it early on so maybe it is not as bad as I remember it. Let me see..

Yes, I see that passing the function down is annoying and that it has a non-zero readability and maintenance cost. But I imagine it’s not too bad. (I tend to agree with sipa here: reviewing many obvious diff lines is not an issue.) Seeing the resulting diff would certainly help to see how bad it is.

furszy commented at 6:41 pm on December 18, 2025:

Maybe I should give the “pass the function to every write call” patch-set another try and compare the resulting changes. I did it early on so maybe it is not as bad as I remember it. Let me see..

Yes, I see that passing the function down is annoying and that it has a non-zero readability and maintenance cost. But I imagine it’s not too bad. (I tend to agree with sipa here: reviewing many obvious diff lines is not an issue.) Seeing the resulting diff would certainly help to see how bad it is.

Ok done, both options available #1777#pullrequestreview-3594435763

in src/secp256k1.c:62 in 7fefa9c851

56@@ -56,20 +57,27 @@
57     } \
58 } while(0)
59 
60+struct secp256k1_hash_context {
61+    sha256_transform_callback fn_sha256_transform;
62+};

real-or-random commented at 8:37 am on December 8, 2025:

I think it will be nice to define this in the internal hash module, just like secp256k1_ecmult_gen_context is defined in the ecmult_gen module.

furszy commented at 9:18 pm on December 16, 2025:

I think it will be nice to define this in the internal hash module, just like secp256k1_ecmult_gen_context is defined in the ecmult_gen module.

Sure. Done as suggested.

in include/secp256k1.h:428 in 7fefa9c851

423+ * In: callback: pointer to a function implementing the transform step.
424+ *               (passing NULL restores the default implementation)
425+ */
426+SECP256K1_API void secp256k1_context_set_sha256_transform_callback(
427+        secp256k1_context *ctx,
428+        void (*sha256_transform_callback)(uint32_t *state, const unsigned char *block, size_t rounds)

real-or-random commented at 8:45 am on December 8, 2025:

I was wondering whether it makes sense to allow for a real return value here to make it possible for the callback to indicate some kind of failure (e.g., couldn’t access external hardware, or malloc failed). We could then fall back to our implementation or simply call the error callback in order to crash.

My current conclusion is no: I can’t imagine this being useful in practice. (If you call external hardware, this will be super slow anyway. If you use malloc for SHA256, you’re doing it wrong.) Even if there’s some failure, the callback will still have the possibility to crash the process.

real-or-random commented at 8:55 am on December 8, 2025:

What’s the motivation for the rounds arg? (Do you think we’ll ever call this with any other value than 1?)

furszy commented at 9:29 pm on December 16, 2025:

What’s the motivation for the rounds arg? (Do you think we’ll ever call this with any other value than 1?)

I think it’s better to introduce it now, even if it’s currently always set to 1, so we don’t end up breaking the API later if we ever need it. But can remove it if you are strongly opposed to it.

real-or-random commented at 9:20 am on December 17, 2025:

so we don’t end up breaking the API later if we ever need it.

I might be missing something but I really tend to think that is too obscure.

I can’t imagine a reason why we would ever need it. Applying the transform more than once to the same data is not done within SHA256. So we’ll need this if we ever implement our own hardened hash function SHA256+ based on SHA256?
Even if it turns out that we need this functionality, we can simply call the transform in a loop. Perhaps that will come with a tiny performance disadvantage due to the overhead of the function call, but that will be acceptable.

furszy commented at 7:33 pm on December 17, 2025:

I was wondering whether it makes sense to allow for a real return value here to make it possible for the callback to indicate some kind of failure (e.g., couldn’t access external hardware, or malloc failed). We could then fall back to our implementation or simply call the error callback in order to crash.

hmm, how would we actually call the error callback from inside hash_impl.h? That would require passing the context all the way down into the sha256 sha_write and sha_finalize functions (hmac, tagged hashes, etc), which would introduce a hash_impl.h dependency on secp256k1.h (unless we move the context definition elsewhere). But this seems to be a low-level primitive file that shouldn’t receive the context.

real-or-random commented at 8:26 pm on December 17, 2025:

But this seems to be a low-level primitive file that shouldn’t receive the context.

Hm, I don’t think there is (or should be) a rule that says low-level primitives shouldn’t receive the error callback. It doesn’t need to be the full context.

But let me just resolve this conversation. As I already wrote above, I doubt that my suggestion was useful in the first place.

furszy commented at 7:05 pm on December 18, 2025:

Ok yeah, agree. Adding this just to repeat the compression on the same block doesn’t make much sense, but (idea) what about processing multiple blocks in a row? An API for that could be useful since implementations would be able to skip per-block-compression variables initialization, cleanups, etc.

sipa commented at 9:38 pm on December 18, 2025:

@furszy That makes more sense - and it’s apparently also how the Bitcoin Core transform implementations work (but call it blocks, not rounds).

furszy commented at 4:20 pm on December 19, 2025:

@furszy That makes more sense - and it’s apparently also how the Bitcoin Core transform implementations work (but call it blocks, not rounds).

ha, that explains where the idea came from. It must have been in the back of my mind.

in include/secp256k1_sha.h:1 in 75ccf99a07 outdated

real-or-random commented at 9:34 am on December 8, 2025:

What’s the motivation for exposing the SHA256 compression?

Unless I’m missing something, then if anything, I think we’d want to expose the high-level SHA256 function. I’m not sure how useful it is, and we tried to keep the library focused on ECC. On the other hand, we’ll have it anyway in the code base, and it’s required for Schnorr sigs (and we anyway expose a tagged hash for Schnorr sigs). So if you ask me, it’s fine to expose SHA256 as long as we add a comment that says that users shouldn’t expect a highly performant implementation.

But perhaps it’s better to debate the exposing in a separate PR.

furszy commented at 9:42 pm on December 16, 2025:

What’s the motivation for exposing the SHA256 compression?

Unless I’m missing something, then if anything, I think we’d want to expose the high-level SHA256 function. I’m not sure how useful it is, and we tried to keep the library focused on ECC. On the other hand, we’ll have it anyway in the code base, and it’s required for Schnorr sigs (and we anyway expose a tagged hash for Schnorr sigs). So if you ask me, it’s fine to expose SHA256 as long as we add a comment that says that users shouldn’t expect a highly performant implementation.

Actually, the motivation wasn’t so much that I expect people to use it directly. It’s more about making an explicit statement in the API: the sha256 transform is something that can be replaced or disabled if you want to (thus I thought it could be a new module). Exposing it makes that contract clear, instead of it being an internal detail that people can obscurely replace.

That said, I agree this is somewhat orthogonal topic.

real-or-random commented at 9:11 am on December 17, 2025:

It’s more about making an explicit statement in the API: the sha256 transform is something that can be replaced or disabled if you want to (thus I thought it could be a new module). Exposing it makes that contract clear, instead of it being an internal detail that people can obscurely replace.

Maybe my thinking is just too simplistic, but the only thing that exposing makes clear to me (as a user) is that I can call it, (and that there’s some kind of guarantee that future releases will contain a function with the same signature and behavior).

That said, I agree this is somewhat orthogonal topic.

I think so, yes.

What could be done to make the API docs more explicit is to point out in the docs for secp256k1_context_set_sha256_transform_callback where this is used by the library without being an implementation detail (namely Schnorr sigs, ECDH, IIRC also ellswift). Would probably suffice to say something like “e.g., Schnorr signatures and the default key derivation function for ECDH”.

in include/secp256k1.h:426 in 7fefa9c851

421+ *
422+ * Args: ctx:    pointer to a context object.
423+ * In: callback: pointer to a function implementing the transform step.
424+ *               (passing NULL restores the default implementation)
425+ */
426+SECP256K1_API void secp256k1_context_set_sha256_transform_callback(

real-or-random commented at 9:38 am on December 8, 2025:

Naming: I wonder whether it makes sense to avoid the word callback here. Sure, it’s a callback, but whenever we said callback in the past 10 years of this library, it was about error handling. We even have type secp256k1_callback (which probably should have been called secp256k1_error_callback but it’s too late now.)

Maybe we can just call it “function " and “function pointer”, like we do in the bring-your-own-nonce-function interfaces.

furszy commented at 9:19 pm on December 16, 2025:

Naming: I wonder whether it makes sense to avoid the word callback here. Sure, it’s a callback, but whenever we said callback in the past 10 years of this library, it was about error handling. We even have type secp256k1_callback (which probably should have been called secp256k1_error_callback but it’s too late now.)

Sure. Renamed to secp256k1_context_set_sha256_transform 👍🏼 .

in src/secp256k1.c:232 in 7fefa9c851

228@@ -220,6 +229,18 @@ void secp256k1_context_set_error_callback(secp256k1_context* ctx, void (*fun)(co
229     ctx->error_callback.data = data;
230 }
231 
232+void secp256k1_context_set_sha256_transform_callback(secp256k1_context *ctx, void (*sha256_transform_callback_)(uint32_t *state, const unsigned char *block, size_t rounds)) {

real-or-random commented at 9:42 am on December 8, 2025:

It will be nice to have a typedef for the function type in the public header.

real-or-random commented at 7:48 pm on December 19, 2025:

If I’m not mistaken, this comment is still unaddressed.

Let me add that the typedef is a good place to document the behavior of the function (e.g., the blocks arg). The naming of the args should be reconsidered because now block may point to more than one block. Maybe (..., const unsigned char *blocks64, size_t n_blocks).

furszy commented at 8:12 pm on December 19, 2025:

If I’m not mistaken, this comment is still unaddressed.

Let me add that the typedef is a good place to document the behavior of the function (e.g., the blocks arg). The naming of the args should be reconsidered because now block may point to more than one block. Maybe (..., const unsigned char *blocks64, size_t n_blocks).

Done, pushed. Sorry for missing this one. Most of the function docs were inside the build-time feature commit. In the module definition; Moved what had here and renamed the args.

in configure.ac:141 in 7fefa9c851 outdated

133@@ -134,6 +134,12 @@ SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS)
134 ### Define config arguments
135 ###
136 
137+AC_ARG_WITH([external-sha256],
138+  AS_HELP_STRING([--with-external-sha256=PATH], [use external SHA256 compression implementation]),
139+  [external_sha256_path="$withval"],
140+  [external_sha256_path=""]
141+)

real-or-random commented at 10:43 am on December 8, 2025:

Regarding the build stuff for the build-time override: We already have the possibility to have build-time overrides for the error callbacks, e.g., see AC_ARG_ENABLE(external_default_callbacks, below.

When it comes to the implementation of how the new override is configured and performed, I think it’s similar to the existing overrides for the callback. (Except that these don’t allow configuring the header to be included, which makes them rather useless… #1461)

So this will need a rework anyway [1]. This belongs to a different PR, of course, but I think the SHA override here is a good opportunity to think about a better design of a compile-time override, so we may want to iterate on this a bit more than expected. No matter how it’s designed, I think we should, in the long run, have only one (non-deprecated) mechanism for a build-time override of a function.

[1] I don’t think what I did for the error callbacks great, and I guess it would be okay to change it (I doubt that many users rely on this). Or even better, we could deprecate it and simply provide ways to provide build-time overrides for standard library symbols like printf, stderr, and abort. See for example the approach taken by mbedTLS (in the past ?) which is built with embedded systems in mind: https://github.com/Mbed-TLS/mbedtls/blob/550a18d4d6b29e62b6824201d8a49b8224e61c97/tf-psa-crypto%2Fdrivers%2Fbuiltin%2Finclude%2Fmbedtls%2Fplatform.h They provide macros for overriding standard library functions. So you have full flexibility as a user: if you’ve implemented a function that aborts, but it’s not called abort() (perhaps because that is a reserved name) then you can link it to the library also with a different name.

furszy commented at 4:21 pm on December 17, 2025:

Hmm, I see. Those are very good points. Agree this worth a deeper discussion.

No problem splitting the changes. We can keep this PR focused on the runtime feature and handle the build-time override design separately. The only change we need from the build-time commit for this PR is exposing the transform function in hash.h, so we can initialize the context default function.

real-or-random commented at 8:23 pm on December 17, 2025:

We can keep this PR focused on the runtime feature and handle the build-time override design separately

Right, I hadn’t considered this option. That’s a good idea. (And I see that sipa suggested the same below.)

in configure.ac:137 in 7fefa9c851 outdated

133@@ -134,6 +134,12 @@ SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS)
134 ### Define config arguments
135 ###
136 
137+AC_ARG_WITH([external-sha256],

real-or-random commented at 10:43 am on December 8, 2025:

nit: I think this one should also be an enable-style configure option instead of a with-style configure options; with is for compiling with external packages e.g., with-libxyz. And what the user provides here is not a package.

real-or-random commented at 10:44 am on December 8, 2025: contributor

Concept ACK, great to see a PR for this!

real-or-random added the label feature on Dec 8, 2025

real-or-random added the label build on Dec 8, 2025

real-or-random added the label performance on Dec 8, 2025

real-or-random added the label needs-changelog on Dec 8, 2025

hebasto commented at 4:24 pm on December 15, 2025: member

Concept ACK.

furszy force-pushed on Dec 16, 2025

furszy commented at 9:48 pm on December 16, 2025: member

Update pushed, thanks for the deep review real_or_random! I haven’t fully finished addressing all the comments yet, but I’ll hopefully finish tomorrow. So many good topics.

sipa commented at 7:44 pm on December 17, 2025: contributor

A few high-level comments:

I don’t think we should do link-time plugging in the same PR as runtime plugging, and when we do, I don’t think it should be done through the “module” abstractions. We use modules for exporting functionality from the library, not for offering replacement of internal dependencies (I find it strange to have base functionality depend on a module!).
“round” is a term of art in cipher design, and SHA-256 has exactly 64 rounds. Using the same term with another meaning is confusing.
I would try hard to avoid storing the transformation function pointer inside the secp256k1_sha256 struct. Reviewing many lines that just add an extra parameter being pass through is not hard.

tests: use local context instead of using global one

No functional change.

Refactor several test implementations to use a local context
variable.

This is a preparatory change that makes it straightforward for a
follow-up to run these tests with a different context instance
(with a different sha256 transform function), without touching
the test code again.

ee448525df

furszy commented at 6:31 pm on December 18, 2025: member

Per feedback, this is how it would look if we pass the function everywhere https://github.com/furszy/secp256k1/commit/8149fc119071c532e9d2d6c3b633af7a37e5a0b8 vs how it looks when the function lives in the sha256 struct (an updated version of what we currently have here): https://github.com/furszy/secp256k1/commit/eb570826371f42a5bd320f8e43800281b1f63b0a

Let me know what you guys think.

sipa commented at 9:41 pm on December 18, 2025: contributor

@furszy From a quick glance, the passing of the function everywhere is fine I think.

I’d suggest passing a pointer to secp256k1_hash_context to the sha256 functions rather than the function pointer itself. That makes it easier to (if ever) add additional fields to that context.

furszy force-pushed on Dec 19, 2025

furszy commented at 5:58 pm on December 19, 2025: member

Updated per feedback. We now pass the hash context to all functions that need it, without storing any state inside the sha256 struct. The API arg “rounds” has also been renamed to “blocks” to prevent confusion with existing terms. Also, the commit introducing the compile-time feature has been removed for inclusion at a later stage (see #1777 (review) for more details).

real-or-random commented at 7:49 pm on December 19, 2025: contributor

Nice, I hope I’ll find the time have another look soon

Add API to override SHA256 transform at runtime

This introduces `secp256k1_context_set_sha256_transform()`,
which allows users to provide their own SHA256 block-compression
function at runtime.

This is useful in setups where the optimal implementation is detected
dynamically, where rebuilding the library is not possible, or when
the compression function is not written in bare C89.

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.

041a6217fa

furszy force-pushed on Dec 19, 2025

real-or-random commented at 8:08 am on December 23, 2025: contributor

Nice, I hope I’ll find the time have another look soon

Sorry, I didn’t manage to find the time before the holidays…

in include/secp256k1.h:408 in 041a6217fa

403@@ -404,6 +404,49 @@ SECP256K1_API void secp256k1_context_set_error_callback(
404     const void *data
405 ) SECP256K1_ARG_NONNULL(1);
406 
407+/**
408+ * SHA256 block compression raw primitive.

real-or-random commented at 10:10 am on January 6, 2026:

0 * A pointer to a function that  implementation of SHA256's internal compression function

“A pointer to” is a bit more in line with the other function pointer types. (" * A pointer to a function that implements SHA256’s internal compression function" would be even more in line, but it’s > 80 chars and duplicates “function”)
I’d call it “internal” rather than “raw” because that’s more precise (to me, at least).
The typical name of the thing is “(one-way) compression function”, see https://en.wikipedia.org/wiki/One-way_compression_function

in include/secp256k1.h:412 in 041a6217fa

403@@ -404,6 +404,49 @@ SECP256K1_API void secp256k1_context_set_error_callback(
404     const void *data
405 ) SECP256K1_ARG_NONNULL(1);
406 
407+/**
408+ * SHA256 block compression raw primitive.
409+ *
410+ * This function performs the SHA256 compression on one or more contiguous
411+ * 64-byte message blocks. It does **not** perform padding, message scheduling
412+ * across blocks, or length encoding.

real-or-random commented at 10:21 am on January 6, 2026:

The function pointed to consumes one or more contiguous 64-byte message blocks and updates the internal SHA256 state accordingly. The function is not responsible for counting consumed blocks or bytes, and it is not responsible for performing padding.

What is “message scheduling across blocks”?
“length encoding” is part of padding, no?

in include/secp256k1.h:419 in 041a6217fa

414+ * Parameters:
415+ * - `state`    Pointer to eight 32-bit words representing the current state.
416+ *              The state is updated in place.
417+ * - `blocks64` Pointer to the first 64-byte block; additional blocks must be
418+ *              contiguous in memory.
419+ * - `n_blocks` Number of contiguous 64-byte blocks to process.

real-or-random commented at 10:23 am on January 6, 2026:

nits:

Formatting should be consistent with other docs for function pointer types, see for example secp256k1_nonce_function. That includes s/Parameters/Args
s/current state/current internal state

in include/secp256k1.h:448 in 041a6217fa

443+ *               (passing NULL restores the default implementation)
444+ */
445+SECP256K1_API void secp256k1_context_set_sha256_transform(
446+        secp256k1_context *ctx,
447+        secp256k1_sha256_transform_fn fn_transform
448+) SECP256K1_ARG_NONNULL(1);

real-or-random commented at 1:24 pm on January 6, 2026:

We should use the same terminology here, too.

I’d vote for “compression function” instead of “transform”. It’s the term you’d find in the cryptographic literature. Some APIs call the state update “update”, some call it “transform”. But in either case, they refer to updating the entire internal state including the byte/block count, which is not what we’re doing here. Moreover, “SHA256 transform” could be misread as (the whole) “SHA256 function”. So I believe “compression function” is the most precise term for our purposes.

in src/modules/ecdh/main_impl.h:64 in 041a6217fa

59@@ -60,7 +60,12 @@ int secp256k1_ecdh(const secp256k1_context* ctx, unsigned char *output, const se
60     secp256k1_fe_get_b32(x, &pt.x);
61     secp256k1_fe_get_b32(y, &pt.y);
62 
63-    ret = hashfp(output, x, y, data);
64+    if (hashfp == NULL) {
65+        /* Use context-aware function by default */

real-or-random commented at 1:29 pm on January 6, 2026:

nit (multiple times):

0        /* Use ctx-aware function by default */

Or “secp256k1_context-aware”. This makes it clear that the comment refers to the context object. I was confused first because “context-aware” is an ordinary English word used in many contexts in which “context” has a much less specific meaning.

in src/modules/ellswift/main_impl.h:584 in 041a6217fa

579@@ -572,8 +580,15 @@ int secp256k1_ellswift_xdh(const secp256k1_context *ctx, unsigned char *output,
580     secp256k1_fe_normalize(&px);
581     secp256k1_fe_get_b32(sx, &px);
582 
583-    /* Invoke hasher */
584-    ret = hashfp(output, sx, ell_a64, ell_b64, data);
585+    /* Invoke hasher.
586+     * Use context-aware function when custom SHA256 is provided */

real-or-random commented at 1:31 pm on January 6, 2026:

0     * Use ctx-aware function by default

This may use the context-aware function even if no custom SHA256 (transform) is provided.

in src/testutil.h:20 in 041a6217fa

15+#define DEFINE_SHA256_TRANSFORM_PROBE(name)                                     \
16+    static int name##_called = 0;                                               \
17+    static void name(uint32_t *s, const unsigned char *msg, size_t rounds) {    \
18+        name##_called = 1;                                                      \
19+        secp256k1_sha256_transform(s, msg, rounds);                             \
20+    }

real-or-random commented at 1:45 pm on January 6, 2026:

I believe what we actually want to test here is not that the custom SHA256 was called but that the ctx-aware function was called. If I’m right, this may be simpler to test: The ctx-aware function will always be the “default” function, e.g., the default ECDH hash function. So shouldn’t it suffice to check correctness of ECDH once with the default function (NULL) and once with a custom hash function (not custom SHA256 transform)?

real-or-random commented at 1:46 pm on January 6, 2026: contributor

Approach ACK

Make SHA256 compression pluggable #1777

Compile-time: External SHA256 module

Runtime: Context callback