BlockLocator: deprecate add & no-args constructor

Deprecate `.add()` and no-args constructor in favor of providing complete
list of hashes at creation time.

Update all usages to use the alternative methods.

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

@@ -19,6 +19,7 @@ package org.bitcoinj.core;
 import org.bitcoinj.base.Sha256Hash;
 import org.bitcoinj.base.internal.InternalUtils;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.stream.Collectors;
@@ -30,6 +31,10 @@ import java.util.stream.Stream;
 public final class BlockLocator {
     private final List<Sha256Hash> hashes;  // unmodifiable list
 
+    /**
+     * @deprecated Use {@link BlockLocator#BlockLocator(List)}
+     */
+    @Deprecated
     public BlockLocator() {
         hashes = Collections.emptyList();
     }
@@ -41,7 +46,15 @@ public final class BlockLocator {
         this.hashes = Collections.unmodifiableList(hashes);
     }
 
+    // Used by tests
+    static BlockLocator ofBlocks(Block... blocks) {
+        return new BlockLocator(Arrays.stream(blocks)
+                .map(Block::getHash)
+                .collect(Collectors.toList()));
+    }
+
     // Create a new BlockLocator by copying an instance and appending an element
+    @Deprecated
     private BlockLocator(BlockLocator old, Sha256Hash hashToAdd) {
         this(Stream.concat(old.hashes.stream(), Stream.of(hashToAdd))
                 .collect(Collectors.toList())
@@ -50,7 +63,9 @@ public final class BlockLocator {
 
     /**
      * Add a {@link Sha256Hash} to a newly created block locator.
+     * @deprecated Use {@link BlockLocator#BlockLocator(List)}
      */
+    @Deprecated
     public BlockLocator add(Sha256Hash hash) {
         return new BlockLocator(this, hash);
     }

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

@@ -25,6 +25,8 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.BufferUnderflowException;
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.List;
 
 import static org.bitcoinj.base.internal.Preconditions.check;
 
@@ -54,12 +56,12 @@ public class GetBlocksMessage extends BaseMessage {
         int startCount = startCountVarInt.intValue();
         if (startCount > 500)
             throw new ProtocolException("Number of locators cannot be > 500, received: " + startCount);
-        BlockLocator locator = new BlockLocator();
+        List<Sha256Hash> hashList = new ArrayList<>();
         for (int i = 0; i < startCount; i++) {
-            locator = locator.add(Sha256Hash.read(payload));
+            hashList.add(Sha256Hash.read(payload));
         }
         Sha256Hash stopHash = Sha256Hash.read(payload);
-        return new GetBlocksMessage(version, locator, stopHash);
+        return new GetBlocksMessage(version, new BlockLocator(hashList), stopHash);
     }
 
     public GetBlocksMessage(long protocolVersion, BlockLocator locator, Sha256Hash stopHash) {

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

@@ -22,6 +22,8 @@ import org.bitcoinj.base.internal.ByteUtils;
 
 import java.nio.BufferUnderflowException;
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.List;
 
 import static org.bitcoinj.base.internal.Preconditions.check;
 
@@ -48,12 +50,12 @@ public class GetHeadersMessage extends GetBlocksMessage {
         int startCount = startCountVarInt.intValue();
         if (startCount > 500)
             throw new ProtocolException("Number of locators cannot be > 500, received: " + startCount);
-        BlockLocator locator = new BlockLocator();
+        List<Sha256Hash> hashList = new ArrayList<>();
         for (int i = 0; i < startCount; i++) {
-            locator = locator.add(Sha256Hash.read(payload));
+            hashList.add(Sha256Hash.read(payload));
         }
         Sha256Hash stopHash = Sha256Hash.read(payload);
-        return new GetHeadersMessage(version, locator, stopHash);
+        return new GetHeadersMessage(version, new BlockLocator(hashList), stopHash);
     }
 
     public GetHeadersMessage(long protocolVersion, BlockLocator locator, Sha256Hash stopHash) {

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

@@ -1415,7 +1415,6 @@ public class Peer extends PeerSocketHandler {
         // headers and then request the blocks from that point onwards. "getheaders" does not send us an inv, it just
         // sends us the data we requested in a "headers" message.
 
-        BlockLocator blockLocator = new BlockLocator();
         // For now we don't do the exponential thinning as suggested here:
         //
         //   https://en.bitcoin.it/wiki/Protocol_specification#getblocks
@@ -1437,9 +1436,10 @@ public class Peer extends PeerSocketHandler {
         if (log.isDebugEnabled())
             log.debug("{}: blockChainDownloadLocked({}) current head = {}",
                     this, toHash, chainHead.getHeader().getHashAsString());
+        List<Sha256Hash> hashes = new ArrayList<>(100);
         StoredBlock cursor = chainHead;
         for (int i = 100; cursor != null && i > 0; i--) {
-            blockLocator = blockLocator.add(cursor.getHeader().getHash());
+            hashes.add(cursor.getHeader().getHash());
             try {
                 cursor = cursor.getPrev(store);
             } catch (BlockStoreException e) {
@@ -1449,7 +1449,8 @@ public class Peer extends PeerSocketHandler {
         }
         // Only add the genesis hash to the locator if we didn't already do so. If the chain is < 100 blocks we already reached it.
         if (cursor != null)
-            blockLocator = blockLocator.add(params.getGenesisBlock().getHash());
+            hashes.add(params.getGenesisBlock().getHash());
+        BlockLocator blockLocator = new BlockLocator(hashes);
 
         // Record that we requested this range of blocks so we can filter out duplicate requests in the event of a
         // block being solved during chain download.

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

@@ -213,8 +213,7 @@ public class BitcoindComparisonTool {
 
         connectedFuture.get();
 
-        BlockLocator locator = new BlockLocator();
+        BlockLocator locator = new BlockLocator(Collections.singletonList(PARAMS.getGenesisBlock().getHash()));
-        locator = locator.add(PARAMS.getGenesisBlock().getHash());
         Sha256Hash hashTo = Sha256Hash.wrap("0000000000000000000000000000000000000000000000000000000000000000");
                 
         int rulesSinceFirstFail = 0;
@@ -302,8 +301,7 @@ public class BitcoindComparisonTool {
                 if (block.throwsException)
                     blocksRequested.remove(nextBlock.getHash());
                 //bitcoind.sendMessage(nextBlock);
-                locator = new BlockLocator();
+                locator = new BlockLocator(Collections.singletonList(bitcoindChainHead));
-                locator = locator.add(bitcoindChainHead);
                 bitcoind.sendMessage(new GetHeadersMessage(PARAMS.getSerializer().getProtocolVersion(), locator, hashTo));
                 bitcoind.sendPing().get();
                 if (!chain.getChainHead().getHeader().getHash().equals(bitcoindChainHead)) {

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

@@ -214,9 +214,9 @@ public class PeerTest extends TestWithNetworkConnections {
         inbound(writeTarget, inv);
 
         GetBlocksMessage getblocks = (GetBlocksMessage)outbound(writeTarget);
-        BlockLocator expectedLocator = new BlockLocator();
+        BlockLocator expectedLocator = BlockLocator.ofBlocks(
-        expectedLocator = expectedLocator.add(b1.getHash());
+                b1,
-        expectedLocator = expectedLocator.add(TESTNET.getGenesisBlock().getHash());
+                TESTNET.getGenesisBlock());
 
         assertEquals(getblocks.getLocator(), expectedLocator);
         assertEquals(getblocks.getStopHash(), b3.getHash());
@@ -384,10 +384,10 @@ public class PeerTest extends TestWithNetworkConnections {
         });
         peer.startBlockChainDownload();
 
-        BlockLocator expectedLocator = new BlockLocator();
+        BlockLocator expectedLocator = BlockLocator.ofBlocks(
-        expectedLocator = expectedLocator.add(b2.getHash());
+            b2,
-        expectedLocator = expectedLocator.add(b1.getHash());
+            b1,
-        expectedLocator = expectedLocator.add(TESTNET.getGenesisBlock().getHash());
+            TESTNET.getGenesisBlock());
 
         GetBlocksMessage message = (GetBlocksMessage) outbound(writeTarget);
         assertEquals(message.getLocator(), expectedLocator);
@@ -471,19 +471,19 @@ public class PeerTest extends TestWithNetworkConnections {
         );
         peer.startBlockChainDownload();
         GetHeadersMessage getheaders = (GetHeadersMessage) outbound(writeTarget);
-        BlockLocator expectedLocator = new BlockLocator();
+        BlockLocator expectedLocator = BlockLocator.ofBlocks(
-        expectedLocator = expectedLocator.add(b1.getHash());
+                b1,
-        expectedLocator = expectedLocator.add(TESTNET.getGenesisBlock().getHash());
+                TESTNET.getGenesisBlock());
         assertEquals(getheaders.getLocator(), expectedLocator);
         assertEquals(getheaders.getStopHash(), Sha256Hash.ZERO_HASH);
         // Now send all the headers.
         HeadersMessage headers = new HeadersMessage(b2.cloneAsHeader(),
                 b3.cloneAsHeader(), b4.cloneAsHeader());
         // We expect to be asked for b3 and b4 again, but this time, with a body.
-        expectedLocator = new BlockLocator();
+        expectedLocator = BlockLocator.ofBlocks(
-        expectedLocator = expectedLocator.add(b2.getHash());
+                b2,
-        expectedLocator = expectedLocator.add(b1.getHash());
+                b1,
-        expectedLocator = expectedLocator.add(TESTNET.getGenesisBlock().getHash());
+                TESTNET.getGenesisBlock());
         inbound(writeTarget, headers);
         GetBlocksMessage getblocks = (GetBlocksMessage) outbound(writeTarget);
         assertEquals(expectedLocator, getblocks.getLocator());