search

haskoin / secp256k1-haskell

Haskell bindings for secp256k1 library
Other
24 stars 36 forks source link

Bip340 support #40

Closed GambolingPangolin closed 2 months ago

GambolingPangolin commented 2 years ago

This change adds bindings to the schnorr signing functionality in libsecp256k1, but behind a flag. The versions of libsecp256k1 that ship with popular linux distros do not have this functionality enabled. However, nix users can add the following shell.nix to explore the changes:

let nixpkgs = import <nixpkgs> {};
in

with nixpkgs.pkgs; mkShell {
  buildInputs = [
    cabal-install
    ghc
    gmp
    pkg-config
    secp256k1
  ];
}

As long as they are using a recent version of nixpkgs.

ysangkok commented 2 years ago

I previously added this in #26 but it was removed in 0f862c2198d3ae47ad9028947f7507321079ab98. But maybe things have changed, I definitely think the library should include this. I'd encourage you to fork the library on Hackage if BIP-340 is still deemed "non-standard".

GambolingPangolin commented 2 years ago

There has been some discussion about whether to include this now that taproot is active over in haskoin-core and I think we reached consensus that we should include it. It looks like your patch is a bit more comprehensive, so I'm gonna try resurrecting it and changing my patch for taproot signing to use your API.

ysangkok commented 2 years ago

Note that my patch was for a pre-release version of secp256k1. It might still be resurrectable, but I wanted to mention that.

ysangkok commented 5 months ago

@jprupp Can this be merged now that Ubuntu 24.04 is out and all target distributions have schnorr support?

jprupp commented 4 months ago

Resolve conflicts, and then I'll review with intention to merge.

GambolingPangolin commented 3 months ago

Resolve conflicts, and then I'll review with intention to merge.

I resolved the conflicts and tried to make the code as consistent as possible with the existing bindings.

ysangkok commented 2 months ago

@jprupp The conflicts were resolved as you requested, would it be possible for you to review this?

jprupp commented 2 months ago

I have to revert this. Testing in Fedora 40 fails because the symbols that these changes depend on are not available in the secp256k1 library available in that platform.