Fix issue 166. Consider intra-block dependencies when deciding whether to send transactions to the wallet.

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

@@ -172,10 +172,8 @@ public class BlockChain {
         // blocks validity so we can skip the merkle root verification if the contents aren't interesting. This saves
         // a lot of time for big blocks.
         boolean contentsImportant = false;
-        HashMap<Wallet, List<Transaction>> walletToTxMap = new HashMap<Wallet, List<Transaction>>();
         if (block.transactions != null) {
-            scanTransactions(block, walletToTxMap);
+            contentsImportant = containsRelevantTransactions(block);
-            contentsImportant = walletToTxMap.size() > 0;
         }
 
         // Prove the block is internally valid: hash is lower than target, etc. This only checks the block contents
@@ -211,7 +209,7 @@ public class BlockChain {
             StoredBlock newStoredBlock = storedPrev.build(block);
             checkDifficultyTransitions(storedPrev, newStoredBlock);
             blockStore.put(newStoredBlock);
-            connectBlock(newStoredBlock, storedPrev, walletToTxMap);
+            connectBlock(newStoredBlock, storedPrev, block.transactions);
         }
 
         if (tryConnecting)
@@ -222,15 +220,15 @@ public class BlockChain {
     }
 
     private void connectBlock(StoredBlock newStoredBlock, StoredBlock storedPrev,
-                              HashMap<Wallet, List<Transaction>> newTransactions)
+                              List<Transaction> transactions)
             throws BlockStoreException, VerificationException {
         StoredBlock head = getChainHead();
         if (storedPrev.equals(head)) {
             // This block connects to the best known block, it is a normal continuation of the system.
             setChainHead(newStoredBlock);
             log.debug("Chain is now {} blocks high", newStoredBlock.getHeight());
-            if (newTransactions != null)
+            if (transactions != null)
-                sendTransactionsToWallet(newStoredBlock, NewBlockType.BEST_CHAIN, newTransactions);
+                sendTransactionsToWallet(newStoredBlock, NewBlockType.BEST_CHAIN, transactions);
         } else {
             // This block connects to somewhere other than the top of the best known chain. We treat these differently.
             //
@@ -245,7 +243,7 @@ public class BlockChain {
                     // newStoredBlock is a part of the same chain, there's no fork. This happens when we receive a block
                     // that we already saw and linked into the chain previously, which isn't the chain head.
                     // Re-processing it is confusing for the wallet so just skip.
-                    log.debug("Saw duplicated block in main chain at height {}: {}",
+                    log.warn("Saw duplicated block in main chain at height {}: {}",
                             newStoredBlock.getHeight(), newStoredBlock.getHeader().getHash());
                     return;
                 }
@@ -262,8 +260,8 @@ public class BlockChain {
             // We may not have any transactions if we received only a header, which can happen during fast catchup.
             // If we do, send them to the wallet but state that they are on a side chain so it knows not to try and
             // spend them until they become activated.
-            if (newTransactions != null) {
+            if (transactions != null) {
-                sendTransactionsToWallet(newStoredBlock, NewBlockType.SIDE_CHAIN, newTransactions);
+                sendTransactionsToWallet(newStoredBlock, NewBlockType.SIDE_CHAIN, transactions);
             }
 
             if (haveNewBestChain)
@@ -352,17 +350,17 @@ public class BlockChain {
     }
 
     private void sendTransactionsToWallet(StoredBlock block, NewBlockType blockType,
-                                          HashMap<Wallet, List<Transaction>> newTransactions) throws VerificationException {
+                                          List<Transaction> transactions) throws VerificationException {
-        for (Map.Entry<Wallet, List<Transaction>> entry : newTransactions.entrySet()) {
+        for (Transaction tx : transactions) {
-            try {
+            for (Wallet wallet : wallets) {
-                List<Transaction> txns = entry.getValue();
+                try {
-                for (Transaction tx : txns) {
+                    if (wallet.isTransactionRelevant(tx, true))
-                    entry.getKey().receiveFromBlock(tx, block, blockType);
+                        wallet.receiveFromBlock(tx, block, blockType);
+                } catch (ScriptException e) {
+                    // We don't want scripts we don't understand to break the block chain so just note that this tx was
+                    // not scanned here and continue.
+                    log.warn("Failed to parse a script: " + e.toString());
                 }
-            } catch (ScriptException e) {
-                // We don't want scripts we don't understand to break the block chain so just note that this tx was
-                // not scanned here and continue.
-                log.warn("Failed to parse a script: " + e.toString());
             }
         }
     }
@@ -495,6 +493,24 @@ public class BlockChain {
         }
     }
 
+    /**
+     * Returns true if any connected wallet considers any transaction in the block to be relevant.
+     */
+    private boolean containsRelevantTransactions(Block block) {
+        for (Transaction tx : block.transactions) {
+            try {
+                for (Wallet wallet : wallets) {
+                    if (wallet.isTransactionRelevant(tx, true)) return true;
+                }
+            } catch (ScriptException e) {
+                // We don't want scripts we don't understand to break the block chain so just note that this tx was
+                // not scanned here and continue.
+                log.warn("Failed to parse a script: " + e.toString());
+            }
+        }
+        return false;
+    }
+
     /**
      * Returns the block at the head of the current best chain. This is the block which represents the greatest
      * amount of cumulative work done.

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

@@ -230,6 +230,27 @@ public class BlockChainTest {
         assertEquals(b3, block[0].getHeader());
     }
 
+    @Test
+    public void intraBlockDependencies() throws Exception {
+        // Covers issue 166 in which transactions that depend on each other inside a block were not always being
+        // considered relevant.
+        Address somebodyElse = new ECKey().toAddress(unitTestParams);
+        Block b1 = unitTestParams.genesisBlock.createNextBlock(somebodyElse);
+        ECKey key = new ECKey();
+        wallet.addKey(key);
+        Address addr = key.toAddress(unitTestParams);
+        // Create a tx that gives us some coins, and another that spends it to someone else in the same block.
+        Transaction t1 = TestUtils.createFakeTx(unitTestParams, Utils.toNanoCoins(1, 0), addr);
+        Transaction t2 = new Transaction(unitTestParams);
+        t2.addInput(t1.getOutputs().get(0));
+        t2.addOutput(Utils.toNanoCoins(2, 0), somebodyElse);
+        b1.addTransaction(t1);
+        b1.addTransaction(t2);
+        b1.solve();
+        chain.add(b1);
+        assertEquals(BigInteger.ZERO, wallet.getBalance());
+    }
+
     // Some blocks from the test net.
     private Block getBlock2() throws Exception {
         Block b2 = new Block(testNet);