xvm_call now returns output- or encoded error data (#139)

* Use `bare_call` to get output data

* Add `xvm_bare_call` for direct cross-pallet calls, return output from XVM precompile

* Change XVM API

* Cleanup the code

* Fix naming

* Reformat source

* Reformat toml

* Remove unused import

* Fix xvm_call test

* Added `sp-runtime/std`

* Fix output buffer encoding

* Post review fixes

* Remove gas from xvm_call precompile output

* XVM chain extension to use xvm_bare_call

* Fix output transcoding

* Removes extra weight

chain-extensions/xvm/src/lib.rs

@@ -18,12 +18,12 @@
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
-use frame_system::RawOrigin;
+use frame_support::dispatch::Encode;
+use frame_support::weights::Weight;
 use pallet_contracts::chain_extension::{ChainExtension, Environment, Ext, InitState, RetVal};
 use pallet_xvm::XvmContext;
 use sp_runtime::DispatchError;
 use sp_std::marker::PhantomData;
-
 use xvm_chain_extension_types::{XvmCallArgs, XvmExecutionResult};
 
 enum XvmFuncId {
@@ -82,29 +82,34 @@ where
                     env: None,
                 };
 
-                let call_result = pallet_xvm::Pallet::<T>::xvm_call(
-                    RawOrigin::Signed(caller).into(),
-                    xvm_context,
-                    to,
-                    input,
-                );
-
-                // Adjust the actual weight used by the call if needed.
-                let actual_weight = match call_result {
-                    Ok(e) => e.actual_weight,
-                    Err(e) => e.post_info.actual_weight,
-                };
-                if let Some(actual_weight) = actual_weight {
-                    env.adjust_weight(charged_weight, actual_weight);
-                }
+                let call_result =
+                    pallet_xvm::Pallet::<T>::xvm_bare_call(xvm_context, caller, to, input);
+
+                let actual_weight = pallet_xvm::consumed_weight(&call_result);
+                env.adjust_weight(charged_weight, Weight::from_ref_time(actual_weight));
+
+                match call_result {
+                    Ok(success) => {
+                        log::trace!(
+                            target: "xvm-extension::xvm_call",
+                            "success: {:?}", success
+                        );
 
-                return match call_result {
-                    Err(e) => {
-                        let mapped_error = XvmExecutionResult::try_from(e.error)?;
-                        Ok(RetVal::Converging(mapped_error as u32))
+                        let buffer: sp_std::vec::Vec<_> = success.output().encode();
+                        env.write(&buffer, false, None)?;
+                        Ok(RetVal::Converging(XvmExecutionResult::Success as u32))
                     }
-                    Ok(_) => Ok(RetVal::Converging(XvmExecutionResult::Success as u32)),
-                };
+
+                    Err(failure) => {
+                        log::trace!(
+                            target: "xvm-extension::xvm_call",
+                            "failure: {:?}", failure
+                        );
+
+                        // TODO Propagate error
+                        Ok(RetVal::Converging(XvmExecutionResult::UnknownError as u32))
+                    }
+                }
             }
         }
     }

frame/pallet-xvm/src/evm.rs

