Conversation
Prover refactoring
…whir into verifier-improvements
| ); | ||
| } | ||
| } | ||
| pub mod lagrange_iterator; |
There was a problem hiding this comment.
Why are these at the end? This file seems to have some conflicts.
| } | ||
|
|
||
| pub struct CommitmentReader<'a, F, MerkleConfig, PowStrategy>( | ||
| // TODO: Refactor to use this. |
There was a problem hiding this comment.
You can even just remove it no?
| folded_domain_size: usize, | ||
| } | ||
|
|
||
| type ToBeReplaced<F> = ( |
There was a problem hiding this comment.
Ah yes, I forgot about this. Will remove.
WizardOfMenlo
left a comment
WizardOfMenlo
left a comment
There was a problem hiding this comment.
I think let's wait a bit to merge, I think things are still in a bit of a rough stage, left some comments.
| // #[derive(Debug, Clone, PartialEq, Eq)] | ||
| // pub struct MultilinearPoint<F>(pub Vec<F>); | ||
|
|
||
| pub fn eq_poly<F>(coords: &MultilinearPoint<F>, point: BinaryHypercubePoint) -> F |
There was a problem hiding this comment.
This now lives in multilinear. Please go ahead and double checked that these are not duplicated.
| ProofResult, | ||
| }; | ||
|
|
||
| use crate::whir::utils::DigestToUnitDeserialize; |
There was a problem hiding this comment.
Maybe we can move this to a common fs_utils/util module? Because it's now WHIR specific anymore.
There was a problem hiding this comment.
Yes, I agree. There is a lot of streamlining and logic separation to do, and I just thought it would be nice to deal with it separately after this is merged. I could do it in this PR if you prefer, though. lmk
| F: FftField, | ||
| MerkleConfig: Config<Leaf = [F]>, | ||
| VerifierState: | ||
| FieldToUnitDeserialize<F> + UnitToField<F> + DigestToUnitDeserialize<MerkleConfig>, |
There was a problem hiding this comment.
Trait bound here can be simplified
| } | ||
| } | ||
|
|
||
| impl Default for CommitmentReader { |
| &ctx.prev_root, | ||
| virtual_evals.iter().map(|a| a.as_ref()), | ||
| ) | ||
| .unwrap() |
There was a problem hiding this comment.
This is done for the final round, so it might be necessary here, too. I don’t yet understand the code well enough to be sure.
| .unwrap() | |
| .unwrap() || merkle_proof.leaf_indexes != r_shift_indexes |
There was a problem hiding this comment.
I suppose the merkle_proof.leaf_indexes are implicitly checked later since the corresponding points become part of the quotient set for that round. However, the final_merkle_proof.leaf_indexes are also implicitly checked against the final_r_shift_indexes later, since the latter are used to get the final_evaluations, which are then compared to the final_folds.
So it seems to me that checking the correctness of (final_)merkle_proof.leaf_indexes is redundant in any case, but doing so might make the error cleaner in case of verification failure. If that cleaner error is irrelevant, then I don’t think the check is necessary at all; if the cleaner error is the reason the check exists for the final indices, I’d argue the code becomes more consistent when my suggestion above is included.
There was a problem hiding this comment.
Side note: feel free to let me know in case this PR is stale, then I’ll stop necroposting.
3 participants
No description provided.