From b8b55be0beb2b97e4a5fb775ffcf1e86879b9fc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 5 Jul 2016 11:51:41 -0400 Subject: [PATCH] Skipping transactions with invalid nonces when pushing to block. (#1545) * Changing some logging levels * Skipping invalid nonce errors --- ethcore/src/miner/miner.rs | 12 +++++++++--- ethcore/src/miner/transaction_queue.rs | 18 +++++++++--------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 215506607..dac25addf 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -200,17 +200,23 @@ impl Miner { let hash = tx.hash(); match open_block.push_transaction(tx, None) { Err(Error::Execution(ExecutionError::BlockGasLimitReached { gas_limit, gas_used, .. })) => { - trace!(target: "miner", "Skipping adding transaction to block because of gas limit: {:?}", hash); + debug!(target: "miner", "Skipping adding transaction to block because of gas limit: {:?}", hash); // Exit early if gas left is smaller then min_tx_gas let min_tx_gas: U256 = 21000.into(); // TODO: figure this out properly. if gas_limit - gas_used < min_tx_gas { break; } }, - Err(Error::Transaction(TransactionError::AlreadyImported)) => {} // already have transaction - ignore + // Invalid nonce error can happen only if previous transaction is skipped because of gas limit. + // If there is errornous state of transaction queue it will be fixed when next block is imported. + Err(Error::Execution(ExecutionError::InvalidNonce { .. })) => { + debug!(target: "miner", "Skipping adding transaction to block because of invalid nonce: {:?}", hash); + }, + // already have transaction - ignore + Err(Error::Transaction(TransactionError::AlreadyImported)) => {}, Err(e) => { invalid_transactions.insert(hash); - trace!(target: "miner", + debug!(target: "miner", "Error adding transaction to block: number={}. transaction_hash={:?}, Error: {:?}", block_number, hash, e); }, diff --git a/ethcore/src/miner/transaction_queue.rs b/ethcore/src/miner/transaction_queue.rs index 0e71b1d83..1fbc8774f 100644 --- a/ethcore/src/miner/transaction_queue.rs +++ b/ethcore/src/miner/transaction_queue.rs @@ -431,10 +431,10 @@ impl TransactionQueue { pub fn add(&mut self, tx: SignedTransaction, fetch_account: &T, origin: TransactionOrigin) -> Result where T: Fn(&Address) -> AccountDetails { - trace!(target: "miner", "Importing: {:?}", tx.hash()); + trace!(target: "txqueue", "Importing: {:?}", tx.hash()); if tx.gas_price < self.minimal_gas_price && origin != TransactionOrigin::Local { - trace!(target: "miner", + trace!(target: "txqueue", "Dropping transaction below minimal gas price threshold: {:?} (gp: {} < {})", tx.hash(), tx.gas_price, @@ -450,7 +450,7 @@ impl TransactionQueue { try!(tx.check_low_s()); if tx.gas > self.gas_limit || tx.gas > self.tx_gas_limit { - trace!(target: "miner", + trace!(target: "txqueue", "Dropping transaction above gas limit: {:?} ({} > min({}, {}))", tx.hash(), tx.gas, @@ -469,7 +469,7 @@ impl TransactionQueue { let cost = vtx.transaction.value + vtx.transaction.gas_price * vtx.transaction.gas; if client_account.balance < cost { - trace!(target: "miner", + trace!(target: "txqueue", "Dropping transaction without sufficient balance: {:?} ({} < {})", vtx.hash(), client_account.balance, @@ -557,7 +557,7 @@ impl TransactionQueue { if k >= current_nonce { self.future.insert(*sender, k, order.update_height(k, current_nonce)); } else { - trace!(target: "miner", "Removing old transaction: {:?} (nonce: {} < {})", order.hash, k, current_nonce); + trace!(target: "txqueue", "Removing old transaction: {:?} (nonce: {} < {})", order.hash, k, current_nonce); // Remove the transaction completely self.by_hash.remove(&order.hash).expect("All transactions in `future` are also in `by_hash`"); } @@ -578,7 +578,7 @@ impl TransactionQueue { if k >= current_nonce { self.future.insert(*sender, k, order.update_height(k, current_nonce)); } else { - trace!(target: "miner", "Removing old transaction: {:?} (nonce: {} < {})", order.hash, k, current_nonce); + trace!(target: "txqueue", "Removing old transaction: {:?} (nonce: {} < {})", order.hash, k, current_nonce); self.by_hash.remove(&order.hash).expect("All transactions in `future` are also in `by_hash`"); } } @@ -666,7 +666,7 @@ impl TransactionQueue { if self.by_hash.get(&tx.hash()).is_some() { // Transaction is already imported. - trace!(target: "miner", "Dropping already imported transaction: {:?}", tx.hash()); + trace!(target: "txqueue", "Dropping already imported transaction: {:?}", tx.hash()); return Err(TransactionError::AlreadyImported); } @@ -683,7 +683,7 @@ impl TransactionQueue { // nonce height would result in overflow. if nonce < state_nonce { // Droping transaction - trace!(target: "miner", "Dropping old transaction: {:?} (nonce: {} < {})", tx.hash(), nonce, next_nonce); + trace!(target: "txqueue", "Dropping old transaction: {:?} (nonce: {} < {})", tx.hash(), nonce, next_nonce); return Err(TransactionError::Old); } else if nonce > next_nonce { // We have a gap - put to future. @@ -719,7 +719,7 @@ impl TransactionQueue { // Trigger error if the transaction we are importing was removed. try!(check_if_removed(&address, &nonce, removed)); - trace!(target: "miner", "status: {:?}", self.status()); + trace!(target: "txqueue", "status: {:?}", self.status()); Ok(TransactionImportResult::Current) }