Squashed 'src/secp256k1/' changes from be8d9c2..fecf436

fecf436 Merge bitcoin-core/secp256k1#1019: build: don't append valgrind CPPFLAGS if not installed (macOS)
2e5e4b6 Merge bitcoin-core/secp256k1#1020: doc: remove use of <0xa0> "no break space"
812ff5c doc: remove use of 0xa0 "no break space"
214042a build: don't append valgrind CPPFLAGS if not installed
793ad90 Merge bitcoin-core/secp256k1#1010: doc: Minor fixes in safegcd_implementation.md
dc9b685 doc: Minor fixes in safegcd_implementation.md
ea5e8a9 Merge bitcoin-core/secp256k1#1012: Fix typos
2332975 Fix typos
7006f1b Merge bitcoin-core/secp256k1#1011: ci: Enable -g if we set CFLAGS manually
72de135 ci: Enable -g if we set CFLAGS manually
74c34e7 Merge bitcoin-core/secp256k1#1009: refactor: Use (int)&(int) in boolean context to avoid compiler warning
16d1322 refactor: Use (int)&(int) in boolean context to avoid compiler warning
c74a7b7 Merge bitcoin-core/secp256k1#1007: doc: Replace apoelstra's GPG key by jonasnick's GPG key
3b157c4 doc: Suggest keys.openpgp.org as keyserver in SECURITY.md
73a7472 doc: Replace apoelstra's GPG key by jonasnick's GPG key
515a5db Merge bitcoin-core/secp256k1#991: Merge all "external" benchmarks into a single bench binary
af6abcb Make bench support selecting which benchmarks to run
9f56bdf Merge bench_schnorrsig into bench
3208557 Merge bench_recover into bench
855e18d Merge bench_ecdh into bench
2a7be67 Combine bench_sign and bench_verify into single bench
8fa4120 Merge bitcoin-core/secp256k1#1002: Make aux_rnd32==NULL behave identical to 0x0000..00.
5324f89 Make aux_rnd32==NULL behave identical to 0x0000..00.
21c188b Merge bitcoin-core/secp256k1#943: VERIFY_CHECK precondition for secp256k1_fe_set_int.
3e7b2ea Merge bitcoin-core/secp256k1#999: bench_ecmult: improve clarity of output
23e2f66 bench: don't return 1 in have_flag() if argc = 1
96b1ad2 bench_ecmult: improve clarity of output
20d791e Merge bitcoin-core/secp256k1#989: Shared benchmark format for command line and CSV outputs
aa1b889 Merge bitcoin-core/secp256k1#996: Fix G.y parity in sage code
044d956 Fix G.y parity in sage code
b4b1306 create csv file from the benchmark output
26a255b Shared benchmark format for command line and CSV outputs
9526874 Merge bitcoin-core/secp256k1#810: Avoid overly-wide multiplications in 5x52 field mul/sqr
920a0e5 Merge bitcoin-core/secp256k1#952: Avoid computing out-of-bounds pointer.
f34b5ca Merge bitcoin-core/secp256k1#983: [RFC] Remove OpenSSL testing support
297ce82 Merge bitcoin-core/secp256k1#966: Make aux_rand32 arg to secp256k1_schnorrsig_sign const
2888640 VERIFY_CHECK precondition for secp256k1_fe_set_int.
d49011f Make _set_fe_int( . , 0 ) set magnitude to 0
bc08599 Remove OpenSSL testing support
10f9bd8 Merge bitcoin-core/secp256k1#987: Fix unused parameter warnings when building without VERIFY
189f6bc Fix unused parameter warnings when building without VERIFY
da0092b Merge bitcoin-core/secp256k1#986: tests: remove `secp256k1_fe_verify` from tests.c and modify `_fe_from_storage` to call `_fe_verify`
d439937 tests: remove `secp256k1_fe_verify` from tests.c and modify `secp256k1_fe_from_storage` to call `secp256k1_fe_verify`
2a3a97c Merge bitcoin-core/secp256k1#976: `secp256k1_schnorrsig_sign_internal` should be static
aa5d34a Merge bitcoin-core/secp256k1#783: Make the public API docs more consistent and explicit
7271387 Add missing static to secp256k1_schnorrsig_sign_internal
db4667d Make aux_rand32 arg to secp256k1_schnorrsig_sign const
9a5a87e Merge bitcoin-core/secp256k1#956: Replace ecmult_context with a generated static array.
20abd52 Add tests for pre_g tables.
6815761 Remove ecmult_context.
f20dcbb Correct typo.
16a3cc0 Generate ecmult_static_pre_g.h
8de2d86 Bump memory limits in advance of making the ecmult context static.
d7ec49a Merge bitcoin-core/secp256k1#969: ci: Fixes after Debian release
5d5c74a tests: Rewrite code to circument potential bug in clang
3d2f492 ci: Install libasan6 (instead of 5) after Debian upgrade
adec5a1 Add missing null check for ctx and input keys in the public API
f4edfc7 Improve consistency for NULL arguments in the public interface
9be7b0f Avoid computing out-of-bounds pointer.
b53e0cd Avoid overly-wide multiplications

