From ca382f32033d8350d49934b30680d8c30dd9bdfc Mon Sep 17 00:00:00 2001 From: PG Herveou Date: Mon, 19 Feb 2024 16:29:47 +0100 Subject: [PATCH] Contracts: xcm host fn fixes (#3086) ## Xcm changes: - Fix `pallet_xcm::execute`, move the logic into The `ExecuteController` so it can be shared with anything that implement that trait. - Make `ExecuteController::execute` retursn `DispatchErrorWithPostInfo` instead of `DispatchError`, so that we don't charge the full `max_weight` provided if the execution is incomplete (useful for force_batch or contracts calls) - Fix docstring for `pallet_xcm::execute`, to reflect the changes from #2405 - Update the signature for `ExecuteController::execute`, we don't need to return the `Outcome` anymore since we only care about `Outcome::Complete` ## Contracts changes: - Update host fn `xcm_exexute`, we don't need to write the `Outcome` to the sandbox memory anymore. This was also not charged as well before so it if fixes this too. - One of the issue was that the dry_run of a contract that call `xcm_execute` would exhaust the `gas_limit`. This is because `XcmExecuteController::execute` takes a `max_weight` argument, and since we don't want the user to specify it manually we were passing everything left by pre-charghing `ctx.ext.gas_meter().gas_left()` - To fix it I added a `fn influence_lowest_limit` on the `Token` trait and make it return false for `RuntimeCost::XcmExecute`. - Got rid of the `RuntimeToken` indirection, we can just use `RuntimeCost` directly. --------- Co-authored-by: command-bot <> --- polkadot/xcm/pallet-xcm/src/lib.rs | 60 +++++++----- polkadot/xcm/pallet-xcm/src/tests/mod.rs | 38 ++++---- polkadot/xcm/xcm-builder/src/controller.rs | 15 ++- .../fixtures/contracts/xcm_execute.rs | 11 ++- .../frame/contracts/mock-network/src/lib.rs | 3 +- .../frame/contracts/mock-network/src/tests.rs | 71 ++++++++++---- substrate/frame/contracts/src/exec.rs | 7 ++ substrate/frame/contracts/src/gas.rs | 9 +- substrate/frame/contracts/src/wasm/mod.rs | 4 + substrate/frame/contracts/src/wasm/runtime.rs | 95 ++++++++----------- substrate/frame/contracts/uapi/src/host.rs | 3 +- .../frame/contracts/uapi/src/host/riscv32.rs | 2 +- .../frame/contracts/uapi/src/host/wasm32.rs | 7 +- 13 files changed, 192 insertions(+), 133 deletions(-) diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 586c275ae980..5e1a3e55f9b6 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -29,7 +29,7 @@ pub mod migration; use codec::{Decode, Encode, EncodeLike, MaxEncodedLen}; use frame_support::{ - dispatch::GetDispatchInfo, + dispatch::{DispatchErrorWithPostInfo, GetDispatchInfo, WithPostDispatchInfo}, pallet_prelude::*, traits::{ Contains, ContainsPair, Currency, Defensive, EnsureOrigin, Get, LockableCurrency, @@ -291,22 +291,38 @@ pub mod pallet { origin: OriginFor, message: Box::RuntimeCall>>, max_weight: Weight, - ) -> Result { - let origin_location = T::ExecuteXcmOrigin::ensure_origin(origin)?; - let mut hash = message.using_encoded(sp_io::hashing::blake2_256); - let message = (*message).try_into().map_err(|()| Error::::BadVersion)?; - let value = (origin_location, message); - ensure!(T::XcmExecuteFilter::contains(&value), Error::::Filtered); - let (origin_location, message) = value; - let outcome = T::XcmExecutor::prepare_and_execute( - origin_location, - message, - &mut hash, - max_weight, - max_weight, - ); + ) -> Result { + log::trace!(target: "xcm::pallet_xcm::execute", "message {:?}, max_weight {:?}", message, max_weight); + let outcome = (|| { + let origin_location = T::ExecuteXcmOrigin::ensure_origin(origin)?; + let mut hash = message.using_encoded(sp_io::hashing::blake2_256); + let message = (*message).try_into().map_err(|()| Error::::BadVersion)?; + let value = (origin_location, message); + ensure!(T::XcmExecuteFilter::contains(&value), Error::::Filtered); + let (origin_location, message) = value; + Ok(T::XcmExecutor::prepare_and_execute( + origin_location, + message, + &mut hash, + max_weight, + max_weight, + )) + })() + .map_err(|e: DispatchError| { + e.with_weight(::execute()) + })?; + Self::deposit_event(Event::Attempted { outcome: outcome.clone() }); - Ok(outcome) + let weight_used = outcome.weight_used(); + outcome.ensure_complete().map_err(|error| { + log::error!(target: "xcm::pallet_xcm::execute", "XCM execution failed with error {:?}", error); + Error::::LocalExecutionIncomplete.with_weight( + weight_used.saturating_add( + ::execute(), + ), + ) + })?; + Ok(weight_used) } } @@ -1009,9 +1025,6 @@ pub mod pallet { /// No more than `max_weight` will be used in its attempted execution. If this is less than /// the maximum amount of weight that the message could take to be executed, then no /// execution attempt will be made. - /// - /// NOTE: A successful return to this does *not* imply that the `msg` was executed - /// successfully to completion; only that it was attempted. #[pallet::call_index(3)] #[pallet::weight(max_weight.saturating_add(T::WeightInfo::execute()))] pub fn execute( @@ -1019,13 +1032,8 @@ pub mod pallet { message: Box::RuntimeCall>>, max_weight: Weight, ) -> DispatchResultWithPostInfo { - log::trace!(target: "xcm::pallet_xcm::execute", "message {:?}, max_weight {:?}", message, max_weight); - let outcome = >::execute(origin, message, max_weight)?; - let weight_used = outcome.weight_used(); - outcome.ensure_complete().map_err(|error| { - log::error!(target: "xcm::pallet_xcm::execute", "XCM execution failed with error {:?}", error); - Error::::LocalExecutionIncomplete - })?; + let weight_used = + >::execute(origin, message, max_weight)?; Ok(Some(weight_used.saturating_add(T::WeightInfo::execute())).into()) } diff --git a/polkadot/xcm/pallet-xcm/src/tests/mod.rs b/polkadot/xcm/pallet-xcm/src/tests/mod.rs index e5ac80c3a142..28c7d197443b 100644 --- a/polkadot/xcm/pallet-xcm/src/tests/mod.rs +++ b/polkadot/xcm/pallet-xcm/src/tests/mod.rs @@ -20,11 +20,12 @@ pub(crate) mod assets_transfer; use crate::{ mock::*, pallet::SupportedVersion, AssetTraps, Config, CurrentMigration, Error, - LatestVersionedLocation, Pallet, Queries, QueryStatus, VersionDiscoveryQueue, - VersionMigrationStage, VersionNotifiers, VersionNotifyTargets, WeightInfo, + ExecuteControllerWeightInfo, LatestVersionedLocation, Pallet, Queries, QueryStatus, + VersionDiscoveryQueue, VersionMigrationStage, VersionNotifiers, VersionNotifyTargets, + WeightInfo, }; use frame_support::{ - assert_noop, assert_ok, + assert_err_ignore_postinfo, assert_noop, assert_ok, traits::{Currency, Hooks}, weights::Weight, }; @@ -450,19 +451,19 @@ fn trapped_assets_can_be_claimed() { assert_eq!(Balances::total_balance(&BOB), INITIAL_BALANCE + SEND_AMOUNT); assert_eq!(AssetTraps::::iter().collect::>(), vec![]); - let weight = BaseXcmWeight::get() * 3; - assert_ok!(>::execute( - RuntimeOrigin::signed(ALICE), - Box::new(VersionedXcm::from(Xcm(vec![ - ClaimAsset { assets: (Here, SEND_AMOUNT).into(), ticket: Here.into() }, - buy_execution((Here, SEND_AMOUNT)), - DepositAsset { assets: AllCounted(1).into(), beneficiary: dest }, - ]))), - weight - )); - let outcome = - Outcome::Incomplete { used: BaseXcmWeight::get(), error: XcmError::UnknownClaim }; - assert_eq!(last_event(), RuntimeEvent::XcmPallet(crate::Event::Attempted { outcome })); + // Can't claim twice. + assert_err_ignore_postinfo!( + XcmPallet::execute( + RuntimeOrigin::signed(ALICE), + Box::new(VersionedXcm::from(Xcm(vec![ + ClaimAsset { assets: (Here, SEND_AMOUNT).into(), ticket: Here.into() }, + buy_execution((Here, SEND_AMOUNT)), + DepositAsset { assets: AllCounted(1).into(), beneficiary: dest }, + ]))), + weight + ), + Error::::LocalExecutionIncomplete + ); }); } @@ -495,11 +496,14 @@ fn incomplete_execute_reverts_side_effects() { // all effects are reverted and balances unchanged for either sender or receiver assert_eq!(Balances::total_balance(&ALICE), INITIAL_BALANCE); assert_eq!(Balances::total_balance(&BOB), INITIAL_BALANCE); + assert_eq!( result, Err(sp_runtime::DispatchErrorWithPostInfo { post_info: frame_support::dispatch::PostDispatchInfo { - actual_weight: None, + actual_weight: Some( + as ExecuteControllerWeightInfo>::execute() + weight + ), pays_fee: frame_support::dispatch::Pays::Yes, }, error: sp_runtime::DispatchError::Module(sp_runtime::ModuleError { diff --git a/polkadot/xcm/xcm-builder/src/controller.rs b/polkadot/xcm/xcm-builder/src/controller.rs index 8ead18b5bd7f..ba2b1fb44b8e 100644 --- a/polkadot/xcm/xcm-builder/src/controller.rs +++ b/polkadot/xcm/xcm-builder/src/controller.rs @@ -18,7 +18,10 @@ //! Controller traits defined in this module are high-level traits that will rely on other traits //! from `xcm-executor` to perform their tasks. -use frame_support::pallet_prelude::DispatchError; +use frame_support::{ + dispatch::{DispatchErrorWithPostInfo, WithPostDispatchInfo}, + pallet_prelude::DispatchError, +}; use sp_std::boxed::Box; use xcm::prelude::*; pub use xcm_executor::traits::QueryHandler; @@ -52,7 +55,8 @@ pub trait ExecuteController { /// Weight information for ExecuteController functions. type WeightInfo: ExecuteControllerWeightInfo; - /// Attempt to execute an XCM locally, and return the outcome. + /// Attempt to execute an XCM locally, returns Ok with the weight consumed if the execution + /// complete successfully, Err otherwise. /// /// # Parameters /// @@ -63,7 +67,7 @@ pub trait ExecuteController { origin: Origin, message: Box>, max_weight: Weight, - ) -> Result; + ) -> Result; } /// Weight functions needed for [`SendController`]. @@ -137,8 +141,9 @@ impl ExecuteController for () { _origin: Origin, _message: Box>, _max_weight: Weight, - ) -> Result { - Ok(Outcome::Error { error: XcmError::Unimplemented }) + ) -> Result { + Err(DispatchError::Other("ExecuteController::execute not implemented") + .with_weight(Weight::zero())) } } diff --git a/substrate/frame/contracts/fixtures/contracts/xcm_execute.rs b/substrate/frame/contracts/fixtures/contracts/xcm_execute.rs index 09d0b6cf9728..1d570ffead71 100644 --- a/substrate/frame/contracts/fixtures/contracts/xcm_execute.rs +++ b/substrate/frame/contracts/fixtures/contracts/xcm_execute.rs @@ -30,10 +30,11 @@ pub extern "C" fn deploy() {} pub extern "C" fn call() { input!(512, msg: [u8],); - let mut outcome = [0u8; 512]; - let outcome = &mut &mut outcome[..]; - #[allow(deprecated)] - api::xcm_execute(msg, outcome).unwrap(); - api::return_value(uapi::ReturnFlags::empty(), outcome); + let err_code = match api::xcm_execute(msg) { + Ok(_) => 0u32, + Err(code) => code as u32, + }; + + api::return_value(uapi::ReturnFlags::empty(), &err_code.to_le_bytes()); } diff --git a/substrate/frame/contracts/mock-network/src/lib.rs b/substrate/frame/contracts/mock-network/src/lib.rs index eea9dde062c8..8a17a3f2fa78 100644 --- a/substrate/frame/contracts/mock-network/src/lib.rs +++ b/substrate/frame/contracts/mock-network/src/lib.rs @@ -26,7 +26,8 @@ use crate::primitives::{AccountId, UNITS}; use sp_runtime::BuildStorage; use xcm::latest::prelude::*; use xcm_executor::traits::ConvertLocation; -use xcm_simulator::{decl_test_network, decl_test_parachain, decl_test_relay_chain, TestExt}; +pub use xcm_simulator::TestExt; +use xcm_simulator::{decl_test_network, decl_test_parachain, decl_test_relay_chain}; // Accounts pub const ADMIN: sp_runtime::AccountId32 = sp_runtime::AccountId32::new([0u8; 32]); diff --git a/substrate/frame/contracts/mock-network/src/tests.rs b/substrate/frame/contracts/mock-network/src/tests.rs index c25373bcbe3a..d22221fe8ee0 100644 --- a/substrate/frame/contracts/mock-network/src/tests.rs +++ b/substrate/frame/contracts/mock-network/src/tests.rs @@ -21,7 +21,6 @@ use crate::{ primitives::{AccountId, CENTS}, relay_chain, MockNet, ParaA, ParachainBalances, Relay, ALICE, BOB, INITIAL_BALANCE, }; -use assert_matches::assert_matches; use codec::{Decode, Encode}; use frame_support::{ assert_err, @@ -31,11 +30,18 @@ use frame_support::{ use pallet_balances::{BalanceLock, Reasons}; use pallet_contracts::{Code, CollectEvents, DebugInfo, Determinism}; use pallet_contracts_fixtures::compile_module; +use pallet_contracts_uapi::ReturnErrorCode; use xcm::{v4::prelude::*, VersionedLocation, VersionedXcm}; use xcm_simulator::TestExt; type ParachainContracts = pallet_contracts::Pallet; +macro_rules! assert_return_code { + ( $x:expr , $y:expr $(,)? ) => {{ + assert_eq!(u32::from_le_bytes($x.data[..].try_into().unwrap()), $y as u32); + }}; +} + /// Instantiate the tests contract, and fund it with some balance and assets. fn instantiate_test_contract(name: &str) -> AccountId { let (wasm, _) = compile_module::(name).unwrap(); @@ -100,19 +106,57 @@ fn test_xcm_execute() { DebugInfo::UnsafeDebug, CollectEvents::UnsafeCollect, Determinism::Enforced, - ) - .result - .unwrap(); + ); - let mut data = &result.data[..]; - let outcome = Outcome::decode(&mut data).expect("Failed to decode xcm_execute Outcome"); - assert_matches!(outcome, Outcome::Complete { .. }); + assert_eq!(result.gas_consumed, result.gas_required); + assert_return_code!(&result.result.unwrap(), ReturnErrorCode::Success); // Check if the funds are subtracted from the account of Alice and added to the account of // Bob. let initial = INITIAL_BALANCE; - assert_eq!(parachain::Assets::balance(0, contract_addr), initial); assert_eq!(ParachainBalances::free_balance(BOB), initial + amount); + assert_eq!(ParachainBalances::free_balance(&contract_addr), initial - amount); + }); +} + +#[test] +fn test_xcm_execute_incomplete() { + MockNet::reset(); + + let contract_addr = instantiate_test_contract("xcm_execute"); + let amount = 10 * CENTS; + + // Execute XCM instructions through the contract. + ParaA::execute_with(|| { + // The XCM used to transfer funds to Bob. + let message: Xcm<()> = Xcm(vec![ + WithdrawAsset(vec![(Here, amount).into()].into()), + // This will fail as the contract does not have enough balance to complete both + // withdrawals. + WithdrawAsset(vec![(Here, INITIAL_BALANCE).into()].into()), + DepositAsset { + assets: All.into(), + beneficiary: AccountId32 { network: None, id: BOB.clone().into() }.into(), + }, + ]); + + let result = ParachainContracts::bare_call( + ALICE, + contract_addr.clone(), + 0, + Weight::MAX, + None, + VersionedXcm::V4(message).encode(), + DebugInfo::UnsafeDebug, + CollectEvents::UnsafeCollect, + Determinism::Enforced, + ); + + assert_eq!(result.gas_consumed, result.gas_required); + assert_return_code!(&result.result.unwrap(), ReturnErrorCode::XcmExecutionFailed); + + assert_eq!(ParachainBalances::free_balance(BOB), INITIAL_BALANCE); + assert_eq!(ParachainBalances::free_balance(&contract_addr), INITIAL_BALANCE - amount); }); } @@ -182,17 +226,10 @@ fn test_xcm_execute_reentrant_call() { DebugInfo::UnsafeDebug, CollectEvents::UnsafeCollect, Determinism::Enforced, - ) - .result - .unwrap(); - - let mut data = &result.data[..]; - let outcome = Outcome::decode(&mut data).expect("Failed to decode xcm_execute Outcome"); - assert_matches!( - outcome, - Outcome::Incomplete { used: _, error: XcmError::ExpectationFalse } ); + assert_return_code!(&result.result.unwrap(), ReturnErrorCode::XcmExecutionFailed); + // Funds should not change hands as the XCM transact failed. assert_eq!(ParachainBalances::free_balance(BOB), INITIAL_BALANCE); }); diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index 6f7131f3ea41..79220021efe4 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/substrate/frame/contracts/src/exec.rs @@ -287,6 +287,9 @@ pub trait Ext: sealing::Sealed { /// Returns `true` if debug message recording is enabled. Otherwise `false` is returned. fn append_debug_buffer(&mut self, msg: &str) -> bool; + /// Returns `true` if debug message recording is enabled. Otherwise `false` is returned. + fn debug_buffer_enabled(&self) -> bool; + /// Call some dispatchable and return the result. fn call_runtime(&self, call: ::RuntimeCall) -> DispatchResultWithPostInfo; @@ -1429,6 +1432,10 @@ where self.top_frame_mut().nested_storage.charge(diff) } + fn debug_buffer_enabled(&self) -> bool { + self.debug_message.is_some() + } + fn append_debug_buffer(&mut self, msg: &str) -> bool { if let Some(buffer) = &mut self.debug_message { buffer diff --git a/substrate/frame/contracts/src/gas.rs b/substrate/frame/contracts/src/gas.rs index 11292dc03f03..9271b615d002 100644 --- a/substrate/frame/contracts/src/gas.rs +++ b/substrate/frame/contracts/src/gas.rs @@ -63,6 +63,11 @@ pub trait Token: Copy + Clone + TestAuxiliaries { /// while calculating the amount. In this case it is ok to use saturating operations /// since on overflow they will return `max_value` which should consume all gas. fn weight(&self) -> Weight; + + /// Returns true if this token is expected to influence the lowest gas limit. + fn influence_lowest_gas_limit(&self) -> bool { + true + } } /// A wrapper around a type-erased trait object of what used to be a `Token`. @@ -160,7 +165,9 @@ impl GasMeter { /// This is when a maximum a priori amount was charged and then should be partially /// refunded to match the actual amount. pub fn adjust_gas>(&mut self, charged_amount: ChargedAmount, token: Tok) { - self.gas_left_lowest = self.gas_left_lowest(); + if token.influence_lowest_gas_limit() { + self.gas_left_lowest = self.gas_left_lowest(); + } let adjustment = charged_amount.0.saturating_sub(token.weight()); self.gas_left = self.gas_left.saturating_add(adjustment).min(self.gas_limit); } diff --git a/substrate/frame/contracts/src/wasm/mod.rs b/substrate/frame/contracts/src/wasm/mod.rs index d00ec2e47521..1c7395ec3bff 100644 --- a/substrate/frame/contracts/src/wasm/mod.rs +++ b/substrate/frame/contracts/src/wasm/mod.rs @@ -687,6 +687,10 @@ mod tests { &mut self.gas_meter } fn charge_storage(&mut self, _diff: &crate::storage::meter::Diff) {} + + fn debug_buffer_enabled(&self) -> bool { + true + } fn append_debug_buffer(&mut self, msg: &str) -> bool { self.debug_buffer.extend(msg.as_bytes()); true diff --git a/substrate/frame/contracts/src/wasm/runtime.rs b/substrate/frame/contracts/src/wasm/runtime.rs index 3af0d04a3ad1..74b19910c371 100644 --- a/substrate/frame/contracts/src/wasm/runtime.rs +++ b/substrate/frame/contracts/src/wasm/runtime.rs @@ -21,7 +21,6 @@ use crate::{ exec::{ExecError, ExecResult, Ext, Key, TopicOf}, gas::{ChargedAmount, Token}, primitives::ExecReturnValue, - schedule::HostFnWeights, BalanceOf, CodeHash, Config, DebugBufferVec, Error, SENTINEL, }; use codec::{Decode, DecodeLimit, Encode, MaxEncodedLen}; @@ -235,6 +234,8 @@ pub enum RuntimeCosts { ChainExtension(Weight), /// Weight charged for calling into the runtime. CallRuntime(Weight), + /// Weight charged for calling xcm_execute. + CallXcmExecute(Weight), /// Weight of calling `seal_set_code_hash` SetCodeHash, /// Weight of calling `ecdsa_to_eth_address` @@ -251,10 +252,18 @@ pub enum RuntimeCosts { RemoveDelegateDependency, } -impl RuntimeCosts { - fn token(&self, s: &HostFnWeights) -> RuntimeToken { +impl Token for RuntimeCosts { + fn influence_lowest_gas_limit(&self) -> bool { + match self { + &Self::CallXcmExecute(_) => false, + _ => true, + } + } + + fn weight(&self) -> Weight { + let s = T::Schedule::get().host_fn_weights; use self::RuntimeCosts::*; - let weight = match *self { + match *self { CopyFromContract(len) => s.return_per_byte.saturating_mul(len.into()), CopyToContract(len) => s.input_per_byte.saturating_mul(len.into()), Caller => s.caller, @@ -323,8 +332,7 @@ impl RuntimeCosts { Sr25519Verify(len) => s .sr25519_verify .saturating_add(s.sr25519_verify_per_byte.saturating_mul(len.into())), - ChainExtension(weight) => weight, - CallRuntime(weight) => weight, + ChainExtension(weight) | CallRuntime(weight) | CallXcmExecute(weight) => weight, SetCodeHash => s.set_code_hash, EcdsaToEthAddress => s.ecdsa_to_eth_address, ReentrantCount => s.reentrance_count, @@ -332,11 +340,6 @@ impl RuntimeCosts { InstantationNonce => s.instantiation_nonce, AddDelegateDependency => s.add_delegate_dependency, RemoveDelegateDependency => s.remove_delegate_dependency, - }; - RuntimeToken { - #[cfg(test)] - _created_from: *self, - weight, } } } @@ -347,25 +350,10 @@ impl RuntimeCosts { /// a function won't work out. macro_rules! charge_gas { ($runtime:expr, $costs:expr) => {{ - let token = $costs.token(&$runtime.ext.schedule().host_fn_weights); - $runtime.ext.gas_meter_mut().charge(token) + $runtime.ext.gas_meter_mut().charge($costs) }}; } -#[cfg_attr(test, derive(Debug, PartialEq, Eq))] -#[derive(Copy, Clone)] -struct RuntimeToken { - #[cfg(test)] - _created_from: RuntimeCosts, - weight: Weight, -} - -impl Token for RuntimeToken { - fn weight(&self) -> Weight { - self.weight - } -} - /// The kind of call that should be performed. enum CallType { /// Execute another instantiated contract @@ -506,28 +494,25 @@ impl<'a, E: Ext + 'a> Runtime<'a, E> { /// This is when a maximum a priori amount was charged and then should be partially /// refunded to match the actual amount. pub fn adjust_gas(&mut self, charged: ChargedAmount, actual_costs: RuntimeCosts) { - let token = actual_costs.token(&self.ext.schedule().host_fn_weights); - self.ext.gas_meter_mut().adjust_gas(charged, token); + self.ext.gas_meter_mut().adjust_gas(charged, actual_costs); } /// Charge, Run and adjust gas, for executing the given dispatchable. - fn call_dispatchable< - ErrorReturnCode: Get, - F: FnOnce(&mut Self) -> DispatchResultWithPostInfo, - >( + fn call_dispatchable>( &mut self, dispatch_info: DispatchInfo, - run: F, + runtime_cost: impl Fn(Weight) -> RuntimeCosts, + run: impl FnOnce(&mut Self) -> DispatchResultWithPostInfo, ) -> Result { use frame_support::dispatch::extract_actual_weight; - let charged = self.charge_gas(RuntimeCosts::CallRuntime(dispatch_info.weight))?; + let charged = self.charge_gas(runtime_cost(dispatch_info.weight))?; let result = run(self); let actual_weight = extract_actual_weight(&result, &dispatch_info); - self.adjust_gas(charged, RuntimeCosts::CallRuntime(actual_weight)); + self.adjust_gas(charged, runtime_cost(actual_weight)); match result { Ok(_) => Ok(ReturnErrorCode::Success), Err(e) => { - if self.ext.append_debug_buffer("") { + if self.ext.debug_buffer_enabled() { self.ext.append_debug_buffer("call failed with: "); self.ext.append_debug_buffer(e.into()); }; @@ -2113,9 +2098,11 @@ pub mod env { ctx.charge_gas(RuntimeCosts::CopyFromContract(call_len))?; let call: ::RuntimeCall = ctx.read_sandbox_memory_as_unbounded(memory, call_ptr, call_len)?; - ctx.call_dispatchable::(call.get_dispatch_info(), |ctx| { - ctx.ext.call_runtime(call) - }) + ctx.call_dispatchable::( + call.get_dispatch_info(), + RuntimeCosts::CallRuntime, + |ctx| ctx.ext.call_runtime(call), + ) } /// Execute an XCM program locally, using the contract's address as the origin. @@ -2126,7 +2113,6 @@ pub mod env { memory: _, msg_ptr: u32, msg_len: u32, - output_ptr: u32, ) -> Result { use frame_support::dispatch::DispatchInfo; use xcm::VersionedXcm; @@ -2135,26 +2121,27 @@ pub mod env { ctx.charge_gas(RuntimeCosts::CopyFromContract(msg_len))?; let message: VersionedXcm> = ctx.read_sandbox_memory_as_unbounded(memory, msg_ptr, msg_len)?; + ensure_executable::(&message)?; let execute_weight = <::Xcm as ExecuteController<_, _>>::WeightInfo::execute(); let weight = ctx.ext.gas_meter().gas_left().max(execute_weight); let dispatch_info = DispatchInfo { weight, ..Default::default() }; - ensure_executable::(&message)?; - ctx.call_dispatchable::(dispatch_info, |ctx| { - let origin = crate::RawOrigin::Signed(ctx.ext.address().clone()).into(); - let outcome = <::Xcm>::execute( - origin, - Box::new(message), - weight.saturating_sub(execute_weight), - )?; + ctx.call_dispatchable::( + dispatch_info, + RuntimeCosts::CallXcmExecute, + |ctx| { + let origin = crate::RawOrigin::Signed(ctx.ext.address().clone()).into(); + let weight_used = <::Xcm>::execute( + origin, + Box::new(message), + weight.saturating_sub(execute_weight), + )?; - ctx.write_sandbox_memory(memory, output_ptr, &outcome.encode())?; - let pre_dispatch_weight = - <::Xcm as ExecuteController<_, _>>::WeightInfo::execute(); - Ok(Some(outcome.weight_used().saturating_add(pre_dispatch_weight)).into()) - }) + Ok(Some(weight_used.saturating_add(execute_weight)).into()) + }, + ) } /// Send an XCM program from the contract to the specified destination. diff --git a/substrate/frame/contracts/uapi/src/host.rs b/substrate/frame/contracts/uapi/src/host.rs index c8b9ae8b2def..7894dc3a210a 100644 --- a/substrate/frame/contracts/uapi/src/host.rs +++ b/substrate/frame/contracts/uapi/src/host.rs @@ -792,7 +792,7 @@ pub trait HostFn { #[deprecated( note = "Unstable function. Behaviour can change without further notice. Use only for testing." )] - fn xcm_execute(msg: &[u8], output: &mut &mut [u8]) -> Result; + fn xcm_execute(msg: &[u8]) -> Result; /// Send an XCM program from the contract to the specified destination. /// This is equivalent to dispatching `pallet_xcm::send` through `call_runtime`, except that @@ -804,7 +804,6 @@ pub trait HostFn { /// traps otherwise. /// - `msg`: The message, should be decodable as a [VersionedXcm](https://paritytech.github.io/polkadot-sdk/master/staging_xcm/enum.VersionedXcm.html), /// traps otherwise. - /// - `output`: A reference to the output data buffer to write the [XcmHash](https://paritytech.github.io/polkadot-sdk/master/staging_xcm/v3/type.XcmHash.html) /// /// # Return /// diff --git a/substrate/frame/contracts/uapi/src/host/riscv32.rs b/substrate/frame/contracts/uapi/src/host/riscv32.rs index b1934cc469e4..dc1c48308df8 100644 --- a/substrate/frame/contracts/uapi/src/host/riscv32.rs +++ b/substrate/frame/contracts/uapi/src/host/riscv32.rs @@ -297,7 +297,7 @@ impl HostFn for HostFnImpl { todo!() } - fn xcm_execute(msg: &[u8], output: &mut &mut [u8]) -> Result { + fn xcm_execute(msg: &[u8]) -> Result { todo!() } diff --git a/substrate/frame/contracts/uapi/src/host/wasm32.rs b/substrate/frame/contracts/uapi/src/host/wasm32.rs index 77cf22891e2f..3df3abeb7dcd 100644 --- a/substrate/frame/contracts/uapi/src/host/wasm32.rs +++ b/substrate/frame/contracts/uapi/src/host/wasm32.rs @@ -160,7 +160,7 @@ mod sys { pub fn weight_to_fee(gas: u64, output_ptr: *mut u8, output_len_ptr: *mut u32); - pub fn xcm_execute(msg_ptr: *const u8, msg_len: u32, output_ptr: *mut u8) -> ReturnCode; + pub fn xcm_execute(msg_ptr: *const u8, msg_len: u32) -> ReturnCode; pub fn xcm_send( dest_ptr: *const u8, @@ -819,9 +819,8 @@ impl HostFn for HostFnImpl { unsafe { sys::reentrance_count() } } - fn xcm_execute(msg: &[u8], output: &mut &mut [u8]) -> Result { - let ret_code = - unsafe { sys::xcm_execute(msg.as_ptr(), msg.len() as _, output.as_mut_ptr()) }; + fn xcm_execute(msg: &[u8]) -> Result { + let ret_code = unsafe { sys::xcm_execute(msg.as_ptr(), msg.len() as _) }; ret_code.into() }