hash: remove redundant `secp256k1_sha256_initialize` in tagged hash midstate functions by w0xlt · Pull Request #1825 · bitcoin-core/secp256k1

w0xlt · 2026-02-17T19:38:18Z

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 · 2026-02-18T09:26:58Z

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

Concept ACK

theStack · 2026-02-23T18:33:22Z

Concept ACK

real-or-random

ACK mod nits

real-or-random · 2026-02-25T09:35:08Z

src/hash_impl.h

+/* Initialize a SHA256 hash state with a precomputed midstate.
+ * The byte counter must be a multiple of 64, i.e., there must be no unwritten
+ * bytes in the buffer. */


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

Done in f48b1bf. Thanks.

real-or-random · 2026-02-25T09:45:28Z

src/tests.c

+        secp256k1_sha256_write(&sha_tagged, msg, split);
+        secp256k1_sha256_write(&sha_tagged, msg + split, msg_len - split);
+        secp256k1_sha256_write(&sha_midstate, msg, msg_len);
+
+        secp256k1_sha256_finalize(&sha_tagged, hash_tagged);
+        secp256k1_sha256_finalize(&sha_midstate, hash_midstate);
+        CHECK(secp256k1_memcmp_var(hash_tagged, hash_midstate, sizeof(hash_tagged)) == 0);


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.

Done in f48b1bf. Thanks.

real-or-random · 2026-02-25T09:47:13Z

src/tests.c

+        size_t split = msg_len / 2;
+
+        secp256k1_sha256_initialize_tagged(&sha_tagged, tag, sizeof(tag) - 1);
+        secp256k1_sha256_initialize_midstate(&sha_midstate, sha_tagged.bytes, sha_tagged.s);


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.

Done in f48b1bf. Thanks.

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.

real-or-random

utACK f48b1bf

theStack

Code-review ACK f48b1bf

b9cb1cbfd7 Merge bitcoin-core/secp256k1#1824: util: introduce and use `ARRAY_SIZE` macro c0a2aba088 Merge bitcoin-core/secp256k1#1811: bench: Update help functions in bench and bench_internal 10f546a2c0 Merge bitcoin-core/secp256k1#1832: testrand: Remove testrand_finish 8d0eda07e9 testrand: Remove testrand_finish 95e6815843 Merge bitcoin-core/secp256k1#1825: hash: remove redundant `secp256k1_sha256_initialize` in tagged hash midstate functions f48b1bfa5d hash: add midstate initializer and use it for tagged hashes 3019186a6d Merge bitcoin-core/secp256k1#1829: ci: Fix leftover use of old ECMULTGENPRECISION 79e9f25237 ci: Fix leftover use of old ECMULTGENPRECISION dfe042feb2 Merge bitcoin-core/secp256k1#1828: Revert "ci, docker: Fix LLVM repository signature failure" 76e92cfeea Revert "ci, docker: Fix LLVM repository signature failure" ac561601b8 Merge bitcoin-core/secp256k1#1760: cmake: Add dynamic test discovery to improve parallelism c7a7f732bd Merge bitcoin-core/secp256k1#1821: ellswift: fix overflow flag handling in secp256k1_ellswift_xdh 921b9711ea util: introduce and use `ARRAY_SIZE` macro b99a94c382 Add tests for bad scalar inputs in ellswift XDH 307b49f1b9 ellswift: fix overflow flag handling in secp256k1_ellswift_xdh 322d0a4358 Merge bitcoin-core/secp256k1#1823: ci: Load Docker image by ID from builder step ed02466d3f ci: Load Docker image by ID from builder step c49c9be504 bench: Update help functions in bench and bench_internal 1d146ac3ed Merge bitcoin-core/secp256k1#1819: tests: Improve secp256k1_scalar_check_overflow tests (Issue bitcoin#1812) f47bbc07f0 test: add unit tests for secp256k1_scalar_check_overflow d071aa56d5 Merge bitcoin-core/secp256k1#1815: refactor: remove unnecessary `malloc` result casts 99ab4a105e Merge bitcoin-core/secp256k1#1817: ci: Disable Docker build summary generation c5da3bde9c Merge bitcoin-core/secp256k1#1818: ci: Enforce base-10 evaluation 97de5120cf Merge bitcoin-core/secp256k1#1804: test: show both CMake and Autotools usage for ctime_tests 4fb7ccf5d4 ci: Enforce base-10 evaluation 3ae72e7867 ci: Disable Docker build summary generation 97b3c47849 refactor: remove unnecessary `malloc` result casts 1bc74a22f8 test: show both Autotools and CMake usage for ctime_tests 8354618e02 cmake: Set `LABELS` property for tests 29f26ec3cf cmake: Integrate DiscoverTests and normalize test names f95b263f23 cmake: Add DiscoverTests module 4ac651144b cmake, refactor: Deduplicate test-related code git-subtree-dir: src/secp256k1 git-subtree-split: b9cb1cbfd72c9391c337ed1e30d71355dde65248

