secp256k1-zkp icon indicating copy to clipboard operation
secp256k1-zkp copied to clipboard

FROST

Open jesseposner opened this issue 3 years ago • 30 comments

This PR implements a BIP-340 compatible threshold signature system based on FROST (Flexible Round-Optimized Schnorr Threshold Signatures).

TODO

  • [x] Key generation APIs
  • [x] Nonce generation APIs
  • [x] Signing APIs
  • [x] Basic tests
  • [x] Refactor APIs
  • [x] Resolve merge conflicts
  • [x] Get CI working
  • [x] Sign list of VSS proofs from all participants to simulate broadcast channel
  • [x] Verify shares with VSS
  • [x] Update keygen protocol based on feedback from @LLFourn
  • [x] refactor nonce and index handling
  • [x] key tweaking (e.g. taproot and bip32)
  • [x] adaptor support
  • [x] Partial sig verification API
  • [x] change indexes to x-only pubkeys
  • [x] revert change to indexes
  • [x] Public VSS Verify
  • [x] Verification checks
  • [x] Serialization APIs
  • [x] Example file
  • [x] API for generating public verification shares
  • [x] Full tests
  • [x] more detailed code comments, including references to RFC
  • [x] Documentation file
  • [ ] Update API diagram [in-progress]
  • [ ] Update Python implementation [in-progress]
  • [ ] FROST BIP [in-progress]
  • [ ] Benchmarks and Stress Testing [in-progress]

FROST Paper

This PR is based upon the FROST paper by Chelsea Komlo and Ian Goldberg and the draft RFC.

Python Implementation [WIP]

https://github.com/jesseposner/FROST-BIP340

Prior Work

@apoelstra worked on a threshold implementation in https://github.com/ElementsProject/secp256k1-zkp/pull/46 and this PR builds on the techniques introduced there.

This PR is patterned on the APIs, and in many instances duplicates code, from the MuSig implementation due to their similarities in nonce generation and signing.

jesseposner avatar Jul 21 '21 22:07 jesseposner

@jesseposner Interesting work! I am currently thinking of a system where a large group of people would maintain ownership over a pool of funds, for example, within a single Bitcoin wallet. The FROST scheme looks like it could be a primitive in this scheme, especially if combined with an accountability protocol to detect/exclude malicious signers. Would this PR be ready for some preliminary experiments in a P2P setting?

devos50 avatar Aug 20 '21 12:08 devos50

@devos50 The PR is currently in a very early state and isn't that useful yet, in particular, it doesn't yet have support for signing or generating nonces. However, I've made significant progress in my local branch and I will be pushing substantial changes to the PR by the end of the month that will have nonce generation and signing and should be sufficient for experimentation.

jesseposner avatar Aug 20 '21 17:08 jesseposner

