From 2e3f13b9b171148016038b6c0df715bd5179d9c1 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 13 Nov 2023 10:12:26 +0000 Subject: [PATCH 1/9] Merge bitcoin/bitcoin#28831: test: Avoid intermittent failures in feature_init 44445ae8f1123c3affdcc0dbd7b3830eff5548ef test: Avoid intermittent failures in feature_init (MarcoFalke) Pull request description: The code not only modifies block dat files, but also leveldb files, which may be of smaller size. Such corruption may not force leveldb to abort, according to the intermittent test failures. Fix the intermittent test failures by reverting https://github.com/bitcoin/bitcoin/commit/5ab6419f380cc0a8cde78b125f3eeee5fcba43ae . ACKs for top commit: kevkevinpal: lgtm ACK [44445ae](https://github.com/bitcoin/bitcoin/pull/28831/commits/44445ae8f1123c3affdcc0dbd7b3830eff5548ef) fjahr: ACK 44445ae8f1123c3affdcc0dbd7b3830eff5548ef theStack: ACK 44445ae8f1123c3affdcc0dbd7b3830eff5548ef Tree-SHA512: 8084e4aeb8a976c1706a1898d7854c55d0c4ec7b5a08f65f97ffc173c935f9b0e0c1caef7be1538a458e4c018f7bd1948173349ec76ca48bc4013a63f284bb0f --- test/functional/feature_init.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index ce12319933de..6625a7e415b4 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -1,11 +1,10 @@ #!/usr/bin/env python3 -# Copyright (c) 2021 The Bitcoin Core developers +# Copyright (c) 2021-present The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Stress tests related to node initialization.""" import os from pathlib import Path -from random import randint import shutil from test_framework.test_framework import BitcoinTestFramework, SkipTest @@ -139,8 +138,8 @@ def check_clean_start(): # Since the genesis block is not checked by -checkblocks, the # perturbation window must be chosen such that a higher block # in blk*.dat is affected. - tf.seek(randint (150, 15000)) - tf.write(b'1' * randint(20, 2000)) + tf.seek(150) + tf.write(b"1" * 200) start_expecting_error(err_fragment) From 0ed18ca6c60659712f3f8d392dee2cf6530be6a1 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 22 Apr 2024 18:04:10 -0400 Subject: [PATCH 2/9] Merge bitcoin/bitcoin#29657: Bugfix: bitcoin-cli: Check length of peer.transport_protocol_type c3e632b44153e314ef946f342c68c2758b1cbc4d Bugfix: bitcoin-cli: Check length of peer.transport_protocol_type (Luke Dashjr) Pull request description: "v" would dereference beyond the string length, and "v10" would show as '1' Turn both of these cases into a blank, like anything else unexpected currently is. ACKs for top commit: sipa: utACK c3e632b44153e314ef946f342c68c2758b1cbc4d. hernanmarino: utACK c3e632b44153e314ef946f342c68c2758b1cbc4d alfonsoromanz: ACK c3e632b44153e314ef946f342c68c2758b1cbc4d achow101: ACK c3e632b44153e314ef946f342c68c2758b1cbc4d Tree-SHA512: f641e4412521adae7c8c8e1f268bdaaa223d9048d8286e3df4b13905faaa0d601155ce581cd649f760cab2acc4122356fa94a44714f1f190845552100105eda0 --- src/bitcoin-cli.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 3b0873970e9f..44a5f9266bce 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -553,7 +553,7 @@ class NetinfoRequestHandler : public BaseRequestHandler peer.is_outbound ? "out" : "in", ConnectionTypeForNetinfo(peer.conn_type), peer.network, - peer.transport_protocol_type.starts_with('v') == 0 ? peer.transport_protocol_type[1] : ' ', + (peer.transport_protocol_type.size() == 2 && peer.transport_protocol_type[0] == 'v') ? peer.transport_protocol_type[1] : ' ', PingTimeToString(peer.min_ping), PingTimeToString(peer.ping), peer.last_send ? ToString(time_now - peer.last_send) : "", From fddc7fc7185c2146375dbd123ad4b2ec2f85c200 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 1 Feb 2024 16:10:13 +0000 Subject: [PATCH 3/9] Merge bitcoin/bitcoin#29189: RFC: Deprecate libconsensus 25dc87e6f84c38c21e109e11f7bbd93f1e1f3183 libconsensus: deprecate (Cory Fields) Pull request description: This library has existed for nearly 10 years with very little known uptake or impact. It has become a maintenance burden. In several cases it dictates our code/library structure (for example necessitating LIBBITCOIN_CRYPTO_BASE), as well as build-system procedures (building multiple copies of object files especially for the lib). Several discussions have arisen wrt migrating it to CMake and it has become difficult to justify adding more complexity for a library that is virtually unused anyway. See for example the discussions: https://github.com/hebasto/bitcoin/pull/41 https://github.com/bitcoin/bitcoin/pull/29123 And here: https://github.com/bitcoin/bitcoin/pull/29180 Where it is pointed out that the libbitcoinconsensus functions are slower than those the internal bitcoind equivalents due to the missing sha2 implementations. Instead, we (fanquake, hebasto, TheCharlatan, and I) propose simply not migrating it to CMake and letting it end with v27. Any remaining use-cases could be handled in the future by libbitcoinkernel. If there are any users currently using libbitcoinconsensus, please chime in with your use-case! Edit: Corrected final release to be v27. ACKs for top commit: TheCharlatan: ACK 25dc87e6f84c38c21e109e11f7bbd93f1e1f3183 fanquake: ACK 25dc87e6f84c38c21e109e11f7bbd93f1e1f3183 - this library has very little, if any impactful real world usage. It has been entirely broken (on various platforms) for long periods of its existence, where nobody even noticed. Pruning this out to save porting, and starting anew with the kernel, is the sane thing to do. Tree-SHA512: baff2b3c4f76f520c96021035f751fdcb51bedf00e767660249e92a7bc7c5c176786bcf2c4cfe2d2351c200f932b39eb886bcfb22fbec824a41617590d6a1638 --- doc/release-notes-29189.md | 15 +++++++++++++++ doc/shared-libraries.md | 1 + 2 files changed, 16 insertions(+) create mode 100644 doc/release-notes-29189.md diff --git a/doc/release-notes-29189.md b/doc/release-notes-29189.md new file mode 100644 index 000000000000..3bed5ab5ef2b --- /dev/null +++ b/doc/release-notes-29189.md @@ -0,0 +1,15 @@ +dashconsensus +======================== + +This library is deprecated and will be removed for v28. + +It has existed for nearly 10 years with very little known uptake or impact. It +has become a maintenance burden. + +The underlying functionality does not change between versions, so any users of +the library can continue to use the final release indefinitely, with the +understanding that it will not receive future consensus updates. + +In the future, a kernel library will provide a much more useful API that is +aware of the UTXO set, and therefore be able to fully validate transactions and +blocks. diff --git a/doc/shared-libraries.md b/doc/shared-libraries.md index 0e8e40e9b66b..f4ad447265b4 100644 --- a/doc/shared-libraries.md +++ b/doc/shared-libraries.md @@ -2,6 +2,7 @@ Shared Libraries ================ ## dashconsensus +***This library is deprecated and will be removed in v28*** The purpose of this library is to make the verification functionality that is critical to Dash's consensus available to other applications, e.g. to language bindings. From 77a16f496c39541d0ad6adaee4a8616fbf0db3ef Mon Sep 17 00:00:00 2001 From: merge-script Date: Mon, 12 Aug 2024 14:04:12 +0100 Subject: [PATCH 4/9] Merge bitcoin/bitcoin#30633: Fixes for GCC 15 compatibility 055bc05792ff5d5b084563044818ebec12bfd748 policy/feerate.h: avoid constraint self-dependency (Matt Whitlock) 138f8671569f7ebb8c84e9d80c44cddeda9e3845 add missing #include for GCC 15 (Matt Whitlock) Pull request description: #30612 with changes made. GCC 15 introduces three build failures: * Two are related to missing includes. You can't use `uint16_t` et al. without including ``. * The third is harder to understand but easy to fix. GCC changed something about the way templates are instantiated when checking type constraints, and now there is a dependency loop while checking `std::optional`. This manifests as the following compile-time mess: ``` In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/format:48, from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/chrono_io.h:39, from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/chrono:3362, from ./util/time.h:9, from ./primitives/block.h:12, from ./blockencodings.h:8, from blockencodings.cpp:5: /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits: In substitution of 'template requires !(is_same_v, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< >::type>) constexpr std::optional::optional(const std::optional<_Tp>&) [with _Up = CFeeRate]': /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:1140:25: required by substitution of 'template using std::__is_constructible_impl = std::__bool_constant<__is_constructible(_Tp, _Args ...)> [with _Tp = CFeeRate; _Args = {std::optional&}]' 1140 | = __bool_constant<__is_constructible(_Tp, _Args...)>; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:1145:12: required from 'struct std::is_constructible&>' 1145 | struct is_constructible | ^~~~~~~~~~~~~~~~ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:178:35: required by substitution of 'template std::__detail::__first_t, typename std::enable_if<(!(bool)(_Bn::value)), void>::type ...> std::__detail::__or_fn(int) [with _Bn = {std::is_constructible&>, std::is_convertible&, CFeeRate>, std::is_constructible >, std::is_convertible, CFeeRate>, std::is_constructible&>, std::is_convertible&, CFeeRate>, std::is_constructible >, std::is_convertible, CFeeRate>}]' 178 | __enable_if_t...>; | ^~~~~ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:196:41: required from 'struct std::__or_&>, std::is_convertible&, CFeeRate>, std::is_constructible >, std::is_convertible, CFeeRate>, std::is_constructible&>, std::is_convertible&, CFeeRate>, std::is_constructible >, std::is_convertible, CFeeRate> >' 196 | : decltype(__detail::__or_fn<_Bn...>(0)) | ~~~~~~~~~~~~~~~~~~~~~~~~~^~~ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:824:45: required from 'constexpr const bool std::optional::__construct_from_contained_value' 824 | = !__converts_from_optional<_Tp, _From>::value; | ^~~~~ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:884:7: required by substitution of 'template requires !(is_same_v, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< >::type>) constexpr std::optional::optional(const std::optional<_Tp>&) [with _Up = CFeeRate]' 884 | && __construct_from_contained_value<_Up> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./validation.h:164:41: required from here 164 | return MempoolAcceptResult(state); | ^ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:886:2: required by the constraints of 'template template requires !(is_same_v, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< >::type>) constexpr std::optional<_Tp>::optional(const std::optional<_From>&)' /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:884:14: error: satisfaction of atomic constraint '__construct_from_contained_value<_Up, typename std::remove_cv< >::type> [with _Tp = _Tp; _Up = _Up]' depends on itself 884 | && __construct_from_contained_value<_Up> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` It is easiest to solve this by changing the `static_assert` in the explicit `CFeeRate` constructor to a SFINAE by using a type constraint on the function template parameter. We already [downstreamed](https://github.com/gentoo/gentoo/pull/38015) these fixes in Gentoo. ACKs for top commit: stickies-v: ACK 055bc05792ff5d5b084563044818ebec12bfd748 Tree-SHA512: ce9cb27bcd9b0f4bbc80951e45cf7127112dcb7f9937bcb0167b362026d35beecb1255354746de0aac82e03c41eaccbe26acbfe0ddff2ee1e5a8634673f4f4ba --- src/chainparamsbase.h | 1 + src/node/interface_ui.h | 3 ++- src/policy/feerate.h | 4 +--- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/chainparamsbase.h b/src/chainparamsbase.h index 718392e04def..c8f94901b495 100644 --- a/src/chainparamsbase.h +++ b/src/chainparamsbase.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_CHAINPARAMSBASE_H #define BITCOIN_CHAINPARAMSBASE_H +#include #include #include diff --git a/src/node/interface_ui.h b/src/node/interface_ui.h index 67a6d0f5b1b0..c4b36ef07cfb 100644 --- a/src/node/interface_ui.h +++ b/src/node/interface_ui.h @@ -6,9 +6,10 @@ #ifndef BITCOIN_NODE_INTERFACE_UI_H #define BITCOIN_NODE_INTERFACE_UI_H +#include #include -#include #include +#include class CBlockIndex; enum class SynchronizationState; diff --git a/src/policy/feerate.h b/src/policy/feerate.h index cee43b254468..7a010a9b4661 100644 --- a/src/policy/feerate.h +++ b/src/policy/feerate.h @@ -38,10 +38,8 @@ class CFeeRate public: /** Fee rate of 0 satoshis per kvB */ CFeeRate() : nSatoshisPerK(0) { } - template + template // Disallow silent float -> int conversion explicit CFeeRate(const I _nSatoshisPerK): nSatoshisPerK(_nSatoshisPerK) { - // We've previously had bugs creep in from silent double->int conversion... - static_assert(std::is_integral::value, "CFeeRate should be used without floats"); } /** From 4bdcbcd3b7e02e0a7a54bf0791a912001d21dc5b Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 21 Jun 2023 13:27:54 +0100 Subject: [PATCH 5/9] Merge bitcoin/bitcoin#27905: validation: add missing insert to m_dirty_blockindex e639364495a26bd67dd08998fc7ec400747f9a15 validation: add missing insert to m_dirty_blockindex (Martin Zumsande) Pull request description: When the status of a block index is changed, we must add it to `m_dirty_blockindex` or the change might not get persisted to disk. This is missing from one spot in `FindMostWorkChain()`, where `BLOCK_FAILED_CHILD` is set. Since we have [code](https://github.com/bitcoin/bitcoin/blob/f0758d8a6696657269d9c057e7aa079ffa9e1c16/src/node/blockstorage.cpp#L284-L287) that later sets missing `BLOCK_FAILED_CHILD` during the next startup, I don't think that this can lead to bad block indexes in practice, but I still think it's worth fixing. ACKs for top commit: TheCharlatan: ACK e639364495a26bd67dd08998fc7ec400747f9a15 stickies-v: ACK e639364495a26bd67dd08998fc7ec400747f9a15 Tree-SHA512: a97af9c173e31b90b677a1f95de822e08078d78013de5fa5fe4c3bec06f45d6e1823b7694cdacb887d031329e4b4afc6a2003916e0ae131279dee71f43e1f478 --- src/validation.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/validation.cpp b/src/validation.cpp index b7528e9b41f3..ee8167fbc76d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3111,6 +3111,7 @@ CBlockIndex* CChainState::FindMostWorkChain() while (pindexTest != pindexFailed) { if (fFailedChain) { pindexFailed->nStatus |= BLOCK_FAILED_CHILD; + m_blockman.m_dirty_blockindex.insert(pindexFailed); } else if (fConflictingChain) { // We don't need data for conflciting blocks pindexFailed->nStatus |= BLOCK_CONFLICT_CHAINLOCK; From a136b273422b870c01e41ba4dc8346e7ef9e44a2 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 19 Jul 2023 11:22:52 +0100 Subject: [PATCH 6/9] Merge bitcoin/bitcoin#28093: subtree: update libsecp256k1 to latest master 5080c9c25f44ae9d16a69d41f0da1d1e06483bf7 build: adapt Windows builds for libsecp256k1 build changes (fanquake) ff061fde183f68d3b77db24b5742f54e58952de6 Squashed 'src/secp256k1/' changes from 705ce7ed8c..c545fdc374 (fanquake) Pull request description: Includes https://github.com/bitcoin-core/secp256k1/pull/1378. Which fixes #28079. Adapts Windows build for https://github.com/bitcoin-core/secp256k1/pull/1367. ACKs for top commit: hebasto: ACK 5080c9c25f44ae9d16a69d41f0da1d1e06483bf7, I've made the `src/secp256k1` subtree update locally and got zero diff with this PR branch. jonasnick: ACK 5080c9c25f44ae9d16a69d41f0da1d1e06483bf7 Tree-SHA512: 37915d420ebacefc6bc82c2511bff3d6884e01d5c92795f19cd61862f96b30aa1fe768aeabec128c9d25c1d8bc62b46b46969626067266074b39566ad9e2f5ba --- configure.ac | 2 + src/secp256k1/.cirrus.yml | 54 +++-- src/secp256k1/CHANGELOG.md | 10 + src/secp256k1/Makefile.am | 6 +- src/secp256k1/ci/cirrus.sh | 19 +- src/secp256k1/ci/linux-debian.Dockerfile | 45 +++- src/secp256k1/configure.ac | 6 - src/secp256k1/doc/ellswift.md | 4 +- src/secp256k1/examples/CMakeLists.txt | 3 - src/secp256k1/examples/examples_util.h | 2 +- src/secp256k1/include/secp256k1.h | 63 +++--- src/secp256k1/include/secp256k1_ecdh.h | 4 +- src/secp256k1/include/secp256k1_ellswift.h | 8 +- src/secp256k1/include/secp256k1_extrakeys.h | 16 +- src/secp256k1/include/secp256k1_schnorrsig.h | 2 +- src/secp256k1/sage/group_prover.sage | 2 +- src/secp256k1/src/CMakeLists.txt | 6 +- src/secp256k1/src/ecdsa_impl.h | 21 +- src/secp256k1/src/ecmult.h | 2 +- src/secp256k1/src/ecmult_const_impl.h | 2 +- .../src/ecmult_gen_compute_table_impl.h | 3 + src/secp256k1/src/ecmult_impl.h | 4 +- src/secp256k1/src/field.h | 34 ++- src/secp256k1/src/field_10x26_impl.h | 4 +- src/secp256k1/src/field_5x52_impl.h | 4 +- src/secp256k1/src/field_impl.h | 14 +- src/secp256k1/src/group.h | 3 +- src/secp256k1/src/group_impl.h | 11 +- src/secp256k1/src/int128_struct_impl.h | 5 + .../src/modules/ellswift/Makefile.am.include | 1 + .../modules/ellswift/tests_exhaustive_impl.h | 39 ++++ .../src/modules/ellswift/tests_impl.h | 4 +- src/secp256k1/src/precompute_ecmult.c | 5 +- src/secp256k1/src/testrand_impl.h | 65 ++---- src/secp256k1/src/tests.c | 204 ++++++------------ src/secp256k1/src/tests_exhaustive.c | 16 ++ src/secp256k1/src/util.h | 13 ++ 37 files changed, 375 insertions(+), 331 deletions(-) create mode 100644 src/secp256k1/src/modules/ellswift/tests_exhaustive_impl.h diff --git a/configure.ac b/configure.ac index 0c97af58e657..991b700ca58a 100644 --- a/configure.ac +++ b/configure.ac @@ -823,6 +823,8 @@ case $host in AC_MSG_ERROR([windres not found]) fi + CORE_CPPFLAGS="$CORE_CPPFLAGS -DSECP256K1_STATIC" + CORE_CPPFLAGS="$CORE_CPPFLAGS -D_MT -DWIN32 -D_WINDOWS -D_WIN32_WINNT=0x0A00 -D_WIN32_IE=0x0A00 -DWIN32_LEAN_AND_MEAN" dnl Prevent the definition of min/max macros. dnl We always want to use the standard library. diff --git a/src/secp256k1/.cirrus.yml b/src/secp256k1/.cirrus.yml index 5a00b65a33fa..ae205c624c14 100644 --- a/src/secp256k1/.cirrus.yml +++ b/src/secp256k1/.cirrus.yml @@ -54,12 +54,6 @@ cat_logs_snippet: &CAT_LOGS cat_ci_env_script: - env -merge_base_script_snippet: &MERGE_BASE - merge_base_script: - - if [ "$CIRRUS_PR" = "" ]; then exit 0; fi - - git fetch --depth=1 $CIRRUS_REPO_CLONE_URL "pull/${CIRRUS_PR}/merge" - - git checkout FETCH_HEAD # Use merged changes to detect silent merge conflicts - linux_container_snippet: &LINUX_CONTAINER container: dockerfile: ci/linux-debian.Dockerfile @@ -68,12 +62,12 @@ linux_container_snippet: &LINUX_CONTAINER # Gives us more CPUs for free if they're available. greedy: true # More than enough for our scripts. - memory: 1G + memory: 2G task: name: "x86_64: Linux (Debian stable)" << : *LINUX_CONTAINER - matrix: &ENV_MATRIX + matrix: - env: {WIDEMUL: int64, RECOVERY: yes} - env: {WIDEMUL: int64, ECDH: yes, SCHNORRSIG: yes, ELLSWIFT: yes} - env: {WIDEMUL: int128} @@ -86,6 +80,7 @@ task: - env: {BUILD: distcheck, WITH_VALGRIND: no, CTIMETESTS: no, BENCH: no} - env: {CPPFLAGS: -DDETERMINISTIC} - env: {CFLAGS: -O0, CTIMETESTS: no} + - env: {CFLAGS: -O1, RECOVERY: yes, ECDH: yes, SCHNORRSIG: yes, ELLSWIFT: yes} - env: { ECMULTGENPRECISION: 2, ECMULTWINDOW: 2 } - env: { ECMULTGENPRECISION: 8, ECMULTWINDOW: 4 } matrix: @@ -93,7 +88,10 @@ task: CC: gcc - env: CC: clang - << : *MERGE_BASE + - env: + CC: gcc-snapshot + - env: + CC: clang-snapshot test_script: - ./ci/cirrus.sh << : *CAT_LOGS @@ -111,7 +109,6 @@ task: CC: i686-linux-gnu-gcc - env: CC: clang --target=i686-pc-linux-gnu -isystem /usr/i686-linux-gnu/include - << : *MERGE_BASE test_script: - ./ci/cirrus.sh << : *CAT_LOGS @@ -125,20 +122,22 @@ task: HOMEBREW_NO_INSTALL_CLEANUP: 1 # Cirrus gives us a fixed number of 4 virtual CPUs. Not that we even have that many jobs at the moment... MAKEFLAGS: -j5 - matrix: - << : *ENV_MATRIX env: ASM: no WITH_VALGRIND: no CTIMETESTS: no + CC: clang matrix: - - env: - CC: gcc - - env: - CC: clang + - env: {WIDEMUL: int64, RECOVERY: yes, ECDH: yes, SCHNORRSIG: yes, ELLSWIFT: yes} + - env: {WIDEMUL: int64, RECOVERY: yes, ECDH: yes, SCHNORRSIG: yes, ELLSWIFT: yes, CC: gcc} + - env: {WIDEMUL: int128_struct, ECMULTGENPRECISION: 2, ECMULTWINDOW: 4} + - env: {WIDEMUL: int128, ECDH: yes, SCHNORRSIG: yes, ELLSWIFT: yes} + - env: {WIDEMUL: int128, RECOVERY: yes, SCHNORRSIG: yes} + - env: {WIDEMUL: int128, RECOVERY: yes, ECDH: yes, SCHNORRSIG: yes, ELLSWIFT: yes, CC: gcc} + - env: {WIDEMUL: int128, RECOVERY: yes, ECDH: yes, SCHNORRSIG: yes, ELLSWIFT: yes, CPPFLAGS: -DVERIFY} + - env: {BUILD: distcheck} brew_script: - brew install automake libtool gcc - << : *MERGE_BASE test_script: - ./ci/cirrus.sh << : *CAT_LOGS @@ -157,7 +156,6 @@ task: SCHNORRSIG: yes ELLSWIFT: yes CTIMETESTS: no - << : *MERGE_BASE test_script: # https://sourceware.org/bugzilla/show_bug.cgi?id=27008 - rm /etc/ld.so.cache @@ -180,7 +178,6 @@ task: matrix: - env: {} - env: {EXPERIMENTAL: yes, ASM: arm32} - << : *MERGE_BASE test_script: - ./ci/cirrus.sh << : *CAT_LOGS @@ -198,7 +195,6 @@ task: SCHNORRSIG: yes ELLSWIFT: yes CTIMETESTS: no - << : *MERGE_BASE test_script: - ./ci/cirrus.sh << : *CAT_LOGS @@ -216,7 +212,6 @@ task: SCHNORRSIG: yes ELLSWIFT: yes CTIMETESTS: no - << : *MERGE_BASE test_script: - ./ci/cirrus.sh << : *CAT_LOGS @@ -237,7 +232,6 @@ task: - name: "i686 (mingw32-w64): Windows (Debian stable, Wine)" env: HOST: i686-w64-mingw32 - << : *MERGE_BASE test_script: - ./ci/cirrus.sh << : *CAT_LOGS @@ -280,7 +274,6 @@ task: CC: /opt/msvc/bin/x86/cl AR: /opt/msvc/bin/x86/lib NM: /opt/msvc/bin/x86/dumpbin -symbols -headers - << : *MERGE_BASE test_script: - ./ci/cirrus.sh << : *CAT_LOGS @@ -325,7 +318,6 @@ task: - env: HOST: i686-linux-gnu CC: i686-linux-gnu-gcc - << : *MERGE_BASE test_script: - ./ci/cirrus.sh << : *CAT_LOGS @@ -352,7 +344,6 @@ task: ECMULTGENPRECISION: 2 ECMULTWINDOW: 2 CFLAGS: "-fsanitize=memory -g -O3" - << : *MERGE_BASE test_script: - ./ci/cirrus.sh << : *CAT_LOGS @@ -369,7 +360,6 @@ task: RECOVERY: yes SCHNORRSIG: yes ELLSWIFT: yes - << : *MERGE_BASE test_script: - ./ci/cirrus.sh << : *CAT_LOGS @@ -401,11 +391,17 @@ task: # Ignore MSBuild warning MSB8029. # See: https://learn.microsoft.com/en-us/visualstudio/msbuild/errors/msb8029?view=vs-2022 IgnoreWarnIntDirInTempDetected: 'true' - merge_script: - - PowerShell -NoLogo -Command if ($env:CIRRUS_PR -ne $null) { git fetch $env:CIRRUS_REPO_CLONE_URL pull/$env:CIRRUS_PR/merge; git reset --hard FETCH_HEAD; } + matrix: + - env: + BUILD_SHARED_LIBS: ON + - env: + BUILD_SHARED_LIBS: OFF + git_show_script: + # Print commit to allow reproducing the job outside of CI. + - git show --no-patch configure_script: - '%x64_NATIVE_TOOLS%' - - cmake -E env CFLAGS="/WX" cmake -G "Visual Studio 17 2022" -A x64 -S . -B build -DSECP256K1_ENABLE_MODULE_RECOVERY=ON -DSECP256K1_BUILD_EXAMPLES=ON + - cmake -E env CFLAGS="/WX" cmake -A x64 -B build -DSECP256K1_ENABLE_MODULE_RECOVERY=ON -DSECP256K1_BUILD_EXAMPLES=ON -DBUILD_SHARED_LIBS=%BUILD_SHARED_LIBS% build_script: - '%x64_NATIVE_TOOLS%' - cmake --build build --config RelWithDebInfo -- -property:UseMultiToolTask=true;CL_MPcount=5 diff --git a/src/secp256k1/CHANGELOG.md b/src/secp256k1/CHANGELOG.md index 8e31edc6ee83..dacb0a3f48c0 100644 --- a/src/secp256k1/CHANGELOG.md +++ b/src/secp256k1/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +#### Added + - New module `ellswift` implements ElligatorSwift encoding for public keys and x-only Diffie-Hellman key exchange for them. + ElligatorSwift permits representing secp256k1 public keys as 64-byte arrays which cannot be distinguished from uniformly random. See: + - Header file `include/secp256k1_ellswift.h` which defines the new API. + - Document `doc/ellswift.md` which explains the mathematical background of the scheme. + - The [paper](https://eprint.iacr.org/2022/759) on which the scheme is based. + +#### Changed + - When consuming libsecp256k1 as a static library on Windows, the user must now define the `SECP256K1_STATIC` macro before including `secp256k1.h`. + ## [0.3.2] - 2023-05-13 We strongly recommend updating to 0.3.2 if you use or plan to use GCC >=13 to compile libsecp256k1. When in doubt, check the GCC version using `gcc -v`. diff --git a/src/secp256k1/Makefile.am b/src/secp256k1/Makefile.am index ee14ac45095e..32bc729a41a2 100644 --- a/src/secp256k1/Makefile.am +++ b/src/secp256k1/Makefile.am @@ -153,7 +153,7 @@ endif if USE_EXAMPLES noinst_PROGRAMS += ecdsa_example ecdsa_example_SOURCES = examples/ecdsa.c -ecdsa_example_CPPFLAGS = -I$(top_srcdir)/include +ecdsa_example_CPPFLAGS = -I$(top_srcdir)/include -DSECP256K1_STATIC ecdsa_example_LDADD = libsecp256k1.la ecdsa_example_LDFLAGS = -static if BUILD_WINDOWS @@ -163,7 +163,7 @@ TESTS += ecdsa_example if ENABLE_MODULE_ECDH noinst_PROGRAMS += ecdh_example ecdh_example_SOURCES = examples/ecdh.c -ecdh_example_CPPFLAGS = -I$(top_srcdir)/include +ecdh_example_CPPFLAGS = -I$(top_srcdir)/include -DSECP256K1_STATIC ecdh_example_LDADD = libsecp256k1.la ecdh_example_LDFLAGS = -static if BUILD_WINDOWS @@ -174,7 +174,7 @@ endif if ENABLE_MODULE_SCHNORRSIG noinst_PROGRAMS += schnorr_example schnorr_example_SOURCES = examples/schnorr.c -schnorr_example_CPPFLAGS = -I$(top_srcdir)/include +schnorr_example_CPPFLAGS = -I$(top_srcdir)/include -DSECP256K1_STATIC schnorr_example_LDADD = libsecp256k1.la schnorr_example_LDFLAGS = -static if BUILD_WINDOWS diff --git a/src/secp256k1/ci/cirrus.sh b/src/secp256k1/ci/cirrus.sh index 8d8281861176..48a9783cc417 100755 --- a/src/secp256k1/ci/cirrus.sh +++ b/src/secp256k1/ci/cirrus.sh @@ -4,7 +4,8 @@ set -eux export LC_ALL=C -# Print relevant CI environment to allow reproducing the job outside of CI. +# Print commit and relevant CI environment to allow reproducing the job outside of CI. +git show --no-patch print_environment() { # Turn off -x because it messes up the output set +x @@ -53,6 +54,22 @@ if [ -n "$WRAPPER_CMD" ]; then $WRAPPER_CMD --version fi +# Workaround for https://bugs.kde.org/show_bug.cgi?id=452758 (fixed in valgrind 3.20.0). +case "${CC:-undefined}" in + clang*) + if [ "$CTIMETESTS" = "yes" ] && [ "$WITH_VALGRIND" = "yes" ] + then + export CFLAGS="${CFLAGS:+$CFLAGS }-gdwarf-4" + else + case "$WRAPPER_CMD" in + valgrind*) + export CFLAGS="${CFLAGS:+$CFLAGS }-gdwarf-4" + ;; + esac + fi + ;; +esac + ./autogen.sh ./configure \ diff --git a/src/secp256k1/ci/linux-debian.Dockerfile b/src/secp256k1/ci/linux-debian.Dockerfile index 54eafcab25f5..dbb1dd291929 100644 --- a/src/secp256k1/ci/linux-debian.Dockerfile +++ b/src/secp256k1/ci/linux-debian.Dockerfile @@ -1,5 +1,7 @@ FROM debian:stable +SHELL ["/bin/bash", "-c"] + RUN dpkg --add-architecture i386 && \ dpkg --add-architecture s390x && \ dpkg --add-architecture armhf && \ @@ -9,11 +11,11 @@ RUN dpkg --add-architecture i386 && \ # dkpg-dev: to make pkg-config work in cross-builds # llvm: for llvm-symbolizer, which is used by clang's UBSan for symbolized stack traces RUN apt-get update && apt-get install --no-install-recommends -y \ - git ca-certificates \ + git ca-certificates wget \ make automake libtool pkg-config dpkg-dev valgrind qemu-user \ - gcc clang llvm libc6-dbg \ + gcc clang llvm libclang-rt-dev libc6-dbg \ g++ \ - gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 libubsan1:i386 libasan6:i386 \ + gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 libubsan1:i386 libasan8:i386 \ gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \ gcc-arm-linux-gnueabihf libc6-dev-armhf-cross libc6-dbg:armhf \ gcc-aarch64-linux-gnu libc6-dev-arm64-cross libc6-dbg:arm64 \ @@ -23,9 +25,44 @@ RUN apt-get update && apt-get install --no-install-recommends -y \ sagemath WORKDIR /root -# The "wine" package provides a convience wrapper that we need + +# Build and install gcc snapshot +ARG GCC_SNAPSHOT_MAJOR=14 +RUN wget --progress=dot:giga --https-only --recursive --accept '*.tar.xz' --level 1 --no-directories "https://gcc.gnu.org/pub/gcc/snapshots/LATEST-${GCC_SNAPSHOT_MAJOR}" && \ + wget "https://gcc.gnu.org/pub/gcc/snapshots/LATEST-${GCC_SNAPSHOT_MAJOR}/sha512.sum" && \ + sha512sum --check --ignore-missing sha512.sum && \ + # We should have downloaded exactly one tar.xz file + ls && \ + [[ $(ls *.tar.xz | wc -l) -eq "1" ]] && \ + tar xf *.tar.xz && \ + mkdir gcc-build && cd gcc-build && \ + apt-get update && apt-get install --no-install-recommends -y libgmp-dev libmpfr-dev libmpc-dev flex && \ + ../*/configure --prefix=/opt/gcc-snapshot --enable-languages=c --disable-bootstrap --disable-multilib --without-isl && \ + make -j $(nproc) && \ + make install && \ + ln -s /opt/gcc-snapshot/bin/gcc /usr/bin/gcc-snapshot + +# Install clang snapshot +RUN wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key | tee /etc/apt/trusted.gpg.d/apt.llvm.org.asc && \ + # Add repository for this Debian release + . /etc/os-release && echo "deb http://apt.llvm.org/${VERSION_CODENAME} llvm-toolchain-${VERSION_CODENAME} main" >> /etc/apt/sources.list && \ + # Install clang snapshot + apt-get update && apt-get install --no-install-recommends -y clang && \ + # Remove just the "clang" symlink again + apt-get remove -y clang && \ + # We should have exactly two clang versions now + ls /usr/bin/clang* && \ + [[ $(ls /usr/bin/clang-?? | sort | wc -l) -eq "2" ]] && \ + # Create symlinks for them + ln -s $(ls /usr/bin/clang-?? | sort | tail -1) /usr/bin/clang-snapshot && \ + ln -s $(ls /usr/bin/clang-?? | sort | head -1) /usr/bin/clang + +# The "wine" package provides a convenience wrapper that we need RUN apt-get update && apt-get install --no-install-recommends -y \ git ca-certificates wine64 wine python3-simplejson python3-six msitools winbind procps && \ +# Workaround for `wine` package failure to employ the Debian alternatives system properly. + ln -s /usr/lib/wine/wine64 /usr/bin/wine64 && \ +# Set of tools for using MSVC on Linux. git clone https://github.com/mstorsjo/msvc-wine && \ mkdir /opt/msvc && \ python3 msvc-wine/vsdownload.py --accept-license --dest /opt/msvc Microsoft.VisualStudio.Workload.VCTools && \ diff --git a/src/secp256k1/configure.ac b/src/secp256k1/configure.ac index 82cf95132d6f..a502d1304a7b 100644 --- a/src/secp256k1/configure.ac +++ b/src/secp256k1/configure.ac @@ -127,12 +127,6 @@ AC_DEFUN([SECP_TRY_APPEND_DEFAULT_CFLAGS], [ SECP_TRY_APPEND_CFLAGS([-wd4267], $1) # Disable warning C4267 "'var' : conversion from 'size_t' to 'type', possible loss of data". # Eliminate deprecation warnings for the older, less secure functions. CPPFLAGS="-D_CRT_SECURE_NO_WARNINGS $CPPFLAGS" - # We pass -ignore:4217 to the MSVC linker to suppress warning 4217 when - # importing variables from a statically linked secp256k1. - # (See the libtool manual, section "Windows DLLs" for background.) - # Unfortunately, libtool tries to be too clever and strips "-Xlinker arg" - # into "arg", so this will be " -Xlinker -ignore:4217" after stripping. - LDFLAGS="-Xlinker -Xlinker -Xlinker -ignore:4217 $LDFLAGS" fi ]) SECP_TRY_APPEND_DEFAULT_CFLAGS(SECP_CFLAGS) diff --git a/src/secp256k1/doc/ellswift.md b/src/secp256k1/doc/ellswift.md index 7fbb7c178798..9d60e6be0b61 100644 --- a/src/secp256k1/doc/ellswift.md +++ b/src/secp256k1/doc/ellswift.md @@ -88,7 +88,7 @@ $$ \begin{array}{lcl} X(u, t) & = & \left\\{\begin{array}{ll} \dfrac{g(u) - t^2}{2t} & a = 0 \\ - \dfrac{g(u) + h(u)(Y_0(u) + X_0(u)t)^2}{X_0(u)(1 + h(u)t^2)} & a \neq 0 + \dfrac{g(u) + h(u)(Y_0(u) - X_0(u)t)^2}{X_0(u)(1 + h(u)t^2)} & a \neq 0 \end{array}\right. \\ Y(u, t) & = & \left\\{\begin{array}{ll} \dfrac{X(u, t) + t}{u \sqrt{-3}} = \dfrac{g(u) + t^2}{2tu\sqrt{-3}} & a = 0 \\ @@ -329,7 +329,7 @@ $t$ value for multiple $c$ inputs (thereby biasing that encoding): it requires $g(u)=0$ which is already outlawed on even-ordered curves and impossible on others; in the second it would trigger division by zero. * Curve-specific special cases also exist that need to be rejected, because they result in $(u,t)$ which is invalid to the decoder, or because of division by zero in the encoder: * For $a=0$ curves, when $u=0$ or when $t=0$. The latter can only be reached by the encoder when $g(u)=0$, which requires an even-ordered curve. - * For $a \neq 0$ curves, when $X_0(u)=0$, when $h(u)t^2 = -1$, or when $2w(u + 2v) = 2X_0(u)$ while also either $w \neq 2Y_0(u)$ or $h(u)=0$. + * For $a \neq 0$ curves, when $X_0(u)=0$, when $h(u)t^2 = -1$, or when $w(u + 2v) = 2X_0(u)$ while also either $w \neq 2Y_0(u)$ or $h(u)=0$. **Define** a version of $G_{c,u}(x)$ which deals with all these cases: * If $a=0$ and $u=0$, return $\bot.$ diff --git a/src/secp256k1/examples/CMakeLists.txt b/src/secp256k1/examples/CMakeLists.txt index e095b7f84fd4..e2ea47300854 100644 --- a/src/secp256k1/examples/CMakeLists.txt +++ b/src/secp256k1/examples/CMakeLists.txt @@ -6,9 +6,6 @@ target_link_libraries(example INTERFACE secp256k1 $<$:bcrypt> ) -if(NOT BUILD_SHARED_LIBS AND MSVC) - target_link_options(example INTERFACE /IGNORE:4217) -endif() add_executable(ecdsa_example ecdsa.c) target_link_libraries(ecdsa_example example) diff --git a/src/secp256k1/examples/examples_util.h b/src/secp256k1/examples/examples_util.h index 8e3a8f00cfd4..3293b640321b 100644 --- a/src/secp256k1/examples/examples_util.h +++ b/src/secp256k1/examples/examples_util.h @@ -95,7 +95,7 @@ static void secure_erase(void *ptr, size_t len) { * As best as we can tell, this is sufficient to break any optimisations that * might try to eliminate "superfluous" memsets. * This method used in memzero_explicit() the Linux kernel, too. Its advantage is that it is - * pretty efficient, because the compiler can still implement the memset() efficently, + * pretty efficient, because the compiler can still implement the memset() efficiently, * just not remove it entirely. See "Dead Store Elimination (Still) Considered Harmful" by * Yang et al. (USENIX Security 2017) for more background. */ diff --git a/src/secp256k1/include/secp256k1.h b/src/secp256k1/include/secp256k1.h index a7a2be7a3a48..936f0b42b7d0 100644 --- a/src/secp256k1/include/secp256k1.h +++ b/src/secp256k1/include/secp256k1.h @@ -133,28 +133,35 @@ typedef int (*secp256k1_nonce_function)( # define SECP256K1_NO_BUILD #endif -/* Symbol visibility. See libtool manual, section "Windows DLLs". */ -#if defined(_WIN32) && !defined(__GNUC__) -# ifdef SECP256K1_BUILD -# ifdef DLL_EXPORT -# define SECP256K1_API __declspec (dllexport) -# define SECP256K1_API_VAR extern __declspec (dllexport) +/* Symbol visibility. */ +#if defined(_WIN32) + /* GCC for Windows (e.g., MinGW) accepts the __declspec syntax + * for MSVC compatibility. A __declspec declaration implies (but is not + * exactly equivalent to) __attribute__ ((visibility("default"))), and so we + * actually want __declspec even on GCC, see "Microsoft Windows Function + * Attributes" in the GCC manual and the recommendations in + * https://gcc.gnu.org/wiki/Visibility. */ +# if defined(SECP256K1_BUILD) +# if defined(DLL_EXPORT) || defined(SECP256K1_DLL_EXPORT) + /* Building libsecp256k1 as a DLL. + * 1. If using Libtool, it defines DLL_EXPORT automatically. + * 2. In other cases, SECP256K1_DLL_EXPORT must be defined. */ +# define SECP256K1_API extern __declspec (dllexport) # endif -# elif defined _MSC_VER -# define SECP256K1_API -# define SECP256K1_API_VAR extern __declspec (dllimport) -# elif defined DLL_EXPORT -# define SECP256K1_API __declspec (dllimport) -# define SECP256K1_API_VAR extern __declspec (dllimport) + /* The user must define SECP256K1_STATIC when consuming libsecp256k1 as a static + * library on Windows. */ +# elif !defined(SECP256K1_STATIC) + /* Consuming libsecp256k1 as a DLL. */ +# define SECP256K1_API extern __declspec (dllimport) # endif #endif #ifndef SECP256K1_API # if defined(__GNUC__) && (__GNUC__ >= 4) && defined(SECP256K1_BUILD) -# define SECP256K1_API __attribute__ ((visibility ("default"))) -# define SECP256K1_API_VAR extern __attribute__ ((visibility ("default"))) + /* Building libsecp256k1 on non-Windows using GCC or compatible. */ +# define SECP256K1_API extern __attribute__ ((visibility ("default"))) # else -# define SECP256K1_API -# define SECP256K1_API_VAR extern + /* All cases not captured above. */ +# define SECP256K1_API extern # endif #endif @@ -226,10 +233,10 @@ typedef int (*secp256k1_nonce_function)( * * It is highly recommended to call secp256k1_selftest before using this context. */ -SECP256K1_API_VAR const secp256k1_context *secp256k1_context_static; +SECP256K1_API const secp256k1_context *secp256k1_context_static; /** Deprecated alias for secp256k1_context_static. */ -SECP256K1_API_VAR const secp256k1_context *secp256k1_context_no_precomp +SECP256K1_API const secp256k1_context *secp256k1_context_no_precomp SECP256K1_DEPRECATED("Use secp256k1_context_static instead"); /** Perform basic self tests (to be used in conjunction with secp256k1_context_static) @@ -626,10 +633,10 @@ SECP256K1_API int secp256k1_ecdsa_signature_normalize( * If a data pointer is passed, it is assumed to be a pointer to 32 bytes of * extra entropy. */ -SECP256K1_API_VAR const secp256k1_nonce_function secp256k1_nonce_function_rfc6979; +SECP256K1_API const secp256k1_nonce_function secp256k1_nonce_function_rfc6979; /** A default safe nonce generation function (currently equal to secp256k1_nonce_function_rfc6979). */ -SECP256K1_API_VAR const secp256k1_nonce_function secp256k1_nonce_function_default; +SECP256K1_API const secp256k1_nonce_function secp256k1_nonce_function_default; /** Create an ECDSA signature. * @@ -733,10 +740,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_negate( * invalid according to secp256k1_ec_seckey_verify, this * function returns 0. seckey will be set to some unspecified * value if this function returns 0. - * In: tweak32: pointer to a 32-byte tweak. If the tweak is invalid according to - * secp256k1_ec_seckey_verify, this function returns 0. For - * uniformly random 32-byte arrays the chance of being invalid - * is negligible (around 1 in 2^128). + * In: tweak32: pointer to a 32-byte tweak, which must be valid according to + * secp256k1_ec_seckey_verify or 32 zero bytes. For uniformly + * random 32-byte tweaks, the chance of being invalid is + * negligible (around 1 in 2^128). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_tweak_add( const secp256k1_context *ctx, @@ -761,10 +768,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_add( * Args: ctx: pointer to a context object. * In/Out: pubkey: pointer to a public key object. pubkey will be set to an * invalid value if this function returns 0. - * In: tweak32: pointer to a 32-byte tweak. If the tweak is invalid according to - * secp256k1_ec_seckey_verify, this function returns 0. For - * uniformly random 32-byte arrays the chance of being invalid - * is negligible (around 1 in 2^128). + * In: tweak32: pointer to a 32-byte tweak, which must be valid according to + * secp256k1_ec_seckey_verify or 32 zero bytes. For uniformly + * random 32-byte tweaks, the chance of being invalid is + * negligible (around 1 in 2^128). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( const secp256k1_context *ctx, diff --git a/src/secp256k1/include/secp256k1_ecdh.h b/src/secp256k1/include/secp256k1_ecdh.h index 837ae2abe505..515e17429986 100644 --- a/src/secp256k1/include/secp256k1_ecdh.h +++ b/src/secp256k1/include/secp256k1_ecdh.h @@ -27,11 +27,11 @@ typedef int (*secp256k1_ecdh_hash_function)( /** An implementation of SHA256 hash function that applies to compressed public key. * Populates the output parameter with 32 bytes. */ -SECP256K1_API_VAR const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_sha256; +SECP256K1_API const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_sha256; /** A default ECDH hash function (currently equal to secp256k1_ecdh_hash_function_sha256). * Populates the output parameter with 32 bytes. */ -SECP256K1_API_VAR const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_default; +SECP256K1_API const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_default; /** Compute an EC Diffie-Hellman secret in constant time * diff --git a/src/secp256k1/include/secp256k1_ellswift.h b/src/secp256k1/include/secp256k1_ellswift.h index 3851f930988c..f79bd883961e 100644 --- a/src/secp256k1/include/secp256k1_ellswift.h +++ b/src/secp256k1/include/secp256k1_ellswift.h @@ -41,6 +41,8 @@ extern "C" { * - The paper uses an additional encoding bit for the parity of y. Here the * parity of t is used (negating t does not affect the decoded x coordinate, * so this is possible). + * + * For mathematical background about the scheme, see the doc/ellswift.md file. */ /** A pointer to a function used by secp256k1_ellswift_xdh to hash the shared X @@ -70,7 +72,7 @@ typedef int (*secp256k1_ellswift_xdh_hash_function)( /** An implementation of an secp256k1_ellswift_xdh_hash_function which uses * SHA256(prefix64 || ell_a64 || ell_b64 || x32), where prefix64 is the 64-byte * array pointed to by data. */ -SECP256K1_API_VAR const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_xdh_hash_function_prefix; +SECP256K1_API const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_xdh_hash_function_prefix; /** An implementation of an secp256k1_ellswift_xdh_hash_function compatible with * BIP324. It returns H_tag(ell_a64 || ell_b64 || x32), where H_tag is the @@ -78,7 +80,7 @@ SECP256K1_API_VAR const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_ * to secp256k1_ellswift_xdh_hash_function_prefix with prefix64 set to * SHA256("bip324_ellswift_xonly_ecdh")||SHA256("bip324_ellswift_xonly_ecdh"). * The data argument is ignored. */ -SECP256K1_API_VAR const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_xdh_hash_function_bip324; +SECP256K1_API const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_xdh_hash_function_bip324; /** Construct a 64-byte ElligatorSwift encoding of a given pubkey. * @@ -159,7 +161,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ellswift_create( /** Given a private key, and ElligatorSwift public keys sent in both directions, * compute a shared secret using x-only Elliptic Curve Diffie-Hellman (ECDH). * - * Returns: 1: shared secret was succesfully computed + * Returns: 1: shared secret was successfully computed * 0: secret was invalid or hashfp returned 0 * Args: ctx: pointer to a context object. * Out: output: pointer to an array to be filled by hashfp. diff --git a/src/secp256k1/include/secp256k1_extrakeys.h b/src/secp256k1/include/secp256k1_extrakeys.h index 673fca01f98e..7fcce68e6818 100644 --- a/src/secp256k1/include/secp256k1_extrakeys.h +++ b/src/secp256k1/include/secp256k1_extrakeys.h @@ -112,10 +112,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_from_pubke * Out: output_pubkey: pointer to a public key to store the result. Will be set * to an invalid value if this function returns 0. * In: internal_pubkey: pointer to an x-only pubkey to apply the tweak to. - * tweak32: pointer to a 32-byte tweak. If the tweak is invalid - * according to secp256k1_ec_seckey_verify, this function - * returns 0. For uniformly random 32-byte arrays the - * chance of being invalid is negligible (around 1 in 2^128). + * tweak32: pointer to a 32-byte tweak, which must be valid + * according to secp256k1_ec_seckey_verify or 32 zero + * bytes. For uniformly random 32-byte tweaks, the chance of + * being invalid is negligible (around 1 in 2^128). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_tweak_add( const secp256k1_context *ctx, @@ -229,10 +229,10 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_xonly_pub( * Args: ctx: pointer to a context object. * In/Out: keypair: pointer to a keypair to apply the tweak to. Will be set to * an invalid value if this function returns 0. - * In: tweak32: pointer to a 32-byte tweak. If the tweak is invalid according - * to secp256k1_ec_seckey_verify, this function returns 0. For - * uniformly random 32-byte arrays the chance of being invalid - * is negligible (around 1 in 2^128). + * In: tweak32: pointer to a 32-byte tweak, which must be valid according to + * secp256k1_ec_seckey_verify or 32 zero bytes. For uniformly + * random 32-byte tweaks, the chance of being invalid is + * negligible (around 1 in 2^128). */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_keypair_xonly_tweak_add( const secp256k1_context *ctx, diff --git a/src/secp256k1/include/secp256k1_schnorrsig.h b/src/secp256k1/include/secp256k1_schnorrsig.h index 1ee665fd19ac..26358533f67f 100644 --- a/src/secp256k1/include/secp256k1_schnorrsig.h +++ b/src/secp256k1/include/secp256k1_schnorrsig.h @@ -61,7 +61,7 @@ typedef int (*secp256k1_nonce_function_hardened)( * Therefore, to create BIP-340 compliant signatures, algo must be set to * "BIP0340/nonce" and algolen to 13. */ -SECP256K1_API_VAR const secp256k1_nonce_function_hardened secp256k1_nonce_function_bip340; +SECP256K1_API const secp256k1_nonce_function_hardened secp256k1_nonce_function_bip340; /** Data structure that contains additional arguments for schnorrsig_sign_custom. * diff --git a/src/secp256k1/sage/group_prover.sage b/src/secp256k1/sage/group_prover.sage index 9305c215d592..bb0929536950 100644 --- a/src/secp256k1/sage/group_prover.sage +++ b/src/secp256k1/sage/group_prover.sage @@ -198,7 +198,7 @@ def normalize_factor(p): (8) * (-bx + ax)^3 ``` """ - # Assert p is not 0 and that its non-zero coeffients are coprime. + # Assert p is not 0 and that its non-zero coefficients are coprime. # (We could just work with the primitive part p/p.content() but we want to be # aware if factor() does not return a primitive part in future sage versions.) assert p.content() == 1 diff --git a/src/secp256k1/src/CMakeLists.txt b/src/secp256k1/src/CMakeLists.txt index 0bba19982ad5..b305751b08da 100644 --- a/src/secp256k1/src/CMakeLists.txt +++ b/src/secp256k1/src/CMakeLists.txt @@ -20,10 +20,10 @@ if(SECP256K1_ASM STREQUAL "arm32") target_link_libraries(secp256k1_asm INTERFACE secp256k1_asm_arm) endif() -# Define our export symbol only for Win32 and only for shared libs. -# This matches libtool's usage of DLL_EXPORT if(WIN32) - set_target_properties(secp256k1 PROPERTIES DEFINE_SYMBOL "DLL_EXPORT") + # Define our export symbol only for shared libs. + set_target_properties(secp256k1 PROPERTIES DEFINE_SYMBOL SECP256K1_DLL_EXPORT) + target_compile_definitions(secp256k1 INTERFACE $<$>:SECP256K1_STATIC>) endif() # Object libs don't know if they're being built for a shared or static lib. diff --git a/src/secp256k1/src/ecdsa_impl.h b/src/secp256k1/src/ecdsa_impl.h index 48e30851b54f..e71254d9f989 100644 --- a/src/secp256k1/src/ecdsa_impl.h +++ b/src/secp256k1/src/ecdsa_impl.h @@ -16,17 +16,8 @@ #include "ecdsa.h" /** Group order for secp256k1 defined as 'n' in "Standards for Efficient Cryptography" (SEC2) 2.7.1 - * sage: for t in xrange(1023, -1, -1): - * .. p = 2**256 - 2**32 - t - * .. if p.is_prime(): - * .. print '%x'%p - * .. break - * 'fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f' - * sage: a = 0 - * sage: b = 7 - * sage: F = FiniteField (p) - * sage: '%x' % (EllipticCurve ([F (a), F (b)]).order()) - * 'fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141' + * $ sage -c 'load("secp256k1_params.sage"); print(hex(N))' + * 0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141 */ static const secp256k1_fe secp256k1_ecdsa_const_order_as_fe = SECP256K1_FE_CONST( 0xFFFFFFFFUL, 0xFFFFFFFFUL, 0xFFFFFFFFUL, 0xFFFFFFFEUL, @@ -35,12 +26,8 @@ static const secp256k1_fe secp256k1_ecdsa_const_order_as_fe = SECP256K1_FE_CONST /** Difference between field and order, values 'p' and 'n' values defined in * "Standards for Efficient Cryptography" (SEC2) 2.7.1. - * sage: p = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F - * sage: a = 0 - * sage: b = 7 - * sage: F = FiniteField (p) - * sage: '%x' % (p - EllipticCurve ([F (a), F (b)]).order()) - * '14551231950b75fc4402da1722fc9baee' + * $ sage -c 'load("secp256k1_params.sage"); print(hex(P-N))' + * 0x14551231950b75fc4402da1722fc9baee */ static const secp256k1_fe secp256k1_ecdsa_const_p_minus_order = SECP256K1_FE_CONST( 0, 0, 0, 1, 0x45512319UL, 0x50B75FC4UL, 0x402DA172UL, 0x2FC9BAEEUL diff --git a/src/secp256k1/src/ecmult.h b/src/secp256k1/src/ecmult.h index e28c60250671..326a5eeb4344 100644 --- a/src/secp256k1/src/ecmult.h +++ b/src/secp256k1/src/ecmult.h @@ -22,7 +22,7 @@ # pragma message DEBUG_CONFIG_DEF(ECMULT_WINDOW_SIZE) #endif -/* Noone will ever need more than a window size of 24. The code might +/* No one will ever need more than a window size of 24. The code might * be correct for larger values of ECMULT_WINDOW_SIZE but this is not * tested. * diff --git a/src/secp256k1/src/ecmult_const_impl.h b/src/secp256k1/src/ecmult_const_impl.h index 26b3e238d81b..06f9e53ffdbd 100644 --- a/src/secp256k1/src/ecmult_const_impl.h +++ b/src/secp256k1/src/ecmult_const_impl.h @@ -276,7 +276,7 @@ static int secp256k1_ecmult_const_xonly(secp256k1_fe* r, const secp256k1_fe *n, * * It is easy to verify that both (n*g, g^2, v) and its negation (n*g, -g^2, v) have affine X * coordinate n/d, and this holds even when the square root function doesn't have a - * determinstic sign. We choose the (n*g, g^2, v) version. + * deterministic sign. We choose the (n*g, g^2, v) version. * * Now switch to the effective affine curve using phi_v, where the input point has coordinates * (n*g, g^2). Compute (X, Y, Z) = q * (n*g, g^2) there. diff --git a/src/secp256k1/src/ecmult_gen_compute_table_impl.h b/src/secp256k1/src/ecmult_gen_compute_table_impl.h index 7d672b9950aa..dfbacdbfde2d 100644 --- a/src/secp256k1/src/ecmult_gen_compute_table_impl.h +++ b/src/secp256k1/src/ecmult_gen_compute_table_impl.h @@ -22,6 +22,9 @@ static void secp256k1_ecmult_gen_compute_table(secp256k1_ge_storage* table, cons secp256k1_gej nums_gej; int i, j; + VERIFY_CHECK(g > 0); + VERIFY_CHECK(n > 0); + /* get the generator */ secp256k1_gej_set_ge(&gj, gen); diff --git a/src/secp256k1/src/ecmult_impl.h b/src/secp256k1/src/ecmult_impl.h index f4624677d7fe..6d14c7ac4be7 100644 --- a/src/secp256k1/src/ecmult_impl.h +++ b/src/secp256k1/src/ecmult_impl.h @@ -288,7 +288,9 @@ static void secp256k1_ecmult_strauss_wnaf(const struct secp256k1_strauss_state * } /* Bring them to the same Z denominator. */ - secp256k1_ge_table_set_globalz(ECMULT_TABLE_SIZE(WINDOW_A) * no, state->pre_a, state->aux); + if (no) { + secp256k1_ge_table_set_globalz(ECMULT_TABLE_SIZE(WINDOW_A) * no, state->pre_a, state->aux); + } for (np = 0; np < no; ++np) { for (i = 0; i < ECMULT_TABLE_SIZE(WINDOW_A); i++) { diff --git a/src/secp256k1/src/field.h b/src/secp256k1/src/field.h index e632f9e3e2ea..bb99f948ef1f 100644 --- a/src/secp256k1/src/field.h +++ b/src/secp256k1/src/field.h @@ -88,8 +88,8 @@ static const secp256k1_fe secp256k1_const_beta = SECP256K1_FE_CONST( # define secp256k1_fe_set_b32_mod secp256k1_fe_impl_set_b32_mod # define secp256k1_fe_set_b32_limit secp256k1_fe_impl_set_b32_limit # define secp256k1_fe_get_b32 secp256k1_fe_impl_get_b32 -# define secp256k1_fe_negate secp256k1_fe_impl_negate -# define secp256k1_fe_mul_int secp256k1_fe_impl_mul_int +# define secp256k1_fe_negate_unchecked secp256k1_fe_impl_negate_unchecked +# define secp256k1_fe_mul_int_unchecked secp256k1_fe_impl_mul_int_unchecked # define secp256k1_fe_add secp256k1_fe_impl_add # define secp256k1_fe_mul secp256k1_fe_impl_mul # define secp256k1_fe_sqr secp256k1_fe_impl_sqr @@ -192,14 +192,14 @@ static int secp256k1_fe_cmp_var(const secp256k1_fe *a, const secp256k1_fe *b); /** Set a field element equal to a provided 32-byte big endian value, reducing it. * - * On input, r does not need to be initalized. a must be a pointer to an initialized 32-byte array. + * On input, r does not need to be initialized. a must be a pointer to an initialized 32-byte array. * On output, r = a (mod p). It will have magnitude 1, and not be normalized. */ static void secp256k1_fe_set_b32_mod(secp256k1_fe *r, const unsigned char *a); /** Set a field element equal to a provided 32-byte big endian value, checking for overflow. * - * On input, r does not need to be initalized. a must be a pointer to an initialized 32-byte array. + * On input, r does not need to be initialized. a must be a pointer to an initialized 32-byte array. * On output, r = a if (a < p), it will be normalized with magnitude 1, and 1 is returned. * If a >= p, 0 is returned, and r will be made invalid (and must not be used without overwriting). */ @@ -214,11 +214,17 @@ static void secp256k1_fe_get_b32(unsigned char *r, const secp256k1_fe *a); /** Negate a field element. * * On input, r does not need to be initialized. a must be a valid field element with - * magnitude not exceeding m. m must be an integer in [0,31]. + * magnitude not exceeding m. m must be an integer constant expression in [0,31]. * Performs {r = -a}. * On output, r will not be normalized, and will have magnitude m+1. */ -static void secp256k1_fe_negate(secp256k1_fe *r, const secp256k1_fe *a, int m); +#define secp256k1_fe_negate(r, a, m) ASSERT_INT_CONST_AND_DO(m, secp256k1_fe_negate_unchecked(r, a, m)) + +/** Like secp256k1_fe_negate_unchecked but m is not checked to be an integer constant expression. + * + * Should not be called directly outside of tests. + */ +static void secp256k1_fe_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m); /** Add a small integer to a field element. * @@ -229,12 +235,18 @@ static void secp256k1_fe_add_int(secp256k1_fe *r, int a); /** Multiply a field element with a small integer. * - * On input, r must be a valid field element. a must be an integer in [0,32]. + * On input, r must be a valid field element. a must be an integer constant expression in [0,32]. * The magnitude of r times a must not exceed 32. * Performs {r *= a}. * On output, r's magnitude is multiplied by a, and r will not be normalized. */ -static void secp256k1_fe_mul_int(secp256k1_fe *r, int a); +#define secp256k1_fe_mul_int(r, a) ASSERT_INT_CONST_AND_DO(a, secp256k1_fe_mul_int_unchecked(r, a)) + +/** Like secp256k1_fe_mul_int but a is not checked to be an integer constant expression. + * + * Should not be called directly outside of tests. + */ +static void secp256k1_fe_mul_int_unchecked(secp256k1_fe *r, int a); /** Increment a field element by another. * @@ -267,8 +279,10 @@ static void secp256k1_fe_sqr(secp256k1_fe *r, const secp256k1_fe *a); /** Compute a square root of a field element. * * On input, a must be a valid field element with magnitude<=8; r need not be initialized. - * Performs {r = sqrt(a)} or {r = sqrt(-a)}, whichever exists. The resulting value - * represented by r will be a square itself. Variables r and a must not point to the same object. + * If sqrt(a) exists, performs {r = sqrt(a)} and returns 1. + * Otherwise, sqrt(-a) exists. The function performs {r = sqrt(-a)} and returns 0. + * The resulting value represented by r will be a square itself. + * Variables r and a must not point to the same object. * On output, r will have magnitude 1 but will not be normalized. */ static int secp256k1_fe_sqrt(secp256k1_fe * SECP256K1_RESTRICT r, const secp256k1_fe * SECP256K1_RESTRICT a); diff --git a/src/secp256k1/src/field_10x26_impl.h b/src/secp256k1/src/field_10x26_impl.h index c1b32b80a8a9..8445db1639e2 100644 --- a/src/secp256k1/src/field_10x26_impl.h +++ b/src/secp256k1/src/field_10x26_impl.h @@ -344,7 +344,7 @@ static void secp256k1_fe_impl_get_b32(unsigned char *r, const secp256k1_fe *a) { r[31] = a->n[0] & 0xff; } -SECP256K1_INLINE static void secp256k1_fe_impl_negate(secp256k1_fe *r, const secp256k1_fe *a, int m) { +SECP256K1_INLINE static void secp256k1_fe_impl_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m) { /* For all legal values of m (0..31), the following properties hold: */ VERIFY_CHECK(0x3FFFC2FUL * 2 * (m + 1) >= 0x3FFFFFFUL * 2 * m); VERIFY_CHECK(0x3FFFFBFUL * 2 * (m + 1) >= 0x3FFFFFFUL * 2 * m); @@ -365,7 +365,7 @@ SECP256K1_INLINE static void secp256k1_fe_impl_negate(secp256k1_fe *r, const sec r->n[9] = 0x03FFFFFUL * 2 * (m + 1) - a->n[9]; } -SECP256K1_INLINE static void secp256k1_fe_impl_mul_int(secp256k1_fe *r, int a) { +SECP256K1_INLINE static void secp256k1_fe_impl_mul_int_unchecked(secp256k1_fe *r, int a) { r->n[0] *= a; r->n[1] *= a; r->n[2] *= a; diff --git a/src/secp256k1/src/field_5x52_impl.h b/src/secp256k1/src/field_5x52_impl.h index 0a4cc1a630a5..ecb70502c2be 100644 --- a/src/secp256k1/src/field_5x52_impl.h +++ b/src/secp256k1/src/field_5x52_impl.h @@ -314,7 +314,7 @@ static void secp256k1_fe_impl_get_b32(unsigned char *r, const secp256k1_fe *a) { r[31] = a->n[0] & 0xFF; } -SECP256K1_INLINE static void secp256k1_fe_impl_negate(secp256k1_fe *r, const secp256k1_fe *a, int m) { +SECP256K1_INLINE static void secp256k1_fe_impl_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m) { /* For all legal values of m (0..31), the following properties hold: */ VERIFY_CHECK(0xFFFFEFFFFFC2FULL * 2 * (m + 1) >= 0xFFFFFFFFFFFFFULL * 2 * m); VERIFY_CHECK(0xFFFFFFFFFFFFFULL * 2 * (m + 1) >= 0xFFFFFFFFFFFFFULL * 2 * m); @@ -329,7 +329,7 @@ SECP256K1_INLINE static void secp256k1_fe_impl_negate(secp256k1_fe *r, const sec r->n[4] = 0x0FFFFFFFFFFFFULL * 2 * (m + 1) - a->n[4]; } -SECP256K1_INLINE static void secp256k1_fe_impl_mul_int(secp256k1_fe *r, int a) { +SECP256K1_INLINE static void secp256k1_fe_impl_mul_int_unchecked(secp256k1_fe *r, int a) { r->n[0] *= a; r->n[1] *= a; r->n[2] *= a; diff --git a/src/secp256k1/src/field_impl.h b/src/secp256k1/src/field_impl.h index f9769a4a390a..7f18ebdc94e0 100644 --- a/src/secp256k1/src/field_impl.h +++ b/src/secp256k1/src/field_impl.h @@ -44,7 +44,7 @@ SECP256K1_INLINE static int secp256k1_fe_equal_var(const secp256k1_fe *a, const return secp256k1_fe_normalizes_to_zero_var(&na); } -static int secp256k1_fe_sqrt(secp256k1_fe *r, const secp256k1_fe *a) { +static int secp256k1_fe_sqrt(secp256k1_fe * SECP256K1_RESTRICT r, const secp256k1_fe * SECP256K1_RESTRICT a) { /** Given that p is congruent to 3 mod 4, we can compute the square root of * a mod p as the (p+1)/4'th power of a. * @@ -289,23 +289,23 @@ SECP256K1_INLINE static void secp256k1_fe_get_b32(unsigned char *r, const secp25 secp256k1_fe_impl_get_b32(r, a); } -static void secp256k1_fe_impl_negate(secp256k1_fe *r, const secp256k1_fe *a, int m); -SECP256K1_INLINE static void secp256k1_fe_negate(secp256k1_fe *r, const secp256k1_fe *a, int m) { +static void secp256k1_fe_impl_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m); +SECP256K1_INLINE static void secp256k1_fe_negate_unchecked(secp256k1_fe *r, const secp256k1_fe *a, int m) { secp256k1_fe_verify(a); VERIFY_CHECK(m >= 0 && m <= 31); VERIFY_CHECK(a->magnitude <= m); - secp256k1_fe_impl_negate(r, a, m); + secp256k1_fe_impl_negate_unchecked(r, a, m); r->magnitude = m + 1; r->normalized = 0; secp256k1_fe_verify(r); } -static void secp256k1_fe_impl_mul_int(secp256k1_fe *r, int a); -SECP256K1_INLINE static void secp256k1_fe_mul_int(secp256k1_fe *r, int a) { +static void secp256k1_fe_impl_mul_int_unchecked(secp256k1_fe *r, int a); +SECP256K1_INLINE static void secp256k1_fe_mul_int_unchecked(secp256k1_fe *r, int a) { secp256k1_fe_verify(r); VERIFY_CHECK(a >= 0 && a <= 32); VERIFY_CHECK(a*r->magnitude <= 32); - secp256k1_fe_impl_mul_int(r, a); + secp256k1_fe_impl_mul_int_unchecked(r, a); r->magnitude *= a; r->normalized = 0; secp256k1_fe_verify(r); diff --git a/src/secp256k1/src/group.h b/src/secp256k1/src/group.h index 877c3eaeeda0..1cc61371615a 100644 --- a/src/secp256k1/src/group.h +++ b/src/secp256k1/src/group.h @@ -106,7 +106,8 @@ static void secp256k1_gej_set_ge(secp256k1_gej *r, const secp256k1_ge *a); /** Check two group elements (jacobian) for equality in variable time. */ static int secp256k1_gej_eq_var(const secp256k1_gej *a, const secp256k1_gej *b); -/** Compare the X coordinate of a group element (jacobian). */ +/** Compare the X coordinate of a group element (jacobian). + * The magnitude of the group element's X coordinate must not exceed 31. */ static int secp256k1_gej_eq_x_var(const secp256k1_fe *x, const secp256k1_gej *a); /** Set r equal to the inverse of a (i.e., mirrored around the X axis) */ diff --git a/src/secp256k1/src/group_impl.h b/src/secp256k1/src/group_impl.h index dcd171f574e5..ffdfeaa10a98 100644 --- a/src/secp256k1/src/group_impl.h +++ b/src/secp256k1/src/group_impl.h @@ -314,13 +314,17 @@ static int secp256k1_gej_eq_var(const secp256k1_gej *a, const secp256k1_gej *b) } static int secp256k1_gej_eq_x_var(const secp256k1_fe *x, const secp256k1_gej *a) { - secp256k1_fe r, r2; + secp256k1_fe r; + +#ifdef VERIFY secp256k1_fe_verify(x); + VERIFY_CHECK(a->x.magnitude <= 31); secp256k1_gej_verify(a); VERIFY_CHECK(!a->infinity); +#endif + secp256k1_fe_sqr(&r, &a->z); secp256k1_fe_mul(&r, &r, x); - r2 = a->x; secp256k1_fe_normalize_weak(&r2); - return secp256k1_fe_equal_var(&r, &r2); + return secp256k1_fe_equal_var(&r, &a->x); } static void secp256k1_gej_neg(secp256k1_gej *r, const secp256k1_gej *a) { @@ -349,7 +353,6 @@ static int secp256k1_ge_is_valid_var(const secp256k1_ge *a) { secp256k1_fe_sqr(&y2, &a->y); secp256k1_fe_sqr(&x3, &a->x); secp256k1_fe_mul(&x3, &x3, &a->x); secp256k1_fe_add_int(&x3, SECP256K1_B); - secp256k1_fe_normalize_weak(&x3); return secp256k1_fe_equal_var(&y2, &x3); } diff --git a/src/secp256k1/src/int128_struct_impl.h b/src/secp256k1/src/int128_struct_impl.h index 990982da8431..962a71d13bc3 100644 --- a/src/secp256k1/src/int128_struct_impl.h +++ b/src/secp256k1/src/int128_struct_impl.h @@ -80,7 +80,12 @@ static SECP256K1_INLINE void secp256k1_u128_rshift(secp256k1_uint128 *r, unsigne r->lo = r->hi >> (n-64); r->hi = 0; } else if (n > 0) { +#if defined(_MSC_VER) && defined(_M_X64) + VERIFY_CHECK(n < 64); + r->lo = __shiftright128(r->lo, r->hi, n); +#else r->lo = ((1U * r->hi) << (64-n)) | r->lo >> n; +#endif r->hi >>= n; } } diff --git a/src/secp256k1/src/modules/ellswift/Makefile.am.include b/src/secp256k1/src/modules/ellswift/Makefile.am.include index e7efea29819b..8251231ea3cd 100644 --- a/src/secp256k1/src/modules/ellswift/Makefile.am.include +++ b/src/secp256k1/src/modules/ellswift/Makefile.am.include @@ -2,3 +2,4 @@ include_HEADERS += include/secp256k1_ellswift.h noinst_HEADERS += src/modules/ellswift/bench_impl.h noinst_HEADERS += src/modules/ellswift/main_impl.h noinst_HEADERS += src/modules/ellswift/tests_impl.h +noinst_HEADERS += src/modules/ellswift/tests_exhaustive_impl.h diff --git a/src/secp256k1/src/modules/ellswift/tests_exhaustive_impl.h b/src/secp256k1/src/modules/ellswift/tests_exhaustive_impl.h new file mode 100644 index 000000000000..e002a8c00843 --- /dev/null +++ b/src/secp256k1/src/modules/ellswift/tests_exhaustive_impl.h @@ -0,0 +1,39 @@ +/*********************************************************************** + * Distributed under the MIT software license, see the accompanying * + * file COPYING or https://www.opensource.org/licenses/mit-license.php.* + ***********************************************************************/ + +#ifndef SECP256K1_MODULE_ELLSWIFT_TESTS_EXHAUSTIVE_H +#define SECP256K1_MODULE_ELLSWIFT_TESTS_EXHAUSTIVE_H + +#include "../../../include/secp256k1_ellswift.h" +#include "main_impl.h" + +static void test_exhaustive_ellswift(const secp256k1_context *ctx, const secp256k1_ge *group) { + int i; + + /* Note that SwiftEC/ElligatorSwift are inherently curve operations, not + * group operations, and this test only checks the curve points which are in + * a tiny subgroup. In that sense it can't be really seen as exhaustive as + * it doesn't (and for computational reasons obviously cannot) test the + * entire domain ellswift operates under. */ + for (i = 1; i < EXHAUSTIVE_TEST_ORDER; i++) { + secp256k1_scalar scalar_i; + unsigned char sec32[32]; + unsigned char ell64[64]; + secp256k1_pubkey pub_decoded; + secp256k1_ge ge_decoded; + + /* Construct ellswift pubkey from exhaustive loop scalar i. */ + secp256k1_scalar_set_int(&scalar_i, i); + secp256k1_scalar_get_b32(sec32, &scalar_i); + CHECK(secp256k1_ellswift_create(ctx, ell64, sec32, NULL)); + + /* Decode ellswift pubkey and check that it matches the precomputed group element. */ + secp256k1_ellswift_decode(ctx, &pub_decoded, ell64); + secp256k1_pubkey_load(ctx, &ge_decoded, &pub_decoded); + ge_equals_ge(&ge_decoded, &group[i]); + } +} + +#endif diff --git a/src/secp256k1/src/modules/ellswift/tests_impl.h b/src/secp256k1/src/modules/ellswift/tests_impl.h index 86ca09862b1b..47f443d98045 100644 --- a/src/secp256k1/src/modules/ellswift/tests_impl.h +++ b/src/secp256k1/src/modules/ellswift/tests_impl.h @@ -322,7 +322,9 @@ void run_ellswift_tests(void) { secp256k1_testrand256_test(auxrnd32a); secp256k1_testrand256_test(auxrnd32b); random_scalar_order_test(&seca); - random_scalar_order_test(&secb); + /* Draw secb uniformly at random to make sure that the secret keys + * differ */ + random_scalar_order(&secb); secp256k1_scalar_get_b32(sec32a, &seca); secp256k1_scalar_get_b32(sec32b, &secb); diff --git a/src/secp256k1/src/precompute_ecmult.c b/src/secp256k1/src/precompute_ecmult.c index 742142cf58cf..5ef198a77006 100644 --- a/src/secp256k1/src/precompute_ecmult.c +++ b/src/secp256k1/src/precompute_ecmult.c @@ -56,11 +56,12 @@ static void print_two_tables(FILE *fp, int window_g) { int main(void) { /* Always compute all tables for window sizes up to 15. */ int window_g = (ECMULT_WINDOW_SIZE < 15) ? 15 : ECMULT_WINDOW_SIZE; + const char outfile[] = "src/precomputed_ecmult.c"; FILE* fp; - fp = fopen("src/precomputed_ecmult.c","w"); + fp = fopen(outfile, "w"); if (fp == NULL) { - fprintf(stderr, "Could not open src/precomputed_ecmult.h for writing!\n"); + fprintf(stderr, "Could not open %s for writing!\n", outfile); return -1; } diff --git a/src/secp256k1/src/testrand_impl.h b/src/secp256k1/src/testrand_impl.h index 1b7481a53b8a..fe9762043556 100644 --- a/src/secp256k1/src/testrand_impl.h +++ b/src/secp256k1/src/testrand_impl.h @@ -16,8 +16,6 @@ #include "util.h" static uint64_t secp256k1_test_state[4]; -static uint64_t secp256k1_test_rng_integer; -static int secp256k1_test_rng_integer_bits_left = 0; SECP256K1_INLINE static void secp256k1_testrand_seed(const unsigned char *seed16) { static const unsigned char PREFIX[19] = "secp256k1 test init"; @@ -36,7 +34,6 @@ SECP256K1_INLINE static void secp256k1_testrand_seed(const unsigned char *seed16 for (j = 0; j < 8; ++j) s = (s << 8) | out32[8*i + j]; secp256k1_test_state[i] = s; } - secp256k1_test_rng_integer_bits_left = 0; } SECP256K1_INLINE static uint64_t rotl(const uint64_t x, int k) { @@ -57,58 +54,30 @@ SECP256K1_INLINE static uint64_t secp256k1_testrand64(void) { } SECP256K1_INLINE static uint64_t secp256k1_testrand_bits(int bits) { - uint64_t ret; - if (secp256k1_test_rng_integer_bits_left < bits) { - secp256k1_test_rng_integer = secp256k1_testrand64(); - secp256k1_test_rng_integer_bits_left = 64; - } - ret = secp256k1_test_rng_integer; - secp256k1_test_rng_integer >>= bits; - secp256k1_test_rng_integer_bits_left -= bits; - ret &= ((~((uint64_t)0)) >> (64 - bits)); - return ret; + if (bits == 0) return 0; + return secp256k1_testrand64() >> (64 - bits); } SECP256K1_INLINE static uint32_t secp256k1_testrand32(void) { - return secp256k1_testrand_bits(32); + return secp256k1_testrand64() >> 32; } static uint32_t secp256k1_testrand_int(uint32_t range) { - /* We want a uniform integer between 0 and range-1, inclusive. - * B is the smallest number such that range <= 2**B. - * two mechanisms implemented here: - * - generate B bits numbers until one below range is found, and return it - * - find the largest multiple M of range that is <= 2**(B+A), generate B+A - * bits numbers until one below M is found, and return it modulo range - * The second mechanism consumes A more bits of entropy in every iteration, - * but may need fewer iterations due to M being closer to 2**(B+A) then - * range is to 2**B. The array below (indexed by B) contains a 0 when the - * first mechanism is to be used, and the number A otherwise. - */ - static const int addbits[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 2, 2, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 2, 1, 0}; - uint32_t trange, mult; - int bits = 0; - if (range <= 1) { - return 0; - } - trange = range - 1; - while (trange > 0) { - trange >>= 1; - bits++; + uint32_t mask = 0; + uint32_t range_copy; + /* Reduce range by 1, changing its meaning to "maximum value". */ + VERIFY_CHECK(range != 0); + range -= 1; + /* Count the number of bits in range. */ + range_copy = range; + while (range_copy) { + mask = (mask << 1) | 1U; + range_copy >>= 1; } - if (addbits[bits]) { - bits = bits + addbits[bits]; - mult = ((~((uint32_t)0)) >> (32 - bits)) / range; - trange = range * mult; - } else { - trange = range; - mult = 1; - } - while(1) { - uint32_t x = secp256k1_testrand_bits(bits); - if (x < trange) { - return (mult == 1) ? x : (x % range); - } + /* Generation loop. */ + while (1) { + uint32_t val = secp256k1_testrand64() & mask; + if (val <= range) return val; } } diff --git a/src/secp256k1/src/tests.c b/src/secp256k1/src/tests.c index 8ada3f869b80..920d31b48111 100644 --- a/src/secp256k1/src/tests.c +++ b/src/secp256k1/src/tests.c @@ -89,16 +89,6 @@ static void uncounting_illegal_callback_fn(const char* str, void* data) { (*p)--; } -static void random_field_element_test(secp256k1_fe *fe) { - do { - unsigned char b32[32]; - secp256k1_testrand256_test(b32); - if (secp256k1_fe_set_b32_limit(fe, b32)) { - break; - } - } while(1); -} - static void random_field_element_magnitude(secp256k1_fe *fe) { secp256k1_fe zero; int n = secp256k1_testrand_int(9); @@ -108,17 +98,33 @@ static void random_field_element_magnitude(secp256k1_fe *fe) { } secp256k1_fe_clear(&zero); secp256k1_fe_negate(&zero, &zero, 0); - secp256k1_fe_mul_int(&zero, n - 1); + secp256k1_fe_mul_int_unchecked(&zero, n - 1); secp256k1_fe_add(fe, &zero); #ifdef VERIFY CHECK(fe->magnitude == n); #endif } +static void random_fe_test(secp256k1_fe *x) { + unsigned char bin[32]; + do { + secp256k1_testrand256_test(bin); + if (secp256k1_fe_set_b32_limit(x, bin)) { + return; + } + } while(1); +} + +static void random_fe_non_zero_test(secp256k1_fe *fe) { + do { + random_fe_test(fe); + } while(secp256k1_fe_is_zero(fe)); +} + static void random_group_element_test(secp256k1_ge *ge) { secp256k1_fe fe; do { - random_field_element_test(&fe); + random_fe_test(&fe); if (secp256k1_ge_set_xo_var(ge, &fe, secp256k1_testrand_bits(1))) { secp256k1_fe_normalize(&ge->y); break; @@ -129,12 +135,7 @@ static void random_group_element_test(secp256k1_ge *ge) { static void random_group_element_jacobian_test(secp256k1_gej *gej, const secp256k1_ge *ge) { secp256k1_fe z2, z3; - do { - random_field_element_test(&gej->z); - if (!secp256k1_fe_is_zero(&gej->z)) { - break; - } - } while(1); + random_fe_non_zero_test(&gej->z); secp256k1_fe_sqr(&z2, &gej->z); secp256k1_fe_mul(&z3, &z2, &gej->z); secp256k1_fe_mul(&gej->x, &ge->x, &z2); @@ -180,6 +181,35 @@ static void random_scalar_order_b32(unsigned char *b32) { secp256k1_scalar_get_b32(b32, &num); } +static void run_xoshiro256pp_tests(void) { + { + size_t i; + /* Sanity check that we run before the actual seeding. */ + for (i = 0; i < sizeof(secp256k1_test_state)/sizeof(secp256k1_test_state[0]); i++) { + CHECK(secp256k1_test_state[i] == 0); + } + } + { + int i; + unsigned char buf32[32]; + unsigned char seed16[16] = { + 'C', 'H', 'I', 'C', 'K', 'E', 'N', '!', + 'C', 'H', 'I', 'C', 'K', 'E', 'N', '!', + }; + unsigned char buf32_expected[32] = { + 0xAF, 0xCC, 0xA9, 0x16, 0xB5, 0x6C, 0xE3, 0xF0, + 0x44, 0x3F, 0x45, 0xE0, 0x47, 0xA5, 0x08, 0x36, + 0x4C, 0xCC, 0xC1, 0x18, 0xB2, 0xD8, 0x8F, 0xEF, + 0x43, 0x26, 0x15, 0x57, 0x37, 0x00, 0xEF, 0x30, + }; + secp256k1_testrand_seed(seed16); + for (i = 0; i < 17; i++) { + secp256k1_testrand256(buf32); + } + CHECK(secp256k1_memcmp_var(buf32, buf32_expected, sizeof(buf32)) == 0); + } +} + static void run_selftest_tests(void) { /* Test public API */ secp256k1_selftest(); @@ -823,78 +853,6 @@ static void run_tagged_sha256_tests(void) { CHECK(secp256k1_memcmp_var(hash32, hash_expected, sizeof(hash32)) == 0); } -/***** RANDOM TESTS *****/ - -static void test_rand_bits(int rand32, int bits) { - /* (1-1/2^B)^rounds[B] < 1/10^9, so rounds is the number of iterations to - * get a false negative chance below once in a billion */ - static const unsigned int rounds[7] = {1, 30, 73, 156, 322, 653, 1316}; - /* We try multiplying the results with various odd numbers, which shouldn't - * influence the uniform distribution modulo a power of 2. */ - static const uint32_t mults[6] = {1, 3, 21, 289, 0x9999, 0x80402011}; - /* We only select up to 6 bits from the output to analyse */ - unsigned int usebits = bits > 6 ? 6 : bits; - unsigned int maxshift = bits - usebits; - /* For each of the maxshift+1 usebits-bit sequences inside a bits-bit - number, track all observed outcomes, one per bit in a uint64_t. */ - uint64_t x[6][27] = {{0}}; - unsigned int i, shift, m; - /* Multiply the output of all rand calls with the odd number m, which - should not change the uniformity of its distribution. */ - for (i = 0; i < rounds[usebits]; i++) { - uint32_t r = (rand32 ? secp256k1_testrand32() : secp256k1_testrand_bits(bits)); - CHECK((((uint64_t)r) >> bits) == 0); - for (m = 0; m < sizeof(mults) / sizeof(mults[0]); m++) { - uint32_t rm = r * mults[m]; - for (shift = 0; shift <= maxshift; shift++) { - x[m][shift] |= (((uint64_t)1) << ((rm >> shift) & ((1 << usebits) - 1))); - } - } - } - for (m = 0; m < sizeof(mults) / sizeof(mults[0]); m++) { - for (shift = 0; shift <= maxshift; shift++) { - /* Test that the lower usebits bits of x[shift] are 1 */ - CHECK(((~x[m][shift]) << (64 - (1 << usebits))) == 0); - } - } -} - -/* Subrange must be a whole divisor of range, and at most 64 */ -static void test_rand_int(uint32_t range, uint32_t subrange) { - /* (1-1/subrange)^rounds < 1/10^9 */ - int rounds = (subrange * 2073) / 100; - int i; - uint64_t x = 0; - CHECK((range % subrange) == 0); - for (i = 0; i < rounds; i++) { - uint32_t r = secp256k1_testrand_int(range); - CHECK(r < range); - r = r % subrange; - x |= (((uint64_t)1) << r); - } - /* Test that the lower subrange bits of x are 1. */ - CHECK(((~x) << (64 - subrange)) == 0); -} - -static void run_rand_bits(void) { - size_t b; - test_rand_bits(1, 32); - for (b = 1; b <= 32; b++) { - test_rand_bits(0, b); - } -} - -static void run_rand_int(void) { - static const uint32_t ms[] = {1, 3, 17, 1000, 13771, 999999, 33554432}; - static const uint32_t ss[] = {1, 3, 6, 9, 13, 31, 64}; - unsigned int m, s; - for (m = 0; m < sizeof(ms) / sizeof(ms[0]); m++) { - for (s = 0; s < sizeof(ss) / sizeof(ss[0]); s++) { - test_rand_int(ms[m] * ss[s], ss[s]); - } - } -} - /***** MODINV TESTS *****/ /* Compute the modular inverse of (odd) x mod 2^64. */ @@ -2984,16 +2942,6 @@ static void random_fe(secp256k1_fe *x) { } while(1); } -static void random_fe_test(secp256k1_fe *x) { - unsigned char bin[32]; - do { - secp256k1_testrand256_test(bin); - if (secp256k1_fe_set_b32_limit(x, bin)) { - return; - } - } while(1); -} - static void random_fe_non_zero(secp256k1_fe *nz) { int tries = 10; while (--tries >= 0) { @@ -3234,7 +3182,7 @@ static void run_field_misc(void) { CHECK(q.normalized && q.magnitude == 1); #endif for (j = 0; j < 6; j++) { - secp256k1_fe_negate(&z, &z, j+1); + secp256k1_fe_negate_unchecked(&z, &z, j+1); secp256k1_fe_normalize_var(&q); secp256k1_fe_cmov(&q, &z, (j&1)); #ifdef VERIFY @@ -3820,18 +3768,14 @@ static void test_ge(void) { } /* Generate random zf, and zfi2 = 1/zf^2, zfi3 = 1/zf^3 */ - do { - random_field_element_test(&zf); - } while(secp256k1_fe_is_zero(&zf)); + random_fe_non_zero_test(&zf); random_field_element_magnitude(&zf); secp256k1_fe_inv_var(&zfi3, &zf); secp256k1_fe_sqr(&zfi2, &zfi3); secp256k1_fe_mul(&zfi3, &zfi3, &zfi2); /* Generate random r */ - do { - random_field_element_test(&r); - } while(secp256k1_fe_is_zero(&r)); + random_fe_non_zero_test(&r); for (i1 = 0; i1 < 1 + 4 * runs; i1++) { int i2; @@ -4048,22 +3992,15 @@ static void test_add_neg_y_diff_x(void) { * which this test is a regression test for. * * These points were generated in sage as - * # secp256k1 params - * F = FiniteField (0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F) - * C = EllipticCurve ([F (0), F (7)]) - * G = C.lift_x(0x79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798) - * N = FiniteField(G.order()) * - * # endomorphism values (lambda is 1^{1/3} in N, beta is 1^{1/3} in F) - * x = polygen(N) - * lam = (1 - x^3).roots()[1][0] + * load("secp256k1_params.sage") * * # random "bad pair" * P = C.random_element() - * Q = -int(lam) * P - * print " P: %x %x" % P.xy() - * print " Q: %x %x" % Q.xy() - * print "P + Q: %x %x" % (P + Q).xy() + * Q = -int(LAMBDA) * P + * print(" P: %x %x" % P.xy()) + * print(" Q: %x %x" % Q.xy()) + * print("P + Q: %x %x" % (P + Q).xy()) */ secp256k1_gej aj = SECP256K1_GEJ_CONST( 0x8d24cd95, 0x0a355af1, 0x3c543505, 0x44238d30, @@ -4148,10 +4085,7 @@ static void run_gej(void) { CHECK(!secp256k1_gej_eq_var(&a, &b)); b = a; - random_field_element_test(&fe); - if (secp256k1_fe_is_zero(&fe)) { - continue; - } + random_fe_non_zero_test(&fe); secp256k1_gej_rescale(&a, &fe); CHECK(secp256k1_gej_eq_var(&a, &b)); } @@ -4590,9 +4524,7 @@ static void ecmult_const_mult_xonly(void) { random_scalar_order_test(&q); /* If i is odd, n=d*base.x for random non-zero d */ if (i & 1) { - do { - random_field_element_test(&d); - } while (secp256k1_fe_normalizes_to_zero_var(&d)); + random_fe_non_zero_test(&d); secp256k1_fe_mul(&n, &base.x, &d); } else { n = base.x; @@ -4611,22 +4543,17 @@ static void ecmult_const_mult_xonly(void) { /* Test that secp256k1_ecmult_const_xonly correctly rejects X coordinates not on curve. */ for (i = 0; i < 2*COUNT; ++i) { - secp256k1_fe x, n, d, c, r; + secp256k1_fe x, n, d, r; int res; secp256k1_scalar q; random_scalar_order_test(&q); /* Generate random X coordinate not on the curve. */ do { - random_field_element_test(&x); - secp256k1_fe_sqr(&c, &x); - secp256k1_fe_mul(&c, &c, &x); - secp256k1_fe_add_int(&c, SECP256K1_B); - } while (secp256k1_fe_is_square_var(&c)); + random_fe_test(&x); + } while (secp256k1_ge_x_on_curve_var(&x)); /* If i is odd, n=d*x for random non-zero d. */ if (i & 1) { - do { - random_field_element_test(&d); - } while (secp256k1_fe_normalizes_to_zero_var(&d)); + random_fe_non_zero_test(&d); secp256k1_fe_mul(&n, &x, &d); } else { n = x; @@ -7765,6 +7692,9 @@ int main(int argc, char **argv) { } printf("test count = %i\n", COUNT); + /* run test RNG tests (must run before we really initialize the test RNG) */ + run_xoshiro256pp_tests(); + /* find random seed */ secp256k1_testrand_init(argc > 2 ? argv[2] : NULL); @@ -7802,10 +7732,6 @@ int main(int argc, char **argv) { /* scratch tests */ run_scratch_tests(); - /* randomness tests */ - run_rand_bits(); - run_rand_int(); - /* integer arithmetic tests */ #ifdef SECP256K1_WIDEMUL_INT128 run_int128_tests(); diff --git a/src/secp256k1/src/tests_exhaustive.c b/src/secp256k1/src/tests_exhaustive.c index d35acdd58e89..dbb6b7eb46df 100644 --- a/src/secp256k1/src/tests_exhaustive.c +++ b/src/secp256k1/src/tests_exhaustive.c @@ -13,6 +13,9 @@ #define EXHAUSTIVE_TEST_ORDER 13 #endif +/* These values of B are all values in [1, 8] that result in a curve with even order. */ +#define EXHAUSTIVE_TEST_CURVE_HAS_EVEN_ORDER (SECP256K1_B == 1 || SECP256K1_B == 6 || SECP256K1_B == 8) + #ifdef USE_EXTERNAL_DEFAULT_CALLBACKS #pragma message("Ignoring USE_EXTERNAL_CALLBACKS in exhaustive_tests.") #undef USE_EXTERNAL_DEFAULT_CALLBACKS @@ -395,6 +398,10 @@ static void test_exhaustive_sign(const secp256k1_context *ctx, const secp256k1_g #include "modules/schnorrsig/tests_exhaustive_impl.h" #endif +#ifdef ENABLE_MODULE_ELLSWIFT +#include "modules/ellswift/tests_exhaustive_impl.h" +#endif + int main(int argc, char** argv) { int i; secp256k1_gej groupj[EXHAUSTIVE_TEST_ORDER]; @@ -490,6 +497,15 @@ int main(int argc, char** argv) { #ifdef ENABLE_MODULE_SCHNORRSIG test_exhaustive_schnorrsig(ctx); #endif +#ifdef ENABLE_MODULE_ELLSWIFT + /* The ellswift algorithm does have additional edge cases when operating on + * curves of even order, which are not included in the code as secp256k1 is + * of odd order. Skip the ellswift tests if the used exhaustive tests curve + * is even-ordered accordingly. */ + #if !EXHAUSTIVE_TEST_CURVE_HAS_EVEN_ORDER + test_exhaustive_ellswift(ctx, group); + #endif +#endif secp256k1_context_destroy(ctx); } diff --git a/src/secp256k1/src/util.h b/src/secp256k1/src/util.h index e2ee8a8f190c..801ea0c885ed 100644 --- a/src/secp256k1/src/util.h +++ b/src/secp256k1/src/util.h @@ -51,6 +51,19 @@ static void print_buf_plain(const unsigned char *buf, size_t len) { # define SECP256K1_INLINE inline # endif +/** Assert statically that expr is an integer constant expression, and run stmt. + * + * Useful for example to enforce that magnitude arguments are constant. + */ +#define ASSERT_INT_CONST_AND_DO(expr, stmt) do { \ + switch(42) { \ + case /* ERROR: integer argument is not constant */ expr: \ + break; \ + default: ; \ + } \ + stmt; \ +} while(0) + typedef struct { void (*fn)(const char *text, void* data); const void* data; From 3fe418b9eea51be8531a09dc61d76c9b60f481ef Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 3 May 2024 09:44:00 -0400 Subject: [PATCH 7/9] Merge bitcoin/bitcoin#30026: refactor, test: Always initialize pointer bd2de7ac591d7704b79304089ad1fb57e085da8b refactor, test: Always initialize pointer (Hennadii Stepanov) Pull request description: This change fixes MSVC warning [C4703](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4703). All `DisableSpecificWarnings` dropped from `test_bitcoin.vcxproj` as all remained are inherited from `common.init.vcxproj`. Required to simplify warning suppression porting to the CMake-based build system. ACKs for top commit: maflcko: utACK bd2de7ac591d7704b79304089ad1fb57e085da8b sipsorcery: utACK bd2de7ac591d7704b79304089ad1fb57e085da8b. ryanofsky: Code review ACK bd2de7ac591d7704b79304089ad1fb57e085da8b Tree-SHA512: 006db041d3c3697a77d9df14de86cf7c8a10804b45789df01268b2236cf6452e77dc57e89f5d5a6bc26d4b5cd483f0722d6035649c8a523b57954bb1fc810d0c --- src/test/validation_chainstate_tests.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 73920bda6743..3d7d9e50e368 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -96,14 +97,14 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) BOOST_CHECK_EQUAL(chainman.GetAll().size(), 2); - CChainState& background_cs{*[&] { + CChainState& background_cs{*Assert([&]() -> CChainState* { for (CChainState* cs : chainman.GetAll()) { if (cs != &chainman.ActiveChainstate()) { return cs; } } - assert(false); - }()}; + return nullptr; + }())}; // Create a block to append to the validation chain. std::vector noTxns; From 06ef315c02a8e435f2a77bced9ca063f37251adc Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 31 May 2023 11:12:06 +0100 Subject: [PATCH 8/9] Merge bitcoin/bitcoin#27780: fuzz: Avoid timeout in utxo_total_supply fafb4da121b19ba1b7bd173e25651c64d1982fb4 fuzz: Avoid timeout in utxo_total_supply (MarcoFalke) Pull request description: Looks like for high block counts it may be better to mock the chain, otherwise a high limit will lead to fuzz input bloat and timeouts, see https://github.com/bitcoin/bitcoin/pull/17860#issuecomment-1538252773. It can be checked that the fuzz target can still find the CVE, see https://github.com/bitcoin/bitcoin/pull/17860#pullrequestreview-1410594057 with a diff of: ```diff diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index f949655909..6f4cfb5f51 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -39,8 +39,6 @@ bool CheckTransaction(const CTransaction& tx, TxValidationState& state) // the underlying coins database. std::set vInOutPoints; for (const auto& txin : tx.vin) { - if (!vInOutPoints.insert(txin.prevout).second) - return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-inputs-duplicate"); } if (tx.IsCoinBase()) ``` Also, fix a nit, see https://github.com/bitcoin/bitcoin/pull/17860#discussion_r1186451948 ACKs for top commit: dergoegge: ACK fafb4da121b19ba1b7bd173e25651c64d1982fb4 Tree-SHA512: a28fe9cd6ebb4c9bed5a5b35be76c1c436a87586c8fc3b3c4c8559a4a77ac08098324370da421d794c99579882c0872b6b29415de47ade6a05a08504a3d494c4 --- src/test/fuzz/utxo_total_supply.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index ef3927c7b0a7..cd5f23b8104b 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -122,7 +122,9 @@ FUZZ_TARGET(utxo_total_supply) current_block = PrepareNextBlock(); StoreLastTxo(); - LIMITED_WHILE(fuzzed_data_provider.remaining_bytes(), 100'000) + // Limit to avoid timeout, but enough to cover duplicate_coinbase_height + // and CVE-2018-17144. + LIMITED_WHILE(fuzzed_data_provider.remaining_bytes(), 2'000) { CallOneOf( fuzzed_data_provider, @@ -145,14 +147,14 @@ FUZZ_TARGET(utxo_total_supply) current_block->hashMerkleRoot = BlockMerkleRoot(*current_block); const bool was_valid = !MineBlock(node, current_block).IsNull(); + if (duplicate_coinbase_height == ActiveHeight()) { + // we mined the duplicate coinbase + assert(current_block->vtx.at(0)->vin.at(0).scriptSig == duplicate_coinbase_script); + } + const auto prev_utxo_stats = utxo_stats; if (was_valid) { circulation += GetBlockSubsidy(ActiveTip(), Params().GetConsensus()); - - if (duplicate_coinbase_height == ActiveHeight()) { - // we mined the duplicate coinbase - assert(current_block->vtx.at(0)->vin.at(0).scriptSig == duplicate_coinbase_script); - } } UpdateUtxoStats(); From 8bf7f8dd5ab1275f888b6e7c28689cec5b9125ae Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 7 Nov 2023 11:16:48 +0000 Subject: [PATCH 9/9] Merge bitcoin/bitcoin#28789: fuzz: Avoid utxo_total_supply timeout (take 2) fa7ba926300f44b350d04262e569cc607e4991d8 fuzz: Avoid utxo_total_supply timeout (MarcoFalke) Pull request description: Looks like this still may take a long time to run large fuzz inputs. Thus, reduce it further, but still allow it to catch the regression, if re-introduced: ```diff diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp index f949655909..4bdd15c5ee 100644 --- a/src/consensus/tx_check.cpp +++ b/src/consensus/tx_check.cpp @@ -40,7 +40,7 @@ bool CheckTransaction(const CTransaction& tx, TxValidationState& state) std::set vInOutPoints; for (const auto& txin : tx.vin) { if (!vInOutPoints.insert(txin.prevout).second) - return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-inputs-duplicate"); + {}//return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-inputs-duplicate"); } if (tx.IsCoinBase()) ``` This is the second take, see https://github.com/bitcoin/bitcoin/pull/27780. If in the future it still times out, I think the fuzz test can just be removed. Example input: ``` JREROy5pcnAgQyw7IC4ODg4ODg4ODg4O0dEODg4ODg4ZDg4ODg4ODg4ODg7RDg4ODg4ODg4O0dEODg4ODg4ODg4ODg7R0Q4ODg4ODg4ODtHRDg4ODtHR0dEODg4O0dEODg7R0Q4ODg4ODg4ODtHRDg4ODg4ODg4ODg4O0dEODg4ODg4ODg7R0Q4ODg7R0Q4O0dEODg4ODg4ODg4ODg7R0Q4ODg4ODtHRDg4ODtHR ACKs for top commit: dergoegge: ACK fa7ba926300f44b350d04262e569cc607e4991d8 brunoerg: utACK fa7ba926300f44b350d04262e569cc607e4991d8 Tree-SHA512: 154a4895834babede6ce7b775562a7026637af1097e53e55676e92f6cf966ae0c092300ebf7e51a397eebd11f7b41d020586663e781f70d084efda1c0fe851b4 --- src/test/fuzz/utxo_total_supply.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index cd5f23b8104b..51f4ccd7c9d9 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -97,8 +97,8 @@ FUZZ_TARGET(utxo_total_supply) assert(ActiveHeight() == 0); // Get at which height we duplicate the coinbase // Assuming that the fuzzer will mine relatively short chains (less than 200 blocks), we want the duplicate coinbase to be not too high. - // Up to 2000 seems reasonable. - int64_t duplicate_coinbase_height = fuzzed_data_provider.ConsumeIntegralInRange(0, 20 * COINBASE_MATURITY); + // Up to 300 seems reasonable. + int64_t duplicate_coinbase_height = fuzzed_data_provider.ConsumeIntegralInRange(0, 300); // Always pad with OP_0 at the end to avoid bad-cb-length error const CScript duplicate_coinbase_script = CScript() << duplicate_coinbase_height << OP_0; // Mine the first block with this duplicate @@ -124,7 +124,7 @@ FUZZ_TARGET(utxo_total_supply) // Limit to avoid timeout, but enough to cover duplicate_coinbase_height // and CVE-2018-17144. - LIMITED_WHILE(fuzzed_data_provider.remaining_bytes(), 2'000) + LIMITED_WHILE(fuzzed_data_provider.remaining_bytes(), 2'00) { CallOneOf( fuzzed_data_provider,