diff --git a/Cargo.lock b/Cargo.lock index 95208fcb96..e7aad335b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4054,7 +4054,9 @@ dependencies = [ "bigint", "bus-mapping", "criterion", + "ctor", "digest 0.7.6", + "env_logger", "eth-types", "ethers-core", "ff 0.11.0", @@ -4063,6 +4065,7 @@ dependencies = [ "itertools", "keccak256", "lazy_static", + "log", "mock", "num", "pairing_bn256", diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 43c297cbba..1323186839 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -174,6 +174,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. @@ -204,6 +206,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, reversible_write_counter, @@ -223,6 +226,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), reversible_write_counter: 0, @@ -805,6 +809,9 @@ impl<'a> CircuitInputStateRef<'a> { rw: RW, op: T, ) -> Result<(), Error> { + if matches!(rw, RW::WRITE) { + self.apply_op(&op.clone().into_enum()); + } let op_ref = self.block.container.insert(Operation::new_reversible( self.block_ctx.rwc.inc_pre(), rw, @@ -1076,38 +1083,29 @@ impl<'a> CircuitInputStateRef<'a> { } } - /// Apply reverted op to state and push to container. - fn apply_reverted_op(&mut self, op: OpEnum) -> OperationRef { - match op { + /// Apply op to state. + fn apply_op(&mut self, op: &OpEnum) { + 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); @@ -1118,16 +1116,13 @@ 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!(), - } + }; } /// Handle a reversion group @@ -1141,7 +1136,13 @@ 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); + self.apply_op(&op); + let rev_op_ref = self.block.container.insert_op_enum( + self.block_ctx.rwc.inc_pre(), + RW::WRITE, + false, + op, + ); self.tx.steps[step_index] .bus_mapping_instance .push(rev_op_ref); @@ -1473,6 +1474,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, @@ -1488,8 +1490,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(()) } @@ -2004,6 +2006,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 08fbe5231a..c3847d6ebf 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -31,6 +31,7 @@ mod number; mod origin; mod selfbalance; mod sload; +mod sstore; mod stackonlyop; mod stop; mod swap; @@ -48,6 +49,7 @@ use mstore::Mstore; use origin::Origin; use selfbalance::Selfbalance; use sload::Sload; +use sstore::Sstore; use stackonlyop::StackOnlyOpcode; use stop::Stop; use swap::Swap; @@ -138,7 +140,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/sload.rs b/bus-mapping/src/evm/opcodes/sload.rs index 74cceca98c..4e91a172ef 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]) @@ -70,19 +124,27 @@ mod sload_tests { }; use pretty_assertions::assert_eq; - #[test] - fn sload_opcode_impl() { - let code = bytecode! { - // Write 0x6f to storage slot 0 - PUSH1(0x6fu64) - PUSH1(0x00u64) - SSTORE - - // Load storage slot 0 - PUSH1(0x00u64) - SLOAD - STOP + fn test_ok(is_warm: bool) { + let code = if is_warm { + bytecode! { + // Write 0x6f to storage slot 0 + PUSH1(0x6fu64) + PUSH1(0x00u64) + SSTORE + // Load storage slot 0 + PUSH1(0x00u64) + SLOAD + STOP + } + } else { + bytecode! { + // Load storage slot 0 + PUSH1(0x00u64) + SLOAD + STOP + } }; + let expected_loaded_value = if is_warm { 0x6fu64 } else { 0 }; // Get the execution steps from the external tracer let block: GethData = TestContext::<2, 1>::new( @@ -106,7 +168,7 @@ mod sload_tests { .unwrap(); assert_eq!( - [0, 2] + [4, 6] .map(|idx| &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()]) .map(|operation| (operation.rw(), operation.op())), [ @@ -116,12 +178,12 @@ mod sload_tests { ), ( RW::WRITE, - &StackOp::new(1, StackAddress::from(1023), Word::from(0x6fu32)) + &StackOp::new(1, StackAddress::from(1023), Word::from(expected_loaded_value)) ) ] ); - let storage_op = &builder.block.container.storage[step.bus_mapping_instance[1].as_usize()]; + let storage_op = &builder.block.container.storage[step.bus_mapping_instance[5].as_usize()]; assert_eq!( (storage_op.rw(), storage_op.op()), ( @@ -129,12 +191,38 @@ mod sload_tests { &StorageOp::new( MOCK_ACCOUNTS[0], Word::from(0x0u32), - Word::from(0x6fu32), - Word::from(0x6fu32), + Word::from(expected_loaded_value), + Word::from(expected_loaded_value), 1, - Word::from(0x6fu32), + Word::from(0x0u32), ) ) + ); + + let access_list_op = &builder.block.container.tx_access_list_account_storage + [step.bus_mapping_instance[7].as_usize()]; + assert_eq!( + (access_list_op.rw(), access_list_op.op()), + ( + RW::WRITE, + &TxAccessListAccountStorageOp { + tx_id: 1, + address: MOCK_ACCOUNTS[0], + key: Word::from(0x0u32), + value: true, + value_prev: is_warm, + }, + ) ) } + + #[test] + fn sload_opcode_impl_warm() { + test_ok(true) + } + + #[test] + fn sload_opcode_impl_cold() { + test_ok(false) + } } diff --git a/bus-mapping/src/evm/opcodes/sstore.rs b/bus-mapping/src/evm/opcodes/sstore.rs new file mode 100644 index 0000000000..5356fb6d53 --- /dev/null +++ b/bus-mapping/src/evm/opcodes/sstore.rs @@ -0,0 +1,243 @@ +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 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::circuit_input_builder::ExecState; + use crate::mock::BlockData; + use crate::operation::StackOp; + use eth_types::bytecode; + use eth_types::evm_types::{OpcodeId, StackAddress}; + use eth_types::geth_types::GethData; + use eth_types::Word; + use mock::test_ctx::helpers::tx_from_1_to_0; + use mock::{TestContext, MOCK_ACCOUNTS}; + use pretty_assertions::assert_eq; + + fn test_ok(is_warm: bool) { + let code = if is_warm { + bytecode! { + // Write 0x00 to storage slot 0 + PUSH1(0x00u64) + PUSH1(0x00u64) + SSTORE + // Write 0x6f to storage slot 0 + PUSH1(0x6fu64) + PUSH1(0x00u64) + SSTORE + STOP + } + } else { + bytecode! { + // Write 0x6f to storage slot 0 + PUSH1(0x6fu64) + PUSH1(0x00u64) + SSTORE + STOP + } + }; + let expected_prev_value = if !is_warm { 0x6fu64 } else { 0x00u64 }; + + // Get the execution steps from the external tracer + let block: GethData = TestContext::<2, 1>::new( + None, + |accs| { + accs[0] + .address(MOCK_ACCOUNTS[0]) + .balance(Word::from(10u64.pow(19))) + .code(code) + .storage(vec![(0x00u64.into(), 0x6fu64.into())].into_iter()); + accs[1] + .address(MOCK_ACCOUNTS[1]) + .balance(Word::from(10u64.pow(19))); + }, + 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() + .rev() // find last sstore + .find(|step| step.exec_state == ExecState::Op(OpcodeId::SSTORE)) + .unwrap(); + + assert_eq!( + [4, 5] + .map(|idx| &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()]) + .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 = &builder.block.container.storage[step.bus_mapping_instance[6].as_usize()]; + assert_eq!( + (storage_op.rw(), storage_op.op()), + ( + RW::WRITE, + &StorageOp::new( + MOCK_ACCOUNTS[0], + Word::from(0x0u32), + Word::from(0x6fu32), + Word::from(expected_prev_value), + 1, + Word::from(0x6fu32), + ) + ) + ); + let refund_op = &builder.block.container.tx_refund[step.bus_mapping_instance[8].as_usize()]; + assert_eq!( + (refund_op.rw(), refund_op.op()), + ( + RW::WRITE, + &TxRefundOp { + tx_id: 1, + value_prev: if is_warm { 0x12c0 } else { 0 }, + value: if is_warm { 0xaf0 } else { 0 } + } + ) + ); + } + + #[test] + fn sstore_opcode_impl_warm() { + test_ok(true) + } + + #[test] + fn sstore_opcode_impl_cold() { + test_ok(false) + } +} 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..d899cebbed 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)) diff --git a/bus-mapping/src/state_db.rs b/bus-mapping/src/state_db.rs index 30c9e6b788..68806d6daa 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` contains 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 4f61229bee..e0d87ba47c 100644 --- a/eth-types/src/evm_types.rs +++ b/eth-types/src/evm_types.rs @@ -53,7 +53,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 { @@ -112,8 +112,9 @@ impl GasCost { pub const SSTORE_SET: Self = Self(20000); /// Constant cost for a storage reset pub const SSTORE_RESET: Self = Self(2900); - /// Constant cost for a storage clear - pub const SSTORE_CLEARS_SCHEDULE: Self = Self(15000); + /// Constant cost for a storage clear. EIP-3529 changed it to 4800 from + /// 15000. + pub const SSTORE_CLEARS_SCHEDULE: Self = Self(4800); /// Constant cost for a non-creation transaction pub const TX: Self = Self(21000); /// Constant cost for a creation transaction diff --git a/eth-types/src/geth_types.rs b/eth-types/src/geth_types.rs index 1c1d573849..765de3e9ce 100644 --- a/eth-types/src/geth_types.rs +++ b/eth-types/src/geth_types.rs @@ -1,7 +1,9 @@ //! Types needed for generating Ethereum traces -use crate::{AccessList, Address, Block, Bytes, Error, GethExecTrace, Word, U64}; -use serde::Serialize; +use crate::{ + AccessList, Address, Block, Bytes, Error, GethExecTrace, Hash, ToBigEndian, Word, U64, +}; +use serde::{Serialize, Serializer}; use std::collections::HashMap; /// Definition of all of the data related to an account. @@ -16,9 +18,21 @@ pub struct Account { /// EVM Code pub code: Bytes, /// Storage + #[serde(serialize_with = "serde_account_storage")] pub storage: HashMap, } +fn serde_account_storage( + to_serialize: &HashMap, + serializer: S, +) -> Result { + to_serialize + .iter() + .map(|(k, v)| (Hash::from(k.to_be_bytes()), Hash::from(v.to_be_bytes()))) + .collect::>() + .serialize(serializer) +} + /// Definition of all of the constants related to an Ethereum block and /// chain to be used as setup for the external tracer. #[derive(Debug, Default, Clone, PartialEq, Eq, Serialize)] 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/external-tracer/src/lib.rs b/external-tracer/src/lib.rs index 8a97c5da18..bcfac8fa8d 100644 --- a/external-tracer/src/lib.rs +++ b/external-tracer/src/lib.rs @@ -26,7 +26,7 @@ pub struct TraceConfig { /// Creates a trace for the specified config pub fn trace(config: &TraceConfig) -> Result, Error> { // Get the trace - let trace_string = geth_utils::trace(&serde_json::to_string(config).unwrap()).map_err( + let trace_string = geth_utils::trace(&serde_json::to_string(&config).unwrap()).map_err( |error| match error { geth_utils::Error::TracingError(error) => Error::TracingError(error), }, 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/integration-tests/docker-compose.yml b/integration-tests/docker-compose.yml index b439cedc55..6c42b848ef 100644 --- a/integration-tests/docker-compose.yml +++ b/integration-tests/docker-compose.yml @@ -1,7 +1,10 @@ version: '3' services: geth0: - image: "ethereum/client-go:stable" + # The reason to use nightly image here is to include `refund` in StructLogRes. + # Related commit [internal/ethapi: add refund to StructLogRes](https://github.com/ethereum/go-ethereum/commit/b5a129ea248f259a367d804cdf396ce442109d85) has not been included in stable release. + # TODO: change image back to stable when Geth v1.10.18 is released and the commit included. + image: "ethereum/client-go:latest" container_name: zkevm-geth0 ports: - 8545:8545 diff --git a/zkevm-circuits/Cargo.toml b/zkevm-circuits/Cargo.toml index 90ed6faeb4..b3c7748d8a 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" } itertools = "0.10.1" diff --git a/zkevm-circuits/src/evm_circuit.rs b/zkevm-circuits/src/evm_circuit.rs index 107f5ddf4d..66fd28a35e 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 10651a6c7e..94497bb29f 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -514,15 +514,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 + ); } } }; @@ -607,6 +614,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/sload.rs b/zkevm-circuits/src/evm_circuit/execution/sload.rs index f9ef47830c..9f43a78bf9 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sload.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sload.rs @@ -176,244 +176,71 @@ impl SloadGasGadget { #[cfg(test)] mod test { - use crate::evm_circuit::{ - param::STACK_CAPACITY, - step::ExecutionState, - table::{CallContextFieldTag, RwTableTag}, - test::{rand_fp, rand_word, run_test_circuit_incomplete_fixed_table}, - witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, - }; - use bus_mapping::evm::OpcodeId; - use eth_types::{address, bytecode, evm_types::GasCost, ToWord, Word}; - use std::convert::TryInto; - - fn test_ok( - tx: eth_types::Transaction, - key: Word, - value: Word, - is_warm: bool, - is_persistent: bool, - ) { - let rw_counter_end_of_reversion = if is_persistent { 0 } else { 19 }; + use crate::{ + evm_circuit::test::rand_word, + test_util::{run_test_circuits, BytecodeTestConfig}, + }; - let call_data_gas_cost = tx - .input - .0 - .iter() - .fold(0, |acc, byte| acc + if *byte == 0 { 4 } else { 16 }); + use eth_types::{bytecode, Word}; + use mock::{test_ctx::helpers::tx_from_1_to_0, TestContext, MOCK_ACCOUNTS}; - let randomness = rand_fp(); - let bytecode = Bytecode::from(&bytecode! { + fn test_ok(key: Word, value: Word) { + // Here we use two bytecodes to test both is_persistent(STOP) or not(REVERT) + // Besides, in bytecode we use two SLOADs, + // the first SLOAD is used to test cold, and the second is used to test warm + let bytecode_success = bytecode! { + PUSH32(key) + SLOAD PUSH32(key) - #[start] SLOAD STOP - }); - let block = Block { - randomness, - txs: vec![Transaction { - id: 1, - nonce: tx.nonce.try_into().unwrap(), - gas: tx.gas.try_into().unwrap(), - gas_price: tx.gas_price.unwrap_or_else(Word::zero), - caller_address: tx.from, - callee_address: tx.to.unwrap(), - is_create: tx.to.is_none(), - value: tx.value, - call_data: tx.input.to_vec(), - call_data_length: tx.input.0.len(), - call_data_gas_cost, - calls: vec![Call { - id: 1, - is_root: true, - is_create: false, - code_source: CodeSource::Account(bytecode.hash), - rw_counter_end_of_reversion, - is_persistent, - is_success: is_persistent, - callee_address: tx.to.unwrap(), - ..Default::default() - }], - steps: vec![ - ExecStep { - rw_indices: [ - vec![ - (RwTableTag::CallContext, 0), - (RwTableTag::CallContext, 1), - (RwTableTag::CallContext, 2), - (RwTableTag::CallContext, 3), - (RwTableTag::Stack, 0), - (RwTableTag::AccountStorage, 0), - (RwTableTag::Stack, 1), - (RwTableTag::TxAccessListAccountStorage, 0), - ], - if is_persistent { - vec![] - } else { - vec![(RwTableTag::TxAccessListAccountStorage, 1)] - }, - ] - .concat(), - execution_state: ExecutionState::SLOAD, - rw_counter: 9, - program_counter: 33, - stack_pointer: STACK_CAPACITY, - gas_left: if is_warm { - GasCost::WARM_ACCESS.as_u64() - } else { - GasCost::COLD_SLOAD.as_u64() - }, - gas_cost: if is_warm { - GasCost::WARM_ACCESS.as_u64() - } else { - GasCost::COLD_SLOAD.as_u64() - }, - opcode: Some(OpcodeId::SLOAD), - ..Default::default() - }, - ExecStep { - execution_state: ExecutionState::STOP, - rw_counter: 17, - program_counter: 34, - stack_pointer: STACK_CAPACITY, - gas_left: 0, - opcode: Some(OpcodeId::STOP), - reversible_write_counter: 1, - ..Default::default() - }, - ], - }], - rws: RwMap( - [ - ( - RwTableTag::Stack, - vec![ - Rw::Stack { - rw_counter: 13, - is_write: false, - call_id: 1, - stack_pointer: STACK_CAPACITY, - value: key, - }, - Rw::Stack { - rw_counter: 15, - is_write: true, - call_id: 1, - stack_pointer: STACK_CAPACITY, - value, - }, - ], - ), - ( - RwTableTag::AccountStorage, - vec![Rw::AccountStorage { - rw_counter: 14, - is_write: false, - account_address: tx.to.unwrap(), - storage_key: key, - value, - value_prev: value, - tx_id: 1, - committed_value: Word::zero(), - }], - ), - ( - RwTableTag::TxAccessListAccountStorage, - [ - vec![Rw::TxAccessListAccountStorage { - rw_counter: 16, - is_write: true, - tx_id: 1, - account_address: tx.to.unwrap(), - storage_key: key, - value: true, - value_prev: is_warm, - }], - if is_persistent { - vec![] - } else { - vec![Rw::TxAccessListAccountStorage { - rw_counter: rw_counter_end_of_reversion, - is_write: true, - tx_id: 1usize, - account_address: tx.to.unwrap(), - storage_key: key, - value: is_warm, - value_prev: true, - }] - }, - ] - .concat(), - ), - ( - RwTableTag::CallContext, - vec![ - Rw::CallContext { - rw_counter: 9, - is_write: false, - call_id: 1, - field_tag: CallContextFieldTag::TxId, - value: Word::one(), - }, - Rw::CallContext { - rw_counter: 10, - is_write: false, - call_id: 1, - field_tag: CallContextFieldTag::RwCounterEndOfReversion, - value: Word::from(rw_counter_end_of_reversion), - }, - Rw::CallContext { - rw_counter: 11, - is_write: false, - call_id: 1, - field_tag: CallContextFieldTag::IsPersistent, - value: Word::from(is_persistent as u64), - }, - Rw::CallContext { - rw_counter: 12, - is_write: false, - call_id: 1, - field_tag: CallContextFieldTag::CalleeAddress, - value: tx.to.unwrap().to_word(), - }, - ], - ), - ] - .into(), - ), - bytecodes: vec![bytecode], - ..Default::default() }; - - assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); - } - - fn mock_tx() -> eth_types::Transaction { - let from = address!("0x00000000000000000000000000000000000000fe"); - let to = address!("0x00000000000000000000000000000000000000ff"); - eth_types::Transaction { - from, - to: Some(to), - ..Default::default() + let bytecode_failure = bytecode! { + PUSH32(key) + SLOAD + PUSH32(key) + SLOAD + PUSH32(0) + PUSH32(0) + REVERT + }; + for bytecode in [bytecode_success, bytecode_failure] { + let ctx = TestContext::<2, 1>::new( + None, + |accs| { + accs[0] + .address(MOCK_ACCOUNTS[0]) + .balance(Word::from(10u64.pow(19))) + .code(bytecode) + .storage(vec![(key, value)].into_iter()); + accs[1] + .address(MOCK_ACCOUNTS[1]) + .balance(Word::from(10u64.pow(19))); + }, + tx_from_1_to_0, + |block, _txs| block, + ) + .unwrap(); + let test_config = BytecodeTestConfig { + enable_state_circuit_test: false, + ..Default::default() + }; + assert_eq!(run_test_circuits(ctx, Some(test_config),), Ok(())); } } #[test] fn sload_gadget_simple() { - test_ok(mock_tx(), 0x030201.into(), 0x060504.into(), true, true); - test_ok(mock_tx(), 0x030201.into(), 0x060504.into(), true, false); - test_ok(mock_tx(), 0x030201.into(), 0x060504.into(), false, true); - test_ok(mock_tx(), 0x030201.into(), 0x060504.into(), false, false); + let key = 0x030201.into(); + let value = 0x060504.into(); + test_ok(key, value); } #[test] fn sload_gadget_rand() { let key = rand_word(); let value = rand_word(); - test_ok(mock_tx(), key, value, true, true); - test_ok(mock_tx(), key, value, true, false); - test_ok(mock_tx(), key, value, false, true); - test_ok(mock_tx(), key, value, false, false); + test_ok(key, value); } } diff --git a/zkevm-circuits/src/evm_circuit/execution/sstore.rs b/zkevm-circuits/src/evm_circuit/execution/sstore.rs index 4772e2f20d..3a8b760381 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sstore.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sstore.rs @@ -15,6 +15,7 @@ use crate::{ }, util::Expr, }; + use eth_types::{evm_types::GasCost, Field, ToLittleEndian, ToScalar}; use halo2_proofs::{ circuit::Region, @@ -30,7 +31,7 @@ pub(crate) struct SstoreGadget { key: Cell, value: Cell, value_prev: Cell, - committed_value: Cell, + original_value: Cell, is_warm: Cell, tx_refund_prev: Cell, gas_cost: SstoreGasGadget, @@ -58,14 +59,14 @@ impl ExecutionGadget for SstoreGadget { cb.stack_pop(value.expr()); let value_prev = cb.query_cell(); - let committed_value = cb.query_cell(); + let original_value = cb.query_cell(); cb.account_storage_write( callee_address.expr(), key.expr(), value.expr(), value_prev.expr(), tx_id.expr(), - committed_value.expr(), + original_value.expr(), Some(&mut reversion_info), ); @@ -83,7 +84,7 @@ impl ExecutionGadget for SstoreGadget { cb, value.clone(), value_prev.clone(), - committed_value.clone(), + original_value.clone(), is_warm.clone(), ); @@ -93,7 +94,7 @@ impl ExecutionGadget for SstoreGadget { tx_refund_prev.clone(), value.clone(), value_prev.clone(), - committed_value.clone(), + original_value.clone(), ); cb.tx_refund_write( tx_id.expr(), @@ -120,7 +121,7 @@ impl ExecutionGadget for SstoreGadget { key, value, value_prev, - committed_value, + original_value, is_warm, tx_refund_prev, gas_cost, @@ -169,7 +170,7 @@ impl ExecutionGadget for SstoreGadget { )), )?; - let (_, value_prev, _, committed_value) = block.rws[step.rw_indices[6]].storage_value_aux(); + let (_, value_prev, _, original_value) = block.rws[step.rw_indices[6]].storage_value_aux(); self.value_prev.assign( region, offset, @@ -178,11 +179,11 @@ impl ExecutionGadget for SstoreGadget { block.randomness, )), )?; - self.committed_value.assign( + self.original_value.assign( region, offset, Some(Word::random_linear_combine( - committed_value.to_le_bytes(), + original_value.to_le_bytes(), block.randomness, )), )?; @@ -191,16 +192,17 @@ impl ExecutionGadget for SstoreGadget { self.is_warm .assign(region, offset, Some(F::from(is_warm as u64)))?; - let (_, tx_refund_prev) = block.rws[step.rw_indices[8]].tx_refund_value_pair(); + let (tx_refund, tx_refund_prev) = block.rws[step.rw_indices[8]].tx_refund_value_pair(); self.tx_refund_prev .assign(region, offset, Some(F::from(tx_refund_prev)))?; self.gas_cost.assign( region, offset, + step.gas_cost, value, value_prev, - committed_value, + original_value, is_warm, block.randomness, )?; @@ -208,13 +210,13 @@ impl ExecutionGadget for SstoreGadget { self.tx_refund.assign( region, offset, + tx_refund, tx_refund_prev, value, value_prev, - committed_value, + original_value, block.randomness, )?; - Ok(()) } } @@ -223,7 +225,7 @@ impl ExecutionGadget for SstoreGadget { pub(crate) struct SstoreGasGadget { value: Cell, value_prev: Cell, - committed_value: Cell, + original_value: Cell, is_warm: Cell, gas_cost: Expression, value_eq_prev: IsEqualGadget, @@ -236,13 +238,13 @@ impl SstoreGasGadget { cb: &mut ConstraintBuilder, value: Cell, value_prev: Cell, - committed_value: Cell, + original_value: Cell, is_warm: Cell, ) -> Self { let value_eq_prev = IsEqualGadget::construct(cb, value.expr(), value_prev.expr()); let original_eq_prev = - IsEqualGadget::construct(cb, committed_value.expr(), value_prev.expr()); - let original_is_zero = IsZeroGadget::construct(cb, committed_value.expr()); + IsEqualGadget::construct(cb, original_value.expr(), value_prev.expr()); + let original_is_zero = IsZeroGadget::construct(cb, original_value.expr()); let warm_case_gas = select::expr( value_eq_prev.expr(), GasCost::WARM_ACCESS.expr(), @@ -265,7 +267,7 @@ impl SstoreGasGadget { Self { value, value_prev, - committed_value, + original_value, is_warm, gas_cost, value_eq_prev, @@ -284,9 +286,10 @@ impl SstoreGasGadget { &self, region: &mut Region<'_, F>, offset: usize, + gas_cost: u64, value: eth_types::Word, value_prev: eth_types::Word, - committed_value: eth_types::Word, + original_value: eth_types::Word, is_warm: bool, randomness: F, ) -> Result<(), Error> { @@ -303,11 +306,11 @@ impl SstoreGasGadget { randomness, )), )?; - self.committed_value.assign( + self.original_value.assign( region, offset, Some(Word::random_linear_combine( - committed_value.to_le_bytes(), + original_value.to_le_bytes(), randomness, )), )?; @@ -322,14 +325,18 @@ impl SstoreGasGadget { self.original_eq_prev.assign( region, offset, - Word::random_linear_combine(committed_value.to_le_bytes(), randomness), + Word::random_linear_combine(original_value.to_le_bytes(), randomness), Word::random_linear_combine(value_prev.to_le_bytes(), randomness), )?; self.original_is_zero.assign( region, offset, - Word::random_linear_combine(committed_value.to_le_bytes(), randomness), + Word::random_linear_combine(original_value.to_le_bytes(), randomness), )?; + debug_assert_eq!( + calc_expected_gas_cost(value, value_prev, original_value, is_warm), + gas_cost + ); Ok(()) } } @@ -340,17 +347,13 @@ pub(crate) struct SstoreTxRefundGadget { tx_refund_new: Expression, 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, + original_value: 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 { @@ -359,80 +362,62 @@ impl SstoreTxRefundGadget { tx_refund_old: Cell, value: Cell, value_prev: Cell, - committed_value: Cell, + original_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 = - 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.expr() - - GasCost::WARM_ACCESS.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.expr() - GasCost::WARM_ACCESS.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_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, original_value.expr()); + + let original_eq_value_gadget = + IsEqualGadget::construct(cb, original_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, original_value.expr(), value_prev.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) && (original_value != value) && (value == + // Word::from(0)) + let delete_slot = + not::expr(prev_eq_value.clone()) * not::expr(original_is_zero.clone()) * value_is_zero; + // (value_prev != value) && (original_value == value) && (original_value != + // Word::from(0)) + let reset_existing = not::expr(prev_eq_value.clone()) + * original_eq_value.clone() + * not::expr(original_is_zero.clone()); + // (value_prev != value) && (original_value == value) && (original_value == + // Word::from(0)) + let reset_inexistent = + not::expr(prev_eq_value.clone()) * original_eq_value * (original_is_zero); + // (value_prev != value) && (original_value != value_prev) && (value_prev == + // Word::from(0)) + let recreate_slot = + not::expr(prev_eq_value) * not::expr(original_eq_prev) * (value_prev_is_zero); + + let tx_refund_new = tx_refund_old.expr() + + delete_slot * GasCost::SSTORE_CLEARS_SCHEDULE.expr() + + reset_existing * (GasCost::SSTORE_RESET.expr() - GasCost::WARM_ACCESS.expr()) + + reset_inexistent * (GasCost::SSTORE_SET.expr() - GasCost::WARM_ACCESS.expr()) + - recreate_slot * (GasCost::SSTORE_CLEARS_SCHEDULE.expr()); Self { value, value_prev, - committed_value, + original_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, } } @@ -446,10 +431,11 @@ impl SstoreTxRefundGadget { &self, region: &mut Region<'_, F>, offset: usize, + tx_refund: u64, tx_refund_old: u64, value: eth_types::Word, value_prev: eth_types::Word, - committed_value: eth_types::Word, + original_value: eth_types::Word, randomness: F, ) -> Result<(), Error> { self.tx_refund_old @@ -467,636 +453,248 @@ impl SstoreTxRefundGadget { randomness, )), )?; - self.committed_value.assign( + self.original_value.assign( region, offset, Some(Word::random_linear_combine( - committed_value.to_le_bytes(), + original_value.to_le_bytes(), 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), + Word::random_linear_combine(original_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(original_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(original_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.as_u64() - GasCost::WARM_ACCESS.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.as_u64() - GasCost::WARM_ACCESS.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)))?; - + debug_assert_eq!( + calc_expected_tx_refund(tx_refund_old, value, value_prev, original_value), + tx_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 bus_mapping::evm::OpcodeId; - use eth_types::{address, bytecode, evm_types::GasCost, ToWord, Word}; - use std::convert::TryInto; - - fn calc_expected_gas_cost( - value: Word, - value_prev: Word, - committed_value: Word, - is_warm: bool, - ) -> u64 { - let warm_case_gas = if value_prev == value { - GasCost::WARM_ACCESS - } else if committed_value == value_prev { - if committed_value == Word::from(0) { - GasCost::SSTORE_SET - } else { - GasCost::SSTORE_RESET - } +fn calc_expected_gas_cost( + value: eth_types::Word, + value_prev: eth_types::Word, + original_value: eth_types::Word, + is_warm: bool, +) -> u64 { + let warm_case_gas = if value_prev == value { + GasCost::WARM_ACCESS + } else if original_value == value_prev { + if original_value == eth_types::Word::from(0) { + GasCost::SSTORE_SET } else { - GasCost::WARM_ACCESS - }; - if is_warm { - warm_case_gas.as_u64() - } else { - warm_case_gas.as_u64() + GasCost::COLD_SLOAD.as_u64() + GasCost::SSTORE_RESET } + } else { + GasCost::WARM_ACCESS + }; + if is_warm { + warm_case_gas.as_u64() + } else { + warm_case_gas.as_u64() + GasCost::COLD_SLOAD.as_u64() } +} - fn calc_expected_tx_refund( - tx_refund_old: u64, - value: Word, - value_prev: Word, - committed_value: Word, - ) -> u64 { - let mut tx_refund_new = tx_refund_old; +fn calc_expected_tx_refund( + tx_refund_old: u64, + value: eth_types::Word, + value_prev: eth_types::Word, + original_value: eth_types::Word, +) -> u64 { + // Same clause tags(like "delete slot (2.1.2b)") used as [`makeGasSStoreFunc` in go-ethereum](https://github.com/ethereum/go-ethereum/blob/9fd8825d5a196edde6d8ef81382979875145b346/core/vm/operations_acl.go#L27) + // Control flow of this function try to follow `makeGasSStoreFunc` for better + // understanding and comparison. + + let mut tx_refund_new = tx_refund_old; + + // The "clearing slot refund" and "resetting value refund" are ADDED together, + // they are NOT MUTUALLY EXCLUSIVE. + // Search "Apply both of the following clauses" in EIP-2200 for more details. + // There can be five total kinds of refund: + // 1. -SSTORE_CLEARS_SCHEDULE + // 2. SSTORE_CLEARS_SCHEDULE + // 3. SSTORE_SET - WARM_ACCESS + // 4. SSTORE_RESET - WARM_ACCESS + // 5. -SSTORE_CLEARS_SCHEDULE + SSTORE_RESET - WARM_ACCESS + // The last case can happen if (original_value, prev_value, value) be (v,0,v) + // where v != 0, + // then both "clearing slot refund" and "resetting value refund" are non zero. + + if value_prev != value { + // refund related to clearing slot + // "delete slot (2.1.2b)" can be safely merged in "delete slot (2.2.1.2)" + if !original_value.is_zero() { + if value_prev.is_zero() { + // recreate slot (2.2.1.1) + tx_refund_new -= GasCost::SSTORE_CLEARS_SCHEDULE.as_u64() + } + if value.is_zero() { + // delete slot (2.2.1.2) + tx_refund_new += GasCost::SSTORE_CLEARS_SCHEDULE.as_u64() + } + } - 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(); - } + // refund related to resetting value + if original_value == value { + if original_value.is_zero() { + // reset to original inexistent slot (2.2.2.1) + tx_refund_new += GasCost::SSTORE_SET.as_u64() - GasCost::WARM_ACCESS.as_u64(); } else { - 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.as_u64() - GasCost::WARM_ACCESS.as_u64(); - } else { - tx_refund_new += - GasCost::SSTORE_RESET.as_u64() - GasCost::WARM_ACCESS.as_u64(); - } - } + // reset to original existing slot (2.2.2.2) + tx_refund_new += GasCost::SSTORE_RESET.as_u64() - GasCost::WARM_ACCESS.as_u64(); } } - - tx_refund_new } - fn test_ok( - tx: eth_types::Transaction, - key: Word, - value: Word, - value_prev: Word, - committed_value: Word, - is_warm: bool, - result: bool, - ) { - let gas = calc_expected_gas_cost(value, value_prev, committed_value, is_warm); - let tx_refund_old = GasCost::SSTORE_SET.as_u64(); - let tx_refund_new = - calc_expected_tx_refund(tx_refund_old, value, value_prev, committed_value); - let rw_counter_end_of_reversion = if result { 0 } else { 14 }; + tx_refund_new +} - let call_data_gas_cost = tx - .input - .0 - .iter() - .fold(0, |acc, byte| acc + if *byte == 0 { 4 } else { 16 }); +#[cfg(test)] +mod test { - let randomness = rand_fp(); - let bytecode = Bytecode::from(&bytecode! { - PUSH32(value) - PUSH32(key) - #[start] - SSTORE - STOP - }); - let block = Block { - randomness, - txs: vec![Transaction { - id: 1, - nonce: tx.nonce.try_into().unwrap(), - gas: tx.gas.try_into().unwrap(), - gas_price: tx.gas_price.unwrap_or_else(Word::zero), - caller_address: tx.from, - callee_address: tx.to.unwrap(), - is_create: tx.to.is_none(), - value: tx.value, - call_data: tx.input.to_vec(), - call_data_length: tx.input.0.len(), - call_data_gas_cost, - calls: vec![Call { - id: 1, - is_root: true, - is_create: false, - code_source: CodeSource::Account(bytecode.hash), - rw_counter_end_of_reversion, - is_persistent: result, - is_success: result, - callee_address: tx.to.unwrap(), - ..Default::default() - }], - steps: vec![ - ExecStep { - rw_indices: [ - vec![ - (RwTableTag::CallContext, 0), - (RwTableTag::CallContext, 1), - (RwTableTag::CallContext, 2), - (RwTableTag::CallContext, 3), - (RwTableTag::Stack, 0), - (RwTableTag::Stack, 1), - (RwTableTag::AccountStorage, 0), - (RwTableTag::TxAccessListAccountStorage, 0), - (RwTableTag::TxRefund, 0), - ], - if result { - vec![] - } else { - vec![ - (RwTableTag::TxRefund, 1), - (RwTableTag::TxAccessListAccountStorage, 1), - (RwTableTag::AccountStorage, 1), - ] - }, - ] - .concat(), - execution_state: ExecutionState::SSTORE, - rw_counter: 1, - program_counter: 66, - stack_pointer: STACK_CAPACITY - 2, - gas_left: gas, - gas_cost: gas, - opcode: Some(OpcodeId::SSTORE), - ..Default::default() - }, - ExecStep { - execution_state: ExecutionState::STOP, - rw_counter: 10, - program_counter: 67, - stack_pointer: STACK_CAPACITY, - gas_left: 0, - opcode: Some(OpcodeId::STOP), - reversible_write_counter: 3, - ..Default::default() - }, - ], - }], - rws: RwMap( - [ - ( - RwTableTag::Stack, - vec![ - Rw::Stack { - rw_counter: 5, - is_write: false, - call_id: 1, - stack_pointer: STACK_CAPACITY - 2, - value: key, - }, - Rw::Stack { - rw_counter: 6, - is_write: false, - call_id: 1, - stack_pointer: STACK_CAPACITY - 1, - value, - }, - ], - ), - ( - RwTableTag::AccountStorage, - [ - vec![Rw::AccountStorage { - rw_counter: 7, - is_write: true, - account_address: tx.to.unwrap(), - storage_key: key, - value, - value_prev, - tx_id: 1usize, - committed_value, - }], - if result { - vec![] - } else { - vec![Rw::AccountStorage { - rw_counter: rw_counter_end_of_reversion, - is_write: true, - account_address: tx.to.unwrap(), - storage_key: key, - value: value_prev, - value_prev: value, - tx_id: 1usize, - committed_value, - }] - }, - ] - .concat(), - ), - ( - RwTableTag::TxAccessListAccountStorage, - [ - vec![Rw::TxAccessListAccountStorage { - rw_counter: 8, - is_write: true, - tx_id: 1usize, - account_address: tx.to.unwrap(), - storage_key: key, - value: true, - value_prev: is_warm, - }], - if result { - vec![] - } else { - vec![Rw::TxAccessListAccountStorage { - rw_counter: rw_counter_end_of_reversion - 1, - is_write: true, - tx_id: 1usize, - account_address: tx.to.unwrap(), - storage_key: key, - value: is_warm, - value_prev: true, - }] - }, - ] - .concat(), - ), - ( - RwTableTag::TxRefund, - [ - vec![Rw::TxRefund { - rw_counter: 9, - is_write: true, - tx_id: 1usize, - value: tx_refund_new, - value_prev: tx_refund_old, - }], - if result { - vec![] - } else { - vec![Rw::TxRefund { - rw_counter: rw_counter_end_of_reversion - 2, - is_write: true, - tx_id: 1usize, - value: tx_refund_old, - value_prev: tx_refund_new, - }] - }, - ] - .concat(), - ), - ( - RwTableTag::CallContext, - vec![ - Rw::CallContext { - rw_counter: 1, - is_write: false, - call_id: 1, - field_tag: CallContextFieldTag::TxId, - value: Word::one(), - }, - Rw::CallContext { - rw_counter: 2, - is_write: false, - call_id: 1, - field_tag: CallContextFieldTag::RwCounterEndOfReversion, - value: Word::from(rw_counter_end_of_reversion), - }, - Rw::CallContext { - rw_counter: 3, - is_write: false, - call_id: 1, - field_tag: CallContextFieldTag::IsPersistent, - value: Word::from(result as u64), - }, - Rw::CallContext { - rw_counter: 4, - is_write: false, - call_id: 1, - field_tag: CallContextFieldTag::CalleeAddress, - value: tx.to.unwrap().to_word(), - }, - ], - ), - ] - .into(), - ), - bytecodes: vec![bytecode], - ..Default::default() - }; + use crate::test_util::{run_test_circuits, BytecodeTestConfig}; - assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); - } - - fn mock_tx() -> eth_types::Transaction { - let from = address!("0x00000000000000000000000000000000000000fe"); - let to = address!("0x00000000000000000000000000000000000000ff"); - eth_types::Transaction { - from, - to: Some(to), - ..Default::default() - } - } + use eth_types::{bytecode, Word}; + use mock::{test_ctx::helpers::tx_from_1_to_0, TestContext, MOCK_ACCOUNTS}; #[test] - fn sstore_gadget_warm_persist() { + fn sstore_gadget_no_refund() { // value_prev == value test_ok( - mock_tx(), 0x030201.into(), 0x060504.into(), 0x060504.into(), 0x060504.into(), - true, - true, - ); - // value_prev != value, original_value == value_prev, original_value != 0 - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0x060505.into(), - 0x060505.into(), - true, - true, - ); - // value_prev != value, original_value == value_prev, original_value == 0 - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0.into(), - 0.into(), - true, - true, - ); - // value_prev != value, original_value != value_prev, value != original_value - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0x060505.into(), - 0x060506.into(), - true, - true, - ); - // value_prev != value, original_value != value_prev, value == original_value - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0x060505.into(), - 0x060504.into(), - true, - true, ); } - fn sstore_gadget_warm_revert() { - // value_prev == value - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0x060504.into(), - 0x060504.into(), - true, - false, - ); - // value_prev != value, original_value == value_prev, original_value != 0 - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0x060505.into(), - 0x060505.into(), - true, - false, - ); - // value_prev != value, original_value == value_prev, original_value == 0 - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0.into(), - 0.into(), - true, - false, - ); - // value_prev != value, original_value != value_prev, value != original_value + #[test] + fn sstore_gadget_delete_slot() { + // value_prev != value, original_value != value, value == 0 test_ok( - mock_tx(), 0x030201.into(), - 0x060504.into(), + 0x0.into(), 0x060505.into(), 0x060506.into(), - true, - false, - ); - // value_prev != value, original_value != value_prev, value == original_value - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0x060505.into(), - 0x060504.into(), - true, - false, ); } #[test] - fn sstore_gadget_cold_persist() { - // value_prev == value - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0x060504.into(), - 0x060504.into(), - false, - true, - ); - // value_prev != value, original_value == value_prev, original_value != 0 - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0x060505.into(), - 0x060505.into(), - false, - true, - ); - // value_prev != value, original_value == value_prev, original_value == 0 - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0.into(), - 0.into(), - false, - true, - ); - // value_prev != value, original_value != value_prev, value != original_value - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0x060505.into(), - 0x060506.into(), - false, - true, - ); - // value_prev != value, original_value != value_prev, value == original_value + fn sstore_gadget_reset_existing() { + // value_prev != value, original_value == value, original_value != 0 test_ok( - mock_tx(), 0x030201.into(), 0x060504.into(), 0x060505.into(), 0x060504.into(), - false, - true, ); } + #[test] + fn sstore_gadget_reset_inexistent() { + // value_prev != value, original_value == value, original_value == 0 + test_ok(0x030201.into(), 0.into(), 0x060505.into(), 0.into()); + } #[test] - fn sstore_gadget_cold_revert() { - // value_prev == value + fn sstore_gadget_recreate_slot() { + // value_prev != value, original_value != value_prev, original_value != value, + // value_prev == 0 test_ok( - mock_tx(), 0x030201.into(), 0x060504.into(), - 0x060504.into(), - 0x060504.into(), - false, - false, - ); - // value_prev != value, original_value == value_prev, original_value != 0 - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0x060505.into(), - 0x060505.into(), - false, - false, - ); - // value_prev != value, original_value == value_prev, original_value == 0 - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0.into(), - 0.into(), - false, - false, - ); - // value_prev != value, original_value != value_prev, value != original_value - test_ok( - mock_tx(), - 0x030201.into(), - 0x060504.into(), - 0x060505.into(), + 0x0.into(), 0x060506.into(), - false, - false, ); - // value_prev != value, original_value != value_prev, value == original_value + } + #[test] + fn sstore_gadget_recreate_slot_and_reset_inexistent() { + // value_prev != value, original_value != value_prev, original_value == value, + // value_prev == 0 test_ok( - mock_tx(), 0x030201.into(), 0x060504.into(), - 0x060505.into(), + 0x0.into(), 0x060504.into(), - false, - false, ); } + + fn test_ok(key: Word, value: Word, value_prev: Word, original_value: Word) { + // Here we use two bytecodes to test both is_persistent(STOP) or not(REVERT) + // Besides, in bytecode we use two SSTOREs, + // the first SSTORE is used to test cold, and the second is used to test warm + let bytecode_success = bytecode! { + PUSH32(value_prev) + PUSH32(key) + SSTORE + PUSH32(value) + PUSH32(key) + SSTORE + STOP + }; + let bytecode_failure = bytecode! { + PUSH32(value_prev) + PUSH32(key) + SSTORE + PUSH32(value) + PUSH32(key) + SSTORE + PUSH32(0) + PUSH32(0) + REVERT + }; + for bytecode in [bytecode_success, bytecode_failure] { + let ctx = TestContext::<2, 1>::new( + None, + |accs| { + accs[0] + .address(MOCK_ACCOUNTS[0]) + .balance(Word::from(10u64.pow(19))) + .code(bytecode) + .storage(vec![(key, original_value)].into_iter()); + accs[1] + .address(MOCK_ACCOUNTS[1]) + .balance(Word::from(10u64.pow(19))); + }, + tx_from_1_to_0, + |block, _txs| block, + ) + .unwrap(); + let test_config = BytecodeTestConfig { + enable_state_circuit_test: false, + ..Default::default() + }; + assert_eq!(run_test_circuits(ctx, Some(test_config),), Ok(())); + } + } } diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 0153506161..ab08678a2c 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}, + plonk::{ + Error, + Expression::{self, Constant}, + }, }; use std::convert::TryInto; @@ -652,8 +655,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 reversible_write( diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index c97dad9d5f..662d87907f 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::Builder::from_env(env_logger::Env::default().default_filter_or("error")).init(); +} pub enum FixedTableConfig { Incomplete, Complete,