Commit Graph

1591 Commits

Author SHA1 Message Date
Pieter Wuille bb36331412 Simplify precompute_ecmult_print_* 2021-12-18 16:12:34 -05:00
Pieter Wuille 38cd84a0cb Compute ecmult tables at runtime for tests_exhaustive 2021-12-18 16:12:33 -05:00
Pieter Wuille e458ec26d6 Move ecmult table computation code to separate file 2021-12-18 16:11:56 -05:00
Pieter Wuille fc1bf9f15f Split ecmult table computation and printing 2021-12-18 16:11:56 -05:00
Pieter Wuille 31feab053b Rename function secp256k1_ecmult_gen_{create_prec -> compute}_table 2021-12-18 16:11:52 -05:00
Pieter Wuille 725370c3f2 Rename ecmult_gen_prec -> ecmult_gen_compute_table 2021-12-17 14:43:45 -05:00
Pieter Wuille 075252c1b7 Rename ecmult_static_pre_g -> precomputed_ecmult 2021-12-17 11:29:17 -05:00
Pieter Wuille 7cf47f72bc Rename ecmult_gen_static_prec_table -> precomputed_ecmult_gen 2021-12-17 11:24:18 -05:00
Pieter Wuille f95b8106d0 Rename gen_ecmult_static_pre_g -> precompute_ecmult 2021-12-17 11:19:45 -05:00
Pieter Wuille bae77685eb Rename gen_ecmult_gen_static_prec_table -> precompute_ecmult_gen 2021-12-17 11:15:37 -05:00
Tim Ruffing 0559fc6e41
Merge bitcoin-core/secp256k1#988: Make signing table fully static
7dfceceea6 build: Remove #undef hack for ASM in the precomputation programs (Tim Ruffing)
bb36fe9be0 ci: Test `make precomp` (Tim Ruffing)
d94a37a20c build: Remove CC_FOR_BUILD stuff (Tim Ruffing)
ad63bb4c29 build: Prebuild and distribute ecmult_gen table (Tim Ruffing)
ac49361ed0 prealloc: Get rid of manual memory management for prealloc contexts (Tim Ruffing)
6573c08f65 ecmult_gen: Tidy precomputed file and save space (Tim Ruffing)
5eba83f17c ecmult_gen: Precompute tables for all values of ECMULT_GEN_PREC_BITS (Tim Ruffing)
fdb33dd122 refactor: Make PREC_BITS a parameter of ecmult_gen_build_prec_table (Tim Ruffing)
a4875e30a6 refactor: Move default callbacks to util.h (Tim Ruffing)
4c94c55bce doc: Remove obsolete hint for valgrind stack size (Tim Ruffing)
5106226991 exhaustive_tests: Fix with ecmult_gen table with custom generator (Tim Ruffing)
e1a76530db refactor: Make generator a parameter of ecmult_gen_create_prec_table (Tim Ruffing)
9ad09f6911 refactor: Rename program that generates static ecmult_gen table (Tim Ruffing)
8ae18f1ab3 refactor: Rename file that contains static ecmult_gen table (Tim Ruffing)
00d2fa116e ecmult_gen: Make code consistent with comment (Tim Ruffing)
3b0c2185ea ecmult_gen: Simplify ecmult_gen context after making table static (Tim Ruffing)
e43ba02cfc refactor: Decouple table generation and ecmult_gen context (Tim Ruffing)
22dc2c0a0d ecmult_gen: Move table creation to new file and force static prec (Tim Ruffing)

