better generic design

keccak256/src/gate_helpers.rs

@@ -1,7 +1,7 @@
 use eth_types::Field;
 use num_bigint::BigUint;
 
-/// Convert a bigUint value to FieldExt
+/// Convert a bigUint value to Field
 ///
 /// We assume the input value is smaller than the field size
 pub fn biguint_to_f<F: Field>(x: &BigUint) -> F {

zkevm-circuits/src/evm_circuit/execution.rs

@@ -197,7 +197,7 @@ pub(crate) trait ExecutionGadget<F: Field> {
 }
 
 #[derive(Clone, Debug)]
-pub(crate) struct ExecutionConfig<F, T> {
+pub(crate) struct ExecutionConfig<F> {
     // EVM Circuit selector, which enables all usable rows.  The rows where this selector is
     // disabled won't verify any constraint (they can be unused rows or rows with blinding
     // factors).
@@ -312,7 +312,7 @@ pub(crate) struct ExecutionConfig<F, T> {
     error_return_data_out_of_bound: Box<ErrorReturnDataOutOfBoundGadget<F>>,
 }
 
-impl<F: Field, T: WordExpr<F>> ExecutionConfig<F, T> {
+impl<F: Field> ExecutionConfig<F> {
     #[allow(clippy::too_many_arguments)]
     #[allow(clippy::redundant_closure_call)]
     pub(crate) fn configure(

zkevm-circuits/src/evm_circuit/execution/add_sub.rs

@@ -10,7 +10,10 @@ use crate::{
         },
         witness::{Block, Call, ExecStep, Transaction},
     },
-    util::{word::Word, Expr},
+    util::{
+        word::{Word, WordExpr},
+        Expr,
+    },
 };
 use bus_mapping::evm::OpcodeId;
 use eth_types::Field;

zkevm-circuits/src/evm_circuit/execution/addmod.rs

@@ -46,8 +46,8 @@ pub(crate) struct AddModGadget<F> {
     muladd_d_n_r: MulAddWords512Gadget<F>,
 
     n_is_zero: IsZeroWordGadget<F>,
-    cmp_r_n: CmpWordsGadget<F, Word32Cell<F>>,
-    cmp_areduced_n: CmpWordsGadget<F, Word32Cell<F>>,
+    cmp_r_n: CmpWordsGadget<F, Word32Cell<F>, Word32Cell<F>>,
+    cmp_areduced_n: CmpWordsGadget<F, Word32Cell<F>, Word32Cell<F>>,
 }
 
 impl<F: Field> ExecutionGadget<F> for AddModGadget<F> {

zkevm-circuits/src/evm_circuit/util.rs

@@ -506,15 +506,15 @@ impl<F: Field, const N: usize> Expr<F> for RandomLinearCombination<F, N> {
 
 pub(crate) type MemoryAddress<F> = RandomLinearCombination<F, N_BYTES_MEMORY_ADDRESS>;
 
-impl<F: FieldExt> WordExpr<F> for MemoryAddress<F> {
+impl<F: Field> WordExpr<F> for MemoryAddress<F> {
     fn to_word(&self) -> Word<Expression<F>> {
         Word::from_lo_unchecked(self.expr())
     }
 }
 
 pub(crate) type AccountAddress<F> = RandomLinearCombination<F, N_BYTES_ACCOUNT_ADDRESS>;
 
-impl<F: FieldExt> WordExpr<F> for AccountAddress<F> {
+impl<F: Field> WordExpr<F> for AccountAddress<F> {
     fn to_word(&self) -> Word<Expression<F>> {
         Word::from_lo_unchecked(self.expr())
     }

zkevm-circuits/src/evm_circuit/util/common_gadget.rs

@@ -7,7 +7,7 @@ use super::{
 };
 use crate::{
     evm_circuit::{
-        param::{N_BYTES_GAS, N_BYTES_MEMORY_WORD_SIZE},
+        param::{N_BYTES_ACCOUNT_ADDRESS, N_BYTES_GAS, N_BYTES_MEMORY_WORD_SIZE},
         step::ExecutionState,
         table::{FixedTableTag, Lookup},
         util::{
@@ -97,7 +97,7 @@ pub(crate) struct RestoreContextGadget<F> {
     caller_id: Cell<F>,
     caller_is_root: Cell<F>,
     caller_is_create: Cell<F>,
-    caller_code_hash: Cell<F>,
+    caller_code_hash: WordCell<F>,
     caller_program_counter: Cell<F>,
     caller_stack_pointer: Cell<F>,
     caller_gas_left: Cell<F>,
@@ -118,11 +118,10 @@ impl<F: Field> RestoreContextGadget<F> {
     ) -> Self {
         // Read caller's context for restore
         let caller_id = cb.call_context(None, CallContextFieldTag::CallerId);
-        let [caller_is_root, caller_is_create, caller_code_hash, caller_program_counter, caller_stack_pointer, caller_gas_left, caller_memory_word_size, caller_reversible_write_counter] =
+        let [caller_is_root, caller_is_create, caller_program_counter, caller_stack_pointer, caller_gas_left, caller_memory_word_size, caller_reversible_write_counter] =
             [
                 CallContextFieldTag::IsRoot,
                 CallContextFieldTag::IsCreate,
-                CallContextFieldTag::CodeHash,
                 CallContextFieldTag::ProgramCounter,
                 CallContextFieldTag::StackPointer,
                 CallContextFieldTag::GasLeft,
@@ -131,6 +130,9 @@ impl<F: Field> RestoreContextGadget<F> {
             ]
             .map(|field_tag| cb.call_context(Some(caller_id.expr()), field_tag));
 
+        let caller_code_hash =
+            cb.call_context_read_as_word(Some(caller_id.expr()), CallContextFieldTag::CodeHash);
+
         // Update caller's last callee information
         // EIP-211 CREATE/CREATE2 call successful case should set RETURNDATASIZE = 0
         let is_call_create_and_success_expr = cb.curr.state.is_create.expr() * is_success.clone();
@@ -156,8 +158,8 @@ impl<F: Field> RestoreContextGadget<F> {
                 ),
             ),
         ] {
-            cb.call_context_lookup(
-                true.expr(),
+            // TODO review and assure range check
+            cb.call_context_lookup_write_unchecked(
                 Some(caller_id.expr()),
                 field_tag,
                 Word::from_lo_unchecked(value),
@@ -195,7 +197,7 @@ impl<F: Field> RestoreContextGadget<F> {
             call_id: To(caller_id.expr()),
             is_root: To(caller_is_root.expr()),
             is_create: To(caller_is_create.expr()),
-            code_hash: To(caller_code_hash.expr()),
+            code_hash: To(caller_code_hash.to_word()),
             program_counter: To(caller_program_counter.expr()),
             stack_pointer: To(caller_stack_pointer.expr()),
             gas_left: To(gas_left),
@@ -260,7 +262,7 @@ impl<F: Field> RestoreContextGadget<F> {
         }
 
         self.caller_code_hash
-            .assign(region, offset, region.word_rlc(caller_code_hash))?;
+            .assign(region, offset, Word::from_u256(caller_code_hash))?;
 
         Ok(())
     }
@@ -364,7 +366,7 @@ pub(crate) struct TransferWithGasFeeGadget<F> {
     receiver: UpdateBalanceGadget<F, 2, true>,
     receiver_exists: Expression<F>,
     must_create: Expression<F>,
-    pub(crate) value_is_zero: IsZeroWordGadget<F>,
+    pub(crate) value_is_zero: IsZeroWordGadget<F, Word32Cell<F>>,
 }
 
 impl<F: Field> TransferWithGasFeeGadget<F> {
@@ -496,7 +498,7 @@ impl<F: Field> TransferWithGasFeeGadget<F> {
 pub(crate) struct TransferGadget<F> {
     sender: UpdateBalanceGadget<F, 2, false>,
     receiver: UpdateBalanceGadget<F, 2, true>,
-    pub(crate) value_is_zero: IsZeroWordGadget<F>,
+    pub(crate) value_is_zero: IsZeroWordGadget<F, Word32Cell<F>>,
 }
 
 impl<F: Field> TransferGadget<F> {
@@ -594,12 +596,12 @@ pub(crate) struct CommonCallGadget<F, const IS_SUCCESS_CALL: bool> {
     pub rd_address: MemoryAddressGadget<F>,
     pub memory_expansion: MemoryExpansionGadget<F, 2, N_BYTES_MEMORY_WORD_SIZE>,
 
-    value_is_zero: IsZeroWordGadget<F>,
+    value_is_zero: IsZeroWordGadget<F, Word32Cell<F>>,
     pub has_value: Expression<F>,
-    pub callee_code_hash: Word32Cell<F>,
-    pub is_empty_code_hash: IsEqualWordGadget<F, Word<Expression<F>>>,
+    pub callee_code_hash: WordCell<F>,
+    pub is_empty_code_hash: IsEqualWordGadget<F, WordCell<F>, Word<Expression<F>>>,
 
-    pub callee_not_exists: IsZeroWordGadget<F>,
+    pub callee_not_exists: IsZeroWordGadget<F, WordCell<F>>,
 }
 
 impl<F: Field, const IS_SUCCESS_CALL: bool> CommonCallGadget<F, IS_SUCCESS_CALL> {
@@ -664,14 +666,14 @@ impl<F: Field, const IS_SUCCESS_CALL: bool> CommonCallGadget<F, IS_SUCCESS_CALL>
             1.expr() - value_is_zero.expr(),
         );
 
-        let callee_code_hash = cb.query_word32();
+        let callee_code_hash = cb.query_word_unchecked();
         cb.account_read(
             callee_address_word.expr(),
             AccountFieldTag::CodeHash,
             callee_code_hash.to_word(),
         );
         let is_empty_code_hash =
-            IsEqualWordGadget::construct(cb, callee_code_hash.to_word(), cb.empty_code_hash_word());
+            IsEqualWordGadget::construct(cb, callee_code_hash, cb.empty_code_hash_word());
         let callee_not_exists = IsZeroWordGadget::construct(cb, callee_code_hash);
 
         Self {
@@ -732,8 +734,13 @@ impl<F: Field, const IS_SUCCESS_CALL: bool> CommonCallGadget<F, IS_SUCCESS_CALL>
         callee_code_hash: U256,
     ) -> Result<u64, Error> {
         self.gas.assign(region, offset, Some(gas.to_le_bytes()))?;
-        self.callee_address
-            .assign(region, offset, Some(callee_address.to_le_bytes()))?;
+        self.callee_address.assign(
+            region,
+            offset,
+            callee_address.to_le_bytes()[0..N_BYTES_ACCOUNT_ADDRESS]
+                .try_into()
+                .unwrap(),
+        )?;
         self.value
             .assign(region, offset, Some(value.to_le_bytes()))?;
         if IS_SUCCESS_CALL {
@@ -971,15 +978,9 @@ impl<F: Field> CommonErrorGadget<F> {
         let rw_counter_end_of_reversion = cb.query_word_unchecked(); // rw_counter_end_of_reversion just used for read lookup, therefore skip range check
 
         // current call must be failed.
-        cb.call_context_lookup(
-            false.expr(),
-            None,
-            CallContextFieldTag::IsSuccess,
-            Word::zero(),
-        );
+        cb.call_context_lookup_read(None, CallContextFieldTag::IsSuccess, Word::zero());
 
-        cb.call_context_lookup(
-            false.expr(),
+        cb.call_context_lookup_read(
             None,
             CallContextFieldTag::RwCounterEndOfReversion,
             rw_counter_end_of_reversion.to_word(),

zkevm-circuits/src/evm_circuit/util/constraint_builder.rs

@@ -11,7 +11,7 @@ use crate::{
     },
     util::{
         build_tx_log_expression,
-        word::{Word, Word16, Word32, Word32Cell, Word4, WordExpr, WordLimbs},
+        word::{Word, Word16, Word32, Word32Cell, Word4, WordCell, WordExpr, WordLimbs},
         Challenges, Expr,
     },
 };
@@ -423,7 +423,7 @@ impl<'a, F: Field> EVMConstraintBuilder<'a, F> {
     // }
 
     // default query_word is 2 limbs. Each limb is not guaranteed to be 128 bits.
-    pub fn query_word_unchecked<const N: usize>(&mut self) -> Word<Cell<F>> {
+    pub fn query_word_unchecked<const N: usize>(&mut self) -> WordCell<F> {
         Word::new(
             self.query_cells(CellType::StoragePhase1, N)
                 .try_into()

zkevm-circuits/src/evm_circuit/util/math_gadget/abs_word.rs

@@ -4,7 +4,10 @@ use crate::{
         math_gadget::*,
         CachedRegion,
     },
-    util::{word::Word32Cell, Expr},
+    util::{
+        word::{Word32Cell, WordExpr},
+        Expr,
+    },
 };
 use eth_types::{Field, ToLittleEndian, Word};
 use gadgets::util::sum;

zkevm-circuits/src/evm_circuit/util/math_gadget/add_words.rs

@@ -4,7 +4,10 @@ use crate::{
         constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder},
         pow_of_two_expr, split_u256, sum, CachedRegion, Cell,
     },
-    util::{word::Word32Cell, Expr},
+    util::{
+        word::{Word32Cell, WordExpr},
+        Expr,
+    },
 };
 use eth_types::{Field, ToLittleEndian, ToScalar, Word};
 use halo2_proofs::{circuit::Value, plonk::Error};

zkevm-circuits/src/evm_circuit/util/math_gadget/cmp_words.rs

@@ -11,16 +11,16 @@ use halo2_proofs::plonk::{Error, Expression};
 
 #[derive(Clone, Debug)]
 /// CmpWordsGadget compares two words, exposing `eq`  and `lt`
-pub(crate) struct CmpWordsGadget<F, T> {
+pub(crate) struct CmpWordsGadget<F, T1, T2> {
     comparison_lo: ComparisonGadget<F, 16>,
     comparison_hi: ComparisonGadget<F, 16>,
     pub eq: Expression<F>,
     pub lt: Expression<F>,
-    marker: PhantomData<T>,
+    _marker: PhantomData<(T1, T2)>,
 }
 
-impl<F: Field, T: WordExpr<F>> CmpWordsGadget<F, T> {
-    pub(crate) fn construct(cb: &mut EVMConstraintBuilder<F>, a: T, b: T) -> Self {
+impl<F: Field, T1: WordExpr<F>, T2: WordExpr<F>> CmpWordsGadget<F, T1, T2> {
+    pub(crate) fn construct(cb: &mut EVMConstraintBuilder<F>, a: T1, b: T2) -> Self {
         let (a_lo, a_hi) = a.to_word().to_lo_hi();
         let (b_lo, b_hi) = b.to_word().to_lo_hi();
         // `a.lo <= b.lo`
@@ -45,7 +45,7 @@ impl<F: Field, T: WordExpr<F>> CmpWordsGadget<F, T> {
             comparison_hi,
             lt,
             eq,
-            marker: Default::default(),
+            _marker: Default::default(),
         }
     }
 
@@ -85,14 +85,14 @@ mod tests {
 
     #[derive(Clone)]
     /// CmpWordGadgetTestContainer: require(a == b if CHECK_EQ else a < b)
-    struct CmpWordGadgetTestContainer<F, const CHECK_EQ: bool, T: WordExpr<F>> {
-        cmp_gadget: CmpWordsGadget<F, T>,
-        a_word: T,
-        b_word: T,
+    struct CmpWordGadgetTestContainer<F, const CHECK_EQ: bool, T1, T2> {
+        cmp_gadget: CmpWordsGadget<F, T1, T2>,
+        a_word: T1,
+        b_word: T2,
     }
 
-    impl<F: Field, const CHECK_EQ: bool, T: WordExpr<F>> MathGadgetContainer<F>
-        for CmpWordGadgetTestContainer<F, CHECK_EQ, T>
+    impl<F: Field, const CHECK_EQ: bool, T1: WordExpr<F> + Clone, T2: WordExpr<F> + Clone>
+        MathGadgetContainer<F> for CmpWordGadgetTestContainer<F, CHECK_EQ, T1, T2>
     {
         fn configure_gadget_container(cb: &mut EVMConstraintBuilder<F>) -> Self {
             let a_word = cb.query_word_unchecked();

zkevm-circuits/src/evm_circuit/util/math_gadget/is_equal_word.rs

@@ -18,23 +18,23 @@ use super::IsZeroGadget;
 
 /// Returns `1` when `lhs == rhs`, and returns `0` otherwise.
 #[derive(Clone, Debug)]
-pub struct IsEqualWordGadget<F, T> {
+pub struct IsEqualWordGadget<F, T1, T2> {
     is_zero_lo: IsZeroGadget<F>,
     is_zero_hi: IsZeroGadget<F>,
-    marker: PhantomData<T>,
+    _marker: PhantomData<(T1, T2)>,
 }
 
-impl<F: Field, T: WordExpr<F>> IsEqualWordGadget<F, T> {
-    pub(crate) fn construct(cb: &mut EVMConstraintBuilder<F>, lhs: T, rhs: T) -> Self {
-        let (lhs_lo, lhs_hi) = lhs.to_lo_hi();
-        let (rhs_lo, rhs_hi) = rhs.to_lo_hi();
+impl<F: Field, T1: WordExpr<F>, T2: WordExpr<F>> IsEqualWordGadget<F, T1, T2> {
+    pub(crate) fn construct(cb: &mut EVMConstraintBuilder<F>, lhs: T1, rhs: T2) -> Self {
+        let (lhs_lo, lhs_hi) = lhs.to_word().to_lo_hi();
+        let (rhs_lo, rhs_hi) = rhs.to_word().to_lo_hi();
         let is_zero_lo = IsZeroGadget::construct(cb, lhs_lo.clone() - rhs_lo.clone());
         let is_zero_hi = IsZeroGadget::construct(cb, lhs_hi.clone() - rhs_hi.clone());
 
         Self {
             is_zero_lo,
             is_zero_hi,
-            marker: Default::default(),
+            _marker: Default::default(),
         }
     }