@@ -88,7 +88,7 @@ where
         );
 
         Ok(XvmCallOk {
-            output: Default::default(), // TODO: Fill output vec with response from the call
+            output: info.value,
             consumed_weight: T::GasWeightMapping::gas_to_weight(
                 info.used_gas.unique_saturated_into(),
                 false,

frame/pallet-xvm/src/lib.rs

@@ -80,6 +80,12 @@ pub struct XvmCallOk {
     consumed_weight: u64,
 }
 
+impl XvmCallOk {
+    pub fn output(&self) -> &[u8] {
+        &self.output
+    }
+}
+
 /// Denotes an successful XVM call execution
 #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, scale_info::TypeInfo)]
 pub struct XvmCallError {
@@ -90,6 +96,12 @@ pub struct XvmCallError {
     consumed_weight: u64,
 }
 
+impl XvmCallError {
+    pub fn error(&self) -> &XvmError {
+        &self.error
+    }
+}
+
 /// Result for executing X-VM calls
 pub type XvmResult = Result<XvmCallOk, XvmCallError>;
 

frame/pallet-xvm/src/pallet/mod.rs

@@ -62,6 +62,30 @@ pub mod pallet {
         XvmQuery { result: Result<Vec<u8>, XvmError> },
     }
 
+    impl<T: Config> Pallet<T> {
+        /// Internal interface for cross-pallet invocation.
+        /// Essentially does the same thing as `xvm_call`, but a bit differently:
+        ///   - It does not verify origin
+        ///   - It does not use `Dispatchable` API (cannot be called from tx)
+        ///   - It does not deposit event upon completion
+        ///   - It returns `XvmResult` letting the caller get return data directly
+        pub fn xvm_bare_call(
+            context: XvmContext,
+            from: T::AccountId,
+            to: Vec<u8>,
+            input: Vec<u8>,
+        ) -> XvmResult {
+            let result = T::SyncVM::xvm_call(context, from, to, input);
+
+            log::trace!(
+                target: "xvm::pallet::xvm_bare_call",
+                "Execution result: {:?}", result
+            );
+
+            result
+        }
+    }
+
     #[pallet::call]
     impl<T: Config> Pallet<T> {
         #[pallet::call_index(0)]

frame/pallet-xvm/src/wasm.rs

@@ -23,8 +23,8 @@ use frame_support::traits::Currency;
 use parity_scale_codec::HasCompact;
 use scale_info::TypeInfo;
 use sp_runtime::traits::Get;
+use sp_runtime::traits::StaticLookup;
 use sp_std::fmt::Debug;
-
 pub struct WASM<I, T>(sp_std::marker::PhantomData<(I, T)>);
 
 type BalanceOf<T> = <<T as pallet_contracts::Config>::Currency as Currency<
@@ -55,39 +55,39 @@ where
             error: XvmError::EncodingFailure,
             consumed_weight: PLACEHOLDER_WEIGHT,
         })?;
-        let res = pallet_contracts::Pallet::<T>::call(
-            frame_support::dispatch::RawOrigin::Signed(from).into(),
+
+        let dest = T::Lookup::lookup(dest).map_err(|error| XvmCallError {
+            error: XvmError::ExecutionError(Into::<&str>::into(error).into()),
+            consumed_weight: PLACEHOLDER_WEIGHT,
+        })?;
+        let call_result = pallet_contracts::Pallet::<T>::bare_call(
+            from, // no need to check origin, we consider it signed here
             dest,
             Default::default(),
             gas_limit.into(),
             None,
             input,
-        )
-        .map_err(|e| {
-            let consumed_weight = if let Some(weight) = e.post_info.actual_weight {
-                weight.ref_time()
-            } else {
-                gas_limit.ref_time()
-            };
-            XvmCallError {
-                error: XvmError::ExecutionError(Into::<&str>::into(e.error).into()),
-                consumed_weight,
-            }
-        })?;
+            false,
+            pallet_contracts::Determinism::Deterministic,
+        );
 
         log::trace!(
             target: "xvm::WASM::xvm_call",
-            "WASM XVM call result: {:?}", res
+            "WASM XVM call result: {:?}", call_result
         );
 
-        let consumed_weight = if let Some(weight) = res.actual_weight {
-            weight.ref_time()
-        } else {
-            gas_limit.ref_time()
-        };
-        Ok(XvmCallOk {
-            output: Default::default(), // TODO: Fill in with output from the call
-            consumed_weight,
-        })
+        let consumed_weight = call_result.gas_consumed.ref_time();
+
+        match call_result.result {
+            Ok(success) => Ok(XvmCallOk {
+                output: success.data,
+                consumed_weight,
+            }),
+
+            Err(error) => Err(XvmCallError {
+                error: XvmError::ExecutionError(Into::<&str>::into(error).into()),
+                consumed_weight,
+            }),
+        }
     }
 }

precompiles/utils/src/lib.rs

@@ -199,10 +199,11 @@ where
         // However while Substrate handle checking weight while not making the sender pay for it,
         // the EVM doesn't. It seems this safer to always record the costs to avoid unmetered
         // computations.
-        let used_weight = call
+        let result = call
             .dispatch(origin)
-            .map_err(|e| revert(alloc::format!("Dispatched call failed with error: {:?}", e)))?
-            .actual_weight;
+            .map_err(|e| revert(alloc::format!("Dispatched call failed with error: {:?}", e)))?;
+
+        let used_weight = result.actual_weight;
 
         let used_gas =
             Runtime::GasWeightMapping::weight_to_gas(used_weight.unwrap_or(dispatch_info.weight));

precompiles/xvm/Cargo.toml

@@ -19,6 +19,7 @@ frame-system = { workspace = true }
 parity-scale-codec = { workspace = true, features = ["max-encoded-len"] }
 sp-core = { workspace = true }
 sp-io = { workspace = true }
+sp-runtime = { workspace = true }
 sp-std = { workspace = true }
 
 # Frontier
@@ -50,4 +51,5 @@ std = [
 	"sp-core/std",
 	"sp-std/std",
 	"sp-io/std",
+	"sp-runtime/std",
 ]

precompiles/xvm/evm_sdk/XVM.sol

@@ -5,14 +5,16 @@ pragma solidity ^0.8.0;
  */
 interface XVM {
     /**
-     * @dev Execute external VM call 
-     * @param context - execution context 
-     * @param to - call recepient 
-     * @param input - SCALE-encoded call arguments 
+     * @dev Execute external VM call
+     * @param context - execution context
+     * @param to - call recepient
+     * @param input - SCALE-encoded call arguments
+     * @return success - operation outcome
+     * @return data - output data if successful, error data on error
      */
     function xvm_call(
         bytes calldata context,
         bytes calldata to,
-        bytes calldata input,
-    ) external;
+        bytes calldata input
+    ) external returns (bool success, bytes memory data);
 }

