move storage keys computation to primitivs (#1254)

Author: svyatonik

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

@@ -1030,14 +1030,13 @@ impl_runtime_apis! {
 					params: MessageProofParams,
 				) -> (millau_messages::FromMillauMessagesProof, Weight) {
 					use crate::millau_messages::WithMillauMessageBridge;
-					use bp_messages::MessageKey;
+					use bp_messages::{MessageKey, storage_keys};
 					use bridge_runtime_common::{
 						messages::MessageBridge,
 						messages_benchmarking::{ed25519_sign, prepare_message_proof},
 					};
 					use codec::Encode;
 					use frame_support::weights::GetDispatchInfo;
-					use pallet_bridge_messages::storage_keys;
 					use sp_runtime::traits::{Header, IdentifyAccount};
 
 					let remark = match params.size {
@@ -1115,7 +1114,7 @@ impl_runtime_apis! {
 
 					prepare_message_delivery_proof::<WithMillauMessageBridge, bp_millau::Hasher, Runtime, (), _, _>(
 						params,
-						|lane_id| pallet_bridge_messages::storage_keys::inbound_lane_data_key(
+						|lane_id| bp_messages::storage_keys::inbound_lane_data_key(
 							<WithMillauMessageBridge as MessageBridge>::BRIDGED_MESSAGES_PALLET_NAME,
 							&lane_id,
 						).0,

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

@@ -428,7 +428,7 @@ pub mod source {
 				// Messages delivery proof is just proof of single storage key read => any error
 				// is fatal.
 				let storage_inbound_lane_data_key =
-					pallet_bridge_messages::storage_keys::inbound_lane_data_key(B::BRIDGED_MESSAGES_PALLET_NAME, &lane);
+					bp_messages::storage_keys::inbound_lane_data_key(B::BRIDGED_MESSAGES_PALLET_NAME, &lane);
 				let raw_inbound_lane_data = storage
 					.read_value(storage_inbound_lane_data_key.0.as_ref())
 					.map_err(|_| "Failed to read inbound lane state from storage proof")?
@@ -674,16 +674,15 @@ pub mod target {
 		B: MessageBridge,
 	{
 		fn read_raw_outbound_lane_data(&self, lane_id: &LaneId) -> Option<Vec<u8>> {
-			let storage_outbound_lane_data_key =
-				pallet_bridge_messages::storage_keys::outbound_lane_data_key(
-					B::BRIDGED_MESSAGES_PALLET_NAME,
-					lane_id,
-				);
+			let storage_outbound_lane_data_key = bp_messages::storage_keys::outbound_lane_data_key(
+				B::BRIDGED_MESSAGES_PALLET_NAME,
+				lane_id,
+			);
 			self.storage.read_value(storage_outbound_lane_data_key.0.as_ref()).ok()?
 		}
 
 		fn read_raw_message(&self, message_key: &MessageKey) -> Option<Vec<u8>> {
-			let storage_message_key = pallet_bridge_messages::storage_keys::message_key(
+			let storage_message_key = bp_messages::storage_keys::message_key(
 				B::BRIDGED_MESSAGES_PALLET_NAME,
 				&message_key.lane_id,
 				message_key.nonce,

bridges/modules/messages/Cargo.toml

@@ -30,8 +30,6 @@ sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master
 sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
 
 [dev-dependencies]
-hex = "0.4"
-hex-literal = "0.3"
 sp-io = { git = "https://github.com/paritytech/substrate", branch = "master" }
 pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "master" }
 

bridges/modules/messages/src/lib.rs

@@ -801,32 +801,6 @@ pub mod pallet {
 	}
 }
 
-/// Getting storage keys for messages and lanes states. These keys are normally used when building
-/// messages and lanes states proofs.
-pub mod storage_keys {
-	use super::*;
-	use sp_core::storage::StorageKey;
-
-	/// Storage key of the outbound message in the runtime storage.
-	pub fn message_key(pallet_prefix: &str, lane: &LaneId, nonce: MessageNonce) -> StorageKey {
-		bp_runtime::storage_map_final_key_blake2_128concat(
-			pallet_prefix,
-			"OutboundMessages",
-			&MessageKey { lane_id: *lane, nonce }.encode(),
-		)
-	}
-
-	/// Storage key of the outbound message lane state in the runtime storage.
-	pub fn outbound_lane_data_key(pallet_prefix: &str, lane: &LaneId) -> StorageKey {
-		bp_runtime::storage_map_final_key_blake2_128concat(pallet_prefix, "OutboundLanes", lane)
-	}
-
-	/// Storage key of the inbound message lane state in the runtime storage.
-	pub fn inbound_lane_data_key(pallet_prefix: &str, lane: &LaneId) -> StorageKey {
-		bp_runtime::storage_map_final_key_blake2_128concat(pallet_prefix, "InboundLanes", lane)
-	}
-}
-
 /// AccountId of the shared relayer fund account.
 ///
 /// This account is passed to `MessageDeliveryAndDispatchPayment` trait, and depending
@@ -1159,9 +1133,8 @@ mod tests {
 		REGULAR_PAYLOAD, TEST_LANE_ID, TEST_RELAYER_A, TEST_RELAYER_B,
 	};
 	use bp_messages::{UnrewardedRelayer, UnrewardedRelayersState};
-	use frame_support::{assert_noop, assert_ok, weights::Weight};
+	use frame_support::{assert_noop, assert_ok, storage::generator::StorageMap, weights::Weight};
 	use frame_system::{EventRecord, Pallet as System, Phase};
-	use hex_literal::hex;
 	use sp_runtime::DispatchError;
 
 	fn get_ready_for_events() {
@@ -1889,45 +1862,6 @@ mod tests {
 		});
 	}
 
-	#[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.
-		let storage_key = storage_keys::message_key("BridgeMessages", &*b"test", 42).0;
-		assert_eq!(
-			storage_key,
-			hex!("dd16c784ebd3390a9bc0357c7511ed018a395e6242c6813b196ca31ed0547ea79446af0e09063bd4a7874aef8a997cec746573742a00000000000000").to_vec(),
-			"Unexpected storage key: {}",
-			hex::encode(&storage_key),
-		);
-	}
-
-	#[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.
-		let storage_key = storage_keys::outbound_lane_data_key("BridgeMessages", &*b"test").0;
-		assert_eq!(
-			storage_key,
-			hex!("dd16c784ebd3390a9bc0357c7511ed0196c246acb9b55077390e3ca723a0ca1f44a8995dd50b6657a037a7839304535b74657374").to_vec(),
-			"Unexpected storage key: {}",
-			hex::encode(&storage_key),
-		);
-	}
-
-	#[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.
-		let storage_key = storage_keys::inbound_lane_data_key("BridgeMessages", &*b"test").0;
-		assert_eq!(
-			storage_key,
-			hex!("dd16c784ebd3390a9bc0357c7511ed01e5f83cf83f2127eb47afdc35d6e43fab44a8995dd50b6657a037a7839304535b74657374").to_vec(),
-			"Unexpected storage key: {}",
-			hex::encode(&storage_key),
-		);
-	}
-
 	#[test]
 	fn actual_dispatch_weight_does_not_overlow() {
 		run_test(|| {
@@ -2359,4 +2293,25 @@ mod tests {
 			);
 		});
 	}
+
+	#[test]
+	fn storage_keys_computed_properly() {
+		assert_eq!(
+			OutboundMessages::<TestRuntime>::storage_map_final_key(MessageKey {
+				lane_id: TEST_LANE_ID,
+				nonce: 42
+			}),
+			bp_messages::storage_keys::message_key("Messages", &TEST_LANE_ID, 42).0,
+		);
+
+		assert_eq!(
+			OutboundLanes::<TestRuntime>::storage_map_final_key(TEST_LANE_ID),
+			bp_messages::storage_keys::outbound_lane_data_key("Messages", &TEST_LANE_ID).0,
+		);
+
+		assert_eq!(
+			InboundLanes::<TestRuntime>::storage_map_final_key(TEST_LANE_ID),
+			bp_messages::storage_keys::inbound_lane_data_key("Messages", &TEST_LANE_ID).0,
+		);
+	}
 }

bridges/modules/token-swap/src/lib.rs

@@ -95,9 +95,6 @@ pub mod weights_ext;
 
 pub use pallet::*;
 
-/// Name of the `PendingSwaps` storage map.
-pub const PENDING_SWAPS_MAP_NAME: &str = "PendingSwaps";
-
 // comes from #[pallet::event]
 #[allow(clippy::unused_unit)]
 #[frame_support::pallet]
@@ -639,7 +636,7 @@ pub mod pallet {
 mod tests {
 	use super::*;
 	use crate::mock::*;
-	use frame_support::{assert_noop, assert_ok};
+	use frame_support::{assert_noop, assert_ok, storage::generator::StorageMap};
 
 	const CAN_START_BLOCK_NUMBER: u64 = 10;
 	const CAN_CLAIM_BLOCK_NUMBER: u64 = CAN_START_BLOCK_NUMBER + 1;
@@ -1130,4 +1127,12 @@ mod tests {
 			);
 		});
 	}
+
+	#[test]
+	fn storage_keys_computed_properly() {
+		assert_eq!(
+			PendingSwaps::<TestRuntime>::storage_map_final_key(test_swap_hash()),
+			bp_token_swap::storage_keys::pending_swaps_key("TokenSwap", test_swap_hash()).0,
+		);
+	}
 }

bridges/primitives/messages/Cargo.toml

@@ -21,8 +21,13 @@ bp-runtime = { path = "../runtime", default-features = false }
 
 frame-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
 frame-system = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
+sp-core = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
 sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
 
+[dev-dependencies]
+hex = "0.4"
+hex-literal = "0.3"
+
 [features]
 default = ["std"]
 std = [
@@ -32,5 +37,6 @@ std = [
 	"frame-system/std",
 	"scale-info/std",
 	"serde",
+	"sp-core/std",
 	"sp-std/std"
 ]

bridges/primitives/messages/src/lib.rs

@@ -30,6 +30,7 @@ use scale_info::TypeInfo;
 use sp_std::{collections::vec_deque::VecDeque, prelude::*};
 
 pub mod source_chain;
+pub mod storage_keys;
 pub mod target_chain;
 
 // Weight is reexported to avoid additional frame-support dependencies in related crates.

bridges/primitives/messages/src/storage_keys.rs

@@ -0,0 +1,102 @@
+// Copyright 2019-2021 Parity Technologies (UK) Ltd.
+// This file is part of Parity Bridges Common.
+
+// Parity Bridges Common is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity Bridges Common is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity Bridges Common.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Storage keys of bridge messages pallet.
+
+/// Name of the `OutboundMessages` storage map.
+pub const OUTBOUND_MESSAGES_MAP_NAME: &str = "OutboundMessages";
+/// Name of the `OutboundLanes` storage map.
+pub const OUTBOUND_LANES_MAP_NAME: &str = "OutboundLanes";
+/// Name of the `InboundLanes` storage map.
+pub const INBOUND_LANES_MAP_NAME: &str = "InboundLanes";
+
+use crate::{LaneId, MessageKey, MessageNonce};
+
+use codec::Encode;
+use frame_support::Blake2_128Concat;
+use sp_core::storage::StorageKey;
+
+/// Storage key of the outbound message in the runtime storage.
+pub fn message_key(pallet_prefix: &str, lane: &LaneId, nonce: MessageNonce) -> StorageKey {
+	bp_runtime::storage_map_final_key::<Blake2_128Concat>(
+		pallet_prefix,
+		OUTBOUND_MESSAGES_MAP_NAME,
+		&MessageKey { lane_id: *lane, nonce }.encode(),
+	)
+}
+
+/// Storage key of the outbound message lane state in the runtime storage.
+pub fn outbound_lane_data_key(pallet_prefix: &str, lane: &LaneId) -> StorageKey {
+	bp_runtime::storage_map_final_key::<Blake2_128Concat>(
+		pallet_prefix,
+		OUTBOUND_LANES_MAP_NAME,
+		lane,
+	)
+}
+
+/// Storage key of the inbound message lane state in the runtime storage.
+pub fn inbound_lane_data_key(pallet_prefix: &str, lane: &LaneId) -> StorageKey {
+	bp_runtime::storage_map_final_key::<Blake2_128Concat>(
+		pallet_prefix,
+		INBOUND_LANES_MAP_NAME,
+		lane,
+	)
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use hex_literal::hex;
+
+	#[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.
+		let storage_key = message_key("BridgeMessages", &*b"test", 42).0;
+		assert_eq!(
+			storage_key,
+			hex!("dd16c784ebd3390a9bc0357c7511ed018a395e6242c6813b196ca31ed0547ea79446af0e09063bd4a7874aef8a997cec746573742a00000000000000").to_vec(),
+			"Unexpected storage key: {}",
+			hex::encode(&storage_key),
+		);
+	}
+
+	#[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.
+		let storage_key = outbound_lane_data_key("BridgeMessages", &*b"test").0;
+		assert_eq!(
+			storage_key,
+			hex!("dd16c784ebd3390a9bc0357c7511ed0196c246acb9b55077390e3ca723a0ca1f44a8995dd50b6657a037a7839304535b74657374").to_vec(),
+			"Unexpected storage key: {}",
+			hex::encode(&storage_key),
+		);
+	}
+
+	#[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.
+		let storage_key = inbound_lane_data_key("BridgeMessages", &*b"test").0;
+		assert_eq!(
+			storage_key,
+			hex!("dd16c784ebd3390a9bc0357c7511ed01e5f83cf83f2127eb47afdc35d6e43fab44a8995dd50b6657a037a7839304535b74657374").to_vec(),
+			"Unexpected storage key: {}",
+			hex::encode(&storage_key),
+		);
+	}
+}

bridges/primitives/runtime/src/lib.rs

@@ -201,47 +201,22 @@ impl<BlockNumber: Copy + Into<u64>, BlockHash: Copy> TransactionEra<BlockNumber,
 }
 
 /// This is a copy of the
