Wallet: fix a bug that could cause a temporarily corrupted balance, when two pending transactions arrive backwards

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.
+        // used by another wallet somewhere else. Also, unconfirmed transactions can arrive from the mempool in more or
-        if (fromChain) {
+        // less random order.
-            for (Transaction pendingTx : pending.values()) {
+        for (Transaction pendingTx : pending.values()) {
-                for (TransactionInput input : pendingTx.getInputs()) {
+            for (TransactionInput input : pendingTx.getInputs()) {
-                    TransactionInput.ConnectionResult result = input.connect(tx, TransactionInput.ConnectMode.ABORT_ON_CONFLICT);
+                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
+                if (result == TransactionInput.ConnectionResult.SUCCESS) {
-                // processTxFromBestChain method.
+                    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

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);