Move storage keys computation to the message-lane pallet (paritytech#478

)

* compute required storage keys in the message-lane pallet

* Update modules/message-lane/src/lib.rs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

bridges/bin/millau/runtime/src/rialto_messages.rs

@@ -21,34 +21,37 @@ use crate::Runtime;
 use bp_message_lane::{
 	source_chain::TargetHeaderChain,
 	target_chain::{ProvedMessages, SourceHeaderChain},
-	InboundLaneData, LaneId, Message, MessageKey, MessageNonce,
+	InboundLaneData, LaneId, Message, MessageNonce,
 };
 use bp_runtime::InstanceId;
-use bridge_runtime_common::messages::{self, MessageBridge};
+use bridge_runtime_common::messages::{self, ChainWithMessageLanes, MessageBridge};
 use frame_support::{
-	storage::generator::StorageMap,
 	weights::{Weight, WeightToFeePolynomial},
 	RuntimeDebug,
 };
-use pallet_message_lane::{DefaultInstance, InboundLanes, OutboundLanes, OutboundMessages};
 use sp_core::storage::StorageKey;
 use sp_trie::StorageProof;
 
 /// Storage key of the Millau -> Rialto message in the runtime storage.
 pub fn message_key(lane: &LaneId, nonce: MessageNonce) -> StorageKey {
-	let message_key = MessageKey { lane_id: *lane, nonce };
-	let raw_storage_key = OutboundMessages::<Runtime, DefaultInstance>::storage_map_final_key(message_key);
-	StorageKey(raw_storage_key)
+	pallet_message_lane::storage_keys::message_key::<Runtime, <Millau as ChainWithMessageLanes>::MessageLaneInstance>(
+		lane, nonce,
+	)
 }
 
 /// Storage key of the Millau -> Rialto message lane state in the runtime storage.
 pub fn outbound_lane_data_key(lane: &LaneId) -> StorageKey {
-	StorageKey(OutboundLanes::<DefaultInstance>::storage_map_final_key(*lane))
+	pallet_message_lane::storage_keys::outbound_lane_data_key::<<Millau as ChainWithMessageLanes>::MessageLaneInstance>(
+		lane,
+	)
 }
 
 /// Storage key of the Rialto -> Millau message lane state in the runtime storage.
 pub fn inbound_lane_data_key(lane: &LaneId) -> StorageKey {
-	StorageKey(InboundLanes::<Runtime, DefaultInstance>::storage_map_final_key(*lane))
+	pallet_message_lane::storage_keys::inbound_lane_data_key::<
+		Runtime,
+		<Millau as ChainWithMessageLanes>::MessageLaneInstance,
+	>(lane)
 }
 
 /// Message payload for Millau -> Rialto messages.
