-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
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.
Just some superficial things I wanted to ask about right now. Didn't finish reviewing every detail.
use ark_ff::PrimeField; | ||
|
||
#[derive(Clone, Eq, PartialEq, Debug)] | ||
pub struct Matrix<T: Clone>(pub Vec<Vec<T>>); |
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 would suggest using a Vec<T>
in row-major order for better performance.
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.
make sense (for better cache performance)
} | ||
|
||
pub fn is_identity(&self) -> bool { | ||
if !self.is_square() { |
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.
Aren't all these matrices square? The transpose method for example seems to assume so. Do we need non-square matrices anywhere?
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.
yes all MDS matrices should be square
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.
Then, we should notate this and make sure that we are using this fact everywhere in the interface and the implementation.
src/poseidon/poseidon_ref.rs
Outdated
pub struct PoseidonConstants<F: PrimeField> { | ||
pub mds_matrices: MdsMatrices<F>, | ||
pub round_constants: Vec<F>, | ||
pub domain_tag: 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.
What is the domain_tag
used for?
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.
It is concatenated with hash input as the final hash input.
For example, if arity is 2, we have 2+1 hash inputs.
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.
for now, domain tag is just 2^ARITY - 1
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.
Is this in the original spec? I thought you were supposed to zero-pad the input like this:
fn hash(x: T, y: T) -> T {
poseidon_permutation_of_width_3(0, x, y)
}
because in plonk optimizations, we might need to combine some ARC with MDS for arity 3.
we can optimize this linear combination operation on plonk gate, specialized at ARITY
this optimization optimized ARITY-3 poseidon from 1124 to 433 gates
there is a bug during `verify_proof` though
moved to Manta-Network/manta-rs#57 |
No description provided.