Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Follow-ups to making all tables fully static #1042

Merged
merged 14 commits into from
Dec 20, 2021
Merged

Follow-ups to making all tables fully static #1042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
bae7768
Rename gen_ecmult_gen_static_prec_table -> precompute_ecmult_gen
sipa Dec 17, 2021
f95b810
Rename gen_ecmult_static_pre_g -> precompute_ecmult
sipa Dec 17, 2021
7cf47f7
Rename ecmult_gen_static_prec_table -> precomputed_ecmult_gen
sipa Dec 17, 2021
075252c
Rename ecmult_static_pre_g -> precomputed_ecmult
sipa Dec 17, 2021
725370c
Rename ecmult_gen_prec -> ecmult_gen_compute_table
sipa Dec 17, 2021
31feab0
Rename function secp256k1_ecmult_gen_{create_prec -> compute}_table
sipa Dec 18, 2021
fc1bf9f
Split ecmult table computation and printing
sipa Dec 17, 2021
e458ec2
Move ecmult table computation code to separate file
sipa Dec 17, 2021
38cd84a
Compute ecmult tables at runtime for tests_exhaustive
sipa Dec 17, 2021
bb36331
Simplify precompute_ecmult_print_*
sipa Dec 17, 2021
1a6691a
Split off .c file from precomputed_ecmult_gen.h
sipa Dec 17, 2021
19d96e1
Split off .c file from precomputed_ecmult.h
sipa Dec 17, 2021
c45386d
Cleanup preprocessor indentation in precompute{,d}_ecmult{,_gen}
sipa Dec 17, 2021
e05da9e
Fix c++ build
sipa Dec 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
src/ecmult_static_pre_g.h linguist-generated
src/ecmult_gen_static_prec_table.h linguist-generated
src/precomputed_ecmult.c linguist-generated
src/precomputed_ecmult_gen.c linguist-generated
4 changes: 2 additions & 2 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ bench_ecmult
bench_internal
tests
exhaustive_tests
gen_ecmult_gen_static_prec_table
gen_ecmult_static_pre_g
precompute_ecmult_gen
precompute_ecmult
valgrind_ctime_test
*.exe
*.so
Expand Down
55 changes: 32 additions & 23 deletions Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ noinst_HEADERS += src/eckey.h
noinst_HEADERS += src/eckey_impl.h
noinst_HEADERS += src/ecmult.h
noinst_HEADERS += src/ecmult_impl.h
noinst_HEADERS += src/ecmult_compute_table.h
noinst_HEADERS += src/ecmult_compute_table_impl.h
noinst_HEADERS += src/ecmult_const.h
noinst_HEADERS += src/ecmult_const_impl.h
noinst_HEADERS += src/ecmult_gen.h
noinst_HEADERS += src/ecmult_gen_impl.h
noinst_HEADERS += src/ecmult_gen_prec.h
noinst_HEADERS += src/ecmult_gen_prec_impl.h
noinst_HEADERS += src/ecmult_gen_compute_table.h
noinst_HEADERS += src/ecmult_gen_compute_table_impl.h
noinst_HEADERS += src/field_10x26.h
noinst_HEADERS += src/field_10x26_impl.h
noinst_HEADERS += src/field_5x52.h
Expand All @@ -42,6 +44,8 @@ noinst_HEADERS += src/modinv32.h
noinst_HEADERS += src/modinv32_impl.h
noinst_HEADERS += src/modinv64.h
noinst_HEADERS += src/modinv64_impl.h
noinst_HEADERS += src/precomputed_ecmult.h
noinst_HEADERS += src/precomputed_ecmult_gen.h
noinst_HEADERS += src/assumptions.h
noinst_HEADERS += src/util.h
noinst_HEADERS += src/scratch.h
Expand All @@ -60,12 +64,17 @@ noinst_HEADERS += contrib/lax_der_parsing.c
noinst_HEADERS += contrib/lax_der_privatekey_parsing.h
noinst_HEADERS += contrib/lax_der_privatekey_parsing.c

