Merge pull request #1525 from ethcore/client-api-refact2

Client API refactoring - limiting errors to crate-level error types

ethcore/src/client/client.rs

@@ -21,7 +21,7 @@ use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrdering};
 use util::*;
 use util::panics::*;
 use views::BlockView;
-use error::{Error, ImportError, ExecutionError, BlockError, ImportResult};
+use error::{ImportError, ExecutionError, BlockError, ImportResult};
 use header::{BlockNumber};
 use state::State;
 use spec::Spec;
@@ -38,7 +38,9 @@ use filter::Filter;
 use log_entry::LocalizedLogEntry;
 use block_queue::{BlockQueue, BlockQueueInfo};
 use blockchain::{BlockChain, BlockProvider, TreeRoute, ImportRoute};
-use client::{BlockID, TransactionID, UncleID, TraceId, ClientConfig, DatabaseCompactionProfile, BlockChainClient, MiningBlockChainClient, TraceFilter, CallAnalytics};
+use client::{BlockID, TransactionID, UncleID, TraceId, ClientConfig, DatabaseCompactionProfile,
+	BlockChainClient, MiningBlockChainClient, TraceFilter, CallAnalytics, TransactionImportError,
+	BlockImportError, TransactionImportResult};
 use client::Error as ClientError;
 use env_info::EnvInfo;
 use executive::{Executive, Executed, TransactOptions, contract_address};
@@ -49,7 +51,7 @@ use trace;
 pub use types::blockchain_info::BlockChainInfo;
 pub use types::block_status::BlockStatus;
 use evm::Factory as EvmFactory;
-use miner::{Miner, MinerService, TransactionImportResult, AccountDetails};
+use miner::{Miner, MinerService, AccountDetails};
 
 const MAX_TX_QUEUE_SIZE: usize = 4096;
 
@@ -654,17 +656,17 @@ impl BlockChainClient for Client {
 		self.chain.block_receipts(hash).map(|receipts| rlp::encode(&receipts).to_vec())
 	}
 
