diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7d1debb2fc09b..d865c6d13068d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -113,9 +113,12 @@ cargo-check-benches: <<: *docker-env script: - BUILD_DUMMY_WASM_BINARY=1 time cargo +nightly check --benches --all + - cd ./primitives/state-machine/fuzz + - time cargo check - sccache -s + cargo-check-subkey: stage: test <<: *docker-env diff --git a/Cargo.lock b/Cargo.lock index cc63cf57f03e5..e3523fd09ed48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3590,6 +3590,7 @@ dependencies = [ "pwasm-utils 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.103 (registry+https://github.com/rust-lang/crates.io-index)", "sp-core 2.0.0", + "sp-historical-data 2.0.0", "sp-io 2.0.0", "sp-runtime 2.0.0", "sp-sandbox 2.0.0", @@ -4535,6 +4536,7 @@ dependencies = [ "rand_chacha 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", "rand_hc 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rand_pcg 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -4642,6 +4644,14 @@ dependencies = [ "rand_core 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "rand_pcg" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "rand_xorshift" version = "0.1.1" @@ -6255,6 +6265,14 @@ dependencies = [ "sp-std 2.0.0", ] +[[package]] +name = "sp-historical-data" +version = "2.0.0" +dependencies = [ + "smallvec 0.6.13 (registry+https://github.com/rust-lang/crates.io-index)", + "sp-std 2.0.0", +] + [[package]] name = "sp-inherents" version = "2.0.0" @@ -6445,6 +6463,7 @@ dependencies = [ name = "sp-state-machine" version = "2.0.0" dependencies = [ + "criterion 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", "hash-db 0.15.2 (registry+https://github.com/rust-lang/crates.io-index)", "hex-literal 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", @@ -6454,6 +6473,7 @@ dependencies = [ "rand 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)", "sp-core 2.0.0", "sp-externalities 2.0.0", + "sp-historical-data 2.0.0", "sp-panic-handler 2.0.0", "sp-trie 2.0.0", "trie-db 0.16.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -6746,6 +6766,7 @@ dependencies = [ "sp-consensus-aura 2.0.0", "sp-consensus-babe 2.0.0", "sp-core 2.0.0", + "sp-historical-data 2.0.0", "sp-inherents 2.0.0", "sp-io 2.0.0", "sp-keyring 2.0.0", @@ -8424,6 +8445,7 @@ dependencies = [ "checksum rand_os 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "7b75f676a1e053fc562eafbb47838d67c84801e38fc1ba459e8f180deabd5071" "checksum rand_os 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "a788ae3edb696cfcba1c19bfd388cc4b8c21f8a408432b199c072825084da58a" "checksum rand_pcg 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "abf9b09b01790cfe0364f52bf32995ea3c39f4d2dd011eac241d2914146d0b44" +"checksum rand_pcg 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "16abd0c1b639e9eb4d7c50c0b8100b0d0f849be2349829c740fe8e6eb4816429" "checksum rand_xorshift 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "cbf7e9e623549b0e21f6e97cf8ecf247c1a8fd2e8a992ae265314300b2455d5c" "checksum rand_xoshiro 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "03b418169fb9c46533f326efd6eed2576699c44ca92d3052a066214a8d828929" "checksum rand_xoshiro 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "0e18c91676f670f6f0312764c759405f13afb98d5d73819840cf72a518487bff" diff --git a/Cargo.toml b/Cargo.toml index 84f412851556d..38554c3b73af4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -124,6 +124,7 @@ members = [ "primitives/api/proc-macro", "primitives/api/test", "primitives/arithmetic", + "primitives/historical-data", "primitives/io", "primitives/runtime", "primitives/sandbox", diff --git a/frame/contracts/Cargo.toml b/frame/contracts/Cargo.toml index b5bd460dee14f..c9ed9f8f46cb1 100644 --- a/frame/contracts/Cargo.toml +++ b/frame/contracts/Cargo.toml @@ -14,6 +14,7 @@ primitives = { package = "sp-core", path = "../../primitives/core", default-fea sp-runtime = { path = "../../primitives/runtime", default-features = false } sp-io = { path = "../../primitives/io", default-features = false } sp-std = { path = "../../primitives/std", default-features = false } +sp-historical-data = { path = "../../primitives/historical-data", default-features = false } sandbox = { package = "sp-sandbox", path = "../../primitives/sandbox", default-features = false } support = { package = "frame-support", path = "../support", default-features = false } system = { package = "frame-system", path = "../system", default-features = false } @@ -35,6 +36,7 @@ std = [ "sp-runtime/std", "sp-io/std", "sp-std/std", + "sp-historical-data/std", "sandbox/std", "support/std", "system/std", diff --git a/frame/contracts/src/account_db.rs b/frame/contracts/src/account_db.rs deleted file mode 100644 index 2971f46f375c2..0000000000000 --- a/frame/contracts/src/account_db.rs +++ /dev/null @@ -1,391 +0,0 @@ -// Copyright 2018-2019 Parity Technologies (UK) Ltd. -// This file is part of Substrate. - -// Substrate is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Substrate is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Substrate. If not, see . - -//! Auxiliaries to help with managing partial changes to accounts state. - -use super::{ - AliveContractInfo, BalanceOf, CodeHash, ContractInfo, ContractInfoOf, Trait, TrieId, - TrieIdGenerator, -}; -use crate::exec::StorageKey; -use sp_std::cell::RefCell; -use sp_std::collections::btree_map::{BTreeMap, Entry}; -use sp_std::prelude::*; -use sp_io::hashing::blake2_256; -use sp_runtime::traits::{Bounded, Zero}; -use support::traits::{Currency, Get, Imbalance, SignedImbalance, UpdateBalanceOutcome}; -use support::{storage::child, StorageMap}; -use system; - -// Note: we don't provide Option because we can't create -// the trie_id in the overlay, thus we provide an overlay on the fields -// specifically. -pub struct ChangeEntry { - /// If Some(_), then the account balance is modified to the value. If None and `reset` is false, - /// the balance unmodified. If None and `reset` is true, the balance is reset to 0. - balance: Option>, - /// If Some(_), then a contract is instantiated with the code hash. If None and `reset` is false, - /// then the contract code is unmodified. If None and `reset` is true, the contract is deleted. - code_hash: Option>, - /// If Some(_), then the rent allowance is set to the value. If None and `reset` is false, then - /// the rent allowance is unmodified. If None and `reset` is true, the contract is deleted. - rent_allowance: Option>, - storage: BTreeMap>>, - /// If true, indicates that the existing contract and all its storage entries should be removed - /// and replaced with the fields on this change entry. Otherwise, the fields on this change - /// entry are updates merged into the existing contract info and storage. - reset: bool, -} - -impl ChangeEntry { - fn balance(&self) -> Option> { - self.balance.or_else(|| { - if self.reset { - Some(>::zero()) - } else { - None - } - }) - } - - fn code_hash(&self) -> Option>> { - if self.reset { - Some(self.code_hash) - } else { - self.code_hash.map(Some) - } - } - - fn rent_allowance(&self) -> Option>> { - if self.reset { - Some(self.rent_allowance) - } else { - self.rent_allowance.map(Some) - } - } - - fn storage(&self, location: &StorageKey) -> Option>> { - let value = self.storage.get(location).cloned(); - if self.reset { - Some(value.unwrap_or(None)) - } else { - value - } - } -} - -// Cannot derive(Default) since it erroneously bounds T by Default. -impl Default for ChangeEntry { - fn default() -> Self { - ChangeEntry { - rent_allowance: Default::default(), - balance: Default::default(), - code_hash: Default::default(), - storage: Default::default(), - reset: false, - } - } -} - -pub type ChangeSet = BTreeMap<::AccountId, ChangeEntry>; - -pub trait AccountDb { - /// Account is used when overlayed otherwise trie_id must be provided. - /// This is for performance reason. - /// - /// Trie id is None iff account doesn't have an associated trie id in >. - /// Because DirectAccountDb bypass the lookup for this association. - fn get_storage(&self, account: &T::AccountId, trie_id: Option<&TrieId>, location: &StorageKey) -> Option>; - /// If account has an alive contract then return the code hash associated. - fn get_code_hash(&self, account: &T::AccountId) -> Option>; - /// If account has an alive contract then return the rent allowance associated. - fn get_rent_allowance(&self, account: &T::AccountId) -> Option>; - /// Returns false iff account has no alive contract nor tombstone. - fn contract_exists(&self, account: &T::AccountId) -> bool; - fn get_balance(&self, account: &T::AccountId) -> BalanceOf; - - fn commit(&mut self, change_set: ChangeSet); -} - -pub struct DirectAccountDb; -impl AccountDb for DirectAccountDb { - fn get_storage( - &self, - _account: &T::AccountId, - trie_id: Option<&TrieId>, - location: &StorageKey - ) -> Option> { - trie_id.and_then(|id| child::get_raw(id, &blake2_256(location))) - } - fn get_code_hash(&self, account: &T::AccountId) -> Option> { - >::get(account).and_then(|i| i.as_alive().map(|i| i.code_hash)) - } - fn get_rent_allowance(&self, account: &T::AccountId) -> Option> { - >::get(account).and_then(|i| i.as_alive().map(|i| i.rent_allowance)) - } - fn contract_exists(&self, account: &T::AccountId) -> bool { - >::exists(account) - } - fn get_balance(&self, account: &T::AccountId) -> BalanceOf { - T::Currency::free_balance(account) - } - fn commit(&mut self, s: ChangeSet) { - let mut total_imbalance = SignedImbalance::zero(); - for (address, changed) in s.into_iter() { - if let Some(balance) = changed.balance() { - let (imbalance, outcome) = T::Currency::make_free_balance_be(&address, balance); - total_imbalance = total_imbalance.merge(imbalance); - if let UpdateBalanceOutcome::AccountKilled = outcome { - // Account killed. This will ultimately lead to calling `OnFreeBalanceZero` callback - // which will make removal of CodeHashOf and AccountStorage for this account. - // In order to avoid writing over the deleted properties we `continue` here. - continue; - } - } - - if changed.code_hash().is_some() - || changed.rent_allowance().is_some() - || !changed.storage.is_empty() - || changed.reset - { - let old_info = match >::get(&address) { - Some(ContractInfo::Alive(alive)) => Some(alive), - None => None, - // Cannot commit changes to tombstone contract - Some(ContractInfo::Tombstone(_)) => continue, - }; - - let mut new_info = match (changed.reset, old_info.clone(), changed.code_hash) { - // Existing contract is being modified. - (false, Some(info), _) => info, - // Existing contract is being removed. - (true, Some(info), None) => { - child::kill_storage(&info.trie_id); - >::remove(&address); - continue; - } - // Existing contract is being replaced by a new one. - (true, Some(info), Some(code_hash)) => { - child::kill_storage(&info.trie_id); - AliveContractInfo:: { - code_hash, - storage_size: T::StorageSizeOffset::get(), - trie_id: ::TrieIdGenerator::trie_id(&address), - deduct_block: >::block_number(), - rent_allowance: >::max_value(), - last_write: None, - } - } - // New contract is being instantiated. - (_, None, Some(code_hash)) => { - AliveContractInfo:: { - code_hash, - storage_size: T::StorageSizeOffset::get(), - trie_id: ::TrieIdGenerator::trie_id(&address), - deduct_block: >::block_number(), - rent_allowance: >::max_value(), - last_write: None, - } - } - // There is no existing at the address nor a new one to be instantiated. - (_, None, None) => continue, - }; - - if let Some(rent_allowance) = changed.rent_allowance { - new_info.rent_allowance = rent_allowance; - } - - if let Some(code_hash) = changed.code_hash { - new_info.code_hash = code_hash; - } - - if !changed.storage.is_empty() { - new_info.last_write = Some(>::block_number()); - } - - for (k, v) in changed.storage.into_iter() { - if let Some(value) = child::get_raw(&new_info.trie_id[..], &blake2_256(&k)) { - new_info.storage_size -= value.len() as u32; - } - if let Some(value) = v { - new_info.storage_size += value.len() as u32; - child::put_raw(&new_info.trie_id[..], &blake2_256(&k), &value[..]); - } else { - child::kill(&new_info.trie_id[..], &blake2_256(&k)); - } - } - - if old_info - .map(|old_info| old_info != new_info) - .unwrap_or(true) - { - >::insert(&address, ContractInfo::Alive(new_info)); - } - } - } - - match total_imbalance { - // If we've detected a positive imbalance as a result of our contract-level machinations - // then it's indicative of a buggy contracts system. - // Panicking is far from ideal as it opens up a DoS attack on block validators, however - // it's a less bad option than allowing arbitrary value to be created. - SignedImbalance::Positive(ref p) if !p.peek().is_zero() => - panic!("contract subsystem resulting in positive imbalance!"), - _ => {} - } - } -} -pub struct OverlayAccountDb<'a, T: Trait + 'a> { - local: RefCell>, - underlying: &'a dyn AccountDb, -} -impl<'a, T: Trait> OverlayAccountDb<'a, T> { - pub fn new(underlying: &'a dyn AccountDb) -> OverlayAccountDb<'a, T> { - OverlayAccountDb { - local: RefCell::new(ChangeSet::new()), - underlying, - } - } - - pub fn into_change_set(self) -> ChangeSet { - self.local.into_inner() - } - - pub fn set_storage( - &mut self, - account: &T::AccountId, - location: StorageKey, - value: Option>, - ) { - self.local.borrow_mut() - .entry(account.clone()) - .or_insert(Default::default()) - .storage - .insert(location, value); - } - - /// Return an error if contract already exists (either if it is alive or tombstone) - pub fn instantiate_contract( - &mut self, - account: &T::AccountId, - code_hash: CodeHash, - ) -> Result<(), &'static str> { - if self.contract_exists(account) { - return Err("Alive contract or tombstone already exists"); - } - - let mut local = self.local.borrow_mut(); - let contract = local.entry(account.clone()).or_insert_with(|| Default::default()); - - contract.code_hash = Some(code_hash); - contract.rent_allowance = Some(>::max_value()); - - Ok(()) - } - - /// Mark a contract as deleted. - pub fn destroy_contract(&mut self, account: &T::AccountId) { - let mut local = self.local.borrow_mut(); - local.insert( - account.clone(), - ChangeEntry { - reset: true, - ..Default::default() - } - ); - } - - /// Assume contract exists - pub fn set_rent_allowance(&mut self, account: &T::AccountId, rent_allowance: BalanceOf) { - self.local - .borrow_mut() - .entry(account.clone()) - .or_insert(Default::default()) - .rent_allowance = Some(rent_allowance); - } - pub fn set_balance(&mut self, account: &T::AccountId, balance: BalanceOf) { - self.local - .borrow_mut() - .entry(account.clone()) - .or_insert(Default::default()) - .balance = Some(balance); - } -} - -impl<'a, T: Trait> AccountDb for OverlayAccountDb<'a, T> { - fn get_storage( - &self, - account: &T::AccountId, - trie_id: Option<&TrieId>, - location: &StorageKey - ) -> Option> { - self.local - .borrow() - .get(account) - .and_then(|changes| changes.storage(location)) - .unwrap_or_else(|| self.underlying.get_storage(account, trie_id, location)) - } - fn get_code_hash(&self, account: &T::AccountId) -> Option> { - self.local - .borrow() - .get(account) - .and_then(|changes| changes.code_hash()) - .unwrap_or_else(|| self.underlying.get_code_hash(account)) - } - fn get_rent_allowance(&self, account: &T::AccountId) -> Option> { - self.local - .borrow() - .get(account) - .and_then(|changes| changes.rent_allowance()) - .unwrap_or_else(|| self.underlying.get_rent_allowance(account)) - } - fn contract_exists(&self, account: &T::AccountId) -> bool { - self.local - .borrow() - .get(account) - .and_then(|changes| changes.code_hash().map(|code_hash| code_hash.is_some())) - .unwrap_or_else(|| self.underlying.contract_exists(account)) - } - fn get_balance(&self, account: &T::AccountId) -> BalanceOf { - self.local - .borrow() - .get(account) - .and_then(|changes| changes.balance()) - .unwrap_or_else(|| self.underlying.get_balance(account)) - } - fn commit(&mut self, s: ChangeSet) { - let mut local = self.local.borrow_mut(); - - for (address, changed) in s.into_iter() { - match local.entry(address) { - Entry::Occupied(e) => { - let mut value = e.into_mut(); - if changed.reset { - *value = changed; - } else { - value.balance = changed.balance.or(value.balance); - value.code_hash = changed.code_hash.or(value.code_hash); - value.rent_allowance = changed.rent_allowance.or(value.rent_allowance); - value.storage.extend(changed.storage.into_iter()); - } - } - Entry::Vacant(e) => { - e.insert(changed); - } - } - } - } -} diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 9243d9f8c13bf..80edd5d300550 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -15,16 +15,16 @@ // along with Substrate. If not, see . use super::{CodeHash, Config, ContractAddressFor, Event, RawEvent, Trait, - TrieId, BalanceOf, ContractInfo}; -use crate::account_db::{AccountDb, DirectAccountDb, OverlayAccountDb}; + TrieId, BalanceOf, ContractInfo, TrieIdGenerator}; use crate::gas::{Gas, GasMeter, Token, approx_gas_for_balance}; use crate::rent; +use crate::storage; use sp_std::prelude::*; -use sp_runtime::traits::{Bounded, CheckedAdd, CheckedSub, Zero}; +use sp_runtime::traits::{Bounded, Zero}; use support::{ storage::unhashed, - traits::{WithdrawReason, Currency, Time, Randomness}, + traits::{ExistenceRequirement, Currency, Time, Randomness}, }; pub type AccountIdOf = ::AccountId; @@ -130,14 +130,14 @@ pub trait Ext { /// Notes a call dispatch. fn note_dispatch_call(&mut self, call: CallOf); - /// Notes a restoration request. - fn note_restore_to( + /// Restores the given destination contract sacrificing the current one. + fn restore_to( &mut self, dest: AccountIdOf, code_hash: CodeHash, rent_allowance: BalanceOf, delta: Vec, - ); + ) -> Result<(), &'static str>; /// Returns a reference to the account id of the caller. fn caller(&self) -> &AccountIdOf; @@ -241,38 +241,18 @@ impl Token for ExecFeeToken { #[cfg_attr(any(feature = "std", test), derive(PartialEq, Eq, Clone))] #[derive(sp_runtime::RuntimeDebug)] pub enum DeferredAction { - DepositEvent { - /// A list of topics this event will be deposited with. - topics: Vec, - /// The event to deposit. - event: Event, - }, DispatchRuntimeCall { /// The account id of the contract who dispatched this call. origin: T::AccountId, /// The call to dispatch. call: ::Call, }, - RestoreTo { - /// The account id of the contract which is removed during the restoration and transfers - /// its storage to the restored contract. - donor: T::AccountId, - /// The account id of the restored contract. - dest: T::AccountId, - /// The code hash of the restored contract. - code_hash: CodeHash, - /// The initial rent allowance to set. - rent_allowance: BalanceOf, - /// The keys to delete upon restoration. - delta: Vec, - }, } pub struct ExecutionContext<'a, T: Trait + 'a, V, L> { pub parent: Option<&'a ExecutionContext<'a, T, V, L>>, pub self_account: T::AccountId, pub self_trie_id: Option, - pub overlay: OverlayAccountDb<'a, T>, pub depth: usize, pub deferred: Vec>, pub config: &'a Config, @@ -297,7 +277,6 @@ where parent: None, self_trie_id: None, self_account: origin, - overlay: OverlayAccountDb::::new(&DirectAccountDb), depth: 0, deferred: Vec::new(), config: &cfg, @@ -315,7 +294,6 @@ where parent: Some(self), self_trie_id: trie_id, self_account: dest, - overlay: OverlayAccountDb::new(&self.overlay), depth: self.depth + 1, deferred: Vec::new(), config: self.config, @@ -351,9 +329,6 @@ where }); } - // Assumption: pay_rent doesn't collide with overlay because - // pay_rent will be done on first call and dest contract and balance - // cannot be changed before the first call let contract_info = rent::pay_rent::(&dest); // Calls to dead contracts always fail. @@ -384,7 +359,7 @@ where // If code_hash is not none, then the destination account is a live contract, otherwise // it is a regular account since tombstone accounts have already been rejected. - match nested.overlay.get_code_hash(&dest) { + match storage::code_hash::(&dest) { Some(dest_code_hash) => { let executable = try_or_exec_error!( nested.loader.load_main(&dest_code_hash), @@ -398,8 +373,11 @@ where gas_meter, )?; + // TODO: Handle the case when after returning from the nested call the balance + // is below existential deposit. + // Destroy contract if insufficient remaining balance. - if nested.overlay.get_balance(&dest) < nested.config.existential_deposit { + if T::Currency::free_balance(&dest) < nested.config.existential_deposit { let parent = nested.parent .expect("a nested execution context must have a parent; qed"); if parent.is_live(&dest) { @@ -408,8 +386,12 @@ where buffer: output.data, }); } - - nested.overlay.destroy_contract(&dest); + storage::destroy_contract::( + &dest, + nested.self_trie_id.as_ref().expect( + "a nested exexcution context must have trie id assgined; qed" + ) + ); } Ok(output) @@ -451,11 +433,20 @@ where ); // TrieId has not been generated yet and storage is empty since contract is new. - let dest_trie_id = None; + // + // Generate it now. + let dest_trie_id = ::TrieIdGenerator::trie_id(&dest); - let output = self.with_nested_context(dest.clone(), dest_trie_id, |nested| { + let output = self.with_nested_context(dest.clone(), Some(dest_trie_id), |nested| { try_or_exec_error!( - nested.overlay.instantiate_contract(&dest, code_hash.clone()), + storage::place_contract::( + &dest, + nested + .self_trie_id + .clone() + .expect("the nested context always has to have self_trie_id"), + code_hash.clone() + ), input_data ); @@ -485,8 +476,10 @@ where gas_meter, )?; + // TODO: Handle the case when after returning from the nested call the balance + // is below existential deposit. // Error out if insufficient remaining balance. - if nested.overlay.get_balance(&dest) < nested.config.existential_deposit { + if T::Currency::free_balance(&dest) < nested.config.existential_deposit { return Err(ExecError { reason: "insufficient remaining balance", buffer: output.data, @@ -494,10 +487,7 @@ where } // Deposit an instantiation event. - nested.deferred.push(DeferredAction::DepositEvent { - event: RawEvent::Instantiated(caller.clone(), dest.clone()), - topics: Vec::new(), - }); + deposit_event::(vec![], RawEvent::Instantiated(caller.clone(), dest.clone())); Ok(output) })?; @@ -519,21 +509,26 @@ where } } + /// Execute the given closure within a nested execution context. fn with_nested_context(&mut self, dest: T::AccountId, trie_id: Option, func: F) -> ExecResult where F: FnOnce(&mut ExecutionContext) -> ExecResult { - let (output, change_set, deferred) = { + let (output, deferred) = { let mut nested = self.nested(dest, trie_id); - let output = func(&mut nested)?; - (output, nested.overlay.into_change_set(), nested.deferred) + let output = crate::util::with_transaction(|| { + let output = func(&mut nested); + match output { + Ok(ref rv) if rv.is_success() => (output, crate::util::TxOutcome::Commit), + _ => (output, crate::util::TxOutcome::Discard), + } + })?; + (output, nested.deferred) }; - if output.is_success() { - self.overlay.commit(change_set); + // self.overlay.commit(change_set); self.deferred.extend(deferred); } - Ok(output) } @@ -607,7 +602,7 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( use self::TransferCause::*; use self::TransferFeeKind::*; - let to_balance = ctx.overlay.get_balance(dest); + let to_balance = T::Currency::free_balance(dest); // `would_create` indicates whether the account will be created if this transfer gets executed. // This flag is orthogonal to `cause. @@ -640,30 +635,8 @@ fn transfer<'a, T: Trait, V: Vm, L: Loader>( return Err("not enough gas to pay transfer fee"); } - // We allow balance to go below the existential deposit here: - let from_balance = ctx.overlay.get_balance(transactor); - let new_from_balance = match from_balance.checked_sub(&value) { - Some(b) => b, - None => return Err("balance too low to send value"), - }; - if would_create && value < ctx.config.existential_deposit { - return Err("value too low to create account"); - } - T::Currency::ensure_can_withdraw(transactor, value, WithdrawReason::Transfer.into(), new_from_balance)?; - - let new_to_balance = match to_balance.checked_add(&value) { - Some(b) => b, - None => return Err("destination balance too high to receive value"), - }; - - if transactor != dest { - ctx.overlay.set_balance(transactor, new_from_balance); - ctx.overlay.set_balance(dest, new_to_balance); - ctx.deferred.push(DeferredAction::DepositEvent { - event: RawEvent::Transfer(transactor.clone(), dest.clone(), value), - topics: Vec::new(), - }); - } + // Make the transfer. + T::Currency::transfer(transactor, dest, value, ExistenceRequirement::AllowDeath)?; Ok(()) } @@ -685,19 +658,30 @@ where type T = T; fn get_storage(&self, key: &StorageKey) -> Option> { - self.ctx.overlay.get_storage(&self.ctx.self_account, self.ctx.self_trie_id.as_ref(), key) + match self.ctx.self_trie_id { + Some(ref trie_id) => storage::read_contract_storage(trie_id, key), + // TODO: My current understanding is that `self.ctx.self_trie_id` can be `None` only + // for the top-level account, i.e. EOA. As such, it cannot issue any reads to the + // storage, because it has no. Furthermore, EOA cannot have contract storage. + None => unimplemented!(), + } } fn set_storage(&mut self, key: StorageKey, value: Option>) -> Result<(), &'static str> { if let Some(ref value) = value { + // TODO: Move this earlier to runtime.rs? This way the buffer won't get loaded. if self.max_value_size() < value.len() as u32 { return Err("value size exceeds maximum"); } } - self.ctx - .overlay - .set_storage(&self.ctx.self_account, key, value); + match self.ctx.self_trie_id { + Some(ref trie_id) => storage::write_contract_storage::(&self.ctx.self_account, trie_id, &key, value), + // TODO: My current understanding is that `self.ctx.self_trie_id` can be `None` only + // for the top-level account, i.e. EOA. As such, it cannot issue any reads to the + // storage, because it has no. Furthermore, EOA cannot have contract storage. + None => unimplemented!(), + } Ok(()) } @@ -728,20 +712,20 @@ where }); } - fn note_restore_to( + fn restore_to( &mut self, dest: AccountIdOf, code_hash: CodeHash, rent_allowance: BalanceOf, delta: Vec, - ) { - self.ctx.deferred.push(DeferredAction::RestoreTo { - donor: self.ctx.self_account.clone(), + ) -> Result<(), &'static str> { + crate::rent::restore_to::( + self.ctx.self_account.clone(), dest, code_hash, rent_allowance, delta, - }); + ) } fn address(&self) -> &T::AccountId { @@ -753,7 +737,7 @@ where } fn balance(&self) -> BalanceOf { - self.ctx.overlay.get_balance(&self.ctx.self_account) + T::Currency::free_balance(&self.ctx.self_account) } fn value_transferred(&self) -> BalanceOf { @@ -773,18 +757,15 @@ where } fn deposit_event(&mut self, topics: Vec, data: Vec) { - self.ctx.deferred.push(DeferredAction::DepositEvent { - topics, - event: RawEvent::Contract(self.ctx.self_account.clone(), data), - }); + deposit_event::(topics, RawEvent::Contract(self.ctx.self_account.clone(), data)); } fn set_rent_allowance(&mut self, rent_allowance: BalanceOf) { - self.ctx.overlay.set_rent_allowance(&self.ctx.self_account, rent_allowance) + storage::set_rent_allowance::(&self.ctx.self_account, rent_allowance); } fn rent_allowance(&self) -> BalanceOf { - self.ctx.overlay.get_rent_allowance(&self.ctx.self_account) + storage::rent_allowance::(&self.ctx.self_account) .unwrap_or(>::max_value()) // Must never be triggered actually } @@ -799,6 +780,16 @@ where } } +fn deposit_event( + topics: Vec, + event: Event, +) { + >::deposit_event_indexed( + &*topics, + ::Event::from(event).into(), + ) +} + /// These tests exercise the executive layer. /// /// In these tests the VM/loader are mocked. Instead of dealing with wasm bytecode they use simple closures. @@ -812,33 +803,31 @@ where #[cfg(test)] mod tests { use super::{ - BalanceOf, ExecFeeToken, ExecutionContext, Ext, Loader, TransferFeeKind, TransferFeeToken, - Vm, ExecResult, RawEvent, DeferredAction, + BalanceOf, Event, ExecFeeToken, ExecResult, ExecutionContext, Ext, Loader, + RawEvent, TransferFeeKind, TransferFeeToken, Vm, }; use crate::{ - account_db::AccountDb, gas::GasMeter, tests::{ExtBuilder, Test}, - exec::{ExecReturnValue, ExecError, STATUS_SUCCESS}, CodeHash, Config, + exec::{ExecError, ExecReturnValue, STATUS_SUCCESS}, + gas::GasMeter, + storage, + tests::{get_balance, place_contract, set_balance, ExtBuilder, MetaEvent, Test}, + CodeHash, Config, }; - use std::{cell::RefCell, rc::Rc, collections::HashMap, marker::PhantomData}; use assert_matches::assert_matches; + use std::{cell::RefCell, collections::HashMap, marker::PhantomData, rc::Rc}; const ALICE: u64 = 1; const BOB: u64 = 2; const CHARLIE: u64 = 3; - impl<'a, T, V, L> ExecutionContext<'a, T, V, L> - where T: crate::Trait - { - fn events(&self) -> Vec> { - self.deferred - .iter() - .filter(|action| match *action { - DeferredAction::DepositEvent { .. } => true, - _ => false, - }) - .cloned() - .collect() - } + fn events() -> Vec> { + >::events() + .into_iter() + .filter_map(|meta| match meta.event { + MetaEvent::contract(contract_event) => Some(contract_event), + _ => None, + }) + .collect() } struct MockCtx<'a> { @@ -947,7 +936,7 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.instantiate_contract(&BOB, exec_ch).unwrap(); + place_contract(&BOB, exec_ch); assert_matches!( ctx.call(BOB, value, &mut gas_meter, data), @@ -969,8 +958,8 @@ mod tests { let loader = MockLoader::empty(); let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.set_balance(&origin, 100); - ctx.overlay.set_balance(&dest, 0); + set_balance(&origin, 100); + set_balance(&dest, 0); let mut gas_meter = GasMeter::::with_limit(1000, 1); @@ -990,7 +979,7 @@ mod tests { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.set_balance(&origin, 100); + set_balance(&origin, 100); let mut gas_meter = GasMeter::::with_limit(1000, 1); @@ -1015,8 +1004,8 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.set_balance(&origin, 100); - ctx.overlay.set_balance(&dest, 0); + set_balance(&origin, 100); + set_balance(&dest, 0); let output = ctx.call( dest, @@ -1026,15 +1015,15 @@ mod tests { ).unwrap(); assert!(output.is_success()); - assert_eq!(ctx.overlay.get_balance(&origin), 45); - assert_eq!(ctx.overlay.get_balance(&dest), 55); + assert_eq!(get_balance(&origin), 45); + assert_eq!(get_balance(&dest), 55); }); } #[test] fn changes_are_reverted_on_failing_call() { - // This test verifies that a contract is able to transfer - // some funds to another account. + // This test verifies that changes are reverted on a call which fails (or equally, returns + // a non-zero status code). let origin = ALICE; let dest = BOB; @@ -1047,9 +1036,9 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.instantiate_contract(&BOB, return_ch).unwrap(); - ctx.overlay.set_balance(&origin, 100); - ctx.overlay.set_balance(&dest, 0); + place_contract(&BOB, return_ch); + set_balance(&origin, 100); + set_balance(&dest, 0); let output = ctx.call( dest, @@ -1059,8 +1048,8 @@ mod tests { ).unwrap(); assert!(!output.is_success()); - assert_eq!(ctx.overlay.get_balance(&origin), 100); - assert_eq!(ctx.overlay.get_balance(&dest), 0); + assert_eq!(get_balance(&origin), 100); + assert_eq!(get_balance(&dest), 0); }); } @@ -1077,8 +1066,8 @@ mod tests { let loader = MockLoader::empty(); let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.set_balance(&origin, 100); - ctx.overlay.set_balance(&dest, 0); + set_balance(&origin, 100); + set_balance(&dest, 0); let mut gas_meter = GasMeter::::with_limit(1000, 1); @@ -1103,8 +1092,8 @@ mod tests { let loader = MockLoader::empty(); let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.set_balance(&origin, 100); - ctx.overlay.set_balance(&dest, 15); + set_balance(&origin, 100); + set_balance(&dest, 15); let mut gas_meter = GasMeter::::with_limit(1000, 1); @@ -1132,8 +1121,8 @@ mod tests { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.set_balance(&origin, 100); - ctx.overlay.set_balance(&dest, 15); + set_balance(&origin, 100); + set_balance(&dest, 15); let mut gas_meter = GasMeter::::with_limit(1000, 1); @@ -1165,7 +1154,7 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.set_balance(&origin, 0); + set_balance(&origin, 0); let result = ctx.call( dest, @@ -1178,8 +1167,8 @@ mod tests { result, Err(ExecError { reason: "balance too low to send value", buffer: _ }) ); - assert_eq!(ctx.overlay.get_balance(&origin), 0); - assert_eq!(ctx.overlay.get_balance(&dest), 0); + assert_eq!(get_balance(&origin), 0); + assert_eq!(get_balance(&dest), 0); }); } @@ -1199,7 +1188,7 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.instantiate_contract(&BOB, return_ch).unwrap(); + place_contract(&BOB, return_ch); let result = ctx.call( dest, @@ -1230,7 +1219,7 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.instantiate_contract(&BOB, return_ch).unwrap(); + place_contract(&BOB, return_ch); let result = ctx.call( dest, @@ -1258,7 +1247,7 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.instantiate_contract(&BOB, input_data_ch).unwrap(); + place_contract(&BOB, input_data_ch); let result = ctx.call( BOB, @@ -1327,7 +1316,7 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.instantiate_contract(&BOB, recurse_ch).unwrap(); + place_contract(&BOB, recurse_ch); let result = ctx.call( BOB, @@ -1372,8 +1361,8 @@ mod tests { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(origin, &cfg, &vm, &loader); - ctx.overlay.instantiate_contract(&dest, bob_ch).unwrap(); - ctx.overlay.instantiate_contract(&CHARLIE, charlie_ch).unwrap(); + place_contract(&dest, bob_ch); + place_contract(&CHARLIE, charlie_ch); let result = ctx.call( dest, @@ -1413,8 +1402,8 @@ mod tests { ExtBuilder::default().build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.instantiate_contract(&BOB, bob_ch).unwrap(); - ctx.overlay.instantiate_contract(&CHARLIE, charlie_ch).unwrap(); + place_contract(&BOB, bob_ch); + place_contract(&CHARLIE, charlie_ch); let result = ctx.call( BOB, @@ -1462,7 +1451,7 @@ mod tests { ExtBuilder::default().existential_deposit(15).build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.set_balance(&ALICE, 1000); + set_balance(&ALICE, 1000); let instantiated_contract_address = assert_matches!( ctx.instantiate( @@ -1476,16 +1465,9 @@ mod tests { // Check that the newly created account has the expected code hash and // there are instantiation event. - assert_eq!(ctx.overlay.get_code_hash(&instantiated_contract_address).unwrap(), dummy_ch); - assert_eq!(&ctx.events(), &[ - DeferredAction::DepositEvent { - event: RawEvent::Transfer(ALICE, instantiated_contract_address, 100), - topics: Vec::new(), - }, - DeferredAction::DepositEvent { - event: RawEvent::Instantiated(ALICE, instantiated_contract_address), - topics: Vec::new(), - } + assert_eq!(storage::code_hash::(&instantiated_contract_address).unwrap(), dummy_ch); + assert_eq!(&events(), &[ + RawEvent::Instantiated(ALICE, instantiated_contract_address) ]); }); } @@ -1502,7 +1484,7 @@ mod tests { ExtBuilder::default().existential_deposit(15).build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.set_balance(&ALICE, 1000); + set_balance(&ALICE, 1000); let instantiated_contract_address = assert_matches!( ctx.instantiate( @@ -1515,8 +1497,8 @@ mod tests { ); // Check that the account has not been created. - assert!(ctx.overlay.get_code_hash(&instantiated_contract_address).is_none()); - assert!(ctx.events().is_empty()); + assert!(storage::code_hash::(&instantiated_contract_address).is_none()); + assert!(events().is_empty()); }); } @@ -1547,8 +1529,8 @@ mod tests { ExtBuilder::default().existential_deposit(15).build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.set_balance(&ALICE, 1000); - ctx.overlay.instantiate_contract(&BOB, instantiator_ch).unwrap(); + set_balance(&ALICE, 1000); + place_contract(&BOB, instantiator_ch); assert_matches!( ctx.call(BOB, 20, &mut GasMeter::::with_limit(1000, 1), vec![]), @@ -1559,20 +1541,9 @@ mod tests { // Check that the newly created account has the expected code hash and // there are instantiation event. - assert_eq!(ctx.overlay.get_code_hash(&instantiated_contract_address).unwrap(), dummy_ch); - assert_eq!(&ctx.events(), &[ - DeferredAction::DepositEvent { - event: RawEvent::Transfer(ALICE, BOB, 20), - topics: Vec::new(), - }, - DeferredAction::DepositEvent { - event: RawEvent::Transfer(BOB, instantiated_contract_address, 15), - topics: Vec::new(), - }, - DeferredAction::DepositEvent { - event: RawEvent::Instantiated(BOB, instantiated_contract_address), - topics: Vec::new(), - }, + assert_eq!(storage::code_hash::(&instantiated_contract_address).unwrap(), dummy_ch); + assert_eq!(&events(), &[ + RawEvent::Instantiated(BOB, instantiated_contract_address) ]); }); } @@ -1606,8 +1577,8 @@ mod tests { ExtBuilder::default().existential_deposit(15).build().execute_with(|| { let cfg = Config::preload(); let mut ctx = ExecutionContext::top_level(ALICE, &cfg, &vm, &loader); - ctx.overlay.set_balance(&ALICE, 1000); - ctx.overlay.instantiate_contract(&BOB, instantiator_ch).unwrap(); + set_balance(&ALICE, 1000); + place_contract(&BOB, instantiator_ch); assert_matches!( ctx.call(BOB, 20, &mut GasMeter::::with_limit(1000, 1), vec![]), @@ -1616,12 +1587,7 @@ mod tests { // The contract wasn't instantiated so we don't expect to see an instantiation // event here. - assert_eq!(&ctx.events(), &[ - DeferredAction::DepositEvent { - event: RawEvent::Transfer(ALICE, BOB, 20), - topics: Vec::new(), - }, - ]); + assert_eq!(&events(), &[]); }); } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index f342a36a7ab87..883da8223b8aa 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -90,17 +90,16 @@ #[macro_use] mod gas; - -mod account_db; +mod storage; mod exec; mod wasm; mod rent; +mod util; #[cfg(test)] mod tests; use crate::exec::ExecutionContext; -use crate::account_db::{AccountDb, DirectAccountDb}; use crate::wasm::{WasmLoader, WasmVm}; pub use crate::gas::{Gas, GasMeter}; @@ -111,7 +110,6 @@ use serde::{Serialize, Deserialize}; use primitives::crypto::UncheckedFrom; use sp_std::{prelude::*, marker::PhantomData, fmt::Debug}; use codec::{Codec, Encode, Decode}; -use sp_io::hashing::blake2_256; use sp_runtime::{ traits::{Hash, StaticLookup, Zero, MaybeSerializeDeserialize, Member, SignedExtension}, transaction_validity::{ @@ -244,6 +242,12 @@ where } } +impl From> for ContractInfo { + fn from(alive_info: AliveContractInfo) -> Self { + Self::Alive(alive_info) + } +} + /// Get a trie id (trie id must be unique and collision resistant depending upon its context). /// Note that it is different than encode because trie id should be collision resistant /// (being a proper unique identifier). @@ -686,12 +690,7 @@ impl Module { .get_alive() .ok_or(GetStorageError::IsTombstone)?; - let maybe_value = AccountDb::::get_storage( - &DirectAccountDb, - &address, - Some(&contract_info.trie_id), - &key, - ); + let maybe_value = storage::read_contract_storage(&contract_info.trie_id, &key); Ok(maybe_value) } } @@ -721,8 +720,13 @@ impl Module { let result = func(&mut ctx, &mut gas_meter); if result.as_ref().map(|output| output.is_success()).unwrap_or(false) { + // TODO: Audit this. + // + // We no longer need to commit now, since all changes are committed eagearly, but + // rollbacked on error. + // Commit all changes that made it thus far into the persistent storage. - DirectAccountDb.commit(ctx.overlay.into_change_set()); + // DirectAccountDb.commit(ctx.overlay.into_change_set()); } // Refund cost of the unused gas. @@ -735,13 +739,6 @@ impl Module { ctx.deferred.into_iter().for_each(|deferred| { use self::exec::DeferredAction::*; match deferred { - DepositEvent { - topics, - event, - } => >::deposit_event_indexed( - &*topics, - ::Event::from(event).into(), - ), DispatchRuntimeCall { origin: who, call, @@ -749,104 +746,19 @@ impl Module { let result = call.dispatch(RawOrigin::Signed(who.clone()).into()); Self::deposit_event(RawEvent::Dispatched(who, result.is_ok())); } - RestoreTo { - donor, - dest, - code_hash, - rent_allowance, - delta, - } => { - let _result = Self::restore_to(donor, dest, code_hash, rent_allowance, delta); - } } }); result } - - fn restore_to( - origin: T::AccountId, - dest: T::AccountId, - code_hash: CodeHash, - rent_allowance: BalanceOf, - delta: Vec - ) -> Result { - let mut origin_contract = >::get(&origin) - .and_then(|c| c.get_alive()) - .ok_or("Cannot restore from inexisting or tombstone contract")?; - - let current_block = >::block_number(); - - if origin_contract.last_write == Some(current_block) { - return Err("Origin TrieId written in the current block"); - } - - let dest_tombstone = >::get(&dest) - .and_then(|c| c.get_tombstone()) - .ok_or("Cannot restore to inexisting or alive contract")?; - - let last_write = if !delta.is_empty() { - Some(current_block) - } else { - origin_contract.last_write - }; - - let key_values_taken = delta.iter() - .filter_map(|key| { - child::get_raw(&origin_contract.trie_id, &blake2_256(key)).map(|value| { - child::kill(&origin_contract.trie_id, &blake2_256(key)); - (key, value) - }) - }) - .collect::>(); - - let tombstone = >::new( - // This operation is cheap enough because last_write (delta not included) - // is not this block as it has been checked earlier. - &sp_io::storage::child_root(&origin_contract.trie_id)[..], - code_hash, - ); - - if tombstone != dest_tombstone { - for (key, value) in key_values_taken { - child::put_raw(&origin_contract.trie_id, &blake2_256(key), &value); - } - - return Err("Tombstones don't match"); - } - - origin_contract.storage_size -= key_values_taken.iter() - .map(|(_, value)| value.len() as u32) - .sum::(); - - >::remove(&origin); - >::insert(&dest, ContractInfo::Alive(RawAliveContractInfo { - trie_id: origin_contract.trie_id, - storage_size: origin_contract.storage_size, - code_hash, - rent_allowance, - deduct_block: current_block, - last_write, - })); - - let origin_free_balance = T::Currency::free_balance(&origin); - T::Currency::make_free_balance_be(&origin, >::zero()); - T::Currency::deposit_creating(&dest, origin_free_balance); - - Ok(()) - } } decl_event! { pub enum Event where - Balance = BalanceOf, ::AccountId, ::Hash { - /// Transfer happened `from` to `to` with given `value` as part of a `call` or `instantiate`. - Transfer(AccountId, AccountId, Balance), - /// Contract deployed by address at the specified address. Instantiated(AccountId, AccountId), diff --git a/frame/contracts/src/rent.rs b/frame/contracts/src/rent.rs index cf96ee2c1b441..f398d74afba8b 100644 --- a/frame/contracts/src/rent.rs +++ b/frame/contracts/src/rent.rs @@ -14,9 +14,12 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -use crate::{BalanceOf, ContractInfo, ContractInfoOf, TombstoneContractInfo, Trait, AliveContractInfo}; +use crate::{BalanceOf, ContractInfo, ContractInfoOf, TombstoneContractInfo, Trait, AliveContractInfo, CodeHash, RawAliveContractInfo, exec}; +use sp_std::prelude::*; use sp_runtime::traits::{Bounded, CheckedDiv, CheckedMul, Saturating, Zero, SaturatedConversion}; +use sp_io::hashing::blake2_256; +use support::storage::child; use support::traits::{Currency, ExistenceRequirement, Get, WithdrawReason, OnUnbalanced}; use support::StorageMap; @@ -41,6 +44,9 @@ pub enum RentOutcome { /// Evict and optionally pay dues (or check account can pay them otherwise) at the current /// block number (modulo `handicap`, read on). /// +/// Returns `None` if the account has no contract info or it was removed along with its +/// `ContractInfo` due to dropping its balance below the subsistence threshold. +/// /// `pay_rent` gives an ability to pay or skip paying rent. /// `handicap` gives a way to check or pay the rent up to a moment in the past instead /// of current block. @@ -53,6 +59,7 @@ fn try_evict_or_and_pay_rent( ) -> (RentOutcome, Option>) { let contract_info = >::get(account); let contract = match contract_info { + // We don't collect rent from non-contract accounts and tombstones. None | Some(ContractInfo::Tombstone(_)) => return (RentOutcome::Exempted, contract_info), Some(ContractInfo::Alive(contract)) => contract, }; @@ -177,6 +184,9 @@ fn try_evict_or_and_pay_rent( /// Make account paying the rent for the current block number /// +/// Returns `None` if the account has no contract info or it was removed along with its +/// `ContractInfo` due to dropping its balance below the subsistence threshold. +/// /// NOTE: This function acts eagerly. pub fn pay_rent(account: &T::AccountId) -> Option> { try_evict_or_and_pay_rent::(account, Zero::zero(), true).1 @@ -192,3 +202,86 @@ pub fn pay_rent(account: &T::AccountId) -> Option> { pub fn try_evict(account: &T::AccountId, handicap: T::BlockNumber) -> RentOutcome { try_evict_or_and_pay_rent::(account, handicap, false).0 } + +/// Restores the destination account using the origin as prototype. +/// +/// The restoration will be performed iff: +/// - origin exists and is alive, +/// - the origin's storage is not written in the current block +/// - the restored account has tombstone +/// - the tombstone matches the hash of the origin storage root, and code hash. +/// +/// Upon succesful restoration, `origin` will be destroyed, all its funds are transferred to +/// the restored account. The restored account will inherit the last write block and its last +/// deduct block will be set to the current block. +pub fn restore_to( + origin: T::AccountId, + dest: T::AccountId, + code_hash: CodeHash, + rent_allowance: BalanceOf, + delta: Vec, +) -> Result<(), &'static str> { + let mut origin_contract = >::get(&origin) + .and_then(|c| c.get_alive()) + .ok_or("Cannot restore from inexisting or tombstone contract")?; + + let current_block = >::block_number(); + + if origin_contract.last_write == Some(current_block) { + return Err("Origin TrieId written in the current block"); + } + + let dest_tombstone = >::get(&dest) + .and_then(|c| c.get_tombstone()) + .ok_or("Cannot restore to inexisting or alive contract")?; + + let last_write = if !delta.is_empty() { + Some(current_block) + } else { + origin_contract.last_write + }; + + let key_values_taken = delta.iter() + .filter_map(|key| { + child::get_raw(&origin_contract.trie_id, &blake2_256(key)).map(|value| { + child::kill(&origin_contract.trie_id, &blake2_256(key)); + (key, value) + }) + }) + .collect::>(); + + let tombstone = >::new( + // This operation is cheap enough because last_write (delta not included) + // is not this block as it has been checked earlier. + &sp_io::storage::child_root(&origin_contract.trie_id)[..], + code_hash, + ); + + if tombstone != dest_tombstone { + for (key, value) in key_values_taken { + child::put_raw(&origin_contract.trie_id, &blake2_256(key), &value); + } + + return Err("Tombstones don't match"); + } + + origin_contract.storage_size -= key_values_taken.iter() + .map(|(_, value)| value.len() as u32) + .sum::(); + + >::remove(&origin); + >::insert(&dest, ContractInfo::Alive(RawAliveContractInfo { + trie_id: origin_contract.trie_id, + storage_size: origin_contract.storage_size, + code_hash, + rent_allowance, + deduct_block: current_block, + last_write, + })); + + let origin_free_balance = T::Currency::free_balance(&origin); + T::Currency::make_free_balance_be(&origin, >::zero()); + T::Currency::deposit_creating(&dest, origin_free_balance); + + Ok(()) +} diff --git a/frame/contracts/src/storage.rs b/frame/contracts/src/storage.rs new file mode 100644 index 0000000000000..42effd0375f9a --- /dev/null +++ b/frame/contracts/src/storage.rs @@ -0,0 +1,118 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +use crate::{ + exec::{AccountIdOf, StorageKey}, + AliveContractInfo, BalanceOf, CodeHash, ContractInfo, ContractInfoOf, Trait, TrieId, +}; +use sp_std::prelude::*; +use sp_io::hashing::blake2_256; +use sp_runtime::traits::Bounded; +use support::{storage::child, traits::Get, StorageMap}; + +pub fn read_contract_storage(trie_id: &TrieId, key: &StorageKey) -> Option> { + child::get_raw(trie_id, &blake2_256(key)) +} + +pub fn write_contract_storage( + account: &AccountIdOf, + trie_id: &TrieId, + key: &StorageKey, + value: Option>, +) { + let hashed_key = blake2_256(key); + + // We need to accurately track the size of the storage of the contract. + // + // For that we query the previous value and get its size. + let existing_size = child::get_raw(trie_id, &hashed_key) + .map(|v| v.len()) + .unwrap_or(0); + let new_size = match value { + Some(v) => { + child::put_raw(trie_id, &hashed_key, &v); + v.len() + } + None => { + child::kill(trie_id, &hashed_key); + 0 + } + }; + + >::mutate(account, |maybe_contract_info| { + match maybe_contract_info { + Some(ContractInfo::Alive(ref mut alive_info)) => { + alive_info.storage_size -= existing_size as u32; + alive_info.storage_size += new_size as u32; + alive_info.last_write = Some(>::block_number()); + } + _ => panic!(), // TODO: Justify this. + } + }); +} + +pub fn rent_allowance(account: &AccountIdOf) -> Option> { + >::get(account).and_then(|i| i.as_alive().map(|i| i.rent_allowance)) +} + +pub fn set_rent_allowance(account: &AccountIdOf, rent_allowance: BalanceOf) { + >::mutate(account, |maybe_contract_info| { + match maybe_contract_info { + Some(ContractInfo::Alive(ref mut alive_info)) => { + alive_info.rent_allowance = rent_allowance + } + _ => panic!(), // TODO: Justify this. + } + }) +} + +pub fn code_hash(account: &AccountIdOf) -> Option> { + >::get(account).and_then(|i| i.as_alive().map(|i| i.code_hash)) +} + +/// Creates a new contract descriptor in the storage with the given code hash at the given address. +/// +/// Returns `Err` if there is already a contract (or a tombstone) exists at the given address. +pub fn place_contract( + account: &AccountIdOf, + trie_id: TrieId, + ch: CodeHash, +) -> Result<(), &'static str> { + >::mutate(account, |maybe_contract_info| { + if maybe_contract_info.is_some() { + return Err("Alive contract or tombstone already exists"); + } + + *maybe_contract_info = Some( + AliveContractInfo:: { + code_hash: ch, + storage_size: T::StorageSizeOffset::get(), + trie_id, + deduct_block: >::block_number(), + rent_allowance: >::max_value(), + last_write: None, + } + .into(), + ); + + Ok(()) + }) +} + +pub fn destroy_contract(address: &AccountIdOf, trie_id: &TrieId) { + >::remove(address); + child::kill_storage(trie_id); +} diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests/mod.rs similarity index 95% rename from frame/contracts/src/tests.rs rename to frame/contracts/src/tests/mod.rs index 153a70d54b184..a776db7853b2a 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests/mod.rs @@ -22,7 +22,7 @@ use crate::{ BalanceOf, ComputeDispatchFee, ContractAddressFor, ContractInfo, ContractInfoOf, GenesisConfig, Module, RawAliveContractInfo, RawEvent, Trait, TrieId, TrieIdFromParentCounter, Schedule, - TrieIdGenerator, CheckBlockGasLimit, account_db::{AccountDb, DirectAccountDb, OverlayAccountDb}, + TrieIdGenerator, CheckBlockGasLimit, CodeHash, storage, }; use assert_matches::assert_matches; use hex_literal::*; @@ -296,6 +296,40 @@ fn compile_module(wabt_module: &str) Ok((wasm, code_hash)) } +pub fn set_balance(who: &u64, amount: u64) { + Balances::deposit_creating(who, amount); +} + +pub fn get_balance(who: &u64) -> u64 { + Balances::free_balance(who) +} + +pub fn place_contract(address: &u64, code_hash: CodeHash) { + use crate::TrieIdGenerator; + let trie_id = ::TrieIdGenerator::trie_id(address); + storage::place_contract::(&address, trie_id, code_hash).unwrap() +} + +pub fn get_storage(who: &u64, key: [u8; 32]) -> Option> { + use crate::ContractInfoOf; + let trie_id = >::get(who) + .expect("no contract info under the given address") + .get_alive() + .expect("the contract exists but not alive") + .trie_id; + storage::read_contract_storage(&trie_id, &key) +} + +pub fn set_storage(who: &u64, key: [u8; 32], value: Option>) { + use crate::ContractInfoOf; + let trie_id = >::get(who) + .expect("no contract info under the given address") + .get_alive() + .expect("the contract exists but not alive") + .trie_id; + storage::write_contract_storage::(who, &trie_id, &key, value); +} + // Perform a simple transfer to a non-existent account supplying way more gas than needed. // Then we check that the all unused gas is refunded. #[test] @@ -313,62 +347,40 @@ fn refunds_unused_gas() { #[test] fn account_removal_removes_storage() { ExtBuilder::default().existential_deposit(100).build().execute_with(|| { - let trie_id1 = ::TrieIdGenerator::trie_id(&1); - let trie_id2 = ::TrieIdGenerator::trie_id(&2); - let key1 = &[1; 32]; - let key2 = &[2; 32]; + let key1 = [1; 32]; + let key2 = [2; 32]; // Set up two accounts with free balance above the existential threshold. { Balances::deposit_creating(&1, 110); - ContractInfoOf::::insert(1, &ContractInfo::Alive(RawAliveContractInfo { - trie_id: trie_id1.clone(), - storage_size: ::StorageSizeOffset::get(), - deduct_block: System::block_number(), - code_hash: H256::repeat_byte(1), - rent_allowance: 40, - last_write: None, - })); - - let mut overlay = OverlayAccountDb::::new(&DirectAccountDb); - overlay.set_storage(&1, key1.clone(), Some(b"1".to_vec())); - overlay.set_storage(&1, key2.clone(), Some(b"2".to_vec())); - DirectAccountDb.commit(overlay.into_change_set()); + place_contract(&1, H256::repeat_byte(1)); + set_storage(&1, key1.clone(), Some(b"1".to_vec())); + set_storage(&1, key2.clone(), Some(b"2".to_vec())); Balances::deposit_creating(&2, 110); - ContractInfoOf::::insert(2, &ContractInfo::Alive(RawAliveContractInfo { - trie_id: trie_id2.clone(), - storage_size: ::StorageSizeOffset::get(), - deduct_block: System::block_number(), - code_hash: H256::repeat_byte(2), - rent_allowance: 40, - last_write: None, - })); - - let mut overlay = OverlayAccountDb::::new(&DirectAccountDb); - overlay.set_storage(&2, key1.clone(), Some(b"3".to_vec())); - overlay.set_storage(&2, key2.clone(), Some(b"4".to_vec())); - DirectAccountDb.commit(overlay.into_change_set()); + place_contract(&2, H256::repeat_byte(2)); + set_storage(&2, key1.clone(), Some(b"3".to_vec())); + set_storage(&2, key2.clone(), Some(b"4".to_vec())); } // Transfer funds from account 1 of such amount that after this transfer // the balance of account 1 will be below the existential threshold. // - // This should lead to the removal of all storage associated with this account. + // This should trigger OnFreeBalanceZero callback and therefore kill the child storage and + // remove the contract info. assert_ok!(Balances::transfer(Origin::signed(1), 2, 20)); - // Verify that all entries from account 1 is removed, while - // entries from account 2 is in place. + // Verify that the account 1 is removed, while the account 2 is in place. { - assert!(>::get_storage(&DirectAccountDb, &1, Some(&trie_id1), key1).is_none()); - assert!(>::get_storage(&DirectAccountDb, &1, Some(&trie_id1), key2).is_none()); + assert!(>::get(&1).is_none()); + assert!(>::get(&2).is_some()); assert_eq!( - >::get_storage(&DirectAccountDb, &2, Some(&trie_id2), key1), + get_storage(&2, key1), Some(b"3".to_vec()) ); assert_eq!( - >::get_storage(&DirectAccountDb, &2, Some(&trie_id2), key2), + get_storage(&2, key2), Some(b"4".to_vec()) ); } @@ -442,10 +454,12 @@ fn instantiate_and_call_and_deposit_event() { topics: vec![], }, EventRecord { - phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Transfer(ALICE, BOB, 100)), - topics: vec![], - }, + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::balances( + balances::RawEvent::Transfer(ALICE, BOB, 100, 0) + ), + topics: vec![], + }, EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::contract(RawEvent::Contract(BOB, vec![1, 2, 3, 4])), @@ -455,7 +469,7 @@ fn instantiate_and_call_and_deposit_event() { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::contract(RawEvent::Instantiated(ALICE, BOB)), topics: vec![], - } + }, ]); assert_ok!(creation); @@ -545,7 +559,9 @@ fn dispatch_call() { }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Transfer(ALICE, BOB, 100)), + event: MetaEvent::balances( + balances::RawEvent::Transfer(ALICE, BOB, 100, 0) + ), topics: vec![], }, EventRecord { @@ -667,7 +683,9 @@ fn dispatch_call_not_dispatched_after_top_level_transaction_failure() { }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Transfer(ALICE, BOB, 100)), + event: MetaEvent::balances( + balances::RawEvent::Transfer(ALICE, BOB, 100, 0) + ), topics: vec![], }, EventRecord { @@ -1218,24 +1236,32 @@ fn default_rent_allowance_on_instantiate() { const CODE_RESTORATION: &str = r#" (module (import "env" "ext_set_storage" (func $ext_set_storage (param i32 i32 i32 i32))) - (import "env" "ext_restore_to" (func $ext_restore_to (param i32 i32 i32 i32 i32 i32 i32 i32))) + (import "env" "ext_restore_to" + (func $ext_restore_to + (param i32 i32 i32 i32 i32 i32 i32 i32) + (result i32) + ) + ) (import "env" "memory" (memory 1 1)) (func (export "call") - (call $ext_restore_to - ;; Pointer and length of the encoded dest buffer. - (i32.const 256) - (i32.const 8) - ;; Pointer and length of the encoded code hash buffer - (i32.const 264) - (i32.const 32) - ;; Pointer and length of the encoded rent_allowance buffer - (i32.const 296) - (i32.const 8) - ;; Pointer and number of items in the delta buffer. - ;; This buffer specifies multiple keys for removal before restoration. - (i32.const 100) - (i32.const 1) + ;; Drop the status code. + (drop + (call $ext_restore_to + ;; Pointer and length of the encoded dest buffer. + (i32.const 256) + (i32.const 8) + ;; Pointer and length of the encoded code hash buffer + (i32.const 264) + (i32.const 32) + ;; Pointer and length of the encoded rent_allowance buffer + (i32.const 296) + (i32.const 8) + ;; Pointer and number of items in the delta buffer. + ;; This buffer specifies multiple keys for removal before restoration. + (i32.const 100) + (i32.const 1) + ) ) ) (func (export "deploy") @@ -1360,10 +1386,10 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: ); assert!(ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); - /// Create another account with the address `DJANGO` with `CODE_RESTORATION`. - /// - /// Note that we can't use `ALICE` for creating `DJANGO` so we create yet another - /// account `CHARLIE` and create `DJANGO` with it. + // Create another account with the address `DJANGO` with `CODE_RESTORATION`. + // + // Note that we can't use `ALICE` for creating `DJANGO` so we create yet another + // account `CHARLIE` and create `DJANGO` with it. Balances::deposit_creating(&CHARLIE, 1_000_000); assert_ok!(Contract::instantiate( Origin::signed(CHARLIE), diff --git a/frame/contracts/src/util.rs b/frame/contracts/src/util.rs new file mode 100644 index 0000000000000..2734cbbffc578 --- /dev/null +++ b/frame/contracts/src/util.rs @@ -0,0 +1,18 @@ +pub enum TxOutcome { + Commit, + Discard, +} + +pub fn with_transaction(f: impl FnOnce() -> (R, TxOutcome)) -> R { + use sp_io::storage::{ + start_transaction, commit_transaction, discard_transaction, + }; + + start_transaction(); + let (result, outcome) = f(); + match outcome { + TxOutcome::Commit => commit_transaction(), + TxOutcome::Discard => discard_transaction(), + } + result +} diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 273b7fb037b83..f4b3d6e45261b 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -263,19 +263,20 @@ mod tests { fn note_dispatch_call(&mut self, call: Call) { self.dispatches.push(DispatchEntry(call)); } - fn note_restore_to( + fn restore_to( &mut self, dest: u64, code_hash: H256, rent_allowance: u64, delta: Vec, - ) { + ) -> Result<(), &'static str> { self.restores.push(RestoreEntry { dest, code_hash, rent_allowance, delta, }); + Ok(()) } fn caller(&self) -> &u64 { &42 @@ -364,14 +365,14 @@ mod tests { fn note_dispatch_call(&mut self, call: Call) { (**self).note_dispatch_call(call) } - fn note_restore_to( + fn restore_to( &mut self, dest: u64, code_hash: H256, rent_allowance: u64, delta: Vec, - ) { - (**self).note_restore_to( + ) -> Result<(), &'static str> { + (**self).restore_to( dest, code_hash, rent_allowance, diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 0204d4eba0d7f..4e3ab06f70c65 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -644,17 +644,18 @@ define_env!(Env, , Ok(()) }, - // Record a request to restore the caller contract to the specified contract. + // Try to restore the given destination contract sacrificing the caller. // - // At the finalization stage, i.e. when all changes from the extrinsic that invoked this - // contract are commited, this function will compute a tombstone hash from the caller's - // storage and the given code hash and if the hash matches the hash found in the tombstone at - // the specified address - kill the caller contract and restore the destination contract and set - // the specified `rent_allowance`. All caller's funds are transfered to the destination. + // This function will compute a tombstone hash from the caller's storage and the given code hash + // and if the hash matches the hash found in the tombstone at the specified address - kill + // the caller contract and restore the destination contract and set the specified `rent_allowance`. + // All caller's funds are transfered to the destination. // - // This function doesn't perform restoration right away but defers it to the end of the - // transaction. If there is no tombstone in the destination address or if the hashes don't match - // then restoration is cancelled and no changes are made. + // If there is no tombstone at the destination address or if the hashes don't match + // then restoration is cancelled and no changes are made. In this case this function + // will return non-zero code. + // + // Otherwise, If the restoration was performed correctly, 0 is returned. // // `dest_ptr`, `dest_len` - the pointer and the length of a buffer that encodes `T::AccountId` // with the address of the to be restored contract. @@ -674,7 +675,7 @@ define_env!(Env, , rent_allowance_len: u32, delta_ptr: u32, delta_count: u32 - ) => { + ) -> u32 => { let dest: <::T as system::Trait>::AccountId = read_sandbox_memory_as(ctx, dest_ptr, dest_len)?; let code_hash: CodeHash<::T> = @@ -702,14 +703,15 @@ define_env!(Env, , delta }; - ctx.ext.note_restore_to( + match ctx.ext.restore_to( dest, code_hash, rent_allowance, delta, - ); - - Ok(()) + ) { + Ok(_) => Ok(0), + Err(_) => Ok(1), + } }, // Returns the size of the scratch buffer. diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index 1d575d07943d8..d65f355ceda20 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -25,6 +25,22 @@ pub mod hashed; pub mod child; pub mod generator; +/// Execute under a transactional layer. +/// +/// If the result of execution is an error, +/// the transactional layer get reverted; otherwhise +/// it is committed. +pub fn with_transaction(f: impl FnOnce() -> Result) -> Result { + sp_io::storage::start_transaction(); + let r = f(); + if r.is_ok() { + sp_io::storage::commit_transaction(); + } else { + sp_io::storage::discard_transaction(); + } + r +} + /// A trait for working with macro-generated storage values under the substrate storage API. /// /// Details on implementation can be found at @@ -423,7 +439,31 @@ pub trait StoragePrefixedMap { mod test { use primitives::hashing::twox_128; use sp_io::TestExternalities; - use crate::storage::{unhashed, StoragePrefixedMap}; + use crate::storage::{unhashed, with_transaction, StoragePrefixedMap}; + + #[test] + fn with_transaction_works() { + type R = Result<(), ()>; + const KEY: &[u8] = b"abc"; + + TestExternalities::default().execute_with(|| { + unhashed::put(KEY, &1u32); + + let _ = with_transaction(|| { + unhashed::put(b"abc", &2u32); + assert_eq!(unhashed::get(b"abc"), Some(2)); + + let _ = with_transaction(|| { + unhashed::put(b"abc", &3u32); + assert_eq!(unhashed::get(b"abc"), Some(3)); + R::Err(()) + }); + assert_eq!(unhashed::get(b"abc"), Some(2)); + R::Ok(()) + }); + assert_eq!(unhashed::get(b"abc"), Some(2)); + }); + } #[test] fn prefixed_map_works() { diff --git a/primitives/api/test/tests/runtime_calls.rs b/primitives/api/test/tests/runtime_calls.rs index 3b09d67a2cf82..863f66c3b8b4b 100644 --- a/primitives/api/test/tests/runtime_calls.rs +++ b/primitives/api/test/tests/runtime_calls.rs @@ -106,7 +106,6 @@ fn calling_with_both_strategy_and_fail_on_native_should_work() { assert_eq!(runtime_api.fail_on_native(&block_id).unwrap(), 1); } - #[test] fn calling_with_native_else_wasm_and_fail_on_wasm_should_work() { let client = TestClientBuilder::new().set_execution_strategy(ExecutionStrategy::NativeElseWasm).build(); @@ -131,6 +130,23 @@ fn use_trie_function() { assert_eq!(runtime_api.use_trie(&block_id).unwrap(), 2); } +#[test] +fn use_history_data() { + let client = TestClientBuilder::new().set_execution_strategy(ExecutionStrategy::AlwaysWasm).build(); + let runtime_api = client.runtime_api(); + let block_id = BlockId::Number(client.info().chain.best_number); + assert!(runtime_api.use_history_data(&block_id).is_ok()); +} + +#[test] +fn test_transactions() { + let client = TestClientBuilder::new().set_execution_strategy(ExecutionStrategy::AlwaysWasm).build(); + let runtime_api = client.runtime_api(); + let block_id = BlockId::Number(client.info().chain.best_number); + assert_eq!(runtime_api.use_transactions(&block_id).unwrap(), 1); +} + + #[test] fn initialize_block_works() { let client = TestClientBuilder::new().set_execution_strategy(ExecutionStrategy::Both).build(); diff --git a/primitives/externalities/src/lib.rs b/primitives/externalities/src/lib.rs index 05121f34d3dec..507a5b74988a0 100644 --- a/primitives/externalities/src/lib.rs +++ b/primitives/externalities/src/lib.rs @@ -156,6 +156,16 @@ pub trait Externalities: ExtensionStore { /// /// Returns the SCALE encoded hash. fn storage_changes_root(&mut self, parent: &[u8]) -> Result>, ()>; + + /// Create a new transactional layer. + fn storage_start_transaction(&mut self); + + /// Discard a transactional layer, pending changes of every transaction behind this layer are + /// dropped (including committed changes) . + fn storage_discard_transaction(&mut self); + + /// Commit a transactional layer. The changes stay attached to parent transaction layer. + fn storage_commit_transaction(&mut self); } /// Extension for the [`Externalities`] trait. diff --git a/primitives/historical-data/Cargo.toml b/primitives/historical-data/Cargo.toml new file mode 100644 index 0000000000000..f4a2a91e4fa3e --- /dev/null +++ b/primitives/historical-data/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "sp-historical-data" +version = "2.0.0" +authors = ["Parity Technologies "] +description = "Data associated with its history" +edition = "2018" + +[dependencies] +rstd = { package = "sp-std", path = "../std", default-features = false } +smallvec = { version = "0.6", default-features = false } + +[features] +default = ["std"] +std = [ + "rstd/std", + "smallvec/std", +] +test-helpers = [] diff --git a/primitives/historical-data/README.md b/primitives/historical-data/README.md new file mode 100644 index 0000000000000..c335b166d7a5b --- /dev/null +++ b/primitives/historical-data/README.md @@ -0,0 +1,18 @@ +## Historical data + +Crate with functionality to manage data that stores its own history. + +This covers: +- linear history driven data, eg. transactional layers for overlay. +- long term storage with multiple branch, eg. offchain storage. + +General design is container where query and update requires global +history context. + +History is serialize as a per item basis. + +This crates is `no_std` compatible as long as the `std` feature is not enabled. + +For more information see + +License: GPL-3.0 diff --git a/primitives/historical-data/src/lib.rs b/primitives/historical-data/src/lib.rs new file mode 100644 index 0000000000000..fd9c2b9d46190 --- /dev/null +++ b/primitives/historical-data/src/lib.rs @@ -0,0 +1,92 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Data storage containing multiple states for a value. +//! This is used to store historical information for an item. + +#![cfg_attr(not(feature = "std"), no_std)] + +pub mod synch_linear_transaction; + +/// An entry at a given history index. +#[derive(Debug, Clone, PartialEq)] +pub struct HistoricalValue { + /// The stored value. + pub value: V, + /// The moment in history when the value got set. + pub index: I, +} + +impl From<(V, I)> for HistoricalValue { + fn from(input: (V, I)) -> HistoricalValue { + HistoricalValue { value: input.0, index: input.1 } + } +} + +impl HistoricalValue { + fn as_mut(&mut self) -> HistoricalValue<&mut V, I> { + HistoricalValue { value: &mut self.value, index: self.index.clone() } + } +} + +#[derive(Debug, PartialEq)] +/// Results from cleaning a data with history. +/// It should be used to update from the calling context, +/// for instance remove this data from a map if it was cleared. +pub enum CleaningResult { + Unchanged, + Changed, + Cleared, +} + +/// History of value and their state. +#[derive(Debug, Clone, PartialEq)] +pub struct History(pub(crate) smallvec::SmallVec<[HistoricalValue; ALLOCATED_HISTORY]>); + +impl Default for History { + fn default() -> Self { + History(Default::default()) + } +} + +/// Size of preallocated history per element. +/// Currently at two for committed and prospective only. +/// It means that using transaction in a module got a direct allocation cost. +const ALLOCATED_HISTORY: usize = 2; + +impl History { + #[cfg(any(test, feature = "test-helpers"))] + /// Get current number of stored historical values. + pub fn len(&self) -> usize { + self.0.len() + } + + #[cfg(any(test, feature = "test-helpers"))] + /// Create an history from an existing history. + pub fn from_iter(input: impl IntoIterator>) -> Self { + let mut history = History::default(); + for v in input { + history.push_unchecked(v); + } + history + } + + /// Push a value without checking if it can overwrite the current + /// state. + pub fn push_unchecked(&mut self, item: HistoricalValue) { + self.0.push(item); + } +} diff --git a/primitives/historical-data/src/synch_linear_transaction.rs b/primitives/historical-data/src/synch_linear_transaction.rs new file mode 100644 index 0000000000000..058d547877f17 --- /dev/null +++ b/primitives/historical-data/src/synch_linear_transaction.rs @@ -0,0 +1,314 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Linear arrangement of historical data with transactional +//! support. +//! +//! Described as a synchronized module as it contains historical +//! data that do not require a separated state for most operation. +//! Global state operation must therefore apply on all local values +//! with history synchronously. +//! +//! # Global state +//! +//! The only global state is a counter of overlayed transaction layer. +//! Committing or discarding a layer must use this counter. +//! +//! # Local state +//! +//! Local state is either a committed state (this is a single first independant level +//! of transaction) or a reference to the transaction counter in use in time of creation. + +use crate::CleaningResult; + +/// Global state is a simple counter to the current overlay layer index. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct States { current_layer: usize } + +impl Default for States { + fn default() -> Self { + // we default to 1 to be able to discard this transaction. + States { current_layer: 1 } + } +} + +impl States { + + /// Get corresponding current state. + pub fn as_state(&self) -> State { + State::Transaction(self.current_layer) + } + + /// Build any state for testing only. + #[cfg(any(test, feature = "test-helpers"))] + pub fn test_state( + current_layer: usize, + ) -> Self { + States { current_layer } + } + + /// By default the current transaction is at least 1. + /// 0 can be use only to apply transaction change, in + /// this case transaction need to be restored to a valid + /// state afterward. + pub fn finalize_discard(&mut self) { + if self.current_layer == 0 { + self.current_layer = 1; + } + } + + /// Discard prospective changes to state. + /// It does not revert actual values. + /// A subsequent update of all related stored history is needed. + pub fn discard_prospective(&mut self) { + self.current_layer = 1; + } + + /// After a prospective was discarded, clear prospective history. + pub fn apply_discard_prospective(value: &mut History) -> CleaningResult { + if value.0.len() == 0 { + return CleaningResult::Cleared; + } + if value.0[0].index == State::Committed { + if value.0.len() == 1 { + return CleaningResult::Unchanged; + } + value.0.truncate(1); + } else { + value.0.clear(); + } + CleaningResult::Changed + } + + /// Commit prospective changes to state. + /// A subsequent update of all related stored history is needed. + pub fn commit_prospective(&mut self) { + self.current_layer = 1; + } + + /// After a prospective was committed, commit pending value and clear existing history. + pub fn apply_commit_prospective(value: &mut History) -> CleaningResult { + if value.0.len() == 0 { + return CleaningResult::Cleared; + } + if value.0.len() == 1 { + if value.0[0].index != State::Committed { + value.0[0].index = State::Committed; + } else { + return CleaningResult::Unchanged; + } + } else if let Some(mut v) = value.0.pop() { + v.index = State::Committed; + value.0.clear(); + value.0.push(v); + } + CleaningResult::Changed + } + + /// Create a new transactional layer. + pub fn start_transaction(&mut self) { + self.current_layer += 1; + } + + /// Discard a transactional layer. + /// It does not revert actual values. + /// A subsequent update of all related stored history is needed. + pub fn discard_transaction(&mut self) { + if self.current_layer > 0 { + self.current_layer -= 1; + } + } + + /// Update a value to previous transaction. + /// Multiple discard can be applied at the same time. + pub fn apply_discard_transaction(&self, value: &mut History) -> CleaningResult { + let init_len = value.0.len(); + for i in (0 .. value.0.len()).rev() { + if let HistoricalValue { + value: _, + index: State::Transaction(ix), + } = value.0[i] { + if ix > self.current_layer { + let _ = value.0.pop(); + } else { break } + } else { break } + } + if value.0.len() == 0 { + CleaningResult::Cleared + } else if value.0.len() != init_len { + CleaningResult::Changed + } else { + CleaningResult::Unchanged + } + } + + /// Discard a transactional layer. + /// It does not revert actual values. + /// A subsequent update of all related stored history is needed. + pub fn commit_transaction(&mut self) { + if self.current_layer > 1 { + self.current_layer -= 1; + } + } + + /// Update a value to be the best historical value + /// after one or more `commit_transaction` calls. + /// Multiple discard can be applied at the same time. + pub fn apply_commit_transaction(&self, value: &mut History) -> CleaningResult { + let mut new_value = None; + for i in (0 .. value.0.len()).rev() { + if let HistoricalValue { + value: _, + index: State::Transaction(ix), + } = value.0[i] { + if ix > self.current_layer { + if let Some(v) = value.0.pop() { + if new_value.is_none() { + new_value = Some(v.value); + } + } + } else if ix == self.current_layer && new_value.is_some() { + let _ = value.0.pop(); + } else { break } + } else { break } + } + if let Some(new_value) = new_value { + value.0.push(HistoricalValue { + value: new_value, + index: State::Transaction(self.current_layer), + }); + return CleaningResult::Changed; + } + CleaningResult::Unchanged + } +} + +/// State for a historical value in a transaction +/// synchronously managed. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum State { + /// Committed, transactional action do not + /// change this state. + Committed, + /// Value in a transaction, contains the current + /// number of transaction at creation. + Transaction(usize), +} + +impl State { + fn transaction_index(&self) -> Option { + if let &State::Transaction(ix) = self { + Some(ix) + } else { + None + } + } +} +/// An entry at a given history height. +pub type HistoricalValue = crate::HistoricalValue; + +/// History of value and their state. +pub type History = crate::History; + +impl History { + + /// Set a value, it uses a global state as parameter. + pub fn set(&mut self, states: &States, value: V) { + if let Some(v) = self.0.last_mut() { + debug_assert!(v.index.transaction_index().unwrap_or(0) <= states.current_layer, + "History expects \ + only new values at the latest state, some state has not \ + synchronized properly"); + if v.index.transaction_index() == Some(states.current_layer) { + v.value = value; + return; + } + } + self.0.push(HistoricalValue { + value, + index: State::Transaction(states.current_layer), + }); + } + + /// Access to the latest pending value. + pub fn get(&self) -> Option<&V> { + self.0.last().map(|h| &h.value) + } + + /// Extracts the latest value if there is one. + pub fn into_pending(mut self) -> Option { + if let Some(v) = self.0.pop() { + Some(v.value) + } else { + None + } + } + + #[cfg(any(test, feature = "test-helpers"))] + pub fn get_prospective(&self) -> Option<&V> { + match self.0.get(0) { + Some(HistoricalValue { + value: _, + index: State::Committed, + }) => { + if let Some(HistoricalValue { + value, + index: State::Transaction(_), + }) = self.0.get(1) { + Some(&value) + } else { + None + } + }, + Some(HistoricalValue { + value, + index: State::Transaction(_), + }) => Some(&value), + None => None, + } + } + + #[cfg(any(test, feature = "test-helpers"))] + pub fn get_committed(&self) -> Option<&V> { + if let Some(HistoricalValue { + value, + index: State::Committed, + }) = self.0.get(0) { + return Some(&value) + } else { + None + } + } + + /// Extracts the committed value if there is one. + pub fn into_committed(mut self) -> Option { + self.0.truncate(1); + if let Some(HistoricalValue { + value, + index: State::Committed, + }) = self.0.pop() { + return Some(value) + } else { + None + } + } + + /// Returns mutable latest pending historical value. + pub fn get_mut(&mut self) -> Option> { + self.0.last_mut().map(|h| h.as_mut()) + } + +} diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index ef4334808a7e3..4f004370a67b5 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -214,6 +214,21 @@ pub trait Storage { self.storage_changes_root(parent_hash).ok().and_then(|h| h) } + /// Start a new transaction. + fn start_transaction(&mut self) { + self.storage_start_transaction(); + } + + /// Discard a transactional layer. + fn discard_transaction(&mut self) { + self.storage_discard_transaction(); + } + + /// Commit a transactional layer. + fn commit_transaction(&mut self) { + self.storage_commit_transaction(); + } + /// Get the next key in storage after the given one in lexicographic order. fn next_key(&mut self, key: &[u8]) -> Option> { self.next_storage_key(&key) diff --git a/primitives/state-machine/Cargo.toml b/primitives/state-machine/Cargo.toml index cd721c17fdf33..8eaa8e895f954 100644 --- a/primitives/state-machine/Cargo.toml +++ b/primitives/state-machine/Cargo.toml @@ -15,12 +15,23 @@ trie = { package = "sp-trie", path = "../trie" } primitives = { package = "sp-core", path = "../core" } panic-handler = { package = "sp-panic-handler", path = "../panic-handler" } codec = { package = "parity-scale-codec", version = "1.0.0" } +sp-historical-data = { path = "../historical-data" } num-traits = "0.2.8" rand = "0.7.2" externalities = { package = "sp-externalities", path = "../externalities" } [dev-dependencies] hex-literal = "0.2.1" +criterion = "0.2" +rand = { version = "0.7.1", features = ["small_rng"] } +sp-historical-data = { path = "../historical-data", features = [ "test-helpers" ] } + +[[bench]] +name = "bench" +harness = false [features] default = [] +test-helpers = [ + "sp-historical-data/test-helpers", +] diff --git a/primitives/state-machine/benches/bench.rs b/primitives/state-machine/benches/bench.rs new file mode 100644 index 0000000000000..665152d7e90d3 --- /dev/null +++ b/primitives/state-machine/benches/bench.rs @@ -0,0 +1,116 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Parity is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Parity is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Parity. If not, see . + +use criterion::{Criterion, black_box, Bencher}; +use criterion::{criterion_group, criterion_main}; +use sp_state_machine::OverlayedChanges; + +const CONTENT_KEY_SIZE: usize = 32; + +fn get_content(seed: u64, len: usize) -> Vec { + use rand::SeedableRng; + use rand::RngCore; + let mut rng = rand::rngs::SmallRng::seed_from_u64(seed); + let mut data = vec![0u8; len]; + rng.fill_bytes(&mut data[..]); + data +} + +fn key_val( + mut inp: &[u8], + len_key: usize, + len_val: usize, +) -> Vec<(Vec, Vec)> { + let mut result = Vec::new(); + loop { + let key = if inp.len() >= len_key { + inp[..len_key].to_vec() + } else { break }; + inp = &inp[len_key..]; + let val = if inp.len() >= len_val { + inp[..len_val].to_vec() + } else { break }; + inp = &inp[len_val..]; + result.push((key, val)); + } + result +} + +fn commit_drop_commit(b: &mut Bencher, input: &Vec) { + let key_vals = key_val(&input[..], 32, 32); + b.iter(move || { + let mut overlayed = OverlayedChanges::default(); + for i in key_vals.iter() { + overlayed.set_storage(i.0.clone(), Some(i.1.clone())); + } + overlayed.commit_prospective(); + for i in key_vals.iter() { + overlayed.set_storage(i.0.clone(), Some(i.1.clone())); + } + overlayed.discard_prospective(); + for i in key_vals.iter() { + overlayed.set_storage(i.0.clone(), Some(i.1.clone())); + } + overlayed.commit_prospective(); + }); +} + +fn commit_drop_commit_and_get(b: &mut Bencher, input: &Vec) { + let key_vals = key_val(&input[..], 32, 32); + b.iter(move || { + let mut overlayed = OverlayedChanges::default(); + for i in key_vals.iter() { + overlayed.set_storage(i.0.clone(), Some(i.1.clone())); + } + for i in key_vals.iter() { + black_box(overlayed.storage(&i.0)); + } + overlayed.commit_prospective(); + for i in key_vals.iter() { + overlayed.set_storage(i.0.clone(), Some(i.1.clone())); + } + for i in key_vals.iter() { + black_box(overlayed.storage(&i.0)); + } + overlayed.discard_prospective(); + for i in key_vals.iter() { + black_box(overlayed.storage(&i.0)); + } + for i in key_vals.iter() { + overlayed.set_storage(i.0.clone(), Some(i.1.clone())); + } + overlayed.commit_prospective(); + }); +} + +fn bench_overlay_commit_drop_commit(c: &mut Criterion) { + let inp = get_content(12, CONTENT_KEY_SIZE * 100); + let inps = vec![inp]; + c.bench_function_over_inputs("commit_drop_commit", commit_drop_commit, inps); +} + +fn bench_overlay_commit_drop_commit_get(c: &mut Criterion) { + let inp = get_content(12, CONTENT_KEY_SIZE * 100); + let inps = vec![inp]; + c.bench_function_over_inputs("commit_drop_commit_get", commit_drop_commit_and_get, inps); +} + +criterion_group!(benches, + bench_overlay_commit_drop_commit, + bench_overlay_commit_drop_commit_get, +); + +criterion_main!(benches); diff --git a/primitives/state-machine/fuzz/Cargo.toml b/primitives/state-machine/fuzz/Cargo.toml new file mode 100644 index 0000000000000..d8ecf770b59a7 --- /dev/null +++ b/primitives/state-machine/fuzz/Cargo.toml @@ -0,0 +1,29 @@ + +[package] +name = "sp-state-machine-fuzz" +version = "0.0.1" +authors = [] +publish = false +edition = "2018" + +[package.metadata] +cargo-fuzz = true + +[dependencies.sp-state-machine] +path = ".." +features = ["test-helpers"] + +[dependencies.libfuzzer-sys] +git = "https://github.com/rust-fuzz/libfuzzer-sys.git" + +# Prevent this from interfering with workspaces +[workspace] +members = ["."] + +[[bin]] +name = "fuzz_transactions_no_compactness" +path = "fuzz_targets/fuzz_transactions.rs" + +[[bin]] +name = "fuzz_transactions_with_compactness" +path = "fuzz_targets/fuzz_transactions_with_compactness.rs" diff --git a/primitives/state-machine/fuzz/fuzz_targets/fuzz_transactions.rs b/primitives/state-machine/fuzz/fuzz_targets/fuzz_transactions.rs new file mode 100644 index 0000000000000..561fb38d4ced6 --- /dev/null +++ b/primitives/state-machine/fuzz/fuzz_targets/fuzz_transactions.rs @@ -0,0 +1,7 @@ +#![no_main] + +use libfuzzer_sys::fuzz_target; + +fuzz_target!(|data: &[u8]| { + sp_state_machine_fuzz::fuzz_transactions(data) +}); diff --git a/primitives/state-machine/fuzz/fuzz_targets/fuzz_transactions_with_compactness.rs b/primitives/state-machine/fuzz/fuzz_targets/fuzz_transactions_with_compactness.rs new file mode 100644 index 0000000000000..07f673627c968 --- /dev/null +++ b/primitives/state-machine/fuzz/fuzz_targets/fuzz_transactions_with_compactness.rs @@ -0,0 +1,7 @@ +#![no_main] + +use libfuzzer_sys::fuzz_target; + +fuzz_target!(|data: &[u8]| { + sp_state_machine_fuzz::fuzz_transactions_then_compactness(data) +}); diff --git a/primitives/state-machine/fuzz/src/lib.rs b/primitives/state-machine/fuzz/src/lib.rs new file mode 100644 index 0000000000000..9ea6eaeeded6b --- /dev/null +++ b/primitives/state-machine/fuzz/src/lib.rs @@ -0,0 +1,259 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Substrate state machine fuzzing implementations. +use sp_state_machine::OverlayedChanges; +use std::collections::HashMap; + +/// Size of key, max 255 +const KEY_SPACE: u8 = 20; + +/// Size of key, max 255 +const VALUE_SPACE: u8 = 50; + +fn fuzz_transactions_inner(input: &[u8], check_compactness: bool) { + let mut input_index: usize = 0; + let mut overlayed = OverlayedChanges::default(); + let mut ref_overlayed = RefOverlayedChanges::default(); + + let mut actions = Vec::new(); + let mut values = Vec::new(); + loop { + let action: Actions = if let Some(v) = input.get(input_index) { + input_index += 1; + (*v).into() + } else { break }; + + actions.push(action); + match action { + Actions::CommitProspective => { + overlayed.commit_prospective(); + ref_overlayed.commit_prospective(); + }, + Actions::DropProspective => { + overlayed.discard_prospective(); + ref_overlayed.discard_prospective(); + }, + Actions::NewTransaction => { + overlayed.start_transaction(); + ref_overlayed.start_transaction(); + }, + Actions::CommitTransaction => { + overlayed.commit_transaction(); + ref_overlayed.commit_transaction(); + }, + Actions::DropTransaction => { + overlayed.discard_transaction(); + ref_overlayed.discard_transaction(); + }, + Actions::Insert => { + let key = if let Some(v) = input.get(input_index) { + input_index += 1; + v % KEY_SPACE + } else { break }; + let value = if let Some(v) = input.get(input_index) { + input_index += 1; + v % VALUE_SPACE + } else { break }; + values.push((key, value)); + overlayed.set_storage(vec![key], Some(vec![value])); + ref_overlayed.set_storage(vec![key], Some(vec![value])); + } + } + + } + + let mut success = true; + + let (check_value, len) = check_values(&overlayed, &ref_overlayed); + success &= check_value; + + if check_compactness { + let reference_size = ref_overlayed.total_length(); + let size = overlayed.top_count_keyvalue_pair(); + if reference_size != size { + println!("inconsistent gc {} {}", size, reference_size); + success = false; + } + let (check_value, len_compactness) = check_values(&overlayed, &ref_overlayed); + success &= check_value; + success &= len_compactness == len; + } + ref_overlayed.commit_prospective(); + let ref_len = ref_overlayed.committed.len(); + if len != ref_len { + println!("inconsistent length {} {}", len, ref_len); + success = false; + } + if !success { + println!("fuzzing: \n {:x?}", (&actions, &values)); + println!("input: \n {:?}", &input); + } + + assert!(success); +} + +fn check_values(overlayed: &OverlayedChanges, ref_overlayed: &RefOverlayedChanges) -> (bool, usize) { + let mut len = 0; + let mut success = true; + for (key, value) in overlayed.iter_values(None) { + let ref_value = ref_overlayed.storage(key); + if Some(value) != ref_value { + println!("at {:x?} different values {:x?} {:x?}", key, Some(value), ref_value); + success = false; + } + len += 1; + } + (success, len) +} + +pub fn fuzz_transactions(input: &[u8]) { + fuzz_transactions_inner(input, false); +} + +pub fn fuzz_transactions_then_compactness(input: &[u8]) { + fuzz_transactions_inner(input, true); +} + +#[derive(Clone, Copy, Debug)] +pub enum Actions { + Insert, + // Delete, same as an insert do not test. + CommitProspective, + DropProspective, + NewTransaction, + CommitTransaction, + DropTransaction, +} + +impl From for Actions { + fn from(v: u8) -> Self { + match (v as usize) * 100 / 255 { + v if v <= 5 => Actions::CommitProspective, + v if v <= 10 => Actions::DropProspective, + v if v <= 20 => Actions::NewTransaction, + v if v <= 30 => Actions::CommitTransaction, + v if v <= 40 => Actions::DropTransaction, + _ => Actions::Insert, + } + } +} + +/// A simple implementation of overlayed change +/// to use as a comparision. +/// It is partly incomplete (no child trie support, no change trie). +#[derive(Debug, Clone, Default)] +pub struct RefOverlayedChanges { + committed: HashMap, Vec>, + prospective: HashMap, Vec>, + transactions: Vec, Vec>>, +} + +impl RefOverlayedChanges { + pub fn discard_prospective(&mut self) { + self.transactions.clear(); + self.prospective.clear(); + } + + pub fn commit_prospective(&mut self) { + for _ in 0 .. self.transactions.len() { + self.commit_transaction(); + } + self.committed.extend(self.prospective.drain()); + } + + pub fn start_transaction(&mut self) { + self.transactions.push(Default::default()); + } + + pub fn discard_transaction(&mut self) { + if self.transactions.len() == 0 { + // clear prospective on no transaction started. + self.prospective.clear(); + } else { + let _ = self.transactions.pop(); + } + } + + /// Commit a transactional layer. + pub fn commit_transaction(&mut self) { + match self.transactions.len() { + 0 => (), + 1 => self.prospective.extend( + self.transactions.pop().expect("length just checked").into_iter() + ), + _ => { + let t = self.transactions.pop().expect("length just checked"); + self.transactions.last_mut().expect("length just checked") + .extend(t.into_iter()); + } + } + } + + pub fn set_storage(&mut self, key: Vec, val: Option>) { + if self.transactions.len() > 0 { + self.transactions.last_mut().expect("length just checked") + .insert(key, val.expect("fuzzer do not delete")); + } else { + self.prospective.insert(key, val.expect("fuzzer do not delete")); + } + } + + pub fn storage(&self, key: &[u8]) -> Option> { + for t in self.transactions.iter().rev() { + if let Some(v) = t.get(key) { + return Some(Some(v)); + } + } + if let Some(v) = self.prospective.get(key) { + return Some(Some(v)); + } + if let Some(v) = self.committed.get(key) { + return Some(Some(v)); + } + None + } + + pub fn total_length(&self) -> usize { + let tr_len: usize = self.transactions.iter() + .map(|l| l.len()).sum(); + self.committed.len() + self.prospective.len() + tr_len + } +} + +// Those are error previously found through fuzzing. +// They were fixed, but we keep the vectors to check +// for regression without needing to run fuzzing. +#[test] +fn previous_fuzzed_error() { + let inputs = [ + vec![0x98,0x98,0xf6,0x12,0xee,0x98,0xf9,], + vec![0xf1,0x0,0x0,0x1,0x38,0xb2,0x0,0x67,], + vec![0x2,0xee,0xee,0x12,0x2,0x16,0x67,0xee,0xee,0xee,], + vec![50, 208, 50, 38, 46, 58, 209, 50, 216, 255, 255], + vec![238, 0, 36, 43, 50, 46, 38, 211, 0, 0, 61], + vec![50, 255, 38, 38, 186, 35, 46, 43, 46, 35, 255, 255, 102, 67], + vec![0x6e, 0xff, 0xf7, 0x0, 0x6e, 0xff, 0xff, 0x2d, 0xff, 0xff, 0xff, 0xe], + vec![0x2e,0x6e,0x22,0x32,0x2e,0x6e,0x22,0x32,0x3f,0x2e,], + vec![0xd9,0xff,0xfd,0x0,0xff,0xff,0xf8,0x1,0x92,0xff,0xbf,0x14,], + vec![0xef,0xdf,0xc1,0x0,0xc1,0xdf,0xc1,0x2b,0xf3,0xf3,0xb0,0x18, + 0xef,0xdf,0x2e,0x3a,0xef,0xdf,0x0,0xc1,0xf3,0x30,0x18,0xef,0xdf, + 0xc1,0x2b,0xf3,0xf3,0x30,0x17,0x0,0xdf,], + ]; + for input in inputs.iter() { + fuzz_transactions_inner(&input[..], true); + } +} diff --git a/primitives/state-machine/src/basic.rs b/primitives/state-machine/src/basic.rs index deae7f2852592..7333f0a977139 100644 --- a/primitives/state-machine/src/basic.rs +++ b/primitives/state-machine/src/basic.rs @@ -265,6 +265,19 @@ impl Externalities for BasicExternalities { fn storage_changes_root(&mut self, _parent: &[u8]) -> Result>, ()> { Ok(None) } + + fn storage_start_transaction(&mut self) { + warn!("No support for storage transaction"); + } + + fn storage_discard_transaction(&mut self) { + panic!("No support for storage transaction"); + } + + fn storage_commit_transaction(&mut self) { + // no need to fail in this case + } + } impl externalities::ExtensionStore for BasicExternalities { diff --git a/primitives/state-machine/src/changes_trie/build.rs b/primitives/state-machine/src/changes_trie/build.rs index 7e082ad83276b..74d8727c93427 100644 --- a/primitives/state-machine/src/changes_trie/build.rs +++ b/primitives/state-machine/src/changes_trie/build.rs @@ -16,7 +16,7 @@ //! Structures and functions required to build changes trie for given block. -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeMap; use std::collections::btree_map::Entry; use codec::{Decode, Encode}; use hash_db::Hasher; @@ -101,19 +101,14 @@ fn prepare_extrinsics_input<'a, B, H, Number>( Number: BlockNumber, { - let mut children_keys = BTreeSet::>::new(); let mut children_result = BTreeMap::new(); - for (storage_key, _) in changes.prospective.children.iter() - .chain(changes.committed.children.iter()) { - children_keys.insert(storage_key.clone()); - } - for storage_key in children_keys { + for (storage_key, _) in changes.changes.children_iter() { let child_index = ChildIndex:: { block: block.clone(), - storage_key: storage_key.clone(), + storage_key: storage_key.to_vec(), }; - let iter = prepare_extrinsics_input_inner(backend, block, changes, Some(storage_key))?; + let iter = prepare_extrinsics_input_inner(backend, block, changes, Some(storage_key.to_vec()))?; children_result.insert(child_index, iter); } @@ -133,13 +128,8 @@ fn prepare_extrinsics_input_inner<'a, B, H, Number>( H: Hasher, Number: BlockNumber, { - let (committed, prospective) = if let Some(sk) = storage_key.as_ref() { - (changes.committed.children.get(sk), changes.prospective.children.get(sk)) - } else { - (Some(&changes.committed.top), Some(&changes.prospective.top)) - }; - committed.iter().flat_map(|c| c.iter()) - .chain(prospective.iter().flat_map(|c| c.iter())) + + changes.changes.iter_overlay(storage_key.as_ref().map(|r| r.as_slice())) .filter(|( _, v)| v.extrinsics.is_some()) .try_fold(BTreeMap::new(), |mut map: BTreeMap<&[u8], (ExtrinsicIndex, Vec)>, (k, v)| { match map.entry(k) { @@ -336,6 +326,7 @@ mod test { use crate::changes_trie::{RootsStorage, Configuration, storage::InMemoryStorage}; use crate::changes_trie::build_cache::{IncompleteCacheAction, IncompleteCachedBuildData}; use crate::overlayed_changes::{OverlayedValue, OverlayedChangeSet}; + use sp_historical_data::synch_linear_transaction::{States, State, History, HistoricalValue}; use super::*; fn prepare_for_build(zero: u64) -> ( @@ -405,51 +396,58 @@ mod test { ]), ]); let changes = OverlayedChanges { - prospective: OverlayedChangeSet { top: vec![ - (vec![100], OverlayedValue { - value: Some(vec![200]), - extrinsics: Some(vec![0, 2].into_iter().collect()) - }), - (vec![103], OverlayedValue { - value: None, - extrinsics: Some(vec![0, 1].into_iter().collect()) - }), - ].into_iter().collect(), - children: vec![ - (child_trie_key1.clone(), vec![ - (vec![100], OverlayedValue { - value: Some(vec![200]), - extrinsics: Some(vec![0, 2].into_iter().collect()) - }) - ].into_iter().collect()), - (child_trie_key2, vec![ - (vec![100], OverlayedValue { + changes: OverlayedChangeSet { + states: States::test_state(0), + top: vec![ + (EXTRINSIC_INDEX.to_vec(), History::from_iter(vec![ + (OverlayedValue { + value: Some(3u32.encode()), + extrinsics: None, + }, State::Committed), + ].into_iter().map(|(value, index)| HistoricalValue { value, index }))), + (vec![100], History::from_iter(vec![ + (OverlayedValue { + value: Some(vec![202]), + extrinsics: Some(vec![3].into_iter().collect()) + }, State::Committed), + (OverlayedValue { value: Some(vec![200]), - extrinsics: Some(vec![0, 2].into_iter().collect()) - }) - ].into_iter().collect()), - ].into_iter().collect() - }, - committed: OverlayedChangeSet { top: vec![ - (EXTRINSIC_INDEX.to_vec(), OverlayedValue { - value: Some(3u32.encode()), - extrinsics: None, - }), - (vec![100], OverlayedValue { - value: Some(vec![202]), - extrinsics: Some(vec![3].into_iter().collect()) - }), - (vec![101], OverlayedValue { - value: Some(vec![203]), - extrinsics: Some(vec![1].into_iter().collect()) - }), - ].into_iter().collect(), + extrinsics: Some(vec![3, 0, 2].into_iter().collect()) + }, State::Transaction(0)), + ].into_iter().map(|(value, index)| HistoricalValue { value, index }))), + (vec![101], History::from_iter(vec![ + (OverlayedValue { + value: Some(vec![203]), + extrinsics: Some(vec![1].into_iter().collect()) + }, State::Committed), + ].into_iter().map(|(value, index)| HistoricalValue { value, index }))), + (vec![103], History::from_iter(vec![ + (OverlayedValue { + value: None, + extrinsics: Some(vec![0, 1].into_iter().collect()) + }, State::Transaction(0)), + ].into_iter().map(|(value, index)| HistoricalValue { value, index }))), + ].into_iter().collect(), children: vec![ (child_trie_key1, vec![ - (vec![100], OverlayedValue { - value: Some(vec![202]), - extrinsics: Some(vec![3].into_iter().collect()) - }) + (vec![100], History::from_iter(vec![ + (OverlayedValue { + value: Some(vec![202]), + extrinsics: Some(vec![3].into_iter().collect()) + }, State::Committed), + (OverlayedValue { + value: Some(vec![200]), + extrinsics: Some(vec![3, 0, 2].into_iter().collect()) + }, State::Transaction(0)), + ].into_iter().map(|(value, index)| HistoricalValue { value, index }))), + ].into_iter().collect()), + (child_trie_key2, vec![ + (vec![100], History::from_iter(vec![ + (OverlayedValue { + value: Some(vec![200]), + extrinsics: Some(vec![0, 2].into_iter().collect()) + }, State::Transaction(0)), + ].into_iter().map(|(value, index)| HistoricalValue { value, index }))), ].into_iter().collect()), ].into_iter().collect(), }, @@ -645,10 +643,12 @@ mod test { let (backend, storage, mut changes, config) = prepare_for_build(zero); // 110: missing from backend, set to None in overlay - changes.prospective.top.insert(vec![110], OverlayedValue { - value: None, - extrinsics: Some(vec![1].into_iter().collect()) - }); + changes.changes.top.insert(vec![110], History::from_iter(vec![ + (OverlayedValue { + value: None, + extrinsics: Some(vec![1].into_iter().collect()), + }, State::Transaction(0)), + ].into_iter().map(|(value, index)| HistoricalValue { value, index }))); let parent = AnchorBlockId { hash: Default::default(), number: zero + 3 }; let changes_trie_nodes = prepare_input( diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 3ac8c19048451..e3e954a3986be 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -162,8 +162,7 @@ where self.backend.pairs().iter() .map(|&(ref k, ref v)| (k.to_vec(), Some(v.to_vec()))) - .chain(self.overlay.committed.top.clone().into_iter().map(|(k, v)| (k, v.value))) - .chain(self.overlay.prospective.top.clone().into_iter().map(|(k, v)| (k, v.value))) + .chain(self.overlay.changes.iter_values(None).map(|(k, v)| (k.to_vec(), v.map(|s| s.to_vec())))) .collect::>() .into_iter() .filter_map(|(k, maybe_val)| maybe_val.map(|val| (k, val))) @@ -465,21 +464,10 @@ where return root.encode(); } - let child_storage_keys = - self.overlay.prospective.children.keys() - .chain(self.overlay.committed.children.keys()); - let child_delta_iter = child_storage_keys.map(|storage_key| - (storage_key.clone(), self.overlay.committed.children.get(storage_key) - .into_iter() - .flat_map(|map| map.iter().map(|(k, v)| (k.clone(), v.value.clone()))) - .chain(self.overlay.prospective.children.get(storage_key) - .into_iter() - .flat_map(|map| map.iter().map(|(k, v)| (k.clone(), v.value.clone())))))); - + let child_delta_iter = self.overlay.changes.owned_children_iter(); // compute and memoize - let delta = self.overlay.committed.top.iter().map(|(k, v)| (k.clone(), v.value.clone())) - .chain(self.overlay.prospective.top.iter().map(|(k, v)| (k.clone(), v.value.clone()))); + let delta = self.overlay.changes.iter_values(None).map(|(k, v)| (k.to_vec(), v.map(|s| s.to_vec()))); let (root, transaction) = self.backend.full_storage_root(delta, child_delta_iter); self.storage_transaction = Some((transaction, root)); @@ -508,16 +496,10 @@ where } else { let storage_key = storage_key.as_ref(); - let (root, is_empty, _) = { - let delta = self.overlay.committed.children.get(storage_key) - .into_iter() - .flat_map(|map| map.clone().into_iter().map(|(k, v)| (k, v.value))) - .chain(self.overlay.prospective.children.get(storage_key) - .into_iter() - .flat_map(|map| map.clone().into_iter().map(|(k, v)| (k, v.value)))); + let delta = self.overlay.changes.iter_values(Some(storage_key)) + .map(|(k, v)| (k.to_vec(), v.map(|s| s.to_vec()))); - self.backend.child_storage_root(storage_key, delta) - }; + let (root, is_empty, _) = self.backend.child_storage_root(storage_key, delta); if is_empty { self.overlay.set_storage(storage_key.into(), None); @@ -560,6 +542,22 @@ where ); result } + + fn storage_start_transaction(&mut self) { + let _guard = panic_handler::AbortGuard::force_abort(); + self.overlay.start_transaction() + } + + fn storage_discard_transaction(&mut self) { + let _guard = panic_handler::AbortGuard::force_abort(); + self.overlay.discard_transaction() + } + + fn storage_commit_transaction(&mut self) { + let _guard = panic_handler::AbortGuard::force_abort(); + self.overlay.commit_transaction() + } + } impl<'a, H, B, T, N> externalities::ExtensionStore for Ext<'a, H, N, B, T> @@ -579,35 +577,43 @@ mod tests { use super::*; use hex_literal::hex; use codec::Encode; - use primitives::{Blake2Hasher, storage::well_known_keys::EXTRINSIC_INDEX}; - use crate::{ - changes_trie::{ - Configuration as ChangesTrieConfiguration, - InMemoryStorage as InMemoryChangesTrieStorage, - }, backend::InMemory, overlayed_changes::OverlayedValue, - }; + use primitives::{Blake2Hasher}; + use primitives::storage::well_known_keys::EXTRINSIC_INDEX; + use crate::backend::InMemory; + use crate::changes_trie::{Configuration as ChangesTrieConfiguration, + InMemoryStorage as InMemoryChangesTrieStorage}; + use crate::overlayed_changes::{OverlayedValue, OverlayedChangeSet}; + use sp_historical_data::synch_linear_transaction::{History, HistoricalValue, State}; type TestBackend = InMemory; type TestChangesTrieStorage = InMemoryChangesTrieStorage; type TestExt<'a> = Ext<'a, Blake2Hasher, u64, TestBackend, TestChangesTrieStorage>; fn prepare_overlay_with_changes() -> OverlayedChanges { + OverlayedChanges { - prospective: vec![ - (EXTRINSIC_INDEX.to_vec(), OverlayedValue { - value: Some(3u32.encode()), - extrinsics: Some(vec![1].into_iter().collect()) - }), - (vec![1], OverlayedValue { - value: Some(vec![100].into_iter().collect()), - extrinsics: Some(vec![1].into_iter().collect()) - }), - ].into_iter().collect(), - committed: Default::default(), changes_trie_config: Some(ChangesTrieConfiguration { digest_interval: 0, digest_levels: 0, }), + changes: OverlayedChangeSet { + states: Default::default(), + children: Default::default(), + top: vec![ + (EXTRINSIC_INDEX.to_vec(), History::from_iter(vec![ + (OverlayedValue { + value: Some(3u32.encode()), + extrinsics: Some(vec![1].into_iter().collect()) + }, State::Committed), + ].into_iter().map(|(value, index)| HistoricalValue { value, index }))), + (vec![1], History::from_iter(vec![ + (OverlayedValue { + value: Some(vec![100]), + extrinsics: Some(vec![1].into_iter().collect()) + }, State::Committed), + ].into_iter().map(|(value, index)| HistoricalValue { value, index }))), + ].into_iter().collect(), + }, } } @@ -644,7 +650,9 @@ mod tests { #[test] fn storage_changes_root_is_some_when_extrinsic_changes_are_empty() { let mut overlay = prepare_overlay_with_changes(); - overlay.prospective.top.get_mut(&vec![1]).unwrap().value = None; + let conf = overlay.remove_changes_trie_config().unwrap(); + overlay.set_storage(vec![1], None); + overlay.set_changes_trie_config(conf); let storage = TestChangesTrieStorage::with_blocks(vec![(99, Default::default())]); let backend = TestBackend::default(); let mut ext = TestExt::new(&mut overlay, &backend, Some(&storage), None); diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 594e539b25fe9..15535a775cfc3 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -26,7 +26,6 @@ use primitives::{ storage::well_known_keys, NativeOrEncoded, NeverNativeValue, traits::CodeExecutor, hexdisplay::HexDisplay, hash::H256, }; -use overlayed_changes::OverlayedChangeSet; use externalities::Extensions; pub mod backend; @@ -289,7 +288,6 @@ impl<'a, B, H, N, T, Exec> StateMachine<'a, B, H, N, T, Exec> where &mut self, compute_tx: bool, mut native_call: Option, - orig_prospective: OverlayedChangeSet, on_consensus_failure: Handler, ) -> ( CallResult, @@ -303,6 +301,7 @@ impl<'a, B, H, N, T, Exec> StateMachine<'a, B, H, N, T, Exec> where CallResult, ) -> CallResult { + let orig_committed = self.overlay.changes.clone(); let (result, was_native, storage_delta, changes_delta) = self.execute_aux( compute_tx, true, @@ -310,7 +309,7 @@ impl<'a, B, H, N, T, Exec> StateMachine<'a, B, H, N, T, Exec> where ); if was_native { - self.overlay.prospective = orig_prospective.clone(); + self.overlay.changes = orig_committed; let (wasm_result, _, wasm_storage_delta, wasm_changes_delta) = self.execute_aux( compute_tx, false, @@ -334,7 +333,6 @@ impl<'a, B, H, N, T, Exec> StateMachine<'a, B, H, N, T, Exec> where &mut self, compute_tx: bool, mut native_call: Option, - orig_prospective: OverlayedChangeSet, ) -> ( CallResult, Option<(B::Transaction, H::Out)>, @@ -352,7 +350,7 @@ impl<'a, B, H, N, T, Exec> StateMachine<'a, B, H, N, T, Exec> where if !was_native || result.is_ok() { (result, storage_delta, changes_delta) } else { - self.overlay.prospective = orig_prospective.clone(); + self.overlay.discard_prospective(); let (wasm_result, _, wasm_storage_delta, wasm_changes_delta) = self.execute_aux( compute_tx, false, @@ -403,14 +401,12 @@ impl<'a, B, H, N, T, Exec> StateMachine<'a, B, H, N, T, Exec> where init_overlay(self.overlay, false, &self.backend)?; let result = { - let orig_prospective = self.overlay.prospective.clone(); let (result, storage_delta, changes_delta) = match manager { ExecutionManager::Both(on_consensus_failure) => { self.execute_call_with_both_strategy( compute_tx, native_call.take(), - orig_prospective, on_consensus_failure, ) }, @@ -418,7 +414,6 @@ impl<'a, B, H, N, T, Exec> StateMachine<'a, B, H, N, T, Exec> where self.execute_call_with_native_else_wasm_strategy( compute_tx, native_call.take(), - orig_prospective, ) }, ExecutionManager::AlwaysWasm(trust_level) => { @@ -733,7 +728,6 @@ fn try_read_overlay_value( mod tests { use std::collections::BTreeMap; use codec::Encode; - use overlayed_changes::OverlayedValue; use super::*; use super::backend::InMemory; use super::ext::Ext; @@ -929,17 +923,14 @@ mod tests { ]; let mut state = InMemory::::from(initial); let backend = state.as_trie_backend().unwrap(); - let mut overlay = OverlayedChanges { - committed: map![ - b"aba".to_vec() => OverlayedValue::from(Some(b"1312".to_vec())), - b"bab".to_vec() => OverlayedValue::from(Some(b"228".to_vec())) + let mut overlay = OverlayedChanges::new_from_top(vec![ + (b"aba".to_vec(), Some(b"1312".to_vec())), + (b"bab".to_vec(), Some(b"228".to_vec())), ], - prospective: map![ - b"abd".to_vec() => OverlayedValue::from(Some(b"69".to_vec())), - b"bbd".to_vec() => OverlayedValue::from(Some(b"42".to_vec())) - ], - ..Default::default() - }; + vec![ + (b"abd".to_vec(), Some(b"69".to_vec())), + (b"bbd".to_vec(), Some(b"42".to_vec())), + ], None); { let changes_trie_storage = InMemoryChangesTrieStorage::::new(); @@ -953,8 +944,11 @@ mod tests { } overlay.commit_prospective(); + overlay.discard_prospective(); + let values: HashMap<_, _> = overlay.changes.iter_values(None) + .map(|(k, v)| (k.to_vec(), v.map(|s| s.to_vec()))).collect(); assert_eq!( - overlay.committed, + values, map![ b"abc".to_vec() => None.into(), b"abb".to_vec() => None.into(), diff --git a/primitives/state-machine/src/overlayed_changes.rs b/primitives/state-machine/src/overlayed_changes.rs index d61d14961da36..a5d634e6a23b0 100644 --- a/primitives/state-machine/src/overlayed_changes.rs +++ b/primitives/state-machine/src/overlayed_changes.rs @@ -22,7 +22,11 @@ use std::collections::{HashMap, BTreeMap, BTreeSet}; use codec::Decode; use crate::changes_trie::{NO_EXTRINSIC_INDEX, Configuration as ChangesTrieConfig}; use primitives::storage::well_known_keys::EXTRINSIC_INDEX; -use std::{mem, ops}; +use sp_historical_data::synch_linear_transaction::{ + History, HistoricalValue, States, +}; +use sp_historical_data::CleaningResult; +use std::ops; /// The overlayed changes to state to be queried on top of the backend. /// @@ -30,10 +34,8 @@ use std::{mem, ops}; /// that can be cleared. #[derive(Debug, Default, Clone)] pub struct OverlayedChanges { - /// Changes that are not yet committed. - pub(crate) prospective: OverlayedChangeSet, - /// Committed changes. - pub(crate) committed: OverlayedChangeSet, + /// Changes with their history. + pub(crate) changes: OverlayedChangeSet, /// Changes trie configuration. None by default, but could be installed by the /// runtime if it supports change tries. pub(crate) changes_trie_config: Option, @@ -50,23 +52,90 @@ pub struct OverlayedValue { pub extrinsics: Option>, } -/// Prospective or committed overlayed change set. -#[derive(Debug, Default, Clone)] +/// Overlayed change set, keep history of values. +/// +/// It stores hashmap containing a linear history of value. +/// Update on commit and discard operation (prospective or +/// overlay), are done in a synchronous manner by updating +/// all keys of the hashmap. +#[derive(Debug, Clone, Default)] #[cfg_attr(test, derive(PartialEq))] pub struct OverlayedChangeSet { + /// Indexed state history. + pub(crate) states: States, /// Top level storage changes. - pub top: BTreeMap, OverlayedValue>, + pub(crate) top: BTreeMap, History>, /// Child storage changes. - pub children: HashMap, BTreeMap, OverlayedValue>>, + pub(crate) children: HashMap, BTreeMap, History>>, } #[cfg(test)] impl FromIterator<(Vec, OverlayedValue)> for OverlayedChangeSet { fn from_iter, OverlayedValue)>>(iter: T) -> Self { - Self { - top: iter.into_iter().collect(), - children: Default::default(), + use sp_historical_data::synch_linear_transaction::State; + let mut result = OverlayedChangeSet::default(); + result.top = iter.into_iter().map(|(k, value)| (k, { + let mut history = History::default(); + history.push_unchecked(HistoricalValue { value, index: State::Committed }); + history + })).collect(); + result + } +} + +/// Variant of `set` value that update extrinsics. +/// It does remove latest dropped values. +fn set_with_extrinsic_overlayed_value( + history: &mut History, + states: &States, + value: Option>, + extrinsic_index: Option, +) { + if let Some(extrinsic) = extrinsic_index { + set_with_extrinsic_inner_overlayed_value(history, states, value, extrinsic) + } else { + history.set(states, OverlayedValue { + value, + extrinsics: None, + }) + } +} + +fn set_with_extrinsic_inner_overlayed_value( + history: &mut History, + states: &States, + value: Option>, + extrinsic_index: u32, +) { + let state = states.as_state(); + if let Some(current) = history.get_mut() { + if current.index == state { + current.value.value = value; + current.value.extrinsics.get_or_insert_with(Default::default) + .insert(extrinsic_index); + } else { + let mut extrinsics = current.value.extrinsics.clone(); + extrinsics.get_or_insert_with(Default::default) + .insert(extrinsic_index); + history.push_unchecked(HistoricalValue { + index: state, + value: OverlayedValue { + value, + extrinsics, + }, + }); } + } else { + let mut extrinsics: Option> = None; + extrinsics.get_or_insert_with(Default::default) + .insert(extrinsic_index); + history.push_unchecked(HistoricalValue { + index: state, + value: OverlayedValue { + value, + extrinsics, + }, + }); } } @@ -76,17 +145,152 @@ impl OverlayedChangeSet { self.top.is_empty() && self.children.is_empty() } - /// Clear the change set. - pub fn clear(&mut self) { - self.top.clear(); - self.children.clear(); + /// Discard prospective changes to state. + pub fn discard_prospective(&mut self) { + self.states.discard_prospective(); + retain(&mut self.top, |_, history| States::apply_discard_prospective(history) != CleaningResult::Cleared); + self.children.retain(|_, mut m| { + retain(&mut m, |_, history| States::apply_discard_prospective(history) != CleaningResult::Cleared); + !m.is_empty() + }); + } + + /// Commit prospective changes to state. + pub fn commit_prospective(&mut self) { + self.states.commit_prospective(); + retain(&mut self.top, |_, history| States::apply_commit_prospective(history) != CleaningResult::Cleared); + self.children.retain(|_, mut m| { + retain(&mut m, |_, history| States::apply_commit_prospective(history) != CleaningResult::Cleared); + !m.is_empty() + }); + } + + /// Create a new transactional layer. + pub fn start_transaction(&mut self) { + self.states.start_transaction(); + } + + /// Discard a transactional layer. + /// A transaction is always running (states always end with pending). + pub fn discard_transaction(&mut self) { + self.states.discard_transaction(); + let states = &self.states; + retain(&mut self.top, |_, history| states.apply_discard_transaction(history) != CleaningResult::Cleared); + self.children.retain(|_, mut m| { + retain(&mut m, |_, history| states.apply_discard_transaction(history) != CleaningResult::Cleared); + !m.is_empty() + }); + self.states.finalize_discard(); + } + + /// Commit a transactional layer. + pub fn commit_transaction(&mut self) { + self.states.commit_transaction(); + let states = &self.states; + retain(&mut self.top, |_, history| states.apply_commit_transaction(history) != CleaningResult::Cleared); + self.children.retain(|_, mut m| { + retain(&mut m, |_, history| states.apply_commit_transaction(history) != CleaningResult::Cleared); + !m.is_empty() + }); + } + + /// Iterator over current state of a given overlay, including change trie information. + pub fn iter_overlay( + &self, + storage_key: Option<&[u8]>, + ) -> impl Iterator { + let option_map = if let Some(storage_key) = storage_key.as_ref() { + self.children.get(*storage_key) + } else { + Some(&self.top) + }; + option_map + .into_iter() + .flat_map(move |map| map.iter() + .filter_map(move |(k, v)| + v.get().map(|v| (k.as_slice(), v))) + ) + } + + /// Iterator over current state of a given overlay, values only. + pub fn iter_values( + &self, + storage_key: Option<&[u8]>, + ) -> impl Iterator)> { + self.iter_overlay(storage_key) + .map(|(k, v)| (k, v.value.as_ref().map(|v| v.as_slice()))) + } + + /// Iterator over current state of all children overlays, values only. + pub fn children_iter( + &self, + ) -> impl Iterator)>)> { + self.children.iter() + .map(move |(keyspace, child)| (keyspace.as_slice(), child.iter() + .filter_map(move |(k, v)| + v.get() + .map(|v| (k.as_slice(), v.value.as_ref().map(|v| v.as_slice())))) + )) + } + + /// Iterator over current state of all children overlays, values only. + /// Similar to `children_iter` but with key and value as `Vec`. + pub fn owned_children_iter<'a>( + &'a self, + ) -> impl Iterator, impl Iterator, Option>)> + 'a)> + 'a { + self.children.iter() + .map(move |(keyspace, child)| (keyspace.to_vec(), child.iter() + .filter_map(move |(k, v)| + v.get() + .map(|v| (k.to_vec(), v.value.as_ref().map(|v| v.to_vec())))) + )) + } + + /// Iterator over current state of all children overlays, including change trie information. + pub fn children_iter_overlay( + &self, + ) -> impl Iterator)> { + self.children.iter() + .map(move |(keyspace, child)| (keyspace.as_slice(), child.iter() + .filter_map(move |(k, v)| + v.get().map(|v| (k.as_slice(), v))) + )) } + + /// Test only method to access current prospective changes. + /// It is here to keep old test compatibility and should be + /// avoid for new tests. + #[cfg(test)] + pub(crate) fn top_prospective(&self) -> BTreeMap, OverlayedValue> { + let mut result = BTreeMap::new(); + for (k, v) in self.top.iter() { + if let Some(v) = v.get_prospective() { + result.insert(k.clone(), v.clone()); + } + } + result + } + + /// Test only method to access current commited changes. + /// It is here to keep old test compatibility and should be + /// avoid for new tests. + #[cfg(test)] + pub(crate) fn top_committed(&self) -> BTreeMap, OverlayedValue> { + let mut result = BTreeMap::new(); + for (k, v) in self.top.iter() { + if let Some(v) = v.get_committed() { + result.insert(k.clone(), v.clone()); + } + } + result + } + } impl OverlayedChanges { /// Whether the overlayed changes are empty. pub fn is_empty(&self) -> bool { - self.prospective.is_empty() && self.committed.is_empty() + self.changes.is_empty() } /// Sets the changes trie configuration. @@ -105,61 +309,65 @@ impl OverlayedChanges { true } + #[cfg(test)] + /// To allow value without extrinsic this can be use in test to disable change trie. + pub(crate) fn remove_changes_trie_config(&mut self) -> Option { + self.changes_trie_config.take() + } + /// Returns a double-Option: None if the key is unknown (i.e. and the query should be refered /// to the backend); Some(None) if the key has been deleted. Some(Some(...)) for a key whose /// value has been set. pub fn storage(&self, key: &[u8]) -> Option> { - self.prospective.top.get(key) - .or_else(|| self.committed.top.get(key)) - .map(|x| x.value.as_ref().map(AsRef::as_ref)) + if let Some(overlay_value) = self.changes.top.get(key) { + if let Some(o_value) = overlay_value.get() { + return Some(o_value.value.as_ref().map(|v| v.as_slice())) + } + } + None } /// Returns a double-Option: None if the key is unknown (i.e. and the query should be refered /// to the backend); Some(None) if the key has been deleted. Some(Some(...)) for a key whose /// value has been set. pub fn child_storage(&self, storage_key: &[u8], key: &[u8]) -> Option> { - if let Some(map) = self.prospective.children.get(storage_key) { - if let Some(val) = map.get(key) { - return Some(val.value.as_ref().map(AsRef::as_ref)); - } - } - - if let Some(map) = self.committed.children.get(storage_key) { - if let Some(val) = map.get(key) { - return Some(val.value.as_ref().map(AsRef::as_ref)); + if let Some(map) = self.changes.children.get(storage_key) { + if let Some(overlay_value) = map.get(key) { + if let Some(o_value) = overlay_value.get() { + return Some(o_value.value.as_ref().map(|v| v.as_slice())) + } } } - None } /// Inserts the given key-value pair into the prospective change set. /// /// `None` can be used to delete a value specified by the given key. - pub(crate) fn set_storage(&mut self, key: Vec, val: Option>) { + pub fn set_storage(&mut self, key: Vec, value: Option>) { let extrinsic_index = self.extrinsic_index(); - let entry = self.prospective.top.entry(key).or_default(); - entry.value = val; - - if let Some(extrinsic) = extrinsic_index { - entry.extrinsics.get_or_insert_with(Default::default) - .insert(extrinsic); - } + let entry = self.changes.top.entry(key).or_default(); + set_with_extrinsic_overlayed_value(entry, &self.changes.states, value, extrinsic_index); } /// Inserts the given key-value pair into the prospective child change set. /// /// `None` can be used to delete a value specified by the given key. - pub(crate) fn set_child_storage(&mut self, storage_key: Vec, key: Vec, val: Option>) { + pub(crate) fn set_child_storage( + &mut self, + storage_key: Vec, + key: Vec, + value: Option>, + ) { let extrinsic_index = self.extrinsic_index(); - let map_entry = self.prospective.children.entry(storage_key).or_default(); + let map_entry = self.changes.children.entry(storage_key).or_default(); let entry = map_entry.entry(key).or_default(); - entry.value = val; - - if let Some(extrinsic) = extrinsic_index { - entry.extrinsics.get_or_insert_with(Default::default) - .insert(extrinsic); - } + set_with_extrinsic_overlayed_value( + entry, + &self.changes.states, + value, + extrinsic_index, + ); } /// Clear child storage of given storage key. @@ -170,32 +378,11 @@ impl OverlayedChanges { /// [`discard_prospective`]: #method.discard_prospective pub(crate) fn clear_child_storage(&mut self, storage_key: &[u8]) { let extrinsic_index = self.extrinsic_index(); - let map_entry = self.prospective.children.entry(storage_key.to_vec()).or_default(); + let states = &self.changes.states; + let map_entry = self.changes.children.entry(storage_key.to_vec()).or_default(); - map_entry.values_mut().for_each(|e| { - if let Some(extrinsic) = extrinsic_index { - e.extrinsics.get_or_insert_with(Default::default) - .insert(extrinsic); - } - - e.value = None; - }); - - if let Some(committed_map) = self.committed.children.get(storage_key) { - for (key, value) in committed_map.iter() { - if !map_entry.contains_key(key) { - map_entry.insert(key.clone(), OverlayedValue { - value: None, - extrinsics: extrinsic_index.map(|i| { - let mut e = value.extrinsics.clone() - .unwrap_or_else(|| BTreeSet::default()); - e.insert(i); - e - }), - }); - } - } - } + map_entry.values_mut() + .for_each(|e| set_with_extrinsic_overlayed_value(e, states, None, extrinsic_index)); } /// Removes all key-value pairs which keys share the given prefix. @@ -207,61 +394,19 @@ impl OverlayedChanges { pub(crate) fn clear_prefix(&mut self, prefix: &[u8]) { let extrinsic_index = self.extrinsic_index(); - // Iterate over all prospective and mark all keys that share - // the given prefix as removed (None). - for (key, entry) in self.prospective.top.iter_mut() { - if key.starts_with(prefix) { - entry.value = None; - - if let Some(extrinsic) = extrinsic_index { - entry.extrinsics.get_or_insert_with(Default::default) - .insert(extrinsic); - } - } - } - - // Then do the same with keys from commited changes. - // NOTE that we are making changes in the prospective change set. - for key in self.committed.top.keys() { + for (key, entry) in self.changes.top.iter_mut() { if key.starts_with(prefix) { - let entry = self.prospective.top.entry(key.clone()).or_default(); - entry.value = None; - - if let Some(extrinsic) = extrinsic_index { - entry.extrinsics.get_or_insert_with(Default::default) - .insert(extrinsic); - } + set_with_extrinsic_overlayed_value(entry, &self.changes.states, None, extrinsic_index); } } } pub(crate) fn clear_child_prefix(&mut self, storage_key: &[u8], prefix: &[u8]) { let extrinsic_index = self.extrinsic_index(); - let map_entry = self.prospective.children.entry(storage_key.to_vec()).or_default(); - - for (key, entry) in map_entry.iter_mut() { - if key.starts_with(prefix) { - entry.value = None; - - if let Some(extrinsic) = extrinsic_index { - entry.extrinsics.get_or_insert_with(Default::default) - .insert(extrinsic); - } - } - } - - if let Some(child_committed) = self.committed.children.get(storage_key) { - // Then do the same with keys from commited changes. - // NOTE that we are making changes in the prospective change set. - for key in child_committed.keys() { + if let Some(child_change) = self.changes.children.get_mut(storage_key) { + for (key, entry) in child_change.iter_mut() { if key.starts_with(prefix) { - let entry = map_entry.entry(key.clone()).or_default(); - entry.value = None; - - if let Some(extrinsic) = extrinsic_index { - entry.extrinsics.get_or_insert_with(Default::default) - .insert(extrinsic); - } + set_with_extrinsic_overlayed_value(entry, &self.changes.states, None, extrinsic_index); } } } @@ -269,61 +414,73 @@ impl OverlayedChanges { /// Discard prospective changes to state. pub fn discard_prospective(&mut self) { - self.prospective.clear(); + self.changes.discard_prospective(); } /// Commit prospective changes to state. pub fn commit_prospective(&mut self) { - if self.committed.is_empty() { - mem::swap(&mut self.prospective, &mut self.committed); - } else { - let top_to_commit = mem::replace(&mut self.prospective.top, BTreeMap::new()); - for (key, val) in top_to_commit.into_iter() { - let entry = self.committed.top.entry(key).or_default(); - entry.value = val.value; - - if let Some(prospective_extrinsics) = val.extrinsics { - entry.extrinsics.get_or_insert_with(Default::default) - .extend(prospective_extrinsics); - } - } - for (storage_key, map) in self.prospective.children.drain() { - let map_dest = self.committed.children.entry(storage_key).or_default(); - for (key, val) in map.into_iter() { - let entry = map_dest.entry(key).or_default(); - entry.value = val.value; - - if let Some(prospective_extrinsics) = val.extrinsics { - entry.extrinsics.get_or_insert_with(Default::default) - .extend(prospective_extrinsics); - } - } - } - } + self.changes.commit_prospective(); + } + + /// Create a new transactional layer. + pub fn start_transaction(&mut self) { + self.changes.start_transaction(); + } + + /// Discard a transactional layer. + /// A transaction is always running (history always end with pending). + pub fn discard_transaction(&mut self) { + self.changes.discard_transaction(); + } + + /// Commit a transactional layer. + pub fn commit_transaction(&mut self) { + self.changes.commit_transaction(); } /// Consume `OverlayedChanges` and take committed set. - /// - /// Panics: - /// Will panic if there are any uncommitted prospective changes. pub fn into_committed(self) -> ( impl Iterator, Option>)>, impl Iterator, impl Iterator, Option>)>)>, ){ - assert!(self.prospective.is_empty()); - (self.committed.top.into_iter().map(|(k, v)| (k, v.value)), - self.committed.children.into_iter() - .map(|(sk, v)| (sk, v.into_iter().map(|(k, v)| (k, v.value))))) + let top = self.changes.top; + let children = self.changes.children; + ( + top.into_iter() + .filter_map(move |(k, v)| v.into_committed() + .map(|v| (k, v.value))), + children.into_iter().map(move |(sk, v)| { + (sk, v.into_iter() + .filter_map(move |(k, v)| v.into_committed() + .map(|v| (k, v.value)))) + }) + ) } /// Inserts storage entry responsible for current extrinsic index. #[cfg(test)] pub(crate) fn set_extrinsic_index(&mut self, extrinsic_index: u32) { use codec::Encode; - self.prospective.top.insert(EXTRINSIC_INDEX.to_vec(), OverlayedValue { - value: Some(extrinsic_index.encode()), - extrinsics: None, - }); + self.set_storage(EXTRINSIC_INDEX.to_vec(), Some(extrinsic_index.encode())); + } + + /// Test only method to build from committed info and prospective. + /// Create an history of two states. + #[cfg(test)] + pub(crate) fn new_from_top( + committed: Vec<(Vec, Option>)>, + prospective: Vec<(Vec, Option>)>, + changes_trie_config: Option, + ) -> Self { + let changes = OverlayedChangeSet::default(); + let mut result = OverlayedChanges { + changes, + changes_trie_config, + }; + committed.into_iter().for_each(|(k, v)| result.set_storage(k, v)); + result.changes.commit_prospective(); + prospective.into_iter().for_each(|(k, v)| result.set_storage(k, v)); + result } /// Returns current extrinsic index to use in changes trie construction. @@ -342,29 +499,38 @@ impl OverlayedChanges { } } + #[cfg(any(test, feature = "test-helpers"))] + /// Iterator over current state of the overlay. + pub fn iter_values( + &self, + storage_key: Option<&[u8]>, + ) -> impl Iterator)> { + self.changes.iter_values(storage_key) + } + + #[cfg(any(test, feature = "test-helpers"))] + /// Count (slow) the number of key value, history included. + /// Only for debugging or testing usage. + pub fn top_count_keyvalue_pair(&self) -> usize { + let mut result = 0; + for (_, v) in self.changes.top.iter() { + result += v.len() + } + result + } + /// Returns the next (in lexicographic order) storage key in the overlayed alongside its value. /// If no value is next then `None` is returned. pub fn next_storage_key_change(&self, key: &[u8]) -> Option<(&[u8], &OverlayedValue)> { let range = (ops::Bound::Excluded(key), ops::Bound::Unbounded); - let next_prospective_key = self.prospective.top - .range::<[u8], _>(range) - .next() - .map(|(k, v)| (&k[..], v)); - - let next_committed_key = self.committed.top - .range::<[u8], _>(range) - .next() - .map(|(k, v)| (&k[..], v)); - - match (next_committed_key, next_prospective_key) { - // Committed is strictly less than prospective - (Some(committed_key), Some(prospective_key)) if committed_key.0 < prospective_key.0 => - Some(committed_key), - (committed_key, None) => committed_key, - // Prospective key is less or equal to committed or committed doesn't exist - (_, prospective_key) => prospective_key, + let mut next_keys = self.changes.top.range::<[u8], _>(range); + while let Some((key, historic_value)) = next_keys.next() { + if let Some(overlay_value) = historic_value.get() { + return Some((key, overlay_value)); + } } + None } /// Returns the next (in lexicographic order) child storage key in the overlayed alongside its @@ -376,21 +542,41 @@ impl OverlayedChanges { ) -> Option<(&[u8], &OverlayedValue)> { let range = (ops::Bound::Excluded(key), ops::Bound::Unbounded); - let next_prospective_key = self.prospective.children.get(storage_key) - .and_then(|map| map.range::<[u8], _>(range).next().map(|(k, v)| (&k[..], v))); - - let next_committed_key = self.committed.children.get(storage_key) - .and_then(|map| map.range::<[u8], _>(range).next().map(|(k, v)| (&k[..], v))); + if let Some(child) = self.changes.children.get(storage_key) { + let mut next_keys = child.range::<[u8], _>(range); + while let Some((key, historic_value)) = next_keys.next() { + if let Some(overlay_value) = historic_value.get() { + return Some((key, overlay_value)); + } + } + } + None + } +} - match (next_committed_key, next_prospective_key) { - // Committed is strictly less than prospective - (Some(committed_key), Some(prospective_key)) if committed_key.0 < prospective_key.0 => - Some(committed_key), - (committed_key, None) => committed_key, - // Prospective key is less or equal to committed or committed doesn't exist - (_, prospective_key) => prospective_key, +/// This is an implementation of retain for btreemap, +/// this is used to keep a similar api with previous +/// hashmap usage. +/// This could also be easilly replace if btreemap gets +/// an implementation for retain in the future. +fn retain(m: &mut BTreeMap, mut f: F) where + F: FnMut(&K, &mut V) -> bool, { + // this is use to discard some historical values when + // their status is cleared. + // Regarding this use case we will only remove individually. + // Rebuilding trie from iterator may be better if there is + // more removal. + // Regarding this use case skipping removal can be a better + // choice. + let mut rem = Vec::new(); + for (k, v) in m.iter_mut() { + if !f(k, v) { + rem.push(k.clone()); } } + for k in rem { + m.remove(&k); + } } #[cfg(test)] @@ -411,12 +597,11 @@ mod tests { use crate::ext::Ext; use super::*; - fn strip_extrinsic_index(map: &BTreeMap, OverlayedValue>) + fn strip_extrinsic_index(mut map: BTreeMap, OverlayedValue>) -> BTreeMap, OverlayedValue> { - let mut clone = map.clone(); - clone.remove(&EXTRINSIC_INDEX.to_vec()); - clone + map.remove(&EXTRINSIC_INDEX.to_vec()); + map } #[test] @@ -456,18 +641,16 @@ mod tests { (b"doug".to_vec(), b"notadog".to_vec()), ].into_iter().collect(); let backend = InMemory::::from(initial); - let mut overlay = OverlayedChanges { - committed: vec![ + let mut overlay = OverlayedChanges::new_from_top( + vec![ (b"dog".to_vec(), Some(b"puppy".to_vec()).into()), (b"dogglesworth".to_vec(), Some(b"catYYY".to_vec()).into()), (b"doug".to_vec(), Some(vec![]).into()), ].into_iter().collect(), - prospective: vec![ + vec![ (b"dogglesworth".to_vec(), Some(b"cat".to_vec()).into()), (b"doug".to_vec(), None.into()), - ].into_iter().collect(), - ..Default::default() - }; + ].into_iter().collect(), None); let changes_trie_storage = InMemoryChangesTrieStorage::::new(); let mut ext = Ext::new( @@ -507,7 +690,7 @@ mod tests { digest_interval: 4, digest_levels: 1, }), true); assert_eq!( - strip_extrinsic_index(&overlay.prospective.top), + strip_extrinsic_index(overlay.changes.top_prospective()), vec![ (vec![1], OverlayedValue { value: Some(vec![2]), extrinsics: Some(vec![0].into_iter().collect()) }), @@ -544,7 +727,7 @@ mod tests { overlay.set_extrinsic_index(2); overlay.set_storage(vec![1], Some(vec![6])); - assert_eq!(strip_extrinsic_index(&overlay.prospective.top), + assert_eq!(strip_extrinsic_index(overlay.changes.top_prospective()), vec![ (vec![1], OverlayedValue { value: Some(vec![6]), extrinsics: Some(vec![0, 2].into_iter().collect()) }), @@ -562,7 +745,7 @@ mod tests { overlay.set_extrinsic_index(4); overlay.set_storage(vec![1], Some(vec![8])); - assert_eq!(strip_extrinsic_index(&overlay.committed.top), + assert_eq!(strip_extrinsic_index(overlay.changes.top_committed()), vec![ (vec![1], OverlayedValue { value: Some(vec![6]), extrinsics: Some(vec![0, 2].into_iter().collect()) }), @@ -572,17 +755,17 @@ mod tests { extrinsics: Some(vec![NO_EXTRINSIC_INDEX].into_iter().collect()) }), ].into_iter().collect()); - assert_eq!(strip_extrinsic_index(&overlay.prospective.top), + assert_eq!(strip_extrinsic_index(overlay.changes.top_prospective()), vec![ (vec![1], OverlayedValue { value: Some(vec![8]), - extrinsics: Some(vec![4].into_iter().collect()) }), + extrinsics: Some(vec![0, 2, 4].into_iter().collect()) }), (vec![3], OverlayedValue { value: Some(vec![7]), - extrinsics: Some(vec![3].into_iter().collect()) }), + extrinsics: Some(vec![1, 3].into_iter().collect()) }), ].into_iter().collect()); overlay.commit_prospective(); - assert_eq!(strip_extrinsic_index(&overlay.committed.top), + assert_eq!(strip_extrinsic_index(overlay.changes.top_committed()), vec![ (vec![1], OverlayedValue { value: Some(vec![8]), extrinsics: Some(vec![0, 2, 4].into_iter().collect()) }), @@ -592,10 +775,83 @@ mod tests { extrinsics: Some(vec![NO_EXTRINSIC_INDEX].into_iter().collect()) }), ].into_iter().collect()); - assert_eq!(overlay.prospective, + assert_eq!(overlay.changes.top_prospective(), Default::default()); } + #[test] + fn overlayed_storage_transactions() { + let mut overlayed = OverlayedChanges::default(); + + let key = vec![42, 69, 169, 142]; + + assert!(overlayed.storage(&key).is_none()); + + // discard transaction similar to discard prospective if no transaction. + + overlayed.set_storage(key.clone(), Some(vec![1, 2, 3])); + assert_eq!(overlayed.storage(&key).unwrap(), Some(&[1, 2, 3][..])); + + overlayed.discard_transaction(); + assert_eq!(overlayed.storage(&key), None); + + overlayed.discard_prospective(); + assert_eq!(overlayed.storage(&key), None); + + overlayed.set_storage(key.clone(), Some(vec![1, 2, 3])); + assert_eq!(overlayed.storage(&key).unwrap(), Some(&[1, 2, 3][..])); + + overlayed.commit_transaction(); + assert_eq!(overlayed.storage(&key).unwrap(), Some(&[1, 2, 3][..])); + + overlayed.discard_transaction(); + assert_eq!(overlayed.storage(&key), None); + // basic transaction test + // tx:1 + overlayed.set_storage(key.clone(), Some(vec![1, 2, 3])); + assert_eq!(overlayed.storage(&key).unwrap(), Some(&[1, 2, 3][..])); + + overlayed.start_transaction(); + // tx:2 + overlayed.set_storage(key.clone(), Some(vec![1, 2, 3, 4])); + assert_eq!(overlayed.storage(&key).unwrap(), Some(&[1, 2, 3, 4][..])); + + overlayed.start_transaction(); + // tx:3 + overlayed.set_storage(key.clone(), None); + assert_eq!(overlayed.storage(&key).unwrap(), None); + + overlayed.discard_transaction(); + // tx:2 + assert_eq!(overlayed.storage(&key).unwrap(), Some(&[1, 2, 3, 4][..])); + + overlayed.start_transaction(); + // tx:3 + overlayed.set_storage(key.clone(), None); + assert_eq!(overlayed.storage(&key).unwrap(), None); + + overlayed.commit_transaction(); + // tx:2 + assert_eq!(overlayed.storage(&key).unwrap(), None); + + overlayed.discard_transaction(); + // tx:1 + assert_eq!(overlayed.storage(&key).unwrap(), Some(&[1, 2, 3][..])); + overlayed.discard_prospective(); + assert_eq!(overlayed.storage(&key), None); + + // multiple transaction end up to prospective value + overlayed.start_transaction(); + overlayed.set_storage(key.clone(), Some(vec![1])); + overlayed.start_transaction(); + overlayed.set_storage(key.clone(), Some(vec![1, 2])); + overlayed.start_transaction(); + overlayed.set_storage(key.clone(), Some(vec![1, 2, 3])); + + overlayed.commit_prospective(); + assert_eq!(overlayed.storage(&key).unwrap(), Some(&[1, 2, 3][..])); + } + #[test] fn next_storage_key_change_works() { let mut overlay = OverlayedChanges::default(); diff --git a/primitives/state-machine/src/testing.rs b/primitives/state-machine/src/testing.rs index 85c9260fba3db..c6ae8b16de76e 100644 --- a/primitives/state-machine/src/testing.rs +++ b/primitives/state-machine/src/testing.rs @@ -107,17 +107,14 @@ impl, N: ChangesTrieBlockNumber> TestExternalities { /// Return a new backend with all pending value. pub fn commit_all(&self) -> InMemory { - let top = self.overlay.committed.top.clone().into_iter() - .chain(self.overlay.prospective.top.clone().into_iter()) - .map(|(k, v)| (None, k, v.value)); - - let children = self.overlay.committed.children.clone().into_iter() - .chain(self.overlay.prospective.children.clone().into_iter()) - .flat_map(|(keyspace, map)| { - map.into_iter() - .map(|(k, v)| (Some(keyspace.clone()), k, v.value)) - .collect::>() - }); + let top = self.overlay.changes.iter_overlay(None) + .map(|(k, v)| (None, k.to_vec(), v.value.clone())); + + let children = self.overlay.changes.children_iter_overlay() + .flat_map(|(keyspace, map)| map + .map(|(k, v)| (Some(keyspace.to_vec()), k.to_vec(), v.value.clone())) + .collect::>() + ); self.backend.update(top.chain(children).collect()) } diff --git a/test-utils/runtime/Cargo.toml b/test-utils/runtime/Cargo.toml index c669cd4141318..a36730900cc17 100644 --- a/test-utils/runtime/Cargo.toml +++ b/test-utils/runtime/Cargo.toml @@ -36,6 +36,7 @@ sc-client = { path = "../../client", optional = true } sp-trie = { path = "../../primitives/trie", default-features = false } sp-transaction-pool = { package = "sp-transaction-pool", path = "../../primitives/transaction-pool", default-features = false } trie-db = { version = "0.16.0", default-features = false } +sp-historical-data = { path = "../../primitives/historical-data", default-features = false } [dev-dependencies] sc-executor = { path = "../../client/executor" } @@ -79,5 +80,6 @@ std = [ "sc-client", "sp-trie/std", "sp-transaction-pool/std", + "sp-historical-data/std", "trie-db/std", ] diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index dcb76e27efbd6..0a62e458bd7c3 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -253,8 +253,12 @@ cfg_if! { fn function_signature_changed() -> u64; fn fail_on_native() -> u64; fn fail_on_wasm() -> u64; - /// trie no_std testing + /// Trie no_std testing. fn use_trie() -> u64; + /// History data no_std testing. + fn use_history_data(); + /// Transactional tests. + fn use_transactions() -> u64; fn benchmark_indirect_call() -> u64; fn benchmark_direct_call() -> u64; fn returns_mutable_static() -> u64; @@ -299,6 +303,10 @@ cfg_if! { fn fail_on_wasm() -> u64; /// trie no_std testing fn use_trie() -> u64; + /// History data no_std testing. + fn use_history_data(); + /// Transactional tests. + fn use_transactions() -> u64; fn benchmark_indirect_call() -> u64; fn benchmark_direct_call() -> u64; fn returns_mutable_static() -> u64; @@ -551,6 +559,14 @@ cfg_if! { code_using_trie() } + fn use_history_data() { + test_historical_data() + } + + fn use_transactions() -> u64 { + system::test_transactions() + } + fn benchmark_indirect_call() -> u64 { let function = benchmark_add_one; (0..1000).fold(0, |p, i| p + function(i)) @@ -740,6 +756,14 @@ cfg_if! { code_using_trie() } + fn use_history_data() { + test_historical_data() + } + + fn use_transactions() -> u64 { + system::test_transactions() + } + fn benchmark_indirect_call() -> u64 { (0..10000).fold(0, |p, i| p + BENCHMARK_ADD_ONE.get()(i)) } @@ -889,6 +913,30 @@ fn test_sr25519_crypto() -> (sr25519::AppSignature, sr25519::AppPublic) { (signature, public0) } +fn test_historical_data() { + let mut states = sp_historical_data::synch_linear_transaction::States::default(); + let mut value = sp_historical_data::synch_linear_transaction::History::default(); + if value.get() != None { + panic!("Got a value for empty data"); + } + + value.set(&states, 42u64); + states.start_transaction(); + if value.get() != Some(&42) { + panic!("Got a wrong result accessing a one element data"); + } + value.set(&states, 43u64); + if value.get() != Some(&43) { + panic!("Got a wrong result accessing a two element data"); + } + states.discard_transaction(); + states.apply_discard_transaction(&mut value); + states.finalize_discard(); + if value.get() != Some(&42) { + panic!("Got a wrong result accessing a one element data after a discard"); + } +} + fn test_read_storage() { const KEY: &[u8] = b":read_storage"; sp_io::storage::set(KEY, b"test"); diff --git a/test-utils/runtime/src/system.rs b/test-utils/runtime/src/system.rs index d305220d5f873..517f258caa4b4 100644 --- a/test-utils/runtime/src/system.rs +++ b/test-utils/runtime/src/system.rs @@ -94,6 +94,35 @@ pub fn take_block_number() -> Option { Number::take() } +/// Return 1 if test successfull, 0 otherwhise. +pub fn test_transactions() -> u64 { + let origin_value: Option = storage::unhashed::get(well_known_keys::EXTRINSIC_INDEX); + let result: Result<(), _> = storage::with_transaction(|| { + if let Ok(val) = storage::with_transaction(|| { + storage::unhashed::put(well_known_keys::EXTRINSIC_INDEX, &99u32); + if storage::unhashed::get(well_known_keys::EXTRINSIC_INDEX) == Some(99u32) { + Ok(99u32) + } else { + Err("Error setting value will revert") + } + }) { + if storage::unhashed::get(well_known_keys::EXTRINSIC_INDEX) == Some(val) { + Err("expected revert for test") + } else { + Err("Unexpected revert") + } + } else { + Err("Unexpected revert") + } + }); + if result == Err("expected revert for test") { + if storage::unhashed::get(well_known_keys::EXTRINSIC_INDEX) == origin_value { + return 1; + } + } + 0 +} + #[derive(Copy, Clone)] enum Mode { Verify,