XCM docs and tests (#2948)

* WIP

* add tests and docs for DoubleEncoded

* reformat parent_count

* add test for match_and_split

* fix append_with docs and add tests

* move Parachain enum variant to tuple

* Fix stuff

* add to append test

* simplify match

* formatting

* format and extend doc comments (including examples)

* fix typo

* add some doc comments

* add test for location inverter

* Add more tests/docs

* Fix build

* matches fungibles

* currency adapter.

* add more tests for location inverter

* extract max length magic number into constant

* adapters.

* Apply suggestions from code review

* Final touches.

* Repot and fixes

* Remove last todo

* Apply suggestions from code review

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

* Update xcm/xcm-builder/src/barriers.rs

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

* Update xcm/xcm-builder/src/barriers.rs

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

* Update xcm/xcm-builder/src/currency_adapter.rs

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

* Update xcm/xcm-builder/src/filter_asset_location.rs

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

* Update xcm/xcm-builder/src/matches_fungible.rs

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

* Update xcm/xcm-executor/src/traits/conversion.rs

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

* Update xcm/xcm-executor/src/traits/conversion.rs

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

* Update xcm/xcm-executor/src/traits/transact_asset.rs

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

* Update xcm/xcm-executor/src/traits/should_execute.rs

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

Co-authored-by: kianenigma <kian@parity.io>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

polkadot/parachain/src/primitives.rs

@@ -137,7 +137,9 @@ impl Id {
 	}
 }
 
+/// Determine if a parachain is a system parachain or not.
 pub trait IsSystem {
+	/// Returns `true` if a parachain is a system parachain, `false` otherwise.
 	fn is_system(&self) -> bool;
 }
 

polkadot/xcm/src/double_encoded.rs

@@ -17,6 +17,8 @@
 use alloc::vec::Vec;
 use parity_scale_codec::{Encode, Decode};
 
+/// Wrapper around the encoded and decoded versions of a value.
+/// Caches the decoded value once computed.
 #[derive(Encode, Decode)]
 #[codec(encode_bound())]
 #[codec(decode_bound())]
