BridgeHubRococo/Wococo nits + updated subtree (#2572)

* Nits (merge before separatelly)

* Small cosmetics for Rococo/Wococo bridge local run

* Squashed 'bridges/' changes from 04b3dda6aa..5fc377ab34

5fc377ab34 Support for kusama-polkadot relaying (#2128)
01f4b7f1ba Fix clippy warnings (#2127)
696ff1c368 BHK/P alignments (#2115)
2a66aa3248 Small fixes (#2126)
7810f1a988 Cosmetics (#2124)
daf250f69c Remove some `expect()` statements (#2123)
1c5fba8274 temporarily remove balance guard (#2121)
3d0e547361 Propagate message receival confirmation errors (#2116)
1c33143f07 Propagate message verification errors (#2114)
b075b00910 Bump time from 0.3.20 to 0.3.21
51a3a51618 Bump serde from 1.0.160 to 1.0.162
da88d044a6 Bump clap from 4.2.5 to 4.2.7
cdca322cd6 Bump sysinfo from 0.28.4 to 0.29.0

git-subtree-dir: bridges
git-subtree-split: 5fc377ab34f7dfd3293099c5feec49255e827812

* Fix

* Allow to change storage constants (DeliveryReward, RequiredStakeForStakeAndSlash) + tests

* Clippy

* New SA for RO/WO

* Squashed 'bridges/' changes from 5fc377ab34..0f6091d481

0f6091d481 Bump polkadot/substrate (#2134)
9233f0a337 Bump tokio from 1.28.0 to 1.28.1
a29c1caa93 Bump serde from 1.0.162 to 1.0.163

git-subtree-dir: bridges
git-subtree-split: 0f6091d48184ebb4f75cb3089befa6b92cf37335

cumulus/Cargo.lock

@@ -815,6 +815,7 @@ dependencies = [
 name = "bp-messages"
 version = "0.1.0"
 dependencies = [
+ "bp-header-chain",
  "bp-runtime",
  "frame-support",
  "hex",
@@ -1193,7 +1194,6 @@ dependencies = [
  "pallet-bridge-relayers",
  "pallet-transaction-payment",
  "pallet-utility",
- "pallet-xcm",
  "parity-scale-codec",
  "scale-info",
  "sp-api",
@@ -1205,7 +1205,6 @@ dependencies = [
  "static_assertions",
  "xcm",
  "xcm-builder",
- "xcm-executor",
 ]
 
 [[package]]

cumulus/bridges/bin/runtime-common/Cargo.toml

@@ -40,10 +40,8 @@ sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", d
 sp-trie = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
 
 # Polkadot dependencies
-pallet-xcm = { git = "https://github.com/paritytech/polkadot", branch = "master", default-features = false }
 xcm = { git = "https://github.com/paritytech/polkadot", branch = "master", default-features = false }
 xcm-builder = { git = "https://github.com/paritytech/polkadot", branch = "master", default-features = false }
-xcm-executor = { git = "https://github.com/paritytech/polkadot", branch = "master", default-features = false }
 
 [dev-dependencies]
 bp-test-utils = { path = "../../primitives/test-utils" }
@@ -68,7 +66,6 @@ std = [
 	"pallet-bridge-relayers/std",
 	"pallet-transaction-payment/std",
 	"pallet-utility/std",
-	"pallet-xcm/std",
 	"scale-info/std",
 	"sp-api/std",
 	"sp-core/std",
@@ -78,13 +75,12 @@ std = [
 	"sp-trie/std",
 	"xcm/std",
 	"xcm-builder/std",
-	"xcm-executor/std",
 ]
 runtime-benchmarks = [
 	"pallet-bridge-grandpa/runtime-benchmarks",
 	"pallet-bridge-messages/runtime-benchmarks",
 	"pallet-bridge-parachains/runtime-benchmarks",
-	"pallet-xcm/runtime-benchmarks",
+	"pallet-bridge-relayers/runtime-benchmarks",
 	"xcm-builder/runtime-benchmarks",
 ]
 integrity-test = [

cumulus/bridges/bin/runtime-common/src/messages.rs

@@ -22,18 +22,19 @@
 
 pub use bp_runtime::{RangeInclusiveExt, UnderlyingChainOf, UnderlyingChainProvider};
 
-use bp_header_chain::{HeaderChain, HeaderChainError};
+use bp_header_chain::HeaderChain;
 use bp_messages::{
 	source_chain::{LaneMessageVerifier, TargetHeaderChain},
 	target_chain::{ProvedLaneMessages, ProvedMessages, SourceHeaderChain},
 	InboundLaneData, LaneId, Message, MessageKey, MessageNonce, MessagePayload, OutboundLaneData,
+	VerificationError,
 };
-use bp_runtime::{Chain, RawStorageProof, Size, StorageProofChecker, StorageProofError};
+use bp_runtime::{Chain, RawStorageProof, Size, StorageProofChecker};
 use codec::{Decode, Encode};
 use frame_support::{traits::Get, weights::Weight, RuntimeDebug};
 use hash_db::Hasher;
 use scale_info::TypeInfo;
-use sp_std::{convert::TryFrom, fmt::Debug, marker::PhantomData, vec::Vec};
+use sp_std::{convert::TryFrom, marker::PhantomData, vec::Vec};
 
 /// Bidirectional message bridge.
 pub trait MessageBridge {
@@ -74,29 +75,6 @@ pub type BalanceOf<C> = bp_runtime::BalanceOf<UnderlyingChainOf<C>>;
 /// Type of origin that is used on the chain.
 pub type OriginOf<C> = <C as ThisChainWithMessages>::RuntimeOrigin;
 
-/// Error that happens during message verification.
-#[derive(Debug, PartialEq, Eq)]
-pub enum Error {
-	/// The message proof is empty.
-	EmptyMessageProof,
-	/// Error returned by the bridged header chain.
-	HeaderChain(HeaderChainError),
-	/// Error returned while reading/decoding inbound lane data from the storage proof.
-	InboundLaneStorage(StorageProofError),
-	/// The declared message weight is incorrect.
-	InvalidMessageWeight,
-	/// Declared messages count doesn't match actual value.
-	MessagesCountMismatch,
-	/// Error returned while reading/decoding message data from the storage proof.
-	MessageStorage(StorageProofError),
-	/// The message is too large.
-	MessageTooLarge,
-	/// Error returned while reading/decoding outbound lane data from the storage proof.
-	OutboundLaneStorage(StorageProofError),
-	/// Storage proof related error.
-	StorageProof(StorageProofError),
-}
-
 /// Sub-module that is declaring types required for processing This -> Bridged chain messages.
 pub mod source {
 	use super::*;
@@ -169,14 +147,12 @@ pub mod source {
 			+ Into<Result<frame_system::RawOrigin<AccountIdOf<ThisChain<B>>>, OriginOf<ThisChain<B>>>>,
 		AccountIdOf<ThisChain<B>>: PartialEq + Clone,
 	{
-		type Error = &'static str;
-
 		fn verify_message(
 			_submitter: &OriginOf<ThisChain<B>>,
 			_lane: &LaneId,
 			_lane_outbound_data: &OutboundLaneData,
 			_payload: &FromThisChainMessagePayload,
-		) -> Result<(), Self::Error> {
+		) -> Result<(), VerificationError> {
 			// IMPORTANT: any error that is returned here is fatal for the bridge, because
 			// this code is executed at the bridge hub and message sender actually lives
 			// at some sibling parachain. So we are failing **after** the message has been
@@ -200,16 +176,15 @@ pub mod source {
 	impl<B: MessageBridge> TargetHeaderChain<FromThisChainMessagePayload, AccountIdOf<ThisChain<B>>>
 		for TargetHeaderChainAdapter<B>
 	{
-		type Error = Error;
 		type MessagesDeliveryProof = FromBridgedChainMessagesDeliveryProof<HashOf<BridgedChain<B>>>;
 
-		fn verify_message(payload: &FromThisChainMessagePayload) -> Result<(), Self::Error> {
+		fn verify_message(payload: &FromThisChainMessagePayload) -> Result<(), VerificationError> {
 			verify_chain_message::<B>(payload)
 		}
 
 		fn verify_messages_delivery_proof(
 			proof: Self::MessagesDeliveryProof,
-		) -> Result<(LaneId, InboundLaneData<AccountIdOf<ThisChain<B>>>), Self::Error> {
+		) -> Result<(LaneId, InboundLaneData<AccountIdOf<ThisChain<B>>>), VerificationError> {
 			verify_messages_delivery_proof::<B>(proof)
 		}
 	}
@@ -221,7 +196,7 @@ pub mod source {
 	/// check) that would reject message (see `FromThisChainMessageVerifier`).
 	pub fn verify_chain_message<B: MessageBridge>(
 		payload: &FromThisChainMessagePayload,
-	) -> Result<(), Error> {
+	) -> Result<(), VerificationError> {
 		// IMPORTANT: any error that is returned here is fatal for the bridge, because
 		// this code is executed at the bridge hub and message sender actually lives
 		// at some sibling parachain. So we are failing **after** the message has been
@@ -241,9 +216,9 @@ pub mod source {
 		// the message itself. The proof is always larger than the message. But unless chain state
 		// is enormously large, it should be several dozens/hundreds of bytes. The delivery
 		// transaction also contains signatures and signed extensions. Because of this, we reserve
-		// 1/3 of the the maximal extrinsic weight for this data.
+		// 1/3 of the the maximal extrinsic size for this data.
 		if payload.len() > maximal_message_size::<B>() as usize {
-			return Err(Error::MessageTooLarge)
+			return Err(VerificationError::MessageTooLarge)
 		}
 
 		Ok(())
@@ -255,31 +230,26 @@ pub mod source {
 	/// parachains, please use the `verify_messages_delivery_proof_from_parachain`.
 	pub fn verify_messages_delivery_proof<B: MessageBridge>(
 		proof: FromBridgedChainMessagesDeliveryProof<HashOf<BridgedChain<B>>>,
-	) -> Result<ParsedMessagesDeliveryProofFromBridgedChain<B>, Error> {
+	) -> Result<ParsedMessagesDeliveryProofFromBridgedChain<B>, VerificationError> {
 		let FromBridgedChainMessagesDeliveryProof { bridged_header_hash, storage_proof, lane } =
 			proof;
-		B::BridgedHeaderChain::parse_finalized_storage_proof(
+		let mut storage =
-			bridged_header_hash,
+			B::BridgedHeaderChain::storage_proof_checker(bridged_header_hash, storage_proof)
-			storage_proof,
+				.map_err(VerificationError::HeaderChain)?;
-			|mut storage| {
 		// Messages delivery proof is just proof of single storage key read => any error
 		// is fatal.
-				let storage_inbound_lane_data_key =
+		let storage_inbound_lane_data_key = bp_messages::storage_keys::inbound_lane_data_key(
-					bp_messages::storage_keys::inbound_lane_data_key(
 			B::BRIDGED_MESSAGES_PALLET_NAME,
 			&lane,
 		);
 		let inbound_lane_data = storage
 			.read_and_decode_mandatory_value(storage_inbound_lane_data_key.0.as_ref())
-					.map_err(Error::InboundLaneStorage)?;
+			.map_err(VerificationError::InboundLaneStorage)?;
 
 		// check that the storage proof doesn't have any untouched trie nodes
-				storage.ensure_no_unused_nodes().map_err(Error::StorageProof)?;
+		storage.ensure_no_unused_nodes().map_err(VerificationError::StorageProof)?;
 
 		Ok((lane, inbound_lane_data))
-			},
-		)
-		.map_err(Error::HeaderChain)?
 	}
 }
 
@@ -335,13 +305,12 @@ pub mod target {
 	pub struct SourceHeaderChainAdapter<B>(PhantomData<B>);
 
 	impl<B: MessageBridge> SourceHeaderChain for SourceHeaderChainAdapter<B> {
-		type Error = Error;
 		type MessagesProof = FromBridgedChainMessagesProof<HashOf<BridgedChain<B>>>;
 
 		fn verify_messages_proof(
 			proof: Self::MessagesProof,
 			messages_count: u32,
-		) -> Result<ProvedMessages<Message>, Self::Error> {
+		) -> Result<ProvedMessages<Message>, VerificationError> {
 			verify_messages_proof::<B>(proof, messages_count)
 		}
 	}
@@ -357,7 +326,7 @@ pub mod target {
 	pub fn verify_messages_proof<B: MessageBridge>(
 		proof: FromBridgedChainMessagesProof<HashOf<BridgedChain<B>>>,
 		messages_count: u32,
-	) -> Result<ProvedMessages<Message>, Error> {
+	) -> Result<ProvedMessages<Message>, VerificationError> {
 		let FromBridgedChainMessagesProof {
 			bridged_header_hash,
 			storage_proof,
@@ -365,19 +334,16 @@ pub mod target {
 			nonces_start,
 			nonces_end,
 		} = proof;
+		let storage =
+			B::BridgedHeaderChain::storage_proof_checker(bridged_header_hash, storage_proof)
+				.map_err(VerificationError::HeaderChain)?;
+		let mut parser = StorageProofCheckerAdapter::<_, B> { storage, _dummy: Default::default() };
 		let nonces_range = nonces_start..=nonces_end;
 
-		B::BridgedHeaderChain::parse_finalized_storage_proof(
-			bridged_header_hash,
-			storage_proof,
-			|storage| {
-				let mut parser =
-					StorageProofCheckerAdapter::<_, B> { storage, _dummy: Default::default() };
-
 		// receiving proofs where end < begin is ok (if proof includes outbound lane state)
 		let messages_in_the_proof = nonces_range.checked_len().unwrap_or(0);
 		if messages_in_the_proof != MessageNonce::from(messages_count) {
-					return Err(Error::MessagesCountMismatch)
+			return Err(VerificationError::MessagesCountMismatch)
 		}
 
 		// Read messages first. All messages that are claimed to be in the proof must
@@ -399,23 +365,21 @@ pub mod target {
 		};
 
 		// Now we may actually check if the proof is empty or not.
-				if proved_lane_messages.lane_state.is_none() &&
+		if proved_lane_messages.lane_state.is_none() && proved_lane_messages.messages.is_empty() {
-					proved_lane_messages.messages.is_empty()
+			return Err(VerificationError::EmptyMessageProof)
-				{
-					return Err(Error::EmptyMessageProof)
 		}
 
 		// check that the storage proof doesn't have any untouched trie nodes
-				parser.storage.ensure_no_unused_nodes().map_err(Error::StorageProof)?;
+		parser
+			.storage
+			.ensure_no_unused_nodes()
+			.map_err(VerificationError::StorageProof)?;
 
 		// We only support single lane messages in this generated_schema
 		let mut proved_messages = ProvedMessages::new();
 		proved_messages.insert(lane, proved_lane_messages);
 
 		Ok(proved_messages)
-			},
-		)
-		.map_err(Error::HeaderChain)?
 	}
 
 	struct StorageProofCheckerAdapter<H: Hasher, B> {
@@ -427,7 +391,7 @@ pub mod target {
 		fn read_and_decode_outbound_lane_data(
 			&mut self,
 			lane_id: &LaneId,
-		) -> Result<Option<OutboundLaneData>, Error> {
+		) -> Result<Option<OutboundLaneData>, VerificationError> {
 			let storage_outbound_lane_data_key = bp_messages::storage_keys::outbound_lane_data_key(
 				B::BRIDGED_MESSAGES_PALLET_NAME,
 				lane_id,
@@ -435,13 +399,13 @@ pub mod target {
 
 			self.storage
 				.read_and_decode_opt_value(storage_outbound_lane_data_key.0.as_ref())
-				.map_err(Error::OutboundLaneStorage)
+				.map_err(VerificationError::OutboundLaneStorage)
 		}
 
 		fn read_and_decode_message_payload(
 			&mut self,
 			message_key: &MessageKey,
-		) -> Result<MessagePayload, Error> {
+		) -> Result<MessagePayload, VerificationError> {
 			let storage_message_key = bp_messages::storage_keys::message_key(
 				B::BRIDGED_MESSAGES_PALLET_NAME,
 				&message_key.lane_id,
@@ -449,7 +413,7 @@ pub mod target {
 			);
 			self.storage
 				.read_and_decode_mandatory_value(storage_message_key.0.as_ref())
-				.map_err(Error::MessageStorage)
+				.map_err(VerificationError::MessageStorage)
 		}
 	}
 }
@@ -470,8 +434,8 @@ mod tests {
 		},
 		mock::*,
 	};
-	use bp_header_chain::StoredHeaderDataBuilder;
+	use bp_header_chain::{HeaderChainError, StoredHeaderDataBuilder};
-	use bp_runtime::HeaderId;
+	use bp_runtime::{HeaderId, StorageProofError};
 	use codec::Encode;
 	use sp_core::H256;
 	use sp_runtime::traits::Header as _;
@@ -559,7 +523,7 @@ mod tests {
 			using_messages_proof(10, None, encode_all_messages, encode_lane_data, |proof| {
 				target::verify_messages_proof::<OnThisChainBridge>(proof, 5)
 			}),
-			Err(Error::MessagesCountMismatch),
+			Err(VerificationError::MessagesCountMismatch),
 		);
 	}
 
@@ -569,7 +533,7 @@ mod tests {
 			using_messages_proof(10, None, encode_all_messages, encode_lane_data, |proof| {
 				target::verify_messages_proof::<OnThisChainBridge>(proof, 15)
 			}),
-			Err(Error::MessagesCountMismatch),
+			Err(VerificationError::MessagesCountMismatch),
 		);
 	}
 
@@ -582,7 +546,7 @@ mod tests {
 				pallet_bridge_grandpa::ImportedHeaders::<TestRuntime>::remove(bridged_header_hash);
 				target::verify_messages_proof::<OnThisChainBridge>(proof, 10)
 			}),
-			Err(Error::HeaderChain(HeaderChainError::UnknownHeader)),
+			Err(VerificationError::HeaderChain(HeaderChainError::UnknownHeader)),
 		);
 	}
 
@@ -605,7 +569,7 @@ mod tests {
 				);
 				target::verify_messages_proof::<OnThisChainBridge>(proof, 10)
 			}),
-			Err(Error::HeaderChain(HeaderChainError::StorageProof(
+			Err(VerificationError::HeaderChain(HeaderChainError::StorageProof(
 				StorageProofError::StorageRootMismatch
 			))),
 		);
@@ -620,7 +584,7 @@ mod tests {
 				proof.storage_proof.push(node);
 				target::verify_messages_proof::<OnThisChainBridge>(proof, 10)
 			},),
-			Err(Error::HeaderChain(HeaderChainError::StorageProof(
+			Err(VerificationError::HeaderChain(HeaderChainError::StorageProof(
 				StorageProofError::DuplicateNodesInProof
 			))),
 		);
@@ -633,7 +597,7 @@ mod tests {
 				proof.storage_proof.push(vec![42]);
 				target::verify_messages_proof::<OnThisChainBridge>(proof, 10)
 			},),
-			Err(Error::StorageProof(StorageProofError::UnusedNodesInTheProof)),
+			Err(VerificationError::StorageProof(StorageProofError::UnusedNodesInTheProof)),
 		);
 	}
 
@@ -647,7 +611,7 @@ mod tests {
 				encode_lane_data,
 				|proof| target::verify_messages_proof::<OnThisChainBridge>(proof, 10)
 			),
-			Err(Error::MessageStorage(StorageProofError::StorageValueEmpty)),
+			Err(VerificationError::MessageStorage(StorageProofError::StorageValueEmpty)),
 		);
 	}
 
@@ -667,7 +631,7 @@ mod tests {
 				encode_lane_data,
 				|proof| target::verify_messages_proof::<OnThisChainBridge>(proof, 10),
 			),
-			Err(Error::MessageStorage(StorageProofError::StorageValueDecodeFailed(_))),
+			Err(VerificationError::MessageStorage(StorageProofError::StorageValueDecodeFailed(_))),
 		);
 	}
 
@@ -689,7 +653,9 @@ mod tests {
 				},
 				|proof| target::verify_messages_proof::<OnThisChainBridge>(proof, 10),
 			),
-			Err(Error::OutboundLaneStorage(StorageProofError::StorageValueDecodeFailed(_))),
+			Err(VerificationError::OutboundLaneStorage(
+				StorageProofError::StorageValueDecodeFailed(_)
+			)),
 		);
 	}
 
@@ -699,7 +665,7 @@ mod tests {
 			using_messages_proof(0, None, encode_all_messages, encode_lane_data, |proof| {
 				target::verify_messages_proof::<OnThisChainBridge>(proof, 0)
 			},),
-			Err(Error::EmptyMessageProof),
+			Err(VerificationError::EmptyMessageProof),
 		);
 	}
 
@@ -773,7 +739,7 @@ mod tests {
 				proof.nonces_end = u64::MAX;
 				target::verify_messages_proof::<OnThisChainBridge>(proof, u32::MAX)
 			},),
-			Err(Error::MessagesCountMismatch),
+			Err(VerificationError::MessagesCountMismatch),
 		);
 	}
 }

cumulus/bridges/bin/runtime-common/src/messages_call_ext.rs

@@ -47,7 +47,7 @@ pub struct BaseMessagesProofInfo {
 impl BaseMessagesProofInfo {
 	/// Returns true if `bundled_range` continues the `0..=best_stored_nonce` range.
 	fn appends_to_stored_nonce(&self) -> bool {
-		*self.bundled_range.start() == self.best_stored_nonce + 1
+		Some(*self.bundled_range.start()) == self.best_stored_nonce.checked_add(1)
 	}
 }
 

cumulus/bridges/bin/runtime-common/src/refund_relayer_extension.rs

@@ -24,7 +24,7 @@ use crate::messages_call_ext::{
 };
 use bp_messages::{LaneId, MessageNonce};
 use bp_relayers::{RewardsAccountOwner, RewardsAccountParams};
-use bp_runtime::{RangeInclusiveExt, StaticStrProvider};
+use bp_runtime::{Parachain, ParachainIdOf, RangeInclusiveExt, StaticStrProvider};
 use codec::{Decode, Encode};
 use frame_support::{
 	dispatch::{CallableCallFor, DispatchInfo, Dispatchable, PostDispatchInfo},
@@ -71,9 +71,9 @@ pub trait RefundableParachainId {
 }
 
 /// Default implementation of `RefundableParachainId`.
-pub struct RefundableParachain<Instance, Id>(PhantomData<(Instance, Id)>);
+pub struct DefaultRefundableParachainId<Instance, Id>(PhantomData<(Instance, Id)>);
 
-impl<Instance, Id> RefundableParachainId for RefundableParachain<Instance, Id>
+impl<Instance, Id> RefundableParachainId for DefaultRefundableParachainId<Instance, Id>
 where
 	Id: Get<u32>,
 {
@@ -81,6 +81,17 @@ where
 	type Id = Id;
 }
 
+/// Implementation of `RefundableParachainId` for `trait Parachain`.
+pub struct RefundableParachain<Instance, Para>(PhantomData<(Instance, Para)>);
+
+impl<Instance, Para> RefundableParachainId for RefundableParachain<Instance, Para>
+where
+	Para: Parachain,
+{
+	type Instance = Instance;
+	type Id = ParachainIdOf<Para>;
+}
+
 /// Trait identifying a bridged messages lane. A relayer might be refunded for delivering messages
 /// coming from this lane.
 pub trait RefundableMessagesLaneId {
@@ -682,7 +693,7 @@ mod tests {
 	bp_runtime::generate_static_str_provider!(TestExtension);
 	type TestExtension = RefundBridgedParachainMessages<
 		TestRuntime,
-		RefundableParachain<(), TestParachain>,
+		DefaultRefundableParachainId<(), TestParachain>,
 		RefundableMessagesLane<(), TestLaneId>,
 		ActualFeeRefund<TestRuntime>,
 		ConstU64<1>,

cumulus/bridges/modules/grandpa/src/lib.rs

@@ -1212,11 +1212,8 @@ mod tests {
 	fn parse_finalized_storage_proof_rejects_proof_on_unknown_header() {
 		run_test(|| {
 			assert_noop!(
-				Pallet::<TestRuntime>::parse_finalized_storage_proof(
+				Pallet::<TestRuntime>::storage_proof_checker(Default::default(), vec![],)
-					Default::default(),
+					.map(|_| ()),
-					vec![],
-					|_| (),
-				),
 				bp_header_chain::HeaderChainError::UnknownHeader,
 			);
 		});
@@ -1235,8 +1232,7 @@ mod tests {
 			<ImportedHeaders<TestRuntime>>::insert(hash, header.build());
 
 			assert_ok!(
-				Pallet::<TestRuntime>::parse_finalized_storage_proof(hash, storage_proof, |_| (),),
+				Pallet::<TestRuntime>::storage_proof_checker(hash, storage_proof).map(|_| ())
-				(),
 			);
 		});
 	}

cumulus/bridges/modules/messages/src/benchmarking.rs

@@ -17,8 +17,8 @@
 //! Messages pallet benchmarking.
 
 use crate::{
-	inbound_lane::InboundLaneStorage, inbound_lane_storage, outbound_lane,
+	inbound_lane::InboundLaneStorage, outbound_lane, weights_ext::EXPECTED_DEFAULT_MESSAGE_LENGTH,
-	weights_ext::EXPECTED_DEFAULT_MESSAGE_LENGTH, Call, OutboundLanes,
+	Call, OutboundLanes, RuntimeInboundLaneStorage,
 };
 
 use bp_messages::{
@@ -443,11 +443,12 @@ benchmarks_instance_pallet! {
 
 fn send_regular_message<T: Config<I>, I: 'static>() {
 	let mut outbound_lane = outbound_lane::<T, I>(T::bench_lane_id());
-	outbound_lane.send_message(vec![]);
+	outbound_lane.send_message(vec![]).expect("We craft valid messages");
 }
 
 fn receive_messages<T: Config<I>, I: 'static>(nonce: MessageNonce) {
-	let mut inbound_lane_storage = inbound_lane_storage::<T, I>(T::bench_lane_id());
+	let mut inbound_lane_storage =
+		RuntimeInboundLaneStorage::<T, I>::from_lane_id(T::bench_lane_id());
 	inbound_lane_storage.set_data(InboundLaneData {
 		relayers: vec![UnrewardedRelayer {
 			relayer: T::bridged_relayer_id(),

cumulus/bridges/modules/messages/src/inbound_lane.rs

@@ -40,7 +40,7 @@ pub trait InboundLaneStorage {
 	/// Return maximal number of unconfirmed messages in inbound lane.
 	fn max_unconfirmed_messages(&self) -> MessageNonce;
 	/// Get lane data from the storage.
-	fn data(&self) -> InboundLaneData<Self::Relayer>;
+	fn get_or_init_data(&mut self) -> InboundLaneData<Self::Relayer>;
 	/// Update lane data in the storage.
 	fn set_data(&mut self, data: InboundLaneData<Self::Relayer>);
 }
@@ -117,9 +117,9 @@ impl<S: InboundLaneStorage> InboundLane<S> {
 		InboundLane { storage }
 	}
 
-	/// Returns storage reference.
+	/// Returns `mut` storage reference.
-	pub fn storage(&self) -> &S {
+	pub fn storage_mut(&mut self) -> &mut S {
-		&self.storage
+		&mut self.storage
 	}
 
 	/// Receive state of the corresponding outbound lane.
@@ -127,7 +127,7 @@ impl<S: InboundLaneStorage> InboundLane<S> {
 		&mut self,
 		outbound_lane_data: OutboundLaneData,
 	) -> Option<MessageNonce> {
-		let mut data = self.storage.data();
+		let mut data = self.storage.get_or_init_data();
 		let last_delivered_nonce = data.last_delivered_nonce();
 
 		if outbound_lane_data.latest_received_nonce > last_delivered_nonce {
@@ -170,9 +170,8 @@ impl<S: InboundLaneStorage> InboundLane<S> {
 		nonce: MessageNonce,
 		message_data: DispatchMessageData<Dispatch::DispatchPayload>,
 	) -> ReceivalResult<Dispatch::DispatchLevelResult> {
-		let mut data = self.storage.data();
+		let mut data = self.storage.get_or_init_data();
-		let is_correct_message = nonce == data.last_delivered_nonce() + 1;
+		if Some(nonce) != data.last_delivered_nonce().checked_add(1) {
-		if !is_correct_message {
 			return ReceivalResult::InvalidNonce
 		}
 
@@ -252,7 +251,7 @@ mod tests {
 				None,
 			);
 
-			assert_eq!(lane.storage.data().last_confirmed_nonce, 0);
+			assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 0);
 		});
 	}
 
@@ -270,7 +269,7 @@ mod tests {
 				}),
 				Some(3),
 			);
-			assert_eq!(lane.storage.data().last_confirmed_nonce, 3);
+			assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 3);
 
 			assert_eq!(
 				lane.receive_state_update(OutboundLaneData {
@@ -279,7 +278,7 @@ mod tests {
 				}),
 				None,
 			);
-			assert_eq!(lane.storage.data().last_confirmed_nonce, 3);
+			assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 3);
 		});
 	}
 
@@ -290,9 +289,9 @@ mod tests {
 			receive_regular_message(&mut lane, 1);
 			receive_regular_message(&mut lane, 2);
 			receive_regular_message(&mut lane, 3);
-			assert_eq!(lane.storage.data().last_confirmed_nonce, 0);
+			assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 0);
 			assert_eq!(
-				lane.storage.data().relayers,
+				lane.storage.get_or_init_data().relayers,
 				vec![unrewarded_relayer(1, 3, TEST_RELAYER_A)]
 			);
 
@@ -303,9 +302,9 @@ mod tests {
 				}),
 				Some(2),
 			);
-			assert_eq!(lane.storage.data().last_confirmed_nonce, 2);
+			assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 2);
 			assert_eq!(
-				lane.storage.data().relayers,
+				lane.storage.get_or_init_data().relayers,
 				vec![unrewarded_relayer(3, 3, TEST_RELAYER_A)]
 			);
 
@@ -316,8 +315,8 @@ mod tests {
 				}),
 				Some(3),
 			);
-			assert_eq!(lane.storage.data().last_confirmed_nonce, 3);
+			assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 3);
-			assert_eq!(lane.storage.data().relayers, vec![]);
+			assert_eq!(lane.storage.get_or_init_data().relayers, vec![]);
 		});
 	}
 
@@ -325,7 +324,7 @@ mod tests {
 	fn receive_status_update_works_with_batches_from_relayers() {
 		run_test(|| {
 			let mut lane = inbound_lane::<TestRuntime, _>(TEST_LANE_ID);
-			let mut seed_storage_data = lane.storage.data();
+			let mut seed_storage_data = lane.storage.get_or_init_data();
 			// Prepare data
 			seed_storage_data.last_confirmed_nonce = 0;
 			seed_storage_data.relayers.push_back(unrewarded_relayer(1, 1, TEST_RELAYER_A));
@@ -341,9 +340,9 @@ mod tests {
 				}),
 				Some(3),
 			);
-			assert_eq!(lane.storage.data().last_confirmed_nonce, 3);
+			assert_eq!(lane.storage.get_or_init_data().last_confirmed_nonce, 3);
 			assert_eq!(
-				lane.storage.data().relayers,
+				lane.storage.get_or_init_data().relayers,
 				vec![
 					unrewarded_relayer(4, 4, TEST_RELAYER_B),
 					unrewarded_relayer(5, 5, TEST_RELAYER_C)
@@ -364,7 +363,7 @@ mod tests {
 				),
 				ReceivalResult::InvalidNonce
 			);
-			assert_eq!(lane.storage.data().last_delivered_nonce(), 0);
+			assert_eq!(lane.storage.get_or_init_data().last_delivered_nonce(), 0);
 		});
 	}
 
@@ -470,7 +469,7 @@ mod tests {
 				ReceivalResult::Dispatched(dispatch_result(0))
 			);
 			assert_eq!(
-				lane.storage.data().relayers,
+				lane.storage.get_or_init_data().relayers,
 				vec![
 					unrewarded_relayer(1, 1, TEST_RELAYER_A),
 					unrewarded_relayer(2, 2, TEST_RELAYER_B),
@@ -508,7 +507,7 @@ mod tests {
 		run_test(|| {
 			let mut lane = inbound_lane::<TestRuntime, _>(TEST_LANE_ID);
 			receive_regular_message(&mut lane, 1);
-			assert_eq!(lane.storage.data().last_delivered_nonce(), 1);
+			assert_eq!(lane.storage.get_or_init_data().last_delivered_nonce(), 1);
 		});
 	}
 

cumulus/bridges/modules/messages/src/lib.rs

@@ -48,7 +48,7 @@ pub use weights_ext::{
 
 use crate::{
 	inbound_lane::{InboundLane, InboundLaneStorage},
-	outbound_lane::{OutboundLane, OutboundLaneStorage, ReceivalConfirmationResult},
+	outbound_lane::{OutboundLane, OutboundLaneStorage, ReceivalConfirmationError},
 };
 
 use bp_messages::{
@@ -59,15 +59,15 @@ use bp_messages::{
 		DeliveryPayments, DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages,
 		SourceHeaderChain,
 	},
-	total_unrewarded_messages, DeliveredMessages, InboundLaneData, InboundMessageDetails, LaneId,
+	DeliveredMessages, InboundLaneData, InboundMessageDetails, LaneId, MessageKey, MessageNonce,
-	MessageKey, MessageNonce, MessagePayload, MessagesOperatingMode, OutboundLaneData,
+	MessagePayload, MessagesOperatingMode, OutboundLaneData, OutboundMessageDetails,
-	OutboundMessageDetails, UnrewardedRelayersState,
+	UnrewardedRelayersState, VerificationError,
 };
 use bp_runtime::{BasicOperatingMode, ChainId, OwnedBridgeModule, PreComputedSize, Size};
 use codec::{Decode, Encode, MaxEncodedLen};
 use frame_support::{dispatch::PostDispatchInfo, ensure, fail, traits::Get};
 use sp_runtime::traits::UniqueSaturatedFrom;
-use sp_std::{cell::RefCell, marker::PhantomData, prelude::*};
+use sp_std::{marker::PhantomData, prelude::*};
 
 mod inbound_lane;
 mod outbound_lane;
@@ -319,7 +319,7 @@ pub mod pallet {
 
 				// subtract extra storage proof bytes from the actual PoV size - there may be
 				// less unrewarded relayers than the maximal configured value
-				let lane_extra_proof_size_bytes = lane.storage().extra_proof_size_bytes();
+				let lane_extra_proof_size_bytes = lane.storage_mut().extra_proof_size_bytes();
 				actual_weight = actual_weight.set_proof_size(
 					actual_weight.proof_size().saturating_sub(lane_extra_proof_size_bytes),
 				);
@@ -332,7 +332,7 @@ pub mod pallet {
 							"Received lane {:?} state update: latest_confirmed_nonce={}. Unrewarded relayers: {:?}",
 							lane_id,
 							updated_latest_confirmed_nonce,
-							UnrewardedRelayersState::from(&lane.storage().data()),
+							UnrewardedRelayersState::from(&lane.storage_mut().get_or_init_data()),
 						);
 					}
 				}
@@ -437,57 +437,21 @@ pub mod pallet {
 
 					Error::<T, I>::InvalidMessagesDeliveryProof
 				})?;
-
-			// verify that the relayer has declared correct `lane_data::relayers` state
-			// (we only care about total number of entries and messages, because this affects call
-			// weight)
 			ensure!(
-				total_unrewarded_messages(&lane_data.relayers).unwrap_or(MessageNonce::MAX) ==
+				relayers_state.is_valid(&lane_data),
-					relayers_state.total_messages &&
-					lane_data.relayers.len() as MessageNonce ==
-						relayers_state.unrewarded_relayer_entries,
-				Error::<T, I>::InvalidUnrewardedRelayersState
-			);
-			// the `last_delivered_nonce` field may also be used by the signed extension. Even
-			// though providing wrong value isn't critical, let's also check it here.
-			ensure!(
-				lane_data.last_delivered_nonce() == relayers_state.last_delivered_nonce,
 				Error::<T, I>::InvalidUnrewardedRelayersState
 			);
 
 			// mark messages as delivered
 			let mut lane = outbound_lane::<T, I>(lane_id);
 			let last_delivered_nonce = lane_data.last_delivered_nonce();
-			let confirmed_messages = match lane.confirm_delivery(
+			let confirmed_messages = lane
+				.confirm_delivery(
 					relayers_state.total_messages,
 					last_delivered_nonce,
 					&lane_data.relayers,
-			) {
+				)
-				ReceivalConfirmationResult::ConfirmedMessages(confirmed_messages) =>
+				.map_err(Error::<T, I>::ReceivalConfirmation)?;
-					Some(confirmed_messages),
-				ReceivalConfirmationResult::NoNewConfirmations => None,
-				ReceivalConfirmationResult::TryingToConfirmMoreMessagesThanExpected(
-					to_confirm_messages_count,
-				) => {
-					log::trace!(
-						target: LOG_TARGET,
-						"Messages delivery proof contains too many messages to confirm: {} vs declared {}",
-						to_confirm_messages_count,
-						relayers_state.total_messages,
-					);
-
-					fail!(Error::<T, I>::TryingToConfirmMoreMessagesThanExpected);
-				},
-				error => {
-					log::trace!(
-						target: LOG_TARGET,
-						"Messages delivery proof contains invalid unrewarded relayers vec: {:?}",
-						error,
-					);
-
-					fail!(Error::<T, I>::InvalidUnrewardedRelayers);
-				},
-			};
 
 			if let Some(confirmed_messages) = confirmed_messages {
 				// emit 'delivered' event
@@ -554,12 +518,12 @@ pub mod pallet {
 		NotOperatingNormally,
 		/// The outbound lane is inactive.
 		InactiveOutboundLane,
-		/// The message is too large to be sent over the bridge.
-		MessageIsTooLarge,
 		/// Message has been treated as invalid by chain verifier.
-		MessageRejectedByChainVerifier,
+		MessageRejectedByChainVerifier(VerificationError),
 		/// Message has been treated as invalid by lane verifier.
-		MessageRejectedByLaneVerifier,
+		MessageRejectedByLaneVerifier(VerificationError),
+		/// Message has been treated as invalid by the pallet logic.
+		MessageRejectedByPallet(VerificationError),
 		/// Submitter has failed to pay fee for delivering and dispatching messages.
 		FailedToWithdrawMessageFee,
 		/// The transaction brings too many messages.
@@ -568,8 +532,6 @@ pub mod pallet {
 		InvalidMessagesProof,
 		/// Invalid messages delivery proof has been submitted.
 		InvalidMessagesDeliveryProof,
-		/// The bridged chain has invalid `UnrewardedRelayers` in its storage (fatal for the lane).
-		InvalidUnrewardedRelayers,
 		/// The relayer has declared invalid unrewarded relayers state in the
 		/// `receive_messages_delivery_proof` call.
 		InvalidUnrewardedRelayersState,
@@ -578,9 +540,8 @@ pub mod pallet {
 		InsufficientDispatchWeight,
 		/// The message someone is trying to work with (i.e. increase fee) is not yet sent.
 		MessageIsNotYetSent,
-		/// The number of actually confirmed messages is going to be larger than the number of
+		/// Error confirming messages receival.
-		/// messages in the proof. This may mean that this or bridged chain storage is corrupted.
+		ReceivalConfirmation(ReceivalConfirmationError),
-		TryingToConfirmMoreMessagesThanExpected,
 		/// Error generated by the `OwnedBridgeModule` trait.
 		BridgeModule(bp_runtime::OwnedBridgeModuleError),
 	}
@@ -732,7 +693,7 @@ fn send_message<T: Config<I>, I: 'static>(
 			err,
 		);
 
-		Error::<T, I>::MessageRejectedByChainVerifier
+		Error::<T, I>::MessageRejectedByChainVerifier(err)
 	})?;
 
 	// now let's enforce any additional lane rules
@@ -746,18 +707,16 @@ fn send_message<T: Config<I>, I: 'static>(
 				err,
 			);
 
-			Error::<T, I>::MessageRejectedByLaneVerifier
+			Error::<T, I>::MessageRejectedByLaneVerifier(err)
 		},
 	)?;
 
 	// finally, save message in outbound storage and emit event
 	let encoded_payload = payload.encode();
 	let encoded_payload_len = encoded_payload.len();
-	ensure!(
+	let nonce = lane
-		encoded_payload_len <= T::MaximalOutboundPayloadSize::get() as usize,
+		.send_message(encoded_payload)
-		Error::<T, I>::MessageIsTooLarge
+		.map_err(Error::<T, I>::MessageRejectedByPallet)?;
-	);
-	let nonce = lane.send_message(encoded_payload);
 
 	log::trace!(
 		target: LOG_TARGET,
@@ -787,18 +746,7 @@ fn ensure_normal_operating_mode<T: Config<I>, I: 'static>() -> Result<(), Error<
 fn inbound_lane<T: Config<I>, I: 'static>(
 	lane_id: LaneId,
 ) -> InboundLane<RuntimeInboundLaneStorage<T, I>> {
-	InboundLane::new(inbound_lane_storage::<T, I>(lane_id))
+	InboundLane::new(RuntimeInboundLaneStorage::from_lane_id(lane_id))
-}
-
-/// Creates new runtime inbound lane storage.
-fn inbound_lane_storage<T: Config<I>, I: 'static>(
-	lane_id: LaneId,
-) -> RuntimeInboundLaneStorage<T, I> {
-	RuntimeInboundLaneStorage {
-		lane_id,
-		cached_data: RefCell::new(None),
-		_phantom: Default::default(),
-	}
 }
 
 /// Creates new outbound lane object, backed by runtime storage.
@@ -811,10 +759,17 @@ fn outbound_lane<T: Config<I>, I: 'static>(
 /// Runtime inbound lane storage.
 struct RuntimeInboundLaneStorage<T: Config<I>, I: 'static = ()> {
 	lane_id: LaneId,
-	cached_data: RefCell<Option<InboundLaneData<T::InboundRelayer>>>,
+	cached_data: Option<InboundLaneData<T::InboundRelayer>>,
 	_phantom: PhantomData<I>,
 }
 
+impl<T: Config<I>, I: 'static> RuntimeInboundLaneStorage<T, I> {
+	/// Creates new runtime inbound lane storage.
+	fn from_lane_id(lane_id: LaneId) -> RuntimeInboundLaneStorage<T, I> {
+		RuntimeInboundLaneStorage { lane_id, cached_data: None, _phantom: Default::default() }
+	}
+}
+
 impl<T: Config<I>, I: 'static> RuntimeInboundLaneStorage<T, I> {
 	/// Returns number of bytes that may be subtracted from the PoV component of
 	/// `receive_messages_proof` call, because the actual inbound lane state is smaller than the
@@ -824,9 +779,9 @@ impl<T: Config<I>, I: 'static> RuntimeInboundLaneStorage<T, I> {
 	/// `MaxUnrewardedRelayerEntriesAtInboundLane` constant from the pallet configuration. The PoV
 	/// of the call includes the maximal size of inbound lane state. If the actual size is smaller,
 	/// we may subtract extra bytes from this component.
-	pub fn extra_proof_size_bytes(&self) -> u64 {
+	pub fn extra_proof_size_bytes(&mut self) -> u64 {
 		let max_encoded_len = StoredInboundLaneData::<T, I>::max_encoded_len();
-		let relayers_count = self.data().relayers.len();
+		let relayers_count = self.get_or_init_data().relayers.len();
 		let actual_encoded_len =
 			InboundLaneData::<T::InboundRelayer>::encoded_size_hint(relayers_count)
 				.unwrap_or(usize::MAX);
@@ -849,26 +804,20 @@ impl<T: Config<I>, I: 'static> InboundLaneStorage for RuntimeInboundLaneStorage<
 		T::MaxUnconfirmedMessagesAtInboundLane::get()
 	}
 
-	fn data(&self) -> InboundLaneData<T::InboundRelayer> {
+	fn get_or_init_data(&mut self) -> InboundLaneData<T::InboundRelayer> {
-		match self.cached_data.clone().into_inner() {
+		match self.cached_data {
-			Some(data) => data,
+			Some(ref data) => data.clone(),
 			None => {
 				let data: InboundLaneData<T::InboundRelayer> =
 					InboundLanes::<T, I>::get(self.lane_id).into();
-				*self.cached_data.try_borrow_mut().expect(
+				self.cached_data = Some(data.clone());
-					"we're in the single-threaded environment;\
-						we have no recursive borrows; qed",
-				) = Some(data.clone());
 				data
 			},
 		}
 	}
 
 	fn set_data(&mut self, data: InboundLaneData<T::InboundRelayer>) {
-		*self.cached_data.try_borrow_mut().expect(
+		self.cached_data = Some(data.clone());
-			"we're in the single-threaded environment;\
-				we have no recursive borrows; qed",
-		) = Some(data.clone());
 		InboundLanes::<T, I>::insert(self.lane_id, StoredInboundLaneData::<T, I>(data))
 	}
 }
@@ -898,15 +847,17 @@ impl<T: Config<I>, I: 'static> OutboundLaneStorage for RuntimeOutboundLaneStorag
 			.map(Into::into)
 	}
 
-	fn save_message(&mut self, nonce: MessageNonce, message_payload: MessagePayload) {
+	fn save_message(
+		&mut self,
+		nonce: MessageNonce,
+		message_payload: MessagePayload,
+	) -> Result<(), VerificationError> {
 		OutboundMessages::<T, I>::insert(
 			MessageKey { lane_id: self.lane_id, nonce },
-			StoredMessagePayload::<T, I>::try_from(message_payload).expect(
+			StoredMessagePayload::<T, I>::try_from(message_payload)
-				"save_message is called after all checks in send_message; \
+				.map_err(|_| VerificationError::MessageTooLarge)?,
-					send_message checks message size; \
-					qed",
-			),
 		);
+		Ok(())
 	}
 
 	fn remove_message(&mut self, nonce: &MessageNonce) {
@@ -918,7 +869,7 @@ impl<T: Config<I>, I: 'static> OutboundLaneStorage for RuntimeOutboundLaneStorag
 fn verify_and_decode_messages_proof<Chain: SourceHeaderChain, DispatchPayload: Decode>(
 	proof: Chain::MessagesProof,
 	messages_count: u32,
-) -> Result<ProvedMessages<DispatchMessage<DispatchPayload>>, Chain::Error> {
+) -> Result<ProvedMessages<DispatchMessage<DispatchPayload>>, VerificationError> {
 	// `receive_messages_proof` weight formula and `MaxUnconfirmedMessagesAtInboundLane` check
 	// guarantees that the `message_count` is sane and Vec<Message> may be allocated.
 	// (tx with too many messages will either be rejected from the pool, or will fail earlier)
@@ -941,13 +892,16 @@ fn verify_and_decode_messages_proof<Chain: SourceHeaderChain, DispatchPayload: D
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use crate::mock::{
+	use crate::{
-		inbound_unrewarded_relayers_state, message, message_payload, run_test, unrewarded_relayer,
+		mock::{
-		AccountId, DbWeight, RuntimeEvent as TestEvent, RuntimeOrigin,
+			inbound_unrewarded_relayers_state, message, message_payload, run_test,
+			unrewarded_relayer, AccountId, DbWeight, RuntimeEvent as TestEvent, RuntimeOrigin,
 			TestDeliveryConfirmationPayments, TestDeliveryPayments, TestMessagesDeliveryProof,
 			TestMessagesProof, TestRelayer, TestRuntime, TestWeightInfo, MAX_OUTBOUND_PAYLOAD_SIZE,
 			PAYLOAD_REJECTED_BY_TARGET_CHAIN, REGULAR_PAYLOAD, TEST_LANE_ID, TEST_LANE_ID_2,
 			TEST_LANE_ID_3, TEST_RELAYER_A, TEST_RELAYER_B,
+		},
+		outbound_lane::ReceivalConfirmationError,
 	};
 	use bp_messages::{BridgeMessagesCall, UnrewardedRelayer, UnrewardedRelayersState};
 	use bp_test_utils::generate_owned_bridge_module_tests;
@@ -1008,9 +962,9 @@ mod tests {
 			))),
 			UnrewardedRelayersState {
 				unrewarded_relayer_entries: 1,
+				messages_in_oldest_entry: 1,
 				total_messages: 1,
 				last_delivered_nonce: 1,
-				..Default::default()
 			},
 		));
 
@@ -1151,7 +1105,9 @@ mod tests {
 					TEST_LANE_ID,
 					message_payload.clone(),
 				),
-				Error::<TestRuntime, ()>::MessageIsTooLarge,
+				Error::<TestRuntime, ()>::MessageRejectedByPallet(
+					VerificationError::MessageTooLarge
+				),
 			);
 
 			// let's check that we're able to send `MAX_OUTBOUND_PAYLOAD_SIZE` messages
@@ -1177,7 +1133,9 @@ mod tests {
 					TEST_LANE_ID,
 					PAYLOAD_REJECTED_BY_TARGET_CHAIN,
 				),
-				Error::<TestRuntime, ()>::MessageRejectedByChainVerifier,
+				Error::<TestRuntime, ()>::MessageRejectedByChainVerifier(VerificationError::Other(
+					mock::TEST_ERROR
+				)),
 			);
 		});
 	}