git-subtree-dir: src/secp256k1
git-subtree-split: fecf436

.cirrus.yml

@@ -278,14 +278,13 @@ task:
   container:
     dockerfile: ci/linux-debian.Dockerfile
     cpu: 1
-    memory: 1G
+    memory: 2G
   env:
     ECDH: yes
     RECOVERY: yes
     EXPERIMENTAL: yes
     SCHNORRSIG: yes
     CTIMETEST: no
-    EXTRAFLAGS: "--disable-openssl-tests"
   matrix:
     - name: "Valgrind (memcheck)"
       env:
@@ -294,8 +293,8 @@ task:
         TEST_ITERS: 16
     - name: "UBSan, ASan, LSan"
       env:
-        CFLAGS: "-fsanitize=undefined,address"
-        CFLAGS_FOR_BUILD: "-fsanitize=undefined,address"
+        CFLAGS: "-fsanitize=undefined,address -g"
+        CFLAGS_FOR_BUILD: "-fsanitize=undefined,address -g"
         UBSAN_OPTIONS: "print_stacktrace=1:halt_on_error=1"
         ASAN_OPTIONS: "strict_string_checks=1:detect_stack_use_after_return=1:detect_leaks=1"
         LSAN_OPTIONS: "use_unaligned=1"
@@ -330,7 +329,7 @@ task:
     # ./configure correctly errors out when given CC=g++.
     # We hack around this by passing CC=g++ only to make.
     CC: gcc
-    MAKEFLAGS: -j2 CC=g++ CFLAGS=-fpermissive
+    MAKEFLAGS: -j2 CC=g++ CFLAGS=-fpermissive\ -g
     WERROR_CFLAGS:
     EXPERIMENTAL: yes
     ECDH: yes

.gitattributes

@@ -0,0 +1 @@
+src/ecmult_static_pre_g.h linguist-generated

.gitignore

@@ -1,18 +1,15 @@
-bench_inv
-bench_ecdh
+bench
 bench_ecmult
-bench_schnorrsig
-bench_sign
-bench_verify
-bench_recover
 bench_internal
 tests
 exhaustive_tests
 gen_context
+gen_ecmult_static_pre_g
 valgrind_ctime_test
 *.exe
 *.so
 *.a
+*.csv
 !.gitignore
 
 Makefile

Makefile.am

@@ -81,13 +81,9 @@ endif
 
 noinst_PROGRAMS =
 if USE_BENCHMARK
-noinst_PROGRAMS += bench_verify bench_sign bench_internal bench_ecmult
-bench_verify_SOURCES = src/bench_verify.c
-bench_verify_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB)
-# SECP_TEST_INCLUDES are only used here for CRYPTO_CPPFLAGS
-bench_verify_CPPFLAGS = $(SECP_TEST_INCLUDES)
-bench_sign_SOURCES = src/bench_sign.c
-bench_sign_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB)
+noinst_PROGRAMS += bench bench_internal bench_ecmult
+bench_SOURCES = src/bench.c
+bench_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB)
 bench_internal_SOURCES = src/bench_internal.c
 bench_internal_LDADD = $(SECP_LIBS) $(COMMON_LIB)
 bench_internal_CPPFLAGS = $(SECP_INCLUDES)
