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

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.

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).

real-or-random added the label tweak/refactor on Feb 18, 2026

w0xlt force-pushed on Feb 19, 2026

real-or-random commented at 2:01 PM on February 19, 2026: contributor

Concept ACK

theStack commented at 6:33 PM on February 23, 2026: contributor

Concept ACK

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 12:31 AM on February 26, 2026:

Done in f48b1bfa5d40a4d7303b196017d2e298520d1066. Thanks.

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 12:31 AM on February 26, 2026:

Done in f48b1bfa5d40a4d7303b196017d2e298520d1066. Thanks.

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 12:31 AM on February 26, 2026:

Done in f48b1bfa5d40a4d7303b196017d2e298520d1066. Thanks.

real-or-random commented at 9:51 AM on February 25, 2026: contributor

ACK mod nits

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

w0xlt force-pushed on Feb 25, 2026

real-or-random approved

real-or-random commented at 8:39 AM on February 27, 2026: contributor

utACK f48b1bfa5d40a4d7303b196017d2e298520d1066

theStack approved

theStack commented at 5:52 PM on February 27, 2026: contributor

Code-review ACK f48b1bfa5d40a4d7303b196017d2e298520d1066

real-or-random merged this on Feb 27, 2026

real-or-random closed this on Feb 27, 2026

w0xlt deleted the branch on Mar 2, 2026

mllwchrry referenced this in commit 48cbd78dfc on Mar 3, 2026

fanquake referenced this in commit 0dab9edba4 on Mar 3, 2026

fanquake referenced this in commit a50dbd2bcd on Mar 4, 2026

real-or-random referenced this in commit 7a5f153d71 on Mar 4, 2026

fanquake referenced this in commit ec7ea286b0 on Mar 10, 2026

fanquake referenced this in commit 6458812af8 on Mar 17, 2026

fanquake referenced this in commit 791949a724 on Mar 21, 2026

fanquake referenced this in commit cffa9320f2 on Mar 23, 2026

theStack referenced this in commit 994739fc33 on Mar 26, 2026

fanquake referenced this in commit dfc5233b8c on Mar 27, 2026

theStack referenced this in commit 0326234c51 on Apr 2, 2026

fanquake referenced this in commit dfd54c959e on Apr 9, 2026

vmta referenced this in commit 1ddc2f947f on Apr 26, 2026

vmta referenced this in commit 56c40fe100 on Apr 27, 2026