Use BinaryNumberChip in LexicographicOrderingConfig to lower degree

zkevm-circuits/src/state_circuit.rs

@@ -11,23 +11,20 @@ mod test;
 use crate::evm_circuit::{
     param::N_BYTES_WORD,
     table::RwTableTag,
-    util::RandomLinearCombination,
     witness::{Rw, RwMap},
 };
+use crate::util::Expr;
 use binary_number::{Chip as BinaryNumberChip, Config as BinaryNumberConfig};
 use constraint_builder::{ConstraintBuilder, Queries};
-use eth_types::{Address, Field, ToLittleEndian};
-use gadgets::is_zero::{IsZeroChip, IsZeroConfig, IsZeroInstruction};
+use eth_types::{Address, Field};
 use halo2_proofs::{
     circuit::{Layouter, SimpleFloorPlanner},
     plonk::{
         Advice, Circuit, Column, ConstraintSystem, Error, Expression, Fixed, Instance, VirtualCells,
     },
     poly::Rotation,
 };
-use lexicographic_ordering::{
-    Chip as LexicographicOrderingChip, Config as LexicographicOrderingConfig,
-};
+use lexicographic_ordering::Config as LexicographicOrderingConfig;
 use lookups::{Chip as LookupsChip, Config as LookupsConfig, Queries as LookupsQueries};
 use multiple_precision_integer::{Chip as MpiChip, Config as MpiConfig, Queries as MpiQueries};
 use random_linear_combination::{Chip as RlcChip, Config as RlcConfig, Queries as RlcQueries};
@@ -40,23 +37,27 @@ const N_LIMBS_ACCOUNT_ADDRESS: usize = 10;
 const N_LIMBS_ID: usize = 2;
 
 /// Config for StateCircuit
