diff --git a/substrate/frame/elections-phragmen/src/lib.rs b/substrate/frame/elections-phragmen/src/lib.rs index 040ef00ac2..05f4d1e83d 100644 --- a/substrate/frame/elections-phragmen/src/lib.rs +++ b/substrate/frame/elections-phragmen/src/lib.rs @@ -277,9 +277,9 @@ decl_module! { fn vote(origin, votes: Vec, #[compact] value: BalanceOf) { let who = ensure_signed(origin)?; - let candidates_count = >::decode_len().unwrap_or(0) as usize; - let members_count = >::decode_len().unwrap_or(0) as usize; - let runners_up_count = >::decode_len().unwrap_or(0) as usize; + let candidates_count = >::decode_len().unwrap_or(0); + let members_count = >::decode_len().unwrap_or(0); + let runners_up_count = >::decode_len().unwrap_or(0); // addition is valid: candidates, members and runners-up will never overlap. let allowed_votes = candidates_count + members_count + runners_up_count; @@ -1196,7 +1196,7 @@ mod tests { assert_eq!(Elections::runners_up(), vec![]); assert_eq!(Elections::candidates(), vec![]); - assert_eq!(>::decode_len().unwrap(), 0); + assert_eq!(>::decode_len(), None); assert!(Elections::is_candidate(&1).is_err()); assert_eq!(all_voters(), vec![]); @@ -1800,7 +1800,7 @@ mod tests { assert_eq!(Elections::runners_up(), vec![]); assert_eq_uvec!(all_voters(), vec![2, 3, 4]); assert_eq!(Elections::candidates(), vec![]); - assert_eq!(>::decode_len().unwrap(), 0); + assert_eq!(>::decode_len(), None); assert_eq!(Elections::election_rounds(), 1); }); diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index 534222ebdf..71042d69b3 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -2337,7 +2337,7 @@ impl Module { // that we need. Then it should be Self::validator_count(). Else it should be all the // candidates. let snapshot_length = >::decode_len() - .map_err(|_| Error::::SnapshotUnavailable)?; + .ok_or_else(|| Error::::SnapshotUnavailable)?; // check the winner length only here and when we know the length of the snapshot validators // length. diff --git a/substrate/frame/support/src/storage/generator/double_map.rs b/substrate/frame/support/src/storage/generator/double_map.rs index 2c18089d38..722a9dcabc 100644 --- a/substrate/frame/support/src/storage/generator/double_map.rs +++ b/substrate/frame/support/src/storage/generator/double_map.rs @@ -17,7 +17,7 @@ use sp_std::prelude::*; use sp_std::borrow::Borrow; use codec::{FullCodec, FullEncode, Decode, Encode, EncodeLike}; -use crate::{storage::{self, unhashed, StorageAppend}, traits::Len, Never}; +use crate::{storage::{self, unhashed, StorageAppend}, Never}; use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher}; /// Generator for `StorageDoubleMap` used by `decl_storage`. @@ -260,23 +260,6 @@ impl storage::StorageDoubleMap for G where sp_io::storage::append(&final_key, item.encode()); } - fn decode_len(key1: KArg1, key2: KArg2) -> Result where - KArg1: EncodeLike, - KArg2: EncodeLike, - V: codec::DecodeLength + Len, - { - let final_key = Self::storage_double_map_final_key(key1, key2); - if let Some(v) = unhashed::get_raw(&final_key) { - ::len(&v).map_err(|e| e.what()) - } else { - let len = G::from_query_to_optional_value(G::from_optional_value_to_query(None)) - .map(|v| v.len()) - .unwrap_or(0); - - Ok(len) - } - } - fn migrate_keys< OldHasher1: StorageHasher, OldHasher2: StorageHasher, diff --git a/substrate/frame/support/src/storage/generator/map.rs b/substrate/frame/support/src/storage/generator/map.rs index 307e3b1c78..fa870ac075 100644 --- a/substrate/frame/support/src/storage/generator/map.rs +++ b/substrate/frame/support/src/storage/generator/map.rs @@ -18,8 +18,10 @@ use sp_std::prelude::*; use sp_std::borrow::Borrow; use codec::{FullCodec, FullEncode, Decode, Encode, EncodeLike}; -use crate::{storage::{self, unhashed, StorageAppend}, traits::Len, Never}; -use crate::hash::{StorageHasher, Twox128, ReversibleStorageHasher}; +use crate::{ + storage::{self, unhashed, StorageAppend}, + Never, hash::{StorageHasher, Twox128, ReversibleStorageHasher}, +}; /// Generator for `StorageMap` used by `decl_storage`. /// @@ -287,21 +289,6 @@ impl> storage::StorageMap sp_io::storage::append(&key, item.encode()); } - fn decode_len>(key: KeyArg) -> Result - where V: codec::DecodeLength + Len - { - let key = Self::storage_map_final_key(key); - if let Some(v) = unhashed::get_raw(key.as_ref()) { - ::len(&v).map_err(|e| e.what()) - } else { - let len = G::from_query_to_optional_value(G::from_optional_value_to_query(None)) - .map(|v| v.len()) - .unwrap_or(0); - - Ok(len) - } - } - fn migrate_key>(key: KeyArg) -> Option { let old_key = { let module_prefix_hashed = Twox128::hash(Self::module_prefix()); diff --git a/substrate/frame/support/src/storage/generator/value.rs b/substrate/frame/support/src/storage/generator/value.rs index f4119ba461..9cc36b6372 100644 --- a/substrate/frame/support/src/storage/generator/value.rs +++ b/substrate/frame/support/src/storage/generator/value.rs @@ -19,7 +19,6 @@ use crate::{ Never, storage::{self, unhashed, StorageAppend}, hash::{Twox128, StorageHasher}, - traits::Len }; /// Generator for `StorageValue` used by `decl_storage`. @@ -141,19 +140,4 @@ impl> storage::StorageValue for G { let key = Self::storage_value_final_key(); sp_io::storage::append(&key, item.encode()); } - - fn decode_len() -> Result where T: codec::DecodeLength, T: Len { - let key = Self::storage_value_final_key(); - - // attempt to get the length directly. - if let Some(k) = unhashed::get_raw(&key) { - ::len(&k).map_err(|e| e.what()) - } else { - let len = G::from_query_to_optional_value(G::from_optional_value_to_query(None)) - .map(|v| v.len()) - .unwrap_or(0); - - Ok(len) - } - } } diff --git a/substrate/frame/support/src/storage/mod.rs b/substrate/frame/support/src/storage/mod.rs index cdbdcbc6ff..9214d931e3 100644 --- a/substrate/frame/support/src/storage/mod.rs +++ b/substrate/frame/support/src/storage/mod.rs @@ -18,7 +18,7 @@ use sp_std::{prelude::*, marker::PhantomData}; use codec::{FullCodec, FullEncode, Encode, EncodeLike, Decode}; -use crate::{traits::Len, hash::{Twox128, StorageHasher}}; +use crate::hash::{Twox128, StorageHasher}; use sp_runtime::generic::{Digest, DigestItem}; pub mod unhashed; @@ -105,11 +105,20 @@ pub trait StorageValue { EncodeLikeItem: EncodeLike, T: StorageAppend; - /// Read the length of the value in a fast way, without decoding the entire value. + /// Read the length of the storage value without decoding the entire value. /// - /// `T` is required to implement `Codec::DecodeLength`. - fn decode_len() -> Result - where T: codec::DecodeLength + Len; + /// `T` is required to implement [`StorageDecodeLength`]. + /// + /// If the value does not exists or it fails to decode the length, `None` is returned. + /// Otherwise `Some(len)` is returned. + /// + /// # Warning + /// + /// `None` does not mean that `get()` does not return a value. The default value is completly + /// ignored by this function. + fn decode_len() -> Option where T: StorageDecodeLength { + T::decode_len(&Self::hashed_key()) + } } /// A strongly-typed map in storage. @@ -175,15 +184,23 @@ pub trait StorageMap { EncodeLikeItem: EncodeLike, V: StorageAppend; - /// Read the length of the value in a fast way, without decoding the entire value. + /// Read the length of the storage value without decoding the entire value under the + /// given `key`. /// - /// `T` is required to implement `Codec::DecodeLength`. + /// `V` is required to implement [`StorageDecodeLength`]. /// - /// Note that `0` is returned as the default value if no encoded value exists at the given key. - /// Therefore, this function cannot be used as a sign of _existence_. use the `::contains_key()` - /// function for this purpose. - fn decode_len>(key: KeyArg) -> Result - where V: codec::DecodeLength + Len; + /// If the value does not exists or it fails to decode the length, `None` is returned. + /// Otherwise `Some(len)` is returned. + /// + /// # Warning + /// + /// `None` does not mean that `get()` does not return a value. The default value is completly + /// ignored by this function. + fn decode_len>(key: KeyArg) -> Option + where V: StorageDecodeLength, + { + V::decode_len(&Self::hashed_key_for(key)) + } /// Migrate an item with the given `key` from a defunct `OldHasher` to the current hasher. /// @@ -348,18 +365,26 @@ pub trait StorageDoubleMap { EncodeLikeItem: EncodeLike, V: StorageAppend; - /// Read the length of the value in a fast way, without decoding the entire value. + /// Read the length of the storage value without decoding the entire value under the + /// given `key1` and `key2`. /// - /// `V` is required to implement `Codec::DecodeLength`. + /// `V` is required to implement [`StorageDecodeLength`]. /// - /// Note that `0` is returned as the default value if no encoded value exists at the given key. - /// Therefore, this function cannot be used as a sign of _existence_. use the `::contains_key()` - /// function for this purpose. - fn decode_len(key1: KArg1, key2: KArg2) -> Result + /// If the value does not exists or it fails to decode the length, `None` is returned. + /// Otherwise `Some(len)` is returned. + /// + /// # Warning + /// + /// `None` does not mean that `get()` does not return a value. The default value is completly + /// ignored by this function. + fn decode_len(key1: KArg1, key2: KArg2) -> Option where KArg1: EncodeLike, KArg2: EncodeLike, - V: codec::DecodeLength + Len; + V: StorageDecodeLength, + { + V::decode_len(&Self::hashed_key_for(key1, key2)) + } /// Migrate an item with the given `key1` and `key2` from defunct `OldHasher1` and /// `OldHasher2` to the current hashers. @@ -493,7 +518,29 @@ pub trait StoragePrefixedMap { /// This trait is sealed. pub trait StorageAppend: private::Sealed {} -/// Provides `Sealed` trait to prevent implementing trait `StorageAppend` outside of this crate. +/// Marker trait that will be implemented for types that support to decode their length in an +/// effificent way. It is expected that the length is at the beginning of the encoded object +/// and that the length is a `Compact`. +/// +/// This trait is sealed. +pub trait StorageDecodeLength: private::Sealed + codec::DecodeLength { + /// Decode the length of the storage value at `key`. + /// + /// This function assumes that the length is at the beginning of the encoded object + /// and is a `Compact`. + /// + /// Returns `None` if the storage value does not exist or the decoding failed. + fn decode_len(key: &[u8]) -> Option { + // `Compact` is 5 bytes in maximum. + let mut data = [0u8; 5]; + let len = sp_io::storage::read(key, &mut data, 0)?; + let len = data.len().min(len as usize); + ::len(&data[..len]).ok() + } +} + +/// Provides `Sealed` trait to prevent implementing trait `StorageAppend` & `StorageDecodeLength` +/// outside of this crate. mod private { use super::*; @@ -504,6 +551,7 @@ mod private { } 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 diff --git a/substrate/frame/support/test/tests/decl_storage.rs b/substrate/frame/support/test/tests/decl_storage.rs index 77e538625a..5ec5bbecfe 100644 --- a/substrate/frame/support/test/tests/decl_storage.rs +++ b/substrate/frame/support/test/tests/decl_storage.rs @@ -593,38 +593,40 @@ mod test_append_and_len { }); } + // `decode_len` should always return `None` for default assigments + // in `decl_storage!`. #[test] - fn len_works_for_default() { + fn len_works_ignores_default_assignment() { TestExternalities::default().execute_with(|| { // vec assert_eq!(JustVec::get(), vec![]); - assert_eq!(JustVec::decode_len(), Ok(0)); + assert_eq!(JustVec::decode_len(), None); assert_eq!(JustVecWithDefault::get(), vec![6, 9]); - assert_eq!(JustVecWithDefault::decode_len(), Ok(2)); + assert_eq!(JustVecWithDefault::decode_len(), None); assert_eq!(OptionVec::get(), None); - assert_eq!(OptionVec::decode_len(), Ok(0)); + assert_eq!(OptionVec::decode_len(), None); // map assert_eq!(MapVec::get(0), vec![]); - assert_eq!(MapVec::decode_len(0), Ok(0)); + assert_eq!(MapVec::decode_len(0), None); assert_eq!(MapVecWithDefault::get(0), vec![6, 9]); - assert_eq!(MapVecWithDefault::decode_len(0), Ok(2)); + assert_eq!(MapVecWithDefault::decode_len(0), None); assert_eq!(OptionMapVec::get(0), None); - assert_eq!(OptionMapVec::decode_len(0), Ok(0)); + assert_eq!(OptionMapVec::decode_len(0), None); // Double map assert_eq!(DoubleMapVec::get(0, 0), vec![]); - assert_eq!(DoubleMapVec::decode_len(0, 1), Ok(0)); + assert_eq!(DoubleMapVec::decode_len(0, 1), None); assert_eq!(DoubleMapVecWithDefault::get(0, 0), vec![6, 9]); - assert_eq!(DoubleMapVecWithDefault::decode_len(0, 1), Ok(2)); + assert_eq!(DoubleMapVecWithDefault::decode_len(0, 1), None); assert_eq!(OptionDoubleMapVec::get(0, 0), None); - assert_eq!(OptionDoubleMapVec::decode_len(0, 1), Ok(0)); + assert_eq!(OptionDoubleMapVec::decode_len(0, 1), None); }); } }