Replace ecmult_context with a generated static array.

[roconnor-blockstream]

Replace ecmult_context with a static array.

[elichai]

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

[roconnor-blockstream]

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]

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

[real-or-random]

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

[real-or-random]

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

src/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

[real-or-random]

should #include <stdio.h>

[real-or-random]

Code is good, here are some nits.

[real-or-random]

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]

<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

[laanwj]

Post-merge re-ACK 20abd52