diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index a3a379463..02cd6678b 100644 --- a/ethcore/src/error.rs +++ b/ethcore/src/error.rs @@ -65,6 +65,10 @@ pub enum ExecutionError { #[derive(Debug)] /// Errors concerning transaction processing. pub enum TransactionError { + /// Transaction is already imported to the queue + AlreadyImported, + /// Transaction is not valid anymore (state already has higher nonce) + Old, /// Transaction's gas price is below threshold. InsufficientGasPrice { /// Minimal expected gas price diff --git a/miner/src/lib.rs b/miner/src/lib.rs index c70caad66..b79a13e24 100644 --- a/miner/src/lib.rs +++ b/miner/src/lib.rs @@ -79,7 +79,7 @@ pub trait MinerService : Send + Sync { fn status(&self) -> MinerStatus; /// Imports transactions to transaction queue. - fn import_transactions(&self, transactions: Vec, fetch_account: T) -> Result<(), Error> + fn import_transactions(&self, transactions: Vec, fetch_account: T) -> Vec> where T: Fn(&Address) -> AccountDetails; /// Returns hashes of transactions currently in pending diff --git a/miner/src/miner.rs b/miner/src/miner.rs index be90b15af..858df7c40 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -135,7 +135,7 @@ impl MinerService for Miner { } } - fn import_transactions(&self, transactions: Vec, fetch_account: T) -> Result<(), Error> + fn import_transactions(&self, transactions: Vec, fetch_account: T) -> Vec> where T: Fn(&Address) -> AccountDetails { let mut transaction_queue = self.transaction_queue.lock().unwrap(); transaction_queue.add_all(transactions, fetch_account) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 107904114..4b76fcbb2 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -335,12 +335,12 @@ impl TransactionQueue { } /// Adds all signed transactions to queue to be verified and imported - pub fn add_all(&mut self, txs: Vec, fetch_account: T) -> Result<(), Error> + pub fn add_all(&mut self, txs: Vec, fetch_account: T) -> Vec> where T: Fn(&Address) -> AccountDetails { - for tx in txs.into_iter() { - try!(self.add(tx, &fetch_account)); - } - Ok(()) + + txs.into_iter() + .map(|tx| self.add(tx, &fetch_account)) + .collect() } /// Add signed transaction to queue to be verified and imported @@ -387,8 +387,7 @@ impl TransactionQueue { })); } - self.import_tx(vtx, account.nonce); - Ok(()) + self.import_tx(vtx, account.nonce).map_err(Error::Transaction) } /// Removes all transactions identified by hashes given in slice @@ -542,12 +541,14 @@ impl TransactionQueue { /// /// It ignores transactions that has already been imported (same `hash`) and replaces the transaction /// iff `(address, nonce)` is the same but `gas_price` is higher. - fn import_tx(&mut self, tx: VerifiedTransaction, state_nonce: U256) { + /// + /// Returns `true` when transaction was imported successfuly + fn import_tx(&mut self, tx: VerifiedTransaction, state_nonce: U256) -> Result<(), TransactionError> { if self.by_hash.get(&tx.hash()).is_some() { // Transaction is already imported. trace!(target: "miner", "Dropping already imported transaction: {:?}", tx.hash()); - return; + return Err(TransactionError::AlreadyImported); } @@ -564,11 +565,11 @@ impl TransactionQueue { // We have a gap - put to future Self::replace_transaction(tx, next_nonce, &mut self.future, &mut self.by_hash); self.future.enforce_limit(&mut self.by_hash); - return; + return Ok(()); } else if nonce < state_nonce { // Droping transaction trace!(target: "miner", "Dropping old transaction: {:?} (nonce: {} < {})", tx.hash(), nonce, next_nonce); - return; + return Err(TransactionError::Old); } Self::replace_transaction(tx, state_nonce, &mut self.current, &mut self.by_hash); @@ -578,6 +579,7 @@ impl TransactionQueue { self.current.enforce_limit(&mut self.by_hash); trace!(target: "miner", "status: {:?}", self.status()); + Ok(()) } /// Replaces transaction in given set (could be `future` or `current`). @@ -1010,7 +1012,7 @@ mod test { let fetch_last_nonce = |_a: &Address| AccountDetails{ nonce: last_nonce, balance: !U256::zero() }; // when - txq.add(tx, &fetch_last_nonce).unwrap(); + txq.add(tx, &fetch_last_nonce).unwrap_err(); // then let stats = txq.status(); @@ -1030,7 +1032,7 @@ mod test { assert_eq!(txq.status().pending, 0); // when - txq.add(tx2.clone(), &nonce).unwrap(); + txq.add(tx2.clone(), &nonce).unwrap_err(); // then let stats = txq.status(); diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 7ecfb86de..5cd1b2966 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -385,7 +385,7 @@ impl Eth for EthClient nonce: client.nonce(a), balance: client.balance(a), }); - match import { + match import.into_iter().collect::, _>>() { Ok(_) => to_value(&hash), Err(e) => { warn!("Error sending transaction: {:?}", e); diff --git a/sync/src/chain.rs b/sync/src/chain.rs index d15d1e79c..52bfa0878 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -951,9 +951,13 @@ impl ChainSync { transactions.push(tx); } let chain = io.chain(); - let fetch_nonce = |a: &Address| chain.nonce(a); - let res = self.miner.import_transactions(transactions, fetch_nonce); - if res.is_ok() { + let fetch_account = |a: &Address| AccountDetails { + nonce: chain.nonce(a), + balance: chain.balance(a), + }; + let res = self.miner.import_transactions(transactions, fetch_account); + let any_transaction_imported = res.into_iter().any(|r| r.is_ok()); + if any_transaction_imported { self.miner.update_sealing(io.chain()); } Ok(())