-#[derive(Clone)]
-pub struct StateConfig<F: Field> {
+#[derive(Clone, Copy)]
+pub struct StateConfig {
     selector: Column<Fixed>, // Figure out why you get errors when this is Selector.
     // https://github.com/privacy-scaling-explorations/zkevm-circuits/issues/407
-    rw_counter: MpiConfig<u32, N_LIMBS_RW_COUNTER>,
+    sort_keys: SortKeysConfig,
     is_write: Column<Advice>,
+    value: Column<Advice>,
+    lexicographic_ordering: LexicographicOrderingConfig,
+    lookups: LookupsConfig,
+    power_of_randomness: [Column<Instance>; N_BYTES_WORD - 1],
+}
+
+/// Keys for sorting the rows of the state circuit
+#[derive(Clone, Copy)]
+pub struct SortKeysConfig {
     tag: BinaryNumberConfig<RwTableTag, 4>,
     id: MpiConfig<u32, N_LIMBS_ID>,
-    is_id_unchanged: IsZeroConfig<F>,
     address: MpiConfig<Address, N_LIMBS_ACCOUNT_ADDRESS>,
     field_tag: Column<Advice>,
     storage_key: RlcConfig<N_BYTES_WORD>,
-    is_storage_key_unchanged: IsZeroConfig<F>,
-    value: Column<Advice>,
-    lookups: LookupsConfig,
-    power_of_randomness: [Column<Instance>; N_BYTES_WORD - 1],
-    lexicographic_ordering: LexicographicOrderingConfig<F>,
+    rw_counter: MpiConfig<u32, N_LIMBS_RW_COUNTER>,
 }
 
 type Lookup<F> = (&'static str, Expression<F>, Expression<F>);
@@ -77,9 +78,9 @@ impl<F: Field, const N_ROWS: usize> StateCircuit<F, N_ROWS> {
         rows.sort_by_key(|row| {
             (
                 row.tag() as u64,
-                row.field_tag().unwrap_or_default(),
                 row.id().unwrap_or_default(),
                 row.address().unwrap_or_default(),
+                row.field_tag().unwrap_or_default(),
                 row.storage_key().unwrap_or_default(),
                 row.rw_counter(),
             )
@@ -101,7 +102,7 @@ impl<F: Field, const N_ROWS: usize> StateCircuit<F, N_ROWS> {
 }
 
 impl<F: Field, const N_ROWS: usize> Circuit<F> for StateCircuit<F, N_ROWS> {
-    type Config = StateConfig<F>;
+    type Config = StateConfig;
     type FloorPlanner = SimpleFloorPlanner;
 
     fn without_witnesses(&self) -> Self {
@@ -113,8 +114,7 @@ impl<F: Field, const N_ROWS: usize> Circuit<F> for StateCircuit<F, N_ROWS> {
         let lookups = LookupsChip::configure(meta);
         let power_of_randomness = [0; N_BYTES_WORD - 1].map(|_| meta.instance_column());
 
-        let [is_write, field_tag, value, is_id_unchanged_column, is_storage_key_unchanged_column] =
-            [0; 5].map(|_| meta.advice_column());
+        let [is_write, field_tag, value] = [0; 3].map(|_| meta.advice_column());
 
         let tag = BinaryNumberChip::configure(meta, selector);
 
@@ -123,49 +123,28 @@ impl<F: Field, const N_ROWS: usize> Circuit<F> for StateCircuit<F, N_ROWS> {
         let storage_key = RlcChip::configure(meta, selector, lookups.u8, power_of_randomness);
         let rw_counter = MpiChip::configure(meta, selector, lookups.u16);
 
-        let lexicographic_ordering = LexicographicOrderingChip::configure(
-            meta,
+        let sort_keys = SortKeysConfig {
             tag,
+            id,
             field_tag,
-            id.limbs,
-            address.limbs,
-            storage_key.bytes,
-            rw_counter.limbs,
-            lookups.u16,
-        );
+            address,
+            storage_key,
+            rw_counter,
+        };
 
-        let is_id_unchanged = IsZeroChip::configure(
-            meta,
-            |meta| meta.query_fixed(lexicographic_ordering.selector, Rotation::cur()),
-            |meta| {
-                meta.query_advice(id.value, Rotation::cur())
-                    - meta.query_advice(id.value, Rotation::prev())
-            },
-            is_id_unchanged_column,
-        );
-        let is_storage_key_unchanged = IsZeroChip::configure(
+        let lexicographic_ordering = LexicographicOrderingConfig::configure(
             meta,
-            |meta| meta.query_fixed(lexicographic_ordering.selector, Rotation::cur()),
-            |meta| {
-                meta.query_advice(storage_key.encoded, Rotation::cur())
-                    - meta.query_advice(storage_key.encoded, Rotation::prev())
-            },
-            is_storage_key_unchanged_column,
+            sort_keys,
+            lookups.u16,
+            power_of_randomness,
         );
 
         let config = Self::Config {
             selector,
-            rw_counter,
+            sort_keys,
             is_write,
-            tag,
-            id,
-            is_id_unchanged,
-            address,
-            field_tag,
-            storage_key,
             value,
             lexicographic_ordering,
-            is_storage_key_unchanged,
             lookups,
             power_of_randomness,
         };
@@ -190,13 +169,7 @@ impl<F: Field, const N_ROWS: usize> Circuit<F> for StateCircuit<F, N_ROWS> {
     ) -> Result<(), Error> {
         LookupsChip::construct(config.lookups).load(&mut layouter)?;
 
-        let is_id_unchanged = IsZeroChip::construct(config.is_id_unchanged.clone());
-        let is_storage_key_unchanged =
-            IsZeroChip::construct(config.is_storage_key_unchanged.clone());
-        let lexicographic_ordering_chip =
-            LexicographicOrderingChip::construct(config.lexicographic_ordering.clone());
-
-        let tag_chip = BinaryNumberChip::construct(config.tag);
+        let tag_chip = BinaryNumberChip::construct(config.sort_keys.tag);
 
         layouter.assign_region(
             || "rw table",
@@ -209,9 +182,11 @@ impl<F: Field, const N_ROWS: usize> Circuit<F> for StateCircuit<F, N_ROWS> {
 
                 for (offset, (row, prev_row)) in rows.zip(prev_rows).enumerate() {
                     region.assign_fixed(|| "selector", config.selector, offset, || Ok(F::one()))?;
-                    config
-                        .rw_counter
-                        .assign(&mut region, offset, row.rw_counter() as u32)?;
+                    config.sort_keys.rw_counter.assign(
+                        &mut region,
+                        offset,
+                        row.rw_counter() as u32,
+                    )?;
                     region.assign_advice(
                         || "is_write",
                         config.is_write,
@@ -220,21 +195,24 @@ impl<F: Field, const N_ROWS: usize> Circuit<F> for StateCircuit<F, N_ROWS> {
                     )?;
                     tag_chip.assign(&mut region, offset, &row.tag())?;
                     if let Some(id) = row.id() {
-                        config.id.assign(&mut region, offset, id as u32)?;
+                        config.sort_keys.id.assign(&mut region, offset, id as u32)?;
                     }
                     if let Some(address) = row.address() {
-                        config.address.assign(&mut region, offset, address)?;
+                        config
+                            .sort_keys
+                            .address
+                            .assign(&mut region, offset, address)?;
                     }
                     if let Some(field_tag) = row.field_tag() {
                         region.assign_advice(
                             || "field_tag",
-                            config.field_tag,
+                            config.sort_keys.field_tag,
                             offset,
                             || Ok(F::from(field_tag as u64)),
                         )?;
                     }
                     if let Some(storage_key) = row.storage_key() {
-                        config.storage_key.assign(
+                        config.sort_keys.storage_key.assign(
                             &mut region,
                             offset,
                             self.randomness,
@@ -249,23 +227,11 @@ impl<F: Field, const N_ROWS: usize> Circuit<F> for StateCircuit<F, N_ROWS> {
                     )?;
 
                     if let Some(prev_row) = prev_row {
-                        lexicographic_ordering_chip.assign(&mut region, offset, &row, &prev_row)?;
-
-                        let id_change = F::from(row.id().unwrap_or_default() as u64)
-                            - F::from(prev_row.id().unwrap_or_default() as u64);
-                        is_id_unchanged.assign(&mut region, offset, Some(id_change))?;
-
-                        let storage_key_change = RandomLinearCombination::random_linear_combine(
-                            row.storage_key().unwrap_or_default().to_le_bytes(),
-                            self.randomness,
-                        ) - RandomLinearCombination::random_linear_combine(
-                            prev_row.storage_key().unwrap_or_default().to_le_bytes(),
-                            self.randomness,
-                        );
-                        is_storage_key_unchanged.assign(
+                        config.lexicographic_ordering.assign(
                             &mut region,
                             offset,
-                            Some(storage_key_change),
+                            &row,
+                            &prev_row,
                         )?;
                     }
                 }
@@ -285,33 +251,69 @@ impl<F: Field, const N_ROWS: usize> Circuit<F> for StateCircuit<F, N_ROWS> {
     }
 }
 
-fn queries<F: Field>(meta: &mut VirtualCells<'_, F>, c: &StateConfig<F>) -> Queries<F> {
+fn queries<F: Field>(meta: &mut VirtualCells<'_, F>, c: &StateConfig) -> Queries<F> {
+    let final_bits_sum = meta.query_advice(
+        c.lexicographic_ordering.first_different_limb.bits[3],
+        Rotation::cur(),
+    ) + meta.query_advice(
+        c.lexicographic_ordering.first_different_limb.bits[4],
+        Rotation::cur(),
+    );
+
     Queries {
         selector: meta.query_fixed(c.selector, Rotation::cur()),
         lexicographic_ordering_selector: meta
             .query_fixed(c.lexicographic_ordering.selector, Rotation::cur()),
-        rw_counter: MpiQueries::new(meta, c.rw_counter),
+        rw_counter: MpiQueries::new(meta, c.sort_keys.rw_counter),
         is_write: meta.query_advice(c.is_write, Rotation::cur()),
-        tag: c.tag.value(Rotation::cur())(meta),
+        tag: c.sort_keys.tag.value(Rotation::cur())(meta),
         tag_bits: c
+            .sort_keys
             .tag
             .bits
             .map(|bit| meta.query_advice(bit, Rotation::cur())),
-        id: MpiQueries::new(meta, c.id),
-        is_id_unchanged: c.is_id_unchanged.is_zero_expression.clone(),
-        address: MpiQueries::new(meta, c.address),
-        field_tag: meta.query_advice(c.field_tag, Rotation::cur()),
-        storage_key: RlcQueries::new(meta, c.storage_key),
+        id: MpiQueries::new(meta, c.sort_keys.id),
+        // this isn't binary! only 0 if most significant 3 bits are all 0 and at most 1 of the two
+        // least significant bits is 1.
+        // TODO: this can mask off just the top 3 bits if you want, since the 4th limb index is
+        // Address9, which is always 0 for Rw::Stack rows.
+        is_tag_and_id_unchanged: 4.expr()
+            * (meta.query_advice(
+                c.lexicographic_ordering.first_different_limb.bits[0],
+                Rotation::cur(),
+            ) + meta.query_advice(
+                c.lexicographic_ordering.first_different_limb.bits[1],
+                Rotation::cur(),
+            ) + meta.query_advice(
+                c.lexicographic_ordering.first_different_limb.bits[2],
+                Rotation::cur(),
+            ))
+            + final_bits_sum.clone() * (1.expr() - final_bits_sum),
+        address: MpiQueries::new(meta, c.sort_keys.address),
+        field_tag: meta.query_advice(c.sort_keys.field_tag, Rotation::cur()),
+        storage_key: RlcQueries::new(meta, c.sort_keys.storage_key),
         value: meta.query_advice(c.value, Rotation::cur()),
         lookups: LookupsQueries::new(meta, c.lookups),
         power_of_randomness: c
             .power_of_randomness
             .map(|c| meta.query_instance(c, Rotation::cur())),
-        is_storage_key_unchanged: c.is_storage_key_unchanged.is_zero_expression.clone(),
-        lexicographic_ordering_upper_limb_difference_is_zero: c
-            .lexicographic_ordering
-            .upper_limb_difference_is_zero
-            .is_zero_expression
-            .clone(),
+        // this isn't binary! only 0 if most significant 4 bits are all 1.
+        first_access: 4.expr()
+            - meta.query_advice(
+                c.lexicographic_ordering.first_different_limb.bits[0],
+                Rotation::cur(),
+            )
+            - meta.query_advice(
+                c.lexicographic_ordering.first_different_limb.bits[1],
+                Rotation::cur(),
+            )
+            - meta.query_advice(
+                c.lexicographic_ordering.first_different_limb.bits[2],
+                Rotation::cur(),
+            )
+            - meta.query_advice(
+                c.lexicographic_ordering.first_different_limb.bits[3],
+                Rotation::cur(),
+            ),
     }
 }

zkevm-circuits/src/state_circuit/binary_number.rs

@@ -169,7 +169,7 @@ where
     }
 }
 
-fn from_bits(bits: &[bool]) -> usize {
+pub fn from_bits(bits: &[bool]) -> usize {
     bits.iter()
         .fold(0, |result, &bit| bit as usize + 2 * result)
 }

zkevm-circuits/src/state_circuit/constraint_builder.rs

@@ -7,7 +7,7 @@ use super::{
 use crate::evm_circuit::{
     param::N_BYTES_WORD,
     table::{AccountFieldTag, RwTableTag},
-    util::{not, or},
+    util::not,
 };
 use crate::util::Expr;
 use eth_types::Field;
@@ -23,15 +23,14 @@ pub struct Queries<F: Field> {
     pub tag: Expression<F>,
     pub tag_bits: [Expression<F>; 4],
     pub id: MpiQueries<F, N_LIMBS_ID>,
-    pub is_id_unchanged: Expression<F>,
+    pub is_tag_and_id_unchanged: Expression<F>,
     pub address: MpiQueries<F, N_LIMBS_ACCOUNT_ADDRESS>,
     pub field_tag: Expression<F>,
     pub storage_key: RlcQueries<F, N_BYTES_WORD>,
     pub value: Expression<F>,
     pub lookups: LookupsQueries<F>,
     pub power_of_randomness: [Expression<F>; N_BYTES_WORD - 1],
-    pub is_storage_key_unchanged: Expression<F>,
-    pub lexicographic_ordering_upper_limb_difference_is_zero: Expression<F>,
+    pub first_access: Expression<F>,
 }
 
 type Constraint<F> = (&'static str, Expression<F>);
@@ -144,9 +143,9 @@ impl<F: Field> ConstraintBuilder<F> {
             "stack address fits into 10 bits",
             (q.address.value.clone(), q.lookups.u10.clone()),
         );
-        self.condition(q.is_id_unchanged.clone(), |cb| {
+        self.condition(q.is_tag_and_id_unchanged.clone(), |cb| {
             cb.require_boolean(
-                "if call id is the same, address change is 0 or 1",
+                "if previous row is also Stack with unchanged call id, address change is 0 or 1",
                 q.address_change(),
             )
         });
@@ -304,13 +303,7 @@ impl<F: Field> Queries<F> {
     }
 
     fn first_access(&self) -> Expression<F> {
-        or::expr(&[
-            not::expr(
-                self.lexicographic_ordering_upper_limb_difference_is_zero
-                    .clone(),
-            ),
-            not::expr(self.is_storage_key_unchanged.clone()),
-        ])
+        self.first_access.clone()
     }
 
     fn address_change(&self) -> Expression<F> {