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.

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

Allow testing on linux using a custom TEST_LIBSECP256K1_PATH #70

Merged

Conversation

bterkuile
Copy link
Contributor

When running the specs on an Apple M1 machine running Fedora linux there were some problems using the secp256k1 library. The changes of this request should allow running all the specs on these (and other) type of linux machines.

@@ -112,6 +112,10 @@ Therefore, some tests require this library. In a Linux environment, `spec/lib/li
so there is no need to do anything. If you want to test in another environment,
please set the library path in the environment variable `TEST_LIBSECP256K1_PATH`.

In case the supplied linux `spec/lib/libsecp256k1.so` is not working (architecture), you might have to compile it yourself.
Since if available in the repository, it might not be compiled using the `./configure --enable-module-recovery` option.
Copy link
Contributor

@azuchi azuchi Jun 19, 2024

Choose a reason for hiding this comment

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

Library in this repository support --enable-module-recovery. Basically, all functions attached by the following methods are enabled.

https://github.com/chaintope/bitcoinrb/blob/master/lib/bitcoin/secp256k1/native.rb#L40-L67

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that the included library in the specs is built in an x86-64 architecture. I'm using an Apple M1 having a Fedora linux (ARM). It all runs, which is nice. The library is even is available through package management:

sudo dnf install libsecp256k1

But linking to this install gives an error on the secp256k1_ecdsa_sign_recoverable attachment. Therefor compiling using de recorery flag was my solution to make all tests pass. Probable there is a better solution, but I did not find it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, repository means package manager, not this github repository. In that case, --enable-module-ellswift may not be used either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To get the tests to run on my local machine was more trial and error than knowledge. The supplied library placed in: spec/lib/libsecp256k1.so crashes on the ffi_lib(ENV['SECP256K1_LIB_PATH']) statement in: lib/bitcoin/secp256k1/native.rb.

So I decided to build it myself. Therefore I used the README.md from bitcoin-core/secp256k1 that is nice and clear to build. Although finding the libs in a hidden .libs folder was a bit of a surprise.

Without configuration flags the compiled libsecp256k1 gives the following error while loading native:
Function 'secp256k1_ecdsa_sign_recoverable' not found

It took me a while to figure out that this could be resolved by compiling using a different configuration. So also put a note in the README.md to help others find the solution faster if they run into this problem.

What the required/recommended configure flags are I don't know. This worked for me to get all the tests green. So a correct statement from an expert on how to compile would be great. I'll post a partial output from the default at the moment below.

Build Options:
  with external callbacks = no
  with benchmarks         = yes
  with tests              = yes
  with ctime tests        = no
  with coverage           = no
  with examples           = no
  module ecdh             = yes
  module recovery         = no
  module extrakeys        = yes
  module schnorrsig       = yes
  module ellswift         = yes

  asm                     = no
  ecmult window size      = 15
  ecmult gen table size   = 22 KiB

  valgrind                = no
  CC                      = gcc
  CPPFLAGS                =

Copy link
Contributor

Choose a reason for hiding this comment

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

I was mistaken and ellswift was also enabled. thanks!

@azuchi azuchi merged commit 570b87a into chaintope:master Jun 20, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants