Skip to content

Commit

Permalink
Add proper commit_all to TestExternalities (paritytech#7808)
Browse files Browse the repository at this point in the history
* Add proper `commit_all` to `TestExternalities`

This pr adds a propoer `commit_all` function to `TestExternalities` to
commit all changes from the overlay to the internal backend. Besides
that it fixes some bugs with handling empty dbs when calculating a delta
storage root. It also changes the way data is added to the in memory
backend.

* Update primitives/state-machine/src/testing.rs

Co-authored-by: cheme <emericchevalier.pro@gmail.com>

* Don't allow self proxies (paritytech#7803)

* Allow council to slash treasury tip (paritytech#7753)

* wk2051 | D4 |Allow council to slash treasury tip | p1

* Update frame/tips/src/lib.rs

Co-authored-by: Xiliang Chen <xlchen1291@gmail.com>

* wk2051 | D5 |Allow council to slash treasury tip | p2

* wk2051 | D5 |Allow council to slash treasury tip | p3

* wk2051 | D5 |Allow council to slash treasury tip | p4

* wk2051 | D5 |Allow council to slash treasury tip | p5

* random change

* cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_tips --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/tips/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* fix typo

* Update frame/tips/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/tips/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/tips/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/tips/src/lib.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update frame/tips/src/tests.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* wk2052 | D1 | Allow council to slash treasury tip | p6

Co-authored-by: Xiliang Chen <xlchen1291@gmail.com>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Parity Benchmarking Bot <admin@parity.io>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Review feedback

* Review feedback

* Update docs

* More docs

* Make it private

* Use `None`

* Use apply transaction

* Update primitives/state-machine/src/testing.rs

Co-authored-by: cheme <emericchevalier.pro@gmail.com>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: RK <r.raajey@gmail.com>
Co-authored-by: Xiliang Chen <xlchen1291@gmail.com>
Co-authored-by: Parity Benchmarking Bot <admin@parity.io>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
  • Loading branch information
8 people authored and darkfriend77 committed Jan 11, 2021
1 parent b87c451 commit 2b0dae3
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 72 deletions.
99 changes: 34 additions & 65 deletions primitives/state-machine/src/in_memory_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,56 +17,21 @@
//! State machine in memory backend.

use crate::{
StorageKey, StorageValue, StorageCollection,
trie_backend::TrieBackend,
StorageKey, StorageValue, StorageCollection, trie_backend::TrieBackend, backend::Backend,
};
use std::{collections::{BTreeMap, HashMap}};
use std::collections::{BTreeMap, HashMap};
use hash_db::Hasher;
use sp_trie::{
MemoryDB, TrieMut,
trie_types::TrieDBMut,
};
use sp_trie::{MemoryDB, empty_trie_root, Layout};
use codec::Codec;
use sp_core::storage::{ChildInfo, Storage};

/// Insert input pairs into memory db.
fn insert_into_memory_db<H, I>(mut root: H::Out, mdb: &mut MemoryDB<H>, input: I) -> H::Out
where
H: Hasher,
I: IntoIterator<Item=(StorageKey, Option<StorageValue>)>,
{
{
let mut trie = if root == Default::default() {
TrieDBMut::<H>::new(mdb, &mut root)
} else {
TrieDBMut::<H>::from_existing(mdb, &mut root).unwrap()
};
for (key, value) in input {
if let Err(e) = match value {
Some(value) => {
trie.insert(&key, &value)
},
None => {
trie.remove(&key)
},
} {
panic!("Failed to write to trie: {}", e);
}
}
trie.commit();
}
root
}

/// Create a new empty instance of in-memory backend.
pub fn new_in_mem<H: Hasher>() -> TrieBackend<MemoryDB<H>, H>
where
H::Out: Codec + Ord,
{
let db = MemoryDB::default();
let mut backend = TrieBackend::new(db, Default::default());
backend.insert(std::iter::empty());
backend
TrieBackend::new(db, empty_trie_root::<Layout<H>>())
}

impl<H: Hasher> TrieBackend<MemoryDB<H>, H>
Expand All @@ -92,32 +57,16 @@ where
&mut self,
changes: T,
) {
let mut new_child_roots = Vec::new();
let mut root_map = None;
let root = self.root().clone();
for (child_info, map) in changes {
if let Some(child_info) = child_info.as_ref() {
let prefix_storage_key = child_info.prefixed_storage_key();
let ch = insert_into_memory_db::<H, _>(root, self.backend_storage_mut(), map.clone().into_iter());
new_child_roots.push((prefix_storage_key.into_inner(), Some(ch.as_ref().into())));
} else {
root_map = Some(map);
}
}
let (top, child) = changes.into_iter().partition::<Vec<_>, _>(|v| v.0.is_none());
let (root, transaction) = self.full_storage_root(
top.iter().map(|(_, v)| v).flatten().map(|(k, v)| (&k[..], v.as_deref())),
child.iter()
.filter_map(|v|
v.0.as_ref().map(|c| (c, v.1.iter().map(|(k, v)| (&k[..], v.as_deref()))))
),
);

let root = match root_map {
Some(map) => insert_into_memory_db::<H, _>(
root,
self.backend_storage_mut(),
map.into_iter().chain(new_child_roots.into_iter()),
),
None => insert_into_memory_db::<H, _>(
root,
self.backend_storage_mut(),
new_child_roots.into_iter(),
),
};
self.essence.set_root(root);
self.apply_transaction(root, transaction);
}

/// Merge trie nodes into this backend.
Expand All @@ -127,6 +76,12 @@ where
Self::new(clone, root)
}

