secp256k1 icon indicating copy to clipboard operation
secp256k1 copied to clipboard

Scope of the library

Open real-or-random opened this issue 2 years ago • 16 comments

I think we should have a documented policy on the scope of the library. The discussion comes up from time to time [0] and has never really been resolved, I think. We don't strictly need answers to these questions but I feel a written policy would avoid some discussions and bikeshedding in the future. And of course, a policy won't be written in stone, we can still change it if have a reason to do so.

One way to approach this is "features that are used by Bitcoin Core now or a have a good chance to be used by Bitcoin Core in the future". ECDH was somewhat of an exception here but maybe it will be used in the future for BIP324 (P2P encryption).

Another approach is to add features that are useful to the wider ecosystem. Of course, this does not mean that we need to accept every feature that meets this definition --- including features means that we commit on maintaining them.

I think with ECDH and Java bindings, the thinking here was more towards the latter approach. Personally I lean more towards the former idea because it avoids the need to take decisions about what to include, and it saves resources. (For example, Java bindings have been removed for a reason.)

A somewhat related discussion is the one about experimental features (#817). For example, if we view musig2 as something that can be experimented around with in a fork such as secp256k1-zkp but then plan on porting it to here [0], then what's the difference to something which can go in as an experimental module? Is musig2 "super-experimental", is it just not clear yet if it has a good chance of ending up in Bitcoin Core or is there just really no difference in the end?

Right now, I think there's a good chance that #995 will be merged, so if we wanted to deprecate experimental modules (as has been proposed in #817 but couldn't find consensus), now would be a good time.

[0] https://github.com/bitcoin/bitcoin/issues/23326#issuecomment-947942068

real-or-random avatar Oct 20 '21 19:10 real-or-random

I lean more towards saying that the scope of the library is more than Bitcoin Core, though that does make the drawing the line somewhat fuzzier.

I think there is a place for experimental features, but they should actually be treated that way. E.g. there should be an actual expected migration to maturity, and we should aim for such features to actually not be in common use until they are mature. For example, https://github.com/bitcoin/bitcoin/issues/23326 makes a good case for having a feature that's expected to be eventually widely used first in an experimental form so it can be, well, experimented with.

sipa avatar Oct 20 '21 20:10 sipa

I think "features that are used by Bitcoin Core now or a have a good chance to be used by Bitcoin Core in the future" is a good rule. Experimental modules should be deprecated.

We added MuSig to -zkp because there was no demand to add it into Core before https://github.com/bitcoin/bitcoin/issues/23326. In light of that, it makes sense to add it to libsecp proper once it has been stabilized. Alternatively we could have a branch here with the MuSig2 implementation. The downside would be that our process of keeping libsecp and libsecp-zkp in sync would get quite a bit more difficult because -zkp only looks at libsecp master.

jonasnick avatar Oct 20 '21 20:10 jonasnick

Hmm, my view would be that libsecp256k1-zkp exists to serve the Elements ecosystem, and libsecp256k1 exists to serve the Bitcoin ecosystem. That shouldn't just mean Bitcoin Core - it's a separate library for a reason - but we should probably document to what extent Bitcoin Core has a special place (if any), e.g. its use of experimental features (if any).

I don't think it would be a desirable outcome that people have to use libsecp256k1-zkp to get access to features that are actually widely used in Bitcoin (even if only outside Bitcoin Core). If they meet that bar, they're probably worth spending the review/productionizing cycles on to get them in libsecp256k1 proper.

The immediate bar for being a non-experimental module in libsecp256k1 should be "what we're willing to maintain as a stable API". But what we should be willing to maintain as a stable API should be prioritized based on features that are needed by the ecosystem.

An open question for me is what the bar should be for being added as an experimental module (again, if any).

sipa avatar Oct 20 '21 20:10 sipa

The appeal of serving Core only is that no one needs to decide what counts as important to the Bitcoin ecosystem and ensures that this continues to be a small and well-maintained library. That is also in the best interest of Core.

I don't see anything fundamentally wrong with people using libsecp-zkp if they want additional features, since -zkp is just libsecp with additional modules.

