From 8738eadcf0e5e885edcd42a5c3a6459db4ba048f Mon Sep 17 00:00:00 2001 From: Dinonard Date: Fri, 28 Oct 2022 15:15:27 +0200 Subject: [PATCH 1/3] Improved translate DB values --- frame/support/src/storage/generator/map.rs | 99 +++++++++++++++++++++- frame/support/src/storage/mod.rs | 26 ++++++ 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/frame/support/src/storage/generator/map.rs b/frame/support/src/storage/generator/map.rs index f6c8eaa270bb3..923642cc63134 100644 --- a/frame/support/src/storage/generator/map.rs +++ b/frame/support/src/storage/generator/map.rs @@ -17,7 +17,10 @@ use crate::{ hash::{ReversibleStorageHasher, StorageHasher}, - storage::{self, storage_prefix, unhashed, KeyPrefixIterator, PrefixIterator, StorageAppend}, + storage::{ + self, storage_prefix, unhashed, KeyPrefixIterator, PrefixIterator, StorageAppend, + TranslateResult, + }, Never, }; use codec::{Decode, Encode, EncodeLike, FullCodec, FullEncode}; @@ -207,6 +210,100 @@ where } } } + + // TODO: break the logic into two functions - single translate & iter translate like below. This + // will help simplify the functions & make them more readable + + fn single_translate Option>( + last_processed_key: Option>, + f: F, + ) -> TranslateResult { + let prefix = G::prefix_hash(); + let previous_key = last_processed_key.unwrap_or(prefix.clone()); + + let mut result = TranslateResult::default(); + + if let Some(next) = + sp_io::storage::next_key(&previous_key).filter(|n| n.starts_with(&prefix)) + { + result.last_processed_key = Some(next.clone()); + result.number += 1; + + let value = match unhashed::get::(&next) { + Some(value) => value, + None => { + log::error!("Invalid translate: fail to decode old value"); + return result; + }, + }; + + let mut key_material = G::Hasher::reverse(&next[prefix.len()..]); + let key = match K::decode(&mut key_material) { + Ok(key) => key, + Err(_) => { + log::error!("Invalid translate: fail to decode key"); + return result; + }, + }; + + match f(key, value) { + Some(new) => unhashed::put::(&previous_key, &new), + None => unhashed::kill(&previous_key), + } + + } else { + result.is_finalized = true; + } + + result + } + + fn new_translate Option>( + limit: Option, + last_processed_key: Option>, + mut f: F, + ) -> TranslateResult { + let prefix = G::prefix_hash(); + let mut previous_key = last_processed_key.unwrap_or(prefix.clone()); + let mut processed_values: u32 = 0; + let limit = limit.unwrap_or(u32::MAX); + while let Some(next) = + sp_io::storage::next_key(&previous_key).filter(|n| n.starts_with(&prefix)) + { + if processed_values >= limit { + break + } + + previous_key = next; + processed_values += 1; + + let value = match unhashed::get::(&previous_key) { + Some(value) => value, + None => { + log::error!("Invalid translate: fail to decode old value"); + continue + }, + }; + + let mut key_material = G::Hasher::reverse(&previous_key[prefix.len()..]); + let key = match K::decode(&mut key_material) { + Ok(key) => key, + Err(_) => { + log::error!("Invalid translate: fail to decode key"); + continue + }, + }; + + match f(key, value) { + Some(new) => unhashed::put::(&previous_key, &new), + None => unhashed::kill(&previous_key), + } + processed_values += 1; + } + + // (processed_values, Some(previous_key)) + Default::default() + } } impl> storage::StorageMap for G { diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 333f4382557b1..461a00672887f 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -295,6 +295,32 @@ pub trait IterableStorageMap: StorageMap { /// /// NOTE: If a value fail to decode because storage is corrupted then it is skipped. fn translate Option>(f: F); + + /// limit - If Some(x), limits number of translations that can be done by up to **x** + /// last_processed_key - If Some(k), starts iteration from **k** + /// + /// returns (number of translated values, last processed key/ if None it means either nothing + /// was processed or everything was processed) + fn new_translate Option>( + limit: Option, + last_processed_key: Option>, + f: F, + ) -> TranslateResult; + + fn single_translate Option>( + last_processed_key: Option>, + f: F, + ) -> TranslateResult; +} + +#[derive(Default)] +pub struct TranslateResult { + /// Number of successfully translated values + pub number: u32, + /// `true` in case no more values remain + pub is_finalized: bool, + /// Previous processed key, if any. + pub previous_key: Option>, } /// A strongly-typed double map in storage whose secondary keys and values can be iterated over. From 11f2948f564b8a2156ebd09ae191f23ca2b1f280 Mon Sep 17 00:00:00 2001 From: Dinonard Date: Tue, 6 Dec 2022 14:56:01 +0100 Subject: [PATCH 2/3] Additional changes --- frame/support/src/storage/generator/map.rs | 60 ++++++++-------------- 1 file changed, 21 insertions(+), 39 deletions(-) diff --git a/frame/support/src/storage/generator/map.rs b/frame/support/src/storage/generator/map.rs index 923642cc63134..0ee3bbe75f063 100644 --- a/frame/support/src/storage/generator/map.rs +++ b/frame/support/src/storage/generator/map.rs @@ -211,12 +211,9 @@ where } } - // TODO: break the logic into two functions - single translate & iter translate like below. This - // will help simplify the functions & make them more readable - fn single_translate Option>( last_processed_key: Option>, - f: F, + mut f: F, ) -> TranslateResult { let prefix = G::prefix_hash(); let previous_key = last_processed_key.unwrap_or(prefix.clone()); @@ -226,7 +223,7 @@ where if let Some(next) = sp_io::storage::next_key(&previous_key).filter(|n| n.starts_with(&prefix)) { - result.last_processed_key = Some(next.clone()); + result.previous_key = Some(next.clone()); result.number += 1; let value = match unhashed::get::(&next) { @@ -250,7 +247,7 @@ where Some(new) => unhashed::put::(&previous_key, &new), None => unhashed::kill(&previous_key), } - + } else { result.is_finalized = true; } @@ -260,49 +257,34 @@ where fn new_translate Option>( limit: Option, - last_processed_key: Option>, + mut last_processed_key: Option>, mut f: F, ) -> TranslateResult { - let prefix = G::prefix_hash(); - let mut previous_key = last_processed_key.unwrap_or(prefix.clone()); - let mut processed_values: u32 = 0; + // let prefix = G::prefix_hash(); + // let mut last_processed_key = last_processed_key.unwrap_or(prefix.clone()); let limit = limit.unwrap_or(u32::MAX); - while let Some(next) = - sp_io::storage::next_key(&previous_key).filter(|n| n.starts_with(&prefix)) - { - if processed_values >= limit { - break - } - previous_key = next; - processed_values += 1; + let mut result = TranslateResult::default(); - let value = match unhashed::get::(&previous_key) { - Some(value) => value, - None => { - log::error!("Invalid translate: fail to decode old value"); - continue - }, - }; + for _ in 0 .. limit { - let mut key_material = G::Hasher::reverse(&previous_key[prefix.len()..]); - let key = match K::decode(&mut key_material) { - Ok(key) => key, - Err(_) => { - log::error!("Invalid translate: fail to decode key"); - continue - }, - }; + let temp_res = Self::single_translate(last_processed_key, &mut f); - match f(key, value) { - Some(new) => unhashed::put::(&previous_key, &new), - None => unhashed::kill(&previous_key), + // TODO: add custom methods to the result object! + + if temp_res.is_finalized { + result.is_finalized = true; + break; } - processed_values += 1; + + result.previous_key = temp_res.previous_key.clone(); + result.number += 1; + + last_processed_key = temp_res.previous_key; + } - // (processed_values, Some(previous_key)) - Default::default() + result } } From c8b868361d7d312d051664be889a5e4e422e9782 Mon Sep 17 00:00:00 2001 From: Dinonard Date: Tue, 6 Dec 2022 15:10:41 +0100 Subject: [PATCH 3/3] Minor cleanup --- frame/support/src/storage/generator/map.rs | 20 +++++++------------- frame/support/src/storage/mod.rs | 14 ++++++++++---- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/frame/support/src/storage/generator/map.rs b/frame/support/src/storage/generator/map.rs index 0ee3bbe75f063..f46ed1e2ec7b6 100644 --- a/frame/support/src/storage/generator/map.rs +++ b/frame/support/src/storage/generator/map.rs @@ -213,7 +213,7 @@ where fn single_translate Option>( last_processed_key: Option>, - mut f: F, + mut f: F, ) -> TranslateResult { let prefix = G::prefix_hash(); let previous_key = last_processed_key.unwrap_or(prefix.clone()); @@ -230,7 +230,7 @@ where Some(value) => value, None => { log::error!("Invalid translate: fail to decode old value"); - return result; + return result }, }; @@ -239,7 +239,7 @@ where Ok(key) => key, Err(_) => { log::error!("Invalid translate: fail to decode key"); - return result; + return result }, }; @@ -247,7 +247,6 @@ where Some(new) => unhashed::put::(&previous_key, &new), None => unhashed::kill(&previous_key), } - } else { result.is_finalized = true; } @@ -255,33 +254,28 @@ where result } - fn new_translate Option>( + fn limited_translate Option>( limit: Option, mut last_processed_key: Option>, mut f: F, ) -> TranslateResult { - // let prefix = G::prefix_hash(); - // let mut last_processed_key = last_processed_key.unwrap_or(prefix.clone()); let limit = limit.unwrap_or(u32::MAX); - let mut result = TranslateResult::default(); - for _ in 0 .. limit { - + for _ in 0..limit { let temp_res = Self::single_translate(last_processed_key, &mut f); - // TODO: add custom methods to the result object! + // TODO: add custom methods to the result object and make this code cleaner! if temp_res.is_finalized { result.is_finalized = true; - break; + break } result.previous_key = temp_res.previous_key.clone(); result.number += 1; last_processed_key = temp_res.previous_key; - } result diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 461a00672887f..32ad52e3ba16d 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -297,16 +297,18 @@ pub trait IterableStorageMap: StorageMap { fn translate Option>(f: F); /// limit - If Some(x), limits number of translations that can be done by up to **x** - /// last_processed_key - If Some(k), starts iteration from **k** + /// last_processed_key - If Some(k), starts iteration from key after **k** /// - /// returns (number of translated values, last processed key/ if None it means either nothing - /// was processed or everything was processed) - fn new_translate Option>( + /// returns report on how much values were processed + fn limited_translate Option>( limit: Option, last_processed_key: Option>, f: F, ) -> TranslateResult; + /// last_processed_key - If Some(k), starts iteration from key after **k** + /// + /// returns report on whether the value was processed and if any more remain fn single_translate Option>( last_processed_key: Option>, f: F, @@ -323,6 +325,10 @@ pub struct TranslateResult { pub previous_key: Option>, } +impl TranslateResult { + // TODO +} + /// A strongly-typed double map in storage whose secondary keys and values can be iterated over. pub trait IterableStorageDoubleMap: StorageDoubleMap