From d5cd4ca066ec6790f66302579adb1e9854e3d7de Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Fri, 1 Feb 2019 15:24:03 +0100 Subject: [PATCH 01/24] fix: wasm file --- core/client/db/src/lib.rs | 4 + core/client/db/src/utils.rs | 2 + core/client/src/children.rs | 168 ++++++++++++++++++++++++++++++++++++ core/client/src/leaves.rs | 4 +- core/client/src/lib.rs | 5 ++ 5 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 core/client/src/children.rs diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index b39fbb9f06adc..79018198b0f09 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -52,6 +52,7 @@ use executor::RuntimeInfo; use state_machine::{CodeExecutor, DBValue}; use crate::utils::{Meta, db_err, meta_keys, open_database, read_db, block_id_to_lookup_key, read_meta}; use client::LeafSet; +use client::ChildrenMap; use state_db::StateDb; use crate::storage_cache::{CachingState, SharedCache, new_shared_cache}; use log::{trace, debug, warn}; @@ -130,15 +131,18 @@ pub struct BlockchainDb { db: Arc, meta: Arc, Block::Hash>>>, leaves: RwLock>>, + children: RwLock>>, } impl BlockchainDb { fn new(db: Arc) -> Result { let meta = read_meta::(&*db, columns::META, columns::HEADER)?; let leaves = LeafSet::read_from_db(&*db, columns::META, meta_keys::LEAF_PREFIX)?; + let children = ChildrenMap::read_from_db(&*db, columns::META, meta_keys::CHILD_PREFIX)?; Ok(BlockchainDb { db, leaves: RwLock::new(leaves), + children: RwLock::new(children), meta: Arc::new(RwLock::new(meta)), }) } diff --git a/core/client/db/src/utils.rs b/core/client/db/src/utils.rs index f1e4f3d2e2325..b4e6aefedc625 100644 --- a/core/client/db/src/utils.rs +++ b/core/client/db/src/utils.rs @@ -51,6 +51,8 @@ pub mod meta_keys { pub const GENESIS_HASH: &[u8; 3] = b"gen"; /// Leaves prefix list key. pub const LEAF_PREFIX: &[u8; 4] = b"leaf"; + /// Child prefix list key. + pub const CHILD_PREFIX: &[u8; 5] = b"child"; } /// Database metadata. diff --git a/core/client/src/children.rs b/core/client/src/children.rs new file mode 100644 index 0000000000000..938129dff7f40 --- /dev/null +++ b/core/client/src/children.rs @@ -0,0 +1,168 @@ +// Copyright 2018 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 std::collections::BTreeMap; +use std::cmp::{Ord, Ordering}; +use kvdb::{KeyValueDB, DBTransaction}; +use runtime_primitives::traits::SimpleArithmetic; +use codec::{Encode, Decode}; +use crate::error; +use std::hash::Hash; +use std::fmt::Debug; + +#[derive(Debug, Clone)] +struct ChildItem { + hash: H, + number: N, +} + +impl Ord for ChildItem where N: Ord { + fn cmp(&self, other: &Self) -> Ordering { + // reverse (descending) order + other.number.cmp(&self.number) + } +} + +impl PartialOrd for ChildItem where N: PartialOrd { + fn partial_cmp(&self, other: &Self) -> Option { + // reverse (descending) order + other.number.partial_cmp(&self.number) + } +} + +impl PartialEq for ChildItem where N: PartialEq { + fn eq(&self, other: &ChildItem) -> bool { + self.number == other.number + } +} + +impl Eq for ChildItem where N: PartialEq {} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ChildrenMap where + H: Ord + Eq + Hash + Clone + Encode + Decode + Debug, + N: Ord + Eq + Hash + Clone + Encode + Decode + Debug, +{ + storage: BTreeMap, Vec>, + pending_added: Vec<(H, ChildItem)>, + pending_removed: Vec<(H, ChildItem)>, +} + +impl ChildrenMap where + H: Ord + Eq + Hash + Clone + Encode + Decode + Debug, + N: Ord + Eq + Hash + Clone + Encode + Decode + Debug, +{ + pub fn new() -> Self { + Self { + storage: BTreeMap::new(), + pending_added: Vec::new(), + pending_removed: Vec::new(), + } + } + + pub fn read_from_db(db: &KeyValueDB, column: Option, prefix: &[u8]) -> error::Result { + let mut storage: BTreeMap> = BTreeMap::new(); + for (key, value) in db.iter_from_prefix(column, prefix) { + if !key.starts_with(prefix) { break } + let raw_hash = &mut &key[prefix.len()..]; + let parent_hash = match Decode::decode(raw_hash) { + Some(hash) => hash, + None => return Err(error::ErrorKind::Backend("Error decoding hash".into()).into()), + }; + let raw_value = &mut &value[..]; + let child = match Decode::decode(raw_value) { + Some(child) => child, + None => return Err(error::ErrorKind::Backend("Error decoding child".into()).into()), + }; + + match storage.get_mut(&parent_hash) { + Some(children) => { + children.push(child); + }, + None => { + storage.insert(parent_hash, vec![child]); + } + }; + } + Ok(Self { + storage, + pending_added: Vec::new(), + pending_removed: Vec::new(), + }) + } + + /// Update the children list on import. + pub fn import(&mut self, parent_hash: H, hash: H) { + match self.storage.get_mut(&parent_hash) { + Some(children) => { + children.push(hash.clone()); + } + None => { + self.storage.insert(parent_hash.clone(), vec![hash.clone()]); + } + }; + self.pending_added.push((parent_hash, hash)); + } + + /// Write the leaf list to the database transaction. + pub fn prepare_transaction(&mut self, tx: &mut DBTransaction, column: Option, prefix: &[u8]) { + let mut buf = prefix.to_vec(); + for (parent, child) in self.pending_added.drain(..) { + parent.using_encoded(|s| buf.extend(s)); + tx.put_vec(column, &buf[..], child.encode()); + buf.truncate(prefix.len()); // reuse allocation. + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn children_works() { + let mut children = ChildrenMap::new(); + + children.import(0u32, 0u32); + children.import(1_1, 1_3); + children.import(1_2, 1_4); + + assert!(children.storage.contains_key(&1_1)); + assert!(children.storage.contains_key(&1_2)); + assert!(!children.storage.contains_key(&1_3)); + assert!(!children.storage.contains_key(&1_4)); + } + + #[test] + fn children_flush() { + const PREFIX: &[u8] = b"marcio"; + let db = ::kvdb_memorydb::create(0); + + let mut children = ChildrenMap::new(); + children.import(0u32, 0u32); + children.import(1_1, 1_3); + children.import(1_2, 1_4); + + let mut tx = DBTransaction::new(); + + children.prepare_transaction(&mut tx, None, PREFIX); + db.write(tx).unwrap(); + + let children2 = ChildrenMap::read_from_db(&db, None, PREFIX).unwrap(); + assert_eq!(children, children2); + } +} \ No newline at end of file diff --git a/core/client/src/leaves.rs b/core/client/src/leaves.rs index 8374155621697..bc723ce0a5da6 100644 --- a/core/client/src/leaves.rs +++ b/core/client/src/leaves.rs @@ -84,7 +84,6 @@ impl LeafSet where /// Read the leaf list from the DB, using given prefix for keys. pub fn read_from_db(db: &KeyValueDB, column: Option, prefix: &[u8]) -> error::Result { let mut storage = BTreeSet::new(); - for (key, value) in db.iter_from_prefix(column, prefix) { if !key.starts_with(prefix) { break } let raw_hash = &mut &key[prefix.len()..]; @@ -92,7 +91,8 @@ impl LeafSet where Some(hash) => hash, None => return Err(error::ErrorKind::Backend("Error decoding hash".into()).into()), }; - let number = match Decode::decode(&mut &value[..]) { + let raw_value = &mut &value[..]; + let number = match Decode::decode(raw_value) { Some(number) => number, None => return Err(error::ErrorKind::Backend("Error decoding number".into()).into()), }; diff --git a/core/client/src/lib.rs b/core/client/src/lib.rs index 7472b358d0d72..61310aa913ecd 100644 --- a/core/client/src/lib.rs +++ b/core/client/src/lib.rs @@ -45,6 +45,9 @@ mod call_executor; mod client; #[cfg(feature = "std")] mod notifications; +#[cfg(feature = "std")] +mod children; + #[cfg(feature = "std")] pub use crate::blockchain::Info as ChainInfo; @@ -63,6 +66,8 @@ pub use crate::notifications::{StorageEventStream, StorageChangeSet}; pub use state_machine::ExecutionStrategy; #[cfg(feature = "std")] pub use crate::leaves::LeafSet; +#[cfg(feature = "std")] +pub use crate::children::ChildrenMap; #[doc(inline)] pub use sr_api_macros::{decl_runtime_apis, impl_runtime_apis}; From a19a704c71dffa13721a60f27db03fbdab8fc6fe Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 28 Jan 2019 19:43:40 +0100 Subject: [PATCH 02/24] feat: add Ord to Hash --- core/client/db/src/lib.rs | 2 +- core/client/src/children.rs | 51 ++++++++------------------------ core/sr-primitives/src/traits.rs | 10 +++---- srml/system/src/lib.rs | 2 +- 4 files changed, 19 insertions(+), 46 deletions(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index 79018198b0f09..32dd05061d265 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -131,7 +131,7 @@ pub struct BlockchainDb { db: Arc, meta: Arc, Block::Hash>>>, leaves: RwLock>>, - children: RwLock>>, + children: RwLock>, } impl BlockchainDb { diff --git a/core/client/src/children.rs b/core/client/src/children.rs index 938129dff7f40..a12c30a00ff96 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -24,47 +24,20 @@ use crate::error; use std::hash::Hash; use std::fmt::Debug; -#[derive(Debug, Clone)] -struct ChildItem { - hash: H, - number: N, -} - -impl Ord for ChildItem where N: Ord { - fn cmp(&self, other: &Self) -> Ordering { - // reverse (descending) order - other.number.cmp(&self.number) - } -} - -impl PartialOrd for ChildItem where N: PartialOrd { - fn partial_cmp(&self, other: &Self) -> Option { - // reverse (descending) order - other.number.partial_cmp(&self.number) - } -} - -impl PartialEq for ChildItem where N: PartialEq { - fn eq(&self, other: &ChildItem) -> bool { - self.number == other.number - } -} - -impl Eq for ChildItem where N: PartialEq {} #[derive(Debug, Clone, PartialEq, Eq)] -pub struct ChildrenMap where - H: Ord + Eq + Hash + Clone + Encode + Decode + Debug, - N: Ord + Eq + Hash + Clone + Encode + Decode + Debug, +pub struct ChildrenMap where + K: Ord + Eq + Hash + Clone + Encode + Decode + Debug, + V: Ord + Eq + Hash + Clone + Encode + Decode + Debug, { - storage: BTreeMap, Vec>, - pending_added: Vec<(H, ChildItem)>, - pending_removed: Vec<(H, ChildItem)>, + storage: BTreeMap>, + pending_added: Vec<(K, V)>, + pending_removed: Vec<(K, V)>, } -impl ChildrenMap where - H: Ord + Eq + Hash + Clone + Encode + Decode + Debug, - N: Ord + Eq + Hash + Clone + Encode + Decode + Debug, +impl ChildrenMap where + K: Ord + Eq + Hash + Clone + Encode + Decode + Debug, + V: Ord + Eq + Hash + Clone + Encode + Decode + Debug, { pub fn new() -> Self { Self { @@ -75,7 +48,7 @@ impl ChildrenMap where } pub fn read_from_db(db: &KeyValueDB, column: Option, prefix: &[u8]) -> error::Result { - let mut storage: BTreeMap> = BTreeMap::new(); + let mut storage: BTreeMap> = BTreeMap::new(); for (key, value) in db.iter_from_prefix(column, prefix) { if !key.starts_with(prefix) { break } let raw_hash = &mut &key[prefix.len()..]; @@ -97,7 +70,7 @@ impl ChildrenMap where storage.insert(parent_hash, vec![child]); } }; - } + } Ok(Self { storage, pending_added: Vec::new(), @@ -106,7 +79,7 @@ impl ChildrenMap where } /// Update the children list on import. - pub fn import(&mut self, parent_hash: H, hash: H) { + pub fn import(&mut self, parent_hash: K, hash: V) { match self.storage.get_mut(&parent_hash) { Some(children) => { children.push(hash.clone()); diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 5e12d571ae201..4adb20e3c3bfc 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -346,10 +346,10 @@ macro_rules! tuple_impl { tuple_impl!(A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X, Y, Z,); /// Abstraction around hashing -pub trait Hash: 'static + MaybeSerializeDebug + Clone + Eq + PartialEq { // Stupid bug in the Rust compiler believes derived +pub trait Hash: 'static + MaybeSerializeDebug + Clone + Eq + PartialEq + PartialOrd + Ord { // Stupid bug in the Rust compiler believes derived // traits must be fulfilled by all type parameters. /// The hash type produced. - type Output: Member + MaybeSerializeDebug + AsRef<[u8]> + AsMut<[u8]>; + type Output: Member + MaybeSerializeDebug + AsRef<[u8]> + AsMut<[u8]> + PartialEq + Eq + PartialOrd + Ord; /// Produce the hash of some byte-slice. fn hash(s: &[u8]) -> Self::Output; @@ -383,7 +383,7 @@ pub trait Hash: 'static + MaybeSerializeDebug + Clone + Eq + PartialEq { // Stup } /// Blake2-256 Hash implementation. -#[derive(PartialEq, Eq, Clone)] +#[derive(PartialEq, Eq, PartialOrd, Ord, Clone)] #[cfg_attr(feature = "std", derive(Debug, Serialize, Deserialize))] pub struct BlakeTwo256; @@ -544,7 +544,7 @@ pub trait Header: Clone + Send + Sync + Codec + Eq + MaybeSerializeDebugButNotDe /// Header number. type Number: Member + MaybeSerializeDebug + ::rstd::hash::Hash + Copy + MaybeDisplay + SimpleArithmetic + Codec; /// Header hash type - type Hash: Member + MaybeSerializeDebug + ::rstd::hash::Hash + Copy + MaybeDisplay + Default + SimpleBitOps + Codec + AsRef<[u8]> + AsMut<[u8]>; + type Hash: Member + MaybeSerializeDebug + ::rstd::hash::Hash + Copy + MaybeDisplay + Default + SimpleBitOps + Codec + AsRef<[u8]> + AsMut<[u8]> + PartialEq + Eq + PartialOrd + Ord; /// Hashing algorithm type Hashing: Hash; /// Digest type @@ -602,7 +602,7 @@ pub trait Block: Clone + Send + Sync + Codec + Eq + MaybeSerializeDebugButNotDes /// Header type. type Header: Header; /// Block hash type. - type Hash: Member + MaybeSerializeDebug + ::rstd::hash::Hash + Copy + MaybeDisplay + Default + SimpleBitOps + Codec + AsRef<[u8]> + AsMut<[u8]>; + type Hash: Member + MaybeSerializeDebug + ::rstd::hash::Hash + Copy + MaybeDisplay + Default + SimpleBitOps + Codec + AsRef<[u8]> + AsMut<[u8]> + PartialEq + Eq + PartialOrd + Ord; /// Returns a reference to the header. fn header(&self) -> &Self::Header; diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 8f3fd1df8cbaf..8bb26d019b2ba 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -78,7 +78,7 @@ pub trait Trait: 'static + Eq + Clone { type Origin: Into>> + From>; type Index: Parameter + Member + MaybeSerializeDebugButNotDeserialize + Default + MaybeDisplay + SimpleArithmetic + Copy; type BlockNumber: Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleArithmetic + Default + Bounded + Copy + rstd::hash::Hash; - type Hash: Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleBitOps + Default + Copy + CheckEqual + rstd::hash::Hash + AsRef<[u8]> + AsMut<[u8]>; + type Hash: Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleBitOps + Default + Copy + CheckEqual + rstd::hash::Hash + AsRef<[u8]> + AsMut<[u8]> + PartialEq + Eq + PartialOrd + Ord; type Hashing: Hash; type Digest: Parameter + Member + MaybeSerializeDebugButNotDeserialize + Default + traits::Digest; type AccountId: Parameter + Member + MaybeSerializeDebug + MaybeDisplay + Ord + Default; From 427ac59a7ab7a0ef2345755259c072e8f4c13d88 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Tue, 29 Jan 2019 09:20:23 +0100 Subject: [PATCH 03/24] feat: add children function to backend --- core/client/db/src/lib.rs | 10 ++++++++++ core/client/src/blockchain.rs | 1 + core/client/src/children.rs | 11 +++++++++-- core/client/src/client.rs | 1 + core/client/src/in_mem.rs | 19 +++++++++++++------ core/client/src/light/blockchain.rs | 4 ++++ 6 files changed, 38 insertions(+), 8 deletions(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index 32dd05061d265..8c314ce7bb6e5 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -253,6 +253,10 @@ impl client::blockchain::Backend for BlockchainDb { fn leaves(&self) -> Result, client::error::Error> { Ok(self.leaves.read().hashes()) } + + fn children(&self, parent_hash: Block::Hash) -> Vec { + self.children.read().hashes(parent_hash) + } } /// Database transaction @@ -765,6 +769,7 @@ impl> Backend { fn try_commit_operation(&self, mut operation: BlockImportOperation) -> Result<(), client::error::Error> { + println!("db inside commit_operation ----------------------------"); let mut transaction = DBTransaction::new(); operation.apply_aux(&mut transaction); @@ -856,7 +861,12 @@ impl> Backend { let displaced_leaf = { let mut leaves = self.blockchain.leaves.write(); let displaced_leaf = leaves.import(hash, number, parent_hash); + println!("db prepare_transaction --------------- {:?}->{:?}", parent_hash, hash); leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX); + + let mut children = self.blockchain().children.write(); + children.import(parent_hash, hash); + children.prepare_transaction(&mut transaction, columns::META, meta_keys::CHILD_PREFIX); displaced_leaf }; diff --git a/core/client/src/blockchain.rs b/core/client/src/blockchain.rs index 986360764d3da..31c294c9b930b 100644 --- a/core/client/src/blockchain.rs +++ b/core/client/src/blockchain.rs @@ -84,6 +84,7 @@ pub trait Backend: HeaderBackend { /// in other words, that have no children, are chain heads. /// Results must be ordered best (longest, heighest) chain first. fn leaves(&self) -> Result>; + fn children(&self, parent_hash: Block::Hash) -> Vec; } /// Blockchain optional data cache. diff --git a/core/client/src/children.rs b/core/client/src/children.rs index a12c30a00ff96..1520cc6059434 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -91,7 +91,7 @@ impl ChildrenMap where self.pending_added.push((parent_hash, hash)); } - /// Write the leaf list to the database transaction. + /// Write the children list to the database transaction. pub fn prepare_transaction(&mut self, tx: &mut DBTransaction, column: Option, prefix: &[u8]) { let mut buf = prefix.to_vec(); for (parent, child) in self.pending_added.drain(..) { @@ -100,6 +100,13 @@ impl ChildrenMap where buf.truncate(prefix.len()); // reuse allocation. } } + + pub fn hashes(&self, parent_hash: K) -> Vec { + match self.storage.get(&parent_hash) { + Some(children) => children.clone(), + None => vec![], + } + } } #[cfg(test)] @@ -122,7 +129,7 @@ mod tests { #[test] fn children_flush() { - const PREFIX: &[u8] = b"marcio"; + const PREFIX: &[u8] = b"a"; let db = ::kvdb_memorydb::create(0); let mut children = ChildrenMap::new(); diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 375e36e81f545..977c04dd4be37 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -622,6 +622,7 @@ impl Client where pub fn lock_import_and_run) -> error::Result>( &self, f: F ) -> error::Result { + println!("in lock_import_and_run ------------------------------------------------"); let inner = || { let _import_lock = self.import_lock.lock(); diff --git a/core/client/src/in_mem.rs b/core/client/src/in_mem.rs index fda962aca01da..1dc4c45f5749b 100644 --- a/core/client/src/in_mem.rs +++ b/core/client/src/in_mem.rs @@ -19,22 +19,24 @@ use std::collections::HashMap; use std::sync::Arc; use parking_lot::RwLock; -use crate::error; -use crate::backend::{self, NewBlockState}; -use crate::light; use primitives::{ChangesTrieConfiguration, storage::well_known_keys}; use runtime_primitives::generic::BlockId; use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, Zero, NumberFor, As, Digest, DigestItem, AuthorityIdFor}; use runtime_primitives::{Justification, StorageOverlay, ChildrenStorageOverlay}; -use crate::blockchain::{self, BlockStatus, HeaderBackend}; use state_machine::backend::{Backend as StateBackend, InMemory, Consolidate}; use state_machine::{self, InMemoryChangesTrieStorage, ChangesTrieAnchorBlockId}; use hash_db::Hasher; use heapsize::HeapSizeOf; -use crate::leaves::LeafSet; use trie::MemoryDB; +use crate::error; +use crate::backend::{self, NewBlockState}; +use crate::light; +use crate::leaves::LeafSet; +use crate::children::ChildrenMap; +use crate::blockchain::{self, BlockStatus, HeaderBackend}; + struct PendingBlock { block: StoredBlock, state: NewBlockState, @@ -97,6 +99,7 @@ struct BlockchainStorage { header_cht_roots: HashMap, Block::Hash>, changes_trie_cht_roots: HashMap, Block::Hash>, leaves: LeafSet>, + children: ChildrenMap, aux: HashMap, Vec>, } @@ -147,6 +150,7 @@ impl Blockchain { header_cht_roots: HashMap::new(), changes_trie_cht_roots: HashMap::new(), leaves: LeafSet::new(), + children: ChildrenMap::new(), aux: HashMap::new(), })); Blockchain { @@ -168,7 +172,6 @@ impl Blockchain { new_state: NewBlockState, ) -> crate::error::Result<()> { let number = header.number().clone(); - if new_state.is_best() { self.apply_head(&header)?; } @@ -362,6 +365,10 @@ impl blockchain::Backend for Blockchain { fn leaves(&self) -> error::Result> { Ok(self.storage.read().leaves.hashes()) } + + fn children(&self, parent_hash: Block::Hash) -> Vec { + self.storage.read().children.hashes(parent_hash) + } } impl backend::AuxStore for Blockchain { diff --git a/core/client/src/light/blockchain.rs b/core/client/src/light/blockchain.rs index 36a236f32e2df..d6210c2b2e0d6 100644 --- a/core/client/src/light/blockchain.rs +++ b/core/client/src/light/blockchain.rs @@ -163,6 +163,10 @@ impl BlockchainBackend for Blockchain where Block: Blo fn leaves(&self) -> ClientResult> { unimplemented!() } + + fn children(&self, _id: Block::Hash) -> Vec { + unimplemented!() + } } #[cfg(test)] From 79ae99432ee453e902f85c3d1fa70895e2bce81c Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Tue, 29 Jan 2019 10:14:48 +0100 Subject: [PATCH 04/24] feat: add test for children hashes --- core/client/src/children.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/core/client/src/children.rs b/core/client/src/children.rs index 1520cc6059434..2d0e6ab7e0b16 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -127,6 +127,19 @@ mod tests { assert!(!children.storage.contains_key(&1_4)); } + #[test] + fn children_hashes() { + let mut children = ChildrenMap::new(); + + children.import(0u32, 0u32); + children.import(1_1, 1_3); + children.import(1_2, 1_4); + children.import(1_2, 1_5); + + assert_eq!(children.hashes(1_1), vec![1_3]); + assert_eq!(children.hashes(1_2), vec![1_4, 1_5]); + } + #[test] fn children_flush() { const PREFIX: &[u8] = b"a"; From a89f6572b537e88bc9150d703b2a6bc9d5e13241 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Tue, 29 Jan 2019 12:32:40 +0100 Subject: [PATCH 05/24] feat: add uncles function to client --- core/client/src/client.rs | 157 +++++++++++++++++++++++++++++++++++++- 1 file changed, 156 insertions(+), 1 deletion(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 977c04dd4be37..5a736e0434745 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -56,7 +56,7 @@ use executor::{RuntimeVersion, RuntimeInfo}; use crate::notifications::{StorageNotifications, StorageEventStream}; use crate::light::{call_executor::prove_execution, fetcher::ChangesProof}; use crate::cht; -use crate::error; +use crate::error::{self, ErrorKind}; use crate::in_mem; use crate::block_builder::{self, api::BlockBuilder as BlockBuilderAPI}; use crate::genesis; @@ -1231,6 +1231,54 @@ impl Client where Ok(None) } + fn get_descendants(&self, target: Block::Hash) -> Vec { + let children = self.backend.blockchain().children(target); + let mut descendants = vec![]; + for child in children { + let d = self.get_descendants(child); + descendants.extend(d); + } + descendants.push(target); + descendants + } + + pub fn uncles(&self, target_hash: Block::Hash, max_generation: NumberFor) + -> error::Result>> + { + let load_header = |id: Block::Hash| { + match self.backend.blockchain().header(BlockId::Hash(id)) { + Ok(Some(hdr)) => Ok(hdr), + Ok(None) => Err(ErrorKind::UnknownBlock(format!("Unknown block {:?}", id)).into()), + Err(e) => Err(e), + } + }; + + let genesis_hash = self.backend.blockchain().info().unwrap().genesis_hash; + let genesis = load_header(genesis_hash)?; + let mut last_ancestor = load_header(target_hash)?; + let mut ancestors = HashSet::new(); + + for generation in 0..max_generation.as_() { + last_ancestor = load_header(*last_ancestor.parent_hash())?; + ancestors.insert(last_ancestor.hash()); + if genesis == last_ancestor { break; } + } + + let mut uncles = self.get_descendants(last_ancestor.hash()); + ancestors.extend(self.get_descendants(target_hash)); + + let mut i = 0; + while i < uncles.len() { + if ancestors.contains(&uncles[i]) { + uncles.remove(i); + } else { + i += 1; + } + } + + Ok(Some(uncles)) + } + fn changes_trie_config(&self) -> Result, Error> { Ok(self.backend.state_at(BlockId::Number(self.backend.blockchain().info()?.best_number))? .storage(well_known_keys::CHANGES_TRIE_CONFIG) @@ -1694,6 +1742,27 @@ pub(crate) mod tests { assert_eq!(genesis_hash.clone(), client.best_containing(genesis_hash.clone(), None).unwrap().unwrap()); } + #[test] + fn uncles_with_genesis_block() { + use test_client::blockchain::Backend; + + // block tree: + // G -> A1 -> A2 + + let client = test_client::new(); + let genesis_hash = client.info().unwrap().chain.genesis_hash; + + // G -> A1 + let a1 = client.new_block().unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a1.clone()).unwrap(); + + // A1 -> A2 + let a2 = client.new_block().unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a2.clone()).unwrap(); + let v: Vec = Vec::new(); + assert_eq!(v, client.uncles(a2.hash(), 3).unwrap().unwrap()); + } + #[test] fn best_containing_with_hash_not_found() { // block tree: @@ -1706,6 +1775,91 @@ pub(crate) mod tests { assert_eq!(None, client.best_containing(uninserted_block.hash().clone(), None).unwrap()); } + #[test] + fn uncles_with_multiple_forks() { + // NOTE: we use the version of the trait from `test_client` + // because that is actually different than the version linked to + // in the test facade crate. + use test_client::blockchain::Backend as BlockchainBackendT; + + // block tree: + // G -> A1 -> A2 -> A3 -> A4 -> A5 + // A1 -> B2 -> B3 -> B4 + // B2 -> C3 + // A1 -> D2 + let client = test_client::new(); + + // G -> A1 + let a1 = client.new_block().unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a1.clone()).unwrap(); + + // A1 -> A2 + let a2 = client.new_block_at(&BlockId::Hash(a1.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a2.clone()).unwrap(); + + // A2 -> A3 + let a3 = client.new_block_at(&BlockId::Hash(a2.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a3.clone()).unwrap(); + + // A3 -> A4 + let a4 = client.new_block_at(&BlockId::Hash(a3.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a4.clone()).unwrap(); + + // A4 -> A5 + let a5 = client.new_block_at(&BlockId::Hash(a4.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a5.clone()).unwrap(); + + // A1 -> B2 + let mut builder = client.new_block_at(&BlockId::Hash(a1.hash())).unwrap(); + // this push is required as otherwise B2 has the same hash as A2 and won't get imported + builder.push_transfer(Transfer { + from: Keyring::Alice.to_raw_public().into(), + to: Keyring::Ferdie.to_raw_public().into(), + amount: 41, + nonce: 0, + }).unwrap(); + let b2 = builder.bake().unwrap(); + client.import(BlockOrigin::Own, b2.clone()).unwrap(); + + // B2 -> B3 + let b3 = client.new_block_at(&BlockId::Hash(b2.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, b3.clone()).unwrap(); + + // B3 -> B4 + let b4 = client.new_block_at(&BlockId::Hash(b3.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, b4.clone()).unwrap(); + + // // B2 -> C3 + let mut builder = client.new_block_at(&BlockId::Hash(b2.hash())).unwrap(); + // this push is required as otherwise C3 has the same hash as B3 and won't get imported + builder.push_transfer(Transfer { + from: Keyring::Alice.to_raw_public().into(), + to: Keyring::Ferdie.to_raw_public().into(), + amount: 1, + nonce: 1, + }).unwrap(); + let c3 = builder.bake().unwrap(); + client.import(BlockOrigin::Own, c3.clone()).unwrap(); + + // A1 -> D2 + let mut builder = client.new_block_at(&BlockId::Hash(a1.hash())).unwrap(); + // this push is required as otherwise D2 has the same hash as B2 and won't get imported + builder.push_transfer(Transfer { + from: Keyring::Alice.to_raw_public().into(), + to: Keyring::Ferdie.to_raw_public().into(), + amount: 1, + nonce: 0, + }).unwrap(); + let d2 = builder.bake().unwrap(); + client.import(BlockOrigin::Own, d2.clone()).unwrap(); + + assert_eq!(client.info().unwrap().chain.best_hash, a5.hash()); + let genesis_hash = client.info().unwrap().chain.genesis_hash; + + let uncles = client.uncles(a4.hash(), 10).unwrap().unwrap(); + assert_eq!(vec![b2.hash(), b3.hash(), b4.hash(), c3.hash(), d2.hash()].len(), uncles.len()); + } + #[test] fn best_containing_with_single_chain_3_blocks() { // block tree: @@ -1727,6 +1881,7 @@ pub(crate) mod tests { assert_eq!(a2.hash(), client.best_containing(a1.hash(), None).unwrap().unwrap()); assert_eq!(a2.hash(), client.best_containing(a2.hash(), None).unwrap().unwrap()); } + #[test] fn best_containing_with_multiple_forks() { From a5dcc25f7ae2dd8fc21d5c3941e1200bae6781e6 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Fri, 1 Feb 2019 15:00:47 +0100 Subject: [PATCH 06/24] fix: improve uncles function adds few more tests --- core/client/src/client.rs | 82 ++++++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 36 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 5a736e0434745..83c2965b21a09 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -622,7 +622,6 @@ impl Client where pub fn lock_import_and_run) -> error::Result>( &self, f: F ) -> error::Result { - println!("in lock_import_and_run ------------------------------------------------"); let inner = || { let _import_lock = self.import_lock.lock(); @@ -1231,17 +1230,21 @@ impl Client where Ok(None) } - fn get_descendants(&self, target: Block::Hash) -> Vec { - let children = self.backend.blockchain().children(target); + fn get_descendants(&self, target: Block::Hash, maybe_skip: Option) -> Vec { + let mut children = self.backend.blockchain().children(target); + if let Some(skip) = maybe_skip { + children.retain(|&c| c != skip); + } let mut descendants = vec![]; for child in children { - let d = self.get_descendants(child); + descendants.push(child); + let d = self.get_descendants(child, None); descendants.extend(d); } - descendants.push(target); descendants } + /// Gets the uncles of the block with `target_hash` going back `max_generation` ancestors. pub fn uncles(&self, target_hash: Block::Hash, max_generation: NumberFor) -> error::Result>> { @@ -1255,25 +1258,17 @@ impl Client where let genesis_hash = self.backend.blockchain().info().unwrap().genesis_hash; let genesis = load_header(genesis_hash)?; - let mut last_ancestor = load_header(target_hash)?; - let mut ancestors = HashSet::new(); - - for generation in 0..max_generation.as_() { - last_ancestor = load_header(*last_ancestor.parent_hash())?; - ancestors.insert(last_ancestor.hash()); - if genesis == last_ancestor { break; } - } + let mut current = load_header(target_hash)?; + let mut uncles = vec![]; - let mut uncles = self.get_descendants(last_ancestor.hash()); - ancestors.extend(self.get_descendants(target_hash)); + if genesis == current { return Ok(Some(uncles)); } + let mut ancestor = load_header(*current.parent_hash())?; - let mut i = 0; - while i < uncles.len() { - if ancestors.contains(&uncles[i]) { - uncles.remove(i); - } else { - i += 1; - } + for _generation in 0..max_generation.as_() { + uncles.extend(self.get_descendants(ancestor.hash(), Some(current.hash()))); + current = ancestor; + if genesis == current { break; } + ancestor = load_header(*current.parent_hash())?; } Ok(Some(uncles)) @@ -1742,6 +1737,18 @@ pub(crate) mod tests { assert_eq!(genesis_hash.clone(), client.best_containing(genesis_hash.clone(), None).unwrap().unwrap()); } + #[test] + fn best_containing_with_hash_not_found() { + // block tree: + // G + + let client = test_client::new(); + + let uninserted_block = client.new_block().unwrap().bake().unwrap(); + + assert_eq!(None, client.best_containing(uninserted_block.hash().clone(), None).unwrap()); + } + #[test] fn uncles_with_genesis_block() { use test_client::blockchain::Backend; @@ -1763,18 +1770,6 @@ pub(crate) mod tests { assert_eq!(v, client.uncles(a2.hash(), 3).unwrap().unwrap()); } - #[test] - fn best_containing_with_hash_not_found() { - // block tree: - // G - - let client = test_client::new(); - - let uninserted_block = client.new_block().unwrap().bake().unwrap(); - - assert_eq!(None, client.best_containing(uninserted_block.hash().clone(), None).unwrap()); - } - #[test] fn uncles_with_multiple_forks() { // NOTE: we use the version of the trait from `test_client` @@ -1856,8 +1851,23 @@ pub(crate) mod tests { assert_eq!(client.info().unwrap().chain.best_hash, a5.hash()); let genesis_hash = client.info().unwrap().chain.genesis_hash; - let uncles = client.uncles(a4.hash(), 10).unwrap().unwrap(); - assert_eq!(vec![b2.hash(), b3.hash(), b4.hash(), c3.hash(), d2.hash()].len(), uncles.len()); + let uncles1 = client.uncles(a4.hash(), 10).unwrap().unwrap(); + assert_eq!(vec![b2.hash(), b3.hash(), b4.hash(), c3.hash(), d2.hash()].len(), uncles1.len()); + + let uncles2 = client.uncles(a4.hash(), 0).unwrap().unwrap(); + assert_eq!(0, uncles2.len()); + + let uncles3 = client.uncles(a1.hash(), 10).unwrap().unwrap(); + assert_eq!(0, uncles3.len()); + + let uncles4 = client.uncles(genesis_hash, 10).unwrap().unwrap(); + assert_eq!(0, uncles4.len()); + + let uncles5 = client.uncles(d2.hash(), 10).unwrap().unwrap(); + assert_eq!(8, uncles5.len()); + + let uncles6 = client.uncles(b3.hash(), 1).unwrap().unwrap(); + assert_eq!(1, uncles6.len()); } #[test] From a2e00c2b199e2f6504b221cbcaac67e081065241 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Fri, 1 Feb 2019 15:23:01 +0100 Subject: [PATCH 07/24] chore: address review --- core/client/db/src/lib.rs | 2 - core/client/src/blockchain.rs | 2 + core/client/src/children.rs | 160 +++++++++++++++++----------------- core/client/src/client.rs | 2 +- 4 files changed, 84 insertions(+), 82 deletions(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index 8c314ce7bb6e5..06befca53d1c3 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -769,7 +769,6 @@ impl> Backend { fn try_commit_operation(&self, mut operation: BlockImportOperation) -> Result<(), client::error::Error> { - println!("db inside commit_operation ----------------------------"); let mut transaction = DBTransaction::new(); operation.apply_aux(&mut transaction); @@ -861,7 +860,6 @@ impl> Backend { let displaced_leaf = { let mut leaves = self.blockchain.leaves.write(); let displaced_leaf = leaves.import(hash, number, parent_hash); - println!("db prepare_transaction --------------- {:?}->{:?}", parent_hash, hash); leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX); let mut children = self.blockchain().children.write(); diff --git a/core/client/src/blockchain.rs b/core/client/src/blockchain.rs index 31c294c9b930b..9541604b7284e 100644 --- a/core/client/src/blockchain.rs +++ b/core/client/src/blockchain.rs @@ -84,6 +84,8 @@ pub trait Backend: HeaderBackend { /// in other words, that have no children, are chain heads. /// Results must be ordered best (longest, heighest) chain first. fn leaves(&self) -> Result>; + + /// Returns the hashes of all blocks that are children of the block with `parent_hash`. fn children(&self, parent_hash: Block::Hash) -> Vec; } diff --git a/core/client/src/children.rs b/core/client/src/children.rs index 2d0e6ab7e0b16..0505db5d13432 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -1,4 +1,4 @@ -// Copyright 2018 Parity Technologies (UK) Ltd. +// Copyright 2019 Parity Technologies (UK) Ltd. // This file is part of Substrate. // Substrate is free software: you can redistribute it and/or modify @@ -16,97 +16,99 @@ use std::collections::BTreeMap; -use std::cmp::{Ord, Ordering}; +use std::cmp::Ord; use kvdb::{KeyValueDB, DBTransaction}; -use runtime_primitives::traits::SimpleArithmetic; use codec::{Encode, Decode}; use crate::error; use std::hash::Hash; use std::fmt::Debug; - +/// Map of children blocks stored in memory for fast access. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ChildrenMap where - K: Ord + Eq + Hash + Clone + Encode + Decode + Debug, - V: Ord + Eq + Hash + Clone + Encode + Decode + Debug, + K: Ord + Eq + Hash + Clone + Encode + Decode + Debug, + V: Ord + Eq + Hash + Clone + Encode + Decode + Debug, { - storage: BTreeMap>, - pending_added: Vec<(K, V)>, - pending_removed: Vec<(K, V)>, + storage: BTreeMap>, + pending_added: Vec<(K, V)>, + pending_removed: Vec<(K, V)>, } impl ChildrenMap where - K: Ord + Eq + Hash + Clone + Encode + Decode + Debug, - V: Ord + Eq + Hash + Clone + Encode + Decode + Debug, + K: Ord + Eq + Hash + Clone + Encode + Decode + Debug, + V: Ord + Eq + Hash + Clone + Encode + Decode + Debug, { - pub fn new() -> Self { - Self { - storage: BTreeMap::new(), - pending_added: Vec::new(), - pending_removed: Vec::new(), - } - } - - pub fn read_from_db(db: &KeyValueDB, column: Option, prefix: &[u8]) -> error::Result { - let mut storage: BTreeMap> = BTreeMap::new(); - for (key, value) in db.iter_from_prefix(column, prefix) { - if !key.starts_with(prefix) { break } - let raw_hash = &mut &key[prefix.len()..]; - let parent_hash = match Decode::decode(raw_hash) { + /// Creates an empty children map. + pub fn new() -> Self { + Self { + storage: BTreeMap::new(), + pending_added: Vec::new(), + pending_removed: Vec::new(), + } + } + + /// Reads a children map from DB. + pub fn read_from_db(db: &KeyValueDB, column: Option, prefix: &[u8]) -> error::Result { + let mut storage: BTreeMap> = BTreeMap::new(); + for (key, value) in db.iter_from_prefix(column, prefix) { + if !key.starts_with(prefix) { break } + let raw_hash = &mut &key[prefix.len()..]; + let parent_hash = match Decode::decode(raw_hash) { Some(hash) => hash, None => return Err(error::ErrorKind::Backend("Error decoding hash".into()).into()), }; - let raw_value = &mut &value[..]; - let child = match Decode::decode(raw_value) { + let raw_value = &mut &value[..]; + let child = match Decode::decode(raw_value) { Some(child) => child, None => return Err(error::ErrorKind::Backend("Error decoding child".into()).into()), }; - match storage.get_mut(&parent_hash) { - Some(children) => { - children.push(child); - }, - None => { - storage.insert(parent_hash, vec![child]); - } - }; - } + match storage.get_mut(&parent_hash) { + Some(children) => { + children.push(child); + }, + None => { + storage.insert(parent_hash, vec![child]); + } + }; + } Ok(Self { storage, pending_added: Vec::new(), pending_removed: Vec::new(), }) - } + } - /// Update the children list on import. + /// Update the children list on import. pub fn import(&mut self, parent_hash: K, hash: V) { match self.storage.get_mut(&parent_hash) { - Some(children) => { - children.push(hash.clone()); - } - None => { - self.storage.insert(parent_hash.clone(), vec![hash.clone()]); - } - }; - self.pending_added.push((parent_hash, hash)); + Some(children) => { + children.push(hash.clone()); + } + None => { + self.storage.insert(parent_hash.clone(), vec![hash.clone()]); + } + }; + self.pending_added.push((parent_hash, hash)); } - /// Write the children list to the database transaction. + /// Write the children list to the database transaction. pub fn prepare_transaction(&mut self, tx: &mut DBTransaction, column: Option, prefix: &[u8]) { let mut buf = prefix.to_vec(); for (parent, child) in self.pending_added.drain(..) { - parent.using_encoded(|s| buf.extend(s)); + parent.using_encoded(|s| buf.extend(s)); tx.put_vec(column, &buf[..], child.encode()); buf.truncate(prefix.len()); // reuse allocation. } } - pub fn hashes(&self, parent_hash: K) -> Vec { - match self.storage.get(&parent_hash) { - Some(children) => children.clone(), - None => vec![], - } - } + /// Gets the hashes of the children blocks of `parent_hash`. + pub fn hashes(&self, parent_hash: K) -> Vec { + match self.storage.get(&parent_hash) { + Some(children) => children.clone(), + None => vec![], + } + } } #[cfg(test)] @@ -115,40 +117,40 @@ mod tests { #[test] fn children_works() { - let mut children = ChildrenMap::new(); + let mut children = ChildrenMap::new(); children.import(0u32, 0u32); - children.import(1_1, 1_3); - children.import(1_2, 1_4); + children.import(1_1, 1_3); + children.import(1_2, 1_4); - assert!(children.storage.contains_key(&1_1)); - assert!(children.storage.contains_key(&1_2)); - assert!(!children.storage.contains_key(&1_3)); - assert!(!children.storage.contains_key(&1_4)); - } + assert!(children.storage.contains_key(&1_1)); + assert!(children.storage.contains_key(&1_2)); + assert!(!children.storage.contains_key(&1_3)); + assert!(!children.storage.contains_key(&1_4)); + } - #[test] - fn children_hashes() { - let mut children = ChildrenMap::new(); + #[test] + fn children_hashes() { + let mut children = ChildrenMap::new(); children.import(0u32, 0u32); - children.import(1_1, 1_3); - children.import(1_2, 1_4); - children.import(1_2, 1_5); + children.import(1_1, 1_3); + children.import(1_2, 1_4); + children.import(1_2, 1_5); - assert_eq!(children.hashes(1_1), vec![1_3]); - assert_eq!(children.hashes(1_2), vec![1_4, 1_5]); - } + assert_eq!(children.hashes(1_1), vec![1_3]); + assert_eq!(children.hashes(1_2), vec![1_4, 1_5]); + } - #[test] - fn children_flush() { - const PREFIX: &[u8] = b"a"; + #[test] + fn children_flush() { + const PREFIX: &[u8] = b"a"; let db = ::kvdb_memorydb::create(0); - let mut children = ChildrenMap::new(); - children.import(0u32, 0u32); - children.import(1_1, 1_3); - children.import(1_2, 1_4); + let mut children = ChildrenMap::new(); + children.import(0u32, 0u32); + children.import(1_1, 1_3); + children.import(1_2, 1_4); let mut tx = DBTransaction::new(); @@ -157,5 +159,5 @@ mod tests { let children2 = ChildrenMap::read_from_db(&db, None, PREFIX).unwrap(); assert_eq!(children, children2); - } + } } \ No newline at end of file diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 83c2965b21a09..7a4549512c627 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1235,7 +1235,7 @@ impl Client where if let Some(skip) = maybe_skip { children.retain(|&c| c != skip); } - let mut descendants = vec![]; + let mut descendants = Vec::new(); for child in children { descendants.push(child); let d = self.get_descendants(child, None); From dbcf786f3c6d84949d299c8062fc1c4397ebf2dc Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Fri, 1 Feb 2019 15:54:07 +0100 Subject: [PATCH 08/24] fix: wasm file, typo, unused import --- core/client/db/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index 06befca53d1c3..7ecd30acf2902 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -862,7 +862,7 @@ impl> Backend { let displaced_leaf = leaves.import(hash, number, parent_hash); leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX); - let mut children = self.blockchain().children.write(); + let mut children = self.blockchain.children.write(); children.import(parent_hash, hash); children.prepare_transaction(&mut transaction, columns::META, meta_keys::CHILD_PREFIX); From 88b8745f0571be041b479bae520602fd67e2a527 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 4 Feb 2019 14:05:02 +0100 Subject: [PATCH 09/24] fix: children datastructure to not keep children on memory --- core/client/Cargo.toml | 1 + core/client/db/src/lib.rs | 7 +- core/client/src/children.rs | 133 +++++++++++------------------------- core/client/src/in_mem.rs | 6 +- 4 files changed, 46 insertions(+), 101 deletions(-) diff --git a/core/client/Cargo.toml b/core/client/Cargo.toml index bc2911a0ae295..849db36ab59b7 100644 --- a/core/client/Cargo.toml +++ b/core/client/Cargo.toml @@ -33,6 +33,7 @@ sr-api-macros = { path = "../sr-api-macros" } test-client = { package = "substrate-test-client", path = "../test-client" } kvdb-memorydb = { git = "https://github.com/paritytech/parity-common", rev="b0317f649ab2c665b7987b8475878fc4d2e1f81d" } + [features] default = ["std"] std = [ diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index 7ecd30acf2902..a2f106efb98c6 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -131,18 +131,15 @@ pub struct BlockchainDb { db: Arc, meta: Arc, Block::Hash>>>, leaves: RwLock>>, - children: RwLock>, } impl BlockchainDb { fn new(db: Arc) -> Result { let meta = read_meta::(&*db, columns::META, columns::HEADER)?; let leaves = LeafSet::read_from_db(&*db, columns::META, meta_keys::LEAF_PREFIX)?; - let children = ChildrenMap::read_from_db(&*db, columns::META, meta_keys::CHILD_PREFIX)?; Ok(BlockchainDb { db, leaves: RwLock::new(leaves), - children: RwLock::new(children), meta: Arc::new(RwLock::new(meta)), }) } @@ -254,8 +251,8 @@ impl client::blockchain::Backend for BlockchainDb { Ok(self.leaves.read().hashes()) } - fn children(&self, parent_hash: Block::Hash) -> Vec { - self.children.read().hashes(parent_hash) + fn children(&self, parent_hash: Block::Hash) -> Result> { + ChildrenMap::hashes(&self.db, columns::META, meta_keys::CHILD_PREFIX, parent_hash) } } diff --git a/core/client/src/children.rs b/core/client/src/children.rs index 0505db5d13432..e9d3e82e7a632 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -23,6 +23,7 @@ use crate::error; use std::hash::Hash; use std::fmt::Debug; + /// Map of children blocks stored in memory for fast access. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ChildrenMap where @@ -30,8 +31,6 @@ pub struct ChildrenMap where V: Ord + Eq + Hash + Clone + Encode + Decode + Debug, { storage: BTreeMap>, - pending_added: Vec<(K, V)>, - pending_removed: Vec<(K, V)>, } impl ChildrenMap where @@ -42,72 +41,44 @@ impl ChildrenMap where pub fn new() -> Self { Self { storage: BTreeMap::new(), - pending_added: Vec::new(), - pending_removed: Vec::new(), } } - /// Reads a children map from DB. - pub fn read_from_db(db: &KeyValueDB, column: Option, prefix: &[u8]) -> error::Result { - let mut storage: BTreeMap> = BTreeMap::new(); - for (key, value) in db.iter_from_prefix(column, prefix) { - if !key.starts_with(prefix) { break } - let raw_hash = &mut &key[prefix.len()..]; - let parent_hash = match Decode::decode(raw_hash) { - Some(hash) => hash, - None => return Err(error::ErrorKind::Backend("Error decoding hash".into()).into()), - }; - let raw_value = &mut &value[..]; - let child = match Decode::decode(raw_value) { - Some(child) => child, - None => return Err(error::ErrorKind::Backend("Error decoding child".into()).into()), - }; - - match storage.get_mut(&parent_hash) { - Some(children) => { - children.push(child); - }, - None => { - storage.insert(parent_hash, vec![child]); - } - }; - } - Ok(Self { - storage, - pending_added: Vec::new(), - pending_removed: Vec::new(), - }) + pub fn hashes(db: &KeyValueDB, column: Option, prefix: &[u8], + parent_hash: K) -> error::Result> { + + let mut buf = prefix.to_vec(); + parent_hash.using_encoded(|s| buf.extend(s)); + let raw_val = match db.get_by_prefix(column, &buf[..]) { + Some(val) => val, + None => return Ok(vec![]), + }; + let children: Vec = match Decode::decode(&mut &raw_val[..]) { + Some(children) => children, + None => return Err(error::ErrorKind::Backend("Error decoding children".into()).into()), + }; + Ok(children) } - /// Update the children list on import. - pub fn import(&mut self, parent_hash: K, hash: V) { + pub fn import(&mut self, parent_hash: K, child_hash: V) { match self.storage.get_mut(&parent_hash) { - Some(children) => { - children.push(hash.clone()); - } + Some(children) => children.push(child_hash), None => { - self.storage.insert(parent_hash.clone(), vec![hash.clone()]); + self.storage.insert(parent_hash, vec![child_hash]); } - }; - self.pending_added.push((parent_hash, hash)); - } - - /// Write the children list to the database transaction. - pub fn prepare_transaction(&mut self, tx: &mut DBTransaction, column: Option, prefix: &[u8]) { - let mut buf = prefix.to_vec(); - for (parent, child) in self.pending_added.drain(..) { - parent.using_encoded(|s| buf.extend(s)); - tx.put_vec(column, &buf[..], child.encode()); - buf.truncate(prefix.len()); // reuse allocation. } } - /// Gets the hashes of the children blocks of `parent_hash`. - pub fn hashes(&self, parent_hash: K) -> Vec { - match self.storage.get(&parent_hash) { - Some(children) => children.clone(), - None => vec![], + pub fn prepare_transaction(&self, db: &KeyValueDB, tx: &mut DBTransaction, column: Option, prefix: &[u8]) + -> error::Result<()> { + for (parent_hash, children) in self.storage.iter() { + let mut children_db = Self::hashes(db, column, prefix, parent_hash.clone())?; + children_db.extend(children.iter().cloned()); + let mut buf = prefix.to_vec(); + parent_hash.using_encoded(|s| buf.extend(s)); + tx.put_vec(column, &buf[..], children_db.encode()); } + Ok(()) } } @@ -116,48 +87,26 @@ mod tests { use super::*; #[test] - fn children_works() { - let mut children = ChildrenMap::new(); - - children.import(0u32, 0u32); - children.import(1_1, 1_3); - children.import(1_2, 1_4); - - assert!(children.storage.contains_key(&1_1)); - assert!(children.storage.contains_key(&1_2)); - assert!(!children.storage.contains_key(&1_3)); - assert!(!children.storage.contains_key(&1_4)); - } - - #[test] - fn children_hashes() { - let mut children = ChildrenMap::new(); - - children.import(0u32, 0u32); - children.import(1_1, 1_3); - children.import(1_2, 1_4); - children.import(1_2, 1_5); - - assert_eq!(children.hashes(1_1), vec![1_3]); - assert_eq!(children.hashes(1_2), vec![1_4, 1_5]); - } - - #[test] - fn children_flush() { - const PREFIX: &[u8] = b"a"; + fn children_write_read() { + const PREFIX: &[u8] = b"children"; let db = ::kvdb_memorydb::create(0); let mut children = ChildrenMap::new(); + let mut tx = DBTransaction::new(); + children.import(0u32, 0u32); children.import(1_1, 1_3); children.import(1_2, 1_4); - - let mut tx = DBTransaction::new(); - - children.prepare_transaction(&mut tx, None, PREFIX); + children.import(1_1, 1_5); + children.import(1_2, 1_6); + + children.prepare_transaction(&db, &mut tx, None, PREFIX); db.write(tx).unwrap(); - - let children2 = ChildrenMap::read_from_db(&db, None, PREFIX).unwrap(); - assert_eq!(children, children2); + + let r1: Vec = ChildrenMap::hashes(&db, None, PREFIX, 1_1).unwrap(); + let r2: Vec = ChildrenMap::hashes(&db, None, PREFIX, 1_2).unwrap(); + + assert_eq!(r1, vec![1_3, 1_5]); + assert_eq!(r2, vec![1_4, 1_6]); } } \ No newline at end of file diff --git a/core/client/src/in_mem.rs b/core/client/src/in_mem.rs index 1dc4c45f5749b..60fd790a91884 100644 --- a/core/client/src/in_mem.rs +++ b/core/client/src/in_mem.rs @@ -99,7 +99,6 @@ struct BlockchainStorage { header_cht_roots: HashMap, Block::Hash>, changes_trie_cht_roots: HashMap, Block::Hash>, leaves: LeafSet>, - children: ChildrenMap, aux: HashMap, Vec>, } @@ -150,7 +149,6 @@ impl Blockchain { header_cht_roots: HashMap::new(), changes_trie_cht_roots: HashMap::new(), leaves: LeafSet::new(), - children: ChildrenMap::new(), aux: HashMap::new(), })); Blockchain { @@ -366,8 +364,8 @@ impl blockchain::Backend for Blockchain { Ok(self.storage.read().leaves.hashes()) } - fn children(&self, parent_hash: Block::Hash) -> Vec { - self.storage.read().children.hashes(parent_hash) + fn children(&self, _id: Block::Hash) -> Vec { + unimplemented!() } } From ef925d7fc65a641ccd4fc0ed14e49b535c001df7 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 4 Feb 2019 14:46:33 +0100 Subject: [PATCH 10/24] fix: types --- core/client/Cargo.toml | 1 - core/client/db/src/lib.rs | 8 ++++---- core/client/src/blockchain.rs | 2 +- core/client/src/client.rs | 11 +++++------ core/client/src/in_mem.rs | 2 +- core/client/src/leaves.rs | 3 +-- core/client/src/light/blockchain.rs | 2 +- 7 files changed, 13 insertions(+), 16 deletions(-) diff --git a/core/client/Cargo.toml b/core/client/Cargo.toml index 849db36ab59b7..bc2911a0ae295 100644 --- a/core/client/Cargo.toml +++ b/core/client/Cargo.toml @@ -33,7 +33,6 @@ sr-api-macros = { path = "../sr-api-macros" } test-client = { package = "substrate-test-client", path = "../test-client" } kvdb-memorydb = { git = "https://github.com/paritytech/parity-common", rev="b0317f649ab2c665b7987b8475878fc4d2e1f81d" } - [features] default = ["std"] std = [ diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index a2f106efb98c6..af0fda0ff059a 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -251,8 +251,8 @@ impl client::blockchain::Backend for BlockchainDb { Ok(self.leaves.read().hashes()) } - fn children(&self, parent_hash: Block::Hash) -> Result> { - ChildrenMap::hashes(&self.db, columns::META, meta_keys::CHILD_PREFIX, parent_hash) + fn children(&self, parent_hash: Block::Hash) -> Result, client::error::Error> { + ChildrenMap::hashes(&*self.db, columns::META, meta_keys::CHILD_PREFIX, parent_hash) } } @@ -859,9 +859,9 @@ impl> Backend { let displaced_leaf = leaves.import(hash, number, parent_hash); leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX); - let mut children = self.blockchain.children.write(); + let mut children = ChildrenMap::new(); children.import(parent_hash, hash); - children.prepare_transaction(&mut transaction, columns::META, meta_keys::CHILD_PREFIX); + children.prepare_transaction(&*self.storage.db, &mut transaction, columns::META, meta_keys::CHILD_PREFIX); displaced_leaf }; diff --git a/core/client/src/blockchain.rs b/core/client/src/blockchain.rs index 9541604b7284e..37ca260b19565 100644 --- a/core/client/src/blockchain.rs +++ b/core/client/src/blockchain.rs @@ -86,7 +86,7 @@ pub trait Backend: HeaderBackend { fn leaves(&self) -> Result>; /// Returns the hashes of all blocks that are children of the block with `parent_hash`. - fn children(&self, parent_hash: Block::Hash) -> Vec; + fn children(&self, parent_hash: Block::Hash) -> Result>; } /// Blockchain optional data cache. diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 7a4549512c627..52ead367f1d57 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1230,18 +1230,18 @@ impl Client where Ok(None) } - fn get_descendants(&self, target: Block::Hash, maybe_skip: Option) -> Vec { - let mut children = self.backend.blockchain().children(target); + fn get_descendants(&self, target: Block::Hash, maybe_skip: Option) -> error::Result> { + let mut children = self.backend.blockchain().children(target)?; if let Some(skip) = maybe_skip { children.retain(|&c| c != skip); } let mut descendants = Vec::new(); for child in children { descendants.push(child); - let d = self.get_descendants(child, None); + let d = self.get_descendants(child, None)?; descendants.extend(d); } - descendants + Ok(descendants) } /// Gets the uncles of the block with `target_hash` going back `max_generation` ancestors. @@ -1265,7 +1265,7 @@ impl Client where let mut ancestor = load_header(*current.parent_hash())?; for _generation in 0..max_generation.as_() { - uncles.extend(self.get_descendants(ancestor.hash(), Some(current.hash()))); + uncles.extend(self.get_descendants(ancestor.hash(), Some(current.hash()))?); current = ancestor; if genesis == current { break; } ancestor = load_header(*current.parent_hash())?; @@ -1891,7 +1891,6 @@ pub(crate) mod tests { assert_eq!(a2.hash(), client.best_containing(a1.hash(), None).unwrap().unwrap()); assert_eq!(a2.hash(), client.best_containing(a2.hash(), None).unwrap().unwrap()); } - #[test] fn best_containing_with_multiple_forks() { diff --git a/core/client/src/in_mem.rs b/core/client/src/in_mem.rs index 60fd790a91884..d415d6956659d 100644 --- a/core/client/src/in_mem.rs +++ b/core/client/src/in_mem.rs @@ -364,7 +364,7 @@ impl blockchain::Backend for Blockchain { Ok(self.storage.read().leaves.hashes()) } - fn children(&self, _id: Block::Hash) -> Vec { + fn children(&self, _id: Block::Hash) -> error::Result> { unimplemented!() } } diff --git a/core/client/src/leaves.rs b/core/client/src/leaves.rs index bc723ce0a5da6..92bdfa64ec640 100644 --- a/core/client/src/leaves.rs +++ b/core/client/src/leaves.rs @@ -91,8 +91,7 @@ impl LeafSet where Some(hash) => hash, None => return Err(error::ErrorKind::Backend("Error decoding hash".into()).into()), }; - let raw_value = &mut &value[..]; - let number = match Decode::decode(raw_value) { + let number = match Decode::decode(&mut &value[..]) { Some(number) => number, None => return Err(error::ErrorKind::Backend("Error decoding number".into()).into()), }; diff --git a/core/client/src/light/blockchain.rs b/core/client/src/light/blockchain.rs index d6210c2b2e0d6..bc956730c9bfb 100644 --- a/core/client/src/light/blockchain.rs +++ b/core/client/src/light/blockchain.rs @@ -164,7 +164,7 @@ impl BlockchainBackend for Blockchain where Block: Blo unimplemented!() } - fn children(&self, _id: Block::Hash) -> Vec { + fn children(&self, _id: Block::Hash) -> ClientResult> { unimplemented!() } } From b94a380418a6642f77a99ead6d7215098b81466e Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 4 Feb 2019 16:36:07 +0100 Subject: [PATCH 11/24] fix tests --- core/client/db/src/lib.rs | 7 +++++-- core/client/src/blockchain.rs | 1 - core/client/src/children.rs | 17 ++++++++++++----- core/client/src/in_mem.rs | 6 ++++-- core/client/src/light/blockchain.rs | 2 +- 5 files changed, 22 insertions(+), 11 deletions(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index af0fda0ff059a..fd70b0e4945e3 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -131,15 +131,18 @@ pub struct BlockchainDb { db: Arc, meta: Arc, Block::Hash>>>, leaves: RwLock>>, + children: RwLock>, } impl BlockchainDb { fn new(db: Arc) -> Result { let meta = read_meta::(&*db, columns::META, columns::HEADER)?; let leaves = LeafSet::read_from_db(&*db, columns::META, meta_keys::LEAF_PREFIX)?; + let children = ChildrenMap::new(); Ok(BlockchainDb { db, leaves: RwLock::new(leaves), + children: RwLock::new(children), meta: Arc::new(RwLock::new(meta)), }) } @@ -252,7 +255,7 @@ impl client::blockchain::Backend for BlockchainDb { } fn children(&self, parent_hash: Block::Hash) -> Result, client::error::Error> { - ChildrenMap::hashes(&*self.db, columns::META, meta_keys::CHILD_PREFIX, parent_hash) + self.children.read().hashes(&*self.db, columns::META, meta_keys::CHILD_PREFIX, parent_hash) } } @@ -861,7 +864,7 @@ impl> Backend { let mut children = ChildrenMap::new(); children.import(parent_hash, hash); - children.prepare_transaction(&*self.storage.db, &mut transaction, columns::META, meta_keys::CHILD_PREFIX); + children.prepare_transaction(&*self.storage.db, &mut transaction, columns::META, meta_keys::CHILD_PREFIX)?; displaced_leaf }; diff --git a/core/client/src/blockchain.rs b/core/client/src/blockchain.rs index 37ca260b19565..f4cdaeb374a2e 100644 --- a/core/client/src/blockchain.rs +++ b/core/client/src/blockchain.rs @@ -85,7 +85,6 @@ pub trait Backend: HeaderBackend { /// Results must be ordered best (longest, heighest) chain first. fn leaves(&self) -> Result>; - /// Returns the hashes of all blocks that are children of the block with `parent_hash`. fn children(&self, parent_hash: Block::Hash) -> Result>; } diff --git a/core/client/src/children.rs b/core/client/src/children.rs index e9d3e82e7a632..9449cacdf4cbc 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -44,12 +44,12 @@ impl ChildrenMap where } } - pub fn hashes(db: &KeyValueDB, column: Option, prefix: &[u8], + pub fn hashes(&self, db: &KeyValueDB, column: Option, prefix: &[u8], parent_hash: K) -> error::Result> { let mut buf = prefix.to_vec(); parent_hash.using_encoded(|s| buf.extend(s)); - let raw_val = match db.get_by_prefix(column, &buf[..]) { + let raw_val = match db.get(column, &buf[..]).unwrap() { Some(val) => val, None => return Ok(vec![]), }; @@ -60,6 +60,13 @@ impl ChildrenMap where Ok(children) } + pub fn hashes_from_mem(&self, parent_hash: K) -> Vec { + match self.storage.get(&parent_hash) { + Some(children) => children.clone(), + None => vec![], + } + } + pub fn import(&mut self, parent_hash: K, child_hash: V) { match self.storage.get_mut(&parent_hash) { Some(children) => children.push(child_hash), @@ -72,7 +79,7 @@ impl ChildrenMap where pub fn prepare_transaction(&self, db: &KeyValueDB, tx: &mut DBTransaction, column: Option, prefix: &[u8]) -> error::Result<()> { for (parent_hash, children) in self.storage.iter() { - let mut children_db = Self::hashes(db, column, prefix, parent_hash.clone())?; + let mut children_db = self.hashes(db, column, prefix, parent_hash.clone())?; children_db.extend(children.iter().cloned()); let mut buf = prefix.to_vec(); parent_hash.using_encoded(|s| buf.extend(s)); @@ -103,8 +110,8 @@ mod tests { children.prepare_transaction(&db, &mut tx, None, PREFIX); db.write(tx).unwrap(); - let r1: Vec = ChildrenMap::hashes(&db, None, PREFIX, 1_1).unwrap(); - let r2: Vec = ChildrenMap::hashes(&db, None, PREFIX, 1_2).unwrap(); + let r1: Vec = children.hashes(&db, None, PREFIX, 1_1).unwrap(); + let r2: Vec = children.hashes(&db, None, PREFIX, 1_2).unwrap(); assert_eq!(r1, vec![1_3, 1_5]); assert_eq!(r2, vec![1_4, 1_6]); diff --git a/core/client/src/in_mem.rs b/core/client/src/in_mem.rs index d415d6956659d..c512b35c0628d 100644 --- a/core/client/src/in_mem.rs +++ b/core/client/src/in_mem.rs @@ -99,6 +99,7 @@ struct BlockchainStorage { header_cht_roots: HashMap, Block::Hash>, changes_trie_cht_roots: HashMap, Block::Hash>, leaves: LeafSet>, + children: ChildrenMap, aux: HashMap, Vec>, } @@ -149,6 +150,7 @@ impl Blockchain { header_cht_roots: HashMap::new(), changes_trie_cht_roots: HashMap::new(), leaves: LeafSet::new(), + children: ChildrenMap::new(), aux: HashMap::new(), })); Blockchain { @@ -364,8 +366,8 @@ impl blockchain::Backend for Blockchain { Ok(self.storage.read().leaves.hashes()) } - fn children(&self, _id: Block::Hash) -> error::Result> { - unimplemented!() + fn children(&self, parent_hash: Block::Hash) -> error::Result> { + Ok(self.storage.read().children.hashes_from_mem(parent_hash)) } } diff --git a/core/client/src/light/blockchain.rs b/core/client/src/light/blockchain.rs index bc956730c9bfb..73d530ce45545 100644 --- a/core/client/src/light/blockchain.rs +++ b/core/client/src/light/blockchain.rs @@ -164,7 +164,7 @@ impl BlockchainBackend for Blockchain where Block: Blo unimplemented!() } - fn children(&self, _id: Block::Hash) -> ClientResult> { + fn children(&self, _parent_hash: Block::Hash) -> ClientResult> { unimplemented!() } } From aab6e23439b8e1d8f83c93a8e962ec9b6eabdc6e Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 4 Feb 2019 18:09:33 +0100 Subject: [PATCH 12/24] fix: it should use the map already created --- core/client/db/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index fd70b0e4945e3..91b3f18bd48b3 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -862,7 +862,7 @@ impl> Backend { let displaced_leaf = leaves.import(hash, number, parent_hash); leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX); - let mut children = ChildrenMap::new(); + let mut children = self.blockchain().children.write(); children.import(parent_hash, hash); children.prepare_transaction(&*self.storage.db, &mut transaction, columns::META, meta_keys::CHILD_PREFIX)?; From afa09cc4387ec3efe2369e0c0e97df959b6395be Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 4 Feb 2019 18:09:59 +0100 Subject: [PATCH 13/24] chore: add documentation --- core/client/src/blockchain.rs | 1 + core/client/src/children.rs | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/core/client/src/blockchain.rs b/core/client/src/blockchain.rs index f4cdaeb374a2e..eb9ce1342eade 100644 --- a/core/client/src/blockchain.rs +++ b/core/client/src/blockchain.rs @@ -85,6 +85,7 @@ pub trait Backend: HeaderBackend { /// Results must be ordered best (longest, heighest) chain first. fn leaves(&self) -> Result>; + /// Return hashes of all blocks that are children of the block with `parent_hash`. fn children(&self, parent_hash: Block::Hash) -> Result>; } diff --git a/core/client/src/children.rs b/core/client/src/children.rs index 9449cacdf4cbc..25495dccffbae 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -44,6 +44,7 @@ impl ChildrenMap where } } + /// Returns the hashes of the children blocks of the block with `parent_hash`. pub fn hashes(&self, db: &KeyValueDB, column: Option, prefix: &[u8], parent_hash: K) -> error::Result> { @@ -60,6 +61,8 @@ impl ChildrenMap where Ok(children) } + /// Returns the hashes of the children blocks of the block with `parent_hash`. + /// It doesn't read the database. pub fn hashes_from_mem(&self, parent_hash: K) -> Vec { match self.storage.get(&parent_hash) { Some(children) => children.clone(), @@ -67,6 +70,8 @@ impl ChildrenMap where } } + /// Import the hash `child_hash` as child of `parent_hash`. + /// It doesn't save changes to database. pub fn import(&mut self, parent_hash: K, child_hash: V) { match self.storage.get_mut(&parent_hash) { Some(children) => children.push(child_hash), @@ -76,7 +81,9 @@ impl ChildrenMap where } } - pub fn prepare_transaction(&self, db: &KeyValueDB, tx: &mut DBTransaction, column: Option, prefix: &[u8]) + /// Prepare the transaction `tx` that saves the content of the ChildrenMap to database. + /// It clears the content of ChildrenMap. + pub fn prepare_transaction(&mut self, db: &KeyValueDB, tx: &mut DBTransaction, column: Option, prefix: &[u8]) -> error::Result<()> { for (parent_hash, children) in self.storage.iter() { let mut children_db = self.hashes(db, column, prefix, parent_hash.clone())?; @@ -85,6 +92,7 @@ impl ChildrenMap where parent_hash.using_encoded(|s| buf.extend(s)); tx.put_vec(column, &buf[..], children_db.encode()); } + self.storage.clear(); Ok(()) } } From 6e1478884f29e6bea4578fcd96f7d6a1f52bdc8c Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 4 Feb 2019 18:25:12 +0100 Subject: [PATCH 14/24] chore: remove unused imports --- core/client/src/children.rs | 2 +- core/client/src/client.rs | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/core/client/src/children.rs b/core/client/src/children.rs index 25495dccffbae..a9da0f04c1291 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -115,7 +115,7 @@ mod tests { children.import(1_1, 1_5); children.import(1_2, 1_6); - children.prepare_transaction(&db, &mut tx, None, PREFIX); + let _ = children.prepare_transaction(&db, &mut tx, None, PREFIX); db.write(tx).unwrap(); let r1: Vec = children.hashes(&db, None, PREFIX, 1_1).unwrap(); diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 52ead367f1d57..7300ebacac983 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1751,13 +1751,10 @@ pub(crate) mod tests { #[test] fn uncles_with_genesis_block() { - use test_client::blockchain::Backend; - // block tree: // G -> A1 -> A2 let client = test_client::new(); - let genesis_hash = client.info().unwrap().chain.genesis_hash; // G -> A1 let a1 = client.new_block().unwrap().bake().unwrap(); @@ -1772,11 +1769,6 @@ pub(crate) mod tests { #[test] fn uncles_with_multiple_forks() { - // NOTE: we use the version of the trait from `test_client` - // because that is actually different than the version linked to - // in the test facade crate. - use test_client::blockchain::Backend as BlockchainBackendT; - // block tree: // G -> A1 -> A2 -> A3 -> A4 -> A5 // A1 -> B2 -> B3 -> B4 From b5c151a011b837e7edee3742ccf58c1a98ca729c Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Wed, 13 Feb 2019 15:53:56 +0100 Subject: [PATCH 15/24] fix tests --- core/client/src/children.rs | 44 ++++++++++----------- core/client/src/client.rs | 67 +++++++++++++++----------------- core/sr-primitives/src/traits.rs | 2 +- 3 files changed, 52 insertions(+), 61 deletions(-) diff --git a/core/client/src/children.rs b/core/client/src/children.rs index a9da0f04c1291..a87f5680183fe 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -15,70 +15,66 @@ // along with Substrate. If not, see . -use std::collections::BTreeMap; -use std::cmp::Ord; +use std::collections::HashMap; use kvdb::{KeyValueDB, DBTransaction}; -use codec::{Encode, Decode}; +use parity_codec::{Encode, Decode}; use crate::error; use std::hash::Hash; -use std::fmt::Debug; /// Map of children blocks stored in memory for fast access. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ChildrenMap where - K: Ord + Eq + Hash + Clone + Encode + Decode + Debug, - V: Ord + Eq + Hash + Clone + Encode + Decode + Debug, + K: Eq + Hash + Clone + Encode + Decode, + V: Eq + Hash + Clone + Encode + Decode, { - storage: BTreeMap>, + storage: HashMap>, } impl ChildrenMap where - K: Ord + Eq + Hash + Clone + Encode + Decode + Debug, - V: Ord + Eq + Hash + Clone + Encode + Decode + Debug, + K: Eq + Hash + Clone + Encode + Decode, + V: Eq + Hash + Clone + Encode + Decode, { /// Creates an empty children map. pub fn new() -> Self { Self { - storage: BTreeMap::new(), + storage: HashMap::new(), } } /// Returns the hashes of the children blocks of the block with `parent_hash`. - pub fn hashes(&self, db: &KeyValueDB, column: Option, prefix: &[u8], - parent_hash: K) -> error::Result> { - + pub fn hashes(&self, db: &KeyValueDB, column: Option, prefix: &[u8], parent_hash: K) -> error::Result> { let mut buf = prefix.to_vec(); parent_hash.using_encoded(|s| buf.extend(s)); - let raw_val = match db.get(column, &buf[..]).unwrap() { + + let raw_val_opt = match db.get(column, &buf[..]) { + Ok(raw_val_opt) => raw_val_opt, + Err(_) => return Err(error::ErrorKind::Backend("Error reading value from database".into()).into()), + }; + + let raw_val = match raw_val_opt { Some(val) => val, None => return Ok(vec![]), }; + let children: Vec = match Decode::decode(&mut &raw_val[..]) { Some(children) => children, None => return Err(error::ErrorKind::Backend("Error decoding children".into()).into()), }; + Ok(children) } /// Returns the hashes of the children blocks of the block with `parent_hash`. /// It doesn't read the database. pub fn hashes_from_mem(&self, parent_hash: K) -> Vec { - match self.storage.get(&parent_hash) { - Some(children) => children.clone(), - None => vec![], - } + self.storage.get(&parent_hash).cloned().unwrap_or_default() } /// Import the hash `child_hash` as child of `parent_hash`. /// It doesn't save changes to database. pub fn import(&mut self, parent_hash: K, child_hash: V) { - match self.storage.get_mut(&parent_hash) { - Some(children) => children.push(child_hash), - None => { - self.storage.insert(parent_hash, vec![child_hash]); - } - } + self.storage.entry(parent_hash).or_insert_with(Vec::new).push(child_hash); } /// Prepare the transaction `tx` that saves the content of the ChildrenMap to database. diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 7300ebacac983..c025d2c6fc9cb 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1230,48 +1230,43 @@ impl Client where Ok(None) } - fn get_descendants(&self, target: Block::Hash, maybe_skip: Option) -> error::Result> { + fn get_children(&self, target: Block::Hash, maybe_skip: Option) -> error::Result> { let mut children = self.backend.blockchain().children(target)?; if let Some(skip) = maybe_skip { children.retain(|&c| c != skip); } - let mut descendants = Vec::new(); - for child in children { - descendants.push(child); - let d = self.get_descendants(child, None)?; - descendants.extend(d); - } - Ok(descendants) + Ok(children) } /// Gets the uncles of the block with `target_hash` going back `max_generation` ancestors. - pub fn uncles(&self, target_hash: Block::Hash, max_generation: NumberFor) - -> error::Result>> - { - let load_header = |id: Block::Hash| { - match self.backend.blockchain().header(BlockId::Hash(id)) { - Ok(Some(hdr)) => Ok(hdr), - Ok(None) => Err(ErrorKind::UnknownBlock(format!("Unknown block {:?}", id)).into()), - Err(e) => Err(e), + pub fn uncles(&self, target_hash: Block::Hash, max_generation: NumberFor) -> error::Result> { + let load_header = |id: Block::Hash| -> error::Result { + match self.backend.blockchain().header(BlockId::Hash(id))? { + Some(hdr) => Ok(hdr), + None => Err(ErrorKind::UnknownBlock(format!("Unknown block {:?}", id)).into()), } }; - let genesis_hash = self.backend.blockchain().info().unwrap().genesis_hash; - let genesis = load_header(genesis_hash)?; - let mut current = load_header(target_hash)?; - let mut uncles = vec![]; + let genesis_hash = self.backend.blockchain().info()?.genesis_hash; + if genesis_hash == target_hash { return Ok(Vec::new()); } - if genesis == current { return Ok(Some(uncles)); } - let mut ancestor = load_header(*current.parent_hash())?; + let mut current_hash = target_hash; + let mut current = load_header(current_hash)?; + let mut ancestor_hash = *current.parent_hash(); + let mut ancestor = load_header(ancestor_hash)?; + let mut uncles = Vec::new(); for _generation in 0..max_generation.as_() { - uncles.extend(self.get_descendants(ancestor.hash(), Some(current.hash()))?); + let children = self.get_children(ancestor_hash, Some(current_hash))?; + uncles.extend(children); + current_hash = ancestor_hash; + if genesis_hash == current_hash { break; } current = ancestor; - if genesis == current { break; } - ancestor = load_header(*current.parent_hash())?; + ancestor_hash = *current.parent_hash(); + ancestor = load_header(ancestor_hash)?; } - Ok(Some(uncles)) + Ok(uncles) } fn changes_trie_config(&self) -> Result, Error> { @@ -1750,7 +1745,7 @@ pub(crate) mod tests { } #[test] - fn uncles_with_genesis_block() { + fn uncles_with_only_ancestors() { // block tree: // G -> A1 -> A2 @@ -1764,7 +1759,7 @@ pub(crate) mod tests { let a2 = client.new_block().unwrap().bake().unwrap(); client.import(BlockOrigin::Own, a2.clone()).unwrap(); let v: Vec = Vec::new(); - assert_eq!(v, client.uncles(a2.hash(), 3).unwrap().unwrap()); + assert_eq!(v, client.uncles(a2.hash(), 3).unwrap()); } #[test] @@ -1843,22 +1838,22 @@ pub(crate) mod tests { assert_eq!(client.info().unwrap().chain.best_hash, a5.hash()); let genesis_hash = client.info().unwrap().chain.genesis_hash; - let uncles1 = client.uncles(a4.hash(), 10).unwrap().unwrap(); - assert_eq!(vec![b2.hash(), b3.hash(), b4.hash(), c3.hash(), d2.hash()].len(), uncles1.len()); + let uncles1 = client.uncles(a4.hash(), 10).unwrap(); + assert_eq!(2, uncles1.len()); - let uncles2 = client.uncles(a4.hash(), 0).unwrap().unwrap(); + let uncles2 = client.uncles(a4.hash(), 0).unwrap(); assert_eq!(0, uncles2.len()); - let uncles3 = client.uncles(a1.hash(), 10).unwrap().unwrap(); + let uncles3 = client.uncles(a1.hash(), 10).unwrap(); assert_eq!(0, uncles3.len()); - let uncles4 = client.uncles(genesis_hash, 10).unwrap().unwrap(); + let uncles4 = client.uncles(genesis_hash, 10).unwrap(); assert_eq!(0, uncles4.len()); - let uncles5 = client.uncles(d2.hash(), 10).unwrap().unwrap(); - assert_eq!(8, uncles5.len()); + let uncles5 = client.uncles(d2.hash(), 10).unwrap(); + assert_eq!(2, uncles5.len()); - let uncles6 = client.uncles(b3.hash(), 1).unwrap().unwrap(); + let uncles6 = client.uncles(b3.hash(), 1).unwrap(); assert_eq!(1, uncles6.len()); } diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 4adb20e3c3bfc..2206072d0d3a3 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -346,7 +346,7 @@ macro_rules! tuple_impl { tuple_impl!(A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X, Y, Z,); /// Abstraction around hashing -pub trait Hash: 'static + MaybeSerializeDebug + Clone + Eq + PartialEq + PartialOrd + Ord { // Stupid bug in the Rust compiler believes derived +pub trait Hash: 'static + MaybeSerializeDebug + Clone + Eq + PartialEq { // Stupid bug in the Rust compiler believes derived // traits must be fulfilled by all type parameters. /// The hash type produced. type Output: Member + MaybeSerializeDebug + AsRef<[u8]> + AsMut<[u8]> + PartialEq + Eq + PartialOrd + Ord; From 107984e1c142a933ab3e1fb8197137d2ac186807 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Wed, 13 Feb 2019 16:26:00 +0100 Subject: [PATCH 16/24] fix: Order of Hash --- core/client/db/src/lib.rs | 2 +- core/client/src/children.rs | 2 +- core/sr-primitives/src/traits.rs | 8 ++++---- srml/system/src/lib.rs | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index 91b3f18bd48b3..87f8b7913fa50 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -862,7 +862,7 @@ impl> Backend { let displaced_leaf = leaves.import(hash, number, parent_hash); leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX); - let mut children = self.blockchain().children.write(); + let mut children = self.blockchain.children.write(); children.import(parent_hash, hash); children.prepare_transaction(&*self.storage.db, &mut transaction, columns::META, meta_keys::CHILD_PREFIX)?; diff --git a/core/client/src/children.rs b/core/client/src/children.rs index a87f5680183fe..2cdbb592ecc15 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -54,7 +54,7 @@ impl ChildrenMap where let raw_val = match raw_val_opt { Some(val) => val, - None => return Ok(vec![]), + None => return Ok(Vec::new()), }; let children: Vec = match Decode::decode(&mut &raw_val[..]) { diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 2206072d0d3a3..1091bce473bf0 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -349,7 +349,7 @@ tuple_impl!(A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, pub trait Hash: 'static + MaybeSerializeDebug + Clone + Eq + PartialEq { // Stupid bug in the Rust compiler believes derived // traits must be fulfilled by all type parameters. /// The hash type produced. - type Output: Member + MaybeSerializeDebug + AsRef<[u8]> + AsMut<[u8]> + PartialEq + Eq + PartialOrd + Ord; + type Output: Member + MaybeSerializeDebug + AsRef<[u8]> + AsMut<[u8]> + PartialEq + Eq; /// Produce the hash of some byte-slice. fn hash(s: &[u8]) -> Self::Output; @@ -383,7 +383,7 @@ pub trait Hash: 'static + MaybeSerializeDebug + Clone + Eq + PartialEq { // Stup } /// Blake2-256 Hash implementation. -#[derive(PartialEq, Eq, PartialOrd, Ord, Clone)] +#[derive(PartialEq, Eq, Clone)] #[cfg_attr(feature = "std", derive(Debug, Serialize, Deserialize))] pub struct BlakeTwo256; @@ -544,7 +544,7 @@ pub trait Header: Clone + Send + Sync + Codec + Eq + MaybeSerializeDebugButNotDe /// Header number. type Number: Member + MaybeSerializeDebug + ::rstd::hash::Hash + Copy + MaybeDisplay + SimpleArithmetic + Codec; /// Header hash type - type Hash: Member + MaybeSerializeDebug + ::rstd::hash::Hash + Copy + MaybeDisplay + Default + SimpleBitOps + Codec + AsRef<[u8]> + AsMut<[u8]> + PartialEq + Eq + PartialOrd + Ord; + type Hash: Member + MaybeSerializeDebug + ::rstd::hash::Hash + Copy + MaybeDisplay + Default + SimpleBitOps + Codec + AsRef<[u8]> + AsMut<[u8]>; /// Hashing algorithm type Hashing: Hash; /// Digest type @@ -602,7 +602,7 @@ pub trait Block: Clone + Send + Sync + Codec + Eq + MaybeSerializeDebugButNotDes /// Header type. type Header: Header; /// Block hash type. - type Hash: Member + MaybeSerializeDebug + ::rstd::hash::Hash + Copy + MaybeDisplay + Default + SimpleBitOps + Codec + AsRef<[u8]> + AsMut<[u8]> + PartialEq + Eq + PartialOrd + Ord; + type Hash: Member + MaybeSerializeDebug + ::rstd::hash::Hash + Copy + MaybeDisplay + Default + SimpleBitOps + Codec + AsRef<[u8]> + AsMut<[u8]>; /// Returns a reference to the header. fn header(&self) -> &Self::Header; diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 8bb26d019b2ba..8f3fd1df8cbaf 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -78,7 +78,7 @@ pub trait Trait: 'static + Eq + Clone { type Origin: Into>> + From>; type Index: Parameter + Member + MaybeSerializeDebugButNotDeserialize + Default + MaybeDisplay + SimpleArithmetic + Copy; type BlockNumber: Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleArithmetic + Default + Bounded + Copy + rstd::hash::Hash; - type Hash: Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleBitOps + Default + Copy + CheckEqual + rstd::hash::Hash + AsRef<[u8]> + AsMut<[u8]> + PartialEq + Eq + PartialOrd + Ord; + type Hash: Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleBitOps + Default + Copy + CheckEqual + rstd::hash::Hash + AsRef<[u8]> + AsMut<[u8]>; type Hashing: Hash; type Digest: Parameter + Member + MaybeSerializeDebugButNotDeserialize + Default + traits::Digest; type AccountId: Parameter + Member + MaybeSerializeDebug + MaybeDisplay + Ord + Default; From 3fae6cafc8840427daf335e81ed9069192da5883 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Wed, 13 Feb 2019 20:48:38 +0100 Subject: [PATCH 17/24] fix: remove get_children function --- core/client/src/client.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index c025d2c6fc9cb..d180b0288f9b4 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1230,14 +1230,6 @@ impl Client where Ok(None) } - fn get_children(&self, target: Block::Hash, maybe_skip: Option) -> error::Result> { - let mut children = self.backend.blockchain().children(target)?; - if let Some(skip) = maybe_skip { - children.retain(|&c| c != skip); - } - Ok(children) - } - /// Gets the uncles of the block with `target_hash` going back `max_generation` ancestors. pub fn uncles(&self, target_hash: Block::Hash, max_generation: NumberFor) -> error::Result> { let load_header = |id: Block::Hash| -> error::Result { @@ -1257,8 +1249,8 @@ impl Client where let mut uncles = Vec::new(); for _generation in 0..max_generation.as_() { - let children = self.get_children(ancestor_hash, Some(current_hash))?; - uncles.extend(children); + let children = self.backend.blockchain().children(ancestor_hash)?; + uncles.extend(children.into_iter().filter(|h| h != ¤t_hash)); current_hash = ancestor_hash; if genesis_hash == current_hash { break; } current = ancestor; From 6809938fb4f2935c936e83f8525735527e519d0a Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Thu, 14 Feb 2019 17:03:59 +0100 Subject: [PATCH 18/24] fix: separate HashMap from db --- core/client/db/src/lib.rs | 10 ++---- core/client/src/children.rs | 70 +++++++++++++------------------------ core/client/src/in_mem.rs | 6 ++-- 3 files changed, 29 insertions(+), 57 deletions(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index 87f8b7913fa50..b8127d93dafd4 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -131,18 +131,15 @@ pub struct BlockchainDb { db: Arc, meta: Arc, Block::Hash>>>, leaves: RwLock>>, - children: RwLock>, } impl BlockchainDb { fn new(db: Arc) -> Result { let meta = read_meta::(&*db, columns::META, columns::HEADER)?; let leaves = LeafSet::read_from_db(&*db, columns::META, meta_keys::LEAF_PREFIX)?; - let children = ChildrenMap::new(); Ok(BlockchainDb { db, leaves: RwLock::new(leaves), - children: RwLock::new(children), meta: Arc::new(RwLock::new(meta)), }) } @@ -255,7 +252,7 @@ impl client::blockchain::Backend for BlockchainDb { } fn children(&self, parent_hash: Block::Hash) -> Result, client::error::Error> { - self.children.read().hashes(&*self.db, columns::META, meta_keys::CHILD_PREFIX, parent_hash) + ChildrenMap::children_hashes(&*self.db, columns::META, meta_keys::CHILD_PREFIX, parent_hash) } } @@ -861,10 +858,7 @@ impl> Backend { let mut leaves = self.blockchain.leaves.write(); let displaced_leaf = leaves.import(hash, number, parent_hash); leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX); - - let mut children = self.blockchain.children.write(); - children.import(parent_hash, hash); - children.prepare_transaction(&*self.storage.db, &mut transaction, columns::META, meta_keys::CHILD_PREFIX)?; + ChildrenMap::prepare_transaction(&*self.storage.db, &mut transaction, columns::META, meta_keys::CHILD_PREFIX, parent_hash, hash)?; displaced_leaf }; diff --git a/core/client/src/children.rs b/core/client/src/children.rs index 2cdbb592ecc15..ae7882d110313 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -14,36 +14,21 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . - -use std::collections::HashMap; use kvdb::{KeyValueDB, DBTransaction}; use parity_codec::{Encode, Decode}; use crate::error; use std::hash::Hash; -/// Map of children blocks stored in memory for fast access. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct ChildrenMap where - K: Eq + Hash + Clone + Encode + Decode, - V: Eq + Hash + Clone + Encode + Decode, -{ - storage: HashMap>, -} - -impl ChildrenMap where - K: Eq + Hash + Clone + Encode + Decode, - V: Eq + Hash + Clone + Encode + Decode, -{ - /// Creates an empty children map. - pub fn new() -> Self { - Self { - storage: HashMap::new(), - } - } +/// Map of father to children blocks. +pub struct ChildrenMap; +impl ChildrenMap { /// Returns the hashes of the children blocks of the block with `parent_hash`. - pub fn hashes(&self, db: &KeyValueDB, column: Option, prefix: &[u8], parent_hash: K) -> error::Result> { + pub fn children_hashes< + K: Eq + Hash + Clone + Encode + Decode, + V: Eq + Hash + Clone + Encode + Decode, + >(db: &KeyValueDB, column: Option, prefix: &[u8], parent_hash: K) -> error::Result> { let mut buf = prefix.to_vec(); parent_hash.using_encoded(|s| buf.extend(s)); @@ -65,30 +50,23 @@ impl ChildrenMap where Ok(children) } - /// Returns the hashes of the children blocks of the block with `parent_hash`. - /// It doesn't read the database. - pub fn hashes_from_mem(&self, parent_hash: K) -> Vec { - self.storage.get(&parent_hash).cloned().unwrap_or_default() - } - - /// Import the hash `child_hash` as child of `parent_hash`. - /// It doesn't save changes to database. - pub fn import(&mut self, parent_hash: K, child_hash: V) { - self.storage.entry(parent_hash).or_insert_with(Vec::new).push(child_hash); - } - - /// Prepare the transaction `tx` that saves the content of the ChildrenMap to database. - /// It clears the content of ChildrenMap. - pub fn prepare_transaction(&mut self, db: &KeyValueDB, tx: &mut DBTransaction, column: Option, prefix: &[u8]) - -> error::Result<()> { - for (parent_hash, children) in self.storage.iter() { - let mut children_db = self.hashes(db, column, prefix, parent_hash.clone())?; - children_db.extend(children.iter().cloned()); - let mut buf = prefix.to_vec(); - parent_hash.using_encoded(|s| buf.extend(s)); - tx.put_vec(column, &buf[..], children_db.encode()); - } - self.storage.clear(); + /// Prepare the database transaction `tx` that adds `child_hash` to the children of `parent_hash`. + pub fn prepare_transaction< + K: Eq + Hash + Clone + Encode + Decode, + V: Eq + Hash + Clone + Encode + Decode, + >( + db: &KeyValueDB, + tx: &mut DBTransaction, + column: Option, + prefix: &[u8], + parent_hash: K, + child_hash: V, + ) -> error::Result<()> { + let mut children_db = Self::children_hashes(db, column, prefix, parent_hash.clone())?; + children_db.push(child_hash); + let mut buf = prefix.to_vec(); + parent_hash.using_encoded(|s| buf.extend(s)); + tx.put_vec(column, &buf[..], children_db.encode()); Ok(()) } } diff --git a/core/client/src/in_mem.rs b/core/client/src/in_mem.rs index c512b35c0628d..8e6fd6b26d187 100644 --- a/core/client/src/in_mem.rs +++ b/core/client/src/in_mem.rs @@ -99,7 +99,7 @@ struct BlockchainStorage { header_cht_roots: HashMap, Block::Hash>, changes_trie_cht_roots: HashMap, Block::Hash>, leaves: LeafSet>, - children: ChildrenMap, + children: HashMap>, aux: HashMap, Vec>, } @@ -150,7 +150,7 @@ impl Blockchain { header_cht_roots: HashMap::new(), changes_trie_cht_roots: HashMap::new(), leaves: LeafSet::new(), - children: ChildrenMap::new(), + children: HashMap::new(), aux: HashMap::new(), })); Blockchain { @@ -367,7 +367,7 @@ impl blockchain::Backend for Blockchain { } fn children(&self, parent_hash: Block::Hash) -> error::Result> { - Ok(self.storage.read().children.hashes_from_mem(parent_hash)) + Ok(self.storage.read().children.get(&parent_hash).cloned().unwrap_or_default()) } } From f587d6917dcbe03ced613914519f0bcc08e12dd7 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Fri, 15 Feb 2019 12:41:06 +0100 Subject: [PATCH 19/24] fix: remove hashmap --- core/client/db/src/lib.rs | 7 ++++-- core/client/db/src/utils.rs | 4 ++-- core/client/src/children.rs | 41 ++++++++++++++++---------------- core/client/src/client.rs | 7 +++--- core/client/src/in_mem.rs | 7 ++---- core/sr-primitives/src/traits.rs | 2 +- node/runtime/src/lib.rs | 2 +- 7 files changed, 34 insertions(+), 36 deletions(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index b8127d93dafd4..081681f8c516d 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -252,7 +252,7 @@ impl client::blockchain::Backend for BlockchainDb { } fn children(&self, parent_hash: Block::Hash) -> Result, client::error::Error> { - ChildrenMap::children_hashes(&*self.db, columns::META, meta_keys::CHILD_PREFIX, parent_hash) + ChildrenMap::children_hashes(&*self.db, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash) } } @@ -858,11 +858,14 @@ impl> Backend { let mut leaves = self.blockchain.leaves.write(); let displaced_leaf = leaves.import(hash, number, parent_hash); leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX); - ChildrenMap::prepare_transaction(&*self.storage.db, &mut transaction, columns::META, meta_keys::CHILD_PREFIX, parent_hash, hash)?; displaced_leaf }; + let mut children = ChildrenMap::children_hashes(&*self.storage.db, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash)?; + children.push(hash); + ChildrenMap::prepare_transaction(&mut transaction, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash, children); + meta_updates.push((hash, number, pending_block.leaf_state.is_best(), finalized)); Some((number, hash, enacted, retracted, displaced_leaf, is_best)) diff --git a/core/client/db/src/utils.rs b/core/client/db/src/utils.rs index b4e6aefedc625..150c1fdd98ccd 100644 --- a/core/client/db/src/utils.rs +++ b/core/client/db/src/utils.rs @@ -51,8 +51,8 @@ pub mod meta_keys { pub const GENESIS_HASH: &[u8; 3] = b"gen"; /// Leaves prefix list key. pub const LEAF_PREFIX: &[u8; 4] = b"leaf"; - /// Child prefix list key. - pub const CHILD_PREFIX: &[u8; 5] = b"child"; + /// Children prefix list key. + pub const CHILDREN_PREFIX: &[u8; 8] = b"children"; } /// Database metadata. diff --git a/core/client/src/children.rs b/core/client/src/children.rs index ae7882d110313..fc86c4fddc9b2 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -20,7 +20,7 @@ use crate::error; use std::hash::Hash; -/// Map of father to children blocks. +/// Used to access children blocks hashes from db. pub struct ChildrenMap; impl ChildrenMap { @@ -50,24 +50,21 @@ impl ChildrenMap { Ok(children) } - /// Prepare the database transaction `tx` that adds `child_hash` to the children of `parent_hash`. + /// Insert the key-value pair (`parent_hash`, `children_hashes`) in the transaction. + /// Any existing value is overwritten upon write. pub fn prepare_transaction< K: Eq + Hash + Clone + Encode + Decode, V: Eq + Hash + Clone + Encode + Decode, >( - db: &KeyValueDB, tx: &mut DBTransaction, column: Option, prefix: &[u8], parent_hash: K, - child_hash: V, - ) -> error::Result<()> { - let mut children_db = Self::children_hashes(db, column, prefix, parent_hash.clone())?; - children_db.push(child_hash); - let mut buf = prefix.to_vec(); - parent_hash.using_encoded(|s| buf.extend(s)); - tx.put_vec(column, &buf[..], children_db.encode()); - Ok(()) + children_hashes: V, + ) { + let mut key = prefix.to_vec(); + parent_hash.using_encoded(|s| key.extend(s)); + tx.put_vec(column, &key[..], children_hashes.encode()); } } @@ -80,20 +77,22 @@ mod tests { const PREFIX: &[u8] = b"children"; let db = ::kvdb_memorydb::create(0); - let mut children = ChildrenMap::new(); let mut tx = DBTransaction::new(); - children.import(0u32, 0u32); - children.import(1_1, 1_3); - children.import(1_2, 1_4); - children.import(1_1, 1_5); - children.import(1_2, 1_6); - - let _ = children.prepare_transaction(&db, &mut tx, None, PREFIX); + let mut children1 = Vec::new(); + children1.push(1_3); + children1.push(1_5); + ChildrenMap::prepare_transaction(&mut tx, None, PREFIX, 1_1, children1); + + let mut children2 = Vec::new(); + children2.push(1_4); + children2.push(1_6); + ChildrenMap::prepare_transaction(&mut tx, None, PREFIX, 1_2, children2); + db.write(tx).unwrap(); - let r1: Vec = children.hashes(&db, None, PREFIX, 1_1).unwrap(); - let r2: Vec = children.hashes(&db, None, PREFIX, 1_2).unwrap(); + let r1: Vec = ChildrenMap::children_hashes(&db, None, PREFIX, 1_1).unwrap(); + let r2: Vec = ChildrenMap::children_hashes(&db, None, PREFIX, 1_2).unwrap(); assert_eq!(r1, vec![1_3, 1_5]); assert_eq!(r2, vec![1_4, 1_6]); diff --git a/core/client/src/client.rs b/core/client/src/client.rs index d180b0288f9b4..b2fb5827c5e7c 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1740,7 +1740,6 @@ pub(crate) mod tests { fn uncles_with_only_ancestors() { // block tree: // G -> A1 -> A2 - let client = test_client::new(); // G -> A1 @@ -1831,7 +1830,7 @@ pub(crate) mod tests { let genesis_hash = client.info().unwrap().chain.genesis_hash; let uncles1 = client.uncles(a4.hash(), 10).unwrap(); - assert_eq!(2, uncles1.len()); + assert_eq!(vec![b2.hash(), d2.hash()], uncles1); let uncles2 = client.uncles(a4.hash(), 0).unwrap(); assert_eq!(0, uncles2.len()); @@ -1843,10 +1842,10 @@ pub(crate) mod tests { assert_eq!(0, uncles4.len()); let uncles5 = client.uncles(d2.hash(), 10).unwrap(); - assert_eq!(2, uncles5.len()); + assert_eq!(vec![a2.hash(), b2.hash()], uncles5); let uncles6 = client.uncles(b3.hash(), 1).unwrap(); - assert_eq!(1, uncles6.len()); + assert_eq!(vec![c3.hash()], uncles6); } #[test] diff --git a/core/client/src/in_mem.rs b/core/client/src/in_mem.rs index 8e6fd6b26d187..6c90d9ae3e84f 100644 --- a/core/client/src/in_mem.rs +++ b/core/client/src/in_mem.rs @@ -34,7 +34,6 @@ use crate::error; use crate::backend::{self, NewBlockState}; use crate::light; use crate::leaves::LeafSet; -use crate::children::ChildrenMap; use crate::blockchain::{self, BlockStatus, HeaderBackend}; struct PendingBlock { @@ -99,7 +98,6 @@ struct BlockchainStorage { header_cht_roots: HashMap, Block::Hash>, changes_trie_cht_roots: HashMap, Block::Hash>, leaves: LeafSet>, - children: HashMap>, aux: HashMap, Vec>, } @@ -150,7 +148,6 @@ impl Blockchain { header_cht_roots: HashMap::new(), changes_trie_cht_roots: HashMap::new(), leaves: LeafSet::new(), - children: HashMap::new(), aux: HashMap::new(), })); Blockchain { @@ -366,8 +363,8 @@ impl blockchain::Backend for Blockchain { Ok(self.storage.read().leaves.hashes()) } - fn children(&self, parent_hash: Block::Hash) -> error::Result> { - Ok(self.storage.read().children.get(&parent_hash).cloned().unwrap_or_default()) + fn children(&self, _parent_hash: Block::Hash) -> error::Result> { + unimplemented!() } } diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 1091bce473bf0..5e12d571ae201 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -349,7 +349,7 @@ tuple_impl!(A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, pub trait Hash: 'static + MaybeSerializeDebug + Clone + Eq + PartialEq { // Stupid bug in the Rust compiler believes derived // traits must be fulfilled by all type parameters. /// The hash type produced. - type Output: Member + MaybeSerializeDebug + AsRef<[u8]> + AsMut<[u8]> + PartialEq + Eq; + type Output: Member + MaybeSerializeDebug + AsRef<[u8]> + AsMut<[u8]>; /// Produce the hash of some byte-slice. fn hash(s: &[u8]) -> Self::Output; diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 2a3b8d873fd6b..89f3a58030fa1 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -61,7 +61,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { impl_name: create_runtime_str!("substrate-node"), authoring_version: 10, spec_version: 29, - impl_version: 29, + impl_version: 30, apis: RUNTIME_API_VERSIONS, }; From 6b55176a5aed3963e8a62bc2cb0c5c4f918a2e52 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 18 Feb 2019 10:19:17 +0100 Subject: [PATCH 20/24] feat: add tests for backend --- core/client/db/src/lib.rs | 6 ++ core/client/src/client.rs | 1 - core/test-client/src/trait_tests.rs | 90 +++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index 081681f8c516d..a294c664f67b4 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -1802,6 +1802,12 @@ mod tests { test_client::trait_tests::test_leaves_for_backend(backend); } + #[test] + fn test_children_with_complex_block_tree() { + let backend: Arc> = Arc::new(Backend::new_test(20, 20)); + test_client::trait_tests::test_children_for_backend(backend); + } + #[test] fn test_blockchain_query_by_number_gets_canonical() { let backend: Arc> = Arc::new(Backend::new_test(20, 20)); diff --git a/core/client/src/client.rs b/core/client/src/client.rs index b2fb5827c5e7c..99f276ea2bbaf 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1826,7 +1826,6 @@ pub(crate) mod tests { let d2 = builder.bake().unwrap(); client.import(BlockOrigin::Own, d2.clone()).unwrap(); - assert_eq!(client.info().unwrap().chain.best_hash, a5.hash()); let genesis_hash = client.info().unwrap().chain.genesis_hash; let uncles1 = client.uncles(a4.hash(), 10).unwrap(); diff --git a/core/test-client/src/trait_tests.rs b/core/test-client/src/trait_tests.rs index 7fadc128faf2c..7a1fb2885ba39 100644 --- a/core/test-client/src/trait_tests.rs +++ b/core/test-client/src/trait_tests.rs @@ -144,6 +144,96 @@ pub fn test_leaves_for_backend(backend: Arc) where vec![a5.hash(), b4.hash(), c3.hash(), d2.hash()]); } +/// helper to test the `leaves` implementation for various backends +pub fn test_children_for_backend(backend: Arc) where + B: backend::LocalBackend, +{ + // block tree: + // G -> A1 -> A2 -> A3 -> A4 -> A5 + // A1 -> B2 -> B3 -> B4 + // B2 -> C3 + // A1 -> D2 + + let client = new_with_backend(backend.clone(), false); + + // G -> A1 + let a1 = client.new_block().unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a1.clone()).unwrap(); + + // A1 -> A2 + let a2 = client.new_block_at(&BlockId::Hash(a1.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a2.clone()).unwrap(); + + // A2 -> A3 + let a3 = client.new_block_at(&BlockId::Hash(a2.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a3.clone()).unwrap(); + + // A3 -> A4 + let a4 = client.new_block_at(&BlockId::Hash(a3.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a4.clone()).unwrap(); + + // A4 -> A5 + let a5 = client.new_block_at(&BlockId::Hash(a4.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, a5.clone()).unwrap(); + + // A1 -> B2 + let mut builder = client.new_block_at(&BlockId::Hash(a1.hash())).unwrap(); + // this push is required as otherwise B2 has the same hash as A2 and won't get imported + builder.push_transfer(Transfer { + from: Keyring::Alice.to_raw_public().into(), + to: Keyring::Ferdie.to_raw_public().into(), + amount: 41, + nonce: 0, + }).unwrap(); + let b2 = builder.bake().unwrap(); + client.import(BlockOrigin::Own, b2.clone()).unwrap(); + + // B2 -> B3 + let b3 = client.new_block_at(&BlockId::Hash(b2.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, b3.clone()).unwrap(); + + // B3 -> B4 + let b4 = client.new_block_at(&BlockId::Hash(b3.hash())).unwrap().bake().unwrap(); + client.import(BlockOrigin::Own, b4.clone()).unwrap(); + + // // B2 -> C3 + let mut builder = client.new_block_at(&BlockId::Hash(b2.hash())).unwrap(); + // this push is required as otherwise C3 has the same hash as B3 and won't get imported + builder.push_transfer(Transfer { + from: Keyring::Alice.to_raw_public().into(), + to: Keyring::Ferdie.to_raw_public().into(), + amount: 1, + nonce: 1, + }).unwrap(); + let c3 = builder.bake().unwrap(); + client.import(BlockOrigin::Own, c3.clone()).unwrap(); + + // A1 -> D2 + let mut builder = client.new_block_at(&BlockId::Hash(a1.hash())).unwrap(); + // this push is required as otherwise D2 has the same hash as B2 and won't get imported + builder.push_transfer(Transfer { + from: Keyring::Alice.to_raw_public().into(), + to: Keyring::Ferdie.to_raw_public().into(), + amount: 1, + nonce: 0, + }).unwrap(); + let d2 = builder.bake().unwrap(); + client.import(BlockOrigin::Own, d2.clone()).unwrap(); + + let genesis_hash = client.info().unwrap().chain.genesis_hash; + + let children1 = backend.blockchain().children(a4.hash()).unwrap(); + assert_eq!(vec![a5.hash()], children1); + + let children2 = backend.blockchain().children(a1.hash()).unwrap(); + assert_eq!(vec![a2.hash(), b2.hash(), d2.hash()], children2); + + let children3 = backend.blockchain().children(genesis_hash).unwrap(); + assert_eq!(vec![a1.hash()], children3); + + let children4 = backend.blockchain().children(b2.hash()).unwrap(); + assert_eq!(vec![b3.hash(), c3.hash()], children4); +} pub fn test_blockchain_query_by_number_gets_canonical(backend: Arc) where B: backend::LocalBackend, From adfb706dc455573d340cfb76b9fc82f2604cb24b Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 25 Feb 2019 09:44:00 +0100 Subject: [PATCH 21/24] chore: free functions --- core/client/db/src/lib.rs | 8 ++-- core/client/src/children.rs | 87 ++++++++++++++++++------------------- core/client/src/lib.rs | 6 +-- 3 files changed, 48 insertions(+), 53 deletions(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index a294c664f67b4..c69aa156ebbee 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -52,7 +52,7 @@ use executor::RuntimeInfo; use state_machine::{CodeExecutor, DBValue}; use crate::utils::{Meta, db_err, meta_keys, open_database, read_db, block_id_to_lookup_key, read_meta}; use client::LeafSet; -use client::ChildrenMap; +use client::children; use state_db::StateDb; use crate::storage_cache::{CachingState, SharedCache, new_shared_cache}; use log::{trace, debug, warn}; @@ -252,7 +252,7 @@ impl client::blockchain::Backend for BlockchainDb { } fn children(&self, parent_hash: Block::Hash) -> Result, client::error::Error> { - ChildrenMap::children_hashes(&*self.db, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash) + children::children_hashes(&*self.db, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash) } } @@ -862,9 +862,9 @@ impl> Backend { displaced_leaf }; - let mut children = ChildrenMap::children_hashes(&*self.storage.db, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash)?; + let mut children = children::children_hashes(&*self.storage.db, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash)?; children.push(hash); - ChildrenMap::prepare_transaction(&mut transaction, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash, children); + children::prepare_transaction(&mut transaction, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash, children); meta_updates.push((hash, number, pending_block.leaf_state.is_best(), finalized)); diff --git a/core/client/src/children.rs b/core/client/src/children.rs index fc86c4fddc9b2..ae63a73f6065d 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -14,58 +14,55 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . +//! Functionality for reading and storing children hashes from db. + use kvdb::{KeyValueDB, DBTransaction}; use parity_codec::{Encode, Decode}; use crate::error; use std::hash::Hash; -/// Used to access children blocks hashes from db. -pub struct ChildrenMap; - -impl ChildrenMap { - /// Returns the hashes of the children blocks of the block with `parent_hash`. - pub fn children_hashes< - K: Eq + Hash + Clone + Encode + Decode, - V: Eq + Hash + Clone + Encode + Decode, - >(db: &KeyValueDB, column: Option, prefix: &[u8], parent_hash: K) -> error::Result> { - let mut buf = prefix.to_vec(); - parent_hash.using_encoded(|s| buf.extend(s)); +/// Returns the hashes of the children blocks of the block with `parent_hash`. +pub fn children_hashes< + K: Eq + Hash + Clone + Encode + Decode, + V: Eq + Hash + Clone + Encode + Decode, +>(db: &KeyValueDB, column: Option, prefix: &[u8], parent_hash: K) -> error::Result> { + let mut buf = prefix.to_vec(); + parent_hash.using_encoded(|s| buf.extend(s)); - let raw_val_opt = match db.get(column, &buf[..]) { - Ok(raw_val_opt) => raw_val_opt, - Err(_) => return Err(error::ErrorKind::Backend("Error reading value from database".into()).into()), - }; + let raw_val_opt = match db.get(column, &buf[..]) { + Ok(raw_val_opt) => raw_val_opt, + Err(_) => return Err(error::ErrorKind::Backend("Error reading value from database".into()).into()), + }; - let raw_val = match raw_val_opt { - Some(val) => val, - None => return Ok(Vec::new()), - }; + let raw_val = match raw_val_opt { + Some(val) => val, + None => return Ok(Vec::new()), + }; - let children: Vec = match Decode::decode(&mut &raw_val[..]) { - Some(children) => children, - None => return Err(error::ErrorKind::Backend("Error decoding children".into()).into()), - }; + let children: Vec = match Decode::decode(&mut &raw_val[..]) { + Some(children) => children, + None => return Err(error::ErrorKind::Backend("Error decoding children".into()).into()), + }; - Ok(children) - } + Ok(children) +} - /// Insert the key-value pair (`parent_hash`, `children_hashes`) in the transaction. - /// Any existing value is overwritten upon write. - pub fn prepare_transaction< - K: Eq + Hash + Clone + Encode + Decode, - V: Eq + Hash + Clone + Encode + Decode, - >( - tx: &mut DBTransaction, - column: Option, - prefix: &[u8], - parent_hash: K, - children_hashes: V, - ) { - let mut key = prefix.to_vec(); - parent_hash.using_encoded(|s| key.extend(s)); - tx.put_vec(column, &key[..], children_hashes.encode()); - } +/// Insert the key-value pair (`parent_hash`, `children_hashes`) in the transaction. +/// Any existing value is overwritten upon write. +pub fn prepare_transaction< + K: Eq + Hash + Clone + Encode + Decode, + V: Eq + Hash + Clone + Encode + Decode, +>( + tx: &mut DBTransaction, + column: Option, + prefix: &[u8], + parent_hash: K, + children_hashes: V, +) { + let mut key = prefix.to_vec(); + parent_hash.using_encoded(|s| key.extend(s)); + tx.put_vec(column, &key[..], children_hashes.encode()); } #[cfg(test)] @@ -82,17 +79,17 @@ mod tests { let mut children1 = Vec::new(); children1.push(1_3); children1.push(1_5); - ChildrenMap::prepare_transaction(&mut tx, None, PREFIX, 1_1, children1); + prepare_transaction(&mut tx, None, PREFIX, 1_1, children1); let mut children2 = Vec::new(); children2.push(1_4); children2.push(1_6); - ChildrenMap::prepare_transaction(&mut tx, None, PREFIX, 1_2, children2); + prepare_transaction(&mut tx, None, PREFIX, 1_2, children2); db.write(tx).unwrap(); - let r1: Vec = ChildrenMap::children_hashes(&db, None, PREFIX, 1_1).unwrap(); - let r2: Vec = ChildrenMap::children_hashes(&db, None, PREFIX, 1_2).unwrap(); + let r1: Vec = children_hashes(&db, None, PREFIX, 1_1).unwrap(); + let r2: Vec = children_hashes(&db, None, PREFIX, 1_2).unwrap(); assert_eq!(r1, vec![1_3, 1_5]); assert_eq!(r2, vec![1_4, 1_6]); diff --git a/core/client/src/lib.rs b/core/client/src/lib.rs index 61310aa913ecd..44eaa449245bb 100644 --- a/core/client/src/lib.rs +++ b/core/client/src/lib.rs @@ -38,6 +38,8 @@ pub mod block_builder; #[cfg(feature = "std")] pub mod light; #[cfg(feature = "std")] +pub mod children; +#[cfg(feature = "std")] mod leaves; #[cfg(feature = "std")] mod call_executor; @@ -45,8 +47,6 @@ mod call_executor; mod client; #[cfg(feature = "std")] mod notifications; -#[cfg(feature = "std")] -mod children; #[cfg(feature = "std")] @@ -66,8 +66,6 @@ pub use crate::notifications::{StorageEventStream, StorageChangeSet}; pub use state_machine::ExecutionStrategy; #[cfg(feature = "std")] pub use crate::leaves::LeafSet; -#[cfg(feature = "std")] -pub use crate::children::ChildrenMap; #[doc(inline)] pub use sr_api_macros::{decl_runtime_apis, impl_runtime_apis}; From bf5a3e8821955a90efe438ce0526faa6b65343d1 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 25 Feb 2019 10:14:52 +0100 Subject: [PATCH 22/24] feat: add remove children --- core/client/db/src/lib.rs | 6 ++--- core/client/src/children.rs | 44 ++++++++++++++++++++++++++++--------- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index c69aa156ebbee..184347afcf558 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -252,7 +252,7 @@ impl client::blockchain::Backend for BlockchainDb { } fn children(&self, parent_hash: Block::Hash) -> Result, client::error::Error> { - children::children_hashes(&*self.db, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash) + children::read_children(&*self.db, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash) } } @@ -862,9 +862,9 @@ impl> Backend { displaced_leaf }; - let mut children = children::children_hashes(&*self.storage.db, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash)?; + let mut children = children::read_children(&*self.storage.db, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash)?; children.push(hash); - children::prepare_transaction(&mut transaction, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash, children); + children::write_children(&mut transaction, columns::META, meta_keys::CHILDREN_PREFIX, parent_hash, children); meta_updates.push((hash, number, pending_block.leaf_state.is_best(), finalized)); diff --git a/core/client/src/children.rs b/core/client/src/children.rs index ae63a73f6065d..d9801836028cd 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -23,7 +23,7 @@ use std::hash::Hash; /// Returns the hashes of the children blocks of the block with `parent_hash`. -pub fn children_hashes< +pub fn read_children< K: Eq + Hash + Clone + Encode + Decode, V: Eq + Hash + Clone + Encode + Decode, >(db: &KeyValueDB, column: Option, prefix: &[u8], parent_hash: K) -> error::Result> { @@ -50,7 +50,7 @@ pub fn children_hashes< /// Insert the key-value pair (`parent_hash`, `children_hashes`) in the transaction. /// Any existing value is overwritten upon write. -pub fn prepare_transaction< +pub fn write_children< K: Eq + Hash + Clone + Encode + Decode, V: Eq + Hash + Clone + Encode + Decode, >( @@ -65,12 +65,27 @@ pub fn prepare_transaction< tx.put_vec(column, &key[..], children_hashes.encode()); } +/// Remove the key-value pair (`parent_hash`, `children_hashes`) in the transaction. +pub fn remove_children< + K: Eq + Hash + Clone + Encode + Decode, +>( + tx: &mut DBTransaction, + column: Option, + prefix: &[u8], + parent_hash: K, +) { + let mut key = prefix.to_vec(); + parent_hash.using_encoded(|s| key.extend(s)); + tx.delete(column, &key[..]); +} + + #[cfg(test)] mod tests { use super::*; #[test] - fn children_write_read() { + fn children_write_read_remove() { const PREFIX: &[u8] = b"children"; let db = ::kvdb_memorydb::create(0); @@ -79,19 +94,28 @@ mod tests { let mut children1 = Vec::new(); children1.push(1_3); children1.push(1_5); - prepare_transaction(&mut tx, None, PREFIX, 1_1, children1); + write_children(&mut tx, None, PREFIX, 1_1, children1); let mut children2 = Vec::new(); children2.push(1_4); children2.push(1_6); - prepare_transaction(&mut tx, None, PREFIX, 1_2, children2); + write_children(&mut tx, None, PREFIX, 1_2, children2); + + db.write(tx.clone()).unwrap(); + + let r1: Vec = read_children(&db, None, PREFIX, 1_1).unwrap(); + let r2: Vec = read_children(&db, None, PREFIX, 1_2).unwrap(); - db.write(tx).unwrap(); - - let r1: Vec = children_hashes(&db, None, PREFIX, 1_1).unwrap(); - let r2: Vec = children_hashes(&db, None, PREFIX, 1_2).unwrap(); - assert_eq!(r1, vec![1_3, 1_5]); assert_eq!(r2, vec![1_4, 1_6]); + + remove_children(&mut tx, None, PREFIX, 1_2); + db.write(tx).unwrap(); + + let r1: Vec = read_children(&db, None, PREFIX, 1_1).unwrap(); + let r2: Vec = read_children(&db, None, PREFIX, 1_2).unwrap(); + + assert_eq!(r1, vec![1_3, 1_5]); + assert_eq!(r2.len(), 0); } } \ No newline at end of file From 96ce77f31ed584e1ac0dd5cd4ef7939dbf1bb9ba Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 25 Feb 2019 10:17:24 +0100 Subject: [PATCH 23/24] fix: remove children when reverting --- core/client/db/src/lib.rs | 1 + core/client/src/children.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index 184347afcf558..99483b428882c 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -1089,6 +1089,7 @@ impl client::backend::Backend for Backend whe let key = utils::number_and_hash_to_lookup_key(best.clone(), &hash); transaction.put(columns::META, meta_keys::BEST_BLOCK, &key); transaction.delete(columns::KEY_LOOKUP, removed.hash().as_ref()); + children::remove_children(&mut transaction, columns::META, meta_keys::CHILDREN_PREFIX, hash); self.storage.db.write(transaction).map_err(db_err)?; self.blockchain.update_meta(hash, best, true, false); self.blockchain.leaves.write().revert(removed.hash().clone(), removed.number().clone(), removed.parent_hash().clone()); diff --git a/core/client/src/children.rs b/core/client/src/children.rs index d9801836028cd..48b39d18cdd60 100644 --- a/core/client/src/children.rs +++ b/core/client/src/children.rs @@ -65,7 +65,7 @@ pub fn write_children< tx.put_vec(column, &key[..], children_hashes.encode()); } -/// Remove the key-value pair (`parent_hash`, `children_hashes`) in the transaction. +/// Prepare transaction to remove the children of `parent_hash`. pub fn remove_children< K: Eq + Hash + Clone + Encode + Decode, >( From b6fc5bff17d389ae351cbfd7907bc0128c963111 Mon Sep 17 00:00:00 2001 From: Marcio Diaz Date: Mon, 25 Feb 2019 10:57:02 +0100 Subject: [PATCH 24/24] fix: typo and spec version --- core/test-client/src/trait_tests.rs | 2 +- node/runtime/src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/test-client/src/trait_tests.rs b/core/test-client/src/trait_tests.rs index 7a1fb2885ba39..8242f30d2e23f 100644 --- a/core/test-client/src/trait_tests.rs +++ b/core/test-client/src/trait_tests.rs @@ -144,7 +144,7 @@ pub fn test_leaves_for_backend(backend: Arc) where vec![a5.hash(), b4.hash(), c3.hash(), d2.hash()]); } -/// helper to test the `leaves` implementation for various backends +/// helper to test the `children` implementation for various backends pub fn test_children_for_backend(backend: Arc) where B: backend::LocalBackend, { diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 36058dee37a11..d7ed78c231d99 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -60,8 +60,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("node"), impl_name: create_runtime_str!("substrate-node"), authoring_version: 10, - spec_version: 31, - impl_version: 31, + spec_version: 30, + impl_version: 30, apis: RUNTIME_API_VERSIONS, };