zkevm-circuits
zkevm-circuits copied to clipboard
Simplify IsZero and BatchedIsZero
For IsZero gadget, we defined IsZeroConfig, IsZeroChip, and IsZeroInstruction. For BatchedIsZero gadget, we defined BatchedIsZeroChip and BatchedIsZeroConfig.
For each case, we can collect all relevant methods and keep only XConfig struct in the gadget. The rests are redundant abstractions.
So that eventually we will have only IsZeroConfig and BatchedIsZeroConfig
- For the
IsZerogadget. TheChiptrait did nothing really interesting. - For the
BatchIsZerogadget. We don't even implementChiptrait for it.
Originally posted by @ChihChengLiang in https://github.com/privacy-scaling-explorations/zkevm-circuits/pull/1397#discussion_r1190040038
I didn't make it clear in my https://github.com/privacy-scaling-explorations/zkevm-circuits/pull/1397#discussion_r1189332694.
What I mean is I couldn't find any xxxCircuitConfig struct having xxxChip defined member fields. Take StateCircuitConfig as example,
pub struct StateCircuitConfig<F> {
...
sort_keys: SortKeysConfig,
is_non_exist: BatchedIsZeroConfig,
lexicographic_ordering: LexicographicOrderingConfig,
lookups: LookupsConfig,
}
and we could also see in SortKeysConfig, tag is BinaryNumberConfig not BinaryNumberChip
pub struct SortKeysConfig {
tag: BinaryNumberConfig<RwTableTag, 4>,
...
}
So, if we declare push_data_left_is_zero in IsZeroConfig type in #1397, it's more consistent to me but it's not a big issue. Besides, I guess we need to decide if we need xxxChip first.
As for this issue, found something interesting while I went through all the gadgets,
- under gadgets/src/, except evm_word.rs, other gadgets implement both xxxChip and xxxConfig (evm_word.rs only has
WordConfig).WordConfighasconfigureandassign_word, but those methods are only available in xxxChip in other files. - some of gadgets in zkevm-circuits/src/state_circuit implement both xxxConfig and xxxChip but
LexicographicOrderingConfigdoesn't have xxxChip.
I think we need a rule for gadgets implementation, either having both xxxChip and xxxConfig (follow Halo2's practice) or make it simple, just having xxxConfig. I don't have any strong opions on either one.
We should keep it simple unless we really need a feature from a Chip implementation.