ZenGo-X / multi-party-ecdsa

Rust implementation of {t,n}-threshold ECDSA (elliptic curve digital signature algorithm).
GNU General Public License v3.0
966 stars 309 forks source link

Replace with pure rust implementation of libsecp256k1 #137

Closed Gauthamastro closed 2 years ago

Gauthamastro commented 2 years ago

PR replaces the secp256k1 with paritytech's libsecp256k1 which is a pure rust implementation.

elichai commented 2 years ago

Thanks for opening a PR :) May I ask what is the motivation behind this PR? Thanks!

Gauthamastro commented 2 years ago

We are using this library inside Intel's software extension guard (SGX) environment, which has issues with C code the current version of libsecp256k1 has. It would be great to have a pure rust version, so we don't need to maintain a fork just for this.

Other than that,

  1. Pretty much no change in API
  2. Pure rust version well maintained and used in production blockchains ( Polkadot, Kusama, to name a few)

So I really don't see a reason why not to add it.

elichai commented 2 years ago

Thanks for responding, Does this patch works for you? because it only changed test related stuff, the actual usage of secp256k1 is inside of the underlying math library https://github.com/ZenGo-X/curv

Also, I'd love to try and help you compile this as is on SGX, there's no reason this won't work.

As for repalcing the secp256k1 backend of curv please see https://github.com/ZenGo-X/curv/pull/44 and https://github.com/ZenGo-X/curv/issues/43

Gauthamastro commented 2 years ago

Thanks for responding, Does this patch works for you? because it only changed test related stuff, the actual usage of secp256k1 is inside of the underlying math library https://github.com/ZenGo-X/curv

Also, I'd love to try and help you compile this as is on SGX, there's no reason this won't work.

As for repalcing the secp256k1 backend of curv please see ZenGo-X/curv#44 and ZenGo-X/curv#43

Here you go, please check this: https://github.com/ZenGo-X/curv/pull/135

Right now, I am using a fork of libsecp256k1 for the PR but as soon as https://github.com/paritytech/libsecp256k1/pull/80 merges, I will change the dependency to official repo

Gauthamastro commented 2 years ago

Closing this as per our discussion in telegram.