Update codehash for successful calls when is_create=True (privacy-sca…

…ling-explorations#908) * Fix test to actually run init code * Fix rw consistency * Write 0 for CREATE(2) if init call reverts * Allow conditional reverisible writes * Implement ToWord for usize and u64 * ToScalar for u64 and usize * Update codehash for sucessful initialization calls * fmt * cleanup * fix build * Rename return to return_revert * Fix build * fmt Co-authored-by: Mason Liang <[email protected]> Co-authored-by: Mason Liang <[email protected]> Co-authored-by: Zhang Zhuo <[email protected]>
lfzkoala · Dec 2, 2022 · 098cd62 · 098cd62
1 parent 3f7e733
commit 098cd62
Show file tree

Hide file tree

Showing 11 changed files with 300 additions and 61 deletions.
diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs
@@ -913,7 +913,7 @@ impl<'a> CircuitInputStateRef<'a> {
 
         let memory_expansion_gas_cost =
             memory_expansion_gas_cost(curr_memory_word_size, next_memory_word_size);
-        let code_deposit_cost = if call.is_create() {
+        let code_deposit_cost = if call.is_create() && call.is_success {
             GasCost::CODE_DEPOSIT_BYTE_COST.as_u64() * last_callee_return_data_length.as_u64()
         } else {
             0

diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs
@@ -42,7 +42,7 @@ mod mload;
 mod mstore;
 mod number;
 mod origin;
-mod r#return;
+mod return_revert;
 mod returndatacopy;
 mod returndatasize;
 mod selfbalance;
@@ -81,7 +81,7 @@ use logs::Log;
 use mload::Mload;
 use mstore::Mstore;
 use origin::Origin;
-use r#return::Return;
+use return_revert::ReturnRevert;
 use returndatacopy::Returndatacopy;
 use returndatasize::Returndatasize;
 use selfbalance::Selfbalance;
@@ -232,9 +232,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps {
         OpcodeId::LOG4 => Log::gen_associated_ops,
         OpcodeId::CALL | OpcodeId::CALLCODE => CallOpcode::<7>::gen_associated_ops,
         OpcodeId::DELEGATECALL | OpcodeId::STATICCALL => CallOpcode::<6>::gen_associated_ops,
-        OpcodeId::RETURN => Return::gen_associated_ops,
-        // REVERT is almost the same as RETURN
-        OpcodeId::REVERT => Return::gen_associated_ops,
+        OpcodeId::RETURN | OpcodeId::REVERT => ReturnRevert::gen_associated_ops,
         OpcodeId::SELFDESTRUCT => {
             warn!("Using dummy gen_selfdestruct_ops for opcode SELFDESTRUCT");
             DummySelfDestruct::gen_associated_ops

diff --git a/bus-mapping/src/evm/opcodes/create.rs b/bus-mapping/src/evm/opcodes/create.rs
@@ -1,8 +1,8 @@
 use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep};
 use crate::evm::Opcode;
-use crate::operation::{AccountField, AccountOp, TxAccessListAccountOp, RW};
+use crate::operation::{AccountField, AccountOp, CallContextField, TxAccessListAccountOp, RW};
 use crate::Error;
-use eth_types::{GethExecStep, ToWord};
+use eth_types::{evm_types::gas_utils::memory_expansion_gas_cost, GethExecStep, ToWord, Word};
 use keccak256::EMPTY_HASH;
 
 #[derive(Debug, Copy, Clone)]
@@ -19,12 +19,14 @@ impl<const IS_CREATE2: bool> Opcode for DummyCreate<IS_CREATE2> {
         let offset = geth_step.stack.nth_last(1)?.as_usize();
         let length = geth_step.stack.nth_last(2)?.as_usize();
 
+        let curr_memory_word_size = (state.call_ctx()?.memory.len() as u64) / 32;
         if length != 0 {
             state
                 .call_ctx_mut()?
                 .memory
                 .extend_at_least(offset + length);
         }
+        let next_memory_word_size = (state.call_ctx()?.memory.len() as u64) / 32;
 
         let mut exec_step = state.new_step(geth_step)?;
 
@@ -43,14 +45,20 @@ impl<const IS_CREATE2: bool> Opcode for DummyCreate<IS_CREATE2> {
             state.create_address()?
         };
 
+        // TODO: rename this to initialization call?
+        let call = state.parse_call(geth_step)?;
         state.stack_write(
             &mut exec_step,
             geth_step.stack.nth_last_filled(n_pop - 1),
-            address.to_word(),
+            if call.is_success {
+                address.to_word()
+            } else {
+                Word::zero()
+            },
         )?;
 
         let tx_id = state.tx_ctx.id();
-        let call = state.parse_call(geth_step)?;
+        let current_call = state.call()?.clone();
 
         // Quote from [EIP-2929](https://eips.ethereum.org/EIPS/eip-2929)
         // > When a CREATE or CREATE2 opcode is called,
@@ -119,6 +127,53 @@ impl<const IS_CREATE2: bool> Opcode for DummyCreate<IS_CREATE2> {
             call.value,
         )?;
 
+        let memory_expansion_gas_cost =
+            memory_expansion_gas_cost(curr_memory_word_size, next_memory_word_size);
+
+        // EIP-150: all but one 64th of the caller's gas is sent to the callee.
+        let caller_gas_left =
+            (geth_step.gas.0 - geth_step.gas_cost.0 - memory_expansion_gas_cost) / 64;
+
+        for (field, value) in [
+            (
+                CallContextField::ProgramCounter,
+                (geth_step.pc.0 + 1).into(),
+            ),
+            (
+                CallContextField::StackPointer,
+                geth_step.stack.nth_last_filled(n_pop - 1).0.into(),
+            ),
+            (CallContextField::GasLeft, caller_gas_left.into()),
+            (CallContextField::MemorySize, next_memory_word_size.into()),
+            (
+                CallContextField::ReversibleWriteCounter,
+                // +3 is because we do some transfers after pushing the call. can be just push the
+                // call later?
+                (exec_step.reversible_write_counter + 3).into(),
+            ),
+        ] {
+            state.call_context_write(&mut exec_step, current_call.call_id, field, value);
+        }
+
+        for (field, value) in [
+            (CallContextField::CallerId, current_call.call_id.into()),
+            (CallContextField::IsSuccess, call.is_success.to_word()),
+            (CallContextField::IsPersistent, call.is_persistent.to_word()),
+            (CallContextField::TxId, state.tx_ctx.id().into()),
+            (
+                CallContextField::CallerAddress,
+                current_call.address.to_word(),
+            ),
+            (CallContextField::CalleeAddress, call.address.to_word()),
+            (
+                CallContextField::RwCounterEndOfReversion,
+                call.rw_counter_end_of_reversion.to_word(),
+            ),
+            (CallContextField::IsPersistent, call.is_persistent.to_word()),
+        ] {
+            state.call_context_write(&mut exec_step, call.call_id, field, value);
+        }
+
         if call.code_hash.to_fixed_bytes() == *EMPTY_HASH {
             // 1. Create with empty initcode.
             state.handle_return(geth_step)?;

diff --git a/bus-mapping/src/evm/opcodes/return.rs → bus-mapping/src/evm/opcodes/return_revert.rs b/bus-mapping/src/evm/opcodes/return.rs → bus-mapping/src/evm/opcodes/return_revert.rs
@@ -1,20 +1,22 @@
 use super::Opcode;
 use crate::circuit_input_builder::{CopyDataType, CopyEvent, NumberOrHash};
+use crate::operation::AccountOp;
 use crate::operation::MemoryOp;
 use crate::{
     circuit_input_builder::CircuitInputStateRef,
     evm::opcodes::ExecStep,
-    operation::{CallContextField, RW},
+    operation::{AccountField, CallContextField, RW},
     Error,
 };
-use eth_types::{Bytecode, GethExecStep, ToWord, H256};
+use eth_types::{Bytecode, GethExecStep, ToWord, Word, H256};
 use ethers_core::utils::keccak256;
+use keccak256::EMPTY_HASH_LE;
 
 #[derive(Debug, Copy, Clone)]
-pub(crate) struct Return;
+pub(crate) struct ReturnRevert;
 
 // TODO: rename to indicate this handles REVERT (and maybe STOP)?
-impl Opcode for Return {
+impl Opcode for ReturnRevert {
     fn gen_associated_ops(
         state: &mut CircuitInputStateRef,
         steps: &[GethExecStep],
@@ -49,7 +51,7 @@ impl Opcode for Return {
         if call.is_create() && call.is_success && length > 0 {
             // Note: handle_return updates state.code_db. All we need to do here is push the
             // copy event.
-            handle_create(
+            let code_hash = handle_create(
                 state,
                 &mut exec_step,
                 Source {
@@ -58,6 +60,29 @@ impl Opcode for Return {
                     length,
                 },
             )?;
+
+            for (field, value) in [
+                (CallContextField::CallerId, call.caller_id.to_word()),
+                (CallContextField::CalleeAddress, call.address.to_word()),
+                (
+                    CallContextField::RwCounterEndOfReversion,
+                    call.rw_counter_end_of_reversion.to_word(),
+                ),
+                (CallContextField::IsPersistent, call.is_persistent.to_word()),
+            ] {
+                state.call_context_read(&mut exec_step, state.call()?.call_id, field, value);
+            }
+
+            state.push_op_reversible(
+                &mut exec_step,
+                RW::WRITE,
+                AccountOp {
+                    address: state.call()?.address,
+                    field: AccountField::CodeHash,
+                    value: code_hash.to_word(),
+                    value_prev: Word::from_little_endian(&*EMPTY_HASH_LE),
+                },
+            )?;
         }
 
         // Case B in the specs.
@@ -196,9 +221,10 @@ fn handle_create(
     state: &mut CircuitInputStateRef,
     step: &mut ExecStep,
     source: Source,
-) -> Result<(), Error> {
+) -> Result<H256, Error> {
     let values = state.call_ctx()?.memory.0[source.offset..source.offset + source.length].to_vec();
-    let dst_id = NumberOrHash::Hash(H256(keccak256(&values)));
+    let code_hash = H256(keccak256(&values));
+    let dst_id = NumberOrHash::Hash(code_hash);
     let bytes: Vec<_> = Bytecode::from(values)
         .code
         .iter()
@@ -227,7 +253,7 @@ fn handle_create(
         bytes,
     });
 
-    Ok(())
+    Ok(code_hash)
 }
 
 #[cfg(test)]

diff --git a/eth-types/src/lib.rs b/eth-types/src/lib.rs
@@ -195,6 +195,20 @@ impl ToWord for bool {
     }
 }
 
+impl ToWord for u64 {
+    fn to_word(&self) -> Word {
+        Word::from(*self)
+    }
+}
+
+impl ToWord for usize {
+    fn to_word(&self) -> Word {
+        u64::try_from(*self)
+            .expect("usize bigger than u64")
+            .to_word()
+    }
+}
+
 impl<F: Field> ToScalar<F> for Address {
     fn to_scalar(&self) -> Option<F> {
         let mut bytes = [0u8; 32];
@@ -210,6 +224,18 @@ impl<F: Field> ToScalar<F> for bool {
     }
 }
 
+impl<F: Field> ToScalar<F> for u64 {
+    fn to_scalar(&self) -> Option<F> {
+        Some(F::from(*self))
+    }
+}
+
+impl<F: Field> ToScalar<F> for usize {
+    fn to_scalar(&self) -> Option<F> {
+        u64::try_from(*self).ok().map(F::from)
+    }
+}
+
 /// Struct used to define the storage proof
 #[derive(Debug, Default, Clone, PartialEq, Eq, Deserialize)]
 pub struct StorageProof {

diff --git a/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs b/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs
@@ -119,7 +119,15 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
         // When it's an internal call, need to restore caller's state as finishing this
         // call. Restore caller state to next StepState
         let restore_context = cb.condition(1.expr() - cb.curr.state.is_root.expr(), |cb| {
-            RestoreContextGadget::construct(cb, 0.expr(), 0.expr(), 0.expr(), 0.expr(), 0.expr())
+            RestoreContextGadget::construct(
+                cb,
+                0.expr(),
+                0.expr(),
+                0.expr(),
+                0.expr(),
+                0.expr(),
+                0.expr(),
+            )
         });
 
         Self {

diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_constant.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_constant.rs
@@ -81,6 +81,7 @@ impl<F: Field> ExecutionGadget<F> for ErrorOOGConstantGadget<F> {
                 0.expr(),
                 0.expr(),
                 0.expr(),
+                0.expr(),
             )
         });