ElementsProject / lightning

Core Lightning — Lightning Network implementation focusing on spec compliance and performance
Other
2.81k stars 889 forks source link

linker missing libsecp256k1 #4283

Closed jsarenik closed 3 years ago

jsarenik commented 3 years ago

Issue and Steps to Reproduce

Here we are back to #4229 (and the whole tree mentioned in there).

This whole issue is again caused by outdated git submodules.

Oroginally I wrote following:

When compiling the latest master I get following on systems which do not have the current libsecp256k1 installed in /usr/local, but rather have merely the (rather old) Ubuntu package (libsecp256k1-dev 0.1~20170810-2).

~/src/lightning$ git describe
v0.9.2-142-g77478408f
~/src/lightning$ ./configure
...
~/src/lightning$ make
...
/usr/bin/ld: common/node_id.o: in function `pubkey32_from_node_id':
/home/nisim/src/lightning/common/node_id.c:34: undefined reference to `secp256k1_xonly_pubkey_from_pubkey'
/usr/bin/ld: bitcoin/pubkey.o: in function `fromwire_pubkey32':
/home/nisim/src/lightning/bitcoin/pubkey.c:136: undefined reference to `secp256k1_xonly_pubkey_parse'
/usr/bin/ld: bitcoin/pubkey.o: in function `towire_pubkey32':
/home/nisim/src/lightning/bitcoin/pubkey.c:148: undefined reference to `secp256k1_xonly_pubkey_serialize'
/usr/bin/ld: bitcoin/pubkey.o: in function `pubkey32_to_hexstr':
/home/nisim/src/lightning/bitcoin/pubkey.c:157: undefined reference to `secp256k1_xonly_pubkey_serialize'
collect2: error: ld returned 1 exit status
make: *** [Makefile:531: lightningd/lightning_hsmd] Error 1

Would it be possible to statically link against any of the secp256k1 which are around?

~/src/lightning$ find . -type d -name '*secp256k1*'
./external/x86_64-linux-gnu/libwally-core-build/src/secp256k1
./external/libwally-core/src/secp256k1
./.git/modules/external/libwally-core/modules/src/secp256k1
jsarenik commented 3 years ago

Oops, it is actually a bit worse than I thought. Even on the system with current libsecp there is the same error now.

jsarenik commented 3 years ago

Caused by outdated submodules. That reminds me #4229 is not fixed yet. https://github.com/ElementsProject/lightning/pull/4234 did not help.

jsarenik commented 3 years ago

Seems to me that others are not experiencing this issue much. Please point out anything I may be doing wrong. Happy to learn.

cdecker commented 3 years ago

The submodule not being present is a bit of a headache for sure. While setting up cirrus-ci I found that the following works reproducibly:

 git submodule init
git submodule update
(cd external/libwally-core/src/; git submodule init; git submodule update)

It would be really cool if we could just git clone --recursive but the submodules are annoying enough for me to consider just copying the submodules in-tree and avoid submodules altogether.

ZmnSCPxj commented 3 years ago

avoid submodules altogether.

Yes, git submodule is fairly unreliable in my experience as well, which is why I just copy dependencies in-tree.

cdecker commented 3 years ago

After getting hit by this a couple of times I think I found one option that seems to be working reliably:

git submodule update --init --recursive

This does pretty much exactly what we want. Funnily enough make submodcheck which uses tools/refresh-submodules.sh actually makes use of this, but does a bit more.

The problem however is that the script doesn't just call the command above, but gates it with a git submodule status on the submodule path, which doesn't seem to consider any sub-submodules in that submodule and therefore wrongly thinks all is good when really we're missing the sub-submodule for secp256k1.

IMHO we should just use the command above in the Makefile and call it a day.

jsarenik commented 3 years ago

:+1: Yes. Thank you @cdecker for having a look!

jsarenik commented 3 years ago

@cdecker I have a current issue with dc38e65ed. Try to start with a fresh clone of lightning and make sure there is no secp256k1_recovery.h in /usr/include or /usr/local/include. After running ./configure, do a parallel make: make -j4 and you should see this:

In file included from ./common/iso4217.h:4,
                 from common/iso4217.c:3:
./wire/wire.h:7:10: fatal error: secp256k1_recovery.h: No such file or directory
    7 | #include <secp256k1_recovery.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

It does not happen when make is run again, meaning when the submodules are already initialized. Because the compiler looks for include files in external/libwally-core/src/secp256k1/include but during a first-run parallel make it is not initialized yet… Does anyone use make -jX where X is more than 1?

jsarenik commented 3 years ago

@cdecker Please have a look at https://git-scm.com/book/en/v2/Git-Tools-Submodules which says this:

If you already cloned the project and forgot --recurse-submodules, you can combine the git submodule init and git submodule update steps by running git submodule update --init. To also initialize, fetch and checkout any nested submodules, you can use the foolproof git submodule update --init --recursive.

jsarenik commented 3 years ago

I verified that either of above really solves all the issues I face when compiling with ./configure; make -j4 in a clean lightning repository (without recursively initializing the submodules first).

fiatjaf commented 3 years ago
git submodule update --init --recursive

Didn't work for me.

The only thing that worked was moving to inside each submodule and submodule of submodule and doing git pull origin master there.

jsarenik commented 3 years ago

@fiatjaf Does #4406 work for you?

jsarenik commented 3 years ago

It is already merged now, but I am wondering if it helps you, given that git submodule update --init --recursive did not.

Ah, did you @fiatjaf do git submodule deinit --force --all before doing the above-mentioned?

fiatjaf commented 3 years ago

No, I didn't do the deinit thing. Git continues to surprise us with weird commands, but I guess it would make sense.

I'll try to remember this next time I'm in trouble.

jsarenik commented 3 years ago

@fiatjaf I am wondering if the change #4406 which is already in master helps you so that nono of the magic commands is needed and things just work. Thanks for feedback!