Fix a crash that can occur if a peer reports a chain height of zero (this is a protocol violation but such crashes were seen in the wild).

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

@@ -1417,39 +1417,11 @@ public class PeerGroup extends AbstractExecutionThreadService implements Transac
      * If multiple heights are tied, the highest is returned. If no peers are connected, returns zero.
      */
     public static int getMostCommonChainHeight(final List<Peer> peers) {
-        int s = peers.size();
-        int[] heights = new int[s];
-        int[] counts = new int[s];
-        int maxCount = 0;
-        // Calculate the frequencies of each reported height.
-        for (Peer peer : peers) {
-            int h = (int) peer.getBestHeight();
-            // Find the index of the peers height in the heights array.
-            for (int cursor = 0; cursor < s; cursor++) {
-                if (heights[cursor] == h) {
-                    maxCount = Math.max(++counts[cursor], maxCount);
-                    break;
-                } else if (heights[cursor] == 0) {
-                    // A new height we didn't see before.
-                    checkState(counts[cursor] == 0);
-                    heights[cursor] = h;
-                    counts[cursor] = 1;
-                    maxCount = Math.max(maxCount, 1);
-                    break;
-                }
-            }
-        }
-        // Find the heights that have the highest frequencies.
-        int[] freqHeights = new int[s];
-        int cursor = 0;
-        for (int i = 0; i < s; i++) {
-            if (counts[i] == maxCount) {
-                freqHeights[cursor++] = heights[i];
-            }
-        }
-        // Return the highest of the most common heights.
-        Arrays.sort(freqHeights);
-        return freqHeights[s - 1];
+        if (peers.isEmpty())
+            return 0;
+        List<Integer> heights = new ArrayList<Integer>(peers.size());
+        for (Peer peer : peers) heights.add((int) peer.getBestHeight());
+        return Utils.maxOfMostFreq(heights);
     }
 
     private static class PeerAndPing {

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

@@ -18,6 +18,9 @@
 package com.google.bitcoin.core;
 
 import com.google.common.base.Charsets;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Ordering;
+import com.google.common.primitives.Ints;
 import com.google.common.primitives.UnsignedLongs;
 import org.spongycastle.crypto.digests.RIPEMD160Digest;
 import org.spongycastle.util.encoders.Hex;
@@ -29,8 +32,7 @@ import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.util.Arrays;
-import java.util.Date;
+import java.util.*;
 import java.util.concurrent.ArrayBlockingQueue;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.TimeUnit;
@@ -609,4 +611,43 @@ public class Utils {
             mockSleepQueue.offer(true);
         }
     }
+
+    private static class Pair implements Comparable<Pair> {
+        int item, count;
+        public Pair(int item, int count) { this.count = count; this.item = item; }
+        @Override public int compareTo(Pair o) { return -Ints.compare(count, o.count); }
+    }
+
+    public static int maxOfMostFreq(int... items) {
+        // Java 6 sucks.
+        ArrayList<Integer> list = new ArrayList<Integer>(items.length);
+        for (int item : items) list.add(item);
+        return maxOfMostFreq(list);
+    }
+
+    public static int maxOfMostFreq(List<Integer> items) {
+        if (items.isEmpty())
+            return 0;
+        // This would be much easier in a functional language (or in Java 8).
+        items = Ordering.natural().reverse().sortedCopy(items);
+        LinkedList<Pair> pairs = Lists.newLinkedList();
+        pairs.add(new Pair(items.get(0), 0));
+        for (int item : items) {
+            Pair pair = pairs.getLast();
+            if (pair.item != item)
+                pairs.add((pair = new Pair(item, 0)));
+            pair.count++;
+        }
+        // pairs now contains a uniqified list of the sorted inputs, with counts for how often that item appeared.
+        // Now sort by how frequently they occur, and pick the max of the most frequent.
+        Collections.sort(pairs);
+        int maxCount = pairs.getFirst().count;
+        int maxItem = pairs.getFirst().item;
+        for (Pair pair : pairs) {
+            if (pair.count != maxCount)
+                break;
+            maxItem = Math.max(maxItem, pair.item);
+        }
+        return maxItem;
+    }
 }

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

@@ -115,4 +115,13 @@ public class UtilsTest {
         Assert.assertArrayEquals(new byte[0], Utils.reverseDwordBytes(new byte[] {4,3,2,1,8,7,6,5}, 0));
         Assert.assertArrayEquals(new byte[0], Utils.reverseDwordBytes(new byte[0], 0));
     }
+
+    @Test
+    public void testMaxOfMostFreq() throws Exception {
+        assertEquals(0, Utils.maxOfMostFreq());
+        assertEquals(0, Utils.maxOfMostFreq(0, 0, 1));
+        assertEquals(2, Utils.maxOfMostFreq(1, 1, 2, 2));
+        assertEquals(1, Utils.maxOfMostFreq(1, 1, 2, 2, 1));
+        assertEquals(-1, Utils.maxOfMostFreq(-1, -1, 2, 2, -1));
+    }
 }