PRECOMPUTED_LIB = libsecp256k1_precomputed.la
noinst_LTLIBRARIES = $(PRECOMPUTED_LIB)
libsecp256k1_precomputed_la_SOURCES = src/precomputed_ecmult.c src/precomputed_ecmult_gen.c
libsecp256k1_precomputed_la_CPPFLAGS = $(SECP_INCLUDES)
Comment on lines +67 to +70
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hebasto This is because we now have a separate compilation unit for the precomputed tables. So Core needs to tell MSVC to build libsecp256k1_precomputed separately and link it to the library.

But I have no idea about the required changes to https://github.com/bitcoin/bitcoin/blob/master/build_msvc/libsecp256k1/libsecp256k1.vcxproj that would be equivalent to this automake snippet.

If this is anyway maintained within the Bitcoin Core org, maybe we should move the vcxproj file to this repo in the future. This will help us add MSVC builds to the CI here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sipsorcery

Could you look into this?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hebasto inlcuding the new precompute .c files in the libsecp25k1.vcxproj got the build working for me.

  <ItemGroup>
    <ClCompile Include="..\..\src\secp256k1\src\precomputed_ecmult.c" />
    <ClCompile Include="..\..\src\secp256k1\src\precomputed_ecmult_gen.c" />
    <ClCompile Include="..\..\src\secp256k1\src\precompute_ecmult.c" />
    <ClCompile Include="..\..\src\secp256k1\src\precompute_ecmult_gen.c" />
    <ClCompile Include="..\..\src\secp256k1\src\secp256k1.c" />
  </ItemGroup>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sipsorcery

Thanks! It works flawlessly.

Copy link
Contributor

@real-or-random real-or-random Feb 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sipsorcery That works but it's too much: You don't need the precompute_ files, the precomputed files should be enough.

Also, this will then recompile the precomputed_ files every time, which makes recompilation somewhat slower than what we have in our Makefile. (This was the reason why we introduced the libsecp256k1_precomputed as a separate object.) But yeah, this only affects compilation time, so it's probably not worth to optimize the vcxproj for this.

edit: Oh, now I see that @sipa already replied in bitcoin/bitcoin#23432 (comment).


if USE_EXTERNAL_ASM
COMMON_LIB = libsecp256k1_common.la
noinst_LTLIBRARIES = $(COMMON_LIB)
else
COMMON_LIB =
endif
noinst_LTLIBRARIES += $(COMMON_LIB)

pkgconfigdir = $(libdir)/pkgconfig
pkgconfig_DATA = libsecp256k1.pc
Expand All @@ -78,7 +87,7 @@ endif

libsecp256k1_la_SOURCES = src/secp256k1.c
libsecp256k1_la_CPPFLAGS = -I$(top_srcdir)/include -I$(top_srcdir)/src $(SECP_INCLUDES)
libsecp256k1_la_LIBADD = $(SECP_LIBS) $(COMMON_LIB)
libsecp256k1_la_LIBADD = $(SECP_LIBS) $(COMMON_LIB) $(PRECOMPUTED_LIB)
libsecp256k1_la_LDFLAGS = -no-undefined

if VALGRIND_ENABLED
Expand All @@ -91,10 +100,10 @@ noinst_PROGRAMS += bench bench_internal bench_ecmult
bench_SOURCES = src/bench.c
bench_LDADD = libsecp256k1.la $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB)
bench_internal_SOURCES = src/bench_internal.c
bench_internal_LDADD = $(SECP_LIBS) $(COMMON_LIB)
bench_internal_LDADD = $(SECP_LIBS) $(COMMON_LIB) $(PRECOMPUTED_LIB)
bench_internal_CPPFLAGS = $(SECP_INCLUDES)
bench_ecmult_SOURCES = src/bench_ecmult.c
bench_ecmult_LDADD = $(SECP_LIBS) $(COMMON_LIB)
bench_ecmult_LDADD = $(SECP_LIBS) $(COMMON_LIB) $(PRECOMPUTED_LIB)
bench_ecmult_CPPFLAGS = $(SECP_INCLUDES)
endif

