-
Notifications
You must be signed in to change notification settings - Fork 74
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
feat(frost-core): simplify trait bounds, don't use the allocator in some places #810
Open
StackOverflowExcept1on
wants to merge
5
commits into
ZcashFoundation:main
Choose a base branch
from
StackOverflowExcept1on:trait-bounds
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+62
−63
Open
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
4074fee
feat(frost-core): simplify trait bounds, don't use the allocator in s…
StackOverflowExcept1on 5a65678
use as_slice()
StackOverflowExcept1on 2c8ff0f
Merge remote-tracking branch 'origin/main' into trait-bounds
StackOverflowExcept1on 4449f6f
Copy -> Clone
StackOverflowExcept1on cb8a368
Merge branch 'main' into trait-bounds
StackOverflowExcept1on File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -25,7 +25,7 @@ use crate::{ | |||||||||||||||||||
/// pass-through, implemented for a type just for the ciphersuite, and calls through to another | ||||||||||||||||||||
/// implementation underneath, so that this trait does not have to be implemented for types you | ||||||||||||||||||||
/// don't own. | ||||||||||||||||||||
pub trait Field: Copy + Clone { | ||||||||||||||||||||
pub trait Field: Copy { | ||||||||||||||||||||
/// An element of the scalar field GF(p). | ||||||||||||||||||||
/// The Eq/PartialEq implementation MUST be constant-time. | ||||||||||||||||||||
type Scalar: Add<Output = Self::Scalar> | ||||||||||||||||||||
|
@@ -37,7 +37,7 @@ pub trait Field: Copy + Clone { | |||||||||||||||||||
+ Sub<Output = Self::Scalar>; | ||||||||||||||||||||
|
||||||||||||||||||||
/// A unique byte array buf of fixed length N. | ||||||||||||||||||||
type Serialization: AsRef<[u8]> + Debug + TryFrom<Vec<u8>>; | ||||||||||||||||||||
type Serialization: Copy + AsRef<[u8]> + AsMut<[u8]> + for<'a> TryFrom<&'a [u8]> + Debug; | ||||||||||||||||||||
|
||||||||||||||||||||
/// Returns the zero element of the field, the additive identity. | ||||||||||||||||||||
fn zero() -> Self::Scalar; | ||||||||||||||||||||
|
@@ -85,7 +85,7 @@ pub type Scalar<C> = <<<C as Ciphersuite>::Group as Group>::Field as Field>::Sca | |||||||||||||||||||
/// pass-through, implemented for a type just for the ciphersuite, and calls through to another | ||||||||||||||||||||
/// implementation underneath, so that this trait does not have to be implemented for types you | ||||||||||||||||||||
/// don't own. | ||||||||||||||||||||
pub trait Group: Copy + Clone + PartialEq { | ||||||||||||||||||||
pub trait Group: Copy + PartialEq { | ||||||||||||||||||||
/// A prime order finite field GF(q) over which all scalar values for our prime order group can | ||||||||||||||||||||
/// be multiplied are defined. | ||||||||||||||||||||
type Field: Field; | ||||||||||||||||||||
|
@@ -102,7 +102,7 @@ pub trait Group: Copy + Clone + PartialEq { | |||||||||||||||||||
/// A unique byte array buf of fixed length N. | ||||||||||||||||||||
/// | ||||||||||||||||||||
/// Little-endian! | ||||||||||||||||||||
type Serialization: AsRef<[u8]> + Debug + TryFrom<Vec<u8>>; | ||||||||||||||||||||
type Serialization: Copy + AsRef<[u8]> + AsMut<[u8]> + for<'a> TryFrom<&'a [u8]> + Debug; | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
|
||||||||||||||||||||
/// The order of the the quotient group when the prime order subgroup divides the order of the | ||||||||||||||||||||
/// full curve group. | ||||||||||||||||||||
|
@@ -147,7 +147,7 @@ pub type Element<C> = <<C as Ciphersuite>::Group as Group>::Element; | |||||||||||||||||||
/// | ||||||||||||||||||||
/// [FROST ciphersuite]: https://datatracker.ietf.org/doc/html/rfc9591#name-ciphersuites | ||||||||||||||||||||
// See https://github.com/ZcashFoundation/frost/issues/693 for reasoning about the 'static bound. | ||||||||||||||||||||
pub trait Ciphersuite: Copy + Clone + PartialEq + Debug + 'static { | ||||||||||||||||||||
pub trait Ciphersuite: Copy + PartialEq + Debug + 'static { | ||||||||||||||||||||
/// The ciphersuite ID string. It should be equal to the contextString in | ||||||||||||||||||||
/// the spec. For new ciphersuites, this should be a string that identifies | ||||||||||||||||||||
/// the ciphersuite; it's recommended to use a similar format to the | ||||||||||||||||||||
|
@@ -162,7 +162,11 @@ pub trait Ciphersuite: Copy + Clone + PartialEq + Debug + 'static { | |||||||||||||||||||
|
||||||||||||||||||||
/// A unique byte array of fixed length that is the `Group::ElementSerialization` + | ||||||||||||||||||||
/// `Group::ScalarSerialization` | ||||||||||||||||||||
type SignatureSerialization: AsRef<[u8]> + TryFrom<Vec<u8>>; | ||||||||||||||||||||
type SignatureSerialization: Copy | ||||||||||||||||||||
+ AsRef<[u8]> | ||||||||||||||||||||
+ AsMut<[u8]> | ||||||||||||||||||||
+ for<'a> TryFrom<&'a [u8]> | ||||||||||||||||||||
+ Debug; | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
|
||||||||||||||||||||
/// [H1] for a FROST ciphersuite. | ||||||||||||||||||||
/// | ||||||||||||||||||||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.
I also need
Copy
for my DKG implementation with encryption. Maybe we could reduce it toClone
instead ofCopy
(to also be compatible with types likeVec<u8>
)?