ci: backport upstream string-initializer fixes and clean up related tests #303
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The previous code is correct and harmless to initialize an array with a
non-terminated character sequence using a string literal.
However, it requires exactly specifying the array size, which can be
cumbersome.
Also, GCC-15 may issue the -Wunterminated-string-initialization warning.
[1]
Fix both issues by using array initialization. This refactoring commit
does not change behavior.
[1] Example warning:
src/modules/schnorrsig/main_impl.h:48:46: error: initializer-string for array of 'unsigned char' is too long [-Werror=unterminated-string-initialization]
48 | static const unsigned char bip340_algo[13] = "BIP0340/nonce";
| ^~~~~~~~~~~~~~~
(cherry picked from commit fa67b6752d8ba3e4c41f6c36b1c6b94a21770419)
Conflicts:
src/testrand_impl.h (kept local name `secp256k1_testrand_seed`)
- Mirror upstream fix (bitcoin-core/secp256k1 fa67b675) - Convert tags, test vectors, and constants in -zkp-only modules (ecdsa_adaptor, ecdsa_s2c, musig, bppp, rangeproof, schnorrsig_halfagg) - Avoid -Wunterminated-string-initialization without changing behavior
This avoids a compiler warning on clang-snapshot about &keypair being uninitialized. (cherry picked from commit 8d967a602b14c86d0f9e43a31fdfe76f39f32091)
The keypair is unused in musig_partial_sign, but clang-snapshot gives a compiler warning anyway.
Switch msg initialization from a string literal to a brace-enclosed array to avoid -Wunterminated-string-initialization. Upstream removed the trailing '!' from the message; this change retains it.
Collaborator
It really doesn't matter. We'll need to replace the entire musig module by the upstream version anyway. |
real-or-random
approved these changes
Sep 10, 2025
Collaborator
real-or-random
left a comment
real-or-random
left a comment
There was a problem hiding this comment.
utACK 7d779c6 thanks!
The remaining CI failures are in the constant-time tests. These will need to be fixed as well but this can be done in a separate PR.
jonasnick
approved these changes
Sep 16, 2025
jonasnick
merged commit f92fd5c
into
BlockstreamResearch:master
95 of 107 checks passed
Contributor
Author
|
Opened a follow-up in #304 to address the remaining MSan failures. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes CI failures caused by
-Werror=unterminated-string-initializationby backporting upstream changes from libsecp256k1. While doing that, have also fixed a few small tests and other CI issues so the complete build passes again.What changed
fa67b675): switched from string literal initializers to brace-enclosed byte arrays.musig/tests: removed dead code and made surekeypairis initialized (clang warning).(From upstream
8d967a60, adjusted for -zkp test vectors.)examples: used brace arrays for demo strings. Inmusig.c, we retained the!at the end of the message (upstream dropped it) to preserve prior example output.bppp,ecdsa_adaptor,ecdsa_s2c,rangeproof,schnorrsig(_halfagg),ellswift, andtestrand_impl.h.Rationale
Minor divergence from upstream