fix address assignment, ci evm unittest

privacy-scaling-explorations · Jun 21, 2023 · f82356f · f82356f
1 parent 0a0d712
commit f82356f
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 14 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -28,6 +28,62 @@ jobs:
           concurrent_skipping: 'same_content_newer'
           paths_ignore: '["**/README.md"]'
 
+  test:
+    needs: [skip_check]
+    if: |
+      github.event.pull_request.draft == false &&
+      (github.event.action == 'ready_for_review' || needs.skip_check.outputs.should_skip != 'true')
+
+    name: Test
+    runs-on: ["${{github.run_id}}", self-hosted, c5.9xlarge]
+
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          override: false
+      - name: Setup golang
+        uses: actions/setup-go@v3
+        with:
+          go-version: ~1.19
+      # Go cache for building geth-utils
+      - name: Go cache
+        uses: actions/cache@v3
+        with:
+          path: |
+            ~/.cache/go-build
+            ~/go/pkg/mod
+          key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ runner.os }}-go-
+      - name: Cargo cache
+        uses: actions/cache@v3
+        with:
+          path: |
+            ~/.cargo/bin/
+            ~/.cargo/registry/index/
+            ~/.cargo/registry/cache/
+            ~/.cargo/git/db/
+            target/
+          key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
+      # TODO add back light tests
+      # - name: Run light tests # light tests are run in parallel
+      #     uses: actions-rs/cargo@v1
+      #     with:
+      #       command: test
+      #       args: --verbose --release --all --all-features --exclude integration-tests --exclude circuit-benchmarks
+      # TODO remove sub circuit light tests
+      - name: Run EVM circuit light tests # light tests are run in parallel
+        uses: actions-rs/cargo@v1
+        with:
+          command: test
+          args: --verbose --release --all --all-features --exclude integration-tests --exclude circuit-benchmarks --package zkevm-circuits --lib -- evm_circuit # ...more subcircuits
+      # - name: Run heavy tests # heavy tests are run serially to avoid OOM
+      #   uses: actions-rs/cargo@v1
+      #   with:
+      #     command: test
+      #     args: --verbose --release --all --all-features --exclude integration-tests --exclude circuit-benchmarks serial_ -- --ignored --test-threads 1
+
   build:
     needs: [skip_check]
     if: |
@@ -174,5 +230,4 @@ jobs:
           args: --all -- --check
 
 # TODO: We plan to bring the following Github action back when merge to main
-# test:
 # bitrot:
diff --git a/zkevm-circuits/src/evm_circuit/execution/balance.rs b/zkevm-circuits/src/evm_circuit/execution/balance.rs
@@ -19,7 +19,7 @@ use crate::{
         Expr,
     },
 };
-use eth_types::{evm_types::GasCost, Field, ToWord};
+use eth_types::{evm_types::GasCost, Field, ToAddress, ToWord};
 use halo2_proofs::{circuit::Value, plonk::Error};
 
 #[derive(Clone, Debug)]
@@ -118,7 +118,8 @@ impl<F: Field> ExecutionGadget<F> for BalanceGadget<F> {
         self.same_context.assign_exec_step(region, offset, step)?;
 
         let address = block.get_rws(step, 0).stack_value();
-        self.address.assign_u256(region, offset, address)?;
+        self.address
+            .assign_h160(region, offset, address.to_address())?;
 
         self.tx_id
             .assign(region, offset, Value::known(F::from(tx.id as u64)))?;

diff --git a/zkevm-circuits/src/evm_circuit/execution/callop.rs b/zkevm-circuits/src/evm_circuit/execution/callop.rs
@@ -25,7 +25,7 @@ use crate::{
     util::Expr,
 };
 use bus_mapping::evm::OpcodeId;
-use eth_types::{evm_types::GAS_STIPEND_CALL_WITH_VALUE, Field, U256};
+use eth_types::{evm_types::GAS_STIPEND_CALL_WITH_VALUE, Field, ToAddress, U256};
 use halo2_proofs::{circuit::Value, plonk::Error};
 
 /// Gadget for call related opcodes. It supports `OpcodeId::CALL`,
@@ -561,10 +561,16 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
             call.rw_counter_end_of_reversion,
             call.is_persistent,
         )?;
-        self.current_callee_address
-            .assign_u256(region, offset, current_callee_address)?;
-        self.current_caller_address
-            .assign_u256(region, offset, current_caller_address)?;
+        self.current_callee_address.assign_h160(
+            region,
+            offset,
+            current_callee_address.to_address(),
+        )?;
+        self.current_caller_address.assign_h160(
+            region,
+            offset,
+            current_caller_address.to_address(),
+        )?;
         self.current_value
             .assign_u256(region, offset, current_value)?;
         self.is_static

diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs
@@ -20,7 +20,7 @@ use crate::{
 };
 use eth_types::{
     evm_types::{GasCost, OpcodeId},
-    Field, U256,
+    Field, ToAddress, U256,
 };
 use halo2_proofs::{circuit::Value, plonk::Error};
 
@@ -187,7 +187,7 @@ impl<F: Field> ExecutionGadget<F> for ErrorOOGMemoryCopyGadget<F> {
         self.tx_id
             .assign(region, offset, Value::known(F::from(transaction.id as u64)))?;
         self.external_address
-            .assign_u256(region, offset, external_address)?;
+            .assign_h160(region, offset, external_address.to_address())?;
         self.src_offset.assign_u256(region, offset, src_offset)?;
         let memory_addr = self
             .dst_memory_addr

diff --git a/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs b/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs
@@ -16,7 +16,7 @@ use crate::{
         Expr,
     },
 };
-use eth_types::{evm_types::OpcodeId, Field, U256};
+use eth_types::{evm_types::OpcodeId, Field, ToAddress, U256};
 use halo2_proofs::{circuit::Value, plonk::Error};
 
 #[derive(Clone, Debug)]
@@ -116,7 +116,7 @@ impl<F: Field> ExecutionGadget<F> for ErrorWriteProtectionGadget<F> {
 
         self.gas.assign_u256(region, offset, gas)?;
         self.code_address
-            .assign_u256(region, offset, code_address)?;
+            .assign_h160(region, offset, code_address.to_address())?;
         self.value.assign_u256(region, offset, value)?;
 
         self.is_call.assign(

diff --git a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs
@@ -27,7 +27,7 @@ use crate::{
     witness::{Block, Call, ExecStep},
 };
 use bus_mapping::state_db::CodeDB;
-use eth_types::{evm_types::GasCost, Field, ToLittleEndian, ToScalar, ToWord, U256};
+use eth_types::{evm_types::GasCost, Field, ToAddress, ToLittleEndian, ToScalar, ToWord, U256};
 use gadgets::util::{select, sum};
 use halo2_proofs::{
     circuit::Value,
@@ -757,7 +757,7 @@ impl<F: Field, const IS_SUCCESS_CALL: bool> CommonCallGadget<F, IS_SUCCESS_CALL>
     ) -> Result<u64, Error> {
         self.gas.assign_u256(region, offset, gas)?;
         self.callee_address_word
-            .assign_u256(region, offset, callee_address)?;
+            .assign_h160(region, offset, callee_address.to_address())?;
         self.value.assign_u256(region, offset, value)?;
         if IS_SUCCESS_CALL {
             self.is_success