-
Notifications
You must be signed in to change notification settings - Fork 400
Add serialization for Polynomials and Evaluations #144
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,11 @@ use crate::domain::{ | |
| DomainCoeff, EvaluationDomain, MixedRadixEvaluationDomain, Radix2EvaluationDomain, | ||
| }; | ||
| use ark_ff::{FftField, FftParameters}; | ||
| use ark_std::vec::Vec; | ||
| use ark_serialize::{CanonicalDeserialize, CanonicalSerialize, SerializationError}; | ||
| use ark_std::{ | ||
| io::{Read, Write}, | ||
| vec::Vec, | ||
| }; | ||
|
|
||
| /// Defines a domain over which finite field (I)FFTs can be performed. | ||
| /// Generally tries to build a radix-2 domain and falls back to a mixed-radix | ||
|
|
@@ -33,6 +37,123 @@ macro_rules! map { | |
| } | ||
| } | ||
|
|
||
| impl<F: FftField> CanonicalSerialize for GeneralEvaluationDomain<F> { | ||
| fn serialize<W: Write>(&self, mut writer: W) -> Result<(), SerializationError> { | ||
| let type_id = match self { | ||
| GeneralEvaluationDomain::Radix2(_) => 0u8, | ||
| GeneralEvaluationDomain::MixedRadix(_) => 1u8, | ||
| }; | ||
| type_id.serialize(&mut writer)?; | ||
|
|
||
| match self { | ||
| GeneralEvaluationDomain::Radix2(d) => d.serialize(&mut writer), | ||
|
Collaborator
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. Should we instead add a serialization method on the underlying domains, instead of serializing the degree directly? Feels to weird to only have the general evaluation domain have serialize, and not the underlying domains
Collaborator
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. oh I see the underlying methods the
Member
Author
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. Yes. I can do the change. I also have implemented the serialization for domains. The serialization of domains is needed to serialize the evaluations. |
||
| GeneralEvaluationDomain::MixedRadix(d) => d.serialize(&mut writer), | ||
| } | ||
| } | ||
|
|
||
| fn serialized_size(&self) -> usize { | ||
| let type_id = match self { | ||
| GeneralEvaluationDomain::Radix2(_) => 0u8, | ||
| GeneralEvaluationDomain::MixedRadix(_) => 1u8, | ||
| }; | ||
|
|
||
| type_id.serialized_size() | ||
| + match self { | ||
| GeneralEvaluationDomain::Radix2(d) => d.serialized_size(), | ||
| GeneralEvaluationDomain::MixedRadix(d) => d.serialized_size(), | ||
| } | ||
| } | ||
|
|
||
| fn serialize_uncompressed<W: Write>(&self, mut writer: W) -> Result<(), SerializationError> { | ||
| let type_id = match self { | ||
| GeneralEvaluationDomain::Radix2(_) => 0u8, | ||
| GeneralEvaluationDomain::MixedRadix(_) => 1u8, | ||
| }; | ||
| type_id.serialize_uncompressed(&mut writer)?; | ||
|
|
||
| match self { | ||
| GeneralEvaluationDomain::Radix2(d) => d.serialize_uncompressed(&mut writer), | ||
| GeneralEvaluationDomain::MixedRadix(d) => d.serialize_uncompressed(&mut writer), | ||
| } | ||
| } | ||
|
|
||
| fn serialize_unchecked<W: Write>(&self, mut writer: W) -> Result<(), SerializationError> { | ||
| let type_id = match self { | ||
| GeneralEvaluationDomain::Radix2(_) => 0u8, | ||
| GeneralEvaluationDomain::MixedRadix(_) => 1u8, | ||
| }; | ||
| type_id.serialize_unchecked(&mut writer)?; | ||
|
|
||
| match self { | ||
| GeneralEvaluationDomain::Radix2(d) => d.serialize_unchecked(&mut writer), | ||
| GeneralEvaluationDomain::MixedRadix(d) => d.serialize_unchecked(&mut writer), | ||
| } | ||
| } | ||
|
|
||
| fn uncompressed_size(&self) -> usize { | ||
| let type_id = match self { | ||
| GeneralEvaluationDomain::Radix2(_) => 0u8, | ||
| GeneralEvaluationDomain::MixedRadix(_) => 1u8, | ||
| }; | ||
|
|
||
| type_id.uncompressed_size() | ||
| + match self { | ||
| GeneralEvaluationDomain::Radix2(d) => d.uncompressed_size(), | ||
| GeneralEvaluationDomain::MixedRadix(d) => d.uncompressed_size(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl<F: FftField> CanonicalDeserialize for GeneralEvaluationDomain<F> { | ||
| fn deserialize<R: Read>(mut reader: R) -> Result<Self, SerializationError> { | ||
| let type_id = u8::deserialize(&mut reader)?; | ||
|
|
||
| if type_id == 0u8 { | ||
| Ok(Self::Radix2(Radix2EvaluationDomain::<F>::deserialize( | ||
| &mut reader, | ||
| )?)) | ||
| } else if type_id == 1u8 { | ||
| Ok(Self::MixedRadix( | ||
| MixedRadixEvaluationDomain::<F>::deserialize(&mut reader)?, | ||
| )) | ||
| } else { | ||
| Err(SerializationError::InvalidData) | ||
| } | ||
| } | ||
|
|
||
| fn deserialize_uncompressed<R: Read>(mut reader: R) -> Result<Self, SerializationError> { | ||
| let type_id = u8::deserialize_uncompressed(&mut reader)?; | ||
|
|
||
| if type_id == 0u8 { | ||
| Ok(Self::Radix2( | ||
| Radix2EvaluationDomain::<F>::deserialize_uncompressed(&mut reader)?, | ||
| )) | ||
| } else if type_id == 1u8 { | ||
| Ok(Self::MixedRadix( | ||
| MixedRadixEvaluationDomain::<F>::deserialize_uncompressed(&mut reader)?, | ||
| )) | ||
| } else { | ||
| Err(SerializationError::InvalidData) | ||
| } | ||
| } | ||
|
|
||
| fn deserialize_unchecked<R: Read>(mut reader: R) -> Result<Self, SerializationError> { | ||
| let type_id = u8::deserialize_unchecked(&mut reader)?; | ||
|
|
||
| if type_id == 0u8 { | ||
| Ok(Self::Radix2( | ||
| Radix2EvaluationDomain::<F>::deserialize_unchecked(&mut reader)?, | ||
| )) | ||
| } else if type_id == 1u8 { | ||
| Ok(Self::MixedRadix( | ||
| MixedRadixEvaluationDomain::<F>::deserialize_unchecked(&mut reader)?, | ||
| )) | ||
| } else { | ||
| Err(SerializationError::InvalidData) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl<F: FftField> EvaluationDomain<F> for GeneralEvaluationDomain<F> { | ||
| type Elements = GeneralElements<F>; | ||
|
|
||
|
|
||
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 PR is a breaking change, can you move it to the relevant section. (Its added a serialization trait bound to all polynomials)
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.
All the polynomial implementations in this repo have now implemented the serialization. I assume there is another project also using polynomial?
Uh oh!
There was an error while loading. Please reload this page.
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.
IDK if there exists another project that implements polynomial, but if they did their implementation is now broken
Having all the reasons your code may be broken after an upgrade is really useful though
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.
also could the ark-poly readme be updated for this change (https://github.com/arkworks-rs/algebra/tree/master/poly)
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.
On the way