Expand All @@ -112,7 +121,7 @@ endif
if !ENABLE_COVERAGE
tests_CPPFLAGS += -DVERIFY
endif
tests_LDADD = $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB)
tests_LDADD = $(SECP_LIBS) $(SECP_TEST_LIBS) $(COMMON_LIB) $(PRECOMPUTED_LIB)
tests_LDFLAGS = -static
TESTS += tests
endif
Expand All @@ -124,38 +133,38 @@ exhaustive_tests_CPPFLAGS = $(SECP_INCLUDES)
if !ENABLE_COVERAGE
exhaustive_tests_CPPFLAGS += -DVERIFY
endif
# Note: do not include $(PRECOMPUTED_LIB) in exhaustive_tests (it uses runtime-generated tables).
exhaustive_tests_LDADD = $(SECP_LIBS) $(COMMON_LIB)
exhaustive_tests_LDFLAGS = -static
TESTS += exhaustive_tests
endif

### Precomputed tables
EXTRA_PROGRAMS = gen_ecmult_static_pre_g gen_ecmult_gen_static_prec_table
EXTRA_PROGRAMS = precompute_ecmult precompute_ecmult_gen
CLEANFILES = $(EXTRA_PROGRAMS)

gen_ecmult_static_pre_g_SOURCES = src/gen_ecmult_static_pre_g.c
gen_ecmult_static_pre_g_CPPFLAGS = $(SECP_INCLUDES)
gen_ecmult_static_pre_g_LDADD = $(SECP_LIBS) $(COMMON_LIB)
precompute_ecmult_SOURCES = src/precompute_ecmult.c
precompute_ecmult_CPPFLAGS = $(SECP_INCLUDES)
precompute_ecmult_LDADD = $(SECP_LIBS) $(COMMON_LIB)

gen_ecmult_gen_static_prec_table_SOURCES = src/gen_ecmult_gen_static_prec_table.c
gen_ecmult_gen_static_prec_table_CPPFLAGS = $(SECP_INCLUDES)
gen_ecmult_gen_static_prec_table_LDADD = $(SECP_LIBS) $(COMMON_LIB)
precompute_ecmult_gen_SOURCES = src/precompute_ecmult_gen.c
precompute_ecmult_gen_CPPFLAGS = $(SECP_INCLUDES)
precompute_ecmult_gen_LDADD = $(SECP_LIBS) $(COMMON_LIB)

# See Automake manual, Section "Errors with distclean".
# We don't list any dependencies for the prebuilt files here because
# otherwise make's decision whether to rebuild them (even in the first
# build by a normal user) depends on mtimes, and thus is very fragile.
# This means that rebuilds of the prebuilt files always need to be
# forced by deleting them, e.g., by invoking `make clean-precomp`.
src/ecmult_static_pre_g.h:
$(MAKE) $(AM_MAKEFLAGS) gen_ecmult_static_pre_g$(EXEEXT)
./gen_ecmult_static_pre_g$(EXEEXT)
src/ecmult_gen_static_prec_table.h:
$(MAKE) $(AM_MAKEFLAGS) gen_ecmult_gen_static_prec_table$(EXEEXT)
./gen_ecmult_gen_static_prec_table$(EXEEXT)

PRECOMP = src/ecmult_gen_static_prec_table.h src/ecmult_static_pre_g.h
noinst_HEADERS += $(PRECOMP)
src/precomputed_ecmult.c:
$(MAKE) $(AM_MAKEFLAGS) precompute_ecmult$(EXEEXT)
./precompute_ecmult$(EXEEXT)
src/precomputed_ecmult_gen.c:
$(MAKE) $(AM_MAKEFLAGS) precompute_ecmult_gen$(EXEEXT)
./precompute_ecmult_gen$(EXEEXT)

PRECOMP = src/precomputed_ecmult_gen.c src/precomputed_ecmult.c
precomp: $(PRECOMP)