@@ -29,11 +31,12 @@ pub struct DoubleEncoded<T> {
 impl<T> Clone for DoubleEncoded<T> {
 	fn clone(&self) -> Self { Self { encoded: self.encoded.clone(), decoded: None } }
 }
-impl<T> Eq for DoubleEncoded<T> {
+
-}
 impl<T> PartialEq for DoubleEncoded<T> {
 	fn eq(&self, other: &Self) -> bool { self.encoded.eq(&other.encoded) }
 }
+impl<T> Eq for DoubleEncoded<T> {}
+
 impl<T> core::fmt::Debug for DoubleEncoded<T> {
 	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { self.encoded.fmt(f) }
 }
@@ -46,29 +49,66 @@ impl<T> From<Vec<u8>> for DoubleEncoded<T> {
 
 impl<T> DoubleEncoded<T> {
 	pub fn into<S>(self) -> DoubleEncoded<S> { DoubleEncoded::from(self) }
+
 	pub fn from<S>(e: DoubleEncoded<S>) -> Self {
 		Self {
 			encoded: e.encoded,
 			decoded: None,
 		}
 	}
+
+	/// Provides an API similar to `AsRef` that provides access to the inner value.
+	/// `AsRef` implementation would expect an `&Option<T>` return type.
 	pub fn as_ref(&self) -> Option<&T> {
 		self.decoded.as_ref()
 	}
 }
 
 impl<T: Decode> DoubleEncoded<T> {
+	/// Decode the inner encoded value and store it.
+	/// Returns a reference to the value in case of success and `Err(())` in case the decoding fails.
 	pub fn ensure_decoded(&mut self) -> Result<&T, ()> {
 		if self.decoded.is_none() {
 			self.decoded = T::decode(&mut &self.encoded[..]).ok();
 		}
 		self.decoded.as_ref().ok_or(())
 	}
+
+	/// Move the decoded value out or (if not present) decode `encoded`.
 	pub fn take_decoded(&mut self) -> Result<T, ()> {
 		self.decoded.take().or_else(|| T::decode(&mut &self.encoded[..]).ok()).ok_or(())
 	}
+
+	/// Provides an API similar to `TryInto` that allows fallible conversion to the inner value type.
+	/// `TryInto` implementation would collide with std blanket implementation based on `TryFrom`.
 	pub fn try_into(mut self) -> Result<T, ()> {
 		self.ensure_decoded()?;
 		self.decoded.ok_or(())
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	#[test]
+	fn ensure_decoded_works() {
+		let val: u64 = 42;
+		let mut encoded: DoubleEncoded<_> = Encode::encode(&val).into();
+		assert_eq!(encoded.ensure_decoded(), Ok(&val));
+	}
+
+	#[test]
+	fn take_decoded_works() {
+		let val: u64 = 42;
+		let mut encoded: DoubleEncoded<_> = Encode::encode(&val).into();
+		assert_eq!(encoded.take_decoded(), Ok(val));
+	}
+
+	#[test]
+	fn try_into_works() {
+		let val: u64 = 42;
+		let encoded: DoubleEncoded<_> = Encode::encode(&val).into();
+		assert_eq!(encoded.try_into(), Ok(val));
+	}
+}

polkadot/xcm/src/v0/multi_asset.rs

@@ -61,35 +61,34 @@ pub enum AssetInstance {
 /// ### Abstract identifiers
 ///
 /// Abstract identifiers are absolute identifiers that represent a notional asset which can exist within multiple
-/// consensus systems. These tend to be simpler to deal with since their broad meaning is unchanged regardless stay
+/// consensus systems. These tend to be simpler to deal with since their broad meaning is unchanged regardless stay of
-/// of the consensus system in which it is interpreted.
+/// the consensus system in which it is interpreted.
 ///
 /// However, in the attempt to provide uniformity across consensus systems, they may conflate different instantiations
 /// of some notional asset (e.g. the reserve asset and a local reserve-backed derivative of it) under the same name,
-/// leading to confusion. It also implies that one notional asset is accounted for locally in only one way. This may
+/// leading to confusion. It also implies that one notional asset is accounted for locally in only one way. This may not
-/// not be the case, e.g. where there are multiple bridge instances each providing a bridged "BTC" token yet none
+/// be the case, e.g. where there are multiple bridge instances each providing a bridged "BTC" token yet none being
-/// being fungible between the others.
+/// fungible between the others.
 ///
-/// Since they are meant to be absolute and universal, a global registry is needed to ensure that name collisions
+/// Since they are meant to be absolute and universal, a global registry is needed to ensure that name collisions do not
-/// do not occur.
+/// occur.
 ///
 /// An abstract identifier is represented as a simple variable-size byte string. As of writing, no global registry
 /// exists and no proposals have been put forth for asset labeling.
 ///
 /// ### Concrete identifiers
 ///
-/// Concrete identifiers are *relative identifiers* that specifically identify a single asset through its location in
+/// Concrete identifiers are *relative identifiers* that specifically identify a single asset through its location in a
-/// a consensus system relative to the context interpreting. Use of a `MultiLocation` ensures that similar but non
+/// consensus system relative to the context interpreting. Use of a `MultiLocation` ensures that similar but non
-/// fungible variants of the same underlying asset can be properly distinguished, and obviates the need for any kind
+/// fungible variants of the same underlying asset can be properly distinguished, and obviates the need for any kind of
-/// of central registry.
+/// central registry.
 ///
-/// The limitation is that the asset identifier cannot be trivially copied between consensus
+/// The limitation is that the asset identifier cannot be trivially copied between consensus systems and must instead be
-/// systems and must instead be "re-anchored" whenever being moved to a new consensus system, using the two systems'
+/// "re-anchored" whenever being moved to a new consensus system, using the two systems' relative paths.
-/// relative paths.
 ///
-/// Throughout XCM, messages are authored such that *when interpreted from the receiver's point of view* they will
+/// Throughout XCM, messages are authored such that *when interpreted from the receiver's point of view* they will have
-/// have the desired meaning/effect. This means that relative paths should always by constructed to be read from the
+/// the desired meaning/effect. This means that relative paths should always by constructed to be read from the point of
-/// point of view of the receiving system, *which may be have a completely different meaning in the authoring system*.
+/// view of the receiving system, *which may be have a completely different meaning in the authoring system*.
 ///
 /// Concrete identifiers are the preferred way of identifying an asset since they are entirely unambiguous.
 ///
@@ -99,8 +98,8 @@ pub enum AssetInstance {
 ///
 /// - `<chain>/PalletInstance(<id>)` for a Frame chain with a single-asset pallet instance (such as an instance of the
 ///   Balances pallet).
-/// - `<chain>/PalletInstance(<id>)/GeneralIndex(<index>)` for a Frame chain with an indexed multi-asset pallet
+/// - `<chain>/PalletInstance(<id>)/GeneralIndex(<index>)` for a Frame chain with an indexed multi-asset pallet instance
-///   instance (such as an instance of the Assets pallet).
+///   (such as an instance of the Assets pallet).
 /// - `<chain>/AccountId32` for an ERC-20-style single-asset smart-contract on a Frame-based contracts chain.
 /// - `<chain>/AccountKey20` for an ERC-20-style single-asset smart-contract on an Ethereum-like chain.
 ///
@@ -147,6 +146,9 @@ pub enum MultiAsset {
 }
 
 impl MultiAsset {
+	/// Returns `true` if the `MultiAsset` is a wildcard and can refer to classes of assets, instead of just one.
+	///
+	/// Typically can also be inferred by the name starting with `All`.
 	pub fn is_wildcard(&self) -> bool {
 		match self {
 			MultiAsset::None
@@ -209,7 +211,6 @@ impl MultiAsset {
 	fn is_concrete_fungible(&self, id: &MultiLocation) -> bool {
 		match self {
 			MultiAsset::AllFungible => true,
-
 			MultiAsset::AllConcreteFungible { id: i }
 			| MultiAsset::ConcreteFungible { id: i, .. }
 			=> i == id,
@@ -250,8 +251,13 @@ impl MultiAsset {
 
 	fn is_all(&self) -> bool { matches!(self, MultiAsset::All) }
 
+	/// Returns true if `self` is a super-set of the given `inner`.
+	///
+	/// Typically, any wildcard is never contained in anything else, and a wildcard can contain any other non-wildcard.
+	/// For more details, see the implementation and tests.
 	pub fn contains(&self, inner: &MultiAsset) -> bool {
 		use MultiAsset::*;
+
 		// Inner cannot be wild
 		if inner.is_wildcard() { return false }
 		// Everything contains nothing.
@@ -273,15 +279,16 @@ impl MultiAsset {
 			AllConcreteNonFungible { class } => inner.is_concrete_non_fungible(class),
 			AllAbstractNonFungible { class } => inner.is_abstract_non_fungible(class),
 
-			ConcreteFungible { id, amount }
+			ConcreteFungible { id, amount } => matches!(
-			=> matches!(inner, ConcreteFungible { id: i , amount: a } if i == id && a >= amount),
+				inner,
-			AbstractFungible { id, amount }
+				ConcreteFungible { id: inner_id , amount: inner_amount } if inner_id == id && amount >= inner_amount
-			=> matches!(inner, AbstractFungible { id: i , amount: a } if i == id && a >= amount),
+			),
-			ConcreteNonFungible { class, instance }
+			AbstractFungible { id, amount } => matches!(
-			=> matches!(inner, ConcreteNonFungible { class: i , instance: a } if i == class && a == instance),
+				inner,
-			AbstractNonFungible { class, instance }
+				AbstractFungible { id: inner_id , amount: inner_amount } if inner_id == id && amount >= inner_amount
-			=> matches!(inner, AbstractNonFungible { class: i , instance: a } if i == class && a == instance),
+			),
-
+			ConcreteNonFungible { .. } => self == inner,
+			AbstractNonFungible { .. } => self == inner,
 			_ => false,
 		}
 	}
@@ -313,3 +320,60 @@ impl TryFrom<VersionedMultiAsset> for MultiAsset {
 		}
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	#[test]
+	fn contains_works() {
+		use alloc::vec;
+		use MultiAsset::*;
+		// trivial case: all contains any non-wildcard.
+		assert!(All.contains(&None));
+		assert!(All.contains(&AbstractFungible { id: alloc::vec![99u8], amount: 1 }));
+
+		// trivial case: none contains nothing, except itself.
+		assert!(None.contains(&None));
+		assert!(!None.contains(&AllFungible));
+		assert!(!None.contains(&All));
+
+		// A bit more sneaky: Nothing can contain wildcard, even All ir the thing itself.
+		assert!(!All.contains(&All));
+		assert!(!All.contains(&AllFungible));
+		assert!(!AllFungible.contains(&AllFungible));
+		assert!(!AllNonFungible.contains(&AllNonFungible));
+
+		// For fungibles, containing is basically equality, or equal id with higher amount.
+		assert!(
+			!AbstractFungible { id: vec![99u8], amount: 99 }
+			.contains(&AbstractFungible { id: vec![1u8], amount: 99 })
+		);
+		assert!(
+			AbstractFungible { id: vec![99u8], amount: 99 }
+			.contains(&AbstractFungible { id: vec![99u8], amount: 99 })
+		);
+		assert!(
+			AbstractFungible { id: vec![99u8], amount: 99 }
+			.contains(&AbstractFungible { id: vec![99u8], amount: 9 })
+		);
+		assert!(
+			!AbstractFungible { id: vec![99u8], amount: 99 }
+			.contains(&AbstractFungible { id: vec![99u8], amount: 100 })
+		);
+
+		// For non-fungibles, containing is equality.
+		assert!(
+			!AbstractNonFungible {class: vec![99u8], instance: AssetInstance::Index { id: 9 } }
+			.contains(&AbstractNonFungible { class: vec![98u8], instance: AssetInstance::Index { id: 9 } })
+		);
+		assert!(
+			!AbstractNonFungible { class: vec![99u8], instance: AssetInstance::Index { id: 8 } }
+			.contains(&AbstractNonFungible { class: vec![99u8], instance: AssetInstance::Index { id: 9 } })
+		);
+		assert!(
+			AbstractNonFungible { class: vec![99u8], instance: AssetInstance::Index { id: 9 } }
+			.contains(&AbstractNonFungible { class: vec![99u8], instance: AssetInstance::Index { id: 9 } })
+		);
+	}
+}

polkadot/xcm/src/v0/multi_location.rs

@@ -66,6 +66,9 @@ pub enum MultiLocation {
 	X8(Junction, Junction, Junction, Junction, Junction, Junction, Junction, Junction),
 }
 
+/// Maximum number of junctions a multilocation can contain.
+pub const MAX_MULTILOCATION_LENGTH: usize = 8;
+
 impl From<Junction> for MultiLocation {
 	fn from(x: Junction) -> Self {
 		MultiLocation::X1(x)
@@ -442,8 +445,18 @@ impl MultiLocation {
 		MultiLocationReverseIterator(self)
 	}
 
-	/// Ensures that self begins with `prefix` and that it has a single `Junction` item following. If
+	/// Ensures that self begins with `prefix` and that it has a single `Junction` item following.
-	/// so, returns a reference to this `Junction` item.
+	/// If so, returns a reference to this `Junction` item.
+	///
+	/// # Example
+	/// ```rust
+	/// # use xcm::v0::{MultiLocation::*, Junction::*};
+	/// # fn main() {
+	/// let mut m = X3(Parent, PalletInstance(3), OnlyChild);
+	/// assert_eq!(m.match_and_split(&X2(Parent, PalletInstance(3))), Some(&OnlyChild));
+	/// assert_eq!(m.match_and_split(&X1(Parent)), None);
+	/// # }
+	/// ```
 	pub fn match_and_split(&self, prefix: &MultiLocation) -> Option<&Junction> {
 		if prefix.len() + 1 != self.len() {
 			return None
@@ -479,84 +492,69 @@ impl MultiLocation {
 
 	/// Returns the number of `Parent` junctions at the beginning of `self`.
 	pub fn parent_count(&self) -> usize {
+		use Junction::Parent;
 		match self {
-			MultiLocation::X8(
+			MultiLocation::X8(Parent, Parent, Parent, Parent, Parent, Parent, Parent, Parent) => 8,
-				Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent,
-				Junction::Parent, Junction::Parent, Junction::Parent
-			) => 8,
 
-			MultiLocation::X8(
+			MultiLocation::X8(Parent, Parent, Parent, Parent, Parent, Parent, Parent, ..) => 7,
-				Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent,
+			MultiLocation::X7(Parent, Parent, Parent, Parent, Parent, Parent, Parent) => 7,
-				Junction::Parent, Junction::Parent, ..
-			) => 7,
-			MultiLocation::X7(
-				Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent,
-				Junction::Parent, Junction::Parent
-			) => 7,
 
-			MultiLocation::X8(
+			MultiLocation::X8(Parent, Parent, Parent, Parent, Parent, Parent, ..) => 6,
-				Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent,
+			MultiLocation::X7(Parent, Parent, Parent, Parent, Parent, Parent, ..) => 6,
-				Junction::Parent, ..
+			MultiLocation::X6(Parent, Parent, Parent, Parent, Parent, Parent) => 6,
-			) => 6,
-			MultiLocation::X7(
-				Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent,
-				Junction::Parent, ..
-			) => 6,
-			MultiLocation::X6(
-				Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent,
-				Junction::Parent
-			) => 6,
 
-			MultiLocation::X8(
+			MultiLocation::X8(Parent, Parent, Parent, Parent, Parent, ..) => 5,
-				Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, ..
+			MultiLocation::X7(Parent, Parent, Parent, Parent, Parent, ..) => 5,
-			) => 5,
+			MultiLocation::X6(Parent, Parent, Parent, Parent, Parent, ..) => 5,
-			MultiLocation::X7(
+			MultiLocation::X5(Parent, Parent, Parent, Parent, Parent) => 5,
-				Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, ..
-			) => 5,
-			MultiLocation::X6(
-				Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, ..
-			) => 5,
-			MultiLocation::X5(
-				Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent
-			) => 5,
 
-			MultiLocation::X8(Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, ..) => 4,
+			MultiLocation::X8(Parent, Parent, Parent, Parent, ..) => 4,
-			MultiLocation::X7(Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, ..) => 4,
+			MultiLocation::X7(Parent, Parent, Parent, Parent, ..) => 4,
-			MultiLocation::X6(Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, ..) => 4,
+			MultiLocation::X6(Parent, Parent, Parent, Parent, ..) => 4,
-			MultiLocation::X5(Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent, ..) => 4,
+			MultiLocation::X5(Parent, Parent, Parent, Parent, ..) => 4,
-			MultiLocation::X4(Junction::Parent, Junction::Parent, Junction::Parent, Junction::Parent) => 4,
+			MultiLocation::X4(Parent, Parent, Parent, Parent) => 4,
 
-			MultiLocation::X8(Junction::Parent, Junction::Parent, Junction::Parent, ..) => 3,
+			MultiLocation::X8(Parent, Parent, Parent, ..) => 3,
-			MultiLocation::X7(Junction::Parent, Junction::Parent, Junction::Parent, ..) => 3,
+			MultiLocation::X7(Parent, Parent, Parent, ..) => 3,
-			MultiLocation::X6(Junction::Parent, Junction::Parent, Junction::Parent, ..) => 3,
+			MultiLocation::X6(Parent, Parent, Parent, ..) => 3,
-			MultiLocation::X5(Junction::Parent, Junction::Parent, Junction::Parent, ..) => 3,
+			MultiLocation::X5(Parent, Parent, Parent, ..) => 3,
-			MultiLocation::X4(Junction::Parent, Junction::Parent, Junction::Parent, ..) => 3,
+			MultiLocation::X4(Parent, Parent, Parent, ..) => 3,
-			MultiLocation::X3(Junction::Parent, Junction::Parent, Junction::Parent) => 3,
+			MultiLocation::X3(Parent, Parent, Parent) => 3,
 
-			MultiLocation::X8(Junction::Parent, Junction::Parent, ..) => 2,
+			MultiLocation::X8(Parent, Parent, ..) => 2,
-			MultiLocation::X7(Junction::Parent, Junction::Parent, ..) => 2,
+			MultiLocation::X7(Parent, Parent, ..) => 2,
-			MultiLocation::X6(Junction::Parent, Junction::Parent, ..) => 2,
+			MultiLocation::X6(Parent, Parent, ..) => 2,
-			MultiLocation::X5(Junction::Parent, Junction::Parent, ..) => 2,
+			MultiLocation::X5(Parent, Parent, ..) => 2,
-			MultiLocation::X4(Junction::Parent, Junction::Parent, ..) => 2,
+			MultiLocation::X4(Parent, Parent, ..) => 2,
-			MultiLocation::X3(Junction::Parent, Junction::Parent, ..) => 2,
+			MultiLocation::X3(Parent, Parent, ..) => 2,
-			MultiLocation::X2(Junction::Parent, Junction::Parent) => 2,
+			MultiLocation::X2(Parent, Parent) => 2,
 
-			MultiLocation::X8(Junction::Parent, ..) => 1,
+			MultiLocation::X8(Parent, ..) => 1,
-			MultiLocation::X7(Junction::Parent, ..) => 1,
+			MultiLocation::X7(Parent, ..) => 1,
-			MultiLocation::X6(Junction::Parent, ..) => 1,
+			MultiLocation::X6(Parent, ..) => 1,
-			MultiLocation::X5(Junction::Parent, ..) => 1,
+			MultiLocation::X5(Parent, ..) => 1,
-			MultiLocation::X4(Junction::Parent, ..) => 1,
+			MultiLocation::X4(Parent, ..) => 1,
-			MultiLocation::X3(Junction::Parent, ..) => 1,
+			MultiLocation::X3(Parent, ..) => 1,
-			MultiLocation::X2(Junction::Parent, ..) => 1,
+			MultiLocation::X2(Parent, ..) => 1,
-			MultiLocation::X1(Junction::Parent) => 1,
+			MultiLocation::X1(Parent) => 1,
 			_ => 0,
 		}
 	}
 
-	/// Mutate `self` so that it is suffixed with `prefix`. The correct normalized form is returned, removing any
+	/// Mutate `self` so that it is suffixed with `suffix`. The correct normalized form is returned,
-	/// internal `Parent`s.
+	/// removing any internal [Non-Parent, `Parent`]  combinations.
 	///
-	/// Does not modify `self` and returns `Err` with `prefix` in case of overflow.
+	/// Does not modify `self` and returns `Err` with `suffix` in case of overflow.
+	///
+	/// # Example
+	/// ```rust
+	/// # use xcm::v0::{MultiLocation::*, Junction::*};
+	/// # fn main() {
+	/// let mut m = X3(Parent, Parachain(21), OnlyChild);
+	/// assert_eq!(m.append_with(X2(Parent, PalletInstance(3))), Ok(()));
+	/// assert_eq!(m, X3(Parent, Parachain(21), PalletInstance(3)));
+	/// # }
+	/// ```
 	pub fn append_with(&mut self, suffix: MultiLocation) -> Result<(), MultiLocation> {
 		let mut prefix = suffix;
 		core::mem::swap(self, &mut prefix);
@@ -570,21 +568,31 @@ impl MultiLocation {
 		}
 	}
 
-	/// Mutate `self` so that it is prefixed with `prefix`. The correct normalized form is returned, removing any
+	/// Mutate `self` so that it is prefixed with `prefix`. The correct normalized form is returned,
-	/// internal `Parent`s.
+	/// removing any internal [Non-Parent, `Parent`] combinations.
 	///
 	/// Does not modify `self` and returns `Err` with `prefix` in case of overflow.
+	///
+	/// # Example
+	/// ```rust
+	/// # use xcm::v0::{MultiLocation::*, Junction::*, NetworkId::Any};
+	/// # fn main() {
+	/// let mut m = X3(Parent, Parent, PalletInstance(3));
+	/// assert_eq!(m.prepend_with(X3(Parent, Parachain(21), OnlyChild)), Ok(()));
+	/// assert_eq!(m, X2(Parent, PalletInstance(3)));
+	/// # }
+	/// ```
 	pub fn prepend_with(&mut self, prefix: MultiLocation) -> Result<(), MultiLocation> {
 		let self_parents = self.parent_count();
 		let prefix_rest = prefix.len() - prefix.parent_count();
 		let skipped = self_parents.min(prefix_rest);
-		if self.len() + prefix.len() - 2 * skipped > 8 {
+		if self.len() + prefix.len() - 2 * skipped > MAX_MULTILOCATION_LENGTH {
 			return Err(prefix);
 		}
 
 		let mut prefix = prefix;
 		while match (prefix.last(), self.first()) {
-			(Some(x), Some(Junction::Parent)) if x != &Junction::Parent => {
+			(Some(x), Some(Junction::Parent)) if x.is_interior() => {
 				prefix.take_last();
 				self.take_first();
 				true
@@ -593,13 +601,25 @@ impl MultiLocation {
 		} {}
 
 		for j in prefix.into_iter_rev() {
-			self.push_front(j).expect("len + prefix minus 2*skipped is less than 4; qed");
+			self.push_front(j).expect("len + prefix minus 2*skipped is less than max length; qed");
 		}
 		Ok(())
 	}
 
-	/// Returns true iff `self` is an interior location. For this it may not contain any `Junction`s for which
+	/// Returns true iff `self` is an interior location. For this it may not contain any `Junction`s
-	/// `Junction::is_interior` returns `false`. This
+	/// for which `Junction::is_interior` returns `false`. This is generally true, except for the
+	/// `Parent` item.
+	///
+	/// # Example
+	/// ```rust
+	/// # use xcm::v0::{MultiLocation::*, Junction::*, NetworkId::Any};
+	/// # fn main() {
+	/// let parent = X1(Parent);
+	/// assert_eq!(parent.is_interior(), false);
+	/// let m = X2(PalletInstance(12), AccountIndex64 { network: Any, index: 23 });
+	/// assert_eq!(m.is_interior(), true);
+	/// # }
+	/// ```
 	pub fn is_interior(&self) -> bool {
 		self.iter().all(Junction::is_interior)
 	}
@@ -619,3 +639,46 @@ impl TryFrom<VersionedMultiLocation> for MultiLocation {
 		}
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::MultiLocation::*;
+	use crate::opaque::v0::{Junction::*, NetworkId::Any};
+
+	#[test]
+	fn match_and_split_works() {
+		let m = X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 });
+		assert_eq!(m.match_and_split(&X1(Parent)), None);
+		assert_eq!(
+			m.match_and_split(&X2(Parent, Parachain(42))),
+			Some(&AccountIndex64 { network: Any, index: 23 })
+		);
+		assert_eq!(m.match_and_split(&m), None);
+	}
+
+	#[test]
+	fn append_with_works() {
+		let acc = AccountIndex64 { network: Any, index: 23 };
+		let mut m = X2(Parent, Parachain(42));
+		assert_eq!(m.append_with(X2(PalletInstance(3), acc.clone())), Ok(()));
+		assert_eq!(m, X4(Parent, Parachain(42), PalletInstance(3), acc.clone()));
+
+		// cannot append to create overly long multilocation
+		let acc = AccountIndex64 { network: Any, index: 23 };
+		let mut m = X7(Parent, Parent, Parent, Parent, Parent, Parent, Parachain(42));
+		let suffix = X2(PalletInstance(3), acc.clone());
+		assert_eq!(m.append_with(suffix.clone()), Err(suffix));
+	}
+
+	#[test]
+	fn prepend_with_works() {
+		let mut m = X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 });
+		assert_eq!(m.prepend_with(X2(Parent, OnlyChild)), Ok(()));
+		assert_eq!(m, X3(Parent, Parachain(42), AccountIndex64 { network: Any, index: 23 }));
+
+		// cannot prepend to create overly long multilocation
+		let mut m = X7(Parent, Parent, Parent, Parent, Parent, Parent, Parachain(42));
+		let prefix = X2(Parent, Parent);
+		assert_eq!(m.prepend_with(prefix.clone()), Err(prefix));
+	}
+}

polkadot/xcm/src/v0/traits.rs

@@ -162,13 +162,72 @@ impl<C> ExecuteXcm<C> for () {
 
 /// Utility for sending an XCM message.
 ///
-/// These can be amalgamated in tuples to form sophisticated routing systems.
+/// These can be amalgamated in tuples to form sophisticated routing systems. In tuple format, each router might return
+/// `CannotReachDestination` to pass the execution to the next sender item. Note that each `CannotReachDestination`
+/// might alter the destination and the xcm message for to the next router.
+///
+///
+/// # Example
+/// ```rust
+/// # use xcm::v0::{MultiLocation, Xcm, Junction, Error, OriginKind, SendXcm, Result};
+/// # use parity_scale_codec::Encode;
+///
+/// /// A sender that only passes the message through and does nothing.
+/// struct Sender1;
+/// impl SendXcm for Sender1 {
+///     fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result {
+///         return Err(Error::CannotReachDestination(destination, message))
+///     }
+/// }
+///
+/// /// A sender that accepts a message that has an X2 junction, otherwise stops the routing.
+/// struct Sender2;
+/// impl SendXcm for Sender2 {
+///     fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result {
+///         if let MultiLocation::X2(j1, j2) = destination {
+///             Ok(())
+///         } else {
+///             Err(Error::Undefined)
+///         }
+///     }
+/// }
+///
+/// /// A sender that accepts a message from an X1 parent junction, passing through otherwise.
+/// struct Sender3;
+/// impl SendXcm for Sender3 {
+///     fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result {
+///         match destination {
+///             MultiLocation::X1(j) if j == Junction::Parent => Ok(()),
+///             _ => Err(Error::CannotReachDestination(destination, message)),
+///         }
+///     }
+/// }
+///
+/// // A call to send via XCM. We don't really care about this.
+/// # fn main() {
+/// let call: Vec<u8> = ().encode();
+/// let message = Xcm::Transact { origin_type: OriginKind::Superuser, require_weight_at_most: 0, call: call.into() };
+/// let destination = MultiLocation::X1(Junction::Parent);
+///
+/// assert!(
+///     // Sender2 will block this.
+///     <(Sender1, Sender2, Sender3) as SendXcm>::send_xcm(destination.clone(), message.clone())
+///         .is_err()
+/// );
+///
+/// assert!(
+///     // Sender3 will catch this.
+///     <(Sender1, Sender3) as SendXcm>::send_xcm(destination.clone(), message.clone())
+///         .is_ok()
+/// );
+/// # }
+/// ```
 pub trait SendXcm {
 	/// Send an XCM `message` to a given `destination`.
 	///
-	/// If it is not a destination which can be reached with this type but possibly could by others,
+	/// If it is not a destination which can be reached with this type but possibly could by others, then it *MUST*
-	/// then it *MUST* return `CannotReachDestination`. Any other error will cause the tuple implementation to
+	/// return `CannotReachDestination`. Any other error will cause the tuple implementation to exit early without
-	/// exit early without trying other type fields.
+	/// trying other type fields.
 	fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result;
 }
 
@@ -176,6 +235,7 @@ pub trait SendXcm {
 impl SendXcm for Tuple {
 	fn send_xcm(destination: MultiLocation, message: Xcm<()>) -> Result {
 		for_tuples!( #(
+			// we shadow `destination` and `message` in each expansion for the next one.
 			let (destination, message) = match Tuple::send_xcm(destination, message) {
 				Err(Error::CannotReachDestination(d, m)) => (d, m),
 				o @ _ => return o,

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

@@ -14,12 +14,15 @@
 // You should have received a copy of the GNU General Public License
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
+//! Various implementations for `ShouldExecute`.
+
 use sp_std::{result::Result, marker::PhantomData};
 use xcm::v0::{Xcm, Order, MultiLocation, Junction};
 use frame_support::{ensure, traits::Contains, weights::Weight};
 use xcm_executor::traits::{OnResponse, ShouldExecute};
 use polkadot_parachain::primitives::IsSystem;
 
+/// Execution barrier that just takes `shallow_weight` from `weight_credit`.
 pub struct TakeWeightCredit;
 impl ShouldExecute for TakeWeightCredit {
 	fn should_execute<Call>(
@@ -34,6 +37,8 @@ impl ShouldExecute for TakeWeightCredit {
 	}
 }
 
+/// Allows execution from `origin` if it is contained in `T` (i.e. `T::Contains(origin)`) taking payments into
+/// account.
 pub struct AllowTopLevelPaidExecutionFrom<T>(PhantomData<T>);
 impl<T: Contains<MultiLocation>> ShouldExecute for AllowTopLevelPaidExecutionFrom<T> {
 	fn should_execute<Call>(
@@ -59,6 +64,8 @@ impl<T: Contains<MultiLocation>> ShouldExecute for AllowTopLevelPaidExecutionFro
 	}
 }
 
+/// Allows execution from any origin that is contained in `T` (i.e. `T::Contains(origin)`) without any payments.
+/// Use only for executions from trusted origin groups.
 pub struct AllowUnpaidExecutionFrom<T>(PhantomData<T>);
 impl<T: Contains<MultiLocation>> ShouldExecute for AllowUnpaidExecutionFrom<T> {
 	fn should_execute<Call>(
@@ -73,6 +80,7 @@ impl<T: Contains<MultiLocation>> ShouldExecute for AllowUnpaidExecutionFrom<T> {
 	}
 }
 
+/// Allows a message only if it is from a system-level child parachain.
 pub struct IsChildSystemParachain<ParaId>(PhantomData<ParaId>);
 impl<
 	ParaId: IsSystem + From<u32>,
@@ -82,6 +90,7 @@ impl<
 	}
 }
 
+/// Allows only messages if the generic `ResponseHandler` expects them via `expecting_response`.
 pub struct AllowKnownQueryResponses<ResponseHandler>(PhantomData<ResponseHandler>);
 impl<ResponseHandler: OnResponse> ShouldExecute for AllowKnownQueryResponses<ResponseHandler> {
 	fn should_execute<Call>(

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

@@ -14,6 +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/>.
 
+//! Adapters to work with `frame_support::traits::Currency` through XCM.
+
 use sp_std::{result, convert::TryInto, marker::PhantomData};
 use xcm::v0::{Error as XcmError, Result, MultiAsset, MultiLocation};
 use sp_runtime::traits::{SaturatedConversion, CheckedSub};
@@ -33,16 +35,51 @@ enum Error {
 
 impl From<Error> for XcmError {
 	fn from(e: Error) -> Self {
+		use XcmError::FailedToTransactAsset;
 		match e {
-			Error::AssetNotFound => XcmError::FailedToTransactAsset("AssetNotFound"),
+			Error::AssetNotFound => FailedToTransactAsset("AssetNotFound"),
-			Error::AccountIdConversionFailed =>
+			Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"),
-				XcmError::FailedToTransactAsset("AccountIdConversionFailed"),
+			Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"),
-			Error::AmountToBalanceConversionFailed =>
-				XcmError::FailedToTransactAsset("AmountToBalanceConversionFailed"),
 		}
 	}
 }
 
+/// Simple adapter to use a currency as asset transactor. This type can be used as `type AssetTransactor` in
+/// `xcm::Config`.
+///
+/// # Example
+/// ```
+/// use frame_support::parameter_types;
+/// use xcm::v0::{MultiLocation, Junction};
+/// use xcm_builder::{ParentIsDefault, CurrencyAdapter, IsConcrete};
+///
+/// /// Our chain's account id.
+/// type AccountId = sp_runtime::AccountId32;
+///
+/// /// Our relay chain's location.
+/// parameter_types! {
+///     RelayChain: MultiLocation = MultiLocation::X1(Junction::Parent);
+///     CheckingAccount: AccountId = Default::default();
+/// }
+///
+/// /// Some items that implement `Convert<MultiLocation, AccountId>`. Can be more, but for now we just assume we accept
+/// /// messages from the parent (relay chain).
+/// pub type LocationConvertor = (ParentIsDefault<RelayChain>);
+///
+/// /// Final currency adapter. This can be used in `xcm::Config` to specify how asset related transactions happen.
+/// pub type AssetTransactor = CurrencyAdapter<
+///     // Use this balance type:
+///     u128,
+///     // The matcher: use the currency when the asset is a concrete asset in our relay chain.
+///     IsConcrete<RelayChain>,
+///     // The local convertor: default account of the parent relay chain.
+///     LocationConvertor,
+///     // Our chain's account ID type.
+///     AccountId,
+///     // The checking account. Can be any deterministic inaccessible account.
+///     CheckingAccount,
+/// >;
+/// ```
 pub struct CurrencyAdapter<Currency, Matcher, AccountIdConverter, AccountId, CheckedAccount>(
 	PhantomData<(Currency, Matcher, AccountIdConverter, AccountId, CheckedAccount)>
 );

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

@@ -14,11 +14,14 @@
 // You should have received a copy of the GNU General Public License
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
+//! Various implementations of `FilterAssetLocation`.
+
 use sp_std::marker::PhantomData;
 use xcm::v0::{MultiAsset, MultiLocation};
 use frame_support::traits::Get;
 use xcm_executor::traits::FilterAssetLocation;
 
+/// Accepts an asset IFF it is a native asset.
 pub struct NativeAsset;
 impl FilterAssetLocation for NativeAsset {
 	fn filter_asset_location(asset: &MultiAsset, origin: &MultiLocation) -> bool {
@@ -26,6 +29,7 @@ impl FilterAssetLocation for NativeAsset {
 	}
 }
 
+/// Accepts an asset if it is contained in the given `T`'s `Get` impl.
 pub struct Case<T>(PhantomData<T>);
 impl<T: Get<(MultiAsset, MultiLocation)>> FilterAssetLocation for Case<T> {
 	fn filter_asset_location(asset: &MultiAsset, origin: &MultiLocation) -> bool {

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

@@ -14,40 +14,16 @@
 // You should have received a copy of the GNU General Public License
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
+//! Adapters to work with `frame_support::traits::tokens::fungibles` through XCM.
+
 use sp_std::{prelude::*, result, marker::PhantomData, borrow::Borrow};
 use xcm::v0::{Error as XcmError, Result, MultiAsset, MultiLocation, Junction};
 use frame_support::traits::{Get, tokens::fungibles, Contains};
-use xcm_executor::traits::{TransactAsset, Convert};
+use xcm_executor::traits::{TransactAsset, Convert, MatchesFungibles, Error as MatchError};
 
-/// Asset transaction errors.
+/// Converter struct implementing `AssetIdConversion` converting a numeric asset ID (must be TryFrom/TryInto<u128>) into
-pub enum Error {
+/// a `GeneralIndex` junction, prefixed by some `MultiLocation` value. The `MultiLocation` value will typically be a
-	/// Asset not found.
+/// `PalletInstance` junction.
-	AssetNotFound,
-	/// `MultiLocation` to `AccountId` conversion failed.
-	AccountIdConversionFailed,
-	/// `u128` amount to currency `Balance` conversion failed.
-	AmountToBalanceConversionFailed,
-	/// `MultiLocation` to `AssetId` conversion failed.
-	AssetIdConversionFailed,
-}
-
-impl From<Error> for XcmError {
-	fn from(e: Error) -> Self {
-		match e {
-			Error::AssetNotFound => XcmError::FailedToTransactAsset("AssetNotFound"),
-			Error::AccountIdConversionFailed =>
-				XcmError::FailedToTransactAsset("AccountIdConversionFailed"),
-			Error::AmountToBalanceConversionFailed =>
-				XcmError::FailedToTransactAsset("AmountToBalanceConversionFailed"),
-			Error::AssetIdConversionFailed =>
-				XcmError::FailedToTransactAsset("AssetIdConversionFailed"),
-		}
-	}
-}
-
-/// Converter struct implementing `AssetIdConversion` converting a numeric asset ID (must be TryFrom/TryInto<u128>)
-/// into a `GeneralIndex` junction, prefixed by some `MultiLocation` value. The `MultiLocation` value will
-/// typically be a `PalletInstance` junction.
 pub struct AsPrefixedGeneralIndex<Prefix, AssetId, ConvertAssetId>(PhantomData<(Prefix, AssetId, ConvertAssetId)>);
 impl<
 	Prefix: Get<MultiLocation>,
@@ -73,23 +49,6 @@ impl<
 	}
 }
 
-pub trait MatchesFungibles<AssetId, Balance> {
-	fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error>;
-}
-
-#[impl_trait_for_tuples::impl_for_tuples(30)]
-impl<
-	AssetId: Clone,
-	Balance: Clone,
-> MatchesFungibles<AssetId, Balance> for Tuple {
-	fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error> {
-		for_tuples!( #(
-			match Tuple::matches_fungibles(a) { o @ Ok(_) => return o, _ => () }
-		)* );
-		Err(Error::AssetNotFound)
-	}
-}
-
 pub struct ConvertedConcreteAssetId<AssetId, Balance, ConvertAssetId, ConvertBalance>(
 	PhantomData<(AssetId, Balance, ConvertAssetId, ConvertBalance)>
 );
@@ -101,13 +60,13 @@ impl<
 > MatchesFungibles<AssetId, Balance> for
 	ConvertedConcreteAssetId<AssetId, Balance, ConvertAssetId, ConvertBalance>
 {
-	fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error> {
+	fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), MatchError> {
 		let (id, amount) = match a {
 			MultiAsset::ConcreteFungible { id, amount } => (id, amount),
-			_ => return Err(Error::AssetNotFound),
+			_ => return Err(MatchError::AssetNotFound),
 		};
-		let what = ConvertAssetId::convert_ref(id).map_err(|_| Error::AssetIdConversionFailed)?;
+		let what = ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?;
-		let amount = ConvertBalance::convert_ref(amount).map_err(|_| Error::AmountToBalanceConversionFailed)?;
+		let amount = ConvertBalance::convert_ref(amount).map_err(|_| MatchError::AmountToBalanceConversionFailed)?;
 		Ok((what, amount))
 	}
 }
@@ -123,13 +82,13 @@ impl<
 > MatchesFungibles<AssetId, Balance> for
 	ConvertedAbstractAssetId<AssetId, Balance, ConvertAssetId, ConvertBalance>
 {
-	fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error> {
+	fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), MatchError> {
 		let (id, amount) = match a {
 			MultiAsset::AbstractFungible { id, amount } => (id, amount),
-			_ => return Err(Error::AssetNotFound),
+			_ => return Err(MatchError::AssetNotFound),
 		};
-		let what = ConvertAssetId::convert_ref(id).map_err(|_| Error::AssetIdConversionFailed)?;
+		let what = ConvertAssetId::convert_ref(id).map_err(|_| MatchError::AssetIdConversionFailed)?;
-		let amount = ConvertBalance::convert_ref(amount).map_err(|_| Error::AmountToBalanceConversionFailed)?;
+		let amount = ConvertBalance::convert_ref(amount).map_err(|_| MatchError::AmountToBalanceConversionFailed)?;
 		Ok((what, amount))
 	}
 }
@@ -151,9 +110,9 @@ impl<
 		// Check we handle this asset.
 		let (asset_id, amount) = Matcher::matches_fungibles(what)?;
 		let source = AccountIdConverter::convert_ref(from)
-			.map_err(|()| Error::AccountIdConversionFailed)?;
+			.map_err(|()| MatchError::AccountIdConversionFailed)?;
 		let dest = AccountIdConverter::convert_ref(to)
-			.map_err(|()| Error::AccountIdConversionFailed)?;
+			.map_err(|()| MatchError::AccountIdConversionFailed)?;
 		Assets::transfer(asset_id, &source, &dest, amount, true)
 			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))?;
 		Ok(what.clone().into())
@@ -208,7 +167,7 @@ impl<
 		// Check we handle this asset.
 		let (asset_id, amount) = Matcher::matches_fungibles(what)?;
 		let who = AccountIdConverter::convert_ref(who)
-			.map_err(|()| Error::AccountIdConversionFailed)?;
+			.map_err(|()| MatchError::AccountIdConversionFailed)?;
 		Assets::mint_into(asset_id, &who, amount)
 			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))
 	}
@@ -220,7 +179,7 @@ impl<
 		// Check we handle this asset.
 		let (asset_id, amount) = Matcher::matches_fungibles(what)?;
 		let who = AccountIdConverter::convert_ref(who)
-			.map_err(|()| Error::AccountIdConversionFailed)?;
+			.map_err(|()| MatchError::AccountIdConversionFailed)?;
 		Assets::burn_from(asset_id, &who, amount)
 			.map_err(|e| XcmError::FailedToTransactAsset(e.into()))?;
 		Ok(what.clone().into())

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

@@ -14,6 +14,10 @@
 // You should have received a copy of the GNU General Public License
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
+//! # XCM-Builder
+//!
+//! Types and helpers for *building* XCM configuration.
+
 #![cfg_attr(not(feature = "std"), no_std)]
 
 #[cfg(test)]

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

@@ -23,7 +23,6 @@ use xcm::v0::{MultiLocation, NetworkId, Junction};
 use xcm_executor::traits::{InvertLocation, Convert};
 
 pub struct Account32Hash<Network, AccountId>(PhantomData<(Network, AccountId)>);
-
 impl<
 	Network: Get<NetworkId>,
 	AccountId: From<[u8; 32]> + Into<[u8; 32]> + Clone,
@@ -37,6 +36,8 @@ impl<
 	}
 }
 
+/// A [`MultiLocation`] consisting of a single `Parent` [`Junction`] will be converted to the
+/// default value of `AccountId` (e.g. all zeros for `AccountId32`).
 pub struct ParentIsDefault<AccountId>(PhantomData<AccountId>);
 impl<
 	AccountId: Default + Eq + Clone,
@@ -81,7 +82,6 @@ impl<
 }
 
 pub struct SiblingParachainConvertsVia<ParaId, AccountId>(PhantomData<(ParaId, AccountId)>);
-
 impl<
 	ParaId: From<u32> + Into<u32> + AccountIdConversion<AccountId>,
 	AccountId: Clone,
@@ -103,6 +103,7 @@ impl<
 	}
 }
 
+/// Extracts the `AccountId32` from the passed `location` if the network matches.
 pub struct AccountId32Aliases<Network, AccountId>(PhantomData<(Network, AccountId)>);
 impl<
 	Network: Get<NetworkId>,
@@ -142,7 +143,40 @@ impl<
 	}
 }
 
-/// Simple location inverter; give it this location's ancestry and it'll figure out the inverted location.
+/// Simple location inverter; give it this location's ancestry and it'll figure out the inverted
+/// location.
+///
+/// # Example
+/// ## Network Topology
+/// ```txt
+///                    v Source
+/// Relay -> Para 1 -> Account20
+///       -> Para 2 -> Account32
+///                    ^ Target
+/// ```
+/// ```rust
+/// # use frame_support::parameter_types;
+/// # use xcm::v0::{MultiLocation::{self, *}, Junction::*, NetworkId::Any};
+/// # use xcm_builder::LocationInverter;
+/// # use xcm_executor::traits::InvertLocation;
+/// # fn main() {
+/// parameter_types!{
+///     pub Ancestry: MultiLocation = X2(
+///         Parachain(1),
+///         AccountKey20 { network: Any, key: Default::default() },
+///     );
+/// }
+///
+/// let input = X4(Parent, Parent, Parachain(2), AccountId32 { network: Any, id: Default::default() });
+/// let inverted = LocationInverter::<Ancestry>::invert_location(&input);
+/// assert_eq!(inverted, X4(
+///     Parent,
+///     Parent,
+///     Parachain(1),
+///     AccountKey20 { network: Any, key: Default::default() },
+/// ));
+/// # }
+/// ```
 pub struct LocationInverter<Ancestry>(PhantomData<Ancestry>);
 impl<Ancestry: Get<MultiLocation>> InvertLocation for LocationInverter<Ancestry> {
 	fn invert_location(location: &MultiLocation) -> MultiLocation {
@@ -160,3 +194,72 @@ impl<Ancestry: Get<MultiLocation>> InvertLocation for LocationInverter<Ancestry>
 		result
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	use frame_support::parameter_types;
+	use xcm::v0::{MultiLocation::*, Junction::*, NetworkId::Any};
+
+	fn account20() -> Junction {
+		AccountKey20 { network: Any, key: Default::default() }
+	}
+
+	fn account32() -> Junction {
+		AccountId32 { network: Any, id: Default::default() }
+	}
+
+	// Network Topology
+	//                                     v Source
+	// Relay -> Para 1 -> SmartContract -> Account
+	//       -> Para 2 -> Account
+	//                    ^ Target
+	//
+	// Inputs and outputs written as file paths:
+	//
+	// input location (source to target): ../../../para_2/account32_default
+	// ancestry (root to source): para_1/account20_default/account20_default
+	// =>
+	// output (target to source): ../../para_1/account20_default/account20_default
+	#[test]
+	fn inverter_works_in_tree() {
+		parameter_types!{
+			pub Ancestry: MultiLocation = X3(Parachain(1), account20(), account20());
+		}
+
+		let input = X5(Parent, Parent, Parent, Parachain(2), account32());
+		let inverted = LocationInverter::<Ancestry>::invert_location(&input);
+		assert_eq!(inverted, X5(Parent, Parent, Parachain(1), account20(), account20()));
+	}
+
+	// Network Topology
+	//                                     v Source
+	// Relay -> Para 1 -> SmartContract -> Account
+	//          ^ Target
+	#[test]
+	fn inverter_uses_ancestry_as_inverted_location() {
+		parameter_types!{
+			pub Ancestry: MultiLocation = X2(account20(), account20());
+		}
+
+		let input = X2(Parent, Parent);
+		let inverted = LocationInverter::<Ancestry>::invert_location(&input);
+		assert_eq!(inverted, X2(account20(), account20()));
+	}
+
+	// Network Topology
+	//                                        v Source
+	// Relay -> Para 1 -> CollectivePallet -> Plurality
+	//          ^ Target
+	#[test]
+	fn inverter_uses_only_child_on_missing_ancestry() {
+		parameter_types!{
+			pub Ancestry: MultiLocation = X1(PalletInstance(5));
+		}
+
+		let input = X2(Parent, Parent);
+		let inverted = LocationInverter::<Ancestry>::invert_location(&input);
+		assert_eq!(inverted, X2(PalletInstance(5), OnlyChild));
+	}
+}

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

@@ -14,12 +14,35 @@
 // You should have received a copy of the GNU General Public License
 // along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
 
+//! Various implementations for the `MatchesFungible` trait.
+
 use sp_std::{marker::PhantomData, convert::TryFrom};
 use sp_runtime::traits::CheckedConversion;
 use xcm::v0::{MultiAsset, MultiLocation};
 use frame_support::traits::Get;
 use xcm_executor::traits::MatchesFungible;
 
+/// Converts a `MultiAsset` into balance `B` if it is a concrete fungible with an id equal to that
+/// given by `T`'s `Get`.
+///
+/// # Example
+///
+/// ```
+/// use xcm::v0::{MultiAsset, MultiLocation, Junction};
+/// use xcm_builder::IsConcrete;
+/// use xcm_executor::traits::MatchesFungible;
+///
+/// frame_support::parameter_types! {
+/// 	pub TargetLocation: MultiLocation = MultiLocation::X1(Junction::Parent);
+/// }
+///
+/// # fn main() {
+/// let id = MultiLocation::X1(Junction::Parent);
+/// let asset = MultiAsset::ConcreteFungible { id, amount: 999u128 };
+/// // match `asset` if it is a concrete asset in `TargetLocation`.
+/// assert_eq!(<IsConcrete<TargetLocation> as MatchesFungible<u128>>::matches_fungible(&asset), Some(999));
+/// # }
+/// ```
 pub struct IsConcrete<T>(PhantomData<T>);
 impl<T: Get<MultiLocation>, B: TryFrom<u128>> MatchesFungible<B> for IsConcrete<T> {
 	fn matches_fungible(a: &MultiAsset) -> Option<B> {
@@ -30,6 +53,26 @@ impl<T: Get<MultiLocation>, B: TryFrom<u128>> MatchesFungible<B> for IsConcrete<
 		}
 	}
 }
+
+/// Same as [`IsConcrete`] but for a fungible with abstract location.
+///
+/// # Example
+///
+/// ```
+/// use xcm::v0::{MultiAsset};
+/// use xcm_builder::IsAbstract;
+/// use xcm_executor::traits::MatchesFungible;
+///
+/// frame_support::parameter_types! {
+/// 	pub TargetLocation: &'static [u8] = &[7u8];
+/// }
+///
+/// # fn main() {
+/// let asset = MultiAsset::AbstractFungible { id: vec![7u8], amount: 999u128 };
+/// // match `asset` if it is a concrete asset in `TargetLocation`.
+/// assert_eq!(<IsAbstract<TargetLocation> as MatchesFungible<u128>>::matches_fungible(&asset), Some(999));
+/// # }
+/// ```
 pub struct IsAbstract<T>(PhantomData<T>);
 impl<T: Get<&'static [u8]>, B: TryFrom<u128>> MatchesFungible<B> for IsAbstract<T> {
 	fn matches_fungible(a: &MultiAsset) -> Option<B> {

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

@@ -36,8 +36,17 @@ pub use crate::{
 	FixedRateOfConcreteFungible, AllowKnownQueryResponses, LocationInverter,
 };
 
-pub enum TestOrigin { Root, Relay, Signed(u64), Parachain(u32) }
+pub enum TestOrigin {
+	Root,
+	Relay,
+	Signed(u64),
+	Parachain(u32),
+}
 
+/// A dummy call.
+///
+/// Each item contains the amount of weight that it *wants* to consume as the first item, and the actual amount (if
+/// different from the former) in the second option.
 #[derive(Debug, Encode, Decode, Eq, PartialEq, Clone, Copy)]
 pub enum TestCall {
 	OnlyRoot(Weight, Option<Weight>),
@@ -60,17 +69,13 @@ impl Dispatchable for TestCall {
 			=> maybe_actual,
 		};
 		if match (&origin, &self) {
-			(TestOrigin::Parachain(i), TestCall::OnlyParachain(_, _, Some(j)))
+			(TestOrigin::Parachain(i), TestCall::OnlyParachain(_, _, Some(j))) => i == j,
-			=> i == j,
+			(TestOrigin::Signed(i), TestCall::OnlySigned(_, _, Some(j))) => i == j,
-			(TestOrigin::Signed(i), TestCall::OnlySigned(_, _, Some(j)))
-			=> i == j,
-
 			(TestOrigin::Root, TestCall::OnlyRoot(..))
 			| (TestOrigin::Parachain(_), TestCall::OnlyParachain(_, _, None))
 			| (TestOrigin::Signed(_), TestCall::OnlySigned(_, _, None))
 			| (_, TestCall::Any(..))
 			=> true,
-
 			_ => false,
 		} {
 			Ok(post_info)
@@ -151,7 +156,7 @@ pub fn to_account(l: MultiLocation) -> Result<u64, MultiLocation> {
 		X1(Parachain(id)) => 1000 + id as u64,
 		// Self at 3000
 		Null => 3000,
-		// Parent at 3000
+		// Parent at 3001
 		X1(Parent) => 3001,
 		l => return Err(l),
 	})

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

@@ -14,6 +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/>.
 
+//! Various implementations for `ConvertOrigin`.
+
 use sp_std::{marker::PhantomData, convert::TryInto};
 use xcm::v0::{MultiLocation, OriginKind, NetworkId, Junction, BodyId, BodyPart};
 use xcm_executor::traits::{Convert, ConvertOrigin};
@@ -21,8 +23,7 @@ use frame_support::traits::{EnsureOrigin, Get, OriginTrait, GetBacking};
 use frame_system::RawOrigin as SystemRawOrigin;
 use polkadot_parachain::primitives::IsSystem;
 
-/// Sovereign accounts use the system's `Signed` origin with an account ID derived from the
+/// Sovereign accounts use the system's `Signed` origin with an account ID derived from the `LocationConverter`.
-/// `LocationConverter`.
 pub struct SovereignSignedViaLocation<LocationConverter, Origin>(
 	PhantomData<(LocationConverter, Origin)>
 );

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

@@ -28,8 +28,8 @@ use xcm::v0::{
 
 pub mod traits;
 use traits::{
-	TransactAsset, ConvertOrigin, FilterAssetLocation, InvertLocation, WeightBounds, WeightTrader, ShouldExecute,
+	TransactAsset, ConvertOrigin, FilterAssetLocation, InvertLocation, WeightBounds, WeightTrader,
-	OnResponse
+	ShouldExecute, OnResponse
 };
 
 mod assets;
@@ -37,6 +37,7 @@ pub use assets::{Assets, AssetId};
 mod config;
 pub use config::Config;
 
+/// The XCM executor.
 pub struct XcmExecutor<Config>(PhantomData<Config>);
 
 impl<Config: config::Config> ExecuteXcm<Config::Call> for XcmExecutor<Config> {

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

@@ -24,6 +24,9 @@ use xcm::v0::{MultiLocation, OriginKind};
 /// One of `convert`/`convert_ref` and `reverse`/`reverse_ref` MUST be implemented. If possible, implement
 /// `convert_ref`, since this will never result in a clone. Use `convert` when you definitely need to consume
 /// the source value.
+///
+/// Can be amalgamated into tuples. If any of the tuple elements converts into `Ok(_)` it short circuits. Otherwise returns
+/// the `Err(_)` of the last failing conversion (or `Err(())` for ref conversions).
 pub trait Convert<A: Clone, B: Clone> {
 	/// Convert from `value` (of type `A`) into an equivalent value of type `B`, `Err` if not possible.
 	fn convert(value: A) -> Result<B, A> { Self::convert_ref(&value).map_err(|_| value) }
@@ -115,7 +118,49 @@ impl<T: Clone + Encode + Decode> Convert<Vec<u8>, T> for Decoded {
 	fn reverse_ref(value: impl Borrow<T>) -> Result<Vec<u8>, ()> { Ok(value.borrow().encode()) }
 }
 
+/// A convertor trait for origin types.
+///
+/// Can be amalgamated into tuples. If any of the tuple elements returns `Ok(_)`, it short circuits. Else, the `Err(_)`
+/// of the last tuple item is returned. Each intermediate `Err(_)` might return a different `origin` of type `Origin`
+/// which is passed to the next convert item.
+///
+/// ```rust
+/// # use xcm::v0::{MultiLocation, Junction, OriginKind};
+/// # use xcm_executor::traits::ConvertOrigin;
+/// // A convertor that will bump the para id and pass it to the next one.
+/// struct BumpParaId;
+/// impl ConvertOrigin<u32> for BumpParaId {
+/// 	fn convert_origin(origin: MultiLocation, _: OriginKind) -> Result<u32, MultiLocation> {
+/// 		match origin {
+/// 			MultiLocation::X1(Junction::Parachain(id)) => {
+/// 				Err(MultiLocation::X1(Junction::Parachain(id + 1)))
+/// 			}
+/// 			_ => unreachable!()
+/// 		}
+/// 	}
+/// }
+///
+/// struct AcceptPara7;
+/// impl ConvertOrigin<u32> for AcceptPara7 {
+/// 	fn convert_origin(origin: MultiLocation, _: OriginKind) -> Result<u32, MultiLocation> {
+/// 		match origin {
+/// 			MultiLocation::X1(Junction::Parachain(id)) if id == 7 => {
+/// 				Ok(7)
+/// 			}
+/// 			_ => Err(origin)
+/// 		}
+/// 	}
+/// }
+/// # fn main() {
+/// let origin = MultiLocation::X1(Junction::Parachain(6));
+/// assert!(
+/// 	<(BumpParaId, AcceptPara7) as ConvertOrigin<u32>>::convert_origin(origin, OriginKind::Native)
+/// 		.is_ok()
+/// );
+/// # }
+/// ```
 pub trait ConvertOrigin<Origin> {
+	/// Attempt to convert `origin` to the generic `Origin` whilst consuming it.
 	fn convert_origin(origin: MultiLocation, kind: OriginKind) -> Result<Origin, MultiLocation>;
 }
 
@@ -123,14 +168,17 @@ pub trait ConvertOrigin<Origin> {
 impl<O> ConvertOrigin<O> for Tuple {
 	fn convert_origin(origin: MultiLocation, kind: OriginKind) -> Result<O, MultiLocation> {
 		for_tuples!( #(
-			let origin = match Tuple::convert_origin(origin, kind) { Err(o) => o, r => return r };
+			let origin = match Tuple::convert_origin(origin, kind) {
+				Err(o) => o,
+				r => return r
+			};
 		)* );
 		Err(origin)
 	}
 }
 
-/// Means of inverting a location: given a location which describes a `target` interpreted from the `source`, this
+/// Means of inverting a location: given a location which describes a `target` interpreted from the
-/// will provide the corresponding location which describes the `source`
+/// `source`, this will provide the corresponding location which describes the `source`.
 pub trait InvertLocation {
 	fn invert_location(l: &MultiLocation) -> MultiLocation;
 }

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

@@ -16,6 +16,9 @@
 
 use xcm::v0::{MultiAsset, MultiLocation};
 
+/// Filters assets/location pairs.
+///
+/// Can be amalgamated into tuples. If any item returns `true`, it short-circuits, else `false` is returned.
 pub trait FilterAssetLocation {
 	/// A filter to distinguish between asset/location pairs.
 	fn filter_asset_location(asset: &MultiAsset, origin: &MultiLocation) -> bool;

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

@@ -0,0 +1,59 @@
+// 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 xcm::v0::{MultiAsset, Error as XcmError};
+use sp_std::result;
+
+/// Errors associated with [`MatchesFungibles`] operation.
+pub enum Error {
+	/// Asset not found.
+	AssetNotFound,
+	/// `MultiLocation` to `AccountId` conversion failed.
+	AccountIdConversionFailed,
+	/// `u128` amount to currency `Balance` conversion failed.
+	AmountToBalanceConversionFailed,
+	/// `MultiLocation` to `AssetId` conversion failed.
+	AssetIdConversionFailed,
+}
+
+impl From<Error> for XcmError {
+	fn from(e: Error) -> Self {
+		use XcmError::FailedToTransactAsset;
+		match e {
+			Error::AssetNotFound => FailedToTransactAsset("AssetNotFound"),
+			Error::AccountIdConversionFailed => FailedToTransactAsset("AccountIdConversionFailed"),
+			Error::AmountToBalanceConversionFailed => FailedToTransactAsset("AmountToBalanceConversionFailed"),
+			Error::AssetIdConversionFailed => FailedToTransactAsset("AssetIdConversionFailed"),
+		}
+	}
+}
+
+pub trait MatchesFungibles<AssetId, Balance> {
+	fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error>;
+}
+
+#[impl_trait_for_tuples::impl_for_tuples(30)]
+impl<
+	AssetId: Clone,
+	Balance: Clone,
+> MatchesFungibles<AssetId, Balance> for Tuple {
+	fn matches_fungibles(a: &MultiAsset) -> result::Result<(AssetId, Balance), Error> {
+		for_tuples!( #(
+			match Tuple::matches_fungibles(a) { o @ Ok(_) => return o, _ => () }
+		)* );
+		Err(Error::AssetNotFound)
+	}
+}

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

@@ -22,6 +22,8 @@ mod filter_asset_location;
 pub use filter_asset_location::{FilterAssetLocation};
 mod matches_fungible;
 pub use matches_fungible::{MatchesFungible};
+mod matches_fungibles;
+pub use matches_fungibles::{MatchesFungibles, Error};
 mod on_response;
 pub use on_response::OnResponse;
 mod should_execute;

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

@@ -17,8 +17,11 @@
 use xcm::v0::{Response, MultiLocation};
 use frame_support::weights::Weight;
 
+/// Define what needs to be done upon receiving a query response.
 pub trait OnResponse {
+	/// Returns `true` if we are expecting a response from `origin` for query `query_id`.
 	fn expecting_response(origin: &MultiLocation, query_id: u64) -> bool;
+	/// Handler for receiving a `response` from `origin` relating to `query_id`.
 	fn on_response(origin: MultiLocation, query_id: u64, response: Response) -> Weight;
 }
 impl OnResponse for () {

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

@@ -19,18 +19,21 @@ use xcm::v0::{Xcm, MultiLocation};
 use frame_support::weights::Weight;
 
 /// Trait to determine whether the execution engine should actually execute a given XCM.
+///
+/// Can be amalgamated into a tuple to have multiple trials. If any of the tuple elements returns `Ok()`, the
+/// execution stops. Else, `Err(_)` is returned if all elements reject the message.
 pub trait ShouldExecute {
 	/// Returns `true` if the given `message` may be executed.
 	///
 	/// - `origin`: The origin (sender) of the message.
-	/// - `top_level`: `true`` indicates the initial XCM coming from the `origin`, `false` indicates an embedded
+	/// - `top_level`: `true` indicates the initial XCM coming from the `origin`, `false` indicates an embedded XCM
-	///   XCM executed internally as part of another message or an `Order`.
+	///   executed internally as part of another message or an `Order`.
 	/// - `message`: The message itself.
 	/// - `shallow_weight`: The weight of the non-negotiable execution of the message. This does not include any
 	///   embedded XCMs sat behind mechanisms like `BuyExecution` which would need to answer for their own weight.
-	/// - `weight_credit`: The pre-established amount of weight that the system has determined this message
+	/// - `weight_credit`: The pre-established amount of weight that the system has determined this message may utilise
-	///   may utilise in its execution. Typically non-zero only because of prior fee payment, but could
+	///   in its execution. Typically non-zero only because of prior fee payment, but could in principle be due to other
-	///   in principle be due to other factors.
+	///   factors.
 	fn should_execute<Call>(
 		origin: &MultiLocation,
 		top_level: bool,
@@ -51,7 +54,7 @@ impl ShouldExecute for Tuple {
 	) -> Result<(), ()> {
 		for_tuples!( #(
 			match Tuple::should_execute(origin, top_level, message, shallow_weight, weight_credit) {
-				o @ Ok(()) => return o,
+				Ok(()) => return Ok(()),
 				_ => (),
 			}
 		)* );

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

@@ -20,9 +20,11 @@ use crate::Assets;
 
 /// Facility for asset transacting.
 ///
-/// This should work with as many asset/location combinations as possible. Locations to support may include non-
+/// This should work with as many asset/location combinations as possible. Locations to support may include non-account
-/// account locations such as a `MultiLocation::X1(Junction::Parachain)`. Different chains may handle them in
+/// locations such as a `MultiLocation::X1(Junction::Parachain)`. Different chains may handle them in different ways.
-/// different ways.
+///
+/// Can be amalgamated as a tuple of items that implement this trait. In such executions, if any of the transactors
+/// returns `Ok(())`, then it will short circuit. Else, execution is passed to the next transactor.
 pub trait TransactAsset {
 	/// Ensure that `check_in` will result in `Ok`.
 	///