Use SHA256 override for pointers to known aux functions #1900

1. 
   real-or-random commented at 8:43 AM on July 29, 2026: contributor

   Resolves one item in #1835.

2. real-or-random added this to the milestone 0.8.0 on Jul 29, 2026
3. real-or-random added the label user-documentation on Jul 29, 2026
4. real-or-random added the label tweak/refactor on Jul 29, 2026
5. 
   real-or-random commented at 9:13 AM on July 29, 2026: contributor

   cc @furszy

6. in include/secp256k1.h:431 in dfc3e98c8e

    426 | @@ -427,6 +427,13 @@ typedef void (*secp256k1_sha256_compression_function)(
    427 |   * specialized implementation. It is NOT meant for replacing SHA256
    428 |   * with a different hash function.
    429 |   *
    430 | + * Since auxiliary functions exposed by the library via a function
    431 | + * pointer such as secp256k1_nonce_function_default not take a context
   

   ---

   ---

   

   theStack commented at 9:22 AM on July 29, 2026:

   missing verb

    * pointer such as secp256k1_nonce_function_default don't take a context
   

   ---

   

   real-or-random commented at 11:09 AM on July 29, 2026:

   fixed

7. in include/secp256k1.h:435 in dfc3e98c8e

    426 | @@ -427,6 +427,13 @@ typedef void (*secp256k1_sha256_compression_function)(
    427 |   * specialized implementation. It is NOT meant for replacing SHA256
    428 |   * with a different hash function.
    429 |   *
    430 | + * Since auxiliary functions exposed by the library via a function
    431 | + * pointer such as secp256k1_nonce_function_default not take a context
    432 | + * object, they will not use the callback when called directly from user
    433 | + * code. (But they will use the callback when called from other library
    434 | + * functions that do take a context object, e.g., when
    435 | + * secp256k1_nonce_function_default is passed to secp256k1_ecdsa_sign.)
   

   ---

   ---

   

   theStack commented at 10:02 AM on July 29, 2026:

   seems that for secp256k1_ecdsa_sign (and also for secp256k1_ecdh), only passing NULL as function pointer currently leads to usage of the context-aware nonce function call, so the recommendation to pass _nonce_function_default explicitly wouldn't work: https://github.com/bitcoin-core/secp256k1/blob/528863e61f55ba3502f680643c55491bca04c150/src/secp256k1.c#L562-L567

   (maybe something we could still fix? // EDIT: thinking of something like https://github.com/theStack/secp256k1/commit/6847ed4475cca7602efe950ed9139010b3e0ff22)

   ---

   

   real-or-random commented at 10:42 AM on July 29, 2026:

   Ah yes, I thought that's the case because we did the same for schnorrsig (as you noticed) and also for ellswift. I will include it in this PR.

   so the recommendation to pass _nonce_function_default explicitly

   Okay, yes, and I didn't intent to add this recommendation. If users want the default, passing NULL directly is more natural (though it doesn't make a difference in the end). I will try to rephrase this.

   ---

   

   real-or-random commented at 11:09 AM on July 29, 2026:

   added a commit

8. real-or-random force-pushed on Jul 29, 2026
9. real-or-random renamed this:
   header: Add note on SHA256 override and aux functions
   Use SHA256 override for pointers to known aux functions
   on Jul 29, 2026
10. real-or-random removed the label tweak/refactor on Jul 29, 2026
11. real-or-random added the label bug on Jul 29, 2026
12. real-or-random added the label tweak/refactor on Jul 29, 2026
13. 
    theStack commented at 11:19 AM on July 29, 2026: contributor

    ACK modulo s/245/256/ nit in 2f38d0cb27854d8b7c87dc0b958dfe77ce13c2b1 commit subject

14. ecdsa/ecdh: Use SHA256 override if known noncefp/hashfp is passed ed091bc49d
15. header: Add note on SHA256 override and aux functions

    Resolves one item in #1835.

    4147f8bdf6
16. real-or-random force-pushed on Jul 29, 2026
17. 
    real-or-random commented at 11:23 AM on July 29, 2026: contributor

    ACK modulo s/245/256/ nit in 2f38d0c commit subject

    lol, fixed :)

18. theStack approved
19. 
    theStack commented at 11:28 AM on July 29, 2026: contributor

    ACK 4147f8bdf628fa95b42adc1d7ce55436eb63861e

20. 
    real-or-random commented at 2:18 PM on July 29, 2026: contributor

    cc @hebasto want to review this too, so it can safely make it into the release?

21. hebasto approved
22. 
    hebasto commented at 5:42 PM on July 29, 2026: member

    ACK 4147f8bdf628fa95b42adc1d7ce55436eb63861e, I have reviewed the code and it looks OK.

23. in include/secp256k1.h:435 in 4147f8bdf6

     426 | @@ -427,6 +427,14 @@ typedef void (*secp256k1_sha256_compression_function)(
     427 |   * specialized implementation. It is NOT meant for replacing SHA256
     428 |   * with a different hash function.
     429 |   *
     430 | + * Since auxiliary functions exposed by the library via a function
     431 | + * pointer such as secp256k1_nonce_function_default do not take a
     432 | + * context object, they will not use the callback when called directly
     433 | + * from user code. (But they will use the callback when called from
     434 | + * other library functions that do take a context object, e.g., when
     435 | + * noncefp==NULL or noncefp==secp256k1_nonce_function_default is passed
    

    ---

    ---

    

    furszy commented at 7:02 PM on July 29, 2026:

    small note; this comment kinda assumes secp256k1_nonce_function_default will always be equal to the used secp256k1_nonce_function_rfc6979 (we are using that instead of the default in the first commit).

    As there are no plans to change the default value ever, it is not a problem. Just something that itches me a bit.

    ---

    

    theStack commented at 11:04 PM on July 29, 2026:

    I had similar thoughts, but also think it's fine. Even if we ever changed the default value (very unlikely indeed), we would then want to also adapt/extend the if clause(s) accordingly to also use the ctx-aware function for that e.g.

    const secp256k1_nonce_function secp256k1_nonce_function_default = nonce_function_new;
    
    {
        ...
        if (noncefp == NULL || noncefp == secp256k1_nonce_function_new) {
            ret = nonce_function_new_impl(...);
        } else if (noncefp == secp256k1_nonce_function_rfc6979) {
            ret = nonce_function_rfc6979_impl(...);
        } else {
            ret = !!noncefp(...);
        }
        ...
    }
    

    and the comment would still apply.

    ---

    

    real-or-random commented at 2:40 PM on July 30, 2026:

    If I understand correctly, what @furszy wanted to say here is that the docs would get out-of-date should we ever switch to a default function which does not use SHA256.

    • Switching the default nonce function to a more efficient one was discussed earlier. It wouldn't be entirely crazy but at this age of the repo, we'd probably rather do this when (if!) we add a modern ECDSA module.
    • But switchting the default nonce function to one not based on SHA256 would be crazy. This should address @furszy's concern.

    ---

    

    theStack commented at 3:19 PM on July 30, 2026:

    Oh, I see now, sorry for misinterpreting then if that was the case. Yeah, not using SHA256 seems pretty wild and far-fetched :)

24. 
    furszy commented at 7:06 PM on July 29, 2026: member

    ACK 4147f8bdf628fa95b42adc1d7ce55436eb63861e

25. theStack merged this on Jul 29, 2026
26. theStack closed this on Jul 29, 2026

    ---

27. achow101 referenced this in commit 09cc345c3e on Aug 12, 2026
28. achow101 referenced this in commit 625f951ba2 on Aug 12, 2026
29. Kino1994 referenced this in commit e790a9e002 on Aug 19, 2026
30. apoelstra referenced this in commit 037cc6d74c on Sep 5, 2026
31. nuttycom referenced this in commit e9498a5cf6 on Sep 30, 2026
32. Sjors referenced this in commit b204ba877c on Oct 29, 2026

Contributors

real-or-randomtheStackhebastofurszy

Milestone
0.8.0