From 922b6cecbe03626cedffaf956ec7d27727278872 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 6 Jun 2023 14:35:43 +0300 Subject: [PATCH 1/3] Use compact proofs for parachains --- .../src/parachains_benchmarking.rs | 55 ++++++++++++++++--- .../src/refund_relayer_extension.rs | 4 +- modules/parachains/src/call_ext.rs | 2 +- modules/parachains/src/lib.rs | 26 ++++----- primitives/polkadot-core/src/parachains.rs | 9 +-- primitives/test-utils/src/lib.rs | 13 +++-- .../src/parachains/source.rs | 23 +++++--- relays/parachains/src/parachains_loop.rs | 3 +- 8 files changed, 92 insertions(+), 43 deletions(-) diff --git a/bin/runtime-common/src/parachains_benchmarking.rs b/bin/runtime-common/src/parachains_benchmarking.rs index 56cd4bd6b32..a95a90f2455 100644 --- a/bin/runtime-common/src/parachains_benchmarking.rs +++ b/bin/runtime-common/src/parachains_benchmarking.rs @@ -22,12 +22,17 @@ use crate::messages_benchmarking::insert_header_to_grandpa_pallet; use bp_parachains::parachain_head_storage_key_at_source; use bp_polkadot_core::parachains::{ParaHash, ParaHead, ParaHeadsProof, ParaId}; -use bp_runtime::{grow_trie_leaf_value, record_all_trie_keys, StorageProofSize}; +use bp_runtime::{ + grow_trie_leaf_value, record_all_trie_keys, Chain, StorageProofSize, UntrustedVecDb, +}; use codec::Encode; -use frame_support::traits::Get; +use frame_support::{traits::Get, StateVersion}; +use pallet_bridge_grandpa::BridgedChain; use pallet_bridge_parachains::{RelayBlockHash, RelayBlockHasher, RelayBlockNumber}; use sp_std::prelude::*; -use sp_trie::{trie_types::TrieDBMutBuilderV1, LayoutV1, MemoryDB, TrieMut}; +use sp_trie::{ + LayoutV0, LayoutV1, MemoryDB, StorageProof, TrieConfiguration, TrieDBMutBuilder, TrieMut, +}; /// Prepare proof of messages for the `receive_messages_proof` call. /// @@ -43,7 +48,34 @@ where + pallet_bridge_grandpa::Config, PI: 'static, >::BridgedChain: - bp_runtime::Chain, + Chain, +{ + match as Chain>::STATE_VERSION { + StateVersion::V0 => do_prepare_parachain_heads_proof::>( + parachains, + parachain_head_size, + size, + ), + StateVersion::V1 => do_prepare_parachain_heads_proof::>( + parachains, + parachain_head_size, + size, + ), + } +} + +pub fn do_prepare_parachain_heads_proof( + parachains: &[ParaId], + parachain_head_size: u32, + size: StorageProofSize, +) -> (RelayBlockNumber, RelayBlockHash, ParaHeadsProof, Vec<(ParaId, ParaHash)>) +where + R: pallet_bridge_parachains::Config + + pallet_bridge_grandpa::Config, + PI: 'static, + >::BridgedChain: + Chain, + L: TrieConfiguration, { let parachain_head = ParaHead(vec![0u8; parachain_head_size as usize]); @@ -53,8 +85,7 @@ where let mut state_root = Default::default(); let mut mdb = MemoryDB::default(); { - let mut trie = - TrieDBMutBuilderV1::::new(&mut mdb, &mut state_root).build(); + let mut trie = TrieDBMutBuilder::::new(&mut mdb, &mut state_root).build(); // insert parachain heads for (i, parachain) in parachains.into_iter().enumerate() { @@ -68,18 +99,24 @@ where trie.insert(&storage_key.0, &leaf_data) .map_err(|_| "TrieMut::insert has failed") .expect("TrieMut::insert should not fail in benchmarks"); - storage_keys.push(storage_key); + storage_keys.push(storage_key.0); parachain_heads.push((*parachain, parachain_head.hash())) } } // generate heads storage proof - let proof = record_all_trie_keys::, _>(&mdb, &state_root) + let read_proof = record_all_trie_keys::(&mdb, &state_root) .map_err(|_| "record_all_trie_keys has failed") .expect("record_all_trie_keys should not fail in benchmarks"); + let storage_proof = UntrustedVecDb::try_new::( + StorageProof::new(read_proof), + state_root, + storage_keys, + ) + .expect("UntrustedVecDb::try_new() should not fail in benchmarks"); let (relay_block_number, relay_block_hash) = insert_header_to_grandpa_pallet::(state_root); - (relay_block_number, relay_block_hash, ParaHeadsProof(proof), parachain_heads) + (relay_block_number, relay_block_hash, ParaHeadsProof { storage_proof }, parachain_heads) } diff --git a/bin/runtime-common/src/refund_relayer_extension.rs b/bin/runtime-common/src/refund_relayer_extension.rs index a8e9d88988d..91e0b77c3e7 100644 --- a/bin/runtime-common/src/refund_relayer_extension.rs +++ b/bin/runtime-common/src/refund_relayer_extension.rs @@ -792,7 +792,7 @@ mod tests { ParaId(TestParachain::get()), [parachain_head_at_relay_header_number as u8; 32].into(), )], - parachain_heads_proof: ParaHeadsProof(vec![]), + parachain_heads_proof: ParaHeadsProof { storage_proof: Default::default() }, }) } @@ -1366,7 +1366,7 @@ mod tests { (ParaId(TestParachain::get()), [1u8; 32].into()), (ParaId(TestParachain::get() + 1), [1u8; 32].into()), ], - parachain_heads_proof: ParaHeadsProof(vec![]), + parachain_heads_proof: ParaHeadsProof { storage_proof: Default::default() }, }), message_delivery_call(200), ], diff --git a/modules/parachains/src/call_ext.rs b/modules/parachains/src/call_ext.rs index ee3770146c2..0210928f4d5 100644 --- a/modules/parachains/src/call_ext.rs +++ b/modules/parachains/src/call_ext.rs @@ -169,7 +169,7 @@ mod tests { RuntimeCall::Parachains(crate::Call::::submit_parachain_heads { at_relay_block: (num, Default::default()), parachains, - parachain_heads_proof: ParaHeadsProof(Vec::new()), + parachain_heads_proof: ParaHeadsProof { storage_proof: Default::default() }, }) .check_obsolete_submit_parachain_heads() .is_ok() diff --git a/modules/parachains/src/lib.rs b/modules/parachains/src/lib.rs index 773653ab6ed..5cfc54075bb 100644 --- a/modules/parachains/src/lib.rs +++ b/modules/parachains/src/lib.rs @@ -29,7 +29,7 @@ pub use weights_ext::WeightInfoExt; use bp_header_chain::{HeaderChain, HeaderChainError}; use bp_parachains::{parachain_head_storage_key_at_source, ParaInfo, ParaStoredHeaderData}; use bp_polkadot_core::parachains::{ParaHash, ParaHead, ParaHeadsProof, ParaId}; -use bp_runtime::{Chain, HashOf, HeaderId, HeaderIdOf, Parachain, StorageProofError}; +use bp_runtime::{Chain, HashOf, HeaderId, HeaderIdOf, Parachain}; use frame_support::{dispatch::PostDispatchInfo, DefaultNoBound}; use sp_std::{marker::PhantomData, vec::Vec}; @@ -81,7 +81,7 @@ pub mod pallet { }; use bp_runtime::{ BasicOperatingMode, BoundedStorageValue, OwnedBridgeModule, StorageDoubleMapKeyProvider, - StorageMapKeyProvider, + StorageMapKeyProvider, TrustedVecDb, VecDbError, }; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; @@ -333,9 +333,9 @@ pub mod pallet { parachains.len() as _, ); - let mut storage = GrandpaPalletOf::::storage_proof_checker( + let mut storage = GrandpaPalletOf::::verify_vec_db_storage( relay_block_hash, - parachain_heads_proof.0, + parachain_heads_proof.storage_proof, ) .map_err(Error::::HeaderChainStorageProof)?; @@ -438,9 +438,9 @@ pub mod pallet { // => treat this as an error // // (we can throw error here, because now all our calls are transactional) - storage.ensure_no_unused_nodes().map_err(|e| { - Error::::HeaderChainStorageProof(HeaderChainError::StorageProof(e)) - })?; + storage + .ensure_no_unused_keys() + .map_err(|e| Error::::HeaderChainStorageProof(HeaderChainError::VecDb(e)))?; Ok(PostDispatchInfo { actual_weight: Some(actual_weight), pays_fee: Pays::Yes }) } @@ -508,12 +508,12 @@ pub mod pallet { /// Read parachain head from storage proof. fn read_parachain_head( - storage: &mut bp_runtime::StorageProofChecker, + storage: &mut TrustedVecDb, parachain: ParaId, - ) -> Result, StorageProofError> { + ) -> Result, VecDbError> { let parachain_head_key = parachain_head_storage_key_at_source(T::ParasPalletName::get(), parachain); - storage.read_and_decode_value(parachain_head_key.0.as_ref()) + storage.get_and_decode_optional(¶chain_head_key) } /// Try to update parachain head. @@ -701,7 +701,7 @@ pub(crate) mod tests { }; use bp_runtime::{ BasicOperatingMode, OwnedBridgeModuleError, StorageDoubleMapKeyProvider, - StorageMapKeyProvider, + StorageMapKeyProvider, VecDbError, }; use bp_test_utils::{ authority_list, generate_owned_bridge_module_tests, make_default_justification, @@ -1408,8 +1408,8 @@ 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::::HeaderChainStorageProof(HeaderChainError::StorageProof( - StorageProofError::StorageRootMismatch + Error::::HeaderChainStorageProof(HeaderChainError::VecDb( + VecDbError::InvalidProof )) ); }); diff --git a/primitives/polkadot-core/src/parachains.rs b/primitives/polkadot-core/src/parachains.rs index 9cac3279c72..1342a2184c2 100644 --- a/primitives/polkadot-core/src/parachains.rs +++ b/primitives/polkadot-core/src/parachains.rs @@ -22,7 +22,7 @@ //! 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}; +use bp_runtime::{Size, UntrustedVecDb}; use codec::{CompactAs, Decode, Encode, MaxEncodedLen}; use frame_support::RuntimeDebug; use scale_info::TypeInfo; @@ -88,11 +88,12 @@ pub type ParaHasher = crate::Hasher; /// Raw storage proof of parachain heads, stored in polkadot-like chain runtime. #[derive(Clone, Decode, Encode, Eq, PartialEq, RuntimeDebug, TypeInfo)] -pub struct ParaHeadsProof(pub RawStorageProof); +pub struct ParaHeadsProof { + pub storage_proof: UntrustedVecDb, +} impl Size for ParaHeadsProof { fn size(&self) -> u32 { - u32::try_from(self.0.iter().fold(0usize, |sum, node| sum.saturating_add(node.len()))) - .unwrap_or(u32::MAX) + self.storage_proof.size() } } diff --git a/primitives/test-utils/src/lib.rs b/primitives/test-utils/src/lib.rs index 5a7d0cca279..43391a4a2c9 100644 --- a/primitives/test-utils/src/lib.rs +++ b/primitives/test-utils/src/lib.rs @@ -21,12 +21,12 @@ use bp_header_chain::justification::{required_justification_precommits, GrandpaJustification}; use bp_parachains::parachain_head_storage_key_at_source; use bp_polkadot_core::parachains::{ParaHash, ParaHead, ParaHeadsProof, ParaId}; -use bp_runtime::record_all_trie_keys; +use bp_runtime::{record_all_trie_keys, UntrustedVecDb}; use codec::Encode; use sp_consensus_grandpa::{AuthorityId, AuthoritySignature, AuthorityWeight, SetId}; use sp_runtime::traits::{Header as HeaderT, One, Zero}; use sp_std::prelude::*; -use sp_trie::{trie_types::TrieDBMutBuilderV1, LayoutV1, MemoryDB, TrieMut}; +use sp_trie::{trie_types::TrieDBMutBuilderV1, LayoutV1, MemoryDB, StorageProof, TrieMut}; // Re-export all our test account utilities pub use keyring::*; @@ -173,6 +173,7 @@ pub fn prepare_parachain_heads_proof( let mut parachains = Vec::with_capacity(heads.len()); let mut root = Default::default(); let mut mdb = MemoryDB::default(); + let mut storage_keys = vec![]; { let mut trie = TrieDBMutBuilderV1::::new(&mut mdb, &mut root).build(); for (parachain, head) in heads { @@ -181,16 +182,20 @@ pub fn prepare_parachain_heads_proof( trie.insert(&storage_key.0, &head.encode()) .map_err(|_| "TrieMut::insert has failed") .expect("TrieMut::insert should not fail in tests"); + storage_keys.push(storage_key.0); parachains.push((ParaId(parachain), head.hash())); } } // generate storage proof to be delivered to This chain - let storage_proof = record_all_trie_keys::, _>(&mdb, &root) + let read_proof = record_all_trie_keys::, _>(&mdb, &root) .map_err(|_| "record_all_trie_keys has failed") .expect("record_all_trie_keys should not fail in benchmarks"); + let storage_proof = + UntrustedVecDb::try_new::(StorageProof::new(read_proof), root, storage_keys) + .expect("UntrustedVecDb::try_new() should not fail in benchmarks"); - (root, ParaHeadsProof(storage_proof), parachains) + (root, ParaHeadsProof { storage_proof }, parachains) } /// Create signed precommit with given target. diff --git a/relays/lib-substrate-relay/src/parachains/source.rs b/relays/lib-substrate-relay/src/parachains/source.rs index af4c52321e4..fa59662be21 100644 --- a/relays/lib-substrate-relay/src/parachains/source.rs +++ b/relays/lib-substrate-relay/src/parachains/source.rs @@ -22,7 +22,7 @@ use async_std::sync::{Arc, Mutex}; use async_trait::async_trait; use bp_parachains::parachain_head_storage_key_at_source; use bp_polkadot_core::parachains::{ParaHash, ParaHead, ParaHeadsProof, ParaId}; -use bp_runtime::HeaderIdProvider; +use bp_runtime::{HasherOf, HeaderIdProvider, UntrustedVecDb}; use codec::Decode; use parachains_relay::parachains_loop::{AvailableHeader, SourceClient}; use relay_substrate_client::{ @@ -30,6 +30,7 @@ use relay_substrate_client::{ RelayChain, }; use relay_utils::relay_loop::Client as RelayClient; +use sp_runtime::traits::Header; /// Shared updatable reference to the maximal parachain header id that we want to sync from the /// source. @@ -141,12 +142,18 @@ where let parachain = ParaId(P::SourceParachain::PARACHAIN_ID); let storage_key = parachain_head_storage_key_at_source(P::SourceRelayChain::PARAS_PALLET_NAME, parachain); - let parachain_heads_proof = self - .client - .prove_storage(at_block.hash(), vec![storage_key.clone()]) - .await? - .into_iter_nodes() - .collect(); + + let root = *self.client.header_by_hash(at_block.hash()).await?.state_root(); + let read_proof = + self.client.prove_storage(at_block.hash(), vec![storage_key.clone()]).await?; + let storage_proof = UntrustedVecDb::try_new::>( + read_proof, + root, + vec![storage_key.clone()], + ) + .map_err(|e| { + SubstrateError::Custom(format!("Error generating parachain head proof: {:?}", e)) + })?; // why we're reading parachain head here once again (it has already been read at the // `parachain_head`)? that's because `parachain_head` sometimes returns obsolete parachain @@ -166,6 +173,6 @@ where })?; let parachain_head_hash = parachain_head.hash(); - Ok((ParaHeadsProof(parachain_heads_proof), parachain_head_hash)) + Ok((ParaHeadsProof { storage_proof }, parachain_head_hash)) } } diff --git a/relays/parachains/src/parachains_loop.rs b/relays/parachains/src/parachains_loop.rs index 9b9038fd976..0ab174f20e3 100644 --- a/relays/parachains/src/parachains_loop.rs +++ b/relays/parachains/src/parachains_loop.rs @@ -527,7 +527,6 @@ impl SubmittedHeadsTracker

