diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index 72127c754..482a8de01 100644 --- a/ethcore/src/error.rs +++ b/ethcore/src/error.rs @@ -70,7 +70,14 @@ pub enum TransactionError { /// Minimal expected gas price minimal: U256, /// Transaction gas price - got: U256 + got: U256, + }, + /// Sender doesn't have enough funds to pay for this transaction + InsufficientBalance { + /// Senders balance + balance: U256, + /// Transaction cost + cost: U256, }, /// Transaction's gas limit (aka gas) is invalid. InvalidGasLimit(OutOfBounds), diff --git a/miner/src/lib.rs b/miner/src/lib.rs index a431bd44e..3c9775268 100644 --- a/miner/src/lib.rs +++ b/miner/src/lib.rs @@ -62,11 +62,11 @@ extern crate rayon; mod miner; mod transaction_queue; -pub use transaction_queue::TransactionQueue; +pub use transaction_queue::{TransactionQueue, AccountDetails}; pub use miner::{Miner}; use std::sync::Mutex; -use util::{H256, U256, Address, Bytes}; +use util::{H256, Address, Bytes}; use ethcore::client::{BlockChainClient}; use ethcore::block::{ClosedBlock}; use ethcore::error::{Error}; @@ -79,8 +79,8 @@ pub trait MinerService : Send + Sync { fn status(&self) -> MinerStatus; /// Imports transactions to transaction queue. - fn import_transactions(&self, transactions: Vec, fetch_nonce: T) -> Result<(), Error> - where T: Fn(&Address) -> U256; + fn import_transactions(&self, transactions: Vec, fetch_account: T) -> Result<(), Error> + where T: Fn(&Address) -> AccountDetails; /// Returns hashes of transactions currently in pending fn pending_transactions_hashes(&self) -> Vec; diff --git a/miner/src/miner.rs b/miner/src/miner.rs index 4652ab3db..27cf2ded2 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -26,7 +26,7 @@ use ethcore::client::{BlockChainClient, BlockId}; use ethcore::block::{ClosedBlock}; use ethcore::error::{Error}; use ethcore::transaction::SignedTransaction; -use super::{MinerService, MinerStatus, TransactionQueue}; +use super::{MinerService, MinerStatus, TransactionQueue, AccountDetails}; /// Keeps track of transactions using priority queue and holds currently mined block. pub struct Miner { @@ -72,7 +72,7 @@ impl Miner { /// Get the extra_data that we will seal blocks wuth. fn gas_floor_target(&self) -> U256 { - self.gas_floor_target.read().unwrap().clone() + *self.gas_floor_target.read().unwrap() } /// Set the author that we will seal blocks as. @@ -111,10 +111,10 @@ impl MinerService for Miner { } } - fn import_transactions(&self, transactions: Vec, fetch_nonce: T) -> Result<(), Error> - where T: Fn(&Address) -> U256 { + fn import_transactions(&self, transactions: Vec, fetch_account: T) -> Result<(), Error> + where T: Fn(&Address) -> AccountDetails { let mut transaction_queue = self.transaction_queue.lock().unwrap(); - transaction_queue.add_all(transactions, fetch_nonce) + transaction_queue.add_all(transactions, fetch_account) } fn pending_transactions_hashes(&self) -> Vec { @@ -185,7 +185,10 @@ impl MinerService for Miner { let _sender = tx.sender(); } let mut transaction_queue = self.transaction_queue.lock().unwrap(); - let _ = transaction_queue.add_all(txs, |a| chain.nonce(a)); + let _ = transaction_queue.add_all(txs, |a| AccountDetails { + nonce: chain.nonce(a), + balance: chain.balance(a) + }); }); } // First import all transactions and after that remove old ones @@ -207,7 +210,10 @@ impl MinerService for Miner { in_chain.for_each(|txs| { let hashes = txs.iter().map(|tx| tx.hash()).collect::>(); let mut transaction_queue = self.transaction_queue.lock().unwrap(); - transaction_queue.remove_all(&hashes, |a| chain.nonce(a)); + transaction_queue.remove_all(&hashes, |a| AccountDetails { + nonce: chain.nonce(a), + balance: chain.balance(a) + }); }); } diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 4b365bba2..ca4a269ae 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -232,8 +232,6 @@ impl TransactionSet { } } -// Will be used when rpc merged -#[allow(dead_code)] #[derive(Debug)] /// Current status of the queue pub struct TransactionQueueStatus { @@ -243,6 +241,14 @@ pub struct TransactionQueueStatus { pub future: usize, } +/// Details of account +pub struct AccountDetails { + /// Most recent account nonce + pub nonce: U256, + /// Current account balance + pub balance: U256, +} + /// TransactionQueue implementation pub struct TransactionQueue { /// Gas Price threshold for transactions that can be imported to this queue (defaults to 0) @@ -308,49 +314,63 @@ impl TransactionQueue { } /// Adds all signed transactions to queue to be verified and imported - pub fn add_all(&mut self, txs: Vec, fetch_nonce: T) -> Result<(), Error> - where T: Fn(&Address) -> U256 { + pub fn add_all(&mut self, txs: Vec, fetch_account: T) -> Result<(), Error> + where T: Fn(&Address) -> AccountDetails { for tx in txs.into_iter() { - try!(self.add(tx, &fetch_nonce)); + try!(self.add(tx, &fetch_account)); } Ok(()) } /// Add signed transaction to queue to be verified and imported - pub fn add(&mut self, tx: SignedTransaction, fetch_nonce: &T) -> Result<(), Error> - where T: Fn(&Address) -> U256 { + pub fn add(&mut self, tx: SignedTransaction, fetch_account: &T) -> Result<(), Error> + where T: Fn(&Address) -> AccountDetails { + + trace!(target: "miner", "Importing: {:?}", tx.hash()); if tx.gas_price < self.minimal_gas_price { - trace!(target: "sync", + trace!(target: "miner", "Dropping transaction below minimal gas price threshold: {:?} (gp: {} < {})", tx.hash(), tx.gas_price, self.minimal_gas_price ); return Err(Error::Transaction(TransactionError::InsufficientGasPrice{ minimal: self.minimal_gas_price, - got: tx.gas_price + got: tx.gas_price, })); } - self.import_tx(try!(VerifiedTransaction::new(tx)), fetch_nonce); + let vtx = try!(VerifiedTransaction::new(tx)); + let account = fetch_account(&vtx.sender()); + + if account.balance < vtx.transaction.value { + trace!(target: "miner", "Dropping transaction without sufficient balance: {:?} ({} < {})", + vtx.hash(), account.balance, vtx.transaction.value); + return Err(Error::Transaction(TransactionError::InsufficientBalance { + cost: vtx.transaction.value, + balance: account.balance + })); + } + + self.import_tx(vtx, account.nonce); Ok(()) } /// Removes all transactions identified by hashes given in slice /// /// If gap is introduced marks subsequent transactions as future - pub fn remove_all(&mut self, transaction_hashes: &[H256], fetch_nonce: T) - where T: Fn(&Address) -> U256 { + pub fn remove_all(&mut self, transaction_hashes: &[H256], fetch_account: T) + where T: Fn(&Address) -> AccountDetails { for hash in transaction_hashes { - self.remove(&hash, &fetch_nonce); + self.remove(&hash, &fetch_account); } } /// Removes transaction identified by hashes from queue. /// /// If gap is introduced marks subsequent transactions as future - pub fn remove(&mut self, transaction_hash: &H256, fetch_nonce: &T) - where T: Fn(&Address) -> U256 { + pub fn remove(&mut self, transaction_hash: &H256, fetch_account: &T) + where T: Fn(&Address) -> AccountDetails { let transaction = self.by_hash.remove(transaction_hash); if transaction.is_none() { @@ -361,7 +381,7 @@ impl TransactionQueue { let transaction = transaction.unwrap(); let sender = transaction.sender(); let nonce = transaction.nonce(); - let current_nonce = fetch_nonce(&sender); + let current_nonce = fetch_account(&sender).nonce; // Remove from future @@ -403,6 +423,7 @@ impl TransactionQueue { if k >= current_nonce { self.future.insert(*sender, k, order.update_height(k, current_nonce)); } else { + trace!(target: "miner", "Dropping old transaction: {:?} (nonce: {} < {})", order.hash, k, current_nonce); // Remove the transaction completely self.by_hash.remove(&order.hash); } @@ -423,6 +444,7 @@ impl TransactionQueue { if k >= current_nonce { self.future.insert(*sender, k, order.update_height(k, current_nonce)); } else { + trace!(target: "miner", "Dropping old transaction: {:?} (nonce: {} < {})", order.hash, k, current_nonce); self.by_hash.remove(&order.hash); } } @@ -488,13 +510,11 @@ 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, fetch_nonce: &T) - where T: Fn(&Address) -> U256 { - + fn import_tx(&mut self, tx: VerifiedTransaction, state_nonce: U256) { if self.by_hash.get(&tx.hash()).is_some() { // Transaction is already imported. - trace!(target: "sync", "Dropping already imported transaction with hash: {:?}", tx.hash()); + trace!(target: "miner", "Dropping already imported transaction: {:?}", tx.hash()); return; } @@ -502,7 +522,6 @@ impl TransactionQueue { let address = tx.sender(); let nonce = tx.nonce(); - let state_nonce = fetch_nonce(&address); let next_nonce = self.last_nonces .get(&address) .cloned() @@ -516,7 +535,7 @@ impl TransactionQueue { return; } else if nonce < state_nonce { // Droping transaction - trace!(target: "sync", "Dropping transaction with nonce: {} - expecting: {}", nonce, next_nonce); + trace!(target: "miner", "Dropping old transaction: {:?} (nonce: {} < {})", tx.hash(), nonce, next_nonce); return; } @@ -525,6 +544,8 @@ impl TransactionQueue { // But maybe there are some more items waiting in future? self.move_matching_future_to_current(address, nonce + U256::one(), state_nonce); self.current.enforce_limit(&mut self.by_hash); + + trace!(target: "miner", "status: {:?}", self.status()); } /// Replaces transaction in given set (could be `future` or `current`). @@ -583,8 +604,11 @@ mod test { new_unsigned_tx(U256::from(123)).sign(&keypair.secret()) } - fn default_nonce(_address: &Address) -> U256 { - U256::from(123) + fn default_nonce(_address: &Address) -> AccountDetails { + AccountDetails { + nonce: U256::from(123), + balance: !U256::zero() + } } fn new_txs(second_nonce: U256) -> (SignedTransaction, SignedTransaction) { @@ -953,7 +977,8 @@ mod test { #[test] fn should_not_move_to_future_if_state_nonce_is_higher() { // given - let next_nonce = |a: &Address| default_nonce(a) + U256::one(); + let next_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce + U256::one(), balance: + !U256::zero() }; let mut txq = TransactionQueue::new(); let (tx, tx2) = new_txs(U256::from(1)); let tx3 = new_tx(); @@ -1028,8 +1053,10 @@ mod test { #[test] fn should_recalculate_height_when_removing_from_future() { // given - let previous_nonce = |a: &Address| default_nonce(a) - U256::one(); - let next_nonce = |a: &Address| default_nonce(a) + U256::one(); + let previous_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce - U256::one(), balance: + !U256::zero() }; + let next_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce + U256::one(), balance: + !U256::zero() }; let mut txq = TransactionQueue::new(); let (tx1, tx2) = new_txs(U256::one()); txq.add(tx1.clone(), &previous_nonce).unwrap(); diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index fda391304..04b25d5b2 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -19,7 +19,7 @@ use std::collections::HashSet; use std::sync::{Arc, Weak, Mutex}; use std::ops::Deref; use ethsync::{SyncProvider, SyncState}; -use ethminer::{MinerService}; +use ethminer::{MinerService, AccountDetails}; use jsonrpc_core::*; use util::numbers::*; use util::sha3::*; @@ -370,7 +370,10 @@ impl Eth for EthClient let signed_transaction = transaction.sign(&secret); let hash = signed_transaction.hash(); - let import = miner.import_transactions(vec![signed_transaction], |a: &Address| client.nonce(a)); + let import = miner.import_transactions(vec![signed_transaction], |a: &Address| AccountDetails { + nonce: client.nonce(a), + balance: client.balance(a) + }); match import { Ok(_) => to_value(&hash), Err(e) => { diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index 0cddf2a1e..b2aebb29c 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -20,7 +20,7 @@ use ethcore::error::Error; use ethcore::client::BlockChainClient; use ethcore::block::ClosedBlock; use ethcore::transaction::SignedTransaction; -use ethminer::{MinerService, MinerStatus}; +use ethminer::{MinerService, MinerStatus, AccountDetails}; pub struct TestMinerService; @@ -30,7 +30,8 @@ impl MinerService for TestMinerService { fn status(&self) -> MinerStatus { unimplemented!(); } /// Imports transactions to transaction queue. - fn import_transactions(&self, _transactions: Vec, _fetch_nonce: T) -> Result<(), Error> where T: Fn(&Address) -> U256 { unimplemented!(); } + fn import_transactions(&self, _transactions: Vec, _fetch_account: T) -> Result<(), Error> + where T: Fn(&Address) -> AccountDetails { unimplemented!(); } /// Returns hashes of transactions currently in pending fn pending_transactions_hashes(&self) -> Vec { unimplemented!(); } @@ -50,4 +51,4 @@ impl MinerService for TestMinerService { /// Submit `seal` as a valid solution for the header of `pow_hash`. /// Will check the seal, but not actually insert the block into the chain. fn submit_seal(&self, _chain: &BlockChainClient, _pow_hash: H256, _seal: Vec) -> Result<(), Error> { unimplemented!(); } -} \ No newline at end of file +} diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 8f0194289..4f8001ce7 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -38,7 +38,7 @@ use range_collection::{RangeCollection, ToUsize, FromUsize}; use ethcore::error::*; use ethcore::transaction::SignedTransaction; use ethcore::block::Block; -use ethminer::{Miner, MinerService}; +use ethminer::{Miner, MinerService, AccountDetails}; use io::SyncIo; use time; use super::SyncConfig; @@ -940,8 +940,11 @@ impl ChainSync { transactions.push(tx); } let chain = io.chain(); - let fetch_nonce = |a: &Address| chain.nonce(a); - let _ = self.miner.import_transactions(transactions, fetch_nonce); + let fetch_account = |a: &Address| AccountDetails { + nonce: chain.nonce(a), + balance: chain.balance(a) + }; + let _ = self.miner.import_transactions(transactions, fetch_account); Ok(()) }