hash: remove redundant secp256k1_sha256_initialize in tagged hash midstate functions #1825

1. 
   w0xlt commented at 7:38 pm on February 17, 2026: contributor

   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.

2. 
   real-or-random commented at 9:26 am on February 18, 2026: contributor

   This has been brought up before, see #1179. Let’s maybe keep the discussion there (unless it’s really specific to this PR).
3. real-or-random added the label tweak/refactor on Feb 18, 2026
4. w0xlt force-pushed on Feb 19, 2026
5. 
   real-or-random commented at 2:01 pm on February 19, 2026: contributor

   Concept ACK
6. 
   theStack commented at 6:33 pm on February 23, 2026: contributor

   Concept ACK
7. in src/hash_impl.h:45 in 10ecf81eec

   39@@ -40,6 +40,16 @@ static void secp256k1_sha256_initialize(secp256k1_sha256 *hash) {
   40     hash->bytes = 0;
   41 }
   42 
   43+/* Initialize a SHA256 hash state with a precomputed midstate.
   44+ * The byte counter must be a multiple of 64, i.e., there must be no unwritten
   45+ * bytes in the buffer. */
   

   ---

   ---

   

   real-or-random commented at 9:35 am on February 25, 2026:

   nit: I think it’s sufficient (and more consistent with the rest of the code base) to have the comment only in hash.h.

   ---

   

   w0xlt commented at 0:31 am on February 26, 2026:

   Done in f48b1bfa5d40a4d7303b196017d2e298520d1066. Thanks.
8. in src/tests.c:770 in 10ecf81eec

   765+        secp256k1_sha256_write(&sha_tagged, msg + split, msg_len - split);
   766+        secp256k1_sha256_write(&sha_midstate, msg, msg_len);
   767+
   768+        secp256k1_sha256_finalize(&sha_tagged, hash_tagged);
   769+        secp256k1_sha256_finalize(&sha_midstate, hash_midstate);
   770+        CHECK(secp256k1_memcmp_var(hash_tagged, hash_midstate, sizeof(hash_tagged)) == 0);
   

   ---

   ---

   

   real-or-random commented at 9:45 am on February 25, 2026:

   I think these are not necessary given that you check test_sha256_eq. Of course, no test is “necessary” in a strict sense, but did you have any specific reason in mind why to check this?

   If you omit them, you could use test_sha256_tag_midstate.

   ---

   

   w0xlt commented at 0:31 am on February 26, 2026:

   Done in f48b1bfa5d40a4d7303b196017d2e298520d1066. Thanks.
9. in src/tests.c:761 in 10ecf81eec

   756+        unsigned char hash_midstate[32];
   757+        size_t msg_len = msg_lens[i];
   758+        size_t split = msg_len / 2;
   759+
   760+        secp256k1_sha256_initialize_tagged(&sha_tagged, tag, sizeof(tag) - 1);
   761+        secp256k1_sha256_initialize_midstate(&sha_midstate, sha_tagged.bytes, sha_tagged.s);
   

   ---

   ---

   

   real-or-random commented at 9:47 am on February 25, 2026:

   Having the midstate here as a literal would make this test work without accessing the (now) fully encapsulated members of the secp256k1_sha256 struct.

   You could use https://gist.github.com/real-or-random/a4aaae5d04eee9f63e7a2e43d25bc2b1 or simply reuse an existing midstate from some module test.

   ---

   

   w0xlt commented at 0:31 am on February 26, 2026:

   Done in f48b1bfa5d40a4d7303b196017d2e298520d1066. Thanks.
10. 
    real-or-random commented at 9:51 am on February 25, 2026: contributor

    ACK mod nits
11. hash: add midstate initializer and use it for tagged hashes

    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.

    f48b1bfa5d
12. w0xlt force-pushed on Feb 25, 2026
13. real-or-random approved
14. 
    real-or-random commented at 8:39 am on February 27, 2026: contributor

    utACK f48b1bfa5d40a4d7303b196017d2e298520d1066
15. theStack approved
16. 
    theStack commented at 5:52 pm on February 27, 2026: contributor

    Code-review ACK f48b1bfa5d40a4d7303b196017d2e298520d1066
17. real-or-random merged this on Feb 27, 2026
18. real-or-random closed this on Feb 27, 2026

    ---

19. w0xlt deleted the branch on Mar 2, 2026

Contributors
w0xlt real-or-random theStack

Labels
tweak/refactor