Add wrapper structure to work on #66
Conversation
|
I used "telescope" as name for the wrapper structure, I am more than happy to change this. |
|
@djetchev @curiecrypt Are the comments for public functions reasonable? They could be more verbose perhaps? |
There was a problem hiding this comment.
I'm not a big fan of the interface of Telescope::new(). This will just encourage the user to use user-friendly parameters instead of hardcoding Setup, which goes against https://github.com/cardano-scaling/alba/blob/8d8bcbfb130c58d57e7b6148c74737c35babc835/design/design.md#parameter-calculation.
I think new() should accept Setup only.
And I strongly disagree with you as written in the PR of this document. We should not make giving hardcoded parameters the norm, as we have no insurance they were computed correctly, this should also go against usable security principles and how cryptographic primitives are implemented generally. |
| } | ||
|
|
||
| impl Params { | ||
| /// Setup algorithm taking as input the security parameters, the set size |
There was a problem hiding this comment.
There will be a more detailed explanation in the documentation, for sure.
However, we might want to briefly mention why we have 3 different cases to compute the parameters in the function doc.
| @@ -56,8 +58,9 @@ impl Round { | |||
| }) | |||
| } | |||
|
|
|||
| /// Oracle producing a uniformly random value in [0, set_size[ used for round candidates | |||
| /// We also return hash(data) to follow the optimization presented in Section 3.3 | |||
| /// Oracle producing a uniformly random value in [0, set_size[ used for | |||
There was a problem hiding this comment.
The characters [ and ] are special characters for cargo doc. They must be written in the form \[, \].
Also, is this intentional? [0, set_size [ ?
It would be good to test if there is anything complaining while compiling the cargo docs as well.
I tried to put another name but couldn't find a better one. We might think of a more creative one in the future. |
I think most of the comments are quite descriptive. The structure seems pretty good. I suggested a minor improvement in my review. I will put some links to the comments to direct the reader to the related documentation, FYI. |
I see the concerns of both of you @tolikzinovyev, @rrtoledo. |
|
I see your point @curiecrypt. More importantly, I think, we need to be coherent as a team. A design decision should first be discussed on the design document before there is a pull request that contradicts it. Otherwise, what is the point of the design document? |
## Content
A simple wrapper struct around `prove()` and `verify()` functions and
parameter derivation.
## Pre-submit checklist
- Branch
- [x] Tests are provided (if possible)
- [x] Commit sequence broadly makes sense
- [x] Key commits have useful messages
- PR
- [x] No clippy warnings in the CI
- [x] Self-reviewed the diff
- [x] Useful pull request description
- [x] Reviewer requested
- Documentation
- [x] Update README file (if relevant)
- [x] Update documentation website (if relevant)
## Issue(s)
Relates to #66
## Content This PR includes a new folder structure for centralized documentation. Some example markdown files are created and they are linked to the related source files. The content will be filled out after the team approves the doc structure and pr #66 and #73 are merged. ## Pre-submit checklist - Branch - [x] Commit sequence broadly makes sense - [x] Key commits have useful messages - PR - [x] No clippy warnings in the CI - [x] Self-reviewed the diff - [x] Useful pull request description - [x] Reviewer requested - Documentation - [x] Update documentation website (if relevant) ## Issue(s) Closes #88
|
Replaced by #116 |
Content
Restructure the code to have a user facing structure from which we can directly prove and verify.
Pre-submit checklist
Comments
I removed the Params structure as it was not really needed and renamed the Setup one "Params".
I moved back the code into impls so that it is easier to use generics.
Issue(s)
Relates to #59