Formatting

runtime/kusama/src/lib.rs

@@ -1254,6 +1254,9 @@ type LocalOriginConverter = (
 parameter_types! {
 	/// The amount of weight an XCM operation takes. This is a safe overestimate.
 	pub const BaseXcmWeight: Weight = 1_000_000_000;
+	/// Maximum number of instructions in a single XCM fragment. A sanity check against weight
+	/// calculations getting too crazy.
+	pub const MaxInstructions: u32 = 100;
 }
 
 /// The XCM router. When we want to send an XCM message, we use this type. It amalgamates all of our
@@ -1289,14 +1292,15 @@ impl xcm_executor::Config for XcmConfig {
 	type IsTeleporter = TrustedTeleporters;
 	type LocationInverter = LocationInverter<Ancestry>;
 	type Barrier = Barrier;
-	type Weigher = FixedWeightBounds<BaseXcmWeight, Call>;
+	type Weigher = FixedWeightBounds<BaseXcmWeight, Call, MaxInstructions>;
 	// The weight trader piggybacks on the existing transaction-fee conversion logic.
 	type Trader = UsingComponents<WeightToFee, KsmLocation, AccountId, Balances, ToAuthor<Runtime>>;
 	type ResponseHandler = ();
 }
 
 parameter_types! {
 	pub const CouncilBodyId: BodyId = BodyId::Executive;
+	pub const MaxInstructions: u32 = 100;
 }
 
 /// Type to convert an `Origin` type value into a `MultiLocation` value which represents an interior location
@@ -1324,7 +1328,7 @@ impl pallet_xcm::Config for Runtime {
 	type XcmExecutor = XcmExecutor<XcmConfig>;
 	type XcmTeleportFilter = Everything;
 	type XcmReserveTransferFilter = Everything;
-	type Weigher = FixedWeightBounds<BaseXcmWeight, Call>;
+	type Weigher = FixedWeightBounds<BaseXcmWeight, Call, MaxInstructions>;
 	type LocationInverter = LocationInverter<Ancestry>;
 	type Origin = Origin;
 	type Call = Call;

xcm/pallet-xcm/src/tests.rs

@@ -43,9 +43,11 @@ fn report_outcome_notify_works() {
 		assert_eq!(
 			message,
 			Xcm(vec![
-				SetAppendix(Xcm(vec![
-					ReportError { query_id: 0, dest: Parent.into(), max_response_weight: 1_000_000 },
-				])),
+				SetAppendix(Xcm(vec![ReportError {
+					query_id: 0,
+					dest: Parent.into(),
+					max_response_weight: 1_000_000
+				},])),
 				TransferAsset { assets: (Here, SEND_AMOUNT).into(), beneficiary: sender.clone() },
 			])
 		);
@@ -95,9 +97,11 @@ fn report_outcome_works() {
 		assert_eq!(
 			message,
 			Xcm(vec![
-				SetAppendix(Xcm(vec![
-					ReportError { query_id: 0, dest: Parent.into(), max_response_weight: 0 },
-				])),
+				SetAppendix(Xcm(vec![ReportError {
+					query_id: 0,
+					dest: Parent.into(),
+					max_response_weight: 0
+				},])),
 				TransferAsset { assets: (Here, SEND_AMOUNT).into(), beneficiary: sender.clone() },
 			])
 		);

xcm/src/v2/mod.rs

@@ -441,22 +441,22 @@ pub enum Instruction<Call> {
 	RefundSurplus,
 
 	/// Set code that should be called in the case of an error happening.
-	/// 
-	/// An error occuring within execution of this code will _NOT_ result in the error register
+	///
+	/// An error occurring within execution of this code will _NOT_ result in the error register
 	/// being set, nor will an error handler be called due to it. The error handler and appendix
 	/// may each still be set.
-	/// 
+	///
 	/// The apparent weight of this instruction is inclusive of the inner `Xcm`; the executing
 	/// weight however includes only the difference between the previous handler and the new
 	/// handler, which can reasonably be negative, which would result in a surplus.
 	SetErrorHandler(Xcm<Call>),
 
 	/// Set code that should be called after code execution (including the error handler if any)
 	/// is finished. This will be called regardless of whether an error occurred.
-	/// 
-	/// Any error occuring due to execution of this code will result in the error register being
+	///
+	/// Any error occurring due to execution of this code will result in the error register being
 	/// set, and the error handler (if set) firing.
-	/// 
+	///
 	/// The apparent weight of this instruction is inclusive of the inner `Xcm`; the executing
 	/// weight however includes only the difference between the previous appendix and the new
 	/// appendix, which can reasonably be negative, which would result in a surplus.

xcm/xcm-builder/src/mock.rs

@@ -259,6 +259,7 @@ parameter_types! {
 	pub static AllowPaidFrom: Vec<MultiLocation> = vec![];
 	// 1_000_000_000_000 => 1 unit of asset for 1 unit of Weight.
 	pub static WeightPrice: (AssetId, u128) = (From::from(Here), 1_000_000_000_000);
+	pub static MaxInstructions: u32 = 100;
 }
 
 pub type TestBarrier = (
@@ -278,7 +279,7 @@ impl Config for TestConfig {
 	type IsTeleporter = TestIsTeleporter;
 	type LocationInverter = LocationInverter<TestAncestry>;
 	type Barrier = TestBarrier;
-	type Weigher = FixedWeightBounds<UnitWeightCost, TestCall>;
+	type Weigher = FixedWeightBounds<UnitWeightCost, TestCall, MaxInstructions>;
 	type Trader = FixedRateOfFungible<WeightPrice, ()>;
 	type ResponseHandler = TestResponseHandler;
 }

xcm/xcm-builder/src/tests.rs

@@ -219,12 +219,10 @@ fn code_registers_should_work() {
 			ClearError,
 		])),
 		// Set the appendix - this will always fire.
-		SetAppendix(Xcm(vec![
-			TransferAsset {
-				assets: (Here, 4).into(),
-				beneficiary: X1(AccountIndex64 { index: 3, network: Any }).into(),
-			},
-		])),
+		SetAppendix(Xcm(vec![TransferAsset {
+			assets: (Here, 4).into(),
+			beneficiary: X1(AccountIndex64 { index: 3, network: Any }).into(),
+		}])),
 		// First xfer always works ok
 		TransferAsset {
 			assets: (Here, 1).into(),
@@ -241,13 +239,13 @@ fn code_registers_should_work() {
 	assert_eq!(limit, 70);
 
 	let r = XcmExecutor::<TestConfig>::execute_xcm(Here.into(), message.clone(), limit);
-	assert_eq!(r, Outcome::Complete(50));	// We don't pay the 20 weight for the error handler. 
+	assert_eq!(r, Outcome::Complete(50)); // We don't pay the 20 weight for the error handler.
 	assert_eq!(assets(3), vec![(Here, 13).into()]);
 	assert_eq!(assets(3000), vec![(Here, 8).into()]);
 	assert_eq!(sent_xcm(), vec![]);
 
 	let r = XcmExecutor::<TestConfig>::execute_xcm(Here.into(), message, limit);
-	assert_eq!(r, Outcome::Complete(70));	// We pay the full weight here.
+	assert_eq!(r, Outcome::Complete(70)); // We pay the full weight here.
 	assert_eq!(assets(3), vec![(Here, 20).into()]);
 	assert_eq!(assets(3000), vec![(Here, 1).into()]);
 	assert_eq!(sent_xcm(), vec![]);

xcm/xcm-builder/src/weight.rs

@@ -27,22 +27,29 @@ use xcm_executor::{
 	Assets,
 };
 
-pub struct FixedWeightBounds<T, C>(PhantomData<(T, C)>);
-impl<T: Get<Weight>, C: Decode + GetDispatchInfo> WeightBounds<C> for FixedWeightBounds<T, C> {
+pub struct FixedWeightBounds<T, C, M>(PhantomData<(T, C, M)>);
+impl<T: Get<Weight>, C: Decode + GetDispatchInfo, M: Get<u32>> WeightBounds<C>
+	for FixedWeightBounds<T, C, M>
+{
 	fn weight(message: &mut Xcm<C>) -> Result<Weight, ()> {
+		let mut instructions_left = M::get();
+		Self::weight_with_limit(message, &mut instructions_left)
+	}
+}
+
+impl<T: Get<Weight>, C: Decode + GetDispatchInfo> FixedWeightBounds<T, C> {
+	fn weight_with_limit(message: &mut Xcm<C>, instrs_limit: &mut u32) -> Result<Weight, ()> {
 		let mut r = 0;
+		*instrs_limit = instrs_limit.checked_sub(message.0.len() as u32).ok_or(())?;
 		for m in message.0.iter_mut() {
-			r += Self::instr_weight(m)?;
+			r += Self::instr_weight(m, instrs_limit)?;
 		}
 		Ok(r)
 	}
-}
-
-impl<T: Get<Weight>, C: Decode + GetDispatchInfo> FixedWeightBounds<T, C> {
-	fn instr_weight(message: &mut Instruction<C>) -> Result<Weight, ()> {
+	fn instr_weight(message: &mut Instruction<C>, instrs_limit: &mut u32) -> Result<Weight, ()> {
 		Ok(T::get().saturating_add(match message {
 			Transact { require_weight_at_most, .. } => *require_weight_at_most,
-			SetErrorHandler(xcm) | SetAppendix(xcm) => Self::weight(xcm)?,
+			SetErrorHandler(xcm) | SetAppendix(xcm) => Self::weight_with_limit(xcm, instrs_limit)?,
 			_ => 0,
 		}))
 	}

xcm/xcm-executor/src/lib.rs

@@ -86,17 +86,19 @@ impl<Config: config::Config> ExecuteXcm<Config::Call> for XcmExecutor<Config> {
 			return Outcome::Error(XcmError::WeightLimitReached(xcm_weight))
 		}
 		let origin = Some(origin);
-		
+
 		if Config::Barrier::should_execute(
-			&origin, 
-			true, 
-			&mut message, 
-			xcm_weight, 
+			&origin,
+			true,
+			&mut message,
+			xcm_weight,
 			&mut weight_credit,
-		).is_err() {
+		)
+		.is_err()
+		{
 			return Outcome::Error(XcmError::Barrier)
 		}
-		
+
 		let mut vm = Self::new(origin);
 
 		while !message.0.is_empty() {
@@ -147,10 +149,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
 
 	/// Execute the XCM program fragment and report back the error and which instruction caused it,
 	/// or `Ok` if there was no error.
-	fn execute(
-		&mut self,
-		xcm: Xcm<Config::Call>,
-	) -> Result<(), (u32, XcmError)> {
+	fn execute(&mut self, xcm: Xcm<Config::Call>) -> Result<(), (u32, XcmError)> {
 		log::trace!(
 			target: "xcm::execute",
 			"origin: {:?}, total_surplus/refunded: {:?}/{:?}, error_handler_weight: {:?}",
@@ -161,20 +160,20 @@ impl<Config: config::Config> XcmExecutor<Config> {
 		);
 		for (i, instr) in xcm.0.into_iter().enumerate() {
 			if let Err(e) = self.process_instruction(instr) {
-				return Err((i as u32, e));
+				return Err((i as u32, e))
 			}
 		}
 		return Ok(())
 	}
-	
+
 	/// Remove the registered error handler and return it. Do not refund its weight.
-	fn take_error_handler(&mut self) -> Xcm::<Config::Call> {
+	fn take_error_handler(&mut self) -> Xcm<Config::Call> {
 		let mut r = Xcm::<Config::Call>(vec![]);
 		sp_std::mem::swap(&mut self.error_handler, &mut r);
 		self.error_handler_weight = 0;
 		r
 	}
-	
+
 	/// Drop the registered error handler and refund its weight.
 	fn drop_error_handler(&mut self) {
 		self.error_handler = Xcm::<Config::Call>(vec![]);
@@ -183,7 +182,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
 	}
 
 	/// Remove the registered appendix and return it.
-	fn take_appendix(&mut self) -> Xcm::<Config::Call> {
+	fn take_appendix(&mut self) -> Xcm<Config::Call> {
 		let mut r = Xcm::<Config::Call>(vec![]);
 		sp_std::mem::swap(&mut self.appendix, &mut r);
 		self.appendix_weight = 0;
@@ -201,7 +200,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
 		}
 	}
 
-	/// Process a single XCM instruction, mutating the state of the XCM VM.
+	/// Process a single XCM instruction, mutating the state of the XCM virtual machine.
 	fn process_instruction(&mut self, instr: Instruction<Config::Call>) -> Result<(), XcmError> {
 		match instr {
 			WithdrawAsset(assets) => {
@@ -303,7 +302,8 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				Config::ResponseHandler::on_response(origin, query_id, response, max_weight);
 				Ok(())
 			},
-			DescendOrigin(who) => self.origin
+			DescendOrigin(who) => self
+				.origin
 				.as_mut()
 				.ok_or(XcmError::BadOrigin)?
 				.append_with(who)
@@ -371,9 +371,8 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				// should be executed.
 				if let Some(weight) = Option::<u64>::from(weight_limit) {
 					// pay for `weight` using up to `fees` of the holding register.
-					let max_fee = self.holding
-						.try_take(fees.into())
-						.map_err(|_| XcmError::NotHoldingFees)?;
+					let max_fee =
+						self.holding.try_take(fees.into()).map_err(|_| XcmError::NotHoldingFees)?;
 					let unspent = self.trader.buy_weight(weight, max_fee)?;
 					self.holding.subsume_assets(unspent);
 				}
@@ -389,18 +388,18 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				self.error_handler = handler;
 				self.error_handler_weight = handler_weight;
 				Ok(())
-			}
+			},
 			SetAppendix(mut appendix) => {
 				let appendix_weight = Config::Weigher::weight(&mut appendix)?;
 				self.total_surplus = self.total_surplus.saturating_add(self.appendix_weight);
 				self.appendix = appendix;
 				self.appendix_weight = appendix_weight;
 				Ok(())
-			}
+			},
 			ClearError => {
 				self.error = None;
 				Ok(())
-			}
+			},
 			ExchangeAsset { .. } => Err(XcmError::Unimplemented),
 			HrmpNewChannelOpenRequest { .. } => Err(XcmError::Unimplemented),
 			HrmpChannelAccepted { .. } => Err(XcmError::Unimplemented),