Pull request description:

  This resolves #893,  resolves #692 (and also resolves bitcoin/bitcoin#22854).

  - [x] Extract table generation to separate function in separate file (to be used by generation script and exhaustive tests)
  - [x] Tidy up
    - [x] Remove code that deals with non-static tables
    - [x] Make functions that need ecmult_gen not depend on signing context
    - [x] Rename stuff to make it fit the new structure and consistent with how we hande verification tables (#956)
  - [x] Fix exhaustive tests
    - [x] Make table generation function take generator as input
    - [x] Overwrite the static tables with a table with custom generator in exhaustive tests
  - [x] Overhaul script that generates table files
    - [x] Make table generation function take PREC_BITS as input (I have some code already, just not yet in this branch)
    - [x] Change generation script to generate three tables (for all three values of ECMULT_GEN_PREC_BITS)
  - [x] Ship pre-built tables
    - [x] Add pregenerated table file to repo
    - [x] Remove generation of table file from build process (like in #956)
    - [x] Remove left-over stuff (e.g., detecting a compiler running on the build machine) from build system
  - [x] Final cleanups (copyright headers, commit, messages, etc.)
  - [ ] (separate PR:) Make sure link-time optimization remove corresponding static tables (and code) when no signing/verifcation function is called
  - [ ] (separate PR:) Compile precomputation as a separate object file and link it (https://github.com/bitcoin-core/secp256k1/pull/988#issuecomment-977813538)
  - [ ] (separate PR:) Document the backwards-compatible API changes made in this PR and in #956.
    - [ ] Maybe deprecate the static context

ACKs for top commit:
  sipa:
    ACK 7dfceceea6
  robot-dreams:
    ACK 7dfceceea6 (based on range-diff between 56284c7d44c0ed46e636588bfbf6c403b7dfa6c1 and 7dfceceea6)

Tree-SHA512: 6efb3f36f05efe3b79bbd877881fe1409f71fd6488d24c811b2e77d9f053bed78670dd1dcbb42ad780458a51c4ffa36de9cd6567271b22041dc7a122ceb677c5
2021-12-15 11:06:47 +01:00
Tim Ruffing 7dfceceea6 build: Remove #undef hack for ASM in the precomputation programs
This was necessary because we used to cross-compile the library but
compile the precomputation programs for the build host. Now it's no
longer necessary and we can cleanly link even the external ASM
(which was the intent of #935).

On the way, remove an obsolete "-I" parameter.
2021-12-09 20:52:56 +01:00
Tim Ruffing bb36fe9be0 ci: Test `make precomp` 2021-12-09 20:52:28 +01:00
Tim Ruffing d94a37a20c build: Remove CC_FOR_BUILD stuff 2021-12-09 20:52:28 +01:00
Tim Ruffing ad63bb4c29 build: Prebuild and distribute ecmult_gen table
- Improve Makefile.am for both prebuilt tables files
 - On the way, tidy EXTRA_DIST: Move the header files to noinst_HEADERS,
   where they conceptually belong, and add missing SECURITY.md to EXTRA_DIST
2021-12-09 20:52:28 +01:00
Tim Ruffing ac49361ed0 prealloc: Get rid of manual memory management for prealloc contexts 2021-12-09 20:52:28 +01:00
Tim Ruffing 6573c08f65 ecmult_gen: Tidy precomputed file and save space 2021-12-09 20:52:26 +01:00
Tim Ruffing 5eba83f17c ecmult_gen: Precompute tables for all values of ECMULT_GEN_PREC_BITS 2021-12-09 20:51:59 +01:00
Tim Ruffing 5d0dbef018
Merge bitcoin-core/secp256k1#942: Verify that secp256k1_ge_set_gej_zinv does not operate on infinity.
099bad945e Comment and check a parameter for inf in secp256k1_ecmult_const. (Russell O'Connor)
6c0be857f8 Verify that secp256k1_ge_set_gej_zinv does not operate on infinity. a->x and a->y should not be used if the infinity flag is set. (Russell O'Connor)

Pull request description:

  a->x and a->y should not be used if the infinity flag is set.

ACKs for top commit:
  robot-dreams:
    ACK 099bad945e
  real-or-random:
    ACK 099bad945e I inspected all call sites, they all ensure that a is not infinity

Tree-SHA512: 495fcfe4ec4cacb3fc64bd5d04ecc67ab34f6b63666c6169d473abfd63c2041bc501a9a60d817566517435b986406ea2b7db3f5806043cecf30e214eba9892e9
2021-12-07 11:26:10 +01:00
Tim Ruffing 486205aa68
Merge bitcoin-core/secp256k1#920: Test all ecmult functions with many j*2^i combinations
5eb519e1f6 ci: reduce TEST_ITERS in memcheck run (Pieter Wuille)
e2cf77328a Test ecmult functions for all i*2^j for j=0..255 and odd i=1..255. (Pieter Wuille)

Pull request description:

  Instead of just testing properties of the points xG for x=-36..36:
  * also compute all xG where x=j*2^i for i=0..255 and odd j=1..255.
  * test them against known exact results (SHA256 all of them, and compared against an independently created result)
  * test all 4 ecmult functions (and for secp256k1_ecmult and secp256k1_ecmult_multi_var, both as G, and through the generic point input)

ACKs for top commit:
  real-or-random:
    ACK 5eb519e1f6
  jonasnick:
    ACK 5eb519e1f6

Tree-SHA512: 5d3fcbff754e859ba27d4f4581fa91fafb450fa3f7880364667dba51287e7f02f489af19b9de6a6e0f52faa183c0c7ae46db6add05180c3d4f45a6557b00c0ed
2021-12-06 17:57:14 +01:00
Tim Ruffing fdb33dd122 refactor: Make PREC_BITS a parameter of ecmult_gen_build_prec_table 2021-12-05 17:58:26 +01:00
Pieter Wuille 5eb519e1f6 ci: reduce TEST_ITERS in memcheck run 2021-12-05 11:54:05 -05:00
Pieter Wuille e2cf77328a Test ecmult functions for all i*2^j for j=0..255 and odd i=1..255. 2021-12-05 11:53:47 -05:00
Tim Ruffing 61ae37c612
Merge bitcoin-core/secp256k1#1022: build: Windows DLL additions
c0cd7de6d4 build: add -no-undefined to libtool LDFLAGS (fanquake)
fe32a79d35 build: pass win32-dll to LT_INIT (fanquake)

Pull request description:

  This takes care of two of the outstanding issues in #923. One being initializing libtool with `win32-dll` and the other being the addition of `-no-undefined` to the libtool LDFLAGS. See each commit for more details.

  Builders cross-compiling for Windows (including Core) will no-longer see:
  ```bash
  libtool: warning: undefined symbols not allowed in x86_64-w64-mingw32 shared libraries; building static only
  ```

  I'm planning on making some related changes downstream.

ACKs for top commit:
  sipa:
    utACK c0cd7de6d4. We indeed have done the work to propertly mark exported symbols, and AFAIK have no imported symbols apart from standard library ones.
  real-or-random:
    ACK c0cd7de6d4
  hebasto:
    ACK c0cd7de6d4

Tree-SHA512: 6756bc88ac439a27117a1341d82a801cef70354a9e7a563592ab3ac7298fbefdaa0a2c410ea3fba8953d53f254c449dc491069f30468db12791027a65dd02f80
2021-12-05 12:19:35 +01:00
Tim Ruffing 4f01840b82
Merge bitcoin-core/secp256k1#1027: build: Add a check that Valgrind actually supports a host platform
7c7ce872a5 build: Add a check that Valgrind actually supports a host platform (Hennadii Stepanov)

Pull request description:

  This PR adds a check that Valgrind actually supports a host platform.

  On master (49f608de47):
  ```
  $ ./autogen.sh &> /dev/null && ./configure -q --host=riscv64-linux-gnu 2>&1 | grep valgrind
    valgrind                = yes
  ```

  With this PR:
  ```
  $ ./autogen.sh &> /dev/null && ./configure -q --host=riscv64-linux-gnu 2>&1 | grep valgrind
    valgrind                = no
  ```

  Closes #1023.

ACKs for top commit:
  sipa:
    utACK 7c7ce872a5
  real-or-random:
    utACK 7c7ce872a5

Tree-SHA512: 27f660f7b992ab35dba64b525af1c631f33b8cb25b6a990c81ec4d358c609a2dc03b0932847db9d5aa35eaa880929c7ad2bb4e7719785c2402b1b291cfa91ede
2021-12-05 11:48:02 +01:00
Tim Ruffing 6ad908aa00
Merge bitcoin-core/secp256k1#1008: bench.c: add `--help` option and ci: move env variables
592661c22f ci: move test environment variable declaration to .cirrus.yml (siv2r)
dcbe84b841 bench: add --help option to bench. (siv2r)

Pull request description:

  Fixes #1005

  **Changes:**
  - added `--help` option to `bench.c`
      - `help()` function prints the help to command-line
      - `have_invalid_args()` checks if the user has entered an invalid argument
  - moved `secp256k1_bench_iters` and `secp256k1_test_iters` environment variables declaration to `.cirrus.yml`

ACKs for top commit:
  sipa:
    utACK 592661c22f
  real-or-random:
    ACK 592661c22f

Tree-SHA512: ebc6a2e6e47b529212efa1c9b75cc79649fca7f42aa75ce46502db24ac94f46b6cef59c828d13265d1fa69187a81c140d1951e7daeb7c8e008a6c1ad75259741
2021-12-05 11:24:00 +01:00
siv2r 592661c22f ci: move test environment variable declaration to .cirrus.yml
environment var moved:
    1. SECP256K1_TEST_ITERS (replaces TEST_ITERS)
    2. SECP256K1_BENCH_ITERS (replaces BENCH_ITERS)
2021-12-04 22:47:40 +05:30
siv2r dcbe84b841 bench: add --help option to bench.
The following functions were created:
    1. bench.c: help()
        - prints the help to the command-line
    2. bench.h: have_invalid_args()
        - takes a list of arguments that the user is allowed to enter on the command-line
        - returns 1 if the user entered an invalid argument
        - returns 0 if all the user entered arguments are valid
2021-12-04 22:47:30 +05:30
Russell O'Connor 099bad945e Comment and check a parameter for inf in secp256k1_ecmult_const. 2021-12-03 13:57:38 -05:00
Russell O'Connor 6c0be857f8 Verify that secp256k1_ge_set_gej_zinv does not operate on infinity.
a->x and a->y should not be used if the infinity flag is set.
2021-12-03 12:01:41 -05:00
Tim Ruffing 4900227451
Merge bitcoin-core/secp256k1#1025: build: replace backtick command substitution with $()
2b7c7497ef build: replace backtick command substitution with $() (fanquake)

Pull request description:

  This is only needed for the very oldest of non-POSIX-compatible shells.
  Note that this code will also only be executed on macOS, where it'd be
  very unlikely to run into such a shell anyways.

  Followup to https://github.com/bitcoin-core/secp256k1/pull/1019#pullrequestreview-815300521. I had thought there were more usages of this
  syntax, but seems like it's just the one.

  See:
  https://github.com/koalaman/shellcheck/wiki/SC2006

  Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>

ACKs for top commit:
  real-or-random:
    ACK 2b7c7497ef
  hebasto:
    ACK 2b7c7497ef, verified that this is the only case.

Tree-SHA512: 6192f5efe437ff428ce7843ac595049a1aa7969a9e696f649cfd4820b28fc96ad0fabd6eec0ec1ca404763f02e64af6a99e57666a00d8749c6212a0646211991
2021-12-03 17:27:09 +01:00
Hennadii Stepanov 7c7ce872a5
build: Add a check that Valgrind actually supports a host platform 2021-12-03 17:32:26 +02:00
Tim Ruffing a4875e30a6 refactor: Move default callbacks to util.h 2021-12-03 11:23:33 +01:00
Tim Ruffing 4c94c55bce doc: Remove obsolete hint for valgrind stack size
Also don't mention exhaustive_tests without explanation. They're
included in our test suite (`make check`) anyway.
2021-12-03 11:23:33 +01:00
Tim Ruffing 5106226991 exhaustive_tests: Fix with ecmult_gen table with custom generator 2021-12-03 11:23:33 +01:00
Tim Ruffing e1a76530db refactor: Make generator a parameter of ecmult_gen_create_prec_table 2021-12-03 11:23:33 +01:00
Tim Ruffing 9ad09f6911 refactor: Rename program that generates static ecmult_gen table 2021-12-03 11:23:33 +01:00
Tim Ruffing 8ae18f1ab3 refactor: Rename file that contains static ecmult_gen table 2021-12-03 11:23:33 +01:00
Tim Ruffing 00d2fa116e ecmult_gen: Make code consistent with comment
This also fixes a typo in the comment.
2021-12-03 11:23:33 +01:00
Tim Ruffing 3b0c2185ea ecmult_gen: Simplify ecmult_gen context after making table static
This is a backwards-compatible API change: Before this commit, a context
initialized for signing was required to call functions that rely on
ecmult_gen. After this commit, this is no longer necessary because the
static ecmult_gen table is always present. In practice this means that
the corresponding functions will just work instead of calling the
illegal callback when given a context which is not (officially)
initialized for signing.

This is in line with 6815761, which made the analogous change with
respect to ecmult and contexts initialized for signing. But as opposed
to 681571, which removed the ecmult context entirely, we cannot remove
the ecmult_gen context entirely because it is still used for random
blinding. Moreover, since the secp256k1_context_no_precomp context is
const and cannot meaningfully support random blinding, we refrain (for
now) from changing its API, i.e., the illegal callback will still be
called when trying to use ecmult_gen operations with the static
secp256k1_context_no_precomp context.
2021-12-03 11:23:33 +01:00
fanquake 2b7c7497ef
build: replace backtick command substitution with $()
This is only needed for the very oldest of non-POSIX-compatible shells.
Note that this code will also only be executed on macOS, where it'd be
very unlikely to run into such a shell.

Followup to #1019.

See:
https://github.com/koalaman/shellcheck/wiki/SC2006

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2021-12-03 15:22:59 +08:00
Tim Ruffing 49f608de47
Merge bitcoin-core/secp256k1#1004: ecmult: fix definition of STRAUSS_SCRATCH_OBJECTS
60bf8890df ecmult: fix definition of STRAUSS_SCRATCH_OBJECTS (Jonas Nick)

Pull request description:

  This bug was introduced in 7506e064d7 by adding
  an allocation but not updating the constant.

ACKs for top commit:
  robot-dreams:
    ACK 60bf8890df
  real-or-random:
    ACK 60bf8890df

Tree-SHA512: d7782fe9bf09fea8cf22304ab13679223a48f4d8b09081e662ea162a68c4e35f6b5820fbe4c6030fabad02a48dfdd02eb9eef22262c1dbbf02955bb92b75aef8
2021-12-02 21:26:49 +01:00
fanquake c0cd7de6d4
build: add -no-undefined to libtool LDFLAGS
Instruct libtool to not allow undefined symbols when linking a shared
library.

See:
https://autotools.io/libtool/windows.html
https://www.gnu.org/software/libtool/manual/libtool.html#LT_005fINIT
https://www.gnu.org/software/gnulib/manual/html_node/Libtool-and-Windows.html
2021-12-02 11:48:43 +08:00
fanquake fe32a79d35
build: pass win32-dll to LT_INIT
This is the recommended way to support building PE DLLs with modern
mingw toolchains and libtool.

> This option should be used if the package has been ported to build clean
> dlls on win32 platforms.
> If this macro is not used, libtool will assume that the package libraries
> are not dll clean and will build only static libraries on win32 hosts.

See:
https://www.gnu.org/software/libtool/manual/libtool.html#LT_005fINIT
https://www.gnu.org/software/gnulib/manual/html_node/Libtool-and-Windows.html
https://autotools.io/libtool/windows.html
2021-12-02 11:44:13 +08:00
Jonas Nick 60bf8890df ecmult: fix definition of STRAUSS_SCRATCH_OBJECTS
This bug was introduced in 7506e064d7 by adding
an allocation but not updating the constant.
2021-11-30 19:25:40 +00:00
Tim Ruffing fecf436d53
Merge bitcoin-core/secp256k1#1019: build: don't append valgrind CPPFLAGS if not installed (macOS)
214042a170 build: don't append valgrind CPPFLAGS if not installed (fanquake)

Pull request description:

  Valgrinds CPPFLAGS, i.e `-I/usr/local/opt/valgrind/include`, are currently added to CPPFLAGS, regardless of whether valgrind is installed. This changes configure so that they are only added if valgrind is available. i.e the output of `brew list --versions valgrind` is non-null.

ACKs for top commit:
  real-or-random:
    ACK 214042a170
  hebasto:
    ACK 214042a170, tested on macOS Big Sur 11.6.1 (20G224, Intel).

Tree-SHA512: 5101636a0a12f1941b01967ca8eab7aa20f44db0d1ef4571a5ad6026bb89494b983465d34d93c8b17a260b695116792991da53d135bc19a3c9e974f5266a90af
2021-11-24 21:16:00 +01:00
Tim Ruffing 2e5e4b67df
Merge bitcoin-core/secp256k1#1020: doc: remove use of <0xa0> "no break space"
812ff5c747 doc: remove use of 0xa0 "no break space" (fanquake)

Pull request description:

  This is miscellaneous, but I don't think these were being used on purpose?

ACKs for top commit:
  siv2r:
    ACK 812ff5c. The non-breaking space character is replaced with whitespace. Tested with [NBSP highlighter extension](https://marketplace.visualstudio.com/items?itemName=viktorzetterstrom.non-breaking-space-highlighter) on vscode.
  real-or-random:
    ACK 812ff5c747

Tree-SHA512: ccfcc64798f5a5eb0c669eb00f4408ab713e6710d67fd15ee2a4dca0d052e27636d7f0ad312aa94be0cd068c7e7874441aa2e114c4118322d0c764398a4ff695
2021-11-24 14:42:02 +01:00
fanquake 812ff5c747
doc: remove use of 0xa0 "no break space" 2021-11-24 08:11:49 +08:00
fanquake 214042a170
build: don't append valgrind CPPFLAGS if not installed 2021-11-23 11:24:12 +08:00
Tim Ruffing e43ba02cfc refactor: Decouple table generation and ecmult_gen context 2021-11-19 14:03:44 +01:00