Fix some bugs that happen in chainless operation. Make Peer.getBestChainHeight() more accurate by taking into account blocks announced after a peer is connected, not just what it announced in the initial version message.

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

@@ -61,10 +61,14 @@ public class Peer {
     // Whether to try and download blocks and transactions from this peer. Set to false by PeerGroup if not the
     // primary peer. This is to avoid redundant work and concurrency problems with downloading the same chain
     // in parallel.
-    private boolean downloadData = true;
+    private boolean downloadData = false;
     // The version data to announce to the other side of the connections we make: useful for setting our "user agent"
     // equivalent and other things.
     private VersionMessage versionMessage;
+    // How many block messages the peer has announced to us. Peers only announce blocks that attach to their best chain
+    // so we can use this to calculate the height of the peers chain, by adding it to the initial height in the version
+    // message. This method can go wrong if the peer re-orgs onto a shorter (but harder) chain, however, this is rare.
+    private int blocksAnnounced;
     // A class that tracks recent transactions that have been broadcast across the network, counts how many
     // peers announced them and updates the transaction confidence data. It is passed to each Peer.
     private MemoryPool memoryPool;
@@ -98,6 +102,7 @@ public class Peer {
         this.params = Preconditions.checkNotNull(params);
         this.versionMessage = Preconditions.checkNotNull(ver);
         this.blockChain = chain;  // Allowed to be null.
+        this.downloadData = chain != null;
         this.pendingGetBlockFutures = new ArrayList<GetDataFuture<Block>>();
         this.eventListeners = new CopyOnWriteArrayList<PeerEventListener>();
         this.lifecycleListeners = new CopyOnWriteArrayList<PeerLifecycleListener>();
@@ -438,6 +443,20 @@ public class Peer {
             }
         }
 
+        if (transactions.size() == 0 && blocks.size() == 1) {
+            // Single block announcement. If we're downloading the chain this is just a tickle to make us continue
+            // (the block chain download protocol is very implicit and not well thought out). If we're not downloading
+            // the chain then this probably means a new block was solved and the peer believes it connects to the best
+            // chain, so count it. This way getBestChainHeight() can be accurate.
+            if (downloadData) {
+                if (!blockChain.isOrphan(blocks.get(0).hash)) {
+                    blocksAnnounced++;
+                }
+            } else {
+                blocksAnnounced++;
+            }
+        }
+
         GetDataMessage getdata = new GetDataMessage(params);
 
         Iterator<InventoryItem> it = transactions.iterator();
@@ -811,7 +830,7 @@ public class Peer {
      */
     public int getPeerBlockHeightDifference() {
         // Chain will overflow signed int blocks in ~41,000 years.
-        int chainHeight = (int) peerVersionMessage.bestHeight;
+        int chainHeight = (int) getBestHeight();
         // chainHeight should not be zero/negative because we shouldn't have given the user a Peer that is to another
         // client-mode node, nor should it be unconnected. If that happens it means the user overrode us somewhere or
         // there is a bug in the peer management code.
@@ -857,9 +876,9 @@ public class Peer {
     }
 
     /**
-     * @return the height of the best chain as claimed by peer.
+     * @return the height of the best chain as claimed by peer: sum of its ver announcement and blocks announced since.
      */
     public long getBestHeight() {
-      return peerVersionMessage.bestHeight;
+      return peerVersionMessage.bestHeight + blocksAnnounced;
     }
 }

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

@@ -218,6 +218,7 @@ public class PeerGroup {
         ExecutorService workerExecutor = Executors.newCachedThreadPool(new PeerGroupThreadFactory());
         NioClientSocketChannelFactory channelFactory = new NioClientSocketChannelFactory(bossExecutor, workerExecutor);
         ClientBootstrap bs = new ClientBootstrap(channelFactory);
+        bs.setOption("connectionTimeoutMillis", 2000);
         return bs;
     }
 
@@ -755,6 +756,8 @@ public class PeerGroup {
     }
 
     private synchronized void setDownloadPeer(Peer peer) {
+        if (chain == null)
+            return;
         if (downloadPeer != null) {
             log.info("Unsetting download peer: {}", downloadPeer);
             downloadPeer.setDownloadData(false);

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

@@ -41,6 +41,8 @@ import java.util.concurrent.Executors;
 
 import static org.jboss.netty.channel.Channels.write;
 
+// TODO: Remove this class and refactor the way we build Netty pipelines.
+
 /**
  * <p>A {@code TCPNetworkConnection} is used for connecting to a Bitcoin node over the standard TCP/IP protocol.<p>
  *

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

@@ -37,6 +37,7 @@ public class PeerTest extends TestWithNetworkConnections {
     private Peer peer;
     private Capture<DownstreamMessageEvent> event;
     private PeerHandler handler;
+    private static final int OTHER_PEER_CHAIN_HEIGHT = 110;
 
     @Override
     @Before
@@ -58,7 +59,7 @@ public class PeerTest extends TestWithNetworkConnections {
     
     private void connect(PeerHandler handler, Channel channel, ChannelHandlerContext ctx) throws Exception {
         handler.connectRequested(ctx, new UpstreamChannelStateEvent(channel, ChannelState.CONNECTED, socketAddress));
-        VersionMessage peerVersion = new VersionMessage(unitTestParams, 110);
+        VersionMessage peerVersion = new VersionMessage(unitTestParams, OTHER_PEER_CHAIN_HEIGHT);
         DownstreamMessageEvent versionEvent = 
             new DownstreamMessageEvent(channel, Channels.future(channel), peerVersion, null);
         handler.messageReceived(ctx, versionEvent);
@@ -309,15 +310,20 @@ public class PeerTest extends TestWithNetworkConnections {
         inv.addItem(item);
         expect(listener.onPreMessageReceived(eq(peer), eq(inv))).andReturn(inv);
         expect(listener.onPreMessageReceived(eq(peer), eq(b2))).andReturn(b2);
-        listener.onBlocksDownloaded(eq(peer), anyObject(Block.class), eq(108));
+        // We have two blocks in our chain (genesis and b1), so our height is 2. The other peer starts at
+        // OTHER_PEER_CHAIN_HEIGHT and then when it announces an inv, its height is + 1, so the difference
+        // between our height and theirs is OTHER_PEER_CHAIN_HEIGHT + 1 - 2.
+        listener.onBlocksDownloaded(eq(peer), anyObject(Block.class), eq(OTHER_PEER_CHAIN_HEIGHT + 1 - 2));
         expectLastCall();
 
         control.replay();
 
         connect();
         peer.addEventListener(listener);
+        long height = peer.getBestHeight();
         
         inbound(peer, inv);
+        assertEquals(height + 1, peer.getBestHeight());
         // Response to the getdata message.
         inbound(peer, b2);