sync dual code hash part4 (scroll-tech#446)

* sync dual code hash part4

* fix extcodesize

* fix test

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

@@ -5,6 +5,7 @@ use crate::{
     error::ExecError,
     evm::{Opcode, OpcodeId},
     operation::{AccountField, AccountOp, CallContextField, MemoryOp, RW},
+    state_db::CodeDB,
     Error,
 };
 use eth_types::{Bytecode, GethExecStep, ToBigEndian, ToWord, Word, H160, H256};
@@ -66,11 +67,11 @@ impl<const IS_CREATE2: bool> Opcode for Create<IS_CREATE2> {
             },
         )?;
 
-        let mut initialization_code = vec![];
-        if length > 0 {
-            initialization_code =
-                handle_copy(state, &mut exec_step, state.call()?.call_id, offset, length)?;
-        }
+        let (initialization_code, keccak_code_hash, code_hash) = if length > 0 {
+            handle_copy(state, &mut exec_step, state.call()?.call_id, offset, length)?
+        } else {
+            (vec![], H256(keccak256([])), CodeDB::empty_code_hash())
+        };
 
         let tx_id = state.tx_ctx.id();
         let caller = state.call()?.clone();
@@ -191,7 +192,6 @@ impl<const IS_CREATE2: bool> Opcode for Create<IS_CREATE2> {
             caller.depth.to_word(),
         );
 