@@ -116,25 +119,31 @@ impl MessageBridge for WithRialtoMessageBridge {
 pub struct Millau;
 
 impl messages::ChainWithMessageLanes for Millau {
+	type Hash = bp_millau::Hash;
 	type AccountId = bp_millau::AccountId;
 	type Signer = bp_millau::AccountSigner;
 	type Signature = bp_millau::Signature;
 	type Call = crate::Call;
 	type Weight = Weight;
 	type Balance = bp_millau::Balance;
+
+	type MessageLaneInstance = pallet_message_lane::DefaultInstance;
 }
 
 /// Rialto chain from message lane point of view.
 #[derive(RuntimeDebug, Clone, Copy)]
 pub struct Rialto;
 
 impl messages::ChainWithMessageLanes for Rialto {
+	type Hash = bp_rialto::Hash;
 	type AccountId = bp_rialto::AccountId;
 	type Signer = bp_rialto::AccountSigner;
 	type Signature = bp_rialto::Signature;
 	type Call = (); // unknown to us
 	type Weight = Weight;
 	type Balance = bp_rialto::Balance;
+
+	type MessageLaneInstance = pallet_message_lane::DefaultInstance;
 }
 
 impl TargetHeaderChain<ToRialtoMessagePayload, bp_rialto::AccountId> for Rialto {

bridges/bin/rialto/runtime/src/millau_messages.rs

@@ -21,34 +21,37 @@ use crate::Runtime;
 use bp_message_lane::{
 	source_chain::TargetHeaderChain,
 	target_chain::{ProvedMessages, SourceHeaderChain},
-	InboundLaneData, LaneId, Message, MessageKey, MessageNonce,
+	InboundLaneData, LaneId, Message, MessageNonce,
 };
 use bp_runtime::InstanceId;
-use bridge_runtime_common::messages::{self, MessageBridge};
+use bridge_runtime_common::messages::{self, ChainWithMessageLanes, MessageBridge};
 use frame_support::{
-	storage::generator::StorageMap,
 	weights::{Weight, WeightToFeePolynomial},
 	RuntimeDebug,
 };
-use pallet_message_lane::{DefaultInstance, InboundLanes, OutboundLanes, OutboundMessages};
 use sp_core::storage::StorageKey;
 use sp_trie::StorageProof;
 
 /// Storage key of the Rialto -> Millau message in the runtime storage.
 pub fn message_key(lane: &LaneId, nonce: MessageNonce) -> StorageKey {
-	let message_key = MessageKey { lane_id: *lane, nonce };
-	let raw_storage_key = OutboundMessages::<Runtime, DefaultInstance>::storage_map_final_key(message_key);
-	StorageKey(raw_storage_key)
+	pallet_message_lane::storage_keys::message_key::<Runtime, <Rialto as ChainWithMessageLanes>::MessageLaneInstance>(
+		lane, nonce,
+	)
 }
 
 /// Storage key of the Rialto -> Millau message lane state in the runtime storage.
 pub fn outbound_lane_data_key(lane: &LaneId) -> StorageKey {
-	StorageKey(OutboundLanes::<DefaultInstance>::storage_map_final_key(*lane))
+	pallet_message_lane::storage_keys::outbound_lane_data_key::<<Rialto as ChainWithMessageLanes>::MessageLaneInstance>(
+		lane,
+	)
 }
 
 /// Storage key of the Millau -> Rialto message lane state in the runtime storage.
 pub fn inbound_lane_data_key(lane: &LaneId) -> StorageKey {
-	StorageKey(InboundLanes::<Runtime, DefaultInstance>::storage_map_final_key(*lane))
+	pallet_message_lane::storage_keys::inbound_lane_data_key::<
+		Runtime,
+		<Rialto as ChainWithMessageLanes>::MessageLaneInstance,
+	>(lane)
 }
 
 /// Message payload for Rialto -> Millau messages.
@@ -67,6 +70,9 @@ pub type FromMillauMessageDispatch = messages::target::FromBridgedChainMessageDi
 	pallet_bridge_call_dispatch::DefaultInstance,
 >;
 
+/// Messages proof for Millau -> Rialto messages.
+type FromMillauMessagesProof = messages::target::FromBridgedChainMessagesProof<WithMillauMessageBridge>;
+
 /// Millau <-> Rialto message bridge.
 #[derive(RuntimeDebug, Clone, Copy)]
 pub struct WithMillauMessageBridge;
@@ -116,25 +122,31 @@ impl MessageBridge for WithMillauMessageBridge {
 pub struct Rialto;
 
 impl messages::ChainWithMessageLanes for Rialto {
+	type Hash = bp_rialto::Hash;
 	type AccountId = bp_rialto::AccountId;
 	type Signer = bp_rialto::AccountSigner;
 	type Signature = bp_rialto::Signature;
 	type Call = crate::Call;
 	type Weight = Weight;
 	type Balance = bp_rialto::Balance;
+
+	type MessageLaneInstance = pallet_message_lane::DefaultInstance;
 }
 
 /// Millau chain from message lane point of view.
 #[derive(RuntimeDebug, Clone, Copy)]
 pub struct Millau;
 
 impl messages::ChainWithMessageLanes for Millau {
+	type Hash = bp_millau::Hash;
 	type AccountId = bp_millau::AccountId;
 	type Signer = bp_millau::AccountSigner;
 	type Signature = bp_millau::Signature;
 	type Call = (); // unknown to us
 	type Weight = Weight;
 	type Balance = bp_millau::Balance;
+
+	type MessageLaneInstance = pallet_message_lane::DefaultInstance;
 }
 
 impl TargetHeaderChain<ToMillauMessagePayload, bp_millau::AccountId> for Millau {
@@ -167,7 +179,7 @@ impl SourceHeaderChain<bp_millau::Balance> for Millau {
 	// - the storage proof of one or several keys;
 	// - id of the lane we prove messages for;
 	// - inclusive range of messages nonces that are proved.
-	type MessagesProof = (bp_millau::Hash, StorageProof, LaneId, MessageNonce, MessageNonce);
+	type MessagesProof = FromMillauMessagesProof;
 
 	fn verify_messages_proof(
 		_proof: Self::MessagesProof,

bridges/bin/runtime-common/Cargo.toml

@@ -16,6 +16,7 @@ bp-message-dispatch = { path = "../../primitives/message-dispatch", default-feat
 bp-message-lane = { path = "../../primitives/message-lane", default-features = false }
 bp-runtime = { path = "../../primitives/runtime", default-features = false }
 pallet-bridge-call-dispatch = { path = "../../modules/call-dispatch", default-features = false }
+pallet-message-lane = { path = "../../modules/message-lane", default-features = false }
 
 # Substrate dependencies
 
@@ -33,6 +34,7 @@ std = [
 	"codec/std",
 	"frame-support/std",
 	"pallet-bridge-call-dispatch/std",
+	"pallet-message-lane/std",
 	"sp-runtime/std",
 	"sp-std/std",
 	"sp-trie/std",

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

@@ -28,9 +28,10 @@ use bp_message_lane::{
 };
 use bp_runtime::InstanceId;
 use codec::{Compact, Decode, Input};
-use frame_support::RuntimeDebug;
+use frame_support::{traits::Instance, RuntimeDebug};
 use sp_runtime::traits::{CheckedAdd, CheckedDiv, CheckedMul};
 use sp_std::{cmp::PartialOrd, marker::PhantomData, vec::Vec};
+use sp_trie::StorageProof;
 
 /// Bidirectional message bridge.
 pub trait MessageBridge {
@@ -73,6 +74,8 @@ pub trait MessageBridge {
 
 /// Chain that has `message-lane` and `call-dispatch` modules.
 pub trait ChainWithMessageLanes {
+	/// Hash used in the chain.
+	type Hash: Decode;
 	/// Accound id on the chain.
 	type AccountId;
 	/// Public key of the chain account that may be used to verify signatures.
@@ -88,10 +91,14 @@ pub trait ChainWithMessageLanes {
 	type Weight: From<frame_support::weights::Weight>;
 	/// Type of balances that is used on the chain.
 	type Balance: CheckedAdd + CheckedDiv + CheckedMul + PartialOrd + From<u32> + Copy;
+
+	/// Instance of the message-lane pallet.
+	type MessageLaneInstance: Instance;
 }
 
 pub(crate) type ThisChain<B> = <B as MessageBridge>::ThisChain;
 pub(crate) type BridgedChain<B> = <B as MessageBridge>::BridgedChain;
+pub(crate) type HashOf<C> = <C as ChainWithMessageLanes>::Hash;
 pub(crate) type AccountIdOf<C> = <C as ChainWithMessageLanes>::AccountId;
 pub(crate) type SignerOf<C> = <C as ChainWithMessageLanes>::Signer;
 pub(crate) type SignatureOf<C> = <C as ChainWithMessageLanes>::Signature;
@@ -202,6 +209,20 @@ pub mod target {
 		CallOf<ThisChain<B>>,
 	>;
 
+	/// Messages proof from bridged chain:
+	///
+	/// - hash of finalized header;
+	/// - storage proof of messages and (optionally) outbound lane state;
+	/// - lane id;
+	/// - nonces (inclusive range) of messages which are included in this proof.
+	pub type FromBridgedChainMessagesProof<B> = (
+		HashOf<BridgedChain<B>>,
+		StorageProof,
+		LaneId,
+		MessageNonce,
+		MessageNonce,
+	);
+
 	/// Message payload for Bridged -> This messages.
 	pub struct FromBridgedChainMessagePayload<B: MessageBridge>(pub(crate) FromBridgedChainDecodedMessagePayload<B>);
 
@@ -450,23 +471,29 @@ mod tests {
 	struct ThisChain;
 
 	impl ChainWithMessageLanes for ThisChain {
+		type Hash = ();
 		type AccountId = ThisChainAccountId;
 		type Signer = ThisChainSigner;
 		type Signature = ThisChainSignature;
 		type Call = ThisChainCall;
 		type Weight = frame_support::weights::Weight;
 		type Balance = ThisChainBalance;
+
+		type MessageLaneInstance = pallet_message_lane::DefaultInstance;
 	}
 
 	struct BridgedChain;
 
 	impl ChainWithMessageLanes for BridgedChain {
+		type Hash = ();
 		type AccountId = BridgedChainAccountId;
 		type Signer = BridgedChainSigner;
 		type Signature = BridgedChainSignature;
 		type Call = BridgedChainCall;
 		type Weight = frame_support::weights::Weight;
 		type Balance = BridgedChainBalance;
+
+		type MessageLaneInstance = pallet_message_lane::DefaultInstance;
 	}
 
 	#[test]

bridges/modules/message-lane/Cargo.toml

@@ -19,11 +19,12 @@ bp-runtime = { path = "../../primitives/runtime", default-features = false }
 
 frame-support = { version = "2.0", default-features = false }
 frame-system = { version = "2.0", default-features = false }
+sp-core = { version = "2.0", default-features = false }
 sp-runtime = { version = "2.0", default-features = false }
 sp-std = { version = "2.0", default-features = false }
 
 [dev-dependencies]
-sp-core = "2.0"
+hex-literal = "0.3"
 sp-io = "2.0"
 
 [features]
@@ -35,6 +36,7 @@ std = [
 	"frame-support/std",
 	"frame-system/std",
 	"serde",
+	"sp-core/std",
 	"sp-runtime/std",
 	"sp-std/std",
 ]

bridges/modules/message-lane/src/lib.rs

@@ -452,6 +452,42 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
 	}
 }
 
+/// Getting storage keys for messages and lanes states. These keys are normally used when building
+/// messages and lanes states proofs.
+///
+/// Keep in mind that all functions in this module are **NOT** using passed `T` argument, so any
+/// runtime can be passed. E.g. if you're verifying proof from Runtime1 in Runtime2, you only have
+/// access to Runtime2 and you may pass it to the functions, where required. This is because our
+/// maps are not using any Runtime-specific data in the keys.
+///
+/// On the other side, passing correct instance is required. So if proof has been crafted by the
+/// Instance1, you should verify it using Instance1. This is inconvenient if you're using different
+/// instances on different sides of the bridge. I.e. in Runtime1 it is Instance2, but on Runtime2
+/// it is Instance42. But there's no other way, but to craft this key manually (which is what I'm
+/// trying to avoid here) - by using strings like "Instance2", "OutboundMessages", etc.
+pub mod storage_keys {
+	use super::*;
+	use frame_support::storage::generator::StorageMap;
+	use sp_core::storage::StorageKey;
+
+	/// Storage key of the outbound message in the runtime storage.
+	pub fn message_key<T: Trait<I>, I: Instance>(lane: &LaneId, nonce: MessageNonce) -> StorageKey {
+		let message_key = MessageKey { lane_id: *lane, nonce };
+		let raw_storage_key = OutboundMessages::<T, I>::storage_map_final_key(message_key);
+		StorageKey(raw_storage_key)
+	}
+
+	/// Storage key of the outbound message lane state in the runtime storage.
+	pub fn outbound_lane_data_key<I: Instance>(lane: &LaneId) -> StorageKey {
+		StorageKey(OutboundLanes::<I>::storage_map_final_key(*lane))
+	}
+
+	/// Storage key of the inbound message lane state in the runtime storage.
+	pub fn inbound_lane_data_key<T: Trait<I>, I: Instance>(lane: &LaneId) -> StorageKey {
+		StorageKey(InboundLanes::<T, I>::storage_map_final_key(*lane))
+	}
+}
+
 /// Ensure that the origin is either root, or `ModuleOwner`.
 fn ensure_owner_or_root<T: Trait<I>, I: Instance>(origin: T::Origin) -> Result<(), BadOrigin> {
 	match origin.into() {
@@ -605,6 +641,7 @@ mod tests {
 	};
 	use frame_support::{assert_noop, assert_ok};
 	use frame_system::{EventRecord, Module as System, Phase};
+	use hex_literal::hex;
 	use sp_runtime::DispatchError;
 
 	fn send_regular_message() {
@@ -1018,4 +1055,34 @@ mod tests {
 			assert_eq!(InboundLanes::<TestRuntime>::get(&TEST_LANE_ID).latest_received_nonce, 3,);
 		});
 	}
+
+	#[test]
+	fn storage_message_key_computed_properly() {
+		// If this test fails, then something has been changed in module storage that is breaking all
+		// previously crafted messages proofs.
+		assert_eq!(
+			storage_keys::message_key::<TestRuntime, DefaultInstance>(&*b"test", 42).0,
+			hex!("87f1ffe31b52878f09495ca7482df1a48a395e6242c6813b196ca31ed0547ea79446af0e09063bd4a7874aef8a997cec746573742a00000000000000").to_vec(),
+		);
+	}
+
+	#[test]
+	fn outbound_lane_data_key_computed_properly() {
+		// If this test fails, then something has been changed in module storage that is breaking all
+		// previously crafted outbound lane state proofs.
+		assert_eq!(
+			storage_keys::outbound_lane_data_key::<DefaultInstance>(&*b"test").0,
+			hex!("87f1ffe31b52878f09495ca7482df1a496c246acb9b55077390e3ca723a0ca1f44a8995dd50b6657a037a7839304535b74657374").to_vec(),
+		);
+	}
+
+	#[test]
+	fn inbound_lane_data_key_computed_properly() {
+		// If this test fails, then something has been changed in module storage that is breaking all
+		// previously crafted inbound lane state proofs.
+		assert_eq!(
+			storage_keys::inbound_lane_data_key::<TestRuntime, DefaultInstance>(&*b"test").0,
+			hex!("87f1ffe31b52878f09495ca7482df1a4e5f83cf83f2127eb47afdc35d6e43fab44a8995dd50b6657a037a7839304535b74657374").to_vec(),
+		);
+	}
 }