@jesseposner thanks for your response! I will wait for your changes then and try to build something around it later when the PR is more feature-complete 👍 (I'm going on holiday tomorrow so no need to rush).

devos50 avatar Aug 20 '21 19:08 devos50

Here's another BIP340 FROST implementation (in Go): https://github.com/taurusgroup/multi-party-sig

I think it would be nice to have a chat with them.

real-or-random avatar Sep 24 '21 12:09 real-or-random

By the way, there's a new paper about FROST that has a nice pseudocode in Figure 10: https://eprint.iacr.org/2021/1375.pdf

real-or-random avatar Nov 30 '21 16:11 real-or-random

  • Describe modifications to the FROST protocol [in-progress]

Just curious, are there any significant changes to the protocol itself (ignoring implementation details -- of course the line is fuzzy)?

real-or-random avatar Jan 07 '22 09:01 real-or-random

@real-or-random I'll have a full write-up ready very soon, but the general idea is that I'm using a method for combining MuSig and FROST that provides the following advantages: (1) MuSig keys can be converted to FROST keys without changing the aggregate public key, (2) a "broadcast channel" is implemented using MuSig to sign a hashed list of coefficient commitments, and (3) MuSig APIs are utilized (secp256k1_musig_pubkey_agg, secp256k1_musig_nonce_gen, secp256k1_musig_nonce_agg, secp256k1_musig_nonce_process, secp256k1_musig_partial_sig_agg) allowing for simpler code implementation and maintenance.

The FROST protocol is modified such that the first step is to generate a MuSig key. From there, the participants generate random polynomial coefficients, however, for their first coefficient the participants use their respective individual MuSig private key multiplied by the key aggregation coefficient. This results in the FROST and MuSig aggregate public key being the same, because the FROST aggregate public key is the sum of the commitments to each participant's first polynomial coefficient. This also means that the participants no longer need to distribute a proof of knowledge of their first coefficient, as specified by FROST, because the rogue key attack is prevented via MuSig and the key aggregation coefficients.

Then, when the participants distribute their shares and coefficient commitments, the FROST protocol is modified such that they also distribute a MuSig nonce commitment pair. When they have received coefficient commitments from all the other participants, they generate a hash of the list (i.e. the vss_hash), and sign it using MuSig. The partial MuSig signatures are aggregated, and a valid signature verifies that each participant has received the same commitments (thus simulating the broadcast channel, whose implementation is not specified in FROST). This completes key generation.

At that point, each participant has a FROST share that they can use for threshold signing, however, they use MuSig nonces (MuSig nonce generation and FROST nonce generation are nearly identical: in both cases the protocols use a pair of nonces and nonce commitments that are combined with a binding value).

The example file currently implements this method of key generation, nonce generation, and signing: https://github.com/ElementsProject/secp256k1-zkp/blob/36eba62b3b5450f0acde3459cf7600af2116bc32/examples/frost.c. If we want to use the more standard FROST protocol, the code changes required will be relatively modest (e.g. remove keyagg coefficient from share generation, add a proof of knowledge in share generation, create a secp256k1_frost_nonce_process function based on secp256k1_musig_nonce_process that uses a slightly different binding value and removes the keyagg coefficient when generating the challenge hash, derive aggregate public key in secp256k1_frost_share_agg.). It would also be possible to provide APIs for both protocols (standard FROST and hybrid FROST/MuSig).

jesseposner avatar Jan 07 '22 18:01 jesseposner

I had a closer look at the modified KeyGen (the picture in the first comment) and here are few comments:

  • I'm not sure if it's actually an anti-feature that this is compatible to MuSig key aggregation. I think it's a good mind model to keep threshold and multisigs separate as much as possible because the setup is so different. Do we really want users to reuse their FROST pubkey shares as MuSig public keys? It's strictly less secure than having a separate key for MuSig because you gave others shares of the key. And I don't see where reuse will be helpful. I tend to believe that it is good to use a FROST key only for running FROST, with exactly the group of signers it was supposed to be used.
  • It's a very neat idea to use MuSig during KeyGen but isn't it dangerous right now without a security proof for the modified protocol? This is in particular true when we drop the proofs of knowledge. I'm not entirely sure that the MuSig2 signing can replace those. In any case it will require the AGM then to prove the protocol secure, whereas probably ROM+OMDL will be enough for a proof of the unmodified FROST.
  • I don't think that MuSig can replace a broadcast channel. To be more specific, it may be able to replace a reliable broadcast channel but not a terminating broadcast channel. That is, the current KeyGen protocol still needs broadcast: If you terminates with an aggregate pubkey, you don't know if the others terminated or not. So you can't go ahead and use the pubkey (e.g., send money to it). For example, if everyone else has aborted, it's impossible to sign for that pubkey. This can be mitigated by a third round in which everyone confirms that the first two rounds were correct. If you receive a third-round confirmation from everybody, then you know that signing is possible and you can return the aggregate pubkey to the caller. (This still does not mean that everyone else will have received valid third-round confirmations... So others may be in some uncertain state but that's okayish if they don't timeout and delete their secrets. Signing will be possible, it's just that not everyone knows that it will be possible. To be really sure, you need terminating reliable broadcast, which is almost as strong as consensus.)
  • Have you considered using hashes of the pubkey shares as identities for Lagrange coefficients (instead of 1, 2, 3, ...). This may be nicer in practice because then you don't need to bother with an ordering of the pubkeys. (I learned this trick from https://blog.trailofbits.com/2021/12/21/disclosing-shamirs-secret-sharing-vulnerabilities-and-announcing-zkdocs/ which explains how to use it properly. Hashing is certainly safe.)
  • nit: It shouldn't be necessary to verify the partial sigs and the final sig.

real-or-random avatar Feb 18 '22 10:02 real-or-random

@real-or-random Thank you for the comments!

I'm not sure if it's actually an anti-feature that this is compatible to MuSig key aggregation. I think it's a good mind model to keep threshold and multisigs separate as much as possible because the setup is so different. Do we really want users to reuse their FROST pubkey shares as MuSig public keys? It's strictly less secure than having a separate key for MuSig because you gave others shares of the key. And I don't see where reuse will be helpful. I tend to believe that it is good to use a FROST key only for running FROST, with exactly the group of signers it was supposed to be used.

Reuse is potentially helpful if you have funds secured with a MuSig setup and later decide you'd like to convert to a threshold setup without needing to move the funds. In practice I'm not sure how useful or desirable this actually is and I'm open to the idea that the security downsides outweigh the benefits. Even if that's the case, we can still use MuSig for the broadcast channel (assuming we want to do that), and change the API such that it doesn't allow for converting a pre-existing MuSig key.

It's a very neat idea to use MuSig during KeyGen but isn't it dangerous right now without a security proof for the modified protocol? This is in particular true when we drop the proofs of knowledge. I'm not entirely sure that the MuSig2 signing can replace those. In any case it will require the AGM then to prove the protocol secure, whereas probably ROM+OMDL will be enough for a proof of the unmodified FROST.

I agree that we need a security proof for the modified protocol if we think that's worth pursuing. However, the MuSig partial signatures are also proofs of knowledge, so I think the modified protocol still provides that.

I don't think that MuSig can replace a broadcast channel. To be more specific, it may be able to replace a reliable broadcast channel but not a terminating broadcast channel. That is, the current KeyGen protocol still needs broadcast: If you terminates with an aggregate pubkey, you don't know if the others terminated or not. So you can't go ahead and use the pubkey (e.g., send money to it). For example, if everyone else has aborted, it's impossible to sign for that pubkey. This can be mitigated by a third round in which everyone confirms that the first two rounds were correct. If you receive a third-round confirmation from everybody, then you know that signing is possible and you can return the aggregate pubkey to the caller. (This still does not mean that everyone else will have received valid third-round confirmations... So others may be in some uncertain state but that's okayish if they don't timeout and delete their secrets. Signing will be possible, it's just that not everyone knows that it will be possible. To be really sure, you need terminating reliable broadcast, which is almost as strong as consensus.)

The MuSig partial signatures are not generated until every participant has received their shares, verified their shares, and aggregated their shares. So when a participant receives partial signatures from every other participant, they know that each participant has the shares required to sign for the aggregate public key assuming that all participants verified their shares against the same commitments. This final assumption is validated when the partial signatures are aggregated and the aggregate is verified as a valid signature. So at that point, I believe the participants are left in that same final state you described at the end, where they are potentially uncertain (because they don't know if the other participants were able to verify the aggregate signature), however signing will be possible assuming they don't timeout and delete.

Have you considered using hashes of the pubkey shares as identities for Lagrange coefficients (instead of 1, 2, 3, ...). This may be nicer in practice because then you don't need to bother with an ordering of the pubkeys. (I learned this trick from https://blog.trailofbits.com/2021/12/21/disclosing-shamirs-secret-sharing-vulnerabilities-and-announcing-zkdocs/ which explains how to use it properly. Hashing is certainly safe.)

That looks very useful, thanks for the suggestion!

nit: It shouldn't be necessary to verify the partial sigs and the final sig.

Great point!

jesseposner avatar Feb 18 '22 18:02 jesseposner

I'm not sure if it's actually an anti-feature that this is compatible to MuSig key aggregation. I think it's a good mind model to keep threshold and multisigs separate as much as possible because the setup is so different. Do we really want users to reuse their FROST pubkey shares as MuSig public keys? It's strictly less secure than having a separate key for MuSig because you gave others shares of the key. And I don't see where reuse will be helpful. I tend to believe that it is good to use a FROST key only for running FROST, with exactly the group of signers it was supposed to be used.

Reuse is potentially helpful if you have funds secured with a MuSig setup and later decide you'd like to convert to a threshold setup without needing to move the funds. In practice I'm not sure how useful or desirable this actually is and I'm open to the idea that the security downsides outweigh the benefits. Even if that's the case, we can still use MuSig for the broadcast channel (assuming we want to do that), and change the API such that it doesn't allow for converting a pre-existing MuSig key.

Hm yes, I tend to believe that converting from a MuSig pubkey to a FROST pubkey is rare enough that it doesn't matter to change the pubkey.

It's a very neat idea to use MuSig during KeyGen but isn't it dangerous right now without a security proof for the modified protocol? This is in particular true when we drop the proofs of knowledge. I'm not entirely sure that the MuSig2 signing can replace those. In any case it will require the AGM then to prove the protocol secure, whereas probably ROM+OMDL will be enough for a proof of the unmodified FROST.

I agree that we need a security proof for the modified protocol if we think that's worth pursuing. However, the MuSig partial signatures are also proofs of knowledge, so I think the modified protocol still provides that.

The partial sigs is one part where I'm not sure. A single partial sig is not a proof of knowledge because the hashed nonce is not the partial nonce of the participant. A maybe surprising property is that partial sigs during a MuSig2 run can be forged -- just not all partial signatures of one run because this would imply a real forgery. Now in this case we get all partial sigs (and a full sig) but one would need to see if this really means that all keys can be extracted. I never thought about this to be honest.

I don't think that MuSig can replace a broadcast channel. To be more specific, it may be able to replace a reliable broadcast channel but not a terminating broadcast channel. That is, the current KeyGen protocol still needs broadcast: If you terminates with an aggregate pubkey, you don't know if the others terminated or not. So you can't go ahead and use the pubkey (e.g., send money to it). For example, if everyone else has aborted, it's impossible to sign for that pubkey. This can be mitigated by a third round in which everyone confirms that the first two rounds were correct. If you receive a third-round confirmation from everybody, then you know that signing is possible and you can return the aggregate pubkey to the caller. (This still does not mean that everyone else will have received valid third-round confirmations... So others may be in some uncertain state but that's okayish if they don't timeout and delete their secrets. Signing will be possible, it's just that not everyone knows that it will be possible. To be really sure, you need terminating reliable broadcast, which is almost as strong as consensus.)

The MuSig partial signatures are not generated until every participant has received their shares, verified their shares, and aggregated their shares. So when a participant receives partial signatures from every other participant, they know that each participant has the shares required to sign for the aggregate public key assuming that all participants verified their shares against the same commitments. This final assumption is validated when the partial signatures are aggregated and the aggregate is verified as a valid signature. So at that point, I believe the participants are left in that same final state you described at the end, where they are potentially uncertain (because they don't know if the other participants were able to verify the aggregate signature), however signing will be possible assuming they don't timeout and delete.

Ok yes, I think you're right.

real-or-random avatar Feb 24 '22 22:02 real-or-random

Awsome work @jesseposner, thank you. Do you consider stress testing your code with 10M users? btw Proof-of-concept of your FROST efforts now operational on Android. Delft University students took your code and used it towards building a fully decentralised Taproot-based DAO, based on joint ownership of Bitcoin funds which can be spent by participating in a k-out-of-n Schnorr signature. Specifically, its expanded for distributed usage with our "Trustchain ledger" for potentially numerous signatures from Android users. interesting?

synctext avatar May 13 '22 11:05 synctext

Thanks @synctext! I'm excited to see that your students were able to leverage this code for their project, I will check it out. Please be careful, the code is still early and has not yet been reviewed and should not be considered secure software.

Great suggestion regarding the stress testing, I will add it to the to-do list.

jesseposner avatar May 13 '22 20:05 jesseposner

Thanks for the comments @jonasnick!

As far as I can see, there are no proofs of knowledge involved in key aggregation and, instead, MuSig-aggregation is assumed make PoKs unnecessary. I think this would require a security proof/argument.

The vss_hash signatures are also PoKs. Instead of distributing PoKs in the first round, we distribute them in the second round, and instead of simply using the index and a context string as the message for the signature that serves as the PoK, we use the vss_hash as the message.

By moving the PoK to the second round and including the vss_hash in the PoK, we fulfill the broadcast channel requirement with the same set of signatures used for the PoK ~(otherwise we would need an additional set of signatures to fulfill the broadcast channel requirement)~. As you observed, we already have secure channels between the participants, so we could distribute the vss_hash separately. However, by using the PoKs, that step becomes unnecessary, so it's a little more efficient.

Agree with @real-or-random that we should try to get rid of signer indices entirely if possible.

Great point, I added this to the to-do list.

I pushed a branch with a few fixup! commits and CI integration. Feel free to cherry-pick: https://github.com/jonasnick/secp256k1-zkp/commits/frost-jn.

Thanks, will do!

jesseposner avatar May 17 '22 17:05 jesseposner

@jonasnick

I think one open todo that may be useful to keep in the OP is:

  • [ ] decide on whether to use MuSig key aggregation

done

One problem is that there's no agreed-upon session identifer that is covered by the signature, so signatures are transferrable between sessions. A malicious signer could send a received vss_commitment and signature from a different session to the complaint handler to blame an innocent signer that was present in two keygen sessions.

Here's an idea for how we might incorporate a session ID by building on what you described:

  1. Each signer generates a random session ID for the DKG session. When distributing shares, signers sign the (1) VSS commitment, (2) share, (3) index, and (4) session ID. This signature could serve as the PoK. If it's not serving as the PoK, then it can just be a hash, and we presume that the hash is signed by the secure channel between the signers when it is sent.
  2. Upon share verification failure, the signer submits the signature and its inputs to the complaint handler. I agree, we should have a verification API that a third party handler can use for this purpose.
  3. Instead of signing the vss_hash by itself, the vss_hash is signed along with the session ID. Also, we might not need to double hash here: the signature inputs could just be the inputs to the vss_hash rather than the hash. If this signature isn't serving as the PoK, then we can use a hash instead of a signature and presume the hash is signed via the secure channel.
  4. If a signer receives an unexpected VSS set from another signer, all signers submit all the signatures they received from step 1 and from step 3 to the complaint handler, along with the inputs to those signatures. The session IDs prevent data from another session being used to blame an innocent signer.

jesseposner avatar May 21 '22 00:05 jesseposner

Regarding the issue of identifying dishonest signers in the key generation phase, @real-or-random rightfully points out that transcript + signatures only helps if the signers already agree on the individual public keys. If there's disagreement, the signature is worthless. Establishing agreement is difficult. I'm not sure how much sense it makes in practice to simply assume that there is already agreement on the public keys.

jonasnick avatar May 25 '22 19:05 jonasnick

@jonasnick Perhaps we can use the key pair provided to secp256k1_frost_share_gen when keygen is initialized as an authentication foundation. We require the participants to agree upon individual public keys before beginning the protocol, and they provide their respective key pairs to secp256k1_frost_share_gen. The participants can use this key for signing various commitments during keygen, and we require explicit signatures instead of implied secure channels in all cases, and the third party handler would also be familiar with these keys as part of the public key agreement phase.

jesseposner avatar May 25 '22 23:05 jesseposner

We can't just use the x-coord-based shares. A malicious signer can use public key lift_x(group_size) (which happens to be a valid curve point) because group_size == 0 for scalars. So this would reveal the entire secret.

Duplicate keys

One possible reason to support duplicate keys is to avoid issues if a malicious signer copies the pubkey from an honest signer. If duplicate keys are supported, the protocol could just continue. This was one of the main reasons to support duplicate keys in MuSig2. But I think that argument doesn't apply here, independently of the PoKs: The malicious signer (with the copied pubkey) wouldn't be able to generate private shares for honest signers, so the DKG would fail anyway.

Another reason to support duplicate keys is to support "weighted" FROST, where signers can have different weights. For example, you could have a 3-of-5 with only 3 signers of weight 1 and a special signer of weight 2. This can always be emulated by letting each signer generate w secret keys (where w is its weight). This simple method makes key generation slower for large weights but I think signing would really not be effected. So if a signer with w > 1 is present, it can simply add up its nonces and partial sigs. And if it's not present, other signers can do the same with the interpolated contributions to the partial sigs. So the cost in signing is only scalar operations. I believe weighted FROST is certainly useful in some contexts but I don't know if it's worth including support for this right now. If people want this, it could still be added later.

How to create the shares?

For selecting the positions where we evaluate the polynomial, I see the following rough directions:

  1. We could derive the positions where we evaluate the polynomial via H(pk||sub_index) where sub_index is only relevant when there are weights > 1. The sub_index ensures that signers with weights > 1 get multiple shares. For now, we could fix sub_index == 0 and keep it simple, but we'd still be able to extend this in the future. (And this would avoid the critical issue of using x-coord-based shares.)
  2. We could rely on a global index. This makes the implementation simpler but I believe the API would be rather annoying.

One caveat is of approach 1 is that I'm not exactly sure how it interacts with the order of hashing in the vss_hash. (But I tend to think that's similar to the hashing the list of pubkeys in the MuSig2 key aggregation, so this shouldn't be a problem.

A larger concern is that approach 1 is really sensitive to changes of the pubkeys. So say you're an honest signer and you run DKG with some malicious signer claiming to have pk. For some reason, the DKG aborts but the malicious signer has learned the share for H(pk||0). Now you try to run again, and the malicious signer claims to have pk' and learns the share for H(pk'||0), which is bad because it now has two shares. So if DKG fails, you should throw stop using your secret key and take a new one. (What makes it worse: depending on the state, you cannot even delete the key because you may not know whether the DKG has finished, see the discussion above). So that's bad but I don't know... It's bad in general. With approach 2, the same can happen if you change the order of keys or the threshold for example. So approach 1 just makes the more likely but we'll anyway need some defense against the user trying DKGs multiple times, and this will be hard.

There are still many things unclear, so I think a proper writeup would really help. And it would probably make sense to look into DKGs in more detail, even on the level of provable security...

real-or-random avatar Jun 08 '22 15:06 real-or-random

We can't just use the x-coord-based shares. A malicious signer can use public key lift_x(group_size) (which happens to be a valid curve point) because group_size == 0 for scalars. So this would reveal the entire secret.

Ah, I see that the code aborts on overflow (thanks @jonasnick), so everything is fine. Sorry for the fuss. https://github.com/ElementsProject/secp256k1-zkp/blob/745dcb0985dbd689dae50765cdecc00a5f85b303/src/modules/frost/keygen_impl.h#L74-L77

real-or-random avatar Jun 08 '22 15:06 real-or-random

Sorry for the fuss.

No problem at all @real-or-random, and thank you for the illuminating analysis of these issues!

jesseposner avatar Jun 09 '22 05:06 jesseposner

@real-or-random I've added a session ID to share_gen (https://github.com/ElementsProject/secp256k1-zkp/pull/138/commits/c5cabf353d62fe9ec62305b83d37b5076805582e) so that each DKG session generates new coefficients for the non-constant terms of the polynomial. This will make shares generated in each DKG only useful for that DKG session, so if a malicious signer causes the DKG to be re-run, the old shares cannot be combined with the new shares to interpolate the constant term (as long as a new session ID is provided for each DKG session).

jesseposner avatar Jul 05 '22 04:07 jesseposner

Hello, @jesseposner, thank you for the awesome work! I'm working on a C++ layer over the C secp256k1_frost API as part of my work on L15. I have created a stress test with a "500-of-1000" signers configuration. It can be trivially changed for any number of signers like "5 000 000-of-10 000 000". I hope you don’t mind if I share some results and thoughts. I ran the "500-of-1000" configuration successfully on my laptop (Intel Core i7-9750H @ 2.60GHz × 6 cores, 16G RAM, and 32GB swap). No network delays have been implemented yet. Results:

  1. Test script took 1 hour yesterday on a previous version and 2 hours on today’s (Jul 31) version, with 98% CPU load for both cases.

  2. Test consumed totally (for all the peers running on a single computer) about 34G of virtual memory.

  3. Most of the time was spent on aggregated key generation (see listing below). The time that the signing step took is negligible compared to the time of aggregated key generation.

  4. Removing my additional data abstraction types over sekp256k1_frost types (like VSS commitments and key shares) reduced the memory and CPU time consumption by about 15-25 percent.

The system I am implementing assumes a hive of signers where each one contributes to a large number of different signing sub-hives. Reducing signers' node memory consumption is critical to allow a signer to contribute to as many sub-hives as possible. Here are some performance-related suggestions:

  1. Please split the secp256k1_frost_share_gen() function in two:

    • secp256k1_frost_keyshare_commitment_gen()

    • secp256k1_frost_keyshare_gen()

Motivation: I am assuming that a VSS commitment broadcast must take place strictly before the VSS keyshare exchange (to avoid a rogue key attack, for example). The time between these two phases may be significant and depends on the exchange protocol used by a particular system. The current implementation, when used most straightforwardly, implies that a peer should consume memory for its VSS keyshares while generating them at the same time as VSS commitment, or spend CPU time to generate a keyshare for at least one other peer. Keeping them separate will give more flexibility to the user. It looks like an easy change since the current secp256k1_frost_share_gen() is already coded as two independent parts for the VSS commitment and VSS keyshare generation.

  1. It may be worth providing a way to bypass the usage of abstract types like secp256k1_keypair, any secp256k1_frost_...., etc. Consider exposing an API that directly accepts secp256k1_scalar and secp256k1_ge.

Motivation for this comes directly from note 4 on test results. This will most likely reduce the memory consumption otherwise needed to deserialize every "flat structure" into a sec/pub key format and then convert it into a scalar or point. I appreciate your approach being a safeguard against the wrong usage of variables by programmers, so the suggestion is to expose a low-level API as an option.

Register peers (1000) Total time: 12 ms Mean time per sample: 0 ms

Exchange Key shares Total time: 78126 ms Mean time per sample: 78 ms

Aggregate pubkey Total time: 68763809 ms Mean time per sample: 68763 ms

Commit nonces Total time: 28520 ms Mean time per sample: 28 ms

Select active signers (500) Total time: 4 ms Mean time per sample: 0 ms

Preprocess signature Total time: 153 ms Mean time per sample: 0 ms

Distribute sig shares Total time: 0 ms Mean time per sample: 0 ms

Aggregate signature Total time: 0 ms Mean time per sample: 0 ms

l2xl avatar Aug 02 '22 00:08 l2xl

Thank you @l2xl and @ariard for the feedback and review, it's much appreciated!

My top priority currently is getting this PR into a state more appropriate for a thorough review. I'm currently working through the test suite which will likely surface some issues, and in particular, I haven't run constant time tests with valgrind yet. Once that is complete, I'm going to focus on improving the documentation, and then cleaning up the commit history, and then I will move the PR out of draft mode and mark it for review.

Review is also welcome before I get there, but I'm going hold off on making major API changes like the ones suggested by @l2xl until then. I expect to be able to mark it for review by the end of this month.

jesseposner avatar Aug 11 '22 23:08 jesseposner

My top priority currently is getting this PR into a state more appropriate for a thorough review. I'm currently working through the test suite which will likely surface some issues, and in particular, I haven't run constant time tests with valgrind yet. Once that is complete, I'm going to focus on improving the documentation, and then cleaning up the commit history, and then I will move the PR out of draft mode and mark it for review.

Awesome, keeping an eye on this PR. FYI, I opened a corresponding a meta-PR in LDK to track the changes we would need on our side. Whenever you have time, we're eager to collect feedbacks, especially on the implementation and deployment trade-offs.

ariard avatar Aug 13 '22 00:08 ariard

For anyone working on the branch: I'll be cleaning up the commit history tomorrow to help with review. There will be substantially less commits, about 5 or so, after I force push the new history.

jesseposner avatar Aug 30 '22 21:08 jesseposner

Do you have any thoughts about what encryption we might use? Perhaps something built on secp256k1_scalar_chacha20?

My goto answer is always chacha20-poly1305 :), now that we have chacha20 I don't think adding a poly1305 is that terrible, but will let others weigh in on this

elichai avatar Jan 13 '23 16:01 elichai

Proposed refactor based upon the review comments received so far: https://gist.github.com/jesseposner/0168e88f9cc911a3dc9095a78f6efc15

jesseposner avatar Apr 24 '23 21:04 jesseposner

Just found this fork of libsecp256k1 with a FROST module: https://github.com/bancaditalia/secp256k1-frost/blob/frost/src/modules/frost/README.md

real-or-random avatar May 23 '23 11:05 real-or-random

Not specifically related to this PR but I'd like to flag that there is now a 2 BTC bounty for a FROST powered wallet that allows signer membership set to change: https://twitter.com/gladstein/status/1684567113430892545

leishman avatar Jul 27 '23 17:07 leishman

I added three commits to a forked branch that applies the global updates introduced with the recent sync of -zkp to upstream libsecp (see https://github.com/BlockstreamResearch/secp256k1-zkp/discussions/266):

https://github.com/jonasnick/secp256k1-zkp/commits/frost-jn

Feel free to cherry-pick.

jonasnick avatar Aug 21 '23 15:08 jonasnick

@real-or-random and I have been brainstorming about the potential structure of the frost module's DKG, as well as what a potential bip-frost's DKG section might look like. We've consolidated our ideas so far in this repository: https://github.com/jonasnick/bip-frost-dkg. In summary we've been considering SimplPedPop and variants. This is pretty close to what we've already attempted here, but we now have a much better picture of the requirements on the caller and the broadcast channel.

jonasnick avatar Aug 25 '23 18:08 jonasnick