Optimize decode_len (#5975)

* Optimize `decode_len`

Instead of reading the full storage value into the runtime, we only read
at maximum `5bytes` from the storage into the runtime. Furthermore this
drops any handling with regards to set default values in
`decl_storage!`. If the value does not exists or the decoding of the
length fails, it will return `None`. To prevent people from messing
stuff up, this feature relies on the `StorageDecodeLength` trait that is
sealed by `frame-support` (aka only implementable inside this crate).

* Some clean ups

* Update frame/support/src/storage/mod.rs

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>

substrate/frame/elections-phragmen/src/lib.rs

@@ -277,9 +277,9 @@ decl_module! {
 		fn vote(origin, votes: Vec<T::AccountId>, #[compact] value: BalanceOf<T>) {
 			let who = ensure_signed(origin)?;
 
-			let candidates_count = <Candidates<T>>::decode_len().unwrap_or(0) as usize;
-			let members_count = <Members<T>>::decode_len().unwrap_or(0) as usize;
-			let runners_up_count = <RunnersUp<T>>::decode_len().unwrap_or(0) as usize;
+			let candidates_count = <Candidates<T>>::decode_len().unwrap_or(0);
+			let members_count = <Members<T>>::decode_len().unwrap_or(0);
+			let runners_up_count = <RunnersUp<T>>::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!(<Candidates<Test>>::decode_len().unwrap(), 0);
+			assert_eq!(<Candidates<Test>>::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!(<Candidates<Test>>::decode_len().unwrap(), 0);
+			assert_eq!(<Candidates<Test>>::decode_len(), None);
 
 			assert_eq!(Elections::election_rounds(), 1);
 		});

substrate/frame/staking/src/lib.rs

@@ -2337,7 +2337,7 @@ impl<T: Trait> Module<T> {
 		// that we need. Then it should be Self::validator_count(). Else it should be all the
 		// candidates.
 		let snapshot_length = <SnapshotValidators<T>>::decode_len()
-			.map_err(|_| Error::<T>::SnapshotUnavailable)?;
+			.ok_or_else(|| Error::<T>::SnapshotUnavailable)?;
 
 		// check the winner length only here and when we know the length of the snapshot validators
 		// length.

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<K1, K2, V, G> storage::StorageDoubleMap<K1, K2, V> for G where
 		sp_io::storage::append(&final_key, item.encode());
 	}
 
-	fn decode_len<KArg1, KArg2>(key1: KArg1, key2: KArg2) -> Result<usize, &'static str> where
-		KArg1: EncodeLike<K1>,
-		KArg2: EncodeLike<K2>,
-		V: codec::DecodeLength + Len,
-	{
-		let final_key = Self::storage_double_map_final_key(key1, key2);
-		if let Some(v) = unhashed::get_raw(&final_key) {
-			<V as codec::DecodeLength>::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,

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<K: FullEncode, V: FullCodec, G: StorageMap<K, V>> storage::StorageMap<K, V>
 		sp_io::storage::append(&key, item.encode());
 	}
 
-	fn decode_len<KeyArg: EncodeLike<K>>(key: KeyArg) -> Result<usize, &'static str>
-		where V: codec::DecodeLength + Len
-	{
-		let key = Self::storage_map_final_key(key);
-		if let Some(v) = unhashed::get_raw(key.as_ref()) {
-			<V as codec::DecodeLength>::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<OldHasher: StorageHasher, KeyArg: EncodeLike<K>>(key: KeyArg) -> Option<V> {
 		let old_key = {
 			let module_prefix_hashed = Twox128::hash(Self::module_prefix());

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<T: FullCodec, G: StorageValue<T>> storage::StorageValue<T> for G {
 		let key = Self::storage_value_final_key();
 		sp_io::storage::append(&key, item.encode());
 	}
-
-	fn decode_len() -> Result<usize, &'static str> 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) {
-			<T as codec::DecodeLength>::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)
-		}
-	}
 }

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<T: FullCodec> {
 		EncodeLikeItem: EncodeLike<Item>,
 		T: StorageAppend<Item>;
 
-	/// 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<usize, &'static str>
-		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<usize> where T: StorageDecodeLength {
+		T::decode_len(&Self::hashed_key())
+	}
 }
 
 /// A strongly-typed map in storage.
@@ -175,15 +184,23 @@ pub trait StorageMap<K: FullEncode, V: FullCodec> {
 		EncodeLikeItem: EncodeLike<Item>,
 		V: StorageAppend<Item>;
 
-	/// 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<KeyArg: EncodeLike<K>>(key: KeyArg) -> Result<usize, &'static str>
-		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<KeyArg: EncodeLike<K>>(key: KeyArg) -> Option<usize>
+		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<K1: FullEncode, K2: FullEncode, V: FullCodec> {
 		EncodeLikeItem: EncodeLike<Item>,
 		V: StorageAppend<Item>;
 
-	/// 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<KArg1, KArg2>(key1: KArg1, key2: KArg2) -> Result<usize, &'static str>
+	/// 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<KArg1, KArg2>(key1: KArg1, key2: KArg2) -> Option<usize>
 		where
 			KArg1: EncodeLike<K1>,
 			KArg2: EncodeLike<K2>,
-			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<Value: FullCodec> {
 /// This trait is sealed.
 pub trait StorageAppend<Item: Encode>: 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<u32>`.
+///
+/// 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<u32>`.
+	///
+	/// Returns `None` if the storage value does not exist or the decoding failed.
+	fn decode_len(key: &[u8]) -> Option<usize> {
+		// `Compact<u32>` 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);
+		<Self as codec::DecodeLength>::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<T: Encode> StorageAppend<T> for Vec<T> {}
+impl<T: Encode> StorageDecodeLength for Vec<T> {}
 
 /// 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

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);
 		});
 	}
 }