Replace ecmult_context with a generated static array.

roconnor-blockstream commented at 10:47 pm on June 25, 2021: contributor

Replace ecmult_context with a static array.

elichai commented at 9:48 am on June 27, 2021: contributor

Was curious, the binary size on my machine with all modules enabled before this change was: 672K, in this branch: 1.6M.

roconnor-blockstream commented at 1:32 pm on June 27, 2021: contributor

That’s expected since the ecmult_context is now statically allocated instead of dynamically allocated. You should find that your runtime memory use is down by about the same amount.

I’ll also add that you can reduce the binary size by reducing the ECMULT_WINDOW_SIZE through the configuration parameter.

elichai commented at 2:45 pm on June 27, 2021: contributor

That’s expected since the ecmult_context is now statically allocated instead of dynamically allocated. You should find that your runtime memory use is down by about the same amount.

I’ll also add that you can reduce the binary size by reducing the ECMULT_WINDOW_SIZE through the configuration parameter.

Yeah I know :) it wasn’t a complaint or anything, just wanted to write it here for reference. less than a MB for simplifying the API and simplifying usage is pretty good

roconnor-blockstream force-pushed on Jun 28, 2021

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

real-or-random commented at 2:29 pm on June 29, 2021: contributor

@roconnor-blockstream Here’s a branch with two fixups commits: https://github.com/real-or-random/secp256k1/commits/static_ecmult_ctx_20210625

The first commit simply reverts your changes to the build system.

The second commit introduces a very simple approach to change the build system: Just have a configure option --enable-devtools (default no), which enables building of gen_pre_g as a developer tool. The build system will build but never run gen_pre_g. This is by far the simplest we can do. Since the precomputed file will be shipped with the tree, only devs need to update the file and can do this manually by running gen_pre_g. Building gen_pre_g ignores issues of cross-compilation, i.e., if you compile for a foreign target, it will just compile gen_pre_g for that foreign target. Who cares, if you need to run gen_pre_g, simply don’t cross-compile. By doing the same in a later PR for gen_context we could get of the entire cumbersome and error-prone machinery of detecting host compilers (CFLAGS_FOR_BUILD and friends).

Nit on naming: Maybe we could call this gen_ecmult_pre_g to make it clear from the name that it corresponds to the ecmult.h module. Otherwise people (including me) will constantly think that it is for ecmult_gen.h due to the g in the name. In the other PR we could then rename gen_context to gen_ecmult_gen_context. This will make things a little bit clearer. Naming is very confusing at the moment with even more issues, e.g., I claim enable-ecmult-static-precomputation should have been called enable-ecmultgen-static-precomputation.

roconnor-blockstream force-pushed on Jun 30, 2021

in src/gen_ecmult_static_pre_g.c:7 in aa900435c7 outdated

