From 757e25ba9b43bd99a0c2165a49fba74378170f35 Mon Sep 17 00:00:00 2001
From: Mike Hearn <mike@plan99.net>
Date: Thu, 4 Dec 2014 17:09:53 +0100
Subject: [PATCH] Wallet: fix a bug that could cause a temporarily corrupted
 balance, when two pending transactions arrive backwards

---
 .../main/java/org/bitcoinj/core/Wallet.java   | 27 +++++++++++--------
 .../java/org/bitcoinj/core/WalletTest.java    | 25 ++++++++++++++++-
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java
index 958b4365..570c57d0 100644
--- a/core/src/main/java/org/bitcoinj/core/Wallet.java
+++ b/core/src/main/java/org/bitcoinj/core/Wallet.java
@@ -1975,23 +1975,28 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha
         // ever occur because we expect transactions to arrive in temporal order, but this assumption can be violated
         // when we receive a pending transaction from the mempool that is relevant to us, which spends coins that we
         // didn't see arrive on the best chain yet. For instance, because of a chain replay or because of our keys were
-        // used by another wallet somewhere else.
-        if (fromChain) {
-            for (Transaction pendingTx : pending.values()) {
-                for (TransactionInput input : pendingTx.getInputs()) {
-                    TransactionInput.ConnectionResult result = input.connect(tx, TransactionInput.ConnectMode.ABORT_ON_CONFLICT);
+        // used by another wallet somewhere else. Also, unconfirmed transactions can arrive from the mempool in more or
+        // less random order.
+        for (Transaction pendingTx : pending.values()) {
+            for (TransactionInput input : pendingTx.getInputs()) {
+                TransactionInput.ConnectionResult result = input.connect(tx, TransactionInput.ConnectMode.ABORT_ON_CONFLICT);
+                if (fromChain) {
                     // This TX is supposed to have just appeared on the best chain, so its outputs should not be marked
                     // as spent yet. If they are, it means something is happening out of order.
                     checkState(result != TransactionInput.ConnectionResult.ALREADY_SPENT);
-                    if (result == TransactionInput.ConnectionResult.SUCCESS) {
-                        log.info("Connected pending tx input {}:{}",
-                                pendingTx.getHashAsString(), pendingTx.getInputs().indexOf(input));
-                    }
                 }
-                // If the transactions outputs are now all spent, it will be moved into the spent pool by the
-                // processTxFromBestChain method.
+                if (result == TransactionInput.ConnectionResult.SUCCESS) {
+                    log.info("Connected pending tx input {}:{}",
+                            pendingTx.getHashAsString(), pendingTx.getInputs().indexOf(input));
+                }
             }
         }
+        if (!fromChain) {
+            maybeMovePool(tx, "pendingtx");
+        } else {
+            // If the transactions outputs are now all spent, it will be moved into the spent pool by the
+            // processTxFromBestChain method.
+        }
     }
 
     // Updates the wallet when a double spend occurs. overridingTx can be null for the case of coinbases
diff --git a/core/src/test/java/org/bitcoinj/core/WalletTest.java b/core/src/test/java/org/bitcoinj/core/WalletTest.java
index 2c3a72d3..50b444a6 100644
--- a/core/src/test/java/org/bitcoinj/core/WalletTest.java
+++ b/core/src/test/java/org/bitcoinj/core/WalletTest.java
@@ -71,6 +71,9 @@ public class WalletTest extends TestWithWallet {
 
     private SecureRandom secureRandom = new SecureRandom();
 
+    private ECKey someOtherKey = new ECKey();
+    private Address someOtherAddress = someOtherKey.toAddress(params);
+
     @Before
     @Override
     public void setUp() throws Exception {
@@ -1467,7 +1470,7 @@ public class WalletTest extends TestWithWallet {
         Transaction tx1 = createFakeTx(params, value, myAddress);
         Transaction tx2 = new Transaction(params);
         tx2.addInput(tx1.getOutput(0));
-        tx2.addOutput(valueOf(0, 9), new ECKey());
+        tx2.addOutput(valueOf(0, 9), someOtherAddress);
         // Add a change address to ensure this tx is relevant.
         tx2.addOutput(CENT, wallet.getChangeAddress());
         wallet.receivePending(tx2, null);
@@ -1480,6 +1483,26 @@ public class WalletTest extends TestWithWallet {
         assertEquals(0, wallet.getPoolSize(Pool.UNSPENT));
     }
 
+    @Test
+    public void outOfOrderPendingTxns() throws Exception {
+        // Check that if there are two pending transactions which we receive out of order, they are marked as spent
+        // correctly. For instance, we are watching a wallet, someone pays us (A) and we then pay someone else (B)
+        // with a change address but the network delivers the transactions to us in order B then A.
+        Coin value = COIN;
+        Transaction a = createFakeTx(params, value, myAddress);
+        Transaction b = new Transaction(params);
+        b.addInput(a.getOutput(0));
+        b.addOutput(CENT, someOtherAddress);
+        Coin v = COIN.subtract(CENT);
+        b.addOutput(v, wallet.getChangeAddress());
+        a = roundTripTransaction(params, a);
+        b = roundTripTransaction(params, b);
+        wallet.receivePending(b, null);
+        assertEquals(v, wallet.getBalance(Wallet.BalanceType.ESTIMATED));
+        wallet.receivePending(a, null);
+        assertEquals(v, wallet.getBalance(Wallet.BalanceType.ESTIMATED));
+    }
+
     @Test
     public void encryptionDecryptionAESBasic() throws Exception {
         Wallet encryptedWallet = new Wallet(params);