Similar to speeding up serialization; in our parsing logic we did a
bunch of expensive stuff then expensively inverted it. Drop everything
except the essential checks and then memcpy.
`secp256k1_pedersen_commit_serialize` would call `_load` (which does a
sqrt to fully decompress the key, then a conditional negation based on
the flag), then check the Jacobian symbol of the resulting y-coordinate,
then re-serialize based on this.
Instead, don't do any of this stuff. Copy the flag directly out of the
internal representation and copy the x-coordinate directly out of the
internal representation.
Checked that none of the other _serialize methods in the modules do
this.
Fixes#293
9554362b15 tests: call secp256k1_ecmult_multi_var with a non-NULL error callback (Nicolas Iooss)
Pull request description:
Hello,
This Pull Request fixes the issue reported in https://github.com/bitcoin-core/secp256k1/issues/1527. Function `secp256k1_ecmult_multi_var` expects to be called with a non-`NULL` `error_callback` parameter. Fix the invocation in `test_ecmult_accumulate` to do this. While at it, wrap the call in a `CHECK` macro to ensure it succeeds.
ACKs for top commit:
real-or-random:
utACK 9554362b15
siv2r:
ACK 9554362, I have also verified that other invocations of `ecmult_multi_var` (in tests) don’t use `NULL` for the error callback function argument.
Tree-SHA512: 6a9f6c10c575794da75f2254d6fbbc195de889c81a371ce35ab38e2e5483aa1e25ec0bcd5aa8d6a32a1493586f73430208a4bd0613e373571d2f04d63dbc4a1c
9f4c8cd730 cmake: Fix `check_arm32_assembly` when using as subproject (Hennadii Stepanov)
Pull request description:
When integrating libsecpk1 in a downstream project like this:
```cmake
set(SECP256K1_ASM arm32 CACHE STRING "" FORCE)
add_subdirectory(src/secp256k1)
```
it fails to configure:
```
CMake Error at /home/hebasto/git/bitcoin/build/check_arm32_assembly/CMakeFiles/CMakeTmp/CMakeLists.txt:21 (target_sources):
Cannot find source file:
/home/hebasto/git/bitcoin/cmake/source_arm32.s
CMake Error at /home/hebasto/git/bitcoin/build/check_arm32_assembly/CMakeFiles/CMakeTmp/CMakeLists.txt:20 (add_executable):
No SOURCES given to target: cmTC_d0f0b
CMake Error at src/secp256k1/cmake/CheckArm32Assembly.cmake:2 (try_compile):
Failed to generate test project build system.
Call Stack (most recent call first):
src/secp256k1/CMakeLists.txt:127 (check_arm32_assembly)
```
This PR fixes this issue, which was overlooked in https://github.com/bitcoin-core/secp256k1/pull/1304.
ACKs for top commit:
real-or-random:
utACK 9f4c8cd730
theuni:
utACK 9f4c8cd730
Tree-SHA512: 47d97ad0fb2e3779523c2111ea75906671a0fb3f50646e29dee195f53106ace69af5e4abc92c765f0eee6973528ce9195b94377d0157209230c958894d4049fb
Function secp256k1_ecmult_multi_var expects to be called with a non-NULL
error_callback parameter. Fix the invocation in test_ecmult_accumulate
to do this.
While at it, wrap the call in a CHECK macro to ensure it succeeds.
Fixes: https://github.com/bitcoin-core/secp256k1/issues/1527
7040a20247 doc: fix sage code for deriving alternative generator H (Sebastian Falbesoner)
Pull request description:
The line calculating H (in particular, the expression `G.decode('hex')`) fails with the following error message on Sage 9.5:
```
AttributeError: 'str' object has no attribute 'decode'
```
Fix that by converting the hex-string to bytes using `bytes.fromhex`.
(Noticed while reviewing https://github.com/bitcoin/bitcoin/pull/30048 which picks this code snippet comment up.)
ACKs for top commit:
josibake:
ACK 7040a20247
real-or-random:
utACK 7040a20247
Tree-SHA512: 0a44f399b103c2f5840056d163c1483a1d4f032bc0f8d3822507ac6da9d567f46e36caa79c7f5016aebcc8827b79e9aec7ebdb4f21c3c0242dc6875be140f289
The expression `G.decode('hex')` fails with the following error message
on Sage 9.5:
AttributeError: 'str' object has no attribute 'decode'
Fix that by converting the hex-string to bytes using `bytes.fromhex`.
dd695563e6 check-abi: explicitly provide public headers (Jonas Nick)
Pull request description:
Without this commit, the check-abi shell script outputs false positives because it consider some headers public that are actually not public.
ACKs for top commit:
real-or-random:
ACK dd695563e6
hebasto:
ACK dd695563e6, tested on Ubuntu 24.04.
Tree-SHA512: b26e61639061f5fbbdd47569ba04f91c627feeefc43ec3d529a3ac4012ab6487aa1904bd38100ed190dcaebdffe60895a8c99346720d5dee84a0c457ec3b6f94
7d2591ce12 Add secp256k1_pubkey_sort (Jonas Nick)
Pull request description:
This PR adds a `secp256k1_pubkey_sort` function the the public API which was originally part of the musig PR (#1479). However, I opened a separate PR because it adds internal functions that are also used by the WIP silent payments module.
ACKs for top commit:
sipa:
ACK 7d2591ce12
josibake:
ACK 7d2591ce12
real-or-random:
ACK 7d2591ce12
Tree-SHA512: d0e4464dc9cd4bdb35cc5d9bb4c37a7b71233328319165d49bc940d8d3394a2d74a43d2f73ee7bfe8f3f90a466ee8afcdca75cfbbf3969e218d76b89f4af55fb
4c341f89ab Add changelog entry for SDMC (Pieter Wuille)
a043940253 Permit COMB_BITS < 256 for exhaustive tests (Pieter Wuille)
39b2f2a321 Add test case for ecmult_gen recoded = {-1,0,1} (Pieter Wuille)
644e86de9a Reintroduce projective blinding (Pieter Wuille)
07810d9abb Reduce side channels from single-bit reads (Peter Dettman)
a0d32b597d Optimization: use Nx32 representation for recoded bits (Peter Dettman)
e03dcc44b5 Make secp256k1_scalar_get_bits support 32-bit reads (Pieter Wuille)
5005abee60 Rename scalar_get_bits -> scalar_get_bits_limb32; return uint32_t (Pieter Wuille)
6247f485b6 Optimization: avoid unnecessary doublings in precomputation (Peter Dettman)
15d0cca2a6 Optimization: first table lookup needs no point addition (Pieter Wuille)
7a33db35cd Optimization: move (2^COMB_BITS-1)/2 term into ctx->scalar_offset (Pieter Wuille)
ed2a056f3d Provide 3 configurations accessible through ./configure (Pieter Wuille)
5f7be9f6a5 Always generate tables for current (blocks,teeth) config (Pieter Wuille)
fde1dfcd8d Signed-digit multi-comb ecmult_gen algorithm (Peter Dettman)
486518b350 Make exhaustive tests's scalar_inverse(&x,&x) work (Pieter Wuille)
ab45c3e089 Initial gej blinding -> final ge blinding (Pieter Wuille)
aa00a6b892 Introduce CEIL_DIV macro and use it (Tim Ruffing)
Pull request description:
ACKs for top commit:
real-or-random:
reACK 4c341f89ab
jonasnick:
ACK 4c341f89ab
stratospher:
ACK 4c341f8. Did [these benchmarks](https://github.com/bitcoin-core/secp256k1/pull/1058#issuecomment-1002807283) and saw a 12.4% on gcc 13.2.0 and 11.5% on clang 15.0.0. Also summarised how the precomputed table generation works [here](https://github.com/stratospher/blogosphere/blob/main/sdmc.md) for future me :)
Tree-SHA512: 9a11138e4fb98b98e85c82cd46ed78b29fbe63d6efe61654ef519a64b1e175d63395a8a931c1646f9df8c7daacd796d5fe2384899d5a13a2c7ed2ded696ceed5
The existing code needs to deal with the edge case that bit_pos >= 256,
which would lead to an out-of-bounds read from secp256k1_scalar.
Instead, recode the scalar into an array of uint32_t with enough zero
padding at the end to alleviate the issue. This also simplifies the
code, and is necessary for a security improvement in a follow-up
commit.
Original code by Peter Dettman, with modifications by Pieter Wuille.
This introduces the signed-digit multi-comb multiplication algorithm
for constant-time G multiplications (ecmult_gen). It is based on
section 3.3 of "Fast and compact elliptic-curve cryptography" by
Mike Hamburg (see https://eprint.iacr.org/2012/309).
Original implementation by Peter Dettman, with changes by Pieter Wuille
to use scalars for recoding, and additional comments.
Instead of having the starting point of the ecmult_gen computation be
offset, do it with the final point. This enables reasoning over the
set of points reachable in intermediary computations, which can be
leveraged by potential future optimization.
Because the final point is in affine coordinates, its projective
blinding is no longer possible. It will be reintroduced again in
a different way, in a later commit.
Also introduce some more comments and more descriptive names.
f7f0184ba1 msan: notate more variable assignments from assembly code (Cory Fields)
a61339149f change inconsistent array param to pointer (Cory Fields)
Pull request description:
This was missed in 31ba404944 because older versions of clang did not complain about it. But clang-17, at least, does.
The array-as-a-param makes this annoying because `sizeof(l)` is not helpful. I'd be happy to change the size calculation if there are any better suggestions or strong preferences.
ACKs for top commit:
sipa:
utACK f7f0184ba1
real-or-random:
ACK f7f0184ba1 tests work fine with clang 17 and `./configure CFLAGS="-fsanitize=memory -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer -g -O1 -fno-optimize-sibling-calls" CC=clang`
Tree-SHA512: 8ab22209ef322a10f500b123c82ae5e7141ae1da0e7a890cbf90bd7d2eb11f397db4ccfe15a1666f2f49228585cccbf5bec741effebd1e2c6012cb7ea1689675
a5e8ab2484 ci: Add sanitizer env variables to debug output (Tim Ruffing)
84a93de4d2 ci: Add workaround for ASLR bug in sanitizers (Tim Ruffing)
Pull request description:
Fixes#1506.
This also adds the sanitizer env variables to our debug output as suggested in the same issue.
ACKs for top commit:
sipa:
utACK a5e8ab2484
jonasnick:
ACK a5e8ab2484
Tree-SHA512: 5162d14eeec01e088c600ed77e21c5ffd4dec23327b7e81b5ecac59b7c535cac97cd7b7b744c767766036dfc6d9152a9933eb326cf4065d56c46e2ee858da662
3a9b1d46a3 New Experimental Module: Incremental Half-Aggregation for Schnorr Signatures (Benedikt)
Pull request description:
Revisited PR #130 by jonasnick.
I am happy to hear your thoughts.
**Summary of changes compared to #130:**
- Address comments from rustyrussell
- Use tagged hash
- Compute hashes with common prefix by copying midstate
- Allow Incremental Aggregation and make code consistent with the [draft spec](https://github.com/BlockstreamResearch/cross-input-aggregation/blob/master/half-aggregation.mediawiki)
ACKs for top commit:
real-or-random:
ACK 3a9b1d46a3
Tree-SHA512: 27239033f8b28ecf87ea310b3dd5a19dbbe6fd07495db71ef7017f8f444ec25a12897087d1bea0a2e9c3df77d7f17c38b183d7fe768858da2180f26624add4aa
