Skip to content
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

Roundtrip integration test for FROST #11

Merged
merged 5 commits into from
Jan 10, 2024
Merged

Roundtrip integration test for FROST #11

merged 5 commits into from
Jan 10, 2024

Conversation

pdyraga
Copy link
Member

@pdyraga pdyraga commented Jan 9, 2024

The roundtrip test executes round one and round two followed by a call to the coordinator to aggregate signature shares. The produced signature is then validated according to BIP-340. There are two cases covered: when the entire group participates in signing and when only a threshold of signers participate.

Introduced a set of helper functions that will allow us to set up FROST
protocol participants with real secret key shares.
The test executes round 1 and round 2 followed by a signature
aggregation. This is a very-happy-path scenario where all signers of the
group cooperate honestly.
In BIP-340 there are two places in the signing protocol where we need to invert
a scalar if the elliptic curve point for that scalar has even Y coordinate. The
first place is when `d` is calculated, and the second place is where `k` is
calculated. For `d` calculation, we will have to take it into account for the
key generation protocol. In unit tests, we simply do the inversion for the
secret key, as proposed in this commit. For `k` the situation is more
complicated because it is calculated according to the FROST protocol. In unit
tests, we retry signing as proposed in this commit. For ROAST, when we'll be
building it, this is something we need to take into account as well.
From [BIP-340]:
Let k' = int(rand) mod n[13].
Fail if k' = 0.
Let R = k'⋅G.
Let k = k' if has_even_y(R), otherwise let k = n - k' .

Although it is easy to address similar requirement for `d` by inverting
the secret key earlier in the test, for `k` the situation is more
complicated because it is generated by [FROST].
For now, we just retry.
See #12
Added a unit test covering the case when only threshold of signers
participate in FROST.
@pdyraga pdyraga requested a review from eth-r January 10, 2024 10:18
@pdyraga pdyraga marked this pull request as ready for review January 10, 2024 10:18
Copy link
Contributor

@eth-r eth-r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. We may want to increase the max attempt count to improve reliability, but otherwise everything looks alright.

// For now, we just retry.
//
// See https://github.com/threshold-network/roast-go/issues/12
for i := 0; !isSignatureValid && i < maxAttempts; i++ {
Copy link
Contributor

@eth-r eth-r Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With maxAttempts = 5 we have around a 3% chance that this test will not pass due to bad luck. We may want to increase it to e.g. 10 (0.1% chance of not passing) or 20 (0.0001% chance of not passing).

@eth-r eth-r merged commit f173471 into main Jan 10, 2024
2 checks passed
@eth-r eth-r deleted the roundtrip-frost branch January 10, 2024 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants