Fix TStore for reverts and in static calls (#1811)

### Description

The bus mapping TSTORE did not handle the cases where the opcode's
effect is revert and the error that happens if TSTORE is called in a
static call. This adds tests for those two cases and then fixes the bus
mapping.

### Type of change

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update
- [ ] Refactor (no updates to logic)

### How Has This Been Tested?

Added tests that fail before the bus mapping fixes are applied.

```
This PR contains:
- [Add failing revert test](be4b13d)
- [Add failing write protection test for TStore](18845eb)
- [Handle reverts and write protection failure for TSTORE in bus mapping](0105019)
```

---------

Co-authored-by: Mason Liang <[email protected]>

bus-mapping/src/circuit_input_builder/input_state_ref.rs

@@ -935,6 +935,14 @@ impl<'a> CircuitInputStateRef<'a> {
                     None
                 }
             }
+            OperationRef(Target::TransientStorage, idx) => {
+                let operation = &self.block.container.transient_storage[*idx];
+                if operation.rw().is_write() && operation.reversible() {
+                    Some(OpEnum::TransientStorage(operation.op().reverse()))
+                } else {
+                    None
+                }
+            }
             OperationRef(Target::TxAccessListAccount, idx) => {
                 let operation = &self.block.container.tx_access_list_account[*idx];
                 if operation.rw().is_write() && operation.reversible() {
@@ -1436,6 +1444,7 @@ impl<'a> CircuitInputStateRef<'a> {
                     OpcodeId::RETURNDATACOPY => Some(ExecError::ReturnDataOutOfBounds),
                     // Break write protection (CALL with value will be handled below)
                     OpcodeId::SSTORE
+                    | OpcodeId::TSTORE
                     | OpcodeId::CREATE
                     | OpcodeId::CREATE2
                     | OpcodeId::SELFDESTRUCT

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

@@ -31,6 +31,7 @@ impl Opcode for ErrorWriteProtection {
         // assert op code can only be following codes
         assert!([
             OpcodeId::SSTORE,
+            OpcodeId::TSTORE,
             OpcodeId::CREATE,
             OpcodeId::CREATE2,
             OpcodeId::CALL,

bus-mapping/src/operation/container.rs

@@ -124,8 +124,11 @@ impl OperationContainer {
                 OperationRef::from((Target::Storage, self.storage.len() - 1))
             }
             OpEnum::TransientStorage(op) => {
-                self.transient_storage
-                    .push(Operation::new(rwc, rwc_inner_chunk, rw, op));
+                self.transient_storage.push(if reversible {
+                    Operation::new_reversible(rwc, rwc_inner_chunk, rw, op)
+                } else {
+                    Operation::new(rwc, rwc_inner_chunk, rw, op)
+                });
                 OperationRef::from((Target::TransientStorage, self.transient_storage.len() - 1))
             }
             OpEnum::TxAccessListAccount(op) => {

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

@@ -43,7 +43,7 @@ impl<F: Field> ExecutionGadget<F> for ErrorWriteProtectionGadget<F> {
         let value = cb.query_word_unchecked();
         let is_value_zero = cb.is_zero_word(&value);
 
-        // require_in_set method will spilit into more low degree expressions if exceed
+        // require_in_set method will split into more low degree expressions if exceed
         // max_degree. otherwise need to do fixed lookup for these opcodes
         // checking.
         cb.require_in_set(
@@ -52,6 +52,7 @@ impl<F: Field> ExecutionGadget<F> for ErrorWriteProtectionGadget<F> {
             vec![
                 OpcodeId::CALL.expr(),
                 OpcodeId::SSTORE.expr(),
+                OpcodeId::TSTORE.expr(),
                 OpcodeId::CREATE.expr(),
                 OpcodeId::CREATE2.expr(),
                 OpcodeId::SELFDESTRUCT.expr(),
@@ -155,16 +156,26 @@ mod test {
         Account::mock_code_balance(code)
     }
 
+    #[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)
@@ -185,26 +196,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::mock_100_ether(caller_bytecode),

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

@@ -178,4 +178,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();
+    }
 }