custom plonky2 gate: SwitchGate
#5
Conversation
WalkthroughThe recent updates introduce a novel module for handling switch gates and significantly enhance the functionality of permutation circuits within a Plonky2-based system. The addition of Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
Cargo.tomlis excluded by:!**/*.tomlzk-plonky2-permutation-circuit/Cargo.tomlis excluded by:!**/*.toml
Files selected for processing (3)
- zk-plonky2-permutation-circuit/src/custom_gates/mod.rs (1 hunks)
- zk-plonky2-permutation-circuit/src/custom_gates/switch.rs (1 hunks)
- zk-plonky2-permutation-circuit/src/lib.rs (1 hunks)
Additional comments: 4
zk-plonky2-permutation-circuit/src/custom_gates/mod.rs (1)
- 1-1: The module declaration for
switchis correctly added. Ensure that theswitchmodule is properly integrated and utilized within the project.Verification successful
To ensure a comprehensive verification, let's expand the search to include potential indirect references or configurations that might not use the
switch::syntax directly. This will help us determine if theswitchmodule is integrated into the project in some form.
The search results confirm that the
switchmodule is properly integrated and utilized within the project, particularly in the creation and management of switch circuits as evidenced by its active use inlib.rsand detailed implementation inswitch.rs. This addresses the initial concern regarding the module's integration.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the `switch` module is imported and used in other parts of the project. rg --type rust 'switch::'Length of output: 25
Script:
#!/bin/bash # Search for any file mentioning the 'switch' module, which might indicate indirect usage or configuration. rg --type rust 'switch' # Additionally, check if there are any dynamic loading mechanisms that could use the module without direct reference. rg --type rust 'dynamic'Length of output: 2369
zk-plonky2-permutation-circuit/src/lib.rs (1)
- 10-39: The
assert_permutation_circuitfunction is well-structured with clear input validation. However, ensure that the recursive call in the case of larger lists has a well-defined base case to prevent infinite recursion.zk-plonky2-permutation-circuit/src/custom_gates/switch.rs (2)
- 28-70: The evaluation methods
eval_unfilteredandeval_unfiltered_circuitare correctly implemented, accurately reflecting the logic of a switch gate. Good use ofVec::with_capacity(4)for optimizing memory allocation.- 82-96: The metadata methods (
num_wires,num_constants,degree, andnum_constraints) are correctly implemented, providing accurate information about theSwitchGate.
| /// Assert that [a1, a2] is a permutation of [b1, b2]. | ||
| fn assert_permutation_2x2_circuit<F: RichField + Extendable<D>, const D: usize>( | ||
| builder: &mut CircuitBuilder<F, D>, | ||
| a1: Target, | ||
| a2: Target, | ||
| b1: Target, | ||
| b2: Target, | ||
| ) { | ||
| let (_switch, gate_out1, gate_out2) = create_switch_circuit(builder, a1, a2); | ||
| for e in 0..chunk_size { | ||
| builder.connect(b1[e], gate_out1[e]); | ||
| builder.connect(b2[e], gate_out2[e]); | ||
| } | ||
| } |
There was a problem hiding this comment.
The use of an undefined chunk_size variable will cause a compilation error. Ensure that chunk_size is properly defined and passed to the assert_permutation_2x2_circuit function.
- for e in 0..chunk_size {
+ // Assuming chunk_size is intended to be passed as a parameter or defined within the function
+ let chunk_size = /* appropriate value */;
+ for e in 0..chunk_size {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| /// Assert that [a1, a2] is a permutation of [b1, b2]. | |
| fn assert_permutation_2x2_circuit<F: RichField + Extendable<D>, const D: usize>( | |
| builder: &mut CircuitBuilder<F, D>, | |
| a1: Target, | |
| a2: Target, | |
| b1: Target, | |
| b2: Target, | |
| ) { | |
| let (_switch, gate_out1, gate_out2) = create_switch_circuit(builder, a1, a2); | |
| for e in 0..chunk_size { | |
| builder.connect(b1[e], gate_out1[e]); | |
| builder.connect(b2[e], gate_out2[e]); | |
| } | |
| } | |
| /// Assert that [a1, a2] is a permutation of [b1, b2]. | |
| fn assert_permutation_2x2_circuit<F: RichField + Extendable<D>, const D: usize>( | |
| builder: &mut CircuitBuilder<F, D>, | |
| a1: Target, | |
| a2: Target, | |
| b1: Target, | |
| b2: Target, | |
| ) { | |
| let (_switch, gate_out1, gate_out2) = create_switch_circuit(builder, a1, a2); | |
| // Assuming chunk_size is intended to be passed as a parameter or defined within the function | |
| let chunk_size = /* appropriate value */; | |
| for e in 0..chunk_size { | |
| builder.connect(b1[e], gate_out1[e]); | |
| builder.connect(b2[e], gate_out2[e]); | |
| } | |
| } |
| /// Given two input wire chunks, add a new switch to the circuit (by adding one copy to a switch | ||
| /// gate). Returns the wire for the switch boolean, and the two output wire chunks. | ||
| fn create_switch_circuit<F: RichField + Extendable<D>, const D: usize>( | ||
| builder: &mut CircuitBuilder<F, D>, | ||
| a1: Target, | ||
| a2: Target, | ||
| ) -> (Target, Target, Target) { | ||
|
|
||
| let chunk_size = a1.len(); | ||
|
|
||
| let gate = SwitchGate::new_from_config(&builder.config, chunk_size); | ||
| let params = vec![F::from_canonical_usize(chunk_size)]; | ||
| let (row, next_copy) = builder.find_slot(gate, ¶ms, &[]); | ||
|
|
||
| let mut c = Vec::new(); | ||
| let mut d = Vec::new(); | ||
| for e in 0..chunk_size { | ||
| builder.connect( | ||
| a1[e], | ||
| Target::wire(row, gate.wire_first_input(next_copy, e)), | ||
| ); | ||
| builder.connect( | ||
| a2[e], | ||
| Target::wire(row, gate.wire_second_input(next_copy, e)), | ||
| ); | ||
| c.push(Target::wire(row, gate.wire_first_output(next_copy, e))); | ||
| d.push(Target::wire(row, gate.wire_second_output(next_copy, e))); | ||
| } | ||
|
|
||
| let switch = Target::wire(row, gate.wire_switch_bool(next_copy)); | ||
|
|
||
| (switch, c, d) | ||
| } |
There was a problem hiding this comment.
The function create_switch_circuit assumes a1 and a2 to be vectors due to the use of a1.len(), which contradicts their type as Target. Ensure that the inputs are correctly handled according to their actual types.
- let chunk_size = a1.len();
+ // Correct the handling of `a1` and `a2` according to their actual types.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| /// Given two input wire chunks, add a new switch to the circuit (by adding one copy to a switch | |
| /// gate). Returns the wire for the switch boolean, and the two output wire chunks. | |
| fn create_switch_circuit<F: RichField + Extendable<D>, const D: usize>( | |
| builder: &mut CircuitBuilder<F, D>, | |
| a1: Target, | |
| a2: Target, | |
| ) -> (Target, Target, Target) { | |
| let chunk_size = a1.len(); | |
| let gate = SwitchGate::new_from_config(&builder.config, chunk_size); | |
| let params = vec![F::from_canonical_usize(chunk_size)]; | |
| let (row, next_copy) = builder.find_slot(gate, ¶ms, &[]); | |
| let mut c = Vec::new(); | |
| let mut d = Vec::new(); | |
| for e in 0..chunk_size { | |
| builder.connect( | |
| a1[e], | |
| Target::wire(row, gate.wire_first_input(next_copy, e)), | |
| ); | |
| builder.connect( | |
| a2[e], | |
| Target::wire(row, gate.wire_second_input(next_copy, e)), | |
| ); | |
| c.push(Target::wire(row, gate.wire_first_output(next_copy, e))); | |
| d.push(Target::wire(row, gate.wire_second_output(next_copy, e))); | |
| } | |
| let switch = Target::wire(row, gate.wire_switch_bool(next_copy)); | |
| (switch, c, d) | |
| } | |
| /// Given two input wire chunks, add a new switch to the circuit (by adding one copy to a switch | |
| /// gate). Returns the wire for the switch boolean, and the two output wire chunks. | |
| fn create_switch_circuit<F: RichField + Extendable<D>, const D: usize>( | |
| builder: &mut CircuitBuilder<F, D>, | |
| a1: Target, | |
| a2: Target, | |
| ) -> (Target, Target, Target) { | |
| // Correct the handling of `a1` and `a2` according to their actual types. | |
| let gate = SwitchGate::new_from_config(&builder.config, chunk_size); | |
| let params = vec![F::from_canonical_usize(chunk_size)]; | |
| let (row, next_copy) = builder.find_slot(gate, ¶ms, &[]); | |
| let mut c = Vec::new(); | |
| let mut d = Vec::new(); | |
| for e in 0..chunk_size { | |
| builder.connect( | |
| a1[e], | |
| Target::wire(row, gate.wire_first_input(next_copy, e)), | |
| ); | |
| builder.connect( | |
| a2[e], | |
| Target::wire(row, gate.wire_second_input(next_copy, e)), | |
| ); | |
| c.push(Target::wire(row, gate.wire_first_output(next_copy, e))); | |
| d.push(Target::wire(row, gate.wire_second_output(next_copy, e))); | |
| } | |
| let switch = Target::wire(row, gate.wire_switch_bool(next_copy)); | |
| (switch, c, d) | |
| } |
| use std::marker::PhantomData; | ||
|
|
||
| use plonky2::{gates::gate::Gate, hash::hash_types::RichField, iop::generator::WitnessGeneratorRef, util::serialization::Write}; | ||
| use plonky2_field::extension::Extendable; | ||
|
|
||
|
|
||
| /// A gate for conditionally swapping input values based on a boolean. | ||
| #[derive(Copy, Clone, Debug)] | ||
| pub struct SwitchGate<F: RichField + Extendable<D>, const D: usize> { | ||
| _phantom: PhantomData<F> | ||
| } | ||
|
|
||
| impl<F: RichField + Extendable<D>, const D: usize> Gate<F, D> for SwitchGate<F, D> { | ||
| fn id(&self) -> String { | ||
| format!("{self:?}<D={D}>") | ||
| } | ||
|
|
||
| fn serialize(&self, dst: &mut Vec<u8>, common_data: &plonky2::plonk::circuit_data::CommonCircuitData<F, D>) -> plonky2::util::serialization::IoResult<()> { | ||
| dst.write_bool(false) // TODO: remove | ||
| } | ||
|
|
||
| fn deserialize(src: &mut plonky2::util::serialization::Buffer, common_data: &plonky2::plonk::circuit_data::CommonCircuitData<F, D>) -> plonky2::util::serialization::IoResult<Self> | ||
| where | ||
| Self: Sized { | ||
| Ok(Self) | ||
| } |
There was a problem hiding this comment.
The SwitchGate structure and its basic implementation are correctly defined. However, the serialization method contains a placeholder implementation with a TODO comment. Ensure that the serialization functionality is fully implemented and tested.
Would you like assistance in completing the serialization method?
| fn generators(&self, row: usize, local_constants: &[F]) -> Vec<WitnessGeneratorRef<F, D>> { | ||
| let g: Box<WitnessGeneratorRef<F, D>> = Box::new(SwitchGenerator::<F, D>{ | ||
| row, | ||
| gate: *self, | ||
| copy: c, | ||
| }); | ||
| g | ||
| } |
There was a problem hiding this comment.
The generators method contains a reference to an undefined variable c, which will cause a compilation error. Ensure that the witness generators are correctly implemented and that all necessary variables are defined.
- copy: c,
+ // Ensure that the correct variable is passed here.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| fn generators(&self, row: usize, local_constants: &[F]) -> Vec<WitnessGeneratorRef<F, D>> { | |
| let g: Box<WitnessGeneratorRef<F, D>> = Box::new(SwitchGenerator::<F, D>{ | |
| row, | |
| gate: *self, | |
| copy: c, | |
| }); | |
| g | |
| } | |
| fn generators(&self, row: usize, local_constants: &[F]) -> Vec<WitnessGeneratorRef<F, D>> { | |
| let g: Box<WitnessGeneratorRef<F, D>> = Box::new(SwitchGenerator::<F, D>{ | |
| row, | |
| gate: *self, | |
| // Ensure that the correct variable is passed here. | |
| }); | |
| g | |
| } |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
zk-plonky2-permutation-circuit/Cargo.tomlis excluded by:!**/*.toml
Files selected for processing (2)
- zk-plonky2-permutation-circuit/src/custom_gates/switch.rs (1 hunks)
- zk-plonky2-permutation-circuit/src/lib.rs (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- zk-plonky2-permutation-circuit/src/custom_gates/switch.rs
- zk-plonky2-permutation-circuit/src/lib.rs
Summary by CodeRabbit
SwitchGatefor conditionally swapping inputs in permutation circuits.