1aafe15139 Merge bitcoin-core/secp256k1#1777: Make SHA256 compression runtime pluggable b9cb1cbfd7 Merge bitcoin-core/secp256k1#1824: util: introduce and use `ARRAY_SIZE` macro 4d92a083bc sha256: speed up writes using multi-block compression 0753f8b909 Add API to override SHA256 compression at runtime fdb6a91a5e Introduce hash context to support pluggable SHA256 compression c0a2aba088 Merge bitcoin-core/secp256k1#1811: bench: Update help functions in bench and bench_internal 10f546a2c0 Merge bitcoin-core/secp256k1#1832: testrand: Remove testrand_finish 8d0eda07e9 testrand: Remove testrand_finish 95e6815843 Merge bitcoin-core/secp256k1#1825: hash: remove redundant `secp256k1_sha256_initialize` in tagged hash midstate functions f48b1bfa5d hash: add midstate initializer and use it for tagged hashes 3019186a6d Merge bitcoin-core/secp256k1#1829: ci: Fix leftover use of old ECMULTGENPRECISION 79e9f25237 ci: Fix leftover use of old ECMULTGENPRECISION dfe042feb2 Merge bitcoin-core/secp256k1#1828: Revert "ci, docker: Fix LLVM repository signature failure" 76e92cfeea Revert "ci, docker: Fix LLVM repository signature failure" ac561601b8 Merge bitcoin-core/secp256k1#1760: cmake: Add dynamic test discovery to improve parallelism c7a7f732bd Merge bitcoin-core/secp256k1#1821: ellswift: fix overflow flag handling in secp256k1_ellswift_xdh 921b9711ea util: introduce and use `ARRAY_SIZE` macro b99a94c382 Add tests for bad scalar inputs in ellswift XDH 307b49f1b9 ellswift: fix overflow flag handling in secp256k1_ellswift_xdh 322d0a4358 Merge bitcoin-core/secp256k1#1823: ci: Load Docker image by ID from builder step ed02466d3f ci: Load Docker image by ID from builder step c49c9be504 bench: Update help functions in bench and bench_internal 1d146ac3ed Merge bitcoin-core/secp256k1#1819: tests: Improve secp256k1_scalar_check_overflow tests (Issue bitcoin#1812) f47bbc07f0 test: add unit tests for secp256k1_scalar_check_overflow d071aa56d5 Merge bitcoin-core/secp256k1#1815: refactor: remove unnecessary `malloc` result casts 99ab4a105e Merge bitcoin-core/secp256k1#1817: ci: Disable Docker build summary generation c5da3bde9c Merge bitcoin-core/secp256k1#1818: ci: Enforce base-10 evaluation 97de5120cf Merge bitcoin-core/secp256k1#1804: test: show both CMake and Autotools usage for ctime_tests 4fb7ccf5d4 ci: Enforce base-10 evaluation 3ae72e7867 ci: Disable Docker build summary generation 97b3c47849 refactor: remove unnecessary `malloc` result casts 1bc74a22f8 test: show both Autotools and CMake usage for ctime_tests 8354618e02 cmake: Set `LABELS` property for tests 29f26ec3cf cmake: Integrate DiscoverTests and normalize test names f95b263f23 cmake: Add DiscoverTests module 4ac651144b cmake, refactor: Deduplicate test-related code git-subtree-dir: src/secp256k1 git-subtree-split: 1aafe15139976b0142d791aaf4963de3fc1ff736

