Check if discovery service is running before admin_addPeer (#8160)

Signed-off-by: Gabriel-Trintinalia <[email protected]>
hyperledger · Jan 23, 2025 · b150b10 · b150b10
1 parent 09581ea
commit b150b10
Show file tree

Hide file tree

Showing 24 changed files with 103 additions and 49 deletions.
diff --git a/...sl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java b/...sl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java
@@ -183,7 +183,7 @@ public void startNode(final BesuNode node) {
         .vertx(Vertx.vertx())
         .besuController(besuController)
         .ethNetworkConfig(ethNetworkConfig)
-        .discovery(node.isDiscoveryEnabled())
+        .discoveryEnabled(node.isDiscoveryEnabled())
         .p2pAdvertisedHost(node.getHostName())
         .p2pListenPort(0)
         .networkingConfiguration(node.getNetworkingConfiguration())

diff --git a/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java b/besu/src/main/java/org/hyperledger/besu/RunnerBuilder.java
@@ -165,7 +165,7 @@ public class RunnerBuilder {
   private NetworkingConfiguration networkingConfiguration = NetworkingConfiguration.create();
   private final Collection<Bytes> bannedNodeIds = new ArrayList<>();
   private boolean p2pEnabled = true;
-  private boolean discovery;
+  private boolean discoveryEnabled;
   private String p2pAdvertisedHost;
   private String p2pListenInterface = NetworkUtility.INADDR_ANY;
   private int p2pListenPort;
@@ -237,11 +237,11 @@ public RunnerBuilder p2pEnabled(final boolean p2pEnabled) {
   /**
    * Enable Discovery.
    *
-   * @param discovery the discovery
+   * @param discoveryEnabled the discoveryEnabled
    * @return the runner builder
    */
-  public RunnerBuilder discovery(final boolean discovery) {
-    this.discovery = discovery;
+  public RunnerBuilder discoveryEnabled(final boolean discoveryEnabled) {
+    this.discoveryEnabled = discoveryEnabled;
     return this;
   }
 
@@ -618,7 +618,7 @@ public Runner build() {
             .setBindHost(p2pListenInterface)
             .setBindPort(p2pListenPort)
             .setAdvertisedHost(p2pAdvertisedHost);
-    if (discovery) {
+    if (discoveryEnabled) {
       final List<EnodeURL> bootstrap;
       if (ethNetworkConfig.bootNodes() == null) {
         bootstrap = EthNetworkConfig.getNetworkConfig(NetworkName.MAINNET).bootNodes();
@@ -636,7 +636,7 @@ public Runner build() {
       discoveryConfiguration.setFilterOnEnrForkId(
           networkingConfiguration.getDiscovery().isFilterOnEnrForkIdEnabled());
     } else {
-      discoveryConfiguration.setActive(false);
+      discoveryConfiguration.setEnabled(false);
     }
 
     final NodeKey nodeKey = besuController.getNodeKey();

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -2238,7 +2238,7 @@ private Runner synchronize(
             .natMethod(natMethod)
             .natManagerServiceName(unstableNatOptions.getNatManagerServiceName())
             .natMethodFallbackEnabled(unstableNatOptions.getNatMethodFallbackEnabled())
-            .discovery(peerDiscoveryEnabled)
+            .discoveryEnabled(peerDiscoveryEnabled)
             .ethNetworkConfig(ethNetworkConfig)
             .permissioningConfiguration(permissioningConfiguration)
             .p2pAdvertisedHost(p2pAdvertisedHost)

diff --git a/besu/src/test/java/org/hyperledger/besu/RunnerBuilderTest.java b/besu/src/test/java/org/hyperledger/besu/RunnerBuilderTest.java
@@ -155,7 +155,7 @@ public void enodeUrlShouldHaveAdvertisedHostWhenDiscoveryDisabled() {
             .p2pListenPort(p2pListenPort)
             .p2pAdvertisedHost(p2pAdvertisedHost)
             .p2pEnabled(true)
-            .discovery(false)
+            .discoveryEnabled(false)
             .besuController(besuController)
             .ethNetworkConfig(mock(EthNetworkConfig.class))
             .metricsSystem(mock(ObservableMetricsSystem.class))
@@ -196,7 +196,7 @@ public void movingAcrossProtocolSpecsUpdatesNodeRecord() {
     when(protocolContext.getBlockchain()).thenReturn(inMemoryBlockchain);
     final Runner runner =
         new RunnerBuilder()
-            .discovery(true)
+            .discoveryEnabled(true)
             .p2pListenInterface("0.0.0.0")
             .p2pListenPort(p2pListenPort)
             .p2pAdvertisedHost(p2pAdvertisedHost)
@@ -255,7 +255,7 @@ public void whenEngineApiAddedListensOnDefaultPort() {
 
     final Runner runner =
         new RunnerBuilder()
-            .discovery(true)
+            .discoveryEnabled(true)
             .p2pListenInterface("0.0.0.0")
             .p2pListenPort(30303)
             .p2pAdvertisedHost("127.0.0.1")
@@ -299,7 +299,7 @@ public void whenEngineApiAddedWebSocketReadyOnSamePort() {
 
     final Runner runner =
         new RunnerBuilder()
-            .discovery(true)
+            .discoveryEnabled(true)
             .p2pListenInterface("0.0.0.0")
             .p2pListenPort(30303)
             .p2pAdvertisedHost("127.0.0.1")
@@ -342,7 +342,7 @@ public void whenEngineApiAddedEthSubscribeAvailable() {
 
     final Runner runner =
         new RunnerBuilder()
-            .discovery(true)
+            .discoveryEnabled(true)
             .p2pListenInterface("0.0.0.0")
             .p2pListenPort(30303)
             .p2pAdvertisedHost("127.0.0.1")
@@ -387,7 +387,7 @@ public void noEngineApiNoServiceForMethods() {
 
     final Runner runner =
         new RunnerBuilder()
-            .discovery(true)
+            .discoveryEnabled(true)
             .p2pListenInterface("0.0.0.0")
             .p2pListenPort(30303)
             .p2pAdvertisedHost("127.0.0.1")

diff --git a/besu/src/test/java/org/hyperledger/besu/RunnerTest.java b/besu/src/test/java/org/hyperledger/besu/RunnerTest.java
@@ -215,7 +215,7 @@ private void syncFromGenesis(
     final RunnerBuilder runnerBuilder =
         new RunnerBuilder()
             .vertx(vertx)
-            .discovery(true)
+            .discoveryEnabled(true)
             .p2pAdvertisedHost(listenHost)
             .p2pListenPort(0)
             .metricsSystem(noOpMetricsSystem)

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
@@ -256,7 +256,7 @@ public void callingBesuCommandWithoutOptionsMustSyncWithDefaultValues() {
 
     final ArgumentCaptor<EthNetworkConfig> ethNetworkArg =
         ArgumentCaptor.forClass(EthNetworkConfig.class);
-    verify(mockRunnerBuilder).discovery(eq(true));
+    verify(mockRunnerBuilder).discoveryEnabled(eq(true));
     verify(mockRunnerBuilder)
         .ethNetworkConfig(
             new EthNetworkConfig(
@@ -784,7 +784,7 @@ public void p2pOptionsRequiresServiceToBeEnabledToml() throws IOException {
   public void discoveryOptionValueTrueMustBeUsed() {
     parseCommand("--discovery-enabled", "true");
 
-    verify(mockRunnerBuilder).discovery(eq(true));
+    verify(mockRunnerBuilder).discoveryEnabled(eq(true));
     verify(mockRunnerBuilder).build();
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
@@ -795,7 +795,7 @@ public void discoveryOptionValueTrueMustBeUsed() {
   public void discoveryOptionValueFalseMustBeUsed() {
     parseCommand("--discovery-enabled", "false");
 
-    verify(mockRunnerBuilder).discovery(eq(false));
+    verify(mockRunnerBuilder).discoveryEnabled(eq(false));
     verify(mockRunnerBuilder).build();
 
     assertThat(commandOutput.toString(UTF_8)).isEmpty();

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CascadingDefaultProviderTest.java b/besu/src/test/java/org/hyperledger/besu/cli/CascadingDefaultProviderTest.java
@@ -108,7 +108,7 @@ public void overrideDefaultValuesIfKeyIsPresentInConfigFile(final @TempDir File
 
     parseCommand("--config-file", toml.toString());
 
-    verify(mockRunnerBuilder).discovery(eq(false));
+    verify(mockRunnerBuilder).discoveryEnabled(eq(false));
     verify(mockRunnerBuilder).ethNetworkConfig(ethNetworkConfigArgumentCaptor.capture());
     verify(mockRunnerBuilder).p2pAdvertisedHost(eq("1.2.3.4"));
     verify(mockRunnerBuilder).p2pListenPort(eq(1234));
@@ -161,7 +161,7 @@ public void noOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() {
 
     final MetricsConfiguration metricsConfiguration = MetricsConfiguration.builder().build();
 
-    verify(mockRunnerBuilder).discovery(eq(true));
+    verify(mockRunnerBuilder).discoveryEnabled(eq(true));
     verify(mockRunnerBuilder)
         .ethNetworkConfig(
             new EthNetworkConfig(

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java
@@ -317,7 +317,7 @@ public void initMocks() throws Exception {
 
     when(mockRunnerBuilder.vertx(any())).thenReturn(mockRunnerBuilder);
     when(mockRunnerBuilder.besuController(any())).thenReturn(mockRunnerBuilder);
-    when(mockRunnerBuilder.discovery(anyBoolean())).thenReturn(mockRunnerBuilder);
+    when(mockRunnerBuilder.discoveryEnabled(anyBoolean())).thenReturn(mockRunnerBuilder);
     when(mockRunnerBuilder.ethNetworkConfig(any())).thenReturn(mockRunnerBuilder);
     when(mockRunnerBuilder.networkingConfiguration(any())).thenReturn(mockRunnerBuilder);
     when(mockRunnerBuilder.p2pAdvertisedHost(anyString())).thenReturn(mockRunnerBuilder);

diff --git a/...rc/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AdminAddPeer.java b/...rc/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AdminAddPeer.java
@@ -14,7 +14,10 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods;
 
+import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.DISCOVERY_DISABLED;
+
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.p2p.network.P2PNetwork;
@@ -45,6 +48,10 @@ public String getName() {
 
   @Override
   protected JsonRpcResponse performOperation(final Object id, final String enode) {
+    if (peerNetwork.isStopped()) {
+      LOG.debug("Discovery is disabled, not adding ({}) to peers", enode);
+      return new JsonRpcErrorResponse(id, DISCOVERY_DISABLED);
+    }
     LOG.debug("Adding ({}) to peers", enode);
     final EnodeURL enodeURL =
         this.enodeDnsConfiguration.isEmpty()

diff --git a/...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/RpcErrorType.java b/...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/RpcErrorType.java
@@ -136,6 +136,7 @@ public enum RpcErrorType implements RpcMethodError {
   ETH_SEND_TX_REPLACEMENT_UNDERPRICED(-32000, "Replacement transaction underpriced"),
   // P2P related errors
   P2P_DISABLED(-32000, "P2P has been disabled. This functionality is not available"),
+  DISCOVERY_DISABLED(-32000, "Discovery has been disabled. This functionality is not available"),
   P2P_NETWORK_NOT_RUNNING(-32000, "P2P network is not running"),
 
   // Filter & Subscription Errors

diff --git a/...est/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AdminAddPeerTest.java b/...est/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AdminAddPeerTest.java
@@ -181,7 +181,6 @@ public void requestAddsValidEnode() {
   @Test
   public void requestAddsValidDNSEnode() {
     when(p2pNetwork.addMaintainedConnectionPeer(any())).thenReturn(true);
-
     final JsonRpcResponse expectedResponse =
         new JsonRpcSuccessResponse(
             validRequest.getRequest().getId(),
@@ -253,4 +252,14 @@ public void requestReturnsErrorWhenP2pDisabled() {
 
     assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
   }
+
+  @Test
+  public void requestReturnsErrorWhenDiscoveryStopped() {
+    when(p2pNetwork.isStopped()).thenReturn(true);
+    final JsonRpcResponse expectedResponse =
+        new JsonRpcErrorResponse(
+            validRequest.getRequest().getId(), RpcErrorType.DISCOVERY_DISABLED);
+    final JsonRpcResponse actualResponse = method.response(validRequest);
+    assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
+  }
 }
diff --git a/...t/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolPropagationTest.java b/...t/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolPropagationTest.java
@@ -35,7 +35,7 @@
 
 class TransactionPoolPropagationTest {
 
-  final DiscoveryConfiguration noDiscovery = DiscoveryConfiguration.create().setActive(false);
+  final DiscoveryConfiguration noDiscovery = DiscoveryConfiguration.create().setEnabled(false);
 
   private Vertx vertx;
 

diff --git a/ethereum/mock-p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/testing/MockNetwork.java b/ethereum/mock-p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/testing/MockNetwork.java
@@ -219,6 +219,11 @@ public boolean isDiscoveryEnabled() {
       return true;
     }
 
+    @Override
+    public boolean isStopped() {
+      return true;
+    }
+
     @Override
     public Optional<EnodeURL> getLocalEnode() {
       return Optional.empty();

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
@@ -24,7 +24,7 @@
 
 public class DiscoveryConfiguration {
 
-  private boolean active = true;
+  private boolean enabled = true;
   private String bindHost = NetworkUtility.INADDR_ANY;
   private int bindPort = 30303;
   private String advertisedHost = "127.0.0.1";
@@ -70,12 +70,12 @@ public DiscoveryConfiguration setBindPort(final int bindPort) {
     return this;
   }
 
-  public boolean isActive() {
-    return active;
+  public boolean isEnabled() {
+    return enabled;
   }
 
-  public DiscoveryConfiguration setActive(final boolean active) {
-    this.active = active;
+  public DiscoveryConfiguration setEnabled(final boolean enabled) {
+    this.enabled = enabled;
     return this;
   }
 
@@ -151,7 +151,7 @@ public boolean equals(final Object o) {
       return false;
     }
     final DiscoveryConfiguration that = (DiscoveryConfiguration) o;
-    return active == that.active
+    return enabled == that.enabled
         && bindPort == that.bindPort
         && bucketSize == that.bucketSize
         && Objects.equals(bindHost, that.bindHost)
@@ -163,14 +163,14 @@ public boolean equals(final Object o) {
   @Override
   public int hashCode() {
     return Objects.hash(
-        active, bindHost, bindPort, advertisedHost, bucketSize, bootnodes, dnsDiscoveryURL);
+        enabled, bindHost, bindPort, advertisedHost, bucketSize, bootnodes, dnsDiscoveryURL);
   }
 
   @Override
   public String toString() {
     return "DiscoveryConfiguration{"
-        + "active="
-        + active
+        + "enabled="
+        + enabled
         + ", bindHost='"
         + bindHost
         + '\''

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
@@ -98,7 +98,9 @@ public abstract class PeerDiscoveryAgent {
   private Optional<DiscoveryPeer> localNode = Optional.empty();
 
   /* Is discovery enabled? */
-  private boolean isActive = false;
+  private boolean isEnabled = false;
+  protected boolean isStopped = false;
+
   private final VariablesStorage variablesStorage;
   private final Supplier<List<Bytes>> forkIdSupplier;
   private String advertisedAddress;
@@ -148,7 +150,7 @@ protected abstract CompletableFuture<Void> sendOutgoingPacket(
   public abstract CompletableFuture<?> stop();
 
   public CompletableFuture<Integer> start(final int tcpPort) {
-    if (config.isActive()) {
+    if (config.isEnabled()) {
       final String host = config.getBindHost();
       final int port = config.getBindPort();
       LOG.info(
@@ -174,20 +176,20 @@ public CompletableFuture<Integer> start(final int tcpPort) {
                             .discoveryPort(discoveryPort)
                             .build());
                 this.localNode = Optional.of(ourNode);
-                isActive = true;
+                this.isEnabled = true;
                 LOG.info("P2P peer discovery agent started and listening on {}", localAddress);
                 updateNodeRecord();
                 startController(ourNode);
                 return discoveryPort;
               });
     } else {
-      this.isActive = false;
+      this.isEnabled = false;
       return CompletableFuture.completedFuture(0);
     }
   }
 
   public void updateNodeRecord() {
-    if (!config.isActive()) {
+    if (!config.isEnabled()) {
       return;
     }
 
@@ -411,8 +413,20 @@ private static void validateConfiguration(final DiscoveryConfiguration config) {
    *
    * @return true, if the {@link PeerDiscoveryAgent} is active on this node, false, otherwise.
    */
-  public boolean isActive() {
-    return isActive;
+  public boolean isEnabled() {
+    return isEnabled;
+  }
+
+  /**
+   * Returns the current state of the PeerDiscoveryAgent.
+   *
+   * <p>If true, the node is actively listening for new connections. If false, discovery has been
+   * turned off and the node is not listening for connections.
+   *
+   * @return true, if the {@link PeerDiscoveryAgent} is active on this node, false, otherwise.
+   */
+  public boolean isStopped() {
+    return isStopped;
   }
 
   public void bond(final Peer peer) {

diff --git a/...2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java b/...2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java
@@ -198,6 +198,7 @@ public CompletableFuture<?> stop() {
           if (ar.succeeded()) {
             controller.ifPresent(PeerDiscoveryController::stop);
             socket = null;
+            isStopped = true;
             completion.complete(null);
           } else {
             completion.completeExceptionally(ar.cause());

diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/network/DefaultP2PNetwork.java
@@ -464,7 +464,12 @@ public boolean isP2pEnabled() {
 
   @Override
   public boolean isDiscoveryEnabled() {
-    return peerDiscoveryAgent.isActive();
+    return peerDiscoveryAgent.isEnabled();
+  }
+
+  @Override
+  public boolean isStopped() {
+    return peerDiscoveryAgent.isStopped();
   }
 
   @Override