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

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
This commit is contained in:
Gabriel-Trintinalia
2025-01-24 06:29:13 +08:00
committed by GitHub
parent 09581eac20
commit b150b103e2
24 changed files with 103 additions and 49 deletions

View File

@@ -183,7 +183,7 @@ public class ThreadBesuNodeRunner implements BesuNodeRunner {
.vertx(Vertx.vertx())
.besuController(besuController)
.ethNetworkConfig(ethNetworkConfig)
.discovery(node.isDiscoveryEnabled())
.discoveryEnabled(node.isDiscoveryEnabled())
.p2pAdvertisedHost(node.getHostName())
.p2pListenPort(0)
.networkingConfiguration(node.getNetworkingConfiguration())

View File

@@ -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 class RunnerBuilder {
/**
* 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 class RunnerBuilder {
.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 class RunnerBuilder {
discoveryConfiguration.setFilterOnEnrForkId(
networkingConfiguration.getDiscovery().isFilterOnEnrForkIdEnabled());
} else {
discoveryConfiguration.setActive(false);
discoveryConfiguration.setEnabled(false);
}
final NodeKey nodeKey = besuController.getNodeKey();

View File

@@ -2238,7 +2238,7 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
.natMethod(natMethod)
.natManagerServiceName(unstableNatOptions.getNatManagerServiceName())
.natMethodFallbackEnabled(unstableNatOptions.getNatMethodFallbackEnabled())
.discovery(peerDiscoveryEnabled)
.discoveryEnabled(peerDiscoveryEnabled)
.ethNetworkConfig(ethNetworkConfig)
.permissioningConfiguration(permissioningConfiguration)
.p2pAdvertisedHost(p2pAdvertisedHost)

View File

@@ -155,7 +155,7 @@ public final class RunnerBuilderTest {
.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 final class RunnerBuilderTest {
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 final class RunnerBuilderTest {
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 final class RunnerBuilderTest {
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 final class RunnerBuilderTest {
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 final class RunnerBuilderTest {
final Runner runner =
new RunnerBuilder()
.discovery(true)
.discoveryEnabled(true)
.p2pListenInterface("0.0.0.0")
.p2pListenPort(30303)
.p2pAdvertisedHost("127.0.0.1")

View File

@@ -215,7 +215,7 @@ public final class RunnerTest {
final RunnerBuilder runnerBuilder =
new RunnerBuilder()
.vertx(vertx)
.discovery(true)
.discoveryEnabled(true)
.p2pAdvertisedHost(listenHost)
.p2pListenPort(0)
.metricsSystem(noOpMetricsSystem)

View File

@@ -256,7 +256,7 @@ public class BesuCommandTest extends CommandTestAbstract {
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 class BesuCommandTest extends CommandTestAbstract {
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 class BesuCommandTest extends CommandTestAbstract {
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();

View File

@@ -108,7 +108,7 @@ public class CascadingDefaultProviderTest extends CommandTestAbstract {
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 class CascadingDefaultProviderTest extends CommandTestAbstract {
final MetricsConfiguration metricsConfiguration = MetricsConfiguration.builder().build();
verify(mockRunnerBuilder).discovery(eq(true));
verify(mockRunnerBuilder).discoveryEnabled(eq(true));
verify(mockRunnerBuilder)
.ethNetworkConfig(
new EthNetworkConfig(

View File

@@ -317,7 +317,7 @@ public abstract class CommandTestAbstract {
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);

View File

@@ -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 class AdminAddPeer extends AdminModifyPeer {
@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()

View File

@@ -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

View File

@@ -181,7 +181,6 @@ public class AdminAddPeerTest {
@Test
public void requestAddsValidDNSEnode() {
when(p2pNetwork.addMaintainedConnectionPeer(any())).thenReturn(true);
final JsonRpcResponse expectedResponse =
new JsonRpcSuccessResponse(
validRequest.getRequest().getId(),
@@ -253,4 +252,14 @@ public class AdminAddPeerTest {
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);
}
}

View File

@@ -35,7 +35,7 @@ import org.junit.jupiter.api.Test;
class TransactionPoolPropagationTest {
final DiscoveryConfiguration noDiscovery = DiscoveryConfiguration.create().setActive(false);
final DiscoveryConfiguration noDiscovery = DiscoveryConfiguration.create().setEnabled(false);
private Vertx vertx;

View File

@@ -219,6 +219,11 @@ public final class MockNetwork {
return true;
}
@Override
public boolean isStopped() {
return true;
}
@Override
public Optional<EnodeURL> getLocalEnode() {
return Optional.empty();

View File

@@ -24,7 +24,7 @@ import java.util.stream.Collectors;
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 class DiscoveryConfiguration {
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 class DiscoveryConfiguration {
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 class DiscoveryConfiguration {
@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
+ '\''

View File

@@ -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 @@ public abstract class PeerDiscoveryAgent {
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 abstract class PeerDiscoveryAgent {
.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 @@ public abstract class PeerDiscoveryAgent {
*
* @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) {

View File

@@ -198,6 +198,7 @@ public class VertxPeerDiscoveryAgent extends PeerDiscoveryAgent {
if (ar.succeeded()) {
controller.ifPresent(PeerDiscoveryController::stop);
socket = null;
isStopped = true;
completion.complete(null);
} else {
completion.completeExceptionally(ar.cause());

View File

@@ -464,7 +464,12 @@ public class DefaultP2PNetwork implements P2PNetwork {
@Override
public boolean isDiscoveryEnabled() {
return peerDiscoveryAgent.isActive();
return peerDiscoveryAgent.isEnabled();
}
@Override
public boolean isStopped() {
return peerDiscoveryAgent.isStopped();
}
@Override

View File

@@ -90,6 +90,11 @@ public class NoopP2PNetwork implements P2PNetwork {
return false;
}
@Override
public boolean isStopped() {
return true;
}
@Override
public Optional<EnodeURL> getLocalEnode() {
return Optional.empty();

View File

@@ -144,6 +144,13 @@ public interface P2PNetwork extends Closeable {
*/
boolean isDiscoveryEnabled();
/**
* Is discovery stopped?
*
* @return Return true if peer discovery is stopped.
*/
boolean isStopped();
/**
* Returns the EnodeURL used to identify this peer in the network.
*

View File

@@ -829,7 +829,7 @@ public class PeerDiscoveryAgentTest {
final AgentBuilder agentBuilder = helper.agentBuilder().active(true);
final MockPeerDiscoveryAgent agent = helper.startDiscoveryAgent(agentBuilder);
assertThat(agent.isActive()).isTrue();
assertThat(agent.isEnabled()).isTrue();
}
@Test
@@ -837,7 +837,7 @@ public class PeerDiscoveryAgentTest {
final AgentBuilder agentBuilder = helper.agentBuilder().active(false);
final MockPeerDiscoveryAgent agent = helper.startDiscoveryAgent(agentBuilder);
assertThat(agent.isActive()).isFalse();
assertThat(agent.isEnabled()).isFalse();
}
@Test

View File

@@ -220,7 +220,7 @@ public class PeerDiscoveryTestHelper {
private final AtomicInteger nextAvailablePort;
private List<EnodeURL> bootnodes = Collections.emptyList();
private boolean active = true;
private boolean enabled = true;
private PeerPermissions peerPermissions = PeerPermissions.noop();
private String advertisedHost = "127.0.0.1";
private OptionalInt bindPort = OptionalInt.empty();
@@ -258,7 +258,7 @@ public class PeerDiscoveryTestHelper {
}
public AgentBuilder active(final boolean active) {
this.active = active;
this.enabled = active;
return this;
}
@@ -296,7 +296,7 @@ public class PeerDiscoveryTestHelper {
config.setBootnodes(bootnodes);
config.setAdvertisedHost(advertisedHost);
config.setBindPort(port);
config.setActive(active);
config.setEnabled(enabled);
config.setFilterOnEnrForkId(false);
final ForkIdManager mockForkIdManager = mock(ForkIdManager.class);

View File

@@ -83,7 +83,7 @@ public final class DefaultP2PNetworkTest {
private final NetworkingConfiguration config =
NetworkingConfiguration.create()
.setDiscovery(DiscoveryConfiguration.create().setActive(false))
.setDiscovery(DiscoveryConfiguration.create().setEnabled(false))
.setRlpx(
RlpxConfiguration.create()
.setBindPort(0)

View File

@@ -64,7 +64,7 @@ public class P2PNetworkTest {
private final Vertx vertx = Vertx.vertx();
private final NetworkingConfiguration config =
NetworkingConfiguration.create()
.setDiscovery(DiscoveryConfiguration.create().setActive(false))
.setDiscovery(DiscoveryConfiguration.create().setEnabled(false))
.setRlpx(
RlpxConfiguration.create()
.setBindPort(0)

View File

@@ -66,7 +66,7 @@ public class P2PPlainNetworkTest {
private final Vertx vertx = Vertx.vertx();
private final NetworkingConfiguration config =
NetworkingConfiguration.create()
.setDiscovery(DiscoveryConfiguration.create().setActive(false))
.setDiscovery(DiscoveryConfiguration.create().setEnabled(false))
.setRlpx(
RlpxConfiguration.create()
.setBindPort(0)
@@ -141,7 +141,7 @@ public class P2PPlainNetworkTest {
final NodeKey nodeKey = NodeKeyUtils.generate();
final NetworkingConfiguration listenerConfig =
NetworkingConfiguration.create()
.setDiscovery(DiscoveryConfiguration.create().setActive(false))
.setDiscovery(DiscoveryConfiguration.create().setEnabled(false))
.setRlpx(
RlpxConfiguration.create()
.setBindPort(0)