From 81b78292e21ba745dfd51ca7cfdab43fcd44d728 Mon Sep 17 00:00:00 2001
From: Sean Gilligan <sean@msgilligan.com>
Date: Sat, 12 Aug 2023 19:48:41 -0700
Subject: [PATCH] PeerGroup, FilterMerger: deprecate setting false-positive
 rate

In the bitcoinj code itself, the false-positive rate is never changed
after constructing a `PeerGroup` or a `FilterMerger`.

* Deprecate methods for setting Bloom Filter FP rate in both methods
* Add constructor params to `PeerGroup` so they can be set at construction
  time, if non-default values are needed
---
 .../java/org/bitcoinj/core/PeerGroup.java     | 20 ++++++++++++++++---
 .../java/org/bitcoinj/net/FilterMerger.java   |  2 ++
 .../bitcoinj/testing/TestWithPeerGroup.java   |  2 +-
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/core/src/main/java/org/bitcoinj/core/PeerGroup.java b/core/src/main/java/org/bitcoinj/core/PeerGroup.java
index 793bf08c2..b6f719405 100644
--- a/core/src/main/java/org/bitcoinj/core/PeerGroup.java
+++ b/core/src/main/java/org/bitcoinj/core/PeerGroup.java
@@ -417,7 +417,19 @@ public class PeerGroup implements TransactionBroadcaster {
      * @param connectionManager used to create new connections and keep track of existing ones.
      */
     protected PeerGroup(Network network, @Nullable AbstractBlockChain chain, ClientConnectionManager connectionManager) {
-        this(NetworkParameters.of(Objects.requireNonNull(network)), chain, connectionManager);
+        this(NetworkParameters.of(Objects.requireNonNull(network)), chain, connectionManager, DEFAULT_BLOOM_FILTER_FP_RATE);
+    }
+
+
+    /**
+     * Create a PeerGroup for the given network, chain and connection manager.
+     * @param network the P2P network to connect to
+     * @param chain used to process blocks
+     * @param connectionManager used to create new connections and keep track of existing ones.
+     * @param bloomFilterFpRate false positive rate for bloom filters
+     */
+    protected PeerGroup(Network network, @Nullable AbstractBlockChain chain, ClientConnectionManager connectionManager, double bloomFilterFpRate) {
+        this(NetworkParameters.of(Objects.requireNonNull(network)), chain, connectionManager, bloomFilterFpRate);
     }
 
     /**
@@ -425,9 +437,10 @@ public class PeerGroup implements TransactionBroadcaster {
      * @param params the P2P network to connect to
      * @param chain used to process blocks
      * @param connectionManager used to create new connections and keep track of existing ones.
+     * @param bloomFilterFpRate false positive rate for bloom filters
      */
     // For testing only
-    protected PeerGroup(NetworkParameters params, @Nullable AbstractBlockChain chain, ClientConnectionManager connectionManager) {
+    protected PeerGroup(NetworkParameters params, @Nullable AbstractBlockChain chain, ClientConnectionManager connectionManager, double bloomFilterFpRate) {
         Objects.requireNonNull(params);
         Context.getOrCreate(); // create a context for convenience
         this.params = params;
@@ -473,7 +486,7 @@ public class PeerGroup implements TransactionBroadcaster {
         channels = connectionManager;
         peerDiscoverers = new CopyOnWriteArraySet<>();
         runningBroadcasts = Collections.synchronizedSet(new HashSet<TransactionBroadcast>());
-        bloomFilterMerger = new FilterMerger(DEFAULT_BLOOM_FILTER_FP_RATE);
+        bloomFilterMerger = new FilterMerger(bloomFilterFpRate);
         vMinRequiredProtocolVersion = ProtocolVersion.BLOOM_FILTER.intValue();
     }
 
@@ -1430,6 +1443,7 @@ public class PeerGroup implements TransactionBroadcaster {
      * <p>See the docs for {@link BloomFilter#BloomFilter(int, double, int, BloomFilter.BloomUpdate)} for a brief
      * explanation of anonymity when using bloom filters.</p>
      */
+    @Deprecated
     public void setBloomFilterFalsePositiveRate(double bloomFilterFPRate) {
         lock.lock();
         try {
diff --git a/core/src/main/java/org/bitcoinj/net/FilterMerger.java b/core/src/main/java/org/bitcoinj/net/FilterMerger.java
index 062149677..cd8dcc22a 100644
--- a/core/src/main/java/org/bitcoinj/net/FilterMerger.java
+++ b/core/src/main/java/org/bitcoinj/net/FilterMerger.java
@@ -46,6 +46,7 @@ public class FilterMerger {
     // We use a constant tweak to avoid giving up privacy when we regenerate our filter with new keys
     private final int bloomFilterTweak = new Random().nextInt();
 
+    // TODO: Make final after deprecated setBloomFilterFPRate() method is removed
     private volatile double vBloomFilterFPRate;
     private int lastBloomFilterElementCount;
     private BloomFilter lastFilter;
@@ -108,6 +109,7 @@ public class FilterMerger {
         }
     }
 
+    @Deprecated
     public void setBloomFilterFPRate(double bloomFilterFPRate) {
         this.vBloomFilterFPRate = bloomFilterFPRate;
     }
diff --git a/integration-test/src/test/java/org/bitcoinj/testing/TestWithPeerGroup.java b/integration-test/src/test/java/org/bitcoinj/testing/TestWithPeerGroup.java
index daee2de08..42b5dc8bc 100644
--- a/integration-test/src/test/java/org/bitcoinj/testing/TestWithPeerGroup.java
+++ b/integration-test/src/test/java/org/bitcoinj/testing/TestWithPeerGroup.java
@@ -113,7 +113,7 @@ public class TestWithPeerGroup extends TestWithNetworkConnections {
     protected final Semaphore jobBlocks = new Semaphore(0);
 
     private PeerGroup createPeerGroup(final ClientConnectionManager manager) {
-        return new PeerGroup(UNITTEST, blockChain, manager) {
+        return new PeerGroup(UNITTEST, blockChain, manager, PeerGroup.DEFAULT_BLOOM_FILTER_FP_RATE) {
             @Override
             protected ListeningScheduledExecutorService createPrivateExecutor() {
                 return MoreExecutors.listeningDecorator(new ScheduledThreadPoolExecutor(1, new ContextPropagatingThreadFactory("PeerGroup test thread")) {