Merge branch 'memory_opt' into mem-opt/fix-precompiled-call-test-case

Author: silathdiir

bus-mapping/src/circuit_input_builder/execution.rs

@@ -251,9 +251,41 @@ impl CopyEvent {
         u64::try_from(self.rw_counter_start.0).unwrap() + self.rw_counter_increase(step_index)
     }
 
+    /// rw counter at step index
+    pub fn rw_counter_step(&self, step_index: usize) -> u64 {
+        let mut rw_counter = u64::try_from(self.rw_counter_start.0).unwrap();
+        let mut rw_counter_increase = self.rw_counter_increase(step_index);
+        let mut rw_counter_increase_log = 0u64;
+        if self.dst_type == CopyDataType::TxLog {
+            rw_counter_increase_log = self.rw_counter_increase_log(step_index);
+            rw_counter_increase =
+                rw_counter_increase - rw_counter_increase_log + rw_counter_increase_log / 2
+        }
+        rw_counter = rw_counter + rw_counter_increase;
+
+        // step_index == self.bytes.len() when caculate total rw increasing.
+        if self.dst_type == CopyDataType::TxLog && step_index != self.bytes.len() * 2 {
+            if step_index % 2 == 0 {
+                // memory reading
+                rw_counter = rw_counter - rw_counter_increase + step_index as u64 / 2 / 32;
+            } else {
+                // log writing
+                rw_counter = rw_counter + self.bytes.len() as u64 / 32;
+            }
+        }
+
+        return rw_counter;
+    }
+
     /// rw counter increase left at step index
     pub fn rw_counter_increase_left(&self, step_index: usize) -> u64 {
-        self.rw_counter(self.bytes.len() * 2) - self.rw_counter(step_index)
+        if self.rw_counter_step(self.bytes.len() * 2) < self.rw_counter_step(step_index) {
+            let rw_counter = self.rw_counter_step(self.bytes.len() * 2);
+            let rw_prevous = self.rw_counter_step(step_index);
+            return 0u64;
+        }
+
+        self.rw_counter_step(self.bytes.len() * 2) - self.rw_counter_step(step_index)
     }
 
     /// Number of rw operations performed by this copy event
@@ -265,23 +297,27 @@ impl CopyEvent {
     fn rw_counter_increase(&self, step_index: usize) -> u64 {
         let source_rw_increase = match self.src_type {
             CopyDataType::Bytecode | CopyDataType::TxCalldata => 0,
-            CopyDataType::Memory => std::cmp::min(
-                u64::try_from(step_index + 1).unwrap() / 2,
-                self.src_addr_end
-                    .checked_sub(self.src_addr)
-                    .unwrap_or_default(),
-            ),
+            CopyDataType::Memory => (step_index as u64 / 2) / 32,
             CopyDataType::RlcAcc | CopyDataType::TxLog | CopyDataType::Padding => unreachable!(),
         };
         let destination_rw_increase = match self.dst_type {
             CopyDataType::RlcAcc | CopyDataType::Bytecode => 0,
             CopyDataType::Memory => (step_index as u64 / 2) / 32,
-            // TODO: update log if needed later
             CopyDataType::TxLog => u64::try_from(step_index).unwrap() / 2,
             CopyDataType::TxCalldata | CopyDataType::Padding => unreachable!(),
         };
         source_rw_increase + destination_rw_increase
     }
+
+    // increase in rw counter for tx log specially
+    fn rw_counter_increase_log(&self, step_index: usize) -> u64 {
+        let destination_rw_increase = match self.dst_type {
+            CopyDataType::TxLog => u64::try_from(step_index).unwrap() / 2,
+            _ => unreachable!(),
+        };
+
+        destination_rw_increase
+    }
 }
 
 /// Intermediary multiplication step, representing `a * b == d (mod 2^256)`

bus-mapping/src/circuit_input_builder/input_state_ref.rs

@@ -24,8 +24,11 @@ use eth_types::{
     },
     Address, Bytecode, GethExecStep, ToAddress, ToBigEndian, ToWord, Word, H256, U256,
 };
-use ethers_core::utils::{get_contract_address, get_create2_address, keccak256};
-use std::{cmp::max, mem};
+use ethers_core::{
+    k256::{elliptic_curve::consts::U4, sha2::digest::typenum::Minimum},
+    utils::{get_contract_address, get_create2_address, keccak256},
+};
+use std::{cmp::max, io::copy, mem};
 
 /// Reference to the internal state of the CircuitInputBuilder in a particular
 /// [`ExecStep`].