@@ -1190,7 +1148,9 @@ mod tests {
 			message.reject_by_lane_verifier = true;
 			assert_noop!(
 				send_message::<TestRuntime, ()>(RuntimeOrigin::signed(1), TEST_LANE_ID, message,),
-				Error::<TestRuntime, ()>::MessageRejectedByLaneVerifier,
+				Error::<TestRuntime, ()>::MessageRejectedByLaneVerifier(VerificationError::Other(
+					mock::TEST_ERROR
+				)),
 			);
 		});
 	}
@@ -1368,9 +1328,9 @@ mod tests {
 				single_message_delivery_proof,
 				UnrewardedRelayersState {
 					unrewarded_relayer_entries: 1,
+					messages_in_oldest_entry: 1,
 					total_messages: 1,
 					last_delivered_nonce: 1,
-					..Default::default()
 				},
 			);
 			assert_ok!(result);
@@ -1408,9 +1368,9 @@ mod tests {
 				two_messages_delivery_proof,
 				UnrewardedRelayersState {
 					unrewarded_relayer_entries: 2,
+					messages_in_oldest_entry: 1,
 					total_messages: 2,
 					last_delivered_nonce: 2,
-					..Default::default()
 				},
 			);
 			assert_ok!(result);
@@ -1773,9 +1733,9 @@ mod tests {
 				TestMessagesDeliveryProof(messages_1_and_2_proof),
 				UnrewardedRelayersState {
 					unrewarded_relayer_entries: 1,
+					messages_in_oldest_entry: 2,
 					total_messages: 2,
 					last_delivered_nonce: 2,
-					..Default::default()
 				},
 			));
 			// second tx with message 3
