Merge branch 'main' into feat/testool_scroll

.github/workflows/lints.yml

@@ -20,7 +20,7 @@ jobs:
         with:
           cancel_others: 'true'
           concurrent_skipping: 'same_content_newer'
-          paths_ignore: '["**/README.md"]'
+          paths_ignore: '["**/README.md", "mpt-witness-generator/**"]'
 
   lints:
     needs: [skip_check]

.github/workflows/mpt-witness-generator.yml

@@ -0,0 +1,49 @@
+name: MPT Witness Generator Checks
+
+on:
+  merge_group:
+  pull_request:
+    types: [synchronize, opened, reopened, ready_for_review]
+  push:
+    branches:
+      - main
+
+jobs:
+  skip_check:
+    runs-on: ubuntu-latest
+    outputs:
+      should_skip: ${{ steps.skip_check.outputs.should_skip }}
+    steps:
+      - id: skip_check
+        uses: fkirc/skip-duplicate-actions@v5
+        with:
+          cancel_others: 'true'
+          concurrent_skipping: 'same_content_newer'
+          paths: '["mpt-witness-generator/**"]'
+
+
+  build:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v3
+
+    - name: Set up Go
+      uses: actions/setup-go@v4
+      with:
+        go-version: '1.20'
+
+    - name: Format
+      uses: Jerome1337/gofmt-action@v1.0.5
+      with:
+        gofmt-path: './mpt-witness-generator'
+        gofmt-flags: '-l -d'
+
+    - name: Build
+      working-directory: ./mpt-witness-generator
+      run: go build -v ./...
+
+    - name: Test
+      working-directory: ./mpt-witness-generator
+      env:
+        NO_GETH: true
+      run: go test -v ./...

bus-mapping/src/circuit_input_builder.rs

@@ -388,7 +388,7 @@ impl CircuitInputBuilder<DynamicCParams> {
                 .iter()
                 .fold(0, |acc, c| acc + c.bytes.len())
                 * 2
-                + 2;
+                + 4; // disabled and unused rows.
 
             let total_rws_before_padding: usize =
                 <RWCounter as Into<usize>>::into(self.block_ctx.rwc) - 1; // -1 since rwc start from index `1`

bus-mapping/src/circuit_input_builder/input_state_ref.rs

@@ -611,6 +611,43 @@ impl<'a> CircuitInputStateRef<'a> {
         )
     }
 
+    /// Transfer to an address irreversibly.
+    pub fn transfer_to_irreversible(
+        &mut self,
+        step: &mut ExecStep,
+        receiver: Address,
+        receiver_exists: bool,
+        must_create: bool,
+        value: Word,
+    ) -> Result<(), Error> {
+        // If receiver doesn't exist, create it
+        if (!receiver_exists && !value.is_zero()) || must_create {
+            self.account_write(
+                step,
+                receiver,
+                AccountField::CodeHash,
+                CodeDB::empty_code_hash().to_word(),
+                Word::zero(),
+            )?;
+        }
+        if value.is_zero() {
+            // Skip transfer if value == 0
+            return Ok(());
+        }
+        let (_found, receiver_account) = self.sdb.get_account(&receiver);
+        let receiver_balance_prev = receiver_account.balance;
+        let receiver_balance = receiver_account.balance + value;
+        self.account_write(
+            step,
+            receiver,
+            AccountField::Balance,
+            receiver_balance,
+            receiver_balance_prev,
+        )?;
+
+        Ok(())
+    }
+
     /// Fetch and return code for the given code hash from the code DB.
     pub fn code(&self, code_hash: H256) -> Result<Vec<u8>, Error> {
         self.code_db

bus-mapping/src/circuit_input_builder/transaction.rs

@@ -241,7 +241,7 @@ impl Transaction {
                 code_hash,
                 depth: 1,
                 value: eth_tx.value,
-                call_data_length: eth_tx.input.len().try_into().unwrap(),
+                call_data_length: 0,
                 ..Default::default()
             }
         };

bus-mapping/src/error.rs

@@ -92,8 +92,8 @@ pub enum OogError {
     SloadSstore,
     /// Out of Gas for CALL, CALLCODE, DELEGATECALL and STATICCALL
     Call,
-    /// Out of Gas for CREATE2
-    Create2,
+    /// Out of Gas for CREATE and CREATE2
+    Create,
     /// Out of Gas for SELFDESTRUCT
     SelfDestruct,
 }
@@ -105,9 +105,7 @@ impl From<&OpcodeId> for OogError {
             OpcodeId::MLOAD | OpcodeId::MSTORE | OpcodeId::MSTORE8 => {
                 OogError::StaticMemoryExpansion
             }
-            OpcodeId::CREATE | OpcodeId::RETURN | OpcodeId::REVERT => {
-                OogError::DynamicMemoryExpansion
-            }
+            OpcodeId::RETURN | OpcodeId::REVERT => OogError::DynamicMemoryExpansion,
             OpcodeId::CALLDATACOPY
             | OpcodeId::CODECOPY
             | OpcodeId::EXTCODECOPY
@@ -124,7 +122,7 @@ impl From<&OpcodeId> for OogError {
                 OogError::Call
             }
             OpcodeId::SLOAD | OpcodeId::SSTORE => OogError::SloadSstore,
-            OpcodeId::CREATE2 => OogError::Create2,
+            OpcodeId::CREATE | OpcodeId::CREATE2 => OogError::Create,
             OpcodeId::SELFDESTRUCT => OogError::SelfDestruct,
             _ => OogError::Constant,
         }

