Skip to content

Commit

Permalink
[SECP256K1] Remove num/gmp support
Browse files Browse the repository at this point in the history
Summary:
```
The whole "num" API and its libgmp-based implementation are now unused.
Remove them.
```

Partial backport of [[bitcoin-core/secp256k1#831 | secp256k1#831]]:
bitcoin-core/secp256k1@1f233b3

Depends on D9407.

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Subscribers: majcosta

Differential Revision: https://reviews.bitcoinabc.org/D9408
  • Loading branch information
sipa authored and Fabcien committed Apr 14, 2021
1 parent c39b913 commit 9f5afc1
Show file tree
Hide file tree
Showing 25 changed files with 5 additions and 1,032 deletions.
16 changes: 3 additions & 13 deletions .cirrus.yml
@@ -1,6 +1,5 @@
env:
WIDEMUL: auto
BIGNUM: gmp
STATICPRECOMPUTATION: yes
ECMULTGENPRECISION: auto
ASM: no
Expand Down Expand Up @@ -49,9 +48,8 @@ task:
- env: {WIDEMUL: int128, RECOVERY: yes, EXPERIMENTAL: yes, SCHNORRSIG: yes}
- env: {WIDEMUL: int128, ECDH: yes, EXPERIMENTAL: yes, MULTISET: yes, SCHNORRSIG: yes}
- env: {WIDEMUL: int128, ASM: x86_64}
- env: {BIGNUM: no}
- env: {BIGNUM: no, RECOVERY: yes, EXPERIMENTAL: yes, MULTISET: yes, SCHNORRSIG: yes}
- env: {BIGNUM: no, STATICPRECOMPUTATION: no}
- env: { RECOVERY: yes, EXPERIMENTAL: yes, MULTISET: yes, SCHNORRSIG: yes}
- env: { STATICPRECOMPUTATION: no}
- env: {AUTOTOOLS_TARGET: distcheck, CMAKE_TARGET: install, WITH_VALGRIND: no, CTIMETEST: no, BENCH: no}
- env: {AUTOTOOLS_EXTRA_FLAGS: CPPFLAGS=-DDETERMINISTIC, CMAKE_EXTRA_FLAGS: -DCMAKE_C_FLAGS=-DDETERMINISTIC}
- env: {AUTOTOOLS_EXTRA_FLAGS: CFLAGS=-O0, CMAKE_EXTRA_FLAGS: -DCMAKE_BUILD_TYPE=Debug, CTIMETEST: no}
Expand All @@ -63,7 +61,6 @@ task:
CFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer"
LDFLAGS: "-fsanitize=undefined -fno-omit-frame-pointer"
UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1"
BIGNUM: no
ASM: x86_64
ECDH: yes
RECOVERY: yes
Expand All @@ -75,7 +72,6 @@ task:
- env: { ECMULTGENPRECISION: 8 }
- env:
RUN_VALGRIND: yes
BIGNUM: no
ASM: x86_64
ECDH: yes
RECOVERY: yes
Expand Down Expand Up @@ -113,11 +109,6 @@ task:
CC: i686-linux-gnu-gcc
- env:
CC: clang --target=i686-pc-linux-gnu -isystem /usr/i686-linux-gnu/include
matrix:
- env:
BIGNUM: gmp
- env:
BIGNUM: no
test_script:
- ./ci/build_autotools.sh
- ./ci/build_cmake.sh
Expand Down Expand Up @@ -177,7 +168,7 @@ task:
# If we haven't restored from cached (and just run brew install), this is a no-op.
- brew link valgrind
brew_script:
- brew install automake cmake gcc@9 gmp libtool ninja
- brew install automake cmake gcc@9 libtool ninja
test_script:
- ./ci/build_autotools.sh
- ./ci/build_cmake.sh
Expand All @@ -193,7 +184,6 @@ task:
QEMU_CMD: qemu-s390x
HOST: s390x-linux-gnu
WITH_VALGRIND: no
BIGNUM: no
ECDH: yes
RECOVERY: yes
EXPERIMENTAL: yes
Expand Down
14 changes: 0 additions & 14 deletions CMakeLists.txt
Expand Up @@ -73,20 +73,6 @@ set(SECP256K1_PUBLIC_HEADERS
include/secp256k1_preallocated.h
)

option(SECP256K1_ENABLE_BIGNUM "Use the GMP bignum implementation" OFF)
if(SECP256K1_ENABLE_BIGNUM)
# We need to link in GMP
find_package(GMP REQUIRED)
target_link_libraries(secp256k1 GMP::gmp)
set(USE_NUM_GMP 1)
set(USE_FIELD_INV_NUM 1)
set(USE_SCALAR_INV_NUM 1)
else()
set(USE_NUM_NONE 1)
set(USE_FIELD_INV_BUILTIN 1)
set(USE_SCALAR_INV_BUILTIN 1)
endif()

# Guess the target architecture, within the ones with supported ASM.
# First check if the CMAKE_C_COMPILER_TARGET is set (should be when
# cross compiling), then CMAKE_SYSTEM_PROCESSOR as a fallback if meaningful
Expand Down
4 changes: 0 additions & 4 deletions Makefile.am
Expand Up @@ -18,8 +18,6 @@ noinst_HEADERS += src/scalar_8x32_impl.h
noinst_HEADERS += src/scalar_low_impl.h
noinst_HEADERS += src/group.h
noinst_HEADERS += src/group_impl.h
noinst_HEADERS += src/num_gmp.h
noinst_HEADERS += src/num_gmp_impl.h
noinst_HEADERS += src/ecdsa.h
noinst_HEADERS += src/ecdsa_impl.h
noinst_HEADERS += src/eckey.h
Expand All @@ -30,8 +28,6 @@ noinst_HEADERS += src/ecmult_const.h
noinst_HEADERS += src/ecmult_const_impl.h
noinst_HEADERS += src/ecmult_gen.h
noinst_HEADERS += src/ecmult_gen_impl.h
noinst_HEADERS += src/num.h
noinst_HEADERS += src/num_impl.h
noinst_HEADERS += src/field_10x26.h
noinst_HEADERS += src/field_10x26_impl.h
noinst_HEADERS += src/field_5x52.h
Expand Down
13 changes: 0 additions & 13 deletions build-aux/m4/bitcoin_secp.m4
Expand Up @@ -75,19 +75,6 @@ if test x"$has_libcrypto" = x"yes" && test x"$has_openssl_ec" = x; then
fi
])