…17, 1815, 1819, 1823, 1821, 1760, 1828, 1829, 1825, 1832, 1811 48cbd78 modules: Port bitcoin-core/secp256k1#1825 to zkp-specific code (mllwchrry) 126501f modules: Port bitcoin-core/secp256k1#1815 to zkp-specific code (mllwchrry) c49c9be bench: Update help functions in bench and bench_internal (kevkevinpal) 8d0eda0 testrand: Remove testrand_finish (Tim Ruffing) f48b1bf hash: add midstate initializer and use it for tagged hashes (w0xlt) 79e9f25 ci: Fix leftover use of old ECMULTGENPRECISION (Tim Ruffing) 76e92cf Revert "ci, docker: Fix LLVM repository signature failure" (Hennadii Stepanov) 8354618 cmake: Set `LABELS` property for tests (Hennadii Stepanov) 29f26ec cmake: Integrate DiscoverTests and normalize test names (Hennadii Stepanov) f95b263 cmake: Add DiscoverTests module (Hennadii Stepanov) 4ac6511 cmake, refactor: Deduplicate test-related code (Hennadii Stepanov) b99a94c Add tests for bad scalar inputs in ellswift XDH (gzJx0DuTRHytnHe7P5RmMbPf3wKy2BztweVGXTf) 307b49f ellswift: fix overflow flag handling in secp256k1_ellswift_xdh (gzJx0DuTRHytnHe7P5RmMbPf3wKy2BztweVGXTf) ed02466 ci: Load Docker image by ID from builder step (Hennadii Stepanov) f47bbc0 test: add unit tests for secp256k1_scalar_check_overflow (Rohit Yadav) 97b3c47 refactor: remove unnecessary `malloc` result casts (Sebastian Falbesoner) 3ae72e7 ci: Disable Docker build summary generation (Hennadii Stepanov) 4fb7ccf ci: Enforce base-10 evaluation (Hennadii Stepanov) 1bc74a2 test: show both Autotools and CMake usage for ctime_tests (8144225309) 86cae58 build: Add `-Wleading-whitespace=spaces` compiler flag (Hennadii Stepanov) fb229e7 build: Add `-Wtrailing-whitespace=any` compiler flag (Hennadii Stepanov) 13e3bee refactor: Remove trailing whitespace (Hennadii Stepanov) 2ccff6e ci: Add weekly schedule (Hennadii Stepanov) 2f18567 ci: Rotate Docker cache keys every 4 weeks (Hennadii Stepanov) 0ffb174 ci, docker: Fix LLVM repository signature failure (Hennadii Stepanov) 0267b65 release process: mention the `[Unreleased]` link clearly (Jonas Nick) 748c0fd Add CMake build directory patterns to `.gitignore` (Hennadii Stepanov) 7eb86bd autotools: Rename `build-aux` to `autotools-aux` (Hennadii Stepanov) 47eb709 ecmult: Use size_t for array indices in _odd_multiplies_table (Tim Ruffing) bb1d199 ecmult: Use size_t for array indices into tables (Tim Ruffing) Pull request description: Merge bitcoin-core/secp256k1#1794: ecmult: Use size_t for array indices Merge bitcoin-core/secp256k1#1775: Add CMake build directory patterns to `.gitignore` Merge bitcoin-core/secp256k1#1814: release process: mention the `[Unreleased]` link clearly Merge bitcoin-core/secp256k1#1816: ci: Rotate Docker cache keys Merge bitcoin-core/secp256k1#1813: Remove trailing spaces and introduce `-Wtrailing-whitespace=any` compiler flag Merge bitcoin-core/secp256k1#1804: test: show both CMake and Autotools usage for ctime_tests Merge bitcoin-core/secp256k1#1818: ci: Enforce base-10 evaluation Merge bitcoin-core/secp256k1#1817: ci: Disable Docker build summary generation Merge bitcoin-core/secp256k1#1815: refactor: remove unnecessary `malloc` result casts Merge bitcoin-core/secp256k1#1819: tests: Improve secp256k1_scalar_check_overflow tests (Issue #1812) Merge bitcoin-core/secp256k1#1823: ci: Load Docker image by ID from builder step Merge bitcoin-core/secp256k1#1821: ellswift: fix overflow flag handling in secp256k1_ellswift_xdh Merge bitcoin-core/secp256k1#1760: cmake: Add dynamic test discovery to improve parallelism Merge bitcoin-core/secp256k1#1828: Revert "ci, docker: Fix LLVM repository signature failure" Merge bitcoin-core/secp256k1#1829: ci: Fix leftover use of old ECMULTGENPRECISION Merge bitcoin-core/secp256k1#1825: hash: remove redundant `secp256k1_sha256_initialize` in tagged hash midstate functions Merge bitcoin-core/secp256k1#1832: testrand: Remove testrand_finish Merge bitcoin-core/secp256k1#1811: bench: Update help functions in bench and bench_internal This PR can be recreated with `./contrib/sync-upstream.sh -b master range c0a2aba`. Tips: * Use `git show --remerge-diff <pr-branch>` to show the conflict resolution in the merge commit. * Use `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. ACKs for top commit: real-or-random: ACK 48cbd78 Tree-SHA512: 1ea96bf3b7458c02bb472ca7423eb21defe6640ea58a993ac24603d6a8d44ac9bbeab7e2ca5f993918fa1388b9e69b308dcc67277afffd55cb833ecf07051c75

