Update libsecp256k1 subtree

sipa commented at 8:50 pm on June 9, 2020: member

It’s been abound a year since the subtree was updated.

Here is a list of the included PRs:

bitcoin-core/secp256k1#755: Recovery signing: add to constant time test, and eliminate non ct operators
bitcoin-core/secp256k1#754: Fix uninit values passed into cmov
bitcoin-core/secp256k1#752: autoconf: Use “:” instead of “dnl” as a noop
bitcoin-core/secp256k1#750: Add macOS to the CI
bitcoin-core/secp256k1#701: Make ec_ arithmetic more consistent and add documentation
bitcoin-core/secp256k1#732: Retry if r is zero during signing
bitcoin-core/secp256k1#742: Fix typo in ecmult_const_impl.h
bitcoin-core/secp256k1#740: Make recovery/main_impl.h non-executable
bitcoin-core/secp256k1#735: build: fix OpenSSL EC detection on macOS
bitcoin-core/secp256k1#728: Suppress a harmless variable-time optimization by clang in memczero
bitcoin-core/secp256k1#722: Context isn’t freed in the ECDH benchmark
bitcoin-core/secp256k1#700: Allow overriding default flags
bitcoin-core/secp256k1#708: Constant-time behaviour test using valgrind memtest.
bitcoin-core/secp256k1#710: Eliminate harmless non-constant time operations on secret data.
bitcoin-core/secp256k1#718: Clarify that a secp256k1_ecdh_hash_function must return 0 or 1
bitcoin-core/secp256k1#714: doc: document the length requirements of output parameter.
bitcoin-core/secp256k1#682: Remove Java Native Interface
bitcoin-core/secp256k1#713: Docstrings
bitcoin-core/secp256k1#704: README: add a section for test coverage
bitcoin-core/secp256k1#709: Remove secret-dependant non-constant time operation in ecmult_const.
bitcoin-core/secp256k1#703: Overhaul README.md
bitcoin-core/secp256k1#689: Remove “except in benchmarks” exception for fp math
bitcoin-core/secp256k1#679: Add SECURITY.md
bitcoin-core/secp256k1#685: Fix issue where travis does not show the ./tests seed…
bitcoin-core/secp256k1#690: Add valgrind check to travis
bitcoin-core/secp256k1#678: Preventing compiler optimizations in benchmarks without a memory fence
bitcoin-core/secp256k1#688: Fix ASM setting in travis
bitcoin-core/secp256k1#684: Make no-float policy explicit
bitcoin-core/secp256k1#677: Remove note about heap allocation in secp256k1_ecmult_odd_multiples_table_storage_var
bitcoin-core/secp256k1#647: Increase robustness against UB in secp256k1_scalar_cadd_bit
bitcoin-core/secp256k1#664: Remove mention of ec_privkey_export because it doesn’t exist
bitcoin-core/secp256k1#337: variable sized precomputed table for signing
bitcoin-core/secp256k1#661: Make ./configure string consistent
bitcoin-core/secp256k1#657: Fix a nit in the recovery tests
bitcoin-core/secp256k1#650: secp256k1/src/tests.c: Properly handle sscanf return value
bitcoin-core/secp256k1#654: Fix typo (∞)
bitcoin-core/secp256k1#583: JNI: fix use sig array
bitcoin-core/secp256k1#644: Avoid optimizing out a verify_check
bitcoin-core/secp256k1#652: README.md: update instruction to run tests
bitcoin-core/secp256k1#651: Fix typo in secp256k1_preallocated.h
bitcoin-core/secp256k1#640: scalar_impl.h: fix includes
bitcoin-core/secp256k1#655: jni: Use only Guava for hex encoding and decoding
bitcoin-core/secp256k1#634: Add a descriptive comment for secp256k1_ecmult_const.
bitcoin-core/secp256k1#631: typo in comment for secp256k1_ec_pubkey_tweak_mul ()
bitcoin-core/secp256k1#629: Avoid calling _is_zero when _set_b32 fails.
bitcoin-core/secp256k1#630: Note intention of timing sidechannel freeness.
bitcoin-core/secp256k1#628: Fix ability to compile tests without -DVERIFY.
bitcoin-core/secp256k1#627: Guard memcmp in tests against mixed size inputs.
bitcoin-core/secp256k1#578: Avoid implementation-defined and undefined behavior when dealing with sizes
bitcoin-core/secp256k1#595: Allow to use external default callbacks
bitcoin-core/secp256k1#600: scratch space: use single allocation
bitcoin-core/secp256k1#592: Use trivial algorithm in ecmult_multi if scratch space is small
bitcoin-core/secp256k1#566: Enable context creation in preallocated memory
bitcoin-core/secp256k1#596: Make WINDOW_G configurable
bitcoin-core/secp256k1#561: Respect LDFLAGS and #undef STATIC_PRECOMPUTATION if using basic config
bitcoin-core/secp256k1#533: Make sure we’re not using an uninitialized variable in secp256k1_wnaf_const(…)
bitcoin-core/secp256k1#617: Pass scalar by reference in secp256k1_wnaf_const()
bitcoin-core/secp256k1#619: Clear a copied secret key after negation
bitcoin-core/secp256k1#612: Allow field_10x26_arm.s to compile for ARMv7 architecture

