FROST Trusted Dealer #278
FROST Trusted Dealer #278
Conversation
jesseposner
force-pushed
the
frost-trusted-dealer
branch
from
6078d4b
to
ab3c7a6
Compare
include/secp256k1_frost.h
Outdated
| /** Serialize a FROST share | ||
| * | ||
| * Returns: 1 when the share could be serialized, 0 otherwise | ||
| * Args: ctx: a secp256k1 context object |
There was a problem hiding this comment.
nit: see bitcoin-core/secp256k1@aa3dd52 for consistent wording here
include/secp256k1_frost.h
Outdated
| * Each participant _must_ have a secure channel with the trusted dealer with | ||
| * which they can transmit shares to each other. | ||
| * | ||
| * A new seed32 _must_ be used for each key generation session. The trusted |
There was a problem hiding this comment.
nit: s/new/fresh
By the way, we need to say similar things for musig sessions. Feel free to steal some sentences/phrasing ideas from bitcoin-core/secp256k1#1479.
include/secp256k1_frost.h
Outdated
| * start a new session to generate a new key, they must NOT REUSE their | ||
| * respective seed32 again, but instead generate a new one. It is recommended |
There was a problem hiding this comment.
s/again// ("reuse again" is saying the same thing twice)
include/secp256k1_frost.h
Outdated
| const unsigned char *in32 | ||
| ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); | ||
|
|
||
| /** Creates key generation shares |
There was a problem hiding this comment.
"key shares" or "key generation" shares?
include/secp256k1_frost.h
Outdated
| * Each participant _must_ have a secure channel with the trusted dealer with | ||
| * which they can transmit shares to each other. |
There was a problem hiding this comment.
Hm, this may be slightly confusing because one could read it as "must have established a secure channel already", but that's of course not necessary. Maybe: "The trusted dealer must transmit shares over secure channels to participants."
src/modules/frost/keygen_impl.h
Outdated
| secp256k1_sha256_write(&sha, polygen, 16); | ||
| secp256k1_sha256_finalize(&sha, polygen); | ||
|
|
||
| /* Derive share */ |
src/modules/frost/keygen_impl.h
Outdated
| for (i = 0; i < n_participants; i++) { | ||
| secp256k1_scalar share_i, idx; | ||
|
|
||
| secp256k1_scalar_clear(&share_i); |
There was a problem hiding this comment.
This function is just for nuking memory. Use _scalar_set_int(0) to set to zero. (We should probably have a dedicated function, but we don't have one currently.)
| * Args: ctx: pointer to a context object | ||
| * Out: shares: pointer to the key generation shares | ||
| * pubshares: pointer to the public verification shares | ||
| * pk: pointer to the x-only public key |
There was a problem hiding this comment.
Should this be an x-only key? I'm not entirely sure, I would need to think about it.
There was a problem hiding this comment.
The function conditionally inverts the shares to guarantee that the private key will match the x-only key.
Since the primary use-case will be on-chain signing, I think it's a useful simplification to take care of the negation during share generation and makes it easier to reason about in secp256k1_frost_pubkey_get and secp256k1_frost_pubkey_tweak.
|
In general, I think it would be nice to get this rebase this. (Should be mostly trivial.) Also, CMake support is missing because we have this now for all other -zkp modules (#288 ), but that's currently not essential right now. |
| ARG_CHECK(n_pubnonces > 1); | ||
| ARG_CHECK(my_id != 0); | ||
| for (i = 0; i < n_pubnonces; i++) { | ||
| ARG_CHECK(ids[i] != 0); |
There was a problem hiding this comment.
should we also ARG_CHECK for 1 <= ids[i] <= max_participant?
There was a problem hiding this comment.
The function doesn't currently take max_participant as an input, and I'm not sure if it's worth requiring the extra state to perform this check.
| const unsigned char *msg32, | ||
| const secp256k1_xonly_pubkey *agg_pk, | ||
| const unsigned char *extra_input32 | ||
| ) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4); |
There was a problem hiding this comment.
We can also add pubshare as an input argument. MuSig2 hashes the individual pubkey when generating the nonces, but we don't need to make pubshare a mandatory argument like MuSig2 does (more info).
How do we decide the args supplied to the hash function for generating a nonce? Should we throw in all available signer information into the hash function? For instance, can we include the participant identifier too?
This commit adds the foundational configuration and building scripts and an initial structure for the project.
This commit adds trusted share generation, as well as share serialization and parsing.
This commits adds nonce generation, as well as serialization and parsing.
This commits add BIP-341 ("Taproot") and BIP-32 ("ordinary") public key
tweaking.
This commit adds nonce aggregation, as well as adaptor signatures.
This commit adds signature generation and aggregation, as well as partial signature serialization and parsing.
This commit adds an example file to demonstrate how to use the module.
Add api tests, nonce tests, tweak tests, sha256 tag tests, and constant time tests.
This commit adds a documentation file with instructions for how to use the module.
jesseposner
force-pushed
the
frost-trusted-dealer
branch
from
ab3c7a6
to
e94367c
Compare
This PR implements a BIP-340 compatible threshold signature system based on FROST (Flexible Round-Optimized Schnorr Threshold Signatures) with a trusted dealer. This PR does not implement distributed key generation (DKG).
FROST Paper
This PR is based upon the FROST paper by Chelsea Komlo and Ian Goldberg and the draft RFC.
Prior Work
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.