From 6440f6c266acf9bf5e9520060a6ad4f4b8896ed1 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 7 Apr 2021 11:38:58 +0200 Subject: [PATCH 01/15] prototype for shawn --- frame/example/src/lib.rs | 16 +++++ frame/support/src/storage/mod.rs | 120 ++++++++++++++++++++++++++++++- 2 files changed, 133 insertions(+), 3 deletions(-) diff --git a/frame/example/src/lib.rs b/frame/example/src/lib.rs index fd1bc292ac8aa..213760df8b5aa 100644 --- a/frame/example/src/lib.rs +++ b/frame/example/src/lib.rs @@ -598,6 +598,22 @@ pub mod pallet { #[pallet::getter(fn foo)] pub(super) type Foo = StorageValue<_, T::Balance, ValueQuery>; + frame_support::parameter_types! { + pub const Five: usize = 5; + } + + // TODO: we can avoid this probably. + impl Default for Five { + fn default() -> Self { + Self + } + } + + #[pallet::storage] + #[pallet::getter(fn bounded)] + pub(super) type BoundedStuff = + StorageValue<_, frame_support::storage::BoundedVec, ValueQuery>; + // The genesis config type. #[pallet::genesis_config] pub struct GenesisConfig { diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index e00a3fe831829..c6ce51cf144e3 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -811,16 +811,130 @@ mod private { impl Sealed for Vec {} impl Sealed for Digest {} + impl> Sealed for BoundedVec {} } impl StorageAppend for Vec {} impl StorageDecodeLength for Vec {} -/// We abuse the fact that SCALE does not put any marker into the encoding, i.e. -/// we only encode the internal vec and we can append to this vec. We have a test that ensures -/// that if the `Digest` format ever changes, we need to remove this here. +/// We abuse the fact that SCALE does not put any marker into the encoding, i.e. we only encode the +/// internal vec and we can append to this vec. We have a test that ensures that if the `Digest` +/// format ever changes, we need to remove this here. impl StorageAppend> for Digest {} +use crate::traits::Get; +use sp_std::convert::TryFrom; + +pub trait ValueType: FullCodec + Default {} +impl ValueType for T {} + +#[derive(Encode, Decode, Default)] +pub struct BoundedVec>(Vec, sp_std::marker::PhantomData); + +impl> BoundedVec { + fn unchecked_from(t: Vec) -> Self { + Self(t, Default::default()) + } + // TODO: add a subset of the Vec's API here that generally doesn't expand it in any way. Adding + // any API that can insert/append/enqueue is tricky and needs care. +} + +impl> TryFrom> for BoundedVec { + type Error = (); + fn try_from(t: Vec) -> Result { + if t.len() <= S::get() { + Ok(Self::unchecked_from(t)) + } else { + Err(()) + } + } +} + +// It is okay to give a non-mutable reference of the inner vec to anyone. +impl> AsRef> for BoundedVec { + fn as_ref(&self) -> &Vec { + &self.0 + } +} + +impl> codec::DecodeLength for BoundedVec { + fn len(mut self_encoded: &[u8]) -> Result { + use sp_std::convert::TryFrom; + // `BoundedVec` stored just a `Vec`, thus the length is at the beginning in + // `Compact` form, and same implementation as `Vec` can be used. + as codec::DecodeLength>::len(self_encoded) + } +} + +impl> StorageDecodeLength for BoundedVec {} + +pub trait TryAppend> { + fn try_append>(item: LikeT) -> Result<(), ()>; +} + +impl, V: generator::StorageValue>> TryAppend + for V +{ + fn try_append>(item: LikeT) -> Result<(), ()> { + let bound = S::get(); + let current = Self::decode_len().unwrap_or_default(); + dbg!(current, bound); + if (current + 1usize) <= bound { + // NOTE: we cannot reuse the implementation for `Vec` here because we never want to + // mark `BoundedVec` as `StorageAppend`. + let key = Self::storage_value_final_key(); + sp_io::storage::append(&key, item.encode()); + Ok(()) + } else { + Err(()) + } + } +} + +#[cfg(test)] +mod bounded_vec { + use super::*; + use sp_core::hashing::twox_128; + // use crate::hash::Identity; + use sp_io::TestExternalities; + // use generator::StorageValue as _; + use sp_std::convert::TryInto; + use crate::assert_ok; + + crate::parameter_types! { + pub const Seven: usize = 7; + } + + crate::generate_storage_alias! { Prefix, Foo => Value> } + + #[test] + fn decode_len_works() { + TestExternalities::default().execute_with(|| { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + Foo::put(bounded); + assert_eq!(Foo::decode_len().unwrap(), 3); + }); + + // TODO: also for map. + } + + #[test] + fn append_works() { + TestExternalities::default().execute_with(|| { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + Foo::put(bounded); + assert_ok!(Foo::try_append(4)); + assert_ok!(Foo::try_append(5)); + assert_ok!(Foo::try_append(6)); + assert_ok!(Foo::try_append(7)); + assert_eq!(Foo::decode_len().unwrap(), 7); + assert!(Foo::try_append(8).is_err()); + }); + + // TODO: also for map. + } +} + #[cfg(test)] mod test { use super::*; From 8c60a17c9aa40cdf1e8d877cb667e903f453120b Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 8 Apr 2021 10:14:03 +0200 Subject: [PATCH 02/15] Clean and document it --- frame/example/src/lib.rs | 55 ++++---- frame/support/src/storage/mod.rs | 215 +++++++++++++++++++++++++++---- 2 files changed, 216 insertions(+), 54 deletions(-) diff --git a/frame/example/src/lib.rs b/frame/example/src/lib.rs index 213760df8b5aa..ca391ef120300 100644 --- a/frame/example/src/lib.rs +++ b/frame/example/src/lib.rs @@ -26,8 +26,8 @@ //! //! ### Documentation Guidelines: //! -//! -//! +//! //!
    //!
  • Documentation comments (i.e. /// comment) - should //! accompany pallet functions and be restricted to the pallet interface, @@ -37,13 +37,13 @@ //! Capitalize the first word of each documentation comment and end it with //! a full stop. See //! Generic example of annotating source code with documentation comments
    • -//!
  • Self-documenting code - Try to refactor code to be self-documenting.
    • -//!
  • Code comments - Supplement complex code with a brief explanation, not every line of code.
    • -//!
  • Identifiers - surround by backticks (i.e. INHERENT_IDENTIFIER, InherentType, -//! u64)
    • -//!
  • Usage scenarios - should be simple doctests. The compiler should ensure they stay valid.
    • -//!
  • Extended tutorials - should be moved to external files and refer to.
    • +//! target="_blank"> Generic example of annotating source code with documentation +//! comments
  • Self-documenting code - Try to refactor code to be +//! self-documenting.
  • Code comments - Supplement complex code with a brief explanation, +//! not every line of code.
  • Identifiers - surround by backticks (i.e. +//! INHERENT_IDENTIFIER, InherentType, u64)
    • +//!
  • Usage scenarios - should be simple doctests. The compiler should ensure they stay +//! valid.
  • Extended tutorials - should be moved to external files and refer to.
    • //! //!
  • Mandatory - include all of the sections/subsections where MUST is specified.
    • //!
  • Optional - optionally include sections/subsections where CAN is specified.
    • @@ -84,12 +84,13 @@ //! //! \## Terminology //! -//! // Add terminology used in the custom pallet. Include concepts, storage items, or actions that you think -//! // deserve to be noted to give context to the rest of the documentation or pallet usage. The author needs to -//! // use some judgment about what is included. We don't want a list of every storage item nor types - the user -//! // can go to the code for that. For example, "transfer fee" is obvious and should not be included, but -//! // "free balance" and "reserved balance" should be noted to give context to the pallet. -//! // Please do not link to outside resources. The reference docs should be the ultimate source of truth. +//! // Add terminology used in the custom pallet. Include concepts, storage items, or actions that +//! you think // deserve to be noted to give context to the rest of the documentation or pallet +//! usage. The author needs to // use some judgment about what is included. We don't want a list of +//! every storage item nor types - the user // can go to the code for that. For example, "transfer +//! fee" is obvious and should not be included, but // "free balance" and "reserved balance" should +//! be noted to give context to the pallet. // Please do not link to outside resources. The +//! reference docs should be the ultimate source of truth. //! //! //! @@ -106,7 +107,8 @@ //! \#### //! //! // Describe requirements prior to interacting with the custom pallet. -//! // Describe the process of interacting with the custom pallet for this scenario and public API functions used. +//! // Describe the process of interacting with the custom pallet for this scenario and public API +//! functions used. //! //! \## Interface //! @@ -130,14 +132,15 @@ //! //! //! -//! // Reference documentation of aspects such as `storageItems` and `dispatchable` functions should only be -//! // included in the Rustdocs for Substrate and not repeated in the README file. +//! // Reference documentation of aspects such as `storageItems` and `dispatchable` functions should +//! only be // included in the Rustdocs for Substrate and not repeated in the README file. //! //! \### Dispatchable Functions //! //! //! -//! // A brief description of dispatchable functions and a link to the rustdoc with their actual documentation. +//! // A brief description of dispatchable functions and a link to the rustdoc with their actual +//! documentation. //! //! // MUST have link to Call enum //! // MUST have origin information included in function doc @@ -154,7 +157,8 @@ //! //! //! -//! // It is up to the writer of the respective pallet (with respect to how much information to provide). +//! // It is up to the writer of the respective pallet (with respect to how much information to +//! provide). //! //! \#### Public Inspection functions - Immutable (getters) //! @@ -217,7 +221,8 @@ //! //! \### Simple Code Snippet //! -//! // Show a simple example (e.g. how to query a public getter function of ) +//! // Show a simple example (e.g. how to query a public getter function of +//! ) //! //! \### Example from FRAME //! @@ -599,16 +604,10 @@ pub mod pallet { pub(super) type Foo = StorageValue<_, T::Balance, ValueQuery>; frame_support::parameter_types! { + #[derive(Default)] // TODO: might be avoidable after https://github.com/paritytech/substrate/pull/8542/files pub const Five: usize = 5; } - // TODO: we can avoid this probably. - impl Default for Five { - fn default() -> Self { - Self - } - } - #[pallet::storage] #[pallet::getter(fn bounded)] pub(super) type BoundedStuff = diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index c6ce51cf144e3..65f5105008956 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -19,6 +19,7 @@ use sp_core::storage::ChildInfo; use sp_std::prelude::*; +use sp_std::iter; use codec::{FullCodec, FullEncode, Encode, EncodeLike, Decode}; use crate::hash::{Twox128, StorageHasher, ReversibleStorageHasher}; use sp_runtime::generic::{Digest, DigestItem}; @@ -811,7 +812,7 @@ mod private { impl Sealed for Vec {} impl Sealed for Digest {} - impl> Sealed for BoundedVec {} + impl> Sealed for BoundedVec {} } impl StorageAppend for Vec {} @@ -825,21 +826,107 @@ impl StorageAppend> for Digest {} use crate::traits::Get; use sp_std::convert::TryFrom; -pub trait ValueType: FullCodec + Default {} -impl ValueType for T {} +/// Marker trait for types `T` that can be stored in storage as `Vec`. +pub trait Value: FullCodec + Default + sp_std::fmt::Debug + Eq + PartialEq {} +impl Value for T {} -#[derive(Encode, Decode, Default)] -pub struct BoundedVec>(Vec, sp_std::marker::PhantomData); +/// A bounded vector. +/// +/// It implementations for efficient append and length decoding, as with a normal `Vec<_>`, once put +/// into a [`StorageValue`]. +/// +/// As the name suggests, the length of the queue is always bounded. All internal operations ensure +/// this bound is respected. +#[derive(Encode, Decode, Default, Clone, crate::RuntimeDebugNoBound)] +pub struct BoundedVec>(Vec, sp_std::marker::PhantomData); + +// NOTE: we could also implement this as: +// impl, S2: Get> PartialEq> for BoundedVec +// to allow comparison of bounded vectors with different bounds. +impl> PartialEq for BoundedVec { + fn eq(&self, rhs: &Self) -> bool { + self.0 == rhs.0 + } +} +impl> Eq for BoundedVec {} -impl> BoundedVec { +impl> BoundedVec { + /// Create `Self` from `t` without any checks. fn unchecked_from(t: Vec) -> Self { Self(t, Default::default()) } - // TODO: add a subset of the Vec's API here that generally doesn't expand it in any way. Adding - // any API that can insert/append/enqueue is tricky and needs care. + + /// Consume self, and return the inner `Vec`. Henceforth, the `Vec<_>` can be altered in an + /// arbitrary way. At some point, if the reverse conversion is required, `TryFrom>` can + /// be used. + /// + /// This is useful for cases if you need access to an internal API of the inner `Vec<_>` which + /// is not provided by the wrapper `BoundedVec`. + pub fn inner(self) -> Vec { + self.0 + } + + //// Return the length of the inner vector. + pub fn len(&self) -> usize { + self.0.len() + } + + /// Exactly the same semantics as [`Vec::insert`], but returns an `Err` (and is a noop) if the + /// new length of the vector exceeds `S`. + /// + /// # Panics + /// + /// Panics if `index > len`. + pub fn try_insert(&mut self, index: usize, element: T) -> Result<(), ()> { + if self.len() < S::get() { + self.0.insert(index, element); + Ok(()) + } else { + Err(()) + } + } + + /// Exactly the same semantics as [`Vec::remove`]. + /// + /// # Panics + /// + /// Panics if `index` is out of bounds. + pub fn remove(&mut self, index: usize) { + self.0.remove(index); + } + + /// Exactly the same semantics as [`Vec::swap_remove`]. + /// + /// # Panics + /// + /// Panics if `index` is out of bounds. + pub fn swap_remove(&mut self, index: usize) { + self.0.swap_remove(index); + } + + /// Returns an iterator over inner values. + pub fn iter(&self) -> sp_std::slice::Iter<'_, T> { + self.into_iter() + } +} + +impl> iter::IntoIterator for BoundedVec { + type Item = T; + type IntoIter = sp_std::vec::IntoIter; + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl<'a, T: Value, S: Get> iter::IntoIterator for &'a BoundedVec { + type Item = as sp_std::iter::Iterator>::Item; + type IntoIter = sp_std::slice::Iter<'a, T>; + fn into_iter(self) -> Self::IntoIter { + self.0.as_slice().into_iter() + } } -impl> TryFrom> for BoundedVec { +impl> TryFrom> for BoundedVec { type Error = (); fn try_from(t: Vec) -> Result { if t.len() <= S::get() { @@ -851,35 +938,42 @@ impl> TryFrom> for BoundedVec { } // It is okay to give a non-mutable reference of the inner vec to anyone. -impl> AsRef> for BoundedVec { +impl> AsRef> for BoundedVec { fn as_ref(&self) -> &Vec { &self.0 } } -impl> codec::DecodeLength for BoundedVec { - fn len(mut self_encoded: &[u8]) -> Result { - use sp_std::convert::TryFrom; +impl> codec::DecodeLength for BoundedVec { + fn len(self_encoded: &[u8]) -> Result { // `BoundedVec` stored just a `Vec`, thus the length is at the beginning in // `Compact` form, and same implementation as `Vec` can be used. as codec::DecodeLength>::len(self_encoded) } } -impl> StorageDecodeLength for BoundedVec {} +impl> StorageDecodeLength for BoundedVec {} -pub trait TryAppend> { +/// Storage value that is *maybe* capable of [`StorageAppend`]. +pub trait TryAppendValue> { fn try_append>(item: LikeT) -> Result<(), ()>; } -impl, V: generator::StorageValue>> TryAppend - for V +/// Storage map that is *maybe* capable of [`StorageAppend`]. +pub trait TryAppendMap> { + fn try_append + Clone, LikeT: EncodeLike>( + key: LikeK, + item: LikeT, + ) -> Result<(), ()>; +} + +impl, StorageValueT: generator::StorageValue>> + TryAppendValue for StorageValueT { fn try_append>(item: LikeT) -> Result<(), ()> { let bound = S::get(); let current = Self::decode_len().unwrap_or_default(); - dbg!(current, bound); - if (current + 1usize) <= bound { + if current < bound { // NOTE: we cannot reuse the implementation for `Vec` here because we never want to // mark `BoundedVec` as `StorageAppend`. let key = Self::storage_value_final_key(); @@ -891,21 +985,42 @@ impl, V: generator::StorageValue>> } } +impl< + K: FullCodec, + T: Value, + S: Get, + StorageMapT: generator::StorageMap>, + > TryAppendMap for StorageMapT +{ + fn try_append + Clone, LikeT: EncodeLike>( + key: LikeK, + item: LikeT, + ) -> Result<(), ()> { + let bound = S::get(); + let current = Self::decode_len(key.clone()).unwrap_or_default(); + if current < bound { + let key = Self::storage_map_final_key(key); + sp_io::storage::append(&key, item.encode()); + Ok(()) + } else { + Err(()) + } + } +} + #[cfg(test)] mod bounded_vec { use super::*; - use sp_core::hashing::twox_128; - // use crate::hash::Identity; use sp_io::TestExternalities; - // use generator::StorageValue as _; use sp_std::convert::TryInto; - use crate::assert_ok; + use crate::{assert_ok, Twox128}; crate::parameter_types! { pub const Seven: usize = 7; } crate::generate_storage_alias! { Prefix, Foo => Value> } + crate::generate_storage_alias! { Prefix, FooMap => Map<(u32, Twox128), BoundedVec> } #[test] fn decode_len_works() { @@ -915,11 +1030,17 @@ mod bounded_vec { assert_eq!(Foo::decode_len().unwrap(), 3); }); - // TODO: also for map. + TestExternalities::default().execute_with(|| { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + FooMap::insert(1, bounded); + assert_eq!(FooMap::decode_len(1).unwrap(), 3); + assert!(FooMap::decode_len(0).is_none()); + assert!(FooMap::decode_len(2).is_none()); + }); } #[test] - fn append_works() { + fn try_append_works() { TestExternalities::default().execute_with(|| { let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); Foo::put(bounded); @@ -931,7 +1052,49 @@ mod bounded_vec { assert!(Foo::try_append(8).is_err()); }); - // TODO: also for map. + TestExternalities::default().execute_with(|| { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + FooMap::insert(1, bounded); + + assert_ok!(FooMap::try_append(1, 4)); + assert_ok!(FooMap::try_append(1, 5)); + assert_ok!(FooMap::try_append(1, 6)); + assert_ok!(FooMap::try_append(1, 7)); + assert_eq!(FooMap::decode_len(1).unwrap(), 7); + assert!(FooMap::try_append(1, 8).is_err()); + + // append to a non-existing + assert!(FooMap::get(2).is_none()); + assert_ok!(FooMap::try_append(2, 4)); + assert_eq!(FooMap::get(2).unwrap(), BoundedVec::::unchecked_from(vec![4])); + assert_ok!(FooMap::try_append(2, 5)); + assert_eq!( + FooMap::get(2).unwrap(), + BoundedVec::::unchecked_from(vec![4, 5]) + ); + }); + } + + #[test] + fn iter_works() { + // consuming + { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + assert_eq!(bounded.into_iter().fold(0u32, |acc, prev| acc + prev), 6); + } + + // non consuming + { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + assert_eq!( + bounded + .iter::>() + .filter(|x| *x % 2u32 == 0u32) + .cloned() + .collect::>(), + vec![2], + ); + } } } From 9fb6bb3538c2fa9b4d7a1dee994e67eb1ab1a35c Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 8 Apr 2021 10:18:59 +0200 Subject: [PATCH 03/15] Add more docs --- frame/example/src/lib.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frame/example/src/lib.rs b/frame/example/src/lib.rs index ca391ef120300..6b5c4646f35f0 100644 --- a/frame/example/src/lib.rs +++ b/frame/example/src/lib.rs @@ -608,6 +608,13 @@ pub mod pallet { pub const Five: usize = 5; } + /// An example of how to create a bounded vector. This is a special 'newtype' that behaves + /// identical to a normal `Vec`, except the size can never exceed `Five`. + /// + /// Similar to normal storage values that contain a vector, the length of the bounded vector can + ///be queried efficiently with `decode_len`. Also, adding to the end of the vector can be done + ///efficiently via `try_append`. See [`frame_support::TryAppendValue`] and + /// [`frame_support::TryAppendMap`]. #[pallet::storage] #[pallet::getter(fn bounded)] pub(super) type BoundedStuff = From 33f7d93b740f2f67b8122c182beba105322a3691 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 8 Apr 2021 13:08:08 +0200 Subject: [PATCH 04/15] Move imports --- frame/support/src/storage/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 65f5105008956..44d9f706c9607 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -19,9 +19,12 @@ use sp_core::storage::ChildInfo; use sp_std::prelude::*; -use sp_std::iter; +use sp_std::{iter, convert::TryFrom}; use codec::{FullCodec, FullEncode, Encode, EncodeLike, Decode}; -use crate::hash::{Twox128, StorageHasher, ReversibleStorageHasher}; +use crate::{ + hash::{Twox128, StorageHasher, ReversibleStorageHasher}, + traits::Get, +}; use sp_runtime::generic::{Digest, DigestItem}; pub use sp_runtime::TransactionOutcome; @@ -823,9 +826,6 @@ impl StorageDecodeLength for Vec {} /// format ever changes, we need to remove this here. impl StorageAppend> for Digest {} -use crate::traits::Get; -use sp_std::convert::TryFrom; - /// Marker trait for types `T` that can be stored in storage as `Vec`. pub trait Value: FullCodec + Default + sp_std::fmt::Debug + Eq + PartialEq {} impl Value for T {} From aa9b8392a68a42145dc6b164edd491f1e1913dfb Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 9 Apr 2021 10:55:07 +0200 Subject: [PATCH 05/15] Some changes for easier compat. --- frame/example/src/lib.rs | 3 +- frame/support/src/lib.rs | 2 +- frame/support/src/storage/mod.rs | 89 +++++++++++++++++++++++--------- 3 files changed, 66 insertions(+), 28 deletions(-) diff --git a/frame/example/src/lib.rs b/frame/example/src/lib.rs index 6b5c4646f35f0..68ff323fbf085 100644 --- a/frame/example/src/lib.rs +++ b/frame/example/src/lib.rs @@ -604,8 +604,7 @@ pub mod pallet { pub(super) type Foo = StorageValue<_, T::Balance, ValueQuery>; frame_support::parameter_types! { - #[derive(Default)] // TODO: might be avoidable after https://github.com/paritytech/substrate/pull/8542/files - pub const Five: usize = 5; + pub const Five: u32 = 5; } /// An example of how to create a bounded vector. This is a special 'newtype' that behaves diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index dc5bb2f5b4f4d..a1ad32bf263a2 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -75,7 +75,7 @@ pub use self::hash::{ }; pub use self::storage::{ StorageValue, StorageMap, StorageDoubleMap, StoragePrefixedMap, IterableStorageMap, - IterableStorageDoubleMap, migration + IterableStorageDoubleMap, migration, BoundedVec, }; pub use self::dispatch::{Parameter, Callable}; pub use sp_runtime::{self, ConsensusEngineId, print, traits::Printable}; diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 44d9f706c9607..bc53a08af3ea7 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -25,6 +25,7 @@ use crate::{ hash::{Twox128, StorageHasher, ReversibleStorageHasher}, traits::Get, }; +use sp_arithmetic::traits::Zero; use sp_runtime::generic::{Digest, DigestItem}; pub use sp_runtime::TransactionOutcome; @@ -815,7 +816,7 @@ mod private { impl Sealed for Vec {} impl Sealed for Digest {} - impl> Sealed for BoundedVec {} + impl> Sealed for BoundedVec {} } impl StorageAppend for Vec {} @@ -827,8 +828,8 @@ impl StorageDecodeLength for Vec {} impl StorageAppend> for Digest {} /// Marker trait for types `T` that can be stored in storage as `Vec`. -pub trait Value: FullCodec + Default + sp_std::fmt::Debug + Eq + PartialEq {} -impl Value for T {} +pub trait Value: FullCodec + Clone + sp_std::fmt::Debug + Eq + PartialEq {} +impl Value for T {} /// A bounded vector. /// @@ -837,25 +838,53 @@ impl Value for T { /// /// As the name suggests, the length of the queue is always bounded. All internal operations ensure /// this bound is respected. -#[derive(Encode, Decode, Default, Clone, crate::RuntimeDebugNoBound)] -pub struct BoundedVec>(Vec, sp_std::marker::PhantomData); +#[derive(Encode, Decode, crate::DefaultNoBound, crate::CloneNoBound, crate::RuntimeDebugNoBound)] +pub struct BoundedVec>(Vec, sp_std::marker::PhantomData); // NOTE: we could also implement this as: -// impl, S2: Get> PartialEq> for BoundedVec +// impl, S2: Get> PartialEq> for BoundedVec // to allow comparison of bounded vectors with different bounds. -impl> PartialEq for BoundedVec { +impl> PartialEq for BoundedVec { fn eq(&self, rhs: &Self) -> bool { self.0 == rhs.0 } } -impl> Eq for BoundedVec {} +impl> Eq for BoundedVec {} -impl> BoundedVec { +impl> BoundedVec { /// Create `Self` from `t` without any checks. + /// + /// # WARNING + /// + /// Only use when you are sure you know what you are doing. fn unchecked_from(t: Vec) -> Self { Self(t, Default::default()) } + /// Create `Self` from `t` without any checks. Logs warnings if the bound is not being + /// respected. The additional scope can be used to indicate where a potential overflow is + /// happening. + /// + /// # WARNING + /// + /// Only use when you are sure you know what you are doing. + pub fn force_from(t: Vec, scope: Option<&'static str>) -> Self { + if t.len() > Self::bound() { + log::warn!( + target: crate::LOG_TARGET, + "length of a bounded vector in scope {} is not respected.", + scope.unwrap_or("UNKNOWN"), + ); + } + + Self::unchecked_from(t) + } + + /// Get the bound of the type in `usize`. + pub fn bound() -> usize { + S::get() as usize + } + /// Consume self, and return the inner `Vec`. Henceforth, the `Vec<_>` can be altered in an /// arbitrary way. At some point, if the reverse conversion is required, `TryFrom>` can /// be used. @@ -871,6 +900,16 @@ impl> BoundedVec { self.0.len() } + /// Exactly the same semantics as [`Vec::is_empty`]. + pub fn is_empty(&self) -> bool { + self.0.len().is_zero() + } + + /// Exactly the same semantics as [`Vec::retain`]. + pub fn retain bool>(&mut self, f: F) { + self.0.retain(f) + } + /// Exactly the same semantics as [`Vec::insert`], but returns an `Err` (and is a noop) if the /// new length of the vector exceeds `S`. /// @@ -878,7 +917,7 @@ impl> BoundedVec { /// /// Panics if `index > len`. pub fn try_insert(&mut self, index: usize, element: T) -> Result<(), ()> { - if self.len() < S::get() { + if self.len() < Self::bound() { self.0.insert(index, element); Ok(()) } else { @@ -910,7 +949,7 @@ impl> BoundedVec { } } -impl> iter::IntoIterator for BoundedVec { +impl> iter::IntoIterator for BoundedVec { type Item = T; type IntoIter = sp_std::vec::IntoIter; fn into_iter(self) -> Self::IntoIter { @@ -918,7 +957,7 @@ impl> iter::IntoIterator for BoundedVec { } } -impl<'a, T: Value, S: Get> iter::IntoIterator for &'a BoundedVec { +impl<'a, T: Value, S: Get> iter::IntoIterator for &'a BoundedVec { type Item = as sp_std::iter::Iterator>::Item; type IntoIter = sp_std::slice::Iter<'a, T>; fn into_iter(self) -> Self::IntoIter { @@ -926,10 +965,10 @@ impl<'a, T: Value, S: Get> iter::IntoIterator for &'a BoundedVec { } } -impl> TryFrom> for BoundedVec { +impl> TryFrom> for BoundedVec { type Error = (); fn try_from(t: Vec) -> Result { - if t.len() <= S::get() { + if t.len() <= Self::bound() { Ok(Self::unchecked_from(t)) } else { Err(()) @@ -938,13 +977,13 @@ impl> TryFrom> for BoundedVec { } // It is okay to give a non-mutable reference of the inner vec to anyone. -impl> AsRef> for BoundedVec { +impl> AsRef> for BoundedVec { fn as_ref(&self) -> &Vec { &self.0 } } -impl> codec::DecodeLength for BoundedVec { +impl> codec::DecodeLength for BoundedVec { fn len(self_encoded: &[u8]) -> Result { // `BoundedVec` stored just a `Vec`, thus the length is at the beginning in // `Compact` form, and same implementation as `Vec` can be used. @@ -952,26 +991,26 @@ impl> codec::DecodeLength for BoundedVec { } } -impl> StorageDecodeLength for BoundedVec {} +impl> StorageDecodeLength for BoundedVec {} /// Storage value that is *maybe* capable of [`StorageAppend`]. -pub trait TryAppendValue> { +pub trait TryAppendValue> { fn try_append>(item: LikeT) -> Result<(), ()>; } /// Storage map that is *maybe* capable of [`StorageAppend`]. -pub trait TryAppendMap> { +pub trait TryAppendMap> { fn try_append + Clone, LikeT: EncodeLike>( key: LikeK, item: LikeT, ) -> Result<(), ()>; } -impl, StorageValueT: generator::StorageValue>> +impl, StorageValueT: generator::StorageValue>> TryAppendValue for StorageValueT { fn try_append>(item: LikeT) -> Result<(), ()> { - let bound = S::get(); + let bound = BoundedVec::::bound(); let current = Self::decode_len().unwrap_or_default(); if current < bound { // NOTE: we cannot reuse the implementation for `Vec` here because we never want to @@ -988,7 +1027,7 @@ impl, StorageValueT: generator::StorageValue, + S: Get, StorageMapT: generator::StorageMap>, > TryAppendMap for StorageMapT { @@ -996,7 +1035,7 @@ impl< key: LikeK, item: LikeT, ) -> Result<(), ()> { - let bound = S::get(); + let bound = BoundedVec::::bound(); let current = Self::decode_len(key.clone()).unwrap_or_default(); if current < bound { let key = Self::storage_map_final_key(key); @@ -1009,14 +1048,14 @@ impl< } #[cfg(test)] -mod bounded_vec { +pub mod bounded_vec { use super::*; use sp_io::TestExternalities; use sp_std::convert::TryInto; use crate::{assert_ok, Twox128}; crate::parameter_types! { - pub const Seven: usize = 7; + pub const Seven: u32 = 7; } crate::generate_storage_alias! { Prefix, Foo => Value> } From d2eed3563b2174aeed819461bd202aec418867e4 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 12 Apr 2021 09:47:26 +0200 Subject: [PATCH 06/15] revert exmaple pallet --- frame/example/src/lib.rs | 63 +++++++++------------------ frame/support/src/storage/mod.rs | 74 ++++++++------------------------ 2 files changed, 40 insertions(+), 97 deletions(-) diff --git a/frame/example/src/lib.rs b/frame/example/src/lib.rs index 68ff323fbf085..fd1bc292ac8aa 100644 --- a/frame/example/src/lib.rs +++ b/frame/example/src/lib.rs @@ -26,8 +26,8 @@ //! //! ### Documentation Guidelines: //! -//! +//! +//! //!
      //!
    • Documentation comments (i.e. /// comment) - should //! accompany pallet functions and be restricted to the pallet interface, @@ -37,13 +37,13 @@ //! Capitalize the first word of each documentation comment and end it with //! a full stop. See //! Generic example of annotating source code with documentation -//! comments
    • Self-documenting code - Try to refactor code to be -//! self-documenting.
    • Code comments - Supplement complex code with a brief explanation, -//! not every line of code.
    • Identifiers - surround by backticks (i.e. -//! INHERENT_IDENTIFIER, InherentType, u64)
      • -//!
    • Usage scenarios - should be simple doctests. The compiler should ensure they stay -//! valid.
    • Extended tutorials - should be moved to external files and refer to.
      • +//! target="_blank"> Generic example of annotating source code with documentation comments +//!
    • Self-documenting code - Try to refactor code to be self-documenting.
      • +//!
    • Code comments - Supplement complex code with a brief explanation, not every line of code.
      • +//!
    • Identifiers - surround by backticks (i.e. INHERENT_IDENTIFIER, InherentType, +//! u64)
      • +//!
    • Usage scenarios - should be simple doctests. The compiler should ensure they stay valid.
      • +//!
    • Extended tutorials - should be moved to external files and refer to.
      • //! //!
    • Mandatory - include all of the sections/subsections where MUST is specified.
      • //!
    • Optional - optionally include sections/subsections where CAN is specified.
      • @@ -84,13 +84,12 @@ //! //! \## Terminology //! -//! // Add terminology used in the custom pallet. Include concepts, storage items, or actions that -//! you think // deserve to be noted to give context to the rest of the documentation or pallet -//! usage. The author needs to // use some judgment about what is included. We don't want a list of -//! every storage item nor types - the user // can go to the code for that. For example, "transfer -//! fee" is obvious and should not be included, but // "free balance" and "reserved balance" should -//! be noted to give context to the pallet. // Please do not link to outside resources. The -//! reference docs should be the ultimate source of truth. +//! // Add terminology used in the custom pallet. Include concepts, storage items, or actions that you think +//! // deserve to be noted to give context to the rest of the documentation or pallet usage. The author needs to +//! // use some judgment about what is included. We don't want a list of every storage item nor types - the user +//! // can go to the code for that. For example, "transfer fee" is obvious and should not be included, but +//! // "free balance" and "reserved balance" should be noted to give context to the pallet. +//! // Please do not link to outside resources. The reference docs should be the ultimate source of truth. //! //! //! @@ -107,8 +106,7 @@ //! \#### //! //! // Describe requirements prior to interacting with the custom pallet. -//! // Describe the process of interacting with the custom pallet for this scenario and public API -//! functions used. +//! // Describe the process of interacting with the custom pallet for this scenario and public API functions used. //! //! \## Interface //! @@ -132,15 +130,14 @@ //! //! //! -//! // Reference documentation of aspects such as `storageItems` and `dispatchable` functions should -//! only be // included in the Rustdocs for Substrate and not repeated in the README file. +//! // Reference documentation of aspects such as `storageItems` and `dispatchable` functions should only be +//! // included in the Rustdocs for Substrate and not repeated in the README file. //! //! \### Dispatchable Functions //! //! //! -//! // A brief description of dispatchable functions and a link to the rustdoc with their actual -//! documentation. +//! // A brief description of dispatchable functions and a link to the rustdoc with their actual documentation. //! //! // MUST have link to Call enum //! // MUST have origin information included in function doc @@ -157,8 +154,7 @@ //! //! //! -//! // It is up to the writer of the respective pallet (with respect to how much information to -//! provide). +//! // It is up to the writer of the respective pallet (with respect to how much information to provide). //! //! \#### Public Inspection functions - Immutable (getters) //! @@ -221,8 +217,7 @@ //! //! \### Simple Code Snippet //! -//! // Show a simple example (e.g. how to query a public getter function of -//! ) +//! // Show a simple example (e.g. how to query a public getter function of ) //! //! \### Example from FRAME //! @@ -603,22 +598,6 @@ pub mod pallet { #[pallet::getter(fn foo)] pub(super) type Foo = StorageValue<_, T::Balance, ValueQuery>; - frame_support::parameter_types! { - pub const Five: u32 = 5; - } - - /// An example of how to create a bounded vector. This is a special 'newtype' that behaves - /// identical to a normal `Vec`, except the size can never exceed `Five`. - /// - /// Similar to normal storage values that contain a vector, the length of the bounded vector can - ///be queried efficiently with `decode_len`. Also, adding to the end of the vector can be done - ///efficiently via `try_append`. See [`frame_support::TryAppendValue`] and - /// [`frame_support::TryAppendMap`]. - #[pallet::storage] - #[pallet::getter(fn bounded)] - pub(super) type BoundedStuff = - StorageValue<_, frame_support::storage::BoundedVec, ValueQuery>; - // The genesis config type. #[pallet::genesis_config] pub struct GenesisConfig { diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index bc53a08af3ea7..4deba44f10f6b 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -19,13 +19,12 @@ use sp_core::storage::ChildInfo; use sp_std::prelude::*; -use sp_std::{iter, convert::TryFrom}; +use sp_std::convert::TryFrom; use codec::{FullCodec, FullEncode, Encode, EncodeLike, Decode}; use crate::{ hash::{Twox128, StorageHasher, ReversibleStorageHasher}, traits::Get, }; -use sp_arithmetic::traits::Zero; use sp_runtime::generic::{Digest, DigestItem}; pub use sp_runtime::TransactionOutcome; @@ -895,21 +894,6 @@ impl> BoundedVec { self.0 } - //// Return the length of the inner vector. - pub fn len(&self) -> usize { - self.0.len() - } - - /// Exactly the same semantics as [`Vec::is_empty`]. - pub fn is_empty(&self) -> bool { - self.0.len().is_zero() - } - - /// Exactly the same semantics as [`Vec::retain`]. - pub fn retain bool>(&mut self, f: F) { - self.0.retain(f) - } - /// Exactly the same semantics as [`Vec::insert`], but returns an `Err` (and is a noop) if the /// new length of the vector exceeds `S`. /// @@ -943,25 +927,9 @@ impl> BoundedVec { self.0.swap_remove(index); } - /// Returns an iterator over inner values. - pub fn iter(&self) -> sp_std::slice::Iter<'_, T> { - self.into_iter() - } -} - -impl> iter::IntoIterator for BoundedVec { - type Item = T; - type IntoIter = sp_std::vec::IntoIter; - fn into_iter(self) -> Self::IntoIter { - self.0.into_iter() - } -} - -impl<'a, T: Value, S: Get> iter::IntoIterator for &'a BoundedVec { - type Item = as sp_std::iter::Iterator>::Item; - type IntoIter = sp_std::slice::Iter<'a, T>; - fn into_iter(self) -> Self::IntoIter { - self.0.as_slice().into_iter() + /// Exactly the same semantics as [`Vec::retain`]. + pub fn retain bool>(&mut self, f: F) { + self.0.retain(f) } } @@ -983,6 +951,15 @@ impl> AsRef> for BoundedVec { } } +// will allow for immutable all operations of `Vec` on `BoundedVec`. +impl> sp_std::ops::Deref for BoundedVec { + type Target = Vec; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + impl> codec::DecodeLength for BoundedVec { fn len(self_encoded: &[u8]) -> Result { // `BoundedVec` stored just a `Vec`, thus the length is at the beginning in @@ -1115,25 +1092,12 @@ pub mod bounded_vec { } #[test] - fn iter_works() { - // consuming - { - let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); - assert_eq!(bounded.into_iter().fold(0u32, |acc, prev| acc + prev), 6); - } - - // non consuming - { - let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); - assert_eq!( - bounded - .iter::>() - .filter(|x| *x % 2u32 == 0u32) - .cloned() - .collect::>(), - vec![2], - ); - } + fn deref_coercion_works() { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + // these methods come from deref-ed vec. + assert_eq!(bounded.len(), 3); + assert!(bounded.iter().next().is_some()); + assert!(!bounded.is_empty()); } } From cdeb27445173f8292e8e93a3291cae0dbca4ae59 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 12 Apr 2021 09:55:31 +0200 Subject: [PATCH 07/15] rename --- frame/support/src/storage/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 4deba44f10f6b..b4e0a48f842f6 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -890,7 +890,7 @@ impl> BoundedVec { /// /// This is useful for cases if you need access to an internal API of the inner `Vec<_>` which /// is not provided by the wrapper `BoundedVec`. - pub fn inner(self) -> Vec { + pub fn into_inner(self) -> Vec { self.0 } From b3e876c025d5af0d9a16a0e02334a19303c4ea4b Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Tue, 13 Apr 2021 13:48:47 +0200 Subject: [PATCH 08/15] BoundedVec for AccountLocks (#8580) * Example with balances * Fix tests * Make it indexable * fix * Fix tests * fix test * Fix collective as well * Fix test --- frame/collective/src/lib.rs | 19 +++++++------------ frame/democracy/src/tests.rs | 3 ++- frame/support/src/storage/mod.rs | 31 +++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/frame/collective/src/lib.rs b/frame/collective/src/lib.rs index 5c33bff3006b7..6284617e89bd9 100644 --- a/frame/collective/src/lib.rs +++ b/frame/collective/src/lib.rs @@ -48,13 +48,12 @@ use sp_io::storage; use sp_runtime::{RuntimeDebug, traits::Hash}; use frame_support::{ + decl_error, decl_event, decl_module, decl_storage, ensure, BoundedVec, codec::{Decode, Encode}, - decl_error, decl_event, decl_module, decl_storage, dispatch::{ DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable, Parameter, PostDispatchInfo, }, - ensure, traits::{ChangeMembers, EnsureOrigin, Get, InitializeMembers, GetBacking, Backing}, weights::{DispatchClass, GetDispatchInfo, Weight, Pays}, }; @@ -195,7 +194,7 @@ pub struct Votes { decl_storage! { trait Store for Module, I: Instance=DefaultInstance> as Collective { /// The hashes of the active proposals. - pub Proposals get(fn proposals): Vec; + pub Proposals get(fn proposals): BoundedVec; /// Actual proposal for a given hash, if it's current. pub ProposalOf get(fn proposal_of): map hasher(identity) T::Hash => Option<>::Proposal>; @@ -471,11 +470,7 @@ decl_module! { } else { let active_proposals = >::try_mutate(|proposals| -> Result { - proposals.push(proposal_hash); - ensure!( - proposals.len() <= T::MaxProposals::get() as usize, - Error::::TooManyProposals - ); + proposals.try_push(proposal_hash).map_err(|_| Error::::TooManyProposals)?; Ok(proposals.len()) })?; let index = Self::proposal_count(); @@ -1086,7 +1081,7 @@ mod tests { fn motions_basic_environment_works() { new_test_ext().execute_with(|| { assert_eq!(Collective::members(), vec![1, 2, 3]); - assert_eq!(Collective::proposals(), Vec::::new()); + assert_eq!(*Collective::proposals(), Vec::::new()); }); } @@ -1316,7 +1311,7 @@ mod tests { let hash = proposal.blake2_256().into(); let end = 4; assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); - assert_eq!(Collective::proposals(), vec![hash]); + assert_eq!(*Collective::proposals(), vec![hash]); assert_eq!(Collective::proposal_of(&hash), Some(proposal)); assert_eq!( Collective::voting(&hash), @@ -1577,9 +1572,9 @@ mod tests { assert_ok!(Collective::propose(Origin::signed(1), 3, Box::new(proposal.clone()), proposal_len)); assert_ok!(Collective::vote(Origin::signed(2), hash.clone(), 0, false)); assert_ok!(Collective::close(Origin::signed(2), hash.clone(), 0, proposal_weight, proposal_len)); - assert_eq!(Collective::proposals(), vec![]); + assert_eq!(*Collective::proposals(), vec![]); assert_ok!(Collective::propose(Origin::signed(1), 2, Box::new(proposal.clone()), proposal_len)); - assert_eq!(Collective::proposals(), vec![hash]); + assert_eq!(*Collective::proposals(), vec![hash]); }); } diff --git a/frame/democracy/src/tests.rs b/frame/democracy/src/tests.rs index 9df594ed07877..73bbb5481dade 100644 --- a/frame/democracy/src/tests.rs +++ b/frame/democracy/src/tests.rs @@ -120,9 +120,10 @@ impl pallet_scheduler::Config for Test { } parameter_types! { pub const ExistentialDeposit: u64 = 1; + pub const MaxLocks: u32 = 10; } impl pallet_balances::Config for Test { - type MaxLocks = (); + type MaxLocks = MaxLocks; type Balance = u64; type Event = Event; type DustRemoval = (); diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index b4e0a48f842f6..73f89416fadc1 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -891,6 +891,7 @@ impl> BoundedVec { /// This is useful for cases if you need access to an internal API of the inner `Vec<_>` which /// is not provided by the wrapper `BoundedVec`. pub fn into_inner(self) -> Vec { + debug_assert!(self.0.len() <= Self::bound()); self.0 } @@ -909,6 +910,21 @@ impl> BoundedVec { } } + /// Exactly the same semantics as [`Vec::push`], but returns an `Err` (and is a noop) if the + /// new length of the vector exceeds `S`. + /// + /// # Panics + /// + /// Panics if the new capacity exceeds isize::MAX bytes. + pub fn try_push(&mut self, element: T) -> Result<(), ()> { + if self.len() < Self::bound() { + self.0.push(element); + Ok(()) + } else { + Err(()) + } + } + /// Exactly the same semantics as [`Vec::remove`]. /// /// # Panics @@ -960,6 +976,21 @@ impl> sp_std::ops::Deref for BoundedVec { } } +impl> sp_std::ops::Index for BoundedVec { + type Output = T; + fn index(&self, index: usize) -> &Self::Output { + self.get(index).expect("index out of bound") + } +} + +impl> sp_std::iter::IntoIterator for BoundedVec { + type Item = T; + type IntoIter = sp_std::vec::IntoIter; + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + impl> codec::DecodeLength for BoundedVec { fn len(self_encoded: &[u8]) -> Result { // `BoundedVec` stored just a `Vec`, thus the length is at the beginning in From 66490c3e6ef391c8165b62c69c658513e272877a Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Tue, 13 Apr 2021 15:29:49 +0200 Subject: [PATCH 09/15] Update frame/support/src/storage/mod.rs Co-authored-by: Peter Goodspeed-Niklaus --- frame/support/src/storage/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 73f89416fadc1..8ba09aa8ab490 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -832,7 +832,7 @@ impl Value for T {} /// A bounded vector. /// -/// It implementations for efficient append and length decoding, as with a normal `Vec<_>`, once put +/// It has implementations for efficient append and length decoding, as with a normal `Vec<_>`, once put /// into a [`StorageValue`]. /// /// As the name suggests, the length of the queue is always bounded. All internal operations ensure From 8d4ce50d53fe1eb6b662faf2cc17702cb1d7d46a Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 14 Apr 2021 07:33:50 +0200 Subject: [PATCH 10/15] Repot and add for value --- frame/support/src/lib.rs | 20 +- frame/support/src/storage/bounded_vec.rs | 443 +++++++++++++++++++++++ frame/support/src/storage/mod.rs | 315 +--------------- frame/support/src/storage/types/map.rs | 1 + frame/support/src/storage/types/value.rs | 23 +- 5 files changed, 481 insertions(+), 321 deletions(-) create mode 100644 frame/support/src/storage/bounded_vec.rs diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 8153c197936dc..fbdc1c87eb413 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -75,7 +75,8 @@ pub use self::hash::{ }; pub use self::storage::{ StorageValue, StorageMap, StorageDoubleMap, StoragePrefixedMap, IterableStorageMap, - IterableStorageDoubleMap, migration, BoundedVec, + IterableStorageDoubleMap, migration, + bounded_vec::{self, BoundedVec}, }; pub use self::dispatch::{Parameter, Callable}; pub use sp_runtime::{self, ConsensusEngineId, print, traits::Printable}; @@ -112,20 +113,20 @@ impl TypeId for PalletId { /// /// // generate a double map from `(u32, u32)` (with hasher `Twox64Concat`) to `Vec` /// generate_storage_alias!( -/// OtherPrefix, OtherStorageName => DoubleMap< +/// OtherPrefix, OtherStorageName => DoubleMap< /// (u32, u32), /// (u32, u32), /// Vec -/// > +/// > /// ); /// /// // generate a map from `Config::AccountId` (with hasher `Twox64Concat`) to `Vec` /// trait Config { type AccountId: codec::FullCodec; } /// generate_storage_alias!( -/// Prefix, GenericStorage => Map<(Twox64Concat, T::AccountId), Vec> +/// Prefix, GenericStorage => Map<(Twox64Concat, T::AccountId), Vec> /// ); /// # fn main() {} -///``` +/// ``` #[macro_export] macro_rules! generate_storage_alias { // without generic for $name. @@ -143,7 +144,7 @@ macro_rules! generate_storage_alias { ($pallet:ident, $name:ident => DoubleMap<($key1:ty, $hasher1:ty), ($key2:ty, $hasher2:ty), $value:ty>) => { $crate::paste::paste! { $crate::generate_storage_alias!(@GENERATE_INSTANCE_STRUCT $pallet, $name); - type $name = $crate::storage::types::StorageMap< + type $name = $crate::storage::types::StorageDoubleMap< [<$name Instance>], $hasher1, $key1, @@ -178,12 +179,11 @@ macro_rules! generate_storage_alias { ( $pallet:ident, $name:ident<$t:ident : $bounds:tt> - => DoubleMap<($key1:ty, $hasher1:ty), ($key2:ty, $hasher2:ty), $value:ty>) - => { + => DoubleMap<($key1:ty, $hasher1:ty), ($key2:ty, $hasher2:ty), $value:ty>) => { $crate::paste::paste! { $crate::generate_storage_alias!(@GENERATE_INSTANCE_STRUCT $pallet, $name); #[allow(type_alias_bounds)] - type $name<$t : $bounds> = $crate::storage::types::StorageMap< + type $name<$t : $bounds> = $crate::storage::types::StorageDoubleMap< [<$name Instance>], $key1, $hasher1, @@ -213,7 +213,7 @@ macro_rules! generate_storage_alias { const STORAGE_PREFIX: &'static str = stringify!($name); } } - } + }; } /// Create new implementations of the [`Get`](crate::traits::Get) trait. diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs new file mode 100644 index 0000000000000..cd6cd5890cbcf --- /dev/null +++ b/frame/support/src/storage/bounded_vec.rs @@ -0,0 +1,443 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Traits, types and structs to support putting a bounded vector into storage, as a raw value, map +//! or a double map. + +use sp_std::prelude::*; +use sp_std::convert::TryFrom; +use codec::{FullCodec, Encode, EncodeLike, Decode}; +use crate::{ + traits::Get, + storage::{generator, StorageDecodeLength, StorageValue, StorageMap, StorageDoubleMap}, +}; + +/// Marker trait for types `T` that can be stored in storage as `BoundedVec`. +pub trait BoundedVecValue: FullCodec + Clone + sp_std::fmt::Debug {} +impl BoundedVecValue for T {} + +/// A bounded vector. +/// +/// It implementations for efficient append and length decoding, as with a normal `Vec<_>`, once put +/// into a [`StorageValue`]. +/// +/// As the name suggests, the length of the queue is always bounded. All internal operations ensure +/// this bound is respected. +#[derive(Encode, Decode, crate::DefaultNoBound, crate::CloneNoBound, crate::DebugNoBound)] +pub struct BoundedVec>(Vec, sp_std::marker::PhantomData); + +// NOTE: we could also implement this as: +// impl, S2: Get> PartialEq> for BoundedVec +// to allow comparison of bounded vectors with different bounds. +impl> PartialEq for BoundedVec { + fn eq(&self, rhs: &Self) -> bool { + self.0 == rhs.0 + } +} +impl> Eq for BoundedVec {} + +impl> BoundedVec { + /// Create `Self` from `t` without any checks. + /// + /// # WARNING + /// + /// Only use when you are sure you know what you are doing. + fn unchecked_from(t: Vec) -> Self { + Self(t, Default::default()) + } + + /// Create `Self` from `t` without any checks. Logs warnings if the bound is not being + /// respected. The additional scope can be used to indicate where a potential overflow is + /// happening. + /// + /// # WARNING + /// + /// Only use when you are sure you know what you are doing. + pub fn force_from(t: Vec, scope: Option<&'static str>) -> Self { + if t.len() > Self::bound() { + log::warn!( + target: crate::LOG_TARGET, + "length of a bounded vector in scope {} is not respected.", + scope.unwrap_or("UNKNOWN"), + ); + } + + Self::unchecked_from(t) + } + + /// Get the bound of the type in `usize`. + pub fn bound() -> usize { + S::get() as usize + } + + /// Consume self, and return the inner `Vec`. Henceforth, the `Vec<_>` can be altered in an + /// arbitrary way. At some point, if the reverse conversion is required, `TryFrom>` can + /// be used. + /// + /// This is useful for cases if you need access to an internal API of the inner `Vec<_>` which + /// is not provided by the wrapper `BoundedVec`. + pub fn into_inner(self) -> Vec { + debug_assert!(self.0.len() <= Self::bound()); + self.0 + } + + /// Consumes self and mutates self via the given `mutate` function. + /// + /// If the outcome of mutation is within bounds, `Some(Self)` is returned. Else, `None` is + /// returned. + /// + /// This is essentially a *consuming* shorthand [`Self::into_inner`] -> `...` -> + /// [`Self::try_from`]. + pub fn try_mutate(mut self, mut mutate: impl FnMut(&mut Vec)) -> Option { + mutate(&mut self.0); + (self.0.len() <= Self::bound()).then(move || self) + } + + /// Exactly the same semantics as [`Vec::insert`], but returns an `Err` (and is a noop) if the + /// new length of the vector exceeds `S`. + /// + /// # Panics + /// + /// Panics if `index > len`. + pub fn try_insert(&mut self, index: usize, element: T) -> Result<(), ()> { + if self.len() < Self::bound() { + self.0.insert(index, element); + Ok(()) + } else { + Err(()) + } + } + + /// Exactly the same semantics as [`Vec::push`], but returns an `Err` (and is a noop) if the + /// new length of the vector exceeds `S`. + /// + /// # Panics + /// + /// Panics if the new capacity exceeds isize::MAX bytes. + pub fn try_push(&mut self, element: T) -> Result<(), ()> { + if self.len() < Self::bound() { + self.0.push(element); + Ok(()) + } else { + Err(()) + } + } + + /// Exactly the same semantics as [`Vec::remove`]. + /// + /// # Panics + /// + /// Panics if `index` is out of bounds. + pub fn remove(&mut self, index: usize) { + self.0.remove(index); + } + + /// Exactly the same semantics as [`Vec::swap_remove`]. + /// + /// # Panics + /// + /// Panics if `index` is out of bounds. + pub fn swap_remove(&mut self, index: usize) { + self.0.swap_remove(index); + } + + /// Exactly the same semantics as [`Vec::retain`]. + pub fn retain bool>(&mut self, f: F) { + self.0.retain(f) + } +} + +impl> TryFrom> for BoundedVec { + type Error = (); + fn try_from(t: Vec) -> Result { + if t.len() <= Self::bound() { + Ok(Self::unchecked_from(t)) + } else { + Err(()) + } + } +} + +// It is okay to give a non-mutable reference of the inner vec to anyone. +impl> AsRef> for BoundedVec { + fn as_ref(&self) -> &Vec { + &self.0 + } +} + +// will allow for immutable all operations of `Vec` on `BoundedVec`. +impl> sp_std::ops::Deref for BoundedVec { + type Target = Vec; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +// Allows for indexing similar to a normal `Vec`. Can panic if out of bound. +impl> sp_std::ops::Index for BoundedVec { + type Output = T; + fn index(&self, index: usize) -> &Self::Output { + self.get(index).expect("index out of bound") + } +} + +impl> sp_std::iter::IntoIterator for BoundedVec { + type Item = T; + type IntoIter = sp_std::vec::IntoIter; + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl> codec::DecodeLength for BoundedVec { + fn len(self_encoded: &[u8]) -> Result { + // `BoundedVec` stored just a `Vec`, thus the length is at the beginning in + // `Compact` form, and same implementation as `Vec` can be used. + as codec::DecodeLength>::len(self_encoded) + } +} + +impl> StorageDecodeLength for BoundedVec {} + +/// Storage value that is *maybe* capable of [`StorageAppend`]. +pub trait TryAppendValue> { + /// Try and append the `item` into the storage item. + /// + /// This might fail if bounds are not respected. + fn try_append>(item: LikeT) -> Result<(), ()>; +} + +/// Storage map that is *maybe* capable of [`StorageAppend`]. +pub trait TryAppendMap> { + /// Try and append the `item` into the storage map at the given `key`. + /// + /// This might fail if bounds are not respected. + fn try_append + Clone, LikeT: EncodeLike>( + key: LikeK, + item: LikeT, + ) -> Result<(), ()>; +} + +/// Storage double map that is *maybe* capable of [`StorageAppend`]. +pub trait TryAppendDoubleMap> { + /// Try and append the `item` into the storage double map at the given `key`. + /// + /// This might fail if bounds are not respected. + fn try_append< + LikeK1: EncodeLike + Clone, + LikeK2: EncodeLike + Clone, + LikeT: EncodeLike, + >( + key1: LikeK1, + key2: LikeK2, + item: LikeT, + ) -> Result<(), ()>; +} + +impl, StorageValueT: generator::StorageValue>> + TryAppendValue for StorageValueT +{ + fn try_append>(item: LikeT) -> Result<(), ()> { + let bound = BoundedVec::::bound(); + let current = Self::decode_len().unwrap_or_default(); + if current < bound { + // NOTE: we cannot reuse the implementation for `Vec` here because we never want to + // mark `BoundedVec` as `StorageAppend`. + let key = Self::storage_value_final_key(); + sp_io::storage::append(&key, item.encode()); + Ok(()) + } else { + Err(()) + } + } +} + +impl< + K: FullCodec, + T: BoundedVecValue, + S: Get, + StorageMapT: generator::StorageMap>, + > TryAppendMap for StorageMapT +{ + fn try_append + Clone, LikeT: EncodeLike>( + key: LikeK, + item: LikeT, + ) -> Result<(), ()> { + let bound = BoundedVec::::bound(); + let current = Self::decode_len(key.clone()).unwrap_or_default(); + if current < bound { + let key = Self::storage_map_final_key(key); + sp_io::storage::append(&key, item.encode()); + Ok(()) + } else { + Err(()) + } + } +} + +impl< + K1: FullCodec, + K2: FullCodec, + T: BoundedVecValue, + S: Get, + StorageDoubleMapT: generator::StorageDoubleMap>, + > TryAppendDoubleMap for StorageDoubleMapT +{ + fn try_append< + LikeK1: EncodeLike + Clone, + LikeK2: EncodeLike + Clone, + LikeT: EncodeLike, + >( + key1: LikeK1, + key2: LikeK2, + item: LikeT, + ) -> Result<(), ()> { + let bound = BoundedVec::::bound(); + let current = Self::decode_len(key1.clone(), key2.clone()).unwrap_or_default(); + if current < bound { + let double_map_key = Self::storage_double_map_final_key(key1, key2); + sp_io::storage::append(&double_map_key, item.encode()); + Ok(()) + } else { + Err(()) + } + } +} + +#[cfg(test)] +pub mod test { + use super::*; + use sp_io::TestExternalities; + use sp_std::convert::TryInto; + use crate::{assert_ok, Twox128}; + + crate::parameter_types! { + pub const Seven: u32 = 7; + } + + crate::generate_storage_alias! { Prefix, Foo => Value> } + crate::generate_storage_alias! { Prefix, FooMap => Map<(u32, Twox128), BoundedVec> } + crate::generate_storage_alias! { + Prefix, + FooDoubleMap => DoubleMap<(u32, Twox128), (u32, Twox128), BoundedVec> + } + + #[test] + fn decode_len_works() { + TestExternalities::default().execute_with(|| { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + Foo::put(bounded); + assert_eq!(Foo::decode_len().unwrap(), 3); + }); + + TestExternalities::default().execute_with(|| { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + FooMap::insert(1, bounded); + assert_eq!(FooMap::decode_len(1).unwrap(), 3); + assert!(FooMap::decode_len(0).is_none()); + assert!(FooMap::decode_len(2).is_none()); + }); + + TestExternalities::default().execute_with(|| { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + FooDoubleMap::insert(1, 1, bounded); + assert_eq!(FooDoubleMap::decode_len(1, 1).unwrap(), 3); + assert!(FooDoubleMap::decode_len(2, 1).is_none()); + assert!(FooDoubleMap::decode_len(1, 2).is_none()); + assert!(FooDoubleMap::decode_len(2, 2).is_none()); + }); + } + + #[test] + fn try_append_works() { + TestExternalities::default().execute_with(|| { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + Foo::put(bounded); + assert_ok!(Foo::try_append(4)); + assert_ok!(Foo::try_append(5)); + assert_ok!(Foo::try_append(6)); + assert_ok!(Foo::try_append(7)); + assert_eq!(Foo::decode_len().unwrap(), 7); + assert!(Foo::try_append(8).is_err()); + }); + + TestExternalities::default().execute_with(|| { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + FooMap::insert(1, bounded); + + assert_ok!(FooMap::try_append(1, 4)); + assert_ok!(FooMap::try_append(1, 5)); + assert_ok!(FooMap::try_append(1, 6)); + assert_ok!(FooMap::try_append(1, 7)); + assert_eq!(FooMap::decode_len(1).unwrap(), 7); + assert!(FooMap::try_append(1, 8).is_err()); + + // append to a non-existing + assert!(FooMap::get(2).is_none()); + assert_ok!(FooMap::try_append(2, 4)); + assert_eq!(FooMap::get(2).unwrap(), BoundedVec::::unchecked_from(vec![4])); + assert_ok!(FooMap::try_append(2, 5)); + assert_eq!( + FooMap::get(2).unwrap(), + BoundedVec::::unchecked_from(vec![4, 5]) + ); + }); + + TestExternalities::default().execute_with(|| { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + FooDoubleMap::insert(1, 1, bounded); + + assert_ok!(FooDoubleMap::try_append(1, 1, 4)); + assert_ok!(FooDoubleMap::try_append(1, 1, 5)); + assert_ok!(FooDoubleMap::try_append(1, 1, 6)); + assert_ok!(FooDoubleMap::try_append(1, 1, 7)); + assert_eq!(FooDoubleMap::decode_len(1, 1).unwrap(), 7); + assert!(FooDoubleMap::try_append(1, 1, 8).is_err()); + + // append to a non-existing + assert!(FooDoubleMap::get(2, 1).is_none()); + assert_ok!(FooDoubleMap::try_append(2, 1, 4)); + assert_eq!( + FooDoubleMap::get(2, 1).unwrap(), + BoundedVec::::unchecked_from(vec![4]) + ); + assert_ok!(FooDoubleMap::try_append(2, 1, 5)); + assert_eq!( + FooDoubleMap::get(2, 1).unwrap(), + BoundedVec::::unchecked_from(vec![4, 5]) + ); + }); + } + + #[test] + fn deref_coercion_works() { + let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + // these methods come from deref-ed vec. + assert_eq!(bounded.len(), 3); + assert!(bounded.iter().next().is_some()); + assert!(!bounded.is_empty()); + } + + #[test] + fn try_mutate_works() { + let bounded: BoundedVec = vec![1, 2, 3, 4, 5, 6].try_into().unwrap(); + let bounded = bounded.try_mutate(|v| v.push(7)).unwrap(); + assert_eq!(bounded.len(), 7); + assert!(bounded.try_mutate(|v| v.push(8)).is_none()); + } +} diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 73f89416fadc1..2158996221a13 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -19,7 +19,6 @@ use sp_core::storage::ChildInfo; use sp_std::prelude::*; -use sp_std::convert::TryFrom; use codec::{FullCodec, FullEncode, Encode, EncodeLike, Decode}; use crate::{ hash::{Twox128, StorageHasher, ReversibleStorageHasher}, @@ -28,13 +27,14 @@ use crate::{ use sp_runtime::generic::{Digest, DigestItem}; pub use sp_runtime::TransactionOutcome; -pub mod unhashed; -pub mod hashed; +pub mod bounded_vec; pub mod child; #[doc(hidden)] pub mod generator; +pub mod hashed; pub mod migration; pub mod types; +pub mod unhashed; #[cfg(all(feature = "std", any(test, debug_assertions)))] mod debug_helper { @@ -810,12 +810,13 @@ pub trait StorageDecodeLength: private::Sealed + codec::DecodeLength { /// outside of this crate. mod private { use super::*; + use bounded_vec::{BoundedVecValue, BoundedVec}; pub trait Sealed {} impl Sealed for Vec {} impl Sealed for Digest {} - impl> Sealed for BoundedVec {} + impl> Sealed for BoundedVec {} } impl StorageAppend for Vec {} @@ -826,312 +827,6 @@ impl StorageDecodeLength for Vec {} /// format ever changes, we need to remove this here. impl StorageAppend> for Digest {} -/// Marker trait for types `T` that can be stored in storage as `Vec`. -pub trait Value: FullCodec + Clone + sp_std::fmt::Debug + Eq + PartialEq {} -impl Value for T {} - -/// A bounded vector. -/// -/// It implementations for efficient append and length decoding, as with a normal `Vec<_>`, once put -/// into a [`StorageValue`]. -/// -/// As the name suggests, the length of the queue is always bounded. All internal operations ensure -/// this bound is respected. -#[derive(Encode, Decode, crate::DefaultNoBound, crate::CloneNoBound, crate::RuntimeDebugNoBound)] -pub struct BoundedVec>(Vec, sp_std::marker::PhantomData); - -// NOTE: we could also implement this as: -// impl, S2: Get> PartialEq> for BoundedVec -// to allow comparison of bounded vectors with different bounds. -impl> PartialEq for BoundedVec { - fn eq(&self, rhs: &Self) -> bool { - self.0 == rhs.0 - } -} -impl> Eq for BoundedVec {} - -impl> BoundedVec { - /// Create `Self` from `t` without any checks. - /// - /// # WARNING - /// - /// Only use when you are sure you know what you are doing. - fn unchecked_from(t: Vec) -> Self { - Self(t, Default::default()) - } - - /// Create `Self` from `t` without any checks. Logs warnings if the bound is not being - /// respected. The additional scope can be used to indicate where a potential overflow is - /// happening. - /// - /// # WARNING - /// - /// Only use when you are sure you know what you are doing. - pub fn force_from(t: Vec, scope: Option<&'static str>) -> Self { - if t.len() > Self::bound() { - log::warn!( - target: crate::LOG_TARGET, - "length of a bounded vector in scope {} is not respected.", - scope.unwrap_or("UNKNOWN"), - ); - } - - Self::unchecked_from(t) - } - - /// Get the bound of the type in `usize`. - pub fn bound() -> usize { - S::get() as usize - } - - /// Consume self, and return the inner `Vec`. Henceforth, the `Vec<_>` can be altered in an - /// arbitrary way. At some point, if the reverse conversion is required, `TryFrom>` can - /// be used. - /// - /// This is useful for cases if you need access to an internal API of the inner `Vec<_>` which - /// is not provided by the wrapper `BoundedVec`. - pub fn into_inner(self) -> Vec { - debug_assert!(self.0.len() <= Self::bound()); - self.0 - } - - /// Exactly the same semantics as [`Vec::insert`], but returns an `Err` (and is a noop) if the - /// new length of the vector exceeds `S`. - /// - /// # Panics - /// - /// Panics if `index > len`. - pub fn try_insert(&mut self, index: usize, element: T) -> Result<(), ()> { - if self.len() < Self::bound() { - self.0.insert(index, element); - Ok(()) - } else { - Err(()) - } - } - - /// Exactly the same semantics as [`Vec::push`], but returns an `Err` (and is a noop) if the - /// new length of the vector exceeds `S`. - /// - /// # Panics - /// - /// Panics if the new capacity exceeds isize::MAX bytes. - pub fn try_push(&mut self, element: T) -> Result<(), ()> { - if self.len() < Self::bound() { - self.0.push(element); - Ok(()) - } else { - Err(()) - } - } - - /// Exactly the same semantics as [`Vec::remove`]. - /// - /// # Panics - /// - /// Panics if `index` is out of bounds. - pub fn remove(&mut self, index: usize) { - self.0.remove(index); - } - - /// Exactly the same semantics as [`Vec::swap_remove`]. - /// - /// # Panics - /// - /// Panics if `index` is out of bounds. - pub fn swap_remove(&mut self, index: usize) { - self.0.swap_remove(index); - } - - /// Exactly the same semantics as [`Vec::retain`]. - pub fn retain bool>(&mut self, f: F) { - self.0.retain(f) - } -} - -impl> TryFrom> for BoundedVec { - type Error = (); - fn try_from(t: Vec) -> Result { - if t.len() <= Self::bound() { - Ok(Self::unchecked_from(t)) - } else { - Err(()) - } - } -} - -// It is okay to give a non-mutable reference of the inner vec to anyone. -impl> AsRef> for BoundedVec { - fn as_ref(&self) -> &Vec { - &self.0 - } -} - -// will allow for immutable all operations of `Vec` on `BoundedVec`. -impl> sp_std::ops::Deref for BoundedVec { - type Target = Vec; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl> sp_std::ops::Index for BoundedVec { - type Output = T; - fn index(&self, index: usize) -> &Self::Output { - self.get(index).expect("index out of bound") - } -} - -impl> sp_std::iter::IntoIterator for BoundedVec { - type Item = T; - type IntoIter = sp_std::vec::IntoIter; - fn into_iter(self) -> Self::IntoIter { - self.0.into_iter() - } -} - -impl> codec::DecodeLength for BoundedVec { - fn len(self_encoded: &[u8]) -> Result { - // `BoundedVec` stored just a `Vec`, thus the length is at the beginning in - // `Compact` form, and same implementation as `Vec` can be used. - as codec::DecodeLength>::len(self_encoded) - } -} - -impl> StorageDecodeLength for BoundedVec {} - -/// Storage value that is *maybe* capable of [`StorageAppend`]. -pub trait TryAppendValue> { - fn try_append>(item: LikeT) -> Result<(), ()>; -} - -/// Storage map that is *maybe* capable of [`StorageAppend`]. -pub trait TryAppendMap> { - fn try_append + Clone, LikeT: EncodeLike>( - key: LikeK, - item: LikeT, - ) -> Result<(), ()>; -} - -impl, StorageValueT: generator::StorageValue>> - TryAppendValue for StorageValueT -{ - fn try_append>(item: LikeT) -> Result<(), ()> { - let bound = BoundedVec::::bound(); - let current = Self::decode_len().unwrap_or_default(); - if current < bound { - // NOTE: we cannot reuse the implementation for `Vec` here because we never want to - // mark `BoundedVec` as `StorageAppend`. - let key = Self::storage_value_final_key(); - sp_io::storage::append(&key, item.encode()); - Ok(()) - } else { - Err(()) - } - } -} - -impl< - K: FullCodec, - T: Value, - S: Get, - StorageMapT: generator::StorageMap>, - > TryAppendMap for StorageMapT -{ - fn try_append + Clone, LikeT: EncodeLike>( - key: LikeK, - item: LikeT, - ) -> Result<(), ()> { - let bound = BoundedVec::::bound(); - let current = Self::decode_len(key.clone()).unwrap_or_default(); - if current < bound { - let key = Self::storage_map_final_key(key); - sp_io::storage::append(&key, item.encode()); - Ok(()) - } else { - Err(()) - } - } -} - -#[cfg(test)] -pub mod bounded_vec { - use super::*; - use sp_io::TestExternalities; - use sp_std::convert::TryInto; - use crate::{assert_ok, Twox128}; - - crate::parameter_types! { - pub const Seven: u32 = 7; - } - - crate::generate_storage_alias! { Prefix, Foo => Value> } - crate::generate_storage_alias! { Prefix, FooMap => Map<(u32, Twox128), BoundedVec> } - - #[test] - fn decode_len_works() { - TestExternalities::default().execute_with(|| { - let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); - Foo::put(bounded); - assert_eq!(Foo::decode_len().unwrap(), 3); - }); - - TestExternalities::default().execute_with(|| { - let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); - FooMap::insert(1, bounded); - assert_eq!(FooMap::decode_len(1).unwrap(), 3); - assert!(FooMap::decode_len(0).is_none()); - assert!(FooMap::decode_len(2).is_none()); - }); - } - - #[test] - fn try_append_works() { - TestExternalities::default().execute_with(|| { - let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); - Foo::put(bounded); - assert_ok!(Foo::try_append(4)); - assert_ok!(Foo::try_append(5)); - assert_ok!(Foo::try_append(6)); - assert_ok!(Foo::try_append(7)); - assert_eq!(Foo::decode_len().unwrap(), 7); - assert!(Foo::try_append(8).is_err()); - }); - - TestExternalities::default().execute_with(|| { - let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); - FooMap::insert(1, bounded); - - assert_ok!(FooMap::try_append(1, 4)); - assert_ok!(FooMap::try_append(1, 5)); - assert_ok!(FooMap::try_append(1, 6)); - assert_ok!(FooMap::try_append(1, 7)); - assert_eq!(FooMap::decode_len(1).unwrap(), 7); - assert!(FooMap::try_append(1, 8).is_err()); - - // append to a non-existing - assert!(FooMap::get(2).is_none()); - assert_ok!(FooMap::try_append(2, 4)); - assert_eq!(FooMap::get(2).unwrap(), BoundedVec::::unchecked_from(vec![4])); - assert_ok!(FooMap::try_append(2, 5)); - assert_eq!( - FooMap::get(2).unwrap(), - BoundedVec::::unchecked_from(vec![4, 5]) - ); - }); - } - - #[test] - fn deref_coercion_works() { - let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); - // these methods come from deref-ed vec. - assert_eq!(bounded.len(), 3); - assert!(bounded.iter().next().is_some()); - assert!(!bounded.is_empty()); - } -} - #[cfg(test)] mod test { use super::*; diff --git a/frame/support/src/storage/types/map.rs b/frame/support/src/storage/types/map.rs index 4af28a77cf2b6..daae78c3ffaf1 100644 --- a/frame/support/src/storage/types/map.rs +++ b/frame/support/src/storage/types/map.rs @@ -22,6 +22,7 @@ use codec::{FullCodec, Decode, EncodeLike, Encode}; use crate::{ storage::{ StorageAppend, StorageDecodeLength, + bounded_vec::{BoundedVec, BoundedVecValue}, types::{OptionQuery, QueryKindTrait, OnEmptyGetter}, }, traits::{GetDefault, StorageInstance}, diff --git a/frame/support/src/storage/types/value.rs b/frame/support/src/storage/types/value.rs index 39f718956eb64..d536d76d76b8e 100644 --- a/frame/support/src/storage/types/value.rs +++ b/frame/support/src/storage/types/value.rs @@ -21,9 +21,10 @@ use codec::{FullCodec, Decode, EncodeLike, Encode}; use crate::{ storage::{ StorageAppend, StorageDecodeLength, + bounded_vec::{BoundedVec, BoundedVecValue}, types::{OptionQuery, QueryKindTrait, OnEmptyGetter}, }, - traits::{GetDefault, StorageInstance}, + traits::{GetDefault, StorageInstance, Get}, }; use frame_metadata::{DefaultByteGetter, StorageEntryModifier}; @@ -60,6 +61,26 @@ where } } +impl + StorageValue, QueryKind, OnEmpty> +where + Prefix: StorageInstance, + QueryKind: QueryKindTrait, OnEmpty>, + OnEmpty: crate::traits::Get + 'static, + VecValue: BoundedVecValue, + VecBound: Get, +{ + /// Try and append the given item to the value in the storage. + /// + /// Is only available if `Value` of the storage is [`BoundedVec`]. + pub fn try_append(item: EncodeLikeItem) -> Result<(), ()> + where + EncodeLikeItem: EncodeLike, + { + >::try_append(item) + } +} + impl StorageValue where Prefix: StorageInstance, From c9e435651147439510aa9305cfb4188125881db7 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 14 Apr 2021 07:42:17 +0200 Subject: [PATCH 11/15] Add for map and double map --- frame/support/src/storage/types/double_map.rs | 49 ++++++++++++++++++- frame/support/src/storage/types/map.rs | 30 +++++++++++- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index f0b5f66eff058..2a59710903a90 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -22,9 +22,10 @@ use codec::{FullCodec, Decode, EncodeLike, Encode}; use crate::{ storage::{ StorageAppend, StorageDecodeLength, + bounded_vec::{BoundedVec, BoundedVecValue}, types::{OptionQuery, QueryKindTrait, OnEmptyGetter}, }, - traits::{GetDefault, StorageInstance}, + traits::{GetDefault, StorageInstance, Get}, }; use frame_metadata::{DefaultByteGetter, StorageEntryModifier}; use sp_std::vec::Vec; @@ -102,6 +103,50 @@ where } } +impl + StorageDoubleMap< + Prefix, + Hasher1, + Key1, + Hasher2, + Key2, + BoundedVec, + QueryKind, + OnEmpty, + > where + Prefix: StorageInstance, + Hasher1: crate::hash::StorageHasher, + Hasher2: crate::hash::StorageHasher, + Key1: FullCodec, + Key2: FullCodec, + QueryKind: QueryKindTrait, OnEmpty>, + OnEmpty: crate::traits::Get + 'static, + VecValue: BoundedVecValue, + VecBound: Get, +{ + /// Try and append the given item to the double map in the storage. + /// + /// Is only available if `Value` of the map is [`BoundedVec`]. + pub fn try_append( + key1: EncodeLikeKey1, + key2: EncodeLikeKey2, + item: EncodeLikeItem, + ) -> Result<(), ()> + where + EncodeLikeKey1: EncodeLike + Clone, + EncodeLikeKey2: EncodeLike + Clone, + EncodeLikeItem: EncodeLike, + { + < + Self + as + crate::storage::bounded_vec::TryAppendDoubleMap + >::try_append( + key1, key2, item, + ) + } +} + impl StorageDoubleMap where @@ -112,7 +157,7 @@ where Key2: FullCodec, Value: FullCodec, QueryKind: QueryKindTrait, - OnEmpty: crate::traits::Get + 'static + OnEmpty: crate::traits::Get + 'static, { /// Get the storage key used to fetch a value corresponding to a specific key. pub fn hashed_key_for(k1: KArg1, k2: KArg2) -> Vec diff --git a/frame/support/src/storage/types/map.rs b/frame/support/src/storage/types/map.rs index daae78c3ffaf1..187323b4ad1ee 100644 --- a/frame/support/src/storage/types/map.rs +++ b/frame/support/src/storage/types/map.rs @@ -25,7 +25,7 @@ use crate::{ bounded_vec::{BoundedVec, BoundedVecValue}, types::{OptionQuery, QueryKindTrait, OnEmptyGetter}, }, - traits::{GetDefault, StorageInstance}, + traits::{GetDefault, StorageInstance, Get}, }; use frame_metadata::{DefaultByteGetter, StorageEntryModifier}; use sp_std::prelude::*; @@ -92,6 +92,34 @@ where } } +impl + StorageMap, QueryKind, OnEmpty> +where + Prefix: StorageInstance, + Hasher: crate::hash::StorageHasher, + Key: FullCodec, + QueryKind: QueryKindTrait, OnEmpty>, + OnEmpty: crate::traits::Get + 'static, + VecValue: BoundedVecValue, + VecBound: Get, +{ + /// Try and append the given item to the map in the storage. + /// + /// Is only available if `Value` of the map is [`BoundedVec`]. + pub fn try_append( + key: EncodeLikeKey, + item: EncodeLikeItem, + ) -> Result<(), ()> + where + EncodeLikeKey: EncodeLike + Clone, + EncodeLikeItem: EncodeLike, + { + >::try_append( + key, item, + ) + } +} + impl StorageMap where From ca43313e5639b553d5702615a92a8fa3af108d1d Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 14 Apr 2021 07:47:44 +0200 Subject: [PATCH 12/15] Final touches. --- frame/support/src/storage/mod.rs | 4 ++-- frame/support/src/storage/types/double_map.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 2158996221a13..adcf44a64620e 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -27,14 +27,14 @@ use crate::{ use sp_runtime::generic::{Digest, DigestItem}; pub use sp_runtime::TransactionOutcome; +pub mod unhashed; +pub mod hashed; pub mod bounded_vec; pub mod child; #[doc(hidden)] pub mod generator; -pub mod hashed; pub mod migration; pub mod types; -pub mod unhashed; #[cfg(all(feature = "std", any(test, debug_assertions)))] mod debug_helper { diff --git a/frame/support/src/storage/types/double_map.rs b/frame/support/src/storage/types/double_map.rs index 2a59710903a90..184d96b3a54f9 100644 --- a/frame/support/src/storage/types/double_map.rs +++ b/frame/support/src/storage/types/double_map.rs @@ -157,7 +157,7 @@ where Key2: FullCodec, Value: FullCodec, QueryKind: QueryKindTrait, - OnEmpty: crate::traits::Get + 'static, + OnEmpty: crate::traits::Get + 'static { /// Get the storage key used to fetch a value corresponding to a specific key. pub fn hashed_key_for(k1: KArg1, k2: KArg2) -> Vec From 07417e5154a46a0bb8ccf5ff276b49c98023fea4 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Wed, 14 Apr 2021 13:25:17 +0200 Subject: [PATCH 13/15] Update frame/support/src/storage/bounded_vec.rs Co-authored-by: Guillaume Thiolliere --- frame/support/src/storage/bounded_vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index 985f02800fc1a..dd3a3401e9064 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -43,7 +43,7 @@ pub struct BoundedVec>(Vec, sp_std::marker::P // NOTE: we could also implement this as: // impl, S2: Get> PartialEq> for BoundedVec // to allow comparison of bounded vectors with different bounds. -impl> PartialEq for BoundedVec { +impl> PartialEq for BoundedVec { fn eq(&self, rhs: &Self) -> bool { self.0 == rhs.0 } From 007047f2efe50922bc15c996416feea021d02e2b Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 15 Apr 2021 09:07:34 +0200 Subject: [PATCH 14/15] Add a few more tests --- frame/support/src/storage/bounded_vec.rs | 37 ++++++++++++++++++++---- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index 985f02800fc1a..eb05c5e6b93b2 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -51,6 +51,11 @@ impl> PartialEq for BoundedVec< impl> Eq for BoundedVec {} impl> BoundedVec { + /// Get the bound of the type in `usize`. + pub fn bound() -> usize { + S::get() as usize + } + /// Create `Self` from `t` without any checks. /// /// # WARNING @@ -79,11 +84,6 @@ impl> BoundedVec { Self::unchecked_from(t) } - /// Get the bound of the type in `usize`. - pub fn bound() -> usize { - S::get() as usize - } - /// Consume self, and return the inner `Vec`. Henceforth, the `Vec<_>` can be altered in an /// arbitrary way. At some point, if the reverse conversion is required, `TryFrom>` can /// be used. @@ -328,6 +328,7 @@ pub mod test { crate::parameter_types! { pub const Seven: u32 = 7; + pub const Four: u32 = 4; } crate::generate_storage_alias! { Prefix, Foo => Value> } @@ -424,6 +425,32 @@ pub mod test { }); } + #[test] + fn try_insert_works() { + let mut bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + bounded.try_insert(1, 0).unwrap(); + assert_eq!(*bounded, vec![1, 0, 2, 3]); + + assert!(bounded.try_insert(0, 9).is_err()); + assert_eq!(*bounded, vec![1, 0, 2, 3]); + } + + #[test] + #[should_panic(expected = "insertion index (is 9) should be <= len (is 3)")] + fn try_inert_panics_if_oob() { + let mut bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + bounded.try_insert(9, 0).unwrap(); + } + + #[test] + fn try_push_works() { + let mut bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); + bounded.try_push(0).unwrap(); + assert_eq!(*bounded, vec![1, 2, 3, 0]); + + assert!(bounded.try_push(9).is_err()); + } + #[test] fn deref_coercion_works() { let bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); From 678bb208af777a08453083a51456ab9b06f67110 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 15 Apr 2021 09:09:44 +0200 Subject: [PATCH 15/15] Add import --- frame/support/src/storage/bounded_vec.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index eb05c5e6b93b2..f1c172a218504 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -19,7 +19,7 @@ //! or a double map. use sp_std::prelude::*; -use sp_std::convert::TryFrom; +use sp_std::{convert::TryFrom, marker::PhantomData}; use codec::{FullCodec, Encode, EncodeLike, Decode}; use crate::{ traits::Get, @@ -38,7 +38,7 @@ impl BoundedVecValue for T {} /// As the name suggests, the length of the queue is always bounded. All internal operations ensure /// this bound is respected. #[derive(Encode, Decode, crate::DefaultNoBound, crate::CloneNoBound, crate::DebugNoBound)] -pub struct BoundedVec>(Vec, sp_std::marker::PhantomData); +pub struct BoundedVec>(Vec, PhantomData); // NOTE: we could also implement this as: // impl, S2: Get> PartialEq> for BoundedVec