Skip to content

Commit

Permalink
clean up linkage of libsecp256k1 into libwallycore and Python extension
Browse files Browse the repository at this point in the history
We have two levels of library embedding:

 * libsecp256k1.la is linked into libwallycore.la via LIBADD. The FIXME
   comment in configure.ac contained an assertion that seems to be
   irrelevant: passing libsecp256k1.la to libtool when linking
   libwallycore.la indeed does not embed the objects from libsecp256k1.a
   into libwallycore.a, but it does add a dependency on libsecp256k1.la
   in libwallycore.la, so any future links against libwallycore.la will
   automagically pull in libsecp256k1.la. This is exactly the scenario
   that libtool was conceived to facilitate. Do note one caveat: if both
   --enable-shared and --enable-static are passed to configure, then
   libwallycore.la will *not* specify a dependency on libsecp256k1.la,
   but this is irrelevant since any future links against libwallycore.la
   will link against libwallycore.so, which *does* contain all of the
   symbols from libsecp256k1.a, so there is no additional dependency.

 * libwallycore.la is linked into the Python native extension library.
   Previously this wasn't really being done at all; rather, all of the
   libwally-core code was being recompiled into some "combined" objects
   that would then be linked into the Python extension. This was a
   needless duplication of work since the compiled objects already exist
   in libwallycore.a and libsecp256k1.a. Simply link those libraries
   into the extension. Note that libwally-core does need to be compiled
   as PIC for this to work, so setup.py passes --with-pic to configure.

Additional cleanup: adjust several #include directives to reference
libsecp256k1 headers in the configured header search path rather than
explicitly specifying paths to the instances beneath src/secp256k1/.
  • Loading branch information
whitslack committed Sep 2, 2023
1 parent 26d068d commit bdc36c6
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 116 deletions.
24 changes: 4 additions & 20 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -259,26 +259,10 @@ fi
#
# libsecp256k1
#
# FIXME: This is needed to force libtool to use all object files from secp.
# We can only build secp properly by recursively invoking
# configure/make, and can't include it as a noinst_ library. Libtool
# assumes that such libraries will be installed along with our library
# target and so won't force all object files in the library to be
# included in ours - despite the fact that we are making a shared
# library and linking to a static one. This is broken and we work
# around it by hacking the secp objects directly into the library
# via the _LDADD variable for wallycore.
# We previously achieved this by adding the libsecp256k1.a archive,
# but changes to libtool and apples linkers mean that
# archives-within-archives no longer work.
# Because automake tries to police its users very strictly and fails
# hard when flags are passed in this way, we have to substitute the
# flags here.
# Because libtool both intercepts -Wl and arbitrarily re-orders its
# command line inputs, we have to concoct a single expression to
# enforce linking that cannot be split, hence the below expression.
LIBADD_SECP256K1="-Wl,secp256k1/src/libsecp256k1_la-secp256k1.${OBJEXT},secp256k1/src/libsecp256k1_precomputed_la-precomputed_ecmult_gen.${OBJEXT},secp256k1/src/libsecp256k1_precomputed_la-precomputed_ecmult.${OBJEXT}"
AC_SUBST([LIBADD_SECP256K1])
libsecp256k1_CFLAGS='-I$(top_srcdir)/src/secp256k1/include'
libsecp256k1_LIBS='$(top_srcdir)/src/secp256k1/libsecp256k1.la'
AC_SUBST([libsecp256k1_CFLAGS])
AC_SUBST([libsecp256k1_LIBS])

#
# Python facilities
Expand Down
10 changes: 4 additions & 6 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,9 @@ def call(args):
'./',
'./src',
'./include',
'./src/ccan',
'./src/secp256k1',
'./src/secp256k1/src/'
'./src/secp256k1/include',
]

if is_windows:
shutil.copyfile('./src/amalgamation/windows_config/libsecp256k1-config.h', 'src/secp256k1/src/libsecp256k1-config.h')
include_dirs = ['./src/amalgamation/windows_config'] + include_dirs
Expand All @@ -83,12 +82,11 @@ def call(args):
'_wallycore',
define_macros=define_macros,
include_dirs=include_dirs,
library_dirs=['src/.libs', 'src/secp256k1/.libs'],
libraries=['wallycore', 'secp256k1'],
extra_compile_args=extra_compile_args,
sources=[
'src/swig_python/swig_wrap.c' if is_windows else 'src/swig_python/swig_python_wrap.c',
'src/amalgamation/combined.c',
'src/amalgamation/combined_ccan.c',
'src/amalgamation/combined_ccan2.c',
],
)

Expand Down
8 changes: 4 additions & 4 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ if USE_SWIG_PYTHON
noinst_LTLIBRARIES += libswig_python.la
libswig_python_la_SOURCES = swig_python/swig_python_wrap.c

