From 670e060a5dd7c082f54406d1a9a1cf4bf5008508 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20P=C3=A9cseli?= <thomas.pecseli@trili.tech>
Date: Fri, 2 Jun 2023 16:28:37 +0100
Subject: [PATCH] EVM on WASM: Make sure REVERT can return data

---
 src/kernel_evm/evm_execution/src/handler.rs | 76 +++++++++++++++++++++
 src/kernel_evm/evm_execution/src/lib.rs     |  6 ++
 2 files changed, 82 insertions(+)

diff --git a/src/kernel_evm/evm_execution/src/handler.rs b/src/kernel_evm/evm_execution/src/handler.rs
index 61c50ec62b17..9add12aef869 100644
--- a/src/kernel_evm/evm_execution/src/handler.rs
+++ b/src/kernel_evm/evm_execution/src/handler.rs
@@ -43,6 +43,10 @@ const MAXIMUM_TRANSACTION_DEPTH: usize = 1024_usize;
 ///
 /// A failed transaction will not refund gas. SputnikVM even treats some kinds of failure,
 /// eg, logging during static call, as out-of-gas.
+///
+/// A _reverted_ transaction will refund gas, as execution continues as normal. It will
+/// rollback transaction effect in the reverted sub-transaction, and it _can_ return data
+/// (like as if it the call ended with a RETURN opcode).
 #[derive(Debug, Eq, PartialEq)]
 pub struct ExecutionOutcome {
     /// How much gas was used for processing an entire transaction.
@@ -1928,4 +1932,76 @@ mod test {
             }
         }
     }
+
+    #[test]
+    fn revert_can_return_a_value() {
+        let mut mock_runtime = MockHost::default();
+        let block = BlockConstants::first_block();
+        let precompiles = precompiles::precompile_set::<MockHost>();
+        let mut evm_account_storage = init_account_storage().unwrap();
+        let config = Config::london();
+        let gas_limit = 1000_u64;
+        let caller = H160::from_low_u64_be(523_u64);
+
+        let mut handler = EvmHandler::new(
+            &mut mock_runtime,
+            &mut evm_account_storage,
+            caller,
+            &block,
+            &config,
+            &precompiles,
+            gas_limit,
+        );
+
+        let address = H160::from_low_u64_be(210_u64);
+        let input = vec![0_u8];
+        let gas_limit: Option<u64> = None;
+        let transaction_context = TransactionContext::new(caller, address, U256::zero());
+        let transfer: Option<Transfer> = None;
+
+        let code: Vec<u8> = vec![
+            Opcode::PUSH8.as_u8(), // push value of return data
+            0,
+            1,
+            2,
+            3,
+            4,
+            5,
+            6,
+            7,
+            Opcode::PUSH1.as_u8(), // push address of return data
+            0,
+            Opcode::MSTORE.as_u8(), // store return data in memory
+            Opcode::PUSH1.as_u8(),  // push size of return data
+            8,
+            Opcode::PUSH1.as_u8(), // push offset in memory of return data
+            24,
+            Opcode::REVERT.as_u8(),
+        ];
+
+        set_code(&mut handler, &address, code);
+        set_balance(&mut handler, &caller, U256::from(99_u32));
+
+        let result = handler.execute_call(
+            address,
+            transfer,
+            input,
+            gas_limit,
+            transaction_context,
+        );
+
+        match result {
+            Ok(result) => {
+                let expected_result = (
+                    ExitReason::Revert(ExitRevert::Reverted),
+                    None,
+                    vec![0, 1, 2, 3, 4, 5, 6, 7],
+                );
+                assert_eq!(expected_result, result);
+            }
+            Err(err) => {
+                panic!("Unexpected error: {:?}", err);
+            }
+        }
+    }
 }
diff --git a/src/kernel_evm/evm_execution/src/lib.rs b/src/kernel_evm/evm_execution/src/lib.rs
index a4f97c7f7fd6..1c001aa446e1 100644
--- a/src/kernel_evm/evm_execution/src/lib.rs
+++ b/src/kernel_evm/evm_execution/src/lib.rs
@@ -99,6 +99,12 @@ impl From<ExitError> for EthereumError {
 }
 
 /// Execute an Ethereum Transaction
+///
+/// The function returns `Err` only if something is wrong with the kernel and/or the
+/// rollup node. If the transaction ends by executing STOP, RETURN or SUICIDE, then this is
+/// a _success_ (by Ethereum definition). Note that a REVERT instruction _can_ return
+/// data even though it will mean rollback of the transaction effect. This is also true
+/// for sub-transactions, ie, REVERT can _always_ return data.
 #[allow(clippy::too_many_arguments)]
 pub fn run_transaction<'a, Host>(
     host: &'a mut Host,
-- 
GitLab