Wallet: keep risk dropped txns around in RAM and don't miss them when a Bloom filtered block includes them. Resolves issue 545.

core/src/main/java/com/google/bitcoin/core/Wallet.java

@@ -134,6 +134,20 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha
     // All transactions together.
     final Map<Sha256Hash, Transaction> transactions;
 
+    // Transactions that were dropped by the risk analysis system. These are not in any pools and not serialized
+    // to disk. We have to keep them around because if we ignore a tx because we think it will never confirm, but
+    // then it actually does confirm and does so within the same network session, remote peers will not resend us
+    // the tx data along with the Bloom filtered block, as they know we already received it once before
+    // (so it would be wasteful to repeat). Thus we keep them around here for a while. If we drop our network
+    // connections then the remote peers will forget that we were sent the tx data previously and send it again
+    // when relaying a filtered merkleblock.
+    private final LinkedHashMap<Sha256Hash, Transaction> riskDropped = new LinkedHashMap<Sha256Hash, Transaction>() {
+        @Override
+        protected boolean removeEldestEntry(Map.Entry<Sha256Hash, Transaction> eldest) {
+            return size() > 1000;
+        }
+    };
+
     // A list of public/private EC keys owned by this user. Access it using addKey[s], hasKey[s] and findPubKeyFromHash.
     private ArrayList<ECKey> keychain;
 
@@ -610,8 +624,14 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha
         try {
             Transaction tx = transactions.get(txHash);
             if (tx == null) {
-                log.error("TX {} not found despite being sent to wallet", txHash);
+                tx = riskDropped.get(txHash);
-                return;
+                if (tx != null) {
+                    // If this happens our risk analysis is probably wrong and should be improved.
+                    log.info("Risk analysis dropped tx {} but was included in block anyway", tx.getHash());
+                } else {
+                    log.error("TX {} not found despite being sent to wallet", txHash);
+                    return;
+                }
             }
             receive(tx, block, blockType, relativityOffset);
         } finally {
@@ -655,8 +675,12 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha
             // race conditions where receivePending may be being called in parallel.
             if (!overrideIsRelevant && !isPendingTransactionRelevant(tx))
                 return;
-            if (isTransactionRisky(tx, dependencies) && !acceptRiskyTransactions)
+            if (isTransactionRisky(tx, dependencies) && !acceptRiskyTransactions) {
+                // isTransactionRisky already logged the reason.
+                riskDropped.put(tx.getHash(), tx);
+                log.warn("There are now {} risk dropped transactions being kept in memory", riskDropped.size());
                 return;
+            }
             BigInteger valueSentToMe = tx.getValueSentToMe(this);
             BigInteger valueSentFromMe = tx.getValueSentFromMe(this);
             if (log.isInfoEnabled()) {
@@ -733,7 +757,6 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha
                 log.debug("Received tx we already saw in a block or created ourselves: " + tx.getHashAsString());
                 return false;
             }
-
             // We only care about transactions that:
             //   - Send us coins
             //   - Spend our coins
@@ -741,12 +764,6 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha
                 log.debug("Received tx that isn't relevant to this wallet, discarding.");
                 return false;
             }
-
-            if (isTransactionRisky(tx, null) && !acceptRiskyTransactions) {
-                log.warn("Received transaction {} with a lock time of {}, but not configured to accept these, discarding",
-                        tx.getHashAsString(), tx.getLockTime());
-                return false;
-            }
             return true;
         } finally {
             lock.unlock();

core/src/test/java/com/google/bitcoin/core/WalletTest.java

@@ -51,6 +51,7 @@ import java.security.SecureRandom;
 import java.util.*;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static com.google.bitcoin.core.Utils.*;
@@ -2319,4 +2320,38 @@ public class WalletTest extends TestWithWallet {
         }
         assertTrue(TransactionSignature.isEncodingCanonical(dummySig));
     }
+
+    @Test
+    public void riskAnalysis() throws Exception {
+        // Send a tx that is considered risky to the wallet, verify it doesn't show up in the balances.
+        final Transaction tx = createFakeTx(params, Utils.COIN, myAddress);
+        final AtomicBoolean bool = new AtomicBoolean();
+        wallet.setRiskAnalyzer(new RiskAnalysis.Analyzer() {
+            @Override
+            public RiskAnalysis create(Wallet wallet, Transaction wtx, List<Transaction> dependencies) {
+                RiskAnalysis.Result result = RiskAnalysis.Result.OK;
+                if (wtx.getHash().equals(tx.getHash()))
+                    result = RiskAnalysis.Result.NON_STANDARD;
+                final RiskAnalysis.Result finalResult = result;
+                return new RiskAnalysis() {
+                    @Override
+                    public Result analyze() {
+                        bool.set(true);
+                        return finalResult;
+                    }
+                };
+            }
+        });
+        assertTrue(wallet.isPendingTransactionRelevant(tx));
+        assertEquals(BigInteger.ZERO, wallet.getBalance());
+        assertEquals(BigInteger.ZERO, wallet.getBalance(Wallet.BalanceType.ESTIMATED));
+        wallet.receivePending(tx, null);
+        assertEquals(BigInteger.ZERO, wallet.getBalance());
+        assertEquals(BigInteger.ZERO, wallet.getBalance(Wallet.BalanceType.ESTIMATED));
+        assertTrue(bool.get());
+        // Confirm it in the same manner as how Bloom filtered blocks do. Verify it shows up.
+        StoredBlock block = createFakeBlock(blockStore, tx).storedBlock;
+        wallet.notifyTransactionIsInBlock(tx.getHash(), block, AbstractBlockChain.NewBlockType.BEST_CHAIN, 1);
+        assertEquals(Utils.COIN, wallet.getBalance());
+    }
 }