Squashed 'src/secp256k1/' changes from 06bff6dec8..4af241b320

4af241b320 Merge bitcoin-core/secp256k1#1535: build: Replace hardcoded "auto" value with default one f473c959f0 Merge bitcoin-core/secp256k1#1543: cmake: Do not modify build types when integrating by downstream project d403eea484 Merge bitcoin-core/secp256k1#1546: cmake: Rename `SECP256K1_LATE_CFLAGS` and switch to Bitcoin Core's approach d7ae25ce6f Merge bitcoin-core/secp256k1#1550: fix: typos in secp256k1.c 0e2fadb20c fix: typos in secp256k1.c 69b2192ad4 Merge bitcoin-core/secp256k1#1545: cmake: Do not set `CTEST_TEST_TARGET_ALIAS` 5dd637f3cf Merge bitcoin-core/secp256k1#1548: README: mention ellswift module 7454a53736 README: mention ellswift module 4706be2cd0 cmake: Reimplement `SECP256K1_APPEND_CFLAGS` using Bitcoin Core approach c2764dbb99 cmake: Rename `SECP256K1_LATE_CFLAGS` to `SECP256K1_APPEND_CFLAGS` f87a3589f4 cmake: Do not set `CTEST_TEST_TARGET_ALIAS` 158f9e5eae cmake: Do not modify build types when integrating by downstream project 35c0fdc86b Merge bitcoin-core/secp256k1#1529: cmake: Fix cache issue when integrating by downstream project 4392f0f717 Merge bitcoin-core/secp256k1#1533: tests: refactor: tidy up util functions (bitcoin#1491) bedffd53d8 Merge bitcoin-core/secp256k1#1488: ci: Add native macOS arm64 job 4b8d5eeacf Merge bitcoin-core/secp256k1#1532: cmake: Disable eager MSan in ctime_tests f55703ba49 autotools: Delete unneeded compiler test 396e885886 autotools: Align MSan checking code with CMake's implementation abde59f52d cmake: Report more compiler details in summary 7abf979a43 cmake: Disable `ctime_tests` if build with `-fsanitize=memory` 4d9645bee0 cmake: Remove "AUTO" value of `SECP256K1_ECMULT_GEN_KB` option a06805ee74 cmake: Remove "AUTO" value of `SECP256K1_ECMULT_WINDOW_SIZE` option 1791f6fce4 Merge bitcoin-core/secp256k1#1517: autotools: Disable eager MSan in ctime_tests 26b94ee92a autotools: Remove "auto" value of `--with-ecmult-gen-kb` option 122dbaeb37 autotools: Remove "auto" value of `--with-ecmult-window` option e73f6f8fd9 tests: refactor: drop `secp256k1_` prefix from testrand.h functions 0ee7453a99 tests: refactor: add `testutil_` prefix to testutil.h functions 0c6bc76dcd tests: refactor: move `random_` helpers from tests.c to testutil.h 0fef8479be tests: refactor: rename `random_field_element_magnitude` -> `random_fe_magnitude` 59db007f0f tests: refactor: rename `random_group_element_...` -> `random_ge_...` ebfb82ee2f ci: Add job with -fsanitize-memory-param-retval e1bef0961c configure: Move "experimental" warning to bottom 55e5d975db autotools: Disable eager MSan in ctime_tests ec4c002faa cmake: Simplify `PROJECT_IS_TOP_LEVEL` emulation cae9a7ad14 cmake: Do not set emulated PROJECT_IS_TOP_LEVEL as cache variable 218f0cc93b ci: Add native macOS arm64 job git-subtree-dir: src/secp256k1 git-subtree-split: 4af241b32099067464e015fa66daac5096206dea
fanquake · Jun 25, 2024 · 1408944 · 1408944
1 parent ca3d945
commit 1408944
Show file tree

Hide file tree

Showing 20 changed files with 596 additions and 494 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -10,8 +10,8 @@ env:
   MAKEFLAGS: -j4
   BUILD: check
   ### secp256k1 config