1aafe15139 Merge bitcoin-core/secp256k1#1777: Make SHA256 compression runtime pluggable b9cb1cbfd7 Merge bitcoin-core/secp256k1#1824: util: introduce and use `ARRAY_SIZE` macro 4d92a083bc sha256: speed up writes using multi-block compression 0753f8b909 Add API to override SHA256 compression at runtime fdb6a91a5e Introduce hash context to support pluggable SHA256 compression c0a2aba088 Merge bitcoin-core/secp256k1#1811: bench: Update help functions in bench and bench_internal 10f546a2c0 Merge bitcoin-core/secp256k1#1832: testrand: Remove testrand_finish 8d0eda07e9 testrand: Remove testrand_finish 95e6815843 Merge bitcoin-core/secp256k1#1825: hash: remove redundant `secp256k1_sha256_initialize` in tagged hash midstate functions f48b1bfa5d hash: add midstate initializer and use it for tagged hashes 3019186a6d Merge bitcoin-core/secp256k1#1829: ci: Fix leftover use of old ECMULTGENPRECISION 79e9f25237 ci: Fix leftover use of old ECMULTGENPRECISION dfe042feb2 Merge bitcoin-core/secp256k1#1828: Revert "ci, docker: Fix LLVM repository signature failure" 76e92cfeea Revert "ci, docker: Fix LLVM repository signature failure" ac561601b8 Merge bitcoin-core/secp256k1#1760: cmake: Add dynamic test discovery to improve parallelism c7a7f732bd Merge bitcoin-core/secp256k1#1821: ellswift: fix overflow flag handling in secp256k1_ellswift_xdh 921b9711ea util: introduce and use `ARRAY_SIZE` macro b99a94c382 Add tests for bad scalar inputs in ellswift XDH 307b49f1b9 ellswift: fix overflow flag handling in secp256k1_ellswift_xdh 322d0a4358 Merge bitcoin-core/secp256k1#1823: ci: Load Docker image by ID from builder step ed02466d3f ci: Load Docker image by ID from builder step c49c9be504 bench: Update help functions in bench and bench_internal 1d146ac3ed Merge bitcoin-core/secp256k1#1819: tests: Improve secp256k1_scalar_check_overflow tests (Issue bitcoin#1812) f47bbc07f0 test: add unit tests for secp256k1_scalar_check_overflow d071aa56d5 Merge bitcoin-core/secp256k1#1815: refactor: remove unnecessary `malloc` result casts 99ab4a105e Merge bitcoin-core/secp256k1#1817: ci: Disable Docker build summary generation c5da3bde9c Merge bitcoin-core/secp256k1#1818: ci: Enforce base-10 evaluation 97de5120cf Merge bitcoin-core/secp256k1#1804: test: show both CMake and Autotools usage for ctime_tests 4fb7ccf5d4 ci: Enforce base-10 evaluation 3ae72e7867 ci: Disable Docker build summary generation 97b3c47849 refactor: remove unnecessary `malloc` result casts 1bc74a22f8 test: show both Autotools and CMake usage for ctime_tests 8354618e02 cmake: Set `LABELS` property for tests 29f26ec3cf cmake: Integrate DiscoverTests and normalize test names f95b263f23 cmake: Add DiscoverTests module 4ac651144b cmake, refactor: Deduplicate test-related code git-subtree-dir: src/secp256k1 git-subtree-split: 1aafe15139976b0142d791aaf4963de3fc1ff736

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

