From ebed6c190ac8a47b858b8fe91653e64ae9b82a0f Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 7 Jun 2023 14:10:28 +0300 Subject: [PATCH] Use compact proofs for parachains (#2194) * Use compact proofs for parachains * Remove StorageProofChecker * Cleanup Reuse messages proof generation from messages pallet (#2195) * reuse messages proof generation from messages pallet * incorrect merge fixed remaining TODOs after messages pallet Config refactoring (#2196) Return UntrustedVecDb from prove_storage() (#2197) Storage proofs related renamings (#2198) * Storage proofs related renamings * Leftovers * StorageSize -> StorageProofSize --- .../extensions/refund_relayer_extension.rs | 6 +- bridges/bin/runtime-common/src/integrity.rs | 81 ++- .../src/messages_benchmarking.rs | 103 +--- .../src/parachains_benchmarking.rs | 51 +- bridges/modules/grandpa/src/lib.rs | 20 +- bridges/modules/messages/src/proofs.rs | 35 +- .../messages/src/tests/messages_generation.rs | 44 +- bridges/modules/parachains/src/call_ext.rs | 4 +- bridges/modules/parachains/src/lib.rs | 18 +- bridges/primitives/header-chain/src/lib.rs | 28 +- bridges/primitives/messages/src/lib.rs | 16 +- .../primitives/messages/src/source_chain.rs | 4 +- .../primitives/messages/src/target_chain.rs | 4 +- .../polkadot-core/src/parachains.rs | 11 +- bridges/primitives/runtime/src/lib.rs | 9 +- .../primitives/runtime/src/storage_proof.rs | 548 +++++++++++------- bridges/primitives/runtime/src/vec_db.rs | 332 ----------- bridges/primitives/test-utils/src/lib.rs | 12 +- .../client-substrate/src/client/caching.rs | 11 +- .../client-substrate/src/client/client.rs | 21 +- .../relays/client-substrate/src/client/rpc.rs | 29 +- bridges/relays/client-substrate/src/error.rs | 3 - .../src/messages/source.rs | 14 +- .../src/messages/target.rs | 12 +- .../src/parachains/source.rs | 11 +- .../relays/parachains/src/parachains_loop.rs | 4 +- 26 files changed, 603 insertions(+), 828 deletions(-) delete mode 100644 bridges/primitives/runtime/src/vec_db.rs diff --git a/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs b/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs index 6a982bd7e576f..24b9ce9b34c28 100644 --- a/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs +++ b/bridges/bin/runtime-common/src/extensions/refund_relayer_extension.rs @@ -1121,7 +1121,7 @@ pub(crate) mod tests { ParaId(BridgedUnderlyingParachain::PARACHAIN_ID), [parachain_head_at_relay_header_number as u8; 32].into(), )], - parachain_heads_proof: ParaHeadsProof { storage_proof: vec![] }, + parachain_heads_proof: ParaHeadsProof { storage_proof: Default::default() }, }) } @@ -1134,7 +1134,7 @@ pub(crate) mod tests { ParaId(BridgedUnderlyingParachain::PARACHAIN_ID), [parachain_head_at_relay_header_number as u8; 32].into(), )], - parachain_heads_proof: ParaHeadsProof { storage_proof: vec![] }, + parachain_heads_proof: ParaHeadsProof { storage_proof: Default::default() }, is_free_execution_expected: false, }) } @@ -2111,7 +2111,7 @@ pub(crate) mod tests { [1u8; 32].into(), ), ], - parachain_heads_proof: ParaHeadsProof { storage_proof: vec![] }, + parachain_heads_proof: ParaHeadsProof { storage_proof: Default::default() }, }), message_delivery_call(200), ], diff --git a/bridges/bin/runtime-common/src/integrity.rs b/bridges/bin/runtime-common/src/integrity.rs index 9b41a4bf4902d..3ea5222d2329c 100644 --- a/bridges/bin/runtime-common/src/integrity.rs +++ b/bridges/bin/runtime-common/src/integrity.rs @@ -19,7 +19,9 @@ //! Most of the tests in this module assume that the bridge is using standard (see `crate::messages` //! module for details) configuration. +use bp_header_chain::ChainWithGrandpa; use bp_messages::{ChainWithMessages, InboundLaneData, MessageNonce}; +use bp_runtime::Chain; use codec::Encode; use frame_support::{storage::generator::StorageValue, traits::Get, weights::Weight}; use frame_system::limits; @@ -54,19 +56,24 @@ macro_rules! assert_bridge_messages_pallet_types( ( runtime: $r:path, with_bridged_chain_messages_instance: $i:path, + this_chain: $this:path, + bridged_chain: $bridged:path, ) => { { // if one of asserts fail, then either bridge isn't configured properly (or alternatively - non-standard // configuration is used), or something has broke existing configuration (meaning that all bridged chains // and relays will stop functioning) use $crate::messages_xcm_extension::XcmAsPlainPayload; + use bp_messages::ChainWithMessages; + use bp_runtime::Chain; use pallet_bridge_messages::Config as MessagesConfig; use static_assertions::assert_type_eq_all; - assert_type_eq_all!(<$r as MessagesConfig<$i>>::OutboundPayload, XcmAsPlainPayload); + assert_type_eq_all!(<$r as MessagesConfig<$i>>::ThisChain, $this); + assert_type_eq_all!(<$r as MessagesConfig<$i>>::BridgedChain, $bridged); - // TODO: https://github.com/paritytech/parity-bridges-common/issues/1666: check ThisChain, BridgedChain - // and BridgedHeaderChain types + assert_type_eq_all!(<$r as MessagesConfig<$i>>::OutboundPayload, XcmAsPlainPayload); + assert_type_eq_all!(<$r as MessagesConfig<$i>>::InboundPayload, XcmAsPlainPayload); } } ); @@ -88,6 +95,8 @@ macro_rules! assert_complete_bridge_types( $crate::assert_bridge_messages_pallet_types!( runtime: $r, with_bridged_chain_messages_instance: $mi, + this_chain: $this, + bridged_chain: $bridged, ); } ); @@ -161,14 +170,22 @@ where "ActiveOutboundLanes ({:?}) must not be empty", R::ActiveOutboundLanes::get(), ); + + assert!( + pallet_bridge_messages::BridgedChainOf::::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX + <= pallet_bridge_messages::BridgedChainOf::::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX, + "MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX ({}) of {:?} is larger than \ + its MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX ({}). This makes \ + no sense", + pallet_bridge_messages::BridgedChainOf::::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX, + pallet_bridge_messages::BridgedChainOf::::ID, + pallet_bridge_messages::BridgedChainOf::::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX, + ); } /// Parameters for asserting bridge pallet names. #[derive(Debug)] pub struct AssertBridgePalletNames<'a> { - /// Name of the messages pallet, deployed at the bridged chain and used to bridge with this - /// chain. - pub with_this_chain_messages_pallet_name: &'a str, /// Name of the GRANDPA pallet, deployed at this chain and used to bridge with the bridged /// chain. pub with_bridged_chain_grandpa_pallet_name: &'a str, @@ -179,16 +196,22 @@ pub struct AssertBridgePalletNames<'a> { /// Tests that bridge pallet names used in `construct_runtime!()` macro call are matching constants /// from chain primitives crates. -pub fn assert_bridge_pallet_names(params: AssertBridgePalletNames) +fn assert_bridge_pallet_names(params: AssertBridgePalletNames) where R: pallet_bridge_grandpa::Config + pallet_bridge_messages::Config, GI: 'static, MI: 'static, { + // check that the bridge GRANDPA pallet has required name assert_eq!( pallet_bridge_grandpa::PalletOwner::::storage_value_final_key().to_vec(), - bp_runtime::storage_value_key(params.with_bridged_chain_grandpa_pallet_name, "PalletOwner",).0, + bp_runtime::storage_value_key( + params.with_bridged_chain_grandpa_pallet_name, + "PalletOwner", + ).0, ); + + // check that the bridge messages pallet has required name assert_eq!( pallet_bridge_messages::PalletOwner::::storage_value_final_key().to_vec(), bp_runtime::storage_value_key( @@ -201,27 +224,53 @@ where /// Parameters for asserting complete standard messages bridge. #[derive(Debug)] -pub struct AssertCompleteBridgeConstants<'a> { +pub struct AssertCompleteBridgeConstants { /// Parameters to assert this chain constants. pub this_chain_constants: AssertChainConstants, - /// Parameters to assert pallet names constants. - pub pallet_names: AssertBridgePalletNames<'a>, } -/// All bridge-related constants tests for the complete standard messages bridge (i.e. with bridge -/// GRANDPA and messages pallets deployed). -pub fn assert_complete_bridge_constants(params: AssertCompleteBridgeConstants) -where +/// All bridge-related constants tests for the complete standard relay-chain messages bridge +/// (i.e. with bridge GRANDPA and messages pallets deployed). +pub fn assert_complete_with_relay_chain_bridge_constants( + params: AssertCompleteBridgeConstants, +) where + R: frame_system::Config + + pallet_bridge_grandpa::Config + + pallet_bridge_messages::Config, + GI: 'static, + MI: 'static, +{ + assert_chain_constants::(params.this_chain_constants); + assert_bridge_grandpa_pallet_constants::(); + assert_bridge_messages_pallet_constants::(); + assert_bridge_pallet_names::(AssertBridgePalletNames { + with_bridged_chain_grandpa_pallet_name: + >::BridgedChain::WITH_CHAIN_GRANDPA_PALLET_NAME, + with_bridged_chain_messages_pallet_name: + >::BridgedChain::WITH_CHAIN_MESSAGES_PALLET_NAME, + }); +} + +/// All bridge-related constants tests for the complete standard parachain messages bridge +/// (i.e. with bridge GRANDPA, parachains and messages pallets deployed). +pub fn assert_complete_with_parachain_bridge_constants( + params: AssertCompleteBridgeConstants, +) where R: frame_system::Config + pallet_bridge_grandpa::Config + pallet_bridge_messages::Config, GI: 'static, MI: 'static, + RelayChain: ChainWithGrandpa, { assert_chain_constants::(params.this_chain_constants); assert_bridge_grandpa_pallet_constants::(); assert_bridge_messages_pallet_constants::(); - assert_bridge_pallet_names::(params.pallet_names); + assert_bridge_pallet_names::(AssertBridgePalletNames { + with_bridged_chain_grandpa_pallet_name: RelayChain::WITH_CHAIN_GRANDPA_PALLET_NAME, + with_bridged_chain_messages_pallet_name: + >::BridgedChain::WITH_CHAIN_MESSAGES_PALLET_NAME, + }); } /// Check that the message lane weights are correct. diff --git a/bridges/bin/runtime-common/src/messages_benchmarking.rs b/bridges/bin/runtime-common/src/messages_benchmarking.rs index 08d103ee32aee..b339605554dac 100644 --- a/bridges/bin/runtime-common/src/messages_benchmarking.rs +++ b/bridges/bin/runtime-common/src/messages_benchmarking.rs @@ -19,42 +19,24 @@ #![cfg(feature = "runtime-benchmarks")] -// use crate::{ -// messages::{BridgedChain, HashOf, ThisChain}, -// messages_generation::{ -// encode_all_messages, encode_lane_data, prepare_message_delivery_storage_proof, -// prepare_messages_storage_proof, -// }, -// }; - use bp_messages::{ source_chain::FromBridgedChainMessagesDeliveryProof, target_chain::FromBridgedChainMessagesProof, MessagePayload, }; use bp_polkadot_core::parachains::ParaHash; -use bp_runtime::{Chain, Parachain, StorageProofSize}; -// TODO: https://github.com/paritytech/parity-bridges-common/issues/1666 - merge with message -// generation methods from messages pallet and use it here - -use bp_messages::{storage_keys, ChainWithMessages}; -use bp_runtime::{ - grow_trie_leaf_value, record_all_trie_keys, AccountIdOf, HashOf, HasherOf, UntrustedVecDb, -}; +use bp_runtime::{AccountIdOf, Chain, HashOf, Parachain, StorageProofSize}; use codec::Encode; use frame_support::weights::Weight; use pallet_bridge_messages::{ benchmarking::{MessageDeliveryProofParams, MessageProofParams}, - messages_generation::{encode_all_messages, encode_lane_data, prepare_messages_storage_proof}, + messages_generation::{ + encode_all_messages, encode_lane_data, prepare_message_delivery_storage_proof, + prepare_messages_storage_proof, + }, BridgedChainOf, ThisChainOf, }; -use sp_runtime::{ - traits::{Header, Zero}, - StateVersion, -}; +use sp_runtime::traits::{Header, Zero}; use sp_std::prelude::*; -use sp_trie::{ - LayoutV0, LayoutV1, MemoryDB, StorageProof, TrieConfiguration, TrieDBMutBuilder, TrieMut, -}; use xcm::latest::prelude::*; /// Prepare inbound bridge message according to given message proof parameters. @@ -201,7 +183,10 @@ where { // prepare storage proof let lane = params.lane; - let (state_root, storage_proof) = prepare_message_delivery_proof::(params); + let (state_root, storage_proof) = prepare_message_delivery_storage_proof::< + BridgedChainOf, + ThisChainOf, + >(params.lane, params.inbound_lane_data, params.size); // update runtime storage let (_, bridged_header_hash) = insert_header_to_grandpa_pallet::(state_root); @@ -229,7 +214,10 @@ where { // prepare storage proof let lane = params.lane; - let (state_root, storage_proof) = prepare_message_delivery_proof::(params); + let (state_root, storage_proof) = prepare_message_delivery_storage_proof::< + BridgedChainOf, + ThisChainOf, + >(params.lane, params.inbound_lane_data, params.size); // update runtime storage let (_, bridged_header_hash) = @@ -242,69 +230,6 @@ where } } -/// Prepare in-memory message delivery proof, without inserting anything to the runtime storage. -fn prepare_message_delivery_proof( - params: MessageDeliveryProofParams>>, -) -> (HashOf>, UntrustedVecDb) -where - R: pallet_bridge_messages::Config, - MI: 'static, -{ - match BridgedChainOf::::STATE_VERSION { - StateVersion::V0 => - do_prepare_message_delivery_proof::>>>( - params, - ), - StateVersion::V1 => - do_prepare_message_delivery_proof::>>>( - params, - ), - } -} - -/// Prepare in-memory message delivery proof, without inserting anything to the runtime storage. -fn do_prepare_message_delivery_proof< - R, - MI, - L: TrieConfiguration>>, ->( - params: MessageDeliveryProofParams>>, -) -> (HashOf>, UntrustedVecDb) -where - R: pallet_bridge_messages::Config, - MI: 'static, -{ - // prepare Bridged chain storage with inbound lane state - let storage_key = storage_keys::inbound_lane_data_key( - R::ThisChain::WITH_CHAIN_MESSAGES_PALLET_NAME, - ¶ms.lane, - ) - .0; - let mut root = Default::default(); - let mut mdb = MemoryDB::default(); - { - let mut trie = TrieDBMutBuilder::::new(&mut mdb, &mut root).build(); - let inbound_lane_data = - grow_trie_leaf_value(params.inbound_lane_data.encode(), params.size); - trie.insert(&storage_key, &inbound_lane_data) - .map_err(|_| "TrieMut::insert has failed") - .expect("TrieMut::insert should not fail in benchmarks"); - } - - // 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, - vec![storage_key], - ) - .unwrap(); - - (root, storage_proof) -} - /// Insert header to the bridge GRANDPA pallet. pub(crate) fn insert_header_to_grandpa_pallet( state_root: bp_runtime::HashOf, diff --git a/bridges/bin/runtime-common/src/parachains_benchmarking.rs b/bridges/bin/runtime-common/src/parachains_benchmarking.rs index 7f43f78b719fc..0456535517063 100644 --- a/bridges/bin/runtime-common/src/parachains_benchmarking.rs +++ b/bridges/bin/runtime-common/src/parachains_benchmarking.rs @@ -22,12 +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, StorageProofSize}; +use bp_runtime::{grow_storage_value, Chain, StorageProofSize, UnverifiedStorageProof}; use codec::Encode; -use frame_support::traits::Get; +use frame_support::{sp_runtime::StateVersion, traits::Get}; +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, TrieConfiguration, TrieDBMutBuilder, TrieMut}; /// Prepare proof of messages for the `receive_messages_proof` call. /// @@ -43,7 +44,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, + ), + } +} + +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,33 +81,32 @@ 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() { let storage_key = parachain_head_storage_key_at_source(R::ParasPalletName::get(), *parachain); let leaf_data = if i == 0 { - grow_trie_leaf_value(parachain_head.encode(), size) + grow_storage_value(parachain_head.encode(), size) } else { parachain_head.encode() }; 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) - .map_err(|_| "record_all_trie_keys has failed") - .expect("record_all_trie_keys should not fail in benchmarks"); + let storage_proof = + UnverifiedStorageProof::try_from_db::(&mdb, state_root, storage_keys) + .expect("UnverifiedStorageProof::try_from_db() 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 { storage_proof: proof }, parachain_heads) + (relay_block_number, relay_block_hash, ParaHeadsProof { storage_proof }, parachain_heads) } diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index 3b77f676870e1..5a74442ae1bfe 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -834,7 +834,7 @@ mod tests { System, TestBridgedChain, TestHeader, TestNumber, TestRuntime, MAX_BRIDGED_AUTHORITIES, }; use bp_header_chain::BridgeGrandpaCall; - use bp_runtime::BasicOperatingMode; + use bp_runtime::{BasicOperatingMode, UnverifiedStorageProof}; use bp_test_utils::{ authority_list, generate_owned_bridge_module_tests, make_default_justification, make_justification_for_header, JustificationGeneratorParams, ALICE, BOB, @@ -1443,11 +1443,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_storage_proof( + Default::default(), + Default::default(), + ) + .map(|_| ()), bp_header_chain::HeaderChainError::UnknownHeader, ); }); @@ -1456,7 +1459,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) = UnverifiedStorageProof::try_from_entries::< + sp_core::Blake2Hasher, + >(Default::default(), &[(b"key1".to_vec(), None)]) + .expect("UnverifiedStorageProof::try_from_entries() shouldn't fail in tests"); let mut header = test_header(2); header.set_state_root(state_root); @@ -1465,9 +1471,7 @@ mod tests { >::put(HeaderId(2, hash)); >::insert(hash, header.build()); - assert_ok!( - Pallet::::storage_proof_checker(hash, storage_proof).map(|_| ()) - ); + assert_ok!(Pallet::::verify_storage_proof(hash, storage_proof).map(|_| ())); }); } diff --git a/bridges/modules/messages/src/proofs.rs b/bridges/modules/messages/src/proofs.rs index 9d331d846d8cf..f2bf1d1b08211 100644 --- a/bridges/modules/messages/src/proofs.rs +++ b/bridges/modules/messages/src/proofs.rs @@ -25,7 +25,7 @@ use bp_messages::{ ChainWithMessages, InboundLaneData, LaneId, Message, MessageKey, MessageNonce, MessagePayload, OutboundLaneData, VerificationError, }; -use bp_runtime::{HashOf, RangeInclusiveExt, TrustedVecDb}; +use bp_runtime::{HashOf, RangeInclusiveExt, VerifiedStorageProof}; use sp_std::vec::Vec; /// 'Parsed' message delivery proof - inbound lane id and its state. @@ -51,7 +51,7 @@ pub fn verify_messages_proof, I: 'static>( nonces_start, nonces_end, } = proof; - let storage = BridgedHeaderChainOf::::verify_vec_db_storage(bridged_header_hash, storage) + let storage = BridgedHeaderChainOf::::verify_storage_proof(bridged_header_hash, storage) .map_err(VerificationError::HeaderChain)?; let mut parser = StorageAdapter:: { storage, _dummy: Default::default() }; let nonces_range = nonces_start..=nonces_end; @@ -85,8 +85,11 @@ pub fn verify_messages_proof, I: 'static>( return Err(VerificationError::EmptyMessageProof) } - // Check that the `VecDb` doesn't have any untouched keys. - parser.storage.ensure_no_unused_keys().map_err(VerificationError::VecDb)?; + // Check that the storage proof doesn't have any untouched keys. + parser + .storage + .ensure_no_unused_keys() + .map_err(VerificationError::StorageProof)?; // We only support single lane messages in this generated_schema let mut proved_messages = ProvedMessages::new(); @@ -101,7 +104,7 @@ pub fn verify_messages_delivery_proof, I: 'static>( ) -> Result, VerificationError> { let FromBridgedChainMessagesDeliveryProof { bridged_header_hash, storage_proof, lane } = proof; let mut storage = - T::BridgedHeaderChain::verify_vec_db_storage(bridged_header_hash, storage_proof) + T::BridgedHeaderChain::verify_storage_proof(bridged_header_hash, storage_proof) .map_err(VerificationError::HeaderChain)?; // Messages delivery proof is just proof of single storage key read => any error // is fatal. @@ -114,13 +117,13 @@ pub fn verify_messages_delivery_proof, I: 'static>( .map_err(VerificationError::InboundLaneStorage)?; // check that the storage proof doesn't have any untouched trie nodes - storage.ensure_no_unused_keys().map_err(VerificationError::VecDb)?; + storage.ensure_no_unused_keys().map_err(VerificationError::StorageProof)?; Ok((lane, inbound_lane_data)) } struct StorageAdapter { - storage: TrustedVecDb, + storage: VerifiedStorageProof, _dummy: sp_std::marker::PhantomData<(T, I)>, } @@ -166,7 +169,7 @@ mod tests { }; use bp_header_chain::{HeaderChainError, StoredHeaderDataBuilder}; - use bp_runtime::{HeaderId, VecDbError}; + use bp_runtime::{HeaderId, StorageProofError}; use codec::Encode; use sp_runtime::traits::Header; @@ -301,7 +304,9 @@ mod tests { verify_messages_proof::(proof, 10) } ), - Err(VerificationError::HeaderChain(HeaderChainError::VecDb(VecDbError::InvalidProof))), + Err(VerificationError::HeaderChain(HeaderChainError::StorageProof( + StorageProofError::InvalidProof + ))), ); } @@ -317,7 +322,9 @@ mod tests { false, |proof| { verify_messages_proof::(proof, 10) }, ), - Err(VerificationError::HeaderChain(HeaderChainError::VecDb(VecDbError::InvalidProof))), + Err(VerificationError::HeaderChain(HeaderChainError::StorageProof( + StorageProofError::InvalidProof + ))), ); } @@ -333,7 +340,7 @@ mod tests { true, |proof| { verify_messages_proof::(proof, 10) }, ), - Err(VerificationError::VecDb(VecDbError::UnusedKey)), + Err(VerificationError::StorageProof(StorageProofError::UnusedKey)), ); } @@ -349,7 +356,7 @@ mod tests { false, |proof| verify_messages_proof::(proof, 10) ), - Err(VerificationError::MessageStorage(VecDbError::EmptyVal)), + Err(VerificationError::MessageStorage(StorageProofError::EmptyVal)), ); } @@ -371,7 +378,7 @@ mod tests { false, |proof| verify_messages_proof::(proof, 10), ), - Err(VerificationError::MessageStorage(VecDbError::DecodeError)), + Err(VerificationError::MessageStorage(StorageProofError::DecodeError)), ); } @@ -395,7 +402,7 @@ mod tests { false, |proof| verify_messages_proof::(proof, 10), ), - Err(VerificationError::OutboundLaneStorage(VecDbError::DecodeError)), + Err(VerificationError::OutboundLaneStorage(StorageProofError::DecodeError)), ); } diff --git a/bridges/modules/messages/src/tests/messages_generation.rs b/bridges/modules/messages/src/tests/messages_generation.rs index 6d0969004f7e2..56df19cf88094 100644 --- a/bridges/modules/messages/src/tests/messages_generation.rs +++ b/bridges/modules/messages/src/tests/messages_generation.rs @@ -15,23 +15,19 @@ // along with Parity Bridges Common. If not, see . //! Helpers for generating message storage proofs, that are used by tests and by benchmarks. -// TODO: remove me in https://github.com/paritytech/parity-bridges-common/issues/1666 -#![allow(dead_code)] use bp_messages::{ storage_keys, ChainWithMessages, InboundLaneData, LaneId, MessageKey, MessageNonce, MessagePayload, OutboundLaneData, }; use bp_runtime::{ - grow_trie_leaf_value, record_all_trie_keys, AccountIdOf, Chain, HashOf, HasherOf, - RangeInclusiveExt, StorageProofSize, UntrustedVecDb, + grow_storage_value, AccountIdOf, Chain, HashOf, HasherOf, RangeInclusiveExt, StorageProofSize, + UnverifiedStorageProof, }; use codec::Encode; use frame_support::sp_runtime::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 { @@ -62,7 +58,7 @@ pub fn prepare_messages_storage_proof Vec, add_duplicate_key: bool, add_unused_key: bool, -) -> (HashOf, UntrustedVecDb) +) -> (HashOf, UnverifiedStorageProof) where HashOf: Copy + Default, { @@ -105,7 +101,7 @@ pub fn prepare_message_delivery_storage_proof>, size: StorageProofSize, -) -> (HashOf, UntrustedVecDb) +) -> (HashOf, UnverifiedStorageProof) where HashOf: Copy + Default, { @@ -137,7 +133,7 @@ fn do_prepare_messages_storage_proof Vec, add_duplicate_key: bool, add_unused_key: bool, -) -> (HashOf, UntrustedVecDb) +) -> (HashOf, UnverifiedStorageProof) where L: TrieConfiguration>, HashOf: Copy + Default, @@ -156,7 +152,7 @@ where let message_payload = match encode_message(nonce, &generate_message(nonce)) { Some(message_payload) => if i == 0 { - grow_trie_leaf_value(message_payload, size) + grow_storage_value(message_payload, size) } else { message_payload }, @@ -203,15 +199,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 = + UnverifiedStorageProof::try_from_db::, _>(&mdb, root, storage_keys) + .expect("UnverifiedStorageProof::try_from_db() should not fail in benchmarks"); + (root, storage) } @@ -222,7 +213,7 @@ fn do_prepare_message_delivery_storage_proof>, size: StorageProofSize, -) -> (HashOf, UntrustedVecDb) +) -> (HashOf, UnverifiedStorageProof) where L: TrieConfiguration>, HashOf: Copy + Default, @@ -234,21 +225,18 @@ where let mut mdb = MemoryDB::default(); { let mut trie = TrieDBMutBuilder::::new(&mut mdb, &mut root).build(); - let inbound_lane_data = grow_trie_leaf_value(inbound_lane_data.encode(), size); + let inbound_lane_data = grow_storage_value(inbound_lane_data.encode(), size); trie.insert(&storage_key, &inbound_lane_data) .map_err(|_| "TrieMut::insert has failed") .expect("TrieMut::insert should not fail in benchmarks"); } // 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), + let storage = UnverifiedStorageProof::try_from_db::, _>( + &mdb, root, vec![storage_key], ) - .unwrap(); + .expect("UnverifiedStorageProof::try_from_db() should not fail in benchmarks"); (root, storage) } diff --git a/bridges/modules/parachains/src/call_ext.rs b/bridges/modules/parachains/src/call_ext.rs index fe6b319205d41..0f77eaf2c5a93 100644 --- a/bridges/modules/parachains/src/call_ext.rs +++ b/bridges/modules/parachains/src/call_ext.rs @@ -289,7 +289,7 @@ mod tests { RuntimeCall::Parachains(crate::Call::::submit_parachain_heads_ex { at_relay_block: (num, [num as u8; 32].into()), parachains, - parachain_heads_proof: ParaHeadsProof { storage_proof: Vec::new() }, + parachain_heads_proof: ParaHeadsProof { storage_proof: Default::default() }, is_free_execution_expected: false, }) .check_obsolete_submit_parachain_heads() @@ -303,7 +303,7 @@ mod tests { RuntimeCall::Parachains(crate::Call::::submit_parachain_heads_ex { at_relay_block: (num, [num as u8; 32].into()), parachains, - parachain_heads_proof: ParaHeadsProof { storage_proof: Vec::new() }, + parachain_heads_proof: ParaHeadsProof { storage_proof: Default::default() }, is_free_execution_expected: true, }) .check_obsolete_submit_parachain_heads() diff --git a/bridges/modules/parachains/src/lib.rs b/bridges/modules/parachains/src/lib.rs index d323aef3b2207..c228cc40571ab 100644 --- a/bridges/modules/parachains/src/lib.rs +++ b/bridges/modules/parachains/src/lib.rs @@ -30,7 +30,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 pallet_bridge_grandpa::SubmitFinalityProofHelper; use sp_std::{marker::PhantomData, vec::Vec}; @@ -83,7 +83,7 @@ pub mod pallet { }; use bp_runtime::{ BasicOperatingMode, BoundedStorageValue, OwnedBridgeModule, StorageDoubleMapKeyProvider, - StorageMapKeyProvider, + StorageMapKeyProvider, StorageProofError, VerifiedStorageProof, }; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; @@ -448,8 +448,7 @@ pub mod pallet { parachains.len() as _, ); - let mut is_updated_something = false; - let mut storage = GrandpaPalletOf::::storage_proof_checker( + let mut storage = GrandpaPalletOf::::verify_storage_proof( relay_block_hash, parachain_heads_proof.storage_proof, ) @@ -541,7 +540,6 @@ pub mod pallet { parachain_head_hash, )?; - is_updated_something = true; if is_free { free_parachain_heads = free_parachain_heads + 1; } @@ -572,7 +570,7 @@ 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| { + storage.ensure_no_unused_keys().map_err(|e| { Error::::HeaderChainStorageProof(HeaderChainError::StorageProof(e)) })?; @@ -635,12 +633,12 @@ pub mod pallet { /// Read parachain head from storage proof. fn read_parachain_head( - storage: &mut bp_runtime::StorageProofChecker, + storage: &mut VerifiedStorageProof, parachain: ParaId, ) -> Result, StorageProofError> { 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. @@ -846,7 +844,7 @@ pub(crate) mod tests { }; use bp_runtime::{ BasicOperatingMode, OwnedBridgeModuleError, StorageDoubleMapKeyProvider, - StorageMapKeyProvider, + StorageMapKeyProvider, StorageProofError, }; use bp_test_utils::{ authority_list, generate_owned_bridge_module_tests, make_default_justification, @@ -1579,7 +1577,7 @@ pub(crate) mod tests { assert_noop!( import_parachain_1_head(0, Default::default(), parachains, proof), Error::::HeaderChainStorageProof(HeaderChainError::StorageProof( - StorageProofError::StorageRootMismatch + StorageProofError::InvalidProof )) ); }); diff --git a/bridges/primitives/header-chain/src/lib.rs b/bridges/primitives/header-chain/src/lib.rs index 22b70d40c1d5e..08d7bcd64f3e9 100644 --- a/bridges/primitives/header-chain/src/lib.rs +++ b/bridges/primitives/header-chain/src/lib.rs @@ -24,9 +24,8 @@ use crate::justification::{ GrandpaJustification, JustificationVerificationContext, JustificationVerificationError, }; use bp_runtime::{ - BasicOperatingMode, BlockNumberOf, Chain, HashOf, HasherOf, HeaderOf, RawStorageProof, - StorageProofChecker, StorageProofError, TrustedVecDb, UnderlyingChainProvider, UntrustedVecDb, - VecDbError, + BasicOperatingMode, BlockNumberOf, Chain, HashOf, HasherOf, HeaderOf, StorageProofError, + UnderlyingChainProvider, UnverifiedStorageProof, VerifiedStorageProof, }; use codec::{Codec, Decode, Encode, EncodeLike, MaxEncodedLen}; use core::{clone::Clone, cmp::Eq, default::Default, fmt::Debug}; @@ -47,10 +46,8 @@ pub mod storage_keys; pub enum HeaderChainError { /// Header with given hash is missing from the chain. UnknownHeader, - /// Storage proof related error. + /// Error generated by the `storage_proof` module. StorageProof(StorageProofError), - /// Error generated by the `vec_db` module. - VecDb(VecDbError), } /// Header data that we're storing on-chain. @@ -81,24 +78,15 @@ impl StoredHeaderDataBuilder for H { pub trait HeaderChain { /// Returns state (storage) root of given finalized header. fn finalized_header_state_root(header_hash: HashOf) -> Option>; - /// Get a `TrustedVecDb` starting from an `UntrustedVecDb`. - fn verify_vec_db_storage( + /// Get a `VerifiedStorageProof` starting from an `UnverifiedStorageProof`. + fn verify_storage_proof( header_hash: HashOf, - db: UntrustedVecDb, - ) -> Result { + db: UnverifiedStorageProof, + ) -> Result { let state_root = Self::finalized_header_state_root(header_hash) .ok_or(HeaderChainError::UnknownHeader)?; 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) + .map_err(HeaderChainError::StorageProof) } } diff --git a/bridges/primitives/messages/src/lib.rs b/bridges/primitives/messages/src/lib.rs index 740e7c1dea09e..e6540dc33af6b 100644 --- a/bridges/primitives/messages/src/lib.rs +++ b/bridges/primitives/messages/src/lib.rs @@ -22,7 +22,7 @@ use bp_header_chain::HeaderChainError; use bp_runtime::{ messages::MessageDispatchResult, AccountIdOf, BasicOperatingMode, Chain, HashOf, OperatingMode, - RangeInclusiveExt, UnderlyingChainOf, UnderlyingChainProvider, VecDbError, + RangeInclusiveExt, StorageProofError, UnderlyingChainOf, UnderlyingChainProvider, }; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::PalletError; @@ -533,19 +533,19 @@ pub enum VerificationError { /// Error returned by the bridged header chain. HeaderChain(HeaderChainError), /// Error returned while reading/decoding inbound lane data from the storage proof. - InboundLaneStorage(VecDbError), + InboundLaneStorage(StorageProofError), /// The declared message weight is incorrect. InvalidMessageWeight, /// Declared messages count doesn't match actual value. MessagesCountMismatch, - /// Error returned while reading/decoding message data from the `VecDb`. - MessageStorage(VecDbError), + /// Error returned while reading/decoding message data from the `VerifiedStorageProof`. + MessageStorage(StorageProofError), /// The message is too large. MessageTooLarge, - /// Error returned while reading/decoding outbound lane data from the `VecDb`. - OutboundLaneStorage(VecDbError), - /// `VecDb` related error. - VecDb(VecDbError), + /// Error returned while reading/decoding outbound lane data from the `VerifiedStorageProof`. + OutboundLaneStorage(StorageProofError), + /// Storage proof related error. + StorageProof(StorageProofError), /// Custom error Other(#[codec(skip)] &'static str), } diff --git a/bridges/primitives/messages/src/source_chain.rs b/bridges/primitives/messages/src/source_chain.rs index 281ea77151f9f..881052b8277a3 100644 --- a/bridges/primitives/messages/src/source_chain.rs +++ b/bridges/primitives/messages/src/source_chain.rs @@ -18,7 +18,7 @@ use crate::{LaneId, MessageNonce, UnrewardedRelayer}; -use bp_runtime::{Size, UntrustedVecDb}; +use bp_runtime::{Size, UnverifiedStorageProof}; use codec::{Decode, Encode}; use scale_info::TypeInfo; use sp_core::RuntimeDebug; @@ -43,7 +43,7 @@ pub struct FromBridgedChainMessagesDeliveryProof { /// Hash of the bridge header the proof is for. pub bridged_header_hash: BridgedHeaderHash, /// Storage trie proof generated for [`Self::bridged_header_hash`]. - pub storage_proof: UntrustedVecDb, + pub storage_proof: UnverifiedStorageProof, /// Lane id of which messages were delivered and the proof is for. pub lane: LaneId, } diff --git a/bridges/primitives/messages/src/target_chain.rs b/bridges/primitives/messages/src/target_chain.rs index 7f678cd13a0fe..974cb38cbc0a4 100644 --- a/bridges/primitives/messages/src/target_chain.rs +++ b/bridges/primitives/messages/src/target_chain.rs @@ -18,7 +18,7 @@ use crate::{LaneId, Message, MessageKey, MessageNonce, MessagePayload, OutboundLaneData}; -use bp_runtime::{messages::MessageDispatchResult, Size, UntrustedVecDb}; +use bp_runtime::{messages::MessageDispatchResult, Size, UnverifiedStorageProof}; use codec::{Decode, Encode, Error as CodecError}; use frame_support::weights::Weight; use scale_info::TypeInfo; @@ -42,7 +42,7 @@ pub struct FromBridgedChainMessagesProof { /// Hash of the finalized bridged header the proof is for. pub bridged_header_hash: BridgedHeaderHash, /// The proved storage containing the messages being delivered. - pub storage: UntrustedVecDb, + pub storage: UnverifiedStorageProof, /// Messages in this proof are sent over this lane. pub lane: LaneId, /// Nonce of the first message being delivered. diff --git a/bridges/primitives/polkadot-core/src/parachains.rs b/bridges/primitives/polkadot-core/src/parachains.rs index 433cd2845abd9..87d706a1f4fa3 100644 --- a/bridges/primitives/polkadot-core/src/parachains.rs +++ b/bridges/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, UnverifiedStorageProof}; use codec::{CompactAs, Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; use sp_core::Hasher; @@ -91,16 +91,11 @@ pub type ParaHasher = crate::Hasher; #[derive(Clone, Decode, Encode, Eq, PartialEq, RuntimeDebug, TypeInfo)] pub struct ParaHeadsProof { /// Unverified storage proof of finalized parachain heads. - pub storage_proof: RawStorageProof, + pub storage_proof: UnverifiedStorageProof, } impl Size for ParaHeadsProof { fn size(&self) -> u32 { - u32::try_from( - self.storage_proof - .iter() - .fold(0usize, |sum, node| sum.saturating_add(node.len())), - ) - .unwrap_or(u32::MAX) + self.storage_proof.size() } } diff --git a/bridges/primitives/runtime/src/lib.rs b/bridges/primitives/runtime/src/lib.rs index b9d29fe06b20d..592242030b253 100644 --- a/bridges/primitives/runtime/src/lib.rs +++ b/bridges/primitives/runtime/src/lib.rs @@ -41,14 +41,10 @@ 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_storage_value, StorageProofError, StorageProofSize, UnverifiedStorageProof, + VerifiedStorageProof, }; 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; @@ -56,7 +52,6 @@ pub mod messages; mod chain; mod storage_proof; mod storage_types; -mod vec_db; // Re-export macro to avoid include paste dependency everywhere pub use sp_runtime::paste; diff --git a/bridges/primitives/runtime/src/storage_proof.rs b/bridges/primitives/runtime/src/storage_proof.rs index 3b1b31e7012d6..abc5a8d86544d 100644 --- a/bridges/primitives/runtime/src/storage_proof.rs +++ b/bridges/primitives/runtime/src/storage_proof.rs @@ -14,271 +14,399 @@ // 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. +//! Logic for working with 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_core::{storage::TrackedStorageKey, RuntimeDebug}; +use sp_runtime::{SaturatedConversion, StateVersion}; +use sp_std::{collections::btree_set::BTreeSet, default::Default, vec, vec::Vec}; use sp_trie::{ - read_trie_value, LayoutV1, MemoryDB, Recorder, StorageProof, Trie, TrieConfiguration, - TrieDBBuilder, TrieError, TrieHash, + generate_trie_proof, verify_trie_proof, LayoutV0, LayoutV1, PrefixedMemoryDB, StorageProof, + TrieDBBuilder, TrieHash, }; +use codec::{Codec, Decode, Encode}; +use hash_db::Hasher; +use scale_info::TypeInfo; +use sp_state_machine::TrieBackend; +use trie_db::{DBValue, Recorder, Trie}; + +use crate::Size; + /// Raw storage proof type (just raw trie nodes). pub type RawStorageProof = Vec>; -/// Storage proof size requirements. +pub type RawStorageKey = Vec; + +/// Errors that can occur when interacting with `UnverifiedStorageProof` and `VerifiedStorageProof`. +#[derive(Clone, Encode, Decode, RuntimeDebug, PartialEq, Eq, PalletError, TypeInfo)] +pub enum StorageProofError { + /// Call to `generate_trie_proof()` failed. + UnableToGenerateTrieProof, + /// Call to `verify_trie_proof()` failed. + InvalidProof, + /// The `Vec` entries weren't sorted as expected. + UnsortedEntries, + /// The provided key wasn't found. + UnavailableKey, + /// The value associated to the provided key is `None`. + EmptyVal, + /// Error decoding value associated to a provided key. + DecodeError, + /// At least one key in the `VerifiedStorageProof` wasn't read. + UnusedKey, +} + +/// Structure representing a key-value database stored as a sorted `Vec` of tuples. /// -/// This is currently used by benchmarks when generating storage proofs. -#[derive(Clone, Copy, Debug)] -pub enum ProofSize { - /// The proof is expected to be minimal. If value size may be changed, then it is expected to - /// have given size. - Minimal(u32), - /// The proof is expected to have at least given size and grow by increasing value that is - /// stored in the trie. - HasLargeLeaf(u32), +/// The structure also contains a proof of the fact that the key-value tuples are actually present +/// in the chain storage. +#[derive(Clone, Default, Decode, Encode, Eq, PartialEq, RuntimeDebug, TypeInfo)] +pub struct UnverifiedStorageProof { + proof: RawStorageProof, + db: Vec<(RawStorageKey, Option)>, } -/// Add extra data to the trie leaf value so that it'll be of given size. -pub fn grow_trie_leaf_value(mut value: Vec, size: ProofSize) -> Vec { - match size { - ProofSize::Minimal(_) => (), - ProofSize::HasLargeLeaf(size) if size as usize > value.len() => { - value.extend(sp_std::iter::repeat(42u8).take(size as usize - value.len())); - }, - ProofSize::HasLargeLeaf(_) => (), +impl UnverifiedStorageProof { + /// Creates a new instance of `UnverifiedStorageProof`. + pub fn try_new( + read_proof: StorageProof, + root: TrieHash>, + mut keys: Vec + Ord>, + ) -> Result { + // It's ok to use `LayoutV1` in this function, no matter the actual underlying layout, + // because we only perform read operations. When reading `LayoutV0` and `LayoutV1` lead to + // the same result. + let mem_db = read_proof.into_memory_db(); + let trie_db = TrieDBBuilder::>::new(&mem_db, &root).build(); + + let trie_proof = generate_trie_proof::, _, _, _>(&mem_db, root, &keys) + .map_err(|_| StorageProofError::UnableToGenerateTrieProof)?; + + let mut entries = Vec::with_capacity(keys.len()); + keys.sort(); + for key in keys { + let val = trie_db.get(key.as_ref()).map_err(|_| StorageProofError::UnavailableKey)?; + entries.push((key.as_ref().to_vec(), val)); + } + + Ok(Self { proof: trie_proof, db: entries }) } - 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>, -} + /// Creates a new instance of `UnverifiedStorageProof` from the provided entries. + /// + /// **This function is used only in tests and benchmarks.** + #[cfg(feature = "std")] + pub fn try_from_entries( + state_version: StateVersion, + entries: &[(RawStorageKey, Option)], + ) -> Result<(H::Out, UnverifiedStorageProof), StorageProofError> + 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(); + + Ok((root, UnverifiedStorageProof::try_from_db(backend.backend_storage(), root, keys)?)) + } -impl StorageProofChecker -where - H: Hasher, -{ - /// Constructs a new storage proof checker. + /// Creates a new instance of `UnverifiedStorageProof` from the provided db. /// - /// 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) + /// **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(|_| StorageProofError::UnavailableKey)?; } - let db = proof.into_memory_db(); - if !db.contains(&root, EMPTY_PREFIX) { - return Err(Error::StorageRootMismatch) - } + 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(); - let recorder = Recorder::default(); - let checker = StorageProofChecker { proof_nodes_count, root, db, recorder }; - Ok(checker) + UnverifiedStorageProof::try_new::(StorageProof::new(raw_read_proof), root, keys) } - /// 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) + /// Validates the contained `db` against the contained proof. If the `db` is valid, converts it + /// into a `VerifiedStorageProof`. + pub fn verify( + mut self, + state_version: StateVersion, + state_root: &TrieHash>, + ) -> Result { + // First we verify the proof for the `UnverifiedStorageProof`. + // Note that `verify_trie_proof()` also checks for duplicate keys and unused nodes. + match state_version { + StateVersion::V0 => + verify_trie_proof::, _, _, _>(state_root, &self.proof, &self.db), + StateVersion::V1 => + verify_trie_proof::, _, _, _>(state_root, &self.proof, &self.db), + } + .map_err(|_| StorageProofError::InvalidProof)?; + + // Fill the `VerifiedStorageProof` + let mut trusted_db = Vec::with_capacity(self.db.len()); + let mut iter = self.db.drain(..).peekable(); + while let Some((key, val)) = iter.next() { + // Let's also make sure that the db is actually sorted. + if let Some((next_key, _)) = iter.peek() { + if next_key <= &key { + return Err(StorageProofError::UnsortedEntries) + } + } + trusted_db.push((TrackedStorageKey::new(key), val)) } + Ok(VerifiedStorageProof { db: trusted_db }) } +} + +impl Size for UnverifiedStorageProof { + fn size(&self) -> u32 { + let proof_size = self.proof.iter().fold(0usize, |sum, node| sum.saturating_add(node.len())); + let entries_size = self.db.iter().fold(0usize, |sum, (key, value)| { + sum.saturating_add(key.len()) + .saturating_add(value.as_ref().unwrap_or(&vec![]).len()) + }); - /// 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) + proof_size.saturating_add(entries_size).saturated_into() } +} + +/// Structure representing a key-value database stored as a sorted `Vec` of tuples. +pub struct VerifiedStorageProof { + db: Vec<(TrackedStorageKey, Option)>, +} - /// 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() - }) +impl VerifiedStorageProof { + /// Returns a reference to the value corresponding to the key. + /// + /// Returns an error if the key doesn't exist. + pub fn get(&mut self, key: &impl AsRef<[u8]>) -> Result<&Option, StorageProofError> { + let idx = self + .db + .binary_search_by(|(db_key, _)| db_key.key.as_slice().cmp(key.as_ref())) + .map_err(|_| StorageProofError::UnavailableKey)?; + let (db_key, db_val) = self.db.get_mut(idx).ok_or(StorageProofError::UnavailableKey)?; + db_key.add_read(); + Ok(db_val) } - /// 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) + /// Returns a reference to the value corresponding to the key. + /// + /// Returns an error if the key doesn't exist or if the value associated to it is `None`. + pub fn get_and_decode_mandatory( + &mut self, + key: &impl AsRef<[u8]>, + ) -> Result { + let val = self.get(key)?.as_ref().ok_or(StorageProofError::EmptyVal)?; + D::decode(&mut &val[..]).map_err(|_| StorageProofError::DecodeError) } - /// 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), + /// Returns a reference to the value corresponding to the key. + /// + /// Returns `None` if the key doesn't exist or if the value associated to it is `None`. + pub fn get_and_decode_optional( + &mut self, + key: &impl AsRef<[u8]>, + ) -> Result, StorageProofError> { + match self.get_and_decode_mandatory(key) { + Ok(val) => Ok(Some(val)), + Err(StorageProofError::UnavailableKey | StorageProofError::EmptyVal) => 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), + /// Checks if each key was read. + pub fn ensure_no_unused_keys(&self) -> Result<(), StorageProofError> { + for (key, _) in &self.db { + if !key.has_been_read() { + return Err(StorageProofError::UnusedKey) + } + } + + Ok(()) + } } -/// Return valid storage proof and state root. +/// Storage proof size requirements. /// -/// 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()) +/// This is currently used by benchmarks when generating storage proofs. +#[derive(Clone, Copy, Debug)] +pub enum StorageProofSize { + /// The storage proof is expected to be minimal. If value size may be changed, then it is + /// expected to have given size. + Minimal(u32), + /// The storage proof is expected to have at least given size and grow by increasing value that + /// is stored in the trie. + HasLargeLeaf(u32), } -/// 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)?; +/// Add extra data to the storage value so that it'll be of given size. +pub fn grow_storage_value(mut value: Vec, size: StorageProofSize) -> Vec { + match size { + StorageProofSize::Minimal(_) => (), + StorageProofSize::HasLargeLeaf(size) if size as usize > value.len() => { + value.extend(sp_std::iter::repeat(42u8).take(size as usize - value.len())); + }, + StorageProofSize::HasLargeLeaf(_) => (), } - - // 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()) + value } #[cfg(test)] -pub mod tests { +mod tests { use super::*; - use codec::Encode; + + type Hasher = sp_core::Blake2Hasher; + + #[test] + fn verify_succeeds_when_used_correctly() { + let (root, db) = UnverifiedStorageProof::try_from_entries::( + StateVersion::default(), + &[(b"key1".to_vec(), None), (b"key2".to_vec(), Some(b"val2".to_vec()))], + ) + .expect("UnverifiedStorageProof::try_from_entries() shouldn't fail in tests"); + + assert!(db.verify::(StateVersion::V1, &root).is_ok()); + } #[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))),); + fn verify_fails_when_proof_contains_unneeded_nodes() { + let (root, mut db) = UnverifiedStorageProof::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("UnverifiedStorageProof::try_from_entries() shouldn't fail in tests"); + assert!(db.db.pop().is_some()); + assert!(matches!( - checker.read_and_decode_value::<[u8; 64]>(b"key4"), - Err(Error::StorageValueDecodeFailed(_)), + db.verify::(StateVersion::V1, &root), + Err(StorageProofError::InvalidProof) )); + } - // checking proof against invalid commitment fails - assert_eq!( - >::new(sp_core::H256::random(), proof).err(), - Some(Error::StorageRootMismatch) - ); + #[test] + fn verify_fails_when_db_contains_duplicate_nodes() { + let (root, mut db) = UnverifiedStorageProof::try_from_entries::( + StateVersion::default(), + &[(b"key".to_vec(), None)], + ) + .expect("UnverifiedStorageProof::try_from_entries() shouldn't fail in tests"); + db.db.push((b"key".to_vec(), None)); + + assert!(matches!( + db.verify::(StateVersion::V1, &root), + Err(StorageProofError::InvalidProof) + )); } #[test] - fn proof_with_duplicate_items_is_rejected() { - let (root, mut proof) = craft_valid_storage_proof(); - proof.push(proof.first().unwrap().clone()); + fn verify_fails_when_entries_are_not_sorted() { + let (root, mut db) = UnverifiedStorageProof::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("UnverifiedStorageProof::try_from_entries() shouldn't fail in tests"); + db.db.reverse(); + + assert!(matches!( + db.verify::(StateVersion::V1, &root), + Err(StorageProofError::UnsortedEntries) + )); + } - assert_eq!( - StorageProofChecker::::new(root, proof).map(drop), - Err(Error::DuplicateNodesInProof), + #[test] + fn get_and_decode_mandatory_works() { + let (root, db) = UnverifiedStorageProof::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("UnverifiedStorageProof::try_from_entries() shouldn't fail in tests"); + let mut trusted_db = db.verify::(StateVersion::V1, &root).unwrap(); + + assert!( + matches!(trusted_db.get_and_decode_mandatory::>(b"key11"), Ok(val) if val == b"val11".to_vec()) + ); + assert!( + matches!(trusted_db.get_and_decode_mandatory::>(b"key2"), Ok(val) if val == b"val2".to_vec()) ); + assert!(matches!( + trusted_db.get_and_decode_mandatory::>(b"key1"), + Err(StorageProofError::EmptyVal) + )); + assert!(matches!( + trusted_db.get_and_decode_mandatory::>(b"key15"), + Err(StorageProofError::DecodeError) + )); + } + + #[test] + fn get_and_decode_optional_works() { + let (root, db) = UnverifiedStorageProof::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("UnverifiedStorageProof::try_from_entries() shouldn't fail in tests"); + let mut trusted_db = db.verify::(StateVersion::V1, &root).unwrap(); + + assert!( + matches!(trusted_db.get_and_decode_optional::>(b"key11"), Ok(Some(val)) if val == + b"val11".to_vec()) + ); + assert!( + matches!(trusted_db.get_and_decode_optional::>(b"key2"), Ok(Some(val)) if val == b"val2".to_vec()) + ); + assert!(matches!(trusted_db.get_and_decode_optional::>(b"key1"), Ok(None))); + assert!(matches!( + trusted_db.get_and_decode_optional::>(b"key15"), + Err(StorageProofError::DecodeError) + )); } #[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)); + fn ensure_no_unused_keys_works_correctly() { + let (root, db) = UnverifiedStorageProof::try_from_entries::( + StateVersion::default(), + &[(b"key1".to_vec(), None), (b"key2".to_vec(), Some(b"val2".to_vec()))], + ) + .expect("UnverifiedStorageProof::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()); + + assert!(matches!(trusted_db.ensure_no_unused_keys(), Err(StorageProofError::UnusedKey))); } } diff --git a/bridges/primitives/runtime/src/vec_db.rs b/bridges/primitives/runtime/src/vec_db.rs deleted file mode 100644 index 6034e102bd305..0000000000000 --- a/bridges/primitives/runtime/src/vec_db.rs +++ /dev/null @@ -1,332 +0,0 @@ -// Copyright 2019-2023 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 . - -//! Logic for working with more efficient storage proofs. - -use frame_support::PalletError; -use sp_core::{storage::TrackedStorageKey, RuntimeDebug}; -use sp_runtime::{SaturatedConversion, StateVersion}; -use sp_std::{default::Default, vec, vec::Vec}; -use sp_trie::{ - generate_trie_proof, verify_trie_proof, LayoutV0, LayoutV1, StorageProof, TrieDBBuilder, - TrieHash, -}; - -use codec::{Decode, Encode}; -use hash_db::Hasher; -use scale_info::TypeInfo; -use trie_db::{DBValue, Trie}; - -use crate::{storage_proof::RawStorageProof, Size}; - -pub type RawStorageKey = Vec; - -/// Errors that can occur when interacting with `UntrustedVecDb` and `TrustedVecDb`. -#[derive(Clone, Encode, Decode, RuntimeDebug, PartialEq, Eq, PalletError, TypeInfo)] -pub enum VecDbError { - /// Call to `generate_trie_proof()` failed. - UnableToGenerateTrieProof, - /// Call to `verify_trie_proof()` failed. - InvalidProof, - /// The `Vec` entries weren't sorted as expected. - UnsortedEntries, - /// The provided key wasn't found. - UnavailableKey, - /// The value associated to the provided key is `None`. - EmptyVal, - /// Error decoding value associated to a provided key. - DecodeError, - /// At least one key in the `VecDb` wasn't read. - UnusedKey, -} - -/// Structure representing a key-value database stored as a sorted `Vec` of tuples. -/// -/// The structure also contains a proof of the fact that the key-value tuples are actually present -/// in the chain storage. -#[derive(Clone, Default, Decode, Encode, Eq, PartialEq, RuntimeDebug, TypeInfo)] -pub struct UntrustedVecDb { - proof: RawStorageProof, - db: Vec<(RawStorageKey, Option)>, -} - -impl UntrustedVecDb { - /// Creates a new instance of `UntrustedVecDb`. - pub fn try_new( - read_proof: StorageProof, - root: TrieHash>, - mut keys: Vec + Ord>, - ) -> Result { - // It's ok to use `LayoutV1` in this function, no matter the actual underlying layout, - // because we only perform read operations. When reading `LayoutV0` and `LayoutV1` lead to - // the same result. - let mem_db = read_proof.into_memory_db(); - let trie_db = TrieDBBuilder::>::new(&mem_db, &root).build(); - - let trie_proof = generate_trie_proof::, _, _, _>(&mem_db, root, &keys) - .map_err(|_| VecDbError::UnableToGenerateTrieProof)?; - - let mut entries = Vec::with_capacity(keys.len()); - keys.sort(); - for key in keys { - let val = trie_db.get(key.as_ref()).map_err(|_| VecDbError::UnavailableKey)?; - entries.push((key.as_ref().to_vec(), val)); - } - - Ok(Self { proof: trie_proof, db: entries }) - } - - /// Validates the contained `db` against the contained proof. If the `db` is valid, converts it - /// into a `TrustedVecDb`. - pub fn verify( - mut self, - state_version: StateVersion, - state_root: &TrieHash>, - ) -> Result { - // First we verify the proof for the `UntrustedVecDb`. - // Note that `verify_trie_proof()` also checks for duplicate keys and unused nodes. - match state_version { - StateVersion::V0 => - verify_trie_proof::, _, _, _>(state_root, &self.proof, &self.db), - StateVersion::V1 => - verify_trie_proof::, _, _, _>(state_root, &self.proof, &self.db), - } - .map_err(|_| VecDbError::InvalidProof)?; - - // Fill the `TrustedVecDb` - let mut trusted_db = Vec::with_capacity(self.db.len()); - let mut iter = self.db.drain(..).peekable(); - while let Some((key, val)) = iter.next() { - // Let's also make sure that the db is actually sorted. - if let Some((next_key, _)) = iter.peek() { - if next_key <= &key { - return Err(VecDbError::UnsortedEntries) - } - } - trusted_db.push((TrackedStorageKey::new(key), val)) - } - Ok(TrustedVecDb { db: trusted_db }) - } -} - -impl Size for UntrustedVecDb { - fn size(&self) -> u32 { - let proof_size = self.proof.iter().fold(0usize, |sum, node| sum.saturating_add(node.len())); - let entries_size = self.db.iter().fold(0usize, |sum, (key, value)| { - sum.saturating_add(key.len()) - .saturating_add(value.as_ref().unwrap_or(&vec![]).len()) - }); - - proof_size.saturating_add(entries_size).saturated_into() - } -} - -/// Structure representing a key-value database stored as a sorted `Vec` of tuples. -pub struct TrustedVecDb { - db: Vec<(TrackedStorageKey, Option)>, -} - -impl TrustedVecDb { - /// Returns a reference to the value corresponding to the key. - /// - /// Returns an error if the key doesn't exist. - pub fn get(&mut self, key: &impl AsRef<[u8]>) -> Result<&Option, VecDbError> { - let idx = self - .db - .binary_search_by(|(db_key, _)| db_key.key.as_slice().cmp(key.as_ref())) - .map_err(|_| VecDbError::UnavailableKey)?; - let (db_key, db_val) = self.db.get_mut(idx).ok_or(VecDbError::UnavailableKey)?; - db_key.add_read(); - Ok(db_val) - } - - /// Returns a reference to the value corresponding to the key. - /// - /// Returns an error if the key doesn't exist or if the value associated to it is `None`. - pub fn get_and_decode_mandatory( - &mut self, - key: &impl AsRef<[u8]>, - ) -> Result { - let val = self.get(key)?.as_ref().ok_or(VecDbError::EmptyVal)?; - D::decode(&mut &val[..]).map_err(|_| VecDbError::DecodeError) - } - - /// Returns a reference to the value corresponding to the key. - /// - /// Returns `None` if the key doesn't exist or if the value associated to it is `None`. - pub fn get_and_decode_optional( - &mut self, - key: &impl AsRef<[u8]>, - ) -> Result, VecDbError> { - match self.get_and_decode_mandatory(key) { - Ok(val) => Ok(Some(val)), - Err(VecDbError::UnavailableKey | VecDbError::EmptyVal) => Ok(None), - Err(e) => Err(e), - } - } - - /// Checks if each key was read. - pub fn ensure_no_unused_keys(&self) -> Result<(), VecDbError> { - for (key, _) in &self.db { - if !key.has_been_read() { - return Err(VecDbError::UnusedKey) - } - } - - Ok(()) - } -} - -#[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(); - - 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(); - assert!(db.db.pop().is_some()); - - assert!(matches!( - db.verify::(StateVersion::V1, &root), - Err(VecDbError::InvalidProof) - )); - } - - #[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(); - db.db.push((b"key".to_vec(), None)); - - assert!(matches!( - db.verify::(StateVersion::V1, &root), - Err(VecDbError::InvalidProof) - )); - } - - #[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(); - db.db.reverse(); - - assert!(matches!( - db.verify::(StateVersion::V1, &root), - Err(VecDbError::UnsortedEntries) - )); - } - - #[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 mut trusted_db = db.verify::(StateVersion::V1, &root).unwrap(); - - assert!( - matches!(trusted_db.get_and_decode_mandatory::>(b"key11"), Ok(val) if val == b"val11".to_vec()) - ); - assert!( - matches!(trusted_db.get_and_decode_mandatory::>(b"key2"), Ok(val) if val == b"val2".to_vec()) - ); - assert!(matches!( - trusted_db.get_and_decode_mandatory::>(b"key1"), - Err(VecDbError::EmptyVal) - )); - assert!(matches!( - trusted_db.get_and_decode_mandatory::>(b"key15"), - Err(VecDbError::DecodeError) - )); - } - - #[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 mut trusted_db = db.verify::(StateVersion::V1, &root).unwrap(); - - assert!( - matches!(trusted_db.get_and_decode_optional::>(b"key11"), Ok(Some(val)) if val == - b"val11".to_vec()) - ); - assert!( - matches!(trusted_db.get_and_decode_optional::>(b"key2"), Ok(Some(val)) if val == b"val2".to_vec()) - ); - assert!(matches!(trusted_db.get_and_decode_optional::>(b"key1"), Ok(None))); - assert!(matches!( - trusted_db.get_and_decode_optional::>(b"key15"), - Err(VecDbError::DecodeError) - )); - } - - #[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 mut trusted_db = db.verify::(StateVersion::V1, &root).unwrap(); - assert!(trusted_db.get(b"key1").is_ok()); - - assert!(matches!(trusted_db.ensure_no_unused_keys(), Err(VecDbError::UnusedKey))); - } -} diff --git a/bridges/primitives/test-utils/src/lib.rs b/bridges/primitives/test-utils/src/lib.rs index f4fe4a242e79c..e2f1fe68c585e 100644 --- a/bridges/primitives/test-utils/src/lib.rs +++ b/bridges/primitives/test-utils/src/lib.rs @@ -22,12 +22,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::UnverifiedStorageProof; 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, MemoryDB, TrieMut}; // Re-export all our test account utilities pub use keyring::*; @@ -177,6 +177,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 { @@ -185,14 +186,15 @@ 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) - .map_err(|_| "record_all_trie_keys has failed") - .expect("record_all_trie_keys should not fail in benchmarks"); + let storage_proof = + UnverifiedStorageProof::try_from_db::(&mdb, root, storage_keys) + .expect("UnverifiedStorageProof::try_from_db() should not fail in benchmarks"); (root, ParaHeadsProof { storage_proof }, parachains) } diff --git a/bridges/relays/client-substrate/src/client/caching.rs b/bridges/relays/client-substrate/src/client/caching.rs index a6116988a4909..e1769fe5bf9dd 100644 --- a/bridges/relays/client-substrate/src/client/caching.rs +++ b/bridges/relays/client-substrate/src/client/caching.rs @@ -31,6 +31,7 @@ use async_std::{ task::JoinHandle, }; use async_trait::async_trait; +use bp_runtime::UnverifiedStorageProof; use codec::Encode; use frame_support::weights::Weight; use futures::{FutureExt, StreamExt}; @@ -41,7 +42,6 @@ use sp_core::{ Bytes, Pair, }; use sp_runtime::{traits::Header as _, transaction_validity::TransactionValidity}; -use sp_trie::StorageProof; use sp_version::RuntimeVersion; /// `quick_cache::unsync::Cache` wrapped in async-aware synchronization primitives. @@ -472,7 +472,12 @@ impl> Client for CachingClient { .await } - async fn prove_storage(&self, at: HashOf, keys: Vec) -> Result { - self.backend.prove_storage(at, keys).await + async fn prove_storage_with_root( + &self, + at: HashOf, + state_root: HashOf, + keys: Vec, + ) -> Result { + self.backend.prove_storage_with_root(at, state_root, keys).await } } diff --git a/bridges/relays/client-substrate/src/client/client.rs b/bridges/relays/client-substrate/src/client/client.rs index 49f5c001c3f7d..7796c69b7805c 100644 --- a/bridges/relays/client-substrate/src/client/client.rs +++ b/bridges/relays/client-substrate/src/client/client.rs @@ -22,7 +22,7 @@ use crate::{ }; use async_trait::async_trait; -use bp_runtime::{StorageDoubleMapKeyProvider, StorageMapKeyProvider}; +use bp_runtime::{StorageDoubleMapKeyProvider, StorageMapKeyProvider, UnverifiedStorageProof}; use codec::{Decode, Encode}; use frame_support::weights::Weight; use sp_core::{ @@ -30,7 +30,6 @@ use sp_core::{ Bytes, Pair, }; use sp_runtime::{traits::Header as _, transaction_validity::TransactionValidity}; -use sp_trie::StorageProof; use sp_version::RuntimeVersion; use std::fmt::Debug; @@ -226,5 +225,21 @@ pub trait Client: 'static + Send + Sync + Clone + Debug { } /// Returns storage proof of given storage keys. - async fn prove_storage(&self, at: HashOf, keys: Vec) -> Result; + async fn prove_storage_with_root( + &self, + at: HashOf, + state_root: HashOf, + keys: Vec, + ) -> Result; + + /// Returns storage proof of given storage keys. + async fn prove_storage( + &self, + at: HashOf, + keys: Vec, + ) -> Result { + let root = *self.header_by_hash(at).await?.state_root(); + + self.prove_storage_with_root(at, root, keys).await + } } diff --git a/bridges/relays/client-substrate/src/client/rpc.rs b/bridges/relays/client-substrate/src/client/rpc.rs index bf7442a95141f..1a0f2a4b3cc94 100644 --- a/bridges/relays/client-substrate/src/client/rpc.rs +++ b/bridges/relays/client-substrate/src/client/rpc.rs @@ -37,7 +37,7 @@ use crate::{ use async_std::sync::{Arc, Mutex, RwLock}; use async_trait::async_trait; -use bp_runtime::HeaderIdProvider; +use bp_runtime::{HasherOf, HeaderIdProvider, UnverifiedStorageProof}; use codec::Encode; use frame_support::weights::Weight; use futures::TryFutureExt; @@ -635,16 +635,25 @@ impl Client for RpcClient { .map_err(|e| Error::failed_state_call::(at, method_clone, arguments_clone, e)) } - async fn prove_storage(&self, at: HashOf, keys: Vec) -> Result { + async fn prove_storage_with_root( + &self, + at: HashOf, + state_root: HashOf, + keys: Vec, + ) -> Result { let keys_clone = keys.clone(); - self.jsonrpsee_execute(move |client| async move { - SubstrateStateClient::::prove_storage(&*client, keys, Some(at)) - .await - .map(|proof| StorageProof::new(proof.proof.into_iter().map(|b| b.0))) - .map_err(Into::into) - }) - .await - .map_err(|e| Error::failed_to_prove_storage::(at, keys_clone, e)) + let read_proof = self + .jsonrpsee_execute(move |client| async move { + SubstrateStateClient::::prove_storage(&*client, keys_clone, Some(at)) + .await + .map(|proof| StorageProof::new(proof.proof.into_iter().map(|b| b.0))) + .map_err(Into::into) + }) + .await + .map_err(|e| Error::failed_to_prove_storage::(at, keys.clone(), e))?; + + UnverifiedStorageProof::try_new::>(read_proof, state_root, keys) + .map_err(|e| Error::Custom(format!("Error generating storage proof: {:?}", e))) } } diff --git a/bridges/relays/client-substrate/src/error.rs b/bridges/relays/client-substrate/src/error.rs index b09e2c7abdc66..ee3c73f806e65 100644 --- a/bridges/relays/client-substrate/src/error.rs +++ b/bridges/relays/client-substrate/src/error.rs @@ -213,9 +213,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), diff --git a/bridges/relays/lib-substrate-relay/src/messages/source.rs b/bridges/relays/lib-substrate-relay/src/messages/source.rs index 877b0438323ea..8f0e1b54460cd 100644 --- a/bridges/relays/lib-substrate-relay/src/messages/source.rs +++ b/bridges/relays/lib-substrate-relay/src/messages/source.rs @@ -36,9 +36,7 @@ use bp_messages::{ ChainWithMessages as _, InboundMessageDetails, LaneId, MessageNonce, MessagePayload, MessagesOperatingMode, OutboundLaneData, OutboundMessageDetails, }; -use bp_runtime::{ - BasicOperatingMode, HasherOf, HeaderIdProvider, RangeInclusiveExt, UntrustedVecDb, -}; +use bp_runtime::{BasicOperatingMode, HeaderIdProvider, RangeInclusiveExt}; use codec::Encode; use frame_support::weights::Weight; use messages_relay::{ @@ -56,7 +54,6 @@ use relay_substrate_client::{ }; use relay_utils::relay_loop::Client as RelayClient; use sp_core::Pair; -use sp_runtime::traits::Header; use std::ops::RangeInclusive; /// Intermediate message proof returned by the source Substrate node. Includes everything @@ -339,14 +336,7 @@ where )); } - let root = *self.source_client.header_by_hash(id.hash()).await?.state_root(); - let storage_proof = - self.source_client.prove_storage(id.hash(), storage_keys.clone()).await?; - let storage = - UntrustedVecDb::try_new::>(storage_proof, root, storage_keys) - .map_err(|e| { - SubstrateError::Custom(format!("Error generating messages storage: {:?}", e)) - })?; + let storage = self.source_client.prove_storage(id.hash(), storage_keys.clone()).await?; let proof = FromBridgedChainMessagesProof { bridged_header_hash: id.1, storage, diff --git a/bridges/relays/lib-substrate-relay/src/messages/target.rs b/bridges/relays/lib-substrate-relay/src/messages/target.rs index fdd2619ffc04d..6a137bda46ee9 100644 --- a/bridges/relays/lib-substrate-relay/src/messages/target.rs +++ b/bridges/relays/lib-substrate-relay/src/messages/target.rs @@ -37,7 +37,6 @@ use bp_messages::{ source_chain::FromBridgedChainMessagesDeliveryProof, storage_keys::inbound_lane_data_key, ChainWithMessages as _, InboundLaneData, LaneId, MessageNonce, UnrewardedRelayersState, }; -use bp_runtime::{HasherOf, UntrustedVecDb}; use messages_relay::{ message_lane::{MessageLane, SourceHeaderIdOf, TargetHeaderIdOf}, message_lane_loop::{NoncesSubmitArtifacts, TargetClient, TargetClientState}, @@ -48,7 +47,6 @@ use relay_substrate_client::{ }; use relay_utils::relay_loop::Client as RelayClient; use sp_core::Pair; -use sp_runtime::traits::Header; use std::{convert::TryFrom, ops::RangeInclusive}; /// Message receiving proof returned by the target Substrate node. @@ -238,16 +236,8 @@ where &self.lane_id, )]; - let root = *self.target_client.header_by_hash(id.hash()).await?.state_root(); - let read_proof = self.target_client.prove_storage(id.hash(), storage_keys.clone()).await?; let storage_proof = - UntrustedVecDb::try_new::>(read_proof, root, storage_keys) - .map_err(|e| { - SubstrateError::Custom(format!( - "Error generating messages delivery confirmation storage: {:?}", - e - )) - })?; + self.target_client.prove_storage(id.hash(), storage_keys.clone()).await?; let proof = FromBridgedChainMessagesDeliveryProof { bridged_header_hash: id.1, storage_proof, diff --git a/bridges/relays/lib-substrate-relay/src/parachains/source.rs b/bridges/relays/lib-substrate-relay/src/parachains/source.rs index 11b9d6dbf5bd3..4ee41c45fafae 100644 --- a/bridges/relays/lib-substrate-relay/src/parachains/source.rs +++ b/bridges/relays/lib-substrate-relay/src/parachains/source.rs @@ -153,12 +153,9 @@ 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 storage_proof = + self.client.prove_storage(at_block.hash(), vec![storage_key.clone()]).await?; // 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 @@ -178,6 +175,6 @@ where })?; let parachain_head_hash = parachain_head.hash(); - Ok((ParaHeadsProof { storage_proof: parachain_heads_proof }, parachain_head_hash)) + Ok((ParaHeadsProof { storage_proof }, parachain_head_hash)) } } diff --git a/bridges/relays/parachains/src/parachains_loop.rs b/bridges/relays/parachains/src/parachains_loop.rs index fd73ca2d46c00..0fd1d72c7075b 100644 --- a/bridges/relays/parachains/src/parachains_loop.rs +++ b/bridges/relays/parachains/src/parachains_loop.rs @@ -680,7 +680,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}; @@ -821,8 +820,7 @@ mod tests { let head_result = SourceClient::::parachain_head(self, at_block).await?; let head = head_result.as_available().unwrap(); - let storage_proof = vec![head.hash().encode()]; - let proof = (ParaHeadsProof { storage_proof }, head.hash()); + let proof = (ParaHeadsProof { storage_proof: Default::default() }, head.hash()); self.data.lock().await.source_proof.clone().map(|_| proof) } }