0@@ -0,0 +1,249 @@
1+/*****************************************************************************************************
2+ * Copyright (c) 2013, 2014, 2017, 2021 Pieter Wuille, Andrew Poelstra, Jonas Nick, Russell O'Connor *
3+ * Distributed under the MIT software license, see the accompanying                                  *
4+ * file COPYING or https://www.opensource.org/licenses/mit-license.php.                              *
5+ *****************************************************************************************************/
6+
7+/* Autotools creates libsecp256k1-config.h, of which ECMULT_GEN_PREC_BITS is needed.

real-or-random commented at 1:47 pm on July 1, 2021:

ECMULT_WINDOW_SIZE

roconnor-blockstream commented at 2:56 pm on July 2, 2021:

Fixed.

in src/gen_ecmult_static_pre_g.c:200 in aa900435c7 outdated

195+    const int window_g_13 = 4;
196+    const int window_g_199 = 8;
197+    FILE* fp;
198+    secp256k1_gej gj;
199+    (void)argc;
200+    (void)argv;

real-or-random commented at 6:43 pm on July 1, 2021:

FWIW, int main(void) is valid C and simpler

roconnor-blockstream commented at 2:56 pm on July 2, 2021:

Done.

in src/gen_ecmult_static_pre_g.c:167 in aa900435c7 outdated

162+    secp256k1_ecmult_odd_multiples_table_storage(ECMULT_TABLE_SIZE(window_g), gj);
163+    fprintf(fp, "static const secp256k1_ge_storage %s[ECMULT_TABLE_SIZE(WINDOW_G)] = {\n", name);
164+    fprintf(fp," S(%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x)\n", SECP256K1_GE_STORAGE_CONST_GET(pre[0]));
165+    j = 1;
166+    for(i = 3; i <= window_g; ++i) {
167+        if (withConditionals) {

real-or-random commented at 6:44 pm on July 1, 2021:

I think we’d prefer snake_case. :P

roconnor-blockstream commented at 2:57 pm on July 2, 2021:

Done.

real-or-random commented at 7:06 pm on July 1, 2021: contributor

If you want, you could add a .gitattributes file with this content:

0src/ecmult_static_pre_g.h linguist-generated

This will exclude this file from GitHub language statistics (x% C Code, y% bash script, …), and fold the file by default when showing diffs. But yeah, it’s not the most important thing in the world. :P

https://github.com/github/linguist/blob/master/docs/overrides.md

roconnor-blockstream force-pushed on Jul 2, 2021

roconnor-blockstream renamed this:
~~Static ecmult ctx 20210625~~
Replace ecmult_context with a generated static array.
on Jul 2, 2021

roconnor-blockstream marked this as ready for review on Jul 2, 2021

in src/gen_ecmult_static_pre_g.c:25 in 4378cdbd5c outdated

20+#include "util.h"
21+#include "field_impl.h"
22+#include "group_impl.h"
23+#include "ecmult.h"
24+
25+static secp256k1_ge_storage pre[ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE) < ECMULT_TABLE_SIZE(8)

roconnor-blockstream commented at 3:08 pm on July 2, 2021:

Could be better to put this array inside printTable, though we would still want to keep it static because we wouldn’t want to allocate it on the stack.

roconnor-blockstream commented at 3:52 pm on July 5, 2021:

Done.

in src/gen_ecmult_static_pre_g.c:214 in 4378cdbd5c outdated

209+    fprintf(fp, "#include \"src/group.h\"\n");
210+    fprintf(fp, "#ifdef S\n");
211+    fprintf(fp, "   #error macro identifier S already in use.\n");
212+    fprintf(fp, "#endif\n");
213+    fprintf(fp, "#define S(a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p) "
214+                "SECP256K1_GE_STORAGE_CONST(0x##a##u,0x##b##u,0x##c##u,0x##d##u,0x##e##u,0x##f##u,0x##g##u,0x##h##u,0x##i##u,0x##j##u,0x##k##u,0x##l##u,0x##m##u,0x##n##u,0x##o##u,0x##p##u)\n");

roconnor-blockstream commented at 3:09 pm on July 2, 2021:

Maybe break up this string across two lines.

roconnor-blockstream commented at 3:52 pm on July 5, 2021:

fixed.

in src/tests.c:3438 in 4378cdbd5c outdated

3393+    CHECK(0 < n);
3394+    secp256k1_ge_from_storage(&p, &pre_g[0]);
3395+    secp256k1_gej_set_ge(&g2, &p);
3396+    secp256k1_gej_double_var(&g2, &g2, NULL);
3397+    secp256k1_ge_set_gej_var(&gg, &g2);
3398+    secp256k1_fe_verify(&p.x);

roconnor-blockstream commented at 3:13 pm on July 2, 2021:

Move these there verification lines up to immediately after the ge_from_storage.

roconnor-blockstream commented at 3:52 pm on July 5, 2021:

fixed.

in src/gen_ecmult_static_pre_g.c:181 in 4378cdbd5c outdated

176+    }
177+    fprintf(fp, "};\n");
178+
179+}
180+
181+void printTwoTables(FILE* fp, int window_g, secp256k1_gej * gj, int with_conditionals) {

real-or-random commented at 3:37 pm on July 2, 2021:

here also snake case and also in printTwoTables

roconnor-blockstream commented at 3:52 pm on July 5, 2021:

Fixed.

real-or-random commented at 3:41 pm on July 2, 2021: contributor

should #include <stdio.h>

in src/gen_ecmult_static_pre_g.c:158 in 4378cdbd5c outdated

153+        /* Store */
154+        secp256k1_ge_to_storage(&pre[i], &p_ge);
155+    }
156+}
157+
158+void printTable(FILE* fp, const char * name, int window_g, const secp256k1_gej * gj, int with_conditionals) {

roconnor-blockstream commented at 6:15 pm on July 2, 2021:

snake_case this function.

roconnor-blockstream commented at 3:52 pm on July 5, 2021:

fixed.

roconnor-blockstream force-pushed on Jul 5, 2021

in src/gen_ecmult_static_pre_g.c:211 in fb68494060 outdated

206+    }
207+
208+    fprintf(fp, "/* This file was automatically generated by gen_ecmult_static_pre_g. */\n");
209+    fprintf(fp, "#ifndef SECP256K1_ECMULT_STATIC_PRE_G_H\n");
210+    fprintf(fp, "#define SECP256K1_ECMULT_STATIC_PRE_G_H\n");
211+    fprintf(fp, "#include \"src/group.h\"\n");