-	fn import_block(&self, bytes: Bytes) -> ImportResult {
+	fn import_block(&self, bytes: Bytes) -> Result<H256, BlockImportError> {
 		{
 			let header = BlockView::new(&bytes).header_view();
 			if self.chain.is_known(&header.sha3()) {
-				return Err(ImportError::AlreadyInChain.into());
+				return Err(BlockImportError::Import(ImportError::AlreadyInChain));
 			}
 			if self.block_status(BlockID::Hash(header.parent_hash())) == BlockStatus::Unknown {
-				return Err(BlockError::UnknownParent(header.parent_hash()).into());
+				return Err(BlockImportError::Block(BlockError::UnknownParent(header.parent_hash())));
 			}
 		}
-		self.block_queue.import_block(bytes)
+		Ok(try!(self.block_queue.import_block(bytes)))
 	}
 
 	fn queue_info(&self) -> BlockQueueInfo {
@@ -778,12 +780,16 @@ impl BlockChainClient for Client {
 		self.build_last_hashes(self.chain.best_block_hash())
 	}
 
-	fn import_transactions(&self, transactions: Vec<SignedTransaction>) -> Vec<Result<TransactionImportResult, Error>> {
+	fn import_transactions(&self, transactions: Vec<SignedTransaction>) -> Vec<Result<TransactionImportResult, TransactionImportError>> {
 		let fetch_account = |a: &Address| AccountDetails {
 			nonce: self.latest_nonce(a),
 			balance: self.latest_balance(a),
 		};
-		self.miner.import_transactions(self, transactions, fetch_account)
+
+		self.miner.import_transactions(self, transactions, &fetch_account)
+			.into_iter()
+			.map(|res| res.map_err(|e| e.into()))
+			.collect()
 	}
 
 	fn queue_transactions(&self, transactions: Vec<Bytes>) {

ethcore/src/client/mod.rs

@@ -47,8 +47,8 @@ use error::{ImportResult, ExecutionError};
 use receipt::LocalizedReceipt;
 use trace::LocalizedTrace;
 use evm::Factory as EvmFactory;
-use miner::{TransactionImportResult};
+pub use block_import_error::BlockImportError;
-use error::Error as EthError;
+pub use transaction_import::{TransactionImportResult, TransactionImportError};
 
 /// Options concerning what analytics we run on the call.
 #[derive(Eq, PartialEq, Default, Clone, Copy, Debug)]
@@ -145,7 +145,7 @@ pub trait BlockChainClient : Sync + Send {
 	fn block_receipts(&self, hash: &H256) -> Option<Bytes>;
 
 	/// Import a block into the blockchain.
-	fn import_block(&self, bytes: Bytes) -> ImportResult;
+	fn import_block(&self, bytes: Bytes) -> Result<H256, BlockImportError>;
 
 	/// Get block queue information.
 	fn queue_info(&self) -> BlockQueueInfo;
@@ -188,7 +188,7 @@ pub trait BlockChainClient : Sync + Send {
 	fn last_hashes(&self) -> LastHashes;
 
 	/// import transactions from network/other 3rd party
-	fn import_transactions(&self, transactions: Vec<SignedTransaction>) -> Vec<Result<TransactionImportResult, EthError>>;
+	fn import_transactions(&self, transactions: Vec<SignedTransaction>) -> Vec<Result<TransactionImportResult, TransactionImportError>>;
 
 	/// Queue transactions for importing.
 	fn queue_transactions(&self, transactions: Vec<Bytes>);

ethcore/src/client/test_client.rs

@@ -20,7 +20,9 @@ use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrder};
 use util::*;
 use transaction::{Transaction, LocalizedTransaction, SignedTransaction, Action};
 use blockchain::TreeRoute;
-use client::{BlockChainClient, MiningBlockChainClient, BlockChainInfo, BlockStatus, BlockID, TransactionID, UncleID, TraceId, TraceFilter, LastHashes, CallAnalytics};
+use client::{BlockChainClient, MiningBlockChainClient, BlockChainInfo, BlockStatus, BlockID,
+	TransactionID, UncleID, TraceId, TraceFilter, LastHashes, CallAnalytics,
+	TransactionImportError, BlockImportError};
 use header::{Header as BlockHeader, BlockNumber};
 use filter::Filter;
 use log_entry::LocalizedLogEntry;
@@ -38,7 +40,6 @@ use error::{ExecutionError};
 use trace::LocalizedTrace;
 
 use miner::{TransactionImportResult, AccountDetails};
-use error::Error as EthError;
 
 /// Test client.
 pub struct TestBlockChainClient {
@@ -402,7 +403,7 @@ impl BlockChainClient for TestBlockChainClient {
 		None
 	}
 
-	fn import_block(&self, b: Bytes) -> ImportResult {
+	fn import_block(&self, b: Bytes) -> Result<H256, BlockImportError> {
 		let header = Rlp::new(&b).val_at::<BlockHeader>(0);
 		let h = header.hash();
 		let number: usize = header.number as usize;
@@ -487,7 +488,7 @@ impl BlockChainClient for TestBlockChainClient {
 		unimplemented!();
 	}
 
-	fn import_transactions(&self, transactions: Vec<SignedTransaction>) -> Vec<Result<TransactionImportResult, EthError>> {
+	fn import_transactions(&self, transactions: Vec<SignedTransaction>) -> Vec<Result<TransactionImportResult, TransactionImportError>> {
 		let nonces = self.nonces.read().unwrap();
 		let balances = self.balances.read().unwrap();
 		let fetch_account = |a: &Address| AccountDetails {
@@ -496,6 +497,9 @@ impl BlockChainClient for TestBlockChainClient {
 		};
 
 		self.miner.import_transactions(self, transactions, &fetch_account)
+			.into_iter()
+			.map(|res| res.map_err(|e| e.into()))
+			.collect()
 	}
 
 	fn queue_transactions(&self, transactions: Vec<Bytes>) {

ethcore/src/error.rs

@@ -20,10 +20,11 @@ use util::*;
 use header::BlockNumber;
 use basic_types::LogBloom;
 use client::Error as ClientError;
-
 pub use types::executed::ExecutionError;
+use ipc::binary::{BinaryConvertError, BinaryConvertable};
+use types::block_import_error::BlockImportError;
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone)]
 /// Errors concerning transaction processing.
 pub enum TransactionError {
 	/// Transaction is already imported to the queue
@@ -312,6 +313,21 @@ impl From<TrieError> for Error {
 	}
 }
 
+impl From<BlockImportError> for Error {
+	fn from(err: BlockImportError) -> Error {
+		match err {
+			BlockImportError::Block(e) => Error::Block(e),
+			BlockImportError::Import(e) => Error::Import(e),
+			BlockImportError::Other(s) => Error::Util(UtilError::SimpleString(s)),
+		}
+	}
+}
+
+binary_fixed_size!(BlockError);
+binary_fixed_size!(ImportError);
+binary_fixed_size!(TransactionError);
+
+
 // TODO: uncomment below once https://github.com/rust-lang/rust/issues/27336 sorted.
 /*#![feature(concat_idents)]
 macro_rules! assimilate {

ethcore/src/miner/miner.rs

@@ -29,8 +29,10 @@ use transaction::SignedTransaction;
 use receipt::{Receipt};
 use spec::Spec;
 use engine::Engine;
-use miner::{MinerService, MinerStatus, TransactionQueue, AccountDetails, TransactionImportResult, TransactionOrigin};
+use miner::{MinerService, MinerStatus, TransactionQueue, AccountDetails, TransactionOrigin};
 use miner::work_notify::WorkPoster;
+use client::TransactionImportResult;
+
 
 /// Different possible definitions for pending transaction set.
 #[derive(Debug)]

ethcore/src/miner/mod.rs

@@ -47,9 +47,10 @@ mod external;
 mod transaction_queue;
 mod work_notify;
 
-pub use self::transaction_queue::{TransactionQueue, AccountDetails, TransactionImportResult, TransactionOrigin};
+pub use self::transaction_queue::{TransactionQueue, AccountDetails, TransactionOrigin};
 pub use self::miner::{Miner, MinerOptions, PendingSet};
 pub use self::external::{ExternalMiner, ExternalMinerService};
+pub use client::TransactionImportResult;
 
 use std::collections::BTreeMap;
 use util::{H256, U256, Address, Bytes};

ethcore/src/miner/transaction_queue.rs

@@ -90,6 +90,7 @@ use util::hash::{Address, H256};
 use util::table::*;
 use transaction::*;
 use error::{Error, TransactionError};
+use client::TransactionImportResult;
 
 /// Transaction origin
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -309,15 +310,6 @@ pub struct TransactionQueueStatus {
 	pub future: usize,
 }
 
-#[derive(Debug, PartialEq)]
-/// Represents the result of importing transaction.
-pub enum TransactionImportResult {
-	/// Transaction was imported to current queue.
-	Current,
-	/// Transaction was imported to future queue.
-	Future
-}
-
 /// Details of account
 pub struct AccountDetails {
 	/// Most recent account nonce
@@ -812,6 +804,7 @@ mod test {
 	use error::{Error, TransactionError};
 	use super::*;
 	use super::{TransactionSet, TransactionOrder, VerifiedTransaction};
+	use client::TransactionImportResult;
 
 	fn unwrap_tx_err(err: Result<TransactionImportResult, Error>) -> TransactionError {
 		match err.unwrap_err() {

ethcore/src/types/block_import_error.rs

@@ -0,0 +1,44 @@
+// 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 <http://www.gnu.org/licenses/>.
+
+//! Block import error related types
+
+use std::mem;
+use ipc::binary::BinaryConvertError;
+use std::collections::VecDeque;
+use error::{ImportError, BlockError, Error};
+use std::convert::From;
+
+/// Error dedicated to import block function
+#[derive(Binary, Debug)]
+pub enum BlockImportError {
+	/// Import error
+	Import(ImportError),
+	/// Block error
+	Block(BlockError),
+	/// Other error
+	Other(String),
+}
+
+impl From<Error> for BlockImportError {
+	fn from(e: Error) -> Self {
+		match e {
+			Error::Block(block_error) => BlockImportError::Block(block_error),
+			Error::Import(import_error) => BlockImportError::Import(import_error),
+			_ => BlockImportError::Other(format!("other block import error: {:?}", e)),
+		}
+	}
+}

ethcore/src/types/mod.rs.in

@@ -25,3 +25,5 @@ pub mod executed;
 pub mod block_status;
 pub mod account_diff;
 pub mod state_diff;
+pub mod transaction_import;
+pub mod block_import_error;

ethcore/src/types/transaction_import.rs

@@ -0,0 +1,52 @@
+// 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 <http://www.gnu.org/licenses/>.
+
+//! Transaction import result related types
+
+use ipc::binary::{BinaryConvertError, BinaryConvertable};
+use std::collections::VecDeque;
+use error::{TransactionError, Error};
+use std::mem;
+use util::Populatable;
+
+#[derive(Debug, Clone, PartialEq)]
+/// Represents the result of importing transaction.
+pub enum TransactionImportResult {
+	/// Transaction was imported to current queue.
+	Current,
+	/// Transaction was imported to future queue.
+	Future
+}
+
+binary_fixed_size!(TransactionImportResult);
+
+/// Api-level error for transaction import
+#[derive(Debug, Clone, Binary)]
+pub enum TransactionImportError {
+	/// Transaction error
+	Transaction(TransactionError),
+	/// Other error
+	Other(String),
+}
+
+impl From<Error> for TransactionImportError {
+	fn from(e: Error) -> Self {
+		match e {
+			Error::Transaction(transaction_error) => TransactionImportError::Transaction(transaction_error),
+			_ => TransactionImportError::Other(format!("other block import error: {:?}", e)),
+		}
+	}
+}

parity/main.rs

@@ -82,8 +82,8 @@ use rustc_serialize::hex::FromHex;
 use ctrlc::CtrlC;
 use util::{H256, ToPretty, NetworkConfiguration, PayloadInfo, Bytes, UtilError, paint, Colour, version};
 use util::panics::{MayPanic, ForwardPanic, PanicHandler};
-use ethcore::client::{BlockID, BlockChainClient, ClientConfig, get_db_path};
+use ethcore::client::{BlockID, BlockChainClient, ClientConfig, get_db_path, BlockImportError};
-use ethcore::error::{Error, ImportError};
+use ethcore::error::{ImportError};
 use ethcore::service::ClientService;
 use ethcore::spec::Spec;
 use ethsync::EthSync;
@@ -458,7 +458,7 @@ fn execute_import(conf: Configuration) {
 		while client.queue_info().is_full() { sleep(Duration::from_secs(1)); }
 		match client.import_block(bytes) {
 			Ok(_) => {}
-			Err(Error::Import(ImportError::AlreadyInChain)) => { trace!("Skipping block already in chain."); }
+			Err(BlockImportError::Import(ImportError::AlreadyInChain)) => { trace!("Skipping block already in chain."); }
 			Err(e) => die!("Cannot import block: {:?}", e)
 		}
 		informant.tick(client.deref(), None);

sync/src/chain.rs

@@ -93,7 +93,7 @@ use util::*;
 use std::mem::{replace};
 use ethcore::views::{HeaderView, BlockView};
 use ethcore::header::{BlockNumber, Header as BlockHeader};
-use ethcore::client::{BlockChainClient, BlockStatus, BlockID, BlockChainInfo};
+use ethcore::client::{BlockChainClient, BlockStatus, BlockID, BlockChainInfo, BlockImportError};
 use ethcore::error::*;
 use ethcore::block::Block;
 use io::SyncIo;
@@ -544,10 +544,10 @@ impl ChainSync {
 			peer.latest_number = Some(header.number());
 		}
 		match io.chain().import_block(block_rlp.as_raw().to_vec()) {
-			Err(Error::Import(ImportError::AlreadyInChain)) => {
+			Err(BlockImportError::Import(ImportError::AlreadyInChain)) => {
 				trace!(target: "sync", "New block already in chain {:?}", h);
 			},
-			Err(Error::Import(ImportError::AlreadyQueued)) => {
+			Err(BlockImportError::Import(ImportError::AlreadyQueued)) => {
 				trace!(target: "sync", "New block already queued {:?}", h);
 			},
 			Ok(_) => {
@@ -557,7 +557,7 @@ impl ChainSync {
 				}
 				trace!(target: "sync", "New block queued {:?} ({})", h, header.number);
 			},
-			Err(Error::Block(BlockError::UnknownParent(p))) => {
+			Err(BlockImportError::Block(BlockError::UnknownParent(p))) => {
 				unknown = true;
 				trace!(target: "sync", "New block with unknown parent ({:?}) {:?}", p, h);
 			},
@@ -841,11 +841,11 @@ impl ChainSync {
 			}
 
 			match io.chain().import_block(block) {
-				Err(Error::Import(ImportError::AlreadyInChain)) => {
+				Err(BlockImportError::Import(ImportError::AlreadyInChain)) => {
 					trace!(target: "sync", "Block already in chain {:?}", h);
 					self.block_imported(&h, number, &parent);
 				},
-				Err(Error::Import(ImportError::AlreadyQueued)) => {
+				Err(BlockImportError::Import(ImportError::AlreadyQueued)) => {
 					trace!(target: "sync", "Block already queued {:?}", h);
 					self.block_imported(&h, number, &parent);
 				},
@@ -854,7 +854,7 @@ impl ChainSync {
 					imported.insert(h.clone());
 					self.block_imported(&h, number, &parent);
 				},
-				Err(Error::Block(BlockError::UnknownParent(_))) if self.state == SyncState::NewBlocks => {
+				Err(BlockImportError::Block(BlockError::UnknownParent(_))) if self.state == SyncState::NewBlocks => {
 					trace!(target: "sync", "Unknown new block parent, restarting sync");
 					break;
 				},

util/src/error.rs

@@ -101,7 +101,7 @@ impl<T: fmt::Debug + fmt::Display> fmt::Display for Mismatch<T> {
 	}
 }
 
-#[derive(Debug, PartialEq, Eq)]
+#[derive(Debug, PartialEq, Eq, Clone)]
 /// Error indicating value found is outside of a valid range.
 pub struct OutOfBounds<T: fmt::Debug> {
 	/// Minimum allowed value.