-
Notifications
You must be signed in to change notification settings - Fork 134
dkg: threshold validation #4268
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
base: main
Are you sure you want to change the base?
Conversation
cb0b9d8 to
1656296
Compare
| threshold := config.Threshold | ||
| if threshold == 0 { | ||
| threshold = cluster.Threshold(len(nodes)) | ||
| } |
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 this a plausible scenario if this is called from the CLI? If the default is 0, can we add a debug log here?
Can a user set a negative number? It might be worth it making it a uint?
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.
Improved this in the new iteration. Thank you for the feedback!
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4268 +/- ##
==========================================
+ Coverage 56.64% 56.66% +0.02%
==========================================
Files 237 237
Lines 31332 31362 +30
==========================================
+ Hits 17748 17772 +24
- Misses 11311 11319 +8
+ Partials 2273 2271 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|



Added threshold validation for
RunDKGandRunReshareDKG. Previous behavior delegated the check down the stack to allow underlying library (kyber) to validate the threshold. Given that Kyber has weak validation that can lead to weird late errors, it is more convenient to verify the threshold upfront.Additionally added verification such that users cannot remove all nodes with "remove operators" command.
category: refactor
ticket: none