-
Notifications
You must be signed in to change notification settings - Fork 10
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
Ferveo Variant docs - also removal of existing stale ferveo docs. #143
Conversation
…longer applicable based on changes we made to the logic.
56b5784
to
12fa4ec
Compare
12fa4ec
to
3eea8fd
Compare
`n - m + 1` nodes would have to not respond for the failure case. | ||
|
||
### Disadvantages | ||
- Since the `m` nodes are arbitrary, combining the decryption shares requires the requester to do a more computationally intensive operation. |
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.
Should we mention the performance overhead in concrete numbers and/or link to benchmarks?
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.
If there is something to link to - yep, no problem, just point me to it.
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.
In the PRs description: #32
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.
Those look like scripts to do the benchmarking, is there an output file containing the benchmarks somewhere or no? If not, perhaps you can add instructions on running those scripts to the docs in a separate PR.
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.
This is the output of cargo bench
and these files are generated locally. We could add a section dedicated to benchmarking with these instructions and other information.
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.
Let's do that in a different PR.
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.
LGTM 🎉
I think it's sufficiently descriptive at this time. Thank you for putting this to paper.
Side note: there is a mention of |
@derekpierre Yes, we use the original |
Type of PR:
Required reviews:
What this does:
Issues fixed/closed:
Fixes #140 .
Why it's needed:
Notes for reviewers:
I tried to keep some of the more relevant existing Ferveo docs. I have no problems with just deleting them if they are just no longer applicable.
Let me know what you think.