Delete !notfound code paths.

core/src/main/java/org/bitcoinj/core/Peer.java

@@ -130,10 +130,6 @@ public class Peer extends PeerSocketHandler {
     private static class GetDataRequest {
         Sha256Hash hash;
         SettableFuture future;
-        // If the peer does not support the notfound message, we'll use ping/pong messages to simulate it. This is
-        // a nasty hack that relies on the fact that bitcoin-qt is single threaded and processes messages in order.
-        // The nonce field records which pong should clear this request as "not found".
-        long nonce;
     }
     private final CopyOnWriteArrayList<GetDataRequest> getDataFutures;
 
@@ -744,7 +740,6 @@ public class Peer extends PeerSocketHandler {
             // Build the request for the missing dependencies.
             List<ListenableFuture<Transaction>> futures = Lists.newArrayList();
             GetDataMessage getdata = new GetDataMessage(params);
-            final long nonce = (long)(Math.random()*Long.MAX_VALUE);
             if (needToRequest.size() > 1)
                 log.info("{}: Requesting {} transactions for dep resolution", getAddress(), needToRequest.size());
             for (Sha256Hash hash : needToRequest) {
@@ -752,9 +747,6 @@ public class Peer extends PeerSocketHandler {
                 GetDataRequest req = new GetDataRequest();
                 req.hash = hash;
                 req.future = SettableFuture.create();
-                if (!isNotFoundMessageSupported()) {
-                    req.nonce = nonce;
-                }
                 futures.add(req.future);
                 getDataFutures.add(req);
             }
@@ -803,25 +795,6 @@ public class Peer extends PeerSocketHandler {
             });
             // Start the operation.
             sendMessage(getdata);
-            if (!isNotFoundMessageSupported()) {
-                // If the peer isn't new enough to support the notfound message, we use a nasty hack instead and
-                // assume if we send a ping message after the getdata message, it'll be processed after all answers
-                // from getdata are done, so we can watch for the pong message as a substitute.
-                log.info("{}: Dep resolution waiting for a pong with nonce {}", this, nonce);
-                ping(nonce).addListener(new Runnable() {
-                    @Override
-                    public void run() {
-                        // The pong came back so clear out any transactions we requested but didn't get.
-                        for (GetDataRequest req : getDataFutures) {
-                            if (req.nonce == nonce) {
-                                log.info("{}: Bottomed out dep tree at {}", this, req.hash);
-                                req.future.cancel(true);
-                                getDataFutures.remove(req);
-                            }
-                        }
-                    }
-                }, Threading.SAME_THREAD);
-            }
         } catch (Exception e) {
             log.error("{}: Couldn't send getdata in downloadDependencies({})", this, tx.getHash());
             resultFuture.setException(e);

core/src/test/java/org/bitcoinj/core/PeerTest.java

@@ -533,18 +533,9 @@ public class PeerTest extends TestWithNetworkConnections {
     }
 
     @Test
-    public void recursiveDownloadNew() throws Exception {
+    public void recursiveDependencyDownload() throws Exception {
-        recursiveDependencyDownload(true);
-    }
-
-    @Test
-    public void recursiveDownloadOld() throws Exception {
-        recursiveDependencyDownload(false);
-    }
-
-    public void recursiveDependencyDownload(boolean useNotFound) throws Exception {
         // Using ping or notfound?
-        connectWithVersion(useNotFound ? 70001 : 60001);
+        connectWithVersion(70001);
         // Check that we can download all dependencies of an unconfirmed relevant transaction from the mempool.
         ECKey to = new ECKey();
 
@@ -604,8 +595,6 @@ public class PeerTest extends TestWithNetworkConnections {
         assertEquals(someHash, getdata.getItems().get(2).hash);
         assertEquals(anotherHash, getdata.getItems().get(3).hash);
         long nonce = -1;
-        if (!useNotFound)
-            nonce = ((Ping) outbound(writeTarget)).getNonce();
         // For some random reason, t4 is delivered at this point before it's needed - perhaps it was a Bloom filter
         // false positive. We do this to check that the mempool is being checked for seen transactions before
         // requesting them.
@@ -613,37 +602,25 @@ public class PeerTest extends TestWithNetworkConnections {
         // Deliver the requested transactions.
         inbound(writeTarget, t2);
         inbound(writeTarget, t3);
-        if (useNotFound) {
+        NotFoundMessage notFound = new NotFoundMessage(unitTestParams);
-            NotFoundMessage notFound = new NotFoundMessage(unitTestParams);
+        notFound.addItem(new InventoryItem(InventoryItem.Type.Transaction, someHash));
-            notFound.addItem(new InventoryItem(InventoryItem.Type.Transaction, someHash));
+        notFound.addItem(new InventoryItem(InventoryItem.Type.Transaction, anotherHash));
-            notFound.addItem(new InventoryItem(InventoryItem.Type.Transaction, anotherHash));
+        inbound(writeTarget, notFound);
-            inbound(writeTarget, notFound);
-        } else {
-            inbound(writeTarget, new Pong(nonce));
-        }
         assertFalse(futures.isDone());
         // It will recursively ask for the dependencies of t2: t5 and t4, but not t3 because it already found t4.
         getdata = (GetDataMessage) outbound(writeTarget);
         assertEquals(getdata.getItems().get(0).hash, t2.getInput(0).getOutpoint().getHash());
         // t5 isn't found and t4 is.
-        if (useNotFound) {
+        notFound = new NotFoundMessage(unitTestParams);
-            NotFoundMessage notFound = new NotFoundMessage(unitTestParams);
+        notFound.addItem(new InventoryItem(InventoryItem.Type.Transaction, t5));
-            notFound.addItem(new InventoryItem(InventoryItem.Type.Transaction, t5));
+        inbound(writeTarget, notFound);
-            inbound(writeTarget, notFound);
-        } else {
-            bouncePing();
-        }
         assertFalse(futures.isDone());
         // Continue to explore the t4 branch and ask for t6, which is in the chain.
         getdata = (GetDataMessage) outbound(writeTarget);
         assertEquals(t6, getdata.getItems().get(0).hash);
-        if (useNotFound) {
+        notFound = new NotFoundMessage(unitTestParams);
-            NotFoundMessage notFound = new NotFoundMessage(unitTestParams);
+        notFound.addItem(new InventoryItem(InventoryItem.Type.Transaction, t6));
-            notFound.addItem(new InventoryItem(InventoryItem.Type.Transaction, t6));
+        inbound(writeTarget, notFound);
-            inbound(writeTarget, notFound);
-        } else {
-            bouncePing();
-        }
         pingAndWait(writeTarget);
         // That's it, we explored the entire tree.
         assertTrue(futures.isDone());
@@ -653,23 +630,9 @@ public class PeerTest extends TestWithNetworkConnections {
         assertTrue(results.contains(t4));
     }
 
-    private void bouncePing() throws Exception {
-        Ping ping = (Ping) outbound(writeTarget);
-        inbound(writeTarget, new Pong(ping.getNonce()));
-    }
-
     @Test
     public void timeLockedTransactionNew() throws Exception {
-        timeLockedTransaction(true);
+        connectWithVersion(70001);
-    }
-
-    @Test
-    public void timeLockedTransactionOld() throws Exception {
-        timeLockedTransaction(false);
-    }
-
-    public void timeLockedTransaction(boolean useNotFound) throws Exception {
-        connectWithVersion(useNotFound ? 70001 : 60001);
         // Test that if we receive a relevant transaction that has a lock time, it doesn't result in a notification
         // until we explicitly opt in to seeing those.
         Wallet wallet = new Wallet(unitTestParams);
@@ -686,11 +649,7 @@ public class PeerTest extends TestWithNetworkConnections {
         Transaction t1 = FakeTxBuilder.createFakeTx(unitTestParams, COIN, key);
         inbound(writeTarget, t1);
         GetDataMessage getdata = (GetDataMessage) outbound(writeTarget);
-        if (useNotFound) {
+        inbound(writeTarget, new NotFoundMessage(unitTestParams, getdata.getItems()));
-            inbound(writeTarget, new NotFoundMessage(unitTestParams, getdata.getItems()));
-        } else {
-            bouncePing();
-        }
         pingAndWait(writeTarget);
         Threading.waitForUserCode();
         assertNotNull(vtx[0]);
@@ -705,45 +664,28 @@ public class PeerTest extends TestWithNetworkConnections {
         wallet.setAcceptRiskyTransactions(true);
         inbound(writeTarget, t2);
         getdata = (GetDataMessage) outbound(writeTarget);
-        if (useNotFound) {
+        inbound(writeTarget, new NotFoundMessage(unitTestParams, getdata.getItems()));
-            inbound(writeTarget, new NotFoundMessage(unitTestParams, getdata.getItems()));
-        } else {
-            bouncePing();
-        }
         pingAndWait(writeTarget);
         Threading.waitForUserCode();
         assertEquals(t2, vtx[0]);
     }
 
     @Test
-    public void rejectTimeLockedDependencyNew() throws Exception {
+    public void rejectTimeLockedDependency() throws Exception {
         // Check that we also verify the lock times of dependencies. Otherwise an attacker could still build a tx that
         // looks legitimate and useful but won't actually ever confirm, by sending us a normal tx that spends a
         // timelocked tx.
-        checkTimeLockedDependency(false, true);
+        checkTimeLockedDependency(false);
     }
 
     @Test
-    public void acceptTimeLockedDependencyNew() throws Exception {
+    public void acceptTimeLockedDependency() throws Exception {
-        checkTimeLockedDependency(true, true);
+        checkTimeLockedDependency(true);
     }
 
-    @Test
+    private void checkTimeLockedDependency(boolean shouldAccept) throws Exception {
-    public void rejectTimeLockedDependencyOld() throws Exception {
-        // Check that we also verify the lock times of dependencies. Otherwise an attacker could still build a tx that
-        // looks legitimate and useful but won't actually ever confirm, by sending us a normal tx that spends a
-        // timelocked tx.
-        checkTimeLockedDependency(false, false);
-    }
-
-    @Test
-    public void acceptTimeLockedDependencyOld() throws Exception {
-        checkTimeLockedDependency(true, false);
-    }
-
-    private void checkTimeLockedDependency(boolean shouldAccept, boolean useNotFound) throws Exception {
         // Initial setup.
-        connectWithVersion(useNotFound ? 70001 : 60001);
+        connectWithVersion(70001);
         Wallet wallet = new Wallet(unitTestParams);
         ECKey key = wallet.freshReceiveKey();
         wallet.setAcceptRiskyTransactions(shouldAccept);
@@ -780,19 +722,13 @@ public class PeerTest extends TestWithNetworkConnections {
         getdata = (GetDataMessage) outbound(writeTarget);
         assertEquals(t2.getHash(), getdata.getItems().get(0).hash);
         inbound(writeTarget, t2);
-        if (!useNotFound)
-            bouncePing();
         // We request t3.
         getdata = (GetDataMessage) outbound(writeTarget);
         assertEquals(t3, getdata.getItems().get(0).hash);
         // Can't find it: bottom of tree.
-        if (useNotFound) {
+        NotFoundMessage notFound = new NotFoundMessage(unitTestParams);
-            NotFoundMessage notFound = new NotFoundMessage(unitTestParams);
+        notFound.addItem(new InventoryItem(InventoryItem.Type.Transaction, t3));
-            notFound.addItem(new InventoryItem(InventoryItem.Type.Transaction, t3));
+        inbound(writeTarget, notFound);
-            inbound(writeTarget, notFound);
-        } else {
-            bouncePing();
-        }
         pingAndWait(writeTarget);
         Threading.waitForUserCode();
         // We're done but still not notified because it was timelocked.