Update libsecp256k1 subtree #19228

sipa · 2020-06-09T20:50:36Z

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

Here is a list of the included PRs:

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

2ed54da Merge bitcoin#755: Recovery signing: add to constant time test, and eliminate non ct operators 2860950 Add tests for the cmov implementations 73596a8 Add ecdsa_sign_recoverable to the ctime tests 2876af4 Split ecdsa_sign logic into a new function and use it from ecdsa_sign and recovery 5e1c885 Merge bitcoin#754: Fix uninit values passed into cmov f79a7ad Add valgrind uninit check to cmovs output 05d315a Merge bitcoin#752: autoconf: Use ":" instead of "dnl" as a noop a39c2b0 Fixed UB(arithmetics on uninit values) in cmovs 3a6fd7f Merge bitcoin#750: Add macOS to the CI 5e8747a autoconf: Use ":" instead of "dnl" as a noop 71757da Explictly pass SECP256K1_BENCH_ITERS to the benchmarks in travis.sh 99bd661 Replace travis_wait with a loop printing "\a" to stdout every minute bc818b1 Bump travis Ubuntu from xenial(16.04) to bionic(18.04) 0c5ff90 Add macOS support to travis b6807d9 Move travis script into a standalone sh file f39f99b Merge bitcoin#701: Make ec_ arithmetic more consistent and add documentation 39198a0 Merge bitcoin#732: Retry if r is zero during signing 59a8de8 Merge bitcoin#742: Fix typo in ecmult_const_impl.h 4e28465 Fix typo in ecmult_const_impl.h f862b4c Merge bitcoin#740: Make recovery/main_impl.h non-executable ffef45c Make recovery/main_impl.h non-executable 2361b37 Merge bitcoin#735: build: fix OpenSSL EC detection on macOS 3b7d26b build: add SECP_TEST_INCLUDES to bench_verify CPPFLAGS 84b5fc5 build: fix OpenSSL EC detection on macOS 37ed51a Make ecdsa_sig_sign constant-time again after reverting 25e3cfb 93d343b Revert "ecdsa_impl: replace scalar if-checks with VERIFY_CHECKs in ecdsa_sig_sign" 7e3952a Clarify documentation of tweak functions. 89853a0 Make tweak function documentation more consistent. 41fc785 Make ec_privkey functions aliases for ec_seckey_negate, ec_seckey_tweak_add and ec_seckey_mul 22911ee Rename private key to secret key in public API (with the exception of function names) 5a73f14 Mention that value is unspecified for In/Out parameters if the function returns 0 f03df0e Define valid ECDSA keys in the documentation of seckey_verify 5894e1f Return 0 if the given seckey is invalid in privkey_negate, privkey_tweak_add and privkey_tweak_mul 8f814cd Add test for boundary conditions of scalar_set_b32 with respect to overflows 3fec982 Use scalar_set_b32_seckey in ecdsa_sign, pubkey_create and seckey_verify 9ab2cbe Add scalar_set_b32_seckey which does the same as scalar_set_b32 and also returns whether it's a valid secret key 4f27e34 Merge bitcoin#728: Suppress a harmless variable-time optimization by clang in memczero 0199387 Add test for memczero() 52a0351 Suppress a harmless variable-time optimization by clang in memczero 8f78e20 Merge bitcoin#722: Context isn't freed in the ECDH benchmark ed1b911 Merge bitcoin#700: Allow overriding default flags 85b35af Add running benchmarks regularly and under valgrind in travis ca4906b Pass num of iters to benchmarks as variable, and define envvar 02dd5f1 free the ctx at the end of bench_ecdh e9fccd4 Merge bitcoin#708: Constant-time behaviour test using valgrind memtest. 08fb6c4 Run valgrind_ctime_test in travis 3d23022 Constant-time behaviour test using valgrind memtest. 96d8ccb Merge bitcoin#710: Eliminate harmless non-constant time operations on secret data. 0585b8b Merge bitcoin#718: Clarify that a secp256k1_ecdh_hash_function must return 0 or 1 7b50483 Adds a declassify operation to aid constant-time analysis. 34a67c7 Eliminate harmless non-constant time operations on secret data. ca739cb Compile with optimization flag -O2 by default instead of -O3 eb45ef3 Clarify that a secp256k1_ecdh_hash_function must return 0 or 1 856a01d Merge bitcoin#714: doc: document the length requirements of output parameter. d72b9e2 Merge bitcoin#682: Remove Java Native Interface 4b48a43 doc: document the length requirements of output parameter. 1b4d256 Merge bitcoin#713: Docstrings dabfea7 field: extend docstring of secp256k1_fe_normalize dc7d8fd scalar: extend docstring of secp256k1_scalar_set_b32 074ab58 Merge bitcoin#704: README: add a section for test coverage acb7f97 README: add a section for test coverage 227a4f2 Merge bitcoin#709: Remove secret-dependant non-constant time operation in ecmult_const. d567b77 Clarify comments about use of rzr on ge functions and abs function. 2241ae6 Remove secret-dependant non-constant time operation in ecmult_const. 642cd06 Remove Java Native Interface 83fb1bc Remove -O2 from default CFLAGS because this would override the -O3 flag (see AC_PROG_CC in the Autoconf manual) ecba813 Append instead of Prepend user-CFLAGS to default CFLAGS allowing the user to override default variables 613c34c Remove test in configure.ac because it doesn't have an effect f45d897 Merge bitcoin#703: Overhaul README.md 2e759ec Overhaul README.md d644dda Merge bitcoin#689: Remove "except in benchmarks" exception for fp math bde2a32 Convert bench.h to fixed-point math 387d723 Merge bitcoin#679: Add SECURITY.md 0db61d2 Merge bitcoin#685: Fix issue where travis does not show the ./tests seed… a0771d1 Explicitly disable buffering for stderr in tests fb424fb Make travis show the ./tests seed by removing stdout buffering and always cat tests.log after a travis run. 22a6031 Merge bitcoin#690: Add valgrind check to travis 544002c Merge bitcoin#678: Preventing compiler optimizations in benchmarks without a memory fence dd98cc9 travis: Added a valgrind test without endro and enabled recovery+ecdh b4c1382 Add valgrind check to travis 0c774d8 Merge bitcoin#688: Fix ASM setting in travis 5c5f71e Fix ASM setting in travis e2625f8 Merge bitcoin#684: Make no-float policy explicit bae1bea Make no-float policy explicit 78c3836 Add SECURITY.md 362bb25 Modified bench_scalar_split so it won't get optimized out 73a30c6 Added accumulators and checks on benchmarks so they won't get optimized out 770b3dc Merge bitcoin#677: Remove note about heap allocation in secp256k1_ecmult_odd_multiples_table_storage_var b76142f Remove note about heap allocation in secp256k1_ecmult_odd_multiples_table_storage_var which was removed in 4704527 137d304 Merge bitcoin#647: Increase robustness against UB in secp256k1_scalar_cadd_bit 0d9540b Merge bitcoin#664: Remove mention of ec_privkey_export because it doesn't exist 59782c6 Remove mention of ec_privkey_export because it doesn't exist 96cd94e Merge bitcoin#337: variable sized precomputed table for signing dcb2e3b variable signing precompute table b4bff99 Merge bitcoin#661: Make ./configure string consistent a467047 Make ./configure string consistent e729cc7 Merge bitcoin#657: Fix a nit in the recovery tests b64a2e2 Fix a nit in the recovery tests e028aa3 Merge bitcoin#650: secp256k1/src/tests.c: Properly handle sscanf return value f1e11d3 Merge bitcoin#654: Fix typo (∞) ef83281 Merge pull request bitcoin#656 from real-or-random/patch-1 556caad Fix typo in docs for _context_set_illegal_callback 0d82732 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. 786dfb4 Merge bitcoin#583: JNI: fix use sig array e95f8ab Merge bitcoin#644: Avoid optimizing out a verify_check 384f556 Merge bitcoin#652: README.md: update instruction to run tests ee56acc Merge bitcoin#651: Fix typo in secp256k1_preallocated.h 7b9b117 Merge bitcoin#640: scalar_impl.h: fix includes d99bec2 Merge bitcoin#655: jni: Use only Guava for hex encoding and decoding 2abcf95 jni: Use only Guava for hex encoding and decoding 271582b Fix typo ce6d438 README.md: update instruction to run tests b1e68cb Fix typo in secp256k1_preallocated.h a11c76c secp256k1/src/tests.c: Properly handle sscanf return value 8fe63e5 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. 94ae7cb Moved a dereference so the null check will be before the dereferencing 2cb73b1 scalar_impl.h: fix includes fa33017 Merge bitcoin#634: Add a descriptive comment for secp256k1_ecmult_const. ee9e68c Add a descriptive comment for secp256k1_ecmult_const. d0d738d Merge bitcoin#631: typo in comment for secp256k1_ec_pubkey_tweak_mul () 6914c25 typo in comment for secp256k1_ec_pubkey_tweak_mul () e541a90 Merge bitcoin#629: Avoid calling _is_zero when _set_b32 fails. f34b0c3 Merge bitcoin#630: Note intention of timing sidechannel freeness. 8d1563b Note intention of timing sidechannel freeness. 1669bb2 Merge bitcoin#628: Fix ability to compile tests without -DVERIFY. ecc94ab Merge bitcoin#627: Guard memcmp in tests against mixed size inputs. 544435f Merge bitcoin#578: Avoid implementation-defined and undefined behavior when dealing with sizes 143dc6e Merge bitcoin#595: Allow to use external default callbacks e49f799 Add missing #(un)defines to base-config.h 77defd2 Add secp256k1_ prefix to default callback functions 908bdce Include stdio.h and stdlib.h explicitly in secp256k1.c 5db782e Allow usage of external default callbacks 6095a86 Replace CHECKs for no_precomp ctx by ARG_CHECKs without a return cd473e0 Avoid calling secp256k1_*_is_zero when secp256k1_*_set_b32 fails. 6c36de7 Merge bitcoin#600: scratch space: use single allocation 98836b1 scratch: replace frames with "checkpoint" system 7623cf2 scratch: save a couple bytes of unnecessarily-allocated memory a7a164f scratch: rename `max_size` to `size`, document that extra will actually be allocated 5a4bc0b scratch: unify allocations c2b028a scratch space: thread `error_callback` into all scratch space functions 0be1a4a scratch: add magic bytes to beginning of structure 92a48a7 scratch space: use single allocation 40839e2 Merge bitcoin#592: Use trivial algorithm in ecmult_multi if scratch space is small dcf3920 Fix ability to compile tests without -DVERIFY. a484e00 Merge bitcoin#566: Enable context creation in preallocated memory 0522caa Explain caller's obligations for preallocated memory 238305f Move _preallocated functions to separate header 695feb6 Export _preallocated functions 814cc78 Add tests for contexts in preallocated memory ba12dd0 Check arguments of _preallocated functions 5feadde Support cloning a context into preallocated memory c4fd5da Switch to a single malloc call ef020de Add size constants for preallocated memory 1bf7c05 Prepare for manual memory management in preallocated memory 248bffb Guard memcmp in tests against mixed size inputs. 36698dc Merge bitcoin#596: Make WINDOW_G configurable a61a93f Clean up ./configure help strings 2842dc5 Make WINDOW_G configurable 1a02d6c Merge bitcoin#626: Revert "Merge bitcoin#620: Install headers automatically" 662918c Revert "Merge bitcoin#620: Install headers automatically" 14c7dbd Simplify control flow in DER parsing ec8f20b Avoid out-of-bound pointers and integer overflows in size comparisons 01ee1b3 Parse DER-enconded length into a size_t instead of an int 912680e Merge bitcoin#561: Respect LDFLAGS and #undef STATIC_PRECOMPUTATION if using basic config 91fae3a Merge bitcoin#620: Install headers automatically 5df77a0 Merge bitcoin#533: Make sure we're not using an uninitialized variable in secp256k1_wnaf_const(...) 975e51e Merge bitcoin#617: Pass scalar by reference in secp256k1_wnaf_const() 735fbde Merge bitcoin#619: Clear a copied secret key after negation 16e8615 Install headers automatically 069870d Clear a copied secret key after negation 8979ec0 Pass scalar by reference in secp256k1_wnaf_const() 84a8085 Merge bitcoin#612: Allow field_10x26_arm.s to compile for ARMv7 architecture d4d270a Allow field_10x26_arm.s to compile for ARMv7 architecture 248f046 Make sure we're not using an uninitialized variable in secp256k1_wnaf_const(...) 9ab96f7 Use trivial algorithm in ecmult_multi if scratch space is small dbed75d Undefine `STATIC_PRECOMPUTATION` if using the basic config 310111e Keep LDFLAGS if `--coverage` 74e2dbd JNI: fix use sig array 3cb057f Fix possible integer overflow in DER parsing git-subtree-dir: src/secp256k1 git-subtree-split: 2ed54da

