better err log: (1) return err for copy circuit overflow (2) log::war… (

#1409) * better err log: (1) return err for copy circuit overflow (2) log::warn for state root mismatch * update poseidon-base
scroll-tech · Aug 29, 2024 · e319bee · e319bee
1 parent 41f0c6d
commit e319bee
Show file tree

Hide file tree

Showing 15 changed files with 32 additions and 25 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bus-mapping/src/circuit_input_builder/block.rs b/bus-mapping/src/circuit_input_builder/block.rs
@@ -257,16 +257,17 @@ impl Blocks {
 
 impl Blocks {
     /// Push a copy event to the block.
-    pub fn add_copy_event(&mut self, event: CopyEvent) {
+    pub fn add_copy_event(&mut self, event: CopyEvent) -> Result<(), Error> {
         self.copy_counter += event.full_length() as usize;
         self.copy_events.push(event);
         // Each byte needs 2 rows
         // TODO: magic num
 
         if self.copy_counter > 500_000 && cfg!(feature = "strict-ccc") {
             log::error!("copy event len overflow {}", self.copy_counter);
-            panic!("copy event len overflow");
+            return Err(Error::InvalidGethExecTrace("copy event len overflow"));
         }
+        Ok(())
     }
     fn copy_event_total_len(&self) -> usize {
         self.copy_events

diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs
@@ -1629,9 +1629,13 @@ impl<'a> CircuitInputStateRef<'a> {
     }
 
     /// Push a copy event to the state.
-    pub fn push_copy(&mut self, step: &mut ExecStep, event: CopyEvent) {
+    pub fn push_copy(&mut self, step: &mut ExecStep, event: CopyEvent) -> Result<(), Error> {
         step.copy_rw_counter_delta += event.rw_counter_delta();
-        self.block.add_copy_event(event);
+        let result = self.block.add_copy_event(event);
+        if result.is_err() {
+            log::error!("push_copy failed {result:?}, step {step:?}");
+        }
+        result
     }
 
     /// Push a exponentiation event to the state.

diff --git a/bus-mapping/src/evm/opcodes/begin_end_tx.rs b/bus-mapping/src/evm/opcodes/begin_end_tx.rs
@@ -319,7 +319,7 @@ pub fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<Vec<ExecSt
                 copy_bytes: CopyBytes::new(bytes, None, None),
                 access_list: vec![],
             },
-        );
+        )?;
     }
 
     let mut precompile_step = None;
@@ -466,7 +466,7 @@ pub fn gen_begin_tx_steps(state: &mut CircuitInputStateRef) -> Result<Vec<ExecSt
                     copy_bytes: CopyBytes::new(copy_steps, None, None),
                     access_list: vec![],
                 },
-            );
+            )?;
 
             let call_success = call.is_success;
             // modexp's oog error is handled in ModExpGadget
@@ -908,7 +908,7 @@ fn add_access_list_address_copy_event(
         log_id: None,
     };
 
-    state.push_copy(exec_step, copy_event);
+    state.push_copy(exec_step, copy_event)?;
 
     Ok(())
 }
@@ -993,7 +993,7 @@ fn add_access_list_storage_key_copy_event(
         log_id: None,
     };
 
-    state.push_copy(exec_step, copy_event);
+    state.push_copy(exec_step, copy_event)?;
 
     Ok(())
 }
diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs
@@ -63,7 +63,7 @@ impl Opcode for Calldatacopy {
         };
 
         let copy_event = gen_copy_event(state, memory_offset, data_offset, length, &mut exec_step)?;
-        state.push_copy(&mut exec_step, copy_event);
+        state.push_copy(&mut exec_step, copy_event)?;
         Ok(vec![exec_step])
     }
 }

diff --git a/bus-mapping/src/evm/opcodes/callop.rs b/bus-mapping/src/evm/opcodes/callop.rs
@@ -415,7 +415,7 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                             copy_bytes: CopyBytes::new(copy_steps, None, None),
                             access_list: vec![],
                         },
-                    );
+                    )?;
                     Some(input_bytes)
                 } else {
                     None
@@ -446,7 +446,7 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                             copy_bytes: CopyBytes::new(copy_steps, None, Some(prev_bytes)),
                             access_list: vec![],
                         },
-                    );
+                    )?;
                     Some(output_bytes)
                 } else {
                     None
@@ -486,7 +486,7 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                             ),
                             access_list: vec![],
                         },
