Fix caller of scilla_calls with keep_origin = 1 (#2033)

Despite the name `keep_origin`, the flag actually means "keep the
caller of the previous call, like in an EVM `DELEGATECALL`". This
fixes the behaviour to match that definition.

It is worth noting that the `keep_origin` flag is entirely
pointless and complicates the precompile implementation massively.
The exact same behaviour can be achieved by calling the precompile
with `DELEGATECALL`, so there was no need for us to add this flag.
However, contracts are now using it in the wild so we are stuck
with it.

zilliqa/src/cfg.rs

@@ -433,6 +433,7 @@ impl Default for Forks {
         vec![Fork {
             at_height: 0,
             failed_scilla_call_from_gas_exempt_caller_causes_revert: true,
+            call_mode_1_sets_caller_to_parent_caller: true,
         }]
         .try_into()
         .unwrap()
@@ -460,6 +461,16 @@ pub struct Fork {
     /// precompile and the inner Scilla call fails, the entire transaction will revert. If false, the normal EVM
     /// semantics apply where the caller can decide how to act based on the success of the inner call.
     pub failed_scilla_call_from_gas_exempt_caller_causes_revert: bool,
+    /// If true, if a call is made to the `scilla_call` precompile with `call_mode` / `keep_origin` set to `1`, the
+    /// `_sender` of the inner Scilla call will be set to the caller of the current call-stack. If false, the `_sender`
+    /// will be set to the original transaction signer.
+    ///
+    /// For example:
+    /// A (EOA) -> B (EVM) -> C (EVM) -> D (Scilla)
+    ///
+    /// When this flag is true, `D` will see the `_sender` as `B`. When this flag is false, `D` will see the `_sender`
+    /// as `A`.
+    pub call_mode_1_sets_caller_to_parent_caller: bool,
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]

zilliqa/src/exec.rs

@@ -429,6 +429,9 @@ pub struct ExternalContext<'a, I> {
     pub scilla_call_gas_exempt_addrs: &'a [Address],
     // This flag is only used for zq1 whitelisted contracts, and it's used to detect if the entire transaction should be marked as failed
     pub enforce_transaction_failure: bool,
+    /// The caller of each call in the call-stack. This is needed because the `scilla_call` precompile needs to peek
+    /// into the call-stack. This will always be non-empty and the first entry will be the transaction signer.
+    pub callers: Vec<Address>,
 }
 
 impl<I: Inspector<PendingState>> GetInspector<PendingState> for ExternalContext<'_, I> {
@@ -520,6 +523,7 @@ impl State {
             fork: self.forks.get(current_block.number),
             scilla_call_gas_exempt_addrs: &self.scilla_call_gas_exempt_addrs,
             enforce_transaction_failure: false,
+            callers: vec![from_addr],
         };
         let pending_state = PendingState::new(self.clone());
         let mut evm = Evm::builder()

zilliqa/src/precompiles/scilla.rs

@@ -335,9 +335,48 @@ impl ContextStatefulPrecompile<PendingState> for ScillaRead {
 pub fn scilla_call_handle_register<I: ScillaInspector>(
     handler: &mut EvmHandler<'_, ExternalContext<I>, PendingState>,
 ) {
+    // Create handler
+    let prev_handle = handler.execution.create.clone();
+    handler.execution.create = Arc::new(move |ctx, inputs| {
+        // Reserve enough space to store the caller.
+        ctx.external.callers.reserve(
+            (ctx.evm.journaled_state.depth + 1).saturating_sub(ctx.external.callers.len()),
+        );
+        for _ in ctx.external.callers.len()..(ctx.evm.journaled_state.depth + 1) {
+            ctx.external.callers.push(Address::ZERO);
+        }
+        ctx.external.callers[ctx.evm.journaled_state.depth] = inputs.caller;
+
+        prev_handle(ctx, inputs)
+    });
+
+    // EOF create handler
+    let prev_handle = handler.execution.eofcreate.clone();
+    handler.execution.eofcreate = Arc::new(move |ctx, inputs| {
+        // Reserve enough space to store the caller.
+        ctx.external.callers.reserve(
+            (ctx.evm.journaled_state.depth + 1).saturating_sub(ctx.external.callers.len()),
+        );
+        for _ in ctx.external.callers.len()..(ctx.evm.journaled_state.depth + 1) {
+            ctx.external.callers.push(Address::ZERO);
+        }
+        ctx.external.callers[ctx.evm.journaled_state.depth] = inputs.caller;
+
+        prev_handle(ctx, inputs)
+    });
+
     // Call handler
     let prev_handle = handler.execution.call.clone();
     handler.execution.call = Arc::new(move |ctx, inputs| {
+        // Reserve enough space to store the caller.
+        ctx.external.callers.reserve(
+            (ctx.evm.journaled_state.depth + 1).saturating_sub(ctx.external.callers.len()),
+        );
+        for _ in ctx.external.callers.len()..(ctx.evm.journaled_state.depth + 1) {
+            ctx.external.callers.push(Address::ZERO);
+        }
+        ctx.external.callers[ctx.evm.journaled_state.depth] = inputs.caller;
+
         if inputs.bytecode_address != Address::from(*b"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0ZIL\x53") {
             return prev_handle(ctx, inputs);
         }
@@ -502,7 +541,16 @@ fn scilla_call_precompile<I: ScillaInspector>(
         scilla,
         evmctx.env.tx.caller,
         if keep_origin {
-            evmctx.env.tx.caller
+            if external_context
+                .fork
+                .call_mode_1_sets_caller_to_parent_caller
+            {
+                // Use the caller of the parent call-stack.
+                external_context.callers[evmctx.journaled_state.depth - 1]
+            } else {
+                // Use the original transaction signer.
+                evmctx.env.tx.caller
+            }
         } else {
             input.caller
         },

zilliqa/tests/it/contracts/ScillaInterop.sol

@@ -7,6 +7,35 @@ library ScillaConnector {
     uint private constant SCILLA_CALL_PRECOMPILE_ADDRESS = 0x5a494c53;
     uint private constant SCILLA_STATE_READ_PRECOMPILE_ADDRESS = 0x5a494c92;
 
+    /**
+     * @dev Calls a ZRC2 contract function with two arguments
+     * @param target The address of the ZRC2 contract
+     * @param tran_name The name of the function to call
+     */
+    function call(address target, string memory tran_name) internal {
+        bytes memory encodedArgs = abi.encode(
+            target,
+            tran_name,
+            CALL_SCILLA_WITH_THE_SAME_SENDER
+        );
+        uint256 argsLength = encodedArgs.length;
+
+        assembly {
+            let ok := call(
+                gas(),
+                SCILLA_CALL_PRECOMPILE_ADDRESS,
+                0,
+                add(encodedArgs, 0x20),
+                argsLength,
+                0x20,
+                0
+            )
+            if iszero(ok) {
+                revert(0, 0)
+            }
+        }
+    }
+
     /**
      * @dev Calls a ZRC2 contract function with two arguments
      * @param target The address of the ZRC2 contract
@@ -315,6 +344,13 @@ contract ScillaInterop {
         return scillaContract.readNestedMapUint128(varName, key1, key2);
     }
 
+    function callScillaNoArgs(
+        address scillaContract,
+        string memory transitionName
+    ) public {
+        scillaContract.call(transitionName);
+    }
+
     function callScilla(
         address scillaContract,
         string memory transitionName,

zilliqa/tests/it/zil.rs

@@ -1098,6 +1098,11 @@ async fn scilla_precompiles(mut network: Network) {
           e = {_eventname : "Inserted"; a : a; b : b};
           event e
         end
+
+        transition LogSender()
+          e = {_eventname : "LogSender"; sender : _sender};
+          event e
+        end
     "#;
 
     let data = r#"[
@@ -1129,6 +1134,7 @@ async fn scilla_precompiles(mut network: Network) {
     )
     .await;
     let receipt = wallet.get_transaction_receipt(hash).await.unwrap().unwrap();
+    let evm_contract_address = receipt.contract_address.unwrap();
 
     let read = |fn_name, var_name: &'_ str, keys: &[H160], ty| {
         let abi = &abi;
@@ -1201,7 +1207,7 @@ async fn scilla_precompiles(mut network: Network) {
         Token::Uint(5.into()),
     ];
     let tx = TransactionRequest::new()
-        .to(receipt.contract_address.unwrap())
+        .to(evm_contract_address)
         .data(function.encode_input(input).unwrap())
         .gas(84_000_000);
 
@@ -1262,6 +1268,34 @@ async fn scilla_precompiles(mut network: Network) {
     .into_uint()
     .unwrap();
     assert_eq!(val, 5.into());
+
+    // Construct a transaction which logs the `_sender` from the Scilla call.
+    let function = abi.function("callScillaNoArgs").unwrap();
+    let input = &[
+        Token::Address(scilla_contract_address),
+        Token::String("LogSender".to_owned()),
+    ];
+    let tx = TransactionRequest::new()
+        .to(evm_contract_address)
+        .data(function.encode_input(input).unwrap())
+        .gas(84_000_000);
+
+    // Run the transaction.
+    let tx_hash = wallet.send_transaction(tx, None).await.unwrap().tx_hash();
+    let receipt = network.run_until_receipt(&wallet, tx_hash, 100).await;
+    assert_eq!(receipt.logs.len(), 1);
+    let log = &receipt.logs[0];
+    let data = ethabi::decode(&[ParamType::String], &log.data).unwrap()[0]
+        .clone()
+        .into_string()
+        .unwrap();
+    let scilla_log: Value = serde_json::from_str(&data).unwrap();
+    assert_eq!(scilla_log["_eventname"], "LogSender");
+    assert_eq!(scilla_log["params"][0]["vname"], "sender");
+    assert_eq!(
+        scilla_log["params"][0]["value"],
+        format!("{:?}", wallet.address())
+    );
 }
 
 #[zilliqa_macros::test(restrict_concurrency)]