From fd188d3399cd7b484e0a86409af3415fe327309d Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 6 Jul 2016 18:08:57 +0200 Subject: [PATCH 01/22] memorydb ub patch and other cleanup --- util/src/journaldb/archivedb.rs | 2 +- util/src/journaldb/earlymergedb.rs | 2 +- util/src/journaldb/overlayrecentdb.rs | 6 +- util/src/journaldb/traits.rs | 2 +- util/src/memorydb.rs | 114 ++++++++++++++++---------- util/src/overlaydb.rs | 10 +-- 6 files changed, 82 insertions(+), 54 deletions(-) diff --git a/util/src/journaldb/archivedb.rs b/util/src/journaldb/archivedb.rs index ca436d7ea91..5154cf25e4a 100644 --- a/util/src/journaldb/archivedb.rs +++ b/util/src/journaldb/archivedb.rs @@ -101,7 +101,7 @@ impl HashDB for ArchiveDB { fn get(&self, key: &H256) -> Option<&[u8]> { let k = self.overlay.raw(key); match k { - Some(&(ref d, rc)) if rc > 0 => Some(d), + Some((d, rc)) if rc > 0 => Some(d), _ => { if let Some(x) = self.payload(key) { Some(&self.overlay.denote(key, x).0) diff --git a/util/src/journaldb/earlymergedb.rs b/util/src/journaldb/earlymergedb.rs index b860ae598b6..d811e129ef5 100644 --- a/util/src/journaldb/earlymergedb.rs +++ b/util/src/journaldb/earlymergedb.rs @@ -288,7 +288,7 @@ impl HashDB for EarlyMergeDB { fn get(&self, key: &H256) -> Option<&[u8]> { let k = self.overlay.raw(key); match k { - Some(&(ref d, rc)) if rc > 0 => Some(d), + Some((d, rc)) if rc > 0 => Some(d), _ => { if let Some(x) = self.payload(key) { Some(&self.overlay.denote(key, x).0) diff --git a/util/src/journaldb/overlayrecentdb.rs b/util/src/journaldb/overlayrecentdb.rs index 7afe7e5ac60..455a673b39a 100644 --- a/util/src/journaldb/overlayrecentdb.rs +++ b/util/src/journaldb/overlayrecentdb.rs @@ -265,9 +265,9 @@ impl JournalDB for OverlayRecentDB { { if canon_id == journal.id { for h in &journal.insertions { - if let Some(&(ref d, rc)) = journal_overlay.backing_overlay.raw(h) { + if let Some((d, rc)) = journal_overlay.backing_overlay.raw(h) { if rc > 0 { - canon_insertions.push((h.clone(), d.clone())); //TODO: optimize this to avoid data copy + canon_insertions.push((h.clone(), d.to_owned())); //TODO: optimize this to avoid data copy } } } @@ -319,7 +319,7 @@ impl HashDB for OverlayRecentDB { fn get(&self, key: &H256) -> Option<&[u8]> { let k = self.transaction_overlay.raw(key); match k { - Some(&(ref d, rc)) if rc > 0 => Some(d), + Some((d, rc)) if rc > 0 => Some(d), _ => { let v = self.journal_overlay.read().unwrap().backing_overlay.get(key).map(|v| v.to_vec()); match v { diff --git a/util/src/journaldb/traits.rs b/util/src/journaldb/traits.rs index 74149b0625b..b8198a2fa7c 100644 --- a/util/src/journaldb/traits.rs +++ b/util/src/journaldb/traits.rs @@ -21,7 +21,7 @@ use hashdb::*; /// A `HashDB` which can manage a short-term journal potentially containing many forks of mutually /// exclusive actions. -pub trait JournalDB : HashDB + Send + Sync { +pub trait JournalDB : HashDB { /// Return a copy of ourself, in a box. fn boxed_clone(&self) -> Box; diff --git a/util/src/memorydb.rs b/util/src/memorydb.rs index 73435e40a3a..f21c43fb4cf 100644 --- a/util/src/memorydb.rs +++ b/util/src/memorydb.rs @@ -24,9 +24,10 @@ use hashdb::*; use heapsize::*; use std::mem; use std::collections::HashMap; -use std::default::Default; +use std::cell::UnsafeCell; + +const STATIC_NULL_RLP: (&'static [u8], i32) = (&[0x80; 1], 1); -#[derive(Debug,Clone)] /// Reference-counted memory-based `HashDB` implementation. /// /// Use `new()` to create a new database. Insert items with `insert()`, remove items @@ -70,10 +71,8 @@ use std::default::Default; /// assert!(!m.contains(&k)); /// } /// ``` -#[derive(PartialEq)] pub struct MemoryDB { - data: HashMap, - static_null_rlp: (Bytes, i32), + data: UnsafeCell>, aux: HashMap, } @@ -83,12 +82,27 @@ impl Default for MemoryDB { } } +impl ::std::cmp::PartialEq for MemoryDB { + fn eq(&self, rhs: &Self) -> bool { + let (my_data, rhs_data) = unsafe { (&*self.data.get(), &*rhs.data.get())}; + my_data == rhs_data && self.aux == rhs.aux + } +} + +impl Clone for MemoryDB { + fn clone(&self) -> Self { + MemoryDB { + data: UnsafeCell::new(unsafe { (*self.data.get()).clone() }), + aux: self.aux.clone() + } + } +} + impl MemoryDB { /// Create a new instance of the memory DB. pub fn new() -> MemoryDB { MemoryDB { - data: HashMap::new(), - static_null_rlp: (vec![0x80u8; 1], 1), + data: UnsafeCell::new(HashMap::new()), aux: HashMap::new(), } } @@ -110,33 +124,22 @@ impl MemoryDB { /// } /// ``` pub fn clear(&mut self) { - self.data.clear(); + unsafe { (*self.data.get()).clear() } } /// Purge all zero-referenced data from the database. pub fn purge(&mut self) { - let empties: Vec<_> = self.data.iter() + let data = unsafe { &mut *self.data.get() }; + let empties: Vec<_> = data.iter() .filter(|&(_, &(_, rc))| rc == 0) .map(|(k, _)| k.clone()) .collect(); - for empty in empties { self.data.remove(&empty); } - } - - /// Grab the raw information associated with a key. Returns None if the key - /// doesn't exist. - /// - /// Even when Some is returned, the data is only guaranteed to be useful - /// when the refs > 0. - pub fn raw(&self, key: &H256) -> Option<&(Bytes, i32)> { - if key == &SHA3_NULL_RLP { - return Some(&self.static_null_rlp); - } - self.data.get(key) + for empty in empties { data.remove(&empty); } } /// Return the internal map of hashes to data, clearing the current state. pub fn drain(&mut self) -> HashMap { - mem::replace(&mut self.data, HashMap::new()) + mem::replace(unsafe { &mut *self.data.get() }, HashMap::new()) } /// Return the internal map of auxiliary data, clearing the current state. @@ -144,23 +147,42 @@ impl MemoryDB { mem::replace(&mut self.aux, HashMap::new()) } + /// Grab the raw information associated with a key. Returns None if the key + /// doesn't exist. + /// + /// Even when Some is returned, the data is only guaranteed to be useful + /// when the refs > 0. + pub fn raw(&self, key: &H256) -> Option<(&[u8], i32)> { + if key == &SHA3_NULL_RLP { + return Some(STATIC_NULL_RLP.clone()); + } + unsafe { (*self.data.get()).get(key).map(|&(ref v, x)| (&v[..], x)) } + } + /// Denote than an existing value has the given key. Used when a key gets removed without /// a prior insert and thus has a negative reference with no value. /// /// May safely be called even if the key's value is known, in which case it will be a no-op. - pub fn denote(&self, key: &H256, value: Bytes) -> &(Bytes, i32) { - if self.raw(key) == None { - unsafe { - let p = &self.data as *const HashMap as *mut HashMap; - (*p).insert(key.clone(), (value, 0)); + pub fn denote(&self, key: &H256, value: Bytes) -> (&[u8], i32) { + if key == &SHA3_NULL_RLP { + return STATIC_NULL_RLP.clone(); + } + + unsafe { + let data = self.data.get(); + if let Some(&(ref v, x)) = (*data).get(key) { + return (&v[..], x); } + + (*data).insert(key.clone(), (value, 0)); + let &(ref v, x) = (*data).get(key).unwrap(); + (&v[..], x) } - self.raw(key).unwrap() } /// Returns the size of allocated heap memory pub fn mem_used(&self) -> usize { - self.data.heap_size_of_children() + unsafe { (*self.data.get()).heap_size_of_children() } } } @@ -171,21 +193,22 @@ impl HashDB for MemoryDB { if key == &SHA3_NULL_RLP { return Some(&NULL_RLP_STATIC); } - match self.data.get(key) { + match unsafe { (*self.data.get()).get(key) } { Some(&(ref d, rc)) if rc > 0 => Some(d), _ => None } } fn keys(&self) -> HashMap { - self.data.iter().filter_map(|(k, v)| if v.1 != 0 {Some((k.clone(), v.1))} else {None}).collect() + let data = unsafe { &mut *self.data.get() }; + data.iter().filter_map(|(k, v)| if v.1 != 0 {Some((k.clone(), v.1))} else {None}).collect() } fn contains(&self, key: &H256) -> bool { if key == &SHA3_NULL_RLP { return true; } - match self.data.get(key) { + match unsafe { (*self.data.get()).get(key) } { Some(&(_, x)) if x > 0 => true, _ => false } @@ -196,16 +219,17 @@ impl HashDB for MemoryDB { return SHA3_NULL_RLP.clone(); } let key = value.sha3(); - if match self.data.get_mut(&key) { + let mut data = unsafe { &mut *self.data.get() }; + if match data.get_mut(&key) { Some(&mut (ref mut old_value, ref mut rc @ -0x80000000i32 ... 0)) => { - *old_value = From::from(value); + *old_value = value.into(); *rc += 1; false }, Some(&mut (_, ref mut x)) => { *x += 1; false } , None => true, }{ // ... None falls through into... - self.data.insert(key.clone(), (From::from(value), 1)); + data.insert(key.clone(), (value.into(), 1)); } key } @@ -214,7 +238,9 @@ impl HashDB for MemoryDB { if value == &NULL_RLP { return; } - match self.data.get_mut(&key) { + + let data = unsafe { &mut *self.data.get() }; + match data.get_mut(&key) { Some(&mut (ref mut old_value, ref mut rc @ -0x80000000i32 ... 0)) => { *old_value = value; *rc += 1; @@ -224,18 +250,20 @@ impl HashDB for MemoryDB { None => {}, } // ... None falls through into... - self.data.insert(key, (value, 1)); + data.insert(key, (value, 1)); } fn remove(&mut self, key: &H256) { if key == &SHA3_NULL_RLP { return; } - if match self.data.get_mut(key) { + + let data = unsafe { &mut *self.data.get() }; + if match data.get_mut(key) { Some(&mut (_, ref mut x)) => { *x -= 1; false } None => true }{ // ... None falls through into... - self.data.insert(key.clone(), (Bytes::new(), -1)); + data.insert(key.clone(), (Bytes::new(), -1)); } } @@ -262,9 +290,9 @@ fn memorydb_denote() { for _ in 0..1000 { let r = H256::random(); let k = r.sha3(); - let &(ref v, ref rc) = m.denote(&k, r.to_bytes()); - assert_eq!(v.as_slice(), r.as_slice()); - assert_eq!(*rc, 0); + let (v, rc) = m.denote(&k, r.to_bytes()); + assert_eq!(v, r.as_slice()); + assert_eq!(rc, 0); } assert_eq!(m.get(&hash).unwrap(), b"Hello world!"); diff --git a/util/src/overlaydb.rs b/util/src/overlaydb.rs index 63ec3dd502b..3219693a0ca 100644 --- a/util/src/overlaydb.rs +++ b/util/src/overlaydb.rs @@ -168,7 +168,7 @@ impl OverlayDB { pub fn revert(&mut self) { self.overlay.clear(); } /// Get the number of references that would be committed. - pub fn commit_refs(&self, key: &H256) -> i32 { self.overlay.raw(&key).map_or(0, |&(_, refs)| refs) } + pub fn commit_refs(&self, key: &H256) -> i32 { self.overlay.raw(&key).map_or(0, |(_, refs)| refs) } /// Get the refs and value of the given key. fn payload(&self, key: &H256) -> Option<(Bytes, u32)> { @@ -229,9 +229,9 @@ impl HashDB for OverlayDB { // it positive again. let k = self.overlay.raw(key); match k { - Some(&(ref d, rc)) if rc > 0 => Some(d), + Some((d, rc)) if rc > 0 => Some(d), _ => { - let memrc = k.map_or(0, |&(_, rc)| rc); + let memrc = k.map_or(0, |(_, rc)| rc); match self.payload(key) { Some(x) => { let (d, rc) = x; @@ -254,9 +254,9 @@ impl HashDB for OverlayDB { // it positive again. let k = self.overlay.raw(key); match k { - Some(&(_, rc)) if rc > 0 => true, + Some((_, rc)) if rc > 0 => true, _ => { - let memrc = k.map_or(0, |&(_, rc)| rc); + let memrc = k.map_or(0, |(_, rc)| rc); match self.payload(key) { Some(x) => { let (_, rc) = x; From 45c2c46f30edae46a067399994e740ff2ceb51f8 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 6 Jul 2016 18:30:40 +0200 Subject: [PATCH 02/22] fix denote invocations --- util/src/journaldb/archivedb.rs | 2 +- util/src/journaldb/earlymergedb.rs | 2 +- util/src/journaldb/overlayrecentdb.rs | 53 ++++++++++++++------------- util/src/overlaydb.rs | 2 +- 4 files changed, 31 insertions(+), 28 deletions(-) diff --git a/util/src/journaldb/archivedb.rs b/util/src/journaldb/archivedb.rs index 5154cf25e4a..043dd69a21f 100644 --- a/util/src/journaldb/archivedb.rs +++ b/util/src/journaldb/archivedb.rs @@ -104,7 +104,7 @@ impl HashDB for ArchiveDB { Some((d, rc)) if rc > 0 => Some(d), _ => { if let Some(x) = self.payload(key) { - Some(&self.overlay.denote(key, x).0) + Some(self.overlay.denote(key, x).0) } else { None diff --git a/util/src/journaldb/earlymergedb.rs b/util/src/journaldb/earlymergedb.rs index d811e129ef5..e575e8053a3 100644 --- a/util/src/journaldb/earlymergedb.rs +++ b/util/src/journaldb/earlymergedb.rs @@ -291,7 +291,7 @@ impl HashDB for EarlyMergeDB { Some((d, rc)) if rc > 0 => Some(d), _ => { if let Some(x) = self.payload(key) { - Some(&self.overlay.denote(key, x).0) + Some(self.overlay.denote(key, x).0) } else { None diff --git a/util/src/journaldb/overlayrecentdb.rs b/util/src/journaldb/overlayrecentdb.rs index 455a673b39a..3d88cf0035b 100644 --- a/util/src/journaldb/overlayrecentdb.rs +++ b/util/src/journaldb/overlayrecentdb.rs @@ -246,40 +246,43 @@ impl JournalDB for OverlayRecentDB { journal_overlay.journal.entry(now).or_insert_with(Vec::new).push(JournalEntry { id: id.clone(), insertions: inserted_keys, deletions: removed_keys }); } - let journal_overlay = journal_overlay.deref_mut(); + let journal_overlay = &mut *journal_overlay; // apply old commits' details if let Some((end_era, canon_id)) = end { if let Some(ref mut records) = journal_overlay.journal.get_mut(&end_era) { - let mut canon_insertions: Vec<(H256, Bytes)> = Vec::new(); let mut canon_deletions: Vec = Vec::new(); let mut overlay_deletions: Vec = Vec::new(); let mut index = 0usize; - for mut journal in records.drain(..) { - //delete the record from the db - let mut r = RlpStream::new_list(3); - r.append(&end_era); - r.append(&index); - r.append(&&PADDING[..]); - try!(batch.delete(&r.drain())); - trace!("commit: Delete journal for time #{}.{}: {}, (canon was {}): +{} -{} entries", end_era, index, journal.id, canon_id, journal.insertions.len(), journal.deletions.len()); - { - if canon_id == journal.id { - for h in &journal.insertions { - if let Some((d, rc)) = journal_overlay.backing_overlay.raw(h) { - if rc > 0 { - canon_insertions.push((h.clone(), d.to_owned())); //TODO: optimize this to avoid data copy + + { + let mut canon_insertions = Vec::new(); + for mut journal in records.drain(..) { + //delete the record from the db + let mut r = RlpStream::new_list(3); + r.append(&end_era); + r.append(&index); + r.append(&&PADDING[..]); + try!(batch.delete(&r.drain())); + trace!("commit: Delete journal for time #{}.{}: {}, (canon was {}): +{} -{} entries", end_era, index, journal.id, canon_id, journal.insertions.len(), journal.deletions.len()); + { + if canon_id == journal.id { + for h in &journal.insertions { + if let Some((d, rc)) = journal_overlay.backing_overlay.raw(h) { + if rc > 0 { + canon_insertions.push((h.clone(), d)); + } } } + canon_deletions = journal.deletions; } - canon_deletions = journal.deletions; + overlay_deletions.append(&mut journal.insertions); } - overlay_deletions.append(&mut journal.insertions); + index += 1; + } + // apply canon inserts first + for (k, v) in canon_insertions { + try!(batch.put(&k, v)); } - index += 1; - } - // apply canon inserts first - for (k, v) in canon_insertions { - try!(batch.put(&k, &v)); } // update the overlay for k in overlay_deletions { @@ -324,11 +327,11 @@ impl HashDB for OverlayRecentDB { let v = self.journal_overlay.read().unwrap().backing_overlay.get(key).map(|v| v.to_vec()); match v { Some(x) => { - Some(&self.transaction_overlay.denote(key, x).0) + Some(self.transaction_overlay.denote(key, x).0) } _ => { if let Some(x) = self.payload(key) { - Some(&self.transaction_overlay.denote(key, x).0) + Some(self.transaction_overlay.denote(key, x).0) } else { None diff --git a/util/src/overlaydb.rs b/util/src/overlaydb.rs index 3219693a0ca..2a56fe4674b 100644 --- a/util/src/overlaydb.rs +++ b/util/src/overlaydb.rs @@ -236,7 +236,7 @@ impl HashDB for OverlayDB { Some(x) => { let (d, rc) = x; if rc as i32 + memrc > 0 { - Some(&self.overlay.denote(key, d).0) + Some(self.overlay.denote(key, d).0) } else { None From 07d48a13d4d3e1826e7bfd995d5553095fa06e9d Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 25 Jul 2016 13:04:07 +0200 Subject: [PATCH 03/22] move trie traits into trie module --- util/src/trie/mod.rs | 48 ++++++++++++++++++++++++++-- util/src/trie/trietraits.rs | 62 ------------------------------------- 2 files changed, 45 insertions(+), 65 deletions(-) delete mode 100644 util/src/trie/trietraits.rs diff --git a/util/src/trie/mod.rs b/util/src/trie/mod.rs index d608863cd5c..f6b892c85e5 100644 --- a/util/src/trie/mod.rs +++ b/util/src/trie/mod.rs @@ -20,8 +20,6 @@ use std::fmt; use hash::H256; use hashdb::HashDB; -/// Export the trietraits module. -pub mod trietraits; /// Export the standardmap module. pub mod standardmap; /// Export the journal module. @@ -40,7 +38,6 @@ pub mod sectriedbmut; mod fatdb; mod fatdbmut; -pub use self::trietraits::{Trie, TrieMut}; pub use self::standardmap::{Alphabet, StandardMap, ValueMode}; pub use self::triedbmut::TrieDBMut; pub use self::triedb::{TrieDB, TrieDBIterator}; @@ -62,6 +59,51 @@ impl fmt::Display for TrieError { } } +/// Trie-Item type. +pub type TrieItem<'a> = (Vec, &'a [u8]); + +/// A key-value datastore implemented as a database-backed modified Merkle tree. +pub trait Trie { + /// Return the root of the trie. + fn root(&self) -> &H256; + + /// Is the trie empty? + fn is_empty(&self) -> bool { *self.root() == ::rlp::SHA3_NULL_RLP } + + /// Does the trie contain a given key? + fn contains(&self, key: &[u8]) -> bool; + + /// What is the value of the given key in this trie? + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Option<&'a [u8]> where 'a: 'key; + + /// Returns an iterator over elements of trie. + fn iter<'a>(&'a self) -> Box + 'a>; +} + +/// A key-value datastore implemented as a database-backed modified Merkle tree. +pub trait TrieMut { + /// Return the root of the trie. + fn root(&mut self) -> &H256; + + /// Is the trie empty? + fn is_empty(&self) -> bool; + + /// Does the trie contain a given key? + fn contains(&self, key: &[u8]) -> bool; + + /// What is the value of the given key in this trie? + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Option<&'a [u8]> where 'a: 'key; + + /// Insert a `key`/`value` pair into the trie. An `empty` value is equivalent to removing + /// `key` from the trie. + fn insert(&mut self, key: &[u8], value: &[u8]); + + /// Remove a `key` from the trie. Equivalent to making it equal to the empty + /// value. + fn remove(&mut self, key: &[u8]); +} + + /// Trie types #[derive(Debug, Clone)] pub enum TrieSpec { diff --git a/util/src/trie/trietraits.rs b/util/src/trie/trietraits.rs deleted file mode 100644 index 1f899f139ec..00000000000 --- a/util/src/trie/trietraits.rs +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright 2015, 2016 Ethcore (UK) Ltd. -// This file is part of Parity. - -// Parity is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Parity is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Parity. If not, see . - -use hash::H256; -use rlp::SHA3_NULL_RLP; - -/// Trie-Item type. -pub type TrieItem<'a> = (Vec, &'a [u8]); - -/// A key-value datastore implemented as a database-backed modified Merkle tree. -pub trait Trie { - /// Return the root of the trie. - fn root(&self) -> &H256; - - /// Is the trie empty? - fn is_empty(&self) -> bool { *self.root() == SHA3_NULL_RLP } - - /// Does the trie contain a given key? - fn contains(&self, key: &[u8]) -> bool; - - /// What is the value of the given key in this trie? - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Option<&'a [u8]> where 'a: 'key; - - /// Returns an iterator over elements of trie. - fn iter<'a>(&'a self) -> Box + 'a>; -} - -/// A key-value datastore implemented as a database-backed modified Merkle tree. -pub trait TrieMut { - /// Return the root of the trie. - fn root(&mut self) -> &H256; - - /// Is the trie empty? - fn is_empty(&self) -> bool; - - /// Does the trie contain a given key? - fn contains(&self, key: &[u8]) -> bool; - - /// What is the value of the given key in this trie? - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Option<&'a [u8]> where 'a: 'key; - - /// Insert a `key`/`value` pair into the trie. An `empty` value is equivalent to removing - /// `key` from the trie. - fn insert(&mut self, key: &[u8], value: &[u8]); - - /// Remove a `key` from the trie. Equivalent to making it equal to the empty - /// value. - fn remove(&mut self, key: &[u8]); -} From 2bf55873f82ddc6c26f0add8c037dacfe88da6bc Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 26 Jul 2016 17:42:10 +0200 Subject: [PATCH 04/22] replace "denote" with shim --- util/src/memorydb.rs | 82 +++++++++++++------------------------------- 1 file changed, 24 insertions(+), 58 deletions(-) diff --git a/util/src/memorydb.rs b/util/src/memorydb.rs index d10edc41339..15db8c1ae45 100644 --- a/util/src/memorydb.rs +++ b/util/src/memorydb.rs @@ -73,38 +73,17 @@ use std::default::Default; /// assert!(!m.contains(&k)); /// } /// ``` +#[derive(Default, Clone, PartialEq)] pub struct MemoryDB { - data: UnsafeCell>, + data: HashMap, aux: HashMap, } -impl Default for MemoryDB { - fn default() -> Self { - MemoryDB::new() - } -} - -impl ::std::cmp::PartialEq for MemoryDB { - fn eq(&self, rhs: &Self) -> bool { - let (my_data, rhs_data) = unsafe { (&*self.data.get(), &*rhs.data.get())}; - my_data == rhs_data && self.aux == rhs.aux - } -} - -impl Clone for MemoryDB { - fn clone(&self) -> Self { - MemoryDB { - data: UnsafeCell::new(unsafe { (*self.data.get()).clone() }), - aux: self.aux.clone() - } - } -} - impl MemoryDB { /// Create a new instance of the memory DB. pub fn new() -> MemoryDB { MemoryDB { - data: UnsafeCell::new(HashMap::new()), + data: HashMap::new(), aux: HashMap::new(), } } @@ -126,22 +105,21 @@ impl MemoryDB { /// } /// ``` pub fn clear(&mut self) { - unsafe { (*self.data.get()).clear() } + self.data.clear(); } /// Purge all zero-referenced data from the database. pub fn purge(&mut self) { - let data = unsafe { &mut *self.data.get() }; - let empties: Vec<_> = data.iter() + let empties: Vec<_> = self.data.iter() .filter(|&(_, &(_, rc))| rc == 0) .map(|(k, _)| k.clone()) .collect(); - for empty in empties { data.remove(&empty); } + for empty in empties { self.data.remove(&empty); } } /// Return the internal map of hashes to data, clearing the current state. pub fn drain(&mut self) -> HashMap { - mem::replace(unsafe { &mut *self.data.get() }, HashMap::new()) + mem::replace(&mut self.data, HashMap::new()) } /// Return the internal map of auxiliary data, clearing the current state. @@ -158,7 +136,7 @@ impl MemoryDB { if key == &SHA3_NULL_RLP { return Some(STATIC_NULL_RLP.clone()); } - unsafe { (*self.data.get()).get(key).map(|&(ref v, x)| (&v[..], x)) } + self.data.get(key).map(|&(ref v, x)| (&v[..], x)) } /// Denote than an existing value has the given key. Used when a key gets removed without @@ -166,25 +144,13 @@ impl MemoryDB { /// /// May safely be called even if the key's value is known, in which case it will be a no-op. pub fn denote(&self, key: &H256, value: Bytes) -> (&[u8], i32) { - if key == &SHA3_NULL_RLP { - return STATIC_NULL_RLP.clone(); - } - - unsafe { - let data = self.data.get(); - if let Some(&(ref v, x)) = (*data).get(key) { - return (&v[..], x); - } - - (*data).insert(key.clone(), (value, 0)); - let &(ref v, x) = (*data).get(key).unwrap(); - (&v[..], x) - } + unimplemented!() } /// Returns the size of allocated heap memory pub fn mem_used(&self) -> usize { - unsafe { (*self.data.get()).heap_size_of_children() } + self.data.heap_size_of_children() + + self.aux.heap_size_of_children() } /// Remove an element and delete it from storage if reference count reaches zero. @@ -213,34 +179,36 @@ impl HashDB for MemoryDB { if key == &SHA3_NULL_RLP { return Some(&NULL_RLP_STATIC); } - match unsafe { (*self.data.get()).get(key) } { + + match self.data.get(key) { Some(&(ref d, rc)) if rc > 0 => Some(d), _ => None } } fn keys(&self) -> HashMap { - let data = unsafe { &mut *self.data.get() }; - data.iter().filter_map(|(k, v)| if v.1 != 0 {Some((k.clone(), v.1))} else {None}).collect() + self.data.iter().filter_map(|(k, v)| if v.1 != 0 {Some((k.clone(), v.1))} else {None}).collect() } fn contains(&self, key: &H256) -> bool { if key == &SHA3_NULL_RLP { return true; } - match unsafe { (*self.data.get()).get(key) } { + + match self.data.get(key) { Some(&(_, x)) if x > 0 => true, _ => false } } fn insert(&mut self, value: &[u8]) -> H256 { + use std::i32; + if value == &NULL_RLP { return SHA3_NULL_RLP.clone(); } let key = value.sha3(); - let mut data = unsafe { &mut *self.data.get() }; - if match data.get_mut(&key) { + if match self.data.get_mut(&key) { Some(&mut (ref mut old_value, ref mut rc @ -0x80000000i32 ... 0)) => { *old_value = value.into(); *rc += 1; @@ -249,7 +217,7 @@ impl HashDB for MemoryDB { Some(&mut (_, ref mut x)) => { *x += 1; false } , None => true, }{ // ... None falls through into... - data.insert(key.clone(), (value.into(), 1)); + self.data.insert(key.clone(), (value.into(), 1)); } key } @@ -259,8 +227,7 @@ impl HashDB for MemoryDB { return; } - let data = unsafe { &mut *self.data.get() }; - match data.get_mut(&key) { + match self.data.get_mut(&key) { Some(&mut (ref mut old_value, ref mut rc @ -0x80000000i32 ... 0)) => { *old_value = value; *rc += 1; @@ -270,7 +237,7 @@ impl HashDB for MemoryDB { None => {}, } // ... None falls through into... - data.insert(key, (value, 1)); + self.data.insert(key, (value, 1)); } fn remove(&mut self, key: &H256) { @@ -278,12 +245,11 @@ impl HashDB for MemoryDB { return; } - let data = unsafe { &mut *self.data.get() }; - if match data.get_mut(key) { + if match self.data.get_mut(key) { Some(&mut (_, ref mut x)) => { *x -= 1; false } None => true }{ // ... None falls through into... - data.insert(key.clone(), (Bytes::new(), -1)); + self.data.insert(key.clone(), (Bytes::new(), -1)); } } From c631c6c079ef2db4243d9d84124b058565dc108a Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 26 Jul 2016 18:11:20 +0200 Subject: [PATCH 05/22] triedb returns results and no longer panics --- util/src/trie/fatdb.rs | 11 +-- util/src/trie/fatdbmut.rs | 19 ++-- util/src/trie/mod.rs | 50 ++++++++--- util/src/trie/sectriedb.rs | 11 +-- util/src/trie/sectriedbmut.rs | 19 ++-- util/src/trie/triedb.rs | 95 +++++++++++--------- util/src/trie/triedbmut.rs | 165 ++++++++++++++++++---------------- 7 files changed, 214 insertions(+), 156 deletions(-) diff --git a/util/src/trie/fatdb.rs b/util/src/trie/fatdb.rs index cc5786f43eb..f3e9857d2f8 100644 --- a/util/src/trie/fatdb.rs +++ b/util/src/trie/fatdb.rs @@ -17,8 +17,7 @@ use hash::H256; use sha3::Hashable; use hashdb::HashDB; -use super::{TrieDB, Trie, TrieDBIterator, TrieError}; -use trie::trietraits::TrieItem; +use super::{TrieDB, Trie, TrieDBIterator, TrieError, TrieItem}; /// A `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// Additionaly it stores inserted hash-key mappings for later retrieval. @@ -32,7 +31,7 @@ impl<'db> FatDB<'db> { /// Create a new trie with the backing database `db` and empty `root` /// Initialise to the state entailed by the genesis block. /// This guarantees the trie is built correctly. - pub fn new(db: &'db HashDB, root: &'db H256) -> Result { + pub fn new(db: &'db HashDB, root: &'db H256) -> super::Result { let fatdb = FatDB { raw: try!(TrieDB::new(db, root)) }; @@ -60,11 +59,13 @@ impl<'db> Trie for FatDB<'db> { self.raw.root() } - fn contains(&self, key: &[u8]) -> bool { + fn contains(&self, key: &[u8]) -> super::Result { self.raw.contains(&key.sha3()) } - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Option<&'a [u8]> where 'a: 'key { + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result<&'a [u8]> + where 'a: 'key + { self.raw.get(&key.sha3()) } } diff --git a/util/src/trie/fatdbmut.rs b/util/src/trie/fatdbmut.rs index 0dcc8b406ae..5fe3a0b1b10 100644 --- a/util/src/trie/fatdbmut.rs +++ b/util/src/trie/fatdbmut.rs @@ -17,7 +17,7 @@ use hash::H256; use sha3::Hashable; use hashdb::HashDB; -use super::{TrieDBMut, TrieMut, TrieError}; +use super::{TrieDBMut, TrieMut, TrieItem, TrieError}; /// A mutable `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// Additionaly it stores inserted hash-key mappings for later retrieval. @@ -38,7 +38,7 @@ impl<'db> FatDBMut<'db> { /// Create a new trie with the backing database `db` and `root`. /// /// Returns an error if root does not exist. - pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> Result { + pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> super::Result { Ok(FatDBMut { raw: try!(TrieDBMut::from_existing(db, root)) }) } @@ -62,23 +62,26 @@ impl<'db> TrieMut for FatDBMut<'db> { self.raw.is_empty() } - fn contains(&self, key: &[u8]) -> bool { + fn contains(&self, key: &[u8]) -> super::Result { self.raw.contains(&key.sha3()) } - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Option<&'a [u8]> where 'a: 'key { + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result<&'a [u8]> + where 'a: 'key + { self.raw.get(&key.sha3()) } - fn insert(&mut self, key: &[u8], value: &[u8]) { + fn insert(&mut self, key: &[u8], value: &[u8]) -> super::Result<()> { let hash = key.sha3(); - self.raw.insert(&hash, value); + try!(self.raw.insert(&hash, value)); let db = self.raw.db_mut(); db.insert_aux(hash.to_vec(), key.to_vec()); + Ok(()) } - fn remove(&mut self, key: &[u8]) { - self.raw.remove(&key.sha3()); + fn remove(&mut self, key: &[u8]) -> super::Result<()> { + self.raw.remove(&key.sha3()) } } diff --git a/util/src/trie/mod.rs b/util/src/trie/mod.rs index f6b892c85e5..3e1c7bd8569 100644 --- a/util/src/trie/mod.rs +++ b/util/src/trie/mod.rs @@ -46,22 +46,38 @@ pub use self::sectriedb::SecTrieDB; pub use self::fatdb::{FatDB, FatDBIterator}; pub use self::fatdbmut::FatDBMut; -/// Trie Errors +/// Trie Errors. +/// +/// These borrow the data within them to avoid excessive copying on every +/// trie operation. #[derive(Debug)] pub enum TrieError { /// Attempted to create a trie with a state root not in the DB. - InvalidStateRoot, + InvalidStateRoot(H256), + /// Trie item not found in the database, + IncompleteDatabase(H256), + /// Key not in trie. + NotInTrie, } impl fmt::Display for TrieError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "Trie Error: Invalid state root.") + match *self { + TrieError::InvalidStateRoot(ref root) => write!(f, "Invalid state root: {}", root), + TrieError::IncompleteDatabase(ref missing) => + write!(f, "Database missing expected key: {}", missing), + TrieError::NotInTrie => + write!(f, "Failed query"), + } } } /// Trie-Item type. pub type TrieItem<'a> = (Vec, &'a [u8]); +/// Trie result type. +pub type Result = ::std::result::Result; + /// A key-value datastore implemented as a database-backed modified Merkle tree. pub trait Trie { /// Return the root of the trie. @@ -71,10 +87,16 @@ pub trait Trie { fn is_empty(&self) -> bool { *self.root() == ::rlp::SHA3_NULL_RLP } /// Does the trie contain a given key? - fn contains(&self, key: &[u8]) -> bool; + fn contains(&self, key: &[u8]) -> Result { + match self.get(key) { + Ok(_) => Ok(true), + Err(TrieError::NotInTrie) => Ok(false), + Err(e) => Err(e), + } + } /// What is the value of the given key in this trie? - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Option<&'a [u8]> where 'a: 'key; + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Result<&'a [u8]> where 'a: 'key; /// Returns an iterator over elements of trie. fn iter<'a>(&'a self) -> Box + 'a>; @@ -89,18 +111,24 @@ pub trait TrieMut { fn is_empty(&self) -> bool; /// Does the trie contain a given key? - fn contains(&self, key: &[u8]) -> bool; + fn contains(&self, key: &[u8]) -> Result { + match self.get(key) { + Ok(_) => Ok(true), + Err(TrieError::NotInTrie) => Ok(false), + Err(e) => Err(e) + } + } /// What is the value of the given key in this trie? - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Option<&'a [u8]> where 'a: 'key; + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Result<&'a [u8]> where 'a: 'key; /// Insert a `key`/`value` pair into the trie. An `empty` value is equivalent to removing /// `key` from the trie. - fn insert(&mut self, key: &[u8], value: &[u8]); + fn insert(&mut self, key: &[u8], value: &[u8]) -> Result<()>; /// Remove a `key` from the trie. Equivalent to making it equal to the empty /// value. - fn remove(&mut self, key: &[u8]); + fn remove(&mut self, key: &[u8]) -> Result<()>; } @@ -137,7 +165,7 @@ impl TrieFactory { } /// Create new immutable instance of Trie. - pub fn readonly<'db>(&self, db: &'db HashDB, root: &'db H256) -> Result, TrieError> { + pub fn readonly<'db>(&self, db: &'db HashDB, root: &'db H256) -> Result> { match self.spec { TrieSpec::Generic => Ok(Box::new(try!(TrieDB::new(db, root)))), TrieSpec::Secure => Ok(Box::new(try!(SecTrieDB::new(db, root)))), @@ -155,7 +183,7 @@ impl TrieFactory { } /// Create new mutable instance of trie and check for errors. - pub fn from_existing<'db>(&self, db: &'db mut HashDB, root: &'db mut H256) -> Result, TrieError> { + pub fn from_existing<'db>(&self, db: &'db mut HashDB, root: &'db mut H256) -> Result> { match self.spec { TrieSpec::Generic => Ok(Box::new(try!(TrieDBMut::from_existing(db, root)))), TrieSpec::Secure => Ok(Box::new(try!(SecTrieDBMut::from_existing(db, root)))), diff --git a/util/src/trie/sectriedb.rs b/util/src/trie/sectriedb.rs index 9217fb4fc5b..bcb38d323d4 100644 --- a/util/src/trie/sectriedb.rs +++ b/util/src/trie/sectriedb.rs @@ -18,8 +18,7 @@ use hash::H256; use sha3::Hashable; use hashdb::HashDB; use super::triedb::TrieDB; -use super::trietraits::{Trie, TrieItem}; -use super::TrieError; +use super::{Trie, TrieError, TrieItem}; /// A `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// @@ -34,7 +33,7 @@ impl<'db> SecTrieDB<'db> { /// Initialise to the state entailed by the genesis block. /// This guarantees the trie is built correctly. /// Returns an error if root does not exist. - pub fn new(db: &'db HashDB, root: &'db H256) -> Result { + pub fn new(db: &'db HashDB, root: &'db H256) -> super::Result { Ok(SecTrieDB { raw: try!(TrieDB::new(db, root)) }) } @@ -56,11 +55,13 @@ impl<'db> Trie for SecTrieDB<'db> { fn root(&self) -> &H256 { self.raw.root() } - fn contains(&self, key: &[u8]) -> bool { + fn contains(&self, key: &[u8]) -> super::Result { self.raw.contains(&key.sha3()) } - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Option<&'a [u8]> where 'a: 'key { + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result<&'a [u8]> + where 'a: 'key + { self.raw.get(&key.sha3()) } } diff --git a/util/src/trie/sectriedbmut.rs b/util/src/trie/sectriedbmut.rs index e8eb24c1f96..a5b70c7267b 100644 --- a/util/src/trie/sectriedbmut.rs +++ b/util/src/trie/sectriedbmut.rs @@ -18,8 +18,7 @@ use hash::H256; use sha3::Hashable; use hashdb::HashDB; use super::triedbmut::TrieDBMut; -use super::trietraits::TrieMut; -use super::TrieError; +use super::{TrieMut, TrieError}; /// A mutable `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// @@ -39,7 +38,7 @@ impl<'db> SecTrieDBMut<'db> { /// Create a new trie with the backing database `db` and `root`. /// /// Returns an error if root does not exist. - pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> Result { + pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> super::Result { Ok(SecTrieDBMut { raw: try!(TrieDBMut::from_existing(db, root)) }) } @@ -59,20 +58,22 @@ impl<'db> TrieMut for SecTrieDBMut<'db> { self.raw.is_empty() } - fn contains(&self, key: &[u8]) -> bool { + fn contains(&self, key: &[u8]) -> super::Result { self.raw.contains(&key.sha3()) } - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Option<&'a [u8]> where 'a: 'key { + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result<&'a [u8]> + where 'a: 'key + { self.raw.get(&key.sha3()) } - fn insert(&mut self, key: &[u8], value: &[u8]) { - self.raw.insert(&key.sha3(), value); + fn insert(&mut self, key: &[u8], value: &[u8]) -> super::Result<()> { + self.raw.insert(&key.sha3(), value) } - fn remove(&mut self, key: &[u8]) { - self.raw.remove(&key.sha3()); + fn remove(&mut self, key: &[u8]) -> super::Result<()> { + self.raw.remove(&key.sha3()) } } diff --git a/util/src/trie/triedb.rs b/util/src/trie/triedb.rs index a50904cc164..4e5c92787fd 100644 --- a/util/src/trie/triedb.rs +++ b/util/src/trie/triedb.rs @@ -18,9 +18,8 @@ use common::*; use hashdb::*; use nibbleslice::*; use rlp::*; -use super::trietraits::{Trie, TrieItem}; use super::node::Node; -use super::TrieError; +use super::{Trie, TrieItem, TrieError}; /// A `Trie` implementation using a generic `HashDB` backing database. /// @@ -59,9 +58,9 @@ pub struct TrieDB<'db> { impl<'db> TrieDB<'db> { /// Create a new trie with the backing database `db` and `root` /// Returns an error if `root` does not exist - pub fn new(db: &'db HashDB, root: &'db H256) -> Result { + pub fn new(db: &'db HashDB, root: &'db H256) -> super::Result { if !db.contains(root) { - Err(TrieError::InvalidStateRoot) + Err(TrieError::InvalidStateRoot(*root)) } else { Ok(TrieDB { db: db, @@ -77,11 +76,11 @@ impl<'db> TrieDB<'db> { } /// Determine all the keys in the backing database that belong to the trie. - pub fn keys(&self) -> Vec { + pub fn keys(&self) -> super::Result> { let mut ret: Vec = Vec::new(); ret.push(self.root.clone()); - self.accumulate_keys(self.root_node(), &mut ret); - ret + try!(self.accumulate_keys(try!(self.root_node()), &mut ret)); + Ok(ret) } /// Convert a vector of hashes to a hashmap of hash to occurrences. @@ -95,49 +94,51 @@ impl<'db> TrieDB<'db> { /// Determine occurrences of items in the backing database which are not related to this /// trie. - pub fn db_items_remaining(&self) -> HashMap { + pub fn db_items_remaining(&self) -> super::Result> { let mut ret = self.db.keys(); - for (k, v) in Self::to_map(self.keys()).into_iter() { + for (k, v) in Self::to_map(try!(self.keys())).into_iter() { let keycount = *ret.get(&k).unwrap_or(&0); match keycount <= v as i32 { true => ret.remove(&k), _ => ret.insert(k, keycount - v as i32), }; } - ret + Ok(ret) } /// Recursion helper for `keys`. - fn accumulate_keys(&self, node: Node, acc: &mut Vec) { + fn accumulate_keys(&self, node: Node, acc: &mut Vec) -> super::Result<()> { let mut handle_payload = |payload| { let p = Rlp::new(payload); if p.is_data() && p.size() == 32 { acc.push(p.as_val()); } - self.accumulate_keys(self.get_node(payload), acc); + self.accumulate_keys(try!(self.get_node(payload)), acc) }; match node { - Node::Extension(_, payload) => handle_payload(payload), - Node::Branch(payloads, _) => for payload in &payloads { handle_payload(payload) }, + Node::Extension(_, payload) => try!(handle_payload(payload)), + Node::Branch(payloads, _) => for payload in &payloads { try!(handle_payload(payload)) }, _ => {}, } + + Ok(()) } /// Get the root node's RLP. - fn root_node(&self) -> Node { - Node::decoded(self.root_data()) + fn root_node(&self) -> super::Result { + self.root_data().map(Node::decoded) } /// Get the data of the root node. - fn root_data(&self) -> &[u8] { - self.db.get(&self.root).expect("Trie root not found!") + fn root_data(&self) -> super::Result<&[u8]> { + self.db.get(&self.root).ok_or(TrieError::InvalidStateRoot(*self.root)) } /// Get the root node as a `Node`. - fn get_node<'a>(&'a self, node: &'a [u8]) -> Node { - Node::decoded(self.get_raw_or_lookup(node)) + fn get_node(&'db self, node: &'db [u8]) -> super::Result { + self.get_raw_or_lookup(node).map(Node::decoded) } /// Indentation helper for `formal_all`. @@ -154,7 +155,9 @@ impl<'db> TrieDB<'db> { Node::Leaf(slice, value) => try!(writeln!(f, "'{:?}: {:?}.", slice, value.pretty())), Node::Extension(ref slice, ref item) => { try!(write!(f, "'{:?} ", slice)); - try!(self.fmt_all(self.get_node(item), f, deepness)); + if let Ok(node) = self.get_node(item) { + try!(self.fmt_all(node, f, deepness)); + } }, Node::Branch(ref nodes, ref value) => { try!(writeln!(f, "")); @@ -164,12 +167,15 @@ impl<'db> TrieDB<'db> { } for i in 0..16 { match self.get_node(nodes[i]) { - Node::Empty => {}, - n => { + Ok(Node::Empty) => {}, + Ok(n) => { try!(self.fmt_indent(f, deepness + 1)); try!(write!(f, "'{:x} ", i)); try!(self.fmt_all(n, f, deepness + 1)); } + Err(e) => { + write!(f, "ERROR: {}", e); + } } } }, @@ -182,8 +188,10 @@ impl<'db> TrieDB<'db> { } /// Return optional data for a key given as a `NibbleSlice`. Returns `None` if no data exists. - fn do_lookup<'a, 'key>(&'a self, key: &NibbleSlice<'key>) -> Option<&'a [u8]> where 'a: 'key { - let root_rlp = self.root_data(); + fn do_lookup<'key>(&'db self, key: &NibbleSlice<'key>) -> super::Result<&'db [u8]> + where 'db: 'key + { + let root_rlp = try!(self.root_data()); self.get_from_node(&root_rlp, key) } @@ -191,29 +199,35 @@ impl<'db> TrieDB<'db> { /// value exists for the key. /// /// Note: Not a public API; use Trie trait functions. - fn get_from_node<'a, 'key>(&'a self, node: &'a [u8], key: &NibbleSlice<'key>) -> Option<&'a [u8]> where 'a: 'key { + fn get_from_node<'key>(&'db self, node: &'db [u8], key: &NibbleSlice<'key>) -> super::Result<&'db [u8]> + where 'db: 'key + { match Node::decoded(node) { - Node::Leaf(ref slice, ref value) if key == slice => Some(value), + Node::Leaf(ref slice, ref value) if key == slice => Ok(value), Node::Extension(ref slice, ref item) if key.starts_with(slice) => { - self.get_from_node(self.get_raw_or_lookup(item), &key.mid(slice.len())) + let data = try!(self.get_raw_or_lookup(item)); + self.get_from_node(data, &key.mid(slice.len())) }, Node::Branch(ref nodes, value) => match key.is_empty() { - true => value, - false => self.get_from_node(self.get_raw_or_lookup(nodes[key.at(0) as usize]), &key.mid(1)) + true => value.ok_or(TrieError::NotInTrie), + false => self.get_from_node(try!(self.get_raw_or_lookup(nodes[key.at(0) as usize])), &key.mid(1)) }, - _ => None + _ => Err(TrieError::NotInTrie) } } /// Given some node-describing data `node`, return the actual node RLP. /// This could be a simple identity operation in the case that the node is sufficiently small, but /// may require a database lookup. - fn get_raw_or_lookup<'a>(&'a self, node: &'a [u8]) -> &'a [u8] { + fn get_raw_or_lookup(&'db self, node: &'db [u8]) -> super::Result<&'db [u8]> { // check if its sha3 + len let r = Rlp::new(node); match r.is_data() && r.size() == 32 { - true => self.db.get(&r.as_val::()).unwrap_or_else(|| panic!("Not found! {:?}", r.as_val::())), - false => node + true => { + let key = r.as_val::(); + self.db.get(&key).ok_or(TrieError::IncompleteDatabase(key)) + } + false => Ok(node) } } } @@ -229,7 +243,6 @@ enum Status { #[derive(Clone, Eq, PartialEq)] struct Crumb<'a> { node: Node<'a>, -// key: &'a[u8], status: Status, } @@ -262,7 +275,7 @@ impl<'a> TrieDBIterator<'a> { trail: vec![], key_nibbles: Vec::new(), }; - r.descend(db.root_data()); + r.descend(db.root_data().unwrap()); r } @@ -270,7 +283,7 @@ impl<'a> TrieDBIterator<'a> { fn descend(&mut self, d: &'a [u8]) { self.trail.push(Crumb { status: Status::Entering, - node: self.db.get_node(d) + node: self.db.get_node(d).unwrap(), }); match self.trail.last().unwrap().node { Node::Leaf(n, _) | Node::Extension(n, _) => { self.key_nibbles.extend(n.iter()); }, @@ -342,11 +355,9 @@ impl<'db> Trie for TrieDB<'db> { fn root(&self) -> &H256 { &self.root } - fn contains(&self, key: &[u8]) -> bool { - self.get(key).is_some() - } - - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Option<&'a [u8]> where 'a: 'key { + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result<&'a [u8]> + where 'a: 'key + { self.do_lookup(&NibbleSlice::new(key)) } } diff --git a/util/src/trie/triedbmut.rs b/util/src/trie/triedbmut.rs index c50a66cc8b8..4372326b609 100644 --- a/util/src/trie/triedbmut.rs +++ b/util/src/trie/triedbmut.rs @@ -309,9 +309,9 @@ impl<'a> TrieDBMut<'a> { /// Create a new trie with the backing database `db` and `root. /// Returns an error if `root` does not exist. - pub fn from_existing(db: &'a mut HashDB, root: &'a mut H256) -> Result { + pub fn from_existing(db: &'a mut HashDB, root: &'a mut H256) -> super::Result { if !db.contains(root) { - return Err(TrieError::InvalidStateRoot); + return Err(TrieError::InvalidStateRoot(*root)); } let root_handle = NodeHandle::Hash(*root); @@ -335,23 +335,23 @@ impl<'a> TrieDBMut<'a> { } // cache a node by hash - fn cache(&mut self, hash: H256) -> StorageHandle { - let node_rlp = self.db.get(&hash).expect("Not found!"); + fn cache(&mut self, hash: H256) -> super::Result { + let node_rlp = try!(self.db.get(&hash).ok_or(TrieError::IncompleteDatabase(hash))); let node = Node::from_rlp(node_rlp, &*self.db, &mut self.storage); - self.storage.alloc(Stored::Cached(node, hash)) + Ok(self.storage.alloc(Stored::Cached(node, hash))) } // inspect a node, choosing either to replace, restore, or delete it. // if restored or replaced, returns the new node along with a flag of whether it was changed. - fn inspect(&mut self, stored: Stored, inspector: F) -> Option<(Stored, bool)> - where F: FnOnce(&mut Self, Node) -> Action { - match stored { - Stored::New(node) => match inspector(self, node) { + fn inspect(&mut self, stored: Stored, inspector: F) -> super::Result> + where F: FnOnce(&mut Self, Node) -> super::Result { + Ok(match stored { + Stored::New(node) => match try!(inspector(self, node)) { Action::Restore(node) => Some((Stored::New(node), false)), Action::Replace(node) => Some((Stored::New(node), true)), Action::Delete => None, }, - Stored::Cached(node, hash) => match inspector(self, node) { + Stored::Cached(node, hash) => match try!(inspector(self, node)) { Action::Restore(node) => Some((Stored::Cached(node, hash), false)), Action::Replace(node) => { self.death_row.insert(hash); @@ -362,21 +362,22 @@ impl<'a> TrieDBMut<'a> { None } }, - } + }) } // walk the trie, attempting to find the key's node. - fn lookup<'x, 'key>(&'x self, partial: NibbleSlice<'key>, handle: &NodeHandle) -> Option<&'x [u8]> - where 'x: 'key { + fn lookup<'x, 'key>(&'x self, partial: NibbleSlice<'key>, handle: &NodeHandle) -> super::Result<&'x [u8]> + where 'x: 'key + { match *handle { NodeHandle::Hash(ref hash) => self.do_db_lookup(hash, partial), NodeHandle::InMemory(ref handle) => match self.storage[handle] { - Node::Empty => None, + Node::Empty => Err(TrieError::NotInTrie), Node::Leaf(ref key, ref value) => { if NibbleSlice::from_encoded(key).0 == partial { - Some(value) + Ok(value) } else { - None + Err(TrieError::NotInTrie) } } Node::Extension(ref slice, ref child) => { @@ -384,15 +385,16 @@ impl<'a> TrieDBMut<'a> { if partial.starts_with(&slice) { self.lookup(partial.mid(slice.len()), child) } else { - None + Err(TrieError::NotInTrie) } } Node::Branch(ref children, ref value) => { if partial.is_empty() { - value.as_ref().map(|v| &v[..]) + value.as_ref().map(|v| &v[..]).ok_or(TrieError::NotInTrie) } else { let idx = partial.at(0); - (&children[idx as usize]).as_ref().and_then(|child| self.lookup(partial.mid(1), child)) + (&children[idx as usize]).as_ref().ok_or(TrieError::NotInTrie) + .and_then(|child| self.lookup(partial.mid(1), child)) } } } @@ -400,60 +402,68 @@ impl<'a> TrieDBMut<'a> { } /// Return optional data for a key given as a `NibbleSlice`. Returns `None` if no data exists. - fn do_db_lookup<'x, 'key>(&'x self, hash: &H256, key: NibbleSlice<'key>) -> Option<&'x [u8]> where 'x: 'key { - self.db.get(hash).and_then(|node_rlp| self.get_from_db_node(&node_rlp, key)) + fn do_db_lookup<'x, 'key>(&'x self, hash: &H256, key: NibbleSlice<'key>) -> super::Result<&'x [u8]> + where 'x: 'key + { + self.db.get(hash).ok_or(TrieError::IncompleteDatabase(*hash)) + .and_then(|node_rlp| self.get_from_db_node(&node_rlp, key)) } /// Recursible function to retrieve the value given a `node` and a partial `key`. `None` if no /// value exists for the key. /// /// Note: Not a public API; use Trie trait functions. - fn get_from_db_node<'x, 'key>(&'x self, node: &'x [u8], key: NibbleSlice<'key>) -> Option<&'x [u8]> where 'x: 'key { + fn get_from_db_node<'x, 'key>(&'x self, node: &'x [u8], key: NibbleSlice<'key>) -> super::Result<&'x [u8]> + where 'x: 'key + { match RlpNode::decoded(node) { - RlpNode::Leaf(ref slice, ref value) if &key == slice => Some(value), + RlpNode::Leaf(ref slice, ref value) if &key == slice => Ok(value), RlpNode::Extension(ref slice, ref item) if key.starts_with(slice) => { - self.get_from_db_node(self.get_raw_or_lookup(item), key.mid(slice.len())) + self.get_from_db_node(try!(self.get_raw_or_lookup(item)), key.mid(slice.len())) }, RlpNode::Branch(ref nodes, value) => match key.is_empty() { - true => value, - false => self.get_from_db_node(self.get_raw_or_lookup(nodes[key.at(0) as usize]), key.mid(1)) + true => value.ok_or(TrieError::NotInTrie), + false => self.get_from_db_node(try!(self.get_raw_or_lookup(nodes[key.at(0) as usize])), key.mid(1)) }, - _ => None + _ => Err(TrieError::NotInTrie), } } /// Given some node-describing data `node`, return the actual node RLP. /// This could be a simple identity operation in the case that the node is sufficiently small, but /// may require a database lookup. - fn get_raw_or_lookup<'x>(&'x self, node: &'x [u8]) -> &'x [u8] { + fn get_raw_or_lookup<'x>(&'x self, node: &'x [u8]) -> super::Result<&'x [u8]> { // check if its sha3 + len let r = Rlp::new(node); match r.is_data() && r.size() == 32 { - true => self.db.get(&r.as_val::()).expect("Not found!"), - false => node + true => { + let key = r.as_val::(); + self.db.get(&key).ok_or(TrieError::IncompleteDatabase(key)) + } + false => Ok(node) } } /// insert a key, value pair into the trie, creating new nodes if necessary. - fn insert_at(&mut self, handle: NodeHandle, partial: NibbleSlice, value: Bytes) -> (StorageHandle, bool) { + fn insert_at(&mut self, handle: NodeHandle, partial: NibbleSlice, value: Bytes) -> super::Result<(StorageHandle, bool)> { let h = match handle { NodeHandle::InMemory(h) => h, - NodeHandle::Hash(h) => self.cache(h) + NodeHandle::Hash(h) => try!(self.cache(h)), }; let stored = self.storage.destroy(h); - let (new_stored, changed) = self.inspect(stored, move |trie, stored| { - trie.insert_inspector(stored, partial, value).into_action() - }).expect("Insertion never deletes."); + let (new_stored, changed) = try!(self.inspect(stored, move |trie, stored| { + trie.insert_inspector(stored, partial, value).map(|a| a.into_action()) + })).expect("Insertion never deletes."); - (self.storage.alloc(new_stored), changed) + Ok((self.storage.alloc(new_stored), changed)) } /// the insertion inspector. #[cfg_attr(feature = "dev", allow(cyclomatic_complexity))] - fn insert_inspector(&mut self, node: Node, partial: NibbleSlice, value: Bytes) -> InsertAction { + fn insert_inspector(&mut self, node: Node, partial: NibbleSlice, value: Bytes) -> super::Result { trace!(target: "trie", "augmented (partial: {:?}, value: {:?})", partial, value.pretty()); - match node { + Ok(match node { Node::Empty => { trace!(target: "trie", "empty: COMPOSE"); InsertAction::Replace(Node::Leaf(partial.encoded(true), value)) @@ -473,11 +483,11 @@ impl<'a> TrieDBMut<'a> { let partial = partial.mid(1); if let Some(child) = children[idx].take() { // original had something there. recurse down into it. - let (new_child, changed) = self.insert_at(child, partial, value); + let (new_child, changed) = try!(self.insert_at(child, partial, value)); children[idx] = Some(new_child.into()); if !changed { // the new node we composed didn't change. that means our branch is untouched too. - return InsertAction::Restore(Node::Branch(children, stored_value)); + return Ok(InsertAction::Restore(Node::Branch(children, stored_value))); } } else { // original had nothing there. compose a leaf. @@ -516,7 +526,8 @@ impl<'a> TrieDBMut<'a> { }; // always replace because whatever we get out here is not the branch we started with. - InsertAction::Replace(self.insert_inspector(branch, partial, value).unwrap_node()) + let branch_action = try!(self.insert_inspector(branch, partial, value)).unwrap_node(); + InsertAction::Replace(branch_action) } else if cp == existing_key.len() { trace!(target: "trie", "complete-prefix (cp={:?}): AUGMENT-AT-END", cp); @@ -524,7 +535,7 @@ impl<'a> TrieDBMut<'a> { // make a stub branch and an extension. let branch = Node::Branch(empty_children(), Some(stored_value)); // augment the new branch. - let branch = self.insert_inspector(branch, partial.mid(cp), value).unwrap_node(); + let branch = try!(self.insert_inspector(branch, partial.mid(cp), value)).unwrap_node(); // always replace since we took a leaf and made an extension. let branch_handle = self.storage.alloc(Stored::New(branch)).into(); @@ -537,7 +548,7 @@ impl<'a> TrieDBMut<'a> { let low = Node::Leaf(existing_key.mid(cp).encoded(true), stored_value); // augment it. this will result in the Leaf -> cp == 0 routine, // which creates a branch. - let augmented_low = self.insert_inspector(low, partial.mid(cp), value).unwrap_node(); + let augmented_low = try!(self.insert_inspector(low, partial.mid(cp), value)).unwrap_node(); // make an extension using it. this is a replacement. InsertAction::Replace(Node::Extension( @@ -568,14 +579,15 @@ impl<'a> TrieDBMut<'a> { }; // continue inserting. - InsertAction::Replace(self.insert_inspector(Node::Branch(children, None), partial, value).unwrap_node()) + let branch_action = try!(self.insert_inspector(Node::Branch(children, None), partial, value)).unwrap_node(); + InsertAction::Replace(branch_action) } else if cp == existing_key.len() { trace!(target: "trie", "complete-prefix (cp={:?}): AUGMENT-AT-END", cp); // fully-shared prefix. // insert into the child node. - let (new_child, changed) = self.insert_at(child_branch, partial.mid(cp), value); + let (new_child, changed) = try!(self.insert_at(child_branch, partial.mid(cp), value)); let new_ext = Node::Extension(existing_key.encoded(false), new_child.into()); // if the child branch wasn't changed, meaning this extension remains the same. @@ -589,7 +601,7 @@ impl<'a> TrieDBMut<'a> { // partially-shared. let low = Node::Extension(existing_key.mid(cp).encoded(false), child_branch); // augment the extension. this will take the cp == 0 path, creating a branch. - let augmented_low = self.insert_inspector(low, partial.mid(cp), value).unwrap_node(); + let augmented_low = try!(self.insert_inspector(low, partial.mid(cp), value)).unwrap_node(); // always replace, since this extension is not the one we started with. // this is known because the partial key is only the common prefix. @@ -599,37 +611,38 @@ impl<'a> TrieDBMut<'a> { )) } } - } + }) } /// Remove a node from the trie based on key. - fn remove_at(&mut self, handle: NodeHandle, partial: NibbleSlice) -> Option<(StorageHandle, bool)> { + fn remove_at(&mut self, handle: NodeHandle, partial: NibbleSlice) -> super::Result> { let stored = match handle { NodeHandle::InMemory(h) => self.storage.destroy(h), NodeHandle::Hash(h) => { - let handle = self.cache(h); + let handle = try!(self.cache(h)); self.storage.destroy(handle) } }; - self.inspect(stored, move |trie, node| trie.remove_inspector(node, partial)) - .map(|(new, changed)| (self.storage.alloc(new), changed)) + let opt = try!(self.inspect(stored, move |trie, node| trie.remove_inspector(node, partial))); + + Ok(opt.map(|(new, changed)| (self.storage.alloc(new), changed))) } /// the removal inspector - fn remove_inspector(&mut self, node: Node, partial: NibbleSlice) -> Action { - match (node, partial.is_empty()) { + fn remove_inspector(&mut self, node: Node, partial: NibbleSlice) -> super::Result { + Ok(match (node, partial.is_empty()) { (Node::Empty, _) => Action::Delete, (Node::Branch(c, None), true) => Action::Restore(Node::Branch(c, None)), (Node::Branch(children, _), true) => { // always replace since we took the value out. - Action::Replace(self.fix(Node::Branch(children, None))) + Action::Replace(try!(self.fix(Node::Branch(children, None)))) } (Node::Branch(mut children, value), false) => { let idx = partial.at(0) as usize; if let Some(child) = children[idx].take() { trace!(target: "trie", "removing value out of branch child, partial={:?}", partial); - match self.remove_at(child, partial.mid(1)) { + match try!(self.remove_at(child, partial.mid(1))) { Some((new, changed)) => { children[idx] = Some(new.into()); let branch = Node::Branch(children, value); @@ -644,7 +657,7 @@ impl<'a> TrieDBMut<'a> { // the child we took was deleted. // the node may need fixing. trace!(target: "trie", "branch child deleted, partial={:?}", partial); - Action::Replace(self.fix(Node::Branch(children, value))) + Action::Replace(try!(self.fix(Node::Branch(children, value)))) } } } else { @@ -670,14 +683,14 @@ impl<'a> TrieDBMut<'a> { if cp == existing_len { // try to remove from the child branch. trace!(target: "trie", "removing from extension child, partial={:?}", partial); - match self.remove_at(child_branch, partial.mid(cp)) { + match try!(self.remove_at(child_branch, partial.mid(cp))) { Some((new_child, changed)) => { let new_child = new_child.into(); // if the child branch was unchanged, then the extension is too. // otherwise, this extension may need fixing. match changed { - true => Action::Replace(self.fix(Node::Extension(encoded, new_child))), + true => Action::Replace(try!(self.fix(Node::Extension(encoded, new_child)))), false => Action::Restore(Node::Extension(encoded, new_child)), } } @@ -692,7 +705,7 @@ impl<'a> TrieDBMut<'a> { Action::Restore(Node::Extension(encoded, child_branch)) } } - } + }) } /// Given a node which may be in an _invalid state_, fix it such that it is then in a valid @@ -701,7 +714,7 @@ impl<'a> TrieDBMut<'a> { /// _invalid state_ means: /// - Branch node where there is only a single entry; /// - Extension node followed by anything other than a Branch node. - fn fix(&mut self, node: Node) -> Node { + fn fix(&mut self, node: Node) -> super::Result { match node { Node::Branch(mut children, value) => { // if only a single value, transmute to leaf/extension and feed through fixed. @@ -734,12 +747,12 @@ impl<'a> TrieDBMut<'a> { (UsedIndex::None, Some(value)) => { // make a leaf. trace!(target: "trie", "fixing: branch -> leaf"); - Node::Leaf(NibbleSlice::new(&[]).encoded(true), value) + Ok(Node::Leaf(NibbleSlice::new(&[]).encoded(true), value)) } (_, value) => { // all is well. trace!(target: "trie", "fixing: restoring branch"); - Node::Branch(children, value) + Ok(Node::Branch(children, value)) } } } @@ -747,7 +760,7 @@ impl<'a> TrieDBMut<'a> { let stored = match child { NodeHandle::InMemory(h) => self.storage.destroy(h), NodeHandle::Hash(h) => { - let handle = self.cache(h); + let handle = try!(self.cache(h)); self.storage.destroy(handle) } }; @@ -782,7 +795,7 @@ impl<'a> TrieDBMut<'a> { let new_partial = NibbleSlice::new_composed(&partial, &sub_partial); trace!(target: "trie", "fixing: extension -> leaf. new_partial={:?}", new_partial); - Node::Leaf(new_partial.encoded(true), value) + Ok(Node::Leaf(new_partial.encoded(true), value)) } child_node => { trace!(target: "trie", "fixing: restoring extension"); @@ -794,11 +807,11 @@ impl<'a> TrieDBMut<'a> { Stored::New(child_node) }; - Node::Extension(partial, self.storage.alloc(stored).into()) + Ok(Node::Extension(partial, self.storage.alloc(stored).into())) } } } - other => other, // only ext and branch need fixing. + other => Ok(other), // only ext and branch need fixing. } } @@ -881,29 +894,27 @@ impl<'a> TrieMut for TrieDBMut<'a> { } } - fn get<'b, 'key>(&'b self, key: &'key [u8]) -> Option<&'b [u8]> where 'b: 'key { + fn get<'x, 'key>(&'x self, key: &'key [u8]) -> super::Result<&'x [u8]> where 'x: 'key { self.lookup(NibbleSlice::new(key), &self.root_handle) } - fn contains(&self, key: &[u8]) -> bool { - self.get(key).is_some() - } - fn insert(&mut self, key: &[u8], value: &[u8]) { + fn insert(&mut self, key: &[u8], value: &[u8]) -> super::Result<()> { if value.is_empty() { - self.remove(key); - return; + return self.remove(key); } let root_handle = self.root_handle(); - let (new_handle, _) = self.insert_at(root_handle, NibbleSlice::new(key), value.to_owned()); + let (new_handle, _) = try!(self.insert_at(root_handle, NibbleSlice::new(key), value.to_owned())); self.root_handle = NodeHandle::InMemory(new_handle); + + Ok(()) } - fn remove(&mut self, key: &[u8]) { + fn remove(&mut self, key: &[u8]) -> super::Result<()> { let root_handle = self.root_handle(); let key = NibbleSlice::new(key); - match self.remove_at(root_handle, key) { + match try!(self.remove_at(root_handle, key)) { Some((handle, _)) => { self.root_handle = NodeHandle::InMemory(handle); } @@ -912,6 +923,8 @@ impl<'a> TrieMut for TrieDBMut<'a> { *self.root = SHA3_NULL_RLP; } }; + + Ok(()) } } From b1ff25d27fb963360c447d1d24a8d8b6ca6008fe Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 26 Jul 2016 18:18:49 +0200 Subject: [PATCH 06/22] fix warnings --- util/src/memorydb.rs | 6 +----- util/src/trie/fatdb.rs | 2 +- util/src/trie/fatdbmut.rs | 2 +- util/src/trie/sectriedb.rs | 2 +- util/src/trie/sectriedbmut.rs | 2 +- util/src/trie/triedb.rs | 2 +- 6 files changed, 6 insertions(+), 10 deletions(-) diff --git a/util/src/memorydb.rs b/util/src/memorydb.rs index 15db8c1ae45..209af46fec3 100644 --- a/util/src/memorydb.rs +++ b/util/src/memorydb.rs @@ -24,11 +24,9 @@ use hashdb::*; use heapsize::*; use std::mem; use std::collections::HashMap; -use std::cell::UnsafeCell; const STATIC_NULL_RLP: (&'static [u8], i32) = (&[0x80; 1], 1); use std::collections::hash_map::Entry; -use std::default::Default; /// Reference-counted memory-based `HashDB` implementation. /// @@ -143,7 +141,7 @@ impl MemoryDB { /// a prior insert and thus has a negative reference with no value. /// /// May safely be called even if the key's value is known, in which case it will be a no-op. - pub fn denote(&self, key: &H256, value: Bytes) -> (&[u8], i32) { + pub fn denote(&self, _key: &H256, _value: Bytes) -> (&[u8], i32) { unimplemented!() } @@ -202,8 +200,6 @@ impl HashDB for MemoryDB { } fn insert(&mut self, value: &[u8]) -> H256 { - use std::i32; - if value == &NULL_RLP { return SHA3_NULL_RLP.clone(); } diff --git a/util/src/trie/fatdb.rs b/util/src/trie/fatdb.rs index f3e9857d2f8..6a1e079013f 100644 --- a/util/src/trie/fatdb.rs +++ b/util/src/trie/fatdb.rs @@ -17,7 +17,7 @@ use hash::H256; use sha3::Hashable; use hashdb::HashDB; -use super::{TrieDB, Trie, TrieDBIterator, TrieError, TrieItem}; +use super::{TrieDB, Trie, TrieDBIterator, TrieItem}; /// A `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// Additionaly it stores inserted hash-key mappings for later retrieval. diff --git a/util/src/trie/fatdbmut.rs b/util/src/trie/fatdbmut.rs index 5fe3a0b1b10..b933b442f09 100644 --- a/util/src/trie/fatdbmut.rs +++ b/util/src/trie/fatdbmut.rs @@ -17,7 +17,7 @@ use hash::H256; use sha3::Hashable; use hashdb::HashDB; -use super::{TrieDBMut, TrieMut, TrieItem, TrieError}; +use super::{TrieDBMut, TrieMut}; /// A mutable `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// Additionaly it stores inserted hash-key mappings for later retrieval. diff --git a/util/src/trie/sectriedb.rs b/util/src/trie/sectriedb.rs index bcb38d323d4..b756b815491 100644 --- a/util/src/trie/sectriedb.rs +++ b/util/src/trie/sectriedb.rs @@ -18,7 +18,7 @@ use hash::H256; use sha3::Hashable; use hashdb::HashDB; use super::triedb::TrieDB; -use super::{Trie, TrieError, TrieItem}; +use super::{Trie, TrieItem}; /// A `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// diff --git a/util/src/trie/sectriedbmut.rs b/util/src/trie/sectriedbmut.rs index a5b70c7267b..74205afbe35 100644 --- a/util/src/trie/sectriedbmut.rs +++ b/util/src/trie/sectriedbmut.rs @@ -18,7 +18,7 @@ use hash::H256; use sha3::Hashable; use hashdb::HashDB; use super::triedbmut::TrieDBMut; -use super::{TrieMut, TrieError}; +use super::TrieMut; /// A mutable `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// diff --git a/util/src/trie/triedb.rs b/util/src/trie/triedb.rs index 4e5c92787fd..9a6c4a4205e 100644 --- a/util/src/trie/triedb.rs +++ b/util/src/trie/triedb.rs @@ -174,7 +174,7 @@ impl<'db> TrieDB<'db> { try!(self.fmt_all(n, f, deepness + 1)); } Err(e) => { - write!(f, "ERROR: {}", e); + try!(write!(f, "ERROR: {}", e)); } } } From 1f6f20caad1f77a1e485b67228bd4e8a18c3b0df Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 26 Jul 2016 19:35:03 +0200 Subject: [PATCH 07/22] get ethcore compiling --- ethcore/src/account.rs | 14 ++++++- ethcore/src/header.rs | 2 + ethcore/src/spec/spec.rs | 2 + ethcore/src/state.rs | 73 ++++++++++++++++++++++++++---------- util/src/hashdb.rs | 2 +- util/src/journaldb/traits.rs | 2 +- util/src/lib.rs | 2 +- util/src/standard.rs | 1 - 8 files changed, 73 insertions(+), 25 deletions(-) diff --git a/ethcore/src/account.rs b/ethcore/src/account.rs index ff7bfe70d0f..413da61c230 100644 --- a/ethcore/src/account.rs +++ b/ethcore/src/account.rs @@ -20,6 +20,8 @@ use util::*; use pod_account::*; use account_db::*; +use std::cell::{Ref, RefCell}; + /// Single account in the system. #[derive(Clone)] pub struct Account { @@ -130,13 +132,23 @@ impl Account { /// Get (and cache) the contents of the trie's storage at `key`. pub fn storage_at(&self, db: &AccountDB, key: &H256) -> H256 { + use util::trie::TrieError; + self.storage_overlay.borrow_mut().entry(key.clone()).or_insert_with(||{ let db = SecTrieDB::new(db, &self.storage_root) .expect("Account storage_root initially set to zero (valid) and only altered by SecTrieDBMut. \ SecTrieDBMut would not set it to an invalid state root. Therefore the root is valid and DB creation \ using it will not fail."); - (Filth::Clean, H256::from(db.get(key).map_or(U256::zero(), |v| -> U256 {decode(v)}))) + let item: U256 = match db.get(key).map(decode) { + Ok(x) => x, + Err(TrieError::NotInTrie) => U256::zero(), + Err(e) => { + warn!("Potential DB corruption: {}", e); + U256::zero() + } + }; + (Filth::Clean, item.into()) }).1.clone() } diff --git a/ethcore/src/header.rs b/ethcore/src/header.rs index d5272ce2ebc..e6902814fb2 100644 --- a/ethcore/src/header.rs +++ b/ethcore/src/header.rs @@ -20,6 +20,8 @@ use util::*; use basic_types::*; use time::get_time; +use std::cell::RefCell; + /// Type for Block number pub type BlockNumber = u64; diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index e71d7d4321c..dc28c214e90 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -27,6 +27,8 @@ use ethereum; use basic_authority::BasicAuthority; use ethjson; +use std::cell::RefCell; + /// Parameters common to all engines. #[derive(Debug, PartialEq, Clone)] pub struct CommonParams { diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index effda5f0f24..11f45d54087 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +use std::cell::{RefCell, Ref, RefMut}; + use common::*; use engine::Engine; use executive::{Executive, TransactOptions}; @@ -71,7 +73,7 @@ impl State { /// Creates new state with existing state root pub fn from_existing(db: Box, root: H256, account_start_nonce: U256, trie_factory: TrieFactory) -> Result { if !db.as_hashdb().contains(&root) { - return Err(TrieError::InvalidStateRoot); + return Err(TrieError::InvalidStateRoot(root)); } let state = State { @@ -162,7 +164,14 @@ impl State { /// Determine whether an account exists. pub fn exists(&self, a: &Address) -> bool { let db = self.trie_factory.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); - self.cache.borrow().get(&a).unwrap_or(&None).is_some() || db.contains(&a) + self.cache.borrow().get(&a).unwrap_or(&None).is_some() + || match db.contains(a) { + Ok(x) => x, + Err(e) => { + warn!("Potential DB corruption encountered: {}", e); + false + } + } } /// Get the balance of account `a`. @@ -244,7 +253,9 @@ impl State { /// Commit accounts to SecTrieDBMut. This is similar to cpp-ethereum's dev::eth::commit. /// `accounts` is mutable because we may need to commit the code or storage and record that. #[cfg_attr(feature="dev", allow(match_ref_pats))] - pub fn commit_into(trie_factory: &TrieFactory, db: &mut HashDB, root: &mut H256, accounts: &mut HashMap>) { + pub fn commit_into(trie_factory: &TrieFactory, db: &mut HashDB, root: &mut H256, accounts: &mut HashMap>) + -> Result<(), Error> + { // first, commit the sub trees. // TODO: is this necessary or can we dispense with the `ref mut a` for just `a`? for (address, ref mut a) in accounts.iter_mut() { @@ -262,12 +273,14 @@ impl State { let mut trie = trie_factory.from_existing(db, root).unwrap(); for (address, ref a) in accounts.iter() { match **a { - Some(ref account) if account.is_dirty() => trie.insert(address, &account.rlp()), - None => trie.remove(address), + Some(ref account) if account.is_dirty() => try!(trie.insert(address, &account.rlp())), + None => try!(trie.remove(address)), _ => (), } } } + + Ok(()) } /// Commits our cached account changes into the trie. @@ -325,48 +338,68 @@ impl State { /// Pull account `a` in our cache from the trie DB and return it. /// `require_code` requires that the code be cached, too. - fn get<'a>(&'a self, a: &Address, require_code: bool) -> &'a Option { + fn get<'a>(&'a self, a: &Address, require_code: bool) -> Option> { let have_key = self.cache.borrow().contains_key(a); if !have_key { let db = self.trie_factory.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); - self.insert_cache(a, db.get(&a).map(Account::from_rlp)) + let maybe_acc = match db.get(&a).map(Account::from_rlp) { + Ok(acc) => Some(acc), + Err(TrieError::NotInTrie) => None, + Err(e) => { + warn!("Potential DB corruption encountered: {}", e); + None + } + }; + self.insert_cache(a, maybe_acc); } if require_code { if let Some(ref mut account) = self.cache.borrow_mut().get_mut(a).unwrap().as_mut() { account.cache_code(&AccountDB::new(self.db.as_hashdb(), a)); } } - unsafe { ::std::mem::transmute(self.cache.borrow().get(a).unwrap()) } + + + Some(Ref::map(self.cache.borrow(), |c| c.get(a).unwrap().as_ref().unwrap())) } /// Pull account `a` in our cache from the trie DB. `require_code` requires that the code be cached, too. - fn require<'a>(&'a self, a: &Address, require_code: bool) -> &'a mut Account { + fn require<'a>(&'a self, a: &Address, require_code: bool) -> RefMut<'a, Account> { self.require_or_from(a, require_code, || Account::new_basic(U256::from(0u8), self.account_start_nonce), |_|{}) } /// Pull account `a` in our cache from the trie DB. `require_code` requires that the code be cached, too. /// If it doesn't exist, make account equal the evaluation of `default`. - fn require_or_from<'a, F: FnOnce() -> Account, G: FnOnce(&mut Account)>(&self, a: &Address, require_code: bool, default: F, not_default: G) -> &'a mut Account { - let have_key = self.cache.borrow().contains_key(a); - if !have_key { + fn require_or_from<'a, F: FnOnce() -> Account, G: FnOnce(&mut Account)>(&'a self, a: &Address, require_code: bool, default: F, not_default: G) + -> RefMut<'a, Account> + { + if !{ self.cache.borrow().contains_key(a) } { let db = self.trie_factory.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); - self.insert_cache(a, db.get(&a).map(Account::from_rlp)) + let maybe_acc = match db.get(&a).map(Account::from_rlp) { + Ok(acc) => Some(acc), + Err(TrieError::NotInTrie) => None, + Err(e) => { + warn!("Potential DB corruption encountered: {}", e); + None + } + }; + + self.insert_cache(a, maybe_acc); } else { self.note_cache(a); } - let preexists = self.cache.borrow().get(a).unwrap().is_none(); - if preexists { - self.cache.borrow_mut().insert(a.clone(), Some(default())); - } else { - not_default(self.cache.borrow_mut().get_mut(a).unwrap().as_mut().unwrap()); + + match self.cache.borrow_mut().get_mut(a).unwrap() { + &mut Some(ref mut acc) => not_default(acc), + slot @ &mut None => *slot = Some(default()), } - unsafe { ::std::mem::transmute(self.cache.borrow_mut().get_mut(a).unwrap().as_mut().map(|account| { + RefMut::map(self.cache.borrow_mut(), |c| { + let account = c.get_mut(a).unwrap().as_mut().unwrap(); if require_code { account.cache_code(&AccountDB::new(self.db.as_hashdb(), a)); } account - }).unwrap()) } + }) } } diff --git a/util/src/hashdb.rs b/util/src/hashdb.rs index 51b9d445e11..52b126ac753 100644 --- a/util/src/hashdb.rs +++ b/util/src/hashdb.rs @@ -20,7 +20,7 @@ use bytes::*; use std::collections::HashMap; /// Trait modelling datastore keyed by a 32-byte Keccak hash. -pub trait HashDB: AsHashDB { +pub trait HashDB: AsHashDB + Send + Sync { /// Get the keys in the database together with number of underlying references. fn keys(&self) -> HashMap; diff --git a/util/src/journaldb/traits.rs b/util/src/journaldb/traits.rs index 1ed086ab46a..653ba3315d8 100644 --- a/util/src/journaldb/traits.rs +++ b/util/src/journaldb/traits.rs @@ -21,7 +21,7 @@ use hashdb::*; /// A `HashDB` which can manage a short-term journal potentially containing many forks of mutually /// exclusive actions. -pub trait JournalDB : HashDB { +pub trait JournalDB: HashDB { /// Return a copy of ourself, in a box. fn boxed_clone(&self) -> Box; diff --git a/util/src/lib.rs b/util/src/lib.rs index bcd9df97161..460e52a1ed7 100644 --- a/util/src/lib.rs +++ b/util/src/lib.rs @@ -167,7 +167,7 @@ pub use journaldb::JournalDB; pub use math::*; pub use crypto::*; pub use triehash::*; -pub use trie::*; +pub use trie::{Trie, TrieMut, TrieDB, TrieDBMut, TrieFactory, TrieError, SecTrieDB, SecTrieDBMut}; pub use nibbleslice::*; pub use semantic_version::*; pub use network::*; diff --git a/util/src/standard.rs b/util/src/standard.rs index ac41ef50d28..3d6c93e1ab0 100644 --- a/util/src/standard.rs +++ b/util/src/standard.rs @@ -37,7 +37,6 @@ pub use std::error::Error as StdError; pub use std::ops::*; pub use std::cmp::*; pub use std::sync::Arc; -pub use std::cell::*; pub use std::collections::*; pub use rustc_serialize::json::Json; From 2d2c54a5606c54e34ccaa527c3120809474749c5 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 26 Jul 2016 20:06:55 +0200 Subject: [PATCH 08/22] warn on trie errors in ethcore --- ethcore/src/account.rs | 10 ++-- ethcore/src/client/client.rs | 2 +- ethcore/src/client/error.rs | 13 ++++- ethcore/src/client/test_client.rs | 2 +- ethcore/src/error.rs | 5 +- ethcore/src/ethereum/ethash.rs | 4 +- ethcore/src/pod_account.rs | 4 +- ethcore/src/snapshot/account.rs | 6 +-- ethcore/src/snapshot/mod.rs | 2 +- ethcore/src/spec/spec.rs | 8 ++-- ethcore/src/state.rs | 6 +-- ethcore/src/tests/helpers.rs | 2 +- util/src/trie/mod.rs | 2 +- util/src/trie/sectriedb.rs | 4 +- util/src/trie/triedb.rs | 6 +-- util/src/trie/triedbmut.rs | 80 +++++++++++++++---------------- 16 files changed, 89 insertions(+), 67 deletions(-) diff --git a/ethcore/src/account.rs b/ethcore/src/account.rs index 413da61c230..8e2c5df6192 100644 --- a/ethcore/src/account.rs +++ b/ethcore/src/account.rs @@ -255,9 +255,13 @@ impl Account { if f == &Filth::Dirty { // cast key and value to trait type, // so we can call overloaded `to_bytes` method - match v.is_zero() { - true => { t.remove(k); }, - false => { t.insert(k, &encode(&U256::from(v.as_slice()))); }, + let res = match v.is_zero() { + true => t.remove(k), + false => t.insert(k, &encode(&U256::from(v.as_slice()))), + }; + + if let Err(e) = res { + warn!("Encountered potential DB corruption: {}", e); } *f = Filth::Clean; } diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index e804cde5799..ffcd2191dff 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -194,7 +194,7 @@ impl Client { state_db_config ); - if state_db.is_empty() && spec.ensure_db_good(state_db.as_hashdb_mut()) { + if state_db.is_empty() && try!(spec.ensure_db_good(state_db.as_hashdb_mut())) { state_db.commit(0, &spec.genesis_header().hash(), None).expect("Error commiting genesis state to state DB"); } diff --git a/ethcore/src/client/error.rs b/ethcore/src/client/error.rs index fde22cb10f3..dfb60d87ce4 100644 --- a/ethcore/src/client/error.rs +++ b/ethcore/src/client/error.rs @@ -1,11 +1,15 @@ use trace::Error as TraceError; use std::fmt::{Display, Formatter, Error as FmtError}; +use util::trie::TrieError; + /// Client configuration errors. #[derive(Debug)] pub enum Error { /// TraceDB configuration error. Trace(TraceError), + /// TrieDB-related error. + Trie(TrieError), } impl From for Error { @@ -14,10 +18,17 @@ impl From for Error { } } +impl From for Error { + fn from(err: TrieError) -> Self { + Error::Trie(err) + } +} + impl Display for Error { fn fmt(&self, f: &mut Formatter) -> Result<(), FmtError> { match *self { - Error::Trace(ref err) => write!(f, "{}", err) + Error::Trace(ref err) => write!(f, "{}", err), + Error::Trie(ref err) => write!(f, "{}", err), } } } diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index e0280b3881b..f1b1cbe2afb 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -261,7 +261,7 @@ impl MiningBlockChainClient for TestBlockChainClient { let genesis_header = self.spec.genesis_header(); let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - self.spec.ensure_db_good(db.as_hashdb_mut()); + self.spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let last_hashes = vec![genesis_header.hash()]; OpenBlock::new( diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index 26a64532273..bce4512b4c7 100644 --- a/ethcore/src/error.rs +++ b/ethcore/src/error.rs @@ -258,7 +258,10 @@ pub type ImportResult = Result; impl From for Error { fn from(err: ClientError) -> Error { - Error::Client(err) + match err { + ClientError::Trie(err) => Error::Trie(err), + _ => Error::Client(err) + } } } diff --git a/ethcore/src/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index 802b68e8298..00fbe134fb1 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -163,7 +163,9 @@ impl Engine for Ethash { for u in fields.uncles.iter() { fields.state.add_balance(u.author(), &(reward * U256::from(8 + u.number() - current_number) / U256::from(8))); } - fields.state.commit(); + if let Err(e) = fields.state.commit() { + warn!("Encountered error on state commit: {}", e); + } } fn verify_block_basic(&self, header: &Header, _block: Option<&[u8]>) -> result::Result<(), Error> { diff --git a/ethcore/src/pod_account.rs b/ethcore/src/pod_account.rs index d04833caba7..872232e9a29 100644 --- a/ethcore/src/pod_account.rs +++ b/ethcore/src/pod_account.rs @@ -70,7 +70,9 @@ impl PodAccount { let mut r = H256::new(); let mut t = SecTrieDBMut::new(db, &mut r); for (k, v) in &self.storage { - t.insert(k, &encode(&U256::from(v.as_slice()))); + if let Err(e) = t.insert(k, &encode(&U256::from(v.as_slice()))) { + warn!("Encountered potential DB corruption: {}", e); + } } } } diff --git a/ethcore/src/snapshot/account.rs b/ethcore/src/snapshot/account.rs index 2329b0e34b3..ef0e9194852 100644 --- a/ethcore/src/snapshot/account.rs +++ b/ethcore/src/snapshot/account.rs @@ -22,7 +22,7 @@ use error::Error; use util::{Bytes, HashDB, SHA3_EMPTY, TrieDB}; use util::hash::{FixedHash, H256}; use util::numbers::U256; -use util::rlp::{DecoderError, Rlp, RlpStream, Stream, UntrustedRlp, View}; +use util::rlp::{Rlp, RlpStream, Stream, UntrustedRlp, View}; // An alternate account structure from ::account::Account. #[derive(PartialEq, Clone, Debug)] @@ -102,7 +102,7 @@ impl Account { } // decode a fat rlp, and rebuild the storage trie as we go. - pub fn from_fat_rlp(acct_db: &mut AccountDBMut, rlp: UntrustedRlp) -> Result { + pub fn from_fat_rlp(acct_db: &mut AccountDBMut, rlp: UntrustedRlp) -> Result { use util::{TrieDBMut, TrieMut}; let nonce = try!(rlp.val_at(0)); @@ -123,7 +123,7 @@ impl Account { let k: Bytes = try!(pair_rlp.val_at(0)); let v: Bytes = try!(pair_rlp.val_at(1)); - storage_trie.insert(&k, &v); + try!(storage_trie.insert(&k, &v)); } } Ok(Account { diff --git a/ethcore/src/snapshot/mod.rs b/ethcore/src/snapshot/mod.rs index 6a9d4ba763a..16b32a23d24 100644 --- a/ethcore/src/snapshot/mod.rs +++ b/ethcore/src/snapshot/mod.rs @@ -383,7 +383,7 @@ impl StateRebuilder { }; for (hash, thin_rlp) in pairs { - account_trie.insert(&hash, &thin_rlp); + try!(account_trie.insert(&hash, &thin_rlp)); } } diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index dc28c214e90..9a1034e7556 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -220,21 +220,21 @@ impl Spec { } /// Ensure that the given state DB has the trie nodes in for the genesis state. - pub fn ensure_db_good(&self, db: &mut HashDB) -> bool { + pub fn ensure_db_good(&self, db: &mut HashDB) -> Result { if !db.contains(&self.state_root()) { let mut root = H256::new(); { let mut t = SecTrieDBMut::new(db, &mut root); for (address, account) in self.genesis_state.get().iter() { - t.insert(address.as_slice(), &account.rlp()); + try!(t.insert(address.as_slice(), &account.rlp())); } } for (address, account) in self.genesis_state.get().iter() { account.insert_additional(&mut AccountDBMut::new(db, address)); } assert!(db.contains(&self.state_root())); - true - } else { false } + Ok(true) + } else { Ok(false) } } /// Loads spec from json file. diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index 11f45d54087..d30803d4b9c 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -243,7 +243,7 @@ impl State { // TODO uncomment once to_pod() works correctly. // trace!("Applied transaction. Diff:\n{}\n", state_diff::diff_pod(&old, &self.to_pod())); - self.commit(); + try!(self.commit()); self.clear(); let receipt = Receipt::new(self.root().clone(), e.cumulative_gas_used, e.logs); // trace!("Transaction receipt: {:?}", receipt); @@ -284,9 +284,9 @@ impl State { } /// Commits our cached account changes into the trie. - pub fn commit(&mut self) { + pub fn commit(&mut self) -> Result<(), Error> { assert!(self.snapshots.borrow().is_empty()); - Self::commit_into(&self.trie_factory, self.db.as_hashdb_mut(), &mut self.root, self.cache.borrow_mut().deref_mut()); + Self::commit_into(&self.trie_factory, self.db.as_hashdb_mut(), &mut self.root, &mut *self.cache.borrow_mut()) } /// Clear state cache diff --git a/ethcore/src/tests/helpers.rs b/ethcore/src/tests/helpers.rs index 6e94d2ee365..fea9abc258d 100644 --- a/ethcore/src/tests/helpers.rs +++ b/ethcore/src/tests/helpers.rs @@ -137,7 +137,7 @@ pub fn generate_dummy_client_with_spec_and_data(get_test_spec: F, block_numbe let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - test_spec.ensure_db_good(db.as_hashdb_mut()); + test_spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let vm_factory = Default::default(); let genesis_header = test_spec.genesis_header(); diff --git a/util/src/trie/mod.rs b/util/src/trie/mod.rs index 3e1c7bd8569..97d31632ddd 100644 --- a/util/src/trie/mod.rs +++ b/util/src/trie/mod.rs @@ -50,7 +50,7 @@ pub use self::fatdbmut::FatDBMut; /// /// These borrow the data within them to avoid excessive copying on every /// trie operation. -#[derive(Debug)] +#[derive(Debug, PartialEq, Eq, Clone)] pub enum TrieError { /// Attempted to create a trie with a state root not in the DB. InvalidStateRoot(H256), diff --git a/util/src/trie/sectriedb.rs b/util/src/trie/sectriedb.rs index b756b815491..6bc0118be27 100644 --- a/util/src/trie/sectriedb.rs +++ b/util/src/trie/sectriedb.rs @@ -70,13 +70,13 @@ impl<'db> Trie for SecTrieDB<'db> { fn trie_to_sectrie() { use memorydb::MemoryDB; use super::triedbmut::TrieDBMut; - use super::trietraits::TrieMut; + use super::super::TrieMut; let mut memdb = MemoryDB::new(); let mut root = H256::default(); { let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&(&[0x01u8, 0x23]).sha3(), &[0x01u8, 0x23]); + t.insert(&(&[0x01u8, 0x23]).sha3(), &[0x01u8, 0x23]).unwrap(); } let t = SecTrieDB::new(&memdb, &root).unwrap(); assert_eq!(t.get(&[0x01u8, 0x23]).unwrap(), &[0x01u8, 0x23]); diff --git a/util/src/trie/triedb.rs b/util/src/trie/triedb.rs index 9a6c4a4205e..598d035a2cf 100644 --- a/util/src/trie/triedb.rs +++ b/util/src/trie/triedb.rs @@ -40,7 +40,7 @@ use super::{Trie, TrieItem, TrieError}; /// fn main() { /// let mut memdb = MemoryDB::new(); /// let mut root = H256::new(); -/// TrieDBMut::new(&mut memdb, &mut root).insert(b"foo", b"bar"); +/// TrieDBMut::new(&mut memdb, &mut root).insert(b"foo", b"bar").unwrap(); /// let t = TrieDB::new(&memdb, &root).unwrap(); /// assert!(t.contains(b"foo")); /// assert_eq!(t.get(b"foo").unwrap(), b"bar"); @@ -373,8 +373,8 @@ impl<'db> fmt::Debug for TrieDB<'db> { #[test] fn iterator() { - use super::trietraits::TrieMut; use memorydb::*; + use super::TrieMut; use super::triedbmut::*; let d = vec![ &b"A"[..], &b"AA"[..], &b"AB"[..], &b"B"[..] ]; @@ -384,7 +384,7 @@ fn iterator() { { let mut t = TrieDBMut::new(&mut memdb, &mut root); for x in &d { - t.insert(&x, &x); + t.insert(&x, &x).unwrap(); } } assert_eq!(d.iter().map(|i|i.to_vec()).collect::>(), TrieDB::new(&memdb, &root).unwrap().iter().map(|x|x.0).collect::>()); diff --git a/util/src/trie/triedbmut.rs b/util/src/trie/triedbmut.rs index 4372326b609..43bf02b127e 100644 --- a/util/src/trie/triedbmut.rs +++ b/util/src/trie/triedbmut.rs @@ -273,10 +273,10 @@ impl<'a> Index<&'a StorageHandle> for NodeStorage { /// let mut t = TrieDBMut::new(&mut memdb, &mut root); /// assert!(t.is_empty()); /// assert_eq!(*t.root(), SHA3_NULL_RLP); -/// t.insert(b"foo", b"bar"); +/// t.insert(b"foo", b"bar").unwrap(); /// assert!(t.contains(b"foo")); /// assert_eq!(t.get(b"foo").unwrap(), b"bar"); -/// t.remove(b"foo"); +/// t.remove(b"foo").unwrap(); /// assert!(!t.contains(b"foo")); /// } /// ``` @@ -943,7 +943,7 @@ mod tests { use super::*; use rlp::*; use bytes::ToPretty; - use super::super::trietraits::*; + use super::super::{TrieMut, TrieError}; use super::super::standardmap::*; fn populate_trie<'db>(db: &'db mut HashDB, root: &'db mut H256, v: &[(Vec, Vec)]) -> TrieDBMut<'db> { @@ -951,7 +951,7 @@ mod tests { for i in 0..v.len() { let key: &[u8]= &v[i].0; let val: &[u8] = &v[i].1; - t.insert(&key, &val); + t.insert(&key, &val).unwrap(); } t } @@ -959,7 +959,7 @@ mod tests { fn unpopulate_trie<'db>(t: &mut TrieDBMut<'db>, v: &[(Vec, Vec)]) { for i in v { let key: &[u8]= &i.0; - t.remove(&key); + t.remove(&key).unwrap(); } } @@ -1022,7 +1022,7 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); assert_eq!(*t.root(), trie_root(vec![ (vec![0x01u8, 0x23], vec![0x01u8, 0x23]) ])); } @@ -1033,15 +1033,15 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t1 = TrieDBMut::new(&mut memdb, &mut root); - t1.insert(&[0x01, 0x23], &big_value.to_vec()); - t1.insert(&[0x01, 0x34], &big_value.to_vec()); + t1.insert(&[0x01, 0x23], &big_value.to_vec()).unwrap(); + t1.insert(&[0x01, 0x34], &big_value.to_vec()).unwrap(); let mut memdb2 = MemoryDB::new(); let mut root2 = H256::new(); let mut t2 = TrieDBMut::new(&mut memdb2, &mut root2); - t2.insert(&[0x01], &big_value.to_vec()); - t2.insert(&[0x01, 0x23], &big_value.to_vec()); - t2.insert(&[0x01, 0x34], &big_value.to_vec()); - t2.remove(&[0x01]); + t2.insert(&[0x01], &big_value.to_vec()).unwrap(); + t2.insert(&[0x01, 0x23], &big_value.to_vec()).unwrap(); + t2.insert(&[0x01, 0x34], &big_value.to_vec()).unwrap(); + t2.remove(&[0x01]).unwrap(); } #[test] @@ -1049,8 +1049,8 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); - t.insert(&[0x01u8, 0x23], &[0x23u8, 0x45]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); + t.insert(&[0x01u8, 0x23], &[0x23u8, 0x45]).unwrap(); assert_eq!(*t.root(), trie_root(vec![ (vec![0x01u8, 0x23], vec![0x23u8, 0x45]) ])); } @@ -1059,8 +1059,8 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); - t.insert(&[0x11u8, 0x23], &[0x11u8, 0x23]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); + t.insert(&[0x11u8, 0x23], &[0x11u8, 0x23]).unwrap(); assert_eq!(*t.root(), trie_root(vec![ (vec![0x01u8, 0x23], vec![0x01u8, 0x23]), (vec![0x11u8, 0x23], vec![0x11u8, 0x23]) @@ -1072,9 +1072,9 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); - t.insert(&[0xf1u8, 0x23], &[0xf1u8, 0x23]); - t.insert(&[0x81u8, 0x23], &[0x81u8, 0x23]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); + t.insert(&[0xf1u8, 0x23], &[0xf1u8, 0x23]).unwrap(); + t.insert(&[0x81u8, 0x23], &[0x81u8, 0x23]).unwrap(); assert_eq!(*t.root(), trie_root(vec![ (vec![0x01u8, 0x23], vec![0x01u8, 0x23]), (vec![0x81u8, 0x23], vec![0x81u8, 0x23]), @@ -1087,8 +1087,8 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); - t.insert(&[], &[0x0]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); + t.insert(&[], &[0x0]).unwrap(); assert_eq!(*t.root(), trie_root(vec![ (vec![], vec![0x0]), (vec![0x01u8, 0x23], vec![0x01u8, 0x23]), @@ -1100,8 +1100,8 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); - t.insert(&[0x01u8, 0x34], &[0x01u8, 0x34]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); + t.insert(&[0x01u8, 0x34], &[0x01u8, 0x34]).unwrap(); assert_eq!(*t.root(), trie_root(vec![ (vec![0x01u8, 0x23], vec![0x01u8, 0x23]), (vec![0x01u8, 0x34], vec![0x01u8, 0x34]), @@ -1113,9 +1113,9 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01, 0x23, 0x45], &[0x01]); - t.insert(&[0x01, 0xf3, 0x45], &[0x02]); - t.insert(&[0x01, 0xf3, 0xf5], &[0x03]); + t.insert(&[0x01, 0x23, 0x45], &[0x01]).unwrap(); + t.insert(&[0x01, 0xf3, 0x45], &[0x02]).unwrap(); + t.insert(&[0x01, 0xf3, 0xf5], &[0x03]).unwrap(); assert_eq!(*t.root(), trie_root(vec![ (vec![0x01, 0x23, 0x45], vec![0x01]), (vec![0x01, 0xf3, 0x45], vec![0x02]), @@ -1131,8 +1131,8 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], big_value0); - t.insert(&[0x11u8, 0x23], big_value1); + t.insert(&[0x01u8, 0x23], big_value0).unwrap(); + t.insert(&[0x11u8, 0x23], big_value1).unwrap(); assert_eq!(*t.root(), trie_root(vec![ (vec![0x01u8, 0x23], big_value0.to_vec()), (vec![0x11u8, 0x23], big_value1.to_vec()) @@ -1146,8 +1146,8 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], big_value); - t.insert(&[0x11u8, 0x23], big_value); + t.insert(&[0x01u8, 0x23], big_value).unwrap(); + t.insert(&[0x11u8, 0x23], big_value).unwrap(); assert_eq!(*t.root(), trie_root(vec![ (vec![0x01u8, 0x23], big_value.to_vec()), (vec![0x11u8, 0x23], big_value.to_vec()) @@ -1159,7 +1159,7 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let t = TrieDBMut::new(&mut memdb, &mut root); - assert_eq!(t.get(&[0x5]), None); + assert_eq!(t.get(&[0x5]), Err(TrieError::NotInTrie)); } #[test] @@ -1167,7 +1167,7 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); assert_eq!(t.get(&[0x1, 0x23]).unwrap(), &[0x1u8, 0x23]); t.commit(); assert_eq!(t.get(&[0x1, 0x23]).unwrap(), &[0x1u8, 0x23]); @@ -1178,18 +1178,18 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); - t.insert(&[0xf1u8, 0x23], &[0xf1u8, 0x23]); - t.insert(&[0x81u8, 0x23], &[0x81u8, 0x23]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); + t.insert(&[0xf1u8, 0x23], &[0xf1u8, 0x23]).unwrap(); + t.insert(&[0x81u8, 0x23], &[0x81u8, 0x23]).unwrap(); assert_eq!(t.get(&[0x01, 0x23]).unwrap(), &[0x01u8, 0x23]); assert_eq!(t.get(&[0xf1, 0x23]).unwrap(), &[0xf1u8, 0x23]); assert_eq!(t.get(&[0x81, 0x23]).unwrap(), &[0x81u8, 0x23]); - assert_eq!(t.get(&[0x82, 0x23]), None); + assert_eq!(t.get(&[0x82, 0x23]), Err(TrieError::NotInTrie)); t.commit(); assert_eq!(t.get(&[0x01, 0x23]).unwrap(), &[0x01u8, 0x23]); assert_eq!(t.get(&[0xf1, 0x23]).unwrap(), &[0xf1u8, 0x23]); assert_eq!(t.get(&[0x81, 0x23]).unwrap(), &[0x81u8, 0x23]); - assert_eq!(t.get(&[0x82, 0x23]), None); + assert_eq!(t.get(&[0x82, 0x23]), Err(TrieError::NotInTrie)); } #[test] @@ -1236,7 +1236,7 @@ mod tests { let mut db = MemoryDB::new(); { let mut t = TrieDBMut::new(&mut db, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); } { @@ -1259,13 +1259,13 @@ mod tests { let mut root = H256::new(); let mut t = TrieDBMut::new(&mut db, &mut root); for &(ref key, ref value) in &x { - t.insert(key, value); + t.insert(key, value).unwrap(); } assert_eq!(*t.root(), trie_root(x.clone())); for &(ref key, _) in &x { - t.insert(key, &[]); + t.insert(key, &[]).unwrap(); } assert!(t.is_empty()); From 473cfd04b6611ab6f86d55865052241deb6457d7 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 26 Jul 2016 21:12:31 +0200 Subject: [PATCH 09/22] remove unsafety from node decoder --- util/src/trie/node.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/src/trie/node.rs b/util/src/trie/node.rs index b07410a806f..e91e243f7c8 100644 --- a/util/src/trie/node.rs +++ b/util/src/trie/node.rs @@ -48,7 +48,7 @@ impl<'a> Node<'a> { }, // branch - first 16 are nodes, 17th is a value (or empty). Prototype::List(17) => { - let mut nodes: [&'a [u8]; 16] = unsafe { ::std::mem::uninitialized() }; + let mut nodes: [&'a [u8]; 16] = [&[], &[], &[], &[], &[], &[], &[], &[], &[], &[], &[], &[], &[], &[], &[], &[]]; for i in 0..16 { nodes[i] = r.at(i).as_raw(); } From 99c22066272010284e083641ebbf41496d6c9684 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 27 Jul 2016 14:06:32 +0200 Subject: [PATCH 10/22] restore broken denote behavior for this branch --- util/src/memorydb.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/util/src/memorydb.rs b/util/src/memorydb.rs index 209af46fec3..cc5121bc39b 100644 --- a/util/src/memorydb.rs +++ b/util/src/memorydb.rs @@ -141,8 +141,14 @@ impl MemoryDB { /// a prior insert and thus has a negative reference with no value. /// /// May safely be called even if the key's value is known, in which case it will be a no-op. - pub fn denote(&self, _key: &H256, _value: Bytes) -> (&[u8], i32) { - unimplemented!() + pub fn denote(&self, key: &H256, value: Bytes) -> (&[u8], i32) { + if self.raw(key) == None { + unsafe { + let p = &self.data as *const HashMap as *mut HashMap; + (*p).insert(key.clone(), (value, 0)); + } + } + self.raw(key).unwrap() } /// Returns the size of allocated heap memory From c4b233ff940a138b2d0d428975bd8ec794a0719a Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 27 Jul 2016 14:24:34 +0200 Subject: [PATCH 11/22] fix overlayrecent fallout --- util/src/journaldb/overlayrecentdb.rs | 55 +++++++++++++-------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/util/src/journaldb/overlayrecentdb.rs b/util/src/journaldb/overlayrecentdb.rs index b573c3fe823..5fd28d4a6e4 100644 --- a/util/src/journaldb/overlayrecentdb.rs +++ b/util/src/journaldb/overlayrecentdb.rs @@ -257,43 +257,40 @@ impl JournalDB for OverlayRecentDB { journal_overlay.journal.entry(now).or_insert_with(Vec::new).push(JournalEntry { id: id.clone(), insertions: inserted_keys, deletions: removed_keys }); } - let journal_overlay = &mut *journal_overlay; + let journal_overlay = journal_overlay.deref_mut(); // apply old commits' details if let Some((end_era, canon_id)) = end { if let Some(ref mut records) = journal_overlay.journal.get_mut(&end_era) { + let mut canon_insertions: Vec<(H256, Bytes)> = Vec::new(); let mut canon_deletions: Vec = Vec::new(); let mut overlay_deletions: Vec = Vec::new(); let mut index = 0usize; - - { - let mut canon_insertions = Vec::new(); - for mut journal in records.drain(..) { - //delete the record from the db - let mut r = RlpStream::new_list(3); - r.append(&end_era); - r.append(&index); - r.append(&&PADDING[..]); - try!(batch.delete(&r.drain())); - trace!("commit: Delete journal for time #{}.{}: {}, (canon was {}): +{} -{} entries", end_era, index, journal.id, canon_id, journal.insertions.len(), journal.deletions.len()); - { - if canon_id == journal.id { - for h in &journal.insertions { - if let Some((d, rc)) = journal_overlay.backing_overlay.raw(h) { - if rc > 0 { - canon_insertions.push((h.clone(), d)); - } + for mut journal in records.drain(..) { + //delete the record from the db + let mut r = RlpStream::new_list(3); + r.append(&end_era); + r.append(&index); + r.append(&&PADDING[..]); + try!(batch.delete(&r.drain())); + trace!("commit: Delete journal for time #{}.{}: {}, (canon was {}): +{} -{} entries", end_era, index, journal.id, canon_id, journal.insertions.len(), journal.deletions.len()); + { + if canon_id == journal.id { + for h in &journal.insertions { + if let Some((d, rc)) = journal_overlay.backing_overlay.raw(&OverlayRecentDB::to_short_key(h)) { + if rc > 0 { + canon_insertions.push((h.clone(), d.to_owned())); //TODO: optimize this to avoid data copy } } - canon_deletions = journal.deletions; } - overlay_deletions.append(&mut journal.insertions); + canon_deletions = journal.deletions; } - index += 1; - } - // apply canon inserts first - for (k, v) in canon_insertions { - try!(batch.put(&k, v)); + overlay_deletions.append(&mut journal.insertions); } + index += 1; + } + // apply canon inserts first + for (k, v) in canon_insertions { + try!(batch.put(&k, &v)); } // update the overlay for k in overlay_deletions { @@ -337,11 +334,11 @@ impl HashDB for OverlayRecentDB { let v = self.journal_overlay.read().backing_overlay.get(&OverlayRecentDB::to_short_key(key)).map(|v| v.to_vec()); match v { Some(x) => { - Some(self.transaction_overlay.denote(key, x).0) + Some(&self.transaction_overlay.denote(key, x).0) } _ => { if let Some(x) = self.payload(key) { - Some(self.transaction_overlay.denote(key, x).0) + Some(&self.transaction_overlay.denote(key, x).0) } else { None @@ -905,4 +902,4 @@ mod tests { assert!(jdb.contains(&foo)); assert!(jdb.contains(&bar)); } -} +} \ No newline at end of file From d8d60431030496b6a60979df38151d0aed3b2088 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 27 Jul 2016 14:26:35 +0200 Subject: [PATCH 12/22] fix triedb tests --- util/src/trie/fatdb.rs | 2 +- util/src/trie/fatdbmut.rs | 2 +- util/src/trie/sectriedbmut.rs | 2 +- util/src/trie/triedb.rs | 4 ++-- util/src/trie/triedbmut.rs | 8 ++++---- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/util/src/trie/fatdb.rs b/util/src/trie/fatdb.rs index 6a1e079013f..5c042a0d653 100644 --- a/util/src/trie/fatdb.rs +++ b/util/src/trie/fatdb.rs @@ -106,7 +106,7 @@ fn fatdb_to_trie() { let mut root = H256::default(); { let mut t = FatDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); } let t = FatDB::new(&memdb, &root).unwrap(); assert_eq!(t.get(&[0x01u8, 0x23]).unwrap(), &[0x01u8, 0x23]); diff --git a/util/src/trie/fatdbmut.rs b/util/src/trie/fatdbmut.rs index b933b442f09..1c802ea4342 100644 --- a/util/src/trie/fatdbmut.rs +++ b/util/src/trie/fatdbmut.rs @@ -95,7 +95,7 @@ fn fatdb_to_trie() { let mut root = H256::default(); { let mut t = FatDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); } let t = TrieDB::new(&memdb, &root).unwrap(); assert_eq!(t.get(&(&[0x01u8, 0x23]).sha3()).unwrap(), &[0x01u8, 0x23]); diff --git a/util/src/trie/sectriedbmut.rs b/util/src/trie/sectriedbmut.rs index 74205afbe35..e9db4d6c62f 100644 --- a/util/src/trie/sectriedbmut.rs +++ b/util/src/trie/sectriedbmut.rs @@ -87,7 +87,7 @@ fn sectrie_to_trie() { let mut root = H256::default(); { let mut t = SecTrieDBMut::new(&mut memdb, &mut root); - t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); + t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); } let t = TrieDB::new(&memdb, &root).unwrap(); assert_eq!(t.get(&(&[0x01u8, 0x23]).sha3()).unwrap(), &[0x01u8, 0x23]); diff --git a/util/src/trie/triedb.rs b/util/src/trie/triedb.rs index eb4a2acc0e5..ce593651f02 100644 --- a/util/src/trie/triedb.rs +++ b/util/src/trie/triedb.rs @@ -42,9 +42,9 @@ use super::{Trie, TrieItem, TrieError}; /// let mut root = H256::new(); /// TrieDBMut::new(&mut memdb, &mut root).insert(b"foo", b"bar").unwrap(); /// let t = TrieDB::new(&memdb, &root).unwrap(); -/// assert!(t.contains(b"foo")); +/// assert!(t.contains(b"foo").unwrap()); /// assert_eq!(t.get(b"foo").unwrap(), b"bar"); -/// assert!(t.db_items_remaining().is_empty()); +/// assert!(t.db_items_remaining().unwrap().is_empty()); /// } /// ``` pub struct TrieDB<'db> { diff --git a/util/src/trie/triedbmut.rs b/util/src/trie/triedbmut.rs index a9729f4a37c..fcd15ae0f12 100644 --- a/util/src/trie/triedbmut.rs +++ b/util/src/trie/triedbmut.rs @@ -274,10 +274,10 @@ impl<'a> Index<&'a StorageHandle> for NodeStorage { /// assert!(t.is_empty()); /// assert_eq!(*t.root(), SHA3_NULL_RLP); /// t.insert(b"foo", b"bar").unwrap(); -/// assert!(t.contains(b"foo")); +/// assert!(t.contains(b"foo").unwrap()); /// assert_eq!(t.get(b"foo").unwrap(), b"bar"); /// t.remove(b"foo").unwrap(); -/// assert!(!t.contains(b"foo")); +/// assert!(!t.contains(b"foo").unwrap()); /// } /// ``` pub struct TrieDBMut<'a> { @@ -951,7 +951,7 @@ mod tests { for i in 0..v.len() { let key: &[u8]= &v[i].0; let val: &[u8] = &v[i].1; - t.insert(key, val); + t.insert(key, val).unwrap(); } t } @@ -959,7 +959,7 @@ mod tests { fn unpopulate_trie<'db>(t: &mut TrieDBMut<'db>, v: &[(Vec, Vec)]) { for i in v { let key: &[u8]= &i.0; - t.remove(key); + t.remove(key).unwrap(); } } From 888f9eb901b1973bcf5dd5ea0a38c35e142a6d7f Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 27 Jul 2016 14:53:37 +0200 Subject: [PATCH 13/22] remove unwrap in state --- ethcore/src/state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index cdb3388a0d4..d364fa42874 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -359,7 +359,7 @@ impl State { } - Some(Ref::map(self.cache.borrow(), |c| c.get(a).unwrap().as_ref().unwrap())) + Some(Ref::map(self.cache.borrow(), |c| c.get(a).unwrap().as_ref())) } /// Pull account `a` in our cache from the trie DB. `require_code` requires that the code be cached, too. From 760e6a4cfb8c014535fdc1c6c2bd72552638b840 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sat, 30 Jul 2016 19:04:40 +0200 Subject: [PATCH 14/22] alter Trie::get to return Result> --- ethcore/src/state.rs | 2 -- util/src/trie/fatdb.rs | 4 +-- util/src/trie/fatdbmut.rs | 4 +-- util/src/trie/mod.rs | 24 ++++--------- util/src/trie/sectriedb.rs | 4 +-- util/src/trie/sectriedbmut.rs | 4 +-- util/src/trie/triedb.rs | 20 +++++------ util/src/trie/triedbmut.rs | 64 ++++++++++++++++++----------------- 8 files changed, 57 insertions(+), 69 deletions(-) diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index 5068c81e265..c33174dd1d1 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -344,7 +344,6 @@ impl State { let db = self.trie_factory.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); let maybe_acc = match db.get(&a).map(Account::from_rlp) { Ok(acc) => Some(acc), - Err(TrieError::NotInTrie) => None, Err(e) => { warn!("Potential DB corruption encountered: {}", e); None @@ -376,7 +375,6 @@ impl State { let db = self.trie_factory.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); let maybe_acc = match db.get(&a).map(Account::from_rlp) { Ok(acc) => Some(acc), - Err(TrieError::NotInTrie) => None, Err(e) => { warn!("Potential DB corruption encountered: {}", e); None diff --git a/util/src/trie/fatdb.rs b/util/src/trie/fatdb.rs index 5c042a0d653..2446626709f 100644 --- a/util/src/trie/fatdb.rs +++ b/util/src/trie/fatdb.rs @@ -63,7 +63,7 @@ impl<'db> Trie for FatDB<'db> { self.raw.contains(&key.sha3()) } - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result<&'a [u8]> + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result> where 'a: 'key { self.raw.get(&key.sha3()) @@ -109,6 +109,6 @@ fn fatdb_to_trie() { t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); } let t = FatDB::new(&memdb, &root).unwrap(); - assert_eq!(t.get(&[0x01u8, 0x23]).unwrap(), &[0x01u8, 0x23]); + assert_eq!(t.get(&[0x01u8, 0x23]).unwrap().unwrap(), &[0x01u8, 0x23]); assert_eq!(t.iter().collect::>(), vec![(vec![0x01u8, 0x23], &[0x01u8, 0x23] as &[u8])]); } diff --git a/util/src/trie/fatdbmut.rs b/util/src/trie/fatdbmut.rs index 1c802ea4342..3298541b7a8 100644 --- a/util/src/trie/fatdbmut.rs +++ b/util/src/trie/fatdbmut.rs @@ -66,7 +66,7 @@ impl<'db> TrieMut for FatDBMut<'db> { self.raw.contains(&key.sha3()) } - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result<&'a [u8]> + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result> where 'a: 'key { self.raw.get(&key.sha3()) @@ -98,5 +98,5 @@ fn fatdb_to_trie() { t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); } let t = TrieDB::new(&memdb, &root).unwrap(); - assert_eq!(t.get(&(&[0x01u8, 0x23]).sha3()).unwrap(), &[0x01u8, 0x23]); + assert_eq!(t.get(&(&[0x01u8, 0x23]).sha3()).unwrap().unwrap(), &[0x01u8, 0x23]); } diff --git a/util/src/trie/mod.rs b/util/src/trie/mod.rs index 7d77a4e8ba7..6500059b92a 100644 --- a/util/src/trie/mod.rs +++ b/util/src/trie/mod.rs @@ -56,8 +56,6 @@ pub enum TrieError { InvalidStateRoot(H256), /// Trie item not found in the database, IncompleteDatabase(H256), - /// Key not in trie. - NotInTrie, } impl fmt::Display for TrieError { @@ -66,8 +64,6 @@ impl fmt::Display for TrieError { TrieError::InvalidStateRoot(ref root) => write!(f, "Invalid state root: {}", root), TrieError::IncompleteDatabase(ref missing) => write!(f, "Database missing expected key: {}", missing), - TrieError::NotInTrie => - write!(f, "Failed query"), } } } @@ -75,8 +71,8 @@ impl fmt::Display for TrieError { /// Trie-Item type. pub type TrieItem<'a> = (Vec, &'a [u8]); -/// Trie result type. -pub type Result = ::std::result::Result; +/// Trie result type. Boxed to avoid copying around extra space for `H256`s on successful queries. +pub type Result = ::std::result::Result>; /// A key-value datastore implemented as a database-backed modified Merkle tree. pub trait Trie { @@ -88,15 +84,11 @@ pub trait Trie { /// Does the trie contain a given key? fn contains(&self, key: &[u8]) -> Result { - match self.get(key) { - Ok(_) => Ok(true), - Err(TrieError::NotInTrie) => Ok(false), - Err(e) => Err(e), - } + self.get(key).map(|x| x.is_some()) } /// What is the value of the given key in this trie? - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Result<&'a [u8]> where 'a: 'key; + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Result> where 'a: 'key; /// Returns an iterator over elements of trie. fn iter<'a>(&'a self) -> Box + 'a>; @@ -112,15 +104,11 @@ pub trait TrieMut { /// Does the trie contain a given key? fn contains(&self, key: &[u8]) -> Result { - match self.get(key) { - Ok(_) => Ok(true), - Err(TrieError::NotInTrie) => Ok(false), - Err(e) => Err(e) - } + self.get(key).map(|x| x.is_some()) } /// What is the value of the given key in this trie? - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Result<&'a [u8]> where 'a: 'key; + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> Result> where 'a: 'key; /// Insert a `key`/`value` pair into the trie. An `empty` value is equivalent to removing /// `key` from the trie. diff --git a/util/src/trie/sectriedb.rs b/util/src/trie/sectriedb.rs index 6bc0118be27..7869439a734 100644 --- a/util/src/trie/sectriedb.rs +++ b/util/src/trie/sectriedb.rs @@ -59,7 +59,7 @@ impl<'db> Trie for SecTrieDB<'db> { self.raw.contains(&key.sha3()) } - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result<&'a [u8]> + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result> where 'a: 'key { self.raw.get(&key.sha3()) @@ -79,5 +79,5 @@ fn trie_to_sectrie() { t.insert(&(&[0x01u8, 0x23]).sha3(), &[0x01u8, 0x23]).unwrap(); } let t = SecTrieDB::new(&memdb, &root).unwrap(); - assert_eq!(t.get(&[0x01u8, 0x23]).unwrap(), &[0x01u8, 0x23]); + assert_eq!(t.get(&[0x01u8, 0x23]).unwrap().unwrap(), &[0x01u8, 0x23]); } diff --git a/util/src/trie/sectriedbmut.rs b/util/src/trie/sectriedbmut.rs index e9db4d6c62f..d6980d48b75 100644 --- a/util/src/trie/sectriedbmut.rs +++ b/util/src/trie/sectriedbmut.rs @@ -62,7 +62,7 @@ impl<'db> TrieMut for SecTrieDBMut<'db> { self.raw.contains(&key.sha3()) } - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result<&'a [u8]> + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result> where 'a: 'key { self.raw.get(&key.sha3()) @@ -90,5 +90,5 @@ fn sectrie_to_trie() { t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); } let t = TrieDB::new(&memdb, &root).unwrap(); - assert_eq!(t.get(&(&[0x01u8, 0x23]).sha3()).unwrap(), &[0x01u8, 0x23]); + assert_eq!(t.get(&(&[0x01u8, 0x23]).sha3()).unwrap().unwrap(), &[0x01u8, 0x23]); } diff --git a/util/src/trie/triedb.rs b/util/src/trie/triedb.rs index ce593651f02..f7704c387b7 100644 --- a/util/src/trie/triedb.rs +++ b/util/src/trie/triedb.rs @@ -43,7 +43,7 @@ use super::{Trie, TrieItem, TrieError}; /// TrieDBMut::new(&mut memdb, &mut root).insert(b"foo", b"bar").unwrap(); /// let t = TrieDB::new(&memdb, &root).unwrap(); /// assert!(t.contains(b"foo").unwrap()); -/// assert_eq!(t.get(b"foo").unwrap(), b"bar"); +/// assert_eq!(t.get(b"foo").unwrap().unwrap(), b"bar"); /// assert!(t.db_items_remaining().unwrap().is_empty()); /// } /// ``` @@ -60,7 +60,7 @@ impl<'db> TrieDB<'db> { /// Returns an error if `root` does not exist pub fn new(db: &'db HashDB, root: &'db H256) -> super::Result { if !db.contains(root) { - Err(TrieError::InvalidStateRoot(*root)) + Err(Box::new(TrieError::InvalidStateRoot(*root))) } else { Ok(TrieDB { db: db, @@ -133,7 +133,7 @@ impl<'db> TrieDB<'db> { /// Get the data of the root node. fn root_data(&self) -> super::Result<&[u8]> { - self.db.get(self.root).ok_or(TrieError::InvalidStateRoot(*self.root)) + self.db.get(self.root).ok_or_else(|| Box::new(TrieError::InvalidStateRoot(*self.root))) } /// Get the root node as a `Node`. @@ -188,7 +188,7 @@ impl<'db> TrieDB<'db> { } /// Return optional data for a key given as a `NibbleSlice`. Returns `None` if no data exists. - fn do_lookup<'key>(&'db self, key: &NibbleSlice<'key>) -> super::Result<&'db [u8]> + fn do_lookup<'key>(&'db self, key: &NibbleSlice<'key>) -> super::Result> where 'db: 'key { let root_rlp = try!(self.root_data()); @@ -199,20 +199,20 @@ impl<'db> TrieDB<'db> { /// value exists for the key. /// /// Note: Not a public API; use Trie trait functions. - fn get_from_node<'key>(&'db self, node: &'db [u8], key: &NibbleSlice<'key>) -> super::Result<&'db [u8]> + fn get_from_node<'key>(&'db self, node: &'db [u8], key: &NibbleSlice<'key>) -> super::Result> where 'db: 'key { match Node::decoded(node) { - Node::Leaf(ref slice, ref value) if key == slice => Ok(value), + Node::Leaf(ref slice, ref value) if key == slice => Ok(Some(value)), Node::Extension(ref slice, ref item) if key.starts_with(slice) => { let data = try!(self.get_raw_or_lookup(item)); self.get_from_node(data, &key.mid(slice.len())) }, Node::Branch(ref nodes, value) => match key.is_empty() { - true => value.ok_or(TrieError::NotInTrie), + true => Ok(value), false => self.get_from_node(try!(self.get_raw_or_lookup(nodes[key.at(0) as usize])), &key.mid(1)) }, - _ => Err(TrieError::NotInTrie) + _ => Ok(None) } } @@ -225,7 +225,7 @@ impl<'db> TrieDB<'db> { match r.is_data() && r.size() == 32 { true => { let key = r.as_val::(); - self.db.get(&key).ok_or(TrieError::IncompleteDatabase(key)) + self.db.get(&key).ok_or_else(|| Box::new(TrieError::IncompleteDatabase(key))) } false => Ok(node) } @@ -355,7 +355,7 @@ impl<'db> Trie for TrieDB<'db> { fn root(&self) -> &H256 { self.root } - fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result<&'a [u8]> + fn get<'a, 'key>(&'a self, key: &'key [u8]) -> super::Result> where 'a: 'key { self.do_lookup(&NibbleSlice::new(key)) diff --git a/util/src/trie/triedbmut.rs b/util/src/trie/triedbmut.rs index fcd15ae0f12..d560a0ecf37 100644 --- a/util/src/trie/triedbmut.rs +++ b/util/src/trie/triedbmut.rs @@ -275,7 +275,7 @@ impl<'a> Index<&'a StorageHandle> for NodeStorage { /// assert_eq!(*t.root(), SHA3_NULL_RLP); /// t.insert(b"foo", b"bar").unwrap(); /// assert!(t.contains(b"foo").unwrap()); -/// assert_eq!(t.get(b"foo").unwrap(), b"bar"); +/// assert_eq!(t.get(b"foo").unwrap().unwrap(), b"bar"); /// t.remove(b"foo").unwrap(); /// assert!(!t.contains(b"foo").unwrap()); /// } @@ -311,7 +311,7 @@ impl<'a> TrieDBMut<'a> { /// Returns an error if `root` does not exist. pub fn from_existing(db: &'a mut HashDB, root: &'a mut H256) -> super::Result { if !db.contains(root) { - return Err(TrieError::InvalidStateRoot(*root)); + return Err(Box::new(TrieError::InvalidStateRoot(*root))); } let root_handle = NodeHandle::Hash(*root); @@ -336,7 +336,7 @@ impl<'a> TrieDBMut<'a> { // cache a node by hash fn cache(&mut self, hash: H256) -> super::Result { - let node_rlp = try!(self.db.get(&hash).ok_or(TrieError::IncompleteDatabase(hash))); + let node_rlp = try!(self.db.get(&hash).ok_or_else(|| Box::new(TrieError::IncompleteDatabase(hash)))); let node = Node::from_rlp(node_rlp, &*self.db, &mut self.storage); Ok(self.storage.alloc(Stored::Cached(node, hash))) } @@ -366,18 +366,18 @@ impl<'a> TrieDBMut<'a> { } // walk the trie, attempting to find the key's node. - fn lookup<'x, 'key>(&'x self, partial: NibbleSlice<'key>, handle: &NodeHandle) -> super::Result<&'x [u8]> + fn lookup<'x, 'key>(&'x self, partial: NibbleSlice<'key>, handle: &NodeHandle) -> super::Result> where 'x: 'key { match *handle { NodeHandle::Hash(ref hash) => self.do_db_lookup(hash, partial), NodeHandle::InMemory(ref handle) => match self.storage[handle] { - Node::Empty => Err(TrieError::NotInTrie), + Node::Empty => Ok(None), Node::Leaf(ref key, ref value) => { if NibbleSlice::from_encoded(key).0 == partial { - Ok(value) + Ok(Some(value)) } else { - Err(TrieError::NotInTrie) + Ok(None) } } Node::Extension(ref slice, ref child) => { @@ -385,16 +385,18 @@ impl<'a> TrieDBMut<'a> { if partial.starts_with(&slice) { self.lookup(partial.mid(slice.len()), child) } else { - Err(TrieError::NotInTrie) + Ok(None) } } Node::Branch(ref children, ref value) => { if partial.is_empty() { - value.as_ref().map(|v| &v[..]).ok_or(TrieError::NotInTrie) + Ok(value.as_ref().map(|v| &v[..])) } else { let idx = partial.at(0); - (&children[idx as usize]).as_ref().ok_or(TrieError::NotInTrie) - .and_then(|child| self.lookup(partial.mid(1), child)) + match children[idx as usize].as_ref() { + Some(child) => self.lookup(partial.mid(1), child), + None => Ok(None), + } } } } @@ -402,10 +404,10 @@ impl<'a> TrieDBMut<'a> { } /// Return optional data for a key given as a `NibbleSlice`. Returns `None` if no data exists. - fn do_db_lookup<'x, 'key>(&'x self, hash: &H256, key: NibbleSlice<'key>) -> super::Result<&'x [u8]> + fn do_db_lookup<'x, 'key>(&'x self, hash: &H256, key: NibbleSlice<'key>) -> super::Result> where 'x: 'key { - self.db.get(hash).ok_or(TrieError::IncompleteDatabase(*hash)) + self.db.get(hash).ok_or_else(|| Box::new(TrieError::IncompleteDatabase(*hash))) .and_then(|node_rlp| self.get_from_db_node(node_rlp, key)) } @@ -413,19 +415,19 @@ impl<'a> TrieDBMut<'a> { /// value exists for the key. /// /// Note: Not a public API; use Trie trait functions. - fn get_from_db_node<'x, 'key>(&'x self, node: &'x [u8], key: NibbleSlice<'key>) -> super::Result<&'x [u8]> + fn get_from_db_node<'x, 'key>(&'x self, node: &'x [u8], key: NibbleSlice<'key>) -> super::Result> where 'x: 'key { match RlpNode::decoded(node) { - RlpNode::Leaf(ref slice, ref value) if &key == slice => Ok(value), + RlpNode::Leaf(ref slice, ref value) if &key == slice => Ok(Some(value)), RlpNode::Extension(ref slice, ref item) if key.starts_with(slice) => { self.get_from_db_node(try!(self.get_raw_or_lookup(item)), key.mid(slice.len())) }, RlpNode::Branch(ref nodes, value) => match key.is_empty() { - true => value.ok_or(TrieError::NotInTrie), + true => Ok(value), false => self.get_from_db_node(try!(self.get_raw_or_lookup(nodes[key.at(0) as usize])), key.mid(1)) }, - _ => Err(TrieError::NotInTrie), + _ => Ok(None), } } @@ -438,7 +440,7 @@ impl<'a> TrieDBMut<'a> { match r.is_data() && r.size() == 32 { true => { let key = r.as_val::(); - self.db.get(&key).ok_or(TrieError::IncompleteDatabase(key)) + self.db.get(&key).ok_or_else(|| Box::new(TrieError::IncompleteDatabase(key))) } false => Ok(node) } @@ -894,7 +896,7 @@ impl<'a> TrieMut for TrieDBMut<'a> { } } - fn get<'x, 'key>(&'x self, key: &'key [u8]) -> super::Result<&'x [u8]> where 'x: 'key { + fn get<'x, 'key>(&'x self, key: &'key [u8]) -> super::Result> where 'x: 'key { self.lookup(NibbleSlice::new(key), &self.root_handle) } @@ -943,7 +945,7 @@ mod tests { use super::*; use rlp::*; use bytes::ToPretty; - use super::super::{TrieMut, TrieError}; + use super::super::TrieMut; use super::super::standardmap::*; fn populate_trie<'db>(db: &'db mut HashDB, root: &'db mut H256, v: &[(Vec, Vec)]) -> TrieDBMut<'db> { @@ -1159,7 +1161,7 @@ mod tests { let mut memdb = MemoryDB::new(); let mut root = H256::new(); let t = TrieDBMut::new(&mut memdb, &mut root); - assert_eq!(t.get(&[0x5]), Err(TrieError::NotInTrie)); + assert_eq!(t.get(&[0x5]), Ok(None)); } #[test] @@ -1168,9 +1170,9 @@ mod tests { let mut root = H256::new(); let mut t = TrieDBMut::new(&mut memdb, &mut root); t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); - assert_eq!(t.get(&[0x1, 0x23]).unwrap(), &[0x1u8, 0x23]); + assert_eq!(t.get(&[0x1, 0x23]).unwrap().unwrap(), &[0x1u8, 0x23]); t.commit(); - assert_eq!(t.get(&[0x1, 0x23]).unwrap(), &[0x1u8, 0x23]); + assert_eq!(t.get(&[0x1, 0x23]).unwrap().unwrap(), &[0x1u8, 0x23]); } #[test] @@ -1181,15 +1183,15 @@ mod tests { t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]).unwrap(); t.insert(&[0xf1u8, 0x23], &[0xf1u8, 0x23]).unwrap(); t.insert(&[0x81u8, 0x23], &[0x81u8, 0x23]).unwrap(); - assert_eq!(t.get(&[0x01, 0x23]).unwrap(), &[0x01u8, 0x23]); - assert_eq!(t.get(&[0xf1, 0x23]).unwrap(), &[0xf1u8, 0x23]); - assert_eq!(t.get(&[0x81, 0x23]).unwrap(), &[0x81u8, 0x23]); - assert_eq!(t.get(&[0x82, 0x23]), Err(TrieError::NotInTrie)); + assert_eq!(t.get(&[0x01, 0x23]).unwrap().unwrap(), &[0x01u8, 0x23]); + assert_eq!(t.get(&[0xf1, 0x23]).unwrap().unwrap(), &[0xf1u8, 0x23]); + assert_eq!(t.get(&[0x81, 0x23]).unwrap().unwrap(), &[0x81u8, 0x23]); + assert_eq!(t.get(&[0x82, 0x23]), Ok(None)); t.commit(); - assert_eq!(t.get(&[0x01, 0x23]).unwrap(), &[0x01u8, 0x23]); - assert_eq!(t.get(&[0xf1, 0x23]).unwrap(), &[0xf1u8, 0x23]); - assert_eq!(t.get(&[0x81, 0x23]).unwrap(), &[0x81u8, 0x23]); - assert_eq!(t.get(&[0x82, 0x23]), Err(TrieError::NotInTrie)); + assert_eq!(t.get(&[0x01, 0x23]).unwrap().unwrap(), &[0x01u8, 0x23]); + assert_eq!(t.get(&[0xf1, 0x23]).unwrap().unwrap(), &[0xf1u8, 0x23]); + assert_eq!(t.get(&[0x81, 0x23]).unwrap().unwrap(), &[0x81u8, 0x23]); + assert_eq!(t.get(&[0x82, 0x23]), Ok(None)); } #[test] From a01b0fec6ace700c6c7f912d478ff956c8139357 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sat, 30 Jul 2016 19:29:11 +0200 Subject: [PATCH 15/22] fix refcell error in require --- ethcore/src/state.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index 158a71a9b2b..4a58c4a8d8e 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -374,7 +374,8 @@ impl State { fn require_or_from<'a, F: FnOnce() -> Account, G: FnOnce(&mut Account)>(&'a self, a: &Address, require_code: bool, default: F, not_default: G) -> RefMut<'a, Account> { - if !{ self.cache.borrow().contains_key(a) } { + let contains_key = self.cache.borrow().contains_key(a); + if !contains_key { let db = self.trie_factory.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); let maybe_acc = match db.get(&a) { Ok(acc) => acc.map(Account::from_rlp), From b560a2e86de606ce61dd104ef9f175e7103715fa Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sat, 30 Jul 2016 19:37:21 +0200 Subject: [PATCH 16/22] fix test warnings --- ethcore/src/block.rs | 10 ++++----- ethcore/src/engines/basic_authority.rs | 2 +- ethcore/src/ethereum/ethash.rs | 4 ++-- ethcore/src/ethereum/mod.rs | 2 +- ethcore/src/snapshot/account.rs | 2 +- ethcore/src/state.rs | 30 +++++++++++++------------- 6 files changed, 25 insertions(+), 25 deletions(-) diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index db0bac6144d..98927e59f1b 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -586,7 +586,7 @@ mod tests { let genesis_header = spec.genesis_header(); let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - spec.ensure_db_good(db.as_hashdb_mut()); + spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); let b = OpenBlock::new(engine.deref(), &vm_factory, Default::default(), false, db, &genesis_header, last_hashes, Address::zero(), (3141562.into(), 31415620.into()), vec![]).unwrap(); @@ -603,7 +603,7 @@ mod tests { let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - spec.ensure_db_good(db.as_hashdb_mut()); + spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let vm_factory = Default::default(); let b = OpenBlock::new(engine.deref(), &vm_factory, Default::default(), false, db, &genesis_header, vec![genesis_header.hash()], Address::zero(), (3141562.into(), 31415620.into()), vec![]).unwrap() .close_and_lock().seal(engine.deref(), vec![]).unwrap(); @@ -612,7 +612,7 @@ mod tests { let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - spec.ensure_db_good(db.as_hashdb_mut()); + spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let e = enact_and_seal(&orig_bytes, engine.deref(), false, db, &genesis_header, vec![genesis_header.hash()], &Default::default(), Default::default()).unwrap(); assert_eq!(e.rlp_bytes(), orig_bytes); @@ -631,7 +631,7 @@ mod tests { let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - spec.ensure_db_good(db.as_hashdb_mut()); + spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let vm_factory = Default::default(); let mut open_block = OpenBlock::new(engine.deref(), &vm_factory, Default::default(), false, db, &genesis_header, vec![genesis_header.hash()], Address::zero(), (3141562.into(), 31415620.into()), vec![]).unwrap(); let mut uncle1_header = Header::new(); @@ -647,7 +647,7 @@ mod tests { let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - spec.ensure_db_good(db.as_hashdb_mut()); + spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let e = enact_and_seal(&orig_bytes, engine.deref(), false, db, &genesis_header, vec![genesis_header.hash()], &Default::default(), Default::default()).unwrap(); let bytes = e.rlp_bytes(); diff --git a/ethcore/src/engines/basic_authority.rs b/ethcore/src/engines/basic_authority.rs index 525b825b950..b7c63cfa3b2 100644 --- a/ethcore/src/engines/basic_authority.rs +++ b/ethcore/src/engines/basic_authority.rs @@ -250,7 +250,7 @@ mod tests { let genesis_header = spec.genesis_header(); let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - spec.ensure_db_good(db.as_hashdb_mut()); + spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); let b = OpenBlock::new(engine.deref(), &vm_factory, Default::default(), false, db, &genesis_header, last_hashes, addr, (3141562.into(), 31415620.into()), vec![]).unwrap(); diff --git a/ethcore/src/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index 63baf9aa40b..7b9a5234025 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -354,7 +354,7 @@ mod tests { let genesis_header = spec.genesis_header(); let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - spec.ensure_db_good(db.as_hashdb_mut()); + spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); let b = OpenBlock::new(engine.deref(), &vm_factory, Default::default(), false, db, &genesis_header, last_hashes, Address::zero(), (3141562.into(), 31415620.into()), vec![]).unwrap(); @@ -369,7 +369,7 @@ mod tests { let genesis_header = spec.genesis_header(); let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - spec.ensure_db_good(db.as_hashdb_mut()); + spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); let mut b = OpenBlock::new(engine.deref(), &vm_factory, Default::default(), false, db, &genesis_header, last_hashes, Address::zero(), (3141562.into(), 31415620.into()), vec![]).unwrap(); diff --git a/ethcore/src/ethereum/mod.rs b/ethcore/src/ethereum/mod.rs index f48a433d78b..b77adb81a62 100644 --- a/ethcore/src/ethereum/mod.rs +++ b/ethcore/src/ethereum/mod.rs @@ -67,7 +67,7 @@ mod tests { let genesis_header = spec.genesis_header(); let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - spec.ensure_db_good(db.as_hashdb_mut()); + spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let s = State::from_existing(db, genesis_header.state_root.clone(), engine.account_start_nonce(), Default::default()).unwrap(); assert_eq!(s.balance(&address_from_hex("0000000000000000000000000000000000000001")), U256::from(1u64)); assert_eq!(s.balance(&address_from_hex("0000000000000000000000000000000000000002")), U256::from(1u64)); diff --git a/ethcore/src/snapshot/account.rs b/ethcore/src/snapshot/account.rs index ef0e9194852..ec9566470da 100644 --- a/ethcore/src/snapshot/account.rs +++ b/ethcore/src/snapshot/account.rs @@ -160,7 +160,7 @@ mod tests { { let mut trie = SecTrieDBMut::new(&mut db, &mut root); for (k, v) in map.make() { - trie.insert(&k, &v); + trie.insert(&k, &v).unwrap(); } } root diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index 4a58c4a8d8e..839dc991f0e 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -490,12 +490,12 @@ fn should_work_when_cloned() { let mut state = get_temp_state_in(temp.as_path()); assert_eq!(state.exists(&a), false); state.inc_nonce(&a); - state.commit(); + state.commit().unwrap(); state.clone() }; state.inc_nonce(&a); - state.commit(); + state.commit().unwrap(); } #[test] @@ -1305,7 +1305,7 @@ fn code_from_database() { state.require_or_from(&a, false, ||Account::new_contract(42.into(), 0.into()), |_|{}); state.init_code(&a, vec![1, 2, 3]); assert_eq!(state.code(&a), Some([1u8, 2, 3].to_vec())); - state.commit(); + state.commit().unwrap(); assert_eq!(state.code(&a), Some([1u8, 2, 3].to_vec())); state.drop() }; @@ -1321,7 +1321,7 @@ fn storage_at_from_database() { let (root, db) = { let mut state = get_temp_state_in(temp.as_path()); state.set_storage(&a, H256::from(&U256::from(01u64)), H256::from(&U256::from(69u64))); - state.commit(); + state.commit().unwrap(); state.drop() }; @@ -1337,7 +1337,7 @@ fn get_from_database() { let mut state = get_temp_state_in(temp.as_path()); state.inc_nonce(&a); state.add_balance(&a, &U256::from(69u64)); - state.commit(); + state.commit().unwrap(); assert_eq!(state.balance(&a), U256::from(69u64)); state.drop() }; @@ -1368,7 +1368,7 @@ fn remove_from_database() { let (root, db) = { let mut state = get_temp_state_in(temp.as_path()); state.inc_nonce(&a); - state.commit(); + state.commit().unwrap(); assert_eq!(state.exists(&a), true); assert_eq!(state.nonce(&a), U256::from(1u64)); state.drop() @@ -1379,7 +1379,7 @@ fn remove_from_database() { assert_eq!(state.exists(&a), true); assert_eq!(state.nonce(&a), U256::from(1u64)); state.kill_account(&a); - state.commit(); + state.commit().unwrap(); assert_eq!(state.exists(&a), false); assert_eq!(state.nonce(&a), U256::from(0u64)); state.drop() @@ -1398,16 +1398,16 @@ fn alter_balance() { let b = address_from_u64(1u64); state.add_balance(&a, &U256::from(69u64)); assert_eq!(state.balance(&a), U256::from(69u64)); - state.commit(); + state.commit().unwrap(); assert_eq!(state.balance(&a), U256::from(69u64)); state.sub_balance(&a, &U256::from(42u64)); assert_eq!(state.balance(&a), U256::from(27u64)); - state.commit(); + state.commit().unwrap(); assert_eq!(state.balance(&a), U256::from(27u64)); state.transfer_balance(&a, &b, &U256::from(18u64)); assert_eq!(state.balance(&a), U256::from(9u64)); assert_eq!(state.balance(&b), U256::from(18u64)); - state.commit(); + state.commit().unwrap(); assert_eq!(state.balance(&a), U256::from(9u64)); assert_eq!(state.balance(&b), U256::from(18u64)); } @@ -1421,11 +1421,11 @@ fn alter_nonce() { assert_eq!(state.nonce(&a), U256::from(1u64)); state.inc_nonce(&a); assert_eq!(state.nonce(&a), U256::from(2u64)); - state.commit(); + state.commit().unwrap(); assert_eq!(state.nonce(&a), U256::from(2u64)); state.inc_nonce(&a); assert_eq!(state.nonce(&a), U256::from(3u64)); - state.commit(); + state.commit().unwrap(); assert_eq!(state.nonce(&a), U256::from(3u64)); } @@ -1436,7 +1436,7 @@ fn balance_nonce() { let a = Address::zero(); assert_eq!(state.balance(&a), U256::from(0u64)); assert_eq!(state.nonce(&a), U256::from(0u64)); - state.commit(); + state.commit().unwrap(); assert_eq!(state.balance(&a), U256::from(0u64)); assert_eq!(state.nonce(&a), U256::from(0u64)); } @@ -1447,7 +1447,7 @@ fn ensure_cached() { let mut state = state_result.reference_mut(); let a = Address::zero(); state.require(&a, false); - state.commit(); + state.commit().unwrap(); assert_eq!(state.root().hex(), "0ce23f3c809de377b008a4a3ee94a0834aac8bec1f86e28ffe4fdb5a15b0c785"); } @@ -1487,7 +1487,7 @@ fn snapshot_nested() { fn create_empty() { let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); - state.commit(); + state.commit().unwrap(); assert_eq!(state.root().hex(), "56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421"); } From ffffa2a77001e468c39318688fd3add4771bbb49 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Sun, 31 Jul 2016 17:31:32 +0200 Subject: [PATCH 17/22] fix json tests --- ethcore/src/json_tests/state.rs | 3 ++- ethcore/src/json_tests/trie.rs | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/ethcore/src/json_tests/state.rs b/ethcore/src/json_tests/state.rs index 6ce808611b7..4252c9f3048 100644 --- a/ethcore/src/json_tests/state.rs +++ b/ethcore/src/json_tests/state.rs @@ -62,7 +62,8 @@ pub fn json_chain_test(json_data: &[u8], era: ChainEra) -> Vec { let mut state_result = get_temp_state(); let mut state = state_result.reference_mut(); state.populate_from(pre); - state.commit(); + state.commit() + .expect(&format!("State test {} failed due to internal error.", name)); let vm_factory = Default::default(); let res = state.apply(&env, engine.deref(), &vm_factory, &transaction, false); diff --git a/ethcore/src/json_tests/trie.rs b/ethcore/src/json_tests/trie.rs index e62fd01b3c9..4090ec76222 100644 --- a/ethcore/src/json_tests/trie.rs +++ b/ethcore/src/json_tests/trie.rs @@ -15,7 +15,9 @@ // along with Parity. If not, see . use ethjson; -use util::{H256, MemoryDB, TrieSpec, TrieFactory}; +use util::trie::{TrieFactory, TrieSpec}; +use util::hash::H256; +use util::memorydb::MemoryDB; fn test_trie(json: &[u8], trie: TrieSpec) -> Vec { let tests = ethjson::trie::Test::load(json).unwrap(); @@ -30,7 +32,8 @@ fn test_trie(json: &[u8], trie: TrieSpec) -> Vec { for (key, value) in test.input.data.into_iter() { let key: Vec = key.into(); let value: Vec = value.map_or_else(Vec::new, Into::into); - t.insert(&key, &value); + t.insert(&key, &value) + .expect(&format!("Trie test '{:?}' failed due to internal error", name)) } if *t.root() != test.root.into() { @@ -46,7 +49,7 @@ fn test_trie(json: &[u8], trie: TrieSpec) -> Vec { } mod generic { - use util::TrieSpec; + use util::trie::TrieSpec; fn do_json_test(json: &[u8]) -> Vec { super::test_trie(json, TrieSpec::Generic) @@ -57,7 +60,7 @@ mod generic { } mod secure { - use util::TrieSpec; + use util::trie::TrieSpec; fn do_json_test(json: &[u8]) -> Vec { super::test_trie(json, TrieSpec::Secure) From 82f7a0e22c4588bf348169218c25961f0be8b0c4 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 1 Aug 2016 08:18:22 -0700 Subject: [PATCH 18/22] whitespace [ci:skip] --- ethcore/src/pod_account.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/src/pod_account.rs b/ethcore/src/pod_account.rs index 2a71c558b4d..8c8fb8a915c 100644 --- a/ethcore/src/pod_account.rs +++ b/ethcore/src/pod_account.rs @@ -71,7 +71,7 @@ impl PodAccount { let mut r = H256::new(); let mut t = SecTrieDBMut::new(db, &mut r); for (k, v) in &self.storage { - if let Err(e) = t.insert(k, &encode(&U256::from(v.as_slice()))) { + if let Err(e) = t.insert(k, &encode(&U256::from(v.as_slice()))) { warn!("Encountered potential DB corruption: {}", e); } } From f651ad760ff0cf4261fb0ef1252d4bfd63217d89 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 1 Aug 2016 08:22:03 -0700 Subject: [PATCH 19/22] Avoid unneeded match/indentation --- ethcore/src/state.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index 839dc991f0e..c3c9a5c722e 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -164,14 +164,8 @@ impl State { /// Determine whether an account exists. pub fn exists(&self, a: &Address) -> bool { let db = self.trie_factory.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); - self.cache.borrow().get(&a).unwrap_or(&None).is_some() - || match db.contains(a) { - Ok(x) => x, - Err(e) => { - warn!("Potential DB corruption encountered: {}", e); - false - } - } + self.cache.borrow().get(&a).unwrap_or(&None).is_some() || + db.contains(a).unwrap_or_else(|e| { warn!("Potential DB corruption encountered: {}", e); false }) } /// Get the balance of account `a`. From 65c20efa9f591b77b604e06831c801b0fd429363 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 1 Aug 2016 08:32:37 -0700 Subject: [PATCH 20/22] whitespace --- ethcore/src/state.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index c3c9a5c722e..cf9d4e98418 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -251,9 +251,13 @@ impl State { /// Commit accounts to SecTrieDBMut. This is similar to cpp-ethereum's dev::eth::commit. /// `accounts` is mutable because we may need to commit the code or storage and record that. #[cfg_attr(feature="dev", allow(match_ref_pats))] - pub fn commit_into(trie_factory: &TrieFactory, db: &mut HashDB, root: &mut H256, accounts: &mut HashMap>) - -> Result<(), Error> - { + pub fn commit_into( + trie_factory: &TrieFactory, + db: &mut HashDB, + root: &mut H256, + accounts: &mut HashMap> + ) -> Result<(), Error> { // first, commit the sub trees. // TODO: is this necessary or can we dispense with the `ref mut a` for just `a`? for (address, ref mut a) in accounts.iter_mut() { From 99e20110a606580a527f62a91d66ee7db2abb849 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 2 Aug 2016 12:39:57 +0200 Subject: [PATCH 21/22] prettify map_or_else --- ethcore/src/account.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/src/account.rs b/ethcore/src/account.rs index 072454e04c3..396e057c0a5 100644 --- a/ethcore/src/account.rs +++ b/ethcore/src/account.rs @@ -139,7 +139,7 @@ impl Account { using it will not fail."); let item: U256 = match db.get(key){ - Ok(x) => x.map_or_else(|| U256::zero(), decode), + Ok(x) => x.map_or_else(U256::zero, decode), Err(e) => panic!("Encountered potential DB corruption: {}", e), }; (Filth::Clean, item.into()) From e0ac2ff98e54e6dadebd92dda9fdd6882b41fe1f Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 3 Aug 2016 13:01:27 +0200 Subject: [PATCH 22/22] remove test warning --- ethcore/src/engines/instant_seal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/src/engines/instant_seal.rs b/ethcore/src/engines/instant_seal.rs index d9f519e848e..ae235e04c40 100644 --- a/ethcore/src/engines/instant_seal.rs +++ b/ethcore/src/engines/instant_seal.rs @@ -84,7 +84,7 @@ mod tests { let genesis_header = spec.genesis_header(); let mut db_result = get_temp_journal_db(); let mut db = db_result.take(); - spec.ensure_db_good(db.as_hashdb_mut()); + spec.ensure_db_good(db.as_hashdb_mut()).unwrap(); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); let b = OpenBlock::new(engine.deref(), &vm_factory, Default::default(), false, db, &genesis_header, last_hashes, addr, (3141562.into(), 31415620.into()), vec![]).unwrap();