Move Storage Parser from Bridge Pallet (#793)

* Move storage proof checker to runtime primtives

* Add method for parsing storage proofs

* Use finality-verifier pallet in runtime-common

* Get bridge pallet compiling again

* Use storage prover from bp-runtime in a few more places

* Don't leak `std` items from proof helper into `no-std` builds

* Fix benchmarking compilation

* Remove unused import in fuzzer

bridges/bin/runtime-common/Cargo.toml

@@ -19,7 +19,7 @@ bp-message-lane = { path = "../../primitives/message-lane", default-features = f
 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 }
-pallet-substrate-bridge = { path = "../../modules/substrate", default-features = false }
+pallet-finality-verifier = { path = "../../modules/finality-verifier", default-features = false }
 
 # Substrate dependencies
 
@@ -40,8 +40,8 @@ std = [
  "frame-support/std",
  "hash-db/std",
  "pallet-bridge-call-dispatch/std",
+ "pallet-finality-verifier/std",
  "pallet-message-lane/std",
- "pallet-substrate-bridge/std",
  "sp-core/std",
  "sp-runtime/std",
  "sp-state-machine/std",
@@ -50,7 +50,7 @@ std = [
 ]
 runtime-benchmarks = [
  "ed25519-dalek/u64_backend",
+ "pallet-finality-verifier/runtime-benchmarks",
  "pallet-message-lane/runtime-benchmarks",
- "pallet-substrate-bridge/runtime-benchmarks",
  "sp-state-machine",
 ]

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

@@ -26,11 +26,10 @@ use bp_message_lane::{
  target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages},
  InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce, OutboundLaneData,
 };
-use bp_runtime::{InstanceId, Size};
+use bp_runtime::{InstanceId, Size, StorageProofChecker};
 use codec::{Decode, Encode};
 use frame_support::{traits::Instance, weights::Weight, RuntimeDebug};
 use hash_db::Hasher;
-use pallet_substrate_bridge::StorageProofChecker;
 use sp_runtime::traits::{AtLeast32BitUnsigned, CheckedAdd, CheckedDiv, CheckedMul};
 use sp_std::{cmp::PartialOrd, convert::TryFrom, fmt::Debug, marker::PhantomData, ops::RangeInclusive, vec::Vec};
 use sp_trie::StorageProof;
@@ -352,17 +351,17 @@ pub mod source {
  proof: FromBridgedChainMessagesDeliveryProof<HashOf<BridgedChain<B>>>,
  ) -> Result<ParsedMessagesDeliveryProofFromBridgedChain<B>, &'static str>
  where
- ThisRuntime: pallet_substrate_bridge::Config,
+ ThisRuntime: pallet_finality_verifier::Config,
  ThisRuntime: pallet_message_lane::Config<MessageLaneInstanceOf<BridgedChain<B>>>,
  HashOf<BridgedChain<B>>:
- Into<bp_runtime::HashOf<<ThisRuntime as pallet_substrate_bridge::Config>::BridgedChain>>,
+ Into<bp_runtime::HashOf<<ThisRuntime as pallet_finality_verifier::Config>::BridgedChain>>,
  {
  let FromBridgedChainMessagesDeliveryProof {
  bridged_header_hash,
  storage_proof,
  lane,
  } = proof;
- pallet_substrate_bridge::Module::<ThisRuntime>::parse_finalized_storage_proof(
+ pallet_finality_verifier::Module::<ThisRuntime>::parse_finalized_storage_proof(
  bridged_header_hash.into(),
  StorageProof::new(storage_proof),
  |storage| {
@@ -506,16 +505,16 @@ pub mod target {
  messages_count: u32,
  ) -> Result<ProvedMessages<Message<BalanceOf<BridgedChain<B>>>>, &'static str>
  where
- ThisRuntime: pallet_substrate_bridge::Config,
+ ThisRuntime: pallet_finality_verifier::Config,
  ThisRuntime: pallet_message_lane::Config<MessageLaneInstanceOf<BridgedChain<B>>>,
  HashOf<BridgedChain<B>>:
- Into<bp_runtime::HashOf<<ThisRuntime as pallet_substrate_bridge::Config>::BridgedChain>>,
+ Into<bp_runtime::HashOf<<ThisRuntime as pallet_finality_verifier::Config>::BridgedChain>>,
  {
  verify_messages_proof_with_parser::<B, _, _>(
  proof,
  messages_count,
  |bridged_header_hash, bridged_storage_proof| {
- pallet_substrate_bridge::Module::<ThisRuntime>::parse_finalized_storage_proof(
+ pallet_finality_verifier::Module::<ThisRuntime>::parse_finalized_storage_proof(
  bridged_header_hash.into(),
  StorageProof::new(bridged_storage_proof),
  |storage_adapter| storage_adapter,

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

@@ -73,7 +73,7 @@ pub fn prepare_message_proof<B, H, R, MM, ML, MH>(
 where
  B: MessageBridge,
  H: Hasher,
- R: pallet_substrate_bridge::Config,
+ R: pallet_finality_verifier::Config,
  <R::BridgedChain as bp_runtime::Chain>::Hash: Into<HashOf<BridgedChain<B>>>,
  MM: Fn(MessageKey) -> Vec<u8>,
  ML: Fn(LaneId) -> Vec<u8>,
@@ -129,7 +129,7 @@ where
  // prepare Bridged chain header and insert it into the Substrate pallet
  let bridged_header = make_bridged_header(root);
  let bridged_header_hash = bridged_header.hash();
- pallet_substrate_bridge::initialize_for_benchmarks::<R>(bridged_header);
+ pallet_finality_verifier::initialize_for_benchmarks::<R>(bridged_header);
 
  (
  FromBridgedChainMessagesProof {
@@ -154,7 +154,7 @@ pub fn prepare_message_delivery_proof<B, H, R, ML, MH>(
 where
  B: MessageBridge,
  H: Hasher,
- R: pallet_substrate_bridge::Config,
+ R: pallet_finality_verifier::Config,
  <R::BridgedChain as bp_runtime::Chain>::Hash: Into<HashOf<BridgedChain<B>>>,
  ML: Fn(LaneId) -> Vec<u8>,
  MH: Fn(H::Out) -> <R::BridgedChain as bp_runtime::Chain>::Header,
@@ -181,7 +181,7 @@ where
  // prepare Bridged chain header and insert it into the Substrate pallet
  let bridged_header = make_bridged_header(root);
  let bridged_header_hash = bridged_header.hash();
- pallet_substrate_bridge::initialize_for_benchmarks::<R>(bridged_header);
+ pallet_finality_verifier::initialize_for_benchmarks::<R>(bridged_header);
 
  FromBridgedChainMessagesDeliveryProof {
  bridged_header_hash: bridged_header_hash.into(),

bridges/modules/finality-verifier/Cargo.toml

@@ -25,6 +25,7 @@ frame-system = { git = "https:/paritytech/substrate.git", branch = "
 sp-finality-grandpa = { git = "https:/paritytech/substrate.git", branch = "master" , default-features = false }
 sp-runtime = { git = "https:/paritytech/substrate.git", branch = "master" , default-features = false }
 sp-std = { git = "https:/paritytech/substrate.git", branch = "master" , default-features = false }
+sp-trie = { git = "https:/paritytech/substrate.git", branch = "master" , default-features = false }
 
 [dev-dependencies]
 bp-test-utils = {path = "../../primitives/test-utils" }
@@ -46,4 +47,6 @@ std = [
  "sp-finality-grandpa/std",
  "sp-runtime/std",
  "sp-std/std",
+ "sp-trie/std",
 ]
+runtime-benchmarks = []

bridges/modules/finality-verifier/src/lib.rs

@@ -56,7 +56,7 @@ pub type BridgedBlockNumber<T> = BlockNumberOf<<T as Config>::BridgedChain>;
 /// Block hash of the bridged chain.
 pub type BridgedBlockHash<T> = HashOf<<T as Config>::BridgedChain>;
 /// Hasher of the bridged chain.
-pub type _BridgedBlockHasher<T> = HasherOf<<T as Config>::BridgedChain>;
+pub type BridgedBlockHasher<T> = HasherOf<<T as Config>::BridgedChain>;
 /// Header of the bridged chain.
 pub type BridgedHeader<T> = HeaderOf<<T as Config>::BridgedChain>;
 
@@ -335,6 +335,8 @@ pub mod pallet {
  TooManyRequests,
  /// The header being imported is older than the best finalized header known to the pallet.
  OldHeader,
+ /// The header is unknown to the pallet.
+ UnknownHeader,
  /// The scheduled authority set change found in the header is unsupported by the pallet.
  ///
  /// This is the case for non-standard (e.g forced) authority set changes.
@@ -343,6 +345,8 @@ pub mod pallet {
  AlreadyInitialized,
  /// All pallet operations are halted.
  Halted,
+ /// The storage proof doesn't contains storage root. So it is invalid for given header.
+ StorageRootMismatch,
  }
 
  /// Import the given header to the pallet's storage.
@@ -387,7 +391,7 @@ pub mod pallet {
 
  /// Since this writes to storage with no real checks this should only be used in functions that
  /// were called by a trusted origin.
- fn initialize_bridge<T: Config>(init_params: super::InitializationData<BridgedHeader<T>>) {
+ pub(crate) fn initialize_bridge<T: Config>(init_params: super::InitializationData<BridgedHeader<T>>) {
  let super::InitializationData {
  header,
  authority_list,
@@ -447,6 +451,21 @@ impl<T: Config> Pallet<T> {
  pub fn is_known_header(hash: BridgedBlockHash<T>) -> bool {
  <ImportedHeaders<T>>::contains_key(hash)
  }
+
+ /// Verify that the passed storage proof is valid, given it is crafted using
+ /// known finalized header. If the proof is valid, then the `parse` callback
+ /// is called and the function returns its result.
+ pub fn parse_finalized_storage_proof<R>(
+ hash: BridgedBlockHash<T>,
+ storage_proof: sp_trie::StorageProof,
+ parse: impl FnOnce(bp_runtime::StorageProofChecker<BridgedBlockHasher<T>>) -> R,
+ ) -> Result<R, sp_runtime::DispatchError> {
+ let header = <ImportedHeaders<T>>::get(hash).ok_or(Error::<T>::UnknownHeader)?;
+ let storage_proof_checker = bp_runtime::StorageProofChecker::new(*header.state_root(), storage_proof)
+ .map_err(|_| Error::<T>::StorageRootMismatch)?;
+
+ Ok(parse(storage_proof_checker))
+ }
 }
 
 /// Data required for initializing the bridge pallet.
@@ -499,6 +518,17 @@ pub(crate) fn find_forced_change<H: HeaderT>(
  header.digest().convert_first(|l| l.try_to(id).and_then(filter_log))
 }
 
+/// (Re)initialize bridge with given header for using it in external benchmarks.
+#[cfg(feature = "runtime-benchmarks")]
+pub fn initialize_for_benchmarks<T: Config>(header: BridgedHeader<T>) {
+ initialize_bridge::<T>(InitializationData {
+ header,
+ authority_list: Vec::new(), // we don't verify any proofs in external benchmarks
+ set_id: 0,
+ is_halted: false,
+ });
+}
+
 #[cfg(test)]
 mod tests {
  use super::*;
@@ -864,6 +894,40 @@ mod tests {
  );
  })
  }
+
+ #[test]
+ fn parse_finalized_storage_proof_rejects_proof_on_unknown_header() {
+ run_test(|| {
+ assert_noop!(
+ Module::<TestRuntime>::parse_finalized_storage_proof(
+ Default::default(),
+ sp_trie::StorageProof::new(vec![]),
+ |_| (),
+ ),
+ Error::<TestRuntime>::UnknownHeader,
+ );
+ });
+ }
+
+ #[test]
+ fn parse_finalized_storage_accepts_valid_proof() {
+ run_test(|| {
+ let (state_root, storage_proof) = bp_runtime::craft_valid_storage_proof();
+
+ let mut header = test_header(2);
+ header.set_state_root(state_root);
+
+ let hash = header.hash();
+ <BestFinalized<TestRuntime>>::put(hash);
+ <ImportedHeaders<TestRuntime>>::insert(hash, header);
+
+ assert_ok!(
+ Module::<TestRuntime>::parse_finalized_storage_proof(hash, storage_proof, |_| (),),
+ (),
+ );
+ });
+ }
+
  #[test]
  fn rate_limiter_disallows_imports_once_limit_is_hit_in_single_block() {
  run_test(|| {

bridges/modules/substrate/src/lib.rs

@@ -46,10 +46,7 @@ use sp_trie::StorageProof;
 // Re-export since the node uses these when configuring genesis
 pub use storage::{InitializationData, ScheduledChange};
 
-pub use storage_proof::StorageProofChecker;
-
 mod storage;
-mod storage_proof;
 mod verifier;
 
 #[cfg(test)]
@@ -355,7 +352,7 @@ impl<T: Config> Module<T> {
  pub fn parse_finalized_storage_proof<R>(
  finalized_header_hash: BridgedBlockHash<T>,
  storage_proof: StorageProof,
- parse: impl FnOnce(StorageProofChecker<BridgedBlockHasher<T>>) -> R,
+ parse: impl FnOnce(bp_runtime::StorageProofChecker<BridgedBlockHasher<T>>) -> R,
  ) -> Result<R, sp_runtime::DispatchError> {
  let storage = PalletStorage::<T>::new();
  let header = storage
@@ -365,8 +362,8 @@ impl<T: Config> Module<T> {
  return Err(Error::<T>::UnfinalizedHeader.into());
  }
 
- let storage_proof_checker =
- StorageProofChecker::new(*header.state_root(), storage_proof).map_err(Error::<T>::from)?;
+ let storage_proof_checker = bp_runtime::StorageProofChecker::new(*header.state_root(), storage_proof)
+ .map_err(|_| Error::<T>::StorageRootMismatch)?;
  Ok(parse(storage_proof_checker))
  }
 }
@@ -898,7 +895,7 @@ mod tests {
  fn parse_finalized_storage_accepts_valid_proof() {
  run_test(|| {
  let mut storage = PalletStorage::<TestRuntime>::new();
- let (state_root, storage_proof) = storage_proof::tests::craft_valid_storage_proof();
+ let (state_root, storage_proof) = bp_runtime::craft_valid_storage_proof();
  let mut header = unfinalized_header(1);
  header.is_finalized = true;
  header.header.set_state_root(state_root);

bridges/primitives/runtime/Cargo.toml

@@ -8,6 +8,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 
 [dependencies]
 codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false }
+hash-db = { version = "0.15.2", default-features = false }
 num-traits = { version = "0.2", default-features = false }
 
 # Substrate Dependencies
@@ -17,15 +18,23 @@ sp-core = { git = "https:/paritytech/substrate.git", branch = "maste
 sp-io = { git = "https:/paritytech/substrate.git", branch = "master" , default-features = false }
 sp-runtime = { git = "https:/paritytech/substrate.git", branch = "master" , default-features = false }
 sp-std = { git = "https:/paritytech/substrate.git", branch = "master" , default-features = false }
+sp-state-machine = { git = "https:/paritytech/substrate.git", branch = "master", default-features = false }
+sp-trie = { git = "https:/paritytech/substrate.git", branch = "master" , default-features = false }
+
+[dev-dependencies]
+sp-core = { git = "https:/paritytech/substrate.git", branch = "master" }
 
 [features]
 default = ["std"]
 std = [
  "codec/std",
  "frame-support/std",
+ "hash-db/std",
  "num-traits/std",
  "sp-core/std",
  "sp-io/std",
  "sp-runtime/std",
  "sp-std/std",
+ "sp-state-machine/std",
+ "sp-trie/std",
 ]

bridges/primitives/runtime/src/lib.rs

@@ -24,8 +24,13 @@ use sp_io::hashing::blake2_256;
 use sp_std::convert::TryFrom;
 
 pub use chain::{BlockNumberOf, Chain, HashOf, HasherOf, HeaderOf};
+pub use storage_proof::StorageProofChecker;
+
+#[cfg(feature = "std")]
+pub use storage_proof::craft_valid_storage_proof;
 
 mod chain;
+mod storage_proof;
 
 /// Use this when something must be shared among all instances.
 pub const NO_INSTANCE_ID: InstanceId = [0, 0, 0, 0];