Factored per-query verification core algorithm #14
Conversation
There was a problem hiding this comment.
Thank you! Looks good! I left a few comments inline.
I also wonder if we should try to create much more specialized version(s) of this function. When we move it to Miden assembly, to make it as efficient as possible, it would probably make sense to use very concrete parameters and have several versions of the function. For example, there could be the following versions:
- Folding factor 2, extension degree 2
- Folding factor 2, extension degree 3
- Folding factor 4, extension degree 2
- etc.
We could start with one of these right now, and then generalize.
fri/src/verifier/mod.rs
Outdated
| fn iterate_through_query<B, E, H, const N: usize>( | ||
| layer_commitments: &Vec<H::Digest>, | ||
| folding_roots: &Vec<B>, | ||
| layer_alphas: &Vec<E>, | ||
| query: &Vec<(Vec<<H>::Digest>, [E; N])>, | ||
| position: usize, | ||
| num_partitions: usize, | ||
| folding_factor: usize, | ||
| number_of_layers: usize, | ||
| domain_size: usize, | ||
| evaluation: &E, | ||
| domain_generator: B, | ||
| max_degree_plus_1: usize, | ||
| domain_offset: B, | ||
| ) -> Result<(usize, E, usize, usize), VerifierError> |
There was a problem hiding this comment.
This looks great! A few comments:
- Let's get rid of
num_partitions- we can assume that it is always the same (1, I believe). - Instead of passing
queryas is now, we should passAdviseProviderstruct we've discussed in other places. This struct should provide the following functionality:
a.get_nodefunction as discussed before.
b.unhash_nodefunction which given a node value (Digest) returns a set of values which this digest represents (we can probably chat about this). - Do we need to pass
max_degree_plus_1into this function? It seems like it is not relevant to each query - i.e., we could do the check formax_degree_puls_1in a different function just once, rather than for each query. - Let's assume that
domain_offsetis hard-coded. So, we can create a constant for it rather than pass it as a parameter.
There was a problem hiding this comment.
Thank you Bobbin!
I think I have wrapped my mind on what we are trying to achieve.
Now for the implementation, should the AdviceProvider be generic over Digest? I think this is the most straightforward way as the AdviceProvider would need to be constructed inside the unbatch method of VerifierChannel.
There was a problem hiding this comment.
- Do we need to pass
max_degree_plus_1into this function? It seems like it is not relevant to each query - i.e., we could do the check formax_degree_puls_1in a different function just once, rather than for each query.
We don't need it if we remove the max_degree_plus_1 % N != 0 test.
fri/src/verifier/mod.rs
Outdated
| MerkleTree::<H>::verify(layer_commitments[depth], position_index, &query_proof) | ||
| .map_err(|_| VerifierError::LayerCommitmentMismatch)?; | ||
| num_hash += query_proof.len() - 1; | ||
|
|
||
| let query_value = query_values[cur_pos / target_domain_size]; |
There was a problem hiding this comment.
I think this is the part where we'd need to use the AdviceProvider to read the query value from it.
There was a problem hiding this comment.
I have now implemented a draft of the ideas we discussed. Let me know what you think.
I have merged the unhashing of the values to a node into the get_node method. I can extracted into a separate method once we agree on the design.
Next step is to implement concrete versions of the verification methods for different folding factors and field extension degrees. I propose that we start with the one we discussed, namely N == 2 & d == 2 .
Al-Kindi-0
force-pushed
the
main
branch
2 times, most recently
from
2496f01 to
eea409c
Compare
No description provided.