From a6ec2974089998066585c883df8a20d0d5fdef64 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 7 Nov 2019 18:53:56 +0100 Subject: [PATCH 1/7] make linked_map generic functions better --- .../src/storage/generator/linked_map.rs | 130 +++++++++++------- 1 file changed, 79 insertions(+), 51 deletions(-) diff --git a/srml/support/src/storage/generator/linked_map.rs b/srml/support/src/storage/generator/linked_map.rs index 6643a37f4f50c..87b3063e29354 100644 --- a/srml/support/src/storage/generator/linked_map.rs +++ b/srml/support/src/storage/generator/linked_map.rs @@ -47,16 +47,21 @@ pub trait StorageLinkedMap { /// Hasher used to insert into storage. type Hasher: StorageHasher; + /// The family of key formats used for this map. + type KeyFormat: KeyFormat; + /// Prefix used to prepend each key. fn prefix() -> &'static [u8]; - /// Key used to store linked map head. - fn head_key() -> &'static [u8]; + /// The head key of the linked-map. + fn head_key() -> &'static [u8] { + ::head_key() + } - /// Convert an optionnal value retrieved from storage to the type queried. + /// Convert an optional value retrieved from storage to the type queried. fn from_optional_value_to_query(v: Option) -> Self::Query; - /// Convert a query to an optionnal value into storage. + /// Convert a query to an optional value into storage. fn from_query_to_optional_value(v: Self::Query) -> Option; /// Generate the full key used in top storage. @@ -64,14 +69,37 @@ pub trait StorageLinkedMap { where KeyArg: EncodeLike, { - let mut final_key = Self::prefix().to_vec(); - key.encode_to(&mut final_key); - Self::Hasher::hash(&final_key) + ::storage_linked_map_final_key::(Self::prefix(), key) } /// Generate the hashed key for head fn storage_linked_map_final_head_key() -> ::Output { - Self::Hasher::hash(Self::head_key()) + ::storage_linked_map_final_head_key() + } +} + +/// A type-abstracted key format used for a family of linked-map types. +pub trait KeyFormat { + type Hasher: StorageHasher; + + /// Key used to store linked map head. + fn head_key() -> &'static [u8]; + + /// Generate the full key used in top storage. + fn storage_linked_map_final_key(prefix: &[u8], key: K) + -> ::Output + where + K: Encode, + { + let mut final_key = prefix.to_vec(); + key.encode_to(&mut final_key); + ::hash(&final_key) + } + + fn storage_linked_map_final_head_key() + -> ::Output + { + ::hash(Self::head_key()) } } @@ -105,25 +133,26 @@ struct EncodeLikeLinkage, NKey: EncodeLike, Key: Enco } /// A key-value pair iterator for enumerable map. -pub struct Enumerator> { +pub struct Enumerator { next: Option, - _phantom: PhantomData<(G, V)>, + prefix: &'static [u8], + _phantom: PhantomData<(V, F)>, } -impl Iterator for Enumerator +impl Iterator for Enumerator where K: FullCodec, V: FullCodec, - G: StorageLinkedMap, + F: KeyFormat, { type Item = (K, V); fn next(&mut self) -> Option { let next = self.next.take()?; let (val, linkage): (V, Linkage) = { - let next_full_key = G::storage_linked_map_final_key(&next); + let next_full_key = F::storage_linked_map_final_key(self.prefix, &next); unhashed::get(next_full_key.as_ref()) - .expect("previous/next only contain existing entires; + .expect("previous/next only contain existing entries; we enumerate using next; entry exists; qed") }; @@ -136,33 +165,33 @@ where /// /// Takes care of updating previous and next elements points /// as well as updates head if the element is first or last. -fn remove_linkage(linkage: Linkage) +fn remove_linkage(linkage: Linkage, prefix: &[u8]) where K: FullCodec, V: FullCodec, - G: StorageLinkedMap, + F: KeyFormat, { let next_key = linkage.next.as_ref() - .map(G::storage_linked_map_final_key) + .map(|k| F::storage_linked_map_final_key(prefix, k)) .map(|x| x.as_ref().to_vec()); let prev_key = linkage.previous.as_ref() - .map(G::storage_linked_map_final_key) + .map(|k| F::storage_linked_map_final_key(prefix, k)) .map(|x| x.as_ref().to_vec()); if let Some(prev_key) = prev_key { // Retrieve previous element and update `next` - let mut res = read_with_linkage::<_, _, G>(prev_key.as_ref()) + let mut res = read_with_linkage::(prev_key.as_ref()) .expect("Linkage is updated in case entry is removed; it always points to existing keys; qed"); res.1.next = linkage.next; unhashed::put(prev_key.as_ref(), &res); } else { // we were first so let's update the head - write_head::<_, _, _, G>(linkage.next.as_ref()); + write_head::<&K, K, F>(linkage.next.as_ref()); } if let Some(next_key) = next_key { // Update previous of next element - let mut res = read_with_linkage::<_, _, G>(next_key.as_ref()) + let mut res = read_with_linkage::(next_key.as_ref()) .expect("Linkage is updated in case entry is removed; it always points to existing keys; qed"); res.1.previous = linkage.previous; @@ -171,11 +200,10 @@ where } /// Read the contained data and it's linkage. -fn read_with_linkage(key: &[u8]) -> Option<(V, Linkage)> +fn read_with_linkage(key: &[u8]) -> Option<(V, Linkage)> where K: FullCodec, V: FullCodec, - G: StorageLinkedMap, { unhashed::get(key) } @@ -183,18 +211,18 @@ where /// Generate linkage for newly inserted element. /// /// Takes care of updating head and previous head's pointer. -fn new_head_linkage(key: KeyArg) -> Linkage +fn new_head_linkage(key: KeyArg, prefix: &[u8]) -> Linkage where KeyArg: EncodeLike, K: FullCodec, V: FullCodec, - G: StorageLinkedMap, + F: KeyFormat, { - if let Some(head) = read_head::<_, _, G>() { + if let Some(head) = read_head::() { // update previous head predecessor { - let head_key = G::storage_linked_map_final_key(&head); - let (data, linkage) = read_with_linkage::<_, _, G>(head_key.as_ref()) + let head_key = F::storage_linked_map_final_key(prefix, &head); + let (data, linkage) = read_with_linkage::(head_key.as_ref()) .expect("head is set when first element is inserted and unset when last element is removed; if head is Some then it points to existing key; qed"); @@ -206,41 +234,39 @@ where unhashed::put(head_key.as_ref(), &(data, new_linkage)); } // update to current head - write_head::<_, _, _, G>(Some(key)); + write_head::<_, _, F>(Some(key)); // return linkage with pointer to previous head let mut linkage = Linkage::default(); linkage.next = Some(head); linkage } else { // we are first - update the head and produce empty linkage - write_head::<_, _, _, G>(Some(key)); + write_head::<_, _, F>(Some(key)); Linkage::default() } } /// Read current head pointer. -fn read_head() -> Option +fn read_head() -> Option where K: FullCodec, - V: FullCodec, - G: StorageLinkedMap, + F: KeyFormat, { - unhashed::get(G::storage_linked_map_final_head_key().as_ref()) + unhashed::get(F::storage_linked_map_final_head_key().as_ref()) } /// Overwrite current head pointer. /// /// If `None` is given head is removed from storage. -fn write_head(head: Option) +fn write_head(head: Option) where KeyArg: EncodeLike, K: FullCodec, - V: FullCodec, - G: StorageLinkedMap, + F: KeyFormat, { match head.as_ref() { - Some(head) => unhashed::put(G::storage_linked_map_final_head_key().as_ref(), head), - None => unhashed::kill(G::storage_linked_map_final_head_key().as_ref()), + Some(head) => unhashed::put(F::storage_linked_map_final_head_key().as_ref(), head), + None => unhashed::kill(F::storage_linked_map_final_head_key().as_ref()), } } @@ -252,7 +278,7 @@ where { type Query = G::Query; - type Enumerator = Enumerator; + type Enumerator = Enumerator; fn exists>(key: KeyArg) -> bool { unhashed::exists(Self::storage_linked_map_final_key(key).as_ref()) @@ -264,10 +290,11 @@ where } fn swap, KeyArg2: EncodeLike>(key1: KeyArg1, key2: KeyArg2) { + let prefix = Self::prefix(); let final_key1 = Self::storage_linked_map_final_key(Ref::from(&key1)); let final_key2 = Self::storage_linked_map_final_key(Ref::from(&key2)); - let full_value_1 = read_with_linkage::<_, _, G>(final_key1.as_ref()); - let full_value_2 = read_with_linkage::<_, _, G>(final_key2.as_ref()); + let full_value_1 = read_with_linkage::(final_key1.as_ref()); + let full_value_2 = read_with_linkage::(final_key2.as_ref()); match (full_value_1, full_value_2) { // Just keep linkage in order and only swap values. @@ -278,13 +305,13 @@ where // Remove key and insert the new one. (Some((value, _linkage)), None) => { Self::remove(key1); - let linkage = new_head_linkage::<_, _, _, G>(key2); + let linkage = new_head_linkage::<_, _, V, G::KeyFormat>(key2, prefix); unhashed::put(final_key2.as_ref(), &(value, linkage)); } // Remove key and insert the new one. (None, Some((value, _linkage))) => { Self::remove(key2); - let linkage = new_head_linkage::<_, _, _, G>(key1); + let linkage = new_head_linkage::<_, _, V, G::KeyFormat>(key1, prefix); unhashed::put(final_key1.as_ref(), &(value, linkage)); } // No-op. @@ -294,11 +321,11 @@ where fn insert, ValArg: EncodeLike>(key: KeyArg, val: ValArg) { let final_key = Self::storage_linked_map_final_key(Ref::from(&key)); - let linkage = match read_with_linkage::<_, _, G>(final_key.as_ref()) { + let linkage = match read_with_linkage::<_, V>(final_key.as_ref()) { // overwrite but reuse existing linkage Some((_data, linkage)) => linkage, // create new linkage - None => new_head_linkage::<_, _, _, G>(key), + None => new_head_linkage::<_, _, V, G::KeyFormat>(key, Self::prefix()), }; unhashed::put(final_key.as_ref(), &(val, linkage)) } @@ -310,7 +337,7 @@ where fn mutate, R, F: FnOnce(&mut Self::Query) -> R>(key: KeyArg, f: F) -> R { let final_key = Self::storage_linked_map_final_key(Ref::from(&key)); - let (mut val, _linkage) = read_with_linkage::<_, _, G>(final_key.as_ref()) + let (mut val, _linkage) = read_with_linkage::(final_key.as_ref()) .map(|(data, linkage)| (G::from_optional_value_to_query(Some(data)), Some(linkage))) .unwrap_or_else(|| (G::from_optional_value_to_query(None), None)); @@ -328,7 +355,7 @@ where let full_value: Option<(V, Linkage)> = unhashed::take(final_key.as_ref()); let value = full_value.map(|(data, linkage)| { - remove_linkage::<_, _, G>(linkage); + remove_linkage::(linkage, Self::prefix()); data }); @@ -336,14 +363,15 @@ where } fn enumerate() -> Self::Enumerator { - Enumerator::<_, _, G> { - next: read_head::<_, _, G>(), + Enumerator::<_, _, G::KeyFormat> { + next: read_head::<_, G::KeyFormat>(), + prefix: Self::prefix(), _phantom: Default::default(), } } fn head() -> Option { - read_head::<_, _, G>() + read_head::<_, G::KeyFormat>() } fn decode_len>(key: KeyArg) -> Result From 4b963b359e7312824d841e1a868768a384c2cc0d Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 8 Nov 2019 14:33:33 +0100 Subject: [PATCH 2/7] implement translation of linked map --- .../src/storage/generator/linked_map.rs | 60 ++++++++++++++++--- srml/support/src/storage/mod.rs | 19 ++++++ 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/srml/support/src/storage/generator/linked_map.rs b/srml/support/src/storage/generator/linked_map.rs index 87b3063e29354..74df234ac63ab 100644 --- a/srml/support/src/storage/generator/linked_map.rs +++ b/srml/support/src/storage/generator/linked_map.rs @@ -69,7 +69,7 @@ pub trait StorageLinkedMap { where KeyArg: EncodeLike, { - ::storage_linked_map_final_key::(Self::prefix(), key) + ::storage_linked_map_final_key::(Self::prefix(), &key) } /// Generate the hashed key for head @@ -86,7 +86,7 @@ pub trait KeyFormat { fn head_key() -> &'static [u8]; /// Generate the full key used in top storage. - fn storage_linked_map_final_key(prefix: &[u8], key: K) + fn storage_linked_map_final_key(prefix: &[u8], key: &K) -> ::Output where K: Encode, @@ -151,7 +151,7 @@ where let next = self.next.take()?; let (val, linkage): (V, Linkage) = { let next_full_key = F::storage_linked_map_final_key(self.prefix, &next); - unhashed::get(next_full_key.as_ref()) + read_with_linkage::(next_full_key.as_ref()) .expect("previous/next only contain existing entries; we enumerate using next; entry exists; qed") }; @@ -199,11 +199,11 @@ where } } -/// Read the contained data and it's linkage. +/// Read the contained data and its linkage. fn read_with_linkage(key: &[u8]) -> Option<(V, Linkage)> where - K: FullCodec, - V: FullCodec, + K: Decode, + V: Decode, { unhashed::get(key) } @@ -249,7 +249,7 @@ where /// Read current head pointer. fn read_head() -> Option where - K: FullCodec, + K: Decode, F: KeyFormat, { unhashed::get(F::storage_linked_map_final_head_key().as_ref()) @@ -388,4 +388,50 @@ where Ok(len) } } + + fn translate(translate_key: TK, translate_val: TV) -> Result<(), ()> + where K2: FullCodec + Clone, V2: Decode, TK: Fn(K2) -> K, TV: Fn(V2) -> V + { + let head_key = read_head::().ok_or(())?; + let prefix = G::prefix(); + + let mut current_key = head_key.clone(); + + let translate_linkage = |old: Linkage| -> Linkage { + Linkage { + previous: old.previous.map(&translate_key), + next: old.next.map(&translate_key), + } + }; + + loop { + let old_raw_key = G::KeyFormat::storage_linked_map_final_key(prefix, ¤t_key); + let (val, linkage) = read_with_linkage::(old_raw_key.as_ref()).ok_or(())?; + let previous = linkage.previous.clone(); + + let val = translate_val(val); + let linkage = translate_linkage(linkage); + + // now that we've successfully migrated the storage, kill the old key. + unhashed::kill(old_raw_key.as_ref()); + + // and write in the value and linkage under the new key. + let new_raw_key = G::storage_linked_map_final_key(&translate_key(current_key.clone())); + unhashed::put(new_raw_key.as_ref(), &(&val, &linkage)); + + match previous { + None => break, + Some(prev) => { current_key = prev }, + } + } + + // update head at end, since it means that we made it through the entire + // list. this allows us to early-exit on the first `Linkage` we cannot decode, + // and it's impossible that we would ever decode the first successfully but + // fail on subsequent ones as long as storage was not already in a degenerate state. + // that means the error return indicates no underlying change in storage validity. + write_head::<&K, K, G::KeyFormat>(Some(&translate_key(head_key))); + + Ok(()) + } } diff --git a/srml/support/src/storage/mod.rs b/srml/support/src/storage/mod.rs index f10deb93d241a..acc1e55c53e1d 100644 --- a/srml/support/src/storage/mod.rs +++ b/srml/support/src/storage/mod.rs @@ -230,6 +230,25 @@ pub trait StorageLinkedMap { /// function for this purpose. fn decode_len>(key: KeyArg) -> Result where V: codec::DecodeLength + Len; + + /// Translate the keys and values from some previous `(K2, V2)` to the current type. + /// + /// `TK` translates keys from the old type, and `TV` translates values. + /// + /// Returns `Err` if the map could not be interpreted as the old type, and Ok if it could. + /// + /// # Warning + /// + /// This function must be used with care, before being updated the storage still contains the + /// old type, thus other calls (such as `get`) will fail at decoding it. + /// + /// # Usage + /// + /// This would typically be called inside the module implementation of on_initialize, while + /// ensuring **no usage of this storage are made before the call to `on_initialize`**. (More + /// precisely prior initialized modules doesn't make use of this storage). + fn translate(translate_key: TK, translate_val: TV) -> Result<(), ()> + where K2: FullCodec + Clone, V2: Decode, TK: Fn(K2) -> K, TV: Fn(V2) -> V; } /// An implementation of a map with a two keys. From f1a0c8042acc7de6b1c009f1a7816b64403b2a0b Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 8 Nov 2019 15:04:17 +0100 Subject: [PATCH 3/7] proc-macro for linked map updated --- .../procedural/src/storage/instance_trait.rs | 1 + .../procedural/src/storage/storage_struct.rs | 22 ++++++++++++++----- srml/support/src/storage/generator/mod.rs | 2 +- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/srml/support/procedural/src/storage/instance_trait.rs b/srml/support/procedural/src/storage/instance_trait.rs index 1a7add89a4b80..9a31ea6fcd6d6 100644 --- a/srml/support/procedural/src/storage/instance_trait.rs +++ b/srml/support/procedural/src/storage/instance_trait.rs @@ -28,6 +28,7 @@ pub(crate) const DEFAULT_INSTANTIABLE_TRAIT_NAME: &str = "__GeneratedInstantiabl // prefix for consts in trait Instance pub(crate) const PREFIX_FOR: &str = "PREFIX_FOR_"; pub(crate) const HEAD_KEY_FOR: &str = "HEAD_KEY_FOR_"; +pub(crate) const LINKED_MAP_KEY_FORMAT_FOR: &str = "LINKED_MAP_KEY_FORMAT_FOR_"; // Used to generate the const: // `const $name: &'static str = $value_prefix ++ instance_prefix ++ $value_suffix` diff --git a/srml/support/procedural/src/storage/storage_struct.rs b/srml/support/procedural/src/storage/storage_struct.rs index e195fb53e8a2c..2c50f2f4898db 100644 --- a/srml/support/procedural/src/storage/storage_struct.rs +++ b/srml/support/procedural/src/storage/storage_struct.rs @@ -20,7 +20,7 @@ use proc_macro2::TokenStream; use quote::quote; use super::{ DeclStorageDefExt, StorageLineTypeDef, - instance_trait::{PREFIX_FOR, HEAD_KEY_FOR}, + instance_trait::{PREFIX_FOR, HEAD_KEY_FOR, LINKED_MAP_KEY_FORMAT_FOR}, }; fn from_optional_value_to_query(is_option: bool, default: &Option) -> TokenStream { @@ -156,21 +156,33 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre quote!( #prefix.as_bytes() ) }; + let key_format_struct = syn::Ident::new( + &format!("{}{}", LINKED_MAP_KEY_FORMAT_FOR, line.name), + proc_macro2::Span::call_site(), + ); + quote!( + #visibility struct #key_format_struct; + + impl #scrate::storage::generator::LinkedMapKeyFormat for #key_format_struct { + type Hasher = #scrate::#hasher; + + fn head_key() -> &'static [u8] { + #head_key + } + } + impl<#impl_trait> #scrate::#storage_generator_trait for #storage_struct #optional_storage_where_clause { type Query = #query_type; type Hasher = #scrate::#hasher; + type KeyFormat = #key_format_struct; fn prefix() -> &'static [u8] { #final_prefix } - fn head_key() -> &'static [u8] { - #head_key - } - fn from_optional_value_to_query(v: Option<#value_type>) -> Self::Query { #from_optional_value_to_query } diff --git a/srml/support/src/storage/generator/mod.rs b/srml/support/src/storage/generator/mod.rs index 1bda791023792..5d73d7bf49065 100644 --- a/srml/support/src/storage/generator/mod.rs +++ b/srml/support/src/storage/generator/mod.rs @@ -26,7 +26,7 @@ mod map; mod double_map; mod value; -pub use linked_map::{StorageLinkedMap, Enumerator, Linkage}; +pub use linked_map::{StorageLinkedMap, Enumerator, Linkage, KeyFormat as LinkedMapKeyFormat}; pub use map::StorageMap; pub use double_map::StorageDoubleMap; pub use value::StorageValue; From 77776afb045f5e9400e00bc775cb80b4cb237f13 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 8 Nov 2019 17:11:51 +0100 Subject: [PATCH 4/7] test linked map migration --- .../src/storage/generator/linked_map.rs | 29 +++++++--- srml/support/src/storage/generator/mod.rs | 55 ++++++++++++++++++- 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/srml/support/src/storage/generator/linked_map.rs b/srml/support/src/storage/generator/linked_map.rs index 74df234ac63ab..56edc1e83d981 100644 --- a/srml/support/src/storage/generator/linked_map.rs +++ b/srml/support/src/storage/generator/linked_map.rs @@ -139,6 +139,18 @@ pub struct Enumerator { _phantom: PhantomData<(V, F)>, } +impl Enumerator { + /// Create an explicit enumerator for testing. + #[cfg(test)] + pub fn from_head(head: K, prefix: &'static [u8]) -> Self { + Enumerator { + next: Some(head), + prefix, + _phantom: Default::default(), + } + } +} + impl Iterator for Enumerator where K: FullCodec, @@ -149,10 +161,11 @@ where fn next(&mut self) -> Option { let next = self.next.take()?; + let (val, linkage): (V, Linkage) = { let next_full_key = F::storage_linked_map_final_key(self.prefix, &next); read_with_linkage::(next_full_key.as_ref()) - .expect("previous/next only contain existing entries; + .expect("previous/next only contains existing entries; we enumerate using next; entry exists; qed") }; @@ -200,7 +213,7 @@ where } /// Read the contained data and its linkage. -fn read_with_linkage(key: &[u8]) -> Option<(V, Linkage)> +pub(super) fn read_with_linkage(key: &[u8]) -> Option<(V, Linkage)> where K: Decode, V: Decode, @@ -211,7 +224,7 @@ where /// Generate linkage for newly inserted element. /// /// Takes care of updating head and previous head's pointer. -fn new_head_linkage(key: KeyArg, prefix: &[u8]) -> Linkage +pub(super) fn new_head_linkage(key: KeyArg, prefix: &[u8]) -> Linkage where KeyArg: EncodeLike, K: FullCodec, @@ -247,7 +260,7 @@ where } /// Read current head pointer. -fn read_head() -> Option +pub(crate) fn read_head() -> Option where K: Decode, F: KeyFormat, @@ -258,7 +271,7 @@ where /// Overwrite current head pointer. /// /// If `None` is given head is removed from storage. -fn write_head(head: Option) +pub(super) fn write_head(head: Option) where KeyArg: EncodeLike, K: FullCodec, @@ -407,7 +420,7 @@ where loop { let old_raw_key = G::KeyFormat::storage_linked_map_final_key(prefix, ¤t_key); let (val, linkage) = read_with_linkage::(old_raw_key.as_ref()).ok_or(())?; - let previous = linkage.previous.clone(); + let next = linkage.next.clone(); let val = translate_val(val); let linkage = translate_linkage(linkage); @@ -419,9 +432,9 @@ where let new_raw_key = G::storage_linked_map_final_key(&translate_key(current_key.clone())); unhashed::put(new_raw_key.as_ref(), &(&val, &linkage)); - match previous { + match next { None => break, - Some(prev) => { current_key = prev }, + Some(next) => { current_key = next }, } } diff --git a/srml/support/src/storage/generator/mod.rs b/srml/support/src/storage/generator/mod.rs index 5d73d7bf49065..f546546dc7209 100644 --- a/srml/support/src/storage/generator/mod.rs +++ b/srml/support/src/storage/generator/mod.rs @@ -36,8 +36,8 @@ pub use value::StorageValue; #[allow(dead_code)] mod tests { use runtime_io::TestExternalities; - use codec::Encode; - use crate::storage::{unhashed, generator::StorageValue}; + use codec::{Encode, Decode}; + use crate::storage::{unhashed, generator::{StorageValue, StorageLinkedMap}}; struct Runtime {} pub trait Trait { @@ -54,9 +54,16 @@ mod tests { pub struct Module for enum Call where origin: T::Origin {} } + #[derive(Encode, Decode, Clone, Debug, Eq, PartialEq)] + struct NumberNumber { + a: u32, + b: u32, + } + crate::decl_storage! { trait Store for Module as Runtime { Value get(fn value) config(): (u64, u64); + NumberMap: linked_map NumberNumber => u64; } } @@ -78,4 +85,48 @@ mod tests { assert_eq!(Value::get(), (1111, 2222)); }) } + + #[test] + fn linked_map_translate_works() { + use super::linked_map::{self, Enumerator, KeyFormat}; + + type Format = >::KeyFormat; + + let t = GenesisConfig::default().build_storage().unwrap(); + TestExternalities::new(t).execute_with(|| { + let prefix = NumberMap::prefix(); + + // start with a map of u32 -> u32. + for i in 0u32..100u32 { + let final_key = ::storage_linked_map_final_key( + prefix, &i, + ); + + let linkage = linked_map::new_head_linkage::<_, u32, u32, Format>(&i, prefix); + unhashed::put(final_key.as_ref(), &(&i, linkage)); + } + + let head = linked_map::read_head::().unwrap(); + + assert_eq!( + Enumerator::::from_head(head, prefix).collect::>(), + (0..100).rev().map(|x| (x, x)).collect::>(), + ); + + // do translation. + NumberMap::translate( + |k: u32| NumberNumber { a: k, b: k }, + |v: u32| (v as u64) << 32 | v as u64, + ).unwrap(); + + assert!(linked_map::read_head::().is_some()); + assert_eq!( + NumberMap::enumerate().collect::>(), + (0..100u32).rev().map(|x| ( + NumberNumber { a: x, b: x }, + (x as u64) << 32 | x as u64, + )).collect::>(), + ); + }) + } } From 2e2e987066099da40d1736de5a711defa246c50e Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 8 Nov 2019 17:58:45 +0100 Subject: [PATCH 5/7] account for instances --- .../procedural/src/storage/storage_struct.rs | 42 +++++++++++++------ srml/support/test/tests/final_keys.rs | 2 +- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/srml/support/procedural/src/storage/storage_struct.rs b/srml/support/procedural/src/storage/storage_struct.rs index 2c50f2f4898db..e0fa7325f81e3 100644 --- a/srml/support/procedural/src/storage/storage_struct.rs +++ b/srml/support/procedural/src/storage/storage_struct.rs @@ -156,28 +156,46 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre quote!( #prefix.as_bytes() ) }; - let key_format_struct = syn::Ident::new( - &format!("{}{}", LINKED_MAP_KEY_FORMAT_FOR, line.name), - proc_macro2::Span::call_site(), - ); + let (key_format_type, key_format_decl) = { + let name = syn::Ident::new( + &format!("{}{}", LINKED_MAP_KEY_FORMAT_FOR, line.name), + proc_macro2::Span::call_site(), + ); - quote!( - #visibility struct #key_format_struct; + let format_type = quote!( #name<#optional_instance>); - impl #scrate::storage::generator::LinkedMapKeyFormat for #key_format_struct { - type Hasher = #scrate::#hasher; + let optional_phantom = optional_instance + .as_ref() + .map(|instance| quote!( #scrate::rstd::marker::PhantomData<#instance> )); + + let struct_decl = quote!( + #visibility struct #format_type(#optional_phantom); + ); + + let format_impl = quote!( + impl<#optional_instance_bound> #scrate::storage::generator::LinkedMapKeyFormat + for #format_type + { + type Hasher = #scrate::#hasher; - fn head_key() -> &'static [u8] { - #head_key + fn head_key() -> &'static [u8] { + #head_key + } } - } + ); + + (format_type, quote!( #struct_decl #format_impl )) + }; + + quote!( + #key_format_decl impl<#impl_trait> #scrate::#storage_generator_trait for #storage_struct #optional_storage_where_clause { type Query = #query_type; type Hasher = #scrate::#hasher; - type KeyFormat = #key_format_struct; + type KeyFormat = #key_format_type; fn prefix() -> &'static [u8] { #final_prefix diff --git a/srml/support/test/tests/final_keys.rs b/srml/support/test/tests/final_keys.rs index 44a6b540a7a0e..242923e9dae6a 100644 --- a/srml/support/test/tests/final_keys.rs +++ b/srml/support/test/tests/final_keys.rs @@ -156,7 +156,7 @@ fn final_keys_default_instance() { assert_eq!(unhashed::get::(&runtime_io::blake2_256(&k)), Some(2u32)); assert_eq!(unhashed::get::(&runtime_io::blake2_256(&head)), Some(1u32)); -< instance::LinkedMap2>::insert(1, 2); + >::insert(1, 2); let mut k = b"FinalKeysSome LinkedMap2".to_vec(); k.extend(1u32.encode()); assert_eq!(unhashed::get::(&runtime_io::twox_128(&k)), Some(2u32)); From 5e5422361ecb6a9011ea29d9b36686c5fae76754 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sat, 9 Nov 2019 14:50:43 +0100 Subject: [PATCH 6/7] address grumbles --- srml/support/procedural/src/lib.rs | 4 ++ .../procedural/src/storage/instance_trait.rs | 1 - .../procedural/src/storage/storage_struct.rs | 45 +++++-------------- 3 files changed, 14 insertions(+), 36 deletions(-) diff --git a/srml/support/procedural/src/lib.rs b/srml/support/procedural/src/lib.rs index 792f739f1c357..623f8fde4b345 100644 --- a/srml/support/procedural/src/lib.rs +++ b/srml/support/procedural/src/lib.rs @@ -86,6 +86,10 @@ use proc_macro::TokenStream; /// * `head_key`: `"head of " ++ $module_prefix ++ " " ++ $storage_name` /// * `Hasher`: $hash /// +/// All key formatting logic can be accessed in a type-agnostic format via the +/// [`KeyFormat`](../srml_support/storage/generator/trait.KeyFormat.html) trait, which +/// is implemented for the storage linked map type as well. +/// /// * Double map: `Foo: double_map hasher($hash1) u32, $hash2(u32) => u32`: Implements the /// [`StorageDoubleMap`](../srml_support/storage/trait.StorageDoubleMap.html) trait using the /// [`StorageDoubleMap generator`](../srml_support/storage/generator/trait.StorageDoubleMap.html). diff --git a/srml/support/procedural/src/storage/instance_trait.rs b/srml/support/procedural/src/storage/instance_trait.rs index 9a31ea6fcd6d6..1a7add89a4b80 100644 --- a/srml/support/procedural/src/storage/instance_trait.rs +++ b/srml/support/procedural/src/storage/instance_trait.rs @@ -28,7 +28,6 @@ pub(crate) const DEFAULT_INSTANTIABLE_TRAIT_NAME: &str = "__GeneratedInstantiabl // prefix for consts in trait Instance pub(crate) const PREFIX_FOR: &str = "PREFIX_FOR_"; pub(crate) const HEAD_KEY_FOR: &str = "HEAD_KEY_FOR_"; -pub(crate) const LINKED_MAP_KEY_FORMAT_FOR: &str = "LINKED_MAP_KEY_FORMAT_FOR_"; // Used to generate the const: // `const $name: &'static str = $value_prefix ++ instance_prefix ++ $value_suffix` diff --git a/srml/support/procedural/src/storage/storage_struct.rs b/srml/support/procedural/src/storage/storage_struct.rs index e0fa7325f81e3..5267876a4416c 100644 --- a/srml/support/procedural/src/storage/storage_struct.rs +++ b/srml/support/procedural/src/storage/storage_struct.rs @@ -20,7 +20,7 @@ use proc_macro2::TokenStream; use quote::quote; use super::{ DeclStorageDefExt, StorageLineTypeDef, - instance_trait::{PREFIX_FOR, HEAD_KEY_FOR, LINKED_MAP_KEY_FORMAT_FOR}, + instance_trait::{PREFIX_FOR, HEAD_KEY_FOR}, }; fn from_optional_value_to_query(is_option: bool, default: &Option) -> TokenStream { @@ -156,46 +156,13 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre quote!( #prefix.as_bytes() ) }; - let (key_format_type, key_format_decl) = { - let name = syn::Ident::new( - &format!("{}{}", LINKED_MAP_KEY_FORMAT_FOR, line.name), - proc_macro2::Span::call_site(), - ); - - let format_type = quote!( #name<#optional_instance>); - - let optional_phantom = optional_instance - .as_ref() - .map(|instance| quote!( #scrate::rstd::marker::PhantomData<#instance> )); - - let struct_decl = quote!( - #visibility struct #format_type(#optional_phantom); - ); - - let format_impl = quote!( - impl<#optional_instance_bound> #scrate::storage::generator::LinkedMapKeyFormat - for #format_type - { - type Hasher = #scrate::#hasher; - - fn head_key() -> &'static [u8] { - #head_key - } - } - ); - - (format_type, quote!( #struct_decl #format_impl )) - }; - quote!( - #key_format_decl - impl<#impl_trait> #scrate::#storage_generator_trait for #storage_struct #optional_storage_where_clause { type Query = #query_type; type Hasher = #scrate::#hasher; - type KeyFormat = #key_format_type; + type KeyFormat = Self; fn prefix() -> &'static [u8] { #final_prefix @@ -209,6 +176,14 @@ pub fn decl_and_impl(scrate: &TokenStream, def: &DeclStorageDefExt) -> TokenStre #from_query_to_optional_value } } + + impl<#impl_trait> #scrate::storage::generator::LinkedMapKeyFormat for #storage_struct { + type Hasher = #scrate::#hasher; + + fn head_key() -> &'static [u8] { + #head_key + } + } ) }, StorageLineTypeDef::DoubleMap(map) => { From 1c8e7d25346d7baad59b42327df0cceef2db2acb Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 12 Nov 2019 18:11:00 +0100 Subject: [PATCH 7/7] cut map short if migration fails --- .../src/storage/generator/linked_map.rs | 47 +++++++++++++------ srml/support/src/storage/mod.rs | 4 +- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/srml/support/src/storage/generator/linked_map.rs b/srml/support/src/storage/generator/linked_map.rs index 56edc1e83d981..9e2722d5fc496 100644 --- a/srml/support/src/storage/generator/linked_map.rs +++ b/srml/support/src/storage/generator/linked_map.rs @@ -402,14 +402,17 @@ where } } - fn translate(translate_key: TK, translate_val: TV) -> Result<(), ()> + fn translate(translate_key: TK, translate_val: TV) -> Result<(), Option> where K2: FullCodec + Clone, V2: Decode, TK: Fn(K2) -> K, TV: Fn(V2) -> V { - let head_key = read_head::().ok_or(())?; + let head_key = read_head::().ok_or(None)?; let prefix = G::prefix(); + let mut last_key = None; let mut current_key = head_key.clone(); + write_head::<&K, K, G::KeyFormat>(Some(&translate_key(head_key))); + let translate_linkage = |old: Linkage| -> Linkage { Linkage { previous: old.previous.map(&translate_key), @@ -419,32 +422,46 @@ where loop { let old_raw_key = G::KeyFormat::storage_linked_map_final_key(prefix, ¤t_key); - let (val, linkage) = read_with_linkage::(old_raw_key.as_ref()).ok_or(())?; + let x = unhashed::take(old_raw_key.as_ref()); + let (val, linkage): (V2, Linkage) = match x { + Some(v) => v, + None => { + // we failed to read value and linkage. Update the last key's linkage + // to end the map early, since it's impossible to iterate further. + if let Some(last_key) = last_key { + let last_raw_key = G::storage_linked_map_final_key(&last_key); + if let Some((val, mut linkage)) + = read_with_linkage::(last_raw_key.as_ref()) + { + // defensive: should always happen, since it was just written + // in the last iteration of the loop. + linkage.next = None; + unhashed::put(last_raw_key.as_ref(), &(&val, &linkage)); + } + } + + return Err(Some(current_key)); + } + }; let next = linkage.next.clone(); let val = translate_val(val); let linkage = translate_linkage(linkage); - // now that we've successfully migrated the storage, kill the old key. - unhashed::kill(old_raw_key.as_ref()); - // and write in the value and linkage under the new key. - let new_raw_key = G::storage_linked_map_final_key(&translate_key(current_key.clone())); + let new_key = translate_key(current_key.clone()); + let new_raw_key = G::storage_linked_map_final_key(&new_key); unhashed::put(new_raw_key.as_ref(), &(&val, &linkage)); match next { None => break, - Some(next) => { current_key = next }, + Some(next) => { + last_key = Some(new_key); + current_key = next + }, } } - // update head at end, since it means that we made it through the entire - // list. this allows us to early-exit on the first `Linkage` we cannot decode, - // and it's impossible that we would ever decode the first successfully but - // fail on subsequent ones as long as storage was not already in a degenerate state. - // that means the error return indicates no underlying change in storage validity. - write_head::<&K, K, G::KeyFormat>(Some(&translate_key(head_key))); - Ok(()) } } diff --git a/srml/support/src/storage/mod.rs b/srml/support/src/storage/mod.rs index acc1e55c53e1d..af64d2a179002 100644 --- a/srml/support/src/storage/mod.rs +++ b/srml/support/src/storage/mod.rs @@ -236,6 +236,8 @@ pub trait StorageLinkedMap { /// `TK` translates keys from the old type, and `TV` translates values. /// /// Returns `Err` if the map could not be interpreted as the old type, and Ok if it could. + /// The `Err` contains the first key which could not be migrated, or `None` if the + /// head of the list could not be read. /// /// # Warning /// @@ -247,7 +249,7 @@ pub trait StorageLinkedMap { /// This would typically be called inside the module implementation of on_initialize, while /// ensuring **no usage of this storage are made before the call to `on_initialize`**. (More /// precisely prior initialized modules doesn't make use of this storage). - fn translate(translate_key: TK, translate_val: TV) -> Result<(), ()> + fn translate(translate_key: TK, translate_val: TV) -> Result<(), Option> where K2: FullCodec + Clone, V2: Decode, TK: Fn(K2) -> K, TV: Fn(V2) -> V; }