XCM: Allow reclaim of assets dropped from holding (#3727)

* XCM: Introduce AssetTrap

* Revert reversions

* Remove attempts at weighing and add test

* Less storage use for asset trapping

* Add missing file

* Fixes

* Fixes

* Formatting

* Fixes

* Docs

* Filter types to allow runtimes to dictate which assets/origins should be trapped

* Formatting

* Tests

* Formatting

* Fixes

* Docs

polkadot/xcm/xcm-executor/src/assets.rs

@@ -94,6 +94,11 @@ impl Assets {
 		self.fungible.len() + self.non_fungible.len()
 	}
 
+	/// Returns `true` if `self` contains no assets.
+	pub fn is_empty(&self) -> bool {
+		self.fungible.is_empty() && self.non_fungible.is_empty()
+	}
+
 	/// A borrowing iterator over the fungible assets.
 	pub fn fungible_assets_iter<'a>(&'a self) -> impl Iterator<Item = MultiAsset> + 'a {
 		self.fungible

polkadot/xcm/xcm-executor/src/config.rs

@@ -15,8 +15,8 @@
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
 use crate::traits::{
-	ConvertOrigin, FilterAssetLocation, InvertLocation, OnResponse, ShouldExecute, TransactAsset,
-	WeightBounds, WeightTrader,
+	ClaimAssets, ConvertOrigin, DropAssets, FilterAssetLocation, InvertLocation, OnResponse,
+	ShouldExecute, TransactAsset, WeightBounds, WeightTrader,
 };
 use frame_support::{
 	dispatch::{Dispatchable, Parameter},
@@ -58,4 +58,11 @@ pub trait Config {
 
 	/// What to do when a response of a query is found.
 	type ResponseHandler: OnResponse;
+
+	/// The general asset trap - handler for when assets are left in the Holding Register at the
+	/// end of execution.
+	type AssetTrap: DropAssets;
+
+	/// The handler for when there is an instruction to claim assets.
+	type AssetClaims: ClaimAssets;
 }

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

@@ -31,8 +31,8 @@ use xcm::latest::{
 
 pub mod traits;
 use traits::{
-	ConvertOrigin, FilterAssetLocation, InvertLocation, OnResponse, ShouldExecute, TransactAsset,
-	WeightBounds, WeightTrader,
+	ClaimAssets, ConvertOrigin, DropAssets, FilterAssetLocation, InvertLocation, OnResponse,
+	ShouldExecute, TransactAsset, WeightBounds, WeightTrader,
 };
 
 mod assets;
@@ -86,7 +86,6 @@ impl<Config: config::Config> ExecuteXcm<Config::Call> for XcmExecutor<Config> {
 		if xcm_weight > weight_limit {
 			return Outcome::Error(XcmError::WeightLimitReached(xcm_weight))
 		}
-		let origin = Some(origin);
 
 		if let Err(_) = Config::Barrier::should_execute(
 			&origin,
@@ -98,7 +97,7 @@ impl<Config: config::Config> ExecuteXcm<Config::Call> for XcmExecutor<Config> {
 			return Outcome::Error(XcmError::Barrier)
 		}
 
-		let mut vm = Self::new(origin);
+		let mut vm = Self::new(origin.clone());
 
 		while !message.0.is_empty() {
 			let result = vm.execute(message);
@@ -116,9 +115,12 @@ impl<Config: config::Config> ExecuteXcm<Config::Call> for XcmExecutor<Config> {
 		vm.refund_surplus();
 		drop(vm.trader);
 
-		// TODO #2841: Do something with holding? (Fail-safe AssetTrap?)
+		let mut weight_used = xcm_weight.saturating_sub(vm.total_surplus);
+
+		if !vm.holding.is_empty() {
+			weight_used.saturating_accrue(Config::AssetTrap::drop_assets(&origin, vm.holding));
+		};
 
-		let weight_used = xcm_weight.saturating_sub(vm.total_surplus);
 		match vm.error {
 			None => Outcome::Complete(weight_used),
 			// TODO: #2841 #REALWEIGHT We should deduct the cost of any instructions following
@@ -129,10 +131,10 @@ impl<Config: config::Config> ExecuteXcm<Config::Call> for XcmExecutor<Config> {
 }
 
 impl<Config: config::Config> XcmExecutor<Config> {
-	fn new(origin: Option<MultiLocation>) -> Self {
+	fn new(origin: MultiLocation) -> Self {
 		Self {
 			holding: Assets::new(),
-			origin,
+			origin: Some(origin),
 			trader: Config::Trader::new(),
 			error: None,
 			total_surplus: 0,
@@ -183,7 +185,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
 	/// Drop the registered error handler and refund its weight.
 	fn drop_error_handler(&mut self) {
 		self.error_handler = Xcm::<Config::Call>(vec![]);
-		self.total_surplus = self.total_surplus.saturating_add(self.error_handler_weight);
+		self.total_surplus.saturating_accrue(self.error_handler_weight);
 		self.error_handler_weight = 0;
 	}
 
@@ -199,7 +201,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
 	fn refund_surplus(&mut self) {
 		let current_surplus = self.total_surplus.saturating_sub(self.total_refunded);
 		if current_surplus > 0 {
-			self.total_refunded = self.total_refunded.saturating_add(current_surplus);
+			self.total_refunded.saturating_accrue(current_surplus);
 			if let Some(w) = self.trader.refund_weight(current_surplus) {
 				self.holding.subsume(w);
 			}
@@ -300,7 +302,7 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				// reported back to the caller and this ensures that they account for the total
 				// weight consumed correctly (potentially allowing them to do more operations in a
 				// block than they otherwise would).
-				self.total_surplus = self.total_surplus.saturating_add(surplus);
+				self.total_surplus.saturating_accrue(surplus);
 				Ok(())
 			},
 			QueryResponse { query_id, response, max_weight } => {
@@ -390,14 +392,14 @@ impl<Config: config::Config> XcmExecutor<Config> {
 			},
 			SetErrorHandler(mut handler) => {
 				let handler_weight = Config::Weigher::weight(&mut handler)?;
-				self.total_surplus = self.total_surplus.saturating_add(self.error_handler_weight);
+				self.total_surplus.saturating_accrue(self.error_handler_weight);
 				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.total_surplus.saturating_accrue(self.appendix_weight);
 				self.appendix = appendix;
 				self.appendix_weight = appendix_weight;
 				Ok(())
@@ -406,6 +408,16 @@ impl<Config: config::Config> XcmExecutor<Config> {
 				self.error = None;
 				Ok(())
 			},
+			ClaimAsset { assets, ticket } => {
+				let origin = self.origin.as_ref().ok_or(XcmError::BadOrigin)?;
+				let ok = Config::AssetClaims::claim_assets(origin, &ticket, &assets);
+				ensure!(ok, XcmError::UnknownClaim);
+				for asset in assets.drain().into_iter() {
+					self.holding.subsume(asset);
+				}
+				Ok(())
+			},
+			Trap(code) => Err(XcmError::Trap(code)),
 			ExchangeAsset { .. } => Err(XcmError::Unimplemented),
 			HrmpNewChannelOpenRequest { .. } => Err(XcmError::Unimplemented),
 			HrmpChannelAccepted { .. } => Err(XcmError::Unimplemented),

polkadot/xcm/xcm-executor/src/traits/drop_assets.rs

@@ -0,0 +1,79 @@
+// Copyright 2020 Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Polkadot is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
+
+use crate::Assets;
+use core::marker::PhantomData;
+use frame_support::{traits::Contains, weights::Weight};
+use xcm::latest::{MultiAssets, MultiLocation};
+
+/// Define a handler for when some non-empty `Assets` value should be dropped.
+pub trait DropAssets {
+	/// Handler for receiving dropped assets. Returns the weight consumed by this operation.
+	fn drop_assets(origin: &MultiLocation, assets: Assets) -> Weight;
+}
+impl DropAssets for () {
+	fn drop_assets(_origin: &MultiLocation, _assets: Assets) -> Weight {
+		0
+	}
+}
+
+/// Morph a given `DropAssets` implementation into one which can filter based on assets. This can
+/// be used to ensure that `Assets` values which hold no value are ignored.
+pub struct FilterAssets<D, A>(PhantomData<(D, A)>);
+
+impl<D: DropAssets, A: Contains<Assets>> DropAssets for FilterAssets<D, A> {
+	fn drop_assets(origin: &MultiLocation, assets: Assets) -> Weight {
+		if A::contains(&assets) {
+			D::drop_assets(origin, assets)
+		} else {
+			0
+		}
+	}
+}
+
+/// Morph a given `DropAssets` implementation into one which can filter based on origin. This can
+/// be used to ban origins which don't have proper protections/policies against misuse of the
+/// asset trap facility don't get to use it.
+pub struct FilterOrigin<D, O>(PhantomData<(D, O)>);
+
+impl<D: DropAssets, O: Contains<MultiLocation>> DropAssets for FilterOrigin<D, O> {
+	fn drop_assets(origin: &MultiLocation, assets: Assets) -> Weight {
+		if O::contains(origin) {
+			D::drop_assets(origin, assets)
+		} else {
+			0
+		}
+	}
+}
+
+/// Define any handlers for the `AssetClaim` instruction.
+pub trait ClaimAssets {
+	/// Claim any assets available to `origin` and return them in a single `Assets` value, together
+	/// with the weight used by this operation.
+	fn claim_assets(origin: &MultiLocation, ticket: &MultiLocation, what: &MultiAssets) -> bool;
+}
+
+#[impl_trait_for_tuples::impl_for_tuples(30)]
+impl ClaimAssets for Tuple {
+	fn claim_assets(origin: &MultiLocation, ticket: &MultiLocation, what: &MultiAssets) -> bool {
+		for_tuples!( #(
+			if Tuple::claim_assets(origin, ticket, what) {
+				return true;
+			}
+		)* );
+		false
+	}
+}

polkadot/xcm/xcm-executor/src/traits/mod.rs

@@ -18,6 +18,8 @@
 
 mod conversion;
 pub use conversion::{Convert, ConvertOrigin, Decoded, Encoded, Identity, InvertLocation, JustTry};
+mod drop_assets;
+pub use drop_assets::{ClaimAssets, DropAssets};
 mod filter_asset_location;
 pub use filter_asset_location::FilterAssetLocation;
 mod matches_fungible;

polkadot/xcm/xcm-executor/src/traits/should_execute.rs

@@ -34,7 +34,7 @@ pub trait ShouldExecute {
 	///   message may utilize in its execution. Typically non-zero only because of prior fee
 	///   payment, but could in principle be due to other factors.
 	fn should_execute<Call>(
-		origin: &Option<MultiLocation>,
+		origin: &MultiLocation,
 		top_level: bool,
 		message: &mut Xcm<Call>,
 		max_weight: Weight,
@@ -45,7 +45,7 @@ pub trait ShouldExecute {
 #[impl_trait_for_tuples::impl_for_tuples(30)]
 impl ShouldExecute for Tuple {
 	fn should_execute<Call>(
-		origin: &Option<MultiLocation>,
+		origin: &MultiLocation,
 		top_level: bool,
 		message: &mut Xcm<Call>,
 		max_weight: Weight,