dnl
AC_DEFUN([SECP_GMP_CHECK],[
if test x"$has_gmp" != x"yes"; then
CPPFLAGS_TEMP="$CPPFLAGS"
CPPFLAGS="$GMP_CPPFLAGS $CPPFLAGS"
LIBS_TEMP="$LIBS"
LIBS="$GMP_LIBS $LIBS"
AC_CHECK_HEADER(gmp.h,[AC_CHECK_LIB(gmp, __gmpz_init,[has_gmp=yes; GMP_LIBS="$GMP_LIBS -lgmp"; AC_DEFINE(HAVE_LIBGMP,1,[Define this symbol if libgmp is installed])])])
CPPFLAGS="$CPPFLAGS_TEMP"
LIBS="$LIBS_TEMP"
fi
])

AC_DEFUN([SECP_VALGRIND_CHECK],[
if test x"$has_valgrind" != x"yes"; then
CPPFLAGS_TEMP="$CPPFLAGS"
Expand Down
1 change: 0 additions & 1 deletion ci/build_autotools.sh
Expand Up @@ -34,7 +34,6 @@ pushd buildautotools
../configure \
--enable-experimental=$EXPERIMENTAL \
--with-test-override-wide-multiply=$WIDEMUL \
--with-bignum=$BIGNUM \
--with-asm=$ASM \
--enable-ecmult-static-precomputation=$STATICPRECOMPUTATION \
--with-ecmult-gen-precision=$ECMULTGENPRECISION \
Expand Down
1 change: 0 additions & 1 deletion ci/build_cmake.sh
Expand Up @@ -45,7 +45,6 @@ ${CMAKE_COMMAND} -GNinja .. \
-DSECP256K1_ENABLE_MODULE_EXTRAKEYS=$SCHNORRSIG \
-DSECP256K1_ENABLE_MODULE_SCHNORRSIG=$SCHNORRSIG \
-DSECP256K1_ENABLE_JNI=$JNI \
-DSECP256K1_ENABLE_BIGNUM=$BIGNUM \
-DSECP256K1_USE_ASM=$ASM \
-DSECP256K1_TEST_OVERRIDE_WIDE_MULTIPLY=$WIDEMUL \
$ECMULT_GEN_PRECISION_ARG \
Expand Down
4 changes: 2 additions & 2 deletions ci/linux-debian.Dockerfile
Expand Up @@ -8,7 +8,7 @@ RUN apt-get update
# dkpg-dev: to make pkg-config work in cross-builds
RUN apt-get install --no-install-recommends --no-upgrade -y \
automake default-jdk dpkg-dev libssl-dev libtool make ninja-build pkg-config python3 qemu-user valgrind \
gcc clang libc6-dbg libgmp-dev \
gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 libgmp-dev:i386 \
gcc clang libc6-dbg \
gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 \
gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x
RUN apt-get install -t buster-backports --no-install-recommends --no-upgrade -y cmake
92 changes: 0 additions & 92 deletions cmake/modules/FindGMP.cmake

This file was deleted.

58 changes: 0 additions & 58 deletions configure.ac
Expand Up @@ -48,17 +48,12 @@ case $host_os in
# in expected paths because they may conflict with system files. Ask
# Homebrew where each one is located, then adjust paths accordingly.
openssl_prefix=`$BREW --prefix openssl 2>/dev/null`
gmp_prefix=`$BREW --prefix gmp 2>/dev/null`
valgrind_prefix=`$BREW --prefix valgrind 2>/dev/null`
if test x$openssl_prefix != x; then
PKG_CONFIG_PATH="$openssl_prefix/lib/pkgconfig:$PKG_CONFIG_PATH"
export PKG_CONFIG_PATH
CRYPTO_CPPFLAGS="-I$openssl_prefix/include"
fi
if test x$gmp_prefix != x; then
GMP_CPPFLAGS="-I$gmp_prefix/include"
GMP_LIBS="-L$gmp_prefix/lib"
fi
if test x$valgrind_prefix != x; then
VALGRIND_CPPFLAGS="-I$valgrind_prefix/include"
fi
Expand Down Expand Up @@ -179,9 +174,6 @@ AC_ARG_ENABLE(jni,
[use_jni=$enableval],
[use_jni=no])

AC_ARG_WITH([bignum], [AS_HELP_STRING([--with-bignum=gmp|no|auto],
[bignum implementation to use [default=auto]])],[req_bignum=$withval], [req_bignum=auto])

AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm|no|auto],
[assembly optimizations to use (experimental: arm) [default=auto]])],[req_asm=$withval], [req_asm=auto])

