-
Notifications
You must be signed in to change notification settings - Fork 216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Threshold signatures #46
WIP: Threshold signatures #46
Conversation
Missing a |
bcb1dcd
to
49a5680
Compare
Some more algebra is at https://download.wpsoftware.net/bitcoin/wizardry/2019-02-sfdevs-threshold/slides.pdf |
unsigned char data[32]; | ||
} secp256k1_thresholdsig_keyshard; | ||
|
||
/** Multiplies a secret key by its MuSig coefficient and produces keyshards |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that the secret key is still multiplied by the MuSig coefficient in this iteration of the API
size_t n_coeffs | ||
) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(6) SECP256K1_ARG_NONNULL(11); | ||
|
||
/** Initializes a threshold signing session |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to mention that this is done once the participating signers are known. And that signers
array refers to the participating signers.
@@ -139,6 +139,11 @@ AC_ARG_ENABLE(module_musig, | |||
[enable_module_musig=$enableval], | |||
[enable_module_musig=no]) | |||
|
|||
AC_ARG_ENABLE(module_threshold, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe call the module thresholdsig for consistency with musig and with the api names?
printf("Verifying and combining shards..."); | ||
for (i = 0; i < N_SIGNERS_TOTAL; i++) { | ||
size_t j; | ||
/* Note that on every iteration, the inner loop will overwrite the `signing_pubkeys` array */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know the threshold.md
and secp256k1_thresholdsig.h
state that the seckeys[i]
is modified here too.
But wouldn't it be nice to mention it here again, since we have a comment anyway?
Maybe something like Note that on every iteration, the inner loop will overwrite seckeys[i] and the `signing_pubkeys` array
?
Perhaps worth noting that there's this other threshold scheme http://cacr.uwaterloo.ca/techreports/2001/corr2001-13.ps that @real-or-random dug out which is a bit more complicated and doen't mix as nicely with the MuSig API. But it has the very desirable property that the set of partipating signers can change even after the partial signatures have been collected. This avoids having to implement annoying and fragile protections against a single signer DoSing a signing session by first indicating presence and then dropping out. |
This commit adds three new cryptosystems to libsecp256k1: Pedersen commitments are a system for making blinded commitments to a value. Functionally they work like: commit_b,v = H(blind_b || value_v), except they are additively homorphic, e.g. C(b1, v1) - C(b2, v2) = C(b1 - b2, v1 - v2) and C(b1, v1) - C(b1, v1) = 0, etc. The commitments themselves are EC points, serialized as 33 bytes. In addition to the commit function this implementation includes utility functions for verifying that a set of commitments sums to zero, and for picking blinding factors that sum to zero. If the blinding factors are uniformly random, pedersen commitments have information theoretic privacy. Borromean ring signatures are a novel efficient ring signature construction for AND/OR admissions policies (the code here implements an AND of ORs, each of any size). This construction requires 32 bytes of signature per pubkey used plus 32 bytes of constant overhead. With these you can construct signatures like "Given pubkeys A B C D E F G, the signer knows the discrete logs satisifying (A || B) & (C || D || E) & (F || G)". ZK range proofs allow someone to prove a pedersen commitment is in a particular range (e.g. [0..2^64)) without revealing the specific value. The construction here is based on the above borromean ring signature and uses a radix-4 encoding and other optimizations to maximize efficiency. It also supports encoding proofs with a non-private base-10 exponent and minimum-value to allow trading off secrecy for size and speed (or just avoiding wasting space keeping data private that was already public due to external constraints). A proof for a 32-bit mantissa takes 2564 bytes, but 2048 bytes of this can be used to communicate a private message to a receiver who shares a secret random seed with the prover. Also: get rid of precomputed H tables (Pieter Wuille)
… cleanup Switch to secp256k1_pedersen_commitment by Andrew Poelstra. Switch to quadratic residue based disambiguation by Pieter Wuille.
Including a fix by Jonas Nick.
* add summing function for blinded generators * drop `excess` and `gen` from `verify_tally` * add extra_commit to rangeproof sign and verify
Includes fix and tests by Jonas Nick.
Include test_impl.h
…` constant and set it to 16
…ectionproof-stack surjectionproof: reduce stack usage and limit proofs to anonymity set of 16
…-rebase Fix schnorrsig and musig modules after rebase
f make helper functions static f hash noncedata into nonce in nonce_function_bipschnorr f expose nonce_function_bipschnorr f fix undefined behavior when shifting an int 31 places f add cplusplus ifdef to schnorrsig include file f hash complete pubkey into batch seed f chacha20 for bigendians f add schnorrsig to travis f show in configure if schnorrsig is enabled
Add fixups from upstream schnorrsig PR
d778a85
to
f6e4f86
Compare
Rebased; also added a .md document that tries to summarize where we're at with this |
Cool, I'll have a look. Given our experience with subtle attacks in interactive signatures, my take is that we should not merge this before we have some security proof. But the updated MD helps definitively here. |
secp256k1_pubkey_load(NULL, pt, &ctx->pubcoeff[idx]); | ||
return 1; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "my_index" the "threshold_index", "other_index" the "musig_index"?
procedure this API does not cover. | ||
2. Each signer uses `secp256k1_musig_pubkey_combine` to compute the combined | ||
public key `P`. This function also outputs a list `Y_i` of tweaked public | ||
keys, which will be needed to validate partial signatures. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't get how secp256k1_musig_pubkey_combine outputs a list of tweaked public keys, could be I missed something?
keys, which will be needed to validate partial signatures. | ||
3. Each signer modifies her secret key using `secp256k1_musig_tweak_secret_key` | ||
to produce a secret key `y_i` compatible with her public key `Y_i`. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't see secp256k1_musig_tweak_secret_key called in this threshold module. Maybe related to above jonasnick's comment, the public key are not tweaked beforehand anymore.
this 3. step seems to be specific to musig module, not used in threshold module.
complete signature. If the signature is invalid, at least one of the partial | ||
signatures was invalid. The culprit may be identified using the function | ||
`secp256k1_musig_partial_sig_verify` on the individual partial signatures. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each participant checks if other partial signatures are valid. Whether the combined pubkey is correct is not checked, right? I don't see any risk except wasting time in trying to produce final signature. If would like to avoid this situation, can add checking combined pubkey the same time when checking nonce hash at the beginning of this procedure.
@mizhang thanks a ton for your interest in this PR! Unfortunately it's still in a badly WIP state, and the documents aren't in sync with the PR (they're both basically braindumps of what I was thinking after having implemented an API, decided I didn't like it, then tried to come up with a better one). |
Are you around Austin? Will be possible to meet? :-)
…On Wednesday, July 3, 2019, Mia ***@***.***> wrote:
Andrew,
Any mailing list I can join? I would like to contribute if
WIP. How should I start?
thanks,
miya
On Wed, Jul 3, 2019 at 3:41 PM Andrew Poelstra ***@***.***>
wrote:
> @mizhang <https://github.com/mizhang> thanks a ton for your interest in
> this PR! Unfortunately it's still in a badly WIP state, and the documents
> aren't in sync with the PR (they're both basically braindumps of what I was
> thinking after having implemented an API, decided I didn't like it, then
> tried to come up with a better one).
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#46>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAWH3SVDQPKRIRCLGG5VCB3P5UFJJANCNFSM4GYV2KPQ>
> .
>
|
Hey Miya, yes, I'm back in town :) - send me an email at apoelstra@blockstream.com and we can figure something out. There isn't a good mailing list for this sort of development unfortunately. |
|
||
This PR has been open for a while and is blocked on progress on some | ||
supporting infrastructure; in particular the key setup protocol requires | ||
both a broadcast channel and private communication channels between |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the broadcast channel requirement should be viewed as a blocker for this PR because we can't solve that problem in this library. Assuming security of this scheme holds up, to get unforgeability it should be sufficient to make the implementer ensure that the pubcoeffs are the same for every participant. Obviously this is an enormous footgun.
We could add a function to compare pubcoeff hashes and signatures (independently). But this wouldn't solve the problem in general as it's probably easy to misuse and not straightforward to implement (how do you prevent replay?). Additionally, iit would be overkill for some situations. For example, sometimes there is that trusted party that can check the output on the displays of various hardware wallets match. Or a blockchain could be used as a broadcast channel.
The broadcast requirement at signing time is not as important because it's only DoS-preventing, so it could be safely offloaded to the implementer.
I agree that more in depth security analysis is a blocker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say as long as we trust the broadcast channel only for robustness (= we terminate properly) and we do not trust it for unforgeability, this is not a blocker. And this should be true for this scheme. But yes, a security analysis should show that this is indeed the case.
…ontext-debug add Debug impl to context object
Closing as out of date. We'll redo this on top of musig2 |
No description provided.