diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 9cf78bfcfb..e6c1b6e4b7 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -7463,6 +7463,7 @@ name = "sp-externalities" version = "0.8.0-dev" dependencies = [ "environmental", + "parity-scale-codec", "sp-std", "sp-storage", ] diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index da5681bf10..234e427d3a 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -115,7 +115,8 @@ use sp_runtime::{ use sp_core::{ChangesTrieConfiguration, storage::well_known_keys}; use frame_support::{ - decl_module, decl_event, decl_storage, decl_error, storage, Parameter, ensure, debug, + decl_module, decl_event, decl_storage, decl_error, Parameter, ensure, debug, + storage::{self, generator::StorageValue}, traits::{ Contains, Get, ModuleToIndex, OnNewAccount, OnKilledAccount, IsDeadAccount, Happened, StoredMap, EnsureOrigin, @@ -854,17 +855,10 @@ impl Module { old_event_count }; - // Appending can only fail if `Events` can not be decoded or - // when we try to insert more than `u32::max_value()` events. - // - // We perform early return if we've reached the maximum capacity of the event list, - // so `Events` seems to be corrupted. Also, this has happened after the start of execution - // (since the event list is cleared at the block initialization). - if >::append([event].iter()).is_err() { - // The most sensible thing to do here is to just ignore this event and wait until the - // new block. - return; - } + // We use append api here to avoid bringing all events in the runtime when we push a + // new one in the list. + let encoded_event = event.encode(); + sp_io::storage::append(&Events::::storage_value_final_key()[..], encoded_event); for topic in topics { // The same applies here. diff --git a/substrate/primitives/externalities/Cargo.toml b/substrate/primitives/externalities/Cargo.toml index 81c9c5fb5c..9d7cd1df6b 100644 --- a/substrate/primitives/externalities/Cargo.toml +++ b/substrate/primitives/externalities/Cargo.toml @@ -16,3 +16,4 @@ targets = ["x86_64-unknown-linux-gnu"] sp-storage = { version = "2.0.0-dev", path = "../storage" } sp-std = { version = "2.0.0-dev", path = "../std" } environmental = { version = "1.1.1" } +codec = { package = "parity-scale-codec", version = "1.3.0" } diff --git a/substrate/primitives/externalities/src/lib.rs b/substrate/primitives/externalities/src/lib.rs index f00742b2e8..2b584b512e 100644 --- a/substrate/primitives/externalities/src/lib.rs +++ b/substrate/primitives/externalities/src/lib.rs @@ -41,6 +41,8 @@ pub enum Error { ExtensionsAreNotSupported, /// Extension `TypeId` is not registered. ExtensionIsNotRegistered(TypeId), + /// Failed to update storage, + StorageUpdateFailed(&'static str), } /// The Substrate externalities. @@ -176,6 +178,15 @@ pub trait Externalities: ExtensionStore { child_info: &ChildInfo, ) -> Vec; + /// Append storage item. + /// + /// This assumes specific format of the storage item. Also there is no way to undo this operation. + fn storage_append( + &mut self, + key: Vec, + value: Vec, + ); + /// Get the changes trie root of the current storage overlay at a block with given `parent`. /// /// `parent` expects a SCALE encoded hash. diff --git a/substrate/primitives/io/src/lib.rs b/substrate/primitives/io/src/lib.rs index c7da89ab72..e4787dade0 100644 --- a/substrate/primitives/io/src/lib.rs +++ b/substrate/primitives/io/src/lib.rs @@ -117,6 +117,11 @@ pub trait Storage { Externalities::clear_prefix(*self, prefix) } + /// Append to storage item (assumes it is in "Vec" format). + fn append(&mut self, key: &[u8], value: Vec) { + self.storage_append(key.to_vec(), value); + } + /// "Commit" all existing operations and compute the resulting storage root. /// /// The hashing algorithm is defined by the `Block`. diff --git a/substrate/primitives/state-machine/src/basic.rs b/substrate/primitives/state-machine/src/basic.rs index 54554a0890..a5d573f514 100644 --- a/substrate/primitives/state-machine/src/basic.rs +++ b/substrate/primitives/state-machine/src/basic.rs @@ -257,6 +257,16 @@ impl Externalities for BasicExternalities { } } + fn storage_append( + &mut self, + key: Vec, + value: Vec, + ) { + let previous = self.storage(&key).unwrap_or_default(); + let new = crate::ext::append_to_storage(previous, value).expect("Failed to append to storage"); + self.place_storage(key.clone(), Some(new)); + } + fn chain_id(&self) -> u64 { 42 } fn storage_root(&mut self) -> Vec { diff --git a/substrate/primitives/state-machine/src/ext.rs b/substrate/primitives/state-machine/src/ext.rs index c92b9e894a..a92a10bf0e 100644 --- a/substrate/primitives/state-machine/src/ext.rs +++ b/substrate/primitives/state-machine/src/ext.rs @@ -30,7 +30,7 @@ use sp_core::{ }; use sp_trie::{trie_types::Layout, empty_child_trie_root}; use sp_externalities::{Extensions, Extension}; -use codec::{Decode, Encode}; +use codec::{Compact, Decode, Encode}; use std::{error, fmt, any::{Any, TypeId}}; use log::{warn, trace}; @@ -415,6 +415,26 @@ where }); } + fn storage_append( + &mut self, + key: Vec, + value: Vec, + ) { + let _guard = sp_panic_handler::AbortGuard::force_abort(); + self.mark_dirty(); + + let current_value = self.overlay + .value_mut(&key) + .map(|v| v.take()) + .unwrap_or_else(|| self.backend.storage(&key).expect(EXT_NOT_ALLOWED_TO_FAIL)) + .unwrap_or_default(); + + self.overlay.set_storage( + key, + Some(append_to_storage(current_value, value).expect(EXT_NOT_ALLOWED_TO_FAIL)), + ); + } + fn chain_id(&self) -> u64 { 42 } @@ -551,6 +571,67 @@ where } } +fn extract_length_data(data: &[u8]) -> Result<(u32, usize, usize), &'static str> { + use codec::CompactLen; + + let len = u32::from( + Compact::::decode(&mut &data[..]) + .map_err(|_| "Incorrect updated item encoding")? + ); + let new_len = len + .checked_add(1) + .ok_or_else(|| "New vec length greater than `u32::max_value()`.")?; + + let encoded_len = Compact::::compact_len(&len); + let encoded_new_len = Compact::::compact_len(&new_len); + + Ok((new_len, encoded_len, encoded_new_len)) +} + +pub fn append_to_storage( + mut self_encoded: Vec, + value: Vec, +) -> Result, &'static str> { + // No data present, just encode the given input data. + if self_encoded.is_empty() { + Compact::from(1u32).encode_to(&mut self_encoded); + self_encoded.extend(value); + return Ok(self_encoded); + } + + let (new_len, encoded_len, encoded_new_len) = extract_length_data(&self_encoded)?; + + let replace_len = |dest: &mut Vec| { + Compact(new_len).using_encoded(|e| { + dest[..encoded_new_len].copy_from_slice(e); + }) + }; + + let append_new_elems = |dest: &mut Vec| dest.extend(&value[..]); + + // If old and new encoded len is equal, we don't need to copy the + // already encoded data. + if encoded_len == encoded_new_len { + replace_len(&mut self_encoded); + append_new_elems(&mut self_encoded); + + Ok(self_encoded) + } else { + let size = encoded_new_len + self_encoded.len() - encoded_len; + + let mut res = Vec::with_capacity(size + value.len()); + unsafe { res.set_len(size); } + + // Insert the new encoded len, copy the already encoded data and + // add the new element. + replace_len(&mut res); + res[encoded_new_len..size].copy_from_slice(&self_encoded[encoded_len..]); + append_new_elems(&mut res); + + Ok(res) + } +} + impl<'a, H, B, N> sp_externalities::ExtensionStore for Ext<'a, H, N, B> where H: Hasher, diff --git a/substrate/primitives/state-machine/src/overlayed_changes.rs b/substrate/primitives/state-machine/src/overlayed_changes.rs index cf7b582843..f5b3c88ac8 100644 --- a/substrate/primitives/state-machine/src/overlayed_changes.rs +++ b/substrate/primitives/state-machine/src/overlayed_changes.rs @@ -223,6 +223,13 @@ impl OverlayedChanges { }) } + /// Returns mutable reference to either commited or propsective value. + pub fn value_mut(&mut self, key: &[u8]) -> Option<&mut Option> { + // not using map because of double borrow inside closure + if let Some(entry) = self.prospective.top.get_mut(key) { return Some(&mut entry.value) } + return self.committed.top.get_mut(key).map(|entry| &mut entry.value); + } + /// Returns a double-Option: None if the key is unknown (i.e. and the query should be referred /// to the backend); Some(None) if the key has been deleted. Some(Some(...)) for a key whose /// value has been set.