Squashed 'src/secp256k1/' changes from b19c000063..2ed54da18a

2ed54da18a Merge #755: Recovery signing: add to constant time test, and eliminate non ct operators
28609507e7 Add tests for the cmov implementations
73596a85a2 Add ecdsa_sign_recoverable to the ctime tests
2876af4f8d Split ecdsa_sign logic into a new function and use it from ecdsa_sign and recovery
5e1c885efb Merge #754: Fix uninit values passed into cmov
f79a7adcf5 Add valgrind uninit check to cmovs output
05d315affe Merge #752: autoconf: Use ":" instead of "dnl" as a noop
a39c2b09de Fixed UB(arithmetics on uninit values) in cmovs
3a6fd7f636 Merge #750: Add macOS to the CI
5e8747ae2a autoconf: Use ":" instead of "dnl" as a noop
71757da5cc Explictly pass SECP256K1_BENCH_ITERS to the benchmarks in travis.sh
99bd661d71 Replace travis_wait with a loop printing "\a" to stdout every minute
bc818b160c Bump travis Ubuntu from xenial(16.04) to bionic(18.04)
0c5ff9066e Add macOS support to travis
b6807d91d8 Move travis script into a standalone sh file
f39f99be0e Merge #701: Make ec_ arithmetic more consistent and add documentation
39198a03ea Merge #732: Retry if r is zero during signing
59a8de8f64 Merge #742: Fix typo in ecmult_const_impl.h
4e284655d9 Fix typo in ecmult_const_impl.h
f862b4ca13 Merge #740: Make recovery/main_impl.h non-executable
ffef45c98a Make recovery/main_impl.h non-executable
2361b3719a Merge #735: build: fix OpenSSL EC detection on macOS
3b7d26b23c build: add SECP_TEST_INCLUDES to bench_verify CPPFLAGS
84b5fc5bc3 build: fix OpenSSL EC detection on macOS
37ed51a7ea Make ecdsa_sig_sign constant-time again after reverting 25e3cfb
93d343bfc5 Revert "ecdsa_impl: replace scalar if-checks with VERIFY_CHECKs in ecdsa_sig_sign"
7e3952ae82 Clarify documentation of tweak functions.
89853a0f2e Make tweak function documentation more consistent.
41fc785602 Make ec_privkey functions aliases for ec_seckey_negate, ec_seckey_tweak_add and ec_seckey_mul
22911ee6da Rename private key to secret key in public API (with the exception of function names)
5a73f14d6c Mention that value is unspecified for In/Out parameters if the function returns 0
f03df0e6d7 Define valid ECDSA keys in the documentation of seckey_verify
5894e1f1df Return 0 if the given seckey is invalid in privkey_negate, privkey_tweak_add and privkey_tweak_mul
8f814cddb9 Add test for boundary conditions of scalar_set_b32 with respect to overflows
3fec982608 Use scalar_set_b32_seckey in ecdsa_sign, pubkey_create and seckey_verify
9ab2cbe0eb Add scalar_set_b32_seckey which does the same as scalar_set_b32 and also returns whether it's a valid secret key
4f27e344c6 Merge #728: Suppress a harmless variable-time optimization by clang in memczero
01993878bb Add test for memczero()
52a03512c1 Suppress a harmless variable-time optimization by clang in memczero
8f78e208ad Merge #722: Context isn't freed in the ECDH benchmark
ed1b91171a Merge #700: Allow overriding default flags
85b35afa76 Add running benchmarks regularly and under valgrind in travis
ca4906b02e Pass num of iters to benchmarks as variable, and define envvar
02dd5f1bbb free the ctx at the end of bench_ecdh
e9fccd4de1 Merge #708: Constant-time behaviour test using valgrind memtest.
08fb6c4926 Run valgrind_ctime_test in travis
3d2302257f Constant-time behaviour test using valgrind memtest.
96d8ccbd16 Merge #710: Eliminate harmless non-constant time operations on secret data.
0585b8b2ee Merge #718: Clarify that a secp256k1_ecdh_hash_function must return 0 or 1
7b50483ad7 Adds a declassify operation to aid constant-time analysis.
34a67c773b Eliminate harmless non-constant time operations on secret data.
ca739cba23 Compile with optimization flag -O2 by default instead of -O3
eb45ef3384 Clarify that a secp256k1_ecdh_hash_function must return 0 or 1
856a01d6ad Merge #714: doc: document the length requirements of output parameter.
d72b9e2483 Merge #682: Remove Java Native Interface
4b48a43106 doc: document the length requirements of output parameter.
1b4d256e2e Merge #713: Docstrings
dabfea7e21 field: extend docstring of secp256k1_fe_normalize
dc7d8fd9e2 scalar: extend docstring of secp256k1_scalar_set_b32
074ab582dd Merge #704: README: add a section for test coverage
acb7f97eb8 README: add a section for test coverage
227a4f2d07 Merge #709: Remove secret-dependant non-constant time operation in ecmult_const.
d567b779fe Clarify comments about use of rzr on ge functions and abs function.
2241ae6d14 Remove secret-dependant non-constant time operation in ecmult_const.
642cd062bd Remove Java Native Interface
83fb1bcef4 Remove -O2 from default CFLAGS because this would override the -O3 flag (see AC_PROG_CC in the Autoconf manual)
ecba8138ec Append instead of Prepend user-CFLAGS to default CFLAGS allowing the user to override default variables
613c34cd86 Remove test in configure.ac because it doesn't have an effect
f45d897101 Merge #703: Overhaul README.md
2e759ec753 Overhaul README.md
d644dda5c9 Merge #689: Remove "except in benchmarks" exception for fp math
bde2a32286 Convert bench.h to fixed-point math
387d723c3f Merge #679: Add SECURITY.md
0db61d25c9 Merge #685: Fix issue where travis does not show the ./tests seed…
a0771d15e6 Explicitly disable buffering for stderr in tests
fb424fbba2 Make travis show the ./tests seed by removing stdout buffering and always cat tests.log after a travis run.
22a6031184 Merge #690: Add valgrind check to travis
544002c008 Merge #678: Preventing compiler optimizations in benchmarks without a memory fence
dd98cc988f travis: Added a valgrind test without endro and enabled recovery+ecdh
b4c1382a87 Add valgrind check to travis
0c774d89e6 Merge #688: Fix ASM setting in travis
5c5f71eea5 Fix ASM setting in travis
e2625f8a98 Merge #684: Make no-float policy explicit
bae1bea3c4 Make no-float policy explicit
78c3836341 Add SECURITY.md
362bb25608 Modified bench_scalar_split so it won't get optimized out
73a30c6b58 Added accumulators and checks on benchmarks so they won't get optimized out
770b3dcd6f Merge #677: Remove note about heap allocation in secp256k1_ecmult_odd_multiples_table_storage_var
b76142ff25 Remove note about heap allocation in secp256k1_ecmult_odd_multiples_table_storage_var which was removed in 47045270fa90f81205d989f7107769bce1e71c4d
137d304a6b Merge #647: Increase robustness against UB in secp256k1_scalar_cadd_bit
0d9540b13f Merge #664: Remove mention of ec_privkey_export because it doesn't exist
59782c68b4 Remove mention of ec_privkey_export because it doesn't exist
96cd94e385 Merge #337: variable sized precomputed table for signing
dcb2e3b3ff variable signing precompute table
b4bff99028 Merge #661: Make ./configure string consistent
a467047e11 Make ./configure string consistent
e729cc7f5a Merge #657: Fix a nit in the recovery tests
b64a2e2597 Fix a nit in the recovery tests
e028aa33d3 Merge #650: secp256k1/src/tests.c:  Properly handle sscanf return value
f1e11d363d Merge #654: Fix typo (∞)
ef83281c3a Merge pull request #656 from real-or-random/patch-1
556caad2ca Fix typo in docs for _context_set_illegal_callback
0d82732a9a Improve VERIFY_CHECK of overflow in secp256k1_scalar_cadd_bit. This added check ensures that any curve order overflow doesn't go undetected due a uint32_t overflow.
786dfb49f5 Merge #583: JNI: fix use sig array
e95f8ab098 Merge #644: Avoid optimizing out a verify_check
384f55606a Merge #652: README.md: update instruction to run tests
ee56accd47 Merge #651: Fix typo in secp256k1_preallocated.h
7b9b117230 Merge #640: scalar_impl.h: fix includes
d99bec2e21 Merge #655: jni: Use only Guava for hex encoding and decoding
2abcf951af jni: Use only Guava for hex encoding and decoding
271582b3b7 Fix typo
ce6d438266 README.md: update instruction to run tests
b1e68cb8e6 Fix typo in secp256k1_preallocated.h
a11c76c59a secp256k1/src/tests.c:  Properly handle sscanf return value
8fe63e5654 Increase robustness against UB. Thanks to elichai2 who noted that the literal '1' is a signed integer, and that shifting a signed 32-bit integer by 31 bits causes an overflow and yields undefined behaviour. While 'scalar_low_impl''s 'secp256k1_scalar_cadd_bit' is only used for testing purposes and currently the 'bit' parameter is only 0 or 1, it is better to avoid undefined behaviour in case the used domain of 'secp256k1_scalar_cadd_bit' expands.
94ae7cbf83 Moved a dereference so the null check will be before the dereferencing
2cb73b1064 scalar_impl.h: fix includes
fa33017135 Merge #634: Add a descriptive comment for secp256k1_ecmult_const.
ee9e68cd30 Add a descriptive comment for secp256k1_ecmult_const.
d0d738d32d Merge #631: typo in comment for secp256k1_ec_pubkey_tweak_mul ()
6914c25276 typo in comment for secp256k1_ec_pubkey_tweak_mul ()
e541a90ef6 Merge #629: Avoid calling _is_zero when _set_b32 fails.
f34b0c3f35 Merge #630: Note intention of timing sidechannel freeness.
8d1563b0ff Note intention of timing sidechannel freeness.
1669bb2865 Merge #628: Fix ability to compile tests without -DVERIFY.
ecc94abcc8 Merge #627: Guard memcmp in tests against mixed size inputs.
544435fc90 Merge #578: Avoid implementation-defined and undefined behavior when dealing with sizes
143dc6e9ee Merge #595: Allow to use external default callbacks
e49f7991c2 Add missing #(un)defines to base-config.h
77defd2c3b Add secp256k1_ prefix to default callback functions
908bdce64e Include stdio.h and stdlib.h explicitly in secp256k1.c
5db782e655 Allow usage of external default callbacks
6095a863fa Replace CHECKs for no_precomp ctx by ARG_CHECKs without a return
cd473e02c3 Avoid calling secp256k1_*_is_zero when secp256k1_*_set_b32 fails.
6c36de7a33 Merge #600: scratch space: use single allocation
98836b11f0 scratch: replace frames with "checkpoint" system
7623cf2b97 scratch: save a couple bytes of unnecessarily-allocated memory
a7a164f2c6 scratch: rename `max_size` to `size`, document that extra will actually be allocated
5a4bc0bb95 scratch: unify allocations
c2b028a281 scratch space: thread `error_callback` into all scratch space functions
0be1a4ae62 scratch: add magic bytes to beginning of structure
92a48a764d scratch space: use single allocation
40839e21b9 Merge #592: Use trivial algorithm in ecmult_multi if scratch space is small
dcf392027b Fix ability to compile tests without -DVERIFY.
a484e0008b Merge #566: Enable context creation in preallocated memory
0522caac8f Explain caller's obligations for preallocated memory
238305fdbb Move _preallocated functions to separate header
695feb6fbd Export _preallocated functions
814cc78d71 Add tests for contexts in preallocated memory
ba12dd08da Check arguments of _preallocated functions
5feadde462 Support cloning a context into preallocated memory
c4fd5dab45 Switch to a single malloc call
ef020de16f Add size constants for preallocated memory
1bf7c056ba Prepare for manual memory management in preallocated memory
248bffb052 Guard memcmp in tests against mixed size inputs.
36698dcfee Merge #596: Make WINDOW_G configurable
a61a93ff50 Clean up ./configure help strings
2842dc523e Make WINDOW_G configurable
1a02d6ce51 Merge #626: Revert "Merge #620: Install headers automatically"
662918cb29 Revert "Merge #620: Install headers automatically"
14c7dbd444 Simplify control flow in DER parsing
ec8f20babd Avoid out-of-bound pointers and integer overflows in size comparisons
01ee1b3b3c Parse DER-enconded length into a size_t instead of an int
912680ed86 Merge #561: Respect LDFLAGS and #undef STATIC_PRECOMPUTATION if using basic config
91fae3ace0 Merge #620: Install headers automatically
5df77a0eda Merge #533: Make sure we're not using an uninitialized variable in secp256k1_wnaf_const(...)
975e51e0d9 Merge #617: Pass scalar by reference in secp256k1_wnaf_const()
735fbde04e Merge #619: Clear a copied secret key after negation
16e86150d0 Install headers automatically
069870d92a Clear a copied secret key after negation
8979ec0d9a Pass scalar by reference in secp256k1_wnaf_const()
84a808598b Merge #612: Allow field_10x26_arm.s to compile for ARMv7 architecture
d4d270a59c Allow field_10x26_arm.s to compile for ARMv7 architecture
248f046611 Make sure we're not using an uninitialized variable in secp256k1_wnaf_const(...)
9ab96f7b12 Use trivial algorithm in ecmult_multi if scratch space is small
dbed75d969 Undefine `STATIC_PRECOMPUTATION` if using the basic config
310111e093 Keep LDFLAGS if `--coverage`
74e2dbd68e JNI: fix use sig array
3cb057f842 Fix possible integer overflow in DER parsing