@@ -1784,9 +1744,9 @@ mod tests {
 				TestMessagesDeliveryProof(messages_3_proof),
 				UnrewardedRelayersState {
 					unrewarded_relayer_entries: 1,
+					messages_in_oldest_entry: 1,
 					total_messages: 1,
 					last_delivered_nonce: 3,
-					..Default::default()
 				},
 			));
 		});
@@ -1814,7 +1774,9 @@ mod tests {
 					))),
 					UnrewardedRelayersState { last_delivered_nonce: 1, ..Default::default() },
 				),
-				Error::<TestRuntime, ()>::TryingToConfirmMoreMessagesThanExpected,
+				Error::<TestRuntime, ()>::ReceivalConfirmation(
+					ReceivalConfirmationError::TryingToConfirmMoreMessagesThanExpected
+				),
 			);
 		});
 	}
@@ -2114,10 +2076,10 @@ mod tests {
 		fn storage(relayer_entries: usize) -> RuntimeInboundLaneStorage<TestRuntime, ()> {
 			RuntimeInboundLaneStorage {
 				lane_id: Default::default(),
-				cached_data: RefCell::new(Some(InboundLaneData {
+				cached_data: Some(InboundLaneData {
 					relayers: vec![relayer_entry(); relayer_entries].into_iter().collect(),
 					last_confirmed_nonce: 0,
-				})),
+				}),
 				_phantom: Default::default(),
 			}
 		}

cumulus/bridges/modules/messages/src/mock.rs

@@ -27,7 +27,7 @@ use bp_messages::{
 		ProvedLaneMessages, ProvedMessages, SourceHeaderChain,
 	},
 	DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, MessageNonce, MessagePayload,
-	OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState,
+	OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, VerificationError,
 };
 use bp_runtime::{messages::MessageDispatchResult, Size};
 use codec::{Decode, Encode};
