Skip to content

Conversation

@alvroble
Copy link

Overview

This branch represents a significant effort to update embit to work with the latest secp256k1 library (v0.7.0) and modernize the bindings implementation. This is not intended for direct merge but serves as a foundation for identifying and resolving compatibility issues between embit and the current secp256k1 implementation.

While this is still early work-in-progress, this branch is made public due to the critical importance of secp256k1 functionality for embit's integration with SeedSigner and (my guess) other downstream projects. The secp256k1 library is fundamental to Bitcoin cryptographic operations, making this update essential for maintaining compatibility with latest cryptographic standards.

Upon successful completion of this development, new precompiled secp256k1 v0.7.0 binaries will be generated and uploaded to the codebase.

Related: #97, SeedSigner/seedsigner-os#90

Objectives

  • Update from legacy secp256k1 to v0.7.0
  • Identify test failures causes:
    • Bindings implementation issues
    • Behavioral changes in secp256k1 v0.7.0 (Schnorr signatures)
  • Establish a controlled environment for building secp256k1 from source and running tests

Changes summary

Temporary Docker environment for source building

  • Added temporary Docker setup to build secp256k1 v0.7.0 from source
  • Note: The Docker environment, test scripts, and related files (Dockerfile, run_tests.sh, temporary test files) are not intended for incorporation into the main codebase. They serve as temporary development tools to facilitate testing and debugging of the secp256k1 v0.7.0 integration.

embit library updates

  • Eliminated outdated prebuilt libraries to force source building
  • secp256k1 v0.7.0 integration - Updated to latest stable release
  • Bindings modernization - Updated ctypes bindings

Key technical changes

Updated Schnorr signature bindings

secp256k1.secp256k1_schnorrsig_sign_custom.argtypes = [
    c_void_p,  # ctx
    c_char_p,  # sig  
    c_char_p,  # msg
    c_size_t,  # msglen
    c_char_p,  # keypair
    POINTER(secp256k1_schnorrsig_extraparams),  # extraparams
]

New structure for extra params

class secp256k1_schnorrsig_extraparams(Structure):
    _fields_ = [
        ("magic", c_char * 4),
        ("noncefp", c_void_p),  # nonce function pointer
        ("ndata", c_void_p),
    ]

Function signature updates

  • schnorrsig_sign() now uses secp256k1_schnorrsig_sign_custom as default
  • schnorrsig_sign_32() maintains compatibility with secp256k1_schnorrsig_sign32
  • Updated sign_schnorr in key.py for compatibility with new bindings (matching BIP340's reference)
  • Other bindings updated names, minor changes with localized impact

Current status

Test failures

Several test cases are currently failing:

FAILED tests/tests/test_bindings.py::BindingsTest::test_schnorr
FAILED tests/tests/test_liquid.py::LiquidTest::test_blind_unblind 
FAILED tests/tests/test_liquid.py::LiquidTest::test_rangeproof 
FAILED tests/tests/test_liquid.py::LiquidTest::test_value_commitment 
FAILED tests/tests/test_taproot.py::TaprootTest::test_sign_internal 
FAILED tests/tests/test_taproot.py::TaprootTest::test_sign_taptree 
FAILED tests/tests/test_taproot.py::TaprootTest::test_sign_verify 
FAILED tests/tests/test_threading.py::ThreadingTest::test_threading 

Currently focusing on...

  1. Schnorr signature behavior: Determine if failures are due to:

    • Changes in secp256k1 v0.7.0 signature generation since the currently-used prebuilt version.
    • Updated nonce generation algorithms
    • Modified auxiliary randomness handling
  2. Bindings: Verify that:

    • All function signatures match secp256k1 v0.7.0 API
    • Error handling aligns with library changes
  3. py_secp256k1 / key.py: Ensure compatibility with:

    • Updated Python wrapper expectations
    • Changed return value formats
    • Modified parameter validation

Testing this PR

I leave here how to create the setup for testing this PR, if anybody needs it.

Docker Environment

# Build docker image
docker build -t embit-test .

# Create and run the container
docker run -it -v $(pwd):/app --entrypoint /bin/bash embit-test

# Run run_tests.sh after setting the env variables
# Or just run the commands inside the script
SEEDSIGNER=1 
BUILD_SECP256K1_FROM_SOURCE=1 
./run_tests.sh

# Subsequent tests don't need the whole building process
cd /app & pytest -v

Detailed Commit History

a00a233 - Update test environment scripts and cleanup

  • Cleaned up test environment scripts in run_tests.sh
  • Removed temporary test files (test_secp256k1.py, test_secp_comprehensive.py)
  • Streamlined testing workflow

6627e17 - Update schnorrsig_sign_custom bindings to be the default

  • Modified src/embit/util/ctypes_secp256k1.py to use secp256k1_schnorrsig_sign_custom as the primary signing function
  • Added support for extended parameters structure
  • Maintained backward compatibility with schnorrsig_sign_32

27b4e4e - Update Dockerfile

  • Improved Docker configuration for better secp256k1 building
  • Enhanced container setup for testing environment

0e4f26c - secp256k1 v0.7.0

  • Updated target secp256k1 version to v0.7.0 in build scripts
  • This is the core version bump that triggers all compatibility requirements

317a469 - Update test setup with seedsigner

  • Enhanced run_tests.sh with SeedSigner integration capabilities
  • Added conditional building and testing with SeedSigner project
  • Improved real-world testing scenarios

823d1d5 - Adapt bindings and key.py

  • Updated src/embit/util/ctypes_secp256k1.py with new function signatures
  • Modified src/embit/util/key.py for compatibility
  • Core bindings adaptation for v0.7.0 API changes

103a1d7 - secp256k1 test environment

  • Added comprehensive Docker-based testing environment
  • Created Dockerfile for consistent testing across platforms
  • Added initial test scripts (test_secp256k1.py, test_secp_comprehensive.py)
  • Established run_tests.sh for automated testing

430c243 - Remove prebuilt

  • Removed all prebuilt secp256k1 binaries across platforms:
    • libsecp256k1_darwin_arm64.dylib
    • libsecp256k1_darwin_x86_64.dylib
    • libsecp256k1_linux_aarch64.so
    • libsecp256k1_linux_armv6l.so
    • libsecp256k1_linux_armv7l.so
    • libsecp256k1_linux_x86_64.so
    • libsecp256k1_windows_amd64.dll
  • Forces building from source to ensure v0.7.0 compatibility

Copy link

@qlrd qlrd left a comment

Choose a reason for hiding this comment

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

Maybe both seedsigner and krux could benefit from thi, building both bitcoind, elementsd and secp256k1 from source, wdyt @odudex, @jdlcdl?

Comment on lines +1 to +5
#!/bin/sh
set -e

if [ "$BUILD_SECP256K1_FROM_SOURCE" = "1" ]; then
echo "Cloning and building secp256k1 from https://github.com/bitcoin-core/secp256k1 ..."
Copy link

Choose a reason for hiding this comment

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

I think we could join your approach with the one in #98

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