-/// `frame_support::storage::generator::StorageMap::storage_map_final_key` for `Blake2_128Concat`
-/// maps.
+/// `frame_support::storage::generator::StorageMap::storage_map_final_key` for maps based
+/// on selected hasher.
 ///
 /// We're using it because to call `storage_map_final_key` directly, we need access to the runtime
 /// and pallet instance, which (sometimes) is impossible.
-pub fn storage_map_final_key_blake2_128concat(
+pub fn storage_map_final_key<H: StorageHasher>(
 	pallet_prefix: &str,
 	map_name: &str,
 	key: &[u8],
 ) -> StorageKey {
-	storage_map_final_key_identity(
-		pallet_prefix,
-		map_name,
-		&frame_support::Blake2_128Concat::hash(key),
-	)
-}
-
-///
-pub fn storage_map_final_key_twox64_concat(
-	pallet_prefix: &str,
-	map_name: &str,
-	key: &[u8],
-) -> StorageKey {
-	storage_map_final_key_identity(pallet_prefix, map_name, &frame_support::Twox64Concat::hash(key))
-}
-
-/// This is a copy of the
-/// `frame_support::storage::generator::StorageMap::storage_map_final_key` for `Identity` maps.
-///
-/// We're using it because to call `storage_map_final_key` directly, we need access to the runtime
-/// and pallet instance, which (sometimes) is impossible.
-pub fn storage_map_final_key_identity(
-	pallet_prefix: &str,
-	map_name: &str,
-	key_hashed: &[u8],
-) -> StorageKey {
+	let key_hashed = H::hash(key);
 	let pallet_prefix_hashed = frame_support::Twox128::hash(pallet_prefix.as_bytes());
 	let storage_prefix_hashed = frame_support::Twox128::hash(map_name.as_bytes());
 
 	let mut final_key = Vec::with_capacity(
-		pallet_prefix_hashed.len() + storage_prefix_hashed.len() + key_hashed.len(),
+		pallet_prefix_hashed.len() + storage_prefix_hashed.len() + key_hashed.as_ref().len(),
 	);
 
 	final_key.extend_from_slice(&pallet_prefix_hashed[..]);