Expand Down Expand Up @@ -260,32 +252,6 @@ else
esac
fi

if test x"$req_bignum" = x"auto"; then
SECP_GMP_CHECK
if test x"$has_gmp" = x"yes"; then
set_bignum=gmp
fi

if test x"$set_bignum" = x; then
set_bignum=no
fi
else
set_bignum=$req_bignum
case $set_bignum in
gmp)
SECP_GMP_CHECK
if test x"$has_gmp" != x"yes"; then
AC_MSG_ERROR([gmp bignum explicitly requested but libgmp not available])
fi
;;
no)
;;
*)
AC_MSG_ERROR([invalid bignum implementation selection])
;;
esac
fi

# Select assembly optimization
use_external_asm=no

Expand Down Expand Up @@ -323,24 +289,6 @@ auto)
;;
esac

# Select bignum implementation
case $set_bignum in
gmp)
AC_DEFINE(HAVE_LIBGMP, 1, [Define this symbol if libgmp is installed])
AC_DEFINE(USE_NUM_GMP, 1, [Define this symbol to use the gmp implementation for num])
AC_DEFINE(USE_FIELD_INV_NUM, 1, [Define this symbol to use the num-based field inverse implementation])
AC_DEFINE(USE_SCALAR_INV_NUM, 1, [Define this symbol to use the num-based scalar inverse implementation])
;;
no)
AC_DEFINE(USE_NUM_NONE, 1, [Define this symbol to use no num implementation])
AC_DEFINE(USE_FIELD_INV_BUILTIN, 1, [Define this symbol to use the native field inverse implementation])
AC_DEFINE(USE_SCALAR_INV_BUILTIN, 1, [Define this symbol to use the native scalar inverse implementation])
;;
*)
AC_MSG_ERROR([invalid bignum implementation])
;;
esac

# Set ecmult window size
if test x"$req_ecmult_window" = x"auto"; then
set_ecmult_window=15
Expand Down Expand Up @@ -428,11 +376,6 @@ if test x"$use_jni" != x"no"; then
fi
fi

if test x"$set_bignum" = x"gmp"; then
SECP_LIBS="$SECP_LIBS $GMP_LIBS"
SECP_INCLUDES="$SECP_INCLUDES $GMP_CPPFLAGS"
fi

if test x"$enable_valgrind" = x"yes"; then
SECP_INCLUDES="$SECP_INCLUDES $VALGRIND_CPPFLAGS"
fi
Expand Down Expand Up @@ -627,7 +570,6 @@ echo " module extrakeys = $enable_module_extrakeys"
echo " module schnorrsig = $enable_module_schnorrsig"
echo
echo " asm = $set_asm"
echo " bignum = $set_bignum"
echo " ecmult window size = $set_ecmult_window"
echo " ecmult gen prec. bits = $set_ecmult_gen_precision"
# Hide test-only options unless they're used.
Expand Down
9 changes: 0 additions & 9 deletions src/basic-config.h
Expand Up @@ -13,19 +13,10 @@
#undef USE_ECMULT_STATIC_PRECOMPUTATION
#undef USE_EXTERNAL_ASM
#undef USE_EXTERNAL_DEFAULT_CALLBACKS
#undef USE_FIELD_INV_BUILTIN
#undef USE_FIELD_INV_NUM
#undef USE_NUM_GMP
#undef USE_NUM_NONE
#undef USE_SCALAR_INV_BUILTIN
#undef USE_SCALAR_INV_NUM
#undef USE_FORCE_WIDEMUL_INT64
#undef USE_FORCE_WIDEMUL_INT128
#undef ECMULT_WINDOW_SIZE

#define USE_NUM_NONE 1
#define USE_FIELD_INV_BUILTIN 1
#define USE_SCALAR_INV_BUILTIN 1
#define USE_WIDEMUL_64 1
#define ECMULT_WINDOW_SIZE 15

Expand Down
1 change: 0 additions & 1 deletion src/bench_ecmult.c
Expand Up @@ -9,7 +9,6 @@

#include "util.h"
#include "hash_impl.h"
#include "num_impl.h"
#include "field_impl.h"
#include "group_impl.h"
#include "scalar_impl.h"
Expand Down

0 comments on commit 9f5afc1

Please sign in to comment.