From 939262869a63faea29b9b5580b9117f805bcaf22 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 4 Apr 2024 12:05:16 +0100 Subject: [PATCH] Squashed 'src/secp256k1/' changes from efe85c70a2..d8311688bd d8311688bd Merge bitcoin-core/secp256k1#1515: ci: Note affected clangs in comment on ASLR quirk a85e2233e7 ci: Note affected clangs in comment on ASLR quirk 4b77fec67a Merge bitcoin-core/secp256k1#1512: msan: notate more variable assignments from assembly code f7f0184ba1 msan: notate more variable assignments from assembly code a61339149f change inconsistent array param to pointer 05bfab69ae Merge bitcoin-core/secp256k1#1507: ci: Add workaround for ASLR bug in sanitizers a5e8ab2484 ci: Add sanitizer env variables to debug output 84a93de4d2 ci: Add workaround for ASLR bug in sanitizers 427e86b9ed Merge bitcoin-core/secp256k1#1490: tests: improve fe_sqr test (issue #1472) 2028069df2 doc: clarify input requirements for secp256k1_fe_mul 11420a7a28 tests: improve fe_sqr test cdc9a6258e Merge bitcoin-core/secp256k1#1489: tests: add missing fe comparison checks for inverse field test cases d926510cf7 Merge bitcoin-core/secp256k1#1496: msan: notate variable assignments from assembly code 31ba404944 msan: notate variable assignments from assembly code e7ea32e30a msan: Add SECP256K1_CHECKMEM_MSAN_DEFINE which applies to memory sanitizer and not valgrind e7bdddd9c9 refactor: rename `check_fe_equal` -> `fe_equal` 00111c9c56 tests: add missing fe comparison checks for inverse field test cases 0653a25d50 Merge bitcoin-core/secp256k1#1486: ci: Update cache action 94a14d5290 ci: Update cache action 2483627299 Merge bitcoin-core/secp256k1#1483: cmake: Recommend native CMake commands in README 5ad3aa3dcd Merge bitcoin-core/secp256k1#1484: tests: Drop redundant _scalar_check_overflow calls 51df2d9ab3 tests: Drop redundant _scalar_check_overflow calls 3777e3f36a cmake: Recommend native CMake commands in README e4af41c61b Merge bitcoin-core/secp256k1#1249: cmake: Add `SECP256K1_LATE_CFLAGS` configure option 3bf4d68fc0 Merge bitcoin-core/secp256k1#1482: build: Clean up handling of module dependencies e6822678ea build: Error if required module explicitly off 89ec583ccf build: Clean up handling of module dependencies 44378867a0 Merge bitcoin-core/secp256k1#1468: v0.4.1 release aftermath a9db9f2d75 Merge bitcoin-core/secp256k1#1480: Get rid of untested sizeof(secp256k1_ge_storage) == 64 code path 74b7c3b53e Merge bitcoin-core/secp256k1#1476: include: make docs more consistent b37fdb28ce check-abi: Minor UI improvements ad5f589a94 check-abi: Default to HEAD for new version 9fb7e2f156 release process: Style and formatting nits ba5d72d626 assumptions: Use new STATIC_ASSERT macro e53c2d9ffc Require that sizeof(secp256k1_ge_storage) == 64 d0ba2abbff util: Add STATIC_ASSERT macro da7bc1b803 include: in doc, remove article in front of "pointer" aa3dd5280b include: make doc about ctx more consistent e3f690015a include: remove obvious "cannot be NULL" doc d373bf6d08 Merge bitcoin-core/secp256k1#1474: tests: restore scalar_mul test 79e094517c Merge bitcoin-core/secp256k1#1473: Fix typos 3dbfb48946 tests: restore scalar_mul test d77170a88d Fix typos e7053d065b release process: Add email step 429d21dc79 release process: Run sanity checks on release PR 42f8c51402 cmake: Add `SECP256K1_LATE_CFLAGS` configure option git-subtree-dir: src/secp256k1 git-subtree-split: d8311688bd383d3a923a1b11789cded3cc8e5e03 --- .../install-homebrew-valgrind/action.yml | 2 +- .../actions/run-in-docker-action/action.yml | 5 + CONTRIBUTING.md | 2 +- README.md | 6 +- ci/ci.sh | 3 +- configure.ac | 126 +++--------------- doc/release-process.md | 79 +++++------ src/assumptions.h | 104 ++++++++------- src/checkmem.h | 7 + src/secp256k1/CMakeLists.txt | 41 ++++-- .../cmake/AllTargetsCompileOptions.cmake | 12 ++ src/secp256k1/contrib/lax_der_parsing.h | 4 +- src/secp256k1/include/secp256k1.h | 66 ++++----- src/secp256k1/include/secp256k1_ecdh.h | 2 +- src/secp256k1/include/secp256k1_ellswift.h | 4 +- src/secp256k1/include/secp256k1_extrakeys.h | 10 +- .../include/secp256k1_preallocated.h | 14 +- src/secp256k1/include/secp256k1_recovery.h | 20 +-- src/secp256k1/include/secp256k1_schnorrsig.h | 4 +- src/secp256k1/src/field.h | 4 +- .../src/modules/ellswift/tests_impl.h | 8 +- src/secp256k1/src/scalar_4x64_impl.h | 59 +++++--- src/secp256k1/src/scalar_impl.h | 4 +- src/secp256k1/src/secp256k1.c | 39 ++---- src/secp256k1/src/tests.c | 67 ++++++---- src/secp256k1/src/util.h | 16 ++- src/secp256k1/tools/check-abi.sh | 27 ++-- 27 files changed, 366 insertions(+), 369 deletions(-) create mode 100644 src/secp256k1/cmake/AllTargetsCompileOptions.cmake diff --git a/.github/actions/install-homebrew-valgrind/action.yml b/.github/actions/install-homebrew-valgrind/action.yml index 094ff891f7..ce10eb2686 100644 --- a/.github/actions/install-homebrew-valgrind/action.yml +++ b/.github/actions/install-homebrew-valgrind/action.yml @@ -16,7 +16,7 @@ runs: cat valgrind_fingerprint shell: bash - - uses: actions/cache@v3 + - uses: actions/cache@v4 id: cache with: path: ${{ env.CI_HOMEBREW_CELLAR_VALGRIND }} diff --git a/.github/actions/run-in-docker-action/action.yml b/.github/actions/run-in-docker-action/action.yml index dbfaa4fece..74933686a0 100644 --- a/.github/actions/run-in-docker-action/action.yml +++ b/.github/actions/run-in-docker-action/action.yml @@ -36,6 +36,11 @@ runs: load: true cache-from: type=gha + - # Workaround for https://github.com/google/sanitizers/issues/1614 . + # The underlying issue has been fixed in clang 18.1.3. + run: sudo sysctl -w vm.mmap_rnd_bits=28 + shell: bash + - # Tell Docker to pass environment variables in `env` into the container. run: > docker run \ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a5e457913a..5fbf7332c9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -44,7 +44,7 @@ The Contributor Workflow & Peer Review in libsecp256k1 are similar to Bitcoin Co In addition, libsecp256k1 tries to maintain the following coding conventions: -* No runtime heap allocation (e.g., no `malloc`) unless explicitly requested by the caller (via `secp256k1_context_create` or `secp256k1_scratch_space_create`, for example). Morever, it should be possible to use the library without any heap allocations. +* No runtime heap allocation (e.g., no `malloc`) unless explicitly requested by the caller (via `secp256k1_context_create` or `secp256k1_scratch_space_create`, for example). Moreover, it should be possible to use the library without any heap allocations. * The tests should cover all lines and branches of the library (see [Test coverage](#coverage)). * Operations involving secret data should be tested for being constant time with respect to the secrets (see [src/ctime_tests.c](src/ctime_tests.c)). * Local variables containing secret data should be cleared explicitly to try to delete secrets from memory. diff --git a/README.md b/README.md index cb9b7bdff9..a45d239c13 100644 --- a/README.md +++ b/README.md @@ -101,9 +101,9 @@ To maintain a pristine source tree, CMake encourages to perform an out-of-source $ mkdir build && cd build $ cmake .. - $ make - $ make check # run the test suite - $ sudo make install # optional + $ cmake --build . + $ ctest # run the test suite + $ sudo cmake --build . --target install # optional To compile optional modules (such as Schnorr signatures), you need to run `cmake` with additional flags (such as `-DSECP256K1_ENABLE_MODULE_SCHNORRSIG=ON`). Run `cmake .. -LH` to see the full list of available flags. diff --git a/ci/ci.sh b/ci/ci.sh index 9cc715955e..3999af4f1c 100755 --- a/ci/ci.sh +++ b/ci/ci.sh @@ -17,7 +17,8 @@ print_environment() { SECP256K1_TEST_ITERS BENCH SECP256K1_BENCH_ITERS CTIMETESTS\ EXAMPLES \ HOST WRAPPER_CMD \ - CC CFLAGS CPPFLAGS AR NM + CC CFLAGS CPPFLAGS AR NM \ + UBSAN_OPTIONS ASAN_OPTIONS LSAN_OPTIONS do eval "isset=\${$var+x}" if [ -n "$isset" ]; then diff --git a/configure.ac b/configure.ac index 6e90aa6122..b28d9fe94c 100755 --- a/configure.ac +++ b/configure.ac @@ -1541,126 +1541,34 @@ fi dnl ZMQ check -if test "$use_zmq" = "yes"; then - PKG_CHECK_MODULES([ZMQ], [libzmq >= 4], - AC_DEFINE([ENABLE_ZMQ], [1], [Define this symbol to enable ZMQ functions]), - [AC_MSG_WARN([libzmq version 4.x or greater not found, disabling]) - use_zmq=no]) +# Processing must be done in a reverse topological sorting of the dependency graph +# (dependent module first). +if test x"$enable_module_ellswift" = x"yes"; then + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_ELLSWIFT=1" fi -if test "$use_zmq" = "yes"; then - dnl Assume libzmq was built for static linking - case $host in - *mingw*) - ZMQ_CFLAGS="$ZMQ_CFLAGS -DZMQ_STATIC" - ;; - esac -fi - -AM_CONDITIONAL([ENABLE_ZMQ], [test "$use_zmq" = "yes"]) - -dnl libmultiprocess library check - -libmultiprocess_found=no -if test "$with_libmultiprocess" = "yes" || test "$with_libmultiprocess" = "auto"; then - PKG_CHECK_MODULES([LIBMULTIPROCESS], [libmultiprocess], [ - libmultiprocess_found=yes; - libmultiprocess_prefix=`$PKG_CONFIG --variable=prefix libmultiprocess`; - ], [true]) -elif test "$with_libmultiprocess" != "no"; then - AC_MSG_ERROR([--with-libmultiprocess=$with_libmultiprocess value is not yes, auto, or no]) -fi - -dnl Enable multiprocess check - -if test "$enable_multiprocess" = "yes"; then - if test "$libmultiprocess_found" != "yes"; then - AC_MSG_ERROR([--enable-multiprocess=yes option specified but libmultiprocess library was not found. May need to install libmultiprocess library, or specify install path with PKG_CONFIG_PATH environment variable. Running 'pkg-config --debug libmultiprocess' may be helpful for debugging.]) +if test x"$enable_module_schnorrsig" = x"yes"; then + if test x"$enable_module_extrakeys" = x"no"; then + AC_MSG_ERROR([Module dependency error: You have disabled the extrakeys module explicitly, but it is required by the schnorrsig module.]) fi - build_multiprocess=yes -elif test "$enable_multiprocess" = "auto"; then - build_multiprocess=$libmultiprocess_found -else - build_multiprocess=no + enable_module_extrakeys=yes + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_SCHNORRSIG=1" fi -AM_CONDITIONAL([BUILD_MULTIPROCESS], [test "$build_multiprocess" = "yes"]) -AM_CONDITIONAL([BUILD_BGL_NODE], [test "$build_multiprocess" = "yes"]) -AM_CONDITIONAL([BUILD_BGL_GUI], [test "$build_multiprocess" = "yes"]) - -dnl codegen tools check - -if test "$build_multiprocess" != "no"; then - if test "$with_mpgen" = "yes" || test "$with_mpgen" = "auto"; then - MPGEN_PREFIX="$libmultiprocess_prefix" - elif test "$with_mpgen" != "no"; then - MPGEN_PREFIX="$with_mpgen"; - fi - AC_SUBST(MPGEN_PREFIX) +if test x"$enable_module_extrakeys" = x"yes"; then + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_EXTRAKEYS=1" fi -AC_MSG_CHECKING([whether to build BGLd]) -AM_CONDITIONAL([BUILD_BGLD], [test $build_BGLd = "yes"]) -AC_MSG_RESULT($build_BGLd) - -AC_MSG_CHECKING([whether to build BGL-cli]) -AM_CONDITIONAL([BUILD_BGL_CLI], [test $build_BGL_cli = "yes"]) -AC_MSG_RESULT($build_BGL_cli) - -AC_MSG_CHECKING([whether to build BGL-tx]) -AM_CONDITIONAL([BUILD_BGL_TX], [test $build_BGL_tx = "yes"]) -AC_MSG_RESULT($build_BGL_tx) - -AC_MSG_CHECKING([whether to build BGL-wallet]) -AM_CONDITIONAL([BUILD_BGL_WALLET], [test $build_BGL_wallet = "yes"]) -AC_MSG_RESULT($build_BGL_wallet) - -AC_MSG_CHECKING([whether to build BGL-util]) -AM_CONDITIONAL([BUILD_BGL_UTIL], [test $build_BGL_util = "yes"]) -AC_MSG_RESULT($build_BGL_util) - -AC_MSG_CHECKING([whether to build experimental BGL-chainstate]) -if test "$build_BGL_chainstate" = "yes"; then - if test "$build_experimental_kernel_lib" = "no"; then - AC_MSG_ERROR([experimental BGL-chainstate cannot be built without the experimental BGLkernel library. Use --with-experimental-kernel-lib]); - fi +if test x"$enable_module_recovery" = x"yes"; then + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_RECOVERY=1" fi -AM_CONDITIONAL([BUILD_BGL_CHAINSTATE], [test $build_BGL_chainstate = "yes"]) -AC_MSG_RESULT($build_BGL_chainstate) -AM_CONDITIONAL([BUILD_BGL_KERNEL_LIB], [test "$build_experimental_kernel_lib" != "no" && ( test "$build_experimental_kernel_lib" = "yes" || test "$build_BGL_chainstate" = "yes" )]) - -AC_LANG_POP - -if test "$use_ccache" != "no"; then - AC_MSG_CHECKING([if ccache should be used]) - if test "$CCACHE" = ""; then - if test "$use_ccache" = "yes"; then - AC_MSG_ERROR([ccache not found.]); - else - use_ccache=no - fi - else - use_ccache=yes - CC="$ac_cv_path_CCACHE $CC" - CXX="$ac_cv_path_CCACHE $CXX" - fi - AC_MSG_RESULT($use_ccache) - if test "$use_ccache" = "yes"; then - AX_CHECK_COMPILE_FLAG([-fdebug-prefix-map=A=B], [DEBUG_CXXFLAGS="$DEBUG_CXXFLAGS -fdebug-prefix-map=\$(abs_top_srcdir)=."], [], [$CXXFLAG_WERROR]) - AX_CHECK_PREPROC_FLAG([-fmacro-prefix-map=A=B], [DEBUG_CPPFLAGS="$DEBUG_CPPFLAGS -fmacro-prefix-map=\$(abs_top_srcdir)=."], [], [$CXXFLAG_WERROR]) - fi +if test x"$enable_module_ecdh" = x"yes"; then + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_ECDH=1" fi -dnl enable wallet -AC_MSG_CHECKING([if wallet should be enabled]) -if test "$enable_wallet" != "no"; then - AC_MSG_RESULT([yes]) - AC_DEFINE_UNQUOTED([ENABLE_WALLET],[1],[Define to 1 to enable wallet functions]) - enable_wallet=yes - -else - AC_MSG_RESULT([no]) +if test x"$enable_external_default_callbacks" = x"yes"; then + SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DUSE_EXTERNAL_DEFAULT_CALLBACKS=1" fi dnl enable upnp support diff --git a/doc/release-process.md b/doc/release-process.md index bf7e2de070..e1ef33d299 100644 --- a/doc/release-process.md +++ b/doc/release-process.md @@ -1,4 +1,4 @@ -# Release Process +# Release process This document outlines the process for releasing versions of the form `$MAJOR.$MINOR.$PATCH`. @@ -17,8 +17,9 @@ This process also assumes that there will be no minor releases for old major rel * Update manpages (see previous section) * Write release notes (see "Write the release notes" below). -## Sanity Checks -Perform these checks before creating a release: +## Sanity checks +Perform these checks when reviewing the release PR (see below): + * On both the master branch and the new release branch: - update `CLIENT_VERSION_MAJOR` in [`configure.ac`](../configure.ac) @@ -121,7 +122,7 @@ git fetch origin "v${VERSION}" git checkout "v${VERSION}" popd ``` -<<<<<<< HEAD + Ensure your guix.sigs are up-to-date if you wish to `guix-verify` your builds against other `guix-attest` signatures. @@ -153,28 +154,30 @@ pushd ./guix.sigs git add "${VERSION}/${SIGNER}"/noncodesigned.SHA256SUMS{,.asc} git commit -m "Add attestations by ${SIGNER} for ${VERSION} non-codesigned" popd -======= + +1. Ensure `make distcheck` doesn't fail. + ```shell + ./autogen.sh && ./configure --enable-dev-mode && make distcheck + ``` + 2. Check installation with autotools: -```shell -dir=$(mktemp -d) -./autogen.sh && ./configure --prefix=$dir && make clean && make install && ls -RlAh $dir -gcc -o ecdsa examples/ecdsa.c $(PKG_CONFIG_PATH=$dir/lib/pkgconfig pkg-config --cflags --libs libsecp256k1) -Wl,-rpath,"$dir/lib" && ./ecdsa -``` + ```shell + dir=$(mktemp -d) + ./autogen.sh && ./configure --prefix=$dir && make clean && make install && ls -RlAh $dir + gcc -o ecdsa examples/ecdsa.c $(PKG_CONFIG_PATH=$dir/lib/pkgconfig pkg-config --cflags --libs libsecp256k1) -Wl,-rpath,"$dir/lib" && ./ecdsa + ``` 3. Check installation with CMake: -```shell -dir=$(mktemp -d) -build=$(mktemp -d) -cmake -B $build -DCMAKE_INSTALL_PREFIX=$dir && cmake --build $build --target install && ls -RlAh $dir -gcc -o ecdsa examples/ecdsa.c -I $dir/include -L $dir/lib*/ -l secp256k1 -Wl,-rpath,"$dir/lib",-rpath,"$dir/lib64" && ./ecdsa ->>>>>>> 29fde0223a... Squashed 'src/secp256k1/' changes from 199d27cea3..efe85c70a2 -``` -4. Use the [`check-abi.sh`](/tools/check-abi.sh) tool to ensure there are no unexpected ABI incompatibilities and that the version number and release notes accurately reflect all potential ABI changes. To run this tool, the `abi-dumper` and `abi-compliance-checker` packages are required. - -```shell -tools/check-abi.sh -``` + ```shell + dir=$(mktemp -d) + build=$(mktemp -d) + cmake -B $build -DCMAKE_INSTALL_PREFIX=$dir && cmake --build $build --target install && ls -RlAh $dir + gcc -o ecdsa examples/ecdsa.c -I $dir/include -L $dir/lib*/ -l secp256k1 -Wl,-rpath,"$dir/lib",-rpath,"$dir/lib64" && ./ecdsa + ``` +4. Use the [`check-abi.sh`](/tools/check-abi.sh) tool to verify that there are no unexpected ABI incompatibilities and that the version number and the release notes accurately reflect all potential ABI changes. To run this tool, the `abi-dumper` and `abi-compliance-checker` packages are required. + ```shell + tools/check-abi.sh + ``` -<<<<<<< HEAD Then open a Pull Request to the [guix.sigs repository](https://github.com/bitcoin-core/guix.sigs). ## Codesigning @@ -350,27 +353,29 @@ Notes: * adding a section for the release (make sure that the version number is a link to a diff between the previous and new version), * removing the `[Unreleased]` section header, and * including an entry for `### ABI Compatibility` if it doesn't exist, - * sets `_PKG_VERSION_IS_RELEASE` to `true` in `configure.ac`, and - * if this is not a patch release - * updates `_PKG_VERSION_*` and `_LIB_VERSION_*` in `configure.ac` and + * sets `_PKG_VERSION_IS_RELEASE` to `true` in `configure.ac`, and, + * if this is not a patch release, + * updates `_PKG_VERSION_*` and `_LIB_VERSION_*` in `configure.ac`, and * updates `project(libsecp256k1 VERSION ...)` and `${PROJECT_NAME}_LIB_VERSION_*` in `CMakeLists.txt`. -2. After the PR is merged, tag the commit and push it: +2. Perform the [sanity checks](#sanity-checks) on the PR branch. +3. After the PR is merged, tag the commit, and push the tag: ``` RELEASE_COMMIT= git tag -s v$MAJOR.$MINOR.$PATCH -m "libsecp256k1 $MAJOR.$MINOR.$PATCH" $RELEASE_COMMIT git push git@github.com:bitcoin-core/secp256k1.git v$MAJOR.$MINOR.$PATCH ``` -3. Open a PR to the master branch with a commit (using message `"release cleanup: bump version after $MAJOR.$MINOR.$PATCH"`, for example) that +4. Open a PR to the master branch with a commit (using message `"release cleanup: bump version after $MAJOR.$MINOR.$PATCH"`, for example) that * sets `_PKG_VERSION_IS_RELEASE` to `false` and increments `_PKG_VERSION_PATCH` and `_LIB_VERSION_REVISION` in `configure.ac`, * increments the `$PATCH` component of `project(libsecp256k1 VERSION ...)` and `${PROJECT_NAME}_LIB_VERSION_REVISION` in `CMakeLists.txt`, and * adds an `[Unreleased]` section header to the [CHANGELOG.md](../CHANGELOG.md). If other maintainers are not present to approve the PR, it can be merged without ACKs. -4. Create a new GitHub release with a link to the corresponding entry in [CHANGELOG.md](../CHANGELOG.md). +5. Create a new GitHub release with a link to the corresponding entry in [CHANGELOG.md](../CHANGELOG.md). +6. Send an announcement email to the bitcoin-dev mailing list. ## Maintenance release -Note that bugfixes only need to be backported to releases for which no compatible release without the bug exists. +Note that bug fixes need to be backported only to releases for which no compatible release without the bug exists. 1. If there's no maintenance branch `$MAJOR.$MINOR`, create one: ``` @@ -378,20 +383,18 @@ Note that bugfixes only need to be backported to releases for which no compatibl git push git@github.com:bitcoin-core/secp256k1.git $MAJOR.$MINOR ``` 2. Open a pull request to the `$MAJOR.$MINOR` branch that - * includes the bugfixes, + * includes the bug fixes, * finalizes the release notes similar to a regular release, * increments `_PKG_VERSION_PATCH` and `_LIB_VERSION_REVISION` in `configure.ac` and the `$PATCH` component of `project(libsecp256k1 VERSION ...)` and `${PROJECT_NAME}_LIB_VERSION_REVISION` in `CMakeLists.txt` (with commit message `"release: bump versions for $MAJOR.$MINOR.$PATCH"`, for example). -3. After the PRs are merged, update the release branch and tag the commit: +3. Perform the [sanity checks](#sanity-checks) on the PR branch. +4. After the PRs are merged, update the release branch, tag the commit, and push the tag: ``` git checkout $MAJOR.$MINOR && git pull git tag -s v$MAJOR.$MINOR.$PATCH -m "libsecp256k1 $MAJOR.$MINOR.$PATCH" - ``` -4. Push tag: - ``` git push git@github.com:bitcoin-core/secp256k1.git v$MAJOR.$MINOR.$PATCH ``` -5. Create a new GitHub release with a link to the corresponding entry in [CHANGELOG.md](../CHANGELOG.md). -6. Open PR to the master branch that includes a commit (with commit message `"release notes: add $MAJOR.$MINOR.$PATCH"`, for example) that adds release notes to [CHANGELOG.md](../CHANGELOG.md). ->>>>>>> 29fde0223a... Squashed 'src/secp256k1/' changes from 199d27cea3..efe85c70a2 +6. Create a new GitHub release with a link to the corresponding entry in [CHANGELOG.md](../CHANGELOG.md). +7. Send an announcement email to the bitcoin-dev mailing list. +8. Open PR to the master branch that includes a commit (with commit message `"release notes: add $MAJOR.$MINOR.$PATCH"`, for example) that adds release notes to [CHANGELOG.md](../CHANGELOG.md). diff --git a/src/assumptions.h b/src/assumptions.h index 77204de2b8..50c150f080 100644 --- a/src/assumptions.h +++ b/src/assumptions.h @@ -16,65 +16,69 @@ reduce the odds of experiencing an unwelcome surprise. */ -struct secp256k1_assumption_checker { - /* This uses a trick to implement a static assertion in C89: a type with an array of negative size is not - allowed. */ - int dummy_array[( - /* Bytes are 8 bits. */ - (CHAR_BIT == 8) && +#if defined(__has_attribute) +# if __has_attribute(__unavailable__) +__attribute__((__unavailable__("Don't call this function. It only exists because STATIC_ASSERT cannot be used outside a function."))) +# endif +#endif +static void secp256k1_assumption_checker(void) { + /* Bytes are 8 bits. */ + STATIC_ASSERT(CHAR_BIT == 8); - /* No integer promotion for uint32_t. This ensures that we can multiply uintXX_t values where XX >= 32 - without signed overflow, which would be undefined behaviour. */ - (UINT_MAX <= UINT32_MAX) && + /* No integer promotion for uint32_t. This ensures that we can multiply uintXX_t values where XX >= 32 + without signed overflow, which would be undefined behaviour. */ + STATIC_ASSERT(UINT_MAX <= UINT32_MAX); - /* Conversions from unsigned to signed outside of the bounds of the signed type are - implementation-defined. Verify that they function as reinterpreting the lower - bits of the input in two's complement notation. Do this for conversions: - - from uint(N)_t to int(N)_t with negative result - - from uint(2N)_t to int(N)_t with negative result - - from int(2N)_t to int(N)_t with negative result - - from int(2N)_t to int(N)_t with positive result */ + /* Conversions from unsigned to signed outside of the bounds of the signed type are + implementation-defined. Verify that they function as reinterpreting the lower + bits of the input in two's complement notation. Do this for conversions: + - from uint(N)_t to int(N)_t with negative result + - from uint(2N)_t to int(N)_t with negative result + - from int(2N)_t to int(N)_t with negative result + - from int(2N)_t to int(N)_t with positive result */ - /* To int8_t. */ - ((int8_t)(uint8_t)0xAB == (int8_t)-(int8_t)0x55) && - ((int8_t)(uint16_t)0xABCD == (int8_t)-(int8_t)0x33) && - ((int8_t)(int16_t)(uint16_t)0xCDEF == (int8_t)(uint8_t)0xEF) && - ((int8_t)(int16_t)(uint16_t)0x9234 == (int8_t)(uint8_t)0x34) && + /* To int8_t. */ + STATIC_ASSERT(((int8_t)(uint8_t)0xAB == (int8_t)-(int8_t)0x55)); + STATIC_ASSERT((int8_t)(uint16_t)0xABCD == (int8_t)-(int8_t)0x33); + STATIC_ASSERT((int8_t)(int16_t)(uint16_t)0xCDEF == (int8_t)(uint8_t)0xEF); + STATIC_ASSERT((int8_t)(int16_t)(uint16_t)0x9234 == (int8_t)(uint8_t)0x34); - /* To int16_t. */ - ((int16_t)(uint16_t)0xBCDE == (int16_t)-(int16_t)0x4322) && - ((int16_t)(uint32_t)0xA1B2C3D4 == (int16_t)-(int16_t)0x3C2C) && - ((int16_t)(int32_t)(uint32_t)0xC1D2E3F4 == (int16_t)(uint16_t)0xE3F4) && - ((int16_t)(int32_t)(uint32_t)0x92345678 == (int16_t)(uint16_t)0x5678) && + /* To int16_t. */ + STATIC_ASSERT((int16_t)(uint16_t)0xBCDE == (int16_t)-(int16_t)0x4322); + STATIC_ASSERT((int16_t)(uint32_t)0xA1B2C3D4 == (int16_t)-(int16_t)0x3C2C); + STATIC_ASSERT((int16_t)(int32_t)(uint32_t)0xC1D2E3F4 == (int16_t)(uint16_t)0xE3F4); + STATIC_ASSERT((int16_t)(int32_t)(uint32_t)0x92345678 == (int16_t)(uint16_t)0x5678); - /* To int32_t. */ - ((int32_t)(uint32_t)0xB2C3D4E5 == (int32_t)-(int32_t)0x4D3C2B1B) && - ((int32_t)(uint64_t)0xA123B456C789D012ULL == (int32_t)-(int32_t)0x38762FEE) && - ((int32_t)(int64_t)(uint64_t)0xC1D2E3F4A5B6C7D8ULL == (int32_t)(uint32_t)0xA5B6C7D8) && - ((int32_t)(int64_t)(uint64_t)0xABCDEF0123456789ULL == (int32_t)(uint32_t)0x23456789) && + /* To int32_t. */ + STATIC_ASSERT((int32_t)(uint32_t)0xB2C3D4E5 == (int32_t)-(int32_t)0x4D3C2B1B); + STATIC_ASSERT((int32_t)(uint64_t)0xA123B456C789D012ULL == (int32_t)-(int32_t)0x38762FEE); + STATIC_ASSERT((int32_t)(int64_t)(uint64_t)0xC1D2E3F4A5B6C7D8ULL == (int32_t)(uint32_t)0xA5B6C7D8); + STATIC_ASSERT((int32_t)(int64_t)(uint64_t)0xABCDEF0123456789ULL == (int32_t)(uint32_t)0x23456789); - /* To int64_t. */ - ((int64_t)(uint64_t)0xB123C456D789E012ULL == (int64_t)-(int64_t)0x4EDC3BA928761FEEULL) && -#if defined(SECP256K1_WIDEMUL_INT128) - ((int64_t)(((uint128_t)0xA1234567B8901234ULL << 64) + 0xC5678901D2345678ULL) == (int64_t)-(int64_t)0x3A9876FE2DCBA988ULL) && - (((int64_t)(int128_t)(((uint128_t)0xB1C2D3E4F5A6B7C8ULL << 64) + 0xD9E0F1A2B3C4D5E6ULL)) == (int64_t)(uint64_t)0xD9E0F1A2B3C4D5E6ULL) && - (((int64_t)(int128_t)(((uint128_t)0xABCDEF0123456789ULL << 64) + 0x0123456789ABCDEFULL)) == (int64_t)(uint64_t)0x0123456789ABCDEFULL) && + /* To int64_t. */ + STATIC_ASSERT((int64_t)(uint64_t)0xB123C456D789E012ULL == (int64_t)-(int64_t)0x4EDC3BA928761FEEULL); +#if defined(SECP256K1_INT128_NATIVE) + STATIC_ASSERT((int64_t)(((uint128_t)0xA1234567B8901234ULL << 64) + 0xC5678901D2345678ULL) == (int64_t)-(int64_t)0x3A9876FE2DCBA988ULL); + STATIC_ASSERT(((int64_t)(int128_t)(((uint128_t)0xB1C2D3E4F5A6B7C8ULL << 64) + 0xD9E0F1A2B3C4D5E6ULL)) == (int64_t)(uint64_t)0xD9E0F1A2B3C4D5E6ULL); + STATIC_ASSERT(((int64_t)(int128_t)(((uint128_t)0xABCDEF0123456789ULL << 64) + 0x0123456789ABCDEFULL)) == (int64_t)(uint64_t)0x0123456789ABCDEFULL); - /* To int128_t. */ - ((int128_t)(((uint128_t)0xB1234567C8901234ULL << 64) + 0xD5678901E2345678ULL) == (int128_t)(-(int128_t)0x8E1648B3F50E80DCULL * 0x8E1648B3F50E80DDULL + 0x5EA688D5482F9464ULL)) && + /* To int128_t. */ + STATIC_ASSERT((int128_t)(((uint128_t)0xB1234567C8901234ULL << 64) + 0xD5678901E2345678ULL) == (int128_t)(-(int128_t)0x8E1648B3F50E80DCULL * 0x8E1648B3F50E80DDULL + 0x5EA688D5482F9464ULL)); #endif - /* Right shift on negative signed values is implementation defined. Verify that it - acts as a right shift in two's complement with sign extension (i.e duplicating - the top bit into newly added bits). */ - ((((int8_t)0xE8) >> 2) == (int8_t)(uint8_t)0xFA) && - ((((int16_t)0xE9AC) >> 4) == (int16_t)(uint16_t)0xFE9A) && - ((((int32_t)0x937C918A) >> 9) == (int32_t)(uint32_t)0xFFC9BE48) && - ((((int64_t)0xA8B72231DF9CF4B9ULL) >> 19) == (int64_t)(uint64_t)0xFFFFF516E4463BF3ULL) && -#if defined(SECP256K1_WIDEMUL_INT128) - ((((int128_t)(((uint128_t)0xCD833A65684A0DBCULL << 64) + 0xB349312F71EA7637ULL)) >> 39) == (int128_t)(((uint128_t)0xFFFFFFFFFF9B0674ULL << 64) + 0xCAD0941B79669262ULL)) && + /* Right shift on negative signed values is implementation defined. Verify that it + acts as a right shift in two's complement with sign extension (i.e duplicating + the top bit into newly added bits). */ + STATIC_ASSERT((((int8_t)0xE8) >> 2) == (int8_t)(uint8_t)0xFA); + STATIC_ASSERT((((int16_t)0xE9AC) >> 4) == (int16_t)(uint16_t)0xFE9A); + STATIC_ASSERT((((int32_t)0x937C918A) >> 9) == (int32_t)(uint32_t)0xFFC9BE48); + STATIC_ASSERT((((int64_t)0xA8B72231DF9CF4B9ULL) >> 19) == (int64_t)(uint64_t)0xFFFFF516E4463BF3ULL); +#if defined(SECP256K1_INT128_NATIVE) + STATIC_ASSERT((((int128_t)(((uint128_t)0xCD833A65684A0DBCULL << 64) + 0xB349312F71EA7637ULL)) >> 39) == (int128_t)(((uint128_t)0xFFFFFFFFFF9B0674ULL << 64) + 0xCAD0941B79669262ULL)); #endif - 1) * 2 - 1]; -}; + + /* This function is not supposed to be called. */ + VERIFY_CHECK(0); +} #endif /* SECP256K1_ASSUMPTIONS_H */ diff --git a/src/checkmem.h b/src/checkmem.h index f2169decfc..7e333ce5f3 100644 --- a/src/checkmem.h +++ b/src/checkmem.h @@ -30,6 +30,8 @@ * - SECP256K1_CHECKMEM_DEFINE(p, len): * - marks the len-byte memory pointed to by p as defined data (public data, in the * context of constant-time checking). + * - SECP256K1_CHECKMEM_MSAN_DEFINE(p, len): + * - Like SECP256K1_CHECKMEM_DEFINE, but applies only to memory_sanitizer. * */ @@ -48,11 +50,16 @@ # define SECP256K1_CHECKMEM_ENABLED 1 # define SECP256K1_CHECKMEM_UNDEFINE(p, len) __msan_allocated_memory((p), (len)) # define SECP256K1_CHECKMEM_DEFINE(p, len) __msan_unpoison((p), (len)) +# define SECP256K1_CHECKMEM_MSAN_DEFINE(p, len) __msan_unpoison((p), (len)) # define SECP256K1_CHECKMEM_CHECK(p, len) __msan_check_mem_is_initialized((p), (len)) # define SECP256K1_CHECKMEM_RUNNING() (1) # endif #endif +#if !defined SECP256K1_CHECKMEM_MSAN_DEFINE +# define SECP256K1_CHECKMEM_MSAN_DEFINE(p, len) SECP256K1_CHECKMEM_NOOP((p), (len)) +#endif + /* If valgrind integration is desired (through the VALGRIND define), implement the * SECP256K1_CHECKMEM_* macros using valgrind. */ #if !defined SECP256K1_CHECKMEM_ENABLED diff --git a/src/secp256k1/CMakeLists.txt b/src/secp256k1/CMakeLists.txt index 71b99b3125..de62ac787e 100644 --- a/src/secp256k1/CMakeLists.txt +++ b/src/secp256k1/CMakeLists.txt @@ -51,29 +51,40 @@ endif() option(SECP256K1_INSTALL "Enable installation." ${PROJECT_IS_TOP_LEVEL}) -option(SECP256K1_ENABLE_MODULE_ECDH "Enable ECDH module." ON) -if(SECP256K1_ENABLE_MODULE_ECDH) - add_compile_definitions(ENABLE_MODULE_ECDH=1) -endif() +## Modules +# We declare all options before processing them, to make sure we can express +# dependendencies while processing. +option(SECP256K1_ENABLE_MODULE_ECDH "Enable ECDH module." ON) option(SECP256K1_ENABLE_MODULE_RECOVERY "Enable ECDSA pubkey recovery module." OFF) -if(SECP256K1_ENABLE_MODULE_RECOVERY) - add_compile_definitions(ENABLE_MODULE_RECOVERY=1) -endif() - option(SECP256K1_ENABLE_MODULE_EXTRAKEYS "Enable extrakeys module." ON) option(SECP256K1_ENABLE_MODULE_SCHNORRSIG "Enable schnorrsig module." ON) +option(SECP256K1_ENABLE_MODULE_ELLSWIFT "Enable ElligatorSwift module." ON) + +# Processing must be done in a topological sorting of the dependency graph +# (dependent module first). +if(SECP256K1_ENABLE_MODULE_ELLSWIFT) + add_compile_definitions(ENABLE_MODULE_ELLSWIFT=1) +endif() + if(SECP256K1_ENABLE_MODULE_SCHNORRSIG) + if(DEFINED SECP256K1_ENABLE_MODULE_EXTRAKEYS AND NOT SECP256K1_ENABLE_MODULE_EXTRAKEYS) + message(FATAL_ERROR "Module dependency error: You have disabled the extrakeys module explicitly, but it is required by the schnorrsig module.") + endif() set(SECP256K1_ENABLE_MODULE_EXTRAKEYS ON) add_compile_definitions(ENABLE_MODULE_SCHNORRSIG=1) endif() + if(SECP256K1_ENABLE_MODULE_EXTRAKEYS) add_compile_definitions(ENABLE_MODULE_EXTRAKEYS=1) endif() -option(SECP256K1_ENABLE_MODULE_ELLSWIFT "Enable ElligatorSwift module." ON) -if(SECP256K1_ENABLE_MODULE_ELLSWIFT) - add_compile_definitions(ENABLE_MODULE_ELLSWIFT=1) +if(SECP256K1_ENABLE_MODULE_RECOVERY) + add_compile_definitions(ENABLE_MODULE_RECOVERY=1) +endif() + +if(SECP256K1_ENABLE_MODULE_ECDH) + add_compile_definitions(ENABLE_MODULE_ECDH=1) endif() option(SECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS "Enable external default callback functions." OFF) @@ -254,9 +265,14 @@ if(SECP256K1_BUILD_BENCHMARK OR SECP256K1_BUILD_TESTS OR SECP256K1_BUILD_EXHAUST enable_testing() endif() +set(SECP256K1_LATE_CFLAGS "" CACHE STRING "Compiler flags that are added to the command line after all other flags added by the build system.") +include(AllTargetsCompileOptions) + add_subdirectory(src) +all_targets_compile_options(src "${SECP256K1_LATE_CFLAGS}") if(SECP256K1_BUILD_EXAMPLES) add_subdirectory(examples) + all_targets_compile_options(examples "${SECP256K1_LATE_CFLAGS}") endif() message("\n") @@ -330,6 +346,9 @@ else() message(" - LDFLAGS for executables ............ ${CMAKE_EXE_LINKER_FLAGS_DEBUG}") message(" - LDFLAGS for shared libraries ....... ${CMAKE_SHARED_LINKER_FLAGS_DEBUG}") endif() +if(SECP256K1_LATE_CFLAGS) + message("SECP256K1_LATE_CFLAGS ................. ${SECP256K1_LATE_CFLAGS}") +endif() message("\n") if(SECP256K1_EXPERIMENTAL) message( diff --git a/src/secp256k1/cmake/AllTargetsCompileOptions.cmake b/src/secp256k1/cmake/AllTargetsCompileOptions.cmake new file mode 100644 index 0000000000..6e420e0fde --- /dev/null +++ b/src/secp256k1/cmake/AllTargetsCompileOptions.cmake @@ -0,0 +1,12 @@ +# Add compile options to all targets added in the subdirectory. +function(all_targets_compile_options dir options) + get_directory_property(targets DIRECTORY ${dir} BUILDSYSTEM_TARGETS) + separate_arguments(options) + set(compiled_target_types STATIC_LIBRARY SHARED_LIBRARY OBJECT_LIBRARY EXECUTABLE) + foreach(target ${targets}) + get_target_property(type ${target} TYPE) + if(type IN_LIST compiled_target_types) + target_compile_options(${target} PRIVATE ${options}) + endif() + endforeach() +endfunction() diff --git a/src/secp256k1/contrib/lax_der_parsing.h b/src/secp256k1/contrib/lax_der_parsing.h index 034a38e6a0..37c8c691f2 100644 --- a/src/secp256k1/contrib/lax_der_parsing.h +++ b/src/secp256k1/contrib/lax_der_parsing.h @@ -67,8 +67,8 @@ extern "C" { * * Returns: 1 when the signature could be parsed, 0 otherwise. * Args: ctx: a secp256k1 context object - * Out: sig: a pointer to a signature object - * In: input: a pointer to the signature to be parsed + * Out: sig: pointer to a signature object + * In: input: pointer to the signature to be parsed * inputlen: the length of the array pointed to be input * * This function will accept any valid DER encoded signature, even if the diff --git a/src/secp256k1/include/secp256k1.h b/src/secp256k1/include/secp256k1.h index 936f0b42b7..f4053f2a93 100644 --- a/src/secp256k1/include/secp256k1.h +++ b/src/secp256k1/include/secp256k1.h @@ -265,7 +265,7 @@ SECP256K1_API void secp256k1_selftest(void); * memory allocation entirely, see secp256k1_context_static and the functions in * secp256k1_preallocated.h. * - * Returns: a newly created context object. + * Returns: pointer to a newly created context object. * In: flags: Always set to SECP256K1_CONTEXT_NONE (see below). * * The only valid non-deprecated flag in recent library versions is @@ -296,8 +296,8 @@ SECP256K1_API secp256k1_context *secp256k1_context_create( * Cloning secp256k1_context_static is not possible, and should not be emulated by * the caller (e.g., using memcpy). Create a new context instead. * - * Returns: a newly created context object. - * Args: ctx: an existing context to copy (not secp256k1_context_static) + * Returns: pointer to a newly created context object. + * Args: ctx: pointer to a context to copy (not secp256k1_context_static). */ SECP256K1_API secp256k1_context *secp256k1_context_clone( const secp256k1_context *ctx @@ -313,7 +313,7 @@ SECP256K1_API secp256k1_context *secp256k1_context_clone( * behaviour is undefined. In that case, secp256k1_context_preallocated_destroy must * be used instead. * - * Args: ctx: an existing context to destroy, constructed using + * Args: ctx: pointer to a context to destroy, constructed using * secp256k1_context_create or secp256k1_context_clone * (i.e., not secp256k1_context_static). */ @@ -350,8 +350,8 @@ SECP256K1_API void secp256k1_context_destroy( * fails. In this case, the corresponding default handler will be called with * the data pointer argument set to NULL. * - * Args: ctx: an existing context object. - * In: fun: a pointer to a function to call when an illegal argument is + * Args: ctx: pointer to a context object. + * In: fun: pointer to a function to call when an illegal argument is * passed to the API, taking a message and an opaque pointer. * (NULL restores the default handler.) * data: the opaque pointer to pass to fun above, must be NULL for the default handler. @@ -377,8 +377,8 @@ SECP256K1_API void secp256k1_context_set_illegal_callback( * for that). After this callback returns, anything may happen, including * crashing. * - * Args: ctx: an existing context object. - * In: fun: a pointer to a function to call when an internal error occurs, + * Args: ctx: pointer to a context object. + * In: fun: pointer to a function to call when an internal error occurs, * taking a message and an opaque pointer (NULL restores the * default handler, see secp256k1_context_set_illegal_callback * for details). @@ -395,7 +395,7 @@ SECP256K1_API void secp256k1_context_set_error_callback( /** Create a secp256k1 scratch space object. * * Returns: a newly created scratch space. - * Args: ctx: an existing context object. + * Args: ctx: pointer to a context object. * In: size: amount of memory to be available as scratch space. Some extra * (<100 bytes) will be allocated for extra accounting. */ @@ -407,7 +407,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT secp256k1_scratch_space *secp256k1_sc /** Destroy a secp256k1 scratch space. * * The pointer may not be used afterwards. - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object. * scratch: space to destroy */ SECP256K1_API void secp256k1_scratch_space_destroy( @@ -419,7 +419,7 @@ SECP256K1_API void secp256k1_scratch_space_destroy( * * Returns: 1 if the public key was fully valid. * 0 if the public key could not be parsed or is invalid. - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object. * Out: pubkey: pointer to a pubkey object. If 1 is returned, it is set to a * parsed version of input. If not, its value is undefined. * In: input: pointer to a serialized public key @@ -439,14 +439,14 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_parse( /** Serialize a pubkey object into a serialized byte sequence. * * Returns: 1 always. - * Args: ctx: a secp256k1 context object. - * Out: output: a pointer to a 65-byte (if compressed==0) or 33-byte (if + * Args: ctx: pointer to a context object. + * Out: output: pointer to a 65-byte (if compressed==0) or 33-byte (if * compressed==1) byte array to place the serialized key * in. - * In/Out: outputlen: a pointer to an integer which is initially set to the + * In/Out: outputlen: pointer to an integer which is initially set to the * size of output, and is overwritten with the written * size. - * In: pubkey: a pointer to a secp256k1_pubkey containing an + * In: pubkey: pointer to a secp256k1_pubkey containing an * initialized public key. * flags: SECP256K1_EC_COMPRESSED if serialization should be in * compressed format, otherwise SECP256K1_EC_UNCOMPRESSED. @@ -464,7 +464,7 @@ SECP256K1_API int secp256k1_ec_pubkey_serialize( * Returns: <0 if the first public key is less than the second * >0 if the first public key is greater than the second * 0 if the two public keys are equal - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object * In: pubkey1: first public key to compare * pubkey2: second public key to compare */ @@ -477,9 +477,9 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_cmp( /** Parse an ECDSA signature in compact (64 bytes) format. * * Returns: 1 when the signature could be parsed, 0 otherwise. - * Args: ctx: a secp256k1 context object - * Out: sig: a pointer to a signature object - * In: input64: a pointer to the 64-byte array to parse + * Args: ctx: pointer to a context object + * Out: sig: pointer to a signature object + * In: input64: pointer to the 64-byte array to parse * * The signature must consist of a 32-byte big endian R value, followed by a * 32-byte big endian S value. If R or S fall outside of [0..order-1], the @@ -498,9 +498,9 @@ SECP256K1_API int secp256k1_ecdsa_signature_parse_compact( /** Parse a DER ECDSA signature. * * Returns: 1 when the signature could be parsed, 0 otherwise. - * Args: ctx: a secp256k1 context object - * Out: sig: a pointer to a signature object - * In: input: a pointer to the signature to be parsed + * Args: ctx: pointer to a context object + * Out: sig: pointer to a signature object + * In: input: pointer to the signature to be parsed * inputlen: the length of the array pointed to be input * * This function will accept any valid DER encoded signature, even if the @@ -520,13 +520,13 @@ SECP256K1_API int secp256k1_ecdsa_signature_parse_der( /** Serialize an ECDSA signature in DER format. * * Returns: 1 if enough space was available to serialize, 0 otherwise - * Args: ctx: a secp256k1 context object - * Out: output: a pointer to an array to store the DER serialization - * In/Out: outputlen: a pointer to a length integer. Initially, this integer + * Args: ctx: pointer to a context object + * Out: output: pointer to an array to store the DER serialization + * In/Out: outputlen: pointer to a length integer. Initially, this integer * should be set to the length of output. After the call * it will be set to the length of the serialization (even * if 0 was returned). - * In: sig: a pointer to an initialized signature object + * In: sig: pointer to an initialized signature object */ SECP256K1_API int secp256k1_ecdsa_signature_serialize_der( const secp256k1_context *ctx, @@ -538,9 +538,9 @@ SECP256K1_API int secp256k1_ecdsa_signature_serialize_der( /** Serialize an ECDSA signature in compact (64 byte) format. * * Returns: 1 - * Args: ctx: a secp256k1 context object - * Out: output64: a pointer to a 64-byte array to store the compact serialization - * In: sig: a pointer to an initialized signature object + * Args: ctx: pointer to a context object + * Out: output64: pointer to a 64-byte array to store the compact serialization + * In: sig: pointer to an initialized signature object * * See secp256k1_ecdsa_signature_parse_compact for details about the encoding. */ @@ -554,7 +554,7 @@ SECP256K1_API int secp256k1_ecdsa_signature_serialize_compact( * * Returns: 1: correct signature * 0: incorrect or unparseable signature - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object * In: sig: the signature being verified. * msghash32: the 32-byte message hash being verified. * The verifier must make sure to apply a cryptographic @@ -585,12 +585,12 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_verify( /** Convert a signature to a normalized lower-S form. * * Returns: 1 if sigin was not normalized, 0 if it already was. - * Args: ctx: a secp256k1 context object - * Out: sigout: a pointer to a signature to fill with the normalized form, + * Args: ctx: pointer to a context object + * Out: sigout: pointer to a signature to fill with the normalized form, * or copy if the input was already normalized. (can be NULL if * you're only interested in whether the input was already * normalized). - * In: sigin: a pointer to a signature to check/normalize (can be identical to sigout) + * In: sigin: pointer to a signature to check/normalize (can be identical to sigout) * * With ECDSA a third-party can forge a second distinct signature of the same * message, given a single initial signature, but without knowing the key. This diff --git a/src/secp256k1/include/secp256k1_ecdh.h b/src/secp256k1/include/secp256k1_ecdh.h index 515e174299..4d9da3461d 100644 --- a/src/secp256k1/include/secp256k1_ecdh.h +++ b/src/secp256k1/include/secp256k1_ecdh.h @@ -39,7 +39,7 @@ SECP256K1_API const secp256k1_ecdh_hash_function secp256k1_ecdh_hash_function_de * 0: scalar was invalid (zero or overflow) or hashfp returned 0 * Args: ctx: pointer to a context object. * Out: output: pointer to an array to be filled by hashfp. - * In: pubkey: a pointer to a secp256k1_pubkey containing an initialized public key. + * In: pubkey: pointer to a secp256k1_pubkey containing an initialized public key. * seckey: a 32-byte scalar with which to multiply the point. * hashfp: pointer to a hash function. If NULL, * secp256k1_ecdh_hash_function_sha256 is used diff --git a/src/secp256k1/include/secp256k1_ellswift.h b/src/secp256k1/include/secp256k1_ellswift.h index f79bd88396..ae37287f82 100644 --- a/src/secp256k1/include/secp256k1_ellswift.h +++ b/src/secp256k1/include/secp256k1_ellswift.h @@ -87,7 +87,7 @@ SECP256K1_API const secp256k1_ellswift_xdh_hash_function secp256k1_ellswift_xdh_ * Returns: 1 always. * Args: ctx: pointer to a context object * Out: ell64: pointer to a 64-byte array to be filled - * In: pubkey: a pointer to a secp256k1_pubkey containing an + * In: pubkey: pointer to a secp256k1_pubkey containing an * initialized public key * rnd32: pointer to 32 bytes of randomness * @@ -169,7 +169,7 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ellswift_create( * (will not be NULL) * ell_b64: pointer to the 64-byte encoded public key of party B * (will not be NULL) - * seckey32: a pointer to our 32-byte secret key + * seckey32: pointer to our 32-byte secret key * party: boolean indicating which party we are: zero if we are * party A, non-zero if we are party B. seckey32 must be * the private key corresponding to that party's ell_?64. diff --git a/src/secp256k1/include/secp256k1_extrakeys.h b/src/secp256k1/include/secp256k1_extrakeys.h index 002153eccd..f74d067d6f 100644 --- a/src/secp256k1/include/secp256k1_extrakeys.h +++ b/src/secp256k1/include/secp256k1_extrakeys.h @@ -39,7 +39,7 @@ typedef struct { * Returns: 1 if the public key was fully valid. * 0 if the public key could not be parsed or is invalid. * - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object. * Out: pubkey: pointer to a pubkey object. If 1 is returned, it is set to a * parsed version of input. If not, it's set to an invalid value. * In: input32: pointer to a serialized xonly_pubkey. @@ -54,9 +54,9 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_xonly_pubkey_parse( * * Returns: 1 always. * - * Args: ctx: a secp256k1 context object. - * Out: output32: a pointer to a 32-byte array to place the serialized key in. - * In: pubkey: a pointer to a secp256k1_xonly_pubkey containing an initialized public key. + * Args: ctx: pointer to a context object. + * Out: output32: pointer to a 32-byte array to place the serialized key in. + * In: pubkey: pointer to a secp256k1_xonly_pubkey containing an initialized public key. */ SECP256K1_API int secp256k1_xonly_pubkey_serialize( const secp256k1_context *ctx, @@ -69,7 +69,7 @@ SECP256K1_API int secp256k1_xonly_pubkey_serialize( * Returns: <0 if the first public key is less than the second * >0 if the first public key is greater than the second * 0 if the two public keys are equal - * Args: ctx: a secp256k1 context object. + * Args: ctx: pointer to a context object. * In: pubkey1: first public key to compare * pubkey2: second public key to compare */ diff --git a/src/secp256k1/include/secp256k1_preallocated.h b/src/secp256k1/include/secp256k1_preallocated.h index 2f67031ec9..4fdb0c7cb4 100644 --- a/src/secp256k1/include/secp256k1_preallocated.h +++ b/src/secp256k1/include/secp256k1_preallocated.h @@ -52,8 +52,8 @@ SECP256K1_API size_t secp256k1_context_preallocated_size( * in the memory. In simpler words, the prealloc pointer (or any pointer derived * from it) should not be used during the lifetime of the context object. * - * Returns: a newly created context object. - * In: prealloc: a pointer to a rewritable contiguous block of memory of + * Returns: pointer to newly created context object. + * In: prealloc: pointer to a rewritable contiguous block of memory of * size at least secp256k1_context_preallocated_size(flags) * bytes, as detailed above. * flags: which parts of the context to initialize. @@ -70,7 +70,7 @@ SECP256K1_API secp256k1_context *secp256k1_context_preallocated_create( * caller-provided memory. * * Returns: the required size of the caller-provided memory block. - * In: ctx: an existing context to copy. + * In: ctx: pointer to a context to copy. */ SECP256K1_API size_t secp256k1_context_preallocated_clone_size( const secp256k1_context *ctx @@ -89,9 +89,9 @@ SECP256K1_API size_t secp256k1_context_preallocated_clone_size( * Cloning secp256k1_context_static is not possible, and should not be emulated by * the caller (e.g., using memcpy). Create a new context instead. * - * Returns: a newly created context object. - * Args: ctx: an existing context to copy (not secp256k1_context_static). - * In: prealloc: a pointer to a rewritable contiguous block of memory of + * Returns: pointer to a newly created context object. + * Args: ctx: pointer to a context to copy (not secp256k1_context_static). + * In: prealloc: pointer to a rewritable contiguous block of memory of * size at least secp256k1_context_preallocated_size(flags) * bytes, as detailed above. */ @@ -116,7 +116,7 @@ SECP256K1_API secp256k1_context *secp256k1_context_preallocated_clone( * preallocated pointer given to secp256k1_context_preallocated_create or * secp256k1_context_preallocated_clone. * - * Args: ctx: an existing context to destroy, constructed using + * Args: ctx: pointer to a context to destroy, constructed using * secp256k1_context_preallocated_create or * secp256k1_context_preallocated_clone * (i.e., not secp256k1_context_static). diff --git a/src/secp256k1/include/secp256k1_recovery.h b/src/secp256k1/include/secp256k1_recovery.h index 16f0c71287..ba7ae31017 100644 --- a/src/secp256k1/include/secp256k1_recovery.h +++ b/src/secp256k1/include/secp256k1_recovery.h @@ -28,9 +28,9 @@ typedef struct { /** Parse a compact ECDSA signature (64 bytes + recovery id). * * Returns: 1 when the signature could be parsed, 0 otherwise - * Args: ctx: a secp256k1 context object - * Out: sig: a pointer to a signature object - * In: input64: a pointer to a 64-byte compact signature + * Args: ctx: pointer to a context object + * Out: sig: pointer to a signature object + * In: input64: pointer to a 64-byte compact signature * recid: the recovery id (0, 1, 2 or 3) */ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_parse_compact( @@ -43,9 +43,9 @@ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_parse_compact( /** Convert a recoverable signature into a normal signature. * * Returns: 1 - * Args: ctx: a secp256k1 context object. - * Out: sig: a pointer to a normal signature. - * In: sigin: a pointer to a recoverable signature. + * Args: ctx: pointer to a context object. + * Out: sig: pointer to a normal signature. + * In: sigin: pointer to a recoverable signature. */ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_convert( const secp256k1_context *ctx, @@ -56,10 +56,10 @@ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_convert( /** Serialize an ECDSA signature in compact format (64 bytes + recovery id). * * Returns: 1 - * Args: ctx: a secp256k1 context object. - * Out: output64: a pointer to a 64-byte array of the compact signature. - * recid: a pointer to an integer to hold the recovery id. - * In: sig: a pointer to an initialized signature object. + * Args: ctx: pointer to a context object. + * Out: output64: pointer to a 64-byte array of the compact signature. + * recid: pointer to an integer to hold the recovery id. + * In: sig: pointer to an initialized signature object. */ SECP256K1_API int secp256k1_ecdsa_recoverable_signature_serialize_compact( const secp256k1_context *ctx, diff --git a/src/secp256k1/include/secp256k1_schnorrsig.h b/src/secp256k1/include/secp256k1_schnorrsig.h index 8ccccd24f8..5398655f13 100644 --- a/src/secp256k1/include/secp256k1_schnorrsig.h +++ b/src/secp256k1/include/secp256k1_schnorrsig.h @@ -169,11 +169,11 @@ SECP256K1_API int secp256k1_schnorrsig_sign_custom( * * Returns: 1: correct signature * 0: incorrect signature - * Args: ctx: a secp256k1 context object, initialized for verification. + * Args: ctx: pointer to a context object. * In: sig64: pointer to the 64-byte signature to verify. * msg: the message being verified. Can only be NULL if msglen is 0. * msglen: length of the message - * pubkey: pointer to an x-only public key to verify with (cannot be NULL) + * pubkey: pointer to an x-only public key to verify with */ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_schnorrsig_verify( const secp256k1_context *ctx, diff --git a/src/secp256k1/src/field.h b/src/secp256k1/src/field.h index bd589bf8a8..8c65a3aff6 100644 --- a/src/secp256k1/src/field.h +++ b/src/secp256k1/src/field.h @@ -255,8 +255,8 @@ static void secp256k1_fe_add(secp256k1_fe *r, const secp256k1_fe *a); /** Multiply two field elements. * * On input, a and b must be valid field elements; r does not need to be initialized. - * r and a may point to the same object, but neither can be equal to b. The magnitudes - * of a and b must not exceed 8. + * r and a may point to the same object, but neither may point to the object pointed + * to by b. The magnitudes of a and b must not exceed 8. * Performs {r = a * b} * On output, r will have magnitude 1, but won't be normalized. */ diff --git a/src/secp256k1/src/modules/ellswift/tests_impl.h b/src/secp256k1/src/modules/ellswift/tests_impl.h index 7d1efbc492..f96e3a1268 100644 --- a/src/secp256k1/src/modules/ellswift/tests_impl.h +++ b/src/secp256k1/src/modules/ellswift/tests_impl.h @@ -188,9 +188,9 @@ void run_ellswift_tests(void) { CHECK(ret == ((testcase->enc_bitmap >> c) & 1)); if (ret) { secp256k1_fe x2; - CHECK(check_fe_equal(&t, &testcase->encs[c])); + CHECK(fe_equal(&t, &testcase->encs[c])); secp256k1_ellswift_xswiftec_var(&x2, &testcase->u, &testcase->encs[c]); - CHECK(check_fe_equal(&testcase->x, &x2)); + CHECK(fe_equal(&testcase->x, &x2)); } } } @@ -203,7 +203,7 @@ void run_ellswift_tests(void) { CHECK(ret); ret = secp256k1_pubkey_load(CTX, &ge, &pubkey); CHECK(ret); - CHECK(check_fe_equal(&testcase->x, &ge.x)); + CHECK(fe_equal(&testcase->x, &ge.x)); CHECK(secp256k1_fe_is_odd(&ge.y) == testcase->odd_y); } for (i = 0; (unsigned)i < sizeof(ellswift_xdh_tests_bip324) / sizeof(ellswift_xdh_tests_bip324[0]); ++i) { @@ -290,7 +290,7 @@ void run_ellswift_tests(void) { secp256k1_ecmult(&resj, &decj, &sec, NULL); secp256k1_ge_set_gej(&res, &resj); /* Compare. */ - CHECK(check_fe_equal(&res.x, &share_x)); + CHECK(fe_equal(&res.x, &share_x)); } /* Verify the joint behavior of secp256k1_ellswift_xdh */ for (i = 0; i < 200 * COUNT; i++) { diff --git a/src/secp256k1/src/scalar_4x64_impl.h b/src/secp256k1/src/scalar_4x64_impl.h index c9f190e712..dc60d2caf7 100644 --- a/src/secp256k1/src/scalar_4x64_impl.h +++ b/src/secp256k1/src/scalar_4x64_impl.h @@ -460,6 +460,14 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l) : "S"(l), "i"(SECP256K1_N_C_0), "i"(SECP256K1_N_C_1) : "rax", "rdx", "r8", "r9", "r10", "r11", "r12", "r13", "r14", "cc"); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m0, sizeof(m0)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m1, sizeof(m1)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m2, sizeof(m2)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m3, sizeof(m3)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m4, sizeof(m4)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m5, sizeof(m5)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&m6, sizeof(m6)); + /* Reduce 385 bits into 258. */ __asm__ __volatile__( /* Preload */ @@ -539,6 +547,12 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l) : "g"(m0), "g"(m1), "g"(m2), "g"(m3), "g"(m4), "g"(m5), "g"(m6), "i"(SECP256K1_N_C_0), "i"(SECP256K1_N_C_1) : "rax", "rdx", "r8", "r9", "r10", "r11", "r12", "r13", "cc"); + SECP256K1_CHECKMEM_MSAN_DEFINE(&p0, sizeof(p0)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&p1, sizeof(p1)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&p2, sizeof(p2)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&p3, sizeof(p3)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&p4, sizeof(p4)); + /* Reduce 258 bits into 256. */ __asm__ __volatile__( /* Preload */ @@ -584,6 +598,10 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l) : "=g"(c) : "g"(p0), "g"(p1), "g"(p2), "g"(p3), "g"(p4), "D"(r), "i"(SECP256K1_N_C_0), "i"(SECP256K1_N_C_1) : "rax", "rdx", "r8", "r9", "r10", "cc", "memory"); + + SECP256K1_CHECKMEM_MSAN_DEFINE(r, sizeof(*r)); + SECP256K1_CHECKMEM_MSAN_DEFINE(&c, sizeof(c)); + #else uint128_t c; uint64_t c0, c1, c2; @@ -657,7 +675,7 @@ static void secp256k1_scalar_reduce_512(secp256k1_scalar *r, const uint64_t *l) secp256k1_scalar_reduce(r, c + secp256k1_scalar_check_overflow(r)); } -static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, const secp256k1_scalar *b) { +static void secp256k1_scalar_mul_512(uint64_t *l8, const secp256k1_scalar *a, const secp256k1_scalar *b) { #ifdef USE_ASM_X86_64 const uint64_t *pb = b->d; __asm__ __volatile__( @@ -672,7 +690,7 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c /* (rax,rdx) = a0 * b0 */ "movq %%r15, %%rax\n" "mulq %%r11\n" - /* Extract l0 */ + /* Extract l8[0] */ "movq %%rax, 0(%%rsi)\n" /* (r8,r9,r10) = (rdx) */ "movq %%rdx, %%r8\n" @@ -690,7 +708,7 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c "addq %%rax, %%r8\n" "adcq %%rdx, %%r9\n" "adcq $0, %%r10\n" - /* Extract l1 */ + /* Extract l8[1] */ "movq %%r8, 8(%%rsi)\n" "xorq %%r8, %%r8\n" /* (r9,r10,r8) += a0 * b2 */ @@ -711,7 +729,7 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c "addq %%rax, %%r9\n" "adcq %%rdx, %%r10\n" "adcq $0, %%r8\n" - /* Extract l2 */ + /* Extract l8[2] */ "movq %%r9, 16(%%rsi)\n" "xorq %%r9, %%r9\n" /* (r10,r8,r9) += a0 * b3 */ @@ -740,7 +758,7 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c "addq %%rax, %%r10\n" "adcq %%rdx, %%r8\n" "adcq $0, %%r9\n" - /* Extract l3 */ + /* Extract l8[3] */ "movq %%r10, 24(%%rsi)\n" "xorq %%r10, %%r10\n" /* (r8,r9,r10) += a1 * b3 */ @@ -761,7 +779,7 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c "addq %%rax, %%r8\n" "adcq %%rdx, %%r9\n" "adcq $0, %%r10\n" - /* Extract l4 */ + /* Extract l8[4] */ "movq %%r8, 32(%%rsi)\n" "xorq %%r8, %%r8\n" /* (r9,r10,r8) += a2 * b3 */ @@ -776,51 +794,54 @@ static void secp256k1_scalar_mul_512(uint64_t l[8], const secp256k1_scalar *a, c "addq %%rax, %%r9\n" "adcq %%rdx, %%r10\n" "adcq $0, %%r8\n" - /* Extract l5 */ + /* Extract l8[5] */ "movq %%r9, 40(%%rsi)\n" /* (r10,r8) += a3 * b3 */ "movq %%r15, %%rax\n" "mulq %%r14\n" "addq %%rax, %%r10\n" "adcq %%rdx, %%r8\n" - /* Extract l6 */ + /* Extract l8[6] */ "movq %%r10, 48(%%rsi)\n" - /* Extract l7 */ + /* Extract l8[7] */ "movq %%r8, 56(%%rsi)\n" : "+d"(pb) - : "S"(l), "D"(a->d) + : "S"(l8), "D"(a->d) : "rax", "rbx", "rcx", "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15", "cc", "memory"); + + SECP256K1_CHECKMEM_MSAN_DEFINE(l8, sizeof(*l8) * 8); + #else /* 160 bit accumulator. */ uint64_t c0 = 0, c1 = 0; uint32_t c2 = 0; - /* l[0..7] = a[0..3] * b[0..3]. */ + /* l8[0..7] = a[0..3] * b[0..3]. */ muladd_fast(a->d[0], b->d[0]); - extract_fast(l[0]); + extract_fast(l8[0]); muladd(a->d[0], b->d[1]); muladd(a->d[1], b->d[0]); - extract(l[1]); + extract(l8[1]); muladd(a->d[0], b->d[2]); muladd(a->d[1], b->d[1]); muladd(a->d[2], b->d[0]); - extract(l[2]); + extract(l8[2]); muladd(a->d[0], b->d[3]); muladd(a->d[1], b->d[2]); muladd(a->d[2], b->d[1]); muladd(a->d[3], b->d[0]); - extract(l[3]); + extract(l8[3]); muladd(a->d[1], b->d[3]); muladd(a->d[2], b->d[2]); muladd(a->d[3], b->d[1]); - extract(l[4]); + extract(l8[4]); muladd(a->d[2], b->d[3]); muladd(a->d[3], b->d[2]); - extract(l[5]); + extract(l8[5]); muladd_fast(a->d[3], b->d[3]); - extract_fast(l[6]); + extract_fast(l8[6]); VERIFY_CHECK(c1 == 0); - l[7] = c0; + l8[7] = c0; #endif } diff --git a/src/secp256k1/src/scalar_impl.h b/src/secp256k1/src/scalar_impl.h index bbba83e937..972d8041b0 100644 --- a/src/secp256k1/src/scalar_impl.h +++ b/src/secp256k1/src/scalar_impl.h @@ -229,7 +229,7 @@ static void secp256k1_scalar_split_lambda(secp256k1_scalar * SECP256K1_RESTRICT * <= {triangle inequality} * a1*|k*b2/n - c1| + a2*|k*(-b1)/n - c2| * < {Lemma 1 and Lemma 2} - * a1*(2^-1 + epslion1) + a2*(2^-1 + epsilon2) + * a1*(2^-1 + epsilon1) + a2*(2^-1 + epsilon2) * < {rounding up to an integer} * (a1 + a2 + 1)/2 * < {rounding up to a power of 2} @@ -247,7 +247,7 @@ static void secp256k1_scalar_split_lambda(secp256k1_scalar * SECP256K1_RESTRICT * <= {triangle inequality} * (-b1)*|k*b2/n - c1| + b2*|k*(-b1)/n - c2| * < {Lemma 1 and Lemma 2} - * (-b1)*(2^-1 + epslion1) + b2*(2^-1 + epsilon2) + * (-b1)*(2^-1 + epsilon1) + b2*(2^-1 + epsilon2) * < {rounding up to an integer} * (-b1 + b2)/2 + 1 * < {rounding up to a power of 2} diff --git a/src/secp256k1/src/secp256k1.c b/src/secp256k1/src/secp256k1.c index 4c11e7f0b8..15a5eede67 100644 --- a/src/secp256k1/src/secp256k1.c +++ b/src/secp256k1/src/secp256k1.c @@ -237,36 +237,25 @@ static SECP256K1_INLINE void secp256k1_declassify(const secp256k1_context* ctx, } static int secp256k1_pubkey_load(const secp256k1_context* ctx, secp256k1_ge* ge, const secp256k1_pubkey* pubkey) { - if (sizeof(secp256k1_ge_storage) == 64) { - /* When the secp256k1_ge_storage type is exactly 64 byte, use its - * representation inside secp256k1_pubkey, as conversion is very fast. - * Note that secp256k1_pubkey_save must use the same representation. */ - secp256k1_ge_storage s; - memcpy(&s, &pubkey->data[0], sizeof(s)); - secp256k1_ge_from_storage(ge, &s); - } else { - /* Otherwise, fall back to 32-byte big endian for X and Y. */ - secp256k1_fe x, y; - ARG_CHECK(secp256k1_fe_set_b32_limit(&x, pubkey->data)); - ARG_CHECK(secp256k1_fe_set_b32_limit(&y, pubkey->data + 32)); - secp256k1_ge_set_xy(ge, &x, &y); - } + secp256k1_ge_storage s; + + /* We require that the secp256k1_ge_storage type is exactly 64 bytes. + * This is formally not guaranteed by the C standard, but should hold on any + * sane compiler in the real world. */ + STATIC_ASSERT(sizeof(secp256k1_ge_storage) == 64); + memcpy(&s, &pubkey->data[0], 64); + secp256k1_ge_from_storage(ge, &s); ARG_CHECK(!secp256k1_fe_is_zero(&ge->x)); return 1; } static void secp256k1_pubkey_save(secp256k1_pubkey* pubkey, secp256k1_ge* ge) { - if (sizeof(secp256k1_ge_storage) == 64) { - secp256k1_ge_storage s; - secp256k1_ge_to_storage(&s, ge); - memcpy(&pubkey->data[0], &s, sizeof(s)); - } else { - VERIFY_CHECK(!secp256k1_ge_is_infinity(ge)); - secp256k1_fe_normalize_var(&ge->x); - secp256k1_fe_normalize_var(&ge->y); - secp256k1_fe_get_b32(pubkey->data, &ge->x); - secp256k1_fe_get_b32(pubkey->data + 32, &ge->y); - } + secp256k1_ge_storage s; + + STATIC_ASSERT(sizeof(secp256k1_ge_storage) == 64); + VERIFY_CHECK(!secp256k1_ge_is_infinity(ge)); + secp256k1_ge_to_storage(&s, ge); + memcpy(&pubkey->data[0], &s, 64); } int secp256k1_ec_pubkey_parse(const secp256k1_context* ctx, secp256k1_pubkey* pubkey, const unsigned char *input, size_t inputlen) { diff --git a/src/secp256k1/src/tests.c b/src/secp256k1/src/tests.c index d7da8dd3a9..67608a6641 100644 --- a/src/secp256k1/src/tests.c +++ b/src/secp256k1/src/tests.c @@ -2928,20 +2928,18 @@ static void run_scalar_tests(void) { secp256k1_scalar_set_b32(&r2, res[i][1], &overflow); CHECK(!overflow); secp256k1_scalar_mul(&z, &x, &y); - CHECK(!secp256k1_scalar_check_overflow(&z)); CHECK(secp256k1_scalar_eq(&r1, &z)); if (!secp256k1_scalar_is_zero(&y)) { secp256k1_scalar_inverse(&zz, &y); - CHECK(!secp256k1_scalar_check_overflow(&zz)); secp256k1_scalar_inverse_var(&zzv, &y); CHECK(secp256k1_scalar_eq(&zzv, &zz)); secp256k1_scalar_mul(&z, &z, &zz); - CHECK(!secp256k1_scalar_check_overflow(&z)); CHECK(secp256k1_scalar_eq(&x, &z)); secp256k1_scalar_mul(&zz, &zz, &y); - CHECK(!secp256k1_scalar_check_overflow(&zz)); CHECK(secp256k1_scalar_eq(&secp256k1_scalar_one, &zz)); } + secp256k1_scalar_mul(&z, &x, &x); + CHECK(secp256k1_scalar_eq(&r2, &z)); } } } @@ -2956,7 +2954,7 @@ static void random_fe_non_square(secp256k1_fe *ns) { } } -static int check_fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) { +static int fe_equal(const secp256k1_fe *a, const secp256k1_fe *b) { secp256k1_fe an = *a; secp256k1_fe bn = *b; secp256k1_fe_normalize_weak(&an); @@ -3093,7 +3091,7 @@ static void run_field_half(void) { #endif secp256k1_fe_normalize_weak(&u); secp256k1_fe_add(&u, &u); - CHECK(check_fe_equal(&t, &u)); + CHECK(fe_equal(&t, &u)); /* Check worst-case input: ensure the LSB is 1 so that P will be added, * which will also cause all carries to be 1, since all limbs that can @@ -3112,7 +3110,7 @@ static void run_field_half(void) { #endif secp256k1_fe_normalize_weak(&u); secp256k1_fe_add(&u, &u); - CHECK(check_fe_equal(&t, &u)); + CHECK(fe_equal(&t, &u)); } } @@ -3139,7 +3137,7 @@ static void run_field_misc(void) { secp256k1_fe_add(&z, &q); /* z = x+v */ q = x; /* q = x */ secp256k1_fe_add_int(&q, v); /* q = x+v */ - CHECK(check_fe_equal(&q, &z)); + CHECK(fe_equal(&q, &z)); /* Test the fe equality and comparison operations. */ CHECK(secp256k1_fe_cmp_var(&x, &x) == 0); CHECK(secp256k1_fe_equal(&x, &x)); @@ -3199,27 +3197,27 @@ static void run_field_misc(void) { secp256k1_fe_add(&y, &x); z = x; secp256k1_fe_mul_int(&z, 3); - CHECK(check_fe_equal(&y, &z)); + CHECK(fe_equal(&y, &z)); secp256k1_fe_add(&y, &x); secp256k1_fe_add(&z, &x); - CHECK(check_fe_equal(&z, &y)); + CHECK(fe_equal(&z, &y)); z = x; secp256k1_fe_mul_int(&z, 5); secp256k1_fe_mul(&q, &x, &fe5); - CHECK(check_fe_equal(&z, &q)); + CHECK(fe_equal(&z, &q)); secp256k1_fe_negate(&x, &x, 1); secp256k1_fe_add(&z, &x); secp256k1_fe_add(&q, &x); - CHECK(check_fe_equal(&y, &z)); - CHECK(check_fe_equal(&q, &y)); + CHECK(fe_equal(&y, &z)); + CHECK(fe_equal(&q, &y)); /* Check secp256k1_fe_half. */ z = x; secp256k1_fe_half(&z); secp256k1_fe_add(&z, &z); - CHECK(check_fe_equal(&x, &z)); + CHECK(fe_equal(&x, &z)); secp256k1_fe_add(&z, &z); secp256k1_fe_half(&z); - CHECK(check_fe_equal(&x, &z)); + CHECK(fe_equal(&x, &z)); } } @@ -3288,18 +3286,31 @@ static void run_fe_mul(void) { } static void run_sqr(void) { - secp256k1_fe x, s; + int i; + secp256k1_fe x, y, lhs, rhs, tmp; - { - int i; - secp256k1_fe_set_int(&x, 1); - secp256k1_fe_negate(&x, &x, 1); + secp256k1_fe_set_int(&x, 1); + secp256k1_fe_negate(&x, &x, 1); - for (i = 1; i <= 512; ++i) { - secp256k1_fe_mul_int(&x, 2); - secp256k1_fe_normalize(&x); - secp256k1_fe_sqr(&s, &x); - } + for (i = 1; i <= 512; ++i) { + secp256k1_fe_mul_int(&x, 2); + secp256k1_fe_normalize(&x); + + /* Check that (x+y)*(x-y) = x^2 - y*2 for some random values y */ + random_fe_test(&y); + + lhs = x; + secp256k1_fe_add(&lhs, &y); /* lhs = x+y */ + secp256k1_fe_negate(&tmp, &y, 1); /* tmp = -y */ + secp256k1_fe_add(&tmp, &x); /* tmp = x-y */ + secp256k1_fe_mul(&lhs, &lhs, &tmp); /* lhs = (x+y)*(x-y) */ + + secp256k1_fe_sqr(&rhs, &x); /* rhs = x^2 */ + secp256k1_fe_sqr(&tmp, &y); /* tmp = y^2 */ + secp256k1_fe_negate(&tmp, &tmp, 1); /* tmp = -y^2 */ + secp256k1_fe_add(&rhs, &tmp); /* rhs = x^2 - y^2 */ + + CHECK(fe_equal(&lhs, &rhs)); } } @@ -3621,9 +3632,9 @@ static void run_inverse_tests(void) for (i = 0; (size_t)i < sizeof(fe_cases)/sizeof(fe_cases[0]); ++i) { for (var = 0; var <= 1; ++var) { test_inverse_field(&x_fe, &fe_cases[i][0], var); - check_fe_equal(&x_fe, &fe_cases[i][1]); + CHECK(fe_equal(&x_fe, &fe_cases[i][1])); test_inverse_field(&x_fe, &fe_cases[i][1], var); - check_fe_equal(&x_fe, &fe_cases[i][0]); + CHECK(fe_equal(&x_fe, &fe_cases[i][0])); } } for (i = 0; (size_t)i < sizeof(scalar_cases)/sizeof(scalar_cases[0]); ++i) { @@ -4564,7 +4575,7 @@ static void ecmult_const_mult_xonly(void) { /* Check that resj's X coordinate corresponds with resx. */ secp256k1_fe_sqr(&v, &resj.z); secp256k1_fe_mul(&v, &v, &resx); - CHECK(check_fe_equal(&v, &resj.x)); + CHECK(fe_equal(&v, &resj.x)); } /* Test that secp256k1_ecmult_const_xonly correctly rejects X coordinates not on curve. */ diff --git a/src/secp256k1/src/util.h b/src/secp256k1/src/util.h index 187bf1c5e0..154d9ebcf1 100644 --- a/src/secp256k1/src/util.h +++ b/src/secp256k1/src/util.h @@ -51,13 +51,27 @@ static void print_buf_plain(const unsigned char *buf, size_t len) { # define SECP256K1_INLINE inline # endif +/** Assert statically that expr is true. + * + * This is a statement-like macro and can only be used inside functions. + */ +#define STATIC_ASSERT(expr) do { \ + switch(0) { \ + case 0: \ + /* If expr evaluates to 0, we have two case labels "0", which is illegal. */ \ + case /* ERROR: static assertion failed */ (expr): \ + ; \ + } \ +} while(0) + /** 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: \ + /* C allows only integer constant expressions as case labels. */ \ + case /* ERROR: integer argument is not constant */ (expr): \ break; \ default: ; \ } \ diff --git a/src/secp256k1/tools/check-abi.sh b/src/secp256k1/tools/check-abi.sh index 8f6119cd8e..55c945ac16 100755 --- a/src/secp256k1/tools/check-abi.sh +++ b/src/secp256k1/tools/check-abi.sh @@ -3,17 +3,19 @@ set -eu default_base_version="$(git describe --match "v*.*.*" --abbrev=0)" -default_new_version="master" +default_new_version="HEAD" display_help_and_exit() { - echo "Usage: $0 " + echo "Usage: $0 [ []]" echo "" echo "Description: This script uses the ABI Compliance Checker tool to determine if the ABI" echo " of a new version of libsecp256k1 has changed in a backward-incompatible way." echo "" echo "Options:" - echo " base_ver Specify the base version (default: $default_base_version)" - echo " new_ver Specify the new version (default: $default_new_version)" + echo " base_ver Specify the base version as a git commit-ish" + echo " (default: most recent reachable tag matching \"v.*.*\", currently \"$default_base_version\")" + echo " new_ver Specify the new version as a git commit-ish" + echo " (default: $default_new_version)" echo " -h, --help Display this help message" exit 0 } @@ -23,9 +25,11 @@ if [ "$#" -eq 0 ]; then new_version="$default_new_version" elif [ "$#" -eq 1 ] && { [ "$1" = "-h" ] || [ "$1" = "--help" ]; }; then display_help_and_exit -elif [ "$#" -eq 2 ]; then +elif [ "$#" -eq 1 ] || [ "$#" -eq 2 ]; then base_version="$1" - new_version="$2" + if [ "$#" -eq 2 ]; then + new_version="$2" + fi else echo "Invalid usage. See help:" echo "" @@ -33,7 +37,8 @@ else fi checkout_and_build() { - git worktree add -d "$1" "$2" + _orig_dir="$(pwd)" + git worktree add --detach "$1" "$2" cd "$1" mkdir build && cd build cmake -S .. --preset dev-mode \ @@ -45,20 +50,18 @@ checkout_and_build() { -DSECP256K1_BUILD_EXAMPLES=OFF cmake --build . -j "$(nproc)" abi-dumper src/libsecp256k1.so -o ABI.dump -lver "$2" + cd "$_orig_dir" } echo "Comparing $base_version (base version) to $new_version (new version)" echo -original_dir="$(pwd)" - -base_source_dir=$(mktemp -d) +base_source_dir="$(mktemp -d)" checkout_and_build "$base_source_dir" "$base_version" -new_source_dir=$(mktemp -d) +new_source_dir="$(mktemp -d)" checkout_and_build "$new_source_dir" "$new_version" -cd "$original_dir" abi-compliance-checker -lib libsecp256k1 -old "${base_source_dir}/build/ABI.dump" -new "${new_source_dir}/build/ABI.dump" git worktree remove "$base_source_dir" git worktree remove "$new_source_dir"