Upstream PRs 1763, 1771, 1761, 1774, 1779, 1784, 1788, 1778, 1783, 1790, 1764, 1793, 1800, 1796, 1808, 1809#332
Merged
real-or-random merged 43 commits intoBlockstreamResearch:masterfrom Mar 3, 2026
Conversation
…ad of hardcoded value 153eea2 bench: Use `ALIGNMENT` macro instead of hardcoded value (Hennadii Stepanov) Pull request description: This PR brings consistency with the rest of the code and appears more correct. ACKs for top commit: real-or-random: utACK bitcoin-core/secp256k1@153eea2 Tree-SHA512: 0fc61a390205ef29b99cfce9cb0d365930b7a93aa0f2aaaa00bfd9e0fc34ac928a3fe13b096f394b7d35e95e9ede7c47ecb054de2846be1e1aa33a84d14942cf
… in "x86_64-macos-native" job f252da7 ci: Use Python virtual environment in "x86_64-macos-native" job (Hennadii Stepanov) Pull request description: Fixes bitcoin-core/secp256k1#1768. This PR explicitly sets up a virtual environment instead of using `uv`, as suggested [here](bitcoin-core/secp256k1#1768 (comment)), for the following reasons: 1. It does not require granting a third-party action access to the repository. 2. This approach is already used in other parts of the CI framework (it’s unclear why it was missed in bitcoin-core/secp256k1#1359 in the first place). ACKs for top commit: real-or-random: ACK f252da7 Tree-SHA512: df167db9b69ea4a055565c64ea0daa8c0d7b6a12510c35c22309f00a192a8cc4fbbdb0920bacd25547ea8c8f6a7853c2603be35234bd2bac88f610ea848f6120
…y usage by 30% 26166c4 ecmult_multi: reduce strauss memory usage by 30% (Jonas Nick) Pull request description: This is a draft because I'm not sure about the cleanest way to implement it. ACKs for top commit: real-or-random: ACK 26166c4 benchmarks show no significant difference (only tried low point counts) siv2r: tACK 26166c4 hebasto: ACK 26166c4, I have reviewed the code and it looks OK. Tree-SHA512: f289daee0b0b51451331eefdd99200a78bd83539365d38465c038dc0e6ad940daf821119f7161b08a2390cf046e3859a8f950f2fe881a427aba16353031def7d
… serialization function into compressed/uncompressed variants f5e815f remove secp256k1_eckey_pubkey_serialize function (Sebastian Falbesoner) 0d3659c use new `_eckey_pubkey_serialize{33,65}` functions in modules (ellswift,musig) (Sebastian Falbesoner) adb76f8 use new `_eckey_pubkey_serialize{33,65}` functions in public API (Sebastian Falbesoner) fc7458c introduce `secp256k1_eckey_pubkey_serialize{33,65}` functions (Sebastian Falbesoner) Pull request description: This PR splits up the pubkey serialization function `secp256k1_eckey_pubkey_serialize` into two variants for the compressed (33 bytes) and uncompressed (65 bytes) public key output format each, where only non-infinity group elements as input are allowed. The motivation is to simplify call-sites significantly, as they currently need to introduce two variables and a VERIFY_CHECKs on the return value and the in/out size parameter within a pre-processor block, typically leading to 8 lines of code. By using the new functions, the code is reduced to a single line of code that just calls the function (see #1773). This is helpful for already existing modules on master (ellswift, musig) and upcoming ones (silentpayments, see #1765). One drawback is that the public API function `secp256k1_ec_pubkey_serialize` is now slightly more complex (we now call one of two functions instead of a single one, depending on whether the compressed flag is set or not), but that should hopefully not be a problem. The commits are intentionally kept small to ease review, happy to squash them if that is preferred. (Kudos to w0xlt for the initial idea (bitcoin-core/secp256k1#1765 (review)) and to real-or-random for the suggestion to split the already existing function (bitcoin-core/secp256k1#1773 (comment)).) ACKs for top commit: real-or-random: utACK f5e815f w0xlt: ACK bitcoin-core/secp256k1@f5e815f Tree-SHA512: da576bbeae477f31ba76c0001f8df08b51fe5e31d67b422a238348ead3341bf37f0c1509ad9d0a93b63e6d61c152707c85beabd02f4eac3b3bdcff129e0ea750
… pointers" elements are non-NULL 8bcda18 test: Add non-NULL checks for "pointer of array" API functions (Sebastian Falbesoner) 5a08c1b Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL (Sebastian Falbesoner) Pull request description: We currently have five public API functions that take an "array of pointers" as input parameter: * `secp256k1_ec_pubkey_combine` (`ins`: array of pointers to public keys to add) * `secp256k1_ec_pubkey_sort` (`pubkeys`: array of pointers to public keys to sort) * `secp256k1_musig_pubkey_agg` (`pubkeys`: array of pointers to public keys to aggregate) * `secp256k1_musig_nonce_agg` (`pubnonces`: array of pointers to public nonces to aggregate) * `secp256k1_musig_partial_sig_agg` (`partial_sigs`: array of pointers to partial signatures to aggregate) Out of these, only `_ec_pubkey_combine` verifies that the individual pointer elements in the array are non-NULL each: https://github.com/bitcoin-core/secp256k1/blob/e7f7083b530a55c83ce9089a7244d2d9d67ac8b2/src/secp256k1.c#L774-L775 This PR adds corresponding `ARG_CHECKS` for the other API functions as well, in order to avoid running into potential UB due to NULL pointer dereference. It seems to me that the tiny run-time overhead is worth it doing this for consistency and to help users in case the arrays are set up incorrectly (I'm thinking e.g. of language binding writers where getting this right might be a bit more involved). Looking into this was motivated by a [review of furszy](bitcoin-core/secp256k1#1765 (comment)) (thanks!), who pointed out that the non-NULL checks are missing in at least one API function in the silentpayments module PR as well. Happy to add some `CHECK_ILLEGAL` tests if there is conceptual support for this PR. ACKs for top commit: kevkevinpal: utACK [8bcda18](bitcoin-core/secp256k1@8bcda18) john-moffett: utACK 8bcda18 real-or-random: utACK 8bcda18 w0xlt: ACK bitcoin-core/secp256k1@8bcda18 Tree-SHA512: 24acd6606526e3acb994e3361fde15771aa6706a6f3e7a6ae70b9a9ddb81ac1eedaac2025a027b890cecf98dab20dc378b94edde6c726888c44b9d35b7581ee1
…1_ec_pubkey_serialize 3daab83 refactor: remove ret from secp256k1_ec_pubkey_serialize (kevkevinpal) Pull request description: This is a follow-up to bitcoin-core/secp256k1#1774 (comment) It is pretty straightforward to remove `ret` and to just return either `0` or `1` ACKs for top commit: real-or-random: utACK 3daab83 theStack: ACK 3daab83 Tree-SHA512: ce598d917455a2d25297436bf2b900a9e88a638617cb79ca22e467135035c334b6815911fe4429ff44dbd877e6d10a346d0b37f2e5a7459e5b35854023832d27
Flags for constant-time masking rely on the values being exactly 0 or 1 rather than 0 or true. Add VERIFY_CHECKs to enforce in VERIFY builds as a preventative measure and add documentation where relevant.
No behavior changes. Refactors the previously monolithic ElligatorSwift test into isolated, independent test cases. Doing so allows the test suite to execute these cases in parallel rather than sequentially. Overall, seen 35-40% tests time reduction locally. This is quite useful for the Debug build with no optimizations, which is noticeably slow. #### Local Debug-build Results (7 jobs): - master: 138.0 seconds. - this PR: 89.3 seconds. (~1.55× speedup, ~35% reduction) #### Local Release-build Results (7 jobs): - master: 9.5 seconds. - this PR: 5.9 seconds. (~1.61× speedup, ~38% reduction)
…st into independent cases d822b29 test: split monolithic ellswift test into independent cases (furszy) Pull request description: No behavior changes. Refactors the previously monolithic ElligatorSwift test into isolated, independent test cases. Doing so allows the test suite to execute these cases in parallel rather than sequentially. Overall, I'm seeing 35-40% tests time reduction locally. This is quite useful for Debug builds with no optimizations, which are noticeably slow. #### Local Debug-build Results (7 jobs): - master: 138.0 seconds. - this PR: 89.3 seconds. (~1.55× speedup, ~35% reduction) #### Local Release-build Results (7 jobs): - master: 9.5 seconds. - this PR: 5.9 seconds. (~1.61× speedup, ~38% reduction) #### rp5 Release-build results (5 jobs): - master: 39 seconds. - this PR: 24 seconds. (~1.62× speedup, ~38% reduction) ACKs for top commit: hebasto: re-ACK d822b29. theStack: ACK d822b29 Tree-SHA512: b3d93f183c5eb5856a146c38719a8ca7ac42ac2a521b3cae3da6950881eb1745a047df9790db2b60dd56501c5ba40fc78e89d0c9b4db777f32520ed3c27ab9d1
…ns to bench error messages 3b5b03f doc/bench: Added cmake build options to bench error messages (kevkevinpal) Pull request description: ## Motivation I wanted to try and run the benchmarking scripts and I noticed the recovery benchmark in `bench.c`. I wanted to run but I was using `cmake` and the error message telling me to use `./configure -enable-module-recovery` wasn't sufficient. I figure rather than forcing users to look into the `CMakeLists.txt` file or anywhere else we should add this to the output ## Solution I appended to the message to include the `-DSECP256K1_ENABLE_MODULE_...=ON` in the message. ACKs for top commit: real-or-random: utACK 3b5b03f hebasto: ACK 3b5b03f, I have reviewed the code and it looks OK. Tree-SHA512: 3a6c966b65ab3f0d6dda81e5dd95529087db3f2901f2686af68c16079ec5b323568f3c9acb155d92a6d50b4102faf0844d0d87216df001adbf69cab4ce86dabc
…n that flags must be 0 or 1 ae00c55 Add VERIFY_CHECKs that flags are 0 or 1 (John Moffett) Pull request description: Flags for constant-time masking rely on the values being exactly `0` or `1` rather than `0` or true (any nonzero). One function, `secp256k1_fe_cmov` [documents](https://github.com/bitcoin-core/secp256k1/blob/e7f7083b530a55c83ce9089a7244d2d9d67ac8b2/src/field.h#L315) and [`VERIFY_CHECK`s](https://github.com/bitcoin-core/secp256k1/blob/e7f7083b530a55c83ce9089a7244d2d9d67ac8b2/src/field_impl.h#L365) this, but most don't. This updates the documentation and adds `VERIFY_CHECK`s enforcing `flag == 0 || flag == 1` for: `secp256k1_fe_storage_cmov` `secp256k1_gej_cmov` `secp256k1_ge_storage_cmov` `secp256k1_scalar_cadd_bit` `secp256k1_scalar_cond_negate` `secp256k1_scalar_cmov` `secp256k1_int_cmov` ACKs for top commit: furszy: ACK ae00c55 hebasto: re-ACK ae00c55. Tree-SHA512: c9d358929d39d93b0aea602d318429f7e82af96bf601f048a1cdeb0621b8adc6d1204648d352aa2060cb0f63db6dcf0da863854375ed313cea44dfad61c19a18
…XTERNAL_DEFAULT_CALLBACKS=ON for cmake 0406cfc doc: include arg -DUSE_EXTERNAL_DEFAULT_CALLBACKS=1 for cmake (kevkevinpal) Pull request description: ### Motivation This is motivated by this comment bitcoin-core/secp256k1#1778 (review) ### Rationale It makes sense to add documentation on how to configure for CMake. I can reword if other wording is preferred ACKs for top commit: hebasto: ACK 0406cfc. real-or-random: ACK 0406cfc Tree-SHA512: 06b8cc84fc2c080045eb1c16af6b236b5d7472696d2488821c64a07b55a12c32e77c65c001a7223e92edeb6150bf15b7a7367dd254307c7cb91debdb924574f0
…directly in other modules 2f73e52 group: Avoid using infinity field directly in other modules (Tim Ruffing) Pull request description: Minor refactoring to make the abstraction cleaner ACKs for top commit: hebasto: ACK 2f73e52, I have reviewed the code and it looks OK. theStack: ACK 2f73e52 Tree-SHA512: eae5ad1ce81f491adb48ab1cbf04211f8d43e41255abcacc958fa3dcb1de5021707d56ed1b009a6f3f6c45cd8f20c1f2677891690a3c0a467fc7e064af2512a8
…h_ecmult In addition a print message saying some tests were skipped was added
…P256K1_BENCH_ITERS env var for bench_ecmult bd5ced1 doc/bench: added help text for SECP256K1_BENCH_ITERS env var for bench_ecmult (kevkevinpal) Pull request description: ACKs for top commit: real-or-random: utACK bd5ced1 hebasto: ACK bd5ced1, I have reviewed the code and it looks OK. Tested on Ubuntu 25.10. jonasnick: ACK bd5ced1 Tree-SHA512: 7cfc1a8915717bdfe2901f20f578e23368ece9937a40f36805a0a5b741f97a0502a085c973f6912b96c2bca921ef1654908cfe2c90c0601a7ffa92de4415dc62
Add assertions to verify that the GLV decomposition constants arise from the Eisenstein integer factorization of the group order N. The group order factors as N = pi * conj(pi) in Z[w], where pi = A - B*w is an Eisenstein prime. The GLV eigenvalue LAMBDA = B/A mod N, which is the image of w^2 under the isomorphism Z[w]/(pi) -> Z/NZ.
…nnection for GLV constants 29ac4d8 sage: verify Eisenstein integer connection for GLV constants (Justsomebuddy) Pull request description: ## Summary Add assertions to `gen_split_lambda_constants.sage` to verify that the GLV decomposition constants arise from the Eisenstein integer factorization of the group order N. Specifically: - `N = a^2 + a*b + b^2` (norm equation in Z[ω]) - `λ = b/a mod N` (eigenvalue from Z[ω]/(π) ≅ Z/NZ isomorphism) This addresses the suggestion in #1798 to document/verify the algebraic origin of these constants in the sage script rather than C comments. ## Details The group order N factors as N = π·π̄ in the Eisenstein integers Z[ω], where: - ω = (-1 + √-3)/2 is a primitive cube root of unity - π = a - b·ω is an Eisenstein prime with norm N(π) = a² + ab + b² The GLV constants (A1, B1) correspond to the Eisenstein factors (b, -a), and the endomorphism eigenvalue λ arises naturally as the image of ω under the quotient map Z[ω] → Z[ω]/(π) ≅ Z/NZ. Closes #1798 ACKs for top commit: real-or-random: utACK 29ac4d8 Tree-SHA512: 6c36dacac00baf513db447a14f49c91d434c80ed79f9282d080938e3e53d39f0b68d07d62900da648d817eba3777505e9ef9306bc129f4521f524b4c64bcda49
In this change the get_iters function was updated to print an error message and then return 0. In the functions that use get_iters they print the help text and then EXIT_FAILURE
…nvalid value for SECP256K1_BENCH_ITERS c09215f bench: fail early if user inputs invalid value for SECP256K1_BENCH_ITERS (kevkevinpal) Pull request description: ### Description Motivated by bitcoin-core/secp256k1#1793 (comment) In this change, the `get_iters` function was updated to print an error message and then return 0. In the functions that use `get_iters` they print the help text and then EXIT_FAILURE ### Before ``` secp256k1 $ SECP256K1_BENCH_ITERS=abc ./build/bin/bench Benchmark , Min(us) , Avg(us) , Max(us) Floating point exception (core dumped) ``` ### After ``` secp256k1 $ SECP256K1_BENCH_ITERS=abc ./build/bin/bench Invalid value for SECP256K1_BENCH_ITERS must be a positive integer: abc Benchmarks the following algorithms: - ECDSA signing/verification - ECDH key exchange (optional module) - Schnorr signatures (optional module) - ElligatorSwift (optional module) The default number of iterations for each benchmark is 20000. This can be customized using the SECP256K1_BENCH_ITERS environment variable. Usage: ./bench [args] By default, all benchmarks will be run. args: help : display this help and exit ecdsa : all ECDSA algorithms--sign, verify, recovery (if enabled) ecdsa_sign : ECDSA siging algorithm ecdsa_verify : ECDSA verification algorithm ec : all EC public key algorithms (keygen) ec_keygen : EC public key generation ecdh : ECDH key exchange algorithm schnorrsig : all Schnorr signature algorithms (sign, verify) schnorrsig_sign : Schnorr sigining algorithm schnorrsig_verify : Schnorr verification algorithm ellswift : all ElligatorSwift benchmarks (encode, decode, keygen, ecdh) ellswift_encode : ElligatorSwift encoding ellswift_decode : ElligatorSwift decoding ellswift_keygen : ElligatorSwift key generation ellswift_ecdh : ECDH on ElligatorSwift keys ``` ACKs for top commit: hebasto: re-ACK c09215f. real-or-random: utACK c09215f Tree-SHA512: 356df69e356db0b201339d40a6ffbcf29e4b7cc1e6aa82c00e1e7a2a7d11c47dd9c51baabcc63cabcff2ab42e2746a3cab659205f871a85122edda4a599d56c8
20a209f release: prepare for 0.7.1 (Jonas Nick) c4b6a81 changelog: update in preparation for the v0.7.1 release (Jonas Nick) Pull request description: ACKs for top commit: sipa: ACK 20a209f real-or-random: ACK 20a209f Tree-SHA512: 4d65218c9cc0cb0d968a972c2d199b2eaee12879a9a98dbea9476c10ae0d24f531dbb839989dfdcc01b3e3420c9a6e14a1fa9e2724c788bfafd33174675598e3
…r 0.7.1 ae7eb72 release cleanup: bump version after 0.7.1 (Jonas Nick) Pull request description: ACKs for top commit: real-or-random: ACK ae7eb72 sipa: ACK ae7eb72 Tree-SHA512: 7319e1aa8fbdc53f95b9673fe6cf8fb2cbdf48b47c5576344ff3cc73fcf7f7f6594f26d020f418943f15008a0780bb582fa46f027991a41d0bd75eb004384ee0
Contributor
Author
|
As for porting bitcoin-core/secp256k1#1796, upstream For consistency, I can open a separate PR adding |
Member
real-or-random
left a comment
real-or-random
left a comment
There was a problem hiding this comment.
However, since
get_iters()already prints a descriptive error message, this should be sufficient for users to understand and fix the issue
Yes, this should be good enough.
mllwchrry
force-pushed
the
temp-merge-1809
branch
from
dc2a104 to
dc0bda5
Compare
This was referenced Mar 3, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 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.
Merge bitcoin-core/secp256k1#1763: bench: Use
ALIGNMENTmacro instead of hardcoded valueMerge bitcoin-core/secp256k1#1771: ci: Use Python virtual environment in "x86_64-macos-native" job
Merge bitcoin-core/secp256k1#1761: ecmult_multi: reduce strauss memory usage by 30%
Merge bitcoin-core/secp256k1#1774: refactor: split up internal pubkey serialization function into compressed/uncompressed variants
Merge bitcoin-core/secp256k1#1779: Add ARG_CHECKs to ensure "array of pointers" elements are non-NULL
Merge bitcoin-core/secp256k1#1784: refactor: remove ret from secp256k1_ec_pubkey_serialize
Merge bitcoin-core/secp256k1#1788: test: split monolithic ellswift test into independent cases
Merge bitcoin-core/secp256k1#1778: doc/bench: Added cmake build options to bench error messages
Merge bitcoin-core/secp256k1#1783: Add VERIFY_CHECKs and documentation that flags must be 0 or 1
Merge bitcoin-core/secp256k1#1790: doc: include arg -DSECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS=ON for cmake
Merge bitcoin-core/secp256k1#1764: group: Avoid using infinity field directly in other modules
Merge bitcoin-core/secp256k1#1793: doc/bench: added help text for SECP256K1_BENCH_ITERS env var for bench_ecmult
Merge bitcoin-core/secp256k1#1800: sage: verify Eisenstein integer connection for GLV constants
Merge bitcoin-core/secp256k1#1796: bench: fail early if user inputs invalid value for SECP256K1_BENCH_ITERS
Merge bitcoin-core/secp256k1#1808: Prepare for 0.7.1
Merge bitcoin-core/secp256k1#1809: release cleanup: bump version after 0.7.1
This PR can be recreated with
./contrib/sync-upstream.sh -b master range c7a52400.Tips:
git show --remerge-diff <pr-branch>to show the conflict resolution in the merge commit.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.