/// Apply the given transaction to this backend and set the root to the given value.
pub fn apply_transaction(&mut self, root: H::Out, transaction: MemoryDB<H>) {
self.backend_storage_mut().consolidate(transaction);
self.essence.set_root(root);
}

/// Compare with another in-memory backend.
pub fn eq(&self, other: &Self) -> bool {
self.root() == other.root()
Expand Down Expand Up @@ -158,7 +113,9 @@ where
{
fn from(inner: HashMap<Option<ChildInfo>, BTreeMap<StorageKey, StorageValue>>) -> Self {
let mut backend = new_in_mem();
backend.insert(inner.into_iter().map(|(k, m)| (k, m.into_iter().map(|(k, v)| (k, Some(v))).collect())));
backend.insert(
inner.into_iter().map(|(k, m)| (k, m.into_iter().map(|(k, v)| (k, Some(v))).collect())),
);
backend
}
}
Expand Down Expand Up @@ -232,4 +189,16 @@ mod tests {
let storage_key = child_info.prefixed_storage_key();
assert!(trie_backend.storage(storage_key.as_slice()).unwrap().is_some());
}

#[test]
fn insert_multiple_times_child_data_works() {
let mut storage = new_in_mem::<BlakeTwo256>();
let child_info = ChildInfo::new_default(b"1");

storage.insert(vec![(Some(child_info.clone()), vec![(b"2".to_vec(), Some(b"3".to_vec()))])]);
storage.insert(vec![(Some(child_info.clone()), vec![(b"1".to_vec(), Some(b"3".to_vec()))])]);

assert_eq!(storage.child_storage(&child_info, &b"2"[..]), Ok(Some(b"3".to_vec())));
assert_eq!(storage.child_storage(&child_info, &b"1"[..]), Ok(Some(b"3".to_vec())));
}
}
69 changes: 65 additions & 4 deletions primitives/state-machine/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,11 @@ impl<H: Hasher, N: ChangesTrieBlockNumber> TestExternalities<H, N>
&mut self.changes_trie_storage
}

/// Return a new backend with all pending value.
pub fn commit_all(&self) -> InMemoryBackend<H> {
/// Return a new backend with all pending changes.
///
/// In contrast to [`commit_all`](Self::commit_all) this will not panic if there are open
/// transactions.
fn as_backend(&self) -> InMemoryBackend<H> {
let top: Vec<_> = self.overlay.changes()
.map(|(k, v)| (k.clone(), v.value().cloned()))
.collect();
Expand All @@ -172,6 +175,23 @@ impl<H: Hasher, N: ChangesTrieBlockNumber> TestExternalities<H, N>
self.backend.update(transaction)
}

/// Commit all pending changes to the underlying backend.
///
/// # Panic
///
/// This will panic if there are still open transactions.
pub fn commit_all(&mut self) -> Result<(), String> {
let changes = self.overlay.drain_storage_changes::<_, _, N>(
&self.backend,
None,
Default::default(),
&mut Default::default(),
)?;

self.backend.apply_transaction(changes.transaction_storage_root, changes.transaction);
Ok(())
}

/// Execute the given closure while `self` is set as externalities.
///
/// Returns the result of the given closure.
Expand Down Expand Up @@ -209,7 +229,7 @@ impl<H: Hasher, N: ChangesTrieBlockNumber> PartialEq for TestExternalities<H, N>
/// This doesn't test if they are in the same state, only if they contains the
/// same data at this state
fn eq(&self, other: &TestExternalities<H, N>) -> bool {
self.commit_all().eq(&other.commit_all())
self.as_backend().eq(&other.as_backend())
}
}