real-or-random commented at 8:27 am on July 6, 2021:

src/ should be dropped. (At least since #925, we don’t want to require that the project root is in the include path. I assume somehow should submit a PR that tests this on CI…)

roconnor-blockstream commented at 12:33 pm on July 6, 2021:

Fixed.

in src/gen_ecmult_static_pre_g.c:225 in fb68494060 outdated

220+    fprintf(fp, "#endif\n");
221+    fprintf(fp, "#if defined(EXHAUSTIVE_TEST_ORDER)\n");
222+    fprintf(fp, "#if EXHAUSTIVE_TEST_ORDER == 13\n");
223+    fprintf(fp, "#define WINDOW_G %d\n", window_g_13);
224+
225+    secp256k1_gej_set_ge(&gj, &g_13);

real-or-random commented at 8:30 am on July 6, 2021:

Maybe do these inside print_two_tables. (Less duplication and the line that calls print_two_tables will be self-containing.)

roconnor-blockstream commented at 12:33 pm on July 6, 2021:

Done.

in src/gen_ecmult_static_pre_g.c:237 in fb68494060 outdated

232+    print_two_tables(fp, window_g_199, &gj, 0);
233+
234+    fprintf(fp, "#else\n");
235+    fprintf(fp, "   #error No known generator for the specified exhaustive test group order.\n");
236+    fprintf(fp, "#endif\n");
237+    fprintf(fp, "#else\n");

real-or-random commented at 8:38 am on July 6, 2021:

Maybe add /* !defined(EXHAUSTIVE_TEST_ORDER) */. But yeah, I guess readability of the generated file is anyway not a reasonable goal here. :)

roconnor-blockstream commented at 12:33 pm on July 6, 2021:

Done.

in src/gen_ecmult_static_pre_g.c:53 in fb68494060 outdated

