Skip to content

Commit

Permalink
move storage keys computation to primitivs (paritytech#1254)
Browse files Browse the repository at this point in the history
  • Loading branch information
svyatonik authored and serban300 committed Apr 8, 2024
1 parent 3e64f9e commit 3ea1093
Show file tree
Hide file tree
Showing 16 changed files with 240 additions and 123 deletions.
5 changes: 2 additions & 3 deletions bridges/bin/rialto/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
13 changes: 6 additions & 7 deletions bridges/bin/runtime-common/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")?
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions bridges/modules/messages/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }

Expand Down
89 changes: 22 additions & 67 deletions bridges/modules/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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(|| {
Expand Down Expand Up @@ -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,
);
}
}
13 changes: 9 additions & 4 deletions bridges/modules/token-swap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
);
}
}
6 changes: 6 additions & 0 deletions bridges/primitives/messages/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -32,5 +37,6 @@ std = [
"frame-system/std",
"scale-info/std",
"serde",
"sp-core/std",
"sp-std/std"
]
1 change: 1 addition & 0 deletions bridges/primitives/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
102 changes: 102 additions & 0 deletions bridges/primitives/messages/src/storage_keys.rs
Original file line number Diff line number Diff line change
@@ -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),
);
}
}
35 changes: 5 additions & 30 deletions bridges/primitives/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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[..]);
Expand Down
Loading

0 comments on commit 3ea1093

Please sign in to comment.