w0xlt force-pushed the fix-redundant-sha256-initialize branch from 24ae8ac to 10ecf81 Compare February 19, 2026 00:30

w0xlt mentioned this pull request Feb 19, 2026

Unnecessary call to secp256k1_sha256_initialize #1179

Closed

real-or-random reviewed Feb 19, 2026

View reviewed changes

real-or-random reviewed Feb 25, 2026

View reviewed changes

w0xlt force-pushed the fix-redundant-sha256-initialize branch from 10ecf81 to f48b1bf Compare February 25, 2026 23:39

real-or-random approved these changes Feb 27, 2026

View reviewed changes

theStack approved these changes Feb 27, 2026

View reviewed changes

real-or-random merged commit 95e6815 into bitcoin-core:master Feb 27, 2026
122 checks passed

furszy mentioned this pull request Feb 28, 2026

Make SHA256 compression runtime pluggable #1777

Merged

w0xlt deleted the fix-redundant-sha256-initialize branch March 2, 2026 16:30

mllwchrry added a commit to mllwchrry/secp256k1-zkp that referenced this pull request Mar 3, 2026

modules: Port bitcoin-core/secp256k1#1825 to zkp-specific code

48cbd78

mllwchrry mentioned this pull request Mar 3, 2026

Upstream PRs 1794, 1775, 1814, 1816, 1813, 1804, 1818, 1817, 1815, 1819, 1823, 1821, 1760, 1828, 1829, 1825, 1832, 1811 BlockstreamResearch/secp256k1-zkp#333

Merged

theStack mentioned this pull request Mar 4, 2026

Add "silentpayments" module implementing BIP352 (take 4, limited to full-node scanning) #1765

Open

mllwchrry mentioned this pull request Mar 9, 2026

Upstream PRs 1794, 1775, 1814, 1816, 1813, 1804, 1818, 1817, 1815, 1819, 1823, 1821, 1760, 1828, 1829, 1825, 1832, 1811, 1824, 1777 mllwchrry/secp256k1-zkp#10

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

hash: remove redundant `secp256k1_sha256_initialize` in tagged hash midstate functions#1825
real-or-random merged 1 commit intobitcoin-core:masterfrom
w0xlt:fix-redundant-sha256-initialize

w0xlt commented Feb 17, 2026

Uh oh!

real-or-random commented Feb 18, 2026

Uh oh!

real-or-random left a comment

Uh oh!

theStack commented Feb 23, 2026

Uh oh!

real-or-random left a comment

Uh oh!

real-or-random Feb 25, 2026

Uh oh!

w0xlt Feb 26, 2026

Uh oh!

real-or-random Feb 25, 2026

Uh oh!

w0xlt Feb 26, 2026

Uh oh!

real-or-random Feb 25, 2026

Uh oh!

w0xlt Feb 26, 2026

Uh oh!

real-or-random left a comment

Uh oh!

theStack left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

w0xlt commented Feb 17, 2026

Uh oh!

real-or-random commented Feb 18, 2026

Uh oh!

real-or-random left a comment

Choose a reason for hiding this comment

Uh oh!

theStack commented Feb 23, 2026

Uh oh!

real-or-random left a comment

Choose a reason for hiding this comment

Uh oh!

real-or-random Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

w0xlt Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

real-or-random Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

w0xlt Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

real-or-random Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

w0xlt Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

real-or-random left a comment

Choose a reason for hiding this comment

Uh oh!

theStack left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants