Combine JUMPDEST and KECCAK_GENERAL flags. (0xPolygonZero#1259)

* Combine JUMPDEST and KECCAK_GENERAL flags.

* Apply comments

evm/src/cpu/columns/ops.rs

@@ -22,14 +22,12 @@ pub struct OpsColumnsView<T: Copy> {
     pub not_pop: T,
     /// Combines SHL and SHR flags.
     pub shift: T,
-    /// Flag for KECCAK_GENERAL.
-    pub keccak_general: T,
+    /// Combines JUMPDEST and KECCAK_GENERAL flags.
+    pub jumpdest_keccak_general: T, // Combines JUMPDEST and KECCAK_GENERAL flags.
     /// Flag for PROVER_INPUT.
     pub prover_input: T,
     /// Combines JUMP and JUMPI flags.
     pub jumps: T,
-    /// Flag for JUMPDEST.
-    pub jumpdest: T,
     /// Flag for PUSH.
     pub push: T,
     /// Combines DUP and SWAP flags.

evm/src/cpu/control_flow.rs

@@ -8,19 +8,18 @@ use crate::constraint_consumer::{ConstraintConsumer, RecursiveConstraintConsumer
 use crate::cpu::columns::{CpuColumnsView, COL_MAP};
 use crate::cpu::kernel::aggregator::KERNEL;
 
-const NATIVE_INSTRUCTIONS: [usize; 14] = [
+const NATIVE_INSTRUCTIONS: [usize; 13] = [
     COL_MAP.op.binary_op,
     COL_MAP.op.ternary_op,
     COL_MAP.op.fp254_op,
     COL_MAP.op.eq_iszero,
     COL_MAP.op.logic_op,
     COL_MAP.op.not_pop,
     COL_MAP.op.shift,
-    COL_MAP.op.keccak_general,
+    COL_MAP.op.jumpdest_keccak_general,
     COL_MAP.op.prover_input,
     // not JUMPS (possible need to jump)
     COL_MAP.op.pc_push0,
-    COL_MAP.op.jumpdest,
     // not PUSH (need to increment by more than 1)
     COL_MAP.op.dup_swap,
     COL_MAP.op.context_op,

evm/src/cpu/decode.rs

@@ -23,7 +23,7 @@ use crate::cpu::columns::{CpuColumnsView, COL_MAP};
 /// behavior.
 /// Note: invalid opcodes are not represented here. _Any_ opcode is permitted to decode to
 /// `is_invalid`. The kernel then verifies that the opcode was _actually_ invalid.
-const OPCODES: [(u8, usize, bool, usize); 11] = [
+const OPCODES: [(u8, usize, bool, usize); 9] = [
     // (start index of block, number of top bits to check (log2), kernel-only, flag column)
     // ADD, MUL, SUB, DIV, MOD, LT, GT and BYTE flags are handled partly manually here, and partly through the Arithmetic table CTL.
     // ADDMOD, MULMOD and SUBMOD flags are handled partly manually here, and partly through the Arithmetic table CTL.
@@ -32,10 +32,9 @@ const OPCODES: [(u8, usize, bool, usize); 11] = [
     // AND, OR and XOR flags are handled partly manually here, and partly through the Logic table CTL.
     // NOT and POP are handled manually here.
     // SHL and SHR flags are handled partly manually here, and partly through the Logic table CTL.
-    (0x21, 0, true, COL_MAP.op.keccak_general),
+    // JUMPDEST and KECCAK_GENERAL are handled manually here.
     (0x49, 0, true, COL_MAP.op.prover_input),
-    (0x56, 1, false, COL_MAP.op.jumps), // 0x56-0x57
-    (0x5b, 0, false, COL_MAP.op.jumpdest),
+    (0x56, 1, false, COL_MAP.op.jumps),    // 0x56-0x57
     (0x60, 5, false, COL_MAP.op.push),     // 0x60-0x7f
     (0x80, 5, false, COL_MAP.op.dup_swap), // 0x80-0x9f
     (0xee, 0, true, COL_MAP.op.mstore_32bytes),
@@ -48,13 +47,14 @@ const OPCODES: [(u8, usize, bool, usize); 11] = [
 /// List of combined opcodes requiring a special handling.
 /// Each index in the list corresponds to an arbitrary combination
 /// of opcodes defined in evm/src/cpu/columns/ops.rs.
-const COMBINED_OPCODES: [usize; 8] = [
+const COMBINED_OPCODES: [usize; 9] = [
     COL_MAP.op.logic_op,
     COL_MAP.op.fp254_op,
     COL_MAP.op.binary_op,
     COL_MAP.op.ternary_op,
     COL_MAP.op.shift,
     COL_MAP.op.m_op_general,
+    COL_MAP.op.jumpdest_keccak_general,
     COL_MAP.op.not_pop,
     COL_MAP.op.pc_push0,
 ];
@@ -147,6 +147,21 @@ pub fn eval_packed_generic<P: PackedField>(
         * lv.op.m_op_general;
     yield_constr.constraint(m_op_constr);
 
+    // Manually check lv.op.jumpdest_keccak_general.
+    // KECCAK_GENERAL is a kernel-only instruction, but not JUMPDEST.
+    // JUMPDEST is differentiated from KECCAK_GENERAL by its second bit set to 1.
+    yield_constr.constraint(
+        (P::ONES - kernel_mode) * lv.op.jumpdest_keccak_general * (P::ONES - lv.opcode_bits[1]),
+    );
+
+    // Check the JUMPDEST and KERNEL_GENERAL opcodes.
+    let jumpdest_opcode = P::Scalar::from_canonical_usize(0x5b);
+    let keccak_general_opcode = P::Scalar::from_canonical_usize(0x21);
+    let jumpdest_keccak_general_constr = (opcode - keccak_general_opcode)
+        * (opcode - jumpdest_opcode)
+        * lv.op.jumpdest_keccak_general;
+    yield_constr.constraint(jumpdest_keccak_general_constr);
+
     // Manually check lv.op.pc_push0.
     // Both PC and PUSH0 can be called outside of the kernel mode:
     // there is no need to constrain them in that regard.
@@ -267,6 +282,33 @@ pub fn eval_ext_circuit<F: RichField + Extendable<D>, const D: usize>(
 
     yield_constr.constraint(builder, m_op_constr);
 
+    // Manually check lv.op.jumpdest_keccak_general.
+    // KECCAK_GENERAL is a kernel-only instruction, but not JUMPDEST.
+    // JUMPDEST is differentiated from KECCAK_GENERAL by its second bit set to 1.
+    let jumpdest_opcode =
+        builder.constant_extension(F::Extension::from_canonical_usize(0x5b_usize));
+    let keccak_general_opcode =
+        builder.constant_extension(F::Extension::from_canonical_usize(0x21_usize));
+
+    // Check that KECCAK_GENERAL is kernel-only.
+    let mut kernel_general_filter = builder.sub_extension(one, lv.opcode_bits[1]);
+    kernel_general_filter =
+        builder.mul_extension(lv.op.jumpdest_keccak_general, kernel_general_filter);
+    let constr = builder.mul_extension(is_not_kernel_mode, kernel_general_filter);
+    yield_constr.constraint(builder, constr);
+
+    // Check the JUMPDEST and KERNEL_GENERAL opcodes.
+    let jumpdest_constr = builder.sub_extension(opcode, jumpdest_opcode);
+    let keccak_general_constr = builder.sub_extension(opcode, keccak_general_opcode);
+    let mut jumpdest_keccak_general_constr =
+        builder.mul_extension(jumpdest_constr, keccak_general_constr);
+    jumpdest_keccak_general_constr = builder.mul_extension(
+        jumpdest_keccak_general_constr,
+        lv.op.jumpdest_keccak_general,
+    );
+
+    yield_constr.constraint(builder, jumpdest_keccak_general_constr);
+
     // Manually check lv.op.pc_push0.
     // Both PC and PUSH0 can be called outside of the kernel mode:
     // there is no need to constrain them in that regard.

evm/src/cpu/gas.rs

@@ -26,11 +26,10 @@ const SIMPLE_OPCODES: OpsColumnsView<Option<u32>> = OpsColumnsView {
     logic_op: G_VERYLOW,
     not_pop: None, // This is handled manually below
     shift: G_VERYLOW,
-    keccak_general: KERNEL_ONLY_INSTR,
+    jumpdest_keccak_general: None, // This is handled manually below.
     prover_input: KERNEL_ONLY_INSTR,
     jumps: None, // Combined flag handled separately.
     pc_push0: G_BASE,
-    jumpdest: G_JUMPDEST,
     push: G_VERYLOW,
     dup_swap: G_VERYLOW,
     context_op: KERNEL_ONLY_INSTR,
@@ -109,6 +108,15 @@ fn eval_packed_accumulate<P: PackedField>(
         * P::Scalar::from_canonical_u32(G_BASE.unwrap())
         + lv.opcode_bits[0] * P::Scalar::from_canonical_u32(G_VERYLOW.unwrap());
     yield_constr.constraint_transition(lv.op.not_pop * (gas_diff - not_pop_cost));
+
+    // For JUMPDEST and KECCAK_GENERAL.
+    // JUMPDEST is differentiated from KECCAK_GENERAL by its second bit set to 1.
+    let jumpdest_keccak_general_gas_cost = lv.opcode_bits[1]
+        * P::Scalar::from_canonical_u32(G_JUMPDEST.unwrap())
+        + (P::ONES - lv.opcode_bits[1]) * P::Scalar::from_canonical_u32(KERNEL_ONLY_INSTR.unwrap());
+    yield_constr.constraint_transition(
+        lv.op.jumpdest_keccak_general * (gas_diff - jumpdest_keccak_general_gas_cost),
+    );
 }
 
 fn eval_packed_init<P: PackedField>(
@@ -252,6 +260,25 @@ fn eval_ext_circuit_accumulate<F: RichField + Extendable<D>, const D: usize>(
     let not_pop_gas_diff = builder.sub_extension(nv_lv_diff, not_pop_cost);
     let not_pop_constr = builder.mul_extension(filter, not_pop_gas_diff);
     yield_constr.constraint_transition(builder, not_pop_constr);
+
+    // For JUMPDEST and KECCAK_GENERAL.
+    // JUMPDEST is differentiated from KECCAK_GENERAL by its second bit set to 1.
+    let one = builder.one_extension();
+    let filter = lv.op.jumpdest_keccak_general;
+
+    let jumpdest_keccak_general_gas_cost = builder.arithmetic_extension(
+        F::from_canonical_u32(G_JUMPDEST.unwrap())
+            - F::from_canonical_u32(KERNEL_ONLY_INSTR.unwrap()),
+        F::from_canonical_u32(KERNEL_ONLY_INSTR.unwrap()),
+        lv.opcode_bits[1],
+        one,
+        one,
+    );
+
+    let gas_diff = builder.sub_extension(nv_lv_diff, jumpdest_keccak_general_gas_cost);
+    let constr = builder.mul_extension(filter, gas_diff);
+
+    yield_constr.constraint_transition(builder, constr);
 }
 
 fn eval_ext_circuit_init<F: RichField + Extendable<D>, const D: usize>(

evm/src/cpu/stack.rs

@@ -61,6 +61,18 @@ pub(crate) const MLOAD_GENERAL_OP: Option<StackBehavior> = Some(StackBehavior {
     disable_other_channels: false,
 });
 
+pub(crate) const KECCAK_GENERAL_OP: StackBehavior = StackBehavior {
+    num_pops: 4,
+    pushes: true,
+    disable_other_channels: true,
+};
+
+pub(crate) const JUMPDEST_OP: StackBehavior = StackBehavior {
+    num_pops: 0,
+    pushes: false,
+    disable_other_channels: true,
+};
+
 // AUDITORS: If the value below is `None`, then the operation must be manually checked to ensure
 // that every general-purpose memory channel is either disabled or has its read flag and address
 // propertly constrained. The same applies  when `disable_other_channels` is set to `false`,
@@ -78,23 +90,14 @@ pub(crate) const STACK_BEHAVIORS: OpsColumnsView<Option<StackBehavior>> = OpsCol
         pushes: true,
         disable_other_channels: false,
     }),
-    keccak_general: Some(StackBehavior {
-        num_pops: 4,
-        pushes: true,
-        disable_other_channels: true,
-    }),
+    jumpdest_keccak_general: None,
     prover_input: None, // TODO
     jumps: None,        // Depends on whether it's a JUMP or a JUMPI.
     pc_push0: Some(StackBehavior {
         num_pops: 0,
         pushes: true,
         disable_other_channels: true,
     }),
-    jumpdest: Some(StackBehavior {
-        num_pops: 0,
-        pushes: false,
-        disable_other_channels: true,
-    }),
     push: None, // TODO
     dup_swap: None,
     context_op: None,
@@ -261,6 +264,20 @@ pub fn eval_packed<P: PackedField>(
         }
     }
 
+    // Constrain stack for JUMPDEST.
+    let jumpdest_filter = lv.op.jumpdest_keccak_general * lv.opcode_bits[1];
+    eval_packed_one(lv, nv, jumpdest_filter, JUMPDEST_OP, yield_constr);
+
+    // Constrain stack for KECCAK_GENERAL.
+    let keccak_general_filter = lv.op.jumpdest_keccak_general * (P::ONES - lv.opcode_bits[1]);
+    eval_packed_one(
+        lv,
+        nv,
+        keccak_general_filter,
+        KECCAK_GENERAL_OP,
+        yield_constr,
+    );
+
     // Stack constraints for POP.
     // The only constraints POP has are stack constraints.
     // Since POP and NOT are combined into one flag and they have
@@ -526,6 +543,24 @@ pub fn eval_ext_circuit<F: RichField + Extendable<D>, const D: usize>(
         }
     }
 
+    // Constrain stack for JUMPDEST.
+    let jumpdest_filter = builder.mul_extension(lv.op.jumpdest_keccak_general, lv.opcode_bits[1]);
+    eval_ext_circuit_one(builder, lv, nv, jumpdest_filter, JUMPDEST_OP, yield_constr);
+
+    // Constrain stack for KECCAK_GENERAL.
+    let one = builder.one_extension();
+    let mut keccak_general_filter = builder.sub_extension(one, lv.opcode_bits[1]);
+    keccak_general_filter =
+        builder.mul_extension(lv.op.jumpdest_keccak_general, keccak_general_filter);
+    eval_ext_circuit_one(
+        builder,
+        lv,
+        nv,
+        keccak_general_filter,
+        KECCAK_GENERAL_OP,
+        yield_constr,
+    );
+
     // Stack constraints for POP.
     // The only constraints POP has are stack constraints.
     // Since POP and NOT are combined into one flag and they have

evm/src/witness/transition.rs

@@ -173,11 +173,10 @@ fn fill_op_flag<F: Field>(op: Operation, row: &mut CpuColumnsView<F>) {
         | Operation::BinaryArithmetic(arithmetic::BinaryOperator::Shr) => &mut flags.shift,
         Operation::BinaryArithmetic(_) => &mut flags.binary_op,
         Operation::TernaryArithmetic(_) => &mut flags.ternary_op,
-        Operation::KeccakGeneral => &mut flags.keccak_general,
+        Operation::KeccakGeneral | Operation::Jumpdest => &mut flags.jumpdest_keccak_general,
         Operation::ProverInput => &mut flags.prover_input,
         Operation::Jump | Operation::Jumpi => &mut flags.jumps,
         Operation::Pc | Operation::Push(0) => &mut flags.pc_push0,
-        Operation::Jumpdest => &mut flags.jumpdest,
         Operation::GetContext | Operation::SetContext => &mut flags.context_op,
         Operation::Mload32Bytes => &mut flags.mload_32bytes,
         Operation::Mstore32Bytes => &mut flags.mstore_32bytes,
@@ -206,11 +205,10 @@ fn get_op_special_length(op: Operation) -> Option<usize> {
         | Operation::BinaryArithmetic(arithmetic::BinaryOperator::Shr) => STACK_BEHAVIORS.shift,
         Operation::BinaryArithmetic(_) => STACK_BEHAVIORS.binary_op,
         Operation::TernaryArithmetic(_) => STACK_BEHAVIORS.ternary_op,
-        Operation::KeccakGeneral => STACK_BEHAVIORS.keccak_general,
+        Operation::KeccakGeneral | Operation::Jumpdest => STACK_BEHAVIORS.jumpdest_keccak_general,
         Operation::ProverInput => STACK_BEHAVIORS.prover_input,
         Operation::Jump => JUMP_OP,
         Operation::Jumpi => JUMPI_OP,
-        Operation::Jumpdest => STACK_BEHAVIORS.jumpdest,
         Operation::GetContext | Operation::SetContext => None,
         Operation::Mload32Bytes => STACK_BEHAVIORS.mload_32bytes,
         Operation::Mstore32Bytes => STACK_BEHAVIORS.mstore_32bytes,