167+    j = 1;
168+    for(i = 3; i <= window_g; ++i) {
169+        if (with_conditionals) {
170+            fprintf(fp, "#if ECMULT_TABLE_SIZE(WINDOW_G) > %ld\n", ECMULT_TABLE_SIZE(i-1));
171+        }
172+        for(;j < ECMULT_TABLE_SIZE(i); ++j) {

real-or-random commented at 8:50 am on July 6, 2021:

Is there a reason why j = 1 is not defined here?

roconnor-blockstream commented at 11:51 am on July 6, 2021:

j is not reset each time this inner loop is executed. ECMULT_TABLE_SIZE(i) gets larger as i increases and j continues from where it left off.

real-or-random commented at 3:20 pm on July 6, 2021:

Oh sure!

in src/gen_ecmult_static_pre_g.c:183 in fb68494060 outdated

178+    }
179+    fprintf(fp, "};\n");
180+
181+}
182+
183+void print_two_tables(FILE* fp, int window_g, secp256k1_gej * gj, int with_conditionals) {

real-or-random commented at 8:52 am on July 6, 2021:

nit: spacing around *

roconnor-blockstream commented at 12:33 pm on July 6, 2021:

Fixed.

in src/gen_ecmult_static_pre_g.c:173 in fb68494060 outdated

168+    for(i = 3; i <= window_g; ++i) {
169+        if (with_conditionals) {
170+            fprintf(fp, "#if ECMULT_TABLE_SIZE(WINDOW_G) > %ld\n", ECMULT_TABLE_SIZE(i-1));
171+        }
172+        for(;j < ECMULT_TABLE_SIZE(i); ++j) {
173+            fprintf(fp, ",S(%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x,%x)\n", SECP256K1_GE_STORAGE_CONST_GET(pre[j]));

real-or-random commented at 9:04 am on July 6, 2021:

I think these should be %lx (or PRIu32N) because unsigned long is guaranteed to be 32 bits but unsigned int isn’t.

roconnor-blockstream commented at 12:33 pm on July 6, 2021:

Fixed.

in src/tests.c:3419 in 8e8d0e41ba outdated

3415@@ -3416,6 +3416,74 @@ void run_group_decompress(void) {
3416 
3417 /***** ECMULT TESTS *****/
3418 
3419+void test_pre_g_table(const secp256k1_ge_storage * pre_g, size_t n) {

real-or-random commented at 9:16 am on July 6, 2021:

spacing around *

roconnor-blockstream commented at 12:34 pm on July 6, 2021:

Fixed.

jonasnick commented at 2:35 pm on August 13, 2021:

Took me embarassingly long to understand what is going on here. Perhaps worth adding an explanation like:

0For all i in [0, n), p = pre_g[i], q = pre_g[i+1], gg = 2*pre_g[0] we have p + gg = q. Instead of more costly group operations, we check that the 3 points p, gg and -q are distinct, on the curve and on the same line.

roconnor-blockstream commented at 9:12 pm on August 13, 2021:

I added some more docs.

jonasnick commented at 6:55 pm on August 14, 2021:

Thanks!

real-or-random commented at 9:18 am on July 6, 2021: contributor

Code is good, here are some nits.

real-or-random commented at 9:27 am on July 6, 2021: contributor

I wonder what we should do with the context now… For example, the ecdsa_verify still says that it needs a verification context but that won’t be true after this PR. Contexts require some discussion (see for example #780 (comment)) but it should not delay this PR. Maybe leave this as is now and postpone this until we made ecmult_gen stuff mandatory static?

roconnor-blockstream force-pushed on Jul 6, 2021

roconnor-blockstream commented at 12:36 pm on July 6, 2021: contributor

<sipa> on cortex-A73 (Odroid N2+), the optimal table size seems to be 22...
<sipa> 196 us at 15, 189 us at 22
<sipa> for an ecdsa verify
<sipa> that's a 128 MiB table :s

roconnor-blockstream force-pushed on Jul 6, 2021

sipa cross-referenced this on Jul 14, 2021 from issue guix: libsecp256k1 build failure on aarch64 (M1) by fanquake

in src/gen_ecmult_static_pre_g.c:19 in 8de62e3673 outdated

14+#endif
15+
16+/* In principle we could use external 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 external ASM for the build and the host machine. */
19+#undef USE_EXTERNAL_ASM

real-or-random commented at 9:20 am on July 14, 2021:

Since this code is taken from gen_context.c, it should be updated if #965 is merged before this PR.

sipa commented at 11:49 pm on July 24, 2021:

#965 was in fact merged before this PR; what needs to be done?

roconnor-blockstream commented at 5:43 pm on July 27, 2021:

I’ve cargo-cult-copied #965.

in src/gen_ecmult_static_pre_g.c:158 in 8de62e3673 outdated

153+        secp256k1_ge_to_storage(&pre[i], &p_ge);
154+    }
155+}
156+
157+void print_table(FILE *fp, const char *name, int window_g, const secp256k1_gej *gj, int with_conditionals) {
158+    /* This array is only static because it may be unreasonably large to place on the stack. */

elichai commented at 8:42 am on July 21, 2021:

small nit, since this is a binary and not a library you can use malloc if you have concerns about sizes

roconnor-blockstream commented at 12:23 pm on July 21, 2021:

I do think it is somewhat better as a static array, but if other people also prefer a dynamic array, I can change it.

Perhaps even better is to eliminate the allocation entirely and rewrite secp256k1_ecmult_odd_multiples_table_storage to be a simpler and more direct computation. Since we have to call ge_normalize on every output anyways, there is little point in getting fancy. secp256k1_ecmult_odd_multiples_table_storage is only the way it is because I have moved it from ecmult_impl.

sipa commented at 11:51 pm on July 24, 2021:

It’s slightly strange to use a global object just because the size is too large; using the heap would be more natural I’d say. But also, the current code seems to work fine, no need to change it.

roconnor-blockstream commented at 5:44 pm on July 27, 2021:

The array has been nuked.

in src/ecmult_impl.h:48 in d58d75ff3d outdated

56- * because certain expressions will overflow.
57- */
58-#if ECMULT_WINDOW_SIZE < 2 || ECMULT_WINDOW_SIZE > 24
59-#  error Set ECMULT_WINDOW_SIZE to an integer in range [2..24].
60+#if ECMULT_WINDOW_SIZE < WINDOW_G
61+#  error ECMULT_WINDOW_SIZE to small for WINDOW_G.

sipa commented at 11:36 pm on July 24, 2021:

s/to/too/

roconnor-blockstream commented at 5:42 pm on July 27, 2021:

Fixed.

roconnor-blockstream force-pushed on Jul 27, 2021

sipa commented at 3:56 pm on July 28, 2021: contributor

utACK 8e9f75a5888a8ec549fe9026053051c3db7a1282

in src/ecmult_impl.h:145 in 8e9f75a588 outdated

120@@ -142,137 +121,6 @@ static void secp256k1_ecmult_odd_multiples_table_globalz_windowa(secp256k1_ge *p
121     secp256k1_ge_globalz_set_table_gej(ECMULT_TABLE_SIZE(WINDOW_A), pre, globalz, prej, zr);
122 }
123 
124-static void secp256k1_ecmult_odd_multiples_table_storage_var(const int n, secp256k1_ge_storage *pre, const secp256k1_gej *a) {

roconnor-blockstream commented at 6:04 pm on August 9, 2021:

The comment 20ish lines above here is out of date.

roconnor-blockstream commented at 9:01 pm on August 9, 2021:

Resolved.

roconnor-blockstream force-pushed on Aug 9, 2021

laanwj commented at 2:50 pm on August 10, 2021: member

Concept and code review ACK 8e9f75a5888a8ec549fe9026053051c3db7a1282 Embedding the read-only data is useful on embedded systems which have plenty of ROM/FLASH but limited RAM.

Nit: please put an empty line between the commit title and body, this makes it possible for tools that provide one-line summaries (like git log --pretty=oneline) to only show the title.

roconnor-blockstream force-pushed on Aug 10, 2021

roconnor-blockstream commented at 4:52 pm on August 10, 2021: contributor

Reworded the commit messages.

laanwj commented at 5:00 pm on August 10, 2021: member

Thanks! re-ACK f4d79009bff8db0d1e53178b39072e504ec49156

in src/ecmult_impl.h:325 in f4d79009bf outdated

322+            ECMULT_TABLE_GET_GE_STORAGE(&tmpa, secp256k1_pre_g, n, WINDOW_G);
323             secp256k1_gej_add_zinv_var(r, r, &tmpa, &Z);
324         }
325         if (i < bits_ng_128 && (n = wnaf_ng_128[i])) {
326-            ECMULT_TABLE_GET_GE_STORAGE(&tmpa, *ctx->pre_g_128, n, WINDOW_G);
327+            VERIFY_CHECK(secp256k1_pre_g_128 != NULL);

jonasnick commented at 2:05 pm on August 13, 2021:

What does this VERIFY_CHECK do? Isn’t secp256k1_pre_g_128 an array?

roconnor-blockstream commented at 6:23 pm on August 13, 2021:

This is outdated and needs to be removed. I used to have secp256k1_pre_g_128 equal to NULL for EXHAUSTIVE_TEST_ORDER, but later I rewrote it to define it even in that case, even though it is not used at the moment.

roconnor-blockstream commented at 9:12 pm on August 13, 2021:

Fixed.

in src/gen_ecmult_static_pre_g.c:96 in f4d79009bf outdated

91+    if (fp == NULL) {
92+        fprintf(stderr, "Could not open src/ecmult_static_pre_g.h for writing!\n");
93+        return -1;
94+    }
95+
96+    fprintf(fp, "/* This file was automatically generated by gen_ecmult_static_pre_g. */\n");

jonasnick commented at 2:17 pm on August 13, 2021:

I think we removed the comment that documented what the tables actually contain. How about adding something like

0/* This file contains an array secp256k1_pre_g with odd multiples of the base point G and an array secp256k1_pre_g_128 with odd multiples of 2^128*G */

roconnor-blockstream commented at 9:12 pm on August 13, 2021:

Added a comment.

jonasnick commented at 2:36 pm on August 13, 2021: contributor

I didn’t have trouble to build (and run the tests) with window size 22 on my laptop with 32 GB of memory. It looked like it never used more than 20GB. So concept ACK. For posteriority (and this should probably be documented somewhere):

0rm src/ecmult_static_pre_g.h
1./configure --with-ecmult-window=22
2make gen_ecmult_static_pre_g
3./gen_ecmult_static_pre_g
4# ecmult_static_pre_g.h is 294 MB
5make -j 1 # make sure to use one job because otherwise it uses even more memory

roconnor-blockstream commented at 6:18 pm on August 13, 2021: contributor

@jonasnick In principle we ought to be able to support upto a window size of 24 if you want to try that as well.

roconnor-blockstream force-pushed on Aug 13, 2021

jonasnick commented at 6:55 pm on August 14, 2021: contributor

ACK 47f6185d622fa77a6dd6420aa667213f8d8b38f2

I tested with various ecmult_windows sizes.

sipa commented at 11:58 pm on August 14, 2021: contributor

@gmaxwell You want to try building with the table size set to 24?

in src/ecmult.h:15 in 47f6185d62 outdated

21-static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, void **prealloc);
22-static void secp256k1_ecmult_context_finalize_memcpy(secp256k1_ecmult_context *dst, const secp256k1_ecmult_context *src);
23-static void secp256k1_ecmult_context_clear(secp256k1_ecmult_context *ctx);
24-static int secp256k1_ecmult_context_is_built(const secp256k1_ecmult_context *ctx);
25+/* Noone will ever need more than a window size of 24. The code might
26+ * be correct for larger values of ECMULT_WINDOW_SIZE but this is not

sipa commented at 8:50 pm on August 16, 2021:

Not introduced in this PR, so feel free to ignore if you want to keep this move-only, but: not is repeated here.

roconnor-blockstream commented at 2:52 pm on August 17, 2021:

Fixed.

roconnor-blockstream force-pushed on Aug 17, 2021

in configure.ac:182 in 54ada05ef9 outdated

175@@ -176,6 +176,7 @@ AC_ARG_WITH([asm], [AS_HELP_STRING([--with-asm=x86_64|arm|no|auto],
176 AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE|auto],
177 [window size for ecmult precomputation for verification, specified as integer in range [2..24].]
178 [Larger values result in possibly better performance at the cost of an exponentially larger precomputed table.]
179+[For very large window sizes, use "make -j 1" to reduce memory use during compilation.]
180 [The table will store 2^(SIZE-1) * 64 bytes of data but can be larger in memory due to platform-specific padding and alignment.]
181 ["auto" is a reasonable setting for desktop machines (currently 15). [default=auto]]

sipa commented at 9:56 pm on August 17, 2021:

I think it would be good to explain here that setting a value larger than 15 will require regeneration of ecmult_static_pre_g.h (and maybe instructions on how to do that).

Ping @gmaxwell.

real-or-random commented at 10:40 pm on August 17, 2021:

Should be ship a file with values up to 16 or 17 even though 15 is the default?

roconnor-blockstream commented at 2:23 pm on August 18, 2021:

I personally think it is better to stick with 15.

sipa commented at 7:21 pm on August 18, 2021:

I think shipping with 15 is sufficient as long as the default is 15.

roconnor-blockstream commented at 9:10 pm on August 18, 2021:

Further elaborated instructions.

in src/gen_ecmult_static_pre_g.c:110 in 54ada05ef9 outdated

105+    fprintf(fp, "#endif\n");
106+    fprintf(fp, "#define S(a,b,c,d,e,f,g,h,i,j,k,l,m,n,o,p) "
107+                "SECP256K1_GE_STORAGE_CONST(0x##a##u,0x##b##u,0x##c##u,0x##d##u,0x##e##u,0x##f##u,0x##g##u,"
108+                "0x##h##u,0x##i##u,0x##j##u,0x##k##u,0x##l##u,0x##m##u,0x##n##u,0x##o##u,0x##p##u)\n");
109+    fprintf(fp, "#if ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE) > %ld\n", ECMULT_TABLE_SIZE(ECMULT_WINDOW_SIZE));
110+    fprintf(fp, "   #error configuration mismatch, invalid ECMULT_WINDOW_SIZE. Try deleting ecmult_static_pre_g.h before the build.\n");

sipa commented at 9:57 pm on August 17, 2021:

Perhaps here too; just deleting isn’t enough; it also needs a “make src/ecmult_static_pre_g.h” step.

roconnor-blockstream commented at 2:30 pm on August 18, 2021:

Deleting src/ecmult_static_pre_g.h seems to cause it to be rebuilt.

My limited experiments suggests that reconfiguring causes gen_ecmult_static_pre_g to be rebuilt.

Is there some particular problem you are observing?

sipa commented at 7:20 pm on August 18, 2021:

You’re right, I’m unsure why I thought differently. I built from a clean checkout with the default first, then reconfigured with window size 20, rebuilt, got this error, deleted the src/ecmult_static_pre_g.h file, rebuilt again, and it worked.

roconnor-blockstream commented at 9:02 pm on August 18, 2021:

We don’t ship with gen_ecmult_static_pre_g binaries, so you might need to try to reconfigure after creating that binary, but when I tested that situation it still worked for me.

sipa commented at 10:24 pm on August 17, 2021: contributor

FWIW, I believe @gmaxwell successfully built with window size 24.

roconnor-blockstream force-pushed on Aug 18, 2021

real-or-random commented at 2:29 pm on August 20, 2021: contributor

A trivial rebase on master should fix the CI hiccups.

Bump memory limits in advance of making the ecmult context static. 8de2d86a06

Generate ecmult_static_pre_g.h

This header contains a static array that replaces the ecmult_context pre_g and pre_g_128 tables.
The gen_ecmult_static_pre_g program generates this header file.

16a3cc07e8

Correct typo. f20dcbbad1

Remove ecmult_context.

These tables stored in this context are now statically available from the generated ecmult_static_pre_g.h file.

6815761cf5

Add tests for pre_g tables.

We check that the static table entries are all correct.

20abd52c2e

roconnor-blockstream force-pushed on Aug 20, 2021

real-or-random commented at 2:10 pm on August 25, 2021: contributor

ACK 20abd52c2e107e79391a19d2d2f8845e83858dea code inspection and tested some parameters

real-or-random cross-referenced this on Aug 25, 2021 from issue Signed-digit multi-comb for ecmult_gen (by peterdettman) by sipa

sipa commented at 6:36 pm on August 25, 2021: contributor

utACK 20abd52c2e107e79391a19d2d2f8845e83858dea (reviewed diff with earlier reviewed commit 8e9f75a5888a8ec549fe9026053051c3db7a1282)

real-or-random merged this on Aug 25, 2021

real-or-random closed this on Aug 25, 2021

roconnor-blockstream deleted the branch on Aug 25, 2021

laanwj commented at 10:34 am on August 26, 2021: member

Post-merge re-ACK 20abd52c2e107e79391a19d2d2f8845e83858dea

jonasnick cross-referenced this on Sep 15, 2021 from issue Upstream PRs 969, 956, 783, 976 by jonasnick

fanquake referenced this in commit ff458a7b78 on Sep 29, 2021

real-or-random cross-referenced this on Oct 8, 2021 from issue Make signing table fully static by real-or-random

real-or-random referenced this in commit 7812feb896 on Oct 15, 2021

fanquake referenced this in commit 8f5cd5e893 on Oct 20, 2021

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

sipa referenced this in commit f727914d7e on Oct 28, 2021

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

sipa referenced this in commit 440f7ec80e on Oct 31, 2021

dhruv referenced this in commit 395e1155b9 on Nov 2, 2021

dhruv referenced this in commit 184e1fac17 on Nov 2, 2021

real-or-random cross-referenced this on Nov 26, 2021 from issue Reducing context arguments in taproot-related functions by dr-orlovsky

sipa referenced this in commit d057eae556 on Dec 2, 2021

fanquake referenced this in commit c4a1e09a8c on Dec 3, 2021

real-or-random referenced this in commit 0559fc6e41 on Dec 15, 2021

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

fanquake referenced this in commit c06cda3e48 on Dec 18, 2021

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

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

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

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

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

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

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

vmta referenced this in commit e1120c94a1 on Jun 4, 2023

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

apoelstra cross-referenced this on Jul 12, 2023 from issue group: ge(j) should have as invariant that the curve equation holds by real-or-random

Replace ecmult_context with a generated static array. #956