But perhaps that doesn't matter too much because it's hard to find an example for something that is widely used in Bitcoin but not in Core. At least for the MuSig example (ignoring https://github.com/bitcoin/bitcoin/issues/2332) it would be difficult to imagine that it's widely used in the Bitcoin ecosystem but not in Core (perhaps there could be the case where it's used only in the Lightning ecosystem).

jonasnick avatar Oct 20 '21 21:10 jonasnick

Hmm, my view would be that libsecp256k1-zkp exists to serve the Elements ecosystem, and libsecp256k1 exists to serve the Bitcoin ecosystem.

That's a good point, and maybe that's the right view to see it. It's just not what's done in practice. -zkp started for the Elements ecosystem but now it's used as a place to get your Bitcoin schemes merged because (i) it's easier to get things merged there (is it?), and (ii) people may assume that our policy is to accept Core-only features (@jonasnick used exactly this reasoning above for musig! [1]) This again shows we should have a documented policy.

The two examples are ECDSA adaptor sigs and ECDSA sign-to-contract including anti-exfil signing with HW wallets. At least the latter thing is certainly relevant to Core as well.

Of course you could have those in another third fork secp256k1-beyondcore (with a better name obviously) but this would it even harder for users who want features from all three repos then, e.g., hardware wallets. It may be possible to solve this with a proper module system but this introduces a bunch of other problems. (Nevertheless I think we should support every change here that makes it easier for others to implement more modules in their repos.)

[1] The story with MuSig may be a little bit more involved. I assume with MuSig2 there's more demand in Core than there was with MuSig1, and given that MuSig1 is -zkp already, it made sense to PR MuSig2 there.

real-or-random avatar Oct 22 '21 09:10 real-or-random

Hey @real-or-random, Not sure whether this is right place to ask but i'm struggling to find out MuSig1 implementation in this library.

And really got confused on whether "MuSig1 is added to bitcoin main-net as part of Taproot"

Please help me out.

sarvalabs-sai avatar Mar 20 '22 14:03 sarvalabs-sai

If you're looking for a MuSig implementation, the https://github.com/ElementsProject/secp256k1-zkp is the right library to look at. It has an implementation of MuSig2 as an optional module. (It had MuSig1 in the past but it got replaced by MuSig2 recently).

real-or-random avatar Mar 20 '22 18:03 real-or-random

Yes, TQ very much for quick response @real-or-random, gone through that earlier and landed up here.

But wanted you to enlighten me whether MuSig1 been deployed in bitcoin main-net <= asking this because i couldn't see musig in this(https://github.com/bitcoin-core/secp256k1) library

and also on timelines for musig2 to bitcoin mainnet.

Lastly do we have any announcements channel somewhere to get updates?

sarvalabs-sai avatar Mar 20 '22 18:03 sarvalabs-sai

@sarvalabs-sai I think that the comment you're quoting refers to the fact that with the adoption of taproot/schnorrsig in Bitcoin since november 2021, it is now possible to use MuSig signatures in Bitcoin. It's not referring to a specific implementation existing, just that the necessarily protocol changes were made, and can now be used by anyone if they implement MuSig.

sipa avatar Mar 20 '22 18:03 sipa

Awesome understood, TQ @sipa

May i know why i couldn't Musig1 implementation in this library(assuming that this is the core library).

Am i missing something?

sarvalabs-sai avatar Mar 20 '22 18:03 sarvalabs-sai

As @real-or-random pointed out, there is ongoing work on a MuSig2 implementation in a fork of this library. At some point it may be ported to this library as well. Implementation of MuSig1 was abandoned as MuSig2 seems almost strictly better in all regards.

You're of course free to use whatever implementation you like, for Schnorr signatures or MuSig1 or MuSig2 or whatever is compatible with the Bitcoin protocol rules. You could in theory also implement it yourself.

sipa avatar Mar 20 '22 18:03 sipa

Got complete clarity, thank you for that @sipa

sarvalabs-sai avatar Mar 20 '22 18:03 sarvalabs-sai

And OfCourse i'll go with Musig2. Planning to write go bindings for Musig2 implementation in this library

I know it's too much to ask, but do we have any go-bindings repo already working on this?

sarvalabs-sai avatar Mar 20 '22 18:03 sarvalabs-sai

@sarvalabs-sai: Libera IRC channel secp256k1 or Bitcoin StackExchange are better forums for general questions. I think you are going off topic for this issue (scope of this library).

There is a work in progress MuSig2 implementation in Go here. Please take any further questions to the appropriate forum. Thanks.

michaelfolkson avatar Mar 21 '22 10:03 michaelfolkson

I hear you, my apologies.

Thanks

sarvalabs-sai avatar Mar 21 '22 11:03 sarvalabs-sai

I agree with all the above. Intuitively restricting libsecp256k1 to just what is needed in Bitcoin Core seems unnecessarily restrictive but without that restriction the line gets increasingly fuzzy.

e.g.

Hmm, my view would be that libsecp256k1-zkp exists to serve the Elements ecosystem, and libsecp256k1 exists to serve the Bitcoin ecosystem. That shouldn't just mean Bitcoin Core - it's a separate library for a reason - but we should probably document to what extent Bitcoin Core has a special place (if any), e.g. its use of experimental features (if any).

I always considered Liquid a subset of the Bitcoin ecosystem like I consider Lightning a subset of the Bitcoin ecosystem. Applied to the libsecp256k1/libsecp256k1-zkp scope discussion that would mean everything in libsecp256k1-zkp that is active on Liquid would eventually make its way into libsecp256k1 and then libsecp256k1-zkp effectively becomes just a staging ground for libsecp256k1. That seems unnecessarily loose to me.

But perhaps that doesn't matter too much because it's hard to find an example for something that is widely used in Bitcoin but not in Core. At least for the MuSig example (ignoring https://github.com/bitcoin/bitcoin/pull/2332) it would be difficult to imagine that it's widely used in the Bitcoin ecosystem but not in Core (perhaps there could be the case where it's used only in the Lightning ecosystem).

The alternative to defining a clear scope is to just take it case by case. MuSig2 in Lightning provides some interesting case studies.

  1. MuSig2 could be used in Lightning for "recursive" or nested MuSig2 which wouldn't be a Lightning protocol issue (other than supporting Schnorr signatures). Some Lightning implementations might choose to support it while others may not.

  2. Changing the 2-of-2 multisig that is funded by both counterparties when opening the channel to a single pubkey with MuSig2 would be a Lightning protocol issue.

My view would be that supporting the Lightning protocol in 2) would make MuSig2 a no brainer for libsecp256k1. I'm unsure on whether supporting individual Lightning implementations in 1) and not necessarily the Lightning protocol would be a strong enough argument for libsecp256k1.

michaelfolkson avatar Jun 20 '22 16:06 michaelfolkson

As @sipa @real-or-random and I discussed in person, we could document the scope in the README.md or a (to be added) CONTRIBUTING.md. Similar to the scope, we should mention the minimum requirements for new schemes considered for the library. They include a clear specification and high confidence in the scheme's security. We should also document that developers should reach out to the library devs (IRC, ...) before working on a new scheme to avoid disappointment when it turns out to not fulfill the requirements.

jonasnick avatar Oct 19 '22 15:10 jonasnick