Clean up configuration in gen_context

[real-or-random]

gen_context: Don't include basic-config.h

Before this commit, gen_context.c both included libsecp256k1-config.h
and basic-config.h: The former only to obtain ECMULT_GEN_PREC_BITS
and the latter to obtain a basic working configuration to be able to
use the library.

This was inelegant and confusing: It meant that basic-config.h needs
to #undef all the macros defined in libsecp256k1-config.h. Moreover,
it meant that basic-config.h cannot define ECMULT_GEN_PREC_BITS,
essentially making this file specific for use in gen_context.c.

After this commit, gen_context.c include only libsecp256k1-config.h.
basic-config.h is not necessary anymore for the modules used in
gen_context.c because 79f1f7a made the preprocessor detect all the
relevant config options.

On the way, we remove an unused #define in basic-config.h.

---

This now cherrypicks 71d76d1 from #918.

After this PR, basic-config.h is unused. We could simply remove it. But with the goal of improving non-autotools (#622) in mind, I don't think that's the best approach. Here are better options:

• We keep the file and it can help users to facilitate non-autotools builds. (This would mean we'll merge add ECMULT_GEN_PREC_BITS to basic_config.h #916 on top of this PR, which now should work properly now).
• We take a step back and think about a better solution for non-autotools builds. I'm willing to work on a PR.
  • The library currently builds cleanly with a simply compiler invocation except that ECMULT_WINDOW_SIZE and ECMULT_GEN_PREC_BITS are not defined. So I suggest we define default values for those in the corresponding files. Then the library would be cleanly without any necessary config.
  • We provide the user with a template config file that documents the available config options. Creating such a template is not too hard using autotools, which supports this for the specific purpose of allowing non-autotools builds.
  • We add a (then hopefully simple) section to the README that explains building without autotools.
  • We add a CI job that tests that the build works with the command from the README.

[real-or-random]

  * We provide the user with a template config file that documents the available config options. 

I just noticed that if we're going to do this, then gen_context.c should at least include libsecp256k1-config.h. I guess one config files is fine, just two is too much. We can probably just#undef ECMULT_USE_STATIC_PRECOMPUTATION in gen_context.c. I'll update the PR.

edit: I'd still be happy to hear comments on the above plans.

[sipa]

The problem here really stems from the fact that we have two classes of configuration options:

• Things that are specific to the build environment (e.g. presence of ARM assembly, and to a lesser extent the now-autodetected endianness/128bitness)
• Things that are configuration tweaks like table precomputation size

The latter has to be identical for the real build and gen_context, but the former cannot be (because gen_context needs to be built natively, while the main build may be cross-compiled).

Random thought: what about replacing gen_context with a Python script? We have some experience with Python code for EC arithmetic already, it'd be more than performant enough for this purpose, constant-timeness is not a concern, and the result would still be subject to all tests of the resulting build (though we way want to add a test that is designed to access all table entries). That would avoid all complications the current build system has that result from cross-compilation.

Other than that... it feels a bit strange to treat this ECMULT_GEN_PREC_BITS configuration for gen_context different from everything else, but I don't see a better solution.

Another vague thought: I think we should aim for having the library not requiring any #defines at all for building (except for weird platforms where e.g. endianness/64bitness detection fail, or for deviating from defaults like ecmult table size, or for things that require explicit build system support like the arm assembly). With the changes to make endianness and 64bitness automatically detected at compile time, I think we're most of the way there.

[real-or-random]

The problem here really stems from the fact that we have two classes of configuration options:

* Things that are specific to the build environment (e.g. presence of ARM assembly, and to a lesser extent the now-autodetected endianness/128bitness)

* Things that are configuration tweaks like table precomputation size

The latter has to be identical for the real build and gen_context, but the former cannot be (because gen_context needs to be built natively, while the main build may be cross-compiled).

Interesting observation. It may be a good idea to split these options into two sections or two files.

Random thought: what about replacing gen_context with a Python script? We have some experience with Python code for EC arithmetic already, it'd be more than performant enough for this purpose, constant-timeness is not a concern, and the result would still be subject to all tests of the resulting build (though we way want to add a test that is designed to access all table entries). That would avoid all complications the current build system has that result from cross-compilation.

Our current code comes with a lot of autotools complexity and can be annoying for the user when it comes to cross-compilation but the approach is pretty solid nowadays. I'm not convinced that a build dependency on Python will be less annoying on average.

But isn't the real solution here to simply add the generated files to the repo, ideally for all supported values of ECMULT_GEN_PREC_BITS? This will save the user all the trouble and we're free to do whatever we like in the backend (keep the current approach or switch to Python). Here's a table of file sizes:

ECMULT_GEN_PREC_BITS	raw	gzipped
2	107 kB	47 kB
4	213 kB	92 kB
8	1695 kB	720 kB
(current repo size)	1190 kB	218 kB

All of this is not an issue. If we care, we can restrict the constant to be 2 or 4. Nobody will need 8 and we anyway want to make the test matrix smaller.

Other than that... it feels a bit strange to treat this ECMULT_GEN_PREC_BITS configuration for gen_context different from everything else, but I don't see a better solution.

Yeah it's not elegant but it would work.

Another vague thought: I think we should aim for having the library not requiring any #defines at all for building (except for weird platforms where e.g. endianness/64bitness detection fail, or for deviating from defaults like ecmult table size, or for things that require explicit build system support like the arm assembly). With the changes to make endianness and 64bitness automatically detected at compile time, I think we're most of the way there.

Right, as I said above, we're almost there for a basic config. The only missing thing is defaults for the two tweak values of ECMULT_GEN_PREC_BITS and ECMULT_WINDOW_SIZE. After we have those, the library should always compile. You may not get ASM or precomputation but at least it will work out of the box.

[sipa]

@real-or-random With #693 the number of possible configurations for the ecmult_gen table will go up a lot though, and in some cases even pretty big ones may be reasonable to use there.

Also if we'd want to provide a mode where the ecmult table is precomputed, perhaps it is not desirable that all its code is checked in?

A possibility perhaps is shipping the precomputed tables for the default configuration in the repository, and use a runtime created one for others (either through native-compiled code, or Python generated code).

[real-or-random]

@real-or-random With #693 the number of possible configurations for the ecmult_gen table will go up a lot though, and in some cases even pretty big ones may be reasonable to use there.

Oh I always forget about this PR.

Also if we'd want to provide a mode where the ecmult table is precomputed, perhaps it is not desirable that all its code is checked in?

A possibility perhaps is shipping the precomputed tables for the default configuration in the repository, and use a runtime created one for others (either through native-compiled code, or Python generated code).

That sounds very reasonable, and we can still see if we want to ship just one file for a single configuration or more. The precomputed files are probably still smaller than any installation of Python or a native compiler.

Let me still update this PR as planned. What do other people think about the current approach vs Python?

[real-or-random]

I fixed this and edited the initial comment.

[sipa]

utACK 0706796

The switch to Python if desired can always be done later.

[jonasnick]

Nice clean up.

We keep the file and it can help users to facilitate non-autotools builds.

How about removing the USE_BASIC_CONFIG ifdef which seems to have no use? Generally, the "we take a step back plan" sounds like a good solution to #622 to me.

ACK mod nit

[real-or-random]

Nice clean up.

We keep the file and it can help users to facilitate non-autotools builds.

How about removing the USE_BASIC_CONFIG ifdef which seems to have no use? Generally, the "we take a step back plan" sounds like a good solution to #622 to me.

I think if we keep the file for now, then we should keep this ifdef for now to avoid touching the "API". Some people may define the macro conditionally.

We'll probably remove the file later but then at least we interrupt the compile process of users only once then.

[real-or-random]

@sipa said somehere else:

I was thinking something like this:

• In util.h (or another new file) do all autodetection as much as it can for everything, but supports #ifdefs for overriding things (e.g. endianness, int128, …) and other optional features that need build support (arm asm)
• util.h (or this new file) also has an early #ifdef for HAVE_CONFIG_H that includes config.h, which can be written by the build system. Everything else includes just util, not the config directly (because the config only overrides things for autodetection)
• building without -D… anything gives you exactly the defaults/autodetected values, nothing else
• with extensions util.h could also work without HAVE_CONFIG_H

This sounds very reasonable to me.

[jonasnick]

the "API"

Haha, okay.
ACK 0706796

[gmaxwell]

With #693 the number of possible configurations for the ecmult_gen table will go up a lot though,

I know I said this on IRC-- but I see it's not here. The configurations there shouldn't go up: It's not a good use of resources to test a large number of configurations and 99.999999% of users will have no more idea how to set these things than we do. Instead, there should be a small number of exposed configurations (like small, medium, large) just for protecting the sanity of the user and the testability of the software -- and as a side effect it keeps the shipped configurations manageable.

[sipa]

@gmaxwell I mean: the number of supportable configurations goes up, if we keep things through the same kind of set-the-individual-tunables-via-./configure interface. I fully agree we should independently (with or without multicomb), change that to a small/medium/large interface.