Skip to content

Commit

Permalink
lower bound peer limit (#4200)
Browse files Browse the repository at this point in the history
* introducing a lower bound on number of peers
* added lower bound CLI option

Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
Signed-off-by: Roman Vaseev <4833306+Filter94@users.noreply.github.com>
Co-authored-by: Roman Vaseev <4833306+Filter94@users.noreply.github.com>
Co-authored-by: Justin Florentine <justin+github@florentine.us>
  • Loading branch information
3 people committed Aug 9, 2022
1 parent 95d9626 commit 37e0e04
Show file tree
Hide file tree
Showing 17 changed files with 223 additions and 59 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## 22.7.1

### Additions and Improvements
- Add experimental CLI option for `--Xp2p-peer-lower-bound` [#4200](https://github.com/hyperledger/besu/pull/4200)

### Bug Fixes
- Fixes off-by-one error for mainnet TTD fallback [#4223](https://github.com/hyperledger/besu/pull/4223)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ public void startNode(final BesuNode node) {
.build();

final int maxPeers = 25;
final int minPeers = 25;

builder
.synchronizerConfiguration(new SynchronizerConfiguration.Builder().build())
Expand Down Expand Up @@ -198,6 +199,7 @@ public void startNode(final BesuNode node) {
.p2pAdvertisedHost(node.getHostName())
.p2pListenPort(0)
.maxPeers(maxPeers)
.minPeers(minPeers)
.networkingConfiguration(node.getNetworkingConfiguration())
.jsonRpcConfiguration(node.jsonRpcConfiguration())
.webSocketConfiguration(node.webSocketConfiguration())
Expand Down
13 changes: 12 additions & 1 deletion besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ public class RunnerBuilder {
private String natManagerServiceName;
private boolean natMethodFallbackEnabled;
private int maxPeers;
private int minPeers;
private boolean limitRemoteWireConnectionsEnabled = false;
private float fractionRemoteConnectionsAllowed;
private EthNetworkConfig ethNetworkConfig;
Expand Down Expand Up @@ -444,7 +445,8 @@ public Runner build() {
RlpxConfiguration.create()
.setBindHost(p2pListenInterface)
.setBindPort(p2pListenPort)
.setMaxPeers(maxPeers)
.setPeerUpperBound(maxPeers)
.setPeerLowerBound(minPeers)
.setSupportedProtocols(subProtocols)
.setClientId(BesuInfo.nodeName(identityString))
.setLimitRemoteWireConnectionsEnabled(limitRemoteWireConnectionsEnabled)
Expand Down Expand Up @@ -1157,4 +1159,13 @@ private Optional<MetricsService> createMetricsService(
final Vertx vertx, final MetricsConfiguration configuration) {
return MetricsService.create(vertx, configuration, metricsSystem);
}

public int getMinPeers() {
return minPeers;
}

public RunnerBuilder minPeers(final int minPeers) {
this.minPeers = minPeers;
return this;
}
}
23 changes: 22 additions & 1 deletion besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -424,11 +424,13 @@ static class P2PDiscoveryOptionGroup {
private final Integer p2pPort = EnodeURLImpl.DEFAULT_LISTENING_PORT;

@Option(
names = {"--max-peers"},
names = {"--max-peers", "--p2p-peer-upper-bound"},
paramLabel = MANDATORY_INTEGER_FORMAT_HELP,
description = "Maximum P2P connections that can be established (default: ${DEFAULT-VALUE})")
private final Integer maxPeers = DEFAULT_MAX_PEERS;

private int minPeers;

@Option(
names = {"--remote-connections-limit-enabled"},
description =
Expand Down Expand Up @@ -1602,6 +1604,7 @@ private Runner buildRunner() {
p2PDiscoveryOptionGroup.peerDiscoveryEnabled,
ethNetworkConfig,
p2PDiscoveryOptionGroup.maxPeers,
p2PDiscoveryOptionGroup.minPeers,
p2PDiscoveryOptionGroup.p2pHost,
p2PDiscoveryOptionGroup.p2pInterface,
p2PDiscoveryOptionGroup.p2pPort,
Expand Down Expand Up @@ -1724,6 +1727,7 @@ private void validateOptions() {
validateNatParams();
validateNetStatsParams();
validateDnsOptionsParams();
ensureValidPeerBoundParams();
validateRpcOptionsParams();
p2pTLSConfigOptions.checkP2PTLSOptionsDependencies(logger, commandLine);
pkiBlockCreationOptions.checkPkiBlockCreationOptionsDependencies(logger, commandLine);
Expand Down Expand Up @@ -1796,6 +1800,21 @@ private void validateDnsOptionsParams() {
}
}

private void ensureValidPeerBoundParams() {
final int min = unstableNetworkingOptions.toDomainObject().getRlpx().getPeerLowerBound();
final int max = p2PDiscoveryOptionGroup.maxPeers;
if (min > max) {
logger.warn("`--Xp2p-peer-lower-bound` " + min + " must not exceed --max-peers " + max);
// modify the --X lower-bound value if it's not valid, we don't want unstable defaults
// breaking things
logger.warn("setting --Xp2p-peer-lower-bound=" + max);
unstableNetworkingOptions.toDomainObject().getRlpx().setPeerLowerBound(max);
p2PDiscoveryOptionGroup.minPeers = max;
} else {
p2PDiscoveryOptionGroup.minPeers = min;
}
}

public void validateRpcOptionsParams() {
Predicate<String> configuredApis =
apiName ->
Expand Down Expand Up @@ -2770,6 +2789,7 @@ private Runner synchronize(
final boolean peerDiscoveryEnabled,
final EthNetworkConfig ethNetworkConfig,
final int maxPeers,
final int minPeers,
final String p2pAdvertisedHost,
final String p2pListenInterface,
final int p2pListenPort,
Expand Down Expand Up @@ -2804,6 +2824,7 @@ private Runner synchronize(
.p2pListenInterface(p2pListenInterface)
.p2pListenPort(p2pListenPort)
.maxPeers(maxPeers)
.minPeers(minPeers)
.limitRemoteWireConnectionsEnabled(
p2PDiscoveryOptionGroup.isLimitRemoteWireConnectionsEnabled)
.fractionRemoteConnectionsAllowed(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public interface DefaultCommandValues {
JwtAlgorithm DEFAULT_JWT_ALGORITHM = JwtAlgorithm.RS256;
int FAST_SYNC_MIN_PEER_COUNT = 5;
int DEFAULT_MAX_PEERS = 25;
int DEFAULT_P2P_PEER_LOWER_BOUND = 25;
int DEFAULT_HTTP_MAX_CONNECTIONS = 80;
int DEFAULT_WS_MAX_CONNECTIONS = 80;
int DEFAULT_WS_MAX_FRAME_SIZE = 1024 * 1024;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/
package org.hyperledger.besu.cli.options.unstable;

import org.hyperledger.besu.cli.DefaultCommandValues;
import org.hyperledger.besu.cli.options.CLIOptions;
import org.hyperledger.besu.cli.options.OptionParser;
import org.hyperledger.besu.ethereum.p2p.config.NetworkingConfiguration;
Expand All @@ -31,6 +32,7 @@ public class NetworkingOptions implements CLIOptions<NetworkingConfiguration> {
"--Xp2p-check-maintained-connections-frequency";
private final String DNS_DISCOVERY_SERVER_OVERRIDE_FLAG = "--Xp2p-dns-discovery-server";
private final String DISCOVERY_PROTOCOL_V5_ENABLED = "--Xv5-discovery-enabled";
private final String P2P_PEER_LOWER_BOUND_FLAG = "--Xp2p-peer-lower-bound";

@CommandLine.Option(
names = INITIATE_CONNECTIONS_FREQUENCY_FLAG,
Expand Down Expand Up @@ -66,6 +68,13 @@ public class NetworkingOptions implements CLIOptions<NetworkingConfiguration> {
description = "Whether to enable P2P Discovery Protocol v5 (default: ${DEFAULT-VALUE})")
private final Boolean isPeerDiscoveryV5Enabled = false;

@CommandLine.Option(
hidden = true,
names = {P2P_PEER_LOWER_BOUND_FLAG},
description =
"Lower bound on the target number of P2P connections (default: ${DEFAULT-VALUE})")
private final Integer peerLowerBound = DefaultCommandValues.DEFAULT_P2P_PEER_LOWER_BOUND;

private NetworkingOptions() {}

public static NetworkingOptions create() {
Expand All @@ -90,6 +99,7 @@ public NetworkingConfiguration toDomainObject() {
config.setInitiateConnectionsFrequency(initiateConnectionsFrequencySec);
config.setDnsDiscoveryServerOverride(dnsDiscoveryServerOverride);
config.getDiscovery().setDiscoveryV5Enabled(isPeerDiscoveryV5Enabled);
config.getRlpx().setPeerLowerBound(peerLowerBound);
return config;
}

Expand Down
41 changes: 41 additions & 0 deletions besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,47 @@ public void maxpeersOptionMustBeUsed() {
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}

@Test
public void p2pPeerUpperBound_without_p2pPeerLowerBound_shouldSetLowerBoundEqualToUpperBound() {

final int maxPeers = 23;
parseCommand("--p2p-peer-upper-bound", String.valueOf(maxPeers));

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();

verify(mockRunnerBuilder).maxPeers(intArgumentCaptor.capture());
assertThat(intArgumentCaptor.getValue()).isEqualTo(maxPeers);

verify(mockRunnerBuilder).minPeers(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(mockRunnerBuilder).maxPeers(intArgumentCaptor.capture());
assertThat(intArgumentCaptor.getValue()).isEqualTo(maxPeers);

verify(mockRunnerBuilder).minPeers(intArgumentCaptor.capture());
assertThat(intArgumentCaptor.getValue()).isEqualTo(minPeers);

verify(mockRunnerBuilder).build();

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}

@Test
public void remoteConnectionsPercentageOptionMustBeUsed() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ public void initMocks() throws Exception {
when(mockRunnerBuilder.p2pListenInterface(anyString())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.permissioningConfiguration(any())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.maxPeers(anyInt())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.minPeers(anyInt())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.limitRemoteWireConnectionsEnabled(anyBoolean()))
.thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.fractionRemoteConnectionsAllowed(anyFloat()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
import static org.hyperledger.besu.cli.DefaultCommandValues.DEFAULT_P2P_PEER_LOWER_BOUND;

import org.hyperledger.besu.cli.options.unstable.NetworkingOptions;
import org.hyperledger.besu.ethereum.p2p.config.NetworkingConfiguration;
Expand Down Expand Up @@ -125,6 +126,32 @@ public void checkDiscoveryV5Enabled_isNotSet() {
assertThat(commandOutput.toString(UTF_8)).isEmpty();
}

@Test
public void checkP2pPeerLowerBound_isSet() {
final int lowerBound = 13;
final TestBesuCommand cmd = parseCommand("--Xp2p-peer-lower-bound", String.valueOf(lowerBound));

final NetworkingOptions options = cmd.getNetworkingOptions();
final NetworkingConfiguration networkingConfig = options.toDomainObject();
assertThat(networkingConfig.getRlpx().getPeerLowerBound()).isEqualTo(lowerBound);

assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
assertThat(commandOutput.toString(UTF_8)).isEmpty();
}

@Test
public void checkP2pPeerLowerBound_isNotSet() {
final TestBesuCommand cmd = parseCommand();

final NetworkingOptions options = cmd.getNetworkingOptions();
final NetworkingConfiguration networkingConfig = options.toDomainObject();
assertThat(networkingConfig.getRlpx().getPeerLowerBound())
.isEqualTo(DEFAULT_P2P_PEER_LOWER_BOUND);

assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
assertThat(commandOutput.toString(UTF_8)).isEmpty();
}

@Override
NetworkingConfiguration createDefaultDomainObject() {
return NetworkingConfiguration.create();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ public void processMessage(final Capability cap, final Message message) {

if (this.mergePeerFilter.isPresent()) {
if (this.mergePeerFilter.get().disconnectIfGossipingBlocks(message, ethPeer)) {
LOG.debug("Post-merge disconnect: peer still gossiping blocks {}", ethPeer);
handleDisconnect(ethPeer.getConnection(), DisconnectReason.SUBPROTOCOL_TRIGGERED, false);
return;
}
Expand Down Expand Up @@ -389,6 +390,7 @@ private void handleStatusMessage(final EthPeer peer, final MessageData data) {
peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED);
} else if (mergePeerFilter.isPresent()
&& mergePeerFilter.get().disconnectIfPoW(status, peer)) {
LOG.debug("Post-merge disconnect: peer still PoW {}", peer);
handleDisconnect(peer.getConnection(), DisconnectReason.SUBPROTOCOL_TRIGGERED, false);
} else {
LOG.debug("Received status message from {}: {}", peer, status);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public class RlpxConfiguration {
private String clientId = "TestClient/1.0.0";
private String bindHost = NetworkUtility.INADDR_ANY;
private int bindPort = 30303;
private int maxPeers = 25;
private int peerUpperBound = 100;
private int peerLowerBound = 64;
private boolean limitRemoteWireConnectionsEnabled = false;
private float fractionRemoteWireConnectionsAllowed = DEFAULT_FRACTION_REMOTE_CONNECTIONS_ALLOWED;
private List<SubProtocol> supportedProtocols = Collections.emptyList();
Expand Down Expand Up @@ -70,13 +71,13 @@ public RlpxConfiguration setBindPort(final int bindPort) {
return this;
}

public RlpxConfiguration setMaxPeers(final int peers) {
maxPeers = peers;
public RlpxConfiguration setPeerUpperBound(final int peers) {
peerUpperBound = peers;
return this;
}

public int getMaxPeers() {
return maxPeers;
public int getPeerUpperBound() {
return peerUpperBound;
}

public String getClientId() {
Expand Down Expand Up @@ -105,10 +106,10 @@ public RlpxConfiguration setFractionRemoteWireConnectionsAllowed(

public int getMaxRemotelyInitiatedConnections() {
if (!limitRemoteWireConnectionsEnabled) {
return maxPeers;
return peerUpperBound;
}

return (int) Math.floor(maxPeers * fractionRemoteWireConnectionsAllowed);
return (int) Math.floor(peerUpperBound * fractionRemoteWireConnectionsAllowed);
}

@Override
Expand Down Expand Up @@ -136,4 +137,13 @@ public String toString() {
sb.append('}');
return sb.toString();
}

public RlpxConfiguration setPeerLowerBound(final int peers) {
peerLowerBound = peers;
return this;
}

public int getPeerLowerBound() {
return peerLowerBound;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,10 @@ public class DefaultP2PNetwork implements P2PNetwork {
this.nodeId = nodeKey.getPublicKey().getEncodedBytes();
this.peerPermissions = peerPermissions;

final int maxPeers = config.getRlpx().getMaxPeers();
peerDiscoveryAgent.addPeerRequirement(() -> rlpxAgent.getConnectionCount() >= maxPeers);
// set the requirement here that the number of peers be greater than the lower bound
final int peerLowerBound = config.getRlpx().getPeerLowerBound();
LOG.debug("setting peerLowerBound {}", peerLowerBound);
peerDiscoveryAgent.addPeerRequirement(() -> rlpxAgent.getConnectionCount() >= peerLowerBound);
subscribeDisconnect(reputationManager);
}

Expand Down
Loading

0 comments on commit 37e0e04

Please sign in to comment.