jonasnick

ACK ddc2419

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).

maflcko · 2020-06-09T21:56:20Z

Concept ACK

fanquake

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

Can you update our libsecp256 configure args to remove --disable-jni (bitcoin-core/secp256k1#682):

bitcoin/configure.ac

Line 1724 in f8364df

    
           ac_configure_args="${ac_configure_args} --disable-shared --with-pic --enable-benchmark=no --with-bignum=no --enable-module-recovery --disable-jni"

If you want, you could also pull in fanquake@409b228, 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:

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

Subtree check is ok:

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

cc @real-or-random.

sipa · 2020-06-10T06:02:18Z

@fanquake Pulled in.

real-or-random · 2020-06-10T07:38:39Z

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 bitcoin-core/secp256k1@41fc785 . 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:

Remove dead store in ecdsa_signature_parse_der_lax. #11073 removed a dead store in pubkey.cpp. We can simply upstream this change to secp256k1.

Utils: Improvements to ECDSA key-handling code #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.

bitcoin/src/compat/assumptions.h

Lines 55 to 58 in fed1a90

    
           // Assumption: We assume size_t to be 32-bit or 64-bit. 
        
           // Example(s): size_t assumed to be at least 32-bit in ecdsa_signature_parse_der_lax(...). 
        
           //             size_t assumed to be 32-bit or 64-bit in MallocUsage(...). 
        
           static_assert(sizeof(size_t) == 4 || sizeof(size_t) == 8, "size_t assumed to be 32-bit or 64-bit");

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.

configure.ac

elichai · 2020-06-10T09:48:01Z

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

$ git fetch https://github.com/bitcoin-core/secp256k1.git 
remote: Enumerating objects: 449, done.
remote: Counting objects: 100% (449/449), done.
remote: Compressing objects: 100% (28/28), done.
remote: Total 1358 (delta 428), reused 440 (delta 421), pack-reused 909
Receiving objects: 100% (1358/1358), 700.11 KiB | 745.00 KiB/s, done.
Resolving deltas: 100% (923/923), completed with 95 local objects.
From https://github.com/bitcoin-core/secp256k1
 * branch                  HEAD       -> FETCH_HEAD

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

laanwj · 2020-06-10T13:21:28Z

Concept ACK

DrahtBot · 2020-06-10T13:41:43Z

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

Conflicts

No conflicts as of last run.

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

sipa · 2020-06-11T01:32:32Z

@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.

Changes made.

Sjors · 2020-06-12T12:24:29Z

ACK e10439c

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: bitcoin-core/secp256k1@b19c000...2ed54da (or use git log --reverse)

real-or-random · 2020-06-12T17:49:23Z

ACK e10439c 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 · 2020-06-12T20:18:44Z

reACK e10439c

fanquake

ACK e10439c

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

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

e10439c scripted-diff: rename privkey with seckey in secp256k1 interface (Pieter Wuille) ca8bc42 Drop --disable-jni from libsecp256k1 configure options (Pieter Wuille) ddc2419 Update MSVC build config for libsecp256k1 (Pieter Wuille) 67f232b Squashed 'src/secp256k1/' changes from b19c000..2ed54da (Pieter Wuille) Pull request description: 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 ACKs for top commit: real-or-random: ACK e10439c I verified the diff (subtree matches my local tree, manual inspection of other commits) but I didn't tested the resulting code fanquake: ACK e10439c Sjors: ACK e10439c jonasnick: reACK e10439c Tree-SHA512: eb6284a485da78e9d2ed3f771df85560d47c770ebf480a0d4121ab356ad26be101a2b973efe412f26e6c142bc1dbd2efbb5cc08774233e41918c59fe3dff3387

laanwj · 2020-06-16T12:19:11Z

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

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

This change was made in bitcoin/bitcoin without upstreaming. So this is a followup to the comment here: bitcoin/bitcoin#19228 (comment). See also: bitcoin/bitcoin#11073.

Summary: This change was made in bitcoin/bitcoin without upstreaming. So this is a followup to the comment here: bitcoin/bitcoin#19228 (comment). See also: bitcoin/bitcoin#11073. This is a backport of libsecp256k1 [[bitcoin-core/secp256k1#765 | PR765]] Test Plan: ninja check-secp256k1 Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D7592

e10439c scripted-diff: rename privkey with seckey in secp256k1 interface (Pieter Wuille) ca8bc42 Drop --disable-jni from libsecp256k1 configure options (Pieter Wuille) ddc2419 Update MSVC build config for libsecp256k1 (Pieter Wuille) 67f232b Squashed 'src/secp256k1/' changes from b19c000..2ed54da (Pieter Wuille) Pull request description: 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 ACKs for top commit: real-or-random: ACK e10439c I verified the diff (subtree matches my local tree, manual inspection of other commits) but I didn't tested the resulting code fanquake: ACK e10439c Sjors: ACK e10439c jonasnick: reACK e10439c Tree-SHA512: eb6284a485da78e9d2ed3f771df85560d47c770ebf480a0d4121ab356ad26be101a2b973efe412f26e6c142bc1dbd2efbb5cc08774233e41918c59fe3dff3387

sipa added 3 commits June 9, 2020 13:39

Update src/secp256k1 subtree

8903a1a

Update MSVC build config for libsecp256k1

ddc2419

DrahtBot added Build system Docs labels Jun 9, 2020

jonasnick approved these changes Jun 9, 2020

View reviewed changes

maflcko added Utils/log/libs and removed Docs Build system labels Jun 9, 2020

sipa mentioned this pull request Jun 10, 2020

Implement BIP 340-342 validation (Schnorr/taproot/tapscript) #17977

Closed

4 tasks

fanquake previously requested changes Jun 10, 2020

View reviewed changes

sipa force-pushed the 202006_secp256k1_update branch from e0540d5 to c5b4601 Compare June 10, 2020 06:08

elichai reviewed Jun 10, 2020

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

maflcko mentioned this pull request Jun 10, 2020

Use of uninitialized memory when validating successfully deserialized CPubKey #19235

Closed

sipa added 2 commits June 10, 2020 18:15

Drop --disable-jni from libsecp256k1 configure options

ca8bc42

scripted-diff: rename privkey with seckey in secp256k1 interface

e10439c

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

sipa force-pushed the 202006_secp256k1_update branch from c5b4601 to e10439c Compare June 11, 2020 01:31

fanquake self-requested a review June 11, 2020 01:40

real-or-random mentioned this pull request Jun 12, 2020

contrib stuff: Aftermath of Bitcoin Core subtree update bitcoin-core/secp256k1#758

Open

fanquake approved these changes Jun 13, 2020

View reviewed changes

fanquake merged commit 8c97780 into bitcoin:master Jun 13, 2020

fanquake mentioned this pull request Jun 13, 2020

Follow up from recent libsecp256k1 subtree update #19263

Closed

fanquake mentioned this pull request Jun 29, 2020

remove dead store in ecdsa_signature_parse_der_lax bitcoin-core/secp256k1#765

Merged

This was referenced Jul 20, 2020

build: cannot compile on OpenBSD since libsecp256k1 subtree update #19559

Closed

Issue compiling on OpenBSD 6.7 bitcoin-core/secp256k1#768

Closed

real-or-random mentioned this pull request Jul 29, 2020

Remove lax DER parsing functions from contrib bitcoin-core/secp256k1#781

Draft

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update libsecp256k1 subtree #19228

Update libsecp256k1 subtree #19228

Uh oh!

sipa commented Jun 9, 2020

Uh oh!

jonasnick left a comment

Uh oh!

maflcko commented Jun 9, 2020

Uh oh!

fanquake left a comment

Uh oh!

sipa commented Jun 10, 2020

Uh oh!

real-or-random commented Jun 10, 2020 •

edited

Loading

Uh oh!

Uh oh!

elichai commented Jun 10, 2020

Uh oh!

laanwj commented Jun 10, 2020

Uh oh!

DrahtBot commented Jun 10, 2020 •

edited

Loading

Uh oh!

sipa commented Jun 11, 2020

Uh oh!

Sjors commented Jun 12, 2020

Uh oh!

real-or-random commented Jun 12, 2020

Uh oh!

jonasnick commented Jun 12, 2020

Uh oh!

fanquake left a comment

Uh oh!

laanwj commented Jun 16, 2020

Uh oh!

Uh oh!

Update libsecp256k1 subtree #19228

Update libsecp256k1 subtree #19228

Uh oh!

Conversation

sipa commented Jun 9, 2020

Uh oh!

jonasnick left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko commented Jun 9, 2020

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

sipa commented Jun 10, 2020

Uh oh!

real-or-random commented Jun 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

elichai commented Jun 10, 2020

Uh oh!

laanwj commented Jun 10, 2020

Uh oh!

DrahtBot commented Jun 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

sipa commented Jun 11, 2020

Uh oh!

Sjors commented Jun 12, 2020

Uh oh!

real-or-random commented Jun 12, 2020

Uh oh!

jonasnick commented Jun 12, 2020

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

laanwj commented Jun 16, 2020

Uh oh!

Uh oh!

real-or-random commented Jun 10, 2020 •

edited

Loading

DrahtBot commented Jun 10, 2020 •

edited

Loading