From 22ff79dd8a66aca5129c13962261da90034dafe2 Mon Sep 17 00:00:00 2001
From: Mike Hearn <hearn@google.com>
Date: Mon, 24 Dec 2012 23:48:48 +0000
Subject: [PATCH] Re-evaluate download peer when a new peer connects. Add unit
 tests for selecting the best peer.

---
 .../com/google/bitcoin/core/PeerGroup.java    | 21 ++++++++++++++-----
 .../google/bitcoin/core/PeerGroupTest.java    | 15 +++++++++----
 .../google/bitcoin/examples/PeerMonitor.java  |  2 +-
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/core/src/main/java/com/google/bitcoin/core/PeerGroup.java b/core/src/main/java/com/google/bitcoin/core/PeerGroup.java
index ed98f69a..b4a471c5 100644
--- a/core/src/main/java/com/google/bitcoin/core/PeerGroup.java
+++ b/core/src/main/java/com/google/bitcoin/core/PeerGroup.java
@@ -76,7 +76,7 @@ public class PeerGroup extends AbstractIdleService {
     private List<Peer> pendingPeers;
     private Map<Peer, ChannelFuture> channelFutures;
 
-    // The peer we are currently downloading the chain from
+    // The peer that has been selected for the purposes of downloading announced data.
     private Peer downloadPeer;
     // Callback for events related to chain download
     private PeerEventListener downloadListener;
@@ -641,18 +641,21 @@ public class PeerGroup extends AbstractIdleService {
     }
 
     protected synchronized void handleNewPeer(final Peer peer) {
-        // Runs on a netty worker thread for every peer that is newly connected.
+        // Runs on a netty worker thread for every peer that is newly connected. Peer is not locked at this point.
         log.info("{}: New peer", peer);
         // Link the peer to the memory pool so broadcast transactions have their confidence levels updated.
         peer.setMemoryPool(memoryPool);
+        peer.setDownloadData(false);
         // If we want to download the chain, and we aren't currently doing so, do so now.
         if (downloadListener != null && downloadPeer == null && chain != null) {
             log.info("  starting block chain download");
             startBlockChainDownloadFromPeer(peer);
-        } else if (downloadPeer == null) {
-            setDownloadPeer(selectDownloadPeer(peers));
         } else {
-            peer.setDownloadData(false);
+            // Re-evaluate download peers.
+            Peer newDownloadPeer = selectDownloadPeer(peers);
+            if (downloadPeer != newDownloadPeer) {
+                setDownloadPeer(newDownloadPeer);
+            }
         }
         // Make sure the peer knows how to upload transactions that are requested from us.
         peer.addEventListener(getDataListener);
@@ -1134,4 +1137,12 @@ public class PeerGroup extends AbstractIdleService {
             return t;
         }
     }
+
+    /**
+     * Returns the currently selected download peer. Bear in mind that it may have changed as soon as this method
+     * returns. Can return null if no peer was selected.
+     */
+    public Peer getDownloadPeer() {
+        return downloadPeer;
+    }
 }
diff --git a/core/src/test/java/com/google/bitcoin/core/PeerGroupTest.java b/core/src/test/java/com/google/bitcoin/core/PeerGroupTest.java
index b40d9c8c..0d171c1c 100644
--- a/core/src/test/java/com/google/bitcoin/core/PeerGroupTest.java
+++ b/core/src/test/java/com/google/bitcoin/core/PeerGroupTest.java
@@ -292,6 +292,8 @@ public class PeerGroupTest extends TestWithNetworkConnections {
         FakeChannel p1 = connectPeer(1, new VersionMessage(params, 2));
         FakeChannel p2 = connectPeer(2);
 
+        assertNotNull(peerGroup.getDownloadPeer());
+
         control.replay();
 
         peerGroup.setMinBroadcastConnections(2);
@@ -407,13 +409,18 @@ public class PeerGroupTest extends TestWithNetworkConnections {
         peerGroup.startAndWait();
         VersionMessage versionMessage2 = new VersionMessage(params, 2);
         VersionMessage versionMessage3 = new VersionMessage(params, 3);
-        connectPeer(1, versionMessage2);
+        assertNull(peerGroup.getDownloadPeer());
+        Peer a = PeerGroup.peerFromChannel(connectPeer(1, versionMessage2));
         assertEquals(2, peerGroup.getMostCommonChainHeight());
-        connectPeer(2, versionMessage2);
+        assertEquals(a, peerGroup.getDownloadPeer());
+        Peer b = PeerGroup.peerFromChannel(connectPeer(2, versionMessage2));
         assertEquals(2, peerGroup.getMostCommonChainHeight());
-        connectPeer(3, versionMessage3);
+        assertEquals(a, peerGroup.getDownloadPeer());  // No change.
+        Peer c = PeerGroup.peerFromChannel(connectPeer(3, versionMessage3));
         assertEquals(2, peerGroup.getMostCommonChainHeight());
-        connectPeer(4, versionMessage3);
+        assertEquals(a, peerGroup.getDownloadPeer());  // No change yet.
+        Peer d = PeerGroup.peerFromChannel(connectPeer(4, versionMessage3));
         assertEquals(3, peerGroup.getMostCommonChainHeight());
+        assertEquals(c, peerGroup.getDownloadPeer());  // Switch to first peer advertising new height.
     }
 }
diff --git a/examples/src/main/java/com/google/bitcoin/examples/PeerMonitor.java b/examples/src/main/java/com/google/bitcoin/examples/PeerMonitor.java
index 5e0ba4b8..1feac967 100644
--- a/examples/src/main/java/com/google/bitcoin/examples/PeerMonitor.java
+++ b/examples/src/main/java/com/google/bitcoin/examples/PeerMonitor.java
@@ -251,7 +251,7 @@ public class PeerMonitor {
                 setFont(normal);
                 setForeground(Color.LIGHT_GRAY);
             } else {
-                if (model.connectedPeers.get(row).getDownloadData())
+                if (model.connectedPeers.get(row) == peerGroup.getDownloadPeer())
                     setFont(bold);
                 else
                     setFont(normal);