Expand Down Expand Up @@ -258,7 +278,7 @@ impl<H, N> sp_externalities::ExtensionStore for TestExternalities<H, N> where
#[cfg(test)]
mod tests {
use super::*;
use sp_core::{H256, traits::Externalities};
use sp_core::{H256, traits::Externalities, storage::ChildInfo};
use sp_runtime::traits::BlakeTwo256;
use hex_literal::hex;

Expand Down Expand Up @@ -289,4 +309,45 @@ mod tests {
fn assert_send<T: Send>() {}
assert_send::<TestExternalities::<BlakeTwo256, u64>>();
}

#[test]
fn commit_all_and_kill_child_storage() {
let mut ext = TestExternalities::<BlakeTwo256, u64>::default();
let child_info = ChildInfo::new_default(&b"test_child"[..]);

{
let mut ext = ext.ext();
ext.place_child_storage(&child_info, b"doe".to_vec(), Some(b"reindeer".to_vec()));
ext.place_child_storage(&child_info, b"dog".to_vec(), Some(b"puppy".to_vec()));
ext.place_child_storage(&child_info, b"dog2".to_vec(), Some(b"puppy2".to_vec()));
}

ext.commit_all().unwrap();

{
let mut ext = ext.ext();

assert!(!ext.kill_child_storage(&child_info, Some(2)), "Should not delete all keys");

assert!(ext.child_storage(&child_info, &b"doe"[..]).is_none());
assert!(ext.child_storage(&child_info, &b"dog"[..]).is_none());
assert!(ext.child_storage(&child_info, &b"dog2"[..]).is_some());
}
}

#[test]
fn as_backend_generates_same_backend_as_commit_all() {
let mut ext = TestExternalities::<BlakeTwo256, u64>::default();
{
let mut ext = ext.ext();
ext.set_storage(b"doe".to_vec(), b"reindeer".to_vec());
ext.set_storage(b"dog".to_vec(), b"puppy".to_vec());
ext.set_storage(b"dogglesworth".to_vec(), b"cat".to_vec());
}

let backend = ext.as_backend();

ext.commit_all().unwrap();
assert!(ext.backend.eq(&backend), "Both backend should be equal.");
}
}
10 changes: 7 additions & 3 deletions primitives/trie/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ pub fn delta_trie_root<L: TrieConfiguration, I, A, B, DB, V>(
DB: hash_db::HashDB<L::Hash, trie_db::DBValue>,
{
{
let mut trie = TrieDBMut::<L>::from_existing(&mut *db, &mut root)?;
let mut trie = TrieDBMut::<L>::from_existing(db, &mut root)?;

let mut delta = delta.into_iter().collect::<Vec<_>>();
delta.sort_by(|l, r| l.0.borrow().cmp(r.0.borrow()));
Expand Down Expand Up @@ -223,9 +223,13 @@ pub fn read_trie_value_with<
Ok(TrieDB::<L>::new(&*db, root)?.get_with(key, query).map(|x| x.map(|val| val.to_vec()))?)
}

/// Determine the empty trie root.
pub fn empty_trie_root<L: TrieConfiguration>() -> <L::Hash as Hasher>::Out {
L::trie_root::<_, Vec<u8>, Vec<u8>>(core::iter::empty())
}

/// Determine the empty child trie root.
pub fn empty_child_trie_root<L: TrieConfiguration>(
) -> <L::Hash as Hasher>::Out {
pub fn empty_child_trie_root<L: TrieConfiguration>() -> <L::Hash as Hasher>::Out {
L::trie_root::<_, Vec<u8>, Vec<u8>>(core::iter::empty())
}

Expand Down

0 comments on commit 2b0dae3

Please sign in to comment.