XCM: Automatic Version Negotiation (#3736)

* XCM: Automatic Version Negotiation

* Introduce the version instructions and subscription trait

* Notification and subscription data migration

* Version change subscriptions

* Fixes

* Formatting

* Spelling

* Fixes

* Fixes

* Automatic unsubscription

* Formatting

* Expose remote origin in VM and ensure it is unchanged from actual origin in subscription instructions.

* Barrier

* Unsubscription extrinsic

* Remove top_level param

* Formatting

* Fixes

* Automatic subscription

* Formatting

* Spelling

* Unit tests for XCM executor

* Formatting

* Spellin

* Unit test for XCM pallet subscriber side

* Formatting

* More tests

* Formatting

* Fixes

* Subscription-side tests

* Formatting

* Unit tests for XCM pallet

* Formatting

* Update roadmap/implementers-guide/src/types/overseer-protocol.md

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

* Remove commented code

* Grumbles

* Multi-stage XCM version migration

* Formatting

* v1 subscriptions backport

* Warning

* Spelling

* Fix grumbles

* Formatting

* Avoid running through old notifications

* Formatting

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

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

@@ -31,7 +31,6 @@ pub struct TakeWeightCredit;
 impl ShouldExecute for TakeWeightCredit {
 	fn should_execute<Call>(
 		_origin: &MultiLocation,
-		_top_level: bool,
 		_message: &mut Xcm<Call>,
 		max_weight: Weight,
 		weight_credit: &mut Weight,
@@ -44,19 +43,17 @@ impl ShouldExecute for TakeWeightCredit {
 /// Allows execution from `origin` if it is contained in `T` (i.e. `T::Contains(origin)`) taking
 /// payments into account.
 ///
-/// Only allows for `TeleportAsset`, `WithdrawAsset` and `ReserveAssetDeposit` XCMs because they are
-/// the only ones that place assets in the Holding Register to pay for execution.
+/// Only allows for `TeleportAsset`, `WithdrawAsset`, `ClaimAsset` and `ReserveAssetDeposit` XCMs
+/// because they are the only ones that place assets in the Holding Register to pay for execution.
 pub struct AllowTopLevelPaidExecutionFrom<T>(PhantomData<T>);
 impl<T: Contains<MultiLocation>> ShouldExecute for AllowTopLevelPaidExecutionFrom<T> {
 	fn should_execute<Call>(
 		origin: &MultiLocation,
-		top_level: bool,
 		message: &mut Xcm<Call>,
 		max_weight: Weight,
 		_weight_credit: &mut Weight,
 	) -> Result<(), ()> {
 		ensure!(T::contains(origin), ());
-		ensure!(top_level, ());
 		let mut iter = message.0.iter_mut();
 		let i = iter.next().ok_or(())?;
 		match i {
@@ -90,7 +87,6 @@ pub struct AllowUnpaidExecutionFrom<T>(PhantomData<T>);
 impl<T: Contains<MultiLocation>> ShouldExecute for AllowUnpaidExecutionFrom<T> {
 	fn should_execute<Call>(
 		origin: &MultiLocation,
-		_top_level: bool,
 		_message: &mut Xcm<Call>,
 		_max_weight: Weight,
 		_weight_credit: &mut Weight,
@@ -117,7 +113,6 @@ pub struct AllowKnownQueryResponses<ResponseHandler>(PhantomData<ResponseHandler
 impl<ResponseHandler: OnResponse> ShouldExecute for AllowKnownQueryResponses<ResponseHandler> {
 	fn should_execute<Call>(
 		origin: &MultiLocation,
-		_top_level: bool,
 		message: &mut Xcm<Call>,
 		_max_weight: Weight,
 		_weight_credit: &mut Weight,
@@ -130,3 +125,21 @@ impl<ResponseHandler: OnResponse> ShouldExecute for AllowKnownQueryResponses<Res
 		}
 	}
 }
+
+/// Allows execution from `origin` if it is just a straight `SubscribeVerison` or
+/// `UnsubscribeVersion` instruction.
+pub struct AllowSubscriptionsFrom<T>(PhantomData<T>);
+impl<T: Contains<MultiLocation>> ShouldExecute for AllowSubscriptionsFrom<T> {
+	fn should_execute<Call>(
+		origin: &MultiLocation,
+		message: &mut Xcm<Call>,
+		_max_weight: Weight,
+		_weight_credit: &mut Weight,
+	) -> Result<(), ()> {
+		ensure!(T::contains(origin), ());
+		match (message.0.len(), message.0.first()) {
+			(1, Some(SubscribeVersion { .. })) | (1, Some(UnsubscribeVersion)) => Ok(()),
+			_ => Err(()),
+		}
+	}
+}

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

@@ -41,8 +41,8 @@ pub use origin_conversion::{
 
 mod barriers;
 pub use barriers::{
-	AllowKnownQueryResponses, AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom,
-	IsChildSystemParachain, TakeWeightCredit,
+	AllowKnownQueryResponses, AllowSubscriptionsFrom, AllowTopLevelPaidExecutionFrom,
+	AllowUnpaidExecutionFrom, IsChildSystemParachain, TakeWeightCredit,
 };
 
 mod currency_adapter;

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

@@ -14,6 +14,7 @@
 // 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::barriers::AllowSubscriptionsFrom;
 pub use crate::{
 	AllowKnownQueryResponses, AllowTopLevelPaidExecutionFrom, AllowUnpaidExecutionFrom,
 	FixedRateOfFungible, FixedWeightBounds, LocationInverter, TakeWeightCredit,
@@ -35,7 +36,7 @@ pub use sp_std::{
 	marker::PhantomData,
 };
 pub use xcm::latest::prelude::*;
-use xcm_executor::traits::{ClaimAssets, DropAssets};
+use xcm_executor::traits::{ClaimAssets, DropAssets, VersionChangeNotifier};
 pub use xcm_executor::{
 	traits::{ConvertOrigin, FilterAssetLocation, InvertLocation, OnResponse, TransactAsset},
 	Assets, Config,
@@ -258,6 +259,7 @@ parameter_types! {
 	// Nothing is allowed to be paid/unpaid by default.
 	pub static AllowUnpaidFrom: Vec<MultiLocation> = vec![];
 	pub static AllowPaidFrom: Vec<MultiLocation> = vec![];
+	pub static AllowSubsFrom: 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;
@@ -268,6 +270,7 @@ pub type TestBarrier = (
 	AllowKnownQueryResponses<TestResponseHandler>,
 	AllowTopLevelPaidExecutionFrom<IsInVec<AllowPaidFrom>>,
 	AllowUnpaidExecutionFrom<IsInVec<AllowUnpaidFrom>>,
+	AllowSubscriptionsFrom<IsInVec<AllowSubsFrom>>,
 );
 
 parameter_types! {
@@ -301,6 +304,26 @@ impl ClaimAssets for TestAssetTrap {
 	}
 }
 
+parameter_types! {
+	pub static SubscriptionRequests: Vec<(MultiLocation, Option<(QueryId, u64)>)> = vec![];
+}
+pub struct TestSubscriptionService;
+
+impl VersionChangeNotifier for TestSubscriptionService {
+	fn start(location: &MultiLocation, query_id: QueryId, max_weight: u64) -> XcmResult {
+		let mut r = SubscriptionRequests::get();
+		r.push((location.clone(), Some((query_id, max_weight))));
+		SubscriptionRequests::set(r);
+		Ok(())
+	}
+	fn stop(location: &MultiLocation) -> XcmResult {
+		let mut r = SubscriptionRequests::get();
+		r.push((location.clone(), None));
+		SubscriptionRequests::set(r);
+		Ok(())
+	}
+}
+
 pub struct TestConfig;
 impl Config for TestConfig {
 	type Call = TestCall;
@@ -316,4 +339,5 @@ impl Config for TestConfig {
 	type ResponseHandler = TestResponseHandler;
 	type AssetTrap = TestAssetTrap;
 	type AssetClaims = TestAssetTrap;
+	type SubscriptionService = TestSubscriptionService;
 }

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

@@ -1,4 +1,4 @@
-// Copyright 2020 Parity Technologies (UK) Ltd.
+// Copyright 2020 Parity Technologies query_id: (), max_response_weight: ()  query_id: (), max_response_weight: ()  (UK) Ltd.
 // This file is part of Polkadot.
 
 // Polkadot is free software: you can redistribute it and/or modify
@@ -57,23 +57,11 @@ fn take_weight_credit_barrier_should_work() {
 	let mut message =
 		Xcm::<()>(vec![TransferAsset { assets: (Parent, 100).into(), beneficiary: Here.into() }]);
 	let mut weight_credit = 10;
-	let r = TakeWeightCredit::should_execute(
-		&Parent.into(),
-		true,
-		&mut message,
-		10,
-		&mut weight_credit,
-	);
+	let r = TakeWeightCredit::should_execute(&Parent.into(), &mut message, 10, &mut weight_credit);
 	assert_eq!(r, Ok(()));
 	assert_eq!(weight_credit, 0);
 
-	let r = TakeWeightCredit::should_execute(
-		&Parent.into(),
-		true,
-		&mut message,
-		10,
-		&mut weight_credit,
-	);
+	let r = TakeWeightCredit::should_execute(&Parent.into(), &mut message, 10, &mut weight_credit);
 	assert_eq!(r, Err(()));
 	assert_eq!(weight_credit, 0);
 }
@@ -87,7 +75,6 @@ fn allow_unpaid_should_work() {
 
 	let r = AllowUnpaidExecutionFrom::<IsInVec<AllowUnpaidFrom>>::should_execute(
 		&Parachain(1).into(),
-		true,
 		&mut message,
 		10,
 		&mut 0,
@@ -96,7 +83,6 @@ fn allow_unpaid_should_work() {
 
 	let r = AllowUnpaidExecutionFrom::<IsInVec<AllowUnpaidFrom>>::should_execute(
 		&Parent.into(),
-		true,
 		&mut message,
 		10,
 		&mut 0,
@@ -113,7 +99,6 @@ fn allow_paid_should_work() {
 
 	let r = AllowTopLevelPaidExecutionFrom::<IsInVec<AllowPaidFrom>>::should_execute(
 		&Parachain(1).into(),
-		true,
 		&mut message,
 		10,
 		&mut 0,
@@ -129,7 +114,6 @@ fn allow_paid_should_work() {
 
 	let r = AllowTopLevelPaidExecutionFrom::<IsInVec<AllowPaidFrom>>::should_execute(
 		&Parent.into(),
-		true,
 		&mut underpaying_message,
 		30,
 		&mut 0,
@@ -145,7 +129,6 @@ fn allow_paid_should_work() {
 
 	let r = AllowTopLevelPaidExecutionFrom::<IsInVec<AllowPaidFrom>>::should_execute(
 		&Parachain(1).into(),
-		true,
 		&mut paying_message,
 		30,
 		&mut 0,
@@ -154,7 +137,6 @@ fn allow_paid_should_work() {
 
 	let r = AllowTopLevelPaidExecutionFrom::<IsInVec<AllowPaidFrom>>::should_execute(
 		&Parent.into(),
-		true,
 		&mut paying_message,
 		30,
 		&mut 0,
@@ -480,6 +462,120 @@ fn reserve_transfer_should_work() {
 	);
 }
 
+#[test]
+fn simple_version_subscriptions_should_work() {
+	AllowSubsFrom::set(vec![Parent.into()]);
+
+	let origin = Parachain(1000).into();
+	let message = Xcm::<TestCall>(vec![
+		SetAppendix(Xcm(vec![])),
+		SubscribeVersion { query_id: 42, max_response_weight: 5000 },
+	]);
+	let weight_limit = 20;
+	let r = XcmExecutor::<TestConfig>::execute_xcm(origin, message, weight_limit);
+	assert_eq!(r, Outcome::Error(XcmError::Barrier));
+
+	let origin = Parachain(1000).into();
+	let message =
+		Xcm::<TestCall>(vec![SubscribeVersion { query_id: 42, max_response_weight: 5000 }]);
+	let weight_limit = 10;
+	let r = XcmExecutor::<TestConfig>::execute_xcm(origin, message.clone(), weight_limit);
+	assert_eq!(r, Outcome::Error(XcmError::Barrier));
+
+	let origin = Parent.into();
+	let r = XcmExecutor::<TestConfig>::execute_xcm(origin, message, weight_limit);
+	assert_eq!(r, Outcome::Complete(10));
+
+	assert_eq!(SubscriptionRequests::get(), vec![(Parent.into(), Some((42, 5000)))]);
+}
+
+#[test]
+fn version_subscription_instruction_should_work() {
+	let origin = Parachain(1000).into();
+	let message = Xcm::<TestCall>(vec![
+		DescendOrigin(X1(AccountIndex64 { index: 1, network: Any })),
+		SubscribeVersion { query_id: 42, max_response_weight: 5000 },
+	]);
+	let weight_limit = 20;
+	let r = XcmExecutor::<TestConfig>::execute_xcm_in_credit(
+		origin.clone(),
+		message.clone(),
+		weight_limit,
+		weight_limit,
+	);
+	assert_eq!(r, Outcome::Incomplete(20, XcmError::BadOrigin));
+
+	let message = Xcm::<TestCall>(vec![
+		SetAppendix(Xcm(vec![])),
+		SubscribeVersion { query_id: 42, max_response_weight: 5000 },
+	]);
+	let r = XcmExecutor::<TestConfig>::execute_xcm_in_credit(
+		origin,
+		message.clone(),
+		weight_limit,
+		weight_limit,
+	);
+	assert_eq!(r, Outcome::Complete(20));
+
+	assert_eq!(SubscriptionRequests::get(), vec![(Parachain(1000).into(), Some((42, 5000)))]);
+}
+
+#[test]
+fn simple_version_unsubscriptions_should_work() {
+	AllowSubsFrom::set(vec![Parent.into()]);
+
+	let origin = Parachain(1000).into();
+	let message = Xcm::<TestCall>(vec![SetAppendix(Xcm(vec![])), UnsubscribeVersion]);
+	let weight_limit = 20;
+	let r = XcmExecutor::<TestConfig>::execute_xcm(origin, message, weight_limit);
+	assert_eq!(r, Outcome::Error(XcmError::Barrier));
+
+	let origin = Parachain(1000).into();
+	let message = Xcm::<TestCall>(vec![UnsubscribeVersion]);
+	let weight_limit = 10;
+	let r = XcmExecutor::<TestConfig>::execute_xcm(origin, message.clone(), weight_limit);
+	assert_eq!(r, Outcome::Error(XcmError::Barrier));
+
+	let origin = Parent.into();
+	let r = XcmExecutor::<TestConfig>::execute_xcm(origin, message, weight_limit);
+	assert_eq!(r, Outcome::Complete(10));
+
+	assert_eq!(SubscriptionRequests::get(), vec![(Parent.into(), None)]);
+	assert_eq!(sent_xcm(), vec![]);
+}
+
+#[test]
+fn version_unsubscription_instruction_should_work() {
+	let origin = Parachain(1000).into();
+
+	// Not allowed to do it when origin has been changed.
+	let message = Xcm::<TestCall>(vec![
+		DescendOrigin(X1(AccountIndex64 { index: 1, network: Any })),
+		UnsubscribeVersion,
+	]);
+	let weight_limit = 20;
+	let r = XcmExecutor::<TestConfig>::execute_xcm_in_credit(
+		origin.clone(),
+		message.clone(),
+		weight_limit,
+		weight_limit,
+	);
+	assert_eq!(r, Outcome::Incomplete(20, XcmError::BadOrigin));
+
+	// Fine to do it when origin is untouched.
+	let message = Xcm::<TestCall>(vec![SetAppendix(Xcm(vec![])), UnsubscribeVersion]);
+	let r = XcmExecutor::<TestConfig>::execute_xcm_in_credit(
+		origin,
+		message.clone(),
+		weight_limit,
+		weight_limit,
+	);
+	assert_eq!(r, Outcome::Complete(20));
+
+	assert_eq!(SubscriptionRequests::get(), vec![(Parachain(1000).into(), None)]);
+	assert_eq!(sent_xcm(), vec![]);
+}
+
 #[test]
 fn transacting_should_work() {
 	AllowUnpaidFrom::set(vec![Parent.into()]);

polkadot/xcm/xcm-builder/tests/mock/mod.rs

@@ -169,6 +169,7 @@ impl xcm_executor::Config for XcmConfig {
 	type ResponseHandler = XcmPallet;
 	type AssetTrap = XcmPallet;
 	type AssetClaims = XcmPallet;
+	type SubscriptionService = XcmPallet;
 }
 
 pub type LocalOriginToLocation = SignedToAccountId32<Origin, AccountId, KusamaNetwork>;
@@ -187,6 +188,8 @@ impl pallet_xcm::Config for Runtime {
 	type Weigher = FixedWeightBounds<BaseXcmWeight, Call, MaxInstructions>;
 	type Call = Call;
 	type Origin = Origin;
+	const VERSION_DISCOVERY_QUEUE_SIZE: u32 = 100;
+	type AdvertisedXcmVersion = pallet_xcm::CurrentXcmVersion;
 }
 
 impl origin::Config for Runtime {}