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

use max_signers and min_signers instead of num_signers and threshold to better follow spec #157

Merged
merged 1 commit into from
Oct 27, 2022

Conversation

conradoplg
Copy link
Contributor

Depends on #156 because it touches the same code.

The current naming can be confusing because in the spec, num_participants is the number of participants creating a signature, while in the code it's the maximum number of participants.

This renames them to become more similar to the spec. However, I decided to keep "signer" instead of "participant" because we use that terminology in a bazillion other places. We can make the change in a separate PR if we decide to.

This is a cleanup without a matching issue.

@conradoplg conradoplg self-assigned this Oct 21, 2022
@codecov-commenter
Copy link

codecov-commenter commented Oct 21, 2022

Codecov Report

Base: 83.61% // Head: 86.45% // Increases project coverage by +2.83% 🎉

Coverage data is based on head (b8170cb) compared to base (bea4ef0).
Patch coverage: 93.90% of modified lines in pull request are covered.

❗ Current head b8170cb differs from pull request most recent head f1af814. Consider uploading reports for the commit f1af814 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #157      +/-   ##
==========================================
+ Coverage   83.61%   86.45%   +2.83%     
==========================================
  Files          17       18       +1     
  Lines        1398     1779     +381     
==========================================
+ Hits         1169     1538     +369     
- Misses        229      241      +12     
Impacted Files Coverage Δ
frost-core/src/frost/round2.rs 70.42% <ø> (ø)
frost-core/src/lib.rs 72.97% <0.00%> (-6.44%) ⬇️
frost-p256/src/lib.rs 74.34% <75.00%> (+2.19%) ⬆️
frost-ristretto255/src/lib.rs 74.83% <76.47%> (+2.37%) ⬆️
frost-core/src/frost/identifier.rs 75.47% <78.04%> (+1.05%) ⬆️
frost-core/src/frost/keys.rs 80.88% <95.40%> (+1.11%) ⬆️
frost-core/src/frost/keys/dkg.rs 96.55% <96.55%> (ø)
frost-core/src/tests.rs 99.55% <99.31%> (-0.45%) ⬇️
frost-core/src/frost.rs 91.81% <100.00%> (-0.28%) ⬇️
frost-core/src/frost/round1.rs 90.96% <100.00%> (+0.11%) ⬆️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Base automatically changed from use-u16-params to main October 26, 2022 14:41
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

max and min, does what it says on the tin

mergify bot added a commit that referenced this pull request Oct 27, 2022
@mergify mergify bot merged commit 201d6ad into main Oct 27, 2022
@mergify mergify bot deleted the min-max-signers branch October 27, 2022 04:35
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.

3 participants