Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

Remove (almost all) panickers from trie module #1776

Merged
merged 30 commits into from
Aug 3, 2016
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
fd188d3
memorydb ub patch and other cleanup
rphmeier Jul 6, 2016
45c2c46
fix denote invocations
rphmeier Jul 6, 2016
03bd823
merge with master
rphmeier Jul 25, 2016
07d48a1
move trie traits into trie module
rphmeier Jul 25, 2016
2bf5587
replace "denote" with shim
rphmeier Jul 26, 2016
c631c6c
triedb returns results and no longer panics
rphmeier Jul 26, 2016
b1ff25d
fix warnings
rphmeier Jul 26, 2016
1f6f20c
get ethcore compiling
rphmeier Jul 26, 2016
2d2c54a
warn on trie errors in ethcore
rphmeier Jul 26, 2016
473cfd0
remove unsafety from node decoder
rphmeier Jul 26, 2016
b10d230
merge with master
rphmeier Jul 27, 2016
99c2206
restore broken denote behavior for this branch
rphmeier Jul 27, 2016
c4b233f
fix overlayrecent fallout
rphmeier Jul 27, 2016
d8d6043
fix triedb tests
rphmeier Jul 27, 2016
888f9eb
remove unwrap in state
rphmeier Jul 27, 2016
372ea4a
merge with master
rphmeier Jul 30, 2016
760e6a4
alter Trie::get to return Result<Option<_>>
rphmeier Jul 30, 2016
2a2d17f
merge with master, fail fast
rphmeier Jul 30, 2016
a01b0fe
fix refcell error in require
rphmeier Jul 30, 2016
b560a2e
fix test warnings
rphmeier Jul 30, 2016
ffffa2a
fix json tests
rphmeier Jul 31, 2016
82f7a0e
whitespace
gavofyork Aug 1, 2016
f651ad7
Avoid unneeded match/indentation
gavofyork Aug 1, 2016
65c20ef
whitespace
gavofyork Aug 1, 2016
8b54b4f
Merge branch 'trie_errors' of github.com:ethcore/parity into trie_errors
rphmeier Aug 2, 2016
99e2011
prettify map_or_else
rphmeier Aug 2, 2016
f412fd2
Merge branch 'master' into trie_errors
rphmeier Aug 2, 2016
bc70dde
merge with master
rphmeier Aug 3, 2016
e0ac2ff
remove test warning
rphmeier Aug 3, 2016
771fb50
merge with master
rphmeier Aug 3, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions ethcore/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -136,7 +138,11 @@ impl Account {
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){
Ok(x) => x.map_or_else(|| U256::zero(), decode),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x.map_or_else(U256::zero, decode)?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or x.map_or_else(U256::default, decode) to avoid importing Uint trait :)

Err(e) => panic!("Encountered potential DB corruption: {}", e),
};
(Filth::Clean, item.into())
}).1.clone()
}

Expand Down Expand Up @@ -243,9 +249,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;
}
Expand Down
10 changes: 5 additions & 5 deletions ethcore/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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();
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ impl Client {
let tracedb = Arc::new(try!(TraceDB::new(config.tracing, db.clone(), chain.clone())));

let mut state_db = journaldb::new(db.clone(), config.pruning, DB_COL_STATE);
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())) {
let batch = DBTransaction::new(&db);
state_db.commit(&batch, 0, &spec.genesis_header().hash(), None).expect("Error commiting genesis state to state DB");
db.write(batch).expect("Error writing genesis state to state DB");
Expand Down
19 changes: 18 additions & 1 deletion ethcore/src/client/error.rs
Original file line number Diff line number Diff line change
@@ -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<TraceError> for Error {
Expand All @@ -14,10 +18,23 @@ impl From<TraceError> for Error {
}
}

impl From<TrieError> for Error {
fn from(err: TrieError) -> Self {
Error::Trie(err)
}
}

impl<E> From<Box<E>> for Error where Error: From<E> {
fn from(err: Box<E>) -> Self {
Error::from(*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),
}
}
}
2 changes: 1 addition & 1 deletion ethcore/src/client/test_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,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(
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/engines/basic_authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
11 changes: 10 additions & 1 deletion ethcore/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,10 @@ pub type ImportResult = Result<H256, Error>;

impl From<ClientError> for Error {
fn from(err: ClientError) -> Error {
Error::Client(err)
match err {
ClientError::Trie(err) => Error::Trie(err),
_ => Error::Client(err)
}
}
}

Expand Down Expand Up @@ -338,6 +341,12 @@ impl From<BlockImportError> for Error {
}
}

impl<E> From<Box<E>> for Error where Error: From<E> {
fn from(err: Box<E>) -> Error {
Error::from(*err)
}
}

binary_fixed_size!(BlockError);
binary_fixed_size!(ImportError);
binary_fixed_size!(TransactionError);
Expand Down
8 changes: 5 additions & 3 deletions ethcore/src/ethereum/ethash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -352,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();
Expand All @@ -367,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();
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/ethereum/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 2 additions & 0 deletions ethcore/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion ethcore/src/json_tests/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ pub fn json_chain_test(json_data: &[u8], era: ChainEra) -> Vec<String> {
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);

Expand Down
11 changes: 7 additions & 4 deletions ethcore/src/json_tests/trie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
// along with Parity. If not, see <http://www.gnu.org/licenses/>.

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<String> {
let tests = ethjson::trie::Test::load(json).unwrap();
Expand All @@ -30,7 +32,8 @@ fn test_trie(json: &[u8], trie: TrieSpec) -> Vec<String> {
for (key, value) in test.input.data.into_iter() {
let key: Vec<u8> = key.into();
let value: Vec<u8> = 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() {
Expand All @@ -46,7 +49,7 @@ fn test_trie(json: &[u8], trie: TrieSpec) -> Vec<String> {
}

mod generic {
use util::TrieSpec;
use util::trie::TrieSpec;

fn do_json_test(json: &[u8]) -> Vec<String> {
super::test_trie(json, TrieSpec::Generic)
Expand All @@ -57,7 +60,7 @@ mod generic {
}

mod secure {
use util::TrieSpec;
use util::trie::TrieSpec;

fn do_json_test(json: &[u8]) -> Vec<String> {
super::test_trie(json, TrieSpec::Secure)
Expand Down
4 changes: 3 additions & 1 deletion ethcore/src/pod_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,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);
}
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions ethcore/src/snapshot/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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<Self, DecoderError> {
pub fn from_fat_rlp(acct_db: &mut AccountDBMut, rlp: UntrustedRlp) -> Result<Self, Error> {
use util::{TrieDBMut, TrieMut};

let nonce = try!(rlp.val_at(0));
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/snapshot/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ impl StateRebuilder {
};

for (hash, thin_rlp) in pairs {
account_trie.insert(&hash, &thin_rlp);
try!(account_trie.insert(&hash, &thin_rlp));
}
}

Expand Down
10 changes: 6 additions & 4 deletions ethcore/src/spec/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ use super::seal::Generic as GenericSeal;
use ethereum;
use ethjson;

use std::cell::RefCell;

/// Parameters common to all engines.
#[derive(Debug, PartialEq, Clone)]
pub struct CommonParams {
Expand Down Expand Up @@ -225,21 +227,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<bool, Box<TrieError>> {
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.
Expand Down
Loading