From 7e4a25a9ba980f142d47d7efec859ff0e90c1394 Mon Sep 17 00:00:00 2001 From: Matt Whitehead Date: Tue, 23 Jul 2024 10:25:07 +0100 Subject: [PATCH] Add new PoA network option to use bootnodes during any peer table refresh, 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 * Update everything-config list Signed-off-by: Matthew Whitehead * Revert debug setting Signed-off-by: Matthew Whitehead * 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 --------- Signed-off-by: Matthew Whitehead Signed-off-by: Matt Whitehead Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> --- CHANGELOG.md | 2 ++ .../org/hyperledger/besu/RunnerBuilder.java | 15 ++++++++++++ .../org/hyperledger/besu/cli/BesuCommand.java | 14 +++++++++++ .../hyperledger/besu/cli/BesuCommandTest.java | 22 +++++++++++++++++ .../besu/cli/CommandTestAbstract.java | 1 + besu/src/test/resources/complete_config.toml | 1 + .../src/test/resources/everything_config.toml | 1 + .../p2p/config/DiscoveryConfiguration.java | 11 +++++++++ .../p2p/discovery/PeerDiscoveryAgent.java | 1 + .../internal/PeerDiscoveryController.java | 24 +++++++++++++++++-- 10 files changed, 90 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c6cb373ba8..d0e62bc5ef6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) 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 198c26fe2a8..2607f6e5953 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -513,6 +513,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. @@ -2324,6 +2337,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 efeff412d62..c892ffe7fe6 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 8a6a5781198..c77ced5c6fe 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java @@ -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(); diff --git a/besu/src/test/resources/complete_config.toml b/besu/src/test/resources/complete_config.toml index 3243b056135..8ae2105efd6 100644 --- a/besu/src/test/resources/complete_config.toml +++ b/besu/src/test/resources/complete_config.toml @@ -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", diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml index a5e03bd5990..27ea5c16453 100644 --- a/besu/src/test/resources/everything_config.toml +++ b/besu/src/test/resources/everything_config.toml @@ -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", 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..11b5216a82b 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,17 @@ RecursivePeerRefreshState getRecursivePeerRefreshState() { */ private void refreshTable() { final Bytes target = Peer.randomId(); + final List 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(); } @@ -816,6 +829,7 @@ public static class Builder { 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 +863,8 @@ public PeerDiscoveryController build() { metricsSystem, Optional.of(cachedEnrRequests), filterOnEnrForkId, - rlpxAgent); + rlpxAgent, + includeBootnodesOnPeerRefresh); } private void validate() { @@ -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; + } } }