-        let code_hash = keccak256(&initialization_code);
         for (field, value) in [
             (CallContextField::CallerId, caller.call_id.into()),
             (CallContextField::IsSuccess, callee.is_success.to_word()),
@@ -213,7 +213,7 @@ impl<const IS_CREATE2: bool> Opcode for Create<IS_CREATE2> {
             (CallContextField::IsRoot, false.to_word()),
             (CallContextField::IsStatic, false.to_word()),
             (CallContextField::IsCreate, true.to_word()),
-            (CallContextField::CodeHash, Word::from(code_hash)),
+            (CallContextField::CodeHash, code_hash.to_word()),
             (CallContextField::Value, callee.value),
         ] {
             state.call_context_write(&mut exec_step, callee.call_id, field, value);
@@ -226,13 +226,13 @@ impl<const IS_CREATE2: bool> Opcode for Create<IS_CREATE2> {
                 get_create2_address(
                     caller.address,
                     salt.to_be_bytes().to_vec(),
-                    initialization_code.clone()
+                    initialization_code
                 )
             );
             std::iter::once(0xffu8)
                 .chain(caller.address.to_fixed_bytes())
                 .chain(salt.to_be_bytes())
-                .chain(keccak256(&initialization_code))
+                .chain(keccak_code_hash.to_fixed_bytes())
                 .collect::<Vec<_>>()
         } else {
             let mut stream = rlp::RlpStream::new();
@@ -270,9 +270,10 @@ fn handle_copy(
     callee_id: usize,
     offset: usize,
     length: usize,
-) -> Result<Vec<u8>, Error> {
+) -> Result<(Vec<u8>, H256, H256), Error> {
     let initialization_bytes = state.call_ctx()?.memory.0[offset..offset + length].to_vec();
-    let dst_id = NumberOrHash::Hash(H256(keccak256(&initialization_bytes)));
+    let keccak_code_hash = H256(keccak256(&initialization_bytes));
+    let code_hash = CodeDB::hash(&initialization_bytes);
     let bytes: Vec<_> = Bytecode::from(initialization_bytes.clone())
         .code
         .iter()
@@ -298,14 +299,14 @@ fn handle_copy(
             src_addr: offset.try_into().unwrap(),
             src_addr_end: (offset + length).try_into().unwrap(),
             dst_type: CopyDataType::Bytecode,
-            dst_id,
+            dst_id: NumberOrHash::Hash(code_hash),
             dst_addr: 0,
             log_id: None,
             bytes,
         },
     );
 
-    Ok(initialization_bytes)
+    Ok((initialization_bytes, keccak_code_hash, code_hash))
 }
 
 #[cfg(test)]

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

@@ -4,7 +4,7 @@ use crate::{
     operation::{AccountField, CallContextField, TxAccessListAccountOp},
     Error,
 };
-use eth_types::{GethExecStep, ToAddress, ToWord, H256};
+use eth_types::{GethExecStep, ToAddress, ToWord, Word, H256};
 
 #[derive(Debug, Copy, Clone)]
 pub(crate) struct Extcodesize;
@@ -53,29 +53,32 @@ impl Opcode for Extcodesize {
         // Read account code hash and get code length.
         let account = state.sdb.get_account(&address).1;
         let exists = !account.is_empty();
-        let code_hash = if exists {
-            account.code_hash
+        let (code_hash, code_size) = if exists {
+            (account.code_hash, account.code_size)
         } else {
-            H256::zero()
+            (H256::zero(), Word::zero())
         };
         state.account_read(
             &mut exec_step,
             address,
             AccountField::CodeHash,
             code_hash.to_word(),
         );
-        let code_size = if exists {
-            state.code(code_hash)?.len()
-        } else {
-            0
-        };
+        // If "scroll" feature is enabled, CodeSize is read of AccountTrie,
+        // so the full code don't need to be put into bytecode circuit.
+        // TODO: check the bytecode circuit assignment codes, to make sure this optimization
+        // is correctly applied.
+        #[cfg(feature = "scroll")]
+        if exists {
+            state.account_read(&mut exec_step, address, AccountField::CodeSize, code_size);
+        }
 
         // Write the EXTCODESIZE result to stack.
-        debug_assert_eq!(code_size, geth_steps[1].stack.last()?.as_usize());
+        debug_assert_eq!(code_size, geth_steps[1].stack.last()?);
         state.stack_write(
             &mut exec_step,
             geth_steps[1].stack.nth_last_filled(0),
-            code_size.into(),
+            code_size,
         )?;
 
         Ok(vec![exec_step])
@@ -241,12 +244,29 @@ mod extcodesize_tests {
             &AccountOp {
                 address: account.address,
                 field: AccountField::CodeHash,
-                value: if exists { code_hash } else { U256::zero() },
-                value_prev: if exists { code_hash } else { U256::zero() },
+                value: if exists { code_hash } else { Word::zero() },
+                value_prev: if exists { code_hash } else { Word::zero() },
             }
         );
-
-        let operation = &container.stack[indices[6].as_usize()];
+        #[cfg(feature = "scroll")]
+        if exists {
+            let code_size = account.code.len().to_word();
+            let operation = &container.account[indices[6].as_usize()];
+            assert_eq!(operation.rw(), RW::READ);
+            assert_eq!(
+                operation.op(),
+                &AccountOp {
+                    address: account.address,
+                    field: AccountField::CodeSize,
+                    value: code_size,
+                    value_prev: code_size,
+                },
+            );
+        }
+        let rw_offset = 6;
+        #[cfg(feature = "scroll")]
+        let rw_offset = if exists { rw_offset + 1 } else { rw_offset };
+        let operation = &container.stack[indices[rw_offset].as_usize()];
         assert_eq!(operation.rw(), RW::WRITE);
         assert_eq!(
             operation.op(),

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

@@ -7,6 +7,7 @@ use crate::{
     Error,
 };
 use eth_types::{Bytecode, GethExecStep, ToWord, H256};
+use ethers_core::utils::keccak256;
 
 #[derive(Debug, Copy, Clone)]
 pub(crate) struct ReturnRevert;
@@ -47,7 +48,7 @@ impl Opcode for ReturnRevert {
         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.
-            let code_hash = handle_create(
+            let code_info = handle_create(
                 state,
                 &mut exec_step,
                 Source {
@@ -69,15 +70,35 @@ impl Opcode for ReturnRevert {
                 state.call_context_read(&mut exec_step, state.call()?.call_id, field, value);
             }
 
+            #[cfg(feature = "scroll")]
+            state.push_op_reversible(
+                &mut exec_step,
+                AccountOp {
+                    address: state.call()?.address,
+                    field: AccountField::KeccakCodeHash,
+                    value: code_info.keccak_hash.to_word(),
+                    value_prev: crate::util::KECCAK_CODE_HASH_ZERO.to_word(),
+                },
+            )?;
             state.push_op_reversible(
                 &mut exec_step,
                 AccountOp {
                     address: state.call()?.address,
                     field: AccountField::CodeHash,
-                    value: code_hash.to_word(),
+                    value: code_info.hash.to_word(),
                     value_prev: CodeDB::empty_code_hash().to_word(),
                 },
             )?;
+            #[cfg(feature = "scroll")]
+            state.push_op_reversible(
+                &mut exec_step,
+                AccountOp {
+                    address: state.call()?.address,
+                    field: AccountField::CodeSize,
+                    value: code_info.size.to_word(),
+                    value_prev: eth_types::Word::zero(),
+                },
+            )?;
         }
 
         // Case B in the specs.
@@ -199,13 +220,21 @@ fn handle_copy(
     Ok(())
 }
 
+struct AccountCodeInfo {
+    keccak_hash: H256,
+    hash: H256,
+    size: usize,
+}
+
 fn handle_create(
     state: &mut CircuitInputStateRef,
     step: &mut ExecStep,
     source: Source,
-) -> Result<H256, Error> {
+) -> Result<AccountCodeInfo, Error> {
     let values = state.call_ctx()?.memory.0[source.offset..source.offset + source.length].to_vec();
+    let keccak_hash = H256(keccak256(&values));
     let code_hash = CodeDB::hash(&values);
+    let size = values.len();
     let dst_id = NumberOrHash::Hash(code_hash);
     let bytes: Vec<_> = Bytecode::from(values)
         .code
@@ -238,7 +267,11 @@ fn handle_create(
         },
     );
 
-    Ok(code_hash)
+    Ok(AccountCodeInfo {
+        keccak_hash,
+        hash: code_hash,
+        size,
+    })
 }
 
 #[cfg(test)]

zkevm-circuits/src/copy_circuit.rs

@@ -25,7 +25,8 @@ use halo2_proofs::plonk::SecondPhase;
 use crate::{
     evm_circuit::util::{constraint_builder::BaseConstraintBuilder, rlc},
     table::{
-        BytecodeTable, CopyTable, LookupTable, RwTable, RwTableTag, TxContextFieldTag, TxTable,
+        BytecodeFieldTag, BytecodeTable, CopyTable, LookupTable, RwTable, RwTableTag,
+        TxContextFieldTag, TxTable,
     },
     util::{Challenges, SubCircuit, SubCircuitConfig},
     witness,
@@ -384,9 +385,7 @@ impl<F: Field> SubCircuitConfig<F> for CopyCircuitConfig<F> {
             .collect()
         });
 
-        // create case unimplemented for poseidon hash
         meta.lookup_any("Bytecode lookup", |meta| {
-            use crate::table::BytecodeFieldTag;
             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()));
@@ -475,8 +474,6 @@ impl<F: Field> CopyCircuitConfig<F> {
             if is_read {
                 self.q_step.enable(region, *offset)?;
             }
-            // FIXME: finish padding of copy circuit
-            // Now temporarily set it to 0 to make vk univeral
             // q_enable
             region.assign_fixed(
                 || "q_enable",
@@ -513,6 +510,7 @@ impl<F: Field> CopyCircuitConfig<F> {
 
             *offset += 1;
         }
+
         Ok(())
     }
 
@@ -1113,7 +1111,6 @@ mod tests {
         );
     }
 
-    #[ignore = "codehash related"]
     #[test]
     fn copy_circuit_invalid_codecopy() {
         let mut builder = gen_codecopy_data();
@@ -1130,7 +1127,6 @@ mod tests {
         );
     }
 
-    #[ignore = "codehash related"]
     #[test]
     fn copy_circuit_invalid_extcodecopy() {
         let mut builder = gen_extcodecopy_data();

zkevm-circuits/src/evm_circuit/execution/create.rs

@@ -110,6 +110,7 @@ impl<F: Field, const IS_CREATE2: bool, const S: ExecutionState> ExecutionGadget<
         cb.stack_push(callee_is_success.expr() * new_address_rlc);
 
         cb.condition(init_code.has_length(), |cb| {
+            // TODO(rohit): lookup to keccak table to verify keccak code hash?
             cb.copy_table_lookup(
                 cb.curr.state.call_id.expr(),
                 CopyDataType::Memory.expr(),
@@ -123,12 +124,19 @@ impl<F: Field, const IS_CREATE2: bool, const S: ExecutionState> ExecutionGadget<
                 init_code.length(),
             );
         });
-        cb.condition(not::expr(init_code.has_length()), |cb| {
+        cb.condition(not::expr(init_code.has_length()), |_cb| {
+            /* FIXME
             cb.require_equal(
-                "Empty code",
-                create.code_hash_word_rlc(cb),
+                "keccak hash of empty bytes",
+                keccak_code_hash.expr(),
+                cb.empty_keccak_hash_rlc(),
+            );
+            cb.require_equal(
+                "code hash of empty bytes",
+                create.code_hash_keccak_rlc(cb),
                 cb.empty_code_hash_rlc(),
             );
+            */
         });
 
         let tx_id = cb.call_context(None, CallContextFieldTag::TxId);

zkevm-circuits/src/evm_circuit/execution/extcodesize.rs

@@ -61,8 +61,16 @@ impl<F: Field> ExecutionGadget<F> for ExtcodesizeGadget<F> {
 
         let code_size = cb.query_word_rlc();
         cb.condition(exists.expr(), |cb| {
+            #[cfg(feature = "scroll")]
+            cb.account_read(
+                address.expr(),
+                AccountFieldTag::CodeSize,
+                from_bytes::expr(&code_size.cells),
+            );
+            #[cfg(not(feature = "scroll"))]
             cb.bytecode_length(code_hash.expr(), from_bytes::expr(&code_size.cells));
         });
+
         cb.condition(not_exists.expr(), |cb| {
             cb.require_zero("code_size is zero when non_exists", code_size.expr());
         });
@@ -75,8 +83,11 @@ impl<F: Field> ExecutionGadget<F> for ExtcodesizeGadget<F> {
             GasCost::COLD_ACCOUNT_ACCESS.expr(),
         );
 
+        let rw_counter_delta = 7.expr();
+        #[cfg(feature = "scroll")]
+        let rw_counter_delta = rw_counter_delta + exists;
         let step_state_transition = StepStateTransition {
-            rw_counter: Delta(7.expr()),
+            rw_counter: Delta(rw_counter_delta),
             program_counter: Delta(1.expr()),
             stack_pointer: Delta(0.expr()),
             gas_left: Delta(-gas_cost),
@@ -134,7 +145,14 @@ impl<F: Field> ExecutionGadget<F> for ExtcodesizeGadget<F> {
         self.not_exists
             .assign_value(region, offset, region.word_rlc(code_hash))?;
 
-        let code_size = block.rws[step.rw_indices[6]].stack_value().as_u64();
+        let rw_offset = 6;
+        #[cfg(feature = "scroll")]
+        let rw_offset = if code_hash.is_zero() {
+            rw_offset + 1
+        } else {
+            rw_offset
+        };
+        let code_size = block.rws[step.rw_indices[rw_offset]].stack_value().as_u64();
         self.code_size
             .assign(region, offset, Some(code_size.to_le_bytes()))?;