git-subtree-dir: src/secp256k1
git-subtree-split: 2ed54da18add295668ec71c91534b640d2cc029b

67f232b5d8

Update src/secp256k1 subtree 8903a1a0a7

Update MSVC build config for libsecp256k1 ddc2419c09

DrahtBot added the label Build system on Jun 9, 2020

DrahtBot added the label Docs on Jun 9, 2020

jonasnick approved

jonasnick commented at 9:52 pm on June 9, 2020: contributor

ACK ddc2419c090b0af65edc9eb07ac0a736eb351b69

git subtree pull --prefix src/secp256k1 secp256k1 2ed54da18add295668ec71c91534b640d2cc029b --squash gives the same result for me (and it results in an identical directory as my secp256k1 master).

MarcoFalke removed the label Docs on Jun 9, 2020

MarcoFalke added the label Utils/log/libs on Jun 9, 2020

MarcoFalke removed the label Build system on Jun 9, 2020

MarcoFalke commented at 9:56 pm on June 9, 2020: member

Concept ACK

fanquake dismissed

fanquake commented at 5:50 am on June 10, 2020: member

Concept ACK. Thanks for following up and splitting this out.

Can you update our libsecp256 configure args to remove --disable-jni (https://github.com/bitcoin-core/secp256k1/pull/682):

https://github.com/bitcoin/bitcoin/blob/f8364df25070cea08f0fb5bbbb212f1ff72f9d21/configure.ac#L1724

If you want, you could also pull in https://github.com/fanquake/bitcoin/commit/409b228709717e5eea021256c2dc89b40797f989, to enable configure option checking, which will emit warnings if unknown options are passed (can’t be made fatal due to a different issue) i.e:

0configure: WARNING: unrecognized options: --disable-jni

Subtree check is ok:

0./test/lint/git-subtree-check.sh src/secp256k1     
1src/secp256k1 in HEAD currently refers to tree 8ea0c31c64dfe0dd8f6f775c444cd22ef94d17cc
2src/secp256k1 in HEAD was last updated in commit 67f232b5d874b501c114bced5d764db7f4f5ce99 (tree 8ea0c31c64dfe0dd8f6f775c444cd22ef94d17cc)
3src/secp256k1 in HEAD was last updated to upstream commit 2ed54da18add295668ec71c91534b640d2cc029b (tree 8ea0c31c64dfe0dd8f6f775c444cd22ef94d17cc)
4GOOD

cc @real-or-random.

sipa commented at 6:02 am on June 10, 2020: member

@fanquake Pulled in.

sipa force-pushed on Jun 10, 2020

real-or-random commented at 7:38 am on June 10, 2020: member

cc @elichai who worked on many of those PRs.

We renamed the privkey_* functions because the naming was inconsistent. They still work but API users should call seckey_* functions, see https://github.com/bitcoin-core/secp256k1/pull/701/commits/41fc78560223aa035d9fe2cbeedb3ee632c740e2 . Can you add a commit that changes the calls in key.cpp (privkey_negate, privkey_tweak)? This could be done in a separate PR but I think it belongs here conceptually. edit: We may in fact remove the wrapper for privkey_negate because it is used nowhere.

We also made changes to ecdsa_signature_parse_der_lax in contrib/lax_der_parsing.c. Unfortunately this is a small mess now. The copy of this function in pubkey.cpp has diverged from the secp256k1 version due to #10657 and #11073 which were simply merged here without upstreaming them to secp256k1. Some of our secp256k1 changes reinvent those in #10657 (with the exact same diff) because I was not aware of the divergence when I proposed them. The remaining differences are:

#11073 removed a dead store in pubkey.cpp. We can simply upstream this change to secp256k1.
#10657 added static_assert checks. But those are already covered by assumptions.h. In fact, this is even mentioned as an example in assumptions.h. https://github.com/bitcoin/bitcoin/blob/fed1a9043fdf802c7cf7eab1c8aab25ca1c90e8d/src/compat/assumptions.h#L55-L58 So we could simply remove the assertions in pubkey.cpp.

If we do those two changes (the second change belonging to this repo), we will have a clean copy of ecdsa_signature_parse_der_lax again. But maybe this is non-trivial enough to go to a second PR. I’m volunteering to open a PR after I digged through all the stuff now.

in configure.ac:1724 in c5b460170b outdated

1720@@ -1721,7 +1721,7 @@ if test x$need_bundled_univalue = xyes; then
1721   AC_CONFIG_SUBDIRS([src/univalue])
1722 fi
1723 
1724-ac_configure_args="${ac_configure_args} --disable-shared --with-pic --enable-benchmark=no --with-bignum=no --enable-module-recovery --disable-jni"
1725+ac_configure_args="${ac_configure_args} --enable-option-checking --disable-shared --with-pic --enable-benchmark=no --with-bignum=no --enable-module-recovery"

elichai commented at 8:41 am on June 10, 2020:

This isn’t really ergonomic, because ac_configure_args="${ac_configure_args}..." passes to secp’s configure everything you pass into bitcoin’s configure (not sure if this is wanted behavior or not), but now with --enable-option-checking if I pass any bitcoin specific flags like ./configure --with-incompatible-bdb --with-libs --with-daemon --with-gui I get a warning: configure: WARNING: unrecognized options: --with-incompatible-bdb, --with-libs, --with-daemon, --with-gui

fanquake commented at 9:12 am on June 10, 2020:

Right. After suggesting I started working on a change to fix this up the problem you mention (configure options getting passed around), as it’s not very clean at the moment. We can just revert the addition of --enable-option-checking for now and drop --disable-jni.

laanwj commented at 1:08 pm on June 10, 2020:

(not sure if this is wanted behavior or not),

It’s desirable behavior in as far it is the only way to pass configure flags down to secp256k1 and other subtrees. For example to experiment during development, or to enable some specific assembly flag for optimization. I guess if there was another officially supported way to do that it might be unnecessary.

elichai commented at 1:14 pm on June 10, 2020:

in the past I always edited configure.ac to change libsecp’s configure, so good to know :) (I actually have a local patch that enables endomorphism and more features when i compile for my laptop, so I’ll replace the patch with a configure line :) )

