From 4ded369b71b3c6ade0c4fb24b2a4196aafcfb0fd Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 1 Jun 2022 17:34:39 +0300 Subject: [PATCH 01/31] pallet-mmr: fix some typos --- frame/merkle-mountain-range/src/lib.rs | 4 ++-- primitives/merkle-mountain-range/src/lib.rs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index d6cf3240692fc..0b598f5c3dab5 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -116,12 +116,12 @@ pub mod pallet { /// Prefix for elements stored in the Off-chain DB via Indexing API. /// /// Each node of the MMR is inserted both on-chain and off-chain via Indexing API. - /// The former does not store full leaf content, just it's compact version (hash), + /// The former does not store full leaf content, just its compact version (hash), /// and some of the inner mmr nodes might be pruned from on-chain storage. /// The latter will contain all the entries in their full form. /// /// Each node is stored in the Off-chain DB under key derived from the - /// [`Self::INDEXING_PREFIX`] and it's in-tree index (MMR position). + /// [`Self::INDEXING_PREFIX`] and its in-tree index (MMR position). const INDEXING_PREFIX: &'static [u8]; /// A hasher type for MMR. diff --git a/primitives/merkle-mountain-range/src/lib.rs b/primitives/merkle-mountain-range/src/lib.rs index 5a339d069062c..8a2e901aefddf 100644 --- a/primitives/merkle-mountain-range/src/lib.rs +++ b/primitives/merkle-mountain-range/src/lib.rs @@ -81,7 +81,7 @@ pub struct Proof { /// A full leaf content stored in the offchain-db. pub trait FullLeaf: Clone + PartialEq + fmt::Debug { - /// Encode the leaf either in it's full or compact form. + /// Encode the leaf either in its full or compact form. /// /// NOTE the encoding returned here MUST be `Decode`able into `FullLeaf`. fn using_encoded R>(&self, f: F, compact: bool) -> R; @@ -167,18 +167,18 @@ impl EncodableOpaqueLeaf { } } -/// An element representing either full data or it's hash. +/// An element representing either full data or its hash. /// /// See [Compact] to see how it may be used in practice to reduce the size /// of proofs in case multiple [LeafDataProvider]s are composed together. /// This is also used internally by the MMR to differentiate leaf nodes (data) /// and inner nodes (hashes). /// -/// [DataOrHash::hash] method calculates the hash of this element in it's compact form, +/// [DataOrHash::hash] method calculates the hash of this element in its compact form, /// so should be used instead of hashing the encoded form (which will always be non-compact). #[derive(RuntimeDebug, Clone, PartialEq)] pub enum DataOrHash { - /// Arbitrary data in it's full form. + /// Arbitrary data in its full form. Data(L), /// A hash of some data. Hash(H::Output), @@ -339,7 +339,7 @@ where A: FullLeaf, B: FullLeaf, { - /// Retrieve a hash of this item in it's compact form. + /// Retrieve a hash of this item in its compact form. pub fn hash(&self) -> H::Output { self.using_encoded(::hash, true) } @@ -447,7 +447,7 @@ sp_api::decl_runtime_apis! { /// Note this function does not require any on-chain storage - the /// proof is verified against given MMR root hash. /// - /// The leaf data is expected to be encoded in it's compact form. + /// The leaf data is expected to be encoded in its compact form. fn verify_proof_stateless(root: Hash, leaf: EncodableOpaqueLeaf, proof: Proof) -> Result<(), Error>; From eb9eea87032f90c476ba8356cdab193aefc99c4a Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 1 Jun 2022 17:38:01 +0300 Subject: [PATCH 02/31] pallet-mmr: make the MMR resilient to chain forks --- frame/merkle-mountain-range/src/lib.rs | 11 +++++++++-- .../merkle-mountain-range/src/mmr/storage.rs | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index 0b598f5c3dab5..e347061a1b20f 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -254,9 +254,16 @@ where } impl, I: 'static> Pallet { - fn offchain_key(pos: NodeIndex) -> sp_std::prelude::Vec { - (T::INDEXING_PREFIX, pos).encode() + /// Build offchain key from `parent_hash` of block that originally added node `pos` to MMR. + /// + /// This combination makes the offchain (key,value) entry resilient to chain forks. + fn offchain_key( + parent_hash: ::Hash, + pos: NodeIndex, + ) -> sp_std::prelude::Vec { + (T::INDEXING_PREFIX, parent_hash, pos).encode() } + /// Generate a MMR proof for the given `leaf_indices`. /// /// Note this method can only be used from an off-chain context diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 535057ca80da7..0f225bd13fb2d 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -58,6 +58,14 @@ impl Default for Storage { } } +fn parent_hash_of_ancestor_that_added_node( + _pos: NodeIndex, +) -> ::Hash { + // TODO: implement + let block_num_from_pos: ::BlockNumber = 42_u32.into(); + >::block_hash(block_num_from_pos) +} + impl mmr_lib::MMRStore> for Storage where T: Config, @@ -65,7 +73,11 @@ where L: primitives::FullLeaf + codec::Decode, { fn get_elem(&self, pos: NodeIndex) -> mmr_lib::Result>> { - let key = Pallet::::offchain_key(pos); + // Get the parent hash of the ancestor block that added node at index `pos`. + // Use the hash as extra identifier to differentiate between various `pos` entries + // in offchain DB coming from various chain forks. + let parent_hash_of_ancestor = parent_hash_of_ancestor_that_added_node::(pos); + let key = Pallet::::offchain_key(parent_hash_of_ancestor, pos); // Retrieve the element from Off-chain DB. Ok(sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) .and_then(|v| codec::Decode::decode(&mut &*v).ok())) @@ -112,10 +124,13 @@ where let mut leaf_index = leaves; let mut node_index = size; + // Use parent hash of block adding new nodes (this block) as extra identifier + // in offchain DB to avoid DB collisions and overwrites in case of forks. + let parent_hash = >::parent_hash(); for elem in elems { // Indexing API is used to store the full node content (both leaf and inner). elem.using_encoded(|elem| { - offchain_index::set(&Pallet::::offchain_key(node_index), elem) + offchain_index::set(&Pallet::::offchain_key(parent_hash, node_index), elem) }); // On-chain we are going to only store new peaks. From 7c8aac02bfb295e22a0957ffb96bb1ac36c9caef Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 2 Jun 2022 19:34:01 +0300 Subject: [PATCH 03/31] pallet-mmr: get hash for block that added node --- frame/merkle-mountain-range/src/lib.rs | 2 +- .../merkle-mountain-range/src/mmr/storage.rs | 15 ++++-- frame/merkle-mountain-range/src/mmr/utils.rs | 48 +++++++++++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index e347061a1b20f..d892ad2a7b961 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -271,7 +271,7 @@ impl, I: 'static> Pallet { /// all the leaves to be present. /// It may return an error or panic if used incorrectly. pub fn generate_batch_proof( - leaf_indices: Vec, + leaf_indices: Vec, ) -> Result< (Vec>, primitives::BatchProof<>::Hash>), primitives::Error, diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 0f225bd13fb2d..4efcca528c72b 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -59,11 +59,18 @@ impl Default for Storage { } fn parent_hash_of_ancestor_that_added_node( - _pos: NodeIndex, + pos: NodeIndex, ) -> ::Hash { - // TODO: implement - let block_num_from_pos: ::BlockNumber = 42_u32.into(); - >::block_hash(block_num_from_pos) + // TODO: turn this in a pallet storage item recording block-num when pallet was activated. + let block_offset = 0; + + let leaf_idx = NodesUtils::leaf_index_that_added_node(pos); + let block_num: ::BlockNumber = + u32::try_from(leaf_idx + block_offset) + .expect("leaf-idx < block-num; qed") + .into(); + // TODO: I think this only holds recent history, so old block hashes might not be here. + >::block_hash(block_num) } impl mmr_lib::MMRStore> for Storage diff --git a/frame/merkle-mountain-range/src/mmr/utils.rs b/frame/merkle-mountain-range/src/mmr/utils.rs index d9f7e3b671be3..79aea819dbd04 100644 --- a/frame/merkle-mountain-range/src/mmr/utils.rs +++ b/frame/merkle-mountain-range/src/mmr/utils.rs @@ -53,12 +53,60 @@ impl NodesUtils { 64 - self.no_of_leaves.next_power_of_two().leading_zeros() } + + /// Calculate `LeafIndex` for the leaf that added `node_index` to the MMR. + pub fn leaf_index_that_added_node(node_index: NodeIndex) -> LeafIndex { + let rightmost_leaf_pos = Self::rightmost_leaf_node_index_from_pos(node_index); + Self::leaf_node_index_to_leaf_index(rightmost_leaf_pos) + } + + // Translate a _leaf_ `NodeIndex` to its `LeafIndex`. + fn leaf_node_index_to_leaf_index(pos: NodeIndex) -> LeafIndex { + if pos == 0 { + return 0 + } + let (leaf_count, _) = + mmr_lib::helper::get_peaks(pos) + .iter() + .fold((0, 0), |(mut acc, last_peak), peak| { + let leaves = (peak - last_peak) >> 1; + acc += leaves + 1; + // last_peak, leaves, acc); + (acc, peak.clone()) + }); + leaf_count + } + + // Starting from any node position get position of rightmost leaf; this is the leaf + // responsible for the addition of node `pos`. + fn rightmost_leaf_node_index_from_pos(mut pos: NodeIndex) -> NodeIndex { + use mmr_lib::helper::pos_height_in_tree; + if pos > 0 { + let mut current_height = pos_height_in_tree(pos); + let mut right_child_height = pos_height_in_tree(pos - 1); + while right_child_height < current_height { + pos = pos - 1; + current_height = right_child_height; + right_child_height = pos_height_in_tree(pos - 1); + } + } + pos + } } #[cfg(test)] mod tests { use super::*; + #[test] + fn test_leaf_node_index_to_leaf_index() { + use mmr_lib::helper::leaf_index_to_pos; + for index in 0..100000 { + let pos = leaf_index_to_pos(index); + assert_eq!(NodesUtils::leaf_node_index_to_leaf_index(pos), index); + } + } + #[test] fn should_calculate_number_of_leaves_correctly() { assert_eq!( From c82ac2706e34b2592b0b76e81deab977eaa616b9 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 21 Jun 2022 14:34:57 +0300 Subject: [PATCH 04/31] beefy-mmr: add debug logging --- frame/merkle-mountain-range/src/lib.rs | 8 ++++- .../merkle-mountain-range/src/mmr/storage.rs | 34 ++++++++++++++++--- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index d892ad2a7b961..6e6d39100b52b 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -57,7 +57,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use codec::Encode; -use frame_support::weights::Weight; +use frame_support::{log::info, weights::Weight}; use sp_runtime::traits::{self, One, Saturating}; #[cfg(any(feature = "runtime-benchmarks", test))] @@ -199,6 +199,7 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { fn on_initialize(_n: T::BlockNumber) -> Weight { + info!(target: "runtime::mmr", "🥩: initialize block {:?}", _n); use primitives::LeafDataProvider; let leaves = Self::mmr_leaves(); let peaks_before = mmr::utils::NodesUtils::new(leaves).number_of_peaks(); @@ -207,6 +208,8 @@ pub mod pallet { let mut mmr: ModuleMmr = mmr::Mmr::new(leaves); mmr.push(data).expect("MMR push never fails."); + info!(target: "runtime::mmr", "🥩: on_initialize(block-num: {:?}): leaves_before {}, peaks_before {}", _n, leaves, peaks_before); + // update the size let (leaves, root) = mmr.finalize().expect("MMR finalize never fails."); >::on_new_root(&root); @@ -215,6 +218,9 @@ pub mod pallet { >::put(root); let peaks_after = mmr::utils::NodesUtils::new(leaves).number_of_peaks(); + + info!(target: "runtime::mmr", "🥩: on_initialize(block-num: {:?}): leaves {}, peaks {}", _n, leaves, peaks_after); + T::WeightInfo::on_initialize(peaks_before.max(peaks_after)) } } diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 4efcca528c72b..35dcb7382a12e 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -18,6 +18,8 @@ //! A MMR storage implementations. use codec::Encode; +use frame_support::log; +use log::info; use mmr_lib::helper; use sp_io::offchain_index; use sp_std::iter::Peekable; @@ -70,7 +72,13 @@ fn parent_hash_of_ancestor_that_added_node( .expect("leaf-idx < block-num; qed") .into(); // TODO: I think this only holds recent history, so old block hashes might not be here. - >::block_hash(block_num) + let hash = >::block_hash(block_num); + info!( + target: "runtime::mmr", + "🥩: parent of ancestor that added {}: leaf idx {}, block-num {:?} hash {:?}", + pos, leaf_idx, block_num, hash + ); + hash } impl mmr_lib::MMRStore> for Storage @@ -85,6 +93,11 @@ where // in offchain DB coming from various chain forks. let parent_hash_of_ancestor = parent_hash_of_ancestor_that_added_node::(pos); let key = Pallet::::offchain_key(parent_hash_of_ancestor, pos); + info!( + target: "runtime::mmr", + "🥩: get elem {}: key {:?}", + pos, key + ); // Retrieve the element from Off-chain DB. Ok(sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) .and_then(|v| codec::Decode::decode(&mut &*v).ok())) @@ -111,12 +124,18 @@ where } sp_std::if_std! { - frame_support::log::trace!("elems: {:?}", elems.iter().map(|elem| elem.hash()).collect::>()); + frame_support::log::info!("elems: {:?}", elems.iter().map(|elem| elem.hash()).collect::>()); } let leaves = NumberOfLeaves::::get(); let size = NodesUtils::new(leaves).size(); + info!( + target: "runtime::mmr", + "🥩: append elem {}: leaves {} size {}", + pos, leaves, size + ); + if pos != size { return Err(mmr_lib::Error::InconsistentStore) } @@ -134,11 +153,16 @@ where // Use parent hash of block adding new nodes (this block) as extra identifier // in offchain DB to avoid DB collisions and overwrites in case of forks. let parent_hash = >::parent_hash(); + let block_number = >::block_number(); for elem in elems { + let key = Pallet::::offchain_key(parent_hash, node_index); + info!( + target: "runtime::mmr", + "🥩: offchain set: block-num {:?}, parent_hash {:?} node-idx {} key {:?}", + block_number, parent_hash, node_index, key + ); // Indexing API is used to store the full node content (both leaf and inner). - elem.using_encoded(|elem| { - offchain_index::set(&Pallet::::offchain_key(parent_hash, node_index), elem) - }); + elem.using_encoded(|elem| offchain_index::set(&key, elem)); // On-chain we are going to only store new peaks. if peaks_to_store.next_if_eq(&node_index).is_some() { From 98abe4a048793d57191581761e1267fb1ace740b Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 22 Jun 2022 18:52:28 +0300 Subject: [PATCH 05/31] add explanatory comment --- frame/merkle-mountain-range/src/mmr/storage.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 35dcb7382a12e..f60a7955b6ed0 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -66,19 +66,21 @@ fn parent_hash_of_ancestor_that_added_node( // TODO: turn this in a pallet storage item recording block-num when pallet was activated. let block_offset = 0; - let leaf_idx = NodesUtils::leaf_index_that_added_node(pos); - let block_num: ::BlockNumber = - u32::try_from(leaf_idx + block_offset) + let ancestor_leaf_idx = NodesUtils::leaf_index_that_added_node(pos); + // leafs are zero-indexed while block numbers are one-indexed, + // so `parent_block_number == (base + current_leaf_index)`. + let parent_block_num: ::BlockNumber = + u32::try_from(block_offset + ancestor_leaf_idx) .expect("leaf-idx < block-num; qed") .into(); // TODO: I think this only holds recent history, so old block hashes might not be here. - let hash = >::block_hash(block_num); + let parent_hash = >::block_hash(parent_block_num); info!( target: "runtime::mmr", "🥩: parent of ancestor that added {}: leaf idx {}, block-num {:?} hash {:?}", - pos, leaf_idx, block_num, hash + pos, ancestor_leaf_idx, parent_block_num, parent_hash ); - hash + parent_hash } impl mmr_lib::MMRStore> for Storage From db41f813f7b641e879e7e555bdac42194f27bcef Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 22 Jun 2022 20:44:33 +0300 Subject: [PATCH 06/31] account for block offset of pallet activation --- .../merkle-mountain-range/src/mmr/storage.rs | 57 ++++++++++++------- 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index f60a7955b6ed0..3058b47335750 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -22,6 +22,7 @@ use frame_support::log; use log::info; use mmr_lib::helper; use sp_io::offchain_index; +use sp_runtime::traits::Saturating; use sp_std::iter::Peekable; #[cfg(not(feature = "std"))] use sp_std::prelude::*; @@ -60,27 +61,43 @@ impl Default for Storage { } } -fn parent_hash_of_ancestor_that_added_node( - pos: NodeIndex, -) -> ::Hash { - // TODO: turn this in a pallet storage item recording block-num when pallet was activated. - let block_offset = 0; - - let ancestor_leaf_idx = NodesUtils::leaf_index_that_added_node(pos); - // leafs are zero-indexed while block numbers are one-indexed, - // so `parent_block_number == (base + current_leaf_index)`. - let parent_block_num: ::BlockNumber = - u32::try_from(block_offset + ancestor_leaf_idx) +impl Storage +where + T: Config, + I: 'static, + L: primitives::FullLeaf + codec::Decode, +{ + fn parent_hash_of_ancestor_that_added_node( + pos: NodeIndex, + ) -> ::Hash { + let leaves_count: ::BlockNumber = + u32::try_from(NumberOfLeaves::::get()) + .expect("leaf-idx < block-num; qed") + .into(); + let ancestor_leaf_idx = u32::try_from(NodesUtils::leaf_index_that_added_node(pos)) .expect("leaf-idx < block-num; qed") .into(); - // TODO: I think this only holds recent history, so old block hashes might not be here. - let parent_hash = >::block_hash(parent_block_num); - info!( - target: "runtime::mmr", - "🥩: parent of ancestor that added {}: leaf idx {}, block-num {:?} hash {:?}", - pos, ancestor_leaf_idx, parent_block_num, parent_hash - ); - parent_hash + // leaves are zero-indexed and were added one per block since pallet activation, + // while block numbers are one-indexed, so block number that added `leaf_idx` is: + // `block_num = block_num_when_pallet_activated + leaf_idx + 1` + // `block_num = (current_block_num - leaves_count) + leaf_idx + 1` + // `parent_block_num = current_block_num - leaves_count + leaf_idx`. + let parent_block_num: ::BlockNumber = + >::block_number() + .saturating_sub(leaves_count) + .saturating_add(ancestor_leaf_idx); + + // TODO: I think this only holds recent history, so old block hashes might not be here. + let parent_hash = >::block_hash(parent_block_num); + info!( + target: "runtime::mmr", + "🥩: parent of ancestor that added {}: leaf idx {:?}, block-num {:?} (block offset {:?}) hash {:?}", + pos, ancestor_leaf_idx, parent_block_num, + >::block_number().saturating_sub(leaves_count), + parent_hash + ); + parent_hash + } } impl mmr_lib::MMRStore> for Storage @@ -93,7 +110,7 @@ where // Get the parent hash of the ancestor block that added node at index `pos`. // Use the hash as extra identifier to differentiate between various `pos` entries // in offchain DB coming from various chain forks. - let parent_hash_of_ancestor = parent_hash_of_ancestor_that_added_node::(pos); + let parent_hash_of_ancestor = Self::parent_hash_of_ancestor_that_added_node(pos); let key = Pallet::::offchain_key(parent_hash_of_ancestor, pos); info!( target: "runtime::mmr", From bfd4142c696b7d87f8d8acbaca24df685303c046 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 22 Jun 2022 21:36:27 +0300 Subject: [PATCH 07/31] add support for finding all nodes added by leaf --- .../merkle-mountain-range/src/mmr/storage.rs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 3058b47335750..7a0c0ad3f93cd 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -22,6 +22,7 @@ use frame_support::log; use log::info; use mmr_lib::helper; use sp_io::offchain_index; +use sp_mmr_primitives::LeafIndex; use sp_runtime::traits::Saturating; use sp_std::iter::Peekable; #[cfg(not(feature = "std"))] @@ -98,6 +99,32 @@ where ); parent_hash } + + fn nodes_added_by_leaf(leaf_index: LeafIndex) -> Vec { + let leaves = NumberOfLeaves::::get(); + let mmr_size = NodesUtils::new(leaves).size(); + + let pos = helper::leaf_index_to_pos(leaf_index); + let leaf_height = helper::pos_height_in_tree(pos); // either 0 or 1, not sure, but will hardcode once I confirm. + info!( + target: "runtime::mmr", + "🥩: leaf idx {} pos {} height {}", + leaf_index, pos, leaf_height + ); + + let mut nodes_added_by_leaf = vec![pos]; + let mut next_pos = pos + 1; + while next_pos < mmr_size && helper::pos_height_in_tree(next_pos) > leaf_height { + nodes_added_by_leaf.push(next_pos); + next_pos += 1; + } + info!( + target: "runtime::mmr", + "🥩: nodes_added_by_leaf(idx {}, pos {}): {:?}", + leaf_index, pos, nodes_added_by_leaf + ); + return nodes_added_by_leaf + } } impl mmr_lib::MMRStore> for Storage From 417de11b0b5574a1d56fb3053b234887728a77c9 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 23 Jun 2022 15:17:31 +0300 Subject: [PATCH 08/31] minor improvements --- .../merkle-mountain-range/src/mmr/storage.rs | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 7a0c0ad3f93cd..0d15d45237e37 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -18,12 +18,14 @@ //! A MMR storage implementations. use codec::Encode; -use frame_support::log; -use log::info; +use frame_support::{log::info, traits::Get}; use mmr_lib::helper; use sp_io::offchain_index; use sp_mmr_primitives::LeafIndex; -use sp_runtime::traits::Saturating; +use sp_runtime::{ + traits::{One, Saturating}, + SaturatedConversion, +}; use sp_std::iter::Peekable; #[cfg(not(feature = "std"))] use sp_std::prelude::*; @@ -71,13 +73,8 @@ where fn parent_hash_of_ancestor_that_added_node( pos: NodeIndex, ) -> ::Hash { - let leaves_count: ::BlockNumber = - u32::try_from(NumberOfLeaves::::get()) - .expect("leaf-idx < block-num; qed") - .into(); - let ancestor_leaf_idx = u32::try_from(NodesUtils::leaf_index_that_added_node(pos)) - .expect("leaf-idx < block-num; qed") - .into(); + let leaves_count = NumberOfLeaves::::get().saturated_into(); + let ancestor_leaf_idx = NodesUtils::leaf_index_that_added_node(pos).saturated_into(); // leaves are zero-indexed and were added one per block since pallet activation, // while block numbers are one-indexed, so block number that added `leaf_idx` is: // `block_num = block_num_when_pallet_activated + leaf_idx + 1` @@ -103,18 +100,11 @@ where fn nodes_added_by_leaf(leaf_index: LeafIndex) -> Vec { let leaves = NumberOfLeaves::::get(); let mmr_size = NodesUtils::new(leaves).size(); - let pos = helper::leaf_index_to_pos(leaf_index); - let leaf_height = helper::pos_height_in_tree(pos); // either 0 or 1, not sure, but will hardcode once I confirm. - info!( - target: "runtime::mmr", - "🥩: leaf idx {} pos {} height {}", - leaf_index, pos, leaf_height - ); let mut nodes_added_by_leaf = vec![pos]; let mut next_pos = pos + 1; - while next_pos < mmr_size && helper::pos_height_in_tree(next_pos) > leaf_height { + while next_pos < mmr_size && helper::pos_height_in_tree(next_pos) > 0 { nodes_added_by_leaf.push(next_pos); next_pos += 1; } From 25468f9e6b306eaeac67f18084c0d60f2bd6d163 Mon Sep 17 00:00:00 2001 From: Robert Hambrock Date: Thu, 23 Jun 2022 13:51:03 +0200 Subject: [PATCH 09/31] add helper to return all nodes added to mmr with a leaf append --- frame/merkle-mountain-range/src/mmr/utils.rs | 31 ++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/frame/merkle-mountain-range/src/mmr/utils.rs b/frame/merkle-mountain-range/src/mmr/utils.rs index 79aea819dbd04..c5fe6e27ae4c7 100644 --- a/frame/merkle-mountain-range/src/mmr/utils.rs +++ b/frame/merkle-mountain-range/src/mmr/utils.rs @@ -92,6 +92,15 @@ impl NodesUtils { } pos } + + // Starting from any leaf position, get the sequence of positions of the nodes added + // to the mmr when this leaf was added (inclusive of the leaf's position itself). + // That is, all of these nodes are right children of their respective parents. + fn right_branch_ending_in_leaf(leaf_pos: NodeIndex) -> Vec { + let leaf_index = Self::leaf_node_index_to_leaf_index(leaf_pos); + let num_parents = leaf_index.trailing_ones() as u64; + return (leaf_pos..=leaf_pos + num_parents).rev().collect(); + } } #[cfg(test)] @@ -107,6 +116,28 @@ mod tests { } } + #[test] + fn should_calculate_right_branch_correctly() { + fn left_jump_sequence(pos: u64) -> Vec { + let mut right_branch_ending_in_leaf = vec![pos]; + let mut next_pos = pos + 1; + while mmr_lib::helper::pos_height_in_tree(next_pos) > 0 { + right_branch_ending_in_leaf.push(next_pos); + next_pos += 1; + } + right_branch_ending_in_leaf.reverse(); + right_branch_ending_in_leaf + } + + (0..100000u64).for_each(|leaf_index| { + let pos = mmr_lib::helper::leaf_index_to_pos(leaf_index); + assert_eq!( + NodesUtils::right_branch_ending_in_leaf(pos), + left_jump_sequence(pos) + ); + }); + } + #[test] fn should_calculate_number_of_leaves_correctly() { assert_eq!( From 19b35eeb0c196d3b3483f9d86615693df617a96a Mon Sep 17 00:00:00 2001 From: Robert Hambrock Date: Thu, 23 Jun 2022 14:30:46 +0200 Subject: [PATCH 10/31] simplify leaf_node_index_to_leaf_index summing the (shifted) differences in peak positions adds up to the (shifted) final position, so don't need to fold over positions. --- frame/merkle-mountain-range/src/mmr/utils.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/frame/merkle-mountain-range/src/mmr/utils.rs b/frame/merkle-mountain-range/src/mmr/utils.rs index c5fe6e27ae4c7..aad0747bd07dd 100644 --- a/frame/merkle-mountain-range/src/mmr/utils.rs +++ b/frame/merkle-mountain-range/src/mmr/utils.rs @@ -65,16 +65,8 @@ impl NodesUtils { if pos == 0 { return 0 } - let (leaf_count, _) = - mmr_lib::helper::get_peaks(pos) - .iter() - .fold((0, 0), |(mut acc, last_peak), peak| { - let leaves = (peak - last_peak) >> 1; - acc += leaves + 1; - // last_peak, leaves, acc); - (acc, peak.clone()) - }); - leaf_count + let peaks = mmr_lib::helper::get_peaks(pos); + (pos + peaks.len() as u64) >> 1 } // Starting from any node position get position of rightmost leaf; this is the leaf From f9f202ac9575d5beeaf7ba46d793245cbda4c445 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 23 Jun 2022 16:31:34 +0300 Subject: [PATCH 11/31] dead fish: this also doesn't work The idea was to keep a rolling window of `(parent_hash, pos)` leaf entries in the offchain db, with the window matching the one that provides `block_num -> block_hash` mappings in `frame_system`. Once a leaf exits the window it would be "canonicalized" by switching its offchain db key from `(parent_hash, pos)` to simple `pos`. This doesn't work however because there's no way to get leaf contents from offchain db while in runtime context.. so no way to get+clear+set leaf to change its key in offchain db. Ideas: 1. move the "canonicalization" logic to offchain worker 2. enhance IndexingApi with "offchain::move(old_key, new_key)" This is weird, but correct, deterministic and safe AFAICT, so it could be exposed to runtime. --- frame/merkle-mountain-range/src/lib.rs | 7 ++ .../merkle-mountain-range/src/mmr/storage.rs | 83 +++++++++++++------ 2 files changed, 63 insertions(+), 27 deletions(-) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index 6e6d39100b52b..80974018f9773 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -270,6 +270,13 @@ impl, I: 'static> Pallet { (T::INDEXING_PREFIX, parent_hash, pos).encode() } + /// Build finalized offchain key for node `pos` in MMR. + /// + /// Used for nodes added by now finalized blocks. + fn final_offchain_key(pos: NodeIndex) -> sp_std::prelude::Vec { + (T::INDEXING_PREFIX, pos).encode() + } + /// Generate a MMR proof for the given `leaf_indices`. /// /// Note this method can only be used from an off-chain context diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 0d15d45237e37..0e17ae598b51a 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -23,7 +23,7 @@ use mmr_lib::helper; use sp_io::offchain_index; use sp_mmr_primitives::LeafIndex; use sp_runtime::{ - traits::{One, Saturating}, + traits::{One, Saturating, UniqueSaturatedInto}, SaturatedConversion, }; use sp_std::iter::Peekable; @@ -68,13 +68,15 @@ impl Storage where T: Config, I: 'static, - L: primitives::FullLeaf + codec::Decode, + L: primitives::FullLeaf, { - fn parent_hash_of_ancestor_that_added_node( - pos: NodeIndex, + /// Provide the parent hash for the block that added `leaf_index` to the MMR. + /// + /// Should only be called for blocks still available in `>::block_hash`. + fn parent_hash_of_leaf( + leaf_index: LeafIndex, + leaves_count: LeafIndex, ) -> ::Hash { - let leaves_count = NumberOfLeaves::::get().saturated_into(); - let ancestor_leaf_idx = NodesUtils::leaf_index_that_added_node(pos).saturated_into(); // leaves are zero-indexed and were added one per block since pallet activation, // while block numbers are one-indexed, so block number that added `leaf_idx` is: // `block_num = block_num_when_pallet_activated + leaf_idx + 1` @@ -82,24 +84,13 @@ where // `parent_block_num = current_block_num - leaves_count + leaf_idx`. let parent_block_num: ::BlockNumber = >::block_number() - .saturating_sub(leaves_count) - .saturating_add(ancestor_leaf_idx); - - // TODO: I think this only holds recent history, so old block hashes might not be here. - let parent_hash = >::block_hash(parent_block_num); - info!( - target: "runtime::mmr", - "🥩: parent of ancestor that added {}: leaf idx {:?}, block-num {:?} (block offset {:?}) hash {:?}", - pos, ancestor_leaf_idx, parent_block_num, - >::block_number().saturating_sub(leaves_count), - parent_hash - ); - parent_hash + .saturating_sub(leaves_count.saturated_into()) + .saturating_add(leaf_index.saturated_into()); + >::block_hash(parent_block_num) } - fn nodes_added_by_leaf(leaf_index: LeafIndex) -> Vec { - let leaves = NumberOfLeaves::::get(); - let mmr_size = NodesUtils::new(leaves).size(); + fn nodes_added_by_leaf(leaf_index: LeafIndex, leaves_count: LeafIndex) -> Vec { + let mmr_size = NodesUtils::new(leaves_count).size(); let pos = helper::leaf_index_to_pos(leaf_index); let mut nodes_added_by_leaf = vec![pos]; @@ -124,15 +115,17 @@ where L: primitives::FullLeaf + codec::Decode, { fn get_elem(&self, pos: NodeIndex) -> mmr_lib::Result>> { + let leaves_count = NumberOfLeaves::::get(); // Get the parent hash of the ancestor block that added node at index `pos`. // Use the hash as extra identifier to differentiate between various `pos` entries // in offchain DB coming from various chain forks. - let parent_hash_of_ancestor = Self::parent_hash_of_ancestor_that_added_node(pos); + let ancestor_leaf_idx = NodesUtils::leaf_index_that_added_node(pos); + let parent_hash_of_ancestor = Self::parent_hash_of_leaf(ancestor_leaf_idx, leaves_count); let key = Pallet::::offchain_key(parent_hash_of_ancestor, pos); info!( target: "runtime::mmr", - "🥩: get elem {}: key {:?}", - pos, key + "🥩: parent of ancestor that added {}: leaf idx {:?}, hash {:?}, key {:?}", + pos, ancestor_leaf_idx, parent_hash_of_ancestor, key ); // Retrieve the element from Off-chain DB. Ok(sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) @@ -191,15 +184,51 @@ where let parent_hash = >::parent_hash(); let block_number = >::block_number(); for elem in elems { + if helper::pos_height_in_tree(node_index) == 0 { + info!( + target: "runtime::mmr", + "🥩: appending leaf {}, pos {}, block-num {:?}", + leaf_index, node_index, block_number + ); + } else { + info!( + target: "runtime::mmr", + "🥩: appending node pos {}, block-num {:?}", + node_index, block_number + ); + } let key = Pallet::::offchain_key(parent_hash, node_index); info!( target: "runtime::mmr", - "🥩: offchain set: block-num {:?}, parent_hash {:?} node-idx {} key {:?}", - block_number, parent_hash, node_index, key + "🥩: offchain set: pos {} parent_hash {:?} key {:?}", + parent_hash, node_index, key ); // Indexing API is used to store the full node content (both leaf and inner). elem.using_encoded(|elem| offchain_index::set(&key, elem)); + // Now we need to "canonicalize" any old leaves. + let window_size = ::BlockHashCount::get() + .saturating_sub(One::one()) + .unique_saturated_into(); + if leaf_index >= window_size && helper::pos_height_in_tree(node_index) == 0 { + let leaves = leaf_index + 1; + // move the rolling window for `block_num->hash` mappings available in the runtime: + // on every newly added _leaf_, we "canonicalize" the oldest leaf in the offchain db + // still using `(parent_hash, pos)` key. + let leaf_to_canon = leaf_index.saturating_sub(window_size); + let parent_hash_of_leaf = Self::parent_hash_of_leaf(leaf_to_canon, leaves); + let nodes_to_canon = Self::nodes_added_by_leaf(leaf_to_canon, leaves); + for pos in nodes_to_canon { + let key = Pallet::::offchain_key(parent_hash_of_leaf, pos); + // FIXME: this approach also doesn't work because we can't read from offchain db + // in runtime context :(( + let elem = vec![]; // this should have been: `offchain_index::get(&key)` + offchain_index::clear(&key); + let canon_key = Pallet::::final_offchain_key(pos); + offchain_index::set(&canon_key, &elem); + } + } + // On-chain we are going to only store new peaks. if peaks_to_store.next_if_eq(&node_index).is_some() { >::insert(node_index, elem.hash()); From 05c5f2675ab9c027e7e7f27a397a61a460cefaa8 Mon Sep 17 00:00:00 2001 From: Robert Hambrock Date: Thu, 23 Jun 2022 16:18:58 +0200 Subject: [PATCH 12/31] simplify rightmost_leaf_node_index_from_pos --- frame/merkle-mountain-range/src/mmr/utils.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/frame/merkle-mountain-range/src/mmr/utils.rs b/frame/merkle-mountain-range/src/mmr/utils.rs index aad0747bd07dd..5f0f45ac5732c 100644 --- a/frame/merkle-mountain-range/src/mmr/utils.rs +++ b/frame/merkle-mountain-range/src/mmr/utils.rs @@ -73,16 +73,8 @@ impl NodesUtils { // responsible for the addition of node `pos`. fn rightmost_leaf_node_index_from_pos(mut pos: NodeIndex) -> NodeIndex { use mmr_lib::helper::pos_height_in_tree; - if pos > 0 { - let mut current_height = pos_height_in_tree(pos); - let mut right_child_height = pos_height_in_tree(pos - 1); - while right_child_height < current_height { - pos = pos - 1; - current_height = right_child_height; - right_child_height = pos_height_in_tree(pos - 1); - } - } - pos + let mut current_height = pos_height_in_tree(pos); + pos - (pos_height_in_tree(pos) as u64) } // Starting from any leaf position, get the sequence of positions of the nodes added From e15810d693b3272bab9680e839cf50ae99921dd7 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 24 Jun 2022 12:21:26 +0300 Subject: [PATCH 13/31] minor fix --- frame/merkle-mountain-range/src/mmr/utils.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/merkle-mountain-range/src/mmr/utils.rs b/frame/merkle-mountain-range/src/mmr/utils.rs index 5f0f45ac5732c..ab6833b05599c 100644 --- a/frame/merkle-mountain-range/src/mmr/utils.rs +++ b/frame/merkle-mountain-range/src/mmr/utils.rs @@ -71,9 +71,8 @@ impl NodesUtils { // Starting from any node position get position of rightmost leaf; this is the leaf // responsible for the addition of node `pos`. - fn rightmost_leaf_node_index_from_pos(mut pos: NodeIndex) -> NodeIndex { + fn rightmost_leaf_node_index_from_pos(pos: NodeIndex) -> NodeIndex { use mmr_lib::helper::pos_height_in_tree; - let mut current_height = pos_height_in_tree(pos); pos - (pos_height_in_tree(pos) as u64) } From f803ce9a51e27ff2de3f126123378050fbb775f5 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 24 Jun 2022 13:59:29 +0300 Subject: [PATCH 14/31] move leaf canonicalization to offchain worker --- frame/merkle-mountain-range/src/lib.rs | 65 +++++++++++++- .../merkle-mountain-range/src/mmr/storage.rs | 84 +++---------------- frame/merkle-mountain-range/src/mmr/utils.rs | 29 +++---- 3 files changed, 87 insertions(+), 91 deletions(-) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index 80974018f9773..441fe20d0ac79 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -58,7 +58,10 @@ use codec::Encode; use frame_support::{log::info, weights::Weight}; -use sp_runtime::traits::{self, One, Saturating}; +use sp_runtime::{ + traits::{self, One, Saturating, UniqueSaturatedInto}, + SaturatedConversion, +}; #[cfg(any(feature = "runtime-benchmarks", test))] mod benchmarking; @@ -223,6 +226,47 @@ pub mod pallet { T::WeightInfo::on_initialize(peaks_before.max(peaks_after)) } + + fn offchain_worker(_n: T::BlockNumber) { + use sp_core::offchain::StorageKind; + use sp_io::offchain; + info!("Hello World from MMR offchain worker!"); + + let leaves = NumberOfLeaves::::get(); + // our window is one less because we need the parent hash too. + let window_size = ::BlockHashCount::get() + .saturating_sub(One::one()) + .unique_saturated_into(); + if leaves >= window_size { + // move the rolling window towards the end of `block_num->hash` mappings available + // in the runtime: we "canonicalize" the leaf at the end. + let leaf_to_canon = leaves.saturating_sub(window_size); + let parent_hash_of_leaf = Self::parent_hash_of_leaf(leaf_to_canon, leaves); + let nodes_to_canon = + mmr::utils::NodesUtils::right_branch_ending_in_leaf(leaf_to_canon); + info!( + target: "runtime::mmr", + "🥩: nodes to canon for leaf {}: {:?}", + leaf_to_canon, nodes_to_canon + ); + for pos in nodes_to_canon { + let key = Pallet::::offchain_key(parent_hash_of_leaf, pos); + let canon_key = Pallet::::final_offchain_key(pos); + info!( + target: "runtime::mmr", + "🥩: move elem at pos {} from key {:?} to canon key {:?}", + pos, key, canon_key + ); + // Retrieve the element from Off-chain DB. + if let Some(elem) = offchain::local_storage_get(StorageKind::PERSISTENT, &key) { + // Delete entry with old key. + offchain::local_storage_clear(StorageKind::PERSISTENT, &key); + // Add under new key. + offchain::local_storage_set(StorageKind::PERSISTENT, &key, &elem); + } + } + } + } } } @@ -277,6 +321,25 @@ impl, I: 'static> Pallet { (T::INDEXING_PREFIX, pos).encode() } + /// Provide the parent hash for the block that added `leaf_index` to the MMR. + /// + /// Should only be called for blocks still available in `>::block_hash`. + fn parent_hash_of_leaf( + leaf_index: LeafIndex, + leaves_count: LeafIndex, + ) -> ::Hash { + // leaves are zero-indexed and were added one per block since pallet activation, + // while block numbers are one-indexed, so block number that added `leaf_idx` is: + // `block_num = block_num_when_pallet_activated + leaf_idx + 1` + // `block_num = (current_block_num - leaves_count) + leaf_idx + 1` + // `parent_block_num = current_block_num - leaves_count + leaf_idx`. + let parent_block_num: ::BlockNumber = + >::block_number() + .saturating_sub(leaves_count.saturated_into()) + .saturating_add(leaf_index.saturated_into()); + >::block_hash(parent_block_num) + } + /// Generate a MMR proof for the given `leaf_indices`. /// /// Note this method can only be used from an off-chain context diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 0e17ae598b51a..e6bf728f093e7 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -18,14 +18,9 @@ //! A MMR storage implementations. use codec::Encode; -use frame_support::{log::info, traits::Get}; +use frame_support::log::info; use mmr_lib::helper; use sp_io::offchain_index; -use sp_mmr_primitives::LeafIndex; -use sp_runtime::{ - traits::{One, Saturating, UniqueSaturatedInto}, - SaturatedConversion, -}; use sp_std::iter::Peekable; #[cfg(not(feature = "std"))] use sp_std::prelude::*; @@ -64,50 +59,6 @@ impl Default for Storage { } } -impl Storage -where - T: Config, - I: 'static, - L: primitives::FullLeaf, -{ - /// Provide the parent hash for the block that added `leaf_index` to the MMR. - /// - /// Should only be called for blocks still available in `>::block_hash`. - fn parent_hash_of_leaf( - leaf_index: LeafIndex, - leaves_count: LeafIndex, - ) -> ::Hash { - // leaves are zero-indexed and were added one per block since pallet activation, - // while block numbers are one-indexed, so block number that added `leaf_idx` is: - // `block_num = block_num_when_pallet_activated + leaf_idx + 1` - // `block_num = (current_block_num - leaves_count) + leaf_idx + 1` - // `parent_block_num = current_block_num - leaves_count + leaf_idx`. - let parent_block_num: ::BlockNumber = - >::block_number() - .saturating_sub(leaves_count.saturated_into()) - .saturating_add(leaf_index.saturated_into()); - >::block_hash(parent_block_num) - } - - fn nodes_added_by_leaf(leaf_index: LeafIndex, leaves_count: LeafIndex) -> Vec { - let mmr_size = NodesUtils::new(leaves_count).size(); - let pos = helper::leaf_index_to_pos(leaf_index); - - let mut nodes_added_by_leaf = vec![pos]; - let mut next_pos = pos + 1; - while next_pos < mmr_size && helper::pos_height_in_tree(next_pos) > 0 { - nodes_added_by_leaf.push(next_pos); - next_pos += 1; - } - info!( - target: "runtime::mmr", - "🥩: nodes_added_by_leaf(idx {}, pos {}): {:?}", - leaf_index, pos, nodes_added_by_leaf - ); - return nodes_added_by_leaf - } -} - impl mmr_lib::MMRStore> for Storage where T: Config, @@ -120,7 +71,8 @@ where // Use the hash as extra identifier to differentiate between various `pos` entries // in offchain DB coming from various chain forks. let ancestor_leaf_idx = NodesUtils::leaf_index_that_added_node(pos); - let parent_hash_of_ancestor = Self::parent_hash_of_leaf(ancestor_leaf_idx, leaves_count); + let parent_hash_of_ancestor = + Pallet::::parent_hash_of_leaf(ancestor_leaf_idx, leaves_count); let key = Pallet::::offchain_key(parent_hash_of_ancestor, pos); info!( target: "runtime::mmr", @@ -197,6 +149,13 @@ where node_index, block_number ); } + // For now we store this leaf offchain keyed by `(parent_hash, node_index)` + // to make it fork-resistant. + // Offchain worker task will "canonicalize" it `frame_system::BlockHashCount` blocks + // later when we are not worried about forks anymore (highly unlikely to have a fork + // in the chain that deep). + // "Canonicalization" in this case means moving this leaf under a new key based + // only on the leaf's `node_index`. let key = Pallet::::offchain_key(parent_hash, node_index); info!( target: "runtime::mmr", @@ -206,29 +165,6 @@ where // Indexing API is used to store the full node content (both leaf and inner). elem.using_encoded(|elem| offchain_index::set(&key, elem)); - // Now we need to "canonicalize" any old leaves. - let window_size = ::BlockHashCount::get() - .saturating_sub(One::one()) - .unique_saturated_into(); - if leaf_index >= window_size && helper::pos_height_in_tree(node_index) == 0 { - let leaves = leaf_index + 1; - // move the rolling window for `block_num->hash` mappings available in the runtime: - // on every newly added _leaf_, we "canonicalize" the oldest leaf in the offchain db - // still using `(parent_hash, pos)` key. - let leaf_to_canon = leaf_index.saturating_sub(window_size); - let parent_hash_of_leaf = Self::parent_hash_of_leaf(leaf_to_canon, leaves); - let nodes_to_canon = Self::nodes_added_by_leaf(leaf_to_canon, leaves); - for pos in nodes_to_canon { - let key = Pallet::::offchain_key(parent_hash_of_leaf, pos); - // FIXME: this approach also doesn't work because we can't read from offchain db - // in runtime context :(( - let elem = vec![]; // this should have been: `offchain_index::get(&key)` - offchain_index::clear(&key); - let canon_key = Pallet::::final_offchain_key(pos); - offchain_index::set(&canon_key, &elem); - } - } - // On-chain we are going to only store new peaks. if peaks_to_store.next_if_eq(&node_index).is_some() { >::insert(node_index, elem.hash()); diff --git a/frame/merkle-mountain-range/src/mmr/utils.rs b/frame/merkle-mountain-range/src/mmr/utils.rs index ab6833b05599c..4aac3159f352e 100644 --- a/frame/merkle-mountain-range/src/mmr/utils.rs +++ b/frame/merkle-mountain-range/src/mmr/utils.rs @@ -18,6 +18,7 @@ //! Merkle Mountain Range utilities. use crate::primitives::{LeafIndex, NodeIndex}; +use mmr_lib::helper; /// MMR nodes & size -related utilities. pub struct NodesUtils { @@ -65,34 +66,33 @@ impl NodesUtils { if pos == 0 { return 0 } - let peaks = mmr_lib::helper::get_peaks(pos); + let peaks = helper::get_peaks(pos); (pos + peaks.len() as u64) >> 1 } // Starting from any node position get position of rightmost leaf; this is the leaf // responsible for the addition of node `pos`. fn rightmost_leaf_node_index_from_pos(pos: NodeIndex) -> NodeIndex { - use mmr_lib::helper::pos_height_in_tree; - pos - (pos_height_in_tree(pos) as u64) + pos - (helper::pos_height_in_tree(pos) as u64) } - // Starting from any leaf position, get the sequence of positions of the nodes added - // to the mmr when this leaf was added (inclusive of the leaf's position itself). - // That is, all of these nodes are right children of their respective parents. - fn right_branch_ending_in_leaf(leaf_pos: NodeIndex) -> Vec { - let leaf_index = Self::leaf_node_index_to_leaf_index(leaf_pos); + /// Starting from any leaf index, get the sequence of positions of the nodes added + /// to the mmr when this leaf was added (inclusive of the leaf's position itself). + /// That is, all of these nodes are right children of their respective parents. + pub fn right_branch_ending_in_leaf(leaf_index: LeafIndex) -> Vec { + let pos = helper::leaf_index_to_pos(leaf_index); let num_parents = leaf_index.trailing_ones() as u64; - return (leaf_pos..=leaf_pos + num_parents).rev().collect(); + return (pos..=pos + num_parents).collect() } } #[cfg(test)] mod tests { use super::*; + use mmr_lib::helper::leaf_index_to_pos; #[test] fn test_leaf_node_index_to_leaf_index() { - use mmr_lib::helper::leaf_index_to_pos; for index in 0..100000 { let pos = leaf_index_to_pos(index); assert_eq!(NodesUtils::leaf_node_index_to_leaf_index(pos), index); @@ -101,23 +101,20 @@ mod tests { #[test] fn should_calculate_right_branch_correctly() { - fn left_jump_sequence(pos: u64) -> Vec { + fn left_jump_sequence(leaf_index: LeafIndex) -> Vec { + let pos = leaf_index_to_pos(leaf_index); let mut right_branch_ending_in_leaf = vec![pos]; let mut next_pos = pos + 1; while mmr_lib::helper::pos_height_in_tree(next_pos) > 0 { right_branch_ending_in_leaf.push(next_pos); next_pos += 1; } - right_branch_ending_in_leaf.reverse(); right_branch_ending_in_leaf } (0..100000u64).for_each(|leaf_index| { let pos = mmr_lib::helper::leaf_index_to_pos(leaf_index); - assert_eq!( - NodesUtils::right_branch_ending_in_leaf(pos), - left_jump_sequence(pos) - ); + assert_eq!(NodesUtils::right_branch_ending_in_leaf(pos), left_jump_sequence(pos)); }); } From 94e3b61590f3a8c5c4329df439673262a5445cf8 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 24 Jun 2022 14:24:56 +0300 Subject: [PATCH 15/31] move storage related code to storage.rs --- frame/merkle-mountain-range/src/lib.rs | 43 ++-------------- .../merkle-mountain-range/src/mmr/storage.rs | 49 ++++++++++++++++++- 2 files changed, 52 insertions(+), 40 deletions(-) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index 441fe20d0ac79..3b8b73e917e1e 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -59,7 +59,7 @@ use codec::Encode; use frame_support::{log::info, weights::Weight}; use sp_runtime::{ - traits::{self, One, Saturating, UniqueSaturatedInto}, + traits::{self, One, Saturating}, SaturatedConversion, }; @@ -228,44 +228,9 @@ pub mod pallet { } fn offchain_worker(_n: T::BlockNumber) { - use sp_core::offchain::StorageKind; - use sp_io::offchain; - info!("Hello World from MMR offchain worker!"); - - let leaves = NumberOfLeaves::::get(); - // our window is one less because we need the parent hash too. - let window_size = ::BlockHashCount::get() - .saturating_sub(One::one()) - .unique_saturated_into(); - if leaves >= window_size { - // move the rolling window towards the end of `block_num->hash` mappings available - // in the runtime: we "canonicalize" the leaf at the end. - let leaf_to_canon = leaves.saturating_sub(window_size); - let parent_hash_of_leaf = Self::parent_hash_of_leaf(leaf_to_canon, leaves); - let nodes_to_canon = - mmr::utils::NodesUtils::right_branch_ending_in_leaf(leaf_to_canon); - info!( - target: "runtime::mmr", - "🥩: nodes to canon for leaf {}: {:?}", - leaf_to_canon, nodes_to_canon - ); - for pos in nodes_to_canon { - let key = Pallet::::offchain_key(parent_hash_of_leaf, pos); - let canon_key = Pallet::::final_offchain_key(pos); - info!( - target: "runtime::mmr", - "🥩: move elem at pos {} from key {:?} to canon key {:?}", - pos, key, canon_key - ); - // Retrieve the element from Off-chain DB. - if let Some(elem) = offchain::local_storage_get(StorageKind::PERSISTENT, &key) { - // Delete entry with old key. - offchain::local_storage_clear(StorageKind::PERSISTENT, &key); - // Add under new key. - offchain::local_storage_set(StorageKind::PERSISTENT, &key, &elem); - } - } - } + use mmr::storage::{OffchainStorage, Storage}; + info!("Hello World from MMR offchain worker (block {:?})!", _n); + Storage::>::canonicalize_offchain(); } } } diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index e6bf728f093e7..0e5e6ed02849f 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -18,7 +18,7 @@ //! A MMR storage implementations. use codec::Encode; -use frame_support::log::info; +use frame_support::{log::info, traits::Get}; use mmr_lib::helper; use sp_io::offchain_index; use sp_std::iter::Peekable; @@ -59,6 +59,53 @@ impl Default for Storage { } } +impl Storage +where + T: Config, + I: 'static, + L: primitives::FullLeaf, +{ + pub fn canonicalize_offchain() { + use sp_core::offchain::StorageKind; + use sp_io::offchain; + use sp_runtime::traits::{One, Saturating, UniqueSaturatedInto}; + + let leaves = NumberOfLeaves::::get(); + // our window is one less because we need the parent hash too. + let window_size = ::BlockHashCount::get() + .saturating_sub(One::one()) + .unique_saturated_into(); + if leaves >= window_size { + // move the rolling window towards the end of `block_num->hash` mappings available + // in the runtime: we "canonicalize" the leaf at the end. + let leaf_to_canon = leaves.saturating_sub(window_size); + let parent_hash_of_leaf = Pallet::::parent_hash_of_leaf(leaf_to_canon, leaves); + let nodes_to_canon = NodesUtils::right_branch_ending_in_leaf(leaf_to_canon); + info!( + target: "runtime::mmr", + "🥩: nodes to canon for leaf {}: {:?}", + leaf_to_canon, nodes_to_canon + ); + for pos in nodes_to_canon { + let key = Pallet::::offchain_key(parent_hash_of_leaf, pos); + let canon_key = Pallet::::final_offchain_key(pos); + info!( + target: "runtime::mmr", + "🥩: move elem at pos {} from key {:?} to canon key {:?}", + pos, key, canon_key + ); + // Retrieve the element from Off-chain DB. + if let Some(elem) = offchain::local_storage_get(StorageKind::PERSISTENT, &key) { + // Delete entry with old key. + offchain::local_storage_clear(StorageKind::PERSISTENT, &key); + // Add under new key. + offchain::local_storage_set(StorageKind::PERSISTENT, &key, &elem); + } + } + } + } +} + impl mmr_lib::MMRStore> for Storage where T: Config, From ecb77a7535e8680bea86c67f21132db7f57edd2d Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 24 Jun 2022 15:07:43 +0300 Subject: [PATCH 16/31] on offchain reads use canonic key for old leaves --- frame/merkle-mountain-range/src/lib.rs | 2 +- frame/merkle-mountain-range/src/mmr/storage.rs | 13 +++++++++++-- frame/merkle-mountain-range/src/mmr/utils.rs | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index 3b8b73e917e1e..eb0bcfbf14e63 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -229,7 +229,7 @@ pub mod pallet { fn offchain_worker(_n: T::BlockNumber) { use mmr::storage::{OffchainStorage, Storage}; - info!("Hello World from MMR offchain worker (block {:?})!", _n); + info!("🥩 Run MMR offchain worker for block {:?}...", _n); Storage::>::canonicalize_offchain(); } } diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 0e5e6ed02849f..5fdebd6fe26fc 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -123,11 +123,20 @@ where let key = Pallet::::offchain_key(parent_hash_of_ancestor, pos); info!( target: "runtime::mmr", - "🥩: parent of ancestor that added {}: leaf idx {:?}, hash {:?}, key {:?}", + "🥩: offchain get {}: leaf idx {:?}, hash {:?}, key {:?}", pos, ancestor_leaf_idx, parent_hash_of_ancestor, key ); // Retrieve the element from Off-chain DB. Ok(sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) + .or_else(|| { + let key = Pallet::::final_offchain_key(pos); + info!( + target: "runtime::mmr", + "🥩: not found {}: try final key {:?}", + pos, key + ); + sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) + }) .and_then(|v| codec::Decode::decode(&mut &*v).ok())) } @@ -207,7 +216,7 @@ where info!( target: "runtime::mmr", "🥩: offchain set: pos {} parent_hash {:?} key {:?}", - parent_hash, node_index, key + node_index, parent_hash, key ); // Indexing API is used to store the full node content (both leaf and inner). elem.using_encoded(|elem| offchain_index::set(&key, elem)); diff --git a/frame/merkle-mountain-range/src/mmr/utils.rs b/frame/merkle-mountain-range/src/mmr/utils.rs index 4aac3159f352e..efc7267b7d36d 100644 --- a/frame/merkle-mountain-range/src/mmr/utils.rs +++ b/frame/merkle-mountain-range/src/mmr/utils.rs @@ -79,7 +79,7 @@ impl NodesUtils { /// Starting from any leaf index, get the sequence of positions of the nodes added /// to the mmr when this leaf was added (inclusive of the leaf's position itself). /// That is, all of these nodes are right children of their respective parents. - pub fn right_branch_ending_in_leaf(leaf_index: LeafIndex) -> Vec { + pub fn right_branch_ending_in_leaf(leaf_index: LeafIndex) -> crate::Vec { let pos = helper::leaf_index_to_pos(leaf_index); let num_parents = leaf_index.trailing_ones() as u64; return (pos..=pos + num_parents).collect() From a17cb650ed8c70adf5725887bea3aaa3e555abc3 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 24 Jun 2022 16:36:33 +0300 Subject: [PATCH 17/31] fix offchain worker write using canon key --- frame/merkle-mountain-range/src/mmr/storage.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 5fdebd6fe26fc..f8b041865a9d6 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -68,13 +68,11 @@ where pub fn canonicalize_offchain() { use sp_core::offchain::StorageKind; use sp_io::offchain; - use sp_runtime::traits::{One, Saturating, UniqueSaturatedInto}; + use sp_runtime::traits::UniqueSaturatedInto; let leaves = NumberOfLeaves::::get(); - // our window is one less because we need the parent hash too. - let window_size = ::BlockHashCount::get() - .saturating_sub(One::one()) - .unique_saturated_into(); + let window_size = + ::BlockHashCount::get().unique_saturated_into(); if leaves >= window_size { // move the rolling window towards the end of `block_num->hash` mappings available // in the runtime: we "canonicalize" the leaf at the end. @@ -99,7 +97,13 @@ where // Delete entry with old key. offchain::local_storage_clear(StorageKind::PERSISTENT, &key); // Add under new key. - offchain::local_storage_set(StorageKind::PERSISTENT, &key, &elem); + offchain::local_storage_set(StorageKind::PERSISTENT, &canon_key, &elem); + } else { + info!( + target: "runtime::mmr", + "🥩: offchain: could not get elem at pos {} using key {:?}", + pos, key + ); } } } From e9ab363dd42776d3d5ae2d75e79bb96d3fc6cb9c Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 24 Jun 2022 17:08:51 +0300 Subject: [PATCH 18/31] fix pallet-mmr tests --- frame/merkle-mountain-range/src/lib.rs | 4 +-- .../merkle-mountain-range/src/mmr/storage.rs | 4 +-- frame/merkle-mountain-range/src/tests.rs | 25 +++++++++++++++---- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index eb0bcfbf14e63..c4e0185bd2c9d 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -279,10 +279,10 @@ impl, I: 'static> Pallet { (T::INDEXING_PREFIX, parent_hash, pos).encode() } - /// Build finalized offchain key for node `pos` in MMR. + /// Build canonical offchain key for node `pos` in MMR. /// /// Used for nodes added by now finalized blocks. - fn final_offchain_key(pos: NodeIndex) -> sp_std::prelude::Vec { + fn canon_offchain_key(pos: NodeIndex) -> sp_std::prelude::Vec { (T::INDEXING_PREFIX, pos).encode() } diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index f8b041865a9d6..995a9088cc4b3 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -86,7 +86,7 @@ where ); for pos in nodes_to_canon { let key = Pallet::::offchain_key(parent_hash_of_leaf, pos); - let canon_key = Pallet::::final_offchain_key(pos); + let canon_key = Pallet::::canon_offchain_key(pos); info!( target: "runtime::mmr", "🥩: move elem at pos {} from key {:?} to canon key {:?}", @@ -133,7 +133,7 @@ where // Retrieve the element from Off-chain DB. Ok(sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) .or_else(|| { - let key = Pallet::::final_offchain_key(pos); + let key = Pallet::::canon_offchain_key(pos); info!( target: "runtime::mmr", "🥩: not found {}: try final key {:?}", diff --git a/frame/merkle-mountain-range/src/tests.rs b/frame/merkle-mountain-range/src/tests.rs index d025910a9ee5c..a3886e9a8feff 100644 --- a/frame/merkle-mountain-range/src/tests.rs +++ b/frame/merkle-mountain-range/src/tests.rs @@ -113,11 +113,16 @@ fn should_start_empty() { #[test] fn should_append_to_mmr_when_on_initialize_is_called() { + use std::{cell::RefCell, rc::Rc}; + let _ = env_logger::try_init(); + let parent_hashes = Rc::new(RefCell::new(Default::default())); + let ext_parent_hashes = parent_hashes.clone(); let mut ext = new_test_ext(); - ext.execute_with(|| { + ext.execute_with(move || { // when new_block(); + let parent_b1 = >::parent_hash(); // then assert_eq!(crate::NumberOfLeaves::::get(), 1); @@ -136,6 +141,7 @@ fn should_append_to_mmr_when_on_initialize_is_called() { // when new_block(); + let parent_b2 = >::parent_hash(); // then assert_eq!(crate::NumberOfLeaves::::get(), 2); @@ -157,26 +163,35 @@ fn should_append_to_mmr_when_on_initialize_is_called() { hex("672c04a9cd05a644789d769daa552d35d8de7c33129f8a7cbf49e595234c4854"), ) ); + + *ext_parent_hashes.borrow_mut() = (parent_b1, parent_b2); }); + let (parent_b1, parent_b2) = Rc::try_unwrap(parent_hashes).unwrap().into_inner(); + // make sure the leaves end up in the offchain DB ext.persist_offchain_overlay(); let offchain_db = ext.offchain_db(); assert_eq!( - offchain_db.get(&MMR::offchain_key(0)).map(decode_node), + offchain_db.get(&MMR::offchain_key(parent_b1, 0)).map(decode_node), Some(mmr::Node::Data(((0, H256::repeat_byte(1)), LeafData::new(1),))) ); assert_eq!( - offchain_db.get(&MMR::offchain_key(1)).map(decode_node), + offchain_db.get(&MMR::offchain_key(parent_b2, 1)).map(decode_node), Some(mmr::Node::Data(((1, H256::repeat_byte(2)), LeafData::new(2),))) ); assert_eq!( - offchain_db.get(&MMR::offchain_key(2)).map(decode_node), + offchain_db.get(&MMR::offchain_key(parent_b2, 2)).map(decode_node), Some(mmr::Node::Hash(hex( "672c04a9cd05a644789d769daa552d35d8de7c33129f8a7cbf49e595234c4854" ))) ); - assert_eq!(offchain_db.get(&MMR::offchain_key(3)), None); + assert_eq!(offchain_db.get(&MMR::offchain_key(parent_b2, 3)), None); + + assert_eq!(offchain_db.get(&MMR::canon_offchain_key(0)), None); + assert_eq!(offchain_db.get(&MMR::canon_offchain_key(1)), None); + assert_eq!(offchain_db.get(&MMR::canon_offchain_key(2)), None); + assert_eq!(offchain_db.get(&MMR::canon_offchain_key(3)), None); } #[test] From 8856db1a1b78613868d26218f7601ce0b8e779d0 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 27 Jun 2022 15:39:05 +0300 Subject: [PATCH 19/31] add documentation and fix logging --- frame/merkle-mountain-range/src/lib.rs | 20 ++-- .../merkle-mountain-range/src/mmr/storage.rs | 102 +++++++++--------- 2 files changed, 61 insertions(+), 61 deletions(-) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index c4e0185bd2c9d..cc9612d49ecef 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -57,7 +57,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use codec::Encode; -use frame_support::{log::info, weights::Weight}; +use frame_support::weights::Weight; use sp_runtime::{ traits::{self, One, Saturating}, SaturatedConversion, @@ -202,7 +202,6 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { fn on_initialize(_n: T::BlockNumber) -> Weight { - info!(target: "runtime::mmr", "🥩: initialize block {:?}", _n); use primitives::LeafDataProvider; let leaves = Self::mmr_leaves(); let peaks_before = mmr::utils::NodesUtils::new(leaves).number_of_peaks(); @@ -211,8 +210,6 @@ pub mod pallet { let mut mmr: ModuleMmr = mmr::Mmr::new(leaves); mmr.push(data).expect("MMR push never fails."); - info!(target: "runtime::mmr", "🥩: on_initialize(block-num: {:?}): leaves_before {}, peaks_before {}", _n, leaves, peaks_before); - // update the size let (leaves, root) = mmr.finalize().expect("MMR finalize never fails."); >::on_new_root(&root); @@ -222,14 +219,23 @@ pub mod pallet { let peaks_after = mmr::utils::NodesUtils::new(leaves).number_of_peaks(); - info!(target: "runtime::mmr", "🥩: on_initialize(block-num: {:?}): leaves {}, peaks {}", _n, leaves, peaks_after); - T::WeightInfo::on_initialize(peaks_before.max(peaks_after)) } fn offchain_worker(_n: T::BlockNumber) { use mmr::storage::{OffchainStorage, Storage}; - info!("🥩 Run MMR offchain worker for block {:?}...", _n); + // MMR pallet uses offchain storage to hold full MMR and leaves. + // The leaves are saved under fork-unique keys `(parent_hash, pos)`. + // MMR Runtime depends on `frame_system::block_hash(block_num)` mappings to find + // parent hashes for particular nodes or leaves. + // This MMR offchain worker function moves a rolling window of the same size + // as `frame_system::block_hash` map, where nodes/leaves added by blocks that are just + // about to exit the window are "canonicalized" so that their offchain key no longer + // depends on `parent_hash` therefore on access to `frame_system::block_hash`. + // + // This approach works to eliminate fork-induced leaf collisions in offchain db, + // under the assumption that no fork will be deeper than `frame_system::BlockHashCount` + // blocks (2400 blocks on Polkadot, Kusama, Rococo, etc). Storage::>::canonicalize_offchain(); } } diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 995a9088cc4b3..a78a3b4d86fb3 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -18,7 +18,7 @@ //! A MMR storage implementations. use codec::Encode; -use frame_support::{log::info, traits::Get}; +use frame_support::traits::Get; use mmr_lib::helper; use sp_io::offchain_index; use sp_std::iter::Peekable; @@ -65,11 +65,23 @@ where I: 'static, L: primitives::FullLeaf, { + /// Move nodes and leaves added by block `N` in offchain db from _fork-aware key_ to + /// _canonical key_, + /// where `N` is `frame_system::BlockHashCount` blocks behind current block number. + /// + /// This "canonicalization" process is required because the _fork-aware key_ value depends + /// on `frame_system::block_hash(block_num)` map which only holds the last + /// `frame_system::BlockHashCount` blocks. + /// + /// Should only be called from offchain context, because it requires both read and write + /// access to offchain db. pub fn canonicalize_offchain() { use sp_core::offchain::StorageKind; use sp_io::offchain; use sp_runtime::traits::UniqueSaturatedInto; + // Effectively move a rolling window of fork-unique leaves. Once out of the window, leaves + // are "canonicalized" in offchain db by moving them under `Pallet::canon_offchain_key`. let leaves = NumberOfLeaves::::get(); let window_size = ::BlockHashCount::get().unique_saturated_into(); @@ -79,31 +91,36 @@ where let leaf_to_canon = leaves.saturating_sub(window_size); let parent_hash_of_leaf = Pallet::::parent_hash_of_leaf(leaf_to_canon, leaves); let nodes_to_canon = NodesUtils::right_branch_ending_in_leaf(leaf_to_canon); - info!( - target: "runtime::mmr", - "🥩: nodes to canon for leaf {}: {:?}", - leaf_to_canon, nodes_to_canon - ); + sp_std::if_std! { + frame_support::log::debug!( + target: "runtime::mmr", "Nodes to canon for leaf {}: {:?}", + leaf_to_canon, nodes_to_canon + ); + } for pos in nodes_to_canon { let key = Pallet::::offchain_key(parent_hash_of_leaf, pos); let canon_key = Pallet::::canon_offchain_key(pos); - info!( - target: "runtime::mmr", - "🥩: move elem at pos {} from key {:?} to canon key {:?}", - pos, key, canon_key - ); - // Retrieve the element from Off-chain DB. + // Retrieve the element from Off-chain DB under fork-aware key. if let Some(elem) = offchain::local_storage_get(StorageKind::PERSISTENT, &key) { // Delete entry with old key. offchain::local_storage_clear(StorageKind::PERSISTENT, &key); - // Add under new key. + // Add under new canon key. offchain::local_storage_set(StorageKind::PERSISTENT, &canon_key, &elem); + sp_std::if_std! { + frame_support::log::debug!( + target: "runtime::mmr", + "Moved elem at pos {} from key {:?} to canon key {:?}", + pos, key, canon_key + ); + } } else { - info!( - target: "runtime::mmr", - "🥩: offchain: could not get elem at pos {} using key {:?}", - pos, key - ); + sp_std::if_std! { + frame_support::log::debug!( + target: "runtime::mmr", + "Offchain: could not get elem at pos {} using key {:?}", + pos, key + ); + } } } } @@ -125,20 +142,16 @@ where let parent_hash_of_ancestor = Pallet::::parent_hash_of_leaf(ancestor_leaf_idx, leaves_count); let key = Pallet::::offchain_key(parent_hash_of_ancestor, pos); - info!( - target: "runtime::mmr", - "🥩: offchain get {}: leaf idx {:?}, hash {:?}, key {:?}", - pos, ancestor_leaf_idx, parent_hash_of_ancestor, key - ); + sp_std::if_std! { + frame_support::log::debug!( + target: "runtime::mmr", "offchain get {}: leaf idx {:?}, hash {:?}, key {:?}", + pos, ancestor_leaf_idx, parent_hash_of_ancestor, key + ); + } // Retrieve the element from Off-chain DB. Ok(sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) .or_else(|| { let key = Pallet::::canon_offchain_key(pos); - info!( - target: "runtime::mmr", - "🥩: not found {}: try final key {:?}", - pos, key - ); sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) }) .and_then(|v| codec::Decode::decode(&mut &*v).ok())) @@ -165,18 +178,12 @@ where } sp_std::if_std! { - frame_support::log::info!("elems: {:?}", elems.iter().map(|elem| elem.hash()).collect::>()); + frame_support::log::trace!("elems: {:?}", elems.iter().map(|elem| elem.hash()).collect::>()); } let leaves = NumberOfLeaves::::get(); let size = NodesUtils::new(leaves).size(); - info!( - target: "runtime::mmr", - "🥩: append elem {}: leaves {} size {}", - pos, leaves, size - ); - if pos != size { return Err(mmr_lib::Error::InconsistentStore) } @@ -194,21 +201,7 @@ where // Use parent hash of block adding new nodes (this block) as extra identifier // in offchain DB to avoid DB collisions and overwrites in case of forks. let parent_hash = >::parent_hash(); - let block_number = >::block_number(); for elem in elems { - if helper::pos_height_in_tree(node_index) == 0 { - info!( - target: "runtime::mmr", - "🥩: appending leaf {}, pos {}, block-num {:?}", - leaf_index, node_index, block_number - ); - } else { - info!( - target: "runtime::mmr", - "🥩: appending node pos {}, block-num {:?}", - node_index, block_number - ); - } // For now we store this leaf offchain keyed by `(parent_hash, node_index)` // to make it fork-resistant. // Offchain worker task will "canonicalize" it `frame_system::BlockHashCount` blocks @@ -217,11 +210,12 @@ where // "Canonicalization" in this case means moving this leaf under a new key based // only on the leaf's `node_index`. let key = Pallet::::offchain_key(parent_hash, node_index); - info!( - target: "runtime::mmr", - "🥩: offchain set: pos {} parent_hash {:?} key {:?}", - node_index, parent_hash, key - ); + sp_std::if_std! { + frame_support::log::debug!( + target: "runtime::mmr", "offchain set: pos {} parent_hash {:?} key {:?}", + node_index, parent_hash, key + ); + } // Indexing API is used to store the full node content (both leaf and inner). elem.using_encoded(|elem| offchain_index::set(&key, elem)); From dfc5b4570f8682e7d24228067777666deb070921 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 28 Jun 2022 12:05:20 +0300 Subject: [PATCH 20/31] add offchain mmr canonicalization test --- frame/merkle-mountain-range/src/mmr/utils.rs | 15 +- frame/merkle-mountain-range/src/tests.rs | 138 ++++++++++++++++--- 2 files changed, 128 insertions(+), 25 deletions(-) diff --git a/frame/merkle-mountain-range/src/mmr/utils.rs b/frame/merkle-mountain-range/src/mmr/utils.rs index efc7267b7d36d..d1c371e7ed2d2 100644 --- a/frame/merkle-mountain-range/src/mmr/utils.rs +++ b/frame/merkle-mountain-range/src/mmr/utils.rs @@ -92,7 +92,7 @@ mod tests { use mmr_lib::helper::leaf_index_to_pos; #[test] - fn test_leaf_node_index_to_leaf_index() { + fn should_calculate_node_index_from_leaf_index() { for index in 0..100000 { let pos = leaf_index_to_pos(index); assert_eq!(NodesUtils::leaf_node_index_to_leaf_index(pos), index); @@ -112,10 +112,19 @@ mod tests { right_branch_ending_in_leaf } - (0..100000u64).for_each(|leaf_index| { + for leaf_index in 0..100000 { let pos = mmr_lib::helper::leaf_index_to_pos(leaf_index); assert_eq!(NodesUtils::right_branch_ending_in_leaf(pos), left_jump_sequence(pos)); - }); + } + } + + #[test] + fn should_calculate_rightmost_leaf_node_index_from_pos() { + for pos in 0..100000 { + let leaf_pos = NodesUtils::rightmost_leaf_node_index_from_pos(pos); + let leaf_index = NodesUtils::leaf_node_index_to_leaf_index(leaf_pos); + assert!(NodesUtils::right_branch_ending_in_leaf(leaf_index).contains(&pos)); + } } #[test] diff --git a/frame/merkle-mountain-range/src/tests.rs b/frame/merkle-mountain-range/src/tests.rs index a3886e9a8feff..d32f79a8fa215 100644 --- a/frame/merkle-mountain-range/src/tests.rs +++ b/frame/merkle-mountain-range/src/tests.rs @@ -17,7 +17,7 @@ use crate::{mmr::utils, mock::*, *}; -use frame_support::traits::OnInitialize; +use frame_support::traits::{Get, OnInitialize}; use mmr_lib::helper; use sp_core::{ offchain::{testing::TestOffchainExt, OffchainDbExt, OffchainWorkerExt}, @@ -47,7 +47,6 @@ fn new_block() -> u64 { fn peaks_from_leaves_count(leaves_count: NodeIndex) -> Vec { let size = utils::NodesUtils::new(leaves_count).size(); - helper::get_peaks(size) } @@ -73,7 +72,7 @@ fn decode_node( } } -fn init_chain(blocks: usize) { +fn add_blocks(blocks: usize) { // given for _ in 0..blocks { new_block(); @@ -113,13 +112,9 @@ fn should_start_empty() { #[test] fn should_append_to_mmr_when_on_initialize_is_called() { - use std::{cell::RefCell, rc::Rc}; - let _ = env_logger::try_init(); - let parent_hashes = Rc::new(RefCell::new(Default::default())); - let ext_parent_hashes = parent_hashes.clone(); let mut ext = new_test_ext(); - ext.execute_with(move || { + let (parent_b1, parent_b2) = ext.execute_with(|| { // when new_block(); let parent_b1 = >::parent_hash(); @@ -164,13 +159,11 @@ fn should_append_to_mmr_when_on_initialize_is_called() { ) ); - *ext_parent_hashes.borrow_mut() = (parent_b1, parent_b2); + (parent_b1, parent_b2) }); - - let (parent_b1, parent_b2) = Rc::try_unwrap(parent_hashes).unwrap().into_inner(); - // make sure the leaves end up in the offchain DB ext.persist_offchain_overlay(); + let offchain_db = ext.offchain_db(); assert_eq!( offchain_db.get(&MMR::offchain_key(parent_b1, 0)).map(decode_node), @@ -199,7 +192,7 @@ fn should_construct_larger_mmr_correctly() { let _ = env_logger::try_init(); new_test_ext().execute_with(|| { // when - init_chain(7); + add_blocks(7); // then assert_eq!(crate::NumberOfLeaves::::get(), 7); @@ -230,7 +223,7 @@ fn should_generate_proofs_correctly() { let _ = env_logger::try_init(); let mut ext = new_test_ext(); // given - ext.execute_with(|| init_chain(7)); + ext.execute_with(|| add_blocks(7)); ext.persist_offchain_overlay(); // Try to generate proofs now. This requires the offchain extensions to be present @@ -298,7 +291,7 @@ fn should_generate_batch_proof_correctly() { let _ = env_logger::try_init(); let mut ext = new_test_ext(); // given - ext.execute_with(|| init_chain(7)); + ext.execute_with(|| add_blocks(7)); ext.persist_offchain_overlay(); // Try to generate proofs now. This requires the offchain extensions to be present @@ -331,7 +324,7 @@ fn should_verify() { // Start off with chain initialisation and storing indexing data off-chain // (MMR Leafs) let mut ext = new_test_ext(); - ext.execute_with(|| init_chain(7)); + ext.execute_with(|| add_blocks(7)); ext.persist_offchain_overlay(); // Try to generate proof now. This requires the offchain extensions to be present @@ -343,7 +336,7 @@ fn should_verify() { }); ext.execute_with(|| { - init_chain(7); + add_blocks(7); // then assert_eq!(crate::Pallet::::verify_leaves(leaves, proof5), Ok(())); }); @@ -356,7 +349,7 @@ fn should_verify_batch_proof() { // Start off with chain initialisation and storing indexing data off-chain // (MMR Leafs) let mut ext = new_test_ext(); - ext.execute_with(|| init_chain(7)); + ext.execute_with(|| add_blocks(7)); ext.persist_offchain_overlay(); // Try to generate proof now. This requires the offchain extensions to be present @@ -368,7 +361,7 @@ fn should_verify_batch_proof() { }); ext.execute_with(|| { - init_chain(7); + add_blocks(7); // then assert_eq!(crate::Pallet::::verify_leaves(leaves, proof), Ok(())); }); @@ -381,7 +374,7 @@ fn verification_should_be_stateless() { // Start off with chain initialisation and storing indexing data off-chain // (MMR Leafs) let mut ext = new_test_ext(); - ext.execute_with(|| init_chain(7)); + ext.execute_with(|| add_blocks(7)); ext.persist_offchain_overlay(); // Try to generate proof now. This requires the offchain extensions to be present @@ -408,7 +401,7 @@ fn should_verify_batch_proof_statelessly() { // Start off with chain initialisation and storing indexing data off-chain // (MMR Leafs) let mut ext = new_test_ext(); - ext.execute_with(|| init_chain(7)); + ext.execute_with(|| add_blocks(7)); ext.persist_offchain_overlay(); // Try to generate proof now. This requires the offchain extensions to be present @@ -439,7 +432,7 @@ fn should_verify_on_the_next_block_since_there_is_no_pruning_yet() { let _ = env_logger::try_init(); let mut ext = new_test_ext(); // given - ext.execute_with(|| init_chain(7)); + ext.execute_with(|| add_blocks(7)); ext.persist_offchain_overlay(); register_offchain_ext(&mut ext); @@ -453,3 +446,104 @@ fn should_verify_on_the_next_block_since_there_is_no_pruning_yet() { assert_eq!(crate::Pallet::::verify_leaves(leaves, proof5), Ok(())); }); } + +#[test] +fn should_canonicalize_offchain() { + use frame_support::traits::Hooks; + + let _ = env_logger::try_init(); + let mut ext = new_test_ext(); + register_offchain_ext(&mut ext); + + // add 3 blocks and verify leaves and nodes for them have been added to + // offchain MMR using fork-proof keys. + ext.execute_with(|| add_blocks(3)); + ext.persist_offchain_overlay(); + let offchain_db = ext.offchain_db(); + let (parent_b1, parent_b2, parent_b3, block_hash_size) = ext.execute_with(|| { + // check nodes added by 1st block: + // not canon, + assert_eq!(offchain_db.get(&MMR::canon_offchain_key(0)), None); + let parent_b1 = >::block_hash(0); + // but available in fork-proof storage. + assert_eq!( + offchain_db.get(&MMR::offchain_key(parent_b1, 0)).map(decode_node), + Some(mmr::Node::Data(((0, H256::repeat_byte(1)), LeafData::new(1),))) + ); + + // check nodes added by 2nd block: + // not canon, + assert_eq!(offchain_db.get(&MMR::canon_offchain_key(1)), None); + assert_eq!(offchain_db.get(&MMR::canon_offchain_key(2)), None); + let parent_b2 = >::block_hash(1); + // but available in fork-proof storage. + assert_eq!( + offchain_db.get(&MMR::offchain_key(parent_b2, 1)).map(decode_node), + Some(mmr::Node::Data(((1, H256::repeat_byte(2)), LeafData::new(2),))) + ); + assert_eq!( + offchain_db.get(&MMR::offchain_key(parent_b2, 2)).map(decode_node), + Some(mmr::Node::Hash(hex( + "672c04a9cd05a644789d769daa552d35d8de7c33129f8a7cbf49e595234c4854" + ))) + ); + + // check nodes added by 3rd block: + // not canon, + assert_eq!(offchain_db.get(&MMR::canon_offchain_key(3)), None); + let parent_b3 = >::block_hash(2); + // but available in fork-proof storage. + assert_eq!( + offchain_db.get(&MMR::offchain_key(parent_b3, 3)).map(decode_node), + Some(mmr::Node::Data(((2, H256::repeat_byte(3)), LeafData::new(3),))) + ); + + let block_hash_size: u64 = ::BlockHashCount::get(); + (parent_b1, parent_b2, parent_b3, block_hash_size) + }); + + // add another `frame_system::BlockHashCount` blocks and verify all nodes and leaves + // added by our original 3 blocks have now been canonicalized in offchain db. + ext.execute_with(|| { + for blocknum in 3u32..(3 + block_hash_size).try_into().unwrap() { + new_block(); + as Hooks>::offchain_worker(blocknum.into()); + } + }); + ext.persist_offchain_overlay(); + ext.execute_with(|| { + // check nodes added by 1st block: + // no longer available in fork-proof storage, + assert_eq!(offchain_db.get(&MMR::offchain_key(parent_b1, 0)), None); + // but available using canon key. + assert_eq!( + offchain_db.get(&MMR::canon_offchain_key(0)).map(decode_node), + Some(mmr::Node::Data(((0, H256::repeat_byte(1)), LeafData::new(1),))) + ); + + // check nodes added by 2nd block: + // no longer available in fork-proof storage, + assert_eq!(offchain_db.get(&MMR::offchain_key(parent_b2, 1)), None); + assert_eq!(offchain_db.get(&MMR::offchain_key(parent_b2, 2)), None); + // but available using canon key. + assert_eq!( + offchain_db.get(&MMR::canon_offchain_key(1)).map(decode_node), + Some(mmr::Node::Data(((1, H256::repeat_byte(2)), LeafData::new(2),))) + ); + assert_eq!( + offchain_db.get(&MMR::canon_offchain_key(2)).map(decode_node), + Some(mmr::Node::Hash(hex( + "672c04a9cd05a644789d769daa552d35d8de7c33129f8a7cbf49e595234c4854" + ))) + ); + + // check nodes added by 3rd block: + // no longer available in fork-proof storage. + assert_eq!(offchain_db.get(&MMR::offchain_key(parent_b3, 3)), None); + // but available using canon key. + assert_eq!( + offchain_db.get(&MMR::canon_offchain_key(3)).map(decode_node), + Some(mmr::Node::Data(((2, H256::repeat_byte(3)), LeafData::new(3),))) + ); + }); +} From e9edd2894fe71ecc380c794eb344d2608e913348 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 28 Jun 2022 12:39:15 +0300 Subject: [PATCH 21/31] test canon + generate + verify --- frame/merkle-mountain-range/src/tests.rs | 41 ++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/frame/merkle-mountain-range/src/tests.rs b/frame/merkle-mountain-range/src/tests.rs index d32f79a8fa215..e807df2da6f18 100644 --- a/frame/merkle-mountain-range/src/tests.rs +++ b/frame/merkle-mountain-range/src/tests.rs @@ -547,3 +547,44 @@ fn should_canonicalize_offchain() { ); }); } + +#[test] +fn should_verify_canonicalized() { + use frame_support::traits::Hooks; + let _ = env_logger::try_init(); + + // How deep is our fork-aware storage (in terms of blocks/leaves, nodes will be more). + let block_hash_size: u64 = ::BlockHashCount::get(); + + // Start off with chain initialisation and storing indexing data off-chain. + // Create twice as many leaf entries than our fork-aware capacity, + // resulting in ~half of MMR storage to use canonical keys and the other half fork-aware keys. + // Verify that proofs can be generated (using leaves and nodes from full set) and verified. + let mut ext = new_test_ext(); + register_offchain_ext(&mut ext); + ext.execute_with(|| { + for blocknum in 0u32..(2 * block_hash_size).try_into().unwrap() { + new_block(); + as Hooks>::offchain_worker(blocknum.into()); + } + }); + ext.persist_offchain_overlay(); + + // Generate proofs for some blocks. + let (leaves, proofs) = + ext.execute_with(|| crate::Pallet::::generate_batch_proof(vec![0, 4, 5, 7]).unwrap()); + // Verify all previously generated proofs. + ext.execute_with(|| { + assert_eq!(crate::Pallet::::verify_leaves(leaves, proofs), Ok(())); + }); + + // Generate proofs for some new blocks. + let (leaves, proofs) = ext.execute_with(|| { + crate::Pallet::::generate_batch_proof(vec![block_hash_size + 7]).unwrap() + }); + // Add some more blocks then verify all previously generated proofs. + ext.execute_with(|| { + add_blocks(7); + assert_eq!(crate::Pallet::::verify_leaves(leaves, proofs), Ok(())); + }); +} From f472e9baaa514f914237c4ef131336f56887f61d Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 28 Jun 2022 13:29:34 +0300 Subject: [PATCH 22/31] fix pallet-beefy-mmr tests --- frame/beefy-mmr/src/tests.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/frame/beefy-mmr/src/tests.rs b/frame/beefy-mmr/src/tests.rs index d9cd8c8a5d8c8..308c68cdb4918 100644 --- a/frame/beefy-mmr/src/tests.rs +++ b/frame/beefy-mmr/src/tests.rs @@ -44,16 +44,12 @@ pub fn beefy_log(log: ConsensusLog) -> DigestItem { DigestItem::Consensus(BEEFY_ENGINE_ID, log.encode()) } -fn offchain_key(pos: usize) -> Vec { - (::INDEXING_PREFIX, pos as u64).encode() -} - -fn read_mmr_leaf(ext: &mut TestExternalities, index: usize) -> MmrLeaf { +fn read_mmr_leaf(ext: &mut TestExternalities, key: Vec) -> MmrLeaf { type Node = pallet_mmr::primitives::DataOrHash; ext.persist_offchain_overlay(); let offchain_db = ext.offchain_db(); offchain_db - .get(&offchain_key(index)) + .get(&key) .map(|d| Node::decode(&mut &*d).unwrap()) .map(|n| match n { Node::Data(d) => d, @@ -105,12 +101,17 @@ fn should_contain_mmr_digest() { #[test] fn should_contain_valid_leaf_data() { + fn offchain_key(parent_hash: H256, pos: usize) -> Vec { + (::INDEXING_PREFIX, parent_hash, pos as u64).encode() + } + let mut ext = new_test_ext(vec![1, 2, 3, 4]); - ext.execute_with(|| { + let parent_hash = ext.execute_with(|| { init_block(1); + >::parent_hash() }); - let mmr_leaf = read_mmr_leaf(&mut ext, 0); + let mmr_leaf = read_mmr_leaf(&mut ext, offchain_key(parent_hash, 0)); assert_eq!( mmr_leaf, MmrLeaf { @@ -128,11 +129,13 @@ fn should_contain_valid_leaf_data() { ); // build second block on top - ext.execute_with(|| { + let parent_hash = ext.execute_with(|| { init_block(2); + >::parent_hash() }); - let mmr_leaf = read_mmr_leaf(&mut ext, 1); + // let key = + let mmr_leaf = read_mmr_leaf(&mut ext, offchain_key(parent_hash, 1)); assert_eq!( mmr_leaf, MmrLeaf { From 5ed95c7aba75599c1f7e907fc7da2378d09ab1f0 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 28 Jun 2022 20:18:31 +0300 Subject: [PATCH 23/31] implement review suggestions --- frame/beefy-mmr/src/tests.rs | 1 - frame/merkle-mountain-range/src/lib.rs | 5 +- .../merkle-mountain-range/src/mmr/storage.rs | 67 ++++++++----------- 3 files changed, 32 insertions(+), 41 deletions(-) diff --git a/frame/beefy-mmr/src/tests.rs b/frame/beefy-mmr/src/tests.rs index 308c68cdb4918..548944282b3f3 100644 --- a/frame/beefy-mmr/src/tests.rs +++ b/frame/beefy-mmr/src/tests.rs @@ -134,7 +134,6 @@ fn should_contain_valid_leaf_data() { >::parent_hash() }); - // let key = let mmr_leaf = read_mmr_leaf(&mut ext, offchain_key(parent_hash, 1)); assert_eq!( mmr_leaf, diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index cc9612d49ecef..588e6d0ee68bc 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -235,7 +235,10 @@ pub mod pallet { // // This approach works to eliminate fork-induced leaf collisions in offchain db, // under the assumption that no fork will be deeper than `frame_system::BlockHashCount` - // blocks (2400 blocks on Polkadot, Kusama, Rococo, etc). + // blocks (2400 blocks on Polkadot, Kusama, Rococo, etc): + // entries pertaining to block `N` where `N < current-2400` are moved to a key based + // solely on block number. The only way to have collisions is if two competing forks + // are deeper than 2400 blocks and they both "canonicalize" their view of block `N`. Storage::>::canonicalize_offchain(); } } diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index a78a3b4d86fb3..b560b93050178 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -91,12 +91,10 @@ where let leaf_to_canon = leaves.saturating_sub(window_size); let parent_hash_of_leaf = Pallet::::parent_hash_of_leaf(leaf_to_canon, leaves); let nodes_to_canon = NodesUtils::right_branch_ending_in_leaf(leaf_to_canon); - sp_std::if_std! { - frame_support::log::debug!( - target: "runtime::mmr", "Nodes to canon for leaf {}: {:?}", - leaf_to_canon, nodes_to_canon - ); - } + frame_support::log::debug!( + target: "runtime::mmr", "Nodes to canon for leaf {}: {:?}", + leaf_to_canon, nodes_to_canon + ); for pos in nodes_to_canon { let key = Pallet::::offchain_key(parent_hash_of_leaf, pos); let canon_key = Pallet::::canon_offchain_key(pos); @@ -106,21 +104,17 @@ where offchain::local_storage_clear(StorageKind::PERSISTENT, &key); // Add under new canon key. offchain::local_storage_set(StorageKind::PERSISTENT, &canon_key, &elem); - sp_std::if_std! { - frame_support::log::debug!( - target: "runtime::mmr", - "Moved elem at pos {} from key {:?} to canon key {:?}", - pos, key, canon_key - ); - } + frame_support::log::debug!( + target: "runtime::mmr", + "Moved elem at pos {} from key {:?} to canon key {:?}", + pos, key, canon_key + ); } else { - sp_std::if_std! { - frame_support::log::debug!( - target: "runtime::mmr", - "Offchain: could not get elem at pos {} using key {:?}", - pos, key - ); - } + frame_support::log::debug!( + target: "runtime::mmr", + "Offchain: could not get elem at pos {} using key {:?}", + pos, key + ); } } } @@ -142,12 +136,10 @@ where let parent_hash_of_ancestor = Pallet::::parent_hash_of_leaf(ancestor_leaf_idx, leaves_count); let key = Pallet::::offchain_key(parent_hash_of_ancestor, pos); - sp_std::if_std! { - frame_support::log::debug!( - target: "runtime::mmr", "offchain get {}: leaf idx {:?}, hash {:?}, key {:?}", - pos, ancestor_leaf_idx, parent_hash_of_ancestor, key - ); - } + frame_support::log::debug!( + target: "runtime::mmr", "offchain get {}: leaf idx {:?}, hash {:?}, key {:?}", + pos, ancestor_leaf_idx, parent_hash_of_ancestor, key + ); // Retrieve the element from Off-chain DB. Ok(sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) .or_else(|| { @@ -177,9 +169,10 @@ where return Ok(()) } - sp_std::if_std! { - frame_support::log::trace!("elems: {:?}", elems.iter().map(|elem| elem.hash()).collect::>()); - } + frame_support::log::trace!( + "elems: {:?}", + elems.iter().map(|elem| elem.hash()).collect::>() + ); let leaves = NumberOfLeaves::::get(); let size = NodesUtils::new(leaves).size(); @@ -210,12 +203,10 @@ where // "Canonicalization" in this case means moving this leaf under a new key based // only on the leaf's `node_index`. let key = Pallet::::offchain_key(parent_hash, node_index); - sp_std::if_std! { - frame_support::log::debug!( - target: "runtime::mmr", "offchain set: pos {} parent_hash {:?} key {:?}", - node_index, parent_hash, key - ); - } + frame_support::log::debug!( + target: "runtime::mmr", "offchain set: pos {} parent_hash {:?} key {:?}", + node_index, parent_hash, key + ); // Indexing API is used to store the full node content (both leaf and inner). elem.using_encoded(|elem| offchain_index::set(&key, elem)); @@ -251,10 +242,8 @@ fn peaks_to_prune_and_store( // both collections may share a common prefix. let peaks_before = if old_size == 0 { vec![] } else { helper::get_peaks(old_size) }; let peaks_after = helper::get_peaks(new_size); - sp_std::if_std! { - frame_support::log::trace!("peaks_before: {:?}", peaks_before); - frame_support::log::trace!("peaks_after: {:?}", peaks_after); - } + frame_support::log::trace!("peaks_before: {:?}", peaks_before); + frame_support::log::trace!("peaks_after: {:?}", peaks_after); let mut peaks_before = peaks_before.into_iter().peekable(); let mut peaks_after = peaks_after.into_iter().peekable(); From 2c8b101f08cbec9e0ed8b60830ce25c023c878d3 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Wed, 29 Jun 2022 12:23:08 +0300 Subject: [PATCH 24/31] improve test --- frame/merkle-mountain-range/src/tests.rs | 153 ++++++++++++----------- 1 file changed, 79 insertions(+), 74 deletions(-) diff --git a/frame/merkle-mountain-range/src/tests.rs b/frame/merkle-mountain-range/src/tests.rs index e807df2da6f18..e2409c2ea3187 100644 --- a/frame/merkle-mountain-range/src/tests.rs +++ b/frame/merkle-mountain-range/src/tests.rs @@ -455,96 +455,101 @@ fn should_canonicalize_offchain() { let mut ext = new_test_ext(); register_offchain_ext(&mut ext); + // adding 13 blocks that we'll later check have been canonicalized. + let to_canon_count = 13; + // add 3 blocks and verify leaves and nodes for them have been added to // offchain MMR using fork-proof keys. - ext.execute_with(|| add_blocks(3)); + ext.execute_with(|| add_blocks(to_canon_count)); ext.persist_offchain_overlay(); let offchain_db = ext.offchain_db(); - let (parent_b1, parent_b2, parent_b3, block_hash_size) = ext.execute_with(|| { - // check nodes added by 1st block: - // not canon, - assert_eq!(offchain_db.get(&MMR::canon_offchain_key(0)), None); - let parent_b1 = >::block_hash(0); - // but available in fork-proof storage. - assert_eq!( - offchain_db.get(&MMR::offchain_key(parent_b1, 0)).map(decode_node), - Some(mmr::Node::Data(((0, H256::repeat_byte(1)), LeafData::new(1),))) - ); - - // check nodes added by 2nd block: - // not canon, - assert_eq!(offchain_db.get(&MMR::canon_offchain_key(1)), None); - assert_eq!(offchain_db.get(&MMR::canon_offchain_key(2)), None); - let parent_b2 = >::block_hash(1); - // but available in fork-proof storage. - assert_eq!( - offchain_db.get(&MMR::offchain_key(parent_b2, 1)).map(decode_node), - Some(mmr::Node::Data(((1, H256::repeat_byte(2)), LeafData::new(2),))) - ); - assert_eq!( - offchain_db.get(&MMR::offchain_key(parent_b2, 2)).map(decode_node), - Some(mmr::Node::Hash(hex( - "672c04a9cd05a644789d769daa552d35d8de7c33129f8a7cbf49e595234c4854" - ))) - ); - - // check nodes added by 3rd block: - // not canon, - assert_eq!(offchain_db.get(&MMR::canon_offchain_key(3)), None); - let parent_b3 = >::block_hash(2); - // but available in fork-proof storage. - assert_eq!( - offchain_db.get(&MMR::offchain_key(parent_b3, 3)).map(decode_node), - Some(mmr::Node::Data(((2, H256::repeat_byte(3)), LeafData::new(3),))) - ); + ext.execute_with(|| { + // verify leaves added by blocks 1..13 + for block_num in 1..u32::try_from(to_canon_count).unwrap() { + let parent_num: BlockNumber = (block_num - 1).into(); + let leaf_index = u64::from(block_num - 1); + let pos = helper::leaf_index_to_pos(leaf_index.into()); + // not canon, + assert_eq!(offchain_db.get(&MMR::canon_offchain_key(pos)), None); + let parent_hash = >::block_hash(parent_num); + // but available in fork-proof storage. + assert_eq!( + offchain_db.get(&MMR::offchain_key(parent_hash, pos)).map(decode_node), + Some(mmr::Node::Data(( + (leaf_index, H256::repeat_byte(u8::try_from(block_num).unwrap())), + LeafData::new(block_num.into()), + ))) + ); + } - let block_hash_size: u64 = ::BlockHashCount::get(); - (parent_b1, parent_b2, parent_b3, block_hash_size) + // verify a couple of nodes and peaks: + // `pos` is node to verify, + // `leaf_index` is leaf that added node `pos`, + // `expected` is expected value of node at `pos`. + let verify = |pos: NodeIndex, leaf_index: LeafIndex, expected: H256| { + let parent_num: BlockNumber = leaf_index.try_into().unwrap(); + let parent_hash = >::block_hash(parent_num); + // not canon, + assert_eq!(offchain_db.get(&MMR::canon_offchain_key(pos)), None); + // but available in fork-proof storage. + assert_eq!( + offchain_db.get(&MMR::offchain_key(parent_hash, pos)).map(decode_node), + Some(mmr::Node::Hash(expected)) + ); + }; + verify(2, 1, hex("672c04a9cd05a644789d769daa552d35d8de7c33129f8a7cbf49e595234c4854")); + verify(13, 7, hex("441bf63abc7cf9b9e82eb57b8111c883d50ae468d9fd7f301e12269fc0fa1e75")); + verify(21, 11, hex("f323ac1a7f56de5f40ed8df3e97af74eec0ee9d72883679e49122ffad2ffd03b")); }); // add another `frame_system::BlockHashCount` blocks and verify all nodes and leaves - // added by our original 3 blocks have now been canonicalized in offchain db. + // added by our original `to_canon_count` blocks have now been canonicalized in offchain db. ext.execute_with(|| { - for blocknum in 3u32..(3 + block_hash_size).try_into().unwrap() { + let block_hash_size: u64 = ::BlockHashCount::get(); + let base = u32::try_from(to_canon_count).unwrap(); + for blocknum in base..(base + u32::try_from(block_hash_size).unwrap()) { new_block(); as Hooks>::offchain_worker(blocknum.into()); } }); ext.persist_offchain_overlay(); ext.execute_with(|| { - // check nodes added by 1st block: - // no longer available in fork-proof storage, - assert_eq!(offchain_db.get(&MMR::offchain_key(parent_b1, 0)), None); - // but available using canon key. - assert_eq!( - offchain_db.get(&MMR::canon_offchain_key(0)).map(decode_node), - Some(mmr::Node::Data(((0, H256::repeat_byte(1)), LeafData::new(1),))) - ); - - // check nodes added by 2nd block: - // no longer available in fork-proof storage, - assert_eq!(offchain_db.get(&MMR::offchain_key(parent_b2, 1)), None); - assert_eq!(offchain_db.get(&MMR::offchain_key(parent_b2, 2)), None); - // but available using canon key. - assert_eq!( - offchain_db.get(&MMR::canon_offchain_key(1)).map(decode_node), - Some(mmr::Node::Data(((1, H256::repeat_byte(2)), LeafData::new(2),))) - ); - assert_eq!( - offchain_db.get(&MMR::canon_offchain_key(2)).map(decode_node), - Some(mmr::Node::Hash(hex( - "672c04a9cd05a644789d769daa552d35d8de7c33129f8a7cbf49e595234c4854" - ))) - ); + // verify leaves added by blocks 1..13, should be in offchain under canon key. + for block_num in 1..u32::try_from(to_canon_count).unwrap() { + let leaf_index = u64::from(block_num - 1); + let pos = helper::leaf_index_to_pos(leaf_index.into()); + let parent_num: BlockNumber = (block_num - 1).into(); + let parent_hash = >::block_hash(parent_num); + // no longer available in fork-proof storage, + assert_eq!(offchain_db.get(&MMR::offchain_key(parent_hash, pos)), None); + // but available using canon key. + assert_eq!( + offchain_db.get(&MMR::canon_offchain_key(pos)).map(decode_node), + Some(mmr::Node::Data(( + (leaf_index, H256::repeat_byte(u8::try_from(block_num).unwrap())), + LeafData::new(block_num.into()), + ))) + ); + } - // check nodes added by 3rd block: - // no longer available in fork-proof storage. - assert_eq!(offchain_db.get(&MMR::offchain_key(parent_b3, 3)), None); - // but available using canon key. - assert_eq!( - offchain_db.get(&MMR::canon_offchain_key(3)).map(decode_node), - Some(mmr::Node::Data(((2, H256::repeat_byte(3)), LeafData::new(3),))) - ); + // also check some nodes and peaks: + // `pos` is node to verify, + // `leaf_index` is leaf that added node `pos`, + // `expected` is expected value of node at `pos`. + let verify = |pos: NodeIndex, leaf_index: LeafIndex, expected: H256| { + let parent_num: BlockNumber = leaf_index.try_into().unwrap(); + let parent_hash = >::block_hash(parent_num); + // no longer available in fork-proof storage, + assert_eq!(offchain_db.get(&MMR::offchain_key(parent_hash, pos)), None); + // but available using canon key. + assert_eq!( + offchain_db.get(&MMR::canon_offchain_key(pos)).map(decode_node), + Some(mmr::Node::Hash(expected)) + ); + }; + verify(2, 1, hex("672c04a9cd05a644789d769daa552d35d8de7c33129f8a7cbf49e595234c4854")); + verify(13, 7, hex("441bf63abc7cf9b9e82eb57b8111c883d50ae468d9fd7f301e12269fc0fa1e75")); + verify(21, 11, hex("f323ac1a7f56de5f40ed8df3e97af74eec0ee9d72883679e49122ffad2ffd03b")); }); } From 56f2eaa44e3b9d3998235b99e1104d34b8ca2e0b Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 30 Jun 2022 11:37:38 +0300 Subject: [PATCH 25/31] pallet-mmr: add offchain pruning of forks --- frame/beefy-mmr/src/tests.rs | 6 +- frame/merkle-mountain-range/src/lib.rs | 30 ++-- .../merkle-mountain-range/src/mmr/storage.rs | 138 +++++++++++++----- frame/merkle-mountain-range/src/mmr/utils.rs | 2 +- frame/merkle-mountain-range/src/tests.rs | 51 ++++--- 5 files changed, 151 insertions(+), 76 deletions(-) diff --git a/frame/beefy-mmr/src/tests.rs b/frame/beefy-mmr/src/tests.rs index 548944282b3f3..eaa50004ae848 100644 --- a/frame/beefy-mmr/src/tests.rs +++ b/frame/beefy-mmr/src/tests.rs @@ -101,7 +101,7 @@ fn should_contain_mmr_digest() { #[test] fn should_contain_valid_leaf_data() { - fn offchain_key(parent_hash: H256, pos: usize) -> Vec { + fn node_offchain_key(parent_hash: H256, pos: usize) -> Vec { (::INDEXING_PREFIX, parent_hash, pos as u64).encode() } @@ -111,7 +111,7 @@ fn should_contain_valid_leaf_data() { >::parent_hash() }); - let mmr_leaf = read_mmr_leaf(&mut ext, offchain_key(parent_hash, 0)); + let mmr_leaf = read_mmr_leaf(&mut ext, node_offchain_key(parent_hash, 0)); assert_eq!( mmr_leaf, MmrLeaf { @@ -134,7 +134,7 @@ fn should_contain_valid_leaf_data() { >::parent_hash() }); - let mmr_leaf = read_mmr_leaf(&mut ext, offchain_key(parent_hash, 1)); + let mmr_leaf = read_mmr_leaf(&mut ext, node_offchain_key(parent_hash, 1)); assert_eq!( mmr_leaf, MmrLeaf { diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index 588e6d0ee68bc..1301e98bbfcf3 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -222,7 +222,7 @@ pub mod pallet { T::WeightInfo::on_initialize(peaks_before.max(peaks_after)) } - fn offchain_worker(_n: T::BlockNumber) { + fn offchain_worker(n: T::BlockNumber) { use mmr::storage::{OffchainStorage, Storage}; // MMR pallet uses offchain storage to hold full MMR and leaves. // The leaves are saved under fork-unique keys `(parent_hash, pos)`. @@ -239,7 +239,7 @@ pub mod pallet { // entries pertaining to block `N` where `N < current-2400` are moved to a key based // solely on block number. The only way to have collisions is if two competing forks // are deeper than 2400 blocks and they both "canonicalize" their view of block `N`. - Storage::>::canonicalize_offchain(); + Storage::>::canonicalize_offchain(n); } } } @@ -281,7 +281,7 @@ impl, I: 'static> Pallet { /// Build offchain key from `parent_hash` of block that originally added node `pos` to MMR. /// /// This combination makes the offchain (key,value) entry resilient to chain forks. - fn offchain_key( + fn node_offchain_key( parent_hash: ::Hash, pos: NodeIndex, ) -> sp_std::prelude::Vec { @@ -291,27 +291,31 @@ impl, I: 'static> Pallet { /// Build canonical offchain key for node `pos` in MMR. /// /// Used for nodes added by now finalized blocks. - fn canon_offchain_key(pos: NodeIndex) -> sp_std::prelude::Vec { + fn node_canon_offchain_key(pos: NodeIndex) -> sp_std::prelude::Vec { (T::INDEXING_PREFIX, pos).encode() } - /// Provide the parent hash for the block that added `leaf_index` to the MMR. + /// Build offchain key for the pruning map. /// - /// Should only be called for blocks still available in `>::block_hash`. - fn parent_hash_of_leaf( + /// Nodes and leaves are initially saved under fork-specific keys in offchain db, + /// eventually they are "canonicalized" and this map is used to prune non-canon entries. + fn pruning_map_offchain_key() -> sp_std::prelude::Vec { + (T::INDEXING_PREFIX, mmr::storage::OFFCHAIN_PRUNING_MAP_KEY_SUFFIX).encode() + } + + /// Provide the parent number for the block that added `leaf_index` to the MMR. + fn leaf_index_to_parent_block_num( leaf_index: LeafIndex, leaves_count: LeafIndex, - ) -> ::Hash { + ) -> ::BlockNumber { // leaves are zero-indexed and were added one per block since pallet activation, // while block numbers are one-indexed, so block number that added `leaf_idx` is: // `block_num = block_num_when_pallet_activated + leaf_idx + 1` // `block_num = (current_block_num - leaves_count) + leaf_idx + 1` // `parent_block_num = current_block_num - leaves_count + leaf_idx`. - let parent_block_num: ::BlockNumber = - >::block_number() - .saturating_sub(leaves_count.saturated_into()) - .saturating_add(leaf_index.saturated_into()); - >::block_hash(parent_block_num) + >::block_number() + .saturating_sub(leaves_count.saturated_into()) + .saturating_add(leaf_index.saturated_into()) } /// Generate a MMR proof for the given `leaf_indices`. diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index b560b93050178..eee08bb810997 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -20,10 +20,11 @@ use codec::Encode; use frame_support::traits::Get; use mmr_lib::helper; -use sp_io::offchain_index; -use sp_std::iter::Peekable; +use sp_core::offchain::StorageKind; +use sp_io::{offchain, offchain_index}; #[cfg(not(feature = "std"))] use sp_std::prelude::*; +use sp_std::{collections::btree_map::BTreeMap, iter::Peekable}; use crate::{ mmr::{utils::NodesUtils, Node, NodeOf}, @@ -47,6 +48,18 @@ pub struct RuntimeStorage; /// DOES NOT support adding new items to the MMR. pub struct OffchainStorage; +/// Suffix of key for the 'pruning_map'. +/// +/// Nodes and leaves are initially saved under fork-specific keys in offchain db, +/// eventually they are "canonicalized" and this map is used to prune non-canon entries. +pub(crate) const OFFCHAIN_PRUNING_MAP_KEY_SUFFIX: &str = "pruning_map"; + +/// Used to store offchain mappings of `BlockNumber -> Vec[Hash]` to track all forks. +/// Size of this offchain map is at most `frame_system::BlockHashCount`, its entries are pruned +/// as part of the mechanism that prunes the forks this map tracks. +type PruningMap = + BTreeMap<::BlockNumber, Vec<::Hash>>; + /// A storage layer for MMR. /// /// There are two different implementations depending on the use case. @@ -75,47 +88,99 @@ where /// /// Should only be called from offchain context, because it requires both read and write /// access to offchain db. - pub fn canonicalize_offchain() { - use sp_core::offchain::StorageKind; - use sp_io::offchain; + pub(crate) fn canonicalize_offchain(block: T::BlockNumber) { use sp_runtime::traits::UniqueSaturatedInto; + let map_key = Pallet::::pruning_map_offchain_key(); + let mut pruning_map: PruningMap = + offchain::local_storage_get(StorageKind::PERSISTENT, &map_key) + .and_then(|v| codec::Decode::decode(&mut &*v).ok()) + .unwrap_or_default(); + + // Add "block_num -> hash" mapping to offchain db, + // with all forks pushing hashes to same entry (for a particular block number). + let parent_hash = >::parent_hash(); + pruning_map + .entry(block) + .and_modify(|e| e.push(parent_hash)) + .or_insert(vec![parent_hash]); + // Effectively move a rolling window of fork-unique leaves. Once out of the window, leaves - // are "canonicalized" in offchain db by moving them under `Pallet::canon_offchain_key`. + // are "canonicalized" in offchain by moving them under `Pallet::node_canon_offchain_key`. let leaves = NumberOfLeaves::::get(); let window_size = ::BlockHashCount::get().unique_saturated_into(); if leaves >= window_size { - // move the rolling window towards the end of `block_num->hash` mappings available - // in the runtime: we "canonicalize" the leaf at the end. - let leaf_to_canon = leaves.saturating_sub(window_size); - let parent_hash_of_leaf = Pallet::::parent_hash_of_leaf(leaf_to_canon, leaves); - let nodes_to_canon = NodesUtils::right_branch_ending_in_leaf(leaf_to_canon); + // Move the rolling window towards the end of `block_num->hash` mappings available + // in the runtime: we "canonicalize" the leaf at the end, + let to_canon_leaf = leaves.saturating_sub(window_size); + // and all the nodes added by that leaf. + let to_canon_nodes = NodesUtils::right_branch_ending_in_leaf(to_canon_leaf); frame_support::log::debug!( target: "runtime::mmr", "Nodes to canon for leaf {}: {:?}", - leaf_to_canon, nodes_to_canon + to_canon_leaf, to_canon_nodes ); - for pos in nodes_to_canon { - let key = Pallet::::offchain_key(parent_hash_of_leaf, pos); - let canon_key = Pallet::::canon_offchain_key(pos); - // Retrieve the element from Off-chain DB under fork-aware key. - if let Some(elem) = offchain::local_storage_get(StorageKind::PERSISTENT, &key) { - // Delete entry with old key. - offchain::local_storage_clear(StorageKind::PERSISTENT, &key); - // Add under new canon key. - offchain::local_storage_set(StorageKind::PERSISTENT, &canon_key, &elem); - frame_support::log::debug!( - target: "runtime::mmr", - "Moved elem at pos {} from key {:?} to canon key {:?}", - pos, key, canon_key - ); - } else { + // For this block number there may be node entries saved from multiple forks. + let to_canon_block_num = + Pallet::::leaf_index_to_parent_block_num(to_canon_leaf, leaves); + // Only entries under this hash (retrieved from state on current canon fork) are to be + // persisted. All other entries added by same block number will be cleared. + let to_canon_hash = >::block_hash(to_canon_block_num); + + Self::canonicalize_nodes_for_hash(&to_canon_nodes, to_canon_hash); + // Get all the forks to prune, also remove them from the pruning_map. + pruning_map + .remove(&to_canon_block_num) + .map(|forks| { + Self::prune_nodes_for_forks(&to_canon_nodes, forks); + }) + .unwrap_or_else(|| { frame_support::log::debug!( target: "runtime::mmr", - "Offchain: could not get elem at pos {} using key {:?}", - pos, key + "Offchain: could not prune: no entry in pruning map for block {:?}", + to_canon_block_num ); - } + }) + } + // Save new, updated pruning map. + offchain::local_storage_set( + StorageKind::PERSISTENT, + &map_key, + &Encode::encode(&pruning_map), + ); + } + + fn prune_nodes_for_forks(nodes: &[NodeIndex], forks: Vec<::Hash>) { + for hash in forks { + for pos in nodes { + let key = Pallet::::node_offchain_key(hash, *pos); + offchain::local_storage_clear(StorageKind::PERSISTENT, &key); + } + } + } + + fn canonicalize_nodes_for_hash( + to_canon_nodes: &[NodeIndex], + to_canon_hash: ::Hash, + ) { + for pos in to_canon_nodes { + let key = Pallet::::node_offchain_key(to_canon_hash, *pos); + // Retrieve the element from Off-chain DB under fork-aware key. + if let Some(elem) = offchain::local_storage_get(StorageKind::PERSISTENT, &key) { + let canon_key = Pallet::::node_canon_offchain_key(*pos); + // Add under new canon key. + offchain::local_storage_set(StorageKind::PERSISTENT, &canon_key, &elem); + frame_support::log::debug!( + target: "runtime::mmr", + "Moved elem at pos {} from key {:?} to canon key {:?}", + pos, key, canon_key + ); + } else { + frame_support::log::debug!( + target: "runtime::mmr", + "Offchain: could not canonicalize elem at pos {} using key {:?}", + pos, key + ); } } } @@ -133,17 +198,18 @@ where // Use the hash as extra identifier to differentiate between various `pos` entries // in offchain DB coming from various chain forks. let ancestor_leaf_idx = NodesUtils::leaf_index_that_added_node(pos); - let parent_hash_of_ancestor = - Pallet::::parent_hash_of_leaf(ancestor_leaf_idx, leaves_count); - let key = Pallet::::offchain_key(parent_hash_of_ancestor, pos); + let ancestor_parent_block_num = + Pallet::::leaf_index_to_parent_block_num(ancestor_leaf_idx, leaves_count); + let ancestor_parent_hash = >::block_hash(ancestor_parent_block_num); + let key = Pallet::::node_offchain_key(ancestor_parent_hash, pos); frame_support::log::debug!( target: "runtime::mmr", "offchain get {}: leaf idx {:?}, hash {:?}, key {:?}", - pos, ancestor_leaf_idx, parent_hash_of_ancestor, key + pos, ancestor_leaf_idx, ancestor_parent_hash, key ); // Retrieve the element from Off-chain DB. Ok(sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) .or_else(|| { - let key = Pallet::::canon_offchain_key(pos); + let key = Pallet::::node_canon_offchain_key(pos); sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) }) .and_then(|v| codec::Decode::decode(&mut &*v).ok())) @@ -202,7 +268,7 @@ where // in the chain that deep). // "Canonicalization" in this case means moving this leaf under a new key based // only on the leaf's `node_index`. - let key = Pallet::::offchain_key(parent_hash, node_index); + let key = Pallet::::node_offchain_key(parent_hash, node_index); frame_support::log::debug!( target: "runtime::mmr", "offchain set: pos {} parent_hash {:?} key {:?}", node_index, parent_hash, key diff --git a/frame/merkle-mountain-range/src/mmr/utils.rs b/frame/merkle-mountain-range/src/mmr/utils.rs index d1c371e7ed2d2..3734ea514782d 100644 --- a/frame/merkle-mountain-range/src/mmr/utils.rs +++ b/frame/merkle-mountain-range/src/mmr/utils.rs @@ -79,7 +79,7 @@ impl NodesUtils { /// Starting from any leaf index, get the sequence of positions of the nodes added /// to the mmr when this leaf was added (inclusive of the leaf's position itself). /// That is, all of these nodes are right children of their respective parents. - pub fn right_branch_ending_in_leaf(leaf_index: LeafIndex) -> crate::Vec { + pub fn right_branch_ending_in_leaf(leaf_index: LeafIndex) -> crate::Vec { let pos = helper::leaf_index_to_pos(leaf_index); let num_parents = leaf_index.trailing_ones() as u64; return (pos..=pos + num_parents).collect() diff --git a/frame/merkle-mountain-range/src/tests.rs b/frame/merkle-mountain-range/src/tests.rs index e2409c2ea3187..2b5e68873b0a8 100644 --- a/frame/merkle-mountain-range/src/tests.rs +++ b/frame/merkle-mountain-range/src/tests.rs @@ -166,25 +166,25 @@ fn should_append_to_mmr_when_on_initialize_is_called() { let offchain_db = ext.offchain_db(); assert_eq!( - offchain_db.get(&MMR::offchain_key(parent_b1, 0)).map(decode_node), + offchain_db.get(&MMR::node_offchain_key(parent_b1, 0)).map(decode_node), Some(mmr::Node::Data(((0, H256::repeat_byte(1)), LeafData::new(1),))) ); assert_eq!( - offchain_db.get(&MMR::offchain_key(parent_b2, 1)).map(decode_node), + offchain_db.get(&MMR::node_offchain_key(parent_b2, 1)).map(decode_node), Some(mmr::Node::Data(((1, H256::repeat_byte(2)), LeafData::new(2),))) ); assert_eq!( - offchain_db.get(&MMR::offchain_key(parent_b2, 2)).map(decode_node), + offchain_db.get(&MMR::node_offchain_key(parent_b2, 2)).map(decode_node), Some(mmr::Node::Hash(hex( "672c04a9cd05a644789d769daa552d35d8de7c33129f8a7cbf49e595234c4854" ))) ); - assert_eq!(offchain_db.get(&MMR::offchain_key(parent_b2, 3)), None); + assert_eq!(offchain_db.get(&MMR::node_offchain_key(parent_b2, 3)), None); - assert_eq!(offchain_db.get(&MMR::canon_offchain_key(0)), None); - assert_eq!(offchain_db.get(&MMR::canon_offchain_key(1)), None); - assert_eq!(offchain_db.get(&MMR::canon_offchain_key(2)), None); - assert_eq!(offchain_db.get(&MMR::canon_offchain_key(3)), None); + assert_eq!(offchain_db.get(&MMR::node_canon_offchain_key(0)), None); + assert_eq!(offchain_db.get(&MMR::node_canon_offchain_key(1)), None); + assert_eq!(offchain_db.get(&MMR::node_canon_offchain_key(2)), None); + assert_eq!(offchain_db.get(&MMR::node_canon_offchain_key(3)), None); } #[test] @@ -456,25 +456,30 @@ fn should_canonicalize_offchain() { register_offchain_ext(&mut ext); // adding 13 blocks that we'll later check have been canonicalized. - let to_canon_count = 13; + let to_canon_count = 13u32; // add 3 blocks and verify leaves and nodes for them have been added to // offchain MMR using fork-proof keys. - ext.execute_with(|| add_blocks(to_canon_count)); + ext.execute_with(|| { + for blocknum in 0..to_canon_count { + new_block(); + as Hooks>::offchain_worker(blocknum.into()); + } + }); ext.persist_offchain_overlay(); let offchain_db = ext.offchain_db(); ext.execute_with(|| { // verify leaves added by blocks 1..13 - for block_num in 1..u32::try_from(to_canon_count).unwrap() { + for block_num in 1..to_canon_count { let parent_num: BlockNumber = (block_num - 1).into(); let leaf_index = u64::from(block_num - 1); let pos = helper::leaf_index_to_pos(leaf_index.into()); // not canon, - assert_eq!(offchain_db.get(&MMR::canon_offchain_key(pos)), None); + assert_eq!(offchain_db.get(&MMR::node_canon_offchain_key(pos)), None); let parent_hash = >::block_hash(parent_num); // but available in fork-proof storage. assert_eq!( - offchain_db.get(&MMR::offchain_key(parent_hash, pos)).map(decode_node), + offchain_db.get(&MMR::node_offchain_key(parent_hash, pos)).map(decode_node), Some(mmr::Node::Data(( (leaf_index, H256::repeat_byte(u8::try_from(block_num).unwrap())), LeafData::new(block_num.into()), @@ -490,10 +495,10 @@ fn should_canonicalize_offchain() { let parent_num: BlockNumber = leaf_index.try_into().unwrap(); let parent_hash = >::block_hash(parent_num); // not canon, - assert_eq!(offchain_db.get(&MMR::canon_offchain_key(pos)), None); + assert_eq!(offchain_db.get(&MMR::node_canon_offchain_key(pos)), None); // but available in fork-proof storage. assert_eq!( - offchain_db.get(&MMR::offchain_key(parent_hash, pos)).map(decode_node), + offchain_db.get(&MMR::node_offchain_key(parent_hash, pos)).map(decode_node), Some(mmr::Node::Hash(expected)) ); }; @@ -506,7 +511,7 @@ fn should_canonicalize_offchain() { // added by our original `to_canon_count` blocks have now been canonicalized in offchain db. ext.execute_with(|| { let block_hash_size: u64 = ::BlockHashCount::get(); - let base = u32::try_from(to_canon_count).unwrap(); + let base = to_canon_count; for blocknum in base..(base + u32::try_from(block_hash_size).unwrap()) { new_block(); as Hooks>::offchain_worker(blocknum.into()); @@ -515,16 +520,16 @@ fn should_canonicalize_offchain() { ext.persist_offchain_overlay(); ext.execute_with(|| { // verify leaves added by blocks 1..13, should be in offchain under canon key. - for block_num in 1..u32::try_from(to_canon_count).unwrap() { + for block_num in 1..to_canon_count { let leaf_index = u64::from(block_num - 1); let pos = helper::leaf_index_to_pos(leaf_index.into()); let parent_num: BlockNumber = (block_num - 1).into(); let parent_hash = >::block_hash(parent_num); - // no longer available in fork-proof storage, - assert_eq!(offchain_db.get(&MMR::offchain_key(parent_hash, pos)), None); + // no longer available in fork-proof storage (was pruned), + assert_eq!(offchain_db.get(&MMR::node_offchain_key(parent_hash, pos)), None); // but available using canon key. assert_eq!( - offchain_db.get(&MMR::canon_offchain_key(pos)).map(decode_node), + offchain_db.get(&MMR::node_canon_offchain_key(pos)).map(decode_node), Some(mmr::Node::Data(( (leaf_index, H256::repeat_byte(u8::try_from(block_num).unwrap())), LeafData::new(block_num.into()), @@ -539,11 +544,11 @@ fn should_canonicalize_offchain() { let verify = |pos: NodeIndex, leaf_index: LeafIndex, expected: H256| { let parent_num: BlockNumber = leaf_index.try_into().unwrap(); let parent_hash = >::block_hash(parent_num); - // no longer available in fork-proof storage, - assert_eq!(offchain_db.get(&MMR::offchain_key(parent_hash, pos)), None); + // no longer available in fork-proof storage (was pruned), + assert_eq!(offchain_db.get(&MMR::node_offchain_key(parent_hash, pos)), None); // but available using canon key. assert_eq!( - offchain_db.get(&MMR::canon_offchain_key(pos)).map(decode_node), + offchain_db.get(&MMR::node_canon_offchain_key(pos)).map(decode_node), Some(mmr::Node::Hash(expected)) ); }; From 262973433c3457a9ec39071e67f792da015516c4 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 1 Jul 2022 16:44:08 +0300 Subject: [PATCH 26/31] pallet-mmr: improve offchain pruning Instead of keeping pruning map as single blob in offchain db, keep individual parent-hash lists with block-num identifier as part of the offchain key. Signed-off-by: acatangiu --- frame/merkle-mountain-range/src/lib.rs | 12 +-- .../merkle-mountain-range/src/mmr/storage.rs | 69 +++++++++------ frame/merkle-mountain-range/src/tests.rs | 88 ++++++++++++++++++- 3 files changed, 134 insertions(+), 35 deletions(-) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index 1301e98bbfcf3..ed0214af1d3ec 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -239,7 +239,9 @@ pub mod pallet { // entries pertaining to block `N` where `N < current-2400` are moved to a key based // solely on block number. The only way to have collisions is if two competing forks // are deeper than 2400 blocks and they both "canonicalize" their view of block `N`. - Storage::>::canonicalize_offchain(n); + // Once a block is canonicalized, all MMR entries pertaining to sibling blocks from + // other forks are pruned from offchain db. + Storage::>::canonicalize_and_prune(n); } } } @@ -295,14 +297,6 @@ impl, I: 'static> Pallet { (T::INDEXING_PREFIX, pos).encode() } - /// Build offchain key for the pruning map. - /// - /// Nodes and leaves are initially saved under fork-specific keys in offchain db, - /// eventually they are "canonicalized" and this map is used to prune non-canon entries. - fn pruning_map_offchain_key() -> sp_std::prelude::Vec { - (T::INDEXING_PREFIX, mmr::storage::OFFCHAIN_PRUNING_MAP_KEY_SUFFIX).encode() - } - /// Provide the parent number for the block that added `leaf_index` to the MMR. fn leaf_index_to_parent_block_num( leaf_index: LeafIndex, diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index eee08bb810997..92eeda24403cb 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -22,9 +22,9 @@ use frame_support::traits::Get; use mmr_lib::helper; use sp_core::offchain::StorageKind; use sp_io::{offchain, offchain_index}; +use sp_std::iter::Peekable; #[cfg(not(feature = "std"))] use sp_std::prelude::*; -use sp_std::{collections::btree_map::BTreeMap, iter::Peekable}; use crate::{ mmr::{utils::NodesUtils, Node, NodeOf}, @@ -52,13 +52,46 @@ pub struct OffchainStorage; /// /// Nodes and leaves are initially saved under fork-specific keys in offchain db, /// eventually they are "canonicalized" and this map is used to prune non-canon entries. -pub(crate) const OFFCHAIN_PRUNING_MAP_KEY_SUFFIX: &str = "pruning_map"; +const OFFCHAIN_PRUNING_MAP_KEY_SUFFIX: &str = "pruning_map"; /// Used to store offchain mappings of `BlockNumber -> Vec[Hash]` to track all forks. /// Size of this offchain map is at most `frame_system::BlockHashCount`, its entries are pruned /// as part of the mechanism that prunes the forks this map tracks. -type PruningMap = - BTreeMap<::BlockNumber, Vec<::Hash>>; +pub(crate) struct PruningMap(sp_std::marker::PhantomData<(T, I)>); +impl PruningMap +where + T: Config, + I: 'static, +{ + pub(crate) fn pruning_map_offchain_key(block: T::BlockNumber) -> sp_std::prelude::Vec { + (T::INDEXING_PREFIX, block, OFFCHAIN_PRUNING_MAP_KEY_SUFFIX).encode() + } + + /// Append `hash` to the list of parent hashes for `block` in offchain db. + pub fn append(block: T::BlockNumber, hash: ::Hash) { + let map_key = Self::pruning_map_offchain_key(block); + offchain::local_storage_get(StorageKind::PERSISTENT, &map_key) + .and_then(|v| codec::Decode::decode(&mut &*v).ok()) + .or_else(|| Some(Vec::<::Hash>::new())) + .map(|mut parents| { + parents.push(hash); + offchain::local_storage_set( + StorageKind::PERSISTENT, + &map_key, + &Encode::encode(&parents), + ); + }); + } + + /// Remove list of parent hashes for `block` from offchain db and return it. + pub fn remove(block: T::BlockNumber) -> Option::Hash>> { + let map_key = Self::pruning_map_offchain_key(block); + offchain::local_storage_get(StorageKind::PERSISTENT, &map_key).and_then(|v| { + offchain::local_storage_clear(StorageKind::PERSISTENT, &map_key); + codec::Decode::decode(&mut &*v).ok() + }) + } +} /// A storage layer for MMR. /// @@ -86,24 +119,17 @@ where /// on `frame_system::block_hash(block_num)` map which only holds the last /// `frame_system::BlockHashCount` blocks. /// + /// For the canonicalized block, prune all nodes pertaining to other forks from offchain db. + /// /// Should only be called from offchain context, because it requires both read and write /// access to offchain db. - pub(crate) fn canonicalize_offchain(block: T::BlockNumber) { + pub(crate) fn canonicalize_and_prune(block: T::BlockNumber) { use sp_runtime::traits::UniqueSaturatedInto; - let map_key = Pallet::::pruning_map_offchain_key(); - let mut pruning_map: PruningMap = - offchain::local_storage_get(StorageKind::PERSISTENT, &map_key) - .and_then(|v| codec::Decode::decode(&mut &*v).ok()) - .unwrap_or_default(); - // Add "block_num -> hash" mapping to offchain db, - // with all forks pushing hashes to same entry (for a particular block number). + // with all forks pushing hashes to same entry (same block number). let parent_hash = >::parent_hash(); - pruning_map - .entry(block) - .and_modify(|e| e.push(parent_hash)) - .or_insert(vec![parent_hash]); + PruningMap::::append(block, parent_hash); // Effectively move a rolling window of fork-unique leaves. Once out of the window, leaves // are "canonicalized" in offchain by moving them under `Pallet::node_canon_offchain_key`. @@ -128,9 +154,8 @@ where let to_canon_hash = >::block_hash(to_canon_block_num); Self::canonicalize_nodes_for_hash(&to_canon_nodes, to_canon_hash); - // Get all the forks to prune, also remove them from the pruning_map. - pruning_map - .remove(&to_canon_block_num) + // Get all the forks to prune, also remove them from the offchain pruning_map. + PruningMap::::remove(to_canon_block_num) .map(|forks| { Self::prune_nodes_for_forks(&to_canon_nodes, forks); }) @@ -142,12 +167,6 @@ where ); }) } - // Save new, updated pruning map. - offchain::local_storage_set( - StorageKind::PERSISTENT, - &map_key, - &Encode::encode(&pruning_map), - ); } fn prune_nodes_for_forks(nodes: &[NodeIndex], forks: Vec<::Hash>) { diff --git a/frame/merkle-mountain-range/src/tests.rs b/frame/merkle-mountain-range/src/tests.rs index 2b5e68873b0a8..336297cb2995b 100644 --- a/frame/merkle-mountain-range/src/tests.rs +++ b/frame/merkle-mountain-range/src/tests.rs @@ -15,7 +15,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{mmr::utils, mock::*, *}; +use crate::{ + mmr::{storage::PruningMap, utils}, + mock::*, + *, +}; use frame_support::traits::{Get, OnInitialize}; use mmr_lib::helper; @@ -447,6 +451,88 @@ fn should_verify_on_the_next_block_since_there_is_no_pruning_yet() { }); } +#[test] +fn should_verify_pruning_map() { + use sp_core::offchain::StorageKind; + use sp_io::offchain; + + let _ = env_logger::try_init(); + let mut ext = new_test_ext(); + register_offchain_ext(&mut ext); + + ext.execute_with(|| { + type TestPruningMap = PruningMap; + fn offchain_decoded(key: Vec) -> Option> { + offchain::local_storage_get(StorageKind::PERSISTENT, &key) + .and_then(|v| codec::Decode::decode(&mut &*v).ok()) + } + + // test append + { + TestPruningMap::append(1, H256::repeat_byte(1)); + + TestPruningMap::append(2, H256::repeat_byte(21)); + TestPruningMap::append(2, H256::repeat_byte(22)); + + TestPruningMap::append(3, H256::repeat_byte(31)); + TestPruningMap::append(3, H256::repeat_byte(32)); + TestPruningMap::append(3, H256::repeat_byte(33)); + + // `0` not present + let map_key = TestPruningMap::pruning_map_offchain_key(0); + assert_eq!(offchain::local_storage_get(StorageKind::PERSISTENT, &map_key), None); + + // verify `1` entries + let map_key = TestPruningMap::pruning_map_offchain_key(1); + let expected = vec![H256::repeat_byte(1)]; + assert_eq!(offchain_decoded(map_key), Some(expected)); + + // verify `2` entries + let map_key = TestPruningMap::pruning_map_offchain_key(2); + let expected = vec![H256::repeat_byte(21), H256::repeat_byte(22)]; + assert_eq!(offchain_decoded(map_key), Some(expected)); + + // verify `3` entries + let map_key = TestPruningMap::pruning_map_offchain_key(3); + let expected = + vec![H256::repeat_byte(31), H256::repeat_byte(32), H256::repeat_byte(33)]; + assert_eq!(offchain_decoded(map_key), Some(expected)); + + // `4` not present + let map_key = TestPruningMap::pruning_map_offchain_key(4); + assert_eq!(offchain::local_storage_get(StorageKind::PERSISTENT, &map_key), None); + } + + // test remove + { + // `0` doesn't return anything + assert_eq!(TestPruningMap::remove(0), None); + + // remove and verify `1` entries + let expected = vec![H256::repeat_byte(1)]; + assert_eq!(TestPruningMap::remove(1), Some(expected)); + + // remove and verify `2` entries + let expected = vec![H256::repeat_byte(21), H256::repeat_byte(22)]; + assert_eq!(TestPruningMap::remove(2), Some(expected)); + + // remove and verify `3` entries + let expected = + vec![H256::repeat_byte(31), H256::repeat_byte(32), H256::repeat_byte(33)]; + assert_eq!(TestPruningMap::remove(3), Some(expected)); + + // `4` doesn't return anything + assert_eq!(TestPruningMap::remove(4), None); + + // no entries left in offchain map + for block in 0..5 { + let map_key = TestPruningMap::pruning_map_offchain_key(block); + assert_eq!(offchain::local_storage_get(StorageKind::PERSISTENT, &map_key), None); + } + } + }) +} + #[test] fn should_canonicalize_offchain() { use frame_support::traits::Hooks; From 4e6de82a0ce19fe8e01d0992ce182a2db38112e0 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 1 Jul 2022 18:34:21 +0300 Subject: [PATCH 27/31] pallet-mmr: improve MMRStore::get() Do the math and retrieve node using correct (canon or non-canon) offchain db key, instead of blindly looking in both canon and non-canon offchain db locations for each node. Still fallback on looking at both if for any reason it's not where expected. Signed-off-by: acatangiu --- .../merkle-mountain-range/src/mmr/storage.rs | 37 +++++++++++++++---- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 92eeda24403cb..81f5077df2392 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -22,6 +22,7 @@ use frame_support::traits::Get; use mmr_lib::helper; use sp_core::offchain::StorageKind; use sp_io::{offchain, offchain_index}; +use sp_runtime::traits::UniqueSaturatedInto; use sp_std::iter::Peekable; #[cfg(not(feature = "std"))] use sp_std::prelude::*; @@ -124,8 +125,6 @@ where /// Should only be called from offchain context, because it requires both read and write /// access to offchain db. pub(crate) fn canonicalize_and_prune(block: T::BlockNumber) { - use sp_runtime::traits::UniqueSaturatedInto; - // Add "block_num -> hash" mapping to offchain db, // with all forks pushing hashes to same entry (same block number). let parent_hash = >::parent_hash(); @@ -212,13 +211,33 @@ where L: primitives::FullLeaf + codec::Decode, { fn get_elem(&self, pos: NodeIndex) -> mmr_lib::Result>> { - let leaves_count = NumberOfLeaves::::get(); - // Get the parent hash of the ancestor block that added node at index `pos`. - // Use the hash as extra identifier to differentiate between various `pos` entries - // in offchain DB coming from various chain forks. + let leaves = NumberOfLeaves::::get(); + // Find out which leaf added node `pos` in the MMR. let ancestor_leaf_idx = NodesUtils::leaf_index_that_added_node(pos); + + let window_size = + ::BlockHashCount::get().unique_saturated_into(); + // Leaves older than this window should have been canonicalized. + if leaves.saturating_sub(ancestor_leaf_idx) > window_size { + let key = Pallet::::node_canon_offchain_key(pos); + frame_support::log::debug!( + target: "runtime::mmr", "offchain get {}: leaf idx {:?}, key {:?}", + pos, ancestor_leaf_idx, key + ); + // Just for safety, to easily handle runtime upgrades where any of the window params + // change and maybe we mess up storage migration, + // return _if and only if_ node is found (in normal conditions it's always found), + if let Some(elem) = + sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) + { + return Ok(codec::Decode::decode(&mut &*elem).ok()) + } + // BUT if we DID MESS UP, fall through to searching node using fork-specific key. + } + + // Leaves still within the window will be found in offchain db under fork-aware keys. let ancestor_parent_block_num = - Pallet::::leaf_index_to_parent_block_num(ancestor_leaf_idx, leaves_count); + Pallet::::leaf_index_to_parent_block_num(ancestor_leaf_idx, leaves); let ancestor_parent_hash = >::block_hash(ancestor_parent_block_num); let key = Pallet::::node_offchain_key(ancestor_parent_hash, pos); frame_support::log::debug!( @@ -228,6 +247,10 @@ where // Retrieve the element from Off-chain DB. Ok(sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) .or_else(|| { + // Again, this is just us being extra paranoid. + // We get here only if we mess up a storage migration for a runtime upgrades where + // say the window is increased, and for a little while following the upgrade there's + // leaves inside new 'window' that had been already canonicalized before upgrade. let key = Pallet::::node_canon_offchain_key(pos); sp_io::offchain::local_storage_get(sp_core::offchain::StorageKind::PERSISTENT, &key) }) From 6335512b7cd2d8d72f7f9271221ee2f19fe76e50 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 5 Jul 2022 18:01:27 +0300 Subject: [PATCH 28/31] pallet-mmr: storage: improve logs --- .../merkle-mountain-range/src/mmr/storage.rs | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/frame/merkle-mountain-range/src/mmr/storage.rs b/frame/merkle-mountain-range/src/mmr/storage.rs index 81f5077df2392..d31262d4d7f2f 100644 --- a/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/frame/merkle-mountain-range/src/mmr/storage.rs @@ -142,7 +142,7 @@ where // and all the nodes added by that leaf. let to_canon_nodes = NodesUtils::right_branch_ending_in_leaf(to_canon_leaf); frame_support::log::debug!( - target: "runtime::mmr", "Nodes to canon for leaf {}: {:?}", + target: "runtime::mmr::offchain", "Nodes to canon for leaf {}: {:?}", to_canon_leaf, to_canon_nodes ); // For this block number there may be node entries saved from multiple forks. @@ -159,8 +159,8 @@ where Self::prune_nodes_for_forks(&to_canon_nodes, forks); }) .unwrap_or_else(|| { - frame_support::log::debug!( - target: "runtime::mmr", + frame_support::log::error!( + target: "runtime::mmr::offchain", "Offchain: could not prune: no entry in pruning map for block {:?}", to_canon_block_num ); @@ -172,6 +172,11 @@ where for hash in forks { for pos in nodes { let key = Pallet::::node_offchain_key(hash, *pos); + frame_support::log::debug!( + target: "runtime::mmr::offchain", + "Clear elem at pos {} with key {:?}", + pos, key + ); offchain::local_storage_clear(StorageKind::PERSISTENT, &key); } } @@ -189,14 +194,14 @@ where // Add under new canon key. offchain::local_storage_set(StorageKind::PERSISTENT, &canon_key, &elem); frame_support::log::debug!( - target: "runtime::mmr", + target: "runtime::mmr::offchain", "Moved elem at pos {} from key {:?} to canon key {:?}", pos, key, canon_key ); } else { - frame_support::log::debug!( - target: "runtime::mmr", - "Offchain: could not canonicalize elem at pos {} using key {:?}", + frame_support::log::error!( + target: "runtime::mmr::offchain", + "Could not canonicalize elem at pos {} using key {:?}", pos, key ); } @@ -221,7 +226,7 @@ where if leaves.saturating_sub(ancestor_leaf_idx) > window_size { let key = Pallet::::node_canon_offchain_key(pos); frame_support::log::debug!( - target: "runtime::mmr", "offchain get {}: leaf idx {:?}, key {:?}", + target: "runtime::mmr::offchain", "offchain db get {}: leaf idx {:?}, key {:?}", pos, ancestor_leaf_idx, key ); // Just for safety, to easily handle runtime upgrades where any of the window params @@ -241,7 +246,7 @@ where let ancestor_parent_hash = >::block_hash(ancestor_parent_block_num); let key = Pallet::::node_offchain_key(ancestor_parent_hash, pos); frame_support::log::debug!( - target: "runtime::mmr", "offchain get {}: leaf idx {:?}, hash {:?}, key {:?}", + target: "runtime::mmr::offchain", "offchain db get {}: leaf idx {:?}, hash {:?}, key {:?}", pos, ancestor_leaf_idx, ancestor_parent_hash, key ); // Retrieve the element from Off-chain DB. @@ -278,6 +283,7 @@ where } frame_support::log::trace!( + target: "runtime::mmr", "elems: {:?}", elems.iter().map(|elem| elem.hash()).collect::>() ); @@ -312,7 +318,7 @@ where // only on the leaf's `node_index`. let key = Pallet::::node_offchain_key(parent_hash, node_index); frame_support::log::debug!( - target: "runtime::mmr", "offchain set: pos {} parent_hash {:?} key {:?}", + target: "runtime::mmr::offchain", "offchain db set: pos {} parent_hash {:?} key {:?}", node_index, parent_hash, key ); // Indexing API is used to store the full node content (both leaf and inner). @@ -350,8 +356,8 @@ fn peaks_to_prune_and_store( // both collections may share a common prefix. let peaks_before = if old_size == 0 { vec![] } else { helper::get_peaks(old_size) }; let peaks_after = helper::get_peaks(new_size); - frame_support::log::trace!("peaks_before: {:?}", peaks_before); - frame_support::log::trace!("peaks_after: {:?}", peaks_after); + frame_support::log::trace!(target: "runtime::mmr", "peaks_before: {:?}", peaks_before); + frame_support::log::trace!(target: "runtime::mmr", "peaks_after: {:?}", peaks_after); let mut peaks_before = peaks_before.into_iter().peekable(); let mut peaks_after = peaks_after.into_iter().peekable(); From eab718c60803b2f12a17add79061774595dc4401 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 5 Jul 2022 18:02:51 +0300 Subject: [PATCH 29/31] fix tests: correctly persist overlay runtime indexing API works on overlay, whereas offchain context bypasses overlay, so for loops > canon-window, canon would fail. --- frame/merkle-mountain-range/src/tests.rs | 45 ++++++++++++------------ 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/frame/merkle-mountain-range/src/tests.rs b/frame/merkle-mountain-range/src/tests.rs index 336297cb2995b..d7f7de71279f3 100644 --- a/frame/merkle-mountain-range/src/tests.rs +++ b/frame/merkle-mountain-range/src/tests.rs @@ -541,22 +541,23 @@ fn should_canonicalize_offchain() { let mut ext = new_test_ext(); register_offchain_ext(&mut ext); - // adding 13 blocks that we'll later check have been canonicalized. + // adding 13 blocks that we'll later check have been canonicalized, + // (test assumes `13 < frame_system::BlockHashCount`). let to_canon_count = 13u32; // add 3 blocks and verify leaves and nodes for them have been added to // offchain MMR using fork-proof keys. - ext.execute_with(|| { - for blocknum in 0..to_canon_count { + for blocknum in 0..to_canon_count { + ext.execute_with(|| { new_block(); as Hooks>::offchain_worker(blocknum.into()); - } - }); - ext.persist_offchain_overlay(); + }); + ext.persist_offchain_overlay(); + } let offchain_db = ext.offchain_db(); ext.execute_with(|| { - // verify leaves added by blocks 1..13 - for block_num in 1..to_canon_count { + // verify leaves added by blocks 1..=13 + for block_num in 1..=to_canon_count { let parent_num: BlockNumber = (block_num - 1).into(); let leaf_index = u64::from(block_num - 1); let pos = helper::leaf_index_to_pos(leaf_index.into()); @@ -595,18 +596,18 @@ fn should_canonicalize_offchain() { // add another `frame_system::BlockHashCount` blocks and verify all nodes and leaves // added by our original `to_canon_count` blocks have now been canonicalized in offchain db. - ext.execute_with(|| { - let block_hash_size: u64 = ::BlockHashCount::get(); - let base = to_canon_count; - for blocknum in base..(base + u32::try_from(block_hash_size).unwrap()) { + let block_hash_size: u64 = ::BlockHashCount::get(); + let base = to_canon_count; + for blocknum in base..(base + u32::try_from(block_hash_size).unwrap()) { + ext.execute_with(|| { new_block(); as Hooks>::offchain_worker(blocknum.into()); - } - }); - ext.persist_offchain_overlay(); + }); + ext.persist_offchain_overlay(); + } ext.execute_with(|| { - // verify leaves added by blocks 1..13, should be in offchain under canon key. - for block_num in 1..to_canon_count { + // verify leaves added by blocks 1..=13, should be in offchain under canon key. + for block_num in 1..=to_canon_count { let leaf_index = u64::from(block_num - 1); let pos = helper::leaf_index_to_pos(leaf_index.into()); let parent_num: BlockNumber = (block_num - 1).into(); @@ -658,13 +659,13 @@ fn should_verify_canonicalized() { // Verify that proofs can be generated (using leaves and nodes from full set) and verified. let mut ext = new_test_ext(); register_offchain_ext(&mut ext); - ext.execute_with(|| { - for blocknum in 0u32..(2 * block_hash_size).try_into().unwrap() { + for blocknum in 0u32..(2 * block_hash_size).try_into().unwrap() { + ext.execute_with(|| { new_block(); as Hooks>::offchain_worker(blocknum.into()); - } - }); - ext.persist_offchain_overlay(); + }); + ext.persist_offchain_overlay(); + } // Generate proofs for some blocks. let (leaves, proofs) = From 8a8f3d2c59233d1df61ba68ad1638c92f2c09e3a Mon Sep 17 00:00:00 2001 From: Robert Hambrock Date: Wed, 6 Jul 2022 12:50:55 +0200 Subject: [PATCH 30/31] pallet-mmr: fix numeric typo in test --- frame/merkle-mountain-range/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/merkle-mountain-range/src/tests.rs b/frame/merkle-mountain-range/src/tests.rs index d7f7de71279f3..53226f8419988 100644 --- a/frame/merkle-mountain-range/src/tests.rs +++ b/frame/merkle-mountain-range/src/tests.rs @@ -545,7 +545,7 @@ fn should_canonicalize_offchain() { // (test assumes `13 < frame_system::BlockHashCount`). let to_canon_count = 13u32; - // add 3 blocks and verify leaves and nodes for them have been added to + // add 13 blocks and verify leaves and nodes for them have been added to // offchain MMR using fork-proof keys. for blocknum in 0..to_canon_count { ext.execute_with(|| { From ba9fd713724c289d31ed5a62b4f53eb085c62ac2 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 7 Jul 2022 16:47:23 +0300 Subject: [PATCH 31/31] add comment around LeafData requirements Signed-off-by: acatangiu --- frame/merkle-mountain-range/src/lib.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index ed0214af1d3ec..9274e8e72c508 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -165,6 +165,12 @@ pub mod pallet { /// /// Note that the leaf at each block MUST be unique. You may want to include a block hash or /// block number as an easiest way to ensure that. + /// Also note that the leaf added by each block is expected to only reference data coming + /// from ancestor blocks (leaves are saved offchain using `(parent_hash, pos)` key to be + /// fork-resistant, as such conflicts could only happen on 1-block deep forks, which means + /// two forks with identical line of ancestors compete to write the same offchain key, but + /// that's fine as long as leaves only contain data coming from ancestors - conflicting + /// writes are identical). type LeafData: primitives::LeafDataProvider; /// A hook to act on the new MMR root.