Implement PUSH0 instruction (privacy-scaling-explorations#1463)

### Description 1. Add `PUSH0` opcode to eth-types. It could be parsed from `0x5f` or a string of PUSH0. 2. In eth-types, update `is_push` function to return true for `PUSH0 .. PUSH32`, and add a new function `is_push_with_data`, it returns true for `PUSH1 .. PUSH32`. 3. Small fixes to replace `PUSH1 - 1` with PUSH0 value. 4. Add PUSH0 implementation to bus-mapping, and `PushGadget` to support PUSH0 in zkevm-circuits. ### Issue Link Close privacy-scaling-explorations#1473 Reference previous PR privacy-scaling-explorations#1361 zkevm-specs PR privacy-scaling-explorations/zkevm-specs#471 ### Type of change - [x] New feature (non-breaking change which adds functionality) --------- Co-authored-by: Chih Cheng Liang <[email protected]>
lfzkoala · Aug 23, 2023 · 96b3c4e · 96b3c4e
1 parent f30b7eb
commit 96b3c4e
Show file tree

Hide file tree

Showing 9 changed files with 173 additions and 132 deletions.
diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs
@@ -147,12 +147,13 @@ type FnGenAssociatedOps = fn(
 ) -> Result<Vec<ExecStep>, Error>;
 
 fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps {
-    if opcode_id.is_push() {
+    if opcode_id.is_push_with_data() {
         return StackOnlyOpcode::<0, 1>::gen_associated_ops;
     }
 
     match opcode_id {
         OpcodeId::STOP => Stop::gen_associated_ops,
+        OpcodeId::PUSH0 => StackOnlyOpcode::<0, 1>::gen_associated_ops,
         OpcodeId::ADD => StackOnlyOpcode::<2, 1>::gen_associated_ops,
         OpcodeId::MUL => StackOnlyOpcode::<2, 1>::gen_associated_ops,
         OpcodeId::SUB => StackOnlyOpcode::<2, 1>::gen_associated_ops,

diff --git a/bus-mapping/src/evm/opcodes/stackonlyop.rs b/bus-mapping/src/evm/opcodes/stackonlyop.rs
@@ -417,4 +417,17 @@ mod stackonlyop_tests {
             vec![StackOp::new(1, StackAddress(1023), *MOCK_BASEFEE)],
         );
     }
+
+    #[test]
+    fn push0_opcode_impl() {
+        stack_only_opcode_impl::<0, 1>(
+            OpcodeId::PUSH0,
+            bytecode! {
+                PUSH0
+                STOP
+            },
+            vec![],
+            vec![StackOp::new(1, StackAddress(1023), Word::zero())],
+        );
+    }
 }
diff --git a/eth-types/src/bytecode.rs b/eth-types/src/bytecode.rs
@@ -109,9 +109,9 @@ impl Bytecode {
         self
     }
 
-    /// Push
+    /// Push, value is useless for `PUSH0`
     pub fn push<T: ToWord>(&mut self, n: u8, value: T) -> &mut Self {
-        debug_assert!((1..=32).contains(&n), "invalid push");
+        debug_assert!((..=32).contains(&n), "invalid push");
         let value = value.to_word();
 
         // Write the op code
@@ -170,7 +170,7 @@ impl Bytecode {
             OpcodeWithData::Opcode(opcode) => {
                 self.write_op(opcode);
             }
-            OpcodeWithData::Push(n, word) => {
+            OpcodeWithData::PushWithData(n, word) => {
                 self.push(n, word);
             }
         }
@@ -208,18 +208,18 @@ impl Bytecode {
 /// An ASM entry
 #[derive(Clone, PartialEq, Eq)]
 pub enum OpcodeWithData {
-    /// A non-push opcode
+    /// A `PUSH0` or non-push opcode
     Opcode(OpcodeId),
-    /// A push opcode
-    Push(u8, Word),
+    /// A `PUSH1` .. `PUSH32` opcode
+    PushWithData(u8, Word),
 }
 
 impl OpcodeWithData {
     /// get the opcode
     pub fn opcode(&self) -> OpcodeId {
         match self {
             OpcodeWithData::Opcode(op) => *op,
-            OpcodeWithData::Push(n, _) => OpcodeId::push_n(*n).expect("valid push size"),
+            OpcodeWithData::PushWithData(n, _) => OpcodeId::push_n(*n).expect("valid push size"),
         }
     }
 }
@@ -233,28 +233,32 @@ impl FromStr for OpcodeWithData {
         if let Some(push) = op.strip_prefix("PUSH") {
             let n_value: Vec<_> = push.splitn(3, ['(', ')']).collect();
             let n = n_value[0].parse::<u8>().map_err(|_| err())?;
-            if n < 1 || n > 32 {
+            if n > 32 {
                 return Err(err());
             }
-            let value = if n_value[1].starts_with("0x") {
-                Word::from_str_radix(&n_value[1][2..], 16)
-            } else {
-                Word::from_str_radix(n_value[1], 10)
+
+            if n > 0 {
+                let value = if n_value[1].starts_with("0x") {
+                    Word::from_str_radix(&n_value[1][2..], 16)
+                } else {
+                    Word::from_str_radix(n_value[1], 10)
+                }
+                .map_err(|_| err())?;
+
+                return Ok(OpcodeWithData::PushWithData(n, value));
             }
-            .map_err(|_| err())?;
-            Ok(OpcodeWithData::Push(n, value))
-        } else {
-            let opcode = OpcodeId::from_str(op).map_err(|_| err())?;
-            Ok(OpcodeWithData::Opcode(opcode))
         }
+
+        let opcode = OpcodeId::from_str(op).map_err(|_| err())?;
+        Ok(OpcodeWithData::Opcode(opcode))
     }
 }
 
 impl ToString for OpcodeWithData {
     fn to_string(&self) -> String {
         match self {
             OpcodeWithData::Opcode(opcode) => format!("{:?}", opcode),
-            OpcodeWithData::Push(n, word) => format!("PUSH{}({})", n, word),
+            OpcodeWithData::PushWithData(n, word) => format!("PUSH{}({})", n, word),
         }
     }
 }
@@ -267,13 +271,16 @@ impl<'a> Iterator for BytecodeIterator<'a> {
     fn next(&mut self) -> Option<Self::Item> {
         self.0.next().map(|byte| {
             let op = OpcodeId::from(byte.value);
-            if op.is_push() {
-                let n = op.data_len();
+            let n = op.data_len();
+            if n > 0 {
+                assert!(op.is_push_with_data());
+
                 let mut value = vec![0u8; n];
                 for value_byte in value.iter_mut() {
                     *value_byte = self.0.next().unwrap().value;
                 }
-                OpcodeWithData::Push(n as u8, Word::from(value.as_slice()))
+
+                OpcodeWithData::PushWithData(n as u8, Word::from(value.as_slice()))
             } else {
                 OpcodeWithData::Opcode(op)
             }
@@ -289,7 +296,7 @@ impl From<Vec<u8>> for Bytecode {
         while let Some(byte) = input_iter.next() {
             let op = OpcodeId::from(*byte);
             code.write_op(op);
-            if op.is_push() {
+            if op.is_push_with_data() {
                 let n = op.postfix().expect("opcode with postfix");
                 for _ in 0..n {
                     match input_iter.next() {
@@ -327,14 +334,14 @@ macro_rules! bytecode_internal {
     ($code:ident, ) => {};
     // PUSHX op codes
     ($code:ident, $x:ident ($v:expr) $($rest:tt)*) => {{
-        debug_assert!($crate::evm_types::OpcodeId::$x.is_push(), "invalid push");
+        debug_assert!($crate::evm_types::OpcodeId::$x.is_push_with_data(), "invalid push");
         let n = $crate::evm_types::OpcodeId::$x.postfix().expect("opcode with postfix");
         $code.push(n, $v);
         $crate::bytecode_internal!($code, $($rest)*);
     }};
     // Default opcode without any inputs
     ($code:ident, $x:ident $($rest:tt)*) => {{
-        debug_assert!(!$crate::evm_types::OpcodeId::$x.is_push(), "invalid push");
+        debug_assert!(!$crate::evm_types::OpcodeId::$x.is_push_with_data(), "invalid push");
         $code.write_op($crate::evm_types::OpcodeId::$x);
         $crate::bytecode_internal!($code, $($rest)*);
     }};
@@ -350,6 +357,13 @@ macro_rules! bytecode_internal {
     }};
 }
 
+impl Bytecode {
+    /// Helper function for `PUSH0`
+    pub fn op_push0(&mut self) -> &mut Self {
+        self.push(0, Word::zero())
+    }
+}
+
 macro_rules! impl_push_n {
     ($($push_n:ident, $n:expr)*) => {
         #[allow(missing_docs)]
@@ -548,6 +562,8 @@ mod tests {
     #[test]
     fn test_bytecode_roundtrip() {
         let code = bytecode! {
+            PUSH0
+            POP
             PUSH8(0x123)
             POP
             PUSH24(0x321)

diff --git a/eth-types/src/evm_types/opcode_ids.rs b/eth-types/src/evm_types/opcode_ids.rs
@@ -95,6 +95,8 @@ pub enum OpcodeId {
     JUMPDEST,
 
     // PUSHn
+    /// `PUSH0`
+    PUSH0,
     /// `PUSH1`
     PUSH1,
     /// `PUSH2`
@@ -315,8 +317,13 @@ pub enum OpcodeId {
 }
 
 impl OpcodeId {
-    /// Returns `true` if the `OpcodeId` is a `PUSHn`.
+    /// Returns `true` if the `OpcodeId` is a `PUSHn` (including `PUSH0`).
     pub fn is_push(&self) -> bool {
+        self.as_u8() >= Self::PUSH0.as_u8() && self.as_u8() <= Self::PUSH32.as_u8()
+    }
+
+    /// Returns `true` if the `OpcodeId` is a `PUSH1` .. `PUSH32` (excluding `PUSH0`).
+    pub fn is_push_with_data(&self) -> bool {
         self.as_u8() >= Self::PUSH1.as_u8() && self.as_u8() <= Self::PUSH32.as_u8()
     }
 
@@ -402,6 +409,7 @@ impl OpcodeId {
             OpcodeId::PC => 0x58u8,
             OpcodeId::MSIZE => 0x59u8,
             OpcodeId::JUMPDEST => 0x5bu8,
+            OpcodeId::PUSH0 => 0x5fu8,
             OpcodeId::PUSH1 => 0x60u8,
             OpcodeId::PUSH2 => 0x61u8,
             OpcodeId::PUSH3 => 0x62u8,
@@ -580,6 +588,7 @@ impl OpcodeId {
             OpcodeId::MSIZE => GasCost::QUICK,
             OpcodeId::GAS => GasCost::QUICK,
             OpcodeId::JUMPDEST => GasCost::ONE,
+            OpcodeId::PUSH0 => GasCost::QUICK,
             OpcodeId::PUSH1 => GasCost::FASTEST,
             OpcodeId::PUSH2 => GasCost::FASTEST,
             OpcodeId::PUSH3 => GasCost::FASTEST,
@@ -734,6 +743,7 @@ impl OpcodeId {
             OpcodeId::MSIZE => (1, 1024),
             OpcodeId::GAS => (1, 1024),
             OpcodeId::JUMPDEST => (0, 1024),
+            OpcodeId::PUSH0 => (1, 1024),
             OpcodeId::PUSH1 => (1, 1024),
             OpcodeId::PUSH2 => (1, 1024),
             OpcodeId::PUSH3 => (1, 1024),
@@ -840,8 +850,10 @@ impl OpcodeId {
 
     /// Returns PUSHn opcode from parameter n.
     pub fn push_n(n: u8) -> Result<Self, Error> {
-        if (1..=32).contains(&n) {
-            Ok(OpcodeId::from(OpcodeId::PUSH1.as_u8() + n - 1))
+        let op = OpcodeId::from(OpcodeId::PUSH0.as_u8().checked_add(n).unwrap_or_default());
+
+        if op.is_push() {
+            Ok(op)
         } else {
             Err(Error::InvalidOpConversion)
         }
@@ -850,7 +862,7 @@ impl OpcodeId {
     /// If operation has postfix returns it, otherwise None.
     pub fn postfix(&self) -> Option<u8> {
         if self.is_push() {
-            Some(self.as_u8() - OpcodeId::PUSH1.as_u8() + 1)
+            Some(self.as_u8() - OpcodeId::PUSH0.as_u8())
         } else if self.is_dup() {
             Some(self.as_u8() - OpcodeId::DUP1.as_u8() + 1)
         } else if self.is_swap() {
@@ -863,10 +875,10 @@ impl OpcodeId {
     }
 
     /// Returns number of bytes used by immediate data. This is > 0 only for
-    /// push opcodes.
+    /// `PUSH1` .. `PUSH32` opcodes.
     pub fn data_len(&self) -> usize {
-        if self.is_push() {
-            (self.as_u8() - OpcodeId::PUSH1.as_u8() + 1) as usize
+        if self.is_push_with_data() {
+            (self.as_u8() - OpcodeId::PUSH0.as_u8()) as usize
         } else {
             0
         }
@@ -936,6 +948,7 @@ impl From<u8> for OpcodeId {
             0x58u8 => OpcodeId::PC,
             0x59u8 => OpcodeId::MSIZE,
             0x5bu8 => OpcodeId::JUMPDEST,
+            0x5fu8 => OpcodeId::PUSH0,
             0x60u8 => OpcodeId::PUSH1,
             0x61u8 => OpcodeId::PUSH2,
             0x62u8 => OpcodeId::PUSH3,
@@ -1089,6 +1102,7 @@ impl FromStr for OpcodeId {
             "PC" => OpcodeId::PC,
             "MSIZE" => OpcodeId::MSIZE,
             "JUMPDEST" => OpcodeId::JUMPDEST,
+            "PUSH0" => OpcodeId::PUSH0,
             "PUSH1" => OpcodeId::PUSH1,
             "PUSH2" => OpcodeId::PUSH2,
             "PUSH3" => OpcodeId::PUSH3,
@@ -1156,7 +1170,6 @@ impl FromStr for OpcodeId {
             "RETURN" => OpcodeId::RETURN,
             "REVERT" => OpcodeId::REVERT,
             "INVALID" => OpcodeId::INVALID(0xfe),
-            "PUSH0" => OpcodeId::INVALID(0x5f),
             "SHA3" | "KECCAK256" => OpcodeId::SHA3,
             "ADDRESS" => OpcodeId::ADDRESS,
             "BALANCE" => OpcodeId::BALANCE,
@@ -1234,20 +1247,18 @@ mod opcode_ids_tests {
 
     #[test]
     fn push_n() {
+        assert!(matches!(OpcodeId::push_n(0), Ok(OpcodeId::PUSH0)));
         assert!(matches!(OpcodeId::push_n(1), Ok(OpcodeId::PUSH1)));
         assert!(matches!(OpcodeId::push_n(10), Ok(OpcodeId::PUSH10)));
         assert!(matches!(
             OpcodeId::push_n(100),
             Err(Error::InvalidOpConversion)
         ));
-        assert!(matches!(
-            OpcodeId::push_n(0),
-            Err(Error::InvalidOpConversion)
-        ));
     }
 
     #[test]
     fn postfix() {
+        assert_eq!(OpcodeId::PUSH0.postfix(), Some(0));
         assert_eq!(OpcodeId::PUSH1.postfix(), Some(1));
         assert_eq!(OpcodeId::PUSH10.postfix(), Some(10));
         assert_eq!(OpcodeId::LOG2.postfix(), Some(2));
@@ -1256,6 +1267,7 @@ mod opcode_ids_tests {
 
     #[test]
     fn data_len() {
+        assert_eq!(OpcodeId::PUSH0.data_len(), 0);
         assert_eq!(OpcodeId::PUSH1.data_len(), 1);
         assert_eq!(OpcodeId::PUSH10.data_len(), 10);
         assert_eq!(OpcodeId::LOG2.data_len(), 0);

diff --git a/zkevm-circuits/src/bytecode_circuit/circuit.rs b/zkevm-circuits/src/bytecode_circuit/circuit.rs
@@ -734,9 +734,9 @@ impl<F: Field> BytecodeCircuitConfig<F> {
     /// load fixed tables
     pub(crate) fn load_aux_tables(&self, layouter: &mut impl Layouter<F>) -> Result<(), Error> {
         // push table: BYTE -> NUM_PUSHED:
-        // [0, OpcodeId::PUSH1] -> 0
-        // [OpcodeId::PUSH1, OpcodeId::PUSH32] -> [1..32]
-        // [OpcodeId::PUSH32, 256] -> 0
+        // byte < OpcodeId::PUSH1 => 0
+        // byte >= OpcodeId::PUSH1 && byte <= OpcodeId::PUSH32 => [1..32]
+        // byte > OpcodeId::PUSH32 && byte < 256 => 0
         layouter.assign_region(
             || "push table",
             |mut region| {

diff --git a/zkevm-circuits/src/evm_circuit/execution/error_invalid_opcode.rs b/zkevm-circuits/src/evm_circuit/execution/error_invalid_opcode.rs
@@ -81,7 +81,7 @@ mod test {
             vec![0xf6],
             vec![0xfe],
             // Multiple invalid opcodes
-            vec![0x5c, 0x5e, 0x5f],
+            vec![0x5c, 0x5e],
         ];
     }