Make Digest support StorageAppend (#5922)

* Make `Digest` support `StorageAppend`

This adds support for `StorageAppend` to `Digest`. Digest is just a
wrapper around a `Vec` and we abuse the fact that SCALE does not puts
any special marker into the encoding for structs. So, we can just append
to the encoded Digest. A test is added that ensures, if the `Digest`
format ever changes, we remove this optimization.

* Update weight

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

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

* Update frame/system/src/lib.rs

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

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

substrate/client/finality-grandpa/src/light_import.rs

@@ -18,9 +18,7 @@ use std::collections::HashMap;
 use std::sync::Arc;
 use log::{info, trace, warn};
 use parking_lot::RwLock;
-use sc_client_api::{
-	backend::{AuxStore, Backend, Finalizer, TransactionFor},
-};
+use sc_client_api::backend::{AuxStore, Backend, Finalizer, TransactionFor};
 use sp_blockchain::{HeaderBackend, Error as ClientError, well_known_cache_keys};
 use parity_scale_codec::{Encode, Decode};
 use sp_consensus::{
@@ -220,7 +218,7 @@ impl LightAuthoritySet {
 	/// Set new authorities set.
 	pub fn update(&mut self, set_id: u64, authorities: AuthorityList) {
 		self.set_id = set_id;
-		std::mem::replace(&mut self.authorities, authorities);
+		self.authorities = authorities;
 	}
 }
 

substrate/frame/support/src/storage/mod.rs

@@ -19,6 +19,7 @@
 use sp_std::{prelude::*, marker::PhantomData};
 use codec::{FullCodec, FullEncode, Encode, EncodeLike, Decode};
 use crate::{traits::Len, hash::{Twox128, StorageHasher}};
+use sp_runtime::generic::{Digest, DigestItem};
 
 pub mod unhashed;
 pub mod hashed;
@@ -499,15 +500,22 @@ mod private {
 	pub trait Sealed {}
 
 	impl<T: Encode> Sealed for Vec<T> {}
+	impl<Hash: Encode> Sealed for Digest<Hash> {}
 }
 
 impl<T: Encode> StorageAppend<T> 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
+/// that if the `Digest` format ever changes, we need to remove this here.
+impl<Hash: Encode> StorageAppend<DigestItem<Hash>> for Digest<Hash> {}
+
 #[cfg(test)]
 mod test {
+	use super::*;
 	use sp_core::hashing::twox_128;
 	use sp_io::TestExternalities;
-	use crate::storage::{unhashed, StoragePrefixedMap};
+	use generator::StorageValue as _;
 
 	#[test]
 	fn prefixed_map_works() {
@@ -582,4 +590,41 @@ mod test {
 			assert_eq!(unhashed::get(&key_after[..]), Some(33u64));
 		});
 	}
+
+	// This test ensures that the Digest encoding does not change without being noticied.
+	#[test]
+	fn digest_storage_append_works_as_expected() {
+		TestExternalities::default().execute_with(|| {
+			struct Storage;
+			impl generator::StorageValue<Digest<u32>> for Storage {
+				type Query = Digest<u32>;
+
+				fn module_prefix() -> &'static [u8] {
+					b"MyModule"
+				}
+
+				fn storage_prefix() -> &'static [u8] {
+					b"Storage"
+				}
+
+				fn from_optional_value_to_query(v: Option<Digest<u32>>) -> Self::Query {
+					v.unwrap()
+				}
+
+				fn from_query_to_optional_value(v: Self::Query) -> Option<Digest<u32>> {
+					Some(v)
+				}
+			}
+
+			Storage::append(DigestItem::ChangesTrieRoot(1));
+			Storage::append(DigestItem::Other(Vec::new()));
+
+			let value = unhashed::get_raw(&Storage::storage_value_final_key()).unwrap();
+
+			let expected = Digest {
+				logs: vec![DigestItem::ChangesTrieRoot(1), DigestItem::Other(Vec::new())],
+			};
+			assert_eq!(Digest::decode(&mut &value[..]).unwrap(), expected);
+		});
+	}
 }

substrate/frame/system/src/lib.rs

@@ -993,13 +993,11 @@ impl<T: Trait> Module<T> {
 	/// Deposits a log and ensures it matches the block's log data.
 	///
 	/// # <weight>
-	/// - `O(D)` where `D` length of `Digest`
-	/// - 1 storage mutation (codec `O(D)`).
+	/// - `O(1)`
+	/// - 1 storage write (codec `O(1)`)
 	/// # </weight>
 	pub fn deposit_log(item: DigestItemOf<T>) {
-		let mut l = <Digest<T>>::get();
-		l.push(item);
-		<Digest<T>>::put(l);
+		<Digest<T>>::append(item);
 	}
 
 	/// Get the basic externalities for this module, useful for tests.

substrate/primitives/runtime/src/generic/digest.rs

@@ -28,18 +28,22 @@ use sp_core::{ChangesTrieConfiguration, RuntimeDebug};
 /// Generic header digest.
 #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)]
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize, parity_util_mem::MallocSizeOf))]
-pub struct Digest<Hash: Encode + Decode> {
+pub struct Digest<Hash> {
 	/// A list of logs in the digest.
+	#[cfg_attr(
+		feature = "std",
+		serde(bound(serialize = "Hash: codec::Codec", deserialize = "Hash: codec::Codec"))
+	)]
 	pub logs: Vec<DigestItem<Hash>>,
 }
 
-impl<Item: Encode + Decode> Default for Digest<Item> {
+impl<Item> Default for Digest<Item> {
 	fn default() -> Self {
 		Digest { logs: Vec::new(), }
 	}
 }
 
-impl<Hash: Encode + Decode> Digest<Hash> {
+impl<Hash> Digest<Hash> {
 	/// Get reference to all digest items.
 	pub fn logs(&self) -> &[DigestItem<Hash>] {
 		&self.logs