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

Random beacon comments #6590

Merged
merged 10 commits into from
Dec 2, 2024
Merged

Conversation

tarakby
Copy link
Contributor

@tarakby tarakby commented Oct 23, 2024

Updates to the DKG/Random beacon related docs:

  • clarify that the set β„› (the set of nodes able to participate in the random beacon) is a subset of π’Ÿ ∩ π’ž.
  • clarify that the protocol software implements a sanity check but does not guarantee that |β„›| is in the safe range.
  • minor clarifications about the DKG results and indices

@codecov-commenter
Copy link

codecov-commenter commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 25.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 41.71%. Comparing base (af44dde) to head (b0f36be).

Files with missing lines Patch % Lines
model/convert/service_event.go 0.00% 7 Missing ⚠️
cmd/bootstrap/run/qc.go 0.00% 1 Missing ⚠️
...nsus/hotstuff/verification/combined_verifier_v3.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##           feature/efm-recovery    #6590      +/-   ##
========================================================
- Coverage                 41.71%   41.71%   -0.01%     
========================================================
  Files                      2030     2030              
  Lines                    180459   180459              
========================================================
- Hits                      75272    75271       -1     
- Misses                    98991    98995       +4     
+ Partials                   6196     6193       -3     
Flag Coverage Ξ”
unittests 41.71% <25.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

β˜” View full report in Codecov by Sentry.
πŸ“’ Have feedback on the report? Share it here.

@tarakby tarakby marked this pull request as ready for review October 23, 2024 20:43
model/convert/service_event.go Outdated Show resolved Hide resolved
model/flow/dkg.go Outdated Show resolved Hide resolved
module/dkg.go Outdated Show resolved Hide resolved
module/dkg.go Outdated Show resolved Hide resolved
module/dkg/client.go Outdated Show resolved Hide resolved
Copy link
Member

@AlexHentschel AlexHentschel left a comment

Choose a reason for hiding this comment

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

Thank you for the detailed revisions. Love it. So much better and more accurate. No objections. My suggestions are essentially only trying to extend your documentation here and there with additional context to explain why things are the way they are on a more intuitive level. Feel free to reject or modify any or all of my suggestions.

model/convert/service_event.go Outdated Show resolved Hide resolved
model/convert/service_event.go Show resolved Hide resolved
state/protocol/validity.go Outdated Show resolved Hide resolved
state/protocol/validity.go Show resolved Hide resolved
state/protocol/defaults.go Outdated Show resolved Hide resolved
model/flow/dkg.go Outdated Show resolved Hide resolved
model/flow/dkg.go Outdated Show resolved Hide resolved
model/flow/dkg.go Outdated Show resolved Hide resolved
model/flow/dkg.go Outdated Show resolved Hide resolved
model/flow/dkg.go Outdated Show resolved Hide resolved
β€’ DKG key -> Random Beacon key
β€’ DKG private key -> Random Beacon private key
Copy link
Member

@AlexHentschel AlexHentschel left a comment

Choose a reason for hiding this comment

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

🚒

@tarakby tarakby merged commit 7c71c41 into feature/efm-recovery Dec 2, 2024
55 checks passed
@tarakby tarakby deleted the tarak/minor-suggestions branch December 2, 2024 16:02
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.

4 participants