zkevm-circuits icon indicating copy to clipboard operation
zkevm-circuits copied to clipboard

Simplify IsZero and BatchedIsZero

Open ChihChengLiang opened this issue 2 years ago • 2 comments

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 IsZero gadget. The Chip trait did nothing really interesting.
  • For the BatchIsZero gadget. We don't even implement Chip trait for it.

Originally posted by @ChihChengLiang in https://github.com/privacy-scaling-explorations/zkevm-circuits/pull/1397#discussion_r1190040038

ChihChengLiang avatar May 10 '23 15:05 ChihChengLiang

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,

  1. under gadgets/src/, except evm_word.rs, other gadgets implement both xxxChip and xxxConfig (evm_word.rs only has WordConfig). WordConfig has configure and assign_word, but those methods are only available in xxxChip in other files.
  2. some of gadgets in zkevm-circuits/src/state_circuit implement both xxxConfig and xxxChip but LexicographicOrderingConfig doesn'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.

KimiWu123 avatar May 11 '23 05:05 KimiWu123

We should keep it simple unless we really need a feature from a Chip implementation.

ChihChengLiang avatar May 11 '23 14:05 ChihChengLiang