crypto/secp256k1: update libsecp256k1

[MariusVanDerWijden]

Updates the libsecp256k1 dependency to commit: c0d9480fbbf8eccbd4be23ed27f6f2af6f3b211e

PR:

BenchmarkSign-24    	   57756	     21214 ns/op	     164 B/op	       3 allocs/op
BenchmarkRecover-24    	   37156	     33044 ns/op	      80 B/op	       1 allocs/op
BenchmarkEcrecoverSignature-24    	   36889	     32935 ns/op	      80 B/op	       1 allocs/op
BenchmarkVerifySignature-24    	   41163	     29207 ns/op	       0 B/op	       0 allocs/op
BenchmarkDecompressPubkey-24    	  318624	      4062 ns/op	     304 B/op	       6 allocs/op

Master:

BenchmarkSign-24    	   34509	     35330 ns/op	     164 B/op	       3 allocs/op
BenchmarkRecover-24    	   25418	     47725 ns/op	      80 B/op	       1 allocs/op
BenchmarkEcrecoverSignature-24    	   25735	     47591 ns/op	      80 B/op	       1 allocs/op
BenchmarkVerifySignature-24    	   29108	     41097 ns/op	       0 B/op	       0 allocs/op
BenchmarkDecompressPubkey-24    	  294747	      4143 ns/op	     304 B/op	       6 allocs/op

Performance seems to be improved quite significantly:

Sign-24      34.86µ ± 3%   21.66µ ± 2%  -37.86% (p=0.000 n=10)
Recover-24   46.14µ ± 3%   33.24µ ± 2%  -27.95% (p=0.000 n=10)

[MariusVanDerWijden]

yep broken rn, fixing

[MariusVanDerWijden]

It would be nice to sanity check this on all platforms

[fjl]

This doesn't do anything. We are defining C preprocessor macros here. I checked, and libsecp256k1 does not use this macro. It uses macros like this:

#if defined(EXHAUSTIVE_TEST_ORDER)
#include "scalar_low_impl.h"
#elif defined(SECP256K1_WIDEMUL_INT128)
#include "scalar_4x64_impl.h"
#elif defined(SECP256K1_WIDEMUL_INT64)
#include "scalar_8x32_impl.h"
#else
#error "Please select wide multiplication implementation"
#endif

So we need to define either SECP256K1_WIDEMUL_INT128 or SECP256K1_WIDEMUL_INT64 depending on int128 availability.

[fjl]

I think you mistook autoconf feature names for #defines here. For the ECMULTWINDOW setting, you need to define ECMULT_WINDOW_SIZE. The default value is 15, so you can just leave this out.

[fjl]

Ahh, I just checked and libsecp256k1 has compile-time autodetection for int128: https://github.com/bitcoin-core/secp256k1/blob/1b1fc09341c956e8918adba1eeaa43b47d73ea84/src/util.h#L303-L332

So we don't need to define anything here and it will choose the defaults.

[MariusVanDerWijden]

Are you sure we need to define it ourselves? Seems like they did so in util.h

#if defined(USE_FORCE_WIDEMUL_INT128_STRUCT)
/* If USE_FORCE_WIDEMUL_INT128_STRUCT is set, use int128_struct. */
# define SECP256K1_WIDEMUL_INT128 1
# define SECP256K1_INT128_STRUCT 1
#elif defined(USE_FORCE_WIDEMUL_INT128)
/* If USE_FORCE_WIDEMUL_INT128 is set, use int128. */
# define SECP256K1_WIDEMUL_INT128 1
# define SECP256K1_INT128_NATIVE 1
#elif defined(USE_FORCE_WIDEMUL_INT64)
/* If USE_FORCE_WIDEMUL_INT64 is set, use int64. */
# define SECP256K1_WIDEMUL_INT64 1
#elif defined(UINT128_MAX) || defined(__SIZEOF_INT128__)
/* If a native 128-bit integer type exists, use int128. */
# define SECP256K1_WIDEMUL_INT128 1
# define SECP256K1_INT128_NATIVE 1
#elif defined(_MSC_VER) && (defined(_M_X64) || defined(_M_ARM64))
/* On 64-bit MSVC targets (x86_64 and arm64), use int128_struct
 * (which has special logic to implement using intrinsics on those systems). */
# define SECP256K1_WIDEMUL_INT128 1
# define SECP256K1_INT128_STRUCT 1
#elif SIZE_MAX > 0xffffffff
/* Systems with 64-bit pointers (and thus registers) very likely benefit from
 * using 64-bit based arithmetic (even if we need to fall back to 32x32->64 based
 * multiplication logic). */
# define SECP256K1_WIDEMUL_INT128 1
# define SECP256K1_INT128_STRUCT 1
#else
/* Lastly, fall back to int64 based arithmetic. */
# define SECP256K1_WIDEMUL_INT64 1
#endif