From 13968aaa38aa494c243fa1058c4739311a2f4a24 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 7 Jun 2016 20:44:09 +0200 Subject: [PATCH 1/3] Refactor triedb constructors to error on invalid state root (#1230) * add TrieError, refactor Trie DB creation * remove Result type alias due to glob import conflicts * fix fallout in state.rs * add debug, display impl for TrieError * fix fallout in account.rs * ethcore::Error::TrieError variant * fix remaining fallout in ethcore crate * added From impl for Error, removed map_err calls * fix test breakages * fix doc tests * update docs [ci skip] --- ethcore/src/account.rs | 12 +++++++-- ethcore/src/basic_authority.rs | 2 +- ethcore/src/block.rs | 28 ++++++++++++++------ ethcore/src/client/client.rs | 13 ++++------ ethcore/src/error.rs | 9 +++++++ ethcore/src/ethereum/ethash.rs | 4 +-- ethcore/src/ethereum/mod.rs | 2 +- ethcore/src/state.rs | 47 +++++++++++++++++++--------------- util/src/trie/mod.rs | 27 ++++++++++++++----- util/src/trie/sectriedb.rs | 20 +++++++++------ util/src/trie/sectriedbmut.rs | 18 +++++++------ util/src/trie/triedb.rs | 30 ++++++++++++---------- util/src/trie/triedbmut.rs | 26 +++++++++---------- 13 files changed, 146 insertions(+), 92 deletions(-) diff --git a/ethcore/src/account.rs b/ethcore/src/account.rs index 5e0dc335a..29922dda5 100644 --- a/ethcore/src/account.rs +++ b/ethcore/src/account.rs @@ -116,7 +116,12 @@ impl Account { /// Get (and cache) the contents of the trie's storage at `key`. pub fn storage_at(&self, db: &AccountDB, key: &H256) -> H256 { self.storage_overlay.borrow_mut().entry(key.clone()).or_insert_with(||{ - (Filth::Clean, H256::from(SecTrieDB::new(db, &self.storage_root).get(key.bytes()).map_or(U256::zero(), |v| -> U256 {decode(v)}))) + 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.bytes()).map_or(U256::zero(), |v| -> U256 {decode(v)}))) }).1.clone() } @@ -204,7 +209,10 @@ impl Account { /// Commit the `storage_overlay` to the backing DB and update `storage_root`. pub fn commit_storage(&mut self, db: &mut AccountDBMut) { - let mut t = SecTrieDBMut::from_existing(db, &mut self.storage_root); + let mut t = SecTrieDBMut::from_existing(db, &mut 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."); for (k, &mut (ref mut f, ref mut v)) in self.storage_overlay.borrow_mut().iter_mut() { if f == &Filth::Dirty { // cast key and value to trait type, diff --git a/ethcore/src/basic_authority.rs b/ethcore/src/basic_authority.rs index b4a938642..c732ce711 100644 --- a/ethcore/src/basic_authority.rs +++ b/ethcore/src/basic_authority.rs @@ -278,7 +278,7 @@ mod tests { spec.ensure_db_good(db.as_hashdb_mut()); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); - let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, addr.clone(), 3141562.into(), vec![]); + let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, addr.clone(), 3141562.into(), vec![]).unwrap(); let b = b.close_and_lock(); let seal = engine.generate_seal(b.block(), Some(&tap)).unwrap(); diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index 0b75f5a7e..8f2dc263c 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -228,9 +228,20 @@ pub struct SealedBlock { impl<'x> OpenBlock<'x> { #[cfg_attr(feature="dev", allow(too_many_arguments))] /// Create a new `OpenBlock` ready for transaction pushing. - pub fn new(engine: &'x Engine, vm_factory: &'x EvmFactory, tracing: bool, db: Box, parent: &Header, last_hashes: LastHashes, author: Address, gas_floor_target: U256, extra_data: Bytes) -> Self { + pub fn new( + engine: &'x Engine, + vm_factory: &'x EvmFactory, + tracing: bool, + db: Box, + parent: &Header, + last_hashes: LastHashes, + author: Address, + gas_floor_target: U256, + extra_data: Bytes + ) -> Result { + let state = try!(State::from_existing(db, parent.state_root().clone(), engine.account_start_nonce())); let mut r = OpenBlock { - block: ExecutedBlock::new(State::from_existing(db, parent.state_root().clone(), engine.account_start_nonce()), tracing), + block: ExecutedBlock::new(state, tracing), engine: engine, vm_factory: vm_factory, last_hashes: last_hashes, @@ -245,7 +256,7 @@ impl<'x> OpenBlock<'x> { engine.populate_from_parent(&mut r.block.base.header, parent, gas_floor_target); engine.on_new_block(&mut r.block); - r + Ok(r) } /// Alter the author for the block. @@ -464,12 +475,12 @@ impl IsBlock for SealedBlock { pub fn enact(header: &Header, transactions: &[SignedTransaction], uncles: &[Header], engine: &Engine, tracing: bool, db: Box, parent: &Header, last_hashes: LastHashes, vm_factory: &EvmFactory) -> Result { { if ::log::max_log_level() >= ::log::LogLevel::Trace { - let s = State::from_existing(db.boxed_clone(), parent.state_root().clone(), engine.account_start_nonce()); + let s = try!(State::from_existing(db.boxed_clone(), parent.state_root().clone(), engine.account_start_nonce())); trace!("enact(): root={}, author={}, author_balance={}\n", s.root(), header.author(), s.balance(&header.author())); } } - let mut b = OpenBlock::new(engine, vm_factory, tracing, db, parent, last_hashes, header.author().clone(), 3141562.into(), header.extra_data().clone()); + let mut b = try!(OpenBlock::new(engine, vm_factory, tracing, db, parent, last_hashes, header.author().clone(), 3141562.into(), header.extra_data().clone())); b.set_difficulty(*header.difficulty()); b.set_gas_limit(*header.gas_limit()); b.set_timestamp(header.timestamp()); @@ -514,7 +525,7 @@ mod tests { spec.ensure_db_good(db.as_hashdb_mut()); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); - let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]); + let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]).unwrap(); let b = b.close_and_lock(); let _ = b.seal(engine.deref(), vec![]); } @@ -530,7 +541,8 @@ mod tests { let mut db = db_result.take(); spec.ensure_db_good(db.as_hashdb_mut()); let vm_factory = Default::default(); - let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, vec![genesis_header.hash()], Address::zero(), 3141562.into(), vec![]).close_and_lock().seal(engine.deref(), vec![]).unwrap(); + let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, vec![genesis_header.hash()], Address::zero(), 3141562.into(), vec![]).unwrap() + .close_and_lock().seal(engine.deref(), vec![]).unwrap(); let orig_bytes = b.rlp_bytes(); let orig_db = b.drain(); @@ -557,7 +569,7 @@ mod tests { let mut db = db_result.take(); spec.ensure_db_good(db.as_hashdb_mut()); let vm_factory = Default::default(); - let mut open_block = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, vec![genesis_header.hash()], Address::zero(), 3141562.into(), vec![]); + let mut open_block = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, vec![genesis_header.hash()], Address::zero(), 3141562.into(), vec![]).unwrap(); let mut uncle1_header = Header::new(); uncle1_header.extra_data = b"uncle1".to_vec(); let mut uncle2_header = Header::new(); diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 03f38891a..f8eabb2a3 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -386,18 +386,14 @@ impl Client where V: Verifier { let root = HeaderView::new(&header).state_root(); - // TODO [rob]: refactor State::from_existing so we avoid doing redundant lookups. - if !db.contains(&root) { - return None; - } - - Some(State::from_existing(db, root, self.engine.account_start_nonce())) + State::from_existing(db, root, self.engine.account_start_nonce()).ok() }) } /// Get a copy of the best block's state. pub fn state(&self) -> State { State::from_existing(self.state_db.lock().unwrap().boxed_clone(), HeaderView::new(&self.best_block_header()).state_root(), self.engine.account_start_nonce()) + .expect("State root of best block header always valid.") } /// Get info on the cache. @@ -481,7 +477,7 @@ impl BlockChainClient for Client where V: Verifier { } let options = TransactOptions { tracing: analytics.transaction_tracing, vm_tracing: analytics.vm_tracing, check_nonce: false }; let mut ret = Executive::new(&mut state, &env_info, self.engine.deref().deref(), &self.vm_factory).transact(t, options); - + // TODO gav move this into Executive. if analytics.state_diffing { if let Ok(ref mut x) = ret { @@ -776,7 +772,8 @@ impl MiningBlockChainClient for Client where V: Verifier { author, gas_floor_target, extra_data, - ); + ).expect("OpenBlock::new only fails if parent state root invalid. State root of best block's header is never invalid. \ + Therefore creating an OpenBlock with the best block's header will not fail."); // Add uncles self.chain diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index 3d7c148ba..98aec917a 100644 --- a/ethcore/src/error.rs +++ b/ethcore/src/error.rs @@ -224,6 +224,8 @@ pub enum Error { PowHashInvalid, /// The value of the nonce or mishash is invalid. PowInvalid, + /// Error concerning TrieDBs + TrieError(TrieError), } impl fmt::Display for Error { @@ -239,6 +241,7 @@ impl fmt::Display for Error { f.write_fmt(format_args!("Unknown engine name ({})", name)), Error::PowHashInvalid => f.write_str("Invalid or out of date PoW hash."), Error::PowInvalid => f.write_str("Invalid nonce or mishash"), + Error::TrieError(ref err) => f.write_fmt(format_args!("{}", err)), } } } @@ -300,6 +303,12 @@ impl From for Error { } } +impl From for Error { + fn from(err: TrieError) -> Error { + Error::TrieError(err) + } +} + // TODO: uncomment below once https://github.com/rust-lang/rust/issues/27336 sorted. /*#![feature(concat_idents)] macro_rules! assimilate { diff --git a/ethcore/src/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index 1dadb8a65..746da9069 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -308,7 +308,7 @@ mod tests { spec.ensure_db_good(db.as_hashdb_mut()); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); - let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]); + let b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]).unwrap(); let b = b.close(); assert_eq!(b.state().balance(&Address::zero()), U256::from_str("4563918244f40000").unwrap()); } @@ -323,7 +323,7 @@ mod tests { spec.ensure_db_good(db.as_hashdb_mut()); let last_hashes = vec![genesis_header.hash()]; let vm_factory = Default::default(); - let mut b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]); + let mut b = OpenBlock::new(engine.deref(), &vm_factory, false, db, &genesis_header, last_hashes, Address::zero(), 3141562.into(), vec![]).unwrap(); let mut uncle = Header::new(); let uncle_author = address_from_hex("ef2d6d194084c2de36e0dabfce45d046b37d1106"); uncle.author = uncle_author.clone(); diff --git a/ethcore/src/ethereum/mod.rs b/ethcore/src/ethereum/mod.rs index 1f5712c54..9c9e48342 100644 --- a/ethcore/src/ethereum/mod.rs +++ b/ethcore/src/ethereum/mod.rs @@ -62,7 +62,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()); - let s = State::from_existing(db, genesis_header.state_root.clone(), engine.account_start_nonce()); + let s = State::from_existing(db, genesis_header.state_root.clone(), engine.account_start_nonce()).unwrap(); assert_eq!(s.balance(&address_from_hex("0000000000000000000000000000000000000001")), U256::from(1u64)); assert_eq!(s.balance(&address_from_hex("0000000000000000000000000000000000000002")), U256::from(1u64)); assert_eq!(s.balance(&address_from_hex("0000000000000000000000000000000000000003")), U256::from(1u64)); diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index 72bd6c8fd..204cea618 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -44,6 +44,9 @@ pub struct State { account_start_nonce: U256, } +const SEC_TRIE_DB_UNWRAP_STR: &'static str = "A state can only be created with valid root. Creating a SecTrieDB with a valid root will not fail. \ + Therefore creating a SecTrieDB with this state's root will not fail."; + impl State { /// Creates new state with empty state root #[cfg(test)] @@ -64,18 +67,17 @@ impl State { } /// Creates new state with existing state root - pub fn from_existing(db: Box, root: H256, account_start_nonce: U256) -> State { - { - // trie should panic! if root does not exist - let _ = SecTrieDB::new(db.as_hashdb(), &root); - } - - State { - db: db, - root: root, - cache: RefCell::new(HashMap::new()), - snapshots: RefCell::new(Vec::new()), - account_start_nonce: account_start_nonce, + pub fn from_existing(db: Box, root: H256, account_start_nonce: U256) -> Result { + if !db.as_hashdb().contains(&root) { + Err(TrieError::InvalidStateRoot) + } else { + Ok(State { + db: db, + root: root, + cache: RefCell::new(HashMap::new()), + snapshots: RefCell::new(Vec::new()), + account_start_nonce: account_start_nonce, + }) } } @@ -154,7 +156,8 @@ impl State { /// Determine whether an account exists. pub fn exists(&self, a: &Address) -> bool { - self.cache.borrow().get(&a).unwrap_or(&None).is_some() || SecTrieDB::new(self.db.as_hashdb(), &self.root).contains(&a) + let db = SecTrieDB::new(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) } /// Get the balance of account `a`. @@ -245,7 +248,7 @@ impl State { } { - let mut trie = SecTrieDBMut::from_existing(db, root); + let mut trie = SecTrieDBMut::from_existing(db, root).unwrap(); for (address, ref a) in accounts.iter() { match **a { Some(ref account) => trie.insert(address, &account.rlp()), @@ -308,7 +311,8 @@ impl State { fn get<'a>(&'a self, a: &Address, require_code: bool) -> &'a Option { let have_key = self.cache.borrow().contains_key(a); if !have_key { - self.insert_cache(a, SecTrieDB::new(self.db.as_hashdb(), &self.root).get(&a).map(Account::from_rlp)) + let db = SecTrieDB::new(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); + self.insert_cache(a, db.get(&a).map(Account::from_rlp)) } if require_code { if let Some(ref mut account) = self.cache.borrow_mut().get_mut(a).unwrap().as_mut() { @@ -328,7 +332,8 @@ impl State { 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 { - self.insert_cache(a, SecTrieDB::new(self.db.as_hashdb(), &self.root).get(&a).map(Account::from_rlp)) + let db = SecTrieDB::new(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); + self.insert_cache(a, db.get(&a).map(Account::from_rlp)) } else { self.note_cache(a); } @@ -1145,7 +1150,7 @@ fn code_from_database() { state.drop() }; - let state = State::from_existing(db, root, U256::from(0u8)); + let state = State::from_existing(db, root, U256::from(0u8)).unwrap(); assert_eq!(state.code(&a), Some([1u8, 2, 3].to_vec())); } @@ -1160,7 +1165,7 @@ fn storage_at_from_database() { state.drop() }; - let s = State::from_existing(db, root, U256::from(0u8)); + let s = State::from_existing(db, root, U256::from(0u8)).unwrap(); assert_eq!(s.storage_at(&a, &H256::from(&U256::from(01u64))), H256::from(&U256::from(69u64))); } @@ -1177,7 +1182,7 @@ fn get_from_database() { state.drop() }; - let state = State::from_existing(db, root, U256::from(0u8)); + let state = State::from_existing(db, root, U256::from(0u8)).unwrap(); assert_eq!(state.balance(&a), U256::from(69u64)); assert_eq!(state.nonce(&a), U256::from(1u64)); } @@ -1210,7 +1215,7 @@ fn remove_from_database() { }; let (root, db) = { - let mut state = State::from_existing(db, root, U256::from(0u8)); + let mut state = State::from_existing(db, root, U256::from(0u8)).unwrap(); assert_eq!(state.exists(&a), true); assert_eq!(state.nonce(&a), U256::from(1u64)); state.kill_account(&a); @@ -1220,7 +1225,7 @@ fn remove_from_database() { state.drop() }; - let state = State::from_existing(db, root, U256::from(0u8)); + let state = State::from_existing(db, root, U256::from(0u8)).unwrap(); assert_eq!(state.exists(&a), false); assert_eq!(state.nonce(&a), U256::from(0u64)); } diff --git a/util/src/trie/mod.rs b/util/src/trie/mod.rs index b8b98880d..79135162c 100644 --- a/util/src/trie/mod.rs +++ b/util/src/trie/mod.rs @@ -16,6 +16,8 @@ //! Trie interface and implementation. +use std::fmt; + /// Export the trietraits module. pub mod trietraits; /// Export the standardmap module. @@ -33,9 +35,22 @@ pub mod sectriedb; /// Export the sectriedbmut module. pub mod sectriedbmut; -pub use self::trietraits::*; -pub use self::standardmap::*; -pub use self::triedbmut::*; -pub use self::triedb::*; -pub use self::sectriedbmut::*; -pub use self::sectriedb::*; +pub use self::trietraits::{Trie, TrieMut}; +pub use self::standardmap::{Alphabet, StandardMap, ValueMode}; +pub use self::triedbmut::TrieDBMut; +pub use self::triedb::TrieDB; +pub use self::sectriedbmut::SecTrieDBMut; +pub use self::sectriedb::SecTrieDB; + +/// Trie Errors +#[derive(Debug)] +pub enum TrieError { + /// Attempted to create a trie with a state root not in the DB. + InvalidStateRoot, +} + +impl fmt::Display for TrieError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "Trie Error: Invalid state root.") + } +} \ No newline at end of file diff --git a/util/src/trie/sectriedb.rs b/util/src/trie/sectriedb.rs index 5558344fa..eda4ac58b 100644 --- a/util/src/trie/sectriedb.rs +++ b/util/src/trie/sectriedb.rs @@ -16,9 +16,10 @@ use hash::*; use sha3::*; -use hashdb::*; -use super::triedb::*; -use super::trietraits::*; +use hashdb::HashDB; +use super::triedb::TrieDB; +use super::trietraits::Trie; +use super::TrieError; /// A `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// @@ -29,10 +30,12 @@ pub struct SecTrieDB<'db> { impl<'db> SecTrieDB<'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) -> Self { - SecTrieDB { raw: TrieDB::new(db, root) } + /// Returns an error if root does not exist. + pub fn new(db: &'db HashDB, root: &'db H256) -> Result { + Ok(SecTrieDB { raw: try!(TrieDB::new(db, root)) }) } /// Get a reference to the underlying raw `TrieDB` struct. @@ -60,8 +63,9 @@ impl<'db> Trie for SecTrieDB<'db> { #[test] fn trie_to_sectrie() { - use memorydb::*; - use super::triedbmut::*; + use memorydb::MemoryDB; + use super::triedbmut::TrieDBMut; + use super::trietraits::TrieMut; let mut memdb = MemoryDB::new(); let mut root = H256::new(); @@ -69,6 +73,6 @@ fn trie_to_sectrie() { let mut t = TrieDBMut::new(&mut memdb, &mut root); t.insert(&(&[0x01u8, 0x23]).sha3(), &[0x01u8, 0x23]); } - let t = SecTrieDB::new(&memdb, &root); + let t = SecTrieDB::new(&memdb, &root).unwrap(); assert_eq!(t.get(&[0x01u8, 0x23]).unwrap(), &[0x01u8, 0x23]); } diff --git a/util/src/trie/sectriedbmut.rs b/util/src/trie/sectriedbmut.rs index 895a821c9..9c8231af4 100644 --- a/util/src/trie/sectriedbmut.rs +++ b/util/src/trie/sectriedbmut.rs @@ -16,9 +16,10 @@ use hash::*; use sha3::*; -use hashdb::*; -use super::triedbmut::*; -use super::trietraits::*; +use hashdb::HashDB; +use super::triedbmut::TrieDBMut; +use super::trietraits::{Trie, TrieMut}; +use super::TrieError; /// A mutable `Trie` implementation which hashes keys and uses a generic `HashDB` backing database. /// @@ -35,10 +36,11 @@ impl<'db> SecTrieDBMut<'db> { SecTrieDBMut { raw: TrieDBMut::new(db, root) } } - /// Create a new trie with the backing database `db` and `root` - /// Panics, if `root` does not exist - pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> Self { - SecTrieDBMut { raw: TrieDBMut::from_existing(db, root) } + /// 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 { + Ok(SecTrieDBMut { raw: try!(TrieDBMut::from_existing(db, root)) }) } /// Get the backing database. @@ -81,6 +83,6 @@ fn sectrie_to_trie() { let mut t = SecTrieDBMut::new(&mut memdb, &mut root); t.insert(&[0x01u8, 0x23], &[0x01u8, 0x23]); } - let t = TrieDB::new(&memdb, &root); + 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 1c6d2236b..fccd5da90 100644 --- a/util/src/trie/triedb.rs +++ b/util/src/trie/triedb.rs @@ -18,8 +18,9 @@ use common::*; use hashdb::*; use nibbleslice::*; use rlp::*; -use super::trietraits::*; -use super::node::*; +use super::trietraits::Trie; +use super::node::Node; +use super::TrieError; /// A `Trie` implementation using a generic `HashDB` backing database. /// @@ -41,7 +42,7 @@ use super::node::*; /// let mut memdb = MemoryDB::new(); /// let mut root = H256::new(); /// TrieDBMut::new(&mut memdb, &mut root).insert(b"foo", b"bar"); -/// let t = TrieDB::new(&memdb, &root); +/// let t = TrieDB::new(&memdb, &root).unwrap(); /// assert!(t.contains(b"foo")); /// assert_eq!(t.get(b"foo").unwrap(), b"bar"); /// assert!(t.db_items_remaining().is_empty()); @@ -57,16 +58,16 @@ pub struct TrieDB<'db> { #[cfg_attr(feature="dev", allow(wrong_self_convention))] impl<'db> TrieDB<'db> { /// Create a new trie with the backing database `db` and `root` - /// Panics, if `root` does not exist - pub fn new(db: &'db HashDB, root: &'db H256) -> Self { + /// Returns an error if `root` does not exist + pub fn new(db: &'db HashDB, root: &'db H256) -> Result { if !db.contains(root) { - flushln!("TrieDB::new({}): Trie root not found!", root); - panic!("Trie root not found!"); - } - TrieDB { - db: db, - root: root, - hash_count: 0 + Err(TrieError::InvalidStateRoot) + } else { + Ok(TrieDB { + db: db, + root: root, + hash_count: 0 + }) } } @@ -356,6 +357,7 @@ impl<'db> fmt::Debug for TrieDB<'db> { #[test] fn iterator() { + use super::trietraits::TrieMut; use memorydb::*; use super::triedbmut::*; @@ -369,6 +371,6 @@ fn iterator() { t.insert(&x, &x); } } - assert_eq!(d.iter().map(|i|i.to_vec()).collect::>(), TrieDB::new(&memdb, &root).iter().map(|x|x.0).collect::>()); - assert_eq!(d, TrieDB::new(&memdb, &root).iter().map(|x|x.1).collect::>()); + assert_eq!(d.iter().map(|i|i.to_vec()).collect::>(), TrieDB::new(&memdb, &root).unwrap().iter().map(|x|x.0).collect::>()); + assert_eq!(d, TrieDB::new(&memdb, &root).unwrap().iter().map(|x|x.1).collect::>()); } diff --git a/util/src/trie/triedbmut.rs b/util/src/trie/triedbmut.rs index 3d75fa3e1..afabd6437 100644 --- a/util/src/trie/triedbmut.rs +++ b/util/src/trie/triedbmut.rs @@ -18,9 +18,10 @@ use common::*; use hashdb::*; use nibbleslice::*; use rlp::*; -use super::node::*; -use super::journal::*; -use super::trietraits::*; +use super::node::Node; +use super::journal::Journal; +use super::trietraits::{Trie, TrieMut}; +use super::TrieError; /// A `Trie` implementation using a generic `HashDB` backing database. /// @@ -84,17 +85,16 @@ impl<'db> TrieDBMut<'db> { } /// Create a new trie with the backing database `db` and `root`. - /// Panics, if `root` does not exist. - // TODO: return Result - pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> Self { + /// Returns an error if `root` does not exist. + pub fn from_existing(db: &'db mut HashDB, root: &'db mut H256) -> Result { if !db.exists(root) { - flushln!("Trie root not found {}", root); - panic!("Trie root not found!"); - } - TrieDBMut { - db: db, - root: root, - hash_count: 0 + Err(TrieError::InvalidStateRoot) + } else { + Ok(TrieDBMut { + db: db, + root: root, + hash_count: 0 + }) } } From be435cde99e0f804ecbe16aad7ea7d762a1a5106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 7 Jun 2016 22:52:48 +0200 Subject: [PATCH 2/3] Splitting RPC Apis into more fine-grained sets (#1234) --- parity/cli.rs | 6 +-- parity/rpc_apis.rs | 25 +++++++--- rpc/src/v1/impls/ethcore.rs | 37 +------------- rpc/src/v1/impls/ethcore_set.rs | 78 ++++++++++++++++++++++++++++++ rpc/src/v1/impls/mod.rs | 2 + rpc/src/v1/mod.rs | 2 +- rpc/src/v1/tests/mocked/ethcore.rs | 71 ++++++++++++++------------- rpc/src/v1/traits/ethcore.rs | 20 -------- rpc/src/v1/traits/ethcore_set.rs | 51 +++++++++++++++++++ rpc/src/v1/traits/mod.rs | 2 + signer/src/ws_server/session.rs | 4 +- 11 files changed, 195 insertions(+), 103 deletions(-) create mode 100644 rpc/src/v1/impls/ethcore_set.rs create mode 100644 rpc/src/v1/traits/ethcore_set.rs diff --git a/parity/cli.rs b/parity/cli.rs index fbaa8bd89..0b3c96c55 100644 --- a/parity/cli.rs +++ b/parity/cli.rs @@ -76,14 +76,14 @@ API and Console Options: --jsonrpc-apis APIS Specify the APIs available through the JSONRPC interface. APIS is a comma-delimited list of API name. Possible name are web3, eth, net, personal, - ethcore, traces. - [default: web3,eth,net,personal,traces]. + ethcore, ethcore_set, traces. + [default: web3,eth,net,ethcore,personal,traces]. --ipc-off Disable JSON-RPC over IPC service. --ipc-path PATH Specify custom path for JSON-RPC over IPC service [default: $HOME/.parity/jsonrpc.ipc]. --ipc-apis APIS Specify custom API set available via JSON-RPC over - IPC [default: web3,eth,net,personal,traces]. + IPC [default: web3,eth,net,ethcore,personal,traces]. --dapps-off Disable the Dapps server (e.g. status page). --dapps-port PORT Specify the port portion of the Dapps server diff --git a/parity/rpc_apis.rs b/parity/rpc_apis.rs index 34dd3a387..a061b8e1b 100644 --- a/parity/rpc_apis.rs +++ b/parity/rpc_apis.rs @@ -40,7 +40,9 @@ pub enum Api { Net, Eth, Personal, + Signer, Ethcore, + EthcoreSet, Traces, Rpc, } @@ -66,7 +68,9 @@ impl FromStr for Api { "net" => Ok(Net), "eth" => Ok(Eth), "personal" => Ok(Personal), + "signer" => Ok(Signer), "ethcore" => Ok(Ethcore), + "ethcore_set" => Ok(EthcoreSet), "traces" => Ok(Traces), "rpc" => Ok(Rpc), e => Err(ApiError::UnknownApi(e.into())), @@ -94,7 +98,9 @@ fn to_modules(apis: &[Api]) -> BTreeMap { Api::Net => ("net", "1.0"), Api::Eth => ("eth", "1.0"), Api::Personal => ("personal", "1.0"), + Api::Signer => ("signer", "1.0"), Api::Ethcore => ("ethcore", "1.0"), + Api::EthcoreSet => ("ethcore_set", "1.0"), Api::Traces => ("traces", "1.0"), Api::Rpc => ("rpc", "1.0"), }; @@ -115,12 +121,12 @@ pub fn from_str(apis: Vec<&str>) -> Vec { fn list_apis(apis: ApiSet, signer_enabled: bool) -> Vec { match apis { ApiSet::List(apis) => apis, - ApiSet::UnsafeContext if signer_enabled => { - vec![Api::Web3, Api::Net, Api::Eth, Api::Ethcore, Api::Traces, Api::Rpc] - } + ApiSet::UnsafeContext => { + vec![Api::Web3, Api::Net, Api::Eth, Api::Personal, Api::Ethcore, Api::EthcoreSet, Api::Traces, Api::Rpc] + }, _ => { - vec![Api::Web3, Api::Net, Api::Eth, Api::Personal, Api::Ethcore, Api::Traces, Api::Rpc] - } + vec![Api::Web3, Api::Net, Api::Eth, Api::Personal, Api::Signer, Api::Ethcore, Api::EthcoreSet, Api::Traces, Api::Rpc] + }, } } @@ -148,13 +154,16 @@ pub fn setup_rpc(server: T, deps: Arc, apis: ApiSet }, Api::Personal => { server.add_delegate(PersonalClient::new(&deps.secret_store, &deps.client, &deps.miner, deps.signer_enabled).to_delegate()); - if deps.signer_enabled { - server.add_delegate(SignerClient::new(&deps.secret_store, &deps.client, &deps.miner, &deps.signer_queue).to_delegate()); - } + }, + Api::Signer => { + server.add_delegate(SignerClient::new(&deps.secret_store, &deps.client, &deps.miner, &deps.signer_queue).to_delegate()); }, Api::Ethcore => { server.add_delegate(EthcoreClient::new(&deps.miner, deps.logger.clone(), deps.settings.clone()).to_delegate()) }, + Api::EthcoreSet => { + server.add_delegate(EthcoreSetClient::new(&deps.miner).to_delegate()) + }, Api::Traces => { server.add_delegate(TracesClient::new(&deps.client, &deps.miner).to_delegate()) }, diff --git a/rpc/src/v1/impls/ethcore.rs b/rpc/src/v1/impls/ethcore.rs index df88657d9..b3fe894c6 100644 --- a/rpc/src/v1/impls/ethcore.rs +++ b/rpc/src/v1/impls/ethcore.rs @@ -15,7 +15,7 @@ // along with Parity. If not, see . //! Ethcore-specific rpc implementation. -use util::{U256, Address, RotatingLogger}; +use util::RotatingLogger; use util::network_settings::NetworkSettings; use util::misc::version_data; use std::sync::{Arc, Weak}; @@ -48,41 +48,6 @@ impl EthcoreClient where M: MinerService { impl Ethcore for EthcoreClient where M: MinerService + 'static { - fn set_min_gas_price(&self, params: Params) -> Result { - from_params::<(U256,)>(params).and_then(|(gas_price,)| { - take_weak!(self.miner).set_minimal_gas_price(gas_price); - to_value(&true) - }) - } - - fn set_gas_floor_target(&self, params: Params) -> Result { - from_params::<(U256,)>(params).and_then(|(gas_floor_target,)| { - take_weak!(self.miner).set_gas_floor_target(gas_floor_target); - to_value(&true) - }) - } - - fn set_extra_data(&self, params: Params) -> Result { - from_params::<(Bytes,)>(params).and_then(|(extra_data,)| { - take_weak!(self.miner).set_extra_data(extra_data.to_vec()); - to_value(&true) - }) - } - - fn set_author(&self, params: Params) -> Result { - from_params::<(Address,)>(params).and_then(|(author,)| { - take_weak!(self.miner).set_author(author); - to_value(&true) - }) - } - - fn set_transactions_limit(&self, params: Params) -> Result { - from_params::<(usize,)>(params).and_then(|(limit,)| { - take_weak!(self.miner).set_transactions_limit(limit); - to_value(&true) - }) - } - fn transactions_limit(&self, _: Params) -> Result { to_value(&take_weak!(self.miner).transactions_limit()) } diff --git a/rpc/src/v1/impls/ethcore_set.rs b/rpc/src/v1/impls/ethcore_set.rs new file mode 100644 index 000000000..b07dcbf9e --- /dev/null +++ b/rpc/src/v1/impls/ethcore_set.rs @@ -0,0 +1,78 @@ +// 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 . + +/// Ethcore-specific rpc interface for operations altering the settings. +use util::{U256, Address}; +use std::sync::{Arc, Weak}; +use jsonrpc_core::*; +use ethcore::miner::MinerService; +use v1::traits::EthcoreSet; +use v1::types::{Bytes}; + +/// Ethcore-specific rpc interface for operations altering the settings. +pub struct EthcoreSetClient where + M: MinerService { + + miner: Weak, +} + +impl EthcoreSetClient where M: MinerService { + /// Creates new `EthcoreSetClient`. + pub fn new(miner: &Arc) -> Self { + EthcoreSetClient { + miner: Arc::downgrade(miner), + } + } +} + +impl EthcoreSet for EthcoreSetClient where M: MinerService + 'static { + + fn set_min_gas_price(&self, params: Params) -> Result { + from_params::<(U256,)>(params).and_then(|(gas_price,)| { + take_weak!(self.miner).set_minimal_gas_price(gas_price); + to_value(&true) + }) + } + + fn set_gas_floor_target(&self, params: Params) -> Result { + from_params::<(U256,)>(params).and_then(|(gas_floor_target,)| { + take_weak!(self.miner).set_gas_floor_target(gas_floor_target); + to_value(&true) + }) + } + + fn set_extra_data(&self, params: Params) -> Result { + from_params::<(Bytes,)>(params).and_then(|(extra_data,)| { + take_weak!(self.miner).set_extra_data(extra_data.to_vec()); + to_value(&true) + }) + } + + fn set_author(&self, params: Params) -> Result { + from_params::<(Address,)>(params).and_then(|(author,)| { + take_weak!(self.miner).set_author(author); + to_value(&true) + }) + } + + fn set_transactions_limit(&self, params: Params) -> Result { + from_params::<(usize,)>(params).and_then(|(limit,)| { + take_weak!(self.miner).set_transactions_limit(limit); + to_value(&true) + }) + } + +} diff --git a/rpc/src/v1/impls/mod.rs b/rpc/src/v1/impls/mod.rs index 9e154a1c5..7fdf57249 100644 --- a/rpc/src/v1/impls/mod.rs +++ b/rpc/src/v1/impls/mod.rs @@ -37,6 +37,7 @@ mod net; mod personal; mod personal_signer; mod ethcore; +mod ethcore_set; mod traces; mod rpc; @@ -48,6 +49,7 @@ pub use self::net::NetClient; pub use self::personal::PersonalClient; pub use self::personal_signer::SignerClient; pub use self::ethcore::EthcoreClient; +pub use self::ethcore_set::EthcoreSetClient; pub use self::traces::TracesClient; pub use self::rpc::RpcClient; diff --git a/rpc/src/v1/mod.rs b/rpc/src/v1/mod.rs index 54628d892..b4d3693f3 100644 --- a/rpc/src/v1/mod.rs +++ b/rpc/src/v1/mod.rs @@ -25,6 +25,6 @@ pub mod traits; pub mod tests; pub mod types; -pub use self::traits::{Web3, Eth, EthFilter, EthSigning, Personal, PersonalSigner, Net, Ethcore, Traces, Rpc}; +pub use self::traits::{Web3, Eth, EthFilter, EthSigning, Personal, PersonalSigner, Net, Ethcore, EthcoreSet, Traces, Rpc}; pub use self::impls::*; pub use self::helpers::{SigningQueue, ConfirmationsQueue}; diff --git a/rpc/src/v1/tests/mocked/ethcore.rs b/rpc/src/v1/tests/mocked/ethcore.rs index 1685a598b..90103adef 100644 --- a/rpc/src/v1/tests/mocked/ethcore.rs +++ b/rpc/src/v1/tests/mocked/ethcore.rs @@ -17,7 +17,7 @@ use std::sync::Arc; use std::str::FromStr; use jsonrpc_core::IoHandler; -use v1::{Ethcore, EthcoreClient}; +use v1::{Ethcore, EthcoreClient, EthcoreSet, EthcoreSetClient}; use ethcore::miner::MinerService; use v1::tests::helpers::TestMinerService; use util::numbers::*; @@ -49,12 +49,16 @@ fn ethcore_client(miner: &Arc) -> EthcoreClient) -> EthcoreSetClient { + EthcoreSetClient::new(&miner) +} + #[test] fn rpc_ethcore_extra_data() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_extraData", "params": [], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":"0x01020304","id":1}"#; @@ -68,9 +72,9 @@ fn rpc_ethcore_default_extra_data() { use util::ToPretty; let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_defaultExtraData", "params": [], "id": 1}"#; let response = format!(r#"{{"jsonrpc":"2.0","result":"0x{}","id":1}}"#, misc::version_data().to_hex()); @@ -81,9 +85,9 @@ fn rpc_ethcore_default_extra_data() { #[test] fn rpc_ethcore_gas_floor_target() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_gasFloorTarget", "params": [], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":"0x3039","id":1}"#; @@ -94,9 +98,9 @@ fn rpc_ethcore_gas_floor_target() { #[test] fn rpc_ethcore_min_gas_price() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_minGasPrice", "params": [], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":"0x01312d00","id":1}"#; @@ -107,9 +111,9 @@ fn rpc_ethcore_min_gas_price() { #[test] fn rpc_ethcore_set_min_gas_price() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_setMinGasPrice", "params":["0xcd1722f3947def4cf144679da39c4c32bdc35681"], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#; @@ -121,9 +125,9 @@ fn rpc_ethcore_set_min_gas_price() { #[test] fn rpc_ethcore_set_gas_floor_target() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_setGasFloorTarget", "params":["0xcd1722f3947def4cf144679da39c4c32bdc35681"], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#; @@ -135,9 +139,9 @@ fn rpc_ethcore_set_gas_floor_target() { #[test] fn rpc_ethcore_set_extra_data() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_setExtraData", "params":["0xcd1722f3947def4cf144679da39c4c32bdc35681"], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#; @@ -149,9 +153,9 @@ fn rpc_ethcore_set_extra_data() { #[test] fn rpc_ethcore_set_author() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_setAuthor", "params":["0xcd1722f3947def4cf144679da39c4c32bdc35681"], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#; @@ -169,6 +173,7 @@ fn rpc_ethcore_dev_logs() { let ethcore = EthcoreClient::new(&miner, logger.clone(), settings()).to_delegate(); let io = IoHandler::new(); io.add_delegate(ethcore); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_devLogs", "params":[], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":["b","a"],"id":1}"#; @@ -179,9 +184,9 @@ fn rpc_ethcore_dev_logs() { #[test] fn rpc_ethcore_dev_logs_levels() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_devLogsLevels", "params":[], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":"rpc=trace","id":1}"#; @@ -191,9 +196,9 @@ fn rpc_ethcore_dev_logs_levels() { #[test] fn rpc_ethcore_set_transactions_limit() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_setTransactionsLimit", "params":[10240240], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#; @@ -205,9 +210,9 @@ fn rpc_ethcore_set_transactions_limit() { #[test] fn rpc_ethcore_transactions_limit() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_transactionsLimit", "params":[], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":1024,"id":1}"#; @@ -218,9 +223,9 @@ fn rpc_ethcore_transactions_limit() { #[test] fn rpc_ethcore_net_chain() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_netChain", "params":[], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":"testchain","id":1}"#; @@ -231,9 +236,9 @@ fn rpc_ethcore_net_chain() { #[test] fn rpc_ethcore_net_max_peers() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_netMaxPeers", "params":[], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":25,"id":1}"#; @@ -244,9 +249,9 @@ fn rpc_ethcore_net_max_peers() { #[test] fn rpc_ethcore_net_port() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_netPort", "params":[], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":30303,"id":1}"#; @@ -257,9 +262,9 @@ fn rpc_ethcore_net_port() { #[test] fn rpc_ethcore_rpc_settings() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_rpcSettings", "params":[], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":{"enabled":true,"interface":"all","port":8545},"id":1}"#; @@ -270,9 +275,9 @@ fn rpc_ethcore_rpc_settings() { #[test] fn rpc_ethcore_node_name() { let miner = miner_service(); - let ethcore = ethcore_client(&miner).to_delegate(); let io = IoHandler::new(); - io.add_delegate(ethcore); + io.add_delegate(ethcore_client(&miner).to_delegate()); + io.add_delegate(ethcore_set_client(&miner).to_delegate()); let request = r#"{"jsonrpc": "2.0", "method": "ethcore_nodeName", "params":[], "id": 1}"#; let response = r#"{"jsonrpc":"2.0","result":"mynode","id":1}"#; diff --git a/rpc/src/v1/traits/ethcore.rs b/rpc/src/v1/traits/ethcore.rs index a0e12222a..4ce3eca59 100644 --- a/rpc/src/v1/traits/ethcore.rs +++ b/rpc/src/v1/traits/ethcore.rs @@ -21,21 +21,6 @@ use jsonrpc_core::*; /// Ethcore-specific rpc interface. pub trait Ethcore: Sized + Send + Sync + 'static { - /// Sets new minimal gas price for mined blocks. - fn set_min_gas_price(&self, _: Params) -> Result; - - /// Sets new gas floor target for mined blocks. - fn set_gas_floor_target(&self, _: Params) -> Result; - - /// Sets new extra data for mined blocks. - fn set_extra_data(&self, _: Params) -> Result; - - /// Sets new author for mined block. - fn set_author(&self, _: Params) -> Result; - - /// Sets the limits for transaction queue. - fn set_transactions_limit(&self, _: Params) -> Result; - /// Returns current transactions limit. fn transactions_limit(&self, _: Params) -> Result; @@ -75,11 +60,6 @@ pub trait Ethcore: Sized + Send + Sync + 'static { /// Should be used to convert object to io delegate. fn to_delegate(self) -> IoDelegate { let mut delegate = IoDelegate::new(Arc::new(self)); - delegate.add_method("ethcore_setMinGasPrice", Ethcore::set_min_gas_price); - delegate.add_method("ethcore_setGasFloorTarget", Ethcore::set_gas_floor_target); - delegate.add_method("ethcore_setExtraData", Ethcore::set_extra_data); - delegate.add_method("ethcore_setAuthor", Ethcore::set_author); - delegate.add_method("ethcore_setTransactionsLimit", Ethcore::set_transactions_limit); delegate.add_method("ethcore_extraData", Ethcore::extra_data); delegate.add_method("ethcore_gasFloorTarget", Ethcore::gas_floor_target); diff --git a/rpc/src/v1/traits/ethcore_set.rs b/rpc/src/v1/traits/ethcore_set.rs new file mode 100644 index 000000000..332c505b6 --- /dev/null +++ b/rpc/src/v1/traits/ethcore_set.rs @@ -0,0 +1,51 @@ +// 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 . + +//! Ethcore-specific rpc interface for operations altering the settings. + +use std::sync::Arc; +use jsonrpc_core::*; + +/// Ethcore-specific rpc interface for operations altering the settings. +pub trait EthcoreSet: Sized + Send + Sync + 'static { + + /// Sets new minimal gas price for mined blocks. + fn set_min_gas_price(&self, _: Params) -> Result; + + /// Sets new gas floor target for mined blocks. + fn set_gas_floor_target(&self, _: Params) -> Result; + + /// Sets new extra data for mined blocks. + fn set_extra_data(&self, _: Params) -> Result; + + /// Sets new author for mined block. + fn set_author(&self, _: Params) -> Result; + + /// Sets the limits for transaction queue. + fn set_transactions_limit(&self, _: Params) -> Result; + + /// Should be used to convert object to io delegate. + fn to_delegate(self) -> IoDelegate { + let mut delegate = IoDelegate::new(Arc::new(self)); + delegate.add_method("ethcore_setMinGasPrice", EthcoreSet::set_min_gas_price); + delegate.add_method("ethcore_setGasFloorTarget", EthcoreSet::set_gas_floor_target); + delegate.add_method("ethcore_setExtraData", EthcoreSet::set_extra_data); + delegate.add_method("ethcore_setAuthor", EthcoreSet::set_author); + delegate.add_method("ethcore_setTransactionsLimit", EthcoreSet::set_transactions_limit); + + delegate + } +} diff --git a/rpc/src/v1/traits/mod.rs b/rpc/src/v1/traits/mod.rs index d994ffc24..3ca11b654 100644 --- a/rpc/src/v1/traits/mod.rs +++ b/rpc/src/v1/traits/mod.rs @@ -21,6 +21,7 @@ pub mod eth; pub mod net; pub mod personal; pub mod ethcore; +pub mod ethcore_set; pub mod traces; pub mod rpc; @@ -29,6 +30,7 @@ pub use self::eth::{Eth, EthFilter, EthSigning}; pub use self::net::Net; pub use self::personal::{Personal, PersonalSigner}; pub use self::ethcore::Ethcore; +pub use self::ethcore_set::EthcoreSet; pub use self::traces::Traces; pub use self::rpc::Rpc; diff --git a/signer/src/ws_server/session.rs b/signer/src/ws_server/session.rs index 153bf6622..8cc3f5d07 100644 --- a/signer/src/ws_server/session.rs +++ b/signer/src/ws_server/session.rs @@ -77,7 +77,7 @@ impl ws::Handler for Session { // Check request origin and host header. if !origin_is_allowed(&self.self_origin, origin) && !origin_is_allowed(&self.self_origin, host) { warn!(target: "signer", "Blocked connection to Signer API from untrusted origin."); - return Ok(ws::Response::forbidden("You are not allowed to access system ui.".into())); + return Ok(ws::Response::forbidden(format!("You are not allowed to access system ui. Use: http://{}", self.self_origin))); } // Detect if it's a websocket request. @@ -108,7 +108,7 @@ impl ws::Handler for Session { let mut res = ws::Response::ok(f.content.into()); { let mut headers = res.headers_mut(); - headers.push(("Server".into(), b"Parity/SystemUI".to_vec())); + headers.push(("Server".into(), b"Parity/SignerUI".to_vec())); headers.push(("Connection".into(), b"Closed".to_vec())); headers.push(("Content-Length".into(), content_len.as_bytes().to_vec())); headers.push(("Content-Type".into(), f.mime.as_bytes().to_vec())); From b4b883b3418796b22380f89317c1637bc3561965 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Wed, 8 Jun 2016 17:17:37 +0400 Subject: [PATCH 3/3] keys import (#1240) * pattern importing * tests for import * cli options for account import * [options] for import also * removed globbing * removed glob crate refs --- parity/cli.rs | 4 +- parity/main.rs | 5 +++ util/res/pat/p1.json | 21 ++++++++++ util/res/pat/p2.json | 21 ++++++++++ util/src/keys/geth_import.rs | 75 ++++++++++++++++++++++++++++++++++-- util/src/keys/mod.rs | 1 + 6 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 util/res/pat/p1.json create mode 100644 util/res/pat/p2.json diff --git a/parity/cli.rs b/parity/cli.rs index 0b3c96c55..b8aa88299 100644 --- a/parity/cli.rs +++ b/parity/cli.rs @@ -23,7 +23,8 @@ Parity. Ethereum Client. Usage: parity daemon [options] - parity account (new | list) [options] + parity account (new | list ) [options] + parity account import ... [options] parity import [ ] [options] parity export [ ] [options] parity signer new-token [options] @@ -212,6 +213,7 @@ pub struct Args { pub cmd_new_token: bool, pub arg_pid_file: String, pub arg_file: Option, + pub arg_path: Vec, pub flag_chain: String, pub flag_db_path: String, pub flag_identity: String, diff --git a/parity/main.rs b/parity/main.rs index 016b20117..58d5fe18e 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -482,6 +482,11 @@ fn execute_account_cli(conf: Configuration) { for &(addr, _) in &secret_store.accounts().unwrap() { println!("{:?}", addr); } + return; + } + if conf.args.cmd_import { + let imported = util::keys::import_keys_paths(&mut secret_store, &conf.args.arg_path).unwrap(); + println!("Imported {} keys", imported); } } diff --git a/util/res/pat/p1.json b/util/res/pat/p1.json new file mode 100644 index 000000000..afc376774 --- /dev/null +++ b/util/res/pat/p1.json @@ -0,0 +1,21 @@ +{ + "address": "3f49624084b67849c7b4e805c5988c21a430f9d9", + "Crypto": { + "cipher": "aes-128-ctr", + "ciphertext": "9f27e3dd4fc73e7103ed61e5493662189a3eb52223ae49e3d1deacc04c889eae", + "cipherparams": { + "iv": "457494bf05f2618c397dc74dbb5181c0" + }, + "kdf": "scrypt", + "kdfparams": { + "dklen": 32, + "n": 262144, + "p": 1, + "r": 8, + "salt": "db14edb18c41ee7f5ec4397df89c3a2ae4d0af60884c52bb54ce490574f8df33" + }, + "mac": "572d24532438d31fdf513c744a3ff26c933ffda5744ee42bc71661cbe3f2112e" + }, + "id": "62a0ad73-556d-496a-8e1c-0783d30d3ace", + "version": 3 +} diff --git a/util/res/pat/p2.json b/util/res/pat/p2.json new file mode 100644 index 000000000..b14922037 --- /dev/null +++ b/util/res/pat/p2.json @@ -0,0 +1,21 @@ +{ + "address": "5ba4dcf897e97c2bdf8315b9ef26c13c085988cf", + "Crypto": { + "cipher": "aes-128-ctr", + "ciphertext": "d4a08ec930163778273920f6ad1d49b71836337be6fd9863993ac700a612fddd", + "cipherparams": { + "iv": "89ce5ec129fc27cd5bcbeb8c92bdad50" + }, + "kdf": "scrypt", + "kdfparams": { + "dklen": 32, + "n": 262144, + "p": 1, + "r": 8, + "salt": "612ab108dc37e69ee8af37a7b24bf7f2234086d7bbf945bacdeccce331f7f84a" + }, + "mac": "4152caa7444e06784223d735cea80cd2690b4c587ad8db3d5529442227b25695" + }, + "id": "35086353-fb12-4029-b56b-033cd61ce35b", + "version": 3 +} diff --git a/util/src/keys/geth_import.rs b/util/src/keys/geth_import.rs index a72c570fa..509ebc89f 100644 --- a/util/src/keys/geth_import.rs +++ b/util/src/keys/geth_import.rs @@ -80,9 +80,10 @@ pub fn import_geth_key(secret_store: &mut SecretStore, geth_keyfile_path: &Path) } /// Imports all geth keys in the directory -pub fn import_geth_keys(secret_store: &mut SecretStore, geth_keyfiles_directory: &Path) -> Result<(), ImportError> { +pub fn import_geth_keys(secret_store: &mut SecretStore, geth_keyfiles_directory: &Path) -> Result { use std::path::PathBuf; let geth_files = try!(enumerate_geth_keys(geth_keyfiles_directory)); + let mut total = 0; for &(ref address, ref file_path) in &geth_files { let mut path = PathBuf::new(); path.push(geth_keyfiles_directory); @@ -90,18 +91,45 @@ pub fn import_geth_keys(secret_store: &mut SecretStore, geth_keyfiles_directory: if let Err(e) = import_geth_key(secret_store, Path::new(&path)) { warn!("Skipped geth address {}, error importing: {:?}", address, e) } + else { total = total + 1} } - Ok(()) + Ok(total) } /// Gets the default geth keystore directory. -/// -/// Based on https://github.com/ethereum/go-ethereum/blob/e553215/common/path.go#L75 pub fn keystore_dir(is_testnet: bool) -> PathBuf { path::ethereum::with_default(if is_testnet {"testnet/keystore"} else {"keystore"}) } +/// Imports key(s) from provided file/directory +pub fn import_keys_path(secret_store: &mut SecretStore, path: &str) -> Result { + // check if it is just one file or directory + if let Ok(meta) = fs::metadata(path) { + if meta.is_file() { + try!(import_geth_key(secret_store, Path::new(path))); + return Ok(1); + } + else if meta.is_dir() { + return Ok(try!(fs::read_dir(path)).fold( + 0, + |total, p| + total + + match p { + Ok(dir_entry) => import_keys_path(secret_store, dir_entry.path().to_str().unwrap()).unwrap_or_else(|_| 0), + Err(e) => { warn!("Error importing dir entry: {:?}", e); 0 }, + } + )) + } + } + Ok(0) +} + +/// Imports all keys from list of provided files/directories +pub fn import_keys_paths(secret_store: &mut SecretStore, path: &[String]) -> Result { + Ok(path.iter().fold(0, |total, ref p| total + import_keys_path(secret_store, &p).unwrap_or_else(|_| 0))) +} + #[cfg(test)] mod tests { use super::*; @@ -115,10 +143,21 @@ mod tests { } } + fn pat_path() -> &'static str { + match ::std::fs::metadata("res") { + Ok(_) => "res/pat", + Err(_) => "util/res/pat" + } + } + fn test_path_param(param_val: &'static str) -> String { test_path().to_owned() + param_val } + fn pat_path_param(param_val: &'static str) -> String { + pat_path().to_owned() + param_val + } + #[test] fn can_enumerate() { let keys = enumerate_geth_keys(Path::new(test_path())).unwrap(); @@ -191,4 +230,32 @@ mod tests { assert!(val.is_ok()); assert_eq!(32, val.unwrap().len()); } + + #[test] + fn can_import_by_filename() { + let temp = ::devtools::RandomTempPath::create_dir(); + let mut secret_store = SecretStore::new_in(temp.as_path()); + + let amount = import_keys_path(&mut secret_store, &pat_path_param("/p1.json")).unwrap(); + assert_eq!(1, amount); + } + + #[test] + fn can_import_by_dir() { + let temp = ::devtools::RandomTempPath::create_dir(); + let mut secret_store = SecretStore::new_in(temp.as_path()); + + let amount = import_keys_path(&mut secret_store, pat_path()).unwrap(); + assert_eq!(2, amount); + } + + #[test] + fn can_import_mulitple() { + let temp = ::devtools::RandomTempPath::create_dir(); + let mut secret_store = SecretStore::new_in(temp.as_path()); + + let amount = import_keys_paths(&mut secret_store, &[pat_path_param("/p1.json"), pat_path_param("/p2.json")]).unwrap(); + assert_eq!(2, amount); + } + } diff --git a/util/src/keys/mod.rs b/util/src/keys/mod.rs index 38fa51eca..39c39aeef 100644 --- a/util/src/keys/mod.rs +++ b/util/src/keys/mod.rs @@ -23,3 +23,4 @@ mod test_account_provider; pub use self::store::AccountProvider; pub use self::test_account_provider::{TestAccount, TestAccountProvider}; +pub use self::geth_import::import_keys_paths;