@@ -127,12 +123,19 @@ exhaustive_tests_LDFLAGS = -static
 TESTS += exhaustive_tests
 endif
 
+EXTRA_PROGRAMS = gen_ecmult_static_pre_g
+gen_ecmult_static_pre_g_SOURCES = src/gen_ecmult_static_pre_g.c
+# See Automake manual, Section "Errors with distclean"
+src/ecmult_static_pre_g.h:
+	$(MAKE) $(AM_MAKEFLAGS) gen_ecmult_static_pre_g$(EXEEXT)
+	./gen_ecmult_static_pre_g$(EXEEXT)
+
 if USE_ECMULT_STATIC_PRECOMPUTATION
 CPPFLAGS_FOR_BUILD +=-I$(top_srcdir) -I$(builddir)/src
 
 gen_context_OBJECTS = gen_context.o
 gen_context_BIN = gen_context$(BUILD_EXEEXT)
-gen_%.o: src/gen_%.c src/libsecp256k1-config.h
+$(gen_context_OBJECTS): src/gen_context.c src/libsecp256k1-config.h
 	$(CC_FOR_BUILD) $(DEFS) $(CPPFLAGS_FOR_BUILD) $(SECP_CFLAGS_FOR_BUILD) $(CFLAGS_FOR_BUILD) -c $< -o $@
 
 $(gen_context_BIN): $(gen_context_OBJECTS)
@@ -149,7 +152,7 @@ src/ecmult_static_context.h: $(gen_context_BIN)
 CLEANFILES = $(gen_context_BIN) src/ecmult_static_context.h
 endif
 
-EXTRA_DIST = autogen.sh src/gen_context.c src/basic-config.h
+EXTRA_DIST = autogen.sh src/gen_context.c src/ecmult_static_pre_g.h src/basic-config.h
 
 if ENABLE_MODULE_ECDH
 include src/modules/ecdh/Makefile.am.include

README.md

@@ -100,6 +100,18 @@ To create a HTML report with coloured and annotated source code:
     $ mkdir -p coverage
     $ gcovr --exclude 'src/bench*' --html --html-details -o coverage/coverage.html
 
+Benchmark
+------------
+If configured with `--enable-benchmark` (which is the default), binaries for benchmarking the libsecp256k1 functions will be present in the root directory after the build.
+
+To print the benchmark result to the command line:
+
+    $ ./bench_name
+
+To create a CSV file for the benchmark result :
+
+    $ ./bench_name | sed '2d;s/ \{1,\}//g' > bench_name.csv
+
 Reporting a vulnerability
 ------------
 

SECURITY.md

@@ -9,7 +9,7 @@ The following keys may be used to communicate sensitive information to developer
 | Name | Fingerprint |
 |------|-------------|
 | Pieter Wuille | 133E AC17 9436 F14A 5CF1  B794 860F EB80 4E66 9320 |
-| Andrew Poelstra | 699A 63EF C17A D3A9 A34C  FFC0 7AD0 A91C 40BD 0091 |
+| Jonas Nick | 36C7 1A37 C9D9 88BD E825  08D9 B1A7 0E4F 8DCD 0366 |
 | Tim Ruffing | 09E0 3F87 1092 E40E 106E  902B 33BC 86AB 80FF 5516 |
 
-You can import a key by running the following command with that individual’s fingerprint: `gpg --recv-keys "<fingerprint>"` Ensure that you put quotes around fingerprints containing spaces.
+You can import a key by running the following command with that individual’s fingerprint: `gpg --keyserver hkps://keys.openpgp.org --recv-keys "<fingerprint>"` Ensure that you put quotes around fingerprints containing spaces.