@@ -295,13 +295,11 @@ impl Size for TestMessagesDeliveryProof {
 pub struct TestTargetHeaderChain;
 
 impl TargetHeaderChain<TestPayload, TestRelayer> for TestTargetHeaderChain {
-	type Error = &'static str;
-
 	type MessagesDeliveryProof = TestMessagesDeliveryProof;
 
-	fn verify_message(payload: &TestPayload) -> Result<(), Self::Error> {
+	fn verify_message(payload: &TestPayload) -> Result<(), VerificationError> {
 		if *payload == PAYLOAD_REJECTED_BY_TARGET_CHAIN {
-			Err(TEST_ERROR)
+			Err(VerificationError::Other(TEST_ERROR))
 		} else {
 			Ok(())
 		}
@@ -309,8 +307,8 @@ impl TargetHeaderChain<TestPayload, TestRelayer> for TestTargetHeaderChain {
 
 	fn verify_messages_delivery_proof(
 		proof: Self::MessagesDeliveryProof,
-	) -> Result<(LaneId, InboundLaneData<TestRelayer>), Self::Error> {
+	) -> Result<(LaneId, InboundLaneData<TestRelayer>), VerificationError> {
-		proof.0.map_err(|_| TEST_ERROR)
+		proof.0.map_err(|_| VerificationError::Other(TEST_ERROR))
 	}
 }
 
@@ -319,18 +317,16 @@ impl TargetHeaderChain<TestPayload, TestRelayer> for TestTargetHeaderChain {
 pub struct TestLaneMessageVerifier;
 
 impl LaneMessageVerifier<RuntimeOrigin, TestPayload> for TestLaneMessageVerifier {
-	type Error = &'static str;
-
 	fn verify_message(
 		_submitter: &RuntimeOrigin,
 		_lane: &LaneId,
 		_lane_outbound_data: &OutboundLaneData,
 		payload: &TestPayload,
-	) -> Result<(), Self::Error> {
+	) -> Result<(), VerificationError> {
 		if !payload.reject_by_lane_verifier {
 			Ok(())
 		} else {
-			Err(TEST_ERROR)
+			Err(VerificationError::Other(TEST_ERROR))
 		}
 	}
 }
@@ -400,15 +396,16 @@ impl DeliveryConfirmationPayments<AccountId> for TestDeliveryConfirmationPayment
 pub struct TestSourceHeaderChain;
 
 impl SourceHeaderChain for TestSourceHeaderChain {
-	type Error = &'static str;
-
 	type MessagesProof = TestMessagesProof;
 
 	fn verify_messages_proof(
 		proof: Self::MessagesProof,
 		_messages_count: u32,
-	) -> Result<ProvedMessages<Message>, Self::Error> {
+	) -> Result<ProvedMessages<Message>, VerificationError> {
-		proof.result.map(|proof| proof.into_iter().collect()).map_err(|_| TEST_ERROR)
+		proof
+			.result
+			.map(|proof| proof.into_iter().collect())
+			.map_err(|_| VerificationError::Other(TEST_ERROR))
 	}
 }
 

cumulus/bridges/modules/messages/src/outbound_lane.rs

@@ -16,16 +16,19 @@
 
 //! Everything about outgoing messages sending.
 
-use crate::Config;
+use crate::{Config, LOG_TARGET};
 
 use bp_messages::{
 	DeliveredMessages, LaneId, MessageNonce, MessagePayload, OutboundLaneData, UnrewardedRelayer,
+	VerificationError,
 };
+use codec::{Decode, Encode};
 use frame_support::{
 	weights::{RuntimeDbWeight, Weight},
-	BoundedVec, RuntimeDebug,
+	BoundedVec, PalletError, RuntimeDebug,
 };
 use num_traits::Zero;
+use scale_info::TypeInfo;
 use sp_std::collections::vec_deque::VecDeque;
 
 /// Outbound lane storage.
@@ -40,7 +43,11 @@ pub trait OutboundLaneStorage {
 	#[cfg(test)]
 	fn message(&self, nonce: &MessageNonce) -> Option<MessagePayload>;
 	/// Save outbound message in the storage.
-	fn save_message(&mut self, nonce: MessageNonce, message_payload: MessagePayload);
+	fn save_message(
+		&mut self,
+		nonce: MessageNonce,
+		message_payload: MessagePayload,
+	) -> Result<(), VerificationError>;
 	/// Remove outbound message from the storage.
 	fn remove_message(&mut self, nonce: &MessageNonce);
 }
@@ -49,13 +56,8 @@ pub trait OutboundLaneStorage {
 pub type StoredMessagePayload<T, I> = BoundedVec<u8, <T as Config<I>>::MaximalOutboundPayloadSize>;
 
 /// Result of messages receival confirmation.
-#[derive(RuntimeDebug, PartialEq, Eq)]
+#[derive(Encode, Decode, RuntimeDebug, PartialEq, Eq, PalletError, TypeInfo)]
-pub enum ReceivalConfirmationResult {
+pub enum ReceivalConfirmationError {
-	/// New messages have been confirmed by the confirmation transaction.
-	ConfirmedMessages(DeliveredMessages),
-	/// Confirmation transaction brings no new confirmation. This may be a result of relayer
-	/// error or several relayers running.
-	NoNewConfirmations,
 	/// Bridged chain is trying to confirm more messages than we have generated. May be a result
 	/// of invalid bridged chain storage.
 	FailedToConfirmFutureMessages,
@@ -66,7 +68,7 @@ pub enum ReceivalConfirmationResult {
 	/// bridged chain storage.
 	NonConsecutiveUnrewardedRelayerEntries,
 	/// The chain has more messages that need to be confirmed than there is in the proof.
-	TryingToConfirmMoreMessagesThanExpected(MessageNonce),
+	TryingToConfirmMoreMessagesThanExpected,
 }
 
 /// Outbound messages lane.
@@ -88,15 +90,18 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
 	/// Send message over lane.
 	///
 	/// Returns new message nonce.
-	pub fn send_message(&mut self, message_payload: MessagePayload) -> MessageNonce {
+	pub fn send_message(
+		&mut self,
+		message_payload: MessagePayload,
+	) -> Result<MessageNonce, VerificationError> {
 		let mut data = self.storage.data();
 		let nonce = data.latest_generated_nonce + 1;
 		data.latest_generated_nonce = nonce;
 
-		self.storage.save_message(nonce, message_payload);
+		self.storage.save_message(nonce, message_payload)?;
 		self.storage.set_data(data);
 
-		nonce
+		Ok(nonce)
 	}
 
 	/// Confirm messages delivery.
@@ -105,37 +110,39 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
 		max_allowed_messages: MessageNonce,
 		latest_delivered_nonce: MessageNonce,
 		relayers: &VecDeque<UnrewardedRelayer<RelayerId>>,
-	) -> ReceivalConfirmationResult {
+	) -> Result<Option<DeliveredMessages>, ReceivalConfirmationError> {
 		let mut data = self.storage.data();
-		if latest_delivered_nonce <= data.latest_received_nonce {
+		let confirmed_messages = DeliveredMessages {
-			return ReceivalConfirmationResult::NoNewConfirmations
+			begin: data.latest_received_nonce.saturating_add(1),
+			end: latest_delivered_nonce,
+		};
+		if confirmed_messages.total_messages() == 0 {
+			return Ok(None)
 		}
-		if latest_delivered_nonce > data.latest_generated_nonce {
+		if confirmed_messages.end > data.latest_generated_nonce {
-			return ReceivalConfirmationResult::FailedToConfirmFutureMessages
+			return Err(ReceivalConfirmationError::FailedToConfirmFutureMessages)
 		}
-		if latest_delivered_nonce - data.latest_received_nonce > max_allowed_messages {
+		if confirmed_messages.total_messages() > max_allowed_messages {
 			// that the relayer has declared correct number of messages that the proof contains (it
 			// is checked outside of the function). But it may happen (but only if this/bridged
 			// chain storage is corrupted, though) that the actual number of confirmed messages if
 			// larger than declared. This would mean that 'reward loop' will take more time than the
 			// weight formula accounts, so we can't allow that.
-			return ReceivalConfirmationResult::TryingToConfirmMoreMessagesThanExpected(
+			log::trace!(
-				latest_delivered_nonce - data.latest_received_nonce,
+				target: LOG_TARGET,
-			)
+				"Messages delivery proof contains too many messages to confirm: {} vs declared {}",
+				confirmed_messages.total_messages(),
+				max_allowed_messages,
+			);
+			return Err(ReceivalConfirmationError::TryingToConfirmMoreMessagesThanExpected)
 		}
 
-		if let Err(e) = ensure_unrewarded_relayers_are_correct(latest_delivered_nonce, relayers) {
+		ensure_unrewarded_relayers_are_correct(confirmed_messages.end, relayers)?;
-			return e
-		}
 
-		let prev_latest_received_nonce = data.latest_received_nonce;
+		data.latest_received_nonce = confirmed_messages.end;
-		data.latest_received_nonce = latest_delivered_nonce;
 		self.storage.set_data(data);
 
-		ReceivalConfirmationResult::ConfirmedMessages(DeliveredMessages {
+		Ok(Some(confirmed_messages))
-			begin: prev_latest_received_nonce + 1,
-			end: latest_delivered_nonce,
-		})
 	}
 
 	/// Prune at most `max_messages_to_prune` already received messages.
@@ -176,27 +183,24 @@ impl<S: OutboundLaneStorage> OutboundLane<S> {
 fn ensure_unrewarded_relayers_are_correct<RelayerId>(
 	latest_received_nonce: MessageNonce,
 	relayers: &VecDeque<UnrewardedRelayer<RelayerId>>,
-) -> Result<(), ReceivalConfirmationResult> {
+) -> Result<(), ReceivalConfirmationError> {
-	let mut last_entry_end: Option<MessageNonce> = None;
+	let mut expected_entry_begin = relayers.front().map(|entry| entry.messages.begin);
 	for entry in relayers {
 		// unrewarded relayer entry must have at least 1 unconfirmed message
 		// (guaranteed by the `InboundLane::receive_message()`)
 		if entry.messages.end < entry.messages.begin {
-			return Err(ReceivalConfirmationResult::EmptyUnrewardedRelayerEntry)
+			return Err(ReceivalConfirmationError::EmptyUnrewardedRelayerEntry)
 		}
 		// every entry must confirm range of messages that follows previous entry range
 		// (guaranteed by the `InboundLane::receive_message()`)
-		if let Some(last_entry_end) = last_entry_end {
-			let expected_entry_begin = last_entry_end.checked_add(1);
 		if expected_entry_begin != Some(entry.messages.begin) {
-				return Err(ReceivalConfirmationResult::NonConsecutiveUnrewardedRelayerEntries)
+			return Err(ReceivalConfirmationError::NonConsecutiveUnrewardedRelayerEntries)
 		}
-		}
+		expected_entry_begin = entry.messages.end.checked_add(1);
-		last_entry_end = Some(entry.messages.end);
 		// entry can't confirm messages larger than `inbound_lane_data.latest_received_nonce()`
 		// (guaranteed by the `InboundLane::receive_message()`)
 		if entry.messages.end > latest_received_nonce {
-			return Err(ReceivalConfirmationResult::FailedToConfirmFutureMessages)
+			return Err(ReceivalConfirmationError::FailedToConfirmFutureMessages)
 		}
 	}
 
@@ -213,7 +217,7 @@ mod tests {
 		},
 		outbound_lane,
 	};
-	use frame_support::weights::constants::RocksDbWeight;
+	use frame_support::{assert_ok, weights::constants::RocksDbWeight};
 	use sp_std::ops::RangeInclusive;
 
 	fn unrewarded_relayers(
@@ -231,12 +235,12 @@ mod tests {
 	fn assert_3_messages_confirmation_fails(
 		latest_received_nonce: MessageNonce,
 		relayers: &VecDeque<UnrewardedRelayer<TestRelayer>>,
-	) -> ReceivalConfirmationResult {
+	) -> Result<Option<DeliveredMessages>, ReceivalConfirmationError> {
 		run_test(|| {
 			let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
 			assert_eq!(lane.storage.data().latest_generated_nonce, 3);
 			assert_eq!(lane.storage.data().latest_received_nonce, 0);
 			let result = lane.confirm_delivery(3, latest_received_nonce, relayers);
@@ -251,7 +255,7 @@ mod tests {
 		run_test(|| {
 			let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
 			assert_eq!(lane.storage.data().latest_generated_nonce, 0);
-			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1);
+			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(1));
 			assert!(lane.storage.message(&1).is_some());
 			assert_eq!(lane.storage.data().latest_generated_nonce, 1);
 		});
@@ -261,14 +265,14 @@ mod tests {
 	fn confirm_delivery_works() {
 		run_test(|| {
 			let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
-			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 1);
+			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(1));
-			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 2);
+			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(2));
-			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), 3);
+			assert_eq!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)), Ok(3));
 			assert_eq!(lane.storage.data().latest_generated_nonce, 3);
 			assert_eq!(lane.storage.data().latest_received_nonce, 0);
 			assert_eq!(
 				lane.confirm_delivery(3, 3, &unrewarded_relayers(1..=3)),
-				ReceivalConfirmationResult::ConfirmedMessages(delivered_messages(1..=3)),
+				Ok(Some(delivered_messages(1..=3))),
 			);
 			assert_eq!(lane.storage.data().latest_generated_nonce, 3);
 			assert_eq!(lane.storage.data().latest_received_nonce, 3);
@@ -279,26 +283,20 @@ mod tests {
 	fn confirm_delivery_rejects_nonce_lesser_than_latest_received() {
 		run_test(|| {
 			let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
 			assert_eq!(lane.storage.data().latest_generated_nonce, 3);
 			assert_eq!(lane.storage.data().latest_received_nonce, 0);
 			assert_eq!(
 				lane.confirm_delivery(3, 3, &unrewarded_relayers(1..=3)),
-				ReceivalConfirmationResult::ConfirmedMessages(delivered_messages(1..=3)),
+				Ok(Some(delivered_messages(1..=3))),
-			);
-			assert_eq!(
-				lane.confirm_delivery(3, 3, &unrewarded_relayers(1..=3)),
-				ReceivalConfirmationResult::NoNewConfirmations,
 			);
+			assert_eq!(lane.confirm_delivery(3, 3, &unrewarded_relayers(1..=3)), Ok(None),);
 			assert_eq!(lane.storage.data().latest_generated_nonce, 3);
 			assert_eq!(lane.storage.data().latest_received_nonce, 3);
 
-			assert_eq!(
+			assert_eq!(lane.confirm_delivery(1, 2, &unrewarded_relayers(1..=1)), Ok(None),);
-				lane.confirm_delivery(1, 2, &unrewarded_relayers(1..=1)),
-				ReceivalConfirmationResult::NoNewConfirmations,
-			);
 			assert_eq!(lane.storage.data().latest_generated_nonce, 3);
 			assert_eq!(lane.storage.data().latest_received_nonce, 3);
 		});
@@ -308,7 +306,7 @@ mod tests {
 	fn confirm_delivery_rejects_nonce_larger_than_last_generated() {
 		assert_eq!(
 			assert_3_messages_confirmation_fails(10, &unrewarded_relayers(1..=10),),
-			ReceivalConfirmationResult::FailedToConfirmFutureMessages,
+			Err(ReceivalConfirmationError::FailedToConfirmFutureMessages),
 		);
 	}
 
@@ -323,7 +321,7 @@ mod tests {
 					.chain(unrewarded_relayers(3..=3).into_iter())
 					.collect(),
 			),
-			ReceivalConfirmationResult::FailedToConfirmFutureMessages,
+			Err(ReceivalConfirmationError::FailedToConfirmFutureMessages),
 		);
 	}
 
@@ -339,7 +337,7 @@ mod tests {
 					.chain(unrewarded_relayers(2..=3).into_iter())
 					.collect(),
 			),
-			ReceivalConfirmationResult::EmptyUnrewardedRelayerEntry,
+			Err(ReceivalConfirmationError::EmptyUnrewardedRelayerEntry),
 		);
 	}
 
@@ -354,7 +352,7 @@ mod tests {
 					.chain(unrewarded_relayers(2..=2).into_iter())
 					.collect(),
 			),
-			ReceivalConfirmationResult::NonConsecutiveUnrewardedRelayerEntries,
+			Err(ReceivalConfirmationError::NonConsecutiveUnrewardedRelayerEntries),
 		);
 	}
 
@@ -369,9 +367,9 @@ mod tests {
 			);
 			assert_eq!(lane.storage.data().oldest_unpruned_nonce, 1);
 			// when nothing is confirmed, nothing is pruned
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
 			assert!(lane.storage.message(&1).is_some());
 			assert!(lane.storage.message(&2).is_some());
 			assert!(lane.storage.message(&3).is_some());
@@ -383,7 +381,7 @@ mod tests {
 			// after confirmation, some messages are received
 			assert_eq!(
 				lane.confirm_delivery(2, 2, &unrewarded_relayers(1..=2)),
-				ReceivalConfirmationResult::ConfirmedMessages(delivered_messages(1..=2)),
+				Ok(Some(delivered_messages(1..=2))),
 			);
 			assert_eq!(
 				lane.prune_messages(RocksDbWeight::get(), RocksDbWeight::get().writes(101)),
@@ -396,7 +394,7 @@ mod tests {
 			// after last message is confirmed, everything is pruned
 			assert_eq!(
 				lane.confirm_delivery(1, 3, &unrewarded_relayers(3..=3)),
-				ReceivalConfirmationResult::ConfirmedMessages(delivered_messages(3..=3)),
+				Ok(Some(delivered_messages(3..=3))),
 			);
 			assert_eq!(
 				lane.prune_messages(RocksDbWeight::get(), RocksDbWeight::get().writes(101)),
@@ -413,20 +411,20 @@ mod tests {
 	fn confirm_delivery_detects_when_more_than_expected_messages_are_confirmed() {
 		run_test(|| {
 			let mut lane = outbound_lane::<TestRuntime, _>(TEST_LANE_ID);
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
-			lane.send_message(outbound_message_data(REGULAR_PAYLOAD));
+			assert_ok!(lane.send_message(outbound_message_data(REGULAR_PAYLOAD)));
 			assert_eq!(
 				lane.confirm_delivery(0, 3, &unrewarded_relayers(1..=3)),
-				ReceivalConfirmationResult::TryingToConfirmMoreMessagesThanExpected(3),
+				Err(ReceivalConfirmationError::TryingToConfirmMoreMessagesThanExpected),
 			);
 			assert_eq!(
 				lane.confirm_delivery(2, 3, &unrewarded_relayers(1..=3)),
-				ReceivalConfirmationResult::TryingToConfirmMoreMessagesThanExpected(3),
+				Err(ReceivalConfirmationError::TryingToConfirmMoreMessagesThanExpected),
 			);
 			assert_eq!(
 				lane.confirm_delivery(3, 3, &unrewarded_relayers(1..=3)),
-				ReceivalConfirmationResult::ConfirmedMessages(delivered_messages(1..=3)),
+				Ok(Some(delivered_messages(1..=3))),
 			);
 		});
 	}

cumulus/bridges/modules/parachains/src/lib.rs

@@ -91,6 +91,8 @@ pub mod pallet {
 		BoundedStorageValue<<T as Config<I>>::MaxParaHeadDataSize, ParaStoredHeaderData>;
 	/// Weight info of the given parachains pallet.
 	pub type WeightInfoOf<T, I> = <T as Config<I>>::WeightInfo;
+	type GrandpaPalletOf<T, I> =
+		pallet_bridge_grandpa::Pallet<T, <T as Config<I>>::BridgesGrandpaPalletInstance>;
 
 	#[pallet::event]
 	#[pallet::generate_deposit(pub(super) fn deposit_event)]
@@ -125,14 +127,8 @@ pub mod pallet {
 		UnknownRelayChainBlock,
 		/// The number of stored relay block is different from what the relayer has provided.
 		InvalidRelayChainBlockNumber,
-		/// Error generated by a method defined in `bp-header-chain`.
+		/// Parachain heads storage proof is invalid.
-		HeaderChain(HeaderChainError),
+		HeaderChainStorageProof(HeaderChainError),
-		/// Given parachain head is unknown.
-		UnknownParaHead,
-		/// The storage proof doesn't contains storage root. So it is invalid for given header.
-		StorageRootMismatch,
-		/// Failed to extract state root from given parachain head.
-		FailedToExtractStateRoot,
 		/// Error generated by the `OwnedBridgeModule` trait.
 		BridgeModule(bp_runtime::OwnedBridgeModuleError),
 	}
@@ -337,12 +333,14 @@ pub mod pallet {
 				parachains.len() as _,
 			);
 
-			pallet_bridge_grandpa::Pallet::<T, T::BridgesGrandpaPalletInstance>::parse_finalized_storage_proof(
+			let mut storage = GrandpaPalletOf::<T, I>::storage_proof_checker(
 				relay_block_hash,
 				parachain_heads_proof.0,
-				move |mut storage| {
+			)
+			.map_err(Error::<T, I>::HeaderChainStorageProof)?;
+
 			for (parachain, parachain_head_hash) in parachains {
-						let parachain_head = match Pallet::<T, I>::read_parachain_head(&mut storage, parachain) {
+				let parachain_head = match Self::read_parachain_head(&mut storage, parachain) {
 					Ok(Some(parachain_head)) => parachain_head,
 					Ok(None) => {
 						log::trace!(
@@ -356,7 +354,7 @@ pub mod pallet {
 							},
 						);
 						Self::deposit_event(Event::MissingParachainHead { parachain });
-								continue;
+						continue
 					},
 					Err(e) => {
 						log::trace!(
@@ -366,7 +364,7 @@ pub mod pallet {
 							e,
 						);
 						Self::deposit_event(Event::MissingParachainHead { parachain });
-								continue;
+						continue
 					},
 				};
 
@@ -376,7 +374,8 @@ pub mod pallet {
 				if parachain_head_hash != actual_parachain_head_hash {
 					log::trace!(
 						target: LOG_TARGET,
-								"The submitter has specified invalid parachain {:?} head hash: {:?} vs {:?}",
+						"The submitter has specified invalid parachain {:?} head hash: \
+								{:?} vs {:?}",
 						parachain,
 						parachain_head_hash,
 						actual_parachain_head_hash,
@@ -386,14 +385,12 @@ pub mod pallet {
 						parachain_head_hash,
 						actual_parachain_head_hash,
 					});
-							continue;
+					continue
 				}
 
 				// convert from parachain head into stored parachain head data
-						let parachain_head_data = match T::ParaStoredHeaderDataBuilder::try_build(
+				let parachain_head_data =
-							parachain,
+					match T::ParaStoredHeaderDataBuilder::try_build(parachain, &parachain_head) {
-							&parachain_head,
-						) {
 						Some(parachain_head_data) => parachain_head_data,
 						None => {
 							log::trace!(
@@ -402,11 +399,12 @@ pub mod pallet {
 								parachain,
 							);
 							Self::deposit_event(Event::UntrackedParachainRejected { parachain });
-								continue;
+							continue
 						},
 					};
 
-						let update_result: Result<_, ()> = ParasInfo::<T, I>::try_mutate(parachain, |stored_best_head| {
+				let update_result: Result<_, ()> =
+					ParasInfo::<T, I>::try_mutate(parachain, |stored_best_head| {
 						let artifacts = Pallet::<T, I>::update_parachain_head(
 							parachain,
 							stored_best_head.take(),
@@ -421,28 +419,27 @@ pub mod pallet {
 				// we're refunding weight if update has not happened and if pruning has not happened
 				let is_update_happened = matches!(update_result, Ok(_));
 				if !is_update_happened {
-							actual_weight = actual_weight
+					actual_weight = actual_weight.saturating_sub(
-								.saturating_sub(WeightInfoOf::<T, I>::parachain_head_storage_write_weight(T::DbWeight::get()));
+						WeightInfoOf::<T, I>::parachain_head_storage_write_weight(
+							T::DbWeight::get(),
+						),
+					);
 				}
 				let is_prune_happened = matches!(update_result, Ok(true));
 				if !is_prune_happened {
-							actual_weight = actual_weight
+					actual_weight = actual_weight.saturating_sub(
-								.saturating_sub(WeightInfoOf::<T, I>::parachain_head_pruning_weight(T::DbWeight::get()));
+						WeightInfoOf::<T, I>::parachain_head_pruning_weight(T::DbWeight::get()),
+					);
 				}
 			}
 
-					// even though we may have accepted some parachain heads, we can't allow relayers to submit
+			// even though we may have accepted some parachain heads, we can't allow relayers to
-					// proof with unused trie nodes
+			// submit proof with unused trie nodes
 			// => treat this as an error
 			//
 			// (we can throw error here, because now all our calls are transactional)
-					storage.ensure_no_unused_nodes()
+			storage.ensure_no_unused_nodes().map_err(|e| {
-				},
+				Error::<T, I>::HeaderChainStorageProof(HeaderChainError::StorageProof(e))
-			)
-			.and_then(|r| r.map_err(HeaderChainError::StorageProof))
-			.map_err(|e| {
-				log::trace!(target: LOG_TARGET, "Parachain heads storage proof is invalid: {:?}", e);
-				Error::<T, I>::HeaderChain(e)
 			})?;
 
 			Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee: Pays::Yes })
@@ -1438,7 +1435,7 @@ pub(crate) mod tests {
 			// try to import head#5 of parachain#1 at relay chain block #0
 			assert_noop!(
 				import_parachain_1_head(0, Default::default(), parachains, proof),
-				Error::<TestRuntime>::HeaderChain(HeaderChainError::StorageProof(
+				Error::<TestRuntime>::HeaderChainStorageProof(HeaderChainError::StorageProof(
 					StorageProofError::StorageRootMismatch
 				))
 			);

cumulus/bridges/primitives/chain-bridge-hub-kusama/src/lib.rs

@@ -72,12 +72,10 @@ pub type Address = MultiAddress<AccountId, ()>;
 pub const BRIDGE_HUB_KUSAMA_PARACHAIN_ID: u32 = 1002;
 
 /// Name of the With-BridgeHubKusama messages pallet instance that is deployed at bridged chains.
-// TODO: check me (https://github.com/paritytech/parity-bridges-common/issues/1945)
 pub const WITH_BRIDGE_HUB_KUSAMA_MESSAGES_PALLET_NAME: &str = "BridgeKusamaMessages";
 
 /// Name of the With-BridgeHubKusama bridge-relayers pallet instance that is deployed at bridged
 /// chains.
-// TODO: check me (https://github.com/paritytech/parity-bridges-common/issues/1945)
 pub const WITH_BRIDGE_HUB_KUSAMA_RELAYERS_PALLET_NAME: &str = "BridgeRelayers";
 
 decl_bridge_finality_runtime_apis!(bridge_hub_kusama);

cumulus/bridges/primitives/chain-bridge-hub-polkadot/src/lib.rs

@@ -59,16 +59,13 @@ impl Parachain for BridgeHubPolkadot {
 }
 
 /// Identifier of BridgeHubPolkadot in the Polkadot relay chain.
-// TODO: check me (https://github.com/paritytech/parity-bridges-common/issues/1945)
 pub const BRIDGE_HUB_POLKADOT_PARACHAIN_ID: u32 = 1002;
 
 /// Name of the With-BridgeHubPolkadot messages pallet instance that is deployed at bridged chains.
-// TODO: check me (https://github.com/paritytech/parity-bridges-common/issues/1945)
 pub const WITH_BRIDGE_HUB_POLKADOT_MESSAGES_PALLET_NAME: &str = "BridgePolkadotMessages";
 
 /// Name of the With-BridgeHubPolkadot bridge-relayers pallet instance that is deployed at bridged
 /// chains.
-// TODO: check me (https://github.com/paritytech/parity-bridges-common/issues/1945)
 pub const WITH_BRIDGE_HUB_POLKADOT_RELAYERS_PALLET_NAME: &str = "BridgeRelayers";
 
 decl_bridge_finality_runtime_apis!(bridge_hub_polkadot);

cumulus/bridges/primitives/chain-kusama/src/lib.rs

@@ -62,4 +62,11 @@ pub const PARAS_PALLET_NAME: &str = "Paras";
 /// Name of the With-Kusama GRANDPA pallet instance that is deployed at bridged chains.
 pub const WITH_KUSAMA_GRANDPA_PALLET_NAME: &str = "BridgeKusamaGrandpa";
 
+/// Maximal size of encoded `bp_parachains::ParaStoredHeaderData` structure among all Polkadot
+/// parachains.
+///
+/// It includes the block number and state root, so it shall be near 40 bytes, but let's have some
+/// reserve.
+pub const MAX_NESTED_PARACHAIN_HEAD_DATA_SIZE: u32 = 128;
+
 decl_bridge_finality_runtime_apis!(kusama);

cumulus/bridges/primitives/chain-polkadot/src/lib.rs

@@ -62,4 +62,11 @@ pub const PARAS_PALLET_NAME: &str = "Paras";
 /// Name of the With-Polkadot GRANDPA pallet instance that is deployed at bridged chains.
 pub const WITH_POLKADOT_GRANDPA_PALLET_NAME: &str = "BridgePolkadotGrandpa";
 
+/// Maximal size of encoded `bp_parachains::ParaStoredHeaderData` structure among all Polkadot
+/// parachains.
+///
+/// It includes the block number and state root, so it shall be near 40 bytes, but let's have some
+/// reserve.
+pub const MAX_NESTED_PARACHAIN_HEAD_DATA_SIZE: u32 = 128;
+
 decl_bridge_finality_runtime_apis!(polkadot);

cumulus/bridges/primitives/header-chain/src/justification.rs

@@ -49,6 +49,7 @@ pub struct GrandpaJustification<Header: HeaderT> {
 	pub votes_ancestries: Vec<Header>,
 }
 
+// TODO: remove and use `RuntimeDebug` (https://github.com/paritytech/parity-bridges-common/issues/2136)
 impl<Header: HeaderT> sp_std::fmt::Debug for GrandpaJustification<Header> {
 	fn fmt(&self, fmt: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result {
 		#[cfg(feature = "std")]

cumulus/bridges/primitives/header-chain/src/lib.rs

@@ -73,18 +73,14 @@ impl<H: HeaderT> StoredHeaderDataBuilder<H::Number, H::Hash> for H {
 pub trait HeaderChain<C: Chain> {
 	/// Returns state (storage) root of given finalized header.
 	fn finalized_header_state_root(header_hash: HashOf<C>) -> Option<HashOf<C>>;
-	/// Parse storage proof using finalized header.
+	/// Get storage proof checker using finalized header.
-	fn parse_finalized_storage_proof<R>(
+	fn storage_proof_checker(
 		header_hash: HashOf<C>,
 		storage_proof: RawStorageProof,
-		parse: impl FnOnce(StorageProofChecker<HasherOf<C>>) -> R,
+	) -> Result<StorageProofChecker<HasherOf<C>>, HeaderChainError> {
-	) -> Result<R, HeaderChainError> {
 		let state_root = Self::finalized_header_state_root(header_hash)
 			.ok_or(HeaderChainError::UnknownHeader)?;
-		let storage_proof_checker = bp_runtime::StorageProofChecker::new(state_root, storage_proof)
+		StorageProofChecker::new(state_root, storage_proof).map_err(HeaderChainError::StorageProof)
-			.map_err(HeaderChainError::StorageProof)?;
-
-		Ok(parse(storage_proof_checker))
 	}
 }
 

cumulus/bridges/primitives/messages/Cargo.toml

@@ -14,6 +14,7 @@ serde = { version = "1.0", optional = true, features = ["derive"] }
 # Bridge dependencies
 
 bp-runtime = { path = "../runtime", default-features = false }
+bp-header-chain = { path = "../header-chain", default-features = false }
 
 # Substrate Dependencies
 
@@ -29,6 +30,7 @@ hex-literal = "0.4"
 default = ["std"]
 std = [
 	"bp-runtime/std",
+	"bp-header-chain/std",
 	"codec/std",
 	"frame-support/std",
 	"scale-info/std",

cumulus/bridges/primitives/messages/src/lib.rs

@@ -20,9 +20,15 @@
 // RuntimeApi generated functions
 #![allow(clippy::too_many_arguments)]
 
-use bp_runtime::{BasicOperatingMode, OperatingMode, RangeInclusiveExt};
+use bp_header_chain::HeaderChainError;
+use bp_runtime::{
+	messages::MessageDispatchResult, BasicOperatingMode, OperatingMode, RangeInclusiveExt,
+	StorageProofError,
+};
 use codec::{Decode, Encode, MaxEncodedLen};
-use frame_support::RuntimeDebug;
+use frame_support::{PalletError, RuntimeDebug};
+// Weight is reexported to avoid additional frame-support dependencies in related crates.
+pub use frame_support::weights::Weight;
 use scale_info::TypeInfo;
 use source_chain::RelayersRewards;
 use sp_core::TypeId;
@@ -32,10 +38,6 @@ pub mod source_chain;
 pub mod storage_keys;
 pub mod target_chain;
 
-use bp_runtime::messages::MessageDispatchResult;
-// Weight is reexported to avoid additional frame-support dependencies in related crates.
-pub use frame_support::weights::Weight;
-
 /// Messages pallet operating mode.
 #[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
 #[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
@@ -189,6 +191,17 @@ impl<RelayerId> InboundLaneData<RelayerId> {
 			.map(|entry| entry.messages.end)
 			.unwrap_or(self.last_confirmed_nonce)
 	}
+
+	/// Returns the total number of messages in the `relayers` vector,
+	/// saturating in case of underflow or overflow.
+	pub fn total_unrewarded_messages(&self) -> MessageNonce {
+		let relayers = &self.relayers;
+		match (relayers.front(), relayers.back()) {
+			(Some(front), Some(back)) =>
+				(front.messages.begin..=back.messages.end).saturating_len(),
+			_ => 0,
+		}
+	}
 }
 
 /// Outbound message details, returned by runtime APIs.
@@ -285,7 +298,7 @@ impl DeliveredMessages {
 
 	/// Return total count of delivered messages.
 	pub fn total_messages(&self) -> MessageNonce {
-		(self.begin..=self.end).checked_len().unwrap_or(0)
+		(self.begin..=self.end).saturating_len()
 	}
 
 	/// Note new dispatched message.
@@ -316,6 +329,13 @@ pub struct UnrewardedRelayersState {
 	pub last_delivered_nonce: MessageNonce,
 }
 
+impl UnrewardedRelayersState {
+	// Verify that the relayers state corresponds with the `InboundLaneData`.
+	pub fn is_valid<RelayerId>(&self, lane_data: &InboundLaneData<RelayerId>) -> bool {
+		self == &lane_data.into()
+	}
+}
+
 impl<RelayerId> From<&InboundLaneData<RelayerId>> for UnrewardedRelayersState {
 	fn from(lane: &InboundLaneData<RelayerId>) -> UnrewardedRelayersState {
 		UnrewardedRelayersState {
@@ -323,9 +343,9 @@ impl<RelayerId> From<&InboundLaneData<RelayerId>> for UnrewardedRelayersState {
 			messages_in_oldest_entry: lane
 				.relayers
 				.front()
-				.and_then(|entry| (entry.messages.begin..=entry.messages.end).checked_len())
+				.map(|entry| entry.messages.total_messages())
 				.unwrap_or(0),
-			total_messages: total_unrewarded_messages(&lane.relayers).unwrap_or(MessageNonce::MAX),
+			total_messages: lane.total_unrewarded_messages(),
 			last_delivered_nonce: lane.last_delivered_nonce(),
 		}
 	}
@@ -355,24 +375,6 @@ impl Default for OutboundLaneData {
 	}
 }
 
-/// Returns total number of messages in the `InboundLaneData::relayers` vector.
-///
-/// Returns `None` if there are more messages that `MessageNonce` may fit (i.e. `MessageNonce + 1`).
-pub fn total_unrewarded_messages<RelayerId>(
-	relayers: &VecDeque<UnrewardedRelayer<RelayerId>>,
-) -> Option<MessageNonce> {
-	match (relayers.front(), relayers.back()) {
-		(Some(front), Some(back)) => {
-			if let Some(difference) = back.messages.end.checked_sub(front.messages.begin) {
-				difference.checked_add(1)
-			} else {
-				Some(0)
-			}
-		},
-		_ => Some(0),
-	}
-}
-
 /// Calculate the number of messages that the relayers have delivered.
 pub fn calc_relayers_rewards<AccountId>(
 	messages_relayers: VecDeque<UnrewardedRelayer<AccountId>>,
@@ -414,26 +416,50 @@ pub enum BridgeMessagesCall<AccountId, MessagesProof, MessagesDeliveryProof> {
 	},
 }
 
+/// Error that happens during message verification.
+#[derive(Encode, Decode, RuntimeDebug, PartialEq, Eq, PalletError, TypeInfo)]
+pub enum VerificationError {
+	/// The message proof is empty.
+	EmptyMessageProof,
+	/// Error returned by the bridged header chain.
+	HeaderChain(HeaderChainError),
+	/// Error returned while reading/decoding inbound lane data from the storage proof.
+	InboundLaneStorage(StorageProofError),
+	/// The declared message weight is incorrect.
+	InvalidMessageWeight,
+	/// Declared messages count doesn't match actual value.
+	MessagesCountMismatch,
+	/// Error returned while reading/decoding message data from the storage proof.
+	MessageStorage(StorageProofError),
+	/// The message is too large.
+	MessageTooLarge,
+	/// Error returned while reading/decoding outbound lane data from the storage proof.
+	OutboundLaneStorage(StorageProofError),
+	/// Storage proof related error.
+	StorageProof(StorageProofError),
+	/// Custom error
+	Other(#[codec(skip)] &'static str),
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;
 
 	#[test]
 	fn total_unrewarded_messages_does_not_overflow() {
-		assert_eq!(
+		let lane_data = InboundLaneData {
-			total_unrewarded_messages(
+			relayers: vec![
-				&vec![
 				UnrewardedRelayer { relayer: 1, messages: DeliveredMessages::new(0) },
 				UnrewardedRelayer {
 					relayer: 2,
-						messages: DeliveredMessages::new(MessageNonce::MAX)
+					messages: DeliveredMessages::new(MessageNonce::MAX),
 				},
 			]
 			.into_iter()
-				.collect()
+			.collect(),
-			),
+			last_confirmed_nonce: 0,
-			None,
+		};
-		);
+		assert_eq!(lane_data.total_unrewarded_messages(), MessageNonce::MAX);
 	}
 
 	#[test]

cumulus/bridges/primitives/messages/src/source_chain.rs

@@ -16,7 +16,7 @@
 
 //! Primitives of messages module, that are used on the source chain.
 
-use crate::{InboundLaneData, LaneId, MessageNonce, OutboundLaneData};
+use crate::{InboundLaneData, LaneId, MessageNonce, OutboundLaneData, VerificationError};
 
 use crate::UnrewardedRelayer;
 use bp_runtime::Size;
@@ -40,9 +40,6 @@ pub type RelayersRewards<AccountId> = BTreeMap<AccountId, MessageNonce>;
 /// source chain to the target chain. The `AccountId` type here means the account
 /// type used by the source chain.
 pub trait TargetHeaderChain<Payload, AccountId> {
-	/// Error type.
-	type Error: Debug;
-
 	/// Proof that messages have been received by target chain.
 	type MessagesDeliveryProof: Parameter + Size;
 
@@ -58,12 +55,12 @@ pub trait TargetHeaderChain<Payload, AccountId> {
 	/// 1MB. BTC nodes aren't accepting transactions that are larger than 1MB, so relayer
 	/// will be unable to craft valid transaction => this (and all subsequent) messages will
 	/// never be delivered.
-	fn verify_message(payload: &Payload) -> Result<(), Self::Error>;
+	fn verify_message(payload: &Payload) -> Result<(), VerificationError>;
 
 	/// Verify messages delivery proof and return lane && nonce of the latest received message.
 	fn verify_messages_delivery_proof(
 		proof: Self::MessagesDeliveryProof,
-	) -> Result<(LaneId, InboundLaneData<AccountId>), Self::Error>;
+	) -> Result<(LaneId, InboundLaneData<AccountId>), VerificationError>;
 }
 
 /// Lane message verifier.
@@ -75,9 +72,6 @@ pub trait TargetHeaderChain<Payload, AccountId> {
 ///
 /// Any fee requirements should also be enforced here.
 pub trait LaneMessageVerifier<SenderOrigin, Payload> {
-	/// Error type.
-	type Error: Debug + Into<&'static str>;
-
 	/// Verify message payload and return Ok(()) if message is valid and allowed to be sent over the
 	/// lane.
 	fn verify_message(
@@ -85,7 +79,7 @@ pub trait LaneMessageVerifier<SenderOrigin, Payload> {
 		lane: &LaneId,
 		outbound_data: &OutboundLaneData,
 		payload: &Payload,
-	) -> Result<(), Self::Error>;
+	) -> Result<(), VerificationError>;
 }
 
 /// Manages payments that are happening at the source chain during delivery confirmation
@@ -169,31 +163,27 @@ const ALL_OUTBOUND_MESSAGES_REJECTED: &str =
 	"This chain is configured to reject all outbound messages";
 
 impl<Payload, AccountId> TargetHeaderChain<Payload, AccountId> for ForbidOutboundMessages {
-	type Error = &'static str;
-
 	type MessagesDeliveryProof = ();
 
-	fn verify_message(_payload: &Payload) -> Result<(), Self::Error> {
+	fn verify_message(_payload: &Payload) -> Result<(), VerificationError> {
-		Err(ALL_OUTBOUND_MESSAGES_REJECTED)
+		Err(VerificationError::Other(ALL_OUTBOUND_MESSAGES_REJECTED))
 	}
 
 	fn verify_messages_delivery_proof(
 		_proof: Self::MessagesDeliveryProof,
-	) -> Result<(LaneId, InboundLaneData<AccountId>), Self::Error> {
+	) -> Result<(LaneId, InboundLaneData<AccountId>), VerificationError> {
-		Err(ALL_OUTBOUND_MESSAGES_REJECTED)
+		Err(VerificationError::Other(ALL_OUTBOUND_MESSAGES_REJECTED))
 	}
 }
 
 impl<SenderOrigin, Payload> LaneMessageVerifier<SenderOrigin, Payload> for ForbidOutboundMessages {
-	type Error = &'static str;
-
 	fn verify_message(
 		_submitter: &SenderOrigin,
 		_lane: &LaneId,
 		_outbound_data: &OutboundLaneData,
 		_payload: &Payload,
-	) -> Result<(), Self::Error> {
+	) -> Result<(), VerificationError> {
-		Err(ALL_OUTBOUND_MESSAGES_REJECTED)
+		Err(VerificationError::Other(ALL_OUTBOUND_MESSAGES_REJECTED))
 	}
 }
 

cumulus/bridges/primitives/messages/src/target_chain.rs

@@ -16,7 +16,9 @@
 
 //! Primitives of messages module, that are used on the target chain.
 
-use crate::{LaneId, Message, MessageKey, MessageNonce, MessagePayload, OutboundLaneData};
+use crate::{
+	LaneId, Message, MessageKey, MessageNonce, MessagePayload, OutboundLaneData, VerificationError,
+};
 
 use bp_runtime::{messages::MessageDispatchResult, Size};
 use codec::{Decode, Encode, Error as CodecError};
@@ -58,9 +60,6 @@ pub struct DispatchMessage<DispatchPayload> {
 /// can't change. Wrong implementation may lead to invalid lane states (i.e. lane
 /// that's stuck) and/or processing messages without paying fees.
 pub trait SourceHeaderChain {
-	/// Error type.
-	type Error: Debug;
-
 	/// Proof that messages are sent from source chain. This may also include proof
 	/// of corresponding outbound lane states.
 	type MessagesProof: Parameter + Size;
@@ -79,7 +78,7 @@ pub trait SourceHeaderChain {
 	fn verify_messages_proof(
 		proof: Self::MessagesProof,
 		messages_count: u32,
-	) -> Result<ProvedMessages<Message>, Self::Error>;
+	) -> Result<ProvedMessages<Message>, VerificationError>;
 }
 
 /// Called when inbound message is received.
@@ -164,21 +163,20 @@ pub struct ForbidInboundMessages<MessagesProof, DispatchPayload>(
 	PhantomData<(MessagesProof, DispatchPayload)>,
 );
 
-/// Error message that is used in `ForbidOutboundMessages` implementation.
+/// Error message that is used in `ForbidInboundMessages` implementation.
 const ALL_INBOUND_MESSAGES_REJECTED: &str =
 	"This chain is configured to reject all inbound messages";
 
 impl<MessagesProof: Parameter + Size, DispatchPayload> SourceHeaderChain
 	for ForbidInboundMessages<MessagesProof, DispatchPayload>
 {
-	type Error = &'static str;
 	type MessagesProof = MessagesProof;
 
 	fn verify_messages_proof(
 		_proof: Self::MessagesProof,
 		_messages_count: u32,
-	) -> Result<ProvedMessages<Message>, Self::Error> {
+	) -> Result<ProvedMessages<Message>, VerificationError> {
-		Err(ALL_INBOUND_MESSAGES_REJECTED)
+		Err(VerificationError::Other(ALL_INBOUND_MESSAGES_REJECTED))
 	}
 }
 

cumulus/bridges/primitives/polkadot-core/src/parachains.rs

@@ -18,8 +18,8 @@
 //!
 //! Even though this (bridges) repository references polkadot repository, we can't
 //! reference polkadot crates from pallets. That's because bridges repository is
-//! included in the polkadot repository and included pallets are used by polkadot
+//! included in the Cumulus repository and included pallets are used by Cumulus
-//! chains. Having pallets that are referencing polkadot, would mean that there may
+//! parachains. Having pallets that are referencing polkadot, would mean that there may
 //! be two versions of polkadot crates included in the runtime. Which is bad.
 
 use bp_runtime::{RawStorageProof, Size};

cumulus/bridges/primitives/runtime/src/chain.rs

@@ -232,6 +232,14 @@ where
 	const PARACHAIN_ID: u32 = <<T as UnderlyingChainProvider>::Chain as Parachain>::PARACHAIN_ID;
 }
 
+/// Adapter for `Get<u32>` to access `PARACHAIN_ID` from `trait Parachain`
+pub struct ParachainIdOf<Para>(sp_std::marker::PhantomData<Para>);
+impl<Para: Parachain> frame_support::traits::Get<u32> for ParachainIdOf<Para> {
+	fn get() -> u32 {
+		Para::PARACHAIN_ID
+	}
+}
+
 /// Underlying chain type.
 pub type UnderlyingChainOf<C> = <C as UnderlyingChainProvider>::Chain;
 

cumulus/bridges/primitives/runtime/src/lib.rs

@@ -31,11 +31,11 @@ use sp_std::{convert::TryFrom, fmt::Debug, ops::RangeInclusive, vec, vec::Vec};
 
 pub use chain::{
 	AccountIdOf, AccountPublicOf, BalanceOf, BlockNumberOf, Chain, EncodedOrDecodedCall, HashOf,
-	HasherOf, HeaderOf, IndexOf, Parachain, SignatureOf, TransactionEraOf, UnderlyingChainOf,
+	HasherOf, HeaderOf, IndexOf, Parachain, ParachainIdOf, SignatureOf, TransactionEraOf,
-	UnderlyingChainProvider,
+	UnderlyingChainOf, UnderlyingChainProvider,
 };
 pub use frame_support::storage::storage_prefix as storage_value_final_key;
-use num_traits::{CheckedAdd, CheckedSub, One};
+use num_traits::{CheckedAdd, CheckedSub, One, SaturatingAdd, Zero};
 pub use storage_proof::{
 	record_all_keys as record_all_trie_keys, Error as StorageProofError,
 	ProofSize as StorageProofSize, RawStorageProof, StorageProofChecker,
@@ -95,7 +95,7 @@ pub const BRIDGE_HUB_WOCOCO_CHAIN_ID: ChainId = *b"bhwo";
 pub const BRIDGE_HUB_KUSAMA_CHAIN_ID: ChainId = *b"bhks";
 
 /// BridgeHubPolkadot chain id.
-pub const BRIDGE_HUB_POLKADOT_CHAIN_ID: ChainId = *b"bhwo";
+pub const BRIDGE_HUB_POLKADOT_CHAIN_ID: ChainId = *b"bhpd";
 
 /// Generic header Id.
 #[derive(
@@ -527,17 +527,27 @@ impl<T> Debug for StrippableError<T> {
 pub trait RangeInclusiveExt<Idx> {
 	/// Computes the length of the `RangeInclusive`, checking for underflow and overflow.
 	fn checked_len(&self) -> Option<Idx>;
+	/// Computes the length of the `RangeInclusive`, saturating in case of underflow or overflow.
+	fn saturating_len(&self) -> Idx;
 }
 
 impl<Idx> RangeInclusiveExt<Idx> for RangeInclusive<Idx>
 where
-	Idx: CheckedSub + CheckedAdd + One,
+	Idx: CheckedSub + CheckedAdd + SaturatingAdd + One + Zero,
 {
 	fn checked_len(&self) -> Option<Idx> {
 		self.end()
 			.checked_sub(self.start())
 			.and_then(|len| len.checked_add(&Idx::one()))
 	}
+
+	fn saturating_len(&self) -> Idx {
+		let len = match self.end().checked_sub(self.start()) {
+			Some(len) => len,
+			None => return Idx::zero(),
+		};
+		len.saturating_add(&Idx::one())
+	}
 }
 
 #[cfg(test)]

cumulus/bridges/scripts/verify-pallets-build.sh

@@ -127,6 +127,7 @@ cargo check -p pallet-bridge-relayers --features runtime-benchmarks
 cargo check -p pallet-bridge-relayers --features try-runtime
 cargo check -p bridge-runtime-common
 cargo check -p bridge-runtime-common --features runtime-benchmarks
+cargo check -p bridge-runtime-common --features integrity-test
 
 # we're removing lock file after all chechs are done. Otherwise we may use different
 # Substrate/Polkadot/Cumulus commits and our checks will fail

cumulus/parachain-template/runtime/src/xcm_config.rs

@@ -63,7 +63,7 @@ pub type XcmOriginToTransactDispatchOrigin = (
 	// using `LocationToAccountId` and then turn that into the usual `Signed` origin. Useful for
 	// foreign chains who want to have a local sovereign account on this chain which they control.
 	SovereignSignedViaLocation<LocationToAccountId, RuntimeOrigin>,
-	// Native converter for Relay-chain (Parent) location; will converts to a `Relay` origin when
+	// Native converter for Relay-chain (Parent) location; will convert to a `Relay` origin when
 	// recognized.
 	RelayChainAsNative<RelayChainOrigin, RuntimeOrigin>,
 	// Native converter for sibling Parachains; will convert to a `SiblingPara` origin when

cumulus/parachains/runtimes/assets/westmint/src/xcm_config.rs

@@ -174,6 +174,7 @@ match_types! {
 		MultiLocation { parents: 1, interior: X1(Plurality { .. }) }
 	};
 }
+
 /// A call filter for the XCM Transact instruction. This is a temporary measure until we properly
 /// account for proof size weights.
 ///

cumulus/parachains/runtimes/bridge-hubs/README.md

@@ -1,8 +1,8 @@
 - [Bridge-hub Parachains](#bridge-hub-parachains)
-	* [How to test locally Rococo <-> Wococo](#how-to-test-locally-rococo-----wococo)
+	* [Requirements for local run/testing](#requirements-for-local-run-testing)
-		+ [Prepare/Build/Deploy](#prepare-build-deploy)
+	* [How to test locally Rococo <-> Wococo bridge](#how-to-test-locally-rococo-----wococo-bridge)
 		+ [Run chains (Rococo + BridgeHub, Wococo + BridgeHub) with zombienet](#run-chains--rococo---bridgehub--wococo---bridgehub--with-zombienet)
-		+ [Run relayers (Rococo, Wococo)](#run-relayers--rococo--wococo-)
+		+ [Run relayer (BridgeHubRococo, BridgeHubWococo)](#run-relayer--bridgehubrococo--bridgehubwococo-)
 			- [Run with script (alternative 1)](#run-with-script--alternative-1-)
 			- [Run with binary (alternative 2)](#run-with-binary--alternative-2-)
 		+ [Send messages](#send-messages)
@@ -29,9 +29,8 @@ The current trustless bridges planned for the BridgeHub(s) are:
 
 ![](./docs/bridge-hub-parachain-design.jpg "Basic deployment setup")
 
-## How to test locally Rococo <-> Wococo
+## Requirements for local run/testing
 
-### Prepare/Build/Deploy
 ```
 # Prepare empty directory for testing
 mkdir -p ~/local_bridge_testing/bin
@@ -44,7 +43,13 @@ Copy the apropriate binary (zombienet-linux) from the latest release to ~/local_
 # 2. Build polkadot binary
 git clone https://github.com/paritytech/polkadot.git
 cd polkadot
-cargo build --release
+
+# if you want to test Kusama/Polkadot bridge, we need "sudo pallet + fast-runtime",
+# so please, find the latest polkadot's repository branch `it/release-vX.Y.Z-fast-sudo`
+# e.g:
+# git checkout -b it/release-v0.9.42-fast-sudo --track origin/it/release-v0.9.42-fast-sudo
+
+cargo build --release --features fast-runtime
 cp target/release/polkadot ~/local_bridge_testing/bin/polkadot
 
 # 3. Build cumulus polkadot-parachain binary
@@ -69,6 +74,8 @@ cargo build --release --locked -p polkadot-parachain-bin
 cp target/release/polkadot-parachain ~/local_bridge_testing/bin/polkadot-parachain-mint
 ```
 
+## How to test locally Rococo <-> Wococo bridge
+
 ### Run chains (Rococo + BridgeHub, Wococo + BridgeHub) with zombienet
 
 ```
@@ -87,7 +94,7 @@ POLKADOT_PARACHAIN_BINARY_PATH_FOR_WOCKMINT=~/local_bridge_testing/bin/polkadot-
 	~/local_bridge_testing/bin/zombienet-linux --provider native spawn ./zombienet/bridge-hubs/bridge_hub_wococo_local_network.toml
 ```
 
-### Run relayers (Rococo, Wococo)
+### Run relayer (BridgeHubRococo, BridgeHubWococo)
 
 **Accounts of BridgeHub parachains:**
 - `Bob` is pallet owner of all bridge pallets

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_hub_rococo_config.rs

@@ -134,7 +134,7 @@ impl ThisChainWithMessages for BridgeHubRococo {
 /// Signed extension that refunds relayers that are delivering messages from the Wococo parachain.
 pub type BridgeRefundBridgeHubWococoMessages = RefundBridgedParachainMessages<
 	Runtime,
-	RefundableParachain<BridgeParachainWococoInstance, BridgeHubWococoParachainId>,
+	RefundableParachain<BridgeParachainWococoInstance, bp_bridge_hub_wococo::BridgeHubWococo>,
 	RefundableMessagesLane<WithBridgeHubWococoMessagesInstance, BridgeHubWococoMessagesLane>,
 	ActualFeeRefund<Runtime>,
 	PriorityBoostPerMessage,
@@ -144,10 +144,6 @@ bp_runtime::generate_static_str_provider!(BridgeRefundBridgeHubWococoMessages);
 
 parameter_types! {
 	pub const BridgeHubWococoMessagesLane: bp_messages::LaneId = DEFAULT_XCM_LANE_TO_BRIDGE_HUB_WOCOCO;
-	pub const BridgeHubWococoParachainId: u32 = {
-		use bp_runtime::Parachain;
-		BridgeHubWococo::PARACHAIN_ID
-	};
 }
 
 #[cfg(test)]

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/bridge_hub_wococo_config.rs

@@ -134,7 +134,7 @@ impl ThisChainWithMessages for BridgeHubWococo {
 /// Signed extension that refunds relayers that are delivering messages from the Rococo parachain.
 pub type BridgeRefundBridgeHubRococoMessages = RefundBridgedParachainMessages<
 	Runtime,
-	RefundableParachain<BridgeParachainRococoInstance, BridgeHubRococoParachainId>,
+	RefundableParachain<BridgeParachainRococoInstance, bp_bridge_hub_rococo::BridgeHubRococo>,
 	RefundableMessagesLane<WithBridgeHubRococoMessagesInstance, BridgeHubRococoMessagesLane>,
 	ActualFeeRefund<Runtime>,
 	PriorityBoostPerMessage,
@@ -144,10 +144,6 @@ bp_runtime::generate_static_str_provider!(BridgeRefundBridgeHubRococoMessages);
 
 parameter_types! {
 	pub const BridgeHubRococoMessagesLane: bp_messages::LaneId = DEFAULT_XCM_LANE_TO_BRIDGE_HUB_ROCOCO;
-	pub const BridgeHubRococoParachainId: u32 = {
-		use bp_runtime::Parachain;
-		BridgeHubRococo::PARACHAIN_ID
-	};
 }
 
 #[cfg(test)]

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs

@@ -92,8 +92,6 @@ use parachains_common::{
 };
 use xcm_executor::XcmExecutor;
 
-pub const LOG_TARGET: &str = "runtime::bridge-hub";
-
 /// The address format for describing accounts.
 pub type Address = MultiAddress<AccountId, ()>;
 

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/mod.rs

@@ -80,12 +80,12 @@ impl pallet_bridge_messages::WeightInfoExt for pallet_bridge_messages_bridge_mes
 
 impl pallet_bridge_parachains::WeightInfoExt for pallet_bridge_parachains_bridge_parachains_bench_runtime_bridge_parachain_rococo_instance::WeightInfo<crate::Runtime> {
 	fn expected_extra_storage_proof_size() -> u32 {
-		bp_bridge_hub_wococo::EXTRA_STORAGE_PROOF_SIZE
+		bp_bridge_hub_rococo::EXTRA_STORAGE_PROOF_SIZE
 	}
 }
 
 impl pallet_bridge_parachains::WeightInfoExt for pallet_bridge_parachains_bridge_parachains_bench_runtime_bridge_parachain_wococo_instance::WeightInfo<crate::Runtime> {
 	fn expected_extra_storage_proof_size() -> u32 {
-		bp_bridge_hub_rococo::EXTRA_STORAGE_PROOF_SIZE
+		bp_bridge_hub_wococo::EXTRA_STORAGE_PROOF_SIZE
 	}
 }

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/xcm_config.rs

@@ -16,8 +16,9 @@
 
 use super::{
 	AccountId, AllPalletsWithSystem, Balances, BridgeGrandpaRococoInstance,
-	BridgeGrandpaWococoInstance, ParachainInfo, ParachainSystem, PolkadotXcm, Runtime, RuntimeCall,
+	BridgeGrandpaWococoInstance, DeliveryRewardInBalance, ParachainInfo, ParachainSystem,
-	RuntimeEvent, RuntimeOrigin, WeightToFee, XcmpQueue,
+	PolkadotXcm, RequiredStakeForStakeAndSlash, Runtime, RuntimeCall, RuntimeEvent, RuntimeOrigin,
+	WeightToFee, XcmpQueue,
 };
 use crate::{
 	bridge_hub_rococo_config::ToBridgeHubWococoHaulBlobExporter,
@@ -152,6 +153,17 @@ impl Contains<RuntimeCall> for SafeCallFilter {
 			}
 		}
 
+		// Allow to change dedicated storage items (called by governance-like)
+		match call {
+			RuntimeCall::System(frame_system::Call::set_storage { items })
+				if items.iter().any(|(k, _)| {
+					k.eq(&DeliveryRewardInBalance::key()) |
+						k.eq(&RequiredStakeForStakeAndSlash::key())
+				}) =>
+				return true,
+			_ => (),
+		};
+
 		matches!(
 			call,
 			RuntimeCall::PolkadotXcm(pallet_xcm::Call::force_xcm_version { .. }) |
@@ -232,12 +244,12 @@ impl xcm_executor::Config for XcmConfig {
 		UsingComponents<WeightToFee, RelayLocation, AccountId, Balances, ToStakingPot<Runtime>>;
 	type ResponseHandler = PolkadotXcm;
 	type AssetTrap = PolkadotXcm;
+	type AssetLocker = ();
+	type AssetExchanger = ();
 	type AssetClaims = PolkadotXcm;
 	type SubscriptionService = PolkadotXcm;
 	type PalletInstancesInfo = AllPalletsWithSystem;
 	type MaxAssetsIntoHolding = MaxAssetsIntoHolding;
-	type AssetLocker = ();
-	type AssetExchanger = ();
 	type FeeManager = ();
 	type MessageExporter = BridgeHubRococoOrBridgeHubWococoSwitchExporter;
 	type UniversalAliases = Nothing;

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/tests.rs

@@ -18,8 +18,8 @@ pub use bridge_hub_rococo_runtime::{
 	constants::fee::WeightToFee,
 	xcm_config::{RelayNetwork, XcmConfig, XcmRouter},
 	Balances, BridgeGrandpaRococoInstance, BridgeGrandpaWococoInstance, BridgeWococoMessages,
-	ExistentialDeposit, ParachainSystem, PolkadotXcm, Runtime, RuntimeCall, RuntimeEvent,
+	DeliveryRewardInBalance, ExistentialDeposit, ParachainSystem, PolkadotXcm,
-	SessionKeys,
+	RequiredStakeForStakeAndSlash, Runtime, RuntimeCall, RuntimeEvent, SessionKeys,
 };
 use codec::{Decode, Encode};
 use xcm::latest::prelude::*;
@@ -30,7 +30,7 @@ use bridge_hub_rococo_runtime::{
 };
 
 use frame_support::parameter_types;
-use parachains_common::{AccountId, AuraId};
+use parachains_common::{AccountId, AuraId, Balance};
 
 const ALICE: [u8; 32] = [1u8; 32];
 
@@ -80,6 +80,36 @@ mod bridge_hub_rococo_tests {
 		Box::new(|call| RuntimeCall::BridgeWococoGrandpa(call).encode())
 	);
 
+	bridge_hub_test_utils::include_change_storage_constant_by_governance_works!(
+		change_delivery_reward_by_governance_works,
+		Runtime,
+		bridge_hub_test_utils::CollatorSessionKeys::new(
+			AccountId::from(ALICE),
+			AccountId::from(ALICE),
+			SessionKeys { aura: AuraId::from(sp_core::sr25519::Public::from_raw(ALICE)) }
+		),
+		bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID,
+		Box::new(|call| RuntimeCall::System(call).encode()),
+		(DeliveryRewardInBalance, u64),
+		|| (DeliveryRewardInBalance::key().to_vec(), DeliveryRewardInBalance::get()),
+		|old_value| old_value.checked_mul(2).unwrap()
+	);
+
+	bridge_hub_test_utils::include_change_storage_constant_by_governance_works!(
+		change_required_stake_by_governance_works,
+		Runtime,
+		bridge_hub_test_utils::CollatorSessionKeys::new(
+			AccountId::from(ALICE),
+			AccountId::from(ALICE),
+			SessionKeys { aura: AuraId::from(sp_core::sr25519::Public::from_raw(ALICE)) }
+		),
+		bp_bridge_hub_rococo::BRIDGE_HUB_ROCOCO_PARACHAIN_ID,
+		Box::new(|call| RuntimeCall::System(call).encode()),
+		(RequiredStakeForStakeAndSlash, Balance),
+		|| (RequiredStakeForStakeAndSlash::key().to_vec(), RequiredStakeForStakeAndSlash::get()),
+		|old_value| old_value.checked_mul(2).unwrap()
+	);
+
 	bridge_hub_test_utils::include_handle_export_message_from_system_parachain_to_outbound_queue_works!(
 		Runtime,
 		XcmConfig,
@@ -173,6 +203,36 @@ mod bridge_hub_wococo_tests {
 		Box::new(|call| RuntimeCall::BridgeRococoGrandpa(call).encode())
 	);
 
+	bridge_hub_test_utils::include_change_storage_constant_by_governance_works!(
+		change_delivery_reward_by_governance_works,
+		Runtime,
+		bridge_hub_test_utils::CollatorSessionKeys::new(
+			AccountId::from(ALICE),
+			AccountId::from(ALICE),
+			SessionKeys { aura: AuraId::from(sp_core::sr25519::Public::from_raw(ALICE)) }
+		),
+		bp_bridge_hub_wococo::BRIDGE_HUB_WOCOCO_PARACHAIN_ID,
+		Box::new(|call| RuntimeCall::System(call).encode()),
+		(DeliveryRewardInBalance, u64),
+		|| (DeliveryRewardInBalance::key().to_vec(), DeliveryRewardInBalance::get()),
+		|old_value| old_value.checked_mul(2).unwrap()
+	);
+
+	bridge_hub_test_utils::include_change_storage_constant_by_governance_works!(
+		change_required_stake_by_governance_works,
+		Runtime,
+		bridge_hub_test_utils::CollatorSessionKeys::new(
+			AccountId::from(ALICE),
+			AccountId::from(ALICE),
+			SessionKeys { aura: AuraId::from(sp_core::sr25519::Public::from_raw(ALICE)) }
+		),
+		bp_bridge_hub_wococo::BRIDGE_HUB_WOCOCO_PARACHAIN_ID,
+		Box::new(|call| RuntimeCall::System(call).encode()),
+		(RequiredStakeForStakeAndSlash, Balance),
+		|| (RequiredStakeForStakeAndSlash::key().to_vec(), RequiredStakeForStakeAndSlash::get()),
+		|old_value| old_value.checked_mul(2).unwrap()
+	);
+
 	bridge_hub_test_utils::include_handle_export_message_from_system_parachain_to_outbound_queue_works!(
 		Runtime,
 		XcmConfig,

cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases.rs

@@ -84,8 +84,7 @@ pub fn initialize_bridge_by_governance_works<Runtime, GrandpaPalletInstance>(
 			let require_weight_at_most =
 				<Runtime as frame_system::Config>::DbWeight::get().reads_writes(7, 7);
 
-			// execute XCM with Transacts to initialize bridge as governance does
+			// execute XCM with Transacts to `initialize bridge` as governance does
-			// prepare data for xcm::Transact(create)
 			assert_ok!(RuntimeHelper::<Runtime>::execute_as_governance(
 				initialize_call,
 				require_weight_at_most
@@ -123,6 +122,105 @@ macro_rules! include_initialize_bridge_by_governance_works(
 	}
 );
 
+/// Test-case makes sure that `Runtime` can change storage constant via governance-like call
+pub fn change_storage_constant_by_governance_works<Runtime, StorageConstant, StorageConstantType>(
+	collator_session_key: CollatorSessionKeys<Runtime>,
+	runtime_para_id: u32,
+	runtime_call_encode: Box<dyn Fn(frame_system::Call<Runtime>) -> Vec<u8>>,
+	storage_constant_key_value: fn() -> (Vec<u8>, StorageConstantType),
+	new_storage_constant_value: fn(&StorageConstantType) -> StorageConstantType,
+) where
+	Runtime: frame_system::Config
+		+ pallet_balances::Config
+		+ pallet_session::Config
+		+ pallet_xcm::Config
+		+ parachain_info::Config
+		+ pallet_collator_selection::Config
+		+ cumulus_pallet_dmp_queue::Config
+		+ cumulus_pallet_parachain_system::Config,
+	ValidatorIdOf<Runtime>: From<AccountIdOf<Runtime>>,
+	StorageConstant: Get<StorageConstantType>,
+	StorageConstantType: Encode + PartialEq + std::fmt::Debug,
+{
+	ExtBuilder::<Runtime>::default()
+		.with_collators(collator_session_key.collators())
+		.with_session_keys(collator_session_key.session_keys())
+		.with_para_id(runtime_para_id.into())
+		.with_tracing()
+		.build()
+		.execute_with(|| {
+			let (storage_constant_key, storage_constant_init_value): (
+				Vec<u8>,
+				StorageConstantType,
+			) = storage_constant_key_value();
+
+			// check delivery reward constant before (not stored yet, just as default value is used)
+			assert_eq!(StorageConstant::get(), storage_constant_init_value);
+			assert_eq!(sp_io::storage::get(&storage_constant_key), None);
+
+			let new_storage_constant_value =
+				new_storage_constant_value(&storage_constant_init_value);
+			assert_ne!(new_storage_constant_value, storage_constant_init_value);
+
+			// encode `set_storage` call
+			let set_storage_call =
+				runtime_call_encode(frame_system::Call::<Runtime>::set_storage {
+					items: vec![(
+						storage_constant_key.clone(),
+						new_storage_constant_value.encode(),
+					)],
+				});
+
+			// estimate - storing just 1 value
+			use frame_system::WeightInfo;
+			let require_weight_at_most =
+				<Runtime as frame_system::Config>::SystemWeightInfo::set_storage(1);
+
+			// execute XCM with Transact to `set_storage` as governance does
+			assert_ok!(RuntimeHelper::<Runtime>::execute_as_governance(
+				set_storage_call,
+				require_weight_at_most
+			)
+			.ensure_complete());
+
+			// check delivery reward constant after (stored)
+			assert_eq!(StorageConstant::get(), new_storage_constant_value);
+			assert_eq!(
+				sp_io::storage::get(&storage_constant_key),
+				Some(new_storage_constant_value.encode().into())
+			);
+		})
+}
+
+#[macro_export]
+macro_rules! include_change_storage_constant_by_governance_works(
+	(
+		$test_name:tt,
+		$runtime:path,
+		$collator_session_key:expr,
+		$runtime_para_id:expr,
+		$runtime_call_encode:expr,
+		($storage_constant:path, $storage_constant_type:path),
+		$storage_constant_key_value:expr,
+		$new_storage_constant_value:expr
+	) => {
+		#[test]
+		fn $test_name() {
+			$crate::test_cases::change_storage_constant_by_governance_works::<
+				$runtime,
+				$storage_constant,
+				$storage_constant_type,
+			>(
+				$collator_session_key,
+				$runtime_para_id,
+				$runtime_call_encode,
+				$storage_constant_key_value,
+				$new_storage_constant_value,
+			)
+		}
+	}
+);
+
 /// Test-case makes sure that `Runtime` can handle xcm `ExportMessage`:
 /// Checks if received XCM messages is correctly added to the message outbound queue for delivery.
 /// For SystemParachains we expect unpaid execution.

cumulus/parachains/runtimes/testing/penpal/src/xcm_config.rs

@@ -120,7 +120,7 @@ pub type XcmOriginToTransactDispatchOrigin = (
 	// using `LocationToAccountId` and then turn that into the usual `Signed` origin. Useful for
 	// foreign chains who want to have a local sovereign account on this chain which they control.
 	SovereignSignedViaLocation<LocationToAccountId, RuntimeOrigin>,
-	// Native converter for Relay-chain (Parent) location; will converts to a `Relay` origin when
+	// Native converter for Relay-chain (Parent) location; will convert to a `Relay` origin when
 	// recognized.
 	RelayChainAsNative<RelayChainOrigin, RuntimeOrigin>,
 	// Native converter for sibling Parachains; will convert to a `SiblingPara` origin when

cumulus/parachains/runtimes/testing/rococo-parachain/src/lib.rs

@@ -345,7 +345,7 @@ pub type XcmOriginToTransactDispatchOrigin = (
 	// using `LocationToAccountId` and then turn that into the usual `Signed` origin. Useful for
 	// foreign chains who want to have a local sovereign account on this chain which they control.
 	SovereignSignedViaLocation<LocationToAccountId, RuntimeOrigin>,
-	// Native converter for Relay-chain (Parent) location; will converts to a `Relay` origin when
+	// Native converter for Relay-chain (Parent) location; will convert to a `Relay` origin when
 	// recognised.
 	RelayChainAsNative<RelayChainOrigin, RuntimeOrigin>,
 	// Native converter for sibling Parachains; will convert to a `SiblingPara` origin when

cumulus/scripts/bridges_rococo_wococo.sh

@@ -377,10 +377,12 @@ function transfer_asset_via_bridge() {
     local url=$1
     local seed=$2
     local target_account=$3
+    local target_global_consensus=$4
     echo "  calling transfer_asset_via_bridge:"
     echo "      url: ${url}"
     echo "      seed: ${seed}"
     echo "      target_account: ${target_account}"
+    echo "      target_global_consensus: ${target_global_consensus}"
     echo "      params:"
 
     local assets=$(jq --null-input \
@@ -408,6 +410,7 @@ function transfer_asset_via_bridge() {
     local hex_encoded_data=$(cat $tmp_output_file)
 
     local destination=$(jq --null-input \
+                           --arg target_global_consensus "$target_global_consensus" \
                            --argjson hex_encoded_data "$hex_encoded_data" \
         '
             {
@@ -416,7 +419,7 @@ function transfer_asset_via_bridge() {
                     "interior": {
                         "X3": [
                             {
-                                "GlobalConsensus": "Wococo"
+                                "GlobalConsensus": $target_global_consensus
                             },
                             {
                                 "Parachain": 1000
@@ -454,10 +457,12 @@ function ping_via_bridge() {
     local url=$1
     local seed=$2
     local target_account=$3
+    local target_global_consensus=$4
     echo "  calling ping_via_bridge:"
     echo "      url: ${url}"
     echo "      seed: ${seed}"
     echo "      target_account: ${target_account}"
+    echo "      target_global_consensus: ${target_global_consensus}"
     echo "      params:"
 
     local tmp_output_file=$(mktemp)
@@ -465,6 +470,7 @@ function ping_via_bridge() {
     local hex_encoded_data=$(cat $tmp_output_file)
 
     local destination=$(jq --null-input \
+                           --arg target_global_consensus "$target_global_consensus" \
                            --argjson hex_encoded_data "$hex_encoded_data" \
         '
             {
@@ -473,7 +479,7 @@ function ping_via_bridge() {
                     "interior": {
                         "X3": [
                             {
-                                "GlobalConsensus": "Wococo"
+                                "GlobalConsensus": $target_global_consensus
                             },
                             {
                                 "Parachain": 1000
@@ -591,12 +597,19 @@ case "$1" in
           1014 \
           "Rococo" \
           1000
-      # drip SovereignAccount for `MultiLocation { parents: 2, interior: X2(GlobalConsensus(Rococo), Parachain(1000)) }` => 5DHZvp523gmJWxg9UcLVbofyu5nZkPvATeP1ciYncpFpXtiG
+      # drip SovereignAccount for `MultiLocation { parents: 2, interior: X2(GlobalConsensus(Rococo), Parachain(1000)) }` => 5CfNu7eH3SJvqqPt3aJh38T8dcFvhGzEohp9tsd41ANhXDnQ
-      # drip SovereignAccount for `MultiLocation { parents: 2, interior: X2(GlobalConsensus(Rococo), Parachain(1015)) }` => 5FS75NFUdEYhWHuV3y3ncjSG4PFdHfC5X7V6SEzc3rnCciwb
+      #
+      # use sp_core::crypto::Ss58Codec;
+      # println!("{}",
+      #     frame_support::sp_runtime::AccountId32::new(
+      #         GlobalConsensusParachainConvertsFor::<UniversalLocation, [u8; 32]>::convert_ref(
+      #             MultiLocation { parents: 2, interior: X2(GlobalConsensus(Kusama), Parachain(1000)) }).unwrap()
+      #		).to_ss58check_with_version(42_u16.into())
+      # );
       transfer_balance \
           "ws://127.0.0.1:9010" \
           "//Alice" \
-          "5DHZvp523gmJWxg9UcLVbofyu5nZkPvATeP1ciYncpFpXtiG" \
+          "5CfNu7eH3SJvqqPt3aJh38T8dcFvhGzEohp9tsd41ANhXDnQ" \
           $((1000000000 + 50000000000 * 20)) # ExistentialDeposit + maxTargetLocationFee * 20
       # create foreign assets for native Statemine token (yes, Kusama, because we are using Statemine runtime on rococo)
       force_create_foreign_asset \
@@ -605,7 +618,7 @@ case "$1" in
           1000 \
           "ws://127.0.0.1:9010" \
           "Kusama" \
-          "5DHZvp523gmJWxg9UcLVbofyu5nZkPvATeP1ciYncpFpXtiG"
+          "5CfNu7eH3SJvqqPt3aJh38T8dcFvhGzEohp9tsd41ANhXDnQ"
       ;;
   remove-assets-transfer-from-statemine-local)
       ensure_polkadot_js_api
@@ -621,40 +634,47 @@ case "$1" in
       transfer_asset_via_bridge \
           "ws://127.0.0.1:9910" \
           "$STATEMINE_ACCOUNT_SEED_FOR_LOCAL" \
-          "$WOCKMINT_ACCOUNT_ADDRESS_FOR_LOCAL"
+          "$WOCKMINT_ACCOUNT_ADDRESS_FOR_LOCAL" \
+          "Wococo"
       ;;
   transfer-asset-from-statemine-rococo)
       ensure_polkadot_js_api
       transfer_asset_via_bridge \
           "wss://ws-rococo-rockmine2-collator-node-0.parity-testnet.parity.io" \
           "$ROCKMINE2_ACCOUNT_SEED_FOR_ROCOCO" \
-          "$WOCKMINT_ACCOUNT_ADDRESS_FOR_ROCOCO"
+          "$WOCKMINT_ACCOUNT_ADDRESS_FOR_ROCOCO" \
+          "Wococo"
       ;;
   ping-via-bridge-from-statemine-local)
       ensure_polkadot_js_api
       ping_via_bridge \
           "ws://127.0.0.1:9910" \
           "$STATEMINE_ACCOUNT_SEED_FOR_LOCAL" \
-          "$WOCKMINT_ACCOUNT_ADDRESS_FOR_LOCAL"
+          "$WOCKMINT_ACCOUNT_ADDRESS_FOR_LOCAL" \
+          "Wococo"
       ;;
   ping-via-bridge-from-statemine-rococo)
       ensure_polkadot_js_api
       ping_via_bridge \
           "wss://ws-rococo-rockmine2-collator-node-0.parity-testnet.parity.io" \
           "${ROCKMINE2_ACCOUNT_SEED_FOR_ROCOCO}" \
-          "$WOCKMINT_ACCOUNT_ADDRESS_FOR_ROCOCO"
+          "$WOCKMINT_ACCOUNT_ADDRESS_FOR_ROCOCO" \
+          "Wococo"
       ;;
   drip)
       transfer_balance \
           "ws://127.0.0.1:9010" \
           "//Alice" \
-          "5DHZvp523gmJWxg9UcLVbofyu5nZkPvATeP1ciYncpFpXtiG" \
+          "5CfNu7eH3SJvqqPt3aJh38T8dcFvhGzEohp9tsd41ANhXDnQ" \
           $((1000000000 + 50000000000 * 20))
       ;;
   stop)
     pkill -f polkadot
     pkill -f parachain
     ;;
+  import)
+    # to avoid trigger anything here
+    ;;
   *)
     echo "A command is require. Supported commands for:
     Local (zombienet) run:

cumulus/zombienet/bridge-hubs/bridge_hub_rococo_local_network.toml

@@ -73,7 +73,7 @@ cumulus_based = true
 	ws_port = 9910
 	command = "{{POLKADOT_PARACHAIN_BINARY_PATH_FOR_ROCKMINE}}"
 	args = [
-		"-lparachain=debug,xcm=trace",
+		"-lparachain=debug,xcm=trace,runtime::bridge-transfer=trace",
 	]
 	extra_args = [
 		"--no-mdns", "--bootnodes {{'rockmine-collator2'|zombie('multiAddress')}}",
@@ -84,7 +84,7 @@ cumulus_based = true
 	name = "rockmine-collator2"
 	command = "{{POLKADOT_PARACHAIN_BINARY_PATH_FOR_ROCKMINE}}"
 	args = [
-		"-lparachain=debug,xcm=trace",
+		"-lparachain=debug,xcm=trace,runtime::bridge-transfer=trace",
 	]
 	extra_args = [
 		"--no-mdns", "--bootnodes {{'rockmine-collator1'|zombie('multiAddress')}}",

cumulus/zombienet/bridge-hubs/bridge_hub_wococo_local_network.toml

@@ -73,7 +73,7 @@ cumulus_based = true
 	ws_port = 9010
 	command = "{{POLKADOT_PARACHAIN_BINARY_PATH_FOR_WOCKMINT}}"
 	args = [
-		"-lparachain=debug,xcm=trace",
+		"-lparachain=debug,xcm=trace,runtime::bridge-transfer=trace",
 	]
 	extra_args = [
 		"--no-mdns", "--bootnodes {{'wockmint-collator2'|zombie('multiAddress')}}",
@@ -84,7 +84,7 @@ cumulus_based = true
 	name = "wockmint-collator2"
 	command = "{{POLKADOT_PARACHAIN_BINARY_PATH_FOR_WOCKMINT}}"
 	args = [
-		"-lparachain=debug,xcm=trace",
+		"-lparachain=debug,xcm=trace,runtime::bridge-transfer=trace",
 	]
 	extra_args = [
 		"--no-mdns", "--bootnodes {{'wockmint-collator1'|zombie('multiAddress')}}",