Deprecate --Xp2p-peer-lower-bound (hyperledger#6501)

* deprecate p2p-peer-lower-bound

Signed-off-by: Sally MacFarlane <[email protected]>

---------

Signed-off-by: Sally MacFarlane <[email protected]>
Co-authored-by: Stefan Pingel <[email protected]>

CHANGELOG.md

@@ -12,6 +12,7 @@
 
 ### Deprecations
 - `--Xsnapsync-synchronizer-flat-db-healing-enabled` is deprecated (always enabled). [#6499](https://github.com/hyperledger/besu/pull/6499)
+- `--Xp2p-peer-lower-bound` [#6501](https://github.com/hyperledger/besu/pull/6501)
 
 ### Additions and Improvements
 - Upgrade Prometheus and Opentelemetry dependencies [#6422](https://github.com/hyperledger/besu/pull/6422)

acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java

@@ -211,7 +211,6 @@ public void startNode(final BesuNode node) {
                 .map(pkiConfig -> new PkiBlockCreationConfigurationProvider().load(pkiConfig)))
         .evmConfiguration(EvmConfiguration.DEFAULT)
         .maxPeers(maxPeers)
-        .lowerBoundPeers(maxPeers)
         .maxRemotelyInitiatedPeers(15)
         .networkConfiguration(node.getNetworkingConfiguration())
         .randomPeerPriority(false)

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

@@ -21,6 +21,7 @@
 import static java.util.Collections.singletonList;
 import static org.hyperledger.besu.cli.DefaultCommandValues.getDefaultBesuDataPath;
 import static org.hyperledger.besu.cli.config.NetworkName.MAINNET;
+import static org.hyperledger.besu.cli.options.unstable.NetworkingOptions.PEER_LOWER_BOUND_FLAG;
 import static org.hyperledger.besu.cli.util.CommandLineUtils.DEPENDENCY_WARNING_MSG;
 import static org.hyperledger.besu.cli.util.CommandLineUtils.isOptionSet;
 import static org.hyperledger.besu.controller.BesuController.DATABASE_PATH;
@@ -320,7 +321,6 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
 
   private int maxPeers;
   private int maxRemoteInitiatedPeers;
-  private int peersLowerBound;
 
   // CLI options defined by user at runtime.
   // Options parsing is done with CLI library Picocli https://picocli.info/
@@ -1523,18 +1523,11 @@ private void validateDnsOptionsParams() {
 
   private void ensureValidPeerBoundParams() {
     maxPeers = p2PDiscoveryOptionGroup.maxPeers;
-    peersLowerBound = unstableNetworkingOptions.toDomainObject().getPeerLowerBound();
-    if (peersLowerBound > maxPeers) {
-      logger.warn(
-          "`--Xp2p-peer-lower-bound` "
-              + peersLowerBound
-              + " must not exceed --max-peers "
-              + maxPeers);
-      logger.warn("setting --Xp2p-peer-lower-bound=" + maxPeers);
-      peersLowerBound = maxPeers;
-    }
     final Boolean isLimitRemoteWireConnectionsEnabled =
         p2PDiscoveryOptionGroup.isLimitRemoteWireConnectionsEnabled;
+    if (isOptionSet(commandLine, PEER_LOWER_BOUND_FLAG)) {
+      logger.warn(PEER_LOWER_BOUND_FLAG + " is deprecated and will be removed soon.");
+    }
     if (isLimitRemoteWireConnectionsEnabled) {
       final float fraction =
           Fraction.fromPercentage(p2PDiscoveryOptionGroup.maxRemoteConnectionsPercentage)
@@ -1794,7 +1787,6 @@ public BesuControllerBuilder getControllerBuilder() {
         .evmConfiguration(unstableEvmOptions.toDomainObject())
         .dataStorageConfiguration(dataStorageOptions.toDomainObject())
         .maxPeers(p2PDiscoveryOptionGroup.maxPeers)
-        .lowerBoundPeers(peersLowerBound)
         .maxRemotelyInitiatedPeers(maxRemoteInitiatedPeers)
         .randomPeerPriority(p2PDiscoveryOptionGroup.randomPeerPriority)
         .chainPruningConfiguration(unstableChainPruningOptions.toDomainObject())

besu/src/main/java/org/hyperledger/besu/cli/DefaultCommandValues.java

@@ -74,8 +74,6 @@ public interface DefaultCommandValues {
   int SYNC_MIN_PEER_COUNT = 5;
   /** The constant DEFAULT_MAX_PEERS. */
   int DEFAULT_MAX_PEERS = 25;
-  /** The constant DEFAULT_P2P_PEER_LOWER_BOUND. */
-  int DEFAULT_P2P_PEER_LOWER_BOUND = 25;
   /** The constant DEFAULT_HTTP_MAX_CONNECTIONS. */
   int DEFAULT_HTTP_MAX_CONNECTIONS = 80;
   /** The constant DEFAULT_HTTP_MAX_BATCH_SIZE. */

besu/src/main/java/org/hyperledger/besu/cli/options/unstable/NetworkingOptions.java

@@ -80,8 +80,8 @@ public class NetworkingOptions implements CLIOptions<NetworkingConfiguration> {
       hidden = true,
       names = PEER_LOWER_BOUND_FLAG,
       description =
-          "Lower bound on the target number of P2P connections (default: ${DEFAULT-VALUE})")
-  private Integer peerLowerBoundConfig = DefaultCommandValues.DEFAULT_P2P_PEER_LOWER_BOUND;
+          "(Deprecated) Lower bound on the target number of P2P connections (default: ${DEFAULT-VALUE})")
+  private final Integer peerLowerBoundConfig = DefaultCommandValues.DEFAULT_MAX_PEERS;
 
   private NetworkingOptions() {}
 
@@ -107,7 +107,6 @@ public static NetworkingOptions fromConfig(final NetworkingConfiguration network
     cliOptions.initiateConnectionsFrequencySec =
         networkingConfig.getInitiateConnectionsFrequencySec();
     cliOptions.dnsDiscoveryServerOverride = networkingConfig.getDnsDiscoveryServerOverride();
-    cliOptions.peerLowerBoundConfig = networkingConfig.getPeerLowerBound();
 
     return cliOptions;
   }
@@ -120,7 +119,6 @@ public NetworkingConfiguration toDomainObject() {
     config.setDnsDiscoveryServerOverride(dnsDiscoveryServerOverride);
     config.getDiscovery().setDiscoveryV5Enabled(isPeerDiscoveryV5Enabled);
     config.getDiscovery().setFilterOnEnrForkId(filterOnEnrForkId);
-    config.setPeerLowerBound(peerLowerBoundConfig);
     return config;
   }
 

besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java

@@ -178,7 +178,6 @@ public abstract class BesuControllerBuilder implements MiningParameterOverrides
   /** The Max peers. */
   protected int maxPeers;
 
-  private int peerLowerBound;
   private int maxRemotelyInitiatedPeers;
   /** The Chain pruner configuration. */
   protected ChainPrunerConfiguration chainPrunerConfiguration = ChainPrunerConfiguration.DEFAULT;
@@ -475,22 +474,10 @@ public BesuControllerBuilder maxPeers(final int maxPeers) {
     return this;
   }
 
-  /**
-   * Lower bound of peers where we stop actively trying to initiate new outgoing connections
-   *
-   * @param peerLowerBound lower bound of peers where we stop actively trying to initiate new
-   *     outgoing connections
-   * @return the besu controller builder
-   */
-  public BesuControllerBuilder lowerBoundPeers(final int peerLowerBound) {
-    this.peerLowerBound = peerLowerBound;
-    return this;
-  }
-
   /**
    * Maximum number of remotely initiated peer connections
    *
-   * @param maxRemotelyInitiatedPeers aximum number of remotely initiated peer connections
+   * @param maxRemotelyInitiatedPeers maximum number of remotely initiated peer connections
    * @return the besu controller builder
    */
   public BesuControllerBuilder maxRemotelyInitiatedPeers(final int maxRemotelyInitiatedPeers) {
@@ -511,7 +498,7 @@ public BesuControllerBuilder chainPruningConfiguration(
   }
 
   /**
-   * Chain pruning configuration besu controller builder.
+   * Sets the number of blocks to cache.
    *
    * @param numberOfBlocksToCache the number of blocks to cache
    * @return the besu controller builder
@@ -681,7 +668,6 @@ public BesuController build() {
             maxMessageSize,
             messagePermissioningProviders,
             nodeKey.getPublicKey().getEncodedBytes(),
-            peerLowerBound,
             maxPeers,
             maxRemotelyInitiatedPeers,
             randomPeerPriority);

besu/src/test/java/org/hyperledger/besu/RunnerTest.java

@@ -462,7 +462,6 @@ private BesuController getController(
         .networkConfiguration(NetworkingConfiguration.create())
         .randomPeerPriority(Boolean.FALSE)
         .maxPeers(25)
-        .lowerBoundPeers(25)
         .maxRemotelyInitiatedPeers(15)
         .build();
   }

besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java

@@ -253,7 +253,6 @@ public void callingBesuCommandWithoutOptionsMustSyncWithDefaultValues() {
     verify(mockControllerBuilder).storageProvider(storageProviderArgumentCaptor.capture());
     verify(mockControllerBuilder).gasLimitCalculator(eq(GasLimitCalculator.constant()));
     verify(mockControllerBuilder).maxPeers(eq(maxPeers));
-    verify(mockControllerBuilder).lowerBoundPeers(eq(maxPeers));
     verify(mockControllerBuilder).maxRemotelyInitiatedPeers(eq((int) Math.floor(0.6 * maxPeers)));
     verify(mockControllerBuilder).build();
 
@@ -1040,7 +1039,7 @@ public void maxpeersOptionMustBeUsed() {
   }
 
   @Test
-  public void p2pPeerUpperBound_without_p2pPeerLowerBound_shouldSetLowerBoundEqualToUpperBound() {
+  public void p2pPeerUpperBound_without_p2pPeerLowerBound_shouldSetMaxPeers() {
 
     final int maxPeers = 23;
     parseCommand("--p2p-peer-upper-bound", String.valueOf(maxPeers));
@@ -1051,29 +1050,6 @@ public void p2pPeerUpperBound_without_p2pPeerLowerBound_shouldSetLowerBoundEqual
     verify(mockControllerBuilder).maxPeers(intArgumentCaptor.capture());
     assertThat(intArgumentCaptor.getValue()).isEqualTo(maxPeers);
 
-    verify(mockControllerBuilder).lowerBoundPeers(intArgumentCaptor.capture());
-    assertThat(intArgumentCaptor.getValue()).isEqualTo(maxPeers);
-
-    verify(mockRunnerBuilder).build();
-  }
-
-  @Test
-  public void maxpeersSet_p2pPeerLowerBoundSet() {
-
-    final int maxPeers = 123;
-    final int minPeers = 66;
-    parseCommand(
-        "--max-peers",
-        String.valueOf(maxPeers),
-        "--Xp2p-peer-lower-bound",
-        String.valueOf(minPeers));
-
-    verify(mockControllerBuilder).maxPeers(intArgumentCaptor.capture());
-    assertThat(intArgumentCaptor.getValue()).isEqualTo(maxPeers);
-
-    verify(mockControllerBuilder).lowerBoundPeers(intArgumentCaptor.capture());
-    assertThat(intArgumentCaptor.getValue()).isEqualTo(minPeers);
-
     verify(mockRunnerBuilder).build();
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
@@ -3302,20 +3278,6 @@ public void checkpointMergeAtGenesisWithGenesisBlockDifficultyZeroFails() throws
             "PoS checkpoint sync can't be used with TTD = 0 and checkpoint totalDifficulty = 0");
   }
 
-  @Test
-  public void checkP2pPeerLowerBound_isSet() {
-    final int lowerBound = 13;
-    parseCommand("--Xp2p-peer-lower-bound", String.valueOf(lowerBound));
-
-    verify(mockControllerBuilder).lowerBoundPeers(intArgumentCaptor.capture());
-    verify(mockControllerBuilder).build();
-
-    assertThat(intArgumentCaptor.getValue()).isEqualTo(lowerBound);
-
-    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
-    assertThat(commandOutput.toString(UTF_8)).isEmpty();
-  }
-
   @Test
   public void kzgTrustedSetupFileRequiresDataBlobEnabledNetwork() throws IOException {
     final Path genesisFileWithoutBlobs =

besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java

@@ -288,7 +288,6 @@ public void initMocks() throws Exception {
     when(mockControllerBuilder.maxPeers(anyInt())).thenReturn(mockControllerBuilder);
     when(mockControllerBuilder.chainPruningConfiguration(any())).thenReturn(mockControllerBuilder);
     when(mockControllerBuilder.maxPeers(anyInt())).thenReturn(mockControllerBuilder);
-    when(mockControllerBuilder.lowerBoundPeers(anyInt())).thenReturn(mockControllerBuilder);
     when(mockControllerBuilder.maxRemotelyInitiatedPeers(anyInt()))
         .thenReturn(mockControllerBuilder);
     when(mockControllerBuilder.transactionSelectorFactory(any())).thenReturn(mockControllerBuilder);

besu/src/test/java/org/hyperledger/besu/cli/options/NetworkingOptionsTest.java

@@ -176,7 +176,6 @@ protected NetworkingConfiguration createCustomizedDomainObject() {
         NetworkingConfiguration.DEFAULT_INITIATE_CONNECTIONS_FREQUENCY_SEC + 10);
     config.setCheckMaintainedConnectionsFrequency(
         NetworkingConfiguration.DEFAULT_CHECK_MAINTAINED_CONNECTIONS_FREQUENCY_SEC + 10);
-    config.setPeerLowerBound(NetworkingConfiguration.DEFAULT_PEER_LOWER_BOUND - 10);
     return config;
   }
 

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java

@@ -87,7 +87,6 @@ public class EthPeers {
   private final Subscribers<ConnectCallback> connectCallbacks = Subscribers.create();
   private final Subscribers<DisconnectCallback> disconnectCallbacks = Subscribers.create();
   private final Collection<PendingPeerRequest> pendingRequests = new CopyOnWriteArrayList<>();
-  private final int peerLowerBound;
   private final int peerUpperBound;
   private final int maxRemotelyInitiatedConnections;
   private final Boolean randomPeerPriority;
@@ -108,7 +107,6 @@ public EthPeers(
       final int maxMessageSize,
       final List<NodeMessagePermissioningProvider> permissioningProviders,
       final Bytes localNodeId,
-      final int peerLowerBound,
       final int peerUpperBound,
       final int maxRemotelyInitiatedConnections,
       final Boolean randomPeerPriority) {
@@ -119,15 +117,10 @@ public EthPeers(
     this.maxMessageSize = maxMessageSize;
     this.bestPeerComparator = HEAVIEST_CHAIN;
     this.localNodeId = localNodeId;
-    this.peerLowerBound = peerLowerBound;
     this.peerUpperBound = peerUpperBound;
     this.maxRemotelyInitiatedConnections = maxRemotelyInitiatedConnections;
     this.randomPeerPriority = randomPeerPriority;
-    LOG.trace(
-        "MaxPeers: {}, Lower Bound: {}, Max Remote: {}",
-        peerUpperBound,
-        peerLowerBound,
-        maxRemotelyInitiatedConnections);
+    LOG.trace("MaxPeers: {}, Max Remote: {}", peerUpperBound, maxRemotelyInitiatedConnections);
     metricsSystem.createIntegerGauge(
         BesuMetricCategory.ETHEREUM,
         "peer_count",
@@ -175,10 +168,6 @@ public void registerNewConnection(
     }
   }
 
-  public int getPeerLowerBound() {
-    return peerLowerBound;
-  }
-
   @NotNull
   private List<PeerConnection> getIncompleteConnections(final Bytes id) {
     return incompleteConnections.asMap().keySet().stream()

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTestUtil.java

@@ -86,7 +86,6 @@ public static EthProtocolManager create(
             Bytes.random(64),
             25,
             25,
-            25,
             false);
     final EthMessages messages = new EthMessages();
     final EthScheduler ethScheduler = new DeterministicEthScheduler(TimeoutPolicy.NEVER_TIMEOUT);
@@ -206,7 +205,6 @@ public static EthProtocolManager create(
             Bytes.random(64),
             25,
             25,
-            25,
             false);
     final EthMessages messages = new EthMessages();
 
@@ -241,7 +239,6 @@ public static EthProtocolManager create(
             Bytes.random(64),
             25,
             25,
-            25,
             false);
     final EthMessages messages = new EthMessages();
 
@@ -272,7 +269,6 @@ public static EthProtocolManager create(
             Bytes.random(64),
             25,
             25,
-            25,
             false);
     final EthMessages messages = new EthMessages();
 

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java

@@ -118,7 +118,6 @@ public void setupTest() {
                 Bytes.random(64),
                 MAX_PEERS,
                 MAX_PEERS,
-                MAX_PEERS,
                 false));
 
     final EthMessages ethMessages = new EthMessages();

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/AbstractBlockPropagationManagerTest.java

@@ -630,7 +630,6 @@ public void shouldNotImportBlocksThatAreAlreadyBeingImported() {
                 Bytes.random(64),
                 25,
                 25,
-                25,
                 false),
             new EthMessages(),
             ethScheduler);
@@ -769,7 +768,6 @@ public Object answer(final InvocationOnMock invocation) throws Throwable {
                 Bytes.random(64),
                 25,
                 25,
-                25,
                 false),
             new EthMessages(),
             ethScheduler);

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TestNode.java

@@ -147,7 +147,6 @@ public boolean isMessagePermitted(final EnodeURL destinationEnode, final int cod
             Bytes.random(64),
             25,
             25,
-            25,
             false);
 
     final EthScheduler scheduler = new EthScheduler(1, 1, 1, metricsSystem);

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactoryTest.java

@@ -110,7 +110,6 @@ public void setup() {
             Bytes.random(64),
             25,
             25,
-            25,
             false);
     when(ethContext.getEthMessages()).thenReturn(ethMessages);
     when(ethContext.getEthPeers()).thenReturn(ethPeers);

ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/NetworkingConfiguration.java

@@ -30,7 +30,6 @@ public class NetworkingConfiguration {
   private int initiateConnectionsFrequencySec = DEFAULT_INITIATE_CONNECTIONS_FREQUENCY_SEC;
   private int checkMaintainedConnectionsFrequencySec =
       DEFAULT_CHECK_MAINTAINED_CONNECTIONS_FREQUENCY_SEC;
-  private Integer peerLowerBound = DEFAULT_PEER_LOWER_BOUND;
   private Optional<String> dnsDiscoveryServerOverride = Optional.empty();
 
   public static NetworkingConfiguration create() {
@@ -87,16 +86,6 @@ public NetworkingConfiguration setCheckMaintainedConnectionsFrequency(
     return this;
   }
 
-  public Integer getPeerLowerBound() {
-    return peerLowerBound;
-  }
-
-  public NetworkingConfiguration setPeerLowerBound(final Integer peerLowerBoundConfig) {
-    checkArgument(peerLowerBoundConfig > 0);
-    this.peerLowerBound = peerLowerBoundConfig;
-    return this;
-  }
-
   @Override
   public boolean equals(final Object o) {
     if (o == this) {

ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java

@@ -231,7 +231,6 @@ private boolean buildContext(
             localNodeKey,
             MAX_PEERS,
             MAX_PEERS,
-            MAX_PEERS,
             false);
     final SyncState syncState = new SyncState(blockchain, ethPeers);