Contracts: xcm host fn fixes

polkadot/xcm/pallet-xcm/src/lib.rs

@@ -288,7 +288,8 @@ pub mod pallet {
 			origin: OriginFor<T>,
 			message: Box<VersionedXcm<<T as Config>::RuntimeCall>>,
 			max_weight: Weight,
-		) -> Result<Outcome, DispatchError> {
+		) -> Result<Weight, DispatchError> {
+			log::trace!(target: "xcm::pallet_xcm::execute", "message {:?}, max_weight {:?}", message, max_weight);
 			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::<T>::BadVersion)?;
@@ -303,7 +304,13 @@ pub mod pallet {
 				max_weight,
 			);
 			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::<T>::LocalExecutionIncomplete
+			})?;
+			Ok(weight_used)
 		}
 	}
 
@@ -996,23 +1003,15 @@ 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(
 			origin: OriginFor<T>,
 			message: Box<VersionedXcm<<T as Config>::RuntimeCall>>,
 			max_weight: Weight,
 		) -> DispatchResultWithPostInfo {
-			log::trace!(target: "xcm::pallet_xcm::execute", "message {:?}, max_weight {:?}", message, max_weight);
-			let outcome = <Self as ExecuteController<_, _>>::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::<T>::LocalExecutionIncomplete
-			})?;
+			let weight_used =
+				<Self as ExecuteController<_, _>>::execute(origin, message, max_weight)?;
 			Ok(Some(weight_used.saturating_add(T::WeightInfo::execute())).into())
 		}
 

polkadot/xcm/pallet-xcm/src/tests/mod.rs

@@ -23,7 +23,7 @@ use crate::{
 	VersionDiscoveryQueue, VersionMigrationStage, VersionNotifiers, VersionNotifyTargets,
 };
 use frame_support::{
-	assert_noop, assert_ok,
+	assert_err, assert_noop, assert_ok,
 	traits::{Currency, Hooks},
 	weights::Weight,
 };
@@ -449,19 +449,19 @@ fn trapped_assets_can_be_claimed() {
 		assert_eq!(Balances::total_balance(&BOB), INITIAL_BALANCE + SEND_AMOUNT);
 		assert_eq!(AssetTraps::<Test>::iter().collect::<Vec<_>>(), vec![]);
 
-		let weight = BaseXcmWeight::get() * 3;
-		assert_ok!(<XcmPallet as xcm_builder::ExecuteController<_, _>>::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!(
+			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::<Test>::LocalExecutionIncomplete
+		);
 	});
 }
 

polkadot/xcm/xcm-builder/src/controller.rs

@@ -52,7 +52,8 @@ pub trait ExecuteController<Origin, RuntimeCall> {
 	/// 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 +64,7 @@ pub trait ExecuteController<Origin, RuntimeCall> {
 		origin: Origin,
 		message: Box<VersionedXcm<RuntimeCall>>,
 		max_weight: Weight,
-	) -> Result<Outcome, DispatchError>;
+	) -> Result<Weight, DispatchError>;
 }
 
 /// Weight functions needed for [`SendController`].
@@ -137,8 +138,8 @@ impl<Origin, RuntimeCall> ExecuteController<Origin, RuntimeCall> for () {
 		_origin: Origin,
 		_message: Box<VersionedXcm<RuntimeCall>>,
 		_max_weight: Weight,
-	) -> Result<Outcome, DispatchError> {
-		Ok(Outcome::Error { error: XcmError::Unimplemented })
+	) -> Result<Weight, DispatchError> {
+		Err(DispatchError::Other("ExecuteController::execute not implemented"))
 	}
 }
 

substrate/frame/contracts/fixtures/contracts/xcm_execute.rs

@@ -30,10 +30,6 @@ 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);
+	api::xcm_execute(msg).unwrap();
 }

substrate/frame/contracts/mock-network/src/tests.rs

@@ -21,15 +21,14 @@ 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,
+	assert_err, assert_ok,
 	pallet_prelude::Weight,
 	traits::{fungibles::Mutate, Currency},
 };
 use pallet_balances::{BalanceLock, Reasons};
-use pallet_contracts::{Code, CollectEvents, DebugInfo, Determinism};
+use pallet_contracts::{Code, CollectEvents, DebugInfo, Determinism, Error};
 use pallet_contracts_fixtures::compile_module;
 use xcm::{v4::prelude::*, VersionedLocation, VersionedXcm};
 use xcm_simulator::TestExt;
@@ -100,19 +99,16 @@ 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_ok!(result.result);
 
 		// 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);
 	});
 }
 
@@ -183,15 +179,10 @@ fn test_xcm_execute_reentrant_call() {
 			CollectEvents::UnsafeCollect,
 			Determinism::Enforced,
 		)
-		.result
-		.unwrap();
+		.result;
 
-		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 }
-		);
+		// Since we don't refund the precharged gas, the call fails with an out of gas error.
+		assert_err!(result, Error::<parachain::Runtime>::OutOfGas);
 
 		// Funds should not change hands as the XCM transact failed.
 		assert_eq!(ParachainBalances::free_balance(BOB), INITIAL_BALANCE);

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: <Self::T as Config>::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

substrate/frame/contracts/src/gas.rs

@@ -63,6 +63,11 @@ pub trait Token<T: Config>: 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`.
@@ -178,7 +183,9 @@ impl<T: Config> GasMeter<T> {
 	/// 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<Tok: Token<T>>(&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);
 	}

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