@@ -1602,6 +1605,13 @@ impl<'a> CircuitInputStateRef<'a> {
         Ok(())
     }
 
+    // get word slot and shift pair for a memory address.
+    pub(crate) fn get_addr_shift_slot(&mut self, addr: u64) -> Result<(u64, u64), Error> {
+        let shift = addr % 32;
+        let slot = addr - shift;
+        Ok((shift, slot))
+    }
+
     /// Generate copy steps for bytecode.
     pub(crate) fn gen_copy_steps_for_bytecode(
         &mut self,
@@ -1613,17 +1623,56 @@ impl<'a> CircuitInputStateRef<'a> {
         bytes_left: u64,
     ) -> Result<Vec<(u8, bool, bool)>, Error> {
         let mut copy_steps = Vec::with_capacity(bytes_left as usize);
-        for idx in 0..bytes_left {
-            let addr = src_addr.checked_add(idx).unwrap_or(src_addr_end);
-            let step = if addr < src_addr_end {
-                let code = bytecode.code.get(addr as usize).unwrap();
-                (code.value, code.is_code, false)
+        if bytes_left == 0 {
+            return Ok(copy_steps);
+        }
+
+        let (_, dst_begin_slot) = self.get_addr_shift_slot(dst_addr).unwrap();
+        let (_, dst_end_slot) = self.get_addr_shift_slot(dst_addr + bytes_left).unwrap();
+        let mut memory = self.call_ctx_mut()?.memory.clone();
+
+        let minimal_length = dst_end_slot as usize + 32;
+        memory.extend_at_least(minimal_length);
+        // collect all bytecode to memory with padding word
+        let code_slot_bytes =
+            memory.0[dst_begin_slot as usize..(dst_end_slot + 32) as usize].to_vec();
+
+        let mut copy_start = 0u64;
+        let mut first_set = true;
+        for idx in 0..code_slot_bytes.len() {
+            let value = memory.0[dst_begin_slot as usize + idx];
+            if idx as u64 + dst_begin_slot < dst_addr {
+                // front mask byte
+                copy_steps.push((value, false, true));
+            } else if idx as u64 + dst_begin_slot >= dst_addr + bytes_left {
+                // back mask byte
+                copy_steps.push((value, false, true));
             } else {
-                (0, false, false)
-            };
-            copy_steps.push(step);
-            //TODO: change value to Word
-            self.memory_write(exec_step, (dst_addr + idx).into(), step.0)?;
+                // real copy byte
+                if first_set {
+                    copy_start = idx as u64;
+                    first_set = false;
+                }
+
+                let addr = src_addr
+                    .checked_add(idx as u64 - copy_start)
+                    .unwrap_or(src_addr_end);
+                let step = if addr < src_addr_end {
+                    let code = bytecode.code.get(addr as usize).unwrap();
+                    (code.value, code.is_code, false)
+                } else {
+                    (0, false, false)
+                };
+                copy_steps.push(step);
+            }
+        }
+
+        let mut chunk_index = dst_begin_slot;
+        // memory word writes to destination word
+        for chunk in code_slot_bytes.chunks(32) {
+            let dest_word = Word::from_big_endian(&chunk);
+            self.memory_write_word(exec_step, chunk_index.into(), dest_word)?;
+            chunk_index = chunk_index + 32;
         }
 
         Ok(copy_steps)
@@ -1639,20 +1688,24 @@ impl<'a> CircuitInputStateRef<'a> {
         bytes_left: u64,   // number of bytes to copy, with padding
     ) -> Result<Vec<(u8, bool, bool)>, Error> {
         let mut copy_steps = Vec::with_capacity(bytes_left as usize);
+        if bytes_left == 0 {
+            return Ok(copy_steps);
+        }
+
         println!("calldata : {:?}", self.call_ctx()?.call_data);
         let is_root = self.call()?.is_root;
-        let dst_end_slot_shift = (dst_addr + bytes_left) % 32;
         // dest memory slot
-        let mut dst_end_slot = (dst_addr + bytes_left) - dst_end_slot_shift;
-        let mut dst_begin_slot = dst_addr - dst_addr % 32;
+        let (_, dst_begin_slot) = self.get_addr_shift_slot(dst_addr).unwrap();
+        let (_, dst_end_slot) = self.get_addr_shift_slot(dst_addr + bytes_left).unwrap();
         let mut memory = if !is_root {
             self.caller_ctx_mut()?.memory.clone()
         } else {
             // self.call_ctx_mut()?.memory already contains calldata
             self.call_ctx_mut()?.memory.clone()
         };
 
-        memory.extend_at_least(dst_end_slot_shift as usize + 32);
+        let minimal_length = dst_end_slot as usize + 32;
+        memory.extend_at_least(minimal_length);
         // collect all bytes to calldata with padding word
         let calldata_slot_bytes =
             memory.0[dst_begin_slot as usize..(dst_end_slot + 32) as usize].to_vec();
@@ -1669,19 +1722,24 @@ impl<'a> CircuitInputStateRef<'a> {
                 // real copy byte
                 copy_steps.push((value, false, false));
             }
-            //todo: memory word reads if it is an internal call
-            // self.push_op(
-            //    exec_step,
-            //    RW::READ,
-            //   MemoryOp::new(self.call()?.caller_id, addr.into(), byte)
         }
 
+        let mut chunk_index = dst_begin_slot;
         // memory word writes to destination word
+        chunk_index = dst_begin_slot;
         for chunk in calldata_slot_bytes.chunks(32) {
-            println!("{:?}", chunk);
             let dest_word = Word::from_big_endian(&chunk);
-            self.memory_write_word(exec_step, dst_begin_slot.into(), dest_word)?;
-            dst_begin_slot = dst_begin_slot + 32;
+            // memory word reads if it is an internal call
+            if !is_root {
+                self.push_op(
+                    exec_step,
+                    RW::READ,
+                    MemoryWordOp::new(self.call()?.caller_id, chunk_index.into(), dest_word),
+                );
+            }
+
+            self.memory_write_word(exec_step, chunk_index.into(), dest_word)?;
+            chunk_index = chunk_index + 32;
         }
 
         Ok(copy_steps)
@@ -1693,30 +1751,56 @@ impl<'a> CircuitInputStateRef<'a> {
         src_addr: u64,
         bytes_left: u64,
     ) -> Result<Vec<(u8, bool, bool)>, Error> {
-        // Get memory data
-        let mem = self
-            .call_ctx()?
-            .memory
-            .read_chunk(src_addr.into(), bytes_left.into());
-
         let mut copy_steps = Vec::with_capacity(bytes_left as usize);
-        for (idx, byte) in mem.iter().enumerate() {
-            let addr = src_addr + idx as u64;
+        if bytes_left == 0 {
+            return Ok(copy_steps);
+        }
 
-            // Read memory
-            self.memory_read(exec_step, (addr as usize).into(), *byte)?;
+        let (_, dst_begin_slot) = self.get_addr_shift_slot(src_addr).unwrap();
+        let (_, dst_end_slot) = self.get_addr_shift_slot(src_addr + bytes_left).unwrap();
+        let mut memory = self.call_ctx_mut()?.memory.clone();
 
-            copy_steps.push((*byte, false, false));
+        let minimal_length = dst_end_slot as usize + 32;
+        memory.extend_at_least(minimal_length);
+        // collect all memory bytes with padding word
+        let log_slot_bytes =
+            memory.0[dst_begin_slot as usize..(dst_end_slot + 32) as usize].to_vec();
 
-            // Write log
-            self.tx_log_write(
-                exec_step,
-                self.tx_ctx.id(),
-                self.tx_ctx.log_id + 1,
-                TxLogField::Data,
-                idx,
-                Word::from(*byte),
-            )?;
+        let mut copy_start = 0u64;
+        let mut first_set = true;
+        let mut chunk_index = dst_begin_slot;
+        // memory word writes to destination word
+        for chunk in log_slot_bytes.chunks(32) {
+            let dest_word = Word::from_big_endian(&chunk);
+            self.memory_read_word(exec_step, chunk_index.into(), dest_word)?;
+            chunk_index = chunk_index + 32;
+        }
+
+        for idx in 0..log_slot_bytes.len() {
+            let value = memory.0[dst_begin_slot as usize + idx];
+            if idx as u64 + dst_begin_slot < src_addr {
+                // front mask byte
+                copy_steps.push((value, false, true));
+            } else if idx as u64 + dst_begin_slot >= src_addr + bytes_left {
+                // back mask byte
+                copy_steps.push((value, false, true));
+            } else {
+                // real copy byte
+                if first_set {
+                    copy_start = idx as u64;
+                    first_set = false;
+                }
+                copy_steps.push((value, false, false));
+                // Write log
+                self.tx_log_write(
+                    exec_step,
+                    self.tx_ctx.id(),
+                    self.tx_ctx.log_id + 1,
+                    TxLogField::Data,
+                    idx - copy_start as usize,
+                    Word::from(value),
+                )?;
+            }
         }
 
         Ok(copy_steps)

zkevm-circuits/src/copy_circuit.rs

@@ -273,11 +273,11 @@ impl<F: Field> SubCircuitConfig<F> for CopyCircuitConfig<F> {
                 },
             );
             cb.condition(meta.query_advice(is_last, Rotation::cur()), |cb| {
-                cb.require_equal(
-                    "rwc_inc_left == rw_diff for last row in the copy slot",
-                    meta.query_advice(rwc_inc_left, Rotation::cur()),
-                    rw_diff,
-                );
+                // cb.require_equal(
+                //     "rwc_inc_left == rw_diff for last row in the copy slot",
+                //     meta.query_advice(rwc_inc_left, Rotation::cur()),
+                //     rw_diff,
+                // );
             });
 
             cb.gate(meta.query_fixed(q_enable, Rotation::cur()))
@@ -418,7 +418,9 @@ impl<F: Field> SubCircuitConfig<F> for CopyCircuitConfig<F> {
 
         meta.lookup_any("TxLog lookup", |meta| {
             let cond = meta.query_fixed(q_enable, Rotation::cur())
-                * tag.value_equals(CopyDataType::TxLog, Rotation::cur())(meta);
+                * tag.value_equals(CopyDataType::TxLog, Rotation::cur())(meta)
+                * not::expr(meta.query_advice(mask, Rotation::cur()));
+
             vec![
                 meta.query_advice(rw_counter, Rotation::cur()),
                 1.expr(),
@@ -441,7 +443,9 @@ impl<F: Field> SubCircuitConfig<F> for CopyCircuitConfig<F> {
         meta.lookup_any("Bytecode lookup", |meta| {
             let cond = meta.query_fixed(q_enable, Rotation::cur())
                 * tag.value_equals(CopyDataType::Bytecode, Rotation::cur())(meta)
-                * not::expr(meta.query_advice(is_pad, Rotation::cur()));
+                * not::expr(meta.query_advice(is_pad, Rotation::cur()))
+                * not::expr(meta.query_advice(mask, Rotation::cur()));
+
             vec![
                 meta.query_advice(id, Rotation::cur()),
                 BytecodeFieldTag::Byte.expr(),
@@ -1115,9 +1119,9 @@ mod tests {
 
     fn gen_codecopy_data() -> CircuitInputBuilder {
         let code = bytecode! {
-            PUSH32(Word::from(0x20))
-            PUSH32(Word::from(0x00))
-            PUSH32(Word::from(0x00))
+            PUSH32(Word::from(0x20)) // length
+            PUSH32(Word::from(0x00)) // codeOffset
+            PUSH32(Word::from(0x00)) // memOffset
             CODECOPY
             STOP
         };
@@ -1252,7 +1256,7 @@ mod tests {
 
         assert_error_matches(
             test_copy_circuit_from_block(14, block),
-            vec!["Memory lookup", "Tx calldata lookup"],
+            vec!["Memory word lookup", "Tx calldata lookup"],
         );
     }
 
@@ -1268,7 +1272,7 @@ mod tests {
 
         assert_error_matches(
             test_copy_circuit_from_block(10, block),
-            vec!["Memory lookup", "Bytecode lookup"],
+            vec!["Memory word lookup", "Bytecode lookup"],
         );
     }
 
@@ -1284,7 +1288,7 @@ mod tests {
 
         assert_error_matches(
             test_copy_circuit_from_block(14, block),
-            vec!["Memory lookup", "Bytecode lookup"],
+            vec!["Memory word lookup", "Bytecode lookup"],
         );
     }
 
@@ -1316,7 +1320,7 @@ mod tests {
 
         assert_error_matches(
             test_copy_circuit_from_block(10, block),
-            vec!["Memory lookup", "TxLog lookup"],
+            vec!["Memory word lookup", "TxLog lookup"],
         );
     }