feat(nifs): support generic number of sangria c-markers #410
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
**Motivation** Within the sangria folding scheme we have two consistency markers that fold via multiplication by `r`. For use within cyclefold (#373) we need 9 of them, so we make support for an arbitrary markers len. Also the second problem was the availability of an accumulator for the step-circuit instance in nifs::sangria of the columns. Without changes - it would be necessary to do hash counting for an empty set within Cyclefold IVC as part of the sagnria check of the accumulator. **Overview** The first problem was solved by adding generics to all types The second problem was solved by wrapping the accumulator.step_circuit_hash_acc in `Option`, which always zeroes for step-circuit instances, if they not present. To make it more expressive as an `Option`, the custom type `SCInstancesHashAcc` was used - Also, to cure conflicts, I temporarily removed the ivc::cyclefold implementation - Also modules related to sangria were moved to the ivc::sangria submodule.
cyphersnake
force-pushed
the
feat-sangria
branch
from
b29445f to
72770d4
Compare
| )?; | ||
| ) { | ||
| SCInstancesHashAcc::Hash((lhs, rhs)) => { | ||
| SCInstancesHashAcc::Hash(gate.conditional_select(region, lhs, rhs, &condition)?) |
There was a problem hiding this comment.
can you remind me the meaning of lhs, rhs and what does this line try to do?
There was a problem hiding this comment.
In Rust, lhs (left-hand side) and rhs (right-hand side) are common
parameter names used in functions involving binary operations or comparisons.
You can find a lot of usage of it inside Rust stdlib
There was a problem hiding this comment.
I know the lhs, rhs. My question is: mathematically, what lhs and rhs correspond to, or what does this function try to do
There was a problem hiding this comment.
It's condition_select, we just choose one of them here
It's condition select use at the end of the sfc logic
For zero step it will be the input acc
For other steps it will be result of fold fn
| @@ -166,7 +166,7 @@ where | |||
| const R_P: usize = 3; | |||
|
|
|||
| let mut f_tr = RelaxedPlonkTrace { | |||
| U: RelaxedPlonkInstance::new(S.num_challenges, S.round_sizes.len()), | |||
| U: RelaxedPlonkInstance::new(S.num_challenges, S.round_sizes.len(), S.num_io.len() - 1), | |||
There was a problem hiding this comment.
as previous question, why choose S.num_io.len() - 1
There was a problem hiding this comment.
Because this fn arg is number of step circuit instances, not all instances len
| U: RelaxedPlonkInstance::new( | ||
| args.num_challenges, | ||
| args.num_witness, | ||
| args.num_io.len() - 1, // instances |
There was a problem hiding this comment.
why is args.num_io.len()-1 here?
There was a problem hiding this comment.
Because this fn arg is number of step circuit instances, not all instances len
There was a problem hiding this comment.
so the remain one is the accumulator instance?
Motivation
Within the sangria folding scheme we have two consistency markers that fold via multiplication by
r. For use within cyclefold (#373) we need 9 of them, so we make support for an arbitrary markers len.Also the second problem was the availability of an accumulator for the step-circuit instance in nifs::sangria of the columns. Without changes - it would be necessary to do hash counting for an empty set within Cyclefold IVC as part of the sagnria check of the accumulator.
Overview
The first problem was solved by adding generics to all types
The second problem was solved by wrapping the
accumulator.step_circuit_hash_acc in
Option, which always zeroes for step-circuit instances, if they not present. To make it more expressive as anOption, the custom typeSCInstancesHashAccwas usedAlso, to cure conflicts, I temporarily removed the ivc::cyclefold implementation