-                    );
+                    )?;
                     Some(returned_bytes)
                 } else {
                     None

diff --git a/bus-mapping/src/evm/opcodes/codecopy.rs b/bus-mapping/src/evm/opcodes/codecopy.rs
@@ -37,7 +37,7 @@ impl Opcode for Codecopy {
             length.as_u64(),
             &mut exec_step,
         )?;
-        state.push_copy(&mut exec_step, copy_event);
+        state.push_copy(&mut exec_step, copy_event)?;
         Ok(vec![exec_step])
     }
 }

diff --git a/bus-mapping/src/evm/opcodes/create.rs b/bus-mapping/src/evm/opcodes/create.rs
@@ -398,7 +398,7 @@ fn handle_copy(
             copy_bytes: CopyBytes::new(copy_steps, None, None),
             access_list: vec![],
         },
-    );
+    )?;
 
     Ok((initcode, keccak_code_hash, code_hash))
 }

diff --git a/bus-mapping/src/evm/opcodes/extcodecopy.rs b/bus-mapping/src/evm/opcodes/extcodecopy.rs
@@ -80,7 +80,7 @@ impl Opcode for Extcodecopy {
             length,
             &mut exec_step,
         )?;
-        state.push_copy(&mut exec_step, copy_event);
+        state.push_copy(&mut exec_step, copy_event)?;
         Ok(vec![exec_step])
     }
 }

diff --git a/bus-mapping/src/evm/opcodes/logs.rs b/bus-mapping/src/evm/opcodes/logs.rs
@@ -95,7 +95,7 @@ impl Opcode for Log {
             // https://github.com/ethereum/go-ethereum/blob/b80f05bde2c4e93ae64bb3813b6d67266b5fc0e6/core/vm/instructions.go#L850
             let copy_event =
                 gen_copy_event(state, mstart.low_u64(), msize.low_u64(), &mut exec_step)?;
-            state.push_copy(&mut exec_step, copy_event);
+            state.push_copy(&mut exec_step, copy_event)?;
             state.tx_ctx.log_id += 1;
         }
 

diff --git a/bus-mapping/src/evm/opcodes/mcopy.rs b/bus-mapping/src/evm/opcodes/mcopy.rs
@@ -37,7 +37,7 @@ impl Opcode for MCopy {
             length.as_u64(),
             &mut exec_step,
         )?;
-        state.push_copy(&mut exec_step, copy_event);
+        state.push_copy(&mut exec_step, copy_event)?;
         Ok(vec![exec_step])
     }
 }

diff --git a/bus-mapping/src/evm/opcodes/return_revert.rs b/bus-mapping/src/evm/opcodes/return_revert.rs
@@ -296,7 +296,7 @@ fn handle_copy(
             copy_bytes: CopyBytes::new(read_steps, Some(write_steps), Some(dst_data_prev)),
             access_list: vec![],
         },
-    );
+    )?;
 
     Ok(())
 }
@@ -363,7 +363,7 @@ fn handle_create(
             copy_bytes: CopyBytes::new(copy_steps, None, None),
             access_list: vec![],
         },
-    );
+    )?;
 
     Ok(AccountCodeInfo {
         keccak_hash,

diff --git a/bus-mapping/src/evm/opcodes/returndatacopy.rs b/bus-mapping/src/evm/opcodes/returndatacopy.rs
@@ -61,7 +61,7 @@ impl Opcode for Returndatacopy {
         }
 
         let copy_event = gen_copy_event(state, memory_offset, data_offset, length, &mut exec_step)?;
-        state.push_copy(&mut exec_step, copy_event);
+        state.push_copy(&mut exec_step, copy_event)?;
         Ok(vec![exec_step])
     }
 }

diff --git a/bus-mapping/src/evm/opcodes/sha3.rs b/bus-mapping/src/evm/opcodes/sha3.rs
@@ -94,7 +94,7 @@ impl Opcode for Sha3 {
                 copy_bytes: CopyBytes::new(copy_steps, None, None),
                 access_list: vec![],
             },
-        );
+        )?;
 
         Ok(vec![exec_step])
     }

diff --git a/zkevm-circuits/src/witness/mpt.rs b/zkevm-circuits/src/witness/mpt.rs
@@ -183,13 +183,14 @@ impl MptUpdates {
 
         let root_pair2 = (self.old_root, self.new_root);
         if root_pair2 != root_pair {
-            log::error!(
+            log::warn!(
                 "roots non consistent ({:#x},{:#x}) vs ({:#x},{:#x})",
                 root_pair.0,
                 root_pair.1,
                 root_pair2.0,
                 root_pair2.1
             );
+            #[cfg(debug_assertions)]
             wit_gen.dump(
                 self.updates
                     .iter()