# Ensure the prebuilt files will be build first (only if they don't exist,
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ AC_ARG_WITH([ecmult-window], [AS_HELP_STRING([--with-ecmult-window=SIZE|auto],
[window size for ecmult precomputation for verification, specified as integer in range [2..24].]
[Larger values result in possibly better performance at the cost of an exponentially larger precomputed table.]
[The table will store 2^(SIZE-1) * 64 bytes of data but can be larger in memory due to platform-specific padding and alignment.]
[A window size larger than 15 will require you delete the prebuilt ecmult_static_pre_g.h file so that it can be rebuilt.]
[A window size larger than 15 will require you delete the prebuilt precomputed_ecmult.c file so that it can be rebuilt.]
[For very large window sizes, use "make -j 1" to reduce memory use during compilation.]
["auto" is a reasonable setting for desktop machines (currently 15). [default=auto]]
)],
Expand Down
16 changes: 16 additions & 0 deletions src/ecmult_compute_table.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*****************************************************************************************************
* Copyright (c) 2013, 2014, 2017, 2021 Pieter Wuille, Andrew Poelstra, Jonas Nick, Russell O'Connor *
* Distributed under the MIT software license, see the accompanying *
* file COPYING or https://www.opensource.org/licenses/mit-license.php. *
*****************************************************************************************************/

#ifndef SECP256K1_ECMULT_COMPUTE_TABLE_H
#define SECP256K1_ECMULT_COMPUTE_TABLE_H

/* Construct table of all odd multiples of gen in range 1..(2**(window_g-1)-1). */
static void secp256k1_ecmult_compute_table(secp256k1_ge_storage* table, int window_g, const secp256k1_gej* gen);

/* Like secp256k1_ecmult_compute_table, but one for both gen and gen*2^128. */
static void secp256k1_ecmult_compute_two_tables(secp256k1_ge_storage* table, secp256k1_ge_storage* table_128, int window_g, const secp256k1_ge* gen);

#endif /* SECP256K1_ECMULT_COMPUTE_TABLE_H */
49 changes: 49 additions & 0 deletions src/ecmult_compute_table_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*****************************************************************************************************
* Copyright (c) 2013, 2014, 2017, 2021 Pieter Wuille, Andrew Poelstra, Jonas Nick, Russell O'Connor *
* Distributed under the MIT software license, see the accompanying *
* file COPYING or https://www.opensource.org/licenses/mit-license.php. *
*****************************************************************************************************/

#ifndef SECP256K1_ECMULT_COMPUTE_TABLE_IMPL_H
#define SECP256K1_ECMULT_COMPUTE_TABLE_IMPL_H

#include "ecmult_compute_table.h"
#include "group_impl.h"
#include "field_impl.h"
#include "ecmult.h"
#include "util.h"

static void secp256k1_ecmult_compute_table(secp256k1_ge_storage* table, int window_g, const secp256k1_gej* gen) {
secp256k1_gej gj;
secp256k1_ge ge, dgen;
int j;

gj = *gen;
secp256k1_ge_set_gej_var(&ge, &gj);
secp256k1_ge_to_storage(&table[0], &ge);

secp256k1_gej_double_var(&gj, gen, NULL);
secp256k1_ge_set_gej_var(&dgen, &gj);

for (j = 1; j < ECMULT_TABLE_SIZE(window_g); ++j) {
secp256k1_gej_set_ge(&gj, &ge);
secp256k1_gej_add_ge_var(&gj, &gj, &dgen, NULL);
secp256k1_ge_set_gej_var(&ge, &gj);
secp256k1_ge_to_storage(&table[j], &ge);
}
}

/* Like secp256k1_ecmult_compute_table, but one for both gen and gen*2^128. */
static void secp256k1_ecmult_compute_two_tables(secp256k1_ge_storage* table, secp256k1_ge_storage* table_128, int window_g, const secp256k1_ge* gen) {
secp256k1_gej gj;
int i;

secp256k1_gej_set_ge(&gj, gen);
secp256k1_ecmult_compute_table(table, window_g, &gj);
for (i = 0; i < 128; ++i) {
secp256k1_gej_double_var(&gj, &gj, NULL);
}
secp256k1_ecmult_compute_table(table_128, window_g, &gj);
}

#endif /* SECP256K1_ECMULT_COMPUTE_TABLE_IMPL_H */
8 changes: 4 additions & 4 deletions src/ecmult_gen_prec.h → src/ecmult_gen_compute_table.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
* file COPYING or https://www.opensource.org/licenses/mit-license.php.*
***********************************************************************/

#ifndef SECP256K1_ECMULT_GEN_PREC_H
#define SECP256K1_ECMULT_GEN_PREC_H
#ifndef SECP256K1_ECMULT_GEN_COMPUTE_TABLE_H
#define SECP256K1_ECMULT_GEN_COMPUTE_TABLE_H

#include "ecmult_gen.h"

static void secp256k1_ecmult_gen_create_prec_table(secp256k1_ge_storage* table, const secp256k1_ge* gen, int bits);
static void secp256k1_ecmult_gen_compute_table(secp256k1_ge_storage* table, const secp256k1_ge* gen, int bits);

#endif /* SECP256K1_ECMULT_GEN_PREC_H */
#endif /* SECP256K1_ECMULT_GEN_COMPUTE_TABLE_H */
10 changes: 5 additions & 5 deletions src/ecmult_gen_prec_impl.h → src/ecmult_gen_compute_table_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
* file COPYING or https://www.opensource.org/licenses/mit-license.php.*
***********************************************************************/

#ifndef SECP256K1_ECMULT_GEN_PREC_IMPL_H
#define SECP256K1_ECMULT_GEN_PREC_IMPL_H
#ifndef SECP256K1_ECMULT_GEN_COMPUTE_TABLE_IMPL_H
#define SECP256K1_ECMULT_GEN_COMPUTE_TABLE_IMPL_H

#include "ecmult_gen_prec.h"
#include "ecmult_gen_compute_table.h"
#include "group_impl.h"
#include "field_impl.h"
#include "ecmult_gen.h"
#include "util.h"

static void secp256k1_ecmult_gen_create_prec_table(secp256k1_ge_storage* table, const secp256k1_ge* gen, int bits) {
static void secp256k1_ecmult_gen_compute_table(secp256k1_ge_storage* table, const secp256k1_ge* gen, int bits) {
int g = ECMULT_GEN_PREC_G(bits);
int n = ECMULT_GEN_PREC_N(bits);

Expand Down Expand Up @@ -78,4 +78,4 @@ static void secp256k1_ecmult_gen_create_prec_table(secp256k1_ge_storage* table,
free(prec);
}

#endif /* SECP256K1_ECMULT_GEN_PREC_IMPL_H */
#endif /* SECP256K1_ECMULT_GEN_COMPUTE_TABLE_IMPL_H */
2 changes: 1 addition & 1 deletion src/ecmult_gen_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#include "group.h"
#include "ecmult_gen.h"
#include "hash_impl.h"
#include "ecmult_gen_static_prec_table.h"
#include "precomputed_ecmult_gen.h"

static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context *ctx) {
secp256k1_ecmult_gen_blind(ctx, NULL);
Expand Down
4 changes: 2 additions & 2 deletions src/ecmult_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include "group.h"
#include "scalar.h"
#include "ecmult.h"
#include "ecmult_static_pre_g.h"
#include "precomputed_ecmult.h"

#if defined(EXHAUSTIVE_TEST_ORDER)
/* We need to lower these values for exhaustive tests because
Expand Down Expand Up @@ -103,7 +103,7 @@ static void secp256k1_ecmult_odd_multiples_table(int n, secp256k1_gej *prej, sec
* It only operates on tables sized for WINDOW_A wnaf multiples.
*
* To compute a*P + b*G, we compute a table for P using this function,
* and use the precomputed table in <ecmult_static_pre_g.h> for G.
* and use the precomputed table in <precomputed_ecmult.c> for G.
*/
static void secp256k1_ecmult_odd_multiples_table_globalz_windowa(secp256k1_ge *pre, secp256k1_fe *globalz, const secp256k1_gej *a) {
secp256k1_gej prej[ECMULT_TABLE_SIZE(WINDOW_A)];
Expand Down
Loading