From ed4f8a11ffc79a06c9bf67ffb96a398c816fb53e Mon Sep 17 00:00:00 2001 From: cheme Date: Wed, 23 Nov 2022 17:43:44 +0100 Subject: [PATCH 1/4] Add total nb to trie migration rpc --- .../rpc/state-trie-migration-rpc/src/lib.rs | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs b/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs index ab180c7d45d5b..7e24888f61e48 100644 --- a/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs +++ b/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs @@ -44,22 +44,25 @@ use trie_db::{ fn count_migrate<'a, H: Hasher>( storage: &'a dyn trie_db::HashDBRef>, root: &'a H::Out, -) -> std::result::Result<(u64, TrieDB<'a, 'a, H>), String> { +) -> std::result::Result<(u64, u64, TrieDB<'a, 'a, H>), String> { let mut nb = 0u64; + let mut total_nb = 0u64; let trie = TrieDBBuilder::new(storage, root).build(); let iter_node = TrieDBNodeIterator::new(&trie).map_err(|e| format!("TrieDB node iterator error: {}", e))?; for node in iter_node { let node = node.map_err(|e| format!("TrieDB node iterator error: {}", e))?; match node.2.node_plan() { - NodePlan::Leaf { value, .. } | NodePlan::NibbledBranch { value: Some(value), .. } => + NodePlan::Leaf { value, .. } | NodePlan::NibbledBranch { value: Some(value), .. } => { + total_nb += 1; if let ValuePlan::Inline(range) = value { if (range.end - range.start) as u32 >= sp_core::storage::TRIE_VALUE_NODE_THRESHOLD { nb += 1; } - }, + } + }, _ => (), } } @@ -67,7 +70,7 @@ fn count_migrate<'a, H: Hasher>( } /// Check trie migration status. -pub fn migration_status(backend: &B) -> std::result::Result<(u64, u64), String> +pub fn migration_status(backend: &B) -> std::result::Result<(u64, u64, u64, u64), String> where H: Hasher, H::Out: codec::Codec, @@ -75,9 +78,10 @@ where { let trie_backend = backend.as_trie_backend(); let essence = trie_backend.essence(); - let (nb_to_migrate, trie) = count_migrate(essence, essence.root())?; + let (nb_to_migrate, total_nb, trie) = count_migrate(essence, essence.root())?; let mut nb_to_migrate_child = 0; + let mut total_nb_child = 0; let mut child_roots: Vec<(ChildInfo, Vec)> = Vec::new(); // get all child trie roots for key_value in trie.iter().map_err(|e| format!("TrieDB node iterator error: {}", e))? { @@ -94,10 +98,12 @@ where let storage = KeySpacedDB::new(essence, child_info.keyspace()); child_root.as_mut()[..].copy_from_slice(&root[..]); - nb_to_migrate_child += count_migrate(&storage, &child_root)?.0; + let (nb, total_nb, _) = count_migrate(&storage, &child_root)?; + nb_to_migrate_child += nb; + total_nb_child += total_nb; } - Ok((nb_to_migrate, nb_to_migrate_child)) + Ok((nb_to_migrate, nb_to_migrate_child, total_nb, total_nb_child)) } #[derive(Serialize, Deserialize)] @@ -106,6 +112,8 @@ where pub struct MigrationStatusResult { top_remaining_to_migrate: u64, child_remaining_to_migrate: u64, + total_top: u64, + total_child: u64, } /// Migration RPC methods. @@ -146,11 +154,13 @@ where let hash = at.unwrap_or_else(|| self.client.info().best_hash); let state = self.backend.state_at(hash).map_err(error_into_rpc_err)?; - let (top, child) = migration_status(&state).map_err(error_into_rpc_err)?; + let (top, child, total_top, total_child) = migration_status(&state).map_err(error_into_rpc_err)?; Ok(MigrationStatusResult { top_remaining_to_migrate: top, child_remaining_to_migrate: child, + total_top, + total_child, }) } } From 0cc178fb7851afe6299a02a35990c4b42e551426 Mon Sep 17 00:00:00 2001 From: cheme Date: Wed, 23 Nov 2022 17:46:16 +0100 Subject: [PATCH 2/4] fix and format --- utils/frame/rpc/state-trie-migration-rpc/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs b/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs index 7e24888f61e48..56a4c34157e12 100644 --- a/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs +++ b/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs @@ -66,7 +66,7 @@ fn count_migrate<'a, H: Hasher>( _ => (), } } - Ok((nb, trie)) + Ok((nb, total_nb, trie)) } /// Check trie migration status. @@ -154,7 +154,8 @@ where let hash = at.unwrap_or_else(|| self.client.info().best_hash); let state = self.backend.state_at(hash).map_err(error_into_rpc_err)?; - let (top, child, total_top, total_child) = migration_status(&state).map_err(error_into_rpc_err)?; + let (top, child, total_top, total_child) = + migration_status(&state).map_err(error_into_rpc_err)?; Ok(MigrationStatusResult { top_remaining_to_migrate: top, From 19dadf4a4f84e9b09f51105765203c1df77c4667 Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 24 Nov 2022 15:09:43 +0100 Subject: [PATCH 3/4] Use struct instead of tuple --- .../rpc/state-trie-migration-rpc/src/lib.rs | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs b/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs index 56a4c34157e12..7d75338edd712 100644 --- a/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs +++ b/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs @@ -70,7 +70,7 @@ fn count_migrate<'a, H: Hasher>( } /// Check trie migration status. -pub fn migration_status(backend: &B) -> std::result::Result<(u64, u64, u64, u64), String> +pub fn migration_status(backend: &B) -> std::result::Result where H: Hasher, H::Out: codec::Codec, @@ -78,10 +78,10 @@ where { let trie_backend = backend.as_trie_backend(); let essence = trie_backend.essence(); - let (nb_to_migrate, total_nb, trie) = count_migrate(essence, essence.root())?; + let (top_remaining_to_migrate, total_top, trie) = count_migrate(essence, essence.root())?; - let mut nb_to_migrate_child = 0; - let mut total_nb_child = 0; + let mut child_remaining_to_migrate = 0; + let mut total_child = 0; let mut child_roots: Vec<(ChildInfo, Vec)> = Vec::new(); // get all child trie roots for key_value in trie.iter().map_err(|e| format!("TrieDB node iterator error: {}", e))? { @@ -98,12 +98,17 @@ where let storage = KeySpacedDB::new(essence, child_info.keyspace()); child_root.as_mut()[..].copy_from_slice(&root[..]); - let (nb, total_nb, _) = count_migrate(&storage, &child_root)?; - nb_to_migrate_child += nb; - total_nb_child += total_nb; + let (nb, total_top, _) = count_migrate(&storage, &child_root)?; + child_remaining_to_migrate += nb; + total_child += total_top; } - Ok((nb_to_migrate, nb_to_migrate_child, total_nb, total_nb_child)) + Ok(MigrationStatusResult { + top_remaining_to_migrate, + child_remaining_to_migrate, + total_top, + total_child, + }) } #[derive(Serialize, Deserialize)] @@ -154,15 +159,7 @@ where let hash = at.unwrap_or_else(|| self.client.info().best_hash); let state = self.backend.state_at(hash).map_err(error_into_rpc_err)?; - let (top, child, total_top, total_child) = - migration_status(&state).map_err(error_into_rpc_err)?; - - Ok(MigrationStatusResult { - top_remaining_to_migrate: top, - child_remaining_to_migrate: child, - total_top, - total_child, - }) + migration_status(&state).map_err(error_into_rpc_err) } } From ff0062d503309e24083c537445890c511a7b3382 Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 24 Nov 2022 15:30:55 +0100 Subject: [PATCH 4/4] fixes --- frame/state-trie-migration/src/lib.rs | 23 ++++++++++--------- .../rpc/state-trie-migration-rpc/src/lib.rs | 13 +++++++---- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/frame/state-trie-migration/src/lib.rs b/frame/state-trie-migration/src/lib.rs index 5255d4f6f3800..aab92e678e88c 100644 --- a/frame/state-trie-migration/src/lib.rs +++ b/frame/state-trie-migration/src/lib.rs @@ -1606,7 +1606,6 @@ mod test { pub(crate) mod remote_tests { use crate::{AutoLimits, MigrationLimits, Pallet as StateTrieMigration, LOG_TARGET}; use codec::Encode; - use frame_benchmarking::Zero; use frame_support::{ traits::{Get, Hooks}, weights::Weight, @@ -1614,7 +1613,7 @@ pub(crate) mod remote_tests { use frame_system::Pallet as System; use remote_externalities::Mode; use sp_core::H256; - use sp_runtime::traits::{Block as BlockT, HashFor, Header as _, One}; + use sp_runtime::traits::{Block as BlockT, HashFor, Header as _, One, Zero}; use thousands::Separable; #[allow(dead_code)] @@ -1663,18 +1662,20 @@ pub(crate) mod remote_tests { // set the version to 1, as if the upgrade happened. ext.state_version = sp_core::storage::StateVersion::V1; - let (top_left, child_left) = + let status = substrate_state_trie_migration_rpc::migration_status(&ext.as_backend()).unwrap(); assert!( - top_left > 0, + status.top_remaining_to_migrate > 0, "no node needs migrating, this probably means that state was initialized with `StateVersion::V1`", ); log::info!( target: LOG_TARGET, - "initial check: top_left: {}, child_left: {}", - top_left.separate_with_commas(), - child_left.separate_with_commas(), + "initial check: top_left: {}, child_left: {}, total_top {}, total_child {}", + status.top_remaining_to_migrate.separate_with_commas(), + status.child_remaining_to_migrate.separate_with_commas(), + status.total_top.separate_with_commas(), + status.total_child.separate_with_commas(), ); loop { @@ -1722,17 +1723,17 @@ pub(crate) mod remote_tests { ) }); - let (top_left, child_left) = + let status = substrate_state_trie_migration_rpc::migration_status(&ext.as_backend()).unwrap(); - assert_eq!(top_left, 0); - assert_eq!(child_left, 0); + assert_eq!(status.top_remaining_to_migrate, 0); + assert_eq!(status.child_remaining_to_migrate, 0); } } #[cfg(all(test, feature = "remote-test"))] mod remote_tests_local { use super::{ - mock::{Call as MockCall, *}, + mock::{RuntimeCall as MockCall, *}, remote_tests::run_with_limits, *, }; diff --git a/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs b/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs index 7d75338edd712..2140ee8845625 100644 --- a/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs +++ b/utils/frame/rpc/state-trie-migration-rpc/src/lib.rs @@ -111,14 +111,19 @@ where }) } +/// Current state migration status. #[derive(Serialize, Deserialize)] #[serde(rename_all = "camelCase")] #[serde(deny_unknown_fields)] pub struct MigrationStatusResult { - top_remaining_to_migrate: u64, - child_remaining_to_migrate: u64, - total_top: u64, - total_child: u64, + /// Number of top items that should migrate. + pub top_remaining_to_migrate: u64, + /// Number of child items that should migrate. + pub child_remaining_to_migrate: u64, + /// Number of top items that we will iterate on. + pub total_top: u64, + /// Number of child items that we will iterate on. + pub total_child: u64, } /// Migration RPC methods.