build-aux/m4/bitcoin_secp.m4

@@ -9,72 +9,6 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
 AC_MSG_RESULT([$has_64bit_asm])
 ])
 
-dnl
-AC_DEFUN([SECP_OPENSSL_CHECK],[
-  has_libcrypto=no
-  m4_ifdef([PKG_CHECK_MODULES],[
-    PKG_CHECK_MODULES([CRYPTO], [libcrypto], [has_libcrypto=yes],[has_libcrypto=no])
-    if test x"$has_libcrypto" = x"yes"; then
-      TEMP_LIBS="$LIBS"
-      LIBS="$LIBS $CRYPTO_LIBS"
-      AC_CHECK_LIB(crypto, main,[AC_DEFINE(HAVE_LIBCRYPTO,1,[Define this symbol if libcrypto is installed])],[has_libcrypto=no])
-      LIBS="$TEMP_LIBS"
-    fi
-  ])
-  if test x$has_libcrypto = xno; then
-    AC_CHECK_HEADER(openssl/crypto.h,[
-      AC_CHECK_LIB(crypto, main,[
-        has_libcrypto=yes
-        CRYPTO_LIBS=-lcrypto
-        AC_DEFINE(HAVE_LIBCRYPTO,1,[Define this symbol if libcrypto is installed])
-      ])
-    ])
-    LIBS=
-  fi
-if test x"$has_libcrypto" = x"yes" && test x"$has_openssl_ec" = x; then
-  AC_MSG_CHECKING(for EC functions in libcrypto)
-  CPPFLAGS_TEMP="$CPPFLAGS"
-  CPPFLAGS="$CRYPTO_CPPFLAGS $CPPFLAGS"
-  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
-    #include <openssl/bn.h>
-    #include <openssl/ec.h>
-    #include <openssl/ecdsa.h>
-    #include <openssl/obj_mac.h>]],[[
-    # if OPENSSL_VERSION_NUMBER < 0x10100000L
-    void ECDSA_SIG_get0(const ECDSA_SIG *sig, const BIGNUM **pr, const BIGNUM **ps) {(void)sig->r; (void)sig->s;}
-    # endif
-
-    unsigned int zero = 0;
-    const unsigned char *zero_ptr = (unsigned char*)&zero;
-    EC_KEY_free(EC_KEY_new_by_curve_name(NID_secp256k1));
-    EC_KEY *eckey = EC_KEY_new();
-    EC_GROUP *group = EC_GROUP_new_by_curve_name(NID_secp256k1);
-    EC_KEY_set_group(eckey, group);
-    ECDSA_sign(0, NULL, 0, NULL, &zero, eckey);
-    ECDSA_verify(0, NULL, 0, NULL, 0, eckey);
-    o2i_ECPublicKey(&eckey, &zero_ptr, 0);
-    d2i_ECPrivateKey(&eckey, &zero_ptr, 0);
-    EC_KEY_check_key(eckey);
-    EC_KEY_free(eckey);
-    EC_GROUP_free(group);
-    ECDSA_SIG *sig_openssl;
-    sig_openssl = ECDSA_SIG_new();
-    d2i_ECDSA_SIG(&sig_openssl, &zero_ptr, 0);
-    i2d_ECDSA_SIG(sig_openssl, NULL);
-    ECDSA_SIG_get0(sig_openssl, NULL, NULL);
-    ECDSA_SIG_free(sig_openssl);
-    const BIGNUM *bignum = BN_value_one();
-    BN_is_negative(bignum);
-    BN_num_bits(bignum);
-    if (sizeof(zero) >= BN_num_bytes(bignum)) {
-        BN_bn2bin(bignum, (unsigned char*)&zero);
-    }
-  ]])],[has_openssl_ec=yes],[has_openssl_ec=no])
-  AC_MSG_RESULT([$has_openssl_ec])
-  CPPFLAGS="$CPPFLAGS_TEMP"
-fi
-])
-
 AC_DEFUN([SECP_VALGRIND_CHECK],[
 if test x"$has_valgrind" != x"yes"; then
   CPPFLAGS_TEMP="$CPPFLAGS"

ci/cirrus.sh

@@ -26,7 +26,7 @@ make
 
 # Print information about binaries so that we can see that the architecture is correct
 file *tests* || true
-file bench_* || true
+file bench* || true
 file .libs/* || true
 
 # This tells `make check` to wrap test invocations.
@@ -49,21 +49,8 @@ then
     {
         $EXEC ./bench_ecmult
         $EXEC ./bench_internal
-        $EXEC ./bench_sign
-        $EXEC ./bench_verify
+        $EXEC ./bench
     } >> bench.log 2>&1
-    if [ "$RECOVERY" = "yes" ]
-    then
-        $EXEC ./bench_recover >> bench.log 2>&1
-    fi
-    if [ "$ECDH" = "yes" ]
-    then
-        $EXEC ./bench_ecdh >> bench.log 2>&1
-    fi
-    if [ "$SCHNORRSIG" = "yes" ]
-    then
-        $EXEC ./bench_schnorrsig >> bench.log 2>&1
-    fi
 fi
 if [ "$CTIMETEST" = "yes" ]
 then

ci/linux-debian.Dockerfile

@@ -14,7 +14,7 @@ RUN apt-get install --no-install-recommends --no-upgrade -y \
         make automake libtool pkg-config dpkg-dev valgrind qemu-user \
         gcc clang llvm libc6-dbg \
         g++ \
-        gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 libubsan1:i386 libasan5:i386 \
+        gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 libubsan1:i386 libasan6:i386 \
         gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \
         gcc-arm-linux-gnueabihf libc6-dev-armhf-cross libc6-dbg:armhf \
         gcc-aarch64-linux-gnu libc6-dev-arm64-cross libc6-dbg:arm64 \

configure.ac

@@ -21,7 +21,7 @@ AC_PATH_TOOL(STRIP, strip)
 
 # Save definition of AC_PROG_CC because AM_PROG_CC_C_O in automake<=1.13 will
 # redefine AC_PROG_CC to exit with an error, which avoids the user calling it
-# accidently and screwing up the effect of AM_PROG_CC_C_O. However, we'll need
+# accidentally and screwing up the effect of AM_PROG_CC_C_O. However, we'll need
 # AC_PROG_CC later on in AX_PROG_CC_FOR_BUILD, where its usage is fine, and
 # we'll carefully make sure not to call AC_PROG_CC anywhere else.
 m4_copy([AC_PROG_CC], [saved_AC_PROG_CC])
@@ -43,14 +43,8 @@ case $host_os in
          # These Homebrew packages may be keg-only, meaning that they won't be found
          # 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`
-         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$valgrind_prefix != x; then
+         if $BREW list --versions valgrind >/dev/null; then
+           valgrind_prefix=`$BREW --prefix valgrind 2>/dev/null`
            VALGRIND_CPPFLAGS="-I$valgrind_prefix/include"
          fi
        else
@@ -121,11 +115,6 @@ AC_ARG_ENABLE(tests,
     [use_tests=$enableval],
     [use_tests=yes])
 
-AC_ARG_ENABLE(openssl_tests,
-    AS_HELP_STRING([--enable-openssl-tests],[enable OpenSSL tests [default=auto]]),
-    [enable_openssl_tests=$enableval],
-    [enable_openssl_tests=auto])
-
 AC_ARG_ENABLE(experimental,
     AS_HELP_STRING([--enable-experimental],[allow experimental configure options [default=no]]),
     [use_experimental=$enableval],
@@ -171,12 +160,14 @@ AC_ARG_ENABLE(external_default_callbacks,
 AC_ARG_WITH([test-override-wide-multiply], [] ,[set_widemul=$withval], [set_widemul=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])
+[assembly optimizations to use (experimental: arm) [default=auto]])],[req_asm=$withval], [req_asm=auto])
 
 AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE|auto],
 [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 ecmult_static_pre_g.h 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]]
 )],
 [req_ecmult_window=$withval], [req_ecmult_window=auto])
@@ -327,32 +318,6 @@ case $set_ecmult_gen_precision in
   ;;
 esac
 
-if test x"$use_tests" = x"yes"; then
-  SECP_OPENSSL_CHECK
-  if test x"$enable_openssl_tests" != x"no" && test x"$has_openssl_ec" = x"yes"; then
-      enable_openssl_tests=yes
-      AC_DEFINE(ENABLE_OPENSSL_TESTS, 1, [Define this symbol if OpenSSL EC functions are available])
-      SECP_TEST_INCLUDES="$SSL_CFLAGS $CRYPTO_CFLAGS $CRYPTO_CPPFLAGS"
-      SECP_TEST_LIBS="$CRYPTO_LIBS"
-
-      case $host in
-      *mingw*)
-        SECP_TEST_LIBS="$SECP_TEST_LIBS -lgdi32"
-        ;;
-      esac
-  else
-    if test x"$enable_openssl_tests" = x"yes"; then
-      AC_MSG_ERROR([OpenSSL tests requested but OpenSSL with EC support is not available])
-    fi
-    enable_openssl_tests=no
-  fi
-else
-  if test x"$enable_openssl_tests" = x"yes"; then
-    AC_MSG_ERROR([OpenSSL tests requested but tests are not enabled])
-  fi
-  enable_openssl_tests=no
-fi
-
 if test x"$enable_valgrind" = x"yes"; then
   SECP_INCLUDES="$SECP_INCLUDES $VALGRIND_CPPFLAGS"
 fi
@@ -517,7 +482,6 @@ echo "  with ecmult precomp     = $set_precomp"
 echo "  with external callbacks = $use_external_default_callbacks"
 echo "  with benchmarks         = $use_benchmark"
 echo "  with tests              = $use_tests"
-echo "  with openssl tests      = $enable_openssl_tests"
 echo "  with coverage           = $enable_coverage"
 echo "  module ecdh             = $enable_module_ecdh"
 echo "  module recovery         = $enable_module_recovery"

doc/safegcd_implementation.md

@@ -569,8 +569,14 @@ bits efficiently, which is possible on most platforms; it is abstracted here as
 
 ```python
 def count_trailing_zeros(v):
-    """For a non-zero value v, find z such that v=(d<<z) for some odd d."""
-    return (v & -v).bit_length() - 1
+    """
+    When v is zero, consider all N zero bits as "trailing".
+    For a non-zero value v, find z such that v=(d<<z) for some odd d.
+    """
+    if v == 0:
+        return N
+    else:
+        return (v & -v).bit_length() - 1
 
 i = N # divsteps left to do
 while True:
@@ -601,7 +607,7 @@ becomes negative, or when *i* reaches *0*. Combined, this is equivalent to addin
 It is easy to find what that multiple is: we want a number *w* such that *g+w&thinsp;f* has a few bottom
 zero bits. If that number of bits is *L*, we want *g+w&thinsp;f mod 2<sup>L</sup> = 0*, or *w = -g/f mod 2<sup>L</sup>*. Since *f*
 is odd, such a *w* exists for any *L*. *L* cannot be more than *i* steps (as we'd finish the loop before
-doing more) or more than *&eta;+1* steps (as we'd run `eta, f, g = -eta, g, f` at that point), but
+doing more) or more than *&eta;+1* steps (as we'd run `eta, f, g = -eta, g, -f` at that point), but
 apart from that, we're only limited by the complexity of computing *w*.
 
 This code demonstrates how to cancel up to 4 bits per step:
@@ -618,7 +624,7 @@ while True:
         break
     # We know g is odd now
     if eta < 0:
-        eta, f, g = -eta, g, f
+        eta, f, g = -eta, g, -f
     # Compute limit on number of bits to cancel
     limit = min(min(eta + 1, i), 4)
     # Compute w = -g/f mod 2**limit, using the table value for -1/f mod 2**4. Note that f is