diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index fef369afb6..997cba5f7e 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -115,7 +115,6 @@ use extcodecopy::Extcodecopy; use extcodehash::Extcodehash; use extcodesize::Extcodesize; use gasprice::GasPrice; -// use invalid_tx::InvalidTx; use logs::Log; use mload::Mload; use mstore::Mstore; diff --git a/bus-mapping/src/evm/opcodes/tstore.rs b/bus-mapping/src/evm/opcodes/tstore.rs index f0d37d15b2..69b541d407 100644 --- a/bus-mapping/src/evm/opcodes/tstore.rs +++ b/bus-mapping/src/evm/opcodes/tstore.rs @@ -56,11 +56,6 @@ impl Opcode for Tstore { 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); - let key = state.stack_pop(&mut exec_step)?; let value = state.stack_pop(&mut exec_step)?; #[cfg(feature = "enable-stack")] @@ -69,9 +64,6 @@ impl Opcode for Tstore { assert_eq!(value, geth_step.stack.nth_last(1)?); } - // state.stack_read(&mut exec_step, key_stack_position, key)?; - // state.stack_read(&mut exec_step, value_stack_position, value)?; - let (_, value_prev) = state.sdb.get_transient_storage(&contract_addr, &key); let value_prev = *value_prev; @@ -110,7 +102,7 @@ mod tstore_tests { #[test] fn tstore_opcode() { let code = bytecode! { - // Write 0x6f to storage slot 0 + // Write 0x6f to transient storage slot 0 PUSH1(0x6fu64) PUSH1(0x00u64) TSTORE diff --git a/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs b/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs index b909276d04..9502c4c266 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_write_protection.rs @@ -172,62 +172,26 @@ mod test { } } - fn caller(opcode: OpcodeId, stack: Stack, caller_is_success: bool) -> Account { - let is_call = opcode == OpcodeId::CALL; - let terminator = if caller_is_success { - OpcodeId::RETURN - } else { - OpcodeId::REVERT - }; - - let mut bytecode = bytecode! { - PUSH32(Word::from(stack.rd_length)) - PUSH32(Word::from(stack.rd_offset)) - PUSH32(Word::from(stack.cd_length)) - PUSH32(Word::from(stack.cd_offset)) - }; - if is_call { - bytecode.push(32, stack.value); - } - bytecode.append(&bytecode! { - PUSH32(Address::repeat_byte(0xff).to_word()) - PUSH32(Word::from(stack.gas)) - .write_op(opcode) - PUSH32(Word::from(stack.rd_length)) - PUSH32(Word::from(stack.rd_offset)) - PUSH32(Word::from(stack.cd_length)) - PUSH32(Word::from(stack.cd_offset)) - }); - if is_call { - bytecode.push(32, stack.value); - } - bytecode.append(&bytecode! { - PUSH32(Address::repeat_byte(0xff).to_word()) - PUSH32(Word::from(stack.gas)) - .write_op(opcode) - PUSH1(0) - PUSH1(0) - .write_op(terminator) - }); - - Account { - address: Address::repeat_byte(0xfe), - balance: Word::from(10).pow(20.into()), - code: bytecode.to_vec().into(), - ..Default::default() - } + #[derive(Copy, Clone, Debug)] + enum FailureReason { + Sstore, + TStore, + CallWithValue, } #[test] fn test_write_protection() { - // test sstore with write protection error - test_internal_write_protection(false); - // test call with write protection error - test_internal_write_protection(true); + for reason in [ + FailureReason::Sstore, + FailureReason::CallWithValue, + FailureReason::TStore, + ] { + test_internal_write_protection(reason) + } } // ErrorWriteProtection error happen in internal call - fn test_internal_write_protection(is_call: bool) { + fn test_internal_write_protection(reason: FailureReason) { let mut caller_bytecode = bytecode! { PUSH1(0) PUSH1(0) @@ -248,26 +212,36 @@ mod test { PUSH1(0x02) }; - if is_call { - callee_bytecode.append(&bytecode! { - PUSH1(0) - PUSH1(0) - PUSH1(10) - PUSH1(200) // non zero value - PUSH20(Address::repeat_byte(0xff).to_word()) - PUSH2(10000) // gas - //this call got error: ErrorWriteProtection - CALL - RETURN - STOP - }); - } else { - callee_bytecode.append(&bytecode! { - // this SSTORE got error: ErrorWriteProtection - SSTORE - STOP - }); - } + match reason { + FailureReason::CallWithValue => { + callee_bytecode.append(&bytecode! { + PUSH1(0) + PUSH1(0) + PUSH1(10) + PUSH1(200) // non zero value + PUSH20(Address::repeat_byte(0xff).to_word()) + PUSH2(10000) // gas + //this call got error: ErrorWriteProtection + CALL + RETURN + STOP + }); + } + FailureReason::Sstore => { + callee_bytecode.append(&bytecode! { + // this SSTORE got error: ErrorWriteProtection + SSTORE + STOP + }); + } + FailureReason::TStore => { + callee_bytecode.append(&bytecode! { + // this TSTORE got error: ErrorWriteProtection + TSTORE + STOP + }); + } + }; test_ok( Account { diff --git a/zkevm-circuits/src/evm_circuit/execution/tstore.rs b/zkevm-circuits/src/evm_circuit/execution/tstore.rs index a7bd457917..a95f830871 100644 --- a/zkevm-circuits/src/evm_circuit/execution/tstore.rs +++ b/zkevm-circuits/src/evm_circuit/execution/tstore.rs @@ -182,4 +182,36 @@ mod test { CircuitTestBuilder::new_from_test_ctx(ctx).run(); } + + #[test] + fn test_revert() { + let key = Word::from(34); + let value = Word::from(100); + + let bytecode = bytecode! { + PUSH32(value) + PUSH32(key) + TSTORE + PUSH32(0) + PUSH32(0) + REVERT + }; + let ctx = TestContext::<2, 1>::new( + None, + |accs| { + accs[0] + .address(MOCK_ACCOUNTS[0]) + .balance(Word::from(10u64.pow(19))) + .code(bytecode); + accs[1] + .address(MOCK_ACCOUNTS[1]) + .balance(Word::from(10u64.pow(19))); + }, + tx_from_1_to_0, + |block, _txs| block, + ) + .unwrap(); + + CircuitTestBuilder::new_from_test_ctx(ctx).run(); + } } diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 2b265c4b11..4967c725d4 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -1116,7 +1116,7 @@ impl<'a, F: Field> EVMConstraintBuilder<'a, F> { self.rw_lookup( "account_transient_storage_read", false.expr(), - RwTableTag::TransientStorage, + RwTableTag::AccountTransientStorage, RwValues::new( tx_id, account_address, @@ -1142,7 +1142,7 @@ impl<'a, F: Field> EVMConstraintBuilder<'a, F> { ) { self.reversible_write( "account_transient_storage_write", - RwTableTag::TransientStorage, + RwTableTag::AccountTransientStorage, RwValues::new( tx_id, account_address, diff --git a/zkevm-circuits/src/table.rs b/zkevm-circuits/src/table.rs index ad35731f5b..b4b246daf0 100644 --- a/zkevm-circuits/src/table.rs +++ b/zkevm-circuits/src/table.rs @@ -471,7 +471,7 @@ pub enum RwTableTag { /// Account Storage operation AccountStorage, /// Account Transient Storage operation - TransientStorage, + AccountTransientStorage, /// Call Context operation CallContext, /// Tx Log operation @@ -491,7 +491,7 @@ impl RwTableTag { | RwTableTag::TxRefund | RwTableTag::Account | RwTableTag::AccountStorage - | RwTableTag::TransientStorage + | RwTableTag::AccountTransientStorage ) } } diff --git a/zkevm-circuits/src/witness/rw.rs b/zkevm-circuits/src/witness/rw.rs index 1f19c1a4af..54927ae75b 100644 --- a/zkevm-circuits/src/witness/rw.rs +++ b/zkevm-circuits/src/witness/rw.rs @@ -600,8 +600,7 @@ impl Rw { Self::Memory { .. } => RwTableTag::Memory, Self::Stack { .. } => RwTableTag::Stack, Self::AccountStorage { .. } => RwTableTag::AccountStorage, - // TODO: pick one! - Self::AccountTransientStorage { .. } => RwTableTag::TransientStorage, + Self::AccountTransientStorage { .. } => RwTableTag::AccountTransientStorage, Self::TxAccessListAccount { .. } => RwTableTag::TxAccessListAccount, Self::TxAccessListAccountStorage { .. } => RwTableTag::TxAccessListAccountStorage, Self::TxRefund { .. } => RwTableTag::TxRefund, @@ -941,7 +940,7 @@ impl From<&operation::OperationContainer> for RwMap { .collect(), ); rws.insert( - RwTableTag::TransientStorage, + RwTableTag::AccountTransientStorage, container .transient_storage .iter() diff --git a/zkevm-circuits/src/witness/step.rs b/zkevm-circuits/src/witness/step.rs index c68ec33a11..ca40377883 100644 --- a/zkevm-circuits/src/witness/step.rs +++ b/zkevm-circuits/src/witness/step.rs @@ -253,7 +253,7 @@ pub(super) fn step_convert(step: &circuit_input_builder::ExecStep, block_num: u6 operation::Target::Memory => RwTableTag::Memory, operation::Target::Stack => RwTableTag::Stack, operation::Target::Storage => RwTableTag::AccountStorage, - operation::Target::TransientStorage => RwTableTag::TransientStorage, + operation::Target::TransientStorage => RwTableTag::AccountTransientStorage, operation::Target::TxAccessListAccount => RwTableTag::TxAccessListAccount, operation::Target::TxAccessListAccountStorage => { RwTableTag::TxAccessListAccountStorage