-  ECMULTWINDOW: auto
-  ECMULTGENKB: auto
+  ECMULTWINDOW: 15
+  ECMULTGENKB: 22
   ASM: no
   WIDEMUL: auto
   WITH_VALGRIND: yes

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -21,8 +21,8 @@ env:
   MAKEFLAGS: '-j4'
   BUILD: 'check'
   ### secp256k1 config
-  ECMULTWINDOW: 'auto'
-  ECMULTGENKB: 'auto'
+  ECMULTWINDOW: 15
+  ECMULTGENKB: 22
   ASM: 'no'
   WIDEMUL: 'auto'
   WITH_VALGRIND: 'yes'
@@ -485,18 +485,24 @@ jobs:
       matrix:
         configuration:
           - env_vars:
+              CTIMETESTS: 'yes'
               CFLAGS: '-fsanitize=memory -fsanitize-recover=memory -g'
           - env_vars:
               ECMULTGENKB: 2
               ECMULTWINDOW: 2
+              CTIMETESTS: 'yes'
               CFLAGS: '-fsanitize=memory -fsanitize-recover=memory -g -O3'
+          - env_vars:
+              # -fsanitize-memory-param-retval is clang's default, but our build system disables it
+              # when ctime_tests when enabled.
+              CFLAGS: '-fsanitize=memory -fsanitize-recover=memory -fsanitize-memory-param-retval -g'
+              CTIMETESTS: 'no'
 
     env:
       ECDH: 'yes'
       RECOVERY: 'yes'
       SCHNORRSIG: 'yes'
       ELLSWIFT: 'yes'
-      CTIMETESTS: 'yes'
       CC: 'clang'
       SECP256K1_TEST_ITERS: 32
       ASM: 'no'
@@ -585,10 +591,10 @@ jobs:
         run: env
         if: ${{ always() }}
 
-  macos-native:
-    name: "x86_64: macOS Monterey"
+  x86_64-macos-native:
+    name: "x86_64: macOS Monterey, Valgrind"
     # See: https://github.com/actions/runner-images#available-images.
-    runs-on: macos-12 # Use M1 once available https://github.com/github/roadmap/issues/528
+    runs-on: macos-12
 
     env:
       CC: 'clang'
@@ -644,6 +650,62 @@ jobs:
         run: env
         if: ${{ always() }}
 
