From e47613b3d0c9c6f9315289551ffa884741e17e61 Mon Sep 17 00:00:00 2001 From: Zhang Zhuo Date: Fri, 1 Apr 2022 19:39:34 +0800 Subject: [PATCH 1/5] pick --- Cargo.lock | 4 + bus-mapping/Cargo.toml | 1 + bus-mapping/src/circuit_input_builder.rs | 64 ++--- bus-mapping/src/evm/opcodes.rs | 7 +- bus-mapping/src/evm/opcodes/calldatacopy.rs | 35 +-- bus-mapping/src/evm/opcodes/calldatasize.rs | 38 +-- bus-mapping/src/evm/opcodes/caller.rs | 36 +-- bus-mapping/src/evm/opcodes/callvalue.rs | 37 +-- bus-mapping/src/evm/opcodes/dup.rs | 49 ++-- bus-mapping/src/evm/opcodes/extcodehash.rs | 14 +- bus-mapping/src/evm/opcodes/mload.rs | 35 +-- bus-mapping/src/evm/opcodes/mstore.rs | 63 +--- bus-mapping/src/evm/opcodes/number.rs | 4 +- bus-mapping/src/evm/opcodes/selfbalance.rs | 44 +-- bus-mapping/src/evm/opcodes/sload.rs | 117 +++++--- bus-mapping/src/evm/opcodes/sstore.rs | 176 ++++++++++++ bus-mapping/src/evm/opcodes/stackonlyop.rs | 35 +-- bus-mapping/src/evm/opcodes/swap.rs | 42 ++- bus-mapping/src/evm/opcodes/test_util.rs | 86 ++++++ bus-mapping/src/lib.rs | 4 + bus-mapping/src/operation/container.rs | 50 +++- bus-mapping/src/state_db.rs | 48 +++- eth-types/src/evm_types.rs | 2 +- eth-types/src/lib.rs | 17 ++ geth-utils/gethutil/trace.go | 32 ++- zkevm-circuits/Cargo.toml | 5 + zkevm-circuits/src/evm_circuit.rs | 4 + zkevm-circuits/src/evm_circuit/execution.rs | 26 +- .../src/evm_circuit/execution/sstore.rs | 270 ++++++++---------- .../evm_circuit/util/constraint_builder.rs | 20 +- zkevm-circuits/src/test_util.rs | 6 + 31 files changed, 784 insertions(+), 587 deletions(-) create mode 100644 bus-mapping/src/evm/opcodes/sstore.rs create mode 100644 bus-mapping/src/evm/opcodes/test_util.rs diff --git a/Cargo.lock b/Cargo.lock index d58644902c..c641185997 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -344,6 +344,7 @@ dependencies = [ "mock", "pairing_bn256", "pretty_assertions", + "rand", "serde", "serde_json", "tokio", @@ -4037,7 +4038,9 @@ dependencies = [ "bigint", "bus-mapping", "criterion", + "ctor", "digest 0.7.6", + "env_logger", "eth-types", "ethers-core", "ff 0.11.0", @@ -4046,6 +4049,7 @@ dependencies = [ "itertools", "keccak256", "lazy_static", + "log", "mock", "num", "pairing_bn256", diff --git a/bus-mapping/Cargo.toml b/bus-mapping/Cargo.toml index 20ee9bd021..a637e6b98b 100644 --- a/bus-mapping/Cargo.toml +++ b/bus-mapping/Cargo.toml @@ -18,5 +18,6 @@ serde_json = "1.0.66" [dev-dependencies] mock = { path = "../mock" } pretty_assertions = "1.0.0" +rand = "0.8" tokio = { version = "1.13", features = ["macros"] } url = "2.2.2" diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 9d09bd225a..24d7767952 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -178,6 +178,8 @@ pub struct ExecStep { /// overflow", this value will **not** be the actual Gas cost of the /// step. pub gas_cost: GasCost, + /// accumulated gas refund + pub gas_refund: Gas, /// Call index within the [`Transaction`] pub call_index: usize, /// The global counter when this step was executed. @@ -208,6 +210,7 @@ impl ExecStep { memory_size: step.memory.0.len(), gas_left: step.gas, gas_cost: step.gas_cost, + gas_refund: Gas(0), call_index, rwc, swc, @@ -227,6 +230,7 @@ impl Default for ExecStep { memory_size: 0, gas_left: Gas(0), gas_cost: GasCost(0), + gas_refund: Gas(0), call_index: 0, rwc: RWCounter(0), swc: 0, @@ -796,11 +800,7 @@ impl<'a> CircuitInputStateRef<'a> { rw: RW, op: T, ) -> Result<(), Error> { - let op_ref = self.block.container.insert(Operation::new_reversible( - self.block_ctx.rwc.inc_pre(), - rw, - op, - )); + let op_ref = self.apply_op(false, rw, op.into_enum()); step.bus_mapping_instance.push(op_ref); // Increase state_write_counter @@ -1068,37 +1068,28 @@ impl<'a> CircuitInputStateRef<'a> { } /// Apply reverted op to state and push to container. - fn apply_reverted_op(&mut self, op: OpEnum) -> OperationRef { - match op { + fn apply_op(&mut self, is_revert: bool, rw: RW, op: OpEnum) -> OperationRef { + match &op { OpEnum::Storage(op) => { - let (_, account) = self.sdb.get_storage_mut(&op.address, &op.key); - *account = op.value; - self.block.container.insert(Operation::new( - self.block_ctx.rwc.inc_pre(), - RW::WRITE, - op, - )) + self.sdb.set_storage(&op.address, &op.key, &op.value); } OpEnum::TxAccessListAccount(op) => { - if !op.value { + if !op.value_prev && op.value { + self.sdb.add_account_to_access_list(op.address); + } + if op.value_prev && !op.value { self.sdb.remove_account_from_access_list(&op.address); } - self.block.container.insert(Operation::new( - self.block_ctx.rwc.inc_pre(), - RW::WRITE, - op, - )) } OpEnum::TxAccessListAccountStorage(op) => { - if !op.value { + if !op.value_prev && op.value { + self.sdb + .add_account_storage_to_access_list((op.address, op.key)); + } + if op.value_prev && !op.value { self.sdb .remove_account_storage_from_access_list(&(op.address, op.key)); } - self.block.container.insert(Operation::new( - self.block_ctx.rwc.inc_pre(), - RW::WRITE, - op, - )) } OpEnum::Account(op) => { let (_, account) = self.sdb.get_account_mut(&op.address); @@ -1109,16 +1100,19 @@ impl<'a> CircuitInputStateRef<'a> { account.code_hash = op.value.to_be_bytes().into(); } } - self.block.container.insert(Operation::new( - self.block_ctx.rwc.inc_pre(), - RW::WRITE, - op, - )) } - OpEnum::TxRefund(_) => unimplemented!(), + OpEnum::TxRefund(op) => { + self.sdb.set_refund(op.value); + } OpEnum::AccountDestructed(_) => unimplemented!(), _ => unreachable!(), + }; + if is_revert { + debug_assert!(rw == RW::WRITE, "revert read operation"); } + self.block + .container + .insert_op_enum(self.block_ctx.rwc.inc_pre(), rw, !is_revert, op) } /// Handle a reversion group @@ -1132,7 +1126,7 @@ impl<'a> CircuitInputStateRef<'a> { // Apply reversions for (step_index, op_ref) in reversion_group.op_refs.into_iter().rev() { if let Some(op) = self.get_rev_op_by_ref(&op_ref) { - let rev_op_ref = self.apply_reverted_op(op); + let rev_op_ref = self.apply_op(true, RW::WRITE, op); self.tx.steps[step_index] .bus_mapping_instance .push(rev_op_ref); @@ -1463,6 +1457,7 @@ impl<'a> CircuitInputBuilder { for (index, geth_step) in geth_trace.struct_logs.iter().enumerate() { let mut state_ref = self.state_ref(&mut tx, &mut tx_ctx); + log::trace!("handle {}th opcode {:?} ", index, geth_step.op); let exec_steps = gen_associated_ops( &geth_step.op, &mut state_ref, @@ -1478,8 +1473,8 @@ impl<'a> CircuitInputBuilder { let end_tx_step = gen_end_tx_ops(&mut self.state_ref(&mut tx, &mut tx_ctx))?; tx.steps.push(end_tx_step); + self.sdb.commit_tx(); self.block.txs.push(tx); - self.sdb.clear_access_list_and_refund(); Ok(()) } @@ -1994,6 +1989,7 @@ mod tracer_tests { &GethExecTrace { gas: Gas(0), failed: false, + return_value: "".to_owned(), struct_logs: vec![geth_step.clone()], }, false, diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index 5d5bcf68d2..ec1a88fa9b 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -15,6 +15,9 @@ use eth_types::{ }; use log::warn; +#[cfg(test)] +mod test_util; + mod calldatacopy; mod calldatasize; mod caller; @@ -25,6 +28,7 @@ mod mload; mod mstore; mod selfbalance; mod sload; +mod sstore; mod stackonlyop; mod stop; mod swap; @@ -39,6 +43,7 @@ use mload::Mload; use mstore::Mstore; use selfbalance::Selfbalance; use sload::Sload; +use sstore::Sstore; use stackonlyop::StackOnlyOpcode; use stop::Stop; use swap::Swap; @@ -129,7 +134,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { OpcodeId::MSTORE => Mstore::::gen_associated_ops, OpcodeId::MSTORE8 => Mstore::::gen_associated_ops, OpcodeId::SLOAD => Sload::gen_associated_ops, - // OpcodeId::SSTORE => {}, + OpcodeId::SSTORE => Sstore::gen_associated_ops, OpcodeId::JUMP => StackOnlyOpcode::<1, 0>::gen_associated_ops, OpcodeId::JUMPI => StackOnlyOpcode::<2, 0>::gen_associated_ops, OpcodeId::PC => StackOnlyOpcode::<0, 1>::gen_associated_ops, diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 03700ac5fc..49ca5225d4 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -158,18 +158,15 @@ fn gen_memory_copy_steps( #[cfg(test)] mod calldatacopy_tests { use crate::{ - circuit_input_builder::ExecState, - mock::BlockData, + evm::opcodes::test_util::TestCase, operation::{CallContextField, CallContextOp, StackOp, RW}, }; use eth_types::{ bytecode, evm_types::{OpcodeId, StackAddress}, - geth_types::GethData, Word, }; - use mock::test_ctx::{helpers::*, TestContext}; use pretty_assertions::assert_eq; #[test] @@ -182,30 +179,13 @@ mod calldatacopy_tests { STOP }; - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); - - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); - - let step = builder.block.txs()[0] - .steps() - .iter() - .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLDATACOPY)) - .unwrap(); + let test = TestCase::new_from_bytecode(code); + let step = test.step_witness(OpcodeId::CALLDATACOPY, 0); + let call_id = test.tx_witness().calls()[0].call_id; assert_eq!( [0, 1, 2] - .map(|idx| &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()]) + .map(|idx| &step.rws.stack[idx]) .map(|operation| (operation.rw(), operation.op())), [ ( @@ -225,14 +205,13 @@ mod calldatacopy_tests { assert_eq!( { - let operation = - &builder.block.container.call_context[step.bus_mapping_instance[3].as_usize()]; + let operation = &step.rws.call_context[0]; (operation.rw(), operation.op()) }, ( RW::READ, &CallContextOp { - call_id: builder.block.txs()[0].calls()[0].call_id, + call_id, field: CallContextField::TxId, value: Word::from(1), } diff --git a/bus-mapping/src/evm/opcodes/calldatasize.rs b/bus-mapping/src/evm/opcodes/calldatasize.rs index 035bf6bc88..d2808f1ab8 100644 --- a/bus-mapping/src/evm/opcodes/calldatasize.rs +++ b/bus-mapping/src/evm/opcodes/calldatasize.rs @@ -41,17 +41,14 @@ impl Opcode for Calldatasize { #[cfg(test)] mod calldatasize_tests { use crate::{ - circuit_input_builder::ExecState, - mock::BlockData, + evm::opcodes::test_util::TestCase, operation::{CallContextField, CallContextOp, StackOp, RW}, }; use eth_types::{ bytecode, evm_types::{OpcodeId, StackAddress}, - geth_types::GethData, }; - use mock::test_ctx::{helpers::*, TestContext}; use pretty_assertions::assert_eq; #[test] @@ -61,33 +58,13 @@ mod calldatasize_tests { STOP }; - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); - - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); - - let step = builder.block.txs()[0] - .steps() - .iter() - .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLDATASIZE)) - .unwrap(); - - let call_id = builder.block.txs()[0].calls()[0].call_id; - let call_data_size = block.eth_block.transactions[0].input.as_ref().len().into(); + let test = TestCase::new_from_bytecode(code); + let step = test.step_witness(OpcodeId::CALLDATASIZE, 0); + let call_id = test.tx_witness().calls()[0].call_id; + let call_data_size = test.tx_input().input.as_ref().len().into(); assert_eq!( { - let operation = - &builder.block.container.call_context[step.bus_mapping_instance[0].as_usize()]; + let operation = &step.rws.call_context[0]; (operation.rw(), operation.op()) }, ( @@ -101,8 +78,7 @@ mod calldatasize_tests { ); assert_eq!( { - let operation = - &builder.block.container.stack[step.bus_mapping_instance[1].as_usize()]; + let operation = &step.rws.stack[0]; (operation.rw(), operation.op()) }, ( diff --git a/bus-mapping/src/evm/opcodes/caller.rs b/bus-mapping/src/evm/opcodes/caller.rs index f98052c3a8..25ee6fe085 100644 --- a/bus-mapping/src/evm/opcodes/caller.rs +++ b/bus-mapping/src/evm/opcodes/caller.rs @@ -43,15 +43,13 @@ impl Opcode for Caller { #[cfg(test)] mod caller_tests { use super::*; - use crate::{circuit_input_builder::ExecState, mock::BlockData, operation::StackOp}; + use crate::{evm::opcodes::test_util::TestCase, operation::StackOp}; use eth_types::{ bytecode, evm_types::{OpcodeId, StackAddress}, - geth_types::GethData, ToWord, }; - use mock::test_ctx::{helpers::*, TestContext}; use pretty_assertions::assert_eq; #[test] @@ -60,34 +58,15 @@ mod caller_tests { CALLER STOP }; + let test = TestCase::new_from_bytecode(code); - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); + let step = test.step_witness(OpcodeId::CALLER, 0); - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); - - let step = builder.block.txs()[0] - .steps() - .iter() - .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLER)) - .unwrap(); - - let call_id = builder.block.txs()[0].calls()[0].call_id; - let caller_address = block.eth_block.transactions[0].from.to_word(); + let call_id = test.tx_witness().calls()[0].call_id; + let caller_address = test.tx_input().from.to_word(); assert_eq!( { - let operation = - &builder.block.container.call_context[step.bus_mapping_instance[0].as_usize()]; + let operation = &step.rws.call_context[0]; (operation.rw(), operation.op()) }, ( @@ -101,8 +80,7 @@ mod caller_tests { ); assert_eq!( { - let operation = - &builder.block.container.stack[step.bus_mapping_instance[1].as_usize()]; + let operation = &step.rws.stack[0]; (operation.rw(), operation.op()) }, ( diff --git a/bus-mapping/src/evm/opcodes/callvalue.rs b/bus-mapping/src/evm/opcodes/callvalue.rs index 3b044c7534..d170d2f7e7 100644 --- a/bus-mapping/src/evm/opcodes/callvalue.rs +++ b/bus-mapping/src/evm/opcodes/callvalue.rs @@ -43,16 +43,14 @@ impl Opcode for Callvalue { #[cfg(test)] mod callvalue_tests { use crate::{ - circuit_input_builder::ExecState, - mock::BlockData, + evm::opcodes::test_util::TestCase, operation::{CallContextField, CallContextOp, StackOp, RW}, }; use eth_types::{ bytecode, evm_types::{OpcodeId, StackAddress}, - geth_types::GethData, }; - use mock::test_ctx::{helpers::*, TestContext}; + use pretty_assertions::assert_eq; #[test] @@ -62,33 +60,15 @@ mod callvalue_tests { STOP }; - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); - - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); + let test = TestCase::new_from_bytecode(code); - let step = builder.block.txs()[0] - .steps() - .iter() - .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLVALUE)) - .unwrap(); + let step = test.step_witness(OpcodeId::CALLVALUE, 0); - let call_id = builder.block.txs()[0].calls()[0].call_id; - let call_value = block.eth_block.transactions[0].value; + let call_id = test.tx_witness().calls()[0].call_id; + let call_value = test.tx_input().value; assert_eq!( { - let operation = - &builder.block.container.call_context[step.bus_mapping_instance[0].as_usize()]; + let operation = &step.rws.call_context[0]; (operation.rw(), operation.op()) }, ( @@ -102,8 +82,7 @@ mod callvalue_tests { ); assert_eq!( { - let operation = - &builder.block.container.stack[step.bus_mapping_instance[1].as_usize()]; + let operation = &step.rws.stack[0]; (operation.rw(), operation.op()) }, ( diff --git a/bus-mapping/src/evm/opcodes/dup.rs b/bus-mapping/src/evm/opcodes/dup.rs index 1a1f47e515..89e7d4dcd7 100644 --- a/bus-mapping/src/evm/opcodes/dup.rs +++ b/bus-mapping/src/evm/opcodes/dup.rs @@ -34,10 +34,13 @@ impl Opcode for Dup { #[cfg(test)] mod dup_tests { use super::*; - use crate::{mock::BlockData, operation::StackOp}; - use eth_types::{bytecode, evm_types::StackAddress, geth_types::GethData, word}; - use itertools::Itertools; - use mock::test_ctx::{helpers::*, TestContext}; + use crate::{evm::opcodes::test_util::TestCase, operation::StackOp}; + use eth_types::{ + bytecode, + evm_types::{OpcodeId, StackAddress}, + word, + }; + use pretty_assertions::assert_eq; #[test] @@ -52,44 +55,30 @@ mod dup_tests { STOP }; - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); - - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); + let test = TestCase::new_from_bytecode(code); // Generate steps corresponding to DUP1, DUP3, DUP5 - for (i, word) in [word!("0x3"), word!("0x2"), word!("0x1")] - .iter() - .enumerate() + for (i, (op, value)) in [ + (OpcodeId::DUP1, word!("0x3")), + (OpcodeId::DUP3, word!("0x2")), + (OpcodeId::DUP5, word!("0x1")), + ] + .iter() + .enumerate() { - let step = builder.block.txs()[0] - .steps() - .iter() - .filter(|step| step.exec_state.is_dup()) - .collect_vec()[i]; + let step = test.step_witness(*op, 0); assert_eq!( [0, 1] - .map(|idx| &builder.block.container.stack - [step.bus_mapping_instance[idx].as_usize()]) + .map(|idx| &step.rws.stack[idx]) .map(|operation| (operation.rw(), operation.op())), [ ( RW::READ, - &StackOp::new(1, StackAddress(1024 - 3 + i), *word) + &StackOp::new(1, StackAddress(1024 - 3 + i), *value) ), ( RW::WRITE, - &StackOp::new(1, StackAddress(1024 - 4 - i), *word) + &StackOp::new(1, StackAddress(1024 - 4 - i), *value) ) ] ) diff --git a/bus-mapping/src/evm/opcodes/extcodehash.rs b/bus-mapping/src/evm/opcodes/extcodehash.rs index 33354f538a..b448dbf21a 100644 --- a/bus-mapping/src/evm/opcodes/extcodehash.rs +++ b/bus-mapping/src/evm/opcodes/extcodehash.rs @@ -16,14 +16,14 @@ pub(crate) struct Extcodehash; impl Opcode for Extcodehash { fn gen_associated_ops( state: &mut CircuitInputStateRef, - steps: &[GethExecStep], + geth_steps: &[GethExecStep], ) -> Result, Error> { - let step = &steps[0]; - let mut exec_step = state.new_step(step)?; - let stack_address = step.stack.last_filled(); + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; + let stack_address = geth_step.stack.last_filled(); // Pop external address off stack - let external_address = step.stack.last()?.to_address(); + let external_address = geth_step.stack.last()?.to_address(); state.push_stack_op( &mut exec_step, RW::READ, @@ -58,7 +58,7 @@ impl Opcode for Extcodehash { } // Update transaction access list for external_address - let is_warm = match step.gas_cost { + let is_warm = match geth_step.gas_cost { GasCost::WARM_STORAGE_READ_COST => true, GasCost::COLD_ACCOUNT_ACCESS_COST => false, _ => unreachable!(), @@ -119,7 +119,7 @@ impl Opcode for Extcodehash { &mut exec_step, RW::WRITE, stack_address, - steps[1].stack.last()?, + geth_steps[1].stack.last()?, )?; Ok(vec![exec_step]) diff --git a/bus-mapping/src/evm/opcodes/mload.rs b/bus-mapping/src/evm/opcodes/mload.rs index f97d4bd3f2..0dcd1dabc2 100644 --- a/bus-mapping/src/evm/opcodes/mload.rs +++ b/bus-mapping/src/evm/opcodes/mload.rs @@ -61,17 +61,15 @@ impl Opcode for Mload { mod mload_tests { use super::*; use crate::{ - circuit_input_builder::ExecState, - mock::BlockData, + evm::opcodes::test_util::step_witness_for_bytecode, operation::{MemoryOp, StackOp}, }; use eth_types::{ bytecode, evm_types::{OpcodeId, StackAddress}, - geth_types::GethData, }; use itertools::Itertools; - use mock::test_ctx::{helpers::*, TestContext}; + use pretty_assertions::assert_eq; #[test] @@ -83,31 +81,11 @@ mod mload_tests { MLOAD STOP }; - - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); - - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); - - let step = builder.block.txs()[0] - .steps() - .iter() - .find(|step| step.exec_state == ExecState::Op(OpcodeId::MLOAD)) - .unwrap(); + let step = step_witness_for_bytecode(code, OpcodeId::MLOAD); assert_eq!( [0, 1] - .map(|idx| &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()]) + .map(|idx| &step.rws.stack[idx]) .map(|operation| (operation.rw(), operation.op())), [ ( @@ -122,9 +100,8 @@ mod mload_tests { ); assert_eq!( - (2..34) - .map(|idx| &builder.block.container.memory - [step.bus_mapping_instance[idx].as_usize()]) + (0..32) + .map(|idx| &step.rws.memory[idx]) .map(|operation| (operation.rw(), operation.op().clone())) .collect_vec(), Word::from(0x80) diff --git a/bus-mapping/src/evm/opcodes/mstore.rs b/bus-mapping/src/evm/opcodes/mstore.rs index 218707db1f..4f379289ea 100644 --- a/bus-mapping/src/evm/opcodes/mstore.rs +++ b/bus-mapping/src/evm/opcodes/mstore.rs @@ -63,18 +63,16 @@ impl Opcode for Mstore { mod mstore_tests { use super::*; use crate::{ - circuit_input_builder::ExecState, - mock::BlockData, + evm::opcodes::test_util::{step_witness_for_bytecode, TestCase}, operation::{MemoryOp, StackOp, RW}, }; use eth_types::{ bytecode, evm_types::{MemoryAddress, OpcodeId, StackAddress}, - geth_types::GethData, Word, }; use itertools::Itertools; - use mock::test_ctx::{helpers::*, TestContext}; + use pretty_assertions::assert_eq; #[test] @@ -86,32 +84,10 @@ mod mstore_tests { MSTORE STOP }; - - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); - - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); - - let step = builder.block.txs()[0] - .steps() - .iter() - .filter(|step| step.exec_state == ExecState::Op(OpcodeId::MSTORE)) - .nth(1) - .unwrap(); - + let step = TestCase::new_from_bytecode(code).step_witness(OpcodeId::MSTORE, 1); assert_eq!( [0, 1] - .map(|idx| &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()]) + .map(|idx| &step.rws.stack[idx]) .map(|operation| (operation.rw(), operation.op())), [ ( @@ -126,9 +102,8 @@ mod mstore_tests { ); assert_eq!( - (2..34) - .map(|idx| &builder.block.container.memory - [step.bus_mapping_instance[idx].as_usize()]) + (0..32) + .map(|idx| &step.rws.memory[idx]) .map(|operation| (operation.rw(), operation.op().clone())) .collect_vec(), Word::from(0x1234u64) @@ -153,30 +128,10 @@ mod mstore_tests { STOP }; - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); - - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); - - let step = builder.block.txs()[0] - .steps() - .iter() - .find(|step| step.exec_state == ExecState::Op(OpcodeId::MSTORE8)) - .unwrap(); - + let step = step_witness_for_bytecode(code, OpcodeId::MSTORE8); assert_eq!( [0, 1] - .map(|idx| &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()]) + .map(|idx| &step.rws.stack[idx]) .map(|operation| (operation.rw(), operation.op())), [ ( @@ -190,7 +145,7 @@ mod mstore_tests { ] ); - let memory_op = &builder.block.container.memory[step.bus_mapping_instance[2].as_usize()]; + let memory_op = &step.rws.memory[0]; assert_eq!( (memory_op.rw(), memory_op.op()), (RW::WRITE, &MemoryOp::new(1, MemoryAddress(0x100), 0x34)) diff --git a/bus-mapping/src/evm/opcodes/number.rs b/bus-mapping/src/evm/opcodes/number.rs index 58a69578c3..24ead2cdb7 100644 --- a/bus-mapping/src/evm/opcodes/number.rs +++ b/bus-mapping/src/evm/opcodes/number.rs @@ -23,7 +23,9 @@ mod number_tests { TestContext::new_from_geth_data(new_single_tx_trace_code_at_start(&code).unwrap()); let mut builder = block.new_circuit_input_builder(); - builder.handle_tx(&block.eth_tx, &block.geth_trace).unwrap(); + builder + .handle_tx(&block.eth_tx, &block.geth_trace) + .unwrap(); let mut test_builder = block.new_circuit_input_builder(); let mut tx = test_builder diff --git a/bus-mapping/src/evm/opcodes/selfbalance.rs b/bus-mapping/src/evm/opcodes/selfbalance.rs index 979532a185..9ff167ac71 100644 --- a/bus-mapping/src/evm/opcodes/selfbalance.rs +++ b/bus-mapping/src/evm/opcodes/selfbalance.rs @@ -56,16 +56,14 @@ impl Opcode for Selfbalance { mod selfbalance_tests { use super::*; use crate::{ - circuit_input_builder::ExecState, - mock::BlockData, + evm::opcodes::test_util::TestCase, operation::{CallContextField, CallContextOp, StackOp, RW}, }; use eth_types::{ bytecode, evm_types::{OpcodeId, StackAddress}, - geth_types::GethData, }; - use mock::test_ctx::{helpers::*, TestContext}; + use pretty_assertions::assert_eq; #[test] @@ -74,36 +72,16 @@ mod selfbalance_tests { SELFBALANCE STOP }; + let test = TestCase::new_from_bytecode(code); + let step = test.step_witness(OpcodeId::SELFBALANCE, 0); - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); - - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); - - let step = builder.block.txs()[0] - .steps() - .iter() - .find(|step| step.exec_state == ExecState::Op(OpcodeId::SELFBALANCE)) - .unwrap(); - - let call_id = builder.block.txs()[0].calls()[0].call_id; - let callee_address = builder.block.txs()[0].to; - let self_balance = builder.sdb.get_account(&callee_address).1.balance; + let call_id = test.tx_witness().calls()[0].call_id; + let callee_address = test.tx_witness().to; + let self_balance = test.state_db().get_account(&callee_address).1.balance; assert_eq!( { - let operation = - &builder.block.container.call_context[step.bus_mapping_instance[0].as_usize()]; + let operation = &step.rws.call_context[0]; (operation.rw(), operation.op()) }, ( @@ -117,8 +95,7 @@ mod selfbalance_tests { ); assert_eq!( { - let operation = - &builder.block.container.account[step.bus_mapping_instance[1].as_usize()]; + let operation = &step.rws.account[0]; (operation.rw(), operation.op()) }, ( @@ -133,8 +110,7 @@ mod selfbalance_tests { ); assert_eq!( { - let operation = - &builder.block.container.stack[step.bus_mapping_instance[2].as_usize()]; + let operation = &step.rws.stack[0]; (operation.rw(), operation.op()) }, ( diff --git a/bus-mapping/src/evm/opcodes/sload.rs b/bus-mapping/src/evm/opcodes/sload.rs index 74cceca98c..95a87d4091 100644 --- a/bus-mapping/src/evm/opcodes/sload.rs +++ b/bus-mapping/src/evm/opcodes/sload.rs @@ -1,10 +1,11 @@ use super::Opcode; use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; +use crate::operation::{CallContextField, CallContextOp}; use crate::{ - operation::{StorageOp, RW}, + operation::{StorageOp, TxAccessListAccountStorageOp, RW}, Error, }; -use eth_types::GethExecStep; +use eth_types::{GethExecStep, ToWord, Word}; /// Placeholder structure used to implement [`Opcode`] trait over it /// corresponding to the [`OpcodeId::SLOAD`](crate::evm::OpcodeId::SLOAD) @@ -20,34 +21,87 @@ impl Opcode for Sload { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; + let call_id = state.call()?.call_id; + let contract_addr = state.call()?.address; + + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id, + field: CallContextField::TxId, + value: Word::from(state.tx_ctx.id()), + }, + ); + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id, + field: CallContextField::RwCounterEndOfReversion, + value: Word::from(state.call()?.rw_counter_end_of_reversion), + }, + ); + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id, + field: CallContextField::IsPersistent, + value: Word::from(state.call()?.is_persistent as u8), + }, + ); + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id, + field: CallContextField::CalleeAddress, + value: contract_addr.to_word(), + }, + ); + // First stack read - let stack_value_read = geth_step.stack.last()?; + let key = geth_step.stack.last()?; let stack_position = geth_step.stack.last_filled(); // Manage first stack read at latest stack position - state.push_stack_op(&mut exec_step, RW::READ, stack_position, stack_value_read)?; + state.push_stack_op(&mut exec_step, RW::READ, stack_position, key)?; // Storage read - let storage_value_read = geth_step.storage.get_or_err(&stack_value_read)?; + let value = geth_step.storage.get_or_err(&key)?; + + let is_warm = state + .sdb + .check_account_storage_in_access_list(&(contract_addr, key)); + + let (_, committed_value) = state.sdb.get_committed_storage(&contract_addr, &key); + let committed_value = *committed_value; state.push_op( &mut exec_step, RW::READ, StorageOp::new( - state.call()?.address, - stack_value_read, - storage_value_read, - storage_value_read, + contract_addr, + key, + value, + value, state.tx_ctx.id(), - storage_value_read, // TODO: committed_value + committed_value, ), ); // First stack write - state.push_stack_op( + state.push_stack_op(&mut exec_step, RW::WRITE, stack_position, value)?; + state.push_op_reversible( &mut exec_step, RW::WRITE, - stack_position, - storage_value_read, + TxAccessListAccountStorageOp { + tx_id: state.tx_ctx.id(), + address: contract_addr, + key, + value: true, + value_prev: is_warm, + }, )?; Ok(vec![exec_step]) @@ -57,17 +111,13 @@ impl Opcode for Sload { #[cfg(test)] mod sload_tests { use super::*; - use crate::{circuit_input_builder::ExecState, mock::BlockData, operation::StackOp}; + use crate::{evm::opcodes::test_util::step_witness_for_bytecode, operation::StackOp}; use eth_types::{ bytecode, evm_types::{OpcodeId, StackAddress}, - geth_types::GethData, Word, }; - use mock::{ - test_ctx::{helpers::*, TestContext}, - MOCK_ACCOUNTS, - }; + use mock::MOCK_ACCOUNTS; use pretty_assertions::assert_eq; #[test] @@ -84,30 +134,11 @@ mod sload_tests { STOP }; - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); - - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); - - let step = builder.block.txs()[0] - .steps() - .iter() - .find(|step| step.exec_state == ExecState::Op(OpcodeId::SLOAD)) - .unwrap(); + let step = step_witness_for_bytecode(code, OpcodeId::SLOAD); assert_eq!( - [0, 2] - .map(|idx| &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()]) + [0, 1] + .map(|idx| &step.rws.stack[idx]) .map(|operation| (operation.rw(), operation.op())), [ ( @@ -121,7 +152,7 @@ mod sload_tests { ] ); - let storage_op = &builder.block.container.storage[step.bus_mapping_instance[1].as_usize()]; + let storage_op = &step.rws.storage[0]; assert_eq!( (storage_op.rw(), storage_op.op()), ( @@ -132,7 +163,7 @@ mod sload_tests { Word::from(0x6fu32), Word::from(0x6fu32), 1, - Word::from(0x6fu32), + Word::from(0x0u32), ) ) ) diff --git a/bus-mapping/src/evm/opcodes/sstore.rs b/bus-mapping/src/evm/opcodes/sstore.rs new file mode 100644 index 0000000000..c231998038 --- /dev/null +++ b/bus-mapping/src/evm/opcodes/sstore.rs @@ -0,0 +1,176 @@ +use super::Opcode; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; +use crate::operation::{CallContextField, CallContextOp, TxRefundOp}; +use crate::{ + operation::{StorageOp, TxAccessListAccountStorageOp, RW}, + Error, +}; + +use eth_types::{GethExecStep, ToWord, Word}; + +/// Placeholder structure used to implement [`Opcode`] trait over it +/// corresponding to the [`OpcodeId::SSTORE`](crate::evm::OpcodeId::SSTORE) +/// `OpcodeId`. +#[derive(Debug, Copy, Clone)] +pub(crate) struct Sstore; + +impl Opcode for Sstore { + fn gen_associated_ops( + state: &mut CircuitInputStateRef, + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; + + let _call_id = state.call()?.call_id; + let contract_addr = state.call()?.address; + + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id: state.call()?.call_id, + field: CallContextField::TxId, + value: Word::from(state.tx_ctx.id()), + }, + ); + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id: state.call()?.call_id, + field: CallContextField::RwCounterEndOfReversion, + value: Word::from(state.call()?.rw_counter_end_of_reversion), + }, + ); + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id: state.call()?.call_id, + field: CallContextField::IsPersistent, + value: Word::from(state.call()?.is_persistent as u8), + }, + ); + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id: state.call()?.call_id, + field: CallContextField::CalleeAddress, + value: state.call()?.address.to_word(), + }, + ); + + let key = geth_step.stack.nth_last(0)?; + let key_stack_position = geth_step.stack.nth_last_filled(0); + let value = geth_step.stack.nth_last(1)?; + let value_stack_position = geth_step.stack.nth_last_filled(1); + + state.push_stack_op(&mut exec_step, RW::READ, key_stack_position, key)?; + state.push_stack_op(&mut exec_step, RW::READ, value_stack_position, value)?; + + let is_warm = state + .sdb + .check_account_storage_in_access_list(&(contract_addr, key)); + + let (_, value_prev) = state.sdb.get_storage(&contract_addr, &key); + let value_prev = *value_prev; + let (_, committed_value) = state.sdb.get_committed_storage(&contract_addr, &key); + let committed_value = *committed_value; + + state.push_op_reversible( + &mut exec_step, + RW::WRITE, + StorageOp::new( + state.call()?.address, + key, + value, + value_prev, + state.tx_ctx.id(), + committed_value, + ), + )?; + + state.push_op_reversible( + &mut exec_step, + RW::WRITE, + TxAccessListAccountStorageOp { + tx_id: state.tx_ctx.id(), + address: state.call()?.address, + key, + value: true, + value_prev: is_warm, + }, + )?; + + state.push_op_reversible( + &mut exec_step, + RW::WRITE, + TxRefundOp { + tx_id: state.tx_ctx.id(), + value_prev: state.sdb.refund(), + value: geth_step.refund.0, + }, + )?; + + Ok(vec![exec_step]) + } +} + +#[cfg(test)] +mod sstore_tests { + use super::*; + use crate::evm::opcodes::test_util::step_witness_for_bytecode; + use crate::operation::StackOp; + use eth_types::bytecode; + use eth_types::evm_types::{OpcodeId, StackAddress}; + use eth_types::Word; + use mock::MOCK_ACCOUNTS; + use pretty_assertions::assert_eq; + + #[test] + fn sstore_opcode_impl() { + let code = bytecode! { + // Write 0x6f to storage slot 0 + PUSH1(0x6fu64) + PUSH1(0x00u64) + SSTORE + STOP + }; + + let step = step_witness_for_bytecode(code, OpcodeId::SSTORE); + + assert_eq!( + [0, 1] + .map(|idx| &step.rws.stack[idx]) + .map(|operation| (operation.rw(), operation.op())), + [ + ( + RW::READ, + &StackOp::new(1, StackAddress::from(1022), Word::from(0x0u32)) + ), + ( + RW::READ, + &StackOp::new(1, StackAddress::from(1023), Word::from(0x6fu32)) + ), + ] + ); + + let storage_op = &step.rws.storage[0]; + assert_eq!( + (storage_op.rw(), storage_op.op()), + ( + RW::WRITE, + &StorageOp::new( + MOCK_ACCOUNTS[0], + Word::from(0x0u32), + Word::from(0x6fu32), + Word::from(0x0u32), + 1, + Word::from(0x0u32), + ) + ) + ) + } +} diff --git a/bus-mapping/src/evm/opcodes/stackonlyop.rs b/bus-mapping/src/evm/opcodes/stackonlyop.rs index 87292ae4af..7d376cf77d 100644 --- a/bus-mapping/src/evm/opcodes/stackonlyop.rs +++ b/bus-mapping/src/evm/opcodes/stackonlyop.rs @@ -46,15 +46,14 @@ impl Opcode for StackOnlyOpcode( @@ -63,42 +62,18 @@ mod stackonlyop_tests { pops: Vec, pushes: Vec, ) { - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); - - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); - - let step = builder.block.txs()[0] - .steps() - .iter() - .find(|step| step.exec_state == ExecState::Op(opcode)) - .unwrap(); + let step = step_witness_for_bytecode(code, opcode); assert_eq!( (0..N_POP) - .map(|idx| { - &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()] - }) + .map(|idx| { &step.rws.stack[idx] }) .map(|operation| (operation.rw(), operation.op().clone())) .collect_vec(), pops.into_iter().map(|pop| (RW::READ, pop)).collect_vec() ); assert_eq!( (0..N_PUSH) - .map(|idx| { - &builder.block.container.stack - [step.bus_mapping_instance[N_POP + idx].as_usize()] - }) + .map(|idx| { &step.rws.stack[N_POP + idx] }) .map(|operation| (operation.rw(), operation.op().clone())) .collect_vec(), pushes diff --git a/bus-mapping/src/evm/opcodes/swap.rs b/bus-mapping/src/evm/opcodes/swap.rs index 5199d8b464..716ab47d88 100644 --- a/bus-mapping/src/evm/opcodes/swap.rs +++ b/bus-mapping/src/evm/opcodes/swap.rs @@ -55,11 +55,12 @@ impl Opcode for Swap { #[cfg(test)] mod swap_tests { use super::*; - use crate::mock::BlockData; + use crate::evm::opcodes::test_util::TestCase; use crate::operation::StackOp; - use eth_types::{bytecode, evm_types::StackAddress, geth_types::GethData, Word}; - use itertools::Itertools; - use mock::test_ctx::{helpers::*, TestContext}; + + use eth_types::evm_types::OpcodeId; + use eth_types::{bytecode, evm_types::StackAddress, Word}; + use pretty_assertions::assert_eq; #[test] @@ -77,28 +78,18 @@ mod swap_tests { STOP }; - // Get the execution steps from the external tracer - let block: GethData = TestContext::<2, 1>::new( - None, - account_0_code_account_1_no_code(code), - tx_from_1_to_0, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap() - .into(); - - let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder(); - builder - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(); + let test = TestCase::new_from_bytecode(code); // Generate steps corresponding to DUP1, DUP3, DUP5 - for (i, (a, b)) in [(6, 5), (5, 3), (3, 1)].iter().enumerate() { - let step = builder.block.txs()[0] - .steps() - .iter() - .filter(|step| step.exec_state.is_swap()) - .collect_vec()[i]; + for (i, (op, a, b)) in [ + (OpcodeId::SWAP1, 6, 5), + (OpcodeId::SWAP3, 5, 3), + (OpcodeId::SWAP5, 3, 1), + ] + .iter() + .enumerate() + { + let step = test.step_witness(*op, 0); let a_pos = StackAddress(1024 - 6); let b_pos = StackAddress(1024 - 5 + i * 2); @@ -107,8 +98,7 @@ mod swap_tests { assert_eq!( [0, 1, 2, 3] - .map(|idx| &builder.block.container.stack - [step.bus_mapping_instance[idx].as_usize()]) + .map(|idx| &step.rws.stack[idx]) .map(|operation| (operation.rw(), operation.op())), [ (RW::READ, &StackOp::new(1, b_pos, b_val)), diff --git a/bus-mapping/src/evm/opcodes/test_util.rs b/bus-mapping/src/evm/opcodes/test_util.rs new file mode 100644 index 0000000000..b5e6728fe6 --- /dev/null +++ b/bus-mapping/src/evm/opcodes/test_util.rs @@ -0,0 +1,86 @@ +use eth_types::{evm_types::OpcodeId, geth_types::GethData, Bytecode}; +use mock::{ + test_ctx::helpers::{account_0_code_account_1_no_code, tx_from_1_to_0}, + TestContext, +}; + +use crate::{ + circuit_input_builder::{Block, ExecState, ExecStep, Transaction}, + mock::BlockData, + operation::OperationContainer, + state_db::StateDB, +}; + +pub struct StepWitness { + pub step: ExecStep, + pub rws: OperationContainer, +} + +pub struct TestCase { + pub geth_data: GethData, + pub block: Block, + pub state_db: StateDB, +} + +impl TestCase { + pub fn new_from_geth_data(geth_data: GethData) -> Self { + let mut builder = + BlockData::new_from_geth_data(geth_data.clone()).new_circuit_input_builder(); + builder + .handle_block(&geth_data.eth_block, &geth_data.geth_traces) + .unwrap(); + Self { + geth_data, + block: builder.block, + state_db: builder.sdb, + } + } + pub fn new_from_bytecode(code: Bytecode) -> Self { + // Get the execution steps from the external tracer + let geth_data: GethData = TestContext::<2, 1>::new( + None, + account_0_code_account_1_no_code(code), + tx_from_1_to_0, + |block, _tx| block.number(0xcafeu64), + ) + .unwrap() + .into(); + Self::new_from_geth_data(geth_data) + } + + pub fn state_db(&self) -> &StateDB { + &self.state_db + } + pub fn block_input(&self) -> ð_types::Block { + &self.geth_data.eth_block + } + pub fn tx_input(&self) -> ð_types::Transaction { + &self.geth_data.eth_block.transactions[0] + } + pub fn block_witness(&self) -> &Block { + &self.block + } + pub fn tx_witness(&self) -> &Transaction { + &self.block_witness().txs()[0] + } + pub fn step_witness(&self, op: OpcodeId, nth: usize) -> StepWitness { + let block = self.block_witness(); + let tx = self.tx_witness(); + let step = tx + .steps() + .iter() + .filter(|step| step.exec_state == ExecState::Op(op)) + .nth(nth) + .unwrap() + .clone(); + StepWitness { + rws: block.container.slice(&step.bus_mapping_instance), + step, + } + } +} + +/// Most commmonly used case. So we provide a short cut helper function here +pub fn step_witness_for_bytecode(code: Bytecode, op: OpcodeId) -> StepWitness { + TestCase::new_from_bytecode(code).step_witness(op, 0) +} diff --git a/bus-mapping/src/lib.rs b/bus-mapping/src/lib.rs index 56660652bb..8d9854b97c 100644 --- a/bus-mapping/src/lib.rs +++ b/bus-mapping/src/lib.rs @@ -65,6 +65,7 @@ //! "op": "PUSH1", //! "gas": 82, //! "gasCost": 3, +//! "refund": 0, //! "depth": 1, //! "stack": [], //! "memory": [ @@ -78,6 +79,7 @@ //! "op": "MLOAD", //! "gas": 79, //! "gasCost": 3, +//! "refund": 0, //! "depth": 1, //! "stack": [ //! "40" @@ -93,6 +95,7 @@ //! "op": "STOP", //! "gas": 76, //! "gasCost": 0, +//! "refund": 0, //! "depth": 1, //! "stack": [ //! "80" @@ -136,6 +139,7 @@ //! //! let geth_steps: Vec = serde_json::from_str(input_trace).unwrap(); //! let geth_trace = GethExecTrace { +//! return_value: "".to_string(), //! gas: Gas(block.eth_block.transactions[0].gas.as_u64()), //! failed: false, //! struct_logs: geth_steps, diff --git a/bus-mapping/src/operation/container.rs b/bus-mapping/src/operation/container.rs index 8d4c473833..32f58a13fb 100644 --- a/bus-mapping/src/operation/container.rs +++ b/bus-mapping/src/operation/container.rs @@ -1,6 +1,7 @@ use super::{ - AccountDestructedOp, AccountOp, CallContextOp, MemoryOp, Op, OpEnum, Operation, StackOp, - StorageOp, Target, TxAccessListAccountOp, TxAccessListAccountStorageOp, TxRefundOp, + AccountDestructedOp, AccountOp, CallContextOp, MemoryOp, Op, OpEnum, Operation, RWCounter, + StackOp, StorageOp, Target, TxAccessListAccountOp, TxAccessListAccountStorageOp, TxRefundOp, + RW, }; use crate::exec_trace::OperationRef; use itertools::Itertools; @@ -72,7 +73,21 @@ impl OperationContainer { let rwc = op.rwc(); let rw = op.rw(); let reversible = op.reversible(); - match op.op.into_enum() { + self.insert_op_enum(rwc, rw, reversible, op.op.into_enum()) + } + + /// Inserts an [`OpEnum`] into the container returning a lightweight + /// reference to it in the form of an [`OperationRef`] which points to the + /// location of the inserted operation inside the corresponding container + /// vector. + pub fn insert_op_enum( + &mut self, + rwc: RWCounter, + rw: RW, + reversible: bool, + op_enum: OpEnum, + ) -> OperationRef { + match op_enum { OpEnum::Memory(op) => { self.memory.push(Operation::new(rwc, rw, op)); OperationRef::from((Target::Memory, self.memory.len() - 1)) @@ -159,6 +174,35 @@ impl OperationContainer { pub fn sorted_storage(&self) -> Vec> { self.storage.iter().sorted().cloned().collect() } + + /// Returns a subset of operations given some [`OperationRef`] + pub fn slice(&self, indexes: &[OperationRef]) -> OperationContainer { + let mut result = OperationContainer::default(); + for rw_ref in indexes { + match rw_ref.target() { + Target::Memory => result.memory.push(self.memory[rw_ref.as_usize()].clone()), + Target::Stack => result.stack.push(self.stack[rw_ref.as_usize()].clone()), + Target::Storage => result.storage.push(self.storage[rw_ref.as_usize()].clone()), + Target::TxAccessListAccount => result + .tx_access_list_account + .push(self.tx_access_list_account[rw_ref.as_usize()].clone()), + Target::TxAccessListAccountStorage => result + .tx_access_list_account_storage + .push(self.tx_access_list_account_storage[rw_ref.as_usize()].clone()), + Target::TxRefund => result + .tx_refund + .push(self.tx_refund[rw_ref.as_usize()].clone()), + Target::Account => result.account.push(self.account[rw_ref.as_usize()].clone()), + Target::AccountDestructed => result + .account_destructed + .push(self.account_destructed[rw_ref.as_usize()].clone()), + Target::CallContext => result + .call_context + .push(self.call_context[rw_ref.as_usize()].clone()), + } + } + result + } } #[cfg(test)] diff --git a/bus-mapping/src/state_db.rs b/bus-mapping/src/state_db.rs index 30c9e6b788..bc9402a684 100644 --- a/bus-mapping/src/state_db.rs +++ b/bus-mapping/src/state_db.rs @@ -73,9 +73,16 @@ impl Account { #[derive(Debug, Clone, Default)] pub struct StateDB { state: HashMap, + // Fields with transaction lifespan, will be clear in `clear_access_list_and_refund`. access_list_account: HashSet
, access_list_account_storage: HashSet<(Address, U256)>, + // `dirty_storage` constains writes during current transaction. + // When current transaction finishes, `dirty_storage` will be committed into `state`. + // The reason why we need this is that EVM needs committed state, namely + // state before current transaction, to calculate gas cost for some opcodes like sstore. + // So both dirty storage and committed storage are needed. + dirty_storage: HashMap<(Address, Word), Word>, refund: u64, } @@ -86,6 +93,7 @@ impl StateDB { state: HashMap::new(), access_list_account: HashSet::new(), access_list_account_storage: HashSet::new(), + dirty_storage: HashMap::new(), refund: 0, } } @@ -120,7 +128,19 @@ impl StateDB { /// Get a reference to the storage value from [`Account`] at `addr`, at /// `key`. Returns false and a zero [`Word`] when the [`Account`] or `key` /// wasn't found in the state. + /// Returns dirty storage state, which includes writes in current tx pub fn get_storage(&self, addr: &Address, key: &Word) -> (bool, &Word) { + match self.dirty_storage.get(&(*addr, *key)) { + Some(v) => (true, v), + None => self.get_committed_storage(addr, key), + } + } + + /// Get a reference to the storage value from [`Account`] at `addr`, at + /// `key`. Returns false and a zero [`Word`] when the [`Account`] or `key` + /// wasn't found in the state. + /// Returns committed storage, which is storage state before current tx + pub fn get_committed_storage(&self, addr: &Address, key: &Word) -> (bool, &Word) { let (_, acc) = self.get_account(addr); match acc.storage.get(key) { Some(value) => (true, value), @@ -145,6 +165,14 @@ impl StateDB { (found, acc.storage.get_mut(key).expect("key not inserted")) } + /// Set storage value at `addr` and `key`. + /// Writes into dirty_storage during transaction execution. + /// After transaction execution, `dirty_storage` is committed into `storage` + /// in `commit_tx` method. + pub fn set_storage(&mut self, addr: &Address, key: &Word, value: &Word) { + self.dirty_storage.insert((*addr, *key), *value); + } + /// Increase nonce of account with `addr` and return the previous value. pub fn increase_nonce(&mut self, addr: &Address) -> u64 { let (_, account) = self.get_account_mut(addr); @@ -175,16 +203,32 @@ impl StateDB { debug_assert!(self.access_list_account_storage.remove(pair)); } + /// Check whether `(addr, key)` exists in account storage access list. + pub fn check_account_storage_in_access_list(&self, pair: &(Address, Word)) -> bool { + self.access_list_account_storage.contains(pair) + } + /// Retrieve refund. pub fn refund(&self) -> u64 { self.refund } - /// Clear access list and refund. It should be invoked before processing + /// Set refund + pub fn set_refund(&mut self, value: u64) { + self.refund = value; + } + + /// Clear access list and refund, and commit dirty storage. + /// It should be invoked before processing /// with new transaction with the same [`StateDB`]. - pub fn clear_access_list_and_refund(&mut self) { + pub fn commit_tx(&mut self) { self.access_list_account = HashSet::new(); self.access_list_account_storage = HashSet::new(); + for ((addr, key), value) in self.dirty_storage.clone() { + let (_, ptr) = self.get_storage_mut(&addr, &key); + *ptr = value; + } + self.dirty_storage = HashMap::new(); self.refund = 0; } } diff --git a/eth-types/src/evm_types.rs b/eth-types/src/evm_types.rs index c77564880a..b09b06afaa 100644 --- a/eth-types/src/evm_types.rs +++ b/eth-types/src/evm_types.rs @@ -52,7 +52,7 @@ impl ProgramCounter { } /// Defines the gas left to perate. -#[derive(Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Serialize, Deserialize)] +#[derive(Default, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Serialize, Deserialize)] pub struct Gas(pub u64); impl fmt::Debug for Gas { diff --git a/eth-types/src/lib.rs b/eth-types/src/lib.rs index 915f54c632..457fa8135b 100644 --- a/eth-types/src/lib.rs +++ b/eth-types/src/lib.rs @@ -220,6 +220,8 @@ struct GethExecStepInternal { pc: ProgramCounter, op: OpcodeId, gas: Gas, + #[serde(default)] + refund: Gas, #[serde(rename = "gasCost")] gas_cost: GasCost, depth: u16, @@ -243,6 +245,7 @@ pub struct GethExecStep { pub op: OpcodeId, pub gas: Gas, pub gas_cost: GasCost, + pub refund: Gas, pub depth: u16, pub error: Option, // stack is in hex 0x prefixed @@ -298,6 +301,7 @@ impl<'de> Deserialize<'de> for GethExecStep { pc: s.pc, op: s.op, gas: s.gas, + refund: s.refund, gas_cost: s.gas_cost, depth: s.depth, error: s.error, @@ -340,6 +344,8 @@ pub struct GethExecTraceInternal { pub gas: Gas, pub failed: bool, // return_value is a hex encoded byte array + #[serde(rename = "returnValue")] + pub return_value: String, #[serde(rename = "structLogs")] pub struct_logs: Vec, } @@ -355,6 +361,8 @@ pub struct GethExecTrace { pub gas: Gas, /// True when the transaction has failed. pub failed: bool, + /// Return value of execution + pub return_value: String, /// Vector of geth execution steps of the trace. pub struct_logs: Vec, } @@ -396,12 +404,14 @@ impl<'de> Deserialize<'de> for GethExecTrace { gas, failed, mut struct_logs, + return_value, } = GethExecTraceInternal::deserialize(deserializer)?; fix_geth_trace_memory_size(&mut struct_logs); Ok(Self { gas, failed, struct_logs, + return_value, }) } } @@ -459,6 +469,7 @@ mod tests { "op": "PUSH1", "gas": 22705, "gasCost": 3, + "refund": 0, "depth": 1, "stack": [] }, @@ -467,6 +478,7 @@ mod tests { "op": "SLOAD", "gas": 5217, "gasCost": 2100, + "refund": 0, "depth": 1, "stack": [ "0x1003e2d2", @@ -487,6 +499,7 @@ mod tests { "op": "KECCAK256", "gas": 178805, "gasCost": 42, + "refund": 0, "depth": 1, "stack": [ "0x3635c9adc5dea00000", @@ -512,11 +525,13 @@ mod tests { GethExecTraceInternal { gas: Gas(26809), failed: false, + return_value: "".to_owned(), struct_logs: vec![ GethExecStep { pc: ProgramCounter(0), op: OpcodeId::PUSH1, gas: Gas(22705), + refund: Gas(0), gas_cost: GasCost(3), depth: 1, error: None, @@ -528,6 +543,7 @@ mod tests { pc: ProgramCounter(163), op: OpcodeId::SLOAD, gas: Gas(5217), + refund: Gas(0), gas_cost: GasCost(2100), depth: 1, error: None, @@ -539,6 +555,7 @@ mod tests { pc: ProgramCounter(189), op: OpcodeId::SHA3, gas: Gas(178805), + refund: Gas(0), gas_cost: GasCost(42), depth: 1, error: None, diff --git a/geth-utils/gethutil/trace.go b/geth-utils/gethutil/trace.go index e698dde4d2..1a329b96d5 100644 --- a/geth-utils/gethutil/trace.go +++ b/geth-utils/gethutil/trace.go @@ -30,15 +30,16 @@ type ExecutionResult struct { // transaction in debug mode // Copied from github.com/ethereum/go-ethereum/internal/ethapi.StructLogRes type StructLogRes struct { - Pc uint64 `json:"pc"` - Op string `json:"op"` - Gas uint64 `json:"gas"` - GasCost uint64 `json:"gasCost"` - Depth int `json:"depth"` - Error string `json:"error,omitempty"` - Stack *[]string `json:"stack,omitempty"` - Memory *[]string `json:"memory,omitempty"` - Storage *map[string]string `json:"storage,omitempty"` + Pc uint64 `json:"pc"` + Op string `json:"op"` + Gas uint64 `json:"gas"` + GasCost uint64 `json:"gasCost"` + Depth int `json:"depth"` + Error string `json:"error,omitempty"` + Stack *[]string `json:"stack,omitempty"` + Memory *[]string `json:"memory,omitempty"` + Storage *map[string]string `json:"storage,omitempty"` + RefundCounter uint64 `json:"refund,omitempty"` } // Copied from github.com/ethereum/go-ethereum/internal/ethapi.FormatLogs @@ -47,12 +48,13 @@ func FormatLogs(logs []logger.StructLog) []StructLogRes { formatted := make([]StructLogRes, len(logs)) for index, trace := range logs { formatted[index] = StructLogRes{ - Pc: trace.Pc, - Op: trace.Op.String(), - Gas: trace.Gas, - GasCost: trace.GasCost, - Depth: trace.Depth, - Error: trace.ErrorString(), + Pc: trace.Pc, + Op: trace.Op.String(), + Gas: trace.Gas, + GasCost: trace.GasCost, + Depth: trace.Depth, + Error: trace.ErrorString(), + RefundCounter: trace.RefundCounter, } if trace.Stack != nil { stack := make([]string, len(trace.Stack)) diff --git a/zkevm-circuits/Cargo.toml b/zkevm-circuits/Cargo.toml index c7d247f35e..bde5a2ad86 100644 --- a/zkevm-circuits/Cargo.toml +++ b/zkevm-circuits/Cargo.toml @@ -26,8 +26,13 @@ itertools = "0.10.3" lazy_static = "1.4" keccak256 = { path = "../keccak256"} +log = "0.4.14" +env_logger = "0.9" + [dev-dependencies] criterion = "0.3" +ctor = "0.1.22" +env_logger = "0.9.0" hex = "0.4.3" mock = { path = "../mock" } diff --git a/zkevm-circuits/src/evm_circuit.rs b/zkevm-circuits/src/evm_circuit.rs index eaa29a080f..d9f3b9b487 100644 --- a/zkevm-circuits/src/evm_circuit.rs +++ b/zkevm-circuits/src/evm_circuit.rs @@ -409,6 +409,10 @@ pub mod test { .map(|bytecode| bytecode.bytes.len()) .sum::(), )); + let k = k.max(log2_ceil( + 64 + block.txs.iter().map(|tx| tx.steps.len()).sum::() * STEP_HEIGHT, + )); + log::debug!("evm circuit uses k = {}", k); let power_of_randomness = (1..32) .map(|exp| { diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 8a846097b9..bc24f213e8 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -487,15 +487,22 @@ impl ExecutionConfig { macro_rules! lookup { ($id:path, $table:ident, $descrip:expr) => { if let Some(acc_lookups) = acc_lookups_of_table.remove(&$id) { - for input_exprs in acc_lookups { - meta.lookup_any(concat!("LOOKUP: ", stringify!($descrip)), |meta| { - let q_step = meta.query_selector(q_step); - input_exprs - .into_iter() - .zip($table.table_exprs(meta).to_vec().into_iter()) - .map(|(input, table)| (q_step.clone() * input, table)) - .collect::>() - }); + for (lookup_idx, input_exprs) in acc_lookups.into_iter().enumerate() { + let idx = + meta.lookup_any(concat!("LOOKUP: ", stringify!($descrip)), |meta| { + let q_step = meta.query_selector(q_step); + input_exprs + .into_iter() + .zip($table.table_exprs(meta).to_vec().into_iter()) + .map(|(input, table)| (q_step.clone() * input, table)) + .collect::>() + }); + log::debug!( + "LOOKUP TABLE {} <=> {} {}", + idx, + stringify!($descrip), + lookup_idx + ); } } }; @@ -580,6 +587,7 @@ impl ExecutionConfig { call: &Call, step: &ExecStep, ) -> Result<(), Error> { + log::trace!("assign_exec_step offset:{} step:{:?}", offset, step); self.step .assign_exec_step(region, offset, block, transaction, call, step)?; diff --git a/zkevm-circuits/src/evm_circuit/execution/sstore.rs b/zkevm-circuits/src/evm_circuit/execution/sstore.rs index 4fa4c969d1..f5d900b6df 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sstore.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sstore.rs @@ -368,16 +368,12 @@ pub(crate) struct SstoreTxRefundGadget { value: Cell, value_prev: Cell, committed_value: Cell, - value_prev_is_zero: IsZeroGadget, - value_is_zero: IsZeroGadget, - original_is_zero: IsZeroGadget, - original_eq_value: IsEqualGadget, - prev_eq_value: IsEqualGadget, - original_eq_prev: IsEqualGadget, - nz_nz_allne_case_refund: Cell, - nz_ne_ne_case_refund: Cell, - ez_ne_ne_case_refund: Cell, - eq_ne_case_refund: Cell, + value_prev_is_zero_gadget: IsZeroGadget, + value_is_zero_gadget: IsZeroGadget, + original_is_zero_gadget: IsZeroGadget, + original_eq_value_gadget: IsEqualGadget, + prev_eq_value_gadget: IsEqualGadget, + original_eq_prev_gadget: IsEqualGadget, } impl SstoreTxRefundGadget { @@ -388,61 +384,45 @@ impl SstoreTxRefundGadget { value_prev: Cell, committed_value: Cell, ) -> Self { - let value_prev_is_zero = IsZeroGadget::construct(cb, value_prev.expr()); - let value_is_zero = IsZeroGadget::construct(cb, value.expr()); - let original_is_zero = IsZeroGadget::construct(cb, committed_value.expr()); - let original_eq_value = IsEqualGadget::construct(cb, committed_value.expr(), value.expr()); - let prev_eq_value = IsEqualGadget::construct(cb, value_prev.expr(), value.expr()); - let original_eq_prev = + let value_prev_is_zero_gadget = IsZeroGadget::construct(cb, value_prev.expr()); + let value_is_zero_gadget = IsZeroGadget::construct(cb, value.expr()); + let original_is_zero_gadget = IsZeroGadget::construct(cb, committed_value.expr()); + + let original_eq_value_gadget = + IsEqualGadget::construct(cb, committed_value.expr(), value.expr()); + let prev_eq_value_gadget = IsEqualGadget::construct(cb, value_prev.expr(), value.expr()); + let original_eq_prev_gadget = IsEqualGadget::construct(cb, committed_value.expr(), value_prev.expr()); - // original_value, value_prev, value all are different; - // original_value!=0&&value_prev!=0 - let nz_nz_allne_case_refund = cb.copy(select::expr( - value_is_zero.expr(), - tx_refund_old.expr() + GasCost::SSTORE_CLEARS_SCHEDULE.expr(), - tx_refund_old.expr(), - )); - // original_value, value_prev, value all are different; original_value!=0 - let nz_allne_case_refund = select::expr( - value_prev_is_zero.expr(), - tx_refund_old.expr() - GasCost::SSTORE_CLEARS_SCHEDULE.expr(), - nz_nz_allne_case_refund.expr(), - ); - // original_value!=value_prev, value_prev!=value, original_value!=0 - let nz_ne_ne_case_refund = cb.copy(select::expr( - not::expr(original_eq_value.expr()), - nz_allne_case_refund.expr(), - nz_allne_case_refund.expr() + GasCost::SSTORE_RESET_GAS.expr() - - GasCost::SLOAD_GAS.expr(), - )); - // original_value!=value_prev, value_prev!=value, original_value==0 - let ez_ne_ne_case_refund = cb.copy(select::expr( - original_eq_value.expr(), - tx_refund_old.expr() + GasCost::SSTORE_SET_GAS.expr() - GasCost::SLOAD_GAS.expr(), - tx_refund_old.expr(), - )); - // original_value!=value_prev, value_prev!=value - let ne_ne_case_refund = select::expr( - not::expr(original_is_zero.expr()), - nz_ne_ne_case_refund.expr(), - ez_ne_ne_case_refund.expr(), - ); - // original_value==value_prev, value_prev!=value - let eq_ne_case_refund = cb.copy(select::expr( - not::expr(original_is_zero.expr()) * value_is_zero.expr(), - tx_refund_old.expr() + GasCost::SSTORE_CLEARS_SCHEDULE.expr(), - tx_refund_old.expr(), - )); - let tx_refund_new = select::expr( - prev_eq_value.expr(), - tx_refund_old.expr(), - select::expr( - original_eq_prev.expr(), - eq_ne_case_refund.expr(), - ne_ne_case_refund.expr(), - ), - ); + let value_prev_is_zero = value_prev_is_zero_gadget.expr(); + let value_is_zero = value_is_zero_gadget.expr(); + let original_is_zero = original_is_zero_gadget.expr(); + + let original_eq_value = original_eq_value_gadget.expr(); + let prev_eq_value = prev_eq_value_gadget.expr(); + let original_eq_prev = original_eq_prev_gadget.expr(); + + // (value_prev != value) && (committed_value != Word::from(0)) && (value == + // Word::from(0)) + let case_a = + not::expr(prev_eq_value.clone()) * not::expr(original_is_zero.clone()) * value_is_zero; + // (value_prev != value) && (committed_value == value) && (committed_value != + // Word::from(0)) + let case_b = not::expr(prev_eq_value.clone()) + * original_eq_value.clone() + * not::expr(original_is_zero.clone()); + // (value_prev != value) && (committed_value == value) && (committed_value == + // Word::from(0)) + let case_c = not::expr(prev_eq_value.clone()) * original_eq_value * (original_is_zero); + // (value_prev != value) && (committed_value != value_prev) && (value_prev == + // Word::from(0)) + let case_d = not::expr(prev_eq_value) * not::expr(original_eq_prev) * (value_prev_is_zero); + + let tx_refund_new = tx_refund_old.expr() + + case_a * GasCost::SSTORE_CLEARS_SCHEDULE.expr() + + case_b * (GasCost::SSTORE_RESET_GAS.expr() - GasCost::SLOAD_GAS.expr()) + + case_c * (GasCost::SSTORE_SET_GAS.expr() - GasCost::SLOAD_GAS.expr()) + - case_d * (GasCost::SSTORE_CLEARS_SCHEDULE.expr()); Self { value, @@ -450,16 +430,12 @@ impl SstoreTxRefundGadget { committed_value, tx_refund_old, tx_refund_new, - value_prev_is_zero, - value_is_zero, - original_is_zero, - original_eq_value, - prev_eq_value, - original_eq_prev, - nz_nz_allne_case_refund, - nz_ne_ne_case_refund, - ez_ne_ne_case_refund, - eq_ne_case_refund, + value_prev_is_zero_gadget, + value_is_zero_gadget, + original_is_zero_gadget, + original_eq_value_gadget, + prev_eq_value_gadget, + original_eq_prev_gadget, } } @@ -502,95 +478,56 @@ impl SstoreTxRefundGadget { randomness, )), )?; - self.value_prev_is_zero.assign( + self.value_prev_is_zero_gadget.assign( region, offset, Word::random_linear_combine(value_prev.to_le_bytes(), randomness), )?; - self.value_is_zero.assign( + self.value_is_zero_gadget.assign( region, offset, Word::random_linear_combine(value.to_le_bytes(), randomness), )?; - self.original_is_zero.assign( + self.original_is_zero_gadget.assign( region, offset, Word::random_linear_combine(committed_value.to_le_bytes(), randomness), )?; - self.original_eq_value.assign( + self.original_eq_value_gadget.assign( region, offset, Word::random_linear_combine(committed_value.to_le_bytes(), randomness), Word::random_linear_combine(value.to_le_bytes(), randomness), )?; - self.prev_eq_value.assign( + self.prev_eq_value_gadget.assign( region, offset, Word::random_linear_combine(value_prev.to_le_bytes(), randomness), Word::random_linear_combine(value.to_le_bytes(), randomness), )?; - self.original_eq_prev.assign( + self.original_eq_prev_gadget.assign( region, offset, Word::random_linear_combine(committed_value.to_le_bytes(), randomness), Word::random_linear_combine(value_prev.to_le_bytes(), randomness), )?; - - let nz_nz_allne_case_refund = if value == eth_types::Word::zero() { - tx_refund_old + GasCost::SSTORE_CLEARS_SCHEDULE.as_u64() - } else { - tx_refund_old - }; - self.nz_nz_allne_case_refund.assign( - region, - offset, - Some(F::from(nz_nz_allne_case_refund)), - )?; - - let nz_allne_case_refund = if value_prev == eth_types::Word::zero() { - tx_refund_old - GasCost::SSTORE_CLEARS_SCHEDULE.as_u64() - } else { - nz_nz_allne_case_refund - }; - let nz_ne_ne_case_refund = if committed_value != value { - nz_allne_case_refund - } else { - nz_allne_case_refund + GasCost::SSTORE_RESET_GAS.as_u64() - GasCost::SLOAD_GAS.as_u64() - }; - self.nz_ne_ne_case_refund - .assign(region, offset, Some(F::from(nz_ne_ne_case_refund)))?; - - let ez_ne_ne_case_refund = if committed_value == value { - tx_refund_old + GasCost::SSTORE_SET_GAS.as_u64() - GasCost::SLOAD_GAS.as_u64() - } else { - tx_refund_old - }; - self.ez_ne_ne_case_refund - .assign(region, offset, Some(F::from(ez_ne_ne_case_refund)))?; - - let eq_ne_case_refund = - if (committed_value != eth_types::Word::zero()) && (value == eth_types::Word::zero()) { - tx_refund_old + GasCost::SSTORE_CLEARS_SCHEDULE.as_u64() - } else { - tx_refund_old - }; - self.eq_ne_case_refund - .assign(region, offset, Some(F::from(eq_ne_case_refund)))?; - Ok(()) } } #[cfg(test)] mod test { - use crate::evm_circuit::{ - param::STACK_CAPACITY, - step::ExecutionState, - table::{CallContextFieldTag, RwTableTag}, - test::{rand_fp, run_test_circuit_incomplete_fixed_table}, - witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, - }; + use crate::{ + evm_circuit::{ + param::STACK_CAPACITY, + step::ExecutionState, + table::{CallContextFieldTag, RwTableTag}, + test::{rand_fp, run_test_circuit_incomplete_fixed_table}, + witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, + }, + test_util::{test_circuits_using_bytecode, BytecodeTestConfig}, + }; use bus_mapping::evm::OpcodeId; use eth_types::{address, bytecode, evm_types::GasCost, ToWord, Word}; use std::convert::TryInto; @@ -628,29 +565,24 @@ mod test { let mut tx_refund_new = tx_refund_old; if value_prev != value { - if committed_value == value_prev { - if (committed_value != Word::from(0)) && (value == Word::from(0)) { - tx_refund_new += GasCost::SSTORE_CLEARS_SCHEDULE.as_u64(); - } - } else { + if (committed_value != Word::from(0)) && (value == Word::from(0)) { + // CaseA + tx_refund_new += GasCost::SSTORE_CLEARS_SCHEDULE.as_u64(); + } + if committed_value == value { if committed_value != Word::from(0) { - if value_prev == Word::from(0) { - tx_refund_new -= GasCost::SSTORE_CLEARS_SCHEDULE.as_u64() - } - if value == Word::from(0) { - tx_refund_new += GasCost::SSTORE_CLEARS_SCHEDULE.as_u64() - } - } - if committed_value == value { - if committed_value == Word::from(0) { - tx_refund_new += - GasCost::SSTORE_SET_GAS.as_u64() - GasCost::SLOAD_GAS.as_u64(); - } else { - tx_refund_new += - GasCost::SSTORE_RESET_GAS.as_u64() - GasCost::SLOAD_GAS.as_u64(); - } + // CaseB + tx_refund_new += + GasCost::SSTORE_RESET_GAS.as_u64() - GasCost::SLOAD_GAS.as_u64(); + } else { + // CaseC + tx_refund_new += GasCost::SSTORE_SET_GAS.as_u64() - GasCost::SLOAD_GAS.as_u64(); } } + if committed_value != value_prev && value_prev == Word::from(0) { + // CaseD + tx_refund_new -= GasCost::SSTORE_CLEARS_SCHEDULE.as_u64() + } } tx_refund_new @@ -1126,4 +1058,46 @@ mod test { false, ); } + + // TODO: with modularized mock, we can test more cases using real trace later, + // including both code/warm persistent/revert. + #[test] + fn sstore_busmapping_simple() { + let bytecode = bytecode! { + #[start] + PUSH32(0x030201) // value + PUSH32(0x060504) // key + SSTORE + STOP + }; + + let test_config = BytecodeTestConfig { + enable_state_circuit_test: false, + ..Default::default() + }; + assert_eq!( + test_circuits_using_bytecode(bytecode, test_config, None), + Ok(()) + ); + } + + #[test] + fn sstore_busmapping_revert() { + let bytecode = bytecode! { + #[start] + PUSH32(0x030201) // value + PUSH32(0x060504) // key + SSTORE + REVERT + }; + + let test_config = BytecodeTestConfig { + enable_state_circuit_test: false, + ..Default::default() + }; + assert_eq!( + test_circuits_using_bytecode(bytecode, test_config, None), + Ok(()) + ); + } } diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 500b0dcad2..eb87a04c8b 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -10,7 +10,10 @@ use crate::{ }, util::Expr, }; -use halo2_proofs::{arithmetic::FieldExt, plonk::Expression}; +use halo2_proofs::{ + arithmetic::FieldExt, + plonk::Expression::{self, Constant}, +}; use std::convert::TryInto; // Max degree allowed in all expressions passing through the ConstraintBuilder. @@ -608,8 +611,19 @@ impl<'a, F: FieldExt> ConstraintBuilder<'a, F> { tag, values, ); - self.rw_counter_offset = - self.rw_counter_offset.clone() + self.cb.condition.clone().unwrap_or_else(|| 1.expr()); + // Manually constant folding is used here, since halo2 cannot do this + // automatically. Better error message will be printed during circuit + // debugging. + self.rw_counter_offset = match &self.cb.condition { + None => { + if let Constant(v) = self.rw_counter_offset { + Constant(v + F::from(1u64)) + } else { + self.rw_counter_offset.clone() + 1i32.expr() + } + } + Some(c) => self.rw_counter_offset.clone() + c.clone(), + }; } fn state_write( diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index dde21abaff..a044f0b71e 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -8,6 +8,12 @@ use halo2_proofs::dev::{MockProver, VerifyFailure}; use mock::TestContext; use pairing::bn256::Fr; +#[cfg(test)] +#[ctor::ctor] +fn init_env_logger() { + // Enable RUST_LOG during tests + env_logger::init(); +} pub enum FixedTableConfig { Incomplete, Complete, From 10793925cd70b78a7a49e7cd2859cb8ae8ebcdfd Mon Sep 17 00:00:00 2001 From: Zhang Zhuo Date: Fri, 1 Apr 2022 20:05:50 +0800 Subject: [PATCH 2/5] change geth to nightly in integration-tests --- integration-tests/docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/docker-compose.yml b/integration-tests/docker-compose.yml index b439cedc55..a11f608059 100644 --- a/integration-tests/docker-compose.yml +++ b/integration-tests/docker-compose.yml @@ -1,7 +1,7 @@ version: '3' services: geth0: - image: "ethereum/client-go:stable" + image: "ethereum/client-go:latest" container_name: zkevm-geth0 ports: - 8545:8545 From 2077c0e16800606b0b1696cbcb3035c5de70530e Mon Sep 17 00:00:00 2001 From: Zhang Zhuo Date: Mon, 4 Apr 2022 15:33:50 +0800 Subject: [PATCH 3/5] fix conflict --- zkevm-circuits/src/evm_circuit/execution/sstore.rs | 9 ++++----- .../src/evm_circuit/util/constraint_builder.rs | 5 ++++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/sstore.rs b/zkevm-circuits/src/evm_circuit/execution/sstore.rs index f62de45422..3607ebcb64 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sstore.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sstore.rs @@ -393,8 +393,8 @@ impl SstoreTxRefundGadget { let tx_refund_new = tx_refund_old.expr() + case_a * GasCost::SSTORE_CLEARS_SCHEDULE.expr() - + case_b * (GasCost::SSTORE_RESET_GAS.expr() - GasCost::SLOAD_GAS.expr()) - + case_c * (GasCost::SSTORE_SET_GAS.expr() - GasCost::SLOAD_GAS.expr()) + + case_b * (GasCost::SSTORE_RESET.expr() - GasCost::COLD_SLOAD.expr()) + + case_c * (GasCost::SSTORE_SET.expr() - GasCost::COLD_SLOAD.expr()) - case_d * (GasCost::SSTORE_CLEARS_SCHEDULE.expr()); Self { @@ -545,11 +545,10 @@ mod test { if committed_value == value { if committed_value != Word::from(0) { // CaseB - tx_refund_new += - GasCost::SSTORE_RESET_GAS.as_u64() - GasCost::SLOAD_GAS.as_u64(); + tx_refund_new += GasCost::SSTORE_RESET.as_u64() - GasCost::COLD_SLOAD.as_u64(); } else { // CaseC - tx_refund_new += GasCost::SSTORE_SET_GAS.as_u64() - GasCost::SLOAD_GAS.as_u64(); + tx_refund_new += GasCost::SSTORE_SET.as_u64() - GasCost::COLD_SLOAD.as_u64(); } } if committed_value != value_prev && value_prev == Word::from(0) { diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index f9e1d8f48d..8013708a3b 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -13,7 +13,10 @@ use crate::{ use halo2_proofs::{ arithmetic::FieldExt, circuit::Region, - plonk::{Error, Expression::{self, Constant}}, + plonk::{ + Error, + Expression::{self, Constant}, + }, }; use std::convert::TryInto; From 2ba341dafb6ff53a67475e26f37168ceb18d50f0 Mon Sep 17 00:00:00 2001 From: Zhang Zhuo Date: Mon, 4 Apr 2022 15:36:18 +0800 Subject: [PATCH 4/5] minor --- bus-mapping/src/circuit_input_builder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index ca6b935eba..4a8fa998bd 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -174,7 +174,7 @@ pub struct ExecStep { /// overflow", this value will **not** be the actual Gas cost of the /// step. pub gas_cost: GasCost, - /// accumulated gas refund + /// Accumulated gas refund pub gas_refund: Gas, /// Call index within the [`Transaction`] pub call_index: usize, @@ -1070,7 +1070,7 @@ impl<'a> CircuitInputStateRef<'a> { } } - /// Apply reverted op to state and push to container. + /// Apply op to state and push to container. fn apply_op(&mut self, is_revert: bool, rw: RW, op: OpEnum) -> OperationRef { match &op { OpEnum::Storage(op) => { From 11534d4ca5d97dbefaaa58d81da3136d58c848bb Mon Sep 17 00:00:00 2001 From: Zhang Zhuo Date: Tue, 5 Apr 2022 14:06:20 +0800 Subject: [PATCH 5/5] fix conflict --- zkevm-circuits/src/evm_circuit/execution/sstore.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/sstore.rs b/zkevm-circuits/src/evm_circuit/execution/sstore.rs index 3607ebcb64..1b503c5b79 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sstore.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sstore.rs @@ -499,10 +499,11 @@ mod test { test::{rand_fp, run_test_circuit_incomplete_fixed_table}, witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, }, - test_util::{test_circuits_using_bytecode, BytecodeTestConfig}, + test_util::{run_test_circuits, BytecodeTestConfig}, }; use bus_mapping::evm::OpcodeId; use eth_types::{address, bytecode, evm_types::GasCost, ToWord, Word}; + use mock::TestContext; use std::convert::TryInto; fn calc_expected_gas_cost( @@ -1048,7 +1049,10 @@ mod test { ..Default::default() }; assert_eq!( - test_circuits_using_bytecode(bytecode, test_config, None), + run_test_circuits( + TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode).unwrap(), + Some(test_config), + ), Ok(()) ); } @@ -1068,7 +1072,10 @@ mod test { ..Default::default() }; assert_eq!( - test_circuits_using_bytecode(bytecode, test_config, None), + run_test_circuits( + TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode).unwrap(), + Some(test_config), + ), Ok(()) ); }