diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index d1874b65b6..911373bfad 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -321,6 +321,7 @@ macro_rules! parameter_types { (IMPL_CONST $name:ident, $type:ty, $value:expr) => { impl $name { /// Returns the value of this parameter type. + #[allow(unused)] pub const fn get() -> $type { $value } @@ -335,6 +336,7 @@ macro_rules! parameter_types { (IMPL $name:ident, $type:ty, $value:expr) => { impl $name { /// Returns the value of this parameter type. + #[allow(unused)] pub fn get() -> $type { $value } @@ -349,6 +351,7 @@ macro_rules! parameter_types { (IMPL_STORAGE $name:ident, $type:ty, $value:expr) => { impl $name { /// Returns the key for this parameter type. + #[allow(unused)] pub fn key() -> [u8; 16] { $crate::sp_io::hashing::twox_128( concat!(":", stringify!($name), ":").as_bytes() @@ -359,6 +362,7 @@ macro_rules! parameter_types { /// /// This needs to be executed in an externalities provided /// environment. + #[allow(unused)] pub fn set(value: &$type) { $crate::storage::unhashed::put(&Self::key(), value); } @@ -367,6 +371,7 @@ macro_rules! parameter_types { /// /// This needs to be executed in an externalities provided /// environment. + #[allow(unused)] pub fn get() -> $type { $crate::storage::unhashed::get(&Self::key()).unwrap_or_else(|| $value) } diff --git a/substrate/frame/support/src/storage/bounded_btree_map.rs b/substrate/frame/support/src/storage/bounded_btree_map.rs index 7fd0d175fd..8c50557618 100644 --- a/substrate/frame/support/src/storage/bounded_btree_map.rs +++ b/substrate/frame/support/src/storage/bounded_btree_map.rs @@ -32,11 +32,29 @@ use codec::{Encode, Decode}; /// B-Trees represent a fundamental compromise between cache-efficiency and actually minimizing /// the amount of work performed in a search. See [`BTreeMap`] for more details. /// -/// Unlike a standard `BTreeMap`, there is a static, enforced upper limit to the number of items -/// in the map. All internal operations ensure this bound is respected. -#[derive(Encode, Decode)] +/// Unlike a standard `BTreeMap`, there is an enforced upper limit to the number of items in the +/// map. All internal operations ensure this bound is respected. +#[derive(Encode)] pub struct BoundedBTreeMap(BTreeMap, PhantomData); +impl Decode for BoundedBTreeMap +where + BTreeMap: Decode, + S: Get, +{ + fn decode(input: &mut I) -> Result { + let inner = BTreeMap::::decode(input)?; + if inner.len() > S::get() as usize { + return Err("BoundedBTreeMap exceeds its limit".into()); + } + Ok(Self(inner, PhantomData)) + } + + fn skip(input: &mut I) -> Result<(), codec::Error> { + BTreeMap::::skip(input) + } +} + impl BoundedBTreeMap where S: Get, @@ -59,44 +77,6 @@ where BoundedBTreeMap(BTreeMap::new(), PhantomData) } - /// Create `Self` from a primitive `BTreeMap` without any checks. - unsafe fn unchecked_from(map: BTreeMap) -> Self { - Self(map, Default::default()) - } - - /// Create `Self` from a primitive `BTreeMap` without any checks. - /// - /// Logs warnings if the bound is not being respected. The scope is mentioned in the log message - /// to indicate where overflow is happening. - /// - /// # Example - /// - /// ``` - /// # use sp_std::collections::btree_map::BTreeMap; - /// # use frame_support::{parameter_types, storage::bounded_btree_map::BoundedBTreeMap}; - /// parameter_types! { - /// pub const Size: u32 = 5; - /// } - /// let mut map = BTreeMap::new(); - /// map.insert("foo", 1); - /// map.insert("bar", 2); - /// let bounded_map = unsafe {BoundedBTreeMap::<_, _, Size>::force_from(map, "demo")}; - /// ``` - pub unsafe fn force_from(map: BTreeMap, scope: Scope) -> Self - where - Scope: Into>, - { - if map.len() > Self::bound() { - log::warn!( - target: crate::LOG_TARGET, - "length of a bounded btreemap in scope {} is not respected.", - scope.into().unwrap_or("UNKNOWN"), - ); - } - - Self::unchecked_from(map) - } - /// Consume self, and return the inner `BTreeMap`. /// /// This is useful when a mutating API of the inner type is desired, and closure-based mutation @@ -418,4 +398,13 @@ pub mod test { let bounded = boundedmap_from_keys::(&[1, 2, 3, 4, 5, 6]); assert_eq!(bounded, map_from_keys(&[1, 2, 3, 4, 5, 6])); } + + #[test] + fn too_big_fail_to_decode() { + let v: Vec<(u32, u32)> = vec![(1, 1), (2, 2), (3, 3), (4, 4), (5, 5)]; + assert_eq!( + BoundedBTreeMap::::decode(&mut &v.encode()[..]), + Err("BoundedBTreeMap exceeds its limit".into()), + ); + } } diff --git a/substrate/frame/support/src/storage/bounded_btree_set.rs b/substrate/frame/support/src/storage/bounded_btree_set.rs index 586ecca4c8..f551a3cbfa 100644 --- a/substrate/frame/support/src/storage/bounded_btree_set.rs +++ b/substrate/frame/support/src/storage/bounded_btree_set.rs @@ -32,11 +32,29 @@ use codec::{Encode, Decode}; /// B-Trees represent a fundamental compromise between cache-efficiency and actually minimizing /// the amount of work performed in a search. See [`BTreeSet`] for more details. /// -/// Unlike a standard `BTreeSet`, there is a static, enforced upper limit to the number of items -/// in the set. All internal operations ensure this bound is respected. -#[derive(Encode, Decode)] +/// Unlike a standard `BTreeSet`, there is an enforced upper limit to the number of items in the +/// set. All internal operations ensure this bound is respected. +#[derive(Encode)] pub struct BoundedBTreeSet(BTreeSet, PhantomData); +impl Decode for BoundedBTreeSet +where + BTreeSet: Decode, + S: Get, +{ + fn decode(input: &mut I) -> Result { + let inner = BTreeSet::::decode(input)?; + if inner.len() > S::get() as usize { + return Err("BoundedBTreeSet exceeds its limit".into()); + } + Ok(Self(inner, PhantomData)) + } + + fn skip(input: &mut I) -> Result<(), codec::Error> { + BTreeSet::::skip(input) + } +} + impl BoundedBTreeSet where S: Get, @@ -59,44 +77,6 @@ where BoundedBTreeSet(BTreeSet::new(), PhantomData) } - /// Create `Self` from a primitive `BTreeSet` without any checks. - unsafe fn unchecked_from(set: BTreeSet) -> Self { - Self(set, Default::default()) - } - - /// Create `Self` from a primitive `BTreeSet` without any checks. - /// - /// Logs warnings if the bound is not being respected. The scope is mentioned in the log message - /// to indicate where overflow is happening. - /// - /// # Example - /// - /// ``` - /// # use sp_std::collections::btree_set::BTreeSet; - /// # use frame_support::{parameter_types, storage::bounded_btree_set::BoundedBTreeSet}; - /// parameter_types! { - /// pub const Size: u32 = 5; - /// } - /// let mut set = BTreeSet::new(); - /// set.insert("foo"); - /// set.insert("bar"); - /// let bounded_set = unsafe {BoundedBTreeSet::<_, Size>::force_from(set, "demo")}; - /// ``` - pub unsafe fn force_from(set: BTreeSet, scope: Scope) -> Self - where - Scope: Into>, - { - if set.len() > Self::bound() { - log::warn!( - target: crate::LOG_TARGET, - "length of a bounded btreeset in scope {} is not respected.", - scope.into().unwrap_or("UNKNOWN"), - ); - } - - Self::unchecked_from(set) - } - /// Consume self, and return the inner `BTreeSet`. /// /// This is useful when a mutating API of the inner type is desired, and closure-based mutation @@ -404,4 +384,13 @@ pub mod test { let bounded = boundedmap_from_keys::(&[1, 2, 3, 4, 5, 6]); assert_eq!(bounded, map_from_keys(&[1, 2, 3, 4, 5, 6])); } + + #[test] + fn too_big_fail_to_decode() { + let v: Vec = vec![1, 2, 3, 4, 5]; + assert_eq!( + BoundedBTreeSet::::decode(&mut &v.encode()[..]), + Err("BoundedBTreeSet exceeds its limit".into()), + ); + } } diff --git a/substrate/frame/support/src/storage/bounded_vec.rs b/substrate/frame/support/src/storage/bounded_vec.rs index 6bb6ea541c..a4e8c50918 100644 --- a/substrate/frame/support/src/storage/bounded_vec.rs +++ b/substrate/frame/support/src/storage/bounded_vec.rs @@ -20,14 +20,14 @@ use sp_std::prelude::*; use sp_std::{convert::TryFrom, fmt, marker::PhantomData}; -use codec::{FullCodec, Encode, EncodeLike, Decode}; +use codec::{Encode, Decode}; use core::{ ops::{Deref, Index, IndexMut}, slice::SliceIndex, }; use crate::{ traits::{Get, MaxEncodedLen}, - storage::{generator, StorageDecodeLength, StorageValue, StorageMap, StorageDoubleMap}, + storage::{StorageDecodeLength, StorageTryAppend}, }; /// A bounded vector. @@ -37,12 +37,26 @@ use crate::{ /// /// As the name suggests, the length of the queue is always bounded. All internal operations ensure /// this bound is respected. -#[derive(Encode, Decode)] +#[derive(Encode)] pub struct BoundedVec(Vec, PhantomData); +impl> Decode for BoundedVec { + fn decode(input: &mut I) -> Result { + let inner = Vec::::decode(input)?; + if inner.len() > S::get() as usize { + return Err("BoundedVec exceeds its limit".into()); + } + Ok(Self(inner, PhantomData)) + } + + fn skip(input: &mut I) -> Result<(), codec::Error> { + Vec::::skip(input) + } +} + impl BoundedVec { /// Create `Self` from `t` without any checks. - unsafe fn unchecked_from(t: Vec) -> Self { + fn unchecked_from(t: Vec) -> Self { Self(t, Default::default()) } @@ -86,21 +100,6 @@ impl> BoundedVec { S::get() as usize } - /// 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. - pub unsafe 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) - } - /// 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 @@ -147,7 +146,7 @@ impl> BoundedVec { impl Default for BoundedVec { fn default() -> Self { // the bound cannot be below 0, which is satisfied by an empty vector - unsafe { Self::unchecked_from(Vec::default()) } + Self::unchecked_from(Vec::default()) } } @@ -168,7 +167,7 @@ where { fn clone(&self) -> Self { // bound is retained - unsafe { Self::unchecked_from(self.0.clone()) } + Self::unchecked_from(self.0.clone()) } } @@ -177,7 +176,7 @@ impl> TryFrom> for BoundedVec { fn try_from(t: Vec) -> Result { if t.len() <= Self::bound() { // explicit check just above - Ok(unsafe { Self::unchecked_from(t) }) + Ok(Self::unchecked_from(t)) } else { Err(()) } @@ -273,114 +272,9 @@ impl Eq for BoundedVec where T: Eq {} impl StorageDecodeLength for BoundedVec {} -/// Storage value that is *maybe* capable of [`StorageAppend`](crate::storage::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`](crate::storage::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`](crate::storage::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 TryAppendValue for StorageValueT -where - BoundedVec: FullCodec, - T: Encode, - S: Get, - StorageValueT: generator::StorageValue>, -{ - 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 TryAppendMap for StorageMapT -where - K: FullCodec, - BoundedVec: FullCodec, - T: Encode, - S: Get, - StorageMapT: generator::StorageMap>, -{ - 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 TryAppendDoubleMap for StorageDoubleMapT -where - K1: FullCodec, - K2: FullCodec, - BoundedVec: FullCodec, - T: Encode, - S: Get, - StorageDoubleMapT: generator::StorageDoubleMap>, -{ - 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(()) - } +impl> StorageTryAppend for BoundedVec { + fn bound() -> usize { + S::get() as usize } } @@ -405,7 +299,7 @@ pub mod test { use super::*; use sp_io::TestExternalities; use sp_std::convert::TryInto; - use crate::{assert_ok, Twox128}; + use crate::Twox128; crate::parameter_types! { pub const Seven: u32 = 7; @@ -419,6 +313,11 @@ pub mod test { FooDoubleMap => DoubleMap<(u32, Twox128), (u32, Twox128), BoundedVec> } + #[test] + fn try_append_is_correct() { + assert_eq!(BoundedVec::::bound(), 7); + } + #[test] fn decode_len_works() { TestExternalities::default().execute_with(|| { @@ -445,66 +344,6 @@ pub mod test { }); } - #[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(), unsafe { - BoundedVec::::unchecked_from(vec![4]) - }); - assert_ok!(FooMap::try_append(2, 5)); - assert_eq!(FooMap::get(2).unwrap(), unsafe { - 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(), unsafe { - BoundedVec::::unchecked_from(vec![4]) - }); - assert_ok!(FooDoubleMap::try_append(2, 1, 5)); - assert_eq!(FooDoubleMap::get(2, 1).unwrap(), unsafe { - BoundedVec::::unchecked_from(vec![4, 5]) - }); - }); - } - #[test] fn try_insert_works() { let mut bounded: BoundedVec = vec![1, 2, 3].try_into().unwrap(); @@ -559,4 +398,13 @@ pub mod test { let bounded: BoundedVec = vec![1, 2, 3, 4, 5, 6].try_into().unwrap(); assert_eq!(bounded, vec![1, 2, 3, 4, 5, 6]); } + + #[test] + fn too_big_vec_fail_to_decode() { + let v: Vec = vec![1, 2, 3, 4, 5]; + assert_eq!( + BoundedVec::::decode(&mut &v.encode()[..]), + Err("BoundedVec exceeds its limit".into()), + ); + } } diff --git a/substrate/frame/support/src/storage/mod.rs b/substrate/frame/support/src/storage/mod.rs index b779e064ac..34d217f5c3 100644 --- a/substrate/frame/support/src/storage/mod.rs +++ b/substrate/frame/support/src/storage/mod.rs @@ -36,6 +36,7 @@ pub mod hashed; pub mod bounded_btree_map; pub mod bounded_btree_set; pub mod bounded_vec; +pub mod weak_bounded_vec; pub mod child; #[doc(hidden)] pub mod generator; @@ -965,12 +966,14 @@ pub trait StorageDecodeLength: private::Sealed + codec::DecodeLength { mod private { use super::*; use bounded_vec::BoundedVec; + use weak_bounded_vec::WeakBoundedVec; pub trait Sealed {} impl Sealed for Vec {} impl Sealed for Digest {} impl Sealed for BoundedVec {} + impl Sealed for WeakBoundedVec {} impl Sealed for bounded_btree_map::BoundedBTreeMap {} impl Sealed for bounded_btree_set::BoundedBTreeSet {} @@ -1010,13 +1013,132 @@ impl StorageDecodeLength for Vec {} /// format ever changes, we need to remove this here. impl StorageAppend> for Digest {} +/// Marker trait that is implemented for types that support the `storage::append` api with a limit +/// on the number of element. +/// +/// This trait is sealed. +pub trait StorageTryAppend: StorageDecodeLength + private::Sealed { + fn bound() -> usize; +} + +/// Storage value that is capable of [`StorageTryAppend`](crate::storage::StorageTryAppend). +pub trait TryAppendValue, I: Encode> { + /// Try and append the `item` into the storage item. + /// + /// This might fail if bounds are not respected. + fn try_append>(item: LikeI) -> Result<(), ()>; +} + +impl TryAppendValue for StorageValueT +where + I: Encode, + T: FullCodec + StorageTryAppend, + StorageValueT: generator::StorageValue, +{ + fn try_append>(item: LikeI) -> Result<(), ()> { + let bound = T::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(()) + } + } +} + +/// Storage map that is capable of [`StorageTryAppend`](crate::storage::StorageTryAppend). +pub trait TryAppendMap, I: Encode> { + /// 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, LikeI: EncodeLike>( + key: LikeK, + item: LikeI, + ) -> Result<(), ()>; +} + +impl TryAppendMap for StorageMapT +where + K: FullCodec, + T: FullCodec + StorageTryAppend, + I: Encode, + StorageMapT: generator::StorageMap, +{ + fn try_append + Clone, LikeI: EncodeLike>( + key: LikeK, + item: LikeI, + ) -> Result<(), ()> { + let bound = T::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(()) + } + } +} + +/// Storage double map that is capable of [`StorageTryAppend`](crate::storage::StorageTryAppend). +pub trait TryAppendDoubleMap, I: Encode> { + /// 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, + LikeI: EncodeLike, + >( + key1: LikeK1, + key2: LikeK2, + item: LikeI, + ) -> Result<(), ()>; +} + +impl TryAppendDoubleMap for StorageDoubleMapT +where + K1: FullCodec, + K2: FullCodec, + T: FullCodec + StorageTryAppend, + I: Encode, + StorageDoubleMapT: generator::StorageDoubleMap, +{ + fn try_append< + LikeK1: EncodeLike + Clone, + LikeK2: EncodeLike + Clone, + LikeI: EncodeLike, + >( + key1: LikeK1, + key2: LikeK2, + item: LikeI, + ) -> Result<(), ()> { + let bound = T::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)] mod test { use super::*; use sp_core::hashing::twox_128; - use crate::hash::Identity; + use crate::{hash::Identity, assert_ok}; use sp_io::TestExternalities; use generator::StorageValue as _; + use bounded_vec::BoundedVec; + use weak_bounded_vec::WeakBoundedVec; + use core::convert::{TryFrom, TryInto}; #[test] fn prefixed_map_works() { @@ -1225,4 +1347,80 @@ mod test { ); }); } + + crate::parameter_types! { + pub const Seven: u32 = 7; + pub const Four: u32 = 4; + } + + 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 try_append_works() { + TestExternalities::default().execute_with(|| { + let bounded: WeakBoundedVec = 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::::try_from(vec![4]).unwrap(), + ); + assert_ok!(FooMap::try_append(2, 5)); + assert_eq!( + FooMap::get(2).unwrap(), + BoundedVec::::try_from(vec![4, 5]).unwrap(), + ); + }); + + 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::::try_from(vec![4]).unwrap(), + ); + assert_ok!(FooDoubleMap::try_append(2, 1, 5)); + assert_eq!( + FooDoubleMap::get(2, 1).unwrap(), + BoundedVec::::try_from(vec![4, 5]).unwrap(), + ); + }); + } } diff --git a/substrate/frame/support/src/storage/types/double_map.rs b/substrate/frame/support/src/storage/types/double_map.rs index 8c23354817..f0ed1999d9 100644 --- a/substrate/frame/support/src/storage/types/double_map.rs +++ b/substrate/frame/support/src/storage/types/double_map.rs @@ -21,8 +21,7 @@ use codec::{Decode, Encode, EncodeLike, FullCodec}; use crate::{ storage::{ - StorageAppend, StorageDecodeLength, StoragePrefixedMap, - bounded_vec::BoundedVec, + StorageAppend, StorageTryAppend, StorageDecodeLength, StoragePrefixedMap, types::{OptionQuery, QueryKindTrait, OnEmptyGetter}, }, traits::{GetDefault, StorageInstance, Get, MaxEncodedLen, StorageInfo}, @@ -116,52 +115,6 @@ where } } -impl - StorageDoubleMap< - Prefix, - Hasher1, - Key1, - Hasher2, - Key2, - BoundedVec, - QueryKind, - OnEmpty, - MaxValues, - > where - Prefix: StorageInstance, - Hasher1: crate::hash::StorageHasher, - Hasher2: crate::hash::StorageHasher, - Key1: FullCodec, - Key2: FullCodec, - QueryKind: QueryKindTrait, OnEmpty>, - OnEmpty: Get + 'static, - MaxValues: Get>, - VecValue: FullCodec, - 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 @@ -390,6 +343,26 @@ where pub fn translate_values Option>(f: F) { >::translate_values(f) } + + /// Try and append the given item to the value in the storage. + /// + /// Is only available if `Value` of the storage implements [`StorageTryAppend`]. + pub fn try_append( + key1: KArg1, + key2: KArg2, + item: EncodeLikeItem, + ) -> Result<(), ()> + where + KArg1: EncodeLike + Clone, + KArg2: EncodeLike + Clone, + Item: Encode, + EncodeLikeItem: EncodeLike, + Value: StorageTryAppend, + { + < + Self as crate::storage::TryAppendDoubleMap + >::try_append(key1, key2, item) + } } impl diff --git a/substrate/frame/support/src/storage/types/map.rs b/substrate/frame/support/src/storage/types/map.rs index ac2817c688..35062fbc61 100644 --- a/substrate/frame/support/src/storage/types/map.rs +++ b/substrate/frame/support/src/storage/types/map.rs @@ -21,8 +21,7 @@ use codec::{FullCodec, Decode, EncodeLike, Encode}; use crate::{ storage::{ - StorageAppend, StorageDecodeLength, StoragePrefixedMap, - bounded_vec::BoundedVec, + StorageAppend, StorageTryAppend, StorageDecodeLength, StoragePrefixedMap, types::{OptionQuery, QueryKindTrait, OnEmptyGetter}, }, traits::{GetDefault, StorageInstance, Get, MaxEncodedLen, StorageInfo}, @@ -98,35 +97,6 @@ where } } -impl - StorageMap, QueryKind, OnEmpty, MaxValues> -where - Prefix: StorageInstance, - Hasher: crate::hash::StorageHasher, - Key: FullCodec, - QueryKind: QueryKindTrait, OnEmpty>, - OnEmpty: Get + 'static, - MaxValues: Get>, - VecValue: FullCodec, - 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 @@ -289,6 +259,24 @@ where pub fn translate_values Option>(f: F) { >::translate_values(f) } + + /// Try and append the given item to the value in the storage. + /// + /// Is only available if `Value` of the storage implements [`StorageTryAppend`]. + pub fn try_append( + key: KArg, + item: EncodeLikeItem, + ) -> Result<(), ()> + where + KArg: EncodeLike + Clone, + Item: Encode, + EncodeLikeItem: EncodeLike, + Value: StorageTryAppend, + { + < + Self as crate::storage::TryAppendMap + >::try_append(key, item) + } } impl diff --git a/substrate/frame/support/src/storage/types/value.rs b/substrate/frame/support/src/storage/types/value.rs index 67d2e37419..5b37066fc3 100644 --- a/substrate/frame/support/src/storage/types/value.rs +++ b/substrate/frame/support/src/storage/types/value.rs @@ -20,11 +20,10 @@ use codec::{FullCodec, Decode, EncodeLike, Encode}; use crate::{ storage::{ - StorageAppend, StorageDecodeLength, - bounded_vec::BoundedVec, + StorageAppend, StorageTryAppend, StorageDecodeLength, types::{OptionQuery, QueryKindTrait, OnEmptyGetter}, }, - traits::{GetDefault, StorageInstance, Get, MaxEncodedLen, StorageInfo}, + traits::{GetDefault, StorageInstance, MaxEncodedLen, StorageInfo}, }; use frame_metadata::{DefaultByteGetter, StorageEntryModifier}; use sp_arithmetic::traits::SaturatedConversion; @@ -63,26 +62,6 @@ where } } -impl - StorageValue, QueryKind, OnEmpty> -where - Prefix: StorageInstance, - QueryKind: QueryKindTrait, OnEmpty>, - OnEmpty: crate::traits::Get + 'static, - VecValue: FullCodec, - 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, @@ -192,6 +171,18 @@ where pub fn decode_len() -> Option where Value: StorageDecodeLength { >::decode_len() } + + /// Try and append the given item to the value in the storage. + /// + /// Is only available if `Value` of the storage implements [`StorageTryAppend`]. + pub fn try_append(item: EncodeLikeItem) -> Result<(), ()> + where + Item: Encode, + EncodeLikeItem: EncodeLike, + Value: StorageTryAppend, + { + >::try_append(item) + } } /// Part of storage metadata for storage value. diff --git a/substrate/frame/support/src/storage/weak_bounded_vec.rs b/substrate/frame/support/src/storage/weak_bounded_vec.rs new file mode 100644 index 0000000000..606c24de44 --- /dev/null +++ b/substrate/frame/support/src/storage/weak_bounded_vec.rs @@ -0,0 +1,420 @@ +// 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, fmt, marker::PhantomData}; +use codec::{Encode, Decode}; +use core::{ + ops::{Deref, Index, IndexMut}, + slice::SliceIndex, +}; +use crate::{ + traits::{Get, MaxEncodedLen}, + storage::{StorageDecodeLength, StorageTryAppend}, +}; + +/// A weakly bounded vector. +/// +/// It has implementations for efficient append and length decoding, as with a normal `Vec<_>`, once +/// put into storage as a raw value, map or double-map. +/// +/// The length of the vec is not strictly bounded. Decoding a vec with more element that the bound +/// is accepted, and some method allow to bypass the restriction with warnings. +#[derive(Encode)] +pub struct WeakBoundedVec(Vec, PhantomData); + +impl> Decode for WeakBoundedVec { + fn decode(input: &mut I) -> Result { + let inner = Vec::::decode(input)?; + Ok(Self::force_from(inner, Some("decode"))) + } + + fn skip(input: &mut I) -> Result<(), codec::Error> { + Vec::::skip(input) + } +} + +impl WeakBoundedVec { + /// Create `Self` from `t` without any checks. + fn unchecked_from(t: Vec) -> Self { + Self(t, Default::default()) + } + + /// 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 `WeakBoundedVec`. + pub fn into_inner(self) -> Vec { + self.0 + } + + /// 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> WeakBoundedVec { + /// Get the bound of the type in `usize`. + pub fn bound() -> usize { + S::get() as usize + } + + /// 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. + 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) + } + + /// 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(()) + } + } +} + +impl Default for WeakBoundedVec { + fn default() -> Self { + // the bound cannot be below 0, which is satisfied by an empty vector + Self::unchecked_from(Vec::default()) + } +} + +#[cfg(feature = "std")] +impl fmt::Debug for WeakBoundedVec +where + T: fmt::Debug, + S: Get, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("WeakBoundedVec").field(&self.0).field(&Self::bound()).finish() + } +} + +impl Clone for WeakBoundedVec +where + T: Clone, +{ + fn clone(&self) -> Self { + // bound is retained + Self::unchecked_from(self.0.clone()) + } +} + +impl> TryFrom> for WeakBoundedVec { + type Error = (); + fn try_from(t: Vec) -> Result { + if t.len() <= Self::bound() { + // explicit check just above + 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 WeakBoundedVec { + fn as_ref(&self) -> &Vec { + &self.0 + } +} + +impl AsRef<[T]> for WeakBoundedVec { + fn as_ref(&self) -> &[T] { + &self.0 + } +} + +impl AsMut<[T]> for WeakBoundedVec { + fn as_mut(&mut self) -> &mut [T] { + &mut self.0 + } +} + +// will allow for immutable all operations of `Vec` on `WeakBoundedVec`. +impl Deref for WeakBoundedVec { + 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 Index for WeakBoundedVec +where + I: SliceIndex<[T]>, +{ + type Output = I::Output; + + #[inline] + fn index(&self, index: I) -> &Self::Output { + self.0.index(index) + } +} + +impl IndexMut for WeakBoundedVec +where + I: SliceIndex<[T]>, +{ + #[inline] + fn index_mut(&mut self, index: I) -> &mut Self::Output { + self.0.index_mut(index) + } +} + +impl sp_std::iter::IntoIterator for WeakBoundedVec { + type Item = T; + type IntoIter = sp_std::vec::IntoIter; + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl codec::DecodeLength for WeakBoundedVec { + fn len(self_encoded: &[u8]) -> Result { + // `WeakBoundedVec` 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) + } +} + +// NOTE: we could also implement this as: +// impl, S2: Get> PartialEq> for WeakBoundedVec +// to allow comparison of bounded vectors with different bounds. +impl PartialEq for WeakBoundedVec +where + T: PartialEq, +{ + fn eq(&self, rhs: &Self) -> bool { + self.0 == rhs.0 + } +} + +impl> PartialEq> for WeakBoundedVec { + fn eq(&self, other: &Vec) -> bool { + &self.0 == other + } +} + +impl Eq for WeakBoundedVec where T: Eq {} + +impl StorageDecodeLength for WeakBoundedVec {} + +impl> StorageTryAppend for WeakBoundedVec { + fn bound() -> usize { + S::get() as usize + } +} + +impl MaxEncodedLen for WeakBoundedVec +where + T: MaxEncodedLen, + S: Get, + WeakBoundedVec: Encode, +{ + fn max_encoded_len() -> usize { + // WeakBoundedVec encodes like Vec which encodes like [T], which is a compact u32 + // plus each item in the slice: + // https://substrate.dev/rustdocs/v3.0.0/src/parity_scale_codec/codec.rs.html#798-808 + codec::Compact(S::get()) + .encoded_size() + .saturating_add(Self::bound().saturating_mul(T::max_encoded_len())) + } +} + +#[cfg(test)] +pub mod test { + use super::*; + use sp_io::TestExternalities; + use sp_std::convert::TryInto; + use crate::Twox128; + + crate::parameter_types! { + pub const Seven: u32 = 7; + pub const Four: u32 = 4; + } + + crate::generate_storage_alias! { Prefix, Foo => Value> } + crate::generate_storage_alias! { Prefix, FooMap => Map<(u32, Twox128), WeakBoundedVec> } + crate::generate_storage_alias! { + Prefix, + FooDoubleMap => DoubleMap<(u32, Twox128), (u32, Twox128), WeakBoundedVec> + } + + #[test] + fn try_append_is_correct() { + assert_eq!(WeakBoundedVec::::bound(), 7); + } + + #[test] + fn decode_len_works() { + TestExternalities::default().execute_with(|| { + let bounded: WeakBoundedVec = vec![1, 2, 3].try_into().unwrap(); + Foo::put(bounded); + assert_eq!(Foo::decode_len().unwrap(), 3); + }); + + TestExternalities::default().execute_with(|| { + let bounded: WeakBoundedVec = 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: WeakBoundedVec = 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_insert_works() { + let mut bounded: WeakBoundedVec = 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: WeakBoundedVec = vec![1, 2, 3].try_into().unwrap(); + bounded.try_insert(9, 0).unwrap(); + } + + #[test] + fn try_push_works() { + let mut bounded: WeakBoundedVec = 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: WeakBoundedVec = 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: WeakBoundedVec = 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()); + } + + #[test] + fn slice_indexing_works() { + let bounded: WeakBoundedVec = vec![1, 2, 3, 4, 5, 6].try_into().unwrap(); + assert_eq!(&bounded[0..=2], &[1, 2, 3]); + } + + #[test] + fn vec_eq_works() { + let bounded: WeakBoundedVec = vec![1, 2, 3, 4, 5, 6].try_into().unwrap(); + assert_eq!(bounded, vec![1, 2, 3, 4, 5, 6]); + } + + #[test] + fn too_big_succeed_to_decode() { + let v: Vec = vec![1, 2, 3, 4, 5]; + let w = WeakBoundedVec::::decode(&mut &v.encode()[..]).unwrap(); + assert_eq!(v, *w); + } +} diff --git a/substrate/frame/treasury/src/lib.rs b/substrate/frame/treasury/src/lib.rs index 473a570a87..6028f1fbe4 100644 --- a/substrate/frame/treasury/src/lib.rs +++ b/substrate/frame/treasury/src/lib.rs @@ -66,7 +66,7 @@ pub mod weights; use sp_std::prelude::*; use frame_support::{ decl_module, decl_storage, decl_event, ensure, print, decl_error, - PalletId, BoundedVec, bounded_vec::TryAppendValue, + PalletId, BoundedVec, storage::TryAppendValue, }; use frame_support::traits::{ Currency, Get, Imbalance, OnUnbalanced, ExistenceRequirement::KeepAlive,