precompiles/xvm/src/lib.rs

@@ -24,12 +24,12 @@ use frame_support::dispatch::{Dispatchable, GetDispatchInfo, PostDispatchInfo};
 use pallet_evm::{AddressMapping, Precompile};
 use pallet_xvm::XvmContext;
 use parity_scale_codec::Decode;
+use sp_runtime::codec::Encode;
 use sp_std::marker::PhantomData;
 use sp_std::prelude::*;
 
 use precompile_utils::{
     revert, succeed, Bytes, EvmDataWriter, EvmResult, FunctionModifier, PrecompileHandleExt,
-    RuntimeHelper,
 };
 
 #[cfg(test)]
@@ -55,7 +55,7 @@ where
         From<pallet_xvm::Call<R>> + Dispatchable<PostInfo = PostDispatchInfo> + GetDispatchInfo,
 {
     fn execute(handle: &mut impl PrecompileHandle) -> EvmResult<PrecompileOutput> {
-        log::trace!(target: "xcm-precompile", "In XVM precompile");
+        log::trace!(target: "xvm-precompile", "In XVM precompile");
 
         let selector = handle.read_selector()?;
 
@@ -95,18 +95,38 @@ where
         let call_to = input.read::<Bytes>()?.0;
         let call_input = input.read::<Bytes>()?.0;
 
-        // Build call with origin.
-        let origin = Some(R::AddressMapping::into_account_id(handle.context().caller)).into();
-        let call = pallet_xvm::Call::<R>::xvm_call {
-            context,
-            to: call_to,
-            input: call_input,
-        };
-
-        // Dispatch a call.
-        // The underlying logic will handle updating used EVM gas based on the weight of the executed call.
-        RuntimeHelper::<R>::try_dispatch(handle, origin, call)?;
-
-        Ok(succeed(EvmDataWriter::new().write(true).build()))
+        let from = R::AddressMapping::into_account_id(handle.context().caller);
+        match &pallet_xvm::Pallet::<R>::xvm_bare_call(context, from, call_to, call_input) {
+            Ok(success) => {
+                log::trace!(
+                    target: "xvm-precompile::xvm_call",
+                    "success: {:?}", success
+                );
+
+                Ok(succeed(
+                    EvmDataWriter::new()
+                        .write(true)
+                        .write(Bytes(success.output().to_vec())) // TODO redundant clone
+                        .build(),
+                ))
+            }
+
+            Err(failure) => {
+                log::trace!(
+                    target: "xvm-precompile::xvm_call",
+                    "failure: {:?}", failure
+                );
+
+                let mut error_buffer = Vec::new();
+                failure.error().encode_to(&mut error_buffer);
+
+                Ok(succeed(
+                    EvmDataWriter::new()
+                        .write(false)
+                        .write(Bytes(error_buffer))
+                        .build(),
+                ))
+            }
+        }
     }
 }

precompiles/xvm/src/tests.rs

@@ -73,6 +73,11 @@ fn correct_arguments_works() {
                     .build(),
             )
             .expect_no_logs()
-            .execute_returns(EvmDataWriter::new().write(true).build());
+            .execute_returns(
+                EvmDataWriter::new()
+                    .write(false) // the XVM call should succeed but the internal should fail
+                    .write(vec![0u8])
+                    .build(),
+            );
     })
 }