diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d29b8f8af8..001aaab5a82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ ### Additions and Improvements +- 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 ## 24.7.0 diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java index 7ed627cfc17..dfd09165b7d 100644 --- a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java @@ -195,6 +195,7 @@ public class RunnerBuilder { private boolean legacyForkIdEnabled; private Optional enodeDnsConfiguration; private List allowedSubnets = new ArrayList<>(); + private boolean poaDiscoveryRetryBootnodes = true; /** Instantiates a new Runner builder. */ public RunnerBuilder() {} @@ -603,6 +604,17 @@ public RunnerBuilder allowedSubnets(final List 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. * @@ -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()); @@ -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() diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 12752f2e9a1..b17c59c89a7 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -517,6 +517,19 @@ void setBannedNodeIds(final List 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 bannedNodeIds = new ArrayList<>(); // Used to discover the default IP of the client. @@ -2330,6 +2343,7 @@ private Runner synchronize( .rpcEndpointService(rpcEndpointServiceImpl) .enodeDnsConfiguration(getEnodeDnsConfiguration()) .allowedSubnets(p2PDiscoveryOptionGroup.allowedSubnets) + .poaDiscoveryRetryBootnodes(p2PDiscoveryOptionGroup.poaDiscoveryRetryBootnodes) .build(); addShutdownHook(runner); diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index 641e69dba64..fd759f8acee 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -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"); diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java index f620c729b68..db2cd66c6d8 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java @@ -357,6 +357,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(); diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/DiscoveryConfiguration.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/DiscoveryConfiguration.java index 86bb079a298..3f59abf2fdf 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/DiscoveryConfiguration.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/DiscoveryConfiguration.java @@ -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(); @@ -88,6 +89,16 @@ public DiscoveryConfiguration setBootnodes(final List bootnodes) { return this; } + public boolean getIncludeBootnodesOnPeerRefresh() { + return includeBootnodesOnPeerRefresh; + } + + public DiscoveryConfiguration setIncludeBootnodesOnPeerRefresh( + final boolean includeBootnodesOnPeerRefresh) { + this.includeBootnodesOnPeerRefresh = includeBootnodesOnPeerRefresh; + return this; + } + public String getAdvertisedHost() { return advertisedHost; } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java index d1f17b5304e..7efc50750fb 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java @@ -274,6 +274,7 @@ private PeerDiscoveryController createController(final DiscoveryPeer localNode) .filterOnEnrForkId((config.isFilterOnEnrForkIdEnabled())) .rlpxAgent(rlpxAgent) .peerTable(peerTable) + .includeBootnodesOnPeerRefresh(config.getIncludeBootnodesOnPeerRefresh()) .build(); } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 5d0fee18453..c25f8f1b347 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -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, @@ -159,7 +160,8 @@ private PeerDiscoveryController( final MetricsSystem metricsSystem, final Optional> maybeCacheForEnrRequests, final boolean filterOnEnrForkId, - final RlpxAgent rlpxAgent) { + final RlpxAgent rlpxAgent, + final boolean includeBootnodesOnPeerRefresh) { this.timerUtil = timerUtil; this.nodeKey = nodeKey; this.localPeer = localPeer; @@ -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, @@ -483,7 +486,23 @@ RecursivePeerRefreshState getRecursivePeerRefreshState() { */ private void refreshTable() { final Bytes target = Peer.randomId(); - final List initialPeers = peerTable.nearestBondedPeers(Peer.randomId(), 16); + + final List initialPeers; + 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 = + Stream.concat( + bootstrapNodes.stream(), + peerTable.nearestBondedPeers(Peer.randomId(), 16).stream()) + .collect(Collectors.toList()); + } else { + initialPeers = peerTable.nearestBondedPeers(Peer.randomId(), 16); + } recursivePeerRefreshState.start(initialPeers, target); lastRefreshTime = System.currentTimeMillis(); } @@ -812,10 +831,11 @@ public static class Builder { private OutboundMessageHandler outboundMessageHandler = OutboundMessageHandler.NOOP; private PeerRequirement peerRequirement = PeerRequirement.NOOP; private PeerPermissions peerPermissions = PeerPermissions.noop(); - private long tableRefreshIntervalMs = MILLISECONDS.convert(30, TimeUnit.MINUTES); + private long tableRefreshIntervalMs = MILLISECONDS.convert(2, TimeUnit.MINUTES); private long cleanPeerTableIntervalMs = MILLISECONDS.convert(1, TimeUnit.MINUTES); private final List bootstrapNodes = new ArrayList<>(); private PeerTable peerTable; + private boolean includeBootnodesOnPeerRefresh = true; // Required dependencies private NodeKey nodeKey; @@ -849,7 +869,8 @@ public PeerDiscoveryController build() { metricsSystem, Optional.of(cachedEnrRequests), filterOnEnrForkId, - rlpxAgent); + rlpxAgent, + includeBootnodesOnPeerRefresh); } private void validate() { @@ -953,5 +974,10 @@ public Builder rlpxAgent(final RlpxAgent rlpxAgent) { this.rlpxAgent = rlpxAgent; return this; } + + public Builder includeBootnodesOnPeerRefresh(final boolean includeBootnodesOnPeerRefresh) { + this.includeBootnodesOnPeerRefresh = includeBootnodesOnPeerRefresh; + return this; + } } }