From 188e325b2060ab30377ba97764f4fbae6a27da0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 15 Mar 2016 23:01:36 +0100 Subject: [PATCH 01/12] Importing transactions from hashset. Notifying about every block --- ethcore/src/client/client.rs | 2 +- miner/src/miner.rs | 34 +++++++++++++++++++++++----------- miner/src/transaction_queue.rs | 10 ++++++++++ 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index caa92db97..6857416d3 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -317,7 +317,7 @@ impl Client where V: Verifier { } { - if !imported_blocks.is_empty() && self.block_queue.queue_info().is_empty() { + if !imported_blocks.is_empty() { let (enacted, retracted) = self.calculate_enacted_retracted(import_results); io.send(NetworkIoMessage::User(SyncMessage::NewChainBlocks { imported: imported_blocks, diff --git a/miner/src/miner.rs b/miner/src/miner.rs index 6d5b3086e..4652ab3db 100644 --- a/miner/src/miner.rs +++ b/miner/src/miner.rs @@ -18,6 +18,7 @@ use rayon::prelude::*; use std::sync::{Mutex, RwLock, Arc}; use std::sync::atomic; use std::sync::atomic::AtomicBool; +use std::collections::HashSet; use util::{H256, U256, Address, Bytes, Uint}; use ethcore::views::{BlockView}; @@ -174,21 +175,10 @@ impl MinerService for Miner { let block = BlockView::new(&block); block.transactions() } - { - let in_chain = vec![imported, enacted, invalid]; - let in_chain = in_chain - .par_iter() - .flat_map(|h| h.par_iter().map(|h| fetch_transactions(chain, h))); let out_of_chain = retracted .par_iter() .map(|h| fetch_transactions(chain, h)); - - in_chain.for_each(|txs| { - let mut transaction_queue = self.transaction_queue.lock().unwrap(); - let hashes = txs.iter().map(|tx| tx.hash()).collect::>(); - transaction_queue.remove_all(&hashes, |a| chain.nonce(a)); - }); out_of_chain.for_each(|txs| { // populate sender for tx in &txs { @@ -198,6 +188,28 @@ impl MinerService for Miner { let _ = transaction_queue.add_all(txs, |a| chain.nonce(a)); }); } + // First import all transactions and after that remove old ones + { + let in_chain = { + let mut in_chain = HashSet::new(); + in_chain.extend(imported); + in_chain.extend(enacted); + in_chain.extend(invalid); + in_chain + .into_iter() + .collect::>() + }; + + let in_chain = in_chain + .par_iter() + .map(|h: &H256| fetch_transactions(chain, h)); + + 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)); + }); + } if self.sealing_enabled.load(atomic::Ordering::Relaxed) { self.prepare_sealing(chain); diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 324a46364..04febb84d 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -351,6 +351,8 @@ impl TransactionQueue { /// 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 { + + println!("Removing transaction: (hash: {:?})", transaction_hash); let transaction = self.by_hash.remove(transaction_hash); if transaction.is_none() { // We don't know this transaction @@ -362,6 +364,8 @@ impl TransactionQueue { let nonce = transaction.nonce(); let current_nonce = fetch_nonce(&sender); + println!("Removing transaction: ({:?}, {:?}, hash: {:?})", sender, nonce, transaction.hash()); + // Remove from future let order = self.future.drop(&sender, &nonce); if order.is_some() { @@ -489,12 +493,14 @@ impl TransactionQueue { fn import_tx(&mut self, tx: VerifiedTransaction, fetch_nonce: &T) where T: Fn(&Address) -> 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()); return; } + let address = tx.sender(); let nonce = tx.nonce(); @@ -506,6 +512,7 @@ impl TransactionQueue { // Check height if nonce > next_nonce { + println!("[F] Importing transaction: ({:?}, {:?}, hash: {:?}, gas: {:?})", tx.sender(), tx.nonce(), tx.hash(), tx.transaction.gas_price); // 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); @@ -515,6 +522,7 @@ impl TransactionQueue { trace!(target: "sync", "Dropping transaction with nonce: {} - expecting: {}", nonce, next_nonce); return; } + println!("[C] Importing transaction: ({:?}, {:?}, hash: {:?}, gas: {:?})", tx.sender(), tx.nonce(), tx.hash(), tx.transaction.gas_price); Self::replace_transaction(tx, state_nonce, &mut self.current, &mut self.by_hash); self.last_nonces.insert(address, nonce); @@ -540,11 +548,13 @@ impl TransactionQueue { let new_fee = order.gas_price; if old_fee.cmp(&new_fee) == Ordering::Greater { // Put back old transaction since it has greater priority (higher gas_price) + println!("Didn't replace tx (h:{:?}, h:{:?})", hash, old.hash); set.by_address.insert(address, nonce, old); // and remove new one set.by_priority.remove(&order); by_hash.remove(&hash); } else { + println!("Replaced h:{:?} with h:{:?}, ", old.hash, hash); // Make sure we remove old transaction entirely set.by_priority.remove(&old); by_hash.remove(&old.hash); From 974222aabd114ba9e96c125aa622cf66ef408876 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 15 Mar 2016 23:13:53 +0100 Subject: [PATCH 02/12] Removing printlns --- miner/src/transaction_queue.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 04febb84d..4b365bba2 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -352,7 +352,6 @@ impl TransactionQueue { pub fn remove(&mut self, transaction_hash: &H256, fetch_nonce: &T) where T: Fn(&Address) -> U256 { - println!("Removing transaction: (hash: {:?})", transaction_hash); let transaction = self.by_hash.remove(transaction_hash); if transaction.is_none() { // We don't know this transaction @@ -364,7 +363,6 @@ impl TransactionQueue { let nonce = transaction.nonce(); let current_nonce = fetch_nonce(&sender); - println!("Removing transaction: ({:?}, {:?}, hash: {:?})", sender, nonce, transaction.hash()); // Remove from future let order = self.future.drop(&sender, &nonce); @@ -512,7 +510,6 @@ impl TransactionQueue { // Check height if nonce > next_nonce { - println!("[F] Importing transaction: ({:?}, {:?}, hash: {:?}, gas: {:?})", tx.sender(), tx.nonce(), tx.hash(), tx.transaction.gas_price); // 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); @@ -522,7 +519,6 @@ impl TransactionQueue { trace!(target: "sync", "Dropping transaction with nonce: {} - expecting: {}", nonce, next_nonce); return; } - println!("[C] Importing transaction: ({:?}, {:?}, hash: {:?}, gas: {:?})", tx.sender(), tx.nonce(), tx.hash(), tx.transaction.gas_price); Self::replace_transaction(tx, state_nonce, &mut self.current, &mut self.by_hash); self.last_nonces.insert(address, nonce); @@ -548,13 +544,11 @@ impl TransactionQueue { let new_fee = order.gas_price; if old_fee.cmp(&new_fee) == Ordering::Greater { // Put back old transaction since it has greater priority (higher gas_price) - println!("Didn't replace tx (h:{:?}, h:{:?})", hash, old.hash); set.by_address.insert(address, nonce, old); // and remove new one set.by_priority.remove(&order); by_hash.remove(&hash); } else { - println!("Replaced h:{:?} with h:{:?}, ", old.hash, hash); // Make sure we remove old transaction entirely set.by_priority.remove(&old); by_hash.remove(&old.hash); From fdba8de6000e8d629d7bc4a504a8cedb03d6168f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 16 Mar 2016 10:40:33 +0100 Subject: [PATCH 03/12] Validating senders balance before importing transaction to queue --- ethcore/src/error.rs | 9 ++- miner/src/lib.rs | 8 +-- miner/src/miner.rs | 20 ++++-- miner/src/transaction_queue.rs | 81 +++++++++++++++-------- rpc/src/v1/impls/eth.rs | 7 +- rpc/src/v1/tests/helpers/miner_service.rs | 7 +- sync/src/chain.rs | 9 ++- 7 files changed, 94 insertions(+), 47 deletions(-) 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(()) } From d54c95da9d3cae135a43316c9a62f9bf9f81beb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 16 Mar 2016 10:48:31 +0100 Subject: [PATCH 04/12] Removing unused import --- rpc/src/v1/tests/helpers/miner_service.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index b2aebb29c..7a1ed3809 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . -use util::{Address, H256, U256, Bytes}; +use util::{Address, H256, Bytes}; use util::standard::*; use ethcore::error::Error; use ethcore::client::BlockChainClient; From 8741a85443ff6a131cab4bf45af3c941f370e483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 16 Mar 2016 16:47:36 +0100 Subject: [PATCH 05/12] Fixing build --- miner/src/transaction_queue.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index ca4a269ae..abd862a02 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -777,8 +777,10 @@ mod test { #[test] fn should_correctly_update_futures_when_removing() { // given - let prev_nonce = |a: &Address| default_nonce(a) - U256::one(); - let next2_nonce = |a: &Address| default_nonce(a) + U256::from(2); + let prev_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce - U256::one(), balance: + !U256::zero() }; + let next2_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce + U256::from(2), balance: + !U256::zero() }; let mut txq = TransactionQueue::new(); @@ -923,7 +925,7 @@ mod test { let mut txq = TransactionQueue::new(); let tx = new_tx(); let last_nonce = tx.nonce + U256::one(); - let fetch_last_nonce = |_a: &Address| last_nonce; + let fetch_last_nonce = |_a: &Address| AccountDetails{ nonce: last_nonce, balance: !U256::zero() }; // when txq.add(tx, &fetch_last_nonce).unwrap(); @@ -937,7 +939,8 @@ mod test { #[test] fn should_not_insert_same_transaction_twice() { // given - let nonce = |a: &Address| default_nonce(a) + U256::one(); + let 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::from(1)); txq.add(tx2.clone(), &default_nonce).unwrap(); @@ -977,7 +980,7 @@ mod test { #[test] fn should_not_move_to_future_if_state_nonce_is_higher() { // given - let next_nonce = |a: &Address| AccountDetails{ nonce: default_nonce(a).nonce + U256::one(), balance: + 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)); From 0925968840d3d8ed027587c10ae0fdca253c250f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 16 Mar 2016 17:17:49 +0100 Subject: [PATCH 06/12] Adding test --- miner/src/transaction_queue.rs | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index abd862a02..480d6550a 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -34,7 +34,7 @@ //! use util::crypto::KeyPair; //! use util::hash::Address; //! use util::numbers::{Uint, U256}; -//! use ethminer::TransactionQueue; +//! use ethminer::{TransactionQueue, AccountDetails}; //! use ethcore::transaction::*; //! use rustc_serialize::hex::FromHex; //! @@ -47,7 +47,10 @@ //! //! let st1 = t1.sign(&key.secret()); //! let st2 = t2.sign(&key.secret()); -//! let default_nonce = |_a: &Address| U256::from(10); +//! let default_nonce = |_a: &Address| AccountDetails { +//! nonce: U256::from(10), +//! balance: U256::from(10_000), +//! }; //! //! let mut txq = TransactionQueue::new(); //! txq.add(st2.clone(), &default_nonce); @@ -677,6 +680,25 @@ mod test { assert_eq!(stats.pending, 1); } + #[test] + fn should_drop_transactions_from_senders_without_balance() { + // given + let mut txq = TransactionQueue::new(); + let tx = new_tx(); + let account = |a: &Address| AccountDetails { + nonce: default_nonce(a).nonce, + balance: U256::one() + }; + + // when + txq.add(tx, &account).unwrap_err(); + + // then + let stats = txq.status(); + assert_eq!(stats.pending, 0); + assert_eq!(stats.future, 0); + } + #[test] fn should_not_import_transaction_below_min_gas_price_threshold() { // given From 81c36499ea8bd8bde112de40100793efabc08ab2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 10:20:35 +0100 Subject: [PATCH 07/12] Fixing sync test --- sync/src/chain.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 8d392dc1c..0af6008f2 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -1301,6 +1301,7 @@ mod tests { use ::SyncConfig; use util::*; use super::{PeerInfo, PeerAsking}; + use ethcore::views::BlockView; use ethcore::header::*; use ethcore::client::*; use ethminer::{Miner, MinerService}; @@ -1631,6 +1632,13 @@ mod tests { let good_blocks = vec![client.block_hash_delta_minus(2)]; let retracted_blocks = vec![client.block_hash_delta_minus(1)]; + // Add some balance to clients + for h in vec![good_blocks[0], retracted_blocks[0]] { + let block = client.block(BlockId::Hash(h)).unwrap(); + let view = BlockView::new(&block); + client.set_balance(view.transactions()[0].sender().unwrap(), U256::from(10_000)); + } + let mut queue = VecDeque::new(); let mut io = TestIo::new(&mut client, &mut queue, None); From 95dda4aa68ac87c0320a551df7b08c069848ab27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 10:23:09 +0100 Subject: [PATCH 08/12] Full transaction cost --- miner/src/transaction_queue.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 480d6550a..1842f5f35 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -343,14 +343,16 @@ impl TransactionQueue { })); } + let vtx = try!(VerifiedTransaction::new(tx)); let account = fetch_account(&vtx.sender()); - if account.balance < vtx.transaction.value { + let cost = vtx.transaction.value + vtx.transaction.gas_price * vtx.transaction.gas; + if account.balance < cost { trace!(target: "miner", "Dropping transaction without sufficient balance: {:?} ({} < {})", - vtx.hash(), account.balance, vtx.transaction.value); + vtx.hash(), account.balance, cost); return Err(Error::Transaction(TransactionError::InsufficientBalance { - cost: vtx.transaction.value, + cost: cost, balance: account.balance })); } From 0e7778a7b79be72d25c57fce129098abfa68af44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 11:27:38 +0100 Subject: [PATCH 09/12] Increasing balance in tests --- sync/src/chain.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 0af6008f2..06fbcd5d1 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -403,7 +403,7 @@ impl ChainSync { self.remove_downloaded_blocks(number + 1); } if self.have_common_block && number < self.current_base_block() + 1 { - // unkown header + // unkown header debug!(target: "sync", "Old block header {:?} ({}) is unknown, restarting sync", hash, number); self.restart(io); return Ok(()); @@ -1636,7 +1636,7 @@ mod tests { for h in vec![good_blocks[0], retracted_blocks[0]] { let block = client.block(BlockId::Hash(h)).unwrap(); let view = BlockView::new(&block); - client.set_balance(view.transactions()[0].sender().unwrap(), U256::from(10_000)); + client.set_balance(view.transactions()[0].sender().unwrap(), U256::from(1_000_000_000)); } let mut queue = VecDeque::new(); From b1557b547b04abbef6ce8d116c80814c6d710540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 11:47:41 +0100 Subject: [PATCH 10/12] Reverting check if block queue is empty --- ethcore/src/client/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 6857416d3..caa92db97 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -317,7 +317,7 @@ impl Client where V: Verifier { } { - if !imported_blocks.is_empty() { + if !imported_blocks.is_empty() && self.block_queue.queue_info().is_empty() { let (enacted, retracted) = self.calculate_enacted_retracted(import_results); io.send(NetworkIoMessage::User(SyncMessage::NewChainBlocks { imported: imported_blocks, From bc04e0c71304237a2f5aba9c59cb14923169e302 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 11:49:56 +0100 Subject: [PATCH 11/12] Adding missing commas --- rpc/src/v1/impls/eth.rs | 2 +- sync/src/chain.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 04b25d5b2..b566dd848 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/rpc/src/v1/impls/eth.rs @@ -372,7 +372,7 @@ impl Eth for EthClient let import = miner.import_transactions(vec![signed_transaction], |a: &Address| AccountDetails { nonce: client.nonce(a), - balance: client.balance(a) + balance: client.balance(a), }); match import { Ok(_) => to_value(&hash), diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 06fbcd5d1..492481722 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -948,7 +948,7 @@ impl ChainSync { let chain = io.chain(); let fetch_account = |a: &Address| AccountDetails { nonce: chain.nonce(a), - balance: chain.balance(a) + balance: chain.balance(a), }; let _ = self.miner.import_transactions(transactions, fetch_account); Ok(()) From 7247f9e27f9eb59f2d11a666de3e9f1afed90a92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 17 Mar 2016 12:23:15 +0100 Subject: [PATCH 12/12] Fixing doctest --- miner/src/transaction_queue.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 1842f5f35..a1fc20b0f 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -49,12 +49,12 @@ //! let st2 = t2.sign(&key.secret()); //! let default_nonce = |_a: &Address| AccountDetails { //! nonce: U256::from(10), -//! balance: U256::from(10_000), +//! balance: U256::from(1_000_000), //! }; //! //! let mut txq = TransactionQueue::new(); -//! txq.add(st2.clone(), &default_nonce); -//! txq.add(st1.clone(), &default_nonce); +//! txq.add(st2.clone(), &default_nonce).unwrap(); +//! txq.add(st1.clone(), &default_nonce).unwrap(); //! //! // Check status //! assert_eq!(txq.status().pending, 2);