Make signing table fully static

real-or-random commented at 1:37 pm on October 8, 2021: contributor

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

Extract table generation to separate function in separate file (to be used by generation script and exhaustive tests)
Tidy up
- Remove code that deals with non-static tables
- Make functions that need ecmult_gen not depend on signing context
- Rename stuff to make it fit the new structure and consistent with how we hande verification tables (#956)
Fix exhaustive tests
- Make table generation function take generator as input
- Overwrite the static tables with a table with custom generator in exhaustive tests
Overhaul script that generates table files
- Make table generation function take PREC_BITS as input (I have some code already, just not yet in this branch)
- Change generation script to generate three tables (for all three values of ECMULT_GEN_PREC_BITS)
Ship pre-built tables
- Add pregenerated table file to repo
- Remove generation of table file from build process (like in #956)
- Remove left-over stuff (e.g., detecting a compiler running on the build machine) from build system
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

real-or-random commented at 1:37 pm on October 8, 2021: contributor

Here’s an unresolved issue I’d like to discuss before I proceed. It’s not clear to me what we should do with secp256k1_context_no_precomp: https://github.com/bitcoin-core/secp256k1/blob/da0092bccc0ebd67e943e7313143800b1da265ec/include/secp256k1.h#L200-L205

The problem here is that so far, you couldn’t call functions requiring ecmult or ecmult_gen with secp256k1_context_no_precomp. #956 has changed this for ecmult (verification tables) and this PR (in its current state) would change this also for ecmult_gen (signing tables). But while we don’t need any context anymore for ecmult, we’d still retain an ecmult_gen_context for holding blinding values.

With the current PR, using secp256k1_context_no_precomp with an ecmult_gen function would simply give you wrong results because the ecmult_gen_context within there is not correctly initialized, i.e., the blinding values are not consistent.

Here are possible approaches:

Change secp256k1_context_no_precomp to include “default” blinding values. This makes it possible to use this context for ecmult_gen functions but effectively turns off blinding, so this has the same drawbacks as as creating a normal context and not randomizing it. Note that secp256k1_context_no_precomp is a constant, so the user can’t ever randomize it.
- We could then optionally add a warning to secp256k1_context_no_precomp.
- Or we could even deprecate it. Arguably, the main advantage of it is gone when there’s anyway no “precomp” anymore. But a minor advantage of it is that it’s static which may be a useful thing when you really only want to run a parsing function.
Effectively retain the distinction between signing contexts and non-signing contexts. In that world, secp256k1_context_no_precomp is currently the only way to obtain a non-signing context.

After this PR, we should anyway reconsider the context API (#780), so this is only an intermediate step. Having said this, I think both options are acceptable (slightly leaning towards the second) but I’d like to do the bikeshedding before I continue to work on this.

theuni commented at 5:56 pm on October 12, 2021: contributor

Build-side concept ACK. From what I gather this should simplify the common (Core) build significantly.

real-or-random commented at 10:16 am on October 15, 2021: contributor

* Effectively retain the distinction between signing contexts and non-signing contexts. In that world, `secp256k1_context_no_precomp` is currently the only way to obtain a non-signing context.

After discussing with some people elsewhere, I’ll take that route for the simple reason that it’s more conservative by avoiding an API change. We can still change the API later but that’s a different discussion then.

fanquake commented at 1:30 am on October 20, 2021: member

Concept ACK from a Core POV. Thanks for following up with this.

fanquake cross-referenced this on Oct 20, 2021 from issue build: explicitly disable libsecp256k1 openssl based tests by fanquake

Rogelio165 approved

MarcoFalke cross-referenced this on Oct 29, 2021 from issue Update libsecp256k1 subtree to current master by sipa

elichai cross-referenced this on Nov 3, 2021 from issue New context API by real-or-random

real-or-random force-pushed on Nov 8, 2021

real-or-random force-pushed on Nov 9, 2021

real-or-random force-pushed on Nov 10, 2021

real-or-random force-pushed on Nov 13, 2021

real-or-random force-pushed on Nov 19, 2021

ecmult_gen: Move table creation to new file and force static prec 22dc2c0a0d

refactor: Decouple table generation and ecmult_gen context e43ba02cfc

real-or-random cross-referenced this on Nov 19, 2021 from issue Test all ecmult functions with many j*2^i combinations by sipa

real-or-random cross-referenced this on Nov 19, 2021 from issue Replace gen_context.c with a Python implementation by sipa

real-or-random force-pushed on Nov 19, 2021

real-or-random marked this as ready for review on Nov 19, 2021

real-or-random commented at 2:06 pm on November 19, 2021: contributor

Ready for review. Note that this changes the stdout of make due to the use of BUILT_SOURCES in Automake (https://www.gnu.org/software/automake/manual/html_node/Sources.html) but that’s purely cosmetic.

in src/gen_ecmult_gen_static_prec_table.c:53 in f8c6f7ecc5 outdated

61-        fprintf(fp,"{\n");
62-        for(inner = 0; inner != g; inner++) {
63-            fprintf(fp,"    SC(%uu, %uu, %uu, %uu, %uu, %uu, %uu, %uu, %uu, %uu, %uu, %uu, %uu, %uu, %uu, %uu)", SECP256K1_GE_STORAGE_CONST_GET(table[outer * g + inner]));
64-            if (inner != g - 1) {
65-                fprintf(fp,",\n");
66+    for (bits = 2; bits <= 8; bits += 2) {

robot-dreams commented at 6:28 pm on November 22, 2021:

What’s the motivation for bits += 2 instead of bits *= 2 here, given that ecmult_gen.h has the following?

0#if ECMULT_GEN_PREC_BITS != 2 && ECMULT_GEN_PREC_BITS != 4 && ECMULT_GEN_PREC_BITS != 8
1#  error "Set ECMULT_GEN_PREC_BITS to 2, 4 or 8."
2#endif

Does either this loop or the check in ecmult_gen.h need to be updated?

sipa commented at 10:24 pm on November 22, 2021:

Just noticed this too; it adds a table for ECMULT_GEN_PREC_BITS=6, which isn’t allowed.

real-or-random commented at 9:43 am on November 24, 2021:

Lol, will fix this. That’s great, so the file is even shorter!

real-or-random commented at 7:09 pm on November 24, 2021:

Went down from 1.8 MB to 1.4 MB

in src/ecmult_gen.h:21 in 9b5fd6c8fa outdated

29-     * precomputed (call it prec(i, n_i)). The formula now becomes sum(prec(i, n_i), i=0 ... PREC_N-1).
30-     * None of the resulting prec group elements have a known scalar, and neither do any of
31-     * the intermediate sums while computing a*G.
32-     */
33-    secp256k1_ge_storage (*prec)[ECMULT_GEN_PREC_N][ECMULT_GEN_PREC_G]; /* prec[j][i] = (PREC_G)^j * i * G + U_i */
34+    int built;

robot-dreams commented at 6:34 pm on November 22, 2021:

Nit: Is it worth having a comment with the definition of built?

real-or-random commented at 9:45 am on November 24, 2021:

Well, if you ask for it, then probably yes!

robot-dreams commented at 10:50 pm on November 26, 2021:

Oops, my mistake to ask for such a comment! It’s probably better without after all.

in src/ecmult_gen_impl.h:26 in 9b5fd6c8fa outdated

38-static void secp256k1_ecmult_gen_context_finalize_memcpy(secp256k1_ecmult_gen_context *dst, const secp256k1_ecmult_gen_context *src) {
39-    (void)dst, (void)src;
40+    return ctx->built;
41 }
42 
43 static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context *ctx) {

robot-dreams commented at 6:36 pm on November 22, 2021:

Does ctx->built need to be reset here?

real-or-random commented at 9:49 am on November 24, 2021:

Yes, fixing.

robot-dreams commented at 6:43 pm on November 22, 2021: contributor

Concept ACK 65426be684e21e972b87a481f9f257e39930c2c1

Thanks for making this really readable with atomic commits! I still need to review this more carefully but here are some quick initial comments:

sipa commented at 11:35 pm on November 22, 2021: contributor

A suggestion for a follow-up PR: move all precomputed tables to a separate .c/.o file, so it can be compiled just once and included in all binaries?

real-or-random commented at 12:04 pm on November 24, 2021: contributor

A suggestion for a follow-up PR: move all precomputed tables to a separate .c/.o file, so it can be compiled just once and included in all binaries?

Yep, I think separate .c/o. files for the ecmult vs ecmult_gen precomputation are still useful but linking is probably the way to go. But yes, this should go to a follow-up PR. I’m adding it to the list in the opening comment. This PR just makes the ecmult_gen tables use the same build method as the ecmult tables already use.

edit: I didn’t think this through entirely but this could also further simplify the build system. Generating .c files does not need BUILT_SOURCES, see Section “Build bindir.c, not bindir.h.” in https://www.gnu.org/software/automake/manual/html_node/Built-Sources-Example.html.

real-or-random force-pushed on Nov 24, 2021

real-or-random commented at 7:10 pm on November 24, 2021: contributor

Force-pushed to address the initial comments by @robot-dreams.

in src/ecmult_gen.h:25 in 47f862aae6 outdated

35-    secp256k1_gej initial;
36+    /* Whether the context has been built. */
37+    int built;
38+
39+    /* Blinding values used when computing (n-b)G + bG. */
40+    secp256k1_scalar blind; /* b  */

robot-dreams commented at 10:45 pm on November 26, 2021:

Nit: Is ctx->blind actually set to -b here?

secp256k1_ecmult_gen_blind in ecmult_gen_impl.h contains this:

0    secp256k1_ecmult_gen(ctx, &gb, &b);
1    secp256k1_scalar_negate(&b, &b);
2    ctx->blind = b;
3    ctx->initial = gb;

sipa commented at 10:12 pm on December 1, 2021:

Agree with @robot-dreams here.

in src/ecmult_gen_prec.h:12 in f0318588ea outdated

 7+#ifndef SECP256K1_ECMULT_GEN_PREC_H
 8+#define SECP256K1_ECMULT_GEN_PREC_H
 9+
10+#include "ecmult_gen.h"
11+
12+static const size_t ECMULT_GEN_PREC_TABLE_SIZE = ECMULT_GEN_PREC_N(ECMULT_GEN_PREC_BITS) * ECMULT_GEN_PREC_G(ECMULT_GEN_PREC_BITS) * sizeof(secp256k1_ge_storage);

robot-dreams commented at 11:18 pm on November 26, 2021:

Nit: I couldn’t find any references to this; should it be either (1) removed or (2) be a #define with a bits parameter so that it can be used in gen_ecmult_gen_static_prec_table.c?

in .gitignore:6 in d37d22bddf outdated

2@@ -3,7 +3,7 @@ bench_ecmult
3 bench_internal
4 tests
5 exhaustive_tests
6-gen_context
7+gen_ecmult_gen_static_prec_table

robot-dreams commented at 11:30 pm on November 26, 2021:

(No action needed) It’s too bad that gen stands for both “generate” (the static table) and “generator” (of a group) 😅 , but it’s probably not worth changing, especially not as part of this PR.

real-or-random commented at 10:34 am on December 2, 2021:

Yeah, this confused me a lot when starting to work on the library a few years back. And gen_context should have been called gen_gen_context at least, even though that’s ugly.

I tried to avoid this discussion in this PR (only renaming thins I’m touching anyway) and it may be worth unifying the names in the follow-up PR that switches to linking instead of including (e.g., I intentionally used “create” in a function name instead of “generate”) Then we anyway need to touch both “generate” things again, so this would be a good opportunity.

robot-dreams commented at 11:44 pm on November 26, 2021: contributor

ACK f0318588ea5ec27dd2e6a074688e22f2bfae2b47 aside from nits.

I didn’t review changes to build, CI, or test files; however, tests pass locally for me, and make clean-precomp; make precomp keeps the repo unchanged.

I also verified the precomputed tables against an independent implementation.

in src/modules/extrakeys/tests_impl.h:362 in 47f862aae6 outdated

358@@ -359,25 +359,32 @@ void test_keypair(void) {
359     secp256k1_context *none = api_test_context(SECP256K1_CONTEXT_NONE, &ecount);
360     secp256k1_context *sign = api_test_context(SECP256K1_CONTEXT_SIGN, &ecount);
361     secp256k1_context *verify = api_test_context(SECP256K1_CONTEXT_VERIFY, &ecount);
362+    secp256k1_context *sttc = secp256k1_context_clone(secp256k1_context_no_precomp);

sipa commented at 10:17 pm on December 1, 2021:

What does sttc stand for? (not that variable names in this project always mean that much, but stilll…)

EDIT: Is it just “static” ?

real-or-random commented at 10:21 am on December 2, 2021:

Yeah, it’s just “static”. The tests in the other modules all use four letter names none, sign, vrfy (except the one here which has verify. – I could change this), so I tried to stick to this convention. I could change this to static_ctx oder no_precomp or similar if you prefer. Unfortunately static itself is a keyword. ;)

edit: Things should anyway become clearer when we change the API docs and maybe the API itself (see the initial comment here). _no_precomp will just be misleading identifier after this PR.

sipa commented at 11:10 pm on December 1, 2021: contributor

This is looking very good. I want to go over the test and allocation changes another time before I ACK, but I don’t expect any issues.

in src/gen_ecmult_gen_static_prec_table.c:20 in f0318588ea outdated

15+
16+/* In principle we could use ASM, but this yields only a minor speedup in
17+   build time and it's very complicated. In particular when cross-compiling, we'd
18+   need to build the ASM for the build and the host machine. */
19+#undef USE_EXTERNAL_ASM
20+#undef USE_ASM_X86_64

real-or-random commented at 10:37 am on December 2, 2021:

This comment should be updated.

in src/gen_ecmult_gen_static_prec_table.c:58 in f0318588ea outdated

53+
54+    fprintf(fp, "static\n");
55+    fprintf(fp, "#ifndef EXHAUSTIVE_TEST_ORDER\n");
56+    fprintf(fp, "const\n");
57+    fprintf(fp, "#endif\n");
58+    fprintf(fp, "secp256k1_ge_storage secp256k1_ecmult_gen_prec_table[ECMULT_GEN_PREC_N(ECMULT_GEN_PREC_B)][ECMULT_GEN_PREC_G(ECMULT_GEN_PREC_B)] = {\n");

real-or-random commented at 11:09 am on December 2, 2021:

We shouldn’t just make this non-const for the exhaustive test but also omit the initializer entirely. No need to have the static table in the exhaustive test binary just to overwrite it then.

real-or-random force-pushed on Dec 2, 2021

real-or-random commented at 2:08 pm on December 2, 2021: contributor

I forced-push to address the comments and did some further cleanups. I conversed the state before the rebase (old PR branch + fixups here): https://github.com/real-or-random/secp256k1/tree/202108-prebuilt-gen-context-fixups1 for easier re-review.

in src/modules/schnorrsig/tests_impl.h:200 in a76e62d017 outdated

216     CHECK(secp256k1_schnorrsig_sign_custom(sign, sig, msg, sizeof(msg), &keypairs[0], NULL) == 1);
217-    CHECK(ecount == 6);
218+    CHECK(ecount == 4);
219     CHECK(secp256k1_schnorrsig_sign_custom(sign, sig, msg, sizeof(msg), &keypairs[0], &invalid_extraparams) == 0);
220-    CHECK(ecount == 7);
221+    CHECK(ecount == 5);

robot-dreams commented at 5:36 pm on December 2, 2021:

Did you intend to add a check with sttc context here as well?

real-or-random commented at 10:28 am on December 3, 2021:

Fixed.

in src/ecmult_gen.h:20 in f26d43dad6 outdated

33-     * the intermediate sums while computing a*G.
34-     */
35-    secp256k1_ge_storage (*prec)[ECMULT_GEN_PREC_N][ECMULT_GEN_PREC_G]; /* prec[j][i] = (PREC_G)^j * i * G + U_i */
36-    secp256k1_scalar blind;
37-    secp256k1_gej initial;
38+    /* Whether the context has been built. */

robot-dreams commented at 7:16 pm on December 2, 2021:

Looks like my previous comment got automatically hidden because it was in an already resolved conversation: https://github.com/bitcoin-core/secp256k1/pull/988/files#r757712377

In retrospect I shouldn’t have asked for such a comment 😅 apologies for going back and forth, can you please either remove the comment after all, or if you prefer, replace it with something like this:

0/* Whether the blinding values have been generated. */

real-or-random commented at 7:43 pm on December 2, 2021:

Hm, to be honest, I think the current comment is more precise. E.g., if you reset the blinding values, built will still be 1 but it’s debatable whether the blinding values have been generated. I do think the current comment is helpful but of course I also don’t insist on it and can remove it. :D

real-or-random commented at 10:28 am on December 3, 2021:

Ok, I simply kept the current comment.

robot-dreams commented at 7:31 pm on December 2, 2021: contributor

ACK f26d43dad65c8aafdba057dde9cfa1adb4b58012, my remaining inline comments are non-blocking.

I re-reviewed and also looked at build, CI, test to the extent I could (the only things I didn’t review this time are the CC_FOR_BUILD and USE_EXTERNAL_ASM commits).

I also checked that the following now works (no more warning about stack switching):

0./configure --with-valgrind=yes --with-ecmult-gen-precision=8
1make
2valgrind ./exhaustive_tests

In a future PR, does it make sense to simplify secp256k1_context_preallocated_size and secp256k1_context_preallocated_clone_size since the size will always be the same, and the only use case now is to check that the flags are valid?

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.

3b0c2185ea

ecmult_gen: Make code consistent with comment

This also fixes a typo in the comment.

00d2fa116e

refactor: Rename file that contains static ecmult_gen table 8ae18f1ab3

refactor: Rename program that generates static ecmult_gen table 9ad09f6911

refactor: Make generator a parameter of ecmult_gen_create_prec_table e1a76530db

exhaustive_tests: Fix with ecmult_gen table with custom generator 5106226991

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.

4c94c55bce

refactor: Move default callbacks to util.h a4875e30a6

real-or-random force-pushed on Dec 3, 2021

real-or-random commented at 10:29 am on December 3, 2021: contributor

Pushed another fixup to https://github.com/real-or-random/secp256k1/tree/202108-prebuilt-gen-context-fixups1 to address the missing line in the schnorrsig test and force-pushed the rebased version here.

fanquake commented at 10:39 am on December 3, 2021: member

I put together a Bitcoin Core branch to test these changes. It’s master (https://github.com/bitcoin/bitcoin/commit/927a9b07776881d99340259f214c577537dbc77f) with the previous version of this PR (rebased on secp256k1 master) pulled into the subtree, and https://github.com/bitcoin/bitcoin/pull/23383/commits/89277969d7dca605b5d8a9097d19344a27876afc & https://github.com/bitcoin/bitcoin/pull/23383/commits/89e24fa6a6708f0e6a08e633f44070f73c480775 cherry-picked from https://github.com/bitcoin/bitcoin/pull/23383.

Checked that we no-longer have any issues like https://github.com/bitcoin/bitcoin/issues/22854. Also tested on an arm64 (M1) box.

Full Guix build also works (on x86_64):

 0real	134m9.592s
 1user	640m24.791s
 2sys	43m20.544s
 3
 4bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
 5de610013c9fc9f2c0ad54247deea5a481aeaf294f85b29da994e1c214af2eb20  guix-build-13027d952519/output/aarch64-linux-gnu/SHA256SUMS.part
 605e26dc745d94168463ef2a82efbc8b180553919121e94fd753f7f6e11fc88d4  guix-build-13027d952519/output/aarch64-linux-gnu/bitcoin-13027d952519-aarch64-linux-gnu-debug.tar.gz
 7ea2abd132dab4eeb0d675d35548ca9aad3a1b83853fd0d4a24e9fd399cff3159  guix-build-13027d952519/output/aarch64-linux-gnu/bitcoin-13027d952519-aarch64-linux-gnu.tar.gz
 8dc1deace6cf7d392cfc8cf82fca35a9639cf9dcd30883ed0a50758ea3816ec03  guix-build-13027d952519/output/arm-linux-gnueabihf/SHA256SUMS.part
 90fe3ad1c97be391038707fb8e76a2c60cc7666d40efbaf10a2b2a5084deb9b8a  guix-build-13027d952519/output/arm-linux-gnueabihf/bitcoin-13027d952519-arm-linux-gnueabihf-debug.tar.gz
10a2015503d8aac931cb45047bd0096f0c5d04778889e5ac843458253a314787c2  guix-build-13027d952519/output/arm-linux-gnueabihf/bitcoin-13027d952519-arm-linux-gnueabihf.tar.gz
1126d7af59f95300cd0933e288c5f84e4ba053b77716b3173152e93de32c890103  guix-build-13027d952519/output/dist-archive/bitcoin-13027d952519.tar.gz
1208c6a552396d2b580d58169edec6992e21e51674a83927cf83e5dc417f469d3c  guix-build-13027d952519/output/powerpc64-linux-gnu/SHA256SUMS.part
137801a8c55d5aa424ac7dc412ed4532c311d05a4debc9ef8476c3537383240cab  guix-build-13027d952519/output/powerpc64-linux-gnu/bitcoin-13027d952519-powerpc64-linux-gnu-debug.tar.gz
14e63ad72998c898b90a70e8916d4f0c87bc504b880059ac119ae5723fa26ea718  guix-build-13027d952519/output/powerpc64-linux-gnu/bitcoin-13027d952519-powerpc64-linux-gnu.tar.gz
15b1e272e1bf6c8ea5155a943bfe6598c450d4ca3c7223324209ad58ed9366682c  guix-build-13027d952519/output/powerpc64le-linux-gnu/SHA256SUMS.part
16a52b47f5ef50d43552a2050d9434c0ee48d61c0b765df063370e8f4da8113366  guix-build-13027d952519/output/powerpc64le-linux-gnu/bitcoin-13027d952519-powerpc64le-linux-gnu-debug.tar.gz
17cd9d1beb51534ae31cd7d8c9c6479985b942369a5bab1ba4aad698331ba1930c  guix-build-13027d952519/output/powerpc64le-linux-gnu/bitcoin-13027d952519-powerpc64le-linux-gnu.tar.gz
1821f3a1dcaee26d1b035f195367ed1315d83f753cb93a11ee82576411de78ae4c  guix-build-13027d952519/output/riscv64-linux-gnu/SHA256SUMS.part
197eb4d17dd6611db7b5167e237d36862893c8656ce6611804164673def7392789  guix-build-13027d952519/output/riscv64-linux-gnu/bitcoin-13027d952519-riscv64-linux-gnu-debug.tar.gz
20c67d7721514da35e94d58a58a1530fbe4265abfee8c5ca4d9a8dcf17415c6756  guix-build-13027d952519/output/riscv64-linux-gnu/bitcoin-13027d952519-riscv64-linux-gnu.tar.gz
217a94d5a3e494f05077a49013a9341687a408f025ba4ca0c716ff4e881b8aef61  guix-build-13027d952519/output/x86_64-apple-darwin19/SHA256SUMS.part
2225813a82754344e25d35b85c1ed01b94976dc1ab147a9066225d1015c11d4626  guix-build-13027d952519/output/x86_64-apple-darwin19/bitcoin-13027d952519-osx-unsigned.dmg
23809ea888fcb883b8fc0e97b631d8b758d63cc040854dd3a5cf66b7b37e1287be  guix-build-13027d952519/output/x86_64-apple-darwin19/bitcoin-13027d952519-osx-unsigned.tar.gz
24f0ae3c109deaa0d0798e5601964b05b1558e9789f65a672a83a47e7ff54650be  guix-build-13027d952519/output/x86_64-apple-darwin19/bitcoin-13027d952519-osx64.tar.gz
253aa4c0f0eb3944167a3b278f4c2826dfb8f18d1a7b1b33a8fa4a316eb99dfbe7  guix-build-13027d952519/output/x86_64-linux-gnu/SHA256SUMS.part
267a2bbcc399caf2fa17890cc29f46ad9ce940ac1a8f93706fb61ec70238b7ae6b  guix-build-13027d952519/output/x86_64-linux-gnu/bitcoin-13027d952519-x86_64-linux-gnu-debug.tar.gz
277cae0e347222e62dc4e7dba494b26164d5133f471d9e3e060046e805bd1fd142  guix-build-13027d952519/output/x86_64-linux-gnu/bitcoin-13027d952519-x86_64-linux-gnu.tar.gz
28843dc5ff2a2618b08359debe12192be9f80ebd04ad7df8ec1d7bdb14837d8fdb  guix-build-13027d952519/output/x86_64-w64-mingw32/SHA256SUMS.part
2956588107c596008f7b330b125f344a09fd70cbb0b8afb5ea5cdf9c5baa795abc  guix-build-13027d952519/output/x86_64-w64-mingw32/bitcoin-13027d952519-win-unsigned.tar.gz
306df35bd9df17912b8888087d6afb3382e3f3c197b131e3667442e73629727a2d  guix-build-13027d952519/output/x86_64-w64-mingw32/bitcoin-13027d952519-win64-debug.zip
31e1b40b9b507e61018b3350a4e4dd142c13edf6807ba6cf273b4986cebdccec72  guix-build-13027d952519/output/x86_64-w64-mingw32/bitcoin-13027d952519-win64-setup-unsigned.exe
324d351114f736f21ffcd7e93f9eb1ebdbd7618db22f684e16e46e00c8287719ff  guix-build-13027d952519/output/x86_64-w64-mingw32/bitcoin-13027d952519-win64.zip

robot-dreams commented at 3:10 pm on December 3, 2021: contributor

ACK 84b984ba0b69faf6d04483ee8d24d130dc762af6

sipa commented at 9:11 pm on December 3, 2021: contributor

When compiling commit “refactor: Make PREC_BITS a parameter of ecmult_gen_build_prec_table” (ecff25fbd1998238d5fa67f74750f1857e1262e9):

0src/gen_ecmult_gen_static_prec_table.c: In function ‘main’:
1src/gen_ecmult_gen_static_prec_table.c:33:16: error: ‘ECMULT_GEN_PREC_B’ undeclared (first use in this function); did you mean ‘ECMULT_GEN_PREC_G’?
2   33 |     int bits = ECMULT_GEN_PREC_B;
3      |                ^~~~~~~~~~~~~~~~~
4      |                ECMULT_GEN_PREC_G
5src/gen_ecmult_gen_static_prec_table.c:33:16: note: each undeclared identifier is reported only once for each function it appears in
6make: *** [Makefile:1704: gen_ecmult_gen_static_prec_table.o] Error 1

If there’s a good reason why this individual commit doesn’t compile, that’s fine, but I assume it’s a simple oversight here.

refactor: Make PREC_BITS a parameter of ecmult_gen_build_prec_table fdb33dd122

real-or-random force-pushed on Dec 5, 2021

real-or-random commented at 5:04 pm on December 5, 2021: contributor

When compiling commit “refactor: Make PREC_BITS a parameter of ecmult_gen_build_prec_table” (ecff25f):

Forced-push to fix this intermediate commit. The new final tree at commit 56284c7d44c0ed46e636588bfbf6c403b7dfa6c1 is the same tree as at the previous commit 84b984ba0b69faf6d04483ee8d24d130dc762af6.

robot-dreams commented at 6:32 pm on December 5, 2021: contributor

ACK 56284c7d44c0ed46e636588bfbf6c403b7dfa6c1

Confirmed that the final result is unchanged, and that every intermediate commit builds. (Some of the earlier commits fail exhaustive_tests but I don’t have a strong opinion about this.)

real-or-random cross-referenced this on Dec 9, 2021 from issue build: Always assume cross compiling for mingw32 hosts by hebasto

ecmult_gen: Precompute tables for all values of ECMULT_GEN_PREC_BITS 5eba83f17c

ecmult_gen: Tidy precomputed file and save space 6573c08f65

prealloc: Get rid of manual memory management for prealloc contexts ac49361ed0

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

ad63bb4c29

build: Remove CC_FOR_BUILD stuff d94a37a20c

ci: Test `make precomp` bb36fe9be0

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.

7dfceceea6

real-or-random force-pushed on Dec 9, 2021

real-or-random commented at 7:54 pm on December 9, 2021: contributor

Pushed three further minimal fixups commits to https://github.com/real-or-random/secp256k1/tree/202108-prebuilt-gen-context-fixups1 and rebased here… @robot-dreams Sorry, can you reACK? :)

in src/gen_ecmult_gen_static_prec_table.c:36 in 7dfceceea6

31+    fprintf(fp, "/* This file was automatically generated by gen_ecmult_gen_static_prec_table. */\n");
32+    fprintf(fp, "/* See ecmult_gen_impl.h for details about the contents of this file. */\n");
33+    fprintf(fp, "#ifndef SECP256K1_ECMULT_GEN_STATIC_PREC_TABLE_H\n");
34+    fprintf(fp, "#define SECP256K1_ECMULT_GEN_STATIC_PREC_TABLE_H\n");
35+
36+    fprintf(fp, "#include \"group.h\"\n");

robot-dreams commented at 2:35 am on December 10, 2021:

Nice catch!

robot-dreams commented at 2:39 am on December 10, 2021: contributor

ACK 7dfceceea692c4118829c06128c41623b2373ac2 (based on range-diff between 56284c7d44c0ed46e636588bfbf6c403b7dfa6c1 and 7dfceceea692c4118829c06128c41623b2373ac2)

I agree with the removal of the conditional #include "libsecp256k1-config.h", since gen_ecmult_gen_static_prec_table.c doesn’t depend on the value of ECMULT_GEN_PREC_BITS to generate the output .h file.

I assume the generated file doesn’t need a similar conditional include because you’re assuming any file that imports ecmult_gen_static_prec_table.h must have already imported another header that has ECMULT_GEN_PREC_BITS defined?

real-or-random commented at 10:59 am on December 10, 2021: contributor

I assume the generated file doesn’t need a similar conditional include because you’re assuming any file that imports ecmult_gen_static_prec_table.h must have already imported another header that has ECMULT_GEN_PREC_BITS defined?

Yeah, any file including ecmult_gen_static_prec_table.h this should have included ecmult_gen.h. To be honest, that’s only half of the story. I opened #1039 for the other half…

elichai cross-referenced this on Dec 14, 2021 from issue Wasm, Contexts, and Entropy by JeremyRubin

sipa commented at 10:08 pm on December 14, 2021: contributor

ACK 7dfceceea692c4118829c06128c41623b2373ac2

real-or-random merged this on Dec 15, 2021

real-or-random closed this on Dec 15, 2021

sipa referenced this in commit 86dbc4d075 on Dec 15, 2021

sipa cross-referenced this on Dec 15, 2021 from issue Follow-ups to making all tables fully static by sipa

sipa cross-referenced this on Dec 17, 2021 from issue Follow-ups to making all tables fully static by sipa

real-or-random referenced this in commit be6944ade9 on Dec 20, 2021

real-or-random referenced this in commit 70591b8350 on Dec 20, 2021

real-or-random cross-referenced this on Dec 20, 2021 from issue ci: Optimize build matrix by real-or-random

real-or-random referenced this in commit 5a37cdb259 on Dec 20, 2021

real-or-random referenced this in commit 26a022a3a0 on Dec 20, 2021

real-or-random cross-referenced this on Dec 26, 2021 from issue Random isomorphism blinding for ecmult_gen by peterdettman

sipa cross-referenced this on Dec 26, 2021 from issue Suggestions on adding precomputes for verification by xxuejie

jonasnick cross-referenced this on Jan 2, 2022 from issue Sync Upstream by jonasnick

real-or-random referenced this in commit 21e2d65b79 on Jan 5, 2022

real-or-random cross-referenced this on Jan 7, 2022 from issue Better lowmemory feature with precomputed ECMULT by devrandom

real-or-random cross-referenced this on Jan 12, 2022 from issue Reducing context arguments in taproot-related functions by dr-orlovsky

real-or-random cross-referenced this on Jan 17, 2022 from issue Further changes after making tables static by real-or-random

gwillen referenced this in commit 35d6112a72 on May 25, 2022

janus referenced this in commit 879a9a27b9 on Jul 10, 2022

real-or-random cross-referenced this on Jul 18, 2022 from issue API cleanup with respect to contexts by real-or-random

patricklodder referenced this in commit 21badcf9d2 on Jul 25, 2022

fanquake cross-referenced this on Jul 26, 2022 from issue build: Evaluate passing depend's build_{CC,{C,CPP,LD}FLAGS} to bitcoin's configure by dongcarl

patricklodder referenced this in commit 03002a9013 on Jul 28, 2022

real-or-random cross-referenced this on Nov 21, 2022 from issue Makefile: build precomputed_ecmult generators using build-system toolchain to support cross-compiling by whitslack

backpacker69 referenced this in commit 77186f4a04 on Jan 18, 2023

str4d referenced this in commit 6de4698bf9 on Apr 21, 2023

dderjoel referenced this in commit 7724786d03 on May 23, 2023

vmta referenced this in commit e1120c94a1 on Jun 4, 2023

matteonardelli referenced this in commit 0971c39ef3 on Jun 16, 2023

vmta referenced this in commit 8f03457eed on Jul 1, 2023

Make signing table fully static #988