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

Change the type of the identifiers from u8 to u64 #110

Merged
merged 2 commits into from
May 28, 2021

Conversation

oxarbitrage
Copy link
Contributor

Just do the type change from #108

src/frost.rs Outdated
Comment on lines 441 to 442
/// The number of nonces is limited to 255. This limit can be increased if it
/// turns out to be too conservative.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no longer enforced by the type (u8) so we should either change this doc comment or enforce it in the code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The num_nonces is still u8:

num_nonces: u8,

I think the comment is still valid.

Copy link
Contributor

Choose a reason for hiding this comment

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

What was once enforced by the type system is not anymore in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, it is still not clear to me. Can you make a suggestion on how the comment should be ? I am thinking on just removing it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's:

  • create an issue to 'enforce maximum of 256 participants with u64 indexes'
  • mention that as a TODO in this comment

Then I think that'll be fine to get this change merged, that ticket should also be resolved in Marek's PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, i created the ticket and added a suggestion here to change the comment.

Copy link
Member

Choose a reason for hiding this comment

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

Note that the number of these nonces is not really related to the number of participants. These nonces are generated for each participant during the preprocessing stage. Our "message protocol" currently supports only one pair of nonces per signature so that the nonces (and corresponding commitments) are not cached and signing is always two-round.
(Here's the rationale for omitting cached nonces for now: #85 (comment))

src/frost.rs Outdated Show resolved Hide resolved
@oxarbitrage oxarbitrage merged commit e2940a4 into ZcashFoundation:main May 28, 2021
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.

None yet

3 participants