Skip to content

Commit

Permalink
Add new PoA network option to use bootnodes during any peer table ref…
Browse files Browse the repository at this point in the history
…resh, not just the first one (#7314)

* Add new config option to use bootnodes during any peer table refresh, not just the first one

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Update everything-config list

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Revert debug setting

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Update ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java

Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
Signed-off-by: Matt Whitehead <matthew1001@hotmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matt Whitehead <matthew1001@hotmail.com>
Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>
  • Loading branch information
matthew1001 and pinges authored Jul 23, 2024
1 parent bcbfec0 commit 7e4a25a
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
- Add trie log pruner metrics [#7352](https://github.com/hyperledger/besu/pull/7352)
- `--Xbonsai-parallel-tx-processing-enabled` option enables executing transactions in parallel during block processing for Bonsai nodes

- Add option `--poa-discovery-retry-bootnodes` for PoA networks to always use bootnodes during peer refresh, not just on first start [#7314](https://github.com/hyperledger/besu/pull/7314)

### Bug fixes
- Fix `eth_call` deserialization to correctly ignore unknown fields in the transaction object. [#7323](https://github.com/hyperledger/besu/pull/7323)

Expand Down
15 changes: 15 additions & 0 deletions besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ public class RunnerBuilder {
private boolean legacyForkIdEnabled;
private Optional<EnodeDnsConfiguration> enodeDnsConfiguration;
private List<SubnetInfo> allowedSubnets = new ArrayList<>();
private boolean poaDiscoveryRetryBootnodes = true;

/** Instantiates a new Runner builder. */
public RunnerBuilder() {}
Expand Down Expand Up @@ -603,6 +604,17 @@ public RunnerBuilder allowedSubnets(final List<SubnetInfo> allowedSubnets) {
return this;
}

/**
* Flag to indicate if peer table refreshes should always query bootnodes
*
* @param poaDiscoveryRetryBootnodes whether to always query bootnodes
* @return the runner builder
*/
public RunnerBuilder poaDiscoveryRetryBootnodes(final boolean poaDiscoveryRetryBootnodes) {
this.poaDiscoveryRetryBootnodes = poaDiscoveryRetryBootnodes;
return this;
}

/**
* Build Runner instance.
*
Expand All @@ -625,6 +637,8 @@ public Runner build() {
bootstrap = ethNetworkConfig.bootNodes();
}
discoveryConfiguration.setBootnodes(bootstrap);
discoveryConfiguration.setIncludeBootnodesOnPeerRefresh(
besuController.getGenesisConfigOptions().isPoa() && poaDiscoveryRetryBootnodes);
LOG.info("Resolved {} bootnodes.", bootstrap.size());
LOG.debug("Bootnodes = {}", bootstrap);
discoveryConfiguration.setDnsDiscoveryURL(ethNetworkConfig.dnsDiscoveryUrl());
Expand Down Expand Up @@ -694,6 +708,7 @@ public Runner build() {
final boolean fallbackEnabled = natMethod == NatMethod.AUTO || natMethodFallbackEnabled;
final NatService natService = new NatService(buildNatManager(natMethod), fallbackEnabled);
final NetworkBuilder inactiveNetwork = caps -> new NoopP2PNetwork();

final NetworkBuilder activeNetwork =
caps -> {
return DefaultP2PNetwork.builder()
Expand Down
14 changes: 14 additions & 0 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,19 @@ void setBannedNodeIds(final List<String> values) {
}
}

// Boolean option to set that in a PoA network the bootnodes should always be queried during
// peer table refresh. If this flag is disabled bootnodes are only sent FINDN requests on first
// startup, meaning that an offline bootnode or network outage at the client can prevent it
// discovering any peers without a restart.
@Option(
names = {"--poa-discovery-retry-bootnodes"},
description =
"Always use of bootnodes for discovery in PoA networks. Disabling this reverts "
+ " to the same behaviour as non-PoA networks, where neighbours are only discovered from bootnodes on first startup."
+ "(default: ${DEFAULT-VALUE})",
arity = "1")
private final Boolean poaDiscoveryRetryBootnodes = true;

private Collection<Bytes> bannedNodeIds = new ArrayList<>();

// Used to discover the default IP of the client.
Expand Down Expand Up @@ -2324,6 +2337,7 @@ private Runner synchronize(
.rpcEndpointService(rpcEndpointServiceImpl)
.enodeDnsConfiguration(getEnodeDnsConfiguration())
.allowedSubnets(p2PDiscoveryOptionGroup.allowedSubnets)
.poaDiscoveryRetryBootnodes(p2PDiscoveryOptionGroup.poaDiscoveryRetryBootnodes)
.build();

addShutdownHook(runner);
Expand Down
22 changes: 22 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 @@ -804,6 +804,28 @@ public void bootnodesUrlCliArgTakesPrecedenceOverGenesisFile() throws IOExceptio
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
}

@Test
public void poaDiscoveryRetryBootnodesValueTrueMustBeUsed() {
parseCommand("--poa-discovery-retry-bootnodes", "true");

verify(mockRunnerBuilder).poaDiscoveryRetryBootnodes(eq(true));
verify(mockRunnerBuilder).build();

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

@Test
public void poaDiscoveryRetryBootnodesValueFalseMustBeUsed() {
parseCommand("--poa-discovery-retry-bootnodes", "false");

verify(mockRunnerBuilder).poaDiscoveryRetryBootnodes(eq(false));
verify(mockRunnerBuilder).build();

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

@Test
public void callingWithBootnodesOptionButNoValueMustPassEmptyBootnodeList() {
parseCommand("--bootnodes");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ public void initMocks() throws Exception {
when(mockRunnerBuilder.apiConfiguration(any())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.enodeDnsConfiguration(any())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.allowedSubnets(any())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.poaDiscoveryRetryBootnodes(anyBoolean())).thenReturn(mockRunnerBuilder);
when(mockRunnerBuilder.build()).thenReturn(mockRunner);

final SignatureAlgorithm signatureAlgorithm = SignatureAlgorithmFactory.getInstance();
Expand Down
1 change: 1 addition & 0 deletions besu/src/test/resources/complete_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ data-path="/opt/besu" # Path

# network
discovery-enabled=false
poa-discovery-retry-bootnodes=true
bootnodes=[
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567",
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567",
Expand Down
1 change: 1 addition & 0 deletions besu/src/test/resources/everything_config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ nat-method="NONE"
Xnat-kube-service-name="besu"
Xnat-method-fallback-enabled=true
discovery-enabled=false
poa-discovery-retry-bootnodes=true
bootnodes=[
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567",
"enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:4567",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public class DiscoveryConfiguration {
private String dnsDiscoveryURL;
private boolean discoveryV5Enabled = false;
private boolean filterOnEnrForkId = NetworkingConfiguration.DEFAULT_FILTER_ON_ENR_FORK_ID;
private boolean includeBootnodesOnPeerRefresh = true;

public static DiscoveryConfiguration create() {
return new DiscoveryConfiguration();
Expand Down Expand Up @@ -88,6 +89,16 @@ public DiscoveryConfiguration setBootnodes(final List<EnodeURL> bootnodes) {
return this;
}

public boolean getIncludeBootnodesOnPeerRefresh() {
return includeBootnodesOnPeerRefresh;
}

public DiscoveryConfiguration setIncludeBootnodesOnPeerRefresh(
final boolean includeBootnodesOnPeerRefresh) {
this.includeBootnodesOnPeerRefresh = includeBootnodesOnPeerRefresh;
return this;
}

public String getAdvertisedHost() {
return advertisedHost;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ private PeerDiscoveryController createController(final DiscoveryPeer localNode)
.filterOnEnrForkId((config.isFilterOnEnrForkIdEnabled()))
.rlpxAgent(rlpxAgent)
.peerTable(peerTable)
.includeBootnodesOnPeerRefresh(config.getIncludeBootnodesOnPeerRefresh())
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public class PeerDiscoveryController {
private final AtomicBoolean peerTableIsDirty = new AtomicBoolean(false);
private OptionalLong cleanTableTimerId = OptionalLong.empty();
private RecursivePeerRefreshState recursivePeerRefreshState;
private final boolean includeBootnodesOnPeerRefresh;

private PeerDiscoveryController(
final NodeKey nodeKey,
Expand All @@ -159,7 +160,8 @@ private PeerDiscoveryController(
final MetricsSystem metricsSystem,
final Optional<Cache<Bytes, Packet>> maybeCacheForEnrRequests,
final boolean filterOnEnrForkId,
final RlpxAgent rlpxAgent) {
final RlpxAgent rlpxAgent,
final boolean includeBootnodesOnPeerRefresh) {
this.timerUtil = timerUtil;
this.nodeKey = nodeKey;
this.localPeer = localPeer;
Expand All @@ -173,6 +175,7 @@ private PeerDiscoveryController(
this.discoveryProtocolLogger = new DiscoveryProtocolLogger(metricsSystem);
this.peerPermissions = new PeerDiscoveryPermissions(localPeer, peerPermissions);
this.rlpxAgent = rlpxAgent;
this.includeBootnodesOnPeerRefresh = includeBootnodesOnPeerRefresh;

metricsSystem.createIntegerGauge(
BesuMetricCategory.NETWORK,
Expand Down Expand Up @@ -483,7 +486,17 @@ RecursivePeerRefreshState getRecursivePeerRefreshState() {
*/
private void refreshTable() {
final Bytes target = Peer.randomId();

final List<DiscoveryPeer> initialPeers = peerTable.nearestBondedPeers(Peer.randomId(), 16);
if (includeBootnodesOnPeerRefresh) {
bootstrapNodes.stream()
.filter(p -> p.getStatus() != PeerDiscoveryStatus.BONDED)
.forEach(p -> p.setStatus(PeerDiscoveryStatus.KNOWN));

// If configured to retry bootnodes during peer table refresh, include them
// in the initial peers list.
initialPeers.addAll(bootstrapNodes);
}
recursivePeerRefreshState.start(initialPeers, target);
lastRefreshTime = System.currentTimeMillis();
}
Expand Down Expand Up @@ -816,6 +829,7 @@ public static class Builder {
private long cleanPeerTableIntervalMs = MILLISECONDS.convert(1, TimeUnit.MINUTES);
private final List<DiscoveryPeer> bootstrapNodes = new ArrayList<>();
private PeerTable peerTable;
private boolean includeBootnodesOnPeerRefresh = true;

// Required dependencies
private NodeKey nodeKey;
Expand Down Expand Up @@ -849,7 +863,8 @@ public PeerDiscoveryController build() {
metricsSystem,
Optional.of(cachedEnrRequests),
filterOnEnrForkId,
rlpxAgent);
rlpxAgent,
includeBootnodesOnPeerRefresh);
}

private void validate() {
Expand Down Expand Up @@ -953,5 +968,10 @@ public Builder rlpxAgent(final RlpxAgent rlpxAgent) {
this.rlpxAgent = rlpxAgent;
return this;
}

public Builder includeBootnodesOnPeerRefresh(final boolean includeBootnodesOnPeerRefresh) {
this.includeBootnodesOnPeerRefresh = includeBootnodesOnPeerRefresh;
return this;
}
}
}

0 comments on commit 7e4a25a

Please sign in to comment.