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: refactor inner-product prover #57
Conversation
c3ce1cf
to
bc2aa26
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.
utACK
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.
Nice.
I would like one small change though to not hide the potential panic, so we can use a custom fn checked_split_at<T>(my_vec: &[T], n: usize)
instead of pub const fn split_at(&self, mid: usize)
src/range_proof.rs
Outdated
// Split the vectors for folding | ||
let (a_lo, a_hi) = a_li.split_at(n); | ||
let (b_lo, b_hi) = a_ri.split_at(n); | ||
let (gi_base_lo, gi_base_hi) = gi_base.split_at(n); | ||
let (hi_base_lo, hi_base_hi) = hi_base.split_at(n); |
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.
// Split the vectors for folding | |
let (a_lo, a_hi) = a_li.split_at(n); | |
let (b_lo, b_hi) = a_ri.split_at(n); | |
let (gi_base_lo, gi_base_hi) = gi_base.split_at(n); | |
let (hi_base_lo, hi_base_hi) = hi_base.split_at(n); | |
// Split the vectors for folding | |
fn checked_split_at<T>(my_vec: &[T], n: usize) -> Result<(&[T], &[T]), ProofError> { | |
if n <= my_vec.len() { | |
Ok(my_vec.split_at(n)) | |
} else { | |
Err(ProofError::InvalidLength("Index out of bounds".to_string())) | |
} | |
} | |
let (a_lo, a_hi) = checked_split_at(&a_li, n)?; | |
let (b_lo, b_hi) = checked_split_at(&a_ri, n)?; | |
let (gi_base_lo, gi_base_hi) = checked_split_at(&gi_base, n)?; | |
let (hi_base_lo, hi_base_hi) = checked_split_at(&hi_base, n)?; |
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.
Doesn't this basically treat the split the same as before from a panic perspective? The slice-based version has the same issue.
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.
Updated in a new commit.
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.
ACK
The inner-product prover is a bit clunky, as it requires the range prover to set up a struct, perform iterative rounds, and then parse out the proof elements it needs. This also requires consistency checks that it would be nicer to avoid.
This PR moves the inner-product prover into the range prover, which simplifies things at the cost of a longer prover function.
It also fixes some vector allocations that were much bigger than necessary.
Closes #61. Closes #55. Supersedes #56.