From acfabe5431b1bbb5e6d660868ede3b2903c6f4f0 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 6 Jul 2016 10:41:29 +0200 Subject: [PATCH] Skipping transactions with invalid nonces when pushing to block. (#1545) (#1547) * 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 9c73a6485..9706a7687 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -198,17 +198,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 7f5b59c38..344be59ac 100644 --- a/ethcore/src/miner/transaction_queue.rs +++ b/ethcore/src/miner/transaction_queue.rs @@ -439,10 +439,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, @@ -458,7 +458,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, @@ -477,7 +477,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, @@ -565,7 +565,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`"); } @@ -586,7 +586,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`"); } } @@ -674,7 +674,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); } @@ -691,7 +691,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. @@ -727,7 +727,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) }