Add new config option to use bootnodes during any peer table refresh,…

… not just the first one

Signed-off-by: Matthew Whitehead <[email protected]>

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

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()

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

@@ -517,6 +517,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.
@@ -2330,6 +2343,7 @@ private Runner synchronize(
             .rpcEndpointService(rpcEndpointServiceImpl)
             .enodeDnsConfiguration(getEnodeDnsConfiguration())
             .allowedSubnets(p2PDiscoveryOptionGroup.allowedSubnets)
+            .poaDiscoveryRetryBootnodes(p2PDiscoveryOptionGroup.poaDiscoveryRetryBootnodes)
             .build();
 
     addShutdownHook(runner);

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");

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();

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<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;
   }

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();
   }
 

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<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,23 @@ RecursivePeerRefreshState getRecursivePeerRefreshState() {
    */
   private void refreshTable() {
     final Bytes target = Peer.randomId();
-    final List<DiscoveryPeer> initialPeers = peerTable.nearestBondedPeers(Peer.randomId(), 16);
+
+    final List<DiscoveryPeer> 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<DiscoveryPeer> 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;
+    }
   }
 }