+  arm64-macos-native:
+    name: "ARM64: macOS Sonoma"
+    # See: https://github.com/actions/runner-images#available-images.
+    runs-on: macos-14
+
+    env:
+      CC: 'clang'
+      HOMEBREW_NO_AUTO_UPDATE: 1
+      HOMEBREW_NO_INSTALL_CLEANUP: 1
+      WITH_VALGRIND: 'no'
+      CTIMETESTS: 'no'
+
+    strategy:
+      fail-fast: false
+      matrix:
+        env_vars:
+          - { WIDEMUL: 'int64',  RECOVERY: 'yes', ECDH: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes' }
+          - { WIDEMUL: 'int128_struct', ECMULTGENPRECISION: 2, ECMULTWINDOW: 4 }
+          - { WIDEMUL: 'int128',                  ECDH: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes' }
+          - { WIDEMUL: 'int128', RECOVERY: 'yes' }
+          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes' }
+          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes', CC: 'gcc' }
+          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', SCHNORRSIG: 'yes', ELLSWIFT: 'yes', CPPFLAGS: '-DVERIFY' }
+          - BUILD: 'distcheck'
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: Install Homebrew packages
+        run: |
+          brew install automake libtool gcc
+          ln -s $(brew --prefix gcc)/bin/gcc-?? /usr/local/bin/gcc
+
+      - name: CI script
+        env: ${{ matrix.env_vars }}
+        run: ./ci/ci.sh
+
+      - run: cat tests.log || true
+        if: ${{ always() }}
+      - run: cat noverify_tests.log || true
+        if: ${{ always() }}
+      - run: cat exhaustive_tests.log || true
+        if: ${{ always() }}
+      - run: cat ctime_tests.log || true
+        if: ${{ always() }}
+      - run: cat bench.log || true
+        if: ${{ always() }}
+      - run: cat config.log || true
+        if: ${{ always() }}
+      - run: cat test_env.log || true
+        if: ${{ always() }}
+      - name: CI env
+        run: env
+        if: ${{ always() }}
+
   win64-native:
     name: ${{ matrix.configuration.job_name }}
     # See: https://github.com/actions/runner-images#available-images.

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -18,15 +18,14 @@ project(libsecp256k1
 )
 
 if(CMAKE_VERSION VERSION_LESS 3.21)
-  get_directory_property(parent_directory PARENT_DIRECTORY)
-  if(parent_directory)
-    set(PROJECT_IS_TOP_LEVEL OFF CACHE INTERNAL "Emulates CMake 3.21+ behavior.")
-    set(${PROJECT_NAME}_IS_TOP_LEVEL OFF CACHE INTERNAL "Emulates CMake 3.21+ behavior.")
+  # Emulates CMake 3.21+ behavior.
+  if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+    set(PROJECT_IS_TOP_LEVEL ON)
+    set(${PROJECT_NAME}_IS_TOP_LEVEL ON)
   else()
-    set(PROJECT_IS_TOP_LEVEL ON CACHE INTERNAL "Emulates CMake 3.21+ behavior.")
-    set(${PROJECT_NAME}_IS_TOP_LEVEL ON CACHE INTERNAL "Emulates CMake 3.21+ behavior.")
+    set(PROJECT_IS_TOP_LEVEL OFF)
+    set(${PROJECT_NAME}_IS_TOP_LEVEL OFF)
   endif()
-  unset(parent_directory)
 endif()
 
 # The library version is based on libtool versioning of the ABI. The set of
@@ -92,21 +91,15 @@ if(SECP256K1_USE_EXTERNAL_DEFAULT_CALLBACKS)
   add_compile_definitions(USE_EXTERNAL_DEFAULT_CALLBACKS=1)
 endif()
 
-set(SECP256K1_ECMULT_WINDOW_SIZE "AUTO" CACHE STRING "Window size for ecmult precomputation for verification, specified as integer in range [2..24]. \"AUTO\" is a reasonable setting for desktop machines (currently 15). [default=AUTO]")
-set_property(CACHE SECP256K1_ECMULT_WINDOW_SIZE PROPERTY STRINGS "AUTO" 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24)
+set(SECP256K1_ECMULT_WINDOW_SIZE 15 CACHE STRING "Window size for ecmult precomputation for verification, specified as integer in range [2..24]. The default value is a reasonable setting for desktop machines (currently 15). [default=15]")
+set_property(CACHE SECP256K1_ECMULT_WINDOW_SIZE PROPERTY STRINGS 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24)
 include(CheckStringOptionValue)
 check_string_option_value(SECP256K1_ECMULT_WINDOW_SIZE)
-if(SECP256K1_ECMULT_WINDOW_SIZE STREQUAL "AUTO")
-  set(SECP256K1_ECMULT_WINDOW_SIZE 15)
-endif()
 add_compile_definitions(ECMULT_WINDOW_SIZE=${SECP256K1_ECMULT_WINDOW_SIZE})
 
-set(SECP256K1_ECMULT_GEN_KB "AUTO" CACHE STRING "The size of the precomputed table for signing in multiples of 1024 bytes (on typical platforms). Larger values result in possibly better signing or key generation performance at the cost of a larger table. Valid choices are 2, 22, 86. \"AUTO\" is a reasonable setting for desktop machines (currently 22). [default=AUTO]")
-set_property(CACHE SECP256K1_ECMULT_GEN_KB PROPERTY STRINGS "AUTO" 2 22 86)
+set(SECP256K1_ECMULT_GEN_KB 22 CACHE STRING "The size of the precomputed table for signing in multiples of 1024 bytes (on typical platforms). Larger values result in possibly better signing or key generation performance at the cost of a larger table. Valid choices are 2, 22, 86. The default value is a reasonable setting for desktop machines (currently 22). [default=22]")
+set_property(CACHE SECP256K1_ECMULT_GEN_KB PROPERTY STRINGS 2 22 86)
 check_string_option_value(SECP256K1_ECMULT_GEN_KB)
-if(SECP256K1_ECMULT_GEN_KB STREQUAL "AUTO")
-  set(SECP256K1_ECMULT_GEN_KB 22)
-endif()
 if(SECP256K1_ECMULT_GEN_KB EQUAL 2)
   add_compile_definitions(COMB_BLOCKS=2)
   add_compile_definitions(COMB_TEETH=5)
@@ -214,23 +207,25 @@ mark_as_advanced(
   CMAKE_SHARED_LINKER_FLAGS_COVERAGE
 )
 
-get_property(is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
-set(default_build_type "RelWithDebInfo")
-if(is_multi_config)
-  set(CMAKE_CONFIGURATION_TYPES "${default_build_type}" "Release" "Debug" "MinSizeRel" "Coverage" CACHE STRING
-    "Supported configuration types."
-    FORCE
-  )
-else()
-  set_property(CACHE CMAKE_BUILD_TYPE PROPERTY
-    STRINGS "${default_build_type}" "Release" "Debug" "MinSizeRel" "Coverage"
-  )
-  if(NOT CMAKE_BUILD_TYPE)
-    message(STATUS "Setting build type to \"${default_build_type}\" as none was specified")
-    set(CMAKE_BUILD_TYPE "${default_build_type}" CACHE STRING
-      "Choose the type of build."
+if(PROJECT_IS_TOP_LEVEL)
+  get_property(is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
+  set(default_build_type "RelWithDebInfo")
+  if(is_multi_config)
+    set(CMAKE_CONFIGURATION_TYPES "${default_build_type}" "Release" "Debug" "MinSizeRel" "Coverage" CACHE STRING
+      "Supported configuration types."
       FORCE
     )
+  else()
+    set_property(CACHE CMAKE_BUILD_TYPE PROPERTY
+      STRINGS "${default_build_type}" "Release" "Debug" "MinSizeRel" "Coverage"
+    )
+    if(NOT CMAKE_BUILD_TYPE)
+      message(STATUS "Setting build type to \"${default_build_type}\" as none was specified")
+      set(CMAKE_BUILD_TYPE "${default_build_type}" CACHE STRING
+        "Choose the type of build."
+        FORCE
+      )
+    endif()
   endif()
 endif()
 
@@ -263,25 +258,34 @@ endif()
 
 set(CMAKE_C_VISIBILITY_PRESET hidden)
 
-# Ask CTest to create a "check" target (e.g., make check) as alias for the "test" target.
-# CTEST_TEST_TARGET_ALIAS is not documented but supposed to be user-facing.
-# See: https://gitlab.kitware.com/cmake/cmake/-/commit/816c9d1aa1f2b42d40c81a991b68c96eb12b6d2
-set(CTEST_TEST_TARGET_ALIAS check)
+set(print_msan_notice)
+if(SECP256K1_BUILD_CTIME_TESTS)
+  include(CheckMemorySanitizer)
+  check_memory_sanitizer(msan_enabled)
+  if(msan_enabled)
+    try_append_c_flags(-fno-sanitize-memory-param-retval)
+    set(print_msan_notice YES)
+  endif()
+  unset(msan_enabled)
+endif()
+
 include(CTest)
 # We do not use CTest's BUILD_TESTING because a single toggle for all tests is too coarse for our needs.
 mark_as_advanced(BUILD_TESTING)
 if(SECP256K1_BUILD_BENCHMARK OR SECP256K1_BUILD_TESTS OR SECP256K1_BUILD_EXHAUSTIVE_TESTS OR SECP256K1_BUILD_CTIME_TESTS OR SECP256K1_BUILD_EXAMPLES)
   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)
+set(SECP256K1_APPEND_CFLAGS "" CACHE STRING "Compiler flags that are appended to the command line after all other flags added by the build system. This variable is intended for debugging and special builds.")
+if(SECP256K1_APPEND_CFLAGS)
+  # Appending to this low-level rule variable is the only way to
+  # guarantee that the flags appear at the end of the command line.
+  string(APPEND CMAKE_C_COMPILE_OBJECT " ${SECP256K1_APPEND_CFLAGS}")
+endif()
 
 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")
@@ -332,7 +336,7 @@ message("Valgrind .............................. ${SECP256K1_VALGRIND}")
 get_directory_property(definitions COMPILE_DEFINITIONS)
 string(REPLACE ";" " " definitions "${definitions}")
 message("Preprocessor defined macros ........... ${definitions}")
-message("C compiler ............................ ${CMAKE_C_COMPILER}")
+message("C compiler ............................ ${CMAKE_C_COMPILER_ID} ${CMAKE_C_COMPILER_VERSION}, ${CMAKE_C_COMPILER}")
 message("CFLAGS ................................ ${CMAKE_C_FLAGS}")
 get_directory_property(compile_options COMPILE_OPTIONS)
 string(REPLACE ";" " " compile_options "${compile_options}")
@@ -355,10 +359,17 @@ 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}")
+if(SECP256K1_APPEND_CFLAGS)
+  message("SECP256K1_APPEND_CFLAGS ............... ${SECP256K1_APPEND_CFLAGS}")
+endif()
+message("")
+if(print_msan_notice)
+  message(
+    "Note:\n"
+    "  MemorySanitizer detected, tried to add -fno-sanitize-memory-param-retval to compile options\n"
+    "  to avoid false positives in ctime_tests. Pass -DSECP256K1_BUILD_CTIME_TESTS=OFF to avoid this.\n"
+  )
 endif()
-message("\n")
 if(SECP256K1_EXPERIMENTAL)
   message(
     "  ******\n"

diff --git a/README.md b/README.md
@@ -20,6 +20,7 @@ Features:
 * Optional module for public key recovery.
 * Optional module for ECDH key exchange.
 * Optional module for Schnorr signatures according to [BIP-340](https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki).
+* Optional module for ElligatorSwift key exchange according to [BIP-324](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki).
 
 Implementation details
 ----------------------

diff --git a/build-aux/m4/bitcoin_secp.m4 b/build-aux/m4/bitcoin_secp.m4
@@ -45,6 +45,22 @@ fi
 AC_MSG_RESULT($has_valgrind)
 ])
 
+AC_DEFUN([SECP_MSAN_CHECK], [
+AC_MSG_CHECKING(whether MemorySanitizer is enabled)
+AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
+  #if defined(__has_feature)
+  #  if __has_feature(memory_sanitizer)
+       /* MemorySanitizer is enabled. */
+  #  elif
+  #    error "MemorySanitizer is disabled."
+  #  endif
+  #else
+  #  error "__has_feature is not defined."
+  #endif
+  ]])], [msan_enabled=yes], [msan_enabled=no])
+AC_MSG_RESULT([$msan_enabled])
+])
+
 dnl SECP_TRY_APPEND_CFLAGS(flags, VAR)
 dnl Append flags to VAR if CC accepts them.
 AC_DEFUN([SECP_TRY_APPEND_CFLAGS], [

diff --git a/cmake/AllTargetsCompileOptions.cmake b/cmake/AllTargetsCompileOptions.cmake
diff --git a/cmake/CheckMemorySanitizer.cmake b/cmake/CheckMemorySanitizer.cmake
@@ -0,0 +1,18 @@
+include_guard(GLOBAL)
+include(CheckCSourceCompiles)
+
+function(check_memory_sanitizer output)
+  set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY)
+  check_c_source_compiles("
+    #if defined(__has_feature)
+    #  if __has_feature(memory_sanitizer)
+         /* MemorySanitizer is enabled. */
+    #  elif
+    #    error \"MemorySanitizer is disabled.\"
+    #  endif
+    #else
+    #  error \"__has_feature is not defined.\"
+    #endif
+  " HAVE_MSAN)
+  set(${output} ${HAVE_MSAN} PARENT_SCOPE)
+endfunction()