libswig_python_la_CFLAGS = -I$(top_srcdir) $(AM_CFLAGS) $(SWIG_PYTHON_CPPFLAGS) $(SWIG_WARN_CFLAGS) $(NOALIAS_CFLAGS)
libswig_python_la_CFLAGS = -I$(top_srcdir) $(libsecp256k1_CFLAGS) $(AM_CFLAGS) $(SWIG_PYTHON_CPPFLAGS) $(SWIG_WARN_CFLAGS) $(NOALIAS_CFLAGS)
if PYTHON_MANYLINUX
else
libswig_python_la_LIBADD = $(PYTHON_LIBS)
Expand Down Expand Up @@ -83,7 +83,7 @@ noinst_LTLIBRARIES += libswig_java.la
libswig_java_la_SOURCES = \
swig_java/swig_java_wrap.c

libswig_java_la_CFLAGS = -I$(top_srcdir) $(AM_CFLAGS) $(SWIG_JAVA_CPPFLAGS) $(SWIG_WARN_CFLAGS)
libswig_java_la_CFLAGS = -I$(top_srcdir) $(libsecp256k1_CFLAGS) $(AM_CFLAGS) $(SWIG_JAVA_CPPFLAGS) $(SWIG_WARN_CFLAGS)

SWIG_JOPT = $(SWIG_JAVA_OPT) -outdir swig_java -noproxy -package com.blockstream.libwally

Expand Down Expand Up @@ -205,8 +205,8 @@ libwallycore_la_LDFLAGS += -no-undefined
endif
endif # SHARED_BUILD_ENABLED

libwallycore_la_CFLAGS = -I$(top_srcdir) -I$(srcdir)/ccan -DWALLY_CORE_BUILD=1 $(AM_CFLAGS)
libwallycore_la_LIBADD = $(LIBADD_SECP256K1) $(noinst_LTLIBRARIES)
libwallycore_la_CFLAGS = -I$(top_srcdir) -I$(srcdir)/ccan $(libsecp256k1_CFLAGS) -DWALLY_CORE_BUILD=1 $(AM_CFLAGS)
libwallycore_la_LIBADD = $(libsecp256k1_LIBS) $(noinst_LTLIBRARIES)

SUBDIRS = secp256k1

Expand Down
68 changes: 0 additions & 68 deletions src/amalgamation/combined.c

This file was deleted.

4 changes: 0 additions & 4 deletions src/amalgamation/combined_ccan.c

This file was deleted.

3 changes: 0 additions & 3 deletions src/amalgamation/combined_ccan2.c

This file was deleted.

4 changes: 2 additions & 2 deletions src/ecdh.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "internal.h"
#include <include/wally_crypto.h>
#include "secp256k1/include/secp256k1.h"
#include "secp256k1/include/secp256k1_ecdh.h"
#include <secp256k1.h>
#include <secp256k1_ecdh.h>

int wally_ecdh(const unsigned char *pub_key, size_t pub_key_len,
const unsigned char *priv_key, size_t priv_key_len,
Expand Down
8 changes: 4 additions & 4 deletions src/elements.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
#include <include/wally_crypto.h>
#include <include/wally_symmetric.h>
#include <include/wally_transaction.h>
#include "secp256k1/include/secp256k1_generator.h"
#include "secp256k1/include/secp256k1_rangeproof.h"
#include "src/secp256k1/include/secp256k1_surjectionproof.h"
#include "src/secp256k1/include/secp256k1_whitelist.h"
#include <secp256k1_generator.h>
#include <secp256k1_rangeproof.h>
#include <secp256k1_surjectionproof.h>
#include <secp256k1_whitelist.h>


static const unsigned char LABEL_STR[] = {
Expand Down
8 changes: 4 additions & 4 deletions src/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
#pragma GCC diagnostic ignored "-Wunused-parameter"
#endif /* BUILD_ELEMENTS */
#endif /* WALLY_ABI_NO_ELEMENTS */
#include "secp256k1/include/secp256k1.h"
#include "secp256k1/include/secp256k1_recovery.h"
#include "secp256k1/include/secp256k1_extrakeys.h"
#include <secp256k1.h>
#include <secp256k1_recovery.h>
#include <secp256k1_extrakeys.h>
#ifndef BUILD_STANDARD_SECP
#include "secp256k1/include/secp256k1_ecdsa_s2c.h"
#include <secp256k1_ecdsa_s2c.h>
#endif
#include <config.h>
#if defined(HAVE_MEMSET_S)
Expand Down
2 changes: 1 addition & 1 deletion src/sign.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "internal.h"
#include <include/wally_crypto.h>
#include "script_int.h"
#include "secp256k1/include/secp256k1_schnorrsig.h"
#include <secp256k1_schnorrsig.h>
#include "ccan/ccan/build_assert/build_assert.h"

#define EC_FLAGS_TYPES (EC_FLAG_ECDSA | EC_FLAG_SCHNORR)
Expand Down

0 comments on commit bdc36c6

Please sign in to comment.