From 122dbaeb371cf65b8ebbc713bcbdb05203670d49 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 27 May 2024 10:56:37 +0100 Subject: [PATCH 1/4] autotools: Remove "auto" value of `--with-ecmult-window` option "auto" implies that a value is being chosen based on build system introspection or host system capabilities. However, for the `--with-ecmult-window` option, the value "auto" is hardcoded, which might lead to confusion. This change replaces "auto" with a more appropriate default value. --- .cirrus.yml | 2 +- .github/workflows/ci.yml | 2 +- configure.ac | 15 ++++----------- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 6e77403bf5..6431465d73 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -10,7 +10,7 @@ env: MAKEFLAGS: -j4 BUILD: check ### secp256k1 config - ECMULTWINDOW: auto + ECMULTWINDOW: 15 ECMULTGENKB: auto ASM: no WIDEMUL: auto diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d246682044..f42f7852f9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,7 +21,7 @@ env: MAKEFLAGS: '-j4' BUILD: 'check' ### secp256k1 config - ECMULTWINDOW: 'auto' + ECMULTWINDOW: 15 ECMULTGENKB: 'auto' ASM: 'no' WIDEMUL: 'auto' diff --git a/configure.ac b/configure.ac index 8b62e1dbfd..c0f809e721 100644 --- a/configure.ac +++ b/configure.ac @@ -203,15 +203,15 @@ AC_ARG_WITH([test-override-wide-multiply], [] ,[set_widemul=$withval], [set_wide AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm32|no|auto], [assembly to use (experimental: arm32) [default=auto]])],[req_asm=$withval], [req_asm=auto]) -AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE|auto], +AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE], [window size for ecmult precomputation for verification, specified as integer in range [2..24].] [Larger values result in possibly better performance at the cost of an exponentially larger precomputed table.] [The table will store 2^(SIZE-1) * 64 bytes of data but can be larger in memory due to platform-specific padding and alignment.] [A window size larger than 15 will require you delete the prebuilt precomputed_ecmult.c file so that it can be rebuilt.] [For very large window sizes, use "make -j 1" to reduce memory use during compilation.] -["auto" is a reasonable setting for desktop machines (currently 15). [default=auto]] +[The default value is a reasonable setting for desktop machines (currently 15). [default=15]] )], -[req_ecmult_window=$withval], [req_ecmult_window=auto]) +[set_ecmult_window=$withval], [set_ecmult_window=15]) AC_ARG_WITH([ecmult-gen-kb], [AS_HELP_STRING([--with-ecmult-gen-kb=2|22|86|auto], [The size of the precomputed table for signing in multiples of 1024 bytes (on typical platforms).] @@ -335,14 +335,7 @@ auto) ;; esac -# Set ecmult window size -if test x"$req_ecmult_window" = x"auto"; then - set_ecmult_window=15 -else - set_ecmult_window=$req_ecmult_window -fi - -error_window_size=['window size for ecmult precomputation not an integer in range [2..24] or "auto"'] +error_window_size=['window size for ecmult precomputation not an integer in range [2..24]'] case $set_ecmult_window in ''|*[[!0-9]]*) # no valid integer From 26b94ee92a7928d0e3ceb265f8e4b5bbabf01a77 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 27 May 2024 10:46:36 +0100 Subject: [PATCH 2/4] autotools: Remove "auto" value of `--with-ecmult-gen-kb` option "auto" implies that a value is being chosen based on build system introspection or host system capabilities. However, for the `--with-ecmult-gen-kb` option, the value "auto" is hardcoded, which might lead to confusion. This change replaces "auto" with a more appropriate default value. --- .cirrus.yml | 2 +- .github/workflows/ci.yml | 2 +- configure.ac | 15 ++++----------- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 6431465d73..4bd15511a4 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -11,7 +11,7 @@ env: BUILD: check ### secp256k1 config ECMULTWINDOW: 15 - ECMULTGENKB: auto + ECMULTGENKB: 22 ASM: no WIDEMUL: auto WITH_VALGRIND: yes diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f42f7852f9..98f5e2b333 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,7 @@ env: BUILD: 'check' ### secp256k1 config ECMULTWINDOW: 15 - ECMULTGENKB: 'auto' + ECMULTGENKB: 22 ASM: 'no' WIDEMUL: 'auto' WITH_VALGRIND: 'yes' diff --git a/configure.ac b/configure.ac index c0f809e721..ed3c4b80dd 100644 --- a/configure.ac +++ b/configure.ac @@ -213,12 +213,12 @@ AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE], )], [set_ecmult_window=$withval], [set_ecmult_window=15]) -AC_ARG_WITH([ecmult-gen-kb], [AS_HELP_STRING([--with-ecmult-gen-kb=2|22|86|auto], +AC_ARG_WITH([ecmult-gen-kb], [AS_HELP_STRING([--with-ecmult-gen-kb=2|22|86], [The size of the precomputed table for signing in multiples of 1024 bytes (on typical platforms).] [Larger values result in possibly better signing/keygeneration performance at the cost of a larger table.] -["auto" is a reasonable setting for desktop machines (currently 22). [default=auto]] +[The default value is a reasonable setting for desktop machines (currently 22). [default=22]] )], -[req_ecmult_gen_kb=$withval], [req_ecmult_gen_kb=auto]) +[set_ecmult_gen_kb=$withval], [set_ecmult_gen_kb=22]) AC_ARG_WITH([valgrind], [AS_HELP_STRING([--with-valgrind=yes|no|auto], [Build with extra checks for running inside Valgrind [default=auto]] @@ -350,13 +350,6 @@ case $set_ecmult_window in ;; esac -# Set ecmult gen kb -if test x"$req_ecmult_gen_kb" = x"auto"; then - set_ecmult_gen_kb=22 -else - set_ecmult_gen_kb=$req_ecmult_gen_kb -fi - case $set_ecmult_gen_kb in 2) SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DCOMB_BLOCKS=2 -DCOMB_TEETH=5" @@ -368,7 +361,7 @@ case $set_ecmult_gen_kb in SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DCOMB_BLOCKS=43 -DCOMB_TEETH=6" ;; *) - AC_MSG_ERROR(['ecmult gen table size not 2, 22, 86 or "auto"']) + AC_MSG_ERROR(['ecmult gen table size not 2, 22 or 86']) ;; esac From a06805ee7428d2786e631c9b5650c25fb0d46fe0 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 27 May 2024 13:26:03 +0100 Subject: [PATCH 3/4] cmake: Remove "AUTO" value of `SECP256K1_ECMULT_WINDOW_SIZE` option "AUTO" implies that a value is being chosen based on build system introspection or host system capabilities. However, for the `SECP256K1_ECMULT_WINDOW_SIZE` option, the value "AUTO" is hardcoded, which might lead to confusion. This change replaces "AUTO" with a more appropriate default value. --- CMakeLists.txt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 88994d828a..216d6465cf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -92,13 +92,10 @@ 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]") From 4d9645bee06e732f5d7d9e72b0c26c22c5006eb8 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 27 May 2024 13:32:23 +0100 Subject: [PATCH 4/4] cmake: Remove "AUTO" value of `SECP256K1_ECMULT_GEN_KB` option "AUTO" implies that a value is being chosen based on build system introspection or host system capabilities. However, for the `SECP256K1_ECMULT_GEN_KB` option, the value "AUTO" is hardcoded, which might lead to confusion. This change replaces "AUTO" with a more appropriate default value. --- CMakeLists.txt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 216d6465cf..3d0c99b67a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -98,12 +98,9 @@ include(CheckStringOptionValue) check_string_option_value(SECP256K1_ECMULT_WINDOW_SIZE) 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)