feat: implement provably extract LIMIT/OFFSET circuits (Part2)#300
feat: implement provably extract LIMIT/OFFSET circuits (Part2)#300Insun35 wants to merge 7 commits into
Conversation
Insun35
force-pushed
the
feat/implement-extraction-circuits
branch
2 times, most recently
from
438f966 to
cb09bb0
Compare
Insun35
force-pushed
the
feat/implement-extraction-circuits
branch
2 times, most recently
from
1c424f7 to
04aacfb
Compare
Insun35
force-pushed
the
feat/implement-extraction-circuits
branch
from
04aacfb to
7999666
Compare
| child_proof2.max_counter_target(), | ||
| subtree_proof.max_counter_target(), | ||
| ); | ||
|
|
There was a problem hiding this comment.
Aren't we missing the constraints to enforce that offset_range_min and offset_range_max are the same across all the proofs?
There was a problem hiding this comment.
And also the ones that all index_ids are the same?
| b: &mut CBuilder, | ||
| subtree_proof: &PublicInputs<Target>, | ||
| included_chid_proof: &PublicInputs<Target>, | ||
| excluded_child_proof: &PublicInputs<Target>, |
There was a problem hiding this comment.
These should correspond to Publicinputs of the results tree construction circuits
| subtree_proof.accumulator_target(), | ||
| included_chid_proof.accumulator_target(), | ||
| ]); | ||
|
|
There was a problem hiding this comment.
Same as full node: equality constrains on index_ids, offset_range_min and offset_range_max seems to be missing
| } | ||
|
|
||
| /// Subtree proof number = 1, child proof number = 2 | ||
| pub(crate) const NUM_VERIFIED_PROOFS: usize = 3; |
There was a problem hiding this comment.
This should be 2, since the excluded_child proof does not refer to a circuit of this set, but instead to the results tree construction set. Might be better to actually implement this trait in the APIs PR
| b: &mut CBuilder, | ||
| subtree_proof: &PublicInputs<Target>, | ||
| included_chid_proof: &PublicInputs<Target>, | ||
| sibling_proof: &PublicInputs<Target>, |
There was a problem hiding this comment.
Also here subtree_proof and sibling_proof should refer to the results tree construction circuits
| } | ||
|
|
||
| /// Subtree proof number = 1, child proof number = 2 | ||
| pub(crate) const NUM_VERIFIED_PROOFS: usize = 3; |
There was a problem hiding this comment.
This should be 1, but as for partial node circuit, might be better to implement this in the APIs PR
| ); | ||
| // assert max_left + 1 == p.min_counter | ||
| let left_plus_one = b.add(max_left, one); | ||
| b.connect(left_plus_one, subtree_proof.min_counter_target()); |
There was a problem hiding this comment.
Might be more efficient to do b.connect(max_left, min_minus_one)?
| // range specified by the query | ||
| // max_left < p.offset_range_max | ||
| let is_less = less_than(b, max_left, subtree_proof.offset_range_min_target(), 32); | ||
| b.connect(is_less.target, ttrue.target); |
There was a problem hiding this comment.
is_less should be true only if left_child_exists is true, I will also update the specs
| // range specified by the query | ||
| // min_right > p.offset_range_min | ||
| let is_greater = greater_than(b, min_right, subtree_proof.offset_range_max_target(), 32); | ||
| b.connect(is_greater.target, ttrue.target); |
There was a problem hiding this comment.
Similarly as above, is_greater should be true only if right_child_exists is true; will update the specs too
| &zero_u256.to_targets(), | ||
| &zero_u256.to_targets(), | ||
| &zero_u256.to_targets(), | ||
| &[one; 2], |
There was a problem hiding this comment.
Shouldn't these correspond to index_ids? So we might need to add those as witness inputs to the circuit
2 participants
Address issue #181
Refer to the results extraction circuits spec.
This PR should be merged after #288