Revert execute_blob and send_blob (#4266)

Revert "pallet-xcm: Deprecate `execute` and `send` in favor of
`execute_blob` and `send_blob` (#3749)"

This reverts commit feee773d15.

---------

Co-authored-by: Adrian Catangiu <adrian@parity.io>
Co-authored-by: Javier Bullrich <javier@bullrich.dev>

substrate/frame/contracts/mock-network/src/parachain/contracts_config.rs

@@ -14,9 +14,8 @@
 // You should have received a copy of the GNU General Public License
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
-use super::{Balances, Runtime, RuntimeCall, RuntimeEvent};
-use crate::parachain::RuntimeHoldReason;
-use frame_support::{derive_impl, parameter_types};
+use super::{Balances, Runtime, RuntimeCall, RuntimeEvent, RuntimeHoldReason};
+use frame_support::{derive_impl, parameter_types, traits::Contains};
 
 parameter_types! {
 	pub Schedule: pallet_contracts::Schedule<Runtime> = Default::default();
@@ -29,5 +28,14 @@ impl pallet_contracts::Config for Runtime {
 	type Currency = Balances;
 	type Schedule = Schedule;
 	type Time = super::Timestamp;
+	type CallFilter = CallFilter;
 	type Xcm = pallet_xcm::Pallet<Self>;
 }
+
+/// In this mock, we only allow other contract calls via XCM.
+pub struct CallFilter;
+impl Contains<RuntimeCall> for CallFilter {
+	fn contains(call: &RuntimeCall) -> bool {
+		matches!(call, RuntimeCall::Contracts(pallet_contracts::Call::call { .. }))
+	}
+}

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

@@ -22,7 +22,10 @@ use crate::{
 	relay_chain, MockNet, ParaA, ParachainBalances, Relay, ALICE, BOB, INITIAL_BALANCE,
 };
 use codec::{Decode, Encode};
-use frame_support::traits::{fungibles::Mutate, Currency};
+use frame_support::{
+	assert_err,
+	traits::{fungibles::Mutate, Currency},
+};
 use pallet_contracts::{test_utils::builder::*, Code};
 use pallet_contracts_fixtures::compile_module;
 use pallet_contracts_uapi::ReturnErrorCode;
@@ -81,7 +84,7 @@ fn test_xcm_execute() {
 			.build();
 
 		let result = bare_call(contract_addr.clone())
-			.data(VersionedXcm::V4(message).encode().encode())
+			.data(VersionedXcm::V4(message).encode())
 			.build();
 
 		assert_eq!(result.gas_consumed, result.gas_required);
@@ -118,7 +121,7 @@ fn test_xcm_execute_incomplete() {
 			.build();
 
 		let result = bare_call(contract_addr.clone())
-			.data(VersionedXcm::V4(message).encode().encode())
+			.data(VersionedXcm::V4(message).encode())
 			.build();
 
 		assert_eq!(result.gas_consumed, result.gas_required);
@@ -129,6 +132,26 @@ fn test_xcm_execute_incomplete() {
 	});
 }
 
+#[test]
+fn test_xcm_execute_filtered_call() {
+	MockNet::reset();
+
+	let contract_addr = instantiate_test_contract("xcm_execute");
+
+	ParaA::execute_with(|| {
+		// `remark`  should be rejected, as it is not allowed by our CallFilter.
+		let call = parachain::RuntimeCall::System(frame_system::Call::remark { remark: vec![] });
+		let message: Xcm<parachain::RuntimeCall> = Xcm::builder_unsafe()
+			.transact(OriginKind::Native, Weight::MAX, call.encode())
+			.build();
+		let result = bare_call(contract_addr.clone())
+			.data(VersionedXcm::V4(message).encode())
+			.build()
+			.result;
+		assert_err!(result, frame_system::Error::<parachain::Runtime>::CallFiltered);
+	});
+}
+
 #[test]
 fn test_xcm_execute_reentrant_call() {
 	MockNet::reset();
@@ -151,7 +174,7 @@ fn test_xcm_execute_reentrant_call() {
 			.build();
 
 		let result = bare_call(contract_addr.clone())
-			.data(VersionedXcm::V4(message).encode().encode())
+			.data(VersionedXcm::V4(message).encode())
 			.build_and_unwrap_result();
 
 		assert_return_code!(&result, ReturnErrorCode::XcmExecutionFailed);
@@ -182,7 +205,7 @@ fn test_xcm_send() {
 			.build();
 
 		let result = bare_call(contract_addr.clone())
-			.data((dest, VersionedXcm::V4(message).encode()).encode())
+			.data((dest, VersionedXcm::V4(message)).encode())
 			.build_and_unwrap_result();
 
 		let mut data = &result.data[..];

substrate/frame/contracts/src/lib.rs

@@ -307,9 +307,6 @@ pub mod pallet {
 		/// Therefore please make sure to be restrictive about which dispatchables are allowed
 		/// in order to not introduce a new DoS vector like memory allocation patterns that can
 		/// be exploited to drive the runtime into a panic.
-		///
-		/// This filter does not apply to XCM transact calls. To impose restrictions on XCM transact
-		/// calls, you must configure them separately within the XCM pallet itself.
 		#[pallet::no_default_bounds]
 		type CallFilter: Contains<<Self as frame_system::Config>::RuntimeCall>;
 

substrate/frame/contracts/src/wasm/runtime.rs

@@ -25,8 +25,12 @@ use crate::{
 };
 use codec::{Decode, DecodeLimit, Encode, MaxEncodedLen};
 use frame_support::{
-	dispatch::DispatchInfo, ensure, pallet_prelude::DispatchResultWithPostInfo, parameter_types,
-	traits::Get, weights::Weight,
+	dispatch::DispatchInfo,
+	ensure,
+	pallet_prelude::{DispatchResult, DispatchResultWithPostInfo},
+	parameter_types,
+	traits::Get,
+	weights::Weight,
 };
 use pallet_contracts_proc_macro::define_env;
 use pallet_contracts_uapi::{CallFlags, ReturnFlags};
@@ -37,6 +41,9 @@ use sp_runtime::{
 };
 use sp_std::{fmt, prelude::*};
 use wasmi::{core::HostError, errors::LinkerError, Linker, Memory, Store};
+use xcm::VersionedXcm;
+
+type CallOf<T> = <T as frame_system::Config>::RuntimeCall;
 
 /// The maximum nesting depth a contract can use when encoding types.
 const MAX_DECODE_NESTING: u32 = 256;
@@ -371,6 +378,29 @@ fn already_charged(_: u32) -> Option<RuntimeCosts> {
 	None
 }
 
+/// Ensure that the XCM program is executable, by checking that it does not contain any [`Transact`]
+/// instruction with a call that is not allowed by the CallFilter.
+fn ensure_executable<T: Config>(message: &VersionedXcm<CallOf<T>>) -> DispatchResult {
+	use frame_support::traits::Contains;
+	use xcm::prelude::{Transact, Xcm};
+
+	let mut message: Xcm<CallOf<T>> =
+		message.clone().try_into().map_err(|_| Error::<T>::XCMDecodeFailed)?;
+
+	message.iter_mut().try_for_each(|inst| -> DispatchResult {
+		let Transact { ref mut call, .. } = inst else { return Ok(()) };
+		let call = call.ensure_decoded().map_err(|_| Error::<T>::XCMDecodeFailed)?;
+
+		if !<T as Config>::CallFilter::contains(call) {
+			return Err(frame_system::Error::<T>::CallFiltered.into())
+		}
+
+		Ok(())
+	})?;
+
+	Ok(())
+}
+
 /// Can only be used for one call.
 pub struct Runtime<'a, E: Ext + 'a> {
 	ext: &'a mut E,
@@ -2082,13 +2112,16 @@ pub mod env {
 		msg_len: u32,
 	) -> Result<ReturnErrorCode, TrapReason> {
 		use frame_support::dispatch::DispatchInfo;
+		use xcm::VersionedXcm;
 		use xcm_builder::{ExecuteController, ExecuteControllerWeightInfo};
 
 		ctx.charge_gas(RuntimeCosts::CopyFromContract(msg_len))?;
-		let message = ctx.read_sandbox_memory_as_unbounded(memory, msg_ptr, msg_len)?;
+		let message: VersionedXcm<CallOf<E::T>> =
+			ctx.read_sandbox_memory_as_unbounded(memory, msg_ptr, msg_len)?;
+		ensure_executable::<E::T>(&message)?;
 
 		let execute_weight =
-			<<E::T as Config>::Xcm as ExecuteController<_, _>>::WeightInfo::execute_blob();
+			<<E::T as Config>::Xcm as ExecuteController<_, _>>::WeightInfo::execute();
 		let weight = ctx.ext.gas_meter().gas_left().max(execute_weight);
 		let dispatch_info = DispatchInfo { weight, ..Default::default() };
 
@@ -2097,9 +2130,9 @@ pub mod env {
 			RuntimeCosts::CallXcmExecute,
 			|ctx| {
 				let origin = crate::RawOrigin::Signed(ctx.ext.address().clone()).into();
-				let weight_used = <<E::T as Config>::Xcm>::execute_blob(
+				let weight_used = <<E::T as Config>::Xcm>::execute(
 					origin,
-					message,
+					Box::new(message),
 					weight.saturating_sub(execute_weight),
 				)?;
 
@@ -2119,18 +2152,19 @@ pub mod env {
 		msg_len: u32,
 		output_ptr: u32,
 	) -> Result<ReturnErrorCode, TrapReason> {
-		use xcm::VersionedLocation;
+		use xcm::{VersionedLocation, VersionedXcm};
 		use xcm_builder::{SendController, SendControllerWeightInfo};
 
 		ctx.charge_gas(RuntimeCosts::CopyFromContract(msg_len))?;
 		let dest: VersionedLocation = ctx.read_sandbox_memory_as(memory, dest_ptr)?;
 
-		let message = ctx.read_sandbox_memory_as_unbounded(memory, msg_ptr, msg_len)?;
-		let weight = <<E::T as Config>::Xcm as SendController<_>>::WeightInfo::send_blob();
+		let message: VersionedXcm<()> =
+			ctx.read_sandbox_memory_as_unbounded(memory, msg_ptr, msg_len)?;
+		let weight = <<E::T as Config>::Xcm as SendController<_>>::WeightInfo::send();
 		ctx.charge_gas(RuntimeCosts::CallRuntime(weight))?;
 		let origin = crate::RawOrigin::Signed(ctx.ext.address().clone()).into();
 
-		match <<E::T as Config>::Xcm>::send_blob(origin, dest.into(), message) {
+		match <<E::T as Config>::Xcm>::send(origin, dest.into(), message.into()) {
 			Ok(message_id) => {
 				ctx.write_sandbox_memory(memory, output_ptr, &message_id.encode())?;
 				Ok(ReturnErrorCode::Success)

substrate/frame/contracts/uapi/src/host.rs

@@ -790,7 +790,7 @@ pub trait HostFn {
 	///
 	/// # Parameters
 	///
-	/// - `dest`: The XCM destination, should be decodable as [MultiLocation](https://paritytech.github.io/polkadot-sdk/master/staging_xcm/enum.VersionedLocation.html),
+	/// - `dest`: The XCM destination, should be decodable as [VersionedLocation](https://paritytech.github.io/polkadot-sdk/master/staging_xcm/enum.VersionedLocation.html),
 	///   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.