Add new PoA network option to use bootnodes during any peer table ref…

…resh, not just the first one (hyperledger#7314) * Add new config option to use bootnodes during any peer table refresh, not just the first one Signed-off-by: Matthew Whitehead <[email protected]> * Update everything-config list Signed-off-by: Matthew Whitehead <[email protected]> * Revert debug setting Signed-off-by: Matthew Whitehead <[email protected]> * Update ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java Co-authored-by: Stefan Pingel <[email protected]> Signed-off-by: Matt Whitehead <[email protected]> --------- Signed-off-by: Matthew Whitehead <[email protected]> Signed-off-by: Matt Whitehead <[email protected]> Co-authored-by: Stefan Pingel <[email protected]>
Consensys · Jul 23, 2024 · 7e4a25a · 7e4a25a
1 parent bcbfec0
commit 7e4a25a
Show file tree

Hide file tree

Showing 10 changed files with 90 additions and 2 deletions.
diff --git 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
@@ -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() {}
@@ -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.
    *
@@ -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
@@ -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.
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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/...um/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/config/DiscoveryConfiguration.java b/...um/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<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;
   }

diff --git a/...eum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java b/...eum/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/...in/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/...in/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<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;
@@ -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<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();
   }
@@ -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;
@@ -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;
+    }
   }
 }