From 0f4fbadd3628cf20d6f5bed45b0d5ab78e565063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Sun, 24 Apr 2016 19:41:21 +0200 Subject: [PATCH 1/2] Fixing transaction queue last_nonces update --- miner/src/transaction_queue.rs | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index 63b49f9f4..a1147ef78 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -577,6 +577,7 @@ impl TransactionQueue { /// Checks if there are any transactions in `future` that should actually be promoted to `current` /// (because nonce matches). fn move_matching_future_to_current(&mut self, address: Address, mut current_nonce: U256, first_nonce: U256) { + let mut should_update_last_nonces = false; { let by_nonce = self.future.by_address.row_mut(&address); if let None = by_nonce { @@ -590,11 +591,14 @@ impl TransactionQueue { let order = order.update_height(current_nonce, first_nonce); self.current.insert(address, current_nonce, order); current_nonce = current_nonce + U256::one(); + should_update_last_nonces = true; } } self.future.by_address.clear_if_empty(&address); - // Update last inserted nonce - self.last_nonces.insert(address, current_nonce - U256::one()); + if should_update_last_nonces { + // Update last inserted nonce + self.last_nonces.insert(address, current_nonce - U256::one()); + } } /// Adds VerifiedTransaction to this queue. @@ -1457,4 +1461,29 @@ mod test { // then assert!(txq.top_transactions().is_empty()); } + + #[test] + fn should_return_valid_last_nonce_after_remove_all() { + // given + let mut txq = TransactionQueue::new(); + let (tx1, tx2) = new_txs(U256::from(4)); + let sender = tx1.sender().unwrap(); + let (nonce1, nonce2) = (tx1.nonce, tx2.nonce); + let details1 = |_a: &Address| AccountDetails { nonce: nonce1, balance: !U256::zero() }; + + // when + // Insert first transaction + assert_eq!(txq.add(tx1, &details1).unwrap(), TransactionImportResult::Current); + // Second should go to future + assert_eq!(txq.add(tx2, &details1).unwrap(), TransactionImportResult::Future); + // Now block is imported + txq.remove_all(sender, nonce2 - U256::from(1)); + // tx2 should be not be promoted to current + assert_eq!(txq.status().pending, 0); + assert_eq!(txq.status().future, 1); + + // then + assert_eq!(txq.last_nonce(&sender), None); + + } } From b4fc75828eec8889150eab8318c3c67aa53cc2a0 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Sun, 24 Apr 2016 22:08:27 +0100 Subject: [PATCH 2/2] Update transaction_queue.rs Avoid extraneous `U256` operations and split-state local. --- miner/src/transaction_queue.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/miner/src/transaction_queue.rs b/miner/src/transaction_queue.rs index a1147ef78..1cb485bb0 100644 --- a/miner/src/transaction_queue.rs +++ b/miner/src/transaction_queue.rs @@ -577,7 +577,7 @@ impl TransactionQueue { /// Checks if there are any transactions in `future` that should actually be promoted to `current` /// (because nonce matches). fn move_matching_future_to_current(&mut self, address: Address, mut current_nonce: U256, first_nonce: U256) { - let mut should_update_last_nonces = false; + let mut update_last_nonce_to = None; { let by_nonce = self.future.by_address.row_mut(&address); if let None = by_nonce { @@ -590,14 +590,14 @@ impl TransactionQueue { // Put to current let order = order.update_height(current_nonce, first_nonce); self.current.insert(address, current_nonce, order); + update_last_nonce_to = Some(current_nonce); current_nonce = current_nonce + U256::one(); - should_update_last_nonces = true; } } self.future.by_address.clear_if_empty(&address); - if should_update_last_nonces { + if let Some(x) = update_last_nonce_to { // Update last inserted nonce - self.last_nonces.insert(address, current_nonce - U256::one()); + self.last_nonces.insert(address, x); } }