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
Queries serialization improvements #15
Conversation
fa7ce54
to
0eefbd1
Compare
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.
minor comments
assert!(!query_values.is_empty(), "query values cannot be empty"); | ||
let elements_per_query = query_values[0].len(); | ||
assert!( | ||
elements_per_query != 0, |
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.
prefer is_empty
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 usually do too - but in this case we need to cache the elements_per_query
value - so, I think it's OK to have it like this.
/// TODO: return values as a vector of field elements | ||
pub fn into_batch<H: Hasher>(self, num_leaves: usize) -> (BatchMerkleProof, Vec<Vec<u8>>) { | ||
/// Convert a set of queries into a batch Merkle proof and corresponding query values. | ||
pub fn deserialize<H: Hasher, E: FieldElement>( |
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.
Can any zero value or length cause issues or panic (ie division by zero?)
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.
Good point! I've added some assertions here as this code gets called internally from within Winterfell. So, if it is called with wrong parameters, it's ok to panic.
Rescue hash example
This PR addresses #6 and also improves queries serialization/deserialization overall. Specifically:
The overall impact of this PR is ~1% reduction in proof sizes, but it also lays groundwork for switching over to new Merkle tree implementation.