elichai commented at 9:48 am on June 10, 2020: contributor

tACK c5b460170b9df0097026fb8c772ccbde9f181d50 (make check passes with ./configure --with-incompatible-bdb --with-gui)

 0$ git fetch https://github.com/bitcoin-core/secp256k1.git 
 1remote: Enumerating objects: 449, done.
 2remote: Counting objects: 100% (449/449), done.
 3remote: Compressing objects: 100% (28/28), done.
 4remote: Total 1358 (delta 428), reused 440 (delta 421), pack-reused 909
 5Receiving objects: 100% (1358/1358), 700.11 KiB | 745.00 KiB/s, done.
 6Resolving deltas: 100% (923/923), completed with 95 local objects.
 7From https://github.com/bitcoin-core/secp256k1
 8 * branch                  HEAD       -> FETCH_HEAD
 9
10$ ./test/lint/git-subtree-check.sh src/secp256k1
11src/secp256k1 in HEAD currently refers to tree 8ea0c31c64dfe0dd8f6f775c444cd22ef94d17cc
12src/secp256k1 in HEAD was last updated in commit 67f232b5d874b501c114bced5d764db7f4f5ce99 (tree 8ea0c31c64dfe0dd8f6f775c444cd22ef94d17cc)
13src/secp256k1 in HEAD was last updated to upstream commit 2ed54da18add295668ec71c91534b640d2cc029b (tree 8ea0c31c64dfe0dd8f6f775c444cd22ef94d17cc)
14GOOD