{ mod tests { use super::*; use async_std::sync::{Arc, Mutex}; - use codec::Encode; use futures::{SinkExt, StreamExt}; use relay_substrate_client::test_chain::{TestChain, TestParachain}; use relay_utils::{HeaderId, MaybeConnectionError}; @@ -648,7 +647,7 @@ mod tests { _at_block: HeaderIdOf, ) -> Result<(ParaHeadsProof, ParaHash), TestError> { let head = *self.data.lock().await.source_head.clone()?.as_available().unwrap(); - let proof = (ParaHeadsProof(vec![head.hash().encode()]), head.hash()); + let proof = (ParaHeadsProof { storage_proof: Default::default() }, head.hash()); self.data.lock().await.source_proof.clone().map(|_| proof) } } From 57afe9e0467a45dc3e227804fa663eb396a4ad03 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 6 Jun 2023 17:12:22 +0300 Subject: [PATCH 2/3] Remove StorageProofChecker --- fuzz/storage-proof/src/main.rs | 41 ++--- modules/grandpa/src/lib.rs | 18 +- primitives/header-chain/src/lib.rs | 14 +- primitives/runtime/src/lib.rs | 7 +- primitives/runtime/src/storage_proof.rs | 214 +----------------------- primitives/runtime/src/vec_db.rs | 138 ++++++++------- relays/client-substrate/src/error.rs | 3 - 7 files changed, 116 insertions(+), 319 deletions(-) diff --git a/fuzz/storage-proof/src/main.rs b/fuzz/storage-proof/src/main.rs index 9ceda58d163..90de2b530c9 100644 --- a/fuzz/storage-proof/src/main.rs +++ b/fuzz/storage-proof/src/main.rs @@ -21,26 +21,14 @@ use honggfuzz::fuzz; // Logic for checking Substrate storage proofs. -use sp_core::{Blake2Hasher, H256}; -use sp_state_machine::{backend::Backend, prove_read, InMemoryBackend}; +use bp_runtime::UntrustedVecDb; +use sp_core::{storage::StateVersion, Blake2Hasher}; use sp_std::vec::Vec; use std::collections::HashMap; -fn craft_known_storage_proof( - input_vec: Vec<(Vec, Vec)>, -) -> (H256, bp_runtime::RawStorageProof) { - let storage_proof_vec = - vec![(None, input_vec.iter().map(|x| (x.0.clone(), Some(x.1.clone()))).collect())]; - log::info!("Storage proof vec {:?}", storage_proof_vec); - let state_version = sp_runtime::StateVersion::default(); - let backend = >::from((storage_proof_vec, state_version)); - let root = backend.storage_root(std::iter::empty(), state_version).0; - let vector_element_proof = - prove_read(backend, input_vec.iter().map(|x| x.0.as_slice())).unwrap(); - (root, vector_element_proof.iter_nodes().cloned().collect()) -} - -fn transform_into_unique(input_vec: Vec<(Vec, Vec)>) -> Vec<(Vec, Vec)> { +fn transform_into_unique( + input_vec: Vec<(Vec, Option>)>, +) -> Vec<(Vec, Option>)> { let mut output_hashmap = HashMap::new(); let mut output_vec = Vec::new(); for key_value_pair in input_vec { @@ -53,18 +41,23 @@ fn transform_into_unique(input_vec: Vec<(Vec, Vec)>) -> Vec<(Vec, Ve } fn run_fuzzer() { - fuzz!(|input_vec: Vec<(Vec, Vec)>| { + fuzz!(|input_vec: Vec<(Vec, Option>)>| { if input_vec.is_empty() { return } let unique_input_vec = transform_into_unique(input_vec); - let (root, craft_known_storage_proof) = craft_known_storage_proof(unique_input_vec.clone()); - let mut checker = - >::new(root, craft_known_storage_proof) - .expect("Valid proof passed; qed"); - for key_value_pair in unique_input_vec { + let (root, storage_proof) = UntrustedVecDb::try_from_entries::( + StateVersion::default(), + &unique_input_vec, + ) + .expect("UntrustedVecDb::try_from_entries() shouldn't fail"); + let mut storage = storage_proof + .verify::(StateVersion::V1, &root) + .expect("UntrustedVecDb::verify() shouldn't fail"); + + for key_value_pair in &unique_input_vec { log::info!("Reading value for pair {:?}", key_value_pair); - assert_eq!(checker.read_value(&key_value_pair.0), Ok(Some(key_value_pair.1.clone()))); + assert_eq!(storage.get(&key_value_pair.0), Ok(&key_value_pair.1)); } }) } diff --git a/modules/grandpa/src/lib.rs b/modules/grandpa/src/lib.rs index 4d6e8951e52..0eb8eae13c1 100644 --- a/modules/grandpa/src/lib.rs +++ b/modules/grandpa/src/lib.rs @@ -672,7 +672,7 @@ mod tests { TestHeader, TestNumber, TestRuntime, MAX_BRIDGED_AUTHORITIES, }; use bp_header_chain::BridgeGrandpaCall; - use bp_runtime::BasicOperatingMode; + use bp_runtime::{BasicOperatingMode, UntrustedVecDb}; use bp_test_utils::{ authority_list, generate_owned_bridge_module_tests, make_default_justification, make_justification_for_header, JustificationGeneratorParams, ALICE, BOB, @@ -1203,11 +1203,14 @@ mod tests { } #[test] - fn parse_finalized_storage_proof_rejects_proof_on_unknown_header() { + fn verify_vec_db_storage_rejects_unknown_header() { run_test(|| { assert_noop!( - Pallet::::storage_proof_checker(Default::default(), vec![],) - .map(|_| ()), + Pallet::::verify_vec_db_storage( + Default::default(), + Default::default(), + ) + .map(|_| ()), bp_header_chain::HeaderChainError::UnknownHeader, ); }); @@ -1216,7 +1219,10 @@ mod tests { #[test] fn parse_finalized_storage_accepts_valid_proof() { run_test(|| { - let (state_root, storage_proof) = bp_runtime::craft_valid_storage_proof(); + let (state_root, storage_proof) = UntrustedVecDb::try_from_entries::< + sp_core::Blake2Hasher, + >(Default::default(), &[(b"key1".to_vec(), None)]) + .expect("UntrustedVecDb::try_from_entries() shouldn't fail in tests"); let mut header = test_header(2); header.set_state_root(state_root); @@ -1226,7 +1232,7 @@ mod tests { >::insert(hash, header.build()); assert_ok!( - Pallet::::storage_proof_checker(hash, storage_proof).map(|_| ()) + Pallet::::verify_vec_db_storage(hash, storage_proof).map(|_| ()) ); }); } diff --git a/primitives/header-chain/src/lib.rs b/primitives/header-chain/src/lib.rs index 74f6616ad52..9b210d66d15 100644 --- a/primitives/header-chain/src/lib.rs +++ b/primitives/header-chain/src/lib.rs @@ -20,8 +20,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use bp_runtime::{ - BasicOperatingMode, Chain, HashOf, HasherOf, HeaderOf, RawStorageProof, StorageProofChecker, - StorageProofError, TrustedVecDb, UntrustedVecDb, VecDbError, + BasicOperatingMode, Chain, HashOf, HasherOf, HeaderOf, TrustedVecDb, UntrustedVecDb, VecDbError, }; use codec::{Codec, Decode, Encode, EncodeLike, MaxEncodedLen}; use core::{clone::Clone, cmp::Eq, default::Default, fmt::Debug}; @@ -41,8 +40,6 @@ pub mod storage_keys; pub enum HeaderChainError { /// Header with given hash is missing from the chain. UnknownHeader, - /// Storage proof related error. - StorageProof(StorageProofError), /// Error generated by the `vec_db` module. VecDb(VecDbError), } @@ -85,15 +82,6 @@ pub trait HeaderChain { db.verify::>(C::STATE_VERSION, &state_root) .map_err(HeaderChainError::VecDb) } - /// Get storage proof checker using finalized header. - fn storage_proof_checker( - header_hash: HashOf, - storage_proof: RawStorageProof, - ) -> Result>, HeaderChainError> { - let state_root = Self::finalized_header_state_root(header_hash) - .ok_or(HeaderChainError::UnknownHeader)?; - StorageProofChecker::new(state_root, storage_proof).map_err(HeaderChainError::StorageProof) - } } /// A type that can be used as a parameter in a dispatchable function. diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 85be38432c9..ccac68b3744 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -37,15 +37,12 @@ pub use chain::{ pub use frame_support::storage::storage_prefix as storage_value_final_key; use num_traits::{CheckedAdd, CheckedSub, One, SaturatingAdd, Zero}; pub use storage_proof::{ - grow_trie_leaf_value, record_all_keys as record_all_trie_keys, Error as StorageProofError, - ProofSize as StorageProofSize, RawStorageProof, StorageProofChecker, + grow_trie_leaf_value, record_all_keys as record_all_trie_keys, ProofSize as StorageProofSize, + RawStorageProof, }; pub use storage_types::BoundedStorageValue; pub use vec_db::{TrustedVecDb, UntrustedVecDb, VecDbError}; -#[cfg(feature = "std")] -pub use storage_proof::craft_valid_storage_proof; - pub mod extensions; pub mod messages; diff --git a/primitives/runtime/src/storage_proof.rs b/primitives/runtime/src/storage_proof.rs index d6a2c15f78d..256a9590655 100644 --- a/primitives/runtime/src/storage_proof.rs +++ b/primitives/runtime/src/storage_proof.rs @@ -14,18 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . -//! Logic for checking Substrate storage proofs. +//! Additional logic for working with Substrate storage proofs. -use crate::StrippableError; -use codec::{Decode, Encode}; -use frame_support::PalletError; -use hash_db::{HashDB, Hasher, EMPTY_PREFIX}; -use scale_info::TypeInfo; use sp_std::{boxed::Box, collections::btree_set::BTreeSet, vec::Vec}; -use sp_trie::{ - read_trie_value, LayoutV1, MemoryDB, Recorder, StorageProof, Trie, TrieConfiguration, - TrieDBBuilder, TrieError, TrieHash, -}; +use sp_trie::{Recorder, Trie, TrieConfiguration, TrieDBBuilder, TrieError, TrieHash}; /// Raw storage proof type (just raw trie nodes). pub type RawStorageProof = Vec>; @@ -55,150 +47,6 @@ pub fn grow_trie_leaf_value(mut value: Vec, size: ProofSize) -> Vec { value } -/// This struct is used to read storage values from a subset of a Merklized database. The "proof" -/// is a subset of the nodes in the Merkle structure of the database, so that it provides -/// authentication against a known Merkle root as well as the values in the -/// database themselves. -pub struct StorageProofChecker -where - H: Hasher, -{ - proof_nodes_count: usize, - root: H::Out, - db: MemoryDB, - recorder: Recorder>, -} - -impl StorageProofChecker -where - H: Hasher, -{ - /// Constructs a new storage proof checker. - /// - /// This returns an error if the given proof is invalid with respect to the given root. - pub fn new(root: H::Out, proof: RawStorageProof) -> Result { - // 1. we don't want extra items in the storage proof - // 2. `StorageProof` is storing all trie nodes in the `BTreeSet` - // - // => someone could simply add duplicate items to the proof and we won't be - // able to detect that by just using `StorageProof` - // - // => let's check it when we are converting our "raw proof" into `StorageProof` - let proof_nodes_count = proof.len(); - let proof = StorageProof::new(proof); - if proof_nodes_count != proof.iter_nodes().count() { - return Err(Error::DuplicateNodesInProof) - } - - let db = proof.into_memory_db(); - if !db.contains(&root, EMPTY_PREFIX) { - return Err(Error::StorageRootMismatch) - } - - let recorder = Recorder::default(); - let checker = StorageProofChecker { proof_nodes_count, root, db, recorder }; - Ok(checker) - } - - /// Returns error if the proof has some nodes that are left intact by previous `read_value` - /// calls. - pub fn ensure_no_unused_nodes(mut self) -> Result<(), Error> { - let visited_nodes = self - .recorder - .drain() - .into_iter() - .map(|record| record.data) - .collect::>(); - let visited_nodes_count = visited_nodes.len(); - if self.proof_nodes_count == visited_nodes_count { - Ok(()) - } else { - Err(Error::UnusedNodesInTheProof) - } - } - - /// Reads a value from the available subset of storage. If the value cannot be read due to an - /// incomplete or otherwise invalid proof, this function returns an error. - pub fn read_value(&mut self, key: &[u8]) -> Result>, Error> { - // LayoutV1 or LayoutV0 is identical for proof that only read values. - read_trie_value::, _>(&self.db, &self.root, key, Some(&mut self.recorder), None) - .map_err(|_| Error::StorageValueUnavailable) - } - - /// Reads and decodes a value from the available subset of storage. If the value cannot be read - /// due to an incomplete or otherwise invalid proof, this function returns an error. If value is - /// read, but decoding fails, this function returns an error. - pub fn read_and_decode_value(&mut self, key: &[u8]) -> Result, Error> { - self.read_value(key).and_then(|v| { - v.map(|v| T::decode(&mut &v[..]).map_err(|e| Error::StorageValueDecodeFailed(e.into()))) - .transpose() - }) - } - - /// Reads and decodes a value from the available subset of storage. If the value cannot be read - /// due to an incomplete or otherwise invalid proof, or if the value is `None`, this function - /// returns an error. If value is read, but decoding fails, this function returns an error. - pub fn read_and_decode_mandatory_value(&mut self, key: &[u8]) -> Result { - self.read_and_decode_value(key)?.ok_or(Error::StorageValueEmpty) - } - - /// Reads and decodes a value from the available subset of storage. If the value cannot be read - /// due to an incomplete or otherwise invalid proof, this function returns `Ok(None)`. - /// If value is read, but decoding fails, this function returns an error. - pub fn read_and_decode_opt_value(&mut self, key: &[u8]) -> Result, Error> { - match self.read_and_decode_value(key) { - Ok(outbound_lane_data) => Ok(outbound_lane_data), - Err(Error::StorageValueUnavailable) => Ok(None), - Err(e) => Err(e), - } - } -} - -/// Storage proof related errors. -#[derive(Encode, Decode, Clone, Eq, PartialEq, PalletError, Debug, TypeInfo)] -pub enum Error { - /// Duplicate trie nodes are found in the proof. - DuplicateNodesInProof, - /// Unused trie nodes are found in the proof. - UnusedNodesInTheProof, - /// Expected storage root is missing from the proof. - StorageRootMismatch, - /// Unable to reach expected storage value using provided trie nodes. - StorageValueUnavailable, - /// The storage value is `None`. - StorageValueEmpty, - /// Failed to decode storage value. - StorageValueDecodeFailed(StrippableError), -} - -/// Return valid storage proof and state root. -/// -/// NOTE: This should only be used for **testing**. -#[cfg(feature = "std")] -pub fn craft_valid_storage_proof() -> (sp_core::H256, RawStorageProof) { - use sp_state_machine::{backend::Backend, prove_read, InMemoryBackend}; - - let state_version = sp_runtime::StateVersion::default(); - - // construct storage proof - let backend = >::from(( - vec![ - (None, vec![(b"key1".to_vec(), Some(b"value1".to_vec()))]), - (None, vec![(b"key2".to_vec(), Some(b"value2".to_vec()))]), - (None, vec![(b"key3".to_vec(), Some(b"value3".to_vec()))]), - (None, vec![(b"key4".to_vec(), Some((42u64, 42u32, 42u16, 42u8).encode()))]), - // Value is too big to fit in a branch node - (None, vec![(b"key11".to_vec(), Some(vec![0u8; 32]))]), - ], - state_version, - )); - let root = backend.storage_root(std::iter::empty(), state_version).0; - let proof = - prove_read(backend, &[&b"key1"[..], &b"key2"[..], &b"key4"[..], &b"key22"[..]]).unwrap(); - - (root, proof.into_nodes().into_iter().collect()) -} - /// Record all keys for a given root. pub fn record_all_keys( db: &DB, @@ -224,61 +72,3 @@ where .into_iter() .collect()) } - -#[cfg(test)] -pub mod tests { - use super::*; - use codec::Encode; - - #[test] - fn storage_proof_check() { - let (root, proof) = craft_valid_storage_proof(); - - // check proof in runtime - let mut checker = - >::new(root, proof.clone()).unwrap(); - assert_eq!(checker.read_value(b"key1"), Ok(Some(b"value1".to_vec()))); - assert_eq!(checker.read_value(b"key2"), Ok(Some(b"value2".to_vec()))); - assert_eq!(checker.read_value(b"key4"), Ok(Some((42u64, 42u32, 42u16, 42u8).encode()))); - assert_eq!(checker.read_value(b"key11111"), Err(Error::StorageValueUnavailable)); - assert_eq!(checker.read_value(b"key22"), Ok(None)); - assert_eq!(checker.read_and_decode_value(b"key4"), Ok(Some((42u64, 42u32, 42u16, 42u8))),); - assert!(matches!( - checker.read_and_decode_value::<[u8; 64]>(b"key4"), - Err(Error::StorageValueDecodeFailed(_)), - )); - - // checking proof against invalid commitment fails - assert_eq!( - >::new(sp_core::H256::random(), proof).err(), - Some(Error::StorageRootMismatch) - ); - } - - #[test] - fn proof_with_duplicate_items_is_rejected() { - let (root, mut proof) = craft_valid_storage_proof(); - proof.push(proof.first().unwrap().clone()); - - assert_eq!( - StorageProofChecker::::new(root, proof).map(drop), - Err(Error::DuplicateNodesInProof), - ); - } - - #[test] - fn proof_with_unused_items_is_rejected() { - let (root, proof) = craft_valid_storage_proof(); - - let mut checker = - StorageProofChecker::::new(root, proof.clone()).unwrap(); - checker.read_value(b"key1").unwrap(); - checker.read_value(b"key2").unwrap(); - checker.read_value(b"key4").unwrap(); - checker.read_value(b"key22").unwrap(); - assert_eq!(checker.ensure_no_unused_nodes(), Ok(())); - - let checker = StorageProofChecker::::new(root, proof).unwrap(); - assert_eq!(checker.ensure_no_unused_nodes(), Err(Error::UnusedNodesInTheProof)); - } -} diff --git a/primitives/runtime/src/vec_db.rs b/primitives/runtime/src/vec_db.rs index 30467af5501..81cecaa8a6f 100644 --- a/primitives/runtime/src/vec_db.rs +++ b/primitives/runtime/src/vec_db.rs @@ -21,16 +21,20 @@ use sp_core::{storage::TrackedStorageKey, RuntimeDebug}; use sp_runtime::SaturatedConversion; use sp_std::{default::Default, vec, vec::Vec}; use sp_trie::{ - generate_trie_proof, verify_trie_proof, LayoutV0, LayoutV1, StorageProof, TrieDBBuilder, - TrieHash, + generate_trie_proof, verify_trie_proof, LayoutV0, LayoutV1, MemoryDB, StorageProof, + TrieDBBuilder, TrieHash, }; -use codec::{Decode, Encode}; +use codec::{Codec, Decode, Encode}; use hash_db::Hasher; use scale_info::TypeInfo; +use sp_state_machine::TrieBackend; use trie_db::{DBValue, Trie}; -use crate::{storage_proof::RawStorageProof, Size}; +use crate::{ + storage_proof::{record_all_keys, RawStorageProof}, + Size, +}; pub type RawStorageKey = Vec; @@ -89,6 +93,29 @@ impl UntrustedVecDb { Ok(Self { proof: trie_proof, db: entries }) } + /// Creates a new instance of `UntrustedVecDb` from the provided entries. + /// + /// This function is used by the test logic. + #[cfg(feature = "std")] + pub fn try_from_entries( + state_version: StateVersion, + entries: &[(RawStorageKey, Option)], + ) -> Option<(H::Out, UntrustedVecDb)> + where + H::Out: Codec, + { + let keys: Vec<_> = entries.iter().map(|(key, _)| key.clone()).collect(); + let entries: Vec<_> = + entries.iter().cloned().map(|(key, val)| (None, vec![(key, val)])).collect(); + let backend = TrieBackend::, H>::from((entries, state_version)); + let root = *backend.root(); + let read_proof = StorageProof::new( + record_all_keys::, _>(backend.backend_storage(), &root).ok()?, + ); + + Some((root, UntrustedVecDb::try_new::(read_proof, root, keys).ok()?)) + } + /// Validates the contained `db` against the contained proof. If the `db` is valid, converts it /// into a `TrustedVecDb`. pub fn verify( @@ -193,43 +220,30 @@ impl TrustedVecDb { #[cfg(test)] mod tests { use super::*; - use sp_core::H256; - - use sp_state_machine::{prove_read, InMemoryBackend}; type Hasher = sp_core::Blake2Hasher; - fn generate_untrusted_vec_db( - entries: Vec<(RawStorageKey, Option)>, - ) -> (H256, Result) { - let keys: Vec<_> = entries.iter().map(|(key, _)| key.clone()).collect(); - let entries: Vec<_> = - entries.iter().cloned().map(|(key, val)| (None, vec![(key, val)])).collect(); - let backend = InMemoryBackend::::from((entries, StateVersion::V1)); - let root = *backend.root(); - let read_proof = prove_read(backend, &keys).unwrap(); - - (root, UntrustedVecDb::try_new::(read_proof, root, keys)) - } - #[test] fn verify_succeeds_when_used_correctly() { - let (root, maybe_db) = generate_untrusted_vec_db(vec![ - (b"key1".to_vec(), None), - (b"key2".to_vec(), Some(b"val2".to_vec())), - ]); - let db = maybe_db.unwrap(); + let (root, db) = UntrustedVecDb::try_from_entries::( + StateVersion::default(), + &[(b"key1".to_vec(), None), (b"key2".to_vec(), Some(b"val2".to_vec()))], + ) + .expect("UntrustedVecDb::try_from_entries() shouldn't fail in tests"); assert!(db.verify::(StateVersion::V1, &root).is_ok()); } #[test] fn verify_fails_when_proof_contains_unneeded_nodes() { - let (root, maybe_db) = generate_untrusted_vec_db(vec![ - (b"key1".to_vec(), Some(b"val1".to_vec().encode())), - (b"key2".to_vec(), Some(b"val2".to_vec().encode())), - ]); - let mut db = maybe_db.unwrap(); + let (root, mut db) = UntrustedVecDb::try_from_entries::( + StateVersion::default(), + &[ + (b"key1".to_vec(), Some(b"val1".to_vec().encode())), + (b"key2".to_vec(), Some(b"val2".to_vec().encode())), + ], + ) + .expect("UntrustedVecDb::try_from_entries() shouldn't fail in tests"); assert!(db.db.pop().is_some()); assert!(matches!( @@ -240,8 +254,11 @@ mod tests { #[test] fn verify_fails_when_db_contains_duplicate_nodes() { - let (root, maybe_db) = generate_untrusted_vec_db(vec![(b"key".to_vec(), None)]); - let mut db = maybe_db.unwrap(); + let (root, mut db) = UntrustedVecDb::try_from_entries::( + StateVersion::default(), + &[(b"key".to_vec(), None)], + ) + .expect("UntrustedVecDb::try_from_entries() shouldn't fail in tests"); db.db.push((b"key".to_vec(), None)); assert!(matches!( @@ -252,11 +269,14 @@ mod tests { #[test] fn verify_fails_when_entries_are_not_sorted() { - let (root, maybe_db) = generate_untrusted_vec_db(vec![ - (b"key1".to_vec(), Some(b"val1".to_vec().encode())), - (b"key2".to_vec(), Some(b"val2".to_vec().encode())), - ]); - let mut db = maybe_db.unwrap(); + let (root, mut db) = UntrustedVecDb::try_from_entries::( + StateVersion::default(), + &[ + (b"key1".to_vec(), Some(b"val1".to_vec().encode())), + (b"key2".to_vec(), Some(b"val2".to_vec().encode())), + ], + ) + .expect("UntrustedVecDb::try_from_entries() shouldn't fail in tests"); db.db.reverse(); assert!(matches!( @@ -267,13 +287,16 @@ mod tests { #[test] fn get_and_decode_mandatory_works() { - let (root, maybe_db) = generate_untrusted_vec_db(vec![ - (b"key11".to_vec(), Some(b"val11".to_vec().encode())), - (b"key2".to_vec(), Some(b"val2".to_vec().encode())), - (b"key1".to_vec(), None), - (b"key15".to_vec(), Some(b"val15".to_vec())), - ]); - let db = maybe_db.unwrap(); + let (root, db) = UntrustedVecDb::try_from_entries::( + StateVersion::default(), + &[ + (b"key11".to_vec(), Some(b"val11".to_vec().encode())), + (b"key2".to_vec(), Some(b"val2".to_vec().encode())), + (b"key1".to_vec(), None), + (b"key15".to_vec(), Some(b"val15".to_vec())), + ], + ) + .expect("UntrustedVecDb::try_from_entries() shouldn't fail in tests"); let mut trusted_db = db.verify::(StateVersion::V1, &root).unwrap(); assert!( @@ -294,13 +317,16 @@ mod tests { #[test] fn get_and_decode_optional_works() { - let (root, maybe_db) = generate_untrusted_vec_db(vec![ - (b"key11".to_vec(), Some(b"val11".to_vec().encode())), - (b"key2".to_vec(), Some(b"val2".to_vec().encode())), - (b"key1".to_vec(), None), - (b"key15".to_vec(), Some(b"val15".to_vec())), - ]); - let db = maybe_db.unwrap(); + let (root, db) = UntrustedVecDb::try_from_entries::( + StateVersion::default(), + &[ + (b"key11".to_vec(), Some(b"val11".to_vec().encode())), + (b"key2".to_vec(), Some(b"val2".to_vec().encode())), + (b"key1".to_vec(), None), + (b"key15".to_vec(), Some(b"val15".to_vec())), + ], + ) + .expect("UntrustedVecDb::try_from_entries() shouldn't fail in tests"); let mut trusted_db = db.verify::(StateVersion::V1, &root).unwrap(); assert!( @@ -319,11 +345,11 @@ mod tests { #[test] fn ensure_no_unused_keys_works_correctly() { - let (root, maybe_db) = generate_untrusted_vec_db(vec![ - (b"key1".to_vec(), None), - (b"key2".to_vec(), Some(b"val2".to_vec())), - ]); - let db = maybe_db.unwrap(); + let (root, db) = UntrustedVecDb::try_from_entries::( + StateVersion::default(), + &[(b"key1".to_vec(), None), (b"key2".to_vec(), Some(b"val2".to_vec()))], + ) + .expect("UntrustedVecDb::try_from_entries() shouldn't fail in tests"); let mut trusted_db = db.verify::(StateVersion::V1, &root).unwrap(); assert!(trusted_db.get(b"key1").is_ok()); diff --git a/relays/client-substrate/src/error.rs b/relays/client-substrate/src/error.rs index e5bff115a3d..4951f3ea28d 100644 --- a/relays/client-substrate/src/error.rs +++ b/relays/client-substrate/src/error.rs @@ -186,9 +186,6 @@ pub enum Error { /// The bridge pallet is not yet initialized and all transactions will be rejected. #[error("Bridge pallet is not initialized.")] BridgePalletIsNotInitialized, - /// An error has happened when we have tried to parse storage proof. - #[error("Error when parsing storage proof: {0:?}.")] - StorageProofError(bp_runtime::StorageProofError), /// The Substrate transaction is invalid. #[error("Substrate transaction is invalid: {0:?}")] TransactionInvalid(#[from] TransactionValidityError), From 124605a7211cb0faee2dca8394b2baa3ff847fd2 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 7 Jun 2023 11:28:04 +0300 Subject: [PATCH 3/3] Cleanup --- .../src/messages_benchmarking.rs | 17 +++--- .../src/parachains_benchmarking.rs | 19 ++----- .../messages/src/tests/messages_generation.rs | 33 ++++-------- primitives/runtime/src/lib.rs | 5 +- primitives/runtime/src/storage_proof.rs | 32 +---------- primitives/runtime/src/vec_db.rs | 53 ++++++++++++++----- primitives/test-utils/src/lib.rs | 12 ++--- 7 files changed, 67 insertions(+), 104 deletions(-) diff --git a/bin/runtime-common/src/messages_benchmarking.rs b/bin/runtime-common/src/messages_benchmarking.rs index 4b1c28e3391..dfc4d3bd7e1 100644 --- a/bin/runtime-common/src/messages_benchmarking.rs +++ b/bin/runtime-common/src/messages_benchmarking.rs @@ -28,8 +28,8 @@ use bp_messages::{ }; use bp_polkadot_core::parachains::ParaHash; use bp_runtime::{ - grow_trie_leaf_value, record_all_trie_keys, AccountIdOf, Chain, HashOf, HasherOf, Parachain, - StorageProofSize, UntrustedVecDb, + grow_trie_leaf_value, AccountIdOf, Chain, HashOf, HasherOf, Parachain, StorageProofSize, + UntrustedVecDb, }; use codec::Encode; use frame_support::{weights::Weight, StateVersion}; @@ -40,9 +40,7 @@ use pallet_bridge_messages::{ }; use sp_runtime::traits::{Header, Zero}; use sp_std::prelude::*; -use sp_trie::{ - LayoutV0, LayoutV1, MemoryDB, StorageProof, TrieConfiguration, TrieDBMutBuilder, TrieMut, -}; +use sp_trie::{LayoutV0, LayoutV1, MemoryDB, TrieConfiguration, TrieDBMutBuilder, TrieMut}; use xcm::v3::prelude::*; /// Prepare inbound bridge message according to given message proof parameters. @@ -285,15 +283,12 @@ where } // generate storage proof to be delivered to This chain - let read_proof = record_all_trie_keys::(&mdb, &root) - .map_err(|_| "record_all_trie_keys has failed") - .expect("record_all_trie_keys should not fail in benchmarks"); - let storage_proof = UntrustedVecDb::try_new::>>( - StorageProof::new(read_proof), + let storage_proof = UntrustedVecDb::try_from_db::>, _>( + &mdb, root, vec![storage_key], ) - .unwrap(); + .expect("UntrustedVecDb::try_from_db() should not fail in benchmarks"); (root, storage_proof) } diff --git a/bin/runtime-common/src/parachains_benchmarking.rs b/bin/runtime-common/src/parachains_benchmarking.rs index a95a90f2455..4906291f0ad 100644 --- a/bin/runtime-common/src/parachains_benchmarking.rs +++ b/bin/runtime-common/src/parachains_benchmarking.rs @@ -22,17 +22,13 @@ use crate::messages_benchmarking::insert_header_to_grandpa_pallet; use bp_parachains::parachain_head_storage_key_at_source; use bp_polkadot_core::parachains::{ParaHash, ParaHead, ParaHeadsProof, ParaId}; -use bp_runtime::{ - grow_trie_leaf_value, record_all_trie_keys, Chain, StorageProofSize, UntrustedVecDb, -}; +use bp_runtime::{grow_trie_leaf_value, Chain, StorageProofSize, UntrustedVecDb}; use codec::Encode; use frame_support::{traits::Get, StateVersion}; use pallet_bridge_grandpa::BridgedChain; use pallet_bridge_parachains::{RelayBlockHash, RelayBlockHasher, RelayBlockNumber}; use sp_std::prelude::*; -use sp_trie::{ - LayoutV0, LayoutV1, MemoryDB, StorageProof, TrieConfiguration, TrieDBMutBuilder, TrieMut, -}; +use sp_trie::{LayoutV0, LayoutV1, MemoryDB, TrieConfiguration, TrieDBMutBuilder, TrieMut}; /// Prepare proof of messages for the `receive_messages_proof` call. /// @@ -105,15 +101,8 @@ where } // generate heads storage proof - let read_proof = record_all_trie_keys::(&mdb, &state_root) - .map_err(|_| "record_all_trie_keys has failed") - .expect("record_all_trie_keys should not fail in benchmarks"); - let storage_proof = UntrustedVecDb::try_new::( - StorageProof::new(read_proof), - state_root, - storage_keys, - ) - .expect("UntrustedVecDb::try_new() should not fail in benchmarks"); + let storage_proof = UntrustedVecDb::try_from_db::(&mdb, state_root, storage_keys) + .expect("UntrustedVecDb::try_from_db() should not fail in benchmarks"); let (relay_block_number, relay_block_hash) = insert_header_to_grandpa_pallet::(state_root); diff --git a/modules/messages/src/tests/messages_generation.rs b/modules/messages/src/tests/messages_generation.rs index de0b54f0677..5b699a2e707 100644 --- a/modules/messages/src/tests/messages_generation.rs +++ b/modules/messages/src/tests/messages_generation.rs @@ -24,15 +24,13 @@ use bp_messages::{ MessagePayload, OutboundLaneData, }; use bp_runtime::{ - grow_trie_leaf_value, record_all_trie_keys, AccountIdOf, Chain, HashOf, HasherOf, - RangeInclusiveExt, StorageProofSize, UntrustedVecDb, + grow_trie_leaf_value, AccountIdOf, Chain, HashOf, HasherOf, RangeInclusiveExt, + StorageProofSize, UntrustedVecDb, }; use codec::Encode; use frame_support::StateVersion; use sp_std::{ops::RangeInclusive, prelude::*}; -use sp_trie::{ - LayoutV0, LayoutV1, MemoryDB, StorageProof, TrieConfiguration, TrieDBMutBuilder, TrieMut, -}; +use sp_trie::{LayoutV0, LayoutV1, MemoryDB, TrieConfiguration, TrieDBMutBuilder, TrieMut}; /// Dummy message generation function. pub fn generate_dummy_message(_: MessageNonce) -> MessagePayload { @@ -202,15 +200,10 @@ where } // generate storage proof to be delivered to This chain - let read_proof = record_all_trie_keys::(&mdb, &root) - .map_err(|_| "record_all_trie_keys has failed") - .expect("record_all_trie_keys should not fail in benchmarks"); - let storage = UntrustedVecDb::try_new::>( - StorageProof::new(read_proof), - root, - storage_keys, - ) - .unwrap(); + let storage = + UntrustedVecDb::try_from_db::, _>(&mdb, root, storage_keys) + .expect("UntrustedVecDb::try_from_db() should not fail in benchmarks"); + (root, storage) } @@ -240,14 +233,8 @@ where } // generate storage proof to be delivered to This chain - let read_proof = record_all_trie_keys::(&mdb, &root) - .map_err(|_| "record_all_trie_keys has failed") - .expect("record_all_trie_keys should not fail in benchmarks"); - let storage = UntrustedVecDb::try_new::>( - StorageProof::new(read_proof), - root, - vec![storage_key], - ) - .unwrap(); + let storage = + UntrustedVecDb::try_from_db::, _>(&mdb, root, vec![storage_key]) + .expect("UntrustedVecDb::try_from_db() should not fail in benchmarks"); (root, storage) } diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index ccac68b3744..534452ee249 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -36,10 +36,7 @@ pub use chain::{ }; pub use frame_support::storage::storage_prefix as storage_value_final_key; use num_traits::{CheckedAdd, CheckedSub, One, SaturatingAdd, Zero}; -pub use storage_proof::{ - grow_trie_leaf_value, record_all_keys as record_all_trie_keys, ProofSize as StorageProofSize, - RawStorageProof, -}; +pub use storage_proof::{grow_trie_leaf_value, ProofSize as StorageProofSize}; pub use storage_types::BoundedStorageValue; pub use vec_db::{TrustedVecDb, UntrustedVecDb, VecDbError}; diff --git a/primitives/runtime/src/storage_proof.rs b/primitives/runtime/src/storage_proof.rs index 256a9590655..2e160cf5c23 100644 --- a/primitives/runtime/src/storage_proof.rs +++ b/primitives/runtime/src/storage_proof.rs @@ -16,11 +16,7 @@ //! Additional logic for working with Substrate storage proofs. -use sp_std::{boxed::Box, collections::btree_set::BTreeSet, vec::Vec}; -use sp_trie::{Recorder, Trie, TrieConfiguration, TrieDBBuilder, TrieError, TrieHash}; - -/// Raw storage proof type (just raw trie nodes). -pub type RawStorageProof = Vec>; +use sp_std::vec::Vec; /// Storage proof size requirements. /// @@ -46,29 +42,3 @@ pub fn grow_trie_leaf_value(mut value: Vec, size: ProofSize) -> Vec { } value } - -/// Record all keys for a given root. -pub fn record_all_keys( - db: &DB, - root: &TrieHash, -) -> Result>> -where - DB: hash_db::HashDBRef, -{ - let mut recorder = Recorder::::new(); - let trie = TrieDBBuilder::::new(db, root).with_recorder(&mut recorder).build(); - for x in trie.iter()? { - let (key, _) = x?; - trie.get(&key)?; - } - - // recorder may record the same trie node multiple times and we don't want duplicate nodes - // in our proofs => let's deduplicate it by collecting to the BTreeSet first - Ok(recorder - .drain() - .into_iter() - .map(|n| n.data.to_vec()) - .collect::>() - .into_iter() - .collect()) -} diff --git a/primitives/runtime/src/vec_db.rs b/primitives/runtime/src/vec_db.rs index 81cecaa8a6f..9fe30fa5d14 100644 --- a/primitives/runtime/src/vec_db.rs +++ b/primitives/runtime/src/vec_db.rs @@ -19,7 +19,7 @@ use frame_support::{PalletError, StateVersion}; use sp_core::{storage::TrackedStorageKey, RuntimeDebug}; use sp_runtime::SaturatedConversion; -use sp_std::{default::Default, vec, vec::Vec}; +use sp_std::{collections::btree_set::BTreeSet, default::Default, vec, vec::Vec}; use sp_trie::{ generate_trie_proof, verify_trie_proof, LayoutV0, LayoutV1, MemoryDB, StorageProof, TrieDBBuilder, TrieHash, @@ -29,12 +29,12 @@ use codec::{Codec, Decode, Encode}; use hash_db::Hasher; use scale_info::TypeInfo; use sp_state_machine::TrieBackend; -use trie_db::{DBValue, Trie}; +use trie_db::{DBValue, Recorder, Trie}; -use crate::{ - storage_proof::{record_all_keys, RawStorageProof}, - Size, -}; +use crate::Size; + +/// Raw storage proof type (just raw trie nodes). +pub type RawStorageProof = Vec>; pub type RawStorageKey = Vec; @@ -95,12 +95,12 @@ impl UntrustedVecDb { /// Creates a new instance of `UntrustedVecDb` from the provided entries. /// - /// This function is used by the test logic. + /// **This function is used only in tests and benchmarks.** #[cfg(feature = "std")] pub fn try_from_entries( state_version: StateVersion, entries: &[(RawStorageKey, Option)], - ) -> Option<(H::Out, UntrustedVecDb)> + ) -> Result<(H::Out, UntrustedVecDb), VecDbError> where H::Out: Codec, { @@ -109,11 +109,40 @@ impl UntrustedVecDb { entries.iter().cloned().map(|(key, val)| (None, vec![(key, val)])).collect(); let backend = TrieBackend::, H>::from((entries, state_version)); let root = *backend.root(); - let read_proof = StorageProof::new( - record_all_keys::, _>(backend.backend_storage(), &root).ok()?, - ); - Some((root, UntrustedVecDb::try_new::(read_proof, root, keys).ok()?)) + Ok((root, UntrustedVecDb::try_from_db(backend.backend_storage(), root, keys)?)) + } + + /// Creates a new instance of `UntrustedVecDb` from the provided db. + /// + /// **This function is used only in tests and benchmarks.** + pub fn try_from_db( + db: &DB, + root: H::Out, + keys: Vec + Ord>, + ) -> Result + where + DB: hash_db::HashDBRef, + { + let mut recorder = Recorder::>::new(); + let trie = TrieDBBuilder::>::new(db, &root) + .with_recorder(&mut recorder) + .build(); + for key in &keys { + trie.get(key.as_ref()).map_err(|_| VecDbError::UnavailableKey)?; + } + + let raw_read_proof: Vec<_> = recorder + .drain() + .into_iter() + .map(|n| n.data) + // recorder may record the same trie node multiple times and we don't want duplicate + // nodes in our proofs => let's deduplicate it by collecting to the BTreeSet first + .collect::>() + .into_iter() + .collect(); + + UntrustedVecDb::try_new::(StorageProof::new(raw_read_proof), root, keys) } /// Validates the contained `db` against the contained proof. If the `db` is valid, converts it diff --git a/primitives/test-utils/src/lib.rs b/primitives/test-utils/src/lib.rs index 43391a4a2c9..1b0b1ae1723 100644 --- a/primitives/test-utils/src/lib.rs +++ b/primitives/test-utils/src/lib.rs @@ -21,12 +21,12 @@ use bp_header_chain::justification::{required_justification_precommits, GrandpaJustification}; use bp_parachains::parachain_head_storage_key_at_source; use bp_polkadot_core::parachains::{ParaHash, ParaHead, ParaHeadsProof, ParaId}; -use bp_runtime::{record_all_trie_keys, UntrustedVecDb}; +use bp_runtime::UntrustedVecDb; use codec::Encode; use sp_consensus_grandpa::{AuthorityId, AuthoritySignature, AuthorityWeight, SetId}; use sp_runtime::traits::{Header as HeaderT, One, Zero}; use sp_std::prelude::*; -use sp_trie::{trie_types::TrieDBMutBuilderV1, LayoutV1, MemoryDB, StorageProof, TrieMut}; +use sp_trie::{trie_types::TrieDBMutBuilderV1, MemoryDB, TrieMut}; // Re-export all our test account utilities pub use keyring::*; @@ -188,12 +188,8 @@ pub fn prepare_parachain_heads_proof( } // generate storage proof to be delivered to This chain - let read_proof = record_all_trie_keys::, _>(&mdb, &root) - .map_err(|_| "record_all_trie_keys has failed") - .expect("record_all_trie_keys should not fail in benchmarks"); - let storage_proof = - UntrustedVecDb::try_new::(StorageProof::new(read_proof), root, storage_keys) - .expect("UntrustedVecDb::try_new() should not fail in benchmarks"); + let storage_proof = UntrustedVecDb::try_from_db::(&mdb, root, storage_keys) + .expect("UntrustedVecDb::try_from_db() should not fail in benchmarks"); (root, ParaHeadsProof { storage_proof }, parachains) }