bus-mapping/src/evm/opcodes.rs

@@ -270,12 +270,20 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps {
     }
 }
 
-fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option<FnGenAssociatedOps> {
+fn fn_gen_error_state_associated_ops(
+    geth_step: &GethExecStep,
+    error: &ExecError,
+) -> Option<FnGenAssociatedOps> {
     match error {
         ExecError::InvalidJump => Some(InvalidJump::gen_associated_ops),
         ExecError::InvalidOpcode => Some(ErrorSimple::gen_associated_ops),
         ExecError::OutOfGas(OogError::Call) => Some(OOGCall::gen_associated_ops),
         ExecError::OutOfGas(OogError::Constant) => Some(ErrorSimple::gen_associated_ops),
+        ExecError::OutOfGas(OogError::Create) => match geth_step.op {
+            OpcodeId::CREATE => Some(StackOnlyOpcode::<3, 0, true>::gen_associated_ops),
+            OpcodeId::CREATE2 => Some(StackOnlyOpcode::<4, 0, true>::gen_associated_ops),
+            op => unreachable!("OOG Create cannot occur in {op}"),
+        },
         ExecError::OutOfGas(OogError::Exp) => Some(OOGExp::gen_associated_ops),
         ExecError::OutOfGas(OogError::Log) => Some(ErrorOOGLog::gen_associated_ops),
         ExecError::OutOfGas(OogError::MemoryCopy) => Some(OOGMemoryCopy::gen_associated_ops),
@@ -370,7 +378,7 @@ pub fn gen_associated_ops(
         // TODO: after more error state handled, refactor all error handling in
         // fn_gen_error_state_associated_ops method
         // For exceptions that have been implemented
-        if let Some(fn_gen_error_ops) = fn_gen_error_state_associated_ops(&exec_error) {
+        if let Some(fn_gen_error_ops) = fn_gen_error_state_associated_ops(geth_step, &exec_error) {
             return fn_gen_error_ops(state, geth_steps);
         } else {
             // For exceptions that fail to enter next call context, we need

bus-mapping/src/evm/opcodes/begin_end_tx.rs

@@ -277,10 +277,8 @@ fn gen_end_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Error>
     if !found {
         return Err(Error::AccountNotFound(state.block.coinbase));
     }
-    let coinbase_account = coinbase_account.clone();
-    let coinbase_balance_prev = coinbase_account.balance;
+    let coinbase_exist = !coinbase_account.is_empty();
     let coinbase_transfer_value = effective_tip * (state.tx.gas() - exec_step.gas_left);
-    let coinbase_balance = coinbase_balance_prev + coinbase_transfer_value;
     state.account_read(
         &mut exec_step,
         state.block.coinbase,
@@ -291,21 +289,12 @@ fn gen_end_tx_steps(state: &mut CircuitInputStateRef) -> Result<ExecStep, Error>
             coinbase_account.code_hash.to_word()
         },
     );
-    if coinbase_account.is_empty() {
-        state.account_write(
-            &mut exec_step,
-            state.block.coinbase,
-            AccountField::CodeHash,
-            CodeDB::empty_code_hash().to_word(),
-            Word::zero(),
-        )?;
-    }
-    state.account_write(
+    state.transfer_to_irreversible(
         &mut exec_step,
         state.block.coinbase,
-        AccountField::Balance,
-        coinbase_balance,
-        coinbase_balance_prev,
+        coinbase_exist,
+        false,
+        coinbase_transfer_value,
     )?;
 
     // handle tx receipt tag

bus-mapping/src/evm/opcodes/callop.rs

@@ -100,7 +100,8 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
         )?;
 
         let callee_code_hash = call.code_hash;
-        let callee_exists = !state.sdb.get_account(&callee_address).1.is_empty();
+        let callee = state.sdb.get_account(&callee_address).1.clone();
+        let callee_exists = !callee.is_empty();
 
         let (callee_code_hash_word, is_empty_code_hash) = if callee_exists {
             (
@@ -145,11 +146,13 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
         debug_assert!(found);
 
         let caller_balance = sender_account.balance;
+
         let is_call_or_callcode = call.kind == CallKind::Call || call.kind == CallKind::CallCode;
+        let is_sufficient = caller_balance >= call.value;
+        let is_valid_depth = geth_step.depth < 1025;
 
         // Precheck is OK when depth is in range and caller balance is sufficient
-        let is_precheck_ok =
-            geth_step.depth < 1025 && (!is_call_or_callcode || caller_balance >= call.value);
+        let is_precheck_ok = is_valid_depth && (is_sufficient || !is_call_or_callcode);
 
         log::debug!(
             "is_precheck_ok: {}, call type: {:?}, sender_account: {:?} ",
@@ -173,14 +176,17 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
         let is_precompile = code_address
             .map(|ref addr| is_precompiled(addr))
             .unwrap_or(false);
-        // TODO: What about transfer for CALLCODE?
-        // Transfer value only for CALL opcode, is_precheck_ok = true.
-        if call.kind == CallKind::Call && is_precheck_ok {
+        // CALLCODE does not need to do real transfer
+        // Transfer value only when all these conditions met:
+        // - The opcode is CALL
+        // - The precheck passed
+        // - The value to send is not zero
+        if call.kind == CallKind::Call && is_precheck_ok && !call.value.is_zero() {
             state.transfer(
                 &mut exec_step,
                 call.caller_address,
                 call.address,
-                callee_exists || is_precompile,
+                callee_exists,
                 false,
                 call.value,
             )?;
@@ -221,10 +227,9 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
 
         // There are 4 branches from here.
         // add failure case for insufficient balance or error depth in the future.
-        match (!is_precheck_ok, is_precompile, is_empty_code_hash) {
+        match (is_precheck_ok, is_precompile, is_empty_code_hash) {
             // 1. Call to precompiled.
-            (false, true, _) => {
-                assert!(call.is_success, "call to precompile should not fail");
+            (true, true, _) => {
                 let caller_ctx = state.caller_ctx_mut()?;
                 let code_address = code_address.unwrap();
                 let (result, contract_gas_cost) = execute_precompiled(
@@ -275,9 +280,9 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                 Ok(vec![exec_step])
             }
             // 2. Call to account with empty code.
-            (false, _, true) => {
+            (true, _, true) => {
                 for (field, value) in [
-                    (CallContextField::LastCalleeId, 0.into()),
+                    (CallContextField::LastCalleeId, call.call_id.into()),
                     (CallContextField::LastCalleeReturnDataOffset, 0.into()),
                     (CallContextField::LastCalleeReturnDataLength, 0.into()),
                 ] {
@@ -287,7 +292,7 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                 Ok(vec![exec_step])
             }
             // 3. Call to account with non-empty code.
-            (false, _, false) => {
+            (true, _, false) => {
                 for (field, value) in [
                     (CallContextField::ProgramCounter, (geth_step.pc + 1).into()),
                     (
@@ -349,9 +354,9 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
             }
 
             // 4. insufficient balance or error depth cases.
-            (true, _, _) => {
+            (false, _, _) => {
                 for (field, value) in [
-                    (CallContextField::LastCalleeId, 0.into()),
+                    (CallContextField::LastCalleeId, call.call_id.into()),
                     (CallContextField::LastCalleeReturnDataOffset, 0.into()),
                     (CallContextField::LastCalleeReturnDataLength, 0.into()),
                 ] {

bus-mapping/src/evm/opcodes/sload.rs

@@ -82,6 +82,17 @@ impl Opcode for Sload {
 
         // First stack write
         state.stack_write(&mut exec_step, stack_position, value)?;
+        state.push_op(
+            &mut exec_step,
+            RW::READ,
+            TxAccessListAccountStorageOp {
+                tx_id: state.tx_ctx.id(),
+                address: contract_addr,
+                key,
+                is_warm,
+                is_warm_prev: is_warm,
+            },
+        );
         state.push_op_reversible(
             &mut exec_step,
             TxAccessListAccountStorageOp {
@@ -189,7 +200,7 @@ mod sload_tests {
         );
 
         let access_list_op = &builder.block.container.tx_access_list_account_storage
-            [step.bus_mapping_instance[7].as_usize()];
+            [step.bus_mapping_instance[8].as_usize()];
         assert_eq!(
             (access_list_op.rw(), access_list_op.op()),
             (

bus-mapping/src/evm/opcodes/sstore.rs

@@ -1,10 +1,9 @@
 use super::Opcode;
 use crate::{
     circuit_input_builder::{CircuitInputStateRef, ExecStep},
-    operation::{CallContextField, StorageOp, TxAccessListAccountStorageOp, TxRefundOp},
+    operation::{CallContextField, StorageOp, TxAccessListAccountStorageOp, TxRefundOp, RW},
     Error,
 };
-
 use eth_types::{GethExecStep, ToWord, Word};
 
 /// Placeholder structure used to implement [`Opcode`] trait over it
@@ -86,6 +85,17 @@ impl Opcode for Sstore {
             ),
         )?;
 
+        state.push_op(
+            &mut exec_step,
+            RW::READ,
+            TxAccessListAccountStorageOp {
+                tx_id: state.tx_ctx.id(),
+                address: contract_addr,
+                key,
+                is_warm,
+                is_warm_prev: is_warm,
+            },
+        );
         state.push_op_reversible(
             &mut exec_step,
             TxAccessListAccountStorageOp {
@@ -250,7 +260,8 @@ mod sstore_tests {
                 )
             )
         );
-        let refund_op = &builder.block.container.tx_refund[step.bus_mapping_instance[9].as_usize()];
+        let refund_op =
+            &builder.block.container.tx_refund[step.bus_mapping_instance[10].as_usize()];
         assert_eq!(
             (refund_op.rw(), refund_op.op()),
             (