laanwj commented at 1:21 pm on June 10, 2020: member

Concept ACK

DrahtBot commented at 1:41 pm on June 10, 2020: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

Drop --disable-jni from libsecp256k1 configure options ca8bc42330

scripted-diff: rename privkey with seckey in secp256k1 interface

-BEGIN VERIFY SCRIPT-
sed -i 's/privkey/seckey/g' src/key.cpp
-END VERIFY SCRIPT-

e10439ce5a

sipa force-pushed on Jun 11, 2020

sipa commented at 1:32 am on June 11, 2020: member

@real-or-random Thanks for pointing that out. I added a commit to rename privkey to seckey in key.cpp (as a scripted diff). I’ve left the DER parsing code be for now, as it seems unclear what changes to address where. @fanquake I’ve reverted the options checking, and just changed it to dropping –disable-jni.

fanquake requested review from fanquake on Jun 11, 2020

Sjors commented at 12:24 pm on June 12, 2020: member

ACK e10439ce5a54cd13062e4ed07ebc681e385ed5cb

I verified the subtree (see #19258) and scripted diff. The build_msvc variables match the defaults for ./configure.ac.

Upstream commit range, based on running git-subtree-check.sh on master vs here: https://github.com/bitcoin-core/secp256k1/compare/b19c000063be11018b4d1a6b0a85871ab9d0bdcf...2ed54da18add295668ec71c91534b640d2cc029b (or use git log --reverse)

real-or-random commented at 5:49 pm on June 12, 2020: member

ACK e10439ce5a54cd13062e4ed07ebc681e385ed5cb I verified the diff (subtree matches my local tree, manual inspection of other commits) but I didn’t tested the resulting code

@real-or-random Thanks for pointing that out. I added a commit to rename privkey to seckey in key.cpp (as a scripted diff).

This led to changes also in ec_privkey_import_der (or now ec_seckey_import_der). That’s good, we should also do this upstream. (I think we forgot this upstream because it’s in contrib…)

I’ve left the DER parsing code be for now, as it seems unclear what changes to address where.

Ok, let’s do this in a different PR.

jonasnick commented at 8:18 pm on June 12, 2020: contributor

reACK e10439ce5a54cd13062e4ed07ebc681e385ed5cb

fanquake approved

fanquake commented at 1:48 am on June 13, 2020: member

ACK e10439ce5a54cd13062e4ed07ebc681e385ed5cb

0./test/lint/git-subtree-check.sh src/secp256k1
1src/secp256k1 in HEAD currently refers to tree 8ea0c31c64dfe0dd8f6f775c444cd22ef94d17cc
2src/secp256k1 in HEAD was last updated in commit 67f232b5d874b501c114bced5d764db7f4f5ce99 (tree 8ea0c31c64dfe0dd8f6f775c444cd22ef94d17cc)
3src/secp256k1 in HEAD was last updated to upstream commit 2ed54da18add295668ec71c91534b640d2cc029b (tree 8ea0c31c64dfe0dd8f6f775c444cd22ef94d17cc)
4GOOD

I have a branch that may improve the situation with passing custom configure args to sub directories, so will follow up.

fanquake merged this on Jun 13, 2020

fanquake closed this on Jun 13, 2020

sidhujag referenced this in commit 8a3a072968 on Jun 13, 2020

ComputerCraftr referenced this in commit b98f1c6e6c on Jun 16, 2020

laanwj commented at 12:19 pm on June 16, 2020: member

If you get the following error after it, you need to clean your tree (make clean seems to do it in this case).

0make[2]: *** [Makefile:7782: bitcoin-wallet] Error 1                                                                                                                                                                                
1/usr/bin/ld: libbitcoin_common.a(libbitcoin_common_a-key.o): in function `CKey::Negate()':                                                                                                                                          
2/…/bitcoin/src/key.cpp:168: undefined reference to `secp256k1_ec_seckey_negate'                                                                                                                           
3/usr/bin/ld: libbitcoin_common.a(libbitcoin_common_a-key.o): in function `CKey::Derive(CKey&, uint256&, unsigned int, uint256 const&) const':                                                                                       
4/…/bitcoin/src/key.cpp:287: undefined reference to `secp256k1_ec_seckey_tweak_add'

fanquake referenced this in commit f00d6575ca on Jun 29, 2020

jasonbcox referenced this in commit 1f985e5a3b on Sep 27, 2020

deadalnix referenced this in commit 7fe66ef9d1 on Sep 28, 2020

UdjinM6 referenced this in commit 9d36ba6570 on Aug 10, 2021

5tefan referenced this in commit 8ded2caa74 on Aug 12, 2021

DrahtBot locked this on Feb 15, 2022

Update libsecp256k1 subtree #19228

Conflicts