Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ namespace Nethermind.Core.Test.Modules;

public class FixedIpResolver(INetworkConfig networkConfig) : IIPResolver
{
public IPAddress LocalIp => IPAddress.Parse(networkConfig.LocalIp!);
public IPAddress ExternalIp => IPAddress.Parse(networkConfig.ExternalIp!);
public Task Initialize(CancellationToken cancellationToken = default) => Task.CompletedTask;
public ValueTask<IIPResolver.NethermindIp> Resolve(CancellationToken cancellationToken = default) =>
new(new IIPResolver.NethermindIp(
networkConfig.LocalIp is null ? IPAddress.Loopback : IPAddress.Parse(networkConfig.LocalIp),
networkConfig.ExternalIp is null ? IPAddress.None : IPAddress.Parse(networkConfig.ExternalIp)));
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

using System.Net;
using System;
using System.Threading;
using System.Threading.Tasks;
using NSubstitute;
using NUnit.Framework;
using Nethermind.Monitoring.Config;
Expand All @@ -19,7 +21,8 @@ public void HealthChecksWebhookInfo_returns_expected_results()

IIPResolver ipResolver = Substitute.For<IIPResolver>();
byte[] ip = { 1, 2, 3, 4 };
ipResolver.ExternalIp.Returns(new IPAddress(ip));
ipResolver.Resolve(Arg.Any<CancellationToken>())
.Returns(new ValueTask<IIPResolver.NethermindIp>(new IIPResolver.NethermindIp(IPAddress.Loopback, new IPAddress(ip))));

IMetricsConfig metricsConfig = new MetricsConfig() { NodeName = "nodeName" };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public HealthChecksWebhookInfo(string description, IIPResolver ipResolver, IMetr
{
_description = description;
_hostname = hostname;
IPAddress externalIp = ipResolver.ExternalIp;
IPAddress externalIp = ipResolver.Resolve().GetAwaiter().GetResult().ExternalIp;
_ip = externalIp.ToString();
_nodeName = metricsConfig.NodeName;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public class BuiltInStepsModule : Module
typeof(LogHardwareInfo),
typeof(RegisterPluginRpcModules),
typeof(RegisterRpcModules),
typeof(ResolveIps),
typeof(ReviewBlockTree),
typeof(SetupKeyStore),
typeof(StartBlockProcessor),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public static void SetPageSize() =>
[RunnerStepDependencies(
typeof(LoadGenesisBlock),
typeof(SetupKeyStore),
typeof(ResolveIps),
typeof(InitializePlugins),
typeof(InitializeBlockchain))]
#pragma warning disable IDE0290 // Primary constructor would shadow discard `_` used in fire-and-forget patterns
Expand Down
25 changes: 0 additions & 25 deletions src/Nethermind/Nethermind.Init/Steps/ResolveIps.cs

This file was deleted.

12 changes: 5 additions & 7 deletions src/Nethermind/Nethermind.Init/Steps/SetupKeyStore.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only

using System.Net;
using System.Threading;
using System.Threading.Tasks;
using Nethermind.Api;
Expand All @@ -11,16 +10,17 @@
using Nethermind.Crypto;
using Nethermind.KeyStore;
using Nethermind.KeyStore.Config;
using Nethermind.Network;
using Nethermind.Network.Config;
using Nethermind.Wallet;
using System.Linq;

namespace Nethermind.Init.Steps
{
[RunnerStepDependencies(typeof(ResolveIps))]
[RunnerStepDependencies]
public class SetupKeyStore(INethermindApi api) : IStep
{
public Task Execute(CancellationToken cancellationToken)
public async Task Execute(CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

Expand Down Expand Up @@ -63,14 +63,12 @@ public Task Execute(CancellationToken cancellationToken)
set.OriginalSignerKey = nodeKeyManager.LoadSignerKey();
}

IPAddress ipAddress = networkConfig.ExternalIp is not null ? IPAddress.Parse(networkConfig.ExternalIp) : IPAddress.Loopback;
IEnode enode = set.Enode = new Enode(nodeKey.PublicKey, ipAddress, networkConfig.P2PPort);
IIPResolver.NethermindIp ip = await api.IpResolver.Resolve(cancellationToken);
IEnode enode = set.Enode = new Enode(nodeKey.PublicKey, ip.ExternalIp, networkConfig.P2PPort);

get.LogManager.SetGlobalVariable("enode", enode.ToString());

networkConfig.Bootnodes = [.. networkConfig.Bootnodes.Where(bn => bn.NodeId != nodeKey.PublicKey)];

return Task.CompletedTask;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,19 @@ public interface INetworkConfig : IConfig
public const int MaxNettyArenaOrder = 14;
public const int DefaultMaxNettyArenaCount = 8;

/// <remarks>
/// User-facing override only. Code that needs the actual external IP must resolve it through
/// <c>IIPResolver.Resolve</c> instead of reading this property, which is only set when the user
/// supplies an override.
/// </remarks>
[ConfigItem(Description = "The external IP. Use only when the external IP cannot be resolved automatically.", DefaultValue = "null")]
string? ExternalIp { get; set; }

/// <remarks>
/// User-facing override only. Code that needs the actual local IP must resolve it through
/// <c>IIPResolver.Resolve</c> instead of reading this property, which is only set when the user
/// supplies an override.
/// </remarks>
[ConfigItem(Description = "The local IP. Use only when the local IP cannot be resolved automatically.", DefaultValue = "null")]
string? LocalIp { get; set; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void Setup()
_receiverSerializationManager = builder.TestObject;

INodeRecordProvider nodeRecordProvider = Substitute.For<INodeRecordProvider>();
nodeRecordProvider.Current.Returns(_selfNodeRecord);
nodeRecordProvider.GetCurrentAsync(Arg.Any<CancellationToken>()).Returns(new ValueTask<NodeRecord>(_selfNodeRecord));
_nodeStatsManager = Substitute.For<INodeStatsManager>();
_nodeStatsManager.GetOrAdd(Arg.Any<Node>()).Returns(Substitute.For<INodeStats>());

Expand Down
35 changes: 11 additions & 24 deletions src/Nethermind/Nethermind.Network.Discovery.Test/IPResolverTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,44 +13,31 @@ namespace Nethermind.Network.Discovery.Test;
public class IPResolverTests
{
[Test]
public async Task Can_resolve_external_ip()
public async Task Can_resolve_ip_without_override()
{
IPResolver ipResolver = new(new NetworkConfig(), LimboLogs.Instance);
await ipResolver.Initialize();
IPAddress address = ipResolver.ExternalIp;
Assert.That(address, Is.Not.Null);
IIPResolver.NethermindIp ip = await ipResolver.Resolve();
Assert.That(ip.LocalIp, Is.Not.Null);
Assert.That(ip.ExternalIp, Is.Not.Null);
}

[TestCase("99.99.99.99")]
[TestCase("10.50.50.50")]
public async Task Can_resolve_external_ip_with_override(string ipOverride)
{
INetworkConfig networkConfig = new NetworkConfig();
networkConfig.ExternalIp = ipOverride;
INetworkConfig networkConfig = new NetworkConfig { ExternalIp = ipOverride };
IPResolver ipResolver = new(networkConfig, LimboLogs.Instance);
await ipResolver.Initialize();
IPAddress address = ipResolver.ExternalIp;
Assert.That(address, Is.EqualTo(IPAddress.Parse(ipOverride)));
}

[Test]
public async Task Can_resolve_internal_ip()
{
IPResolver ipResolver = new(new NetworkConfig(), LimboLogs.Instance);
await ipResolver.Initialize();
IPAddress address = ipResolver.LocalIp;
Assert.That(address, Is.Not.Null);
IIPResolver.NethermindIp ip = await ipResolver.Resolve();
Assert.That(ip.ExternalIp, Is.EqualTo(IPAddress.Parse(ipOverride)));
}

[Test]
public async Task Can_resolve_local_ip_with_override()
{
string ipOverride = "99.99.99.99";
INetworkConfig networkConfig = new NetworkConfig();
networkConfig.LocalIp = ipOverride;
const string ipOverride = "99.99.99.99";
INetworkConfig networkConfig = new NetworkConfig { LocalIp = ipOverride };
IPResolver ipResolver = new(networkConfig, LimboLogs.Instance);
await ipResolver.Initialize();
IPAddress address = ipResolver.LocalIp;
Assert.That(address, Is.EqualTo(IPAddress.Parse(ipOverride)));
IIPResolver.NethermindIp ip = await ipResolver.Resolve();
Assert.That(ip.LocalIp, Is.EqualTo(IPAddress.Parse(ipOverride)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,15 @@ public class CompositeDiscoveryApp : IDiscoveryApp
public CompositeDiscoveryApp(
INetworkConfig networkConfig,
IDiscoveryConfig discoveryConfig,
IIPResolver ipResolver,
ILogManager logManager,
Func<DiscoveryV5App> discoveryV5Factory, // These two are factory because they are optional.
Func<DiscoveryApp> discoveryV4Factory,
IChannelFactory? channelFactory = null
)
{
_networkConfig = networkConfig;
_connections = new DiscoveryConnectionsPool(logManager.GetClassLogger<DiscoveryConnectionsPool>(), _networkConfig, discoveryConfig);
_connections = new DiscoveryConnectionsPool(logManager.GetClassLogger<DiscoveryConnectionsPool>(), ipResolver, discoveryConfig);
_channelFactory = channelFactory;
_logger = logManager.GetClassLogger<CompositeDiscoveryApp>();

Expand Down
15 changes: 9 additions & 6 deletions src/Nethermind/Nethermind.Network.Discovery/DiscoveryApp.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class DiscoveryApp : IDiscoveryApp, IAsyncDisposable
{
private readonly ILogger _logger;
private readonly INetworkConfig _networkConfig;
private readonly IIPResolver _ipResolver;
private readonly IKademliaNodeSource _kademliaNodeSource;
private readonly DiscoveryPersistenceManager _persistenceManager;
private readonly IKademliaDiscv4Adapter _discv4Adapter;
Expand All @@ -39,12 +40,14 @@ public DiscoveryApp(
[KeyFilter(IProtectedPrivateKey.NodeKey)] IProtectedPrivateKey nodeKey,
INetworkConfig networkConfig,
IDiscoveryConfig discoveryConfig,
IIPResolver ipResolver,
IProcessExitSource processExitSource,
ILogManager logManager,
Action<ContainerBuilder>? configureDiscv4Services = null)
{
_logger = logManager.GetClassLogger<DiscoveryApp>();
_networkConfig = networkConfig;
_ipResolver = ipResolver;
_stopCts = CancellationTokenSource.CreateLinkedTokenSource(processExitSource.Token);

List<Node> bootNodes = [];
Expand Down Expand Up @@ -99,12 +102,11 @@ Func<IChannel, NettyDiscoveryHandler> NettyDiscoveryHandlerFactory
{
}

public Task StartAsync()
public async Task StartAsync()
{
try
{
Initialize();
return Task.CompletedTask;
await Initialize();
}
catch (Exception e)
{
Expand Down Expand Up @@ -161,11 +163,12 @@ private void DetachEventHandlers()

public void AddNodeToDiscovery(Node node) => _kademlia.AddOrRefresh(node);

private void Initialize()
private async Task Initialize()
{
IIPResolver.NethermindIp ip = await _ipResolver.Resolve();
if (_logger.IsDebug)
_logger.Debug($"Discovery : udp://{_networkConfig.ExternalIp}:{_networkConfig.DiscoveryPort}");
ThisNodeInfo.AddInfo("Discovery :", $"udp://{_networkConfig.ExternalIp}:{_networkConfig.DiscoveryPort}");
_logger.Debug($"Discovery : udp://{ip.ExternalIp}:{_networkConfig.DiscoveryPort}");
ThisNodeInfo.AddInfo("Discovery :", $"udp://{ip.ExternalIp}:{_networkConfig.DiscoveryPort}");
}

protected virtual NettyDiscoveryHandler CreateDiscoveryHandler(IChannel channel)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,26 @@
using DotNetty.Transport.Bootstrapping;
using DotNetty.Transport.Channels;
using Nethermind.Logging;
using Nethermind.Network.Config;

namespace Nethermind.Network.Discovery;

/// <summary>
/// Manages connections (Netty <see cref="IChannel"/>) allocated for all Discovery protocol versions.
/// </summary>
/// <remarks> Not thread-safe </remarks>
public class DiscoveryConnectionsPool(ILogger logger, INetworkConfig networkConfig, IDiscoveryConfig discoveryConfig) : IConnectionsPool
public class DiscoveryConnectionsPool(ILogger logger, IIPResolver ipResolver, IDiscoveryConfig discoveryConfig) : IConnectionsPool
{
private readonly ILogger _logger = logger;
private readonly INetworkConfig _networkConfig = networkConfig;
private readonly IIPResolver _ipResolver = ipResolver;
private readonly IDiscoveryConfig _discoveryConfig = discoveryConfig;
private readonly IPAddress _ip = IPAddress.Parse(networkConfig.LocalIp!);
private readonly Dictionary<int, Task<IChannel>> _byPort = [];

public async Task<IChannel> BindAsync(Bootstrap bootstrap, int port)
{
if (_byPort.TryGetValue(port, out Task<IChannel>? task)) return await task;

task = BindAsync(bootstrap, port, _ip);
IPAddress ip = (await _ipResolver.Resolve()).LocalIp;
task = BindAsync(bootstrap, port, ip);
_byPort.Add(port, task);

return await task;
Expand All @@ -39,7 +38,7 @@ private async Task<IChannel> BindAsync(Bootstrap bootstrap, int port, IPAddress
}
catch (Exception e)
{
_logger.Error($"Error when establishing discovery connection on Address: {ip}({_networkConfig.LocalIp}:{port})", e);
_logger.Error($"Error when establishing discovery connection on Address: {ip}:{port}", e);
throw;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public async Task<bool> Ping(Node receiver, CancellationToken token)

PingMsg msg = new(receiver.Address, CalculateExpirationTime(), kademliaConfig.CurrentNodeId.Address)
{
EnrSequence = nodeRecordProvider.Current.EnrSequence // optional and does not seem to be used anywhere.
EnrSequence = (await nodeRecordProvider.GetCurrentAsync(token)).EnrSequence // optional and does not seem to be used anywhere.
};
session.OnPingSent();
DiscoveryResponse<PongMsg> response = await CallAndWaitForResponse(MsgType.Pong, new PongMsgHandler(msg), receiver, session, msg, _pingTimeout, token);
Expand Down Expand Up @@ -238,7 +238,7 @@ private async Task HandleEnrRequest(Node node, NodeSession session, EnrRequestMs
}

Rlp requestRlp = Rlp.Encode(Rlp.Encode(msg.ExpirationTime));
await SendMessage(session, new EnrResponseMsg(node.Address, nodeRecordProvider.Current, Keccak.Compute(requestRlp.Bytes)), token);
await SendMessage(session, new EnrResponseMsg(node.Address, await nodeRecordProvider.GetCurrentAsync(token), Keccak.Compute(requestRlp.Bytes)), token);
}

private async Task HandleFindNode(Node node, NodeSession session, FindNodeMsg msg, CancellationToken token)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ public DiscoveryV5App(
_discoveryDb = discoveryDb;
_legacyDiscoveryDb = legacyDiscoveryDb;
_logManager = logManager;
_allowNonRoutableEnrs = ShouldAcceptNonRoutableEnrs(ipResolver.ExternalIp);
// DiscoveryV5App is resolved during network startup, after SetupKeyStore (a declared dependency of
// InitializeNetwork) has already awaited Resolve() and warmed the cache, so this does not block.
IPAddress externalIp = ipResolver.Resolve().GetAwaiter().GetResult().ExternalIp;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium — Blocking .GetAwaiter().GetResult() in a constructor.

This blocks the DI resolution thread on external HTTP calls if the IP cache hasn't been warmed yet. It's safe in the current startup ordering because InitializeNetwork (which resolves DiscoveryV5App) declares SetupKeyStore as a RunnerStepDependencies, so the cache should already be populated. But this implicit dependency is invisible to the compiler and fragile: a future refactor that resolves IDiscoveryApp earlier (e.g., in a plugin Init() method) would block without any warning.

The externalIp is only used to compute _allowNonRoutableEnrs and populate the ENR builder — both one-time setup operations. The same pattern applied to RlpxHost (which uses Lazy<IPeerPool> to defer resolution) could be applied here: move the IP resolution into StartAsync() and accept the field as IPAddress? with a NotNullAfterStart invariant, or accept IIPResolver and resolve lazily.

If this is intentional and the startup ordering is considered a firm invariant, add an explicit // Called after SetupKeyStore warms the IP cache; GetAwaiter().GetResult() is safe here. comment so future maintainers understand the constraint.

_allowNonRoutableEnrs = ShouldAcceptNonRoutableEnrs(externalIp);
IdentityVerifierV4 identityVerifier = new();

PrivateKey privateKey = nodeKey.Unprotect();
Expand All @@ -91,7 +94,7 @@ .. LoadStoredEnrs(),
EnrBuilder enrBuilder = new EnrBuilder()
.WithIdentityScheme(_sessionOptions.Verifier, _sessionOptions.Signer)
.WithEntry(EnrEntryKey.Id, new EntryId("v4"))
.WithEntry(EnrEntryKey.Ip, new EntryIp(ipResolver.ExternalIp))
.WithEntry(EnrEntryKey.Ip, new EntryIp(externalIp))
.WithEntry(EnrEntryKey.Secp256K1, new EntrySecp256K1(_sessionOptions.Signer.PublicKey))
.WithEntry(EnrEntryKey.Tcp, new EntryTcp(networkConfig.P2PPort))
.WithEntry(EnrEntryKey.Udp, new EntryUdp(networkConfig.DiscoveryPort));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ namespace Nethermind.Network.Discovery;

public interface INodeRecordProvider
{
public NodeRecord Current { get; }
ValueTask<NodeRecord> GetCurrentAsync(CancellationToken cancellationToken = default);
}
Loading
Loading