From 4641f0d3f49f06ef92ec31e2548875da80a1d82b Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Sat, 20 Jun 2026 08:39:19 +0800 Subject: [PATCH 01/12] refactor(init): decouple InitializeNetwork from INethermindApi Register IProtocolsManager in NetworkModule and inject all of InitializeNetwork's dependencies via the constructor (Lazy<> for the conditionally-used ones), removing every _api access from the step. IApiWithNetwork.ProtocolsManager becomes read-only and is resolved from DI, so the FallbackToFieldFromApi redundancy guard requires dropping its setter. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Nethermind.Api/IApiWithNetwork.cs | 2 +- .../Nethermind.Api/NethermindApi.cs | 2 +- .../Nethermind.Init/Modules/NetworkModule.cs | 1 + .../Steps/InitializeNetwork.cs | 106 +++++++++--------- .../MergePluginTests.cs | 2 + .../Nethermind.Runner.Test/Ethereum/Build.cs | 1 - .../Module/NetworkModuleTest.cs | 41 ++++--- 7 files changed, 77 insertions(+), 78 deletions(-) diff --git a/src/Nethermind/Nethermind.Api/IApiWithNetwork.cs b/src/Nethermind/Nethermind.Api/IApiWithNetwork.cs index f26e0877cb2e..1d894c141c65 100644 --- a/src/Nethermind/Nethermind.Api/IApiWithNetwork.cs +++ b/src/Nethermind/Nethermind.Api/IApiWithNetwork.cs @@ -21,7 +21,7 @@ public interface IApiWithNetwork : IApiWithBlockchain IMessageSerializationService MessageSerializationService { get; } IGossipPolicy GossipPolicy { get; set; } IPeerManager? PeerManager { get; } - IProtocolsManager? ProtocolsManager { get; set; } + IProtocolsManager? ProtocolsManager { get; } IProtocolValidator ProtocolValidator { get; } IRlpxHost RlpxPeer { get; } diff --git a/src/Nethermind/Nethermind.Api/NethermindApi.cs b/src/Nethermind/Nethermind.Api/NethermindApi.cs index 083e1b939cd8..228ab30497e4 100644 --- a/src/Nethermind/Nethermind.Api/NethermindApi.cs +++ b/src/Nethermind/Nethermind.Api/NethermindApi.cs @@ -81,7 +81,7 @@ ILifetimeScope Context public IMessageSerializationService MessageSerializationService => Context.Resolve(); public IGossipPolicy GossipPolicy { get; set; } = Policy.FullGossip; public IPeerManager? PeerManager => Context.Resolve(); - public IProtocolsManager? ProtocolsManager { get; set; } + public IProtocolsManager? ProtocolsManager => Context.Resolve(); public IProtocolValidator ProtocolValidator => Context.Resolve(); public IReceiptStorage? ReceiptStorage => Context.Resolve(); public IReceiptFinder ReceiptFinder => Context.Resolve(); diff --git a/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs b/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs index 71629a072848..6b7d952b6952 100644 --- a/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs +++ b/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs @@ -66,6 +66,7 @@ protected override void Load(ContainerBuilder builder) .AddSingleton() .AddSingleton() .AddSingleton() + .AddSingleton() // Handshake .AddMessageSerializer() diff --git a/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs b/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs index 52b3ea8dc29f..307aedb51e1c 100644 --- a/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs +++ b/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs @@ -4,24 +4,23 @@ using System; using System.Threading; using System.Threading.Tasks; -using Autofac.Features.AttributeFilters; using Nethermind.Api; using Nethermind.Api.Extensions; using Nethermind.Api.Steps; +using Nethermind.Blockchain; using Nethermind.Blockchain.Synchronization; using Nethermind.Config; using Nethermind.Core; using Nethermind.Core.Exceptions; -using Nethermind.Db; using Nethermind.Logging; using Nethermind.Network; using Nethermind.Network.Config; using Nethermind.Network.Contract.P2P; using Nethermind.Network.Discovery; -using Nethermind.Network.P2P.ProtocolHandlers; -using Nethermind.Stats; +using Nethermind.Network.Rlpx; using Nethermind.Stats.Model; using Nethermind.Synchronization; +using Nethermind.Synchronization.ParallelSync; using Nethermind.Synchronization.Peers; namespace Nethermind.Init.Steps; @@ -46,54 +45,75 @@ public static void SetPageSize() => #pragma warning disable IDE0290 // Primary constructor would shadow discard `_` used in fire-and-forget patterns public class InitializeNetwork : IStep { - protected readonly IApiWithNetwork _api; - protected readonly INodeStatsManager NodeStatsManager; protected readonly ISynchronizer _synchronizer; protected readonly ISyncPeerPool _syncPeerPool; protected readonly IDiscoveryApp _discoveryApp; protected readonly Lazy _peerPool; - protected readonly INetworkStorage _peerStorage; protected readonly INetworkConfig _networkConfig; + private readonly IBlockTree _blockTree; + private readonly IRlpxHost _rlpxPeer; + private readonly IPeerManager _peerManager; + private readonly ISessionMonitor _sessionMonitor; + private readonly IStaticNodesManager _staticNodesManager; + private readonly ITrustedNodesManager _trustedNodesManager; + private readonly IEnode _enode; + private readonly INethermindPlugin[] _plugins; + private readonly Lazy _protocolsManager; + private readonly Lazy _syncModeSelector; + private readonly Lazy _disposeStack; + private readonly NodeSourceToDiscV4Feeder _enrDiscoveryAppFeeder; private readonly ISyncConfig _syncConfig; private readonly IInitConfig _initConfig; - private readonly IEnode _enode; - protected readonly IProtocolHandlerFactory[] _protocolHandlerFactories; + private readonly ILogManager _logManager; private readonly ILogger _logger; public InitializeNetwork( - INethermindApi api, - INodeStatsManager nodeStatsManager, ISyncServer _, // Need to be resolved at least once ISynchronizer synchronizer, ISyncPeerPool syncPeerPool, NodeSourceToDiscV4Feeder enrDiscoveryAppFeeder, IDiscoveryApp discoveryApp, Lazy peerPool, // Require IRlpxPeer to be created first, hence, lazy. - [KeyFilter(DbNames.PeersDb)] INetworkStorage peerStorage, - IProtocolHandlerFactory[] protocolHandlerFactories, + IBlockTree blockTree, + IRlpxHost rlpxPeer, + IPeerManager peerManager, + ISessionMonitor sessionMonitor, + IStaticNodesManager staticNodesManager, + ITrustedNodesManager trustedNodesManager, + IEnode enode, + INethermindPlugin[] plugins, + Lazy protocolsManager, + Lazy syncModeSelector, + Lazy disposeStack, INetworkConfig networkConfig, ISyncConfig syncConfig, IInitConfig initConfig, - IEnode enode, ILogManager logManager ) { - _api = api; - _enode = enode; - NodeStatsManager = nodeStatsManager; _synchronizer = synchronizer; _syncPeerPool = syncPeerPool; _enrDiscoveryAppFeeder = enrDiscoveryAppFeeder; _discoveryApp = discoveryApp; _peerPool = peerPool; - _peerStorage = peerStorage; - _protocolHandlerFactories = protocolHandlerFactories; + _blockTree = blockTree; + _rlpxPeer = rlpxPeer; + _peerManager = peerManager; + _sessionMonitor = sessionMonitor; + _staticNodesManager = staticNodesManager; + _trustedNodesManager = trustedNodesManager; + _enode = enode; + _plugins = plugins; + _protocolsManager = protocolsManager; + _syncModeSelector = syncModeSelector; + _disposeStack = disposeStack; _networkConfig = networkConfig; _syncConfig = syncConfig; _initConfig = initConfig; + _logManager = logManager; _logger = logManager.GetClassLogger(); } @@ -102,8 +122,6 @@ ILogManager logManager private async Task Initialize(CancellationToken cancellationToken) { - if (_api.BlockTree is null) throw new StepDependencyException(nameof(_api.BlockTree)); - if (_syncConfig.StaticSnapPivot) { if (!_syncConfig.SnapSync) @@ -119,7 +137,7 @@ private async Task Initialize(CancellationToken cancellationToken) if (NetworkDiagTracer.IsEnabled) { - NetworkDiagTracer.Start(_api.LogManager); + NetworkDiagTracer.Start(_logManager); } int maxPeersCount = _networkConfig.ActivePeersMaxCount; @@ -141,8 +159,8 @@ await InitPeer().ContinueWith(initPeerTask => if (_syncConfig.SnapSync && _syncConfig.SnapServingEnabled != true) { SnapCapabilitySwitcher snapCapabilitySwitcher = - new(_api.ProtocolsManager, _api.SyncModeSelector, _api.LogManager); - _api.DisposeStack.Push(snapCapabilitySwitcher); + new(_protocolsManager.Value, _syncModeSelector.Value, _logManager); + _disposeStack.Value.Push(snapCapabilitySwitcher); snapCapabilitySwitcher.EnableSnapCapabilityUntilSynced(); } else if (_logger.IsDebug) _logger.Debug("Skipped enabling snap capability"); @@ -212,32 +230,27 @@ private Task StartDiscovery() private void StartPeer() { - if (_api.PeerManager is null) throw new StepDependencyException(nameof(_api.PeerManager)); - if (_api.SessionMonitor is null) throw new StepDependencyException(nameof(_api.SessionMonitor)); - - if (!_api.Config().PeerManagerEnabled) + if (!_initConfig.PeerManagerEnabled) { if (_logger.IsWarn) _logger.Warn($"Skipping peer manager init due to {nameof(IInitConfig.PeerManagerEnabled)} set to false"); } if (_logger.IsDebug) _logger.Debug("Initializing peer manager"); _peerPool.Value.Start(); - _api.PeerManager.Start(); - _api.SessionMonitor.Start(); + _peerManager.Start(); + _sessionMonitor.Start(); if (_logger.IsDebug) _logger.Debug("Peer manager initialization completed"); } private Task StartSync() { - if (_api.BlockTree is null) throw new StepDependencyException(nameof(_api.BlockTree)); - if (_syncConfig.NetworkingEnabled) { _syncPeerPool.Start(); if (_syncConfig.SynchronizationEnabled) { - if (_logger.IsDebug) _logger.Debug($"Starting synchronization from block {_api.BlockTree.Head?.Header.ToString(BlockHeader.Format.Short)}."); + if (_logger.IsDebug) _logger.Debug($"Starting synchronization from block {_blockTree.Head?.Header.ToString(BlockHeader.Format.Short)}."); _synchronizer.Start(); } else @@ -253,15 +266,11 @@ private Task StartSync() protected virtual async Task InitPeer() { - if (_api.BlockTree is null) throw new StepDependencyException(nameof(_api.BlockTree)); - if (_api.SpecProvider is null) throw new StepDependencyException(nameof(_api.SpecProvider)); - if (_api.TxPool is null) throw new StepDependencyException(nameof(_api.TxPool)); - - _api.ProtocolsManager = CreateProtocolManager(); + IProtocolsManager protocolsManager = _protocolsManager.Value; if (_syncConfig.SnapServingEnabled == true) { - _api.ProtocolsManager!.AddSupportedCapability(new Capability(Protocol.Snap, 1)); + protocolsManager.AddSupportedCapability(new Capability(Protocol.Snap, 1)); } if (!_networkConfig.DisableDiscV4DnsFeeder) { @@ -269,28 +278,17 @@ protected virtual async Task InitPeer() _ = _enrDiscoveryAppFeeder.Run(); } - foreach (INethermindPlugin plugin in _api.Plugins) + foreach (INethermindPlugin plugin in _plugins) { await plugin.InitNetworkProtocol(); } // Capabilities must be finalized before the RLPx listener accepts peers. Otherwise // early sessions can negotiate only the default ETH version and never upgrade. - await _api.RlpxPeer.Init(); + await _rlpxPeer.Init(); - await _api.StaticNodesManager.InitAsync(); + await _staticNodesManager.InitAsync(); - await _api.TrustedNodesManager.InitAsync(); + await _trustedNodesManager.InitAsync(); } - - protected virtual IProtocolsManager CreateProtocolManager() => new ProtocolsManager( - _api.SyncPeerPool!, - _api.TxPool!, - _discoveryApp, - _api.RlpxPeer, - NodeStatsManager, - _api.ProtocolValidator, - _peerStorage, - _protocolHandlerFactories, - _api.LogManager); } diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs index fd76d9db3917..10ca6aa3bdab 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs @@ -101,6 +101,8 @@ private IContainer BuildContainer(IConfigProvider? configProvider = null, Action LimboLogs.Instance)) .AddSingleton(Substitute.For()) .AddSingleton(Substitute.For()) + // Override the real ProtocolsManager with a substitute so tests can assert on capability calls. + .AddSingleton(Substitute.For()) .OnBuild(ctx => { INethermindApi api = ctx.Resolve(); diff --git a/src/Nethermind/Nethermind.Runner.Test/Ethereum/Build.cs b/src/Nethermind/Nethermind.Runner.Test/Ethereum/Build.cs index 11b00878d668..fcecb3c80209 100644 --- a/src/Nethermind/Nethermind.Runner.Test/Ethereum/Build.cs +++ b/src/Nethermind/Nethermind.Runner.Test/Ethereum/Build.cs @@ -94,7 +94,6 @@ public static void MockOutNethermindApi(NethermindApi api) api.BlockProducer = Substitute.For(); api.EngineSigner = Substitute.For(); api.KeyStore = Substitute.For(); - api.ProtocolsManager = Substitute.For(); api.TxSender = Substitute.For(); api.EngineSignerStore = Substitute.For(); api.TransactionComparerProvider = Substitute.For(); diff --git a/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs b/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs index a8a72a16a719..d71c56d5326d 100644 --- a/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs +++ b/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs @@ -32,6 +32,7 @@ using Nethermind.State; using Nethermind.Stats; using Nethermind.Synchronization; +using Nethermind.Synchronization.ParallelSync; using Nethermind.Synchronization.Peers; using Nethermind.Stats.Model; using Nethermind.TxPool; @@ -153,11 +154,9 @@ public async Task Initialize_network_registers_plugin_capabilities_before_starti SubstitutionContext.Current?.ThreadContext?.DequeueAllArgumentSpecifications(); List callOrder = []; - INethermindApi api = Substitute.For(); IRlpxHost rlpxHost = Substitute.For(); IStaticNodesManager staticNodesManager = Substitute.For(); ITrustedNodesManager trustedNodesManager = Substitute.For(); - IWorldStateManager worldStateManager = Substitute.For(); IProtocolsManager protocolsManager = Substitute.For(); INethermindPlugin plugin = new RecordingPlugin(() => callOrder.Add("plugin")); @@ -169,19 +168,12 @@ public async Task Initialize_network_registers_plugin_capabilities_before_starti staticNodesManager.InitAsync().Returns(Task.CompletedTask); trustedNodesManager.InitAsync().Returns(Task.CompletedTask); - api.BlockTree.Returns(Substitute.For()); - api.SpecProvider.Returns(Substitute.For()); - api.TxPool.Returns(Substitute.For()); - api.RlpxPeer.Returns(rlpxHost); - api.StaticNodesManager.Returns(staticNodesManager); - api.TrustedNodesManager.Returns(trustedNodesManager); - api.WorldStateManager.Returns(worldStateManager); - api.Plugins.Returns([plugin]); - worldStateManager.HashServer.Returns(Substitute.For()); - TestInitializeNetwork initializeNetwork = new( - api, + rlpxHost, + staticNodesManager, + trustedNodesManager, protocolsManager, + [plugin], new NetworkConfig { DisableDiscV4DnsFeeder = true }, new SyncConfig(), LimboLogs.Instance); @@ -240,29 +232,36 @@ public Task InitNetworkProtocol() } private sealed class TestInitializeNetwork( - INethermindApi api, + IRlpxHost rlpxHost, + IStaticNodesManager staticNodesManager, + ITrustedNodesManager trustedNodesManager, IProtocolsManager protocolsManager, + INethermindPlugin[] plugins, INetworkConfig networkConfig, ISyncConfig syncConfig, ILogManager logManager) : InitializeNetwork( - api, - Substitute.For(), Substitute.For(), Substitute.For(), Substitute.For(), new NodeSourceToDiscV4Feeder(Substitute.For(), Substitute.For(), Substitute.For()), Substitute.For(), new Lazy(() => Substitute.For()), - Substitute.For(), - [], + Substitute.For(), + rlpxHost, + Substitute.For(), + Substitute.For(), + staticNodesManager, + trustedNodesManager, + Substitute.For(), + plugins, + new Lazy(() => protocolsManager), + new Lazy(() => Substitute.For()), + new Lazy(() => Substitute.For()), networkConfig, syncConfig, Substitute.For(), - Substitute.For(), logManager) { public Task RunInitPeer() => InitPeer(); - - protected override IProtocolsManager CreateProtocolManager() => protocolsManager; } } From de98362823d20e6c02b00bc018b54b4d19e7cd27 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Sat, 20 Jun 2026 09:19:43 +0800 Subject: [PATCH 02/12] refactor(init): register SnapCapabilitySwitcher in DI Address review: register SnapCapabilitySwitcher in NetworkModule instead of constructing it in InitializeNetwork. It is IDisposable so Autofac owns its disposal (replacing the manual DisposeStack.Push), letting the step drop its Lazy and Lazy dependencies. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Nethermind.Init/Modules/NetworkModule.cs | 1 + .../Nethermind.Init/Steps/InitializeNetwork.cs | 15 ++++----------- .../Module/NetworkModuleTest.cs | 4 +--- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs b/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs index 6b7d952b6952..94222454c1b8 100644 --- a/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs +++ b/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs @@ -67,6 +67,7 @@ protected override void Load(ContainerBuilder builder) .AddSingleton() .AddSingleton() .AddSingleton() + .AddSingleton() // Handshake .AddMessageSerializer() diff --git a/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs b/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs index 307aedb51e1c..164e0829fbe7 100644 --- a/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs +++ b/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs @@ -20,7 +20,6 @@ using Nethermind.Network.Rlpx; using Nethermind.Stats.Model; using Nethermind.Synchronization; -using Nethermind.Synchronization.ParallelSync; using Nethermind.Synchronization.Peers; namespace Nethermind.Init.Steps; @@ -60,8 +59,7 @@ public class InitializeNetwork : IStep private readonly IEnode _enode; private readonly INethermindPlugin[] _plugins; private readonly Lazy _protocolsManager; - private readonly Lazy _syncModeSelector; - private readonly Lazy _disposeStack; + private readonly Lazy _snapCapabilitySwitcher; private readonly NodeSourceToDiscV4Feeder _enrDiscoveryAppFeeder; private readonly ISyncConfig _syncConfig; @@ -86,8 +84,7 @@ public InitializeNetwork( IEnode enode, INethermindPlugin[] plugins, Lazy protocolsManager, - Lazy syncModeSelector, - Lazy disposeStack, + Lazy snapCapabilitySwitcher, INetworkConfig networkConfig, ISyncConfig syncConfig, IInitConfig initConfig, @@ -108,8 +105,7 @@ ILogManager logManager _enode = enode; _plugins = plugins; _protocolsManager = protocolsManager; - _syncModeSelector = syncModeSelector; - _disposeStack = disposeStack; + _snapCapabilitySwitcher = snapCapabilitySwitcher; _networkConfig = networkConfig; _syncConfig = syncConfig; _initConfig = initConfig; @@ -158,10 +154,7 @@ await InitPeer().ContinueWith(initPeerTask => if (_syncConfig.SnapSync && _syncConfig.SnapServingEnabled != true) { - SnapCapabilitySwitcher snapCapabilitySwitcher = - new(_protocolsManager.Value, _syncModeSelector.Value, _logManager); - _disposeStack.Value.Push(snapCapabilitySwitcher); - snapCapabilitySwitcher.EnableSnapCapabilityUntilSynced(); + _snapCapabilitySwitcher.Value.EnableSnapCapabilityUntilSynced(); } else if (_logger.IsDebug) _logger.Debug("Skipped enabling snap capability"); diff --git a/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs b/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs index d71c56d5326d..4765a5c05a91 100644 --- a/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs +++ b/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs @@ -32,7 +32,6 @@ using Nethermind.State; using Nethermind.Stats; using Nethermind.Synchronization; -using Nethermind.Synchronization.ParallelSync; using Nethermind.Synchronization.Peers; using Nethermind.Stats.Model; using Nethermind.TxPool; @@ -255,8 +254,7 @@ private sealed class TestInitializeNetwork( Substitute.For(), plugins, new Lazy(() => protocolsManager), - new Lazy(() => Substitute.For()), - new Lazy(() => Substitute.For()), + new Lazy(() => null!), networkConfig, syncConfig, Substitute.For(), From a7e1bc0d421035ae8bf516d4e6a0722346e962b7 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Sat, 20 Jun 2026 09:49:11 +0800 Subject: [PATCH 03/12] test(merge): drop redundant comment on ProtocolsManager substitute Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs index 10ca6aa3bdab..e7ca6cd6036f 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs @@ -101,7 +101,6 @@ private IContainer BuildContainer(IConfigProvider? configProvider = null, Action LimboLogs.Instance)) .AddSingleton(Substitute.For()) .AddSingleton(Substitute.For()) - // Override the real ProtocolsManager with a substitute so tests can assert on capability calls. .AddSingleton(Substitute.For()) .OnBuild(ctx => { From 377a6ecbb996dc9600b65b26fea5de2f1d4d8bf5 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Sat, 20 Jun 2026 10:56:36 +0800 Subject: [PATCH 04/12] test: remove unused usings flagged by IDE0005 lint The InitializeNetwork decoupling left several usings orphaned in NetworkModuleTest and Build (Core.Specs, State, TxPool, Stats, Network). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/Nethermind/Nethermind.Runner.Test/Ethereum/Build.cs | 1 - .../Nethermind.Runner.Test/Module/NetworkModuleTest.cs | 4 ---- 2 files changed, 5 deletions(-) diff --git a/src/Nethermind/Nethermind.Runner.Test/Ethereum/Build.cs b/src/Nethermind/Nethermind.Runner.Test/Ethereum/Build.cs index fcecb3c80209..227528c3bc7d 100644 --- a/src/Nethermind/Nethermind.Runner.Test/Ethereum/Build.cs +++ b/src/Nethermind/Nethermind.Runner.Test/Ethereum/Build.cs @@ -17,7 +17,6 @@ using Nethermind.Consensus.Validators; using Nethermind.Core.Specs; using Nethermind.Logging; -using Nethermind.Network; using Nethermind.KeyStore; using Nethermind.Serialization.Json; using Nethermind.Specs.ChainSpecStyle; diff --git a/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs b/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs index 4765a5c05a91..fe8b758a4dd6 100644 --- a/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs +++ b/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs @@ -13,7 +13,6 @@ using Nethermind.Blockchain; using Nethermind.Blockchain.Synchronization; using Nethermind.Config; -using Nethermind.Core.Specs; using Nethermind.Core; using Nethermind.Core.Test.Builders; using Nethermind.Core.Test.Modules; @@ -29,12 +28,9 @@ using Nethermind.Network.P2P.ProtocolHandlers; using Nethermind.Network.Rlpx; using Nethermind.Logging; -using Nethermind.State; -using Nethermind.Stats; using Nethermind.Synchronization; using Nethermind.Synchronization.Peers; using Nethermind.Stats.Model; -using Nethermind.TxPool; using NSubstitute; using NSubstitute.Core; using NUnit.Framework; From a566093a5030519b5697819b479b4f42237a13ac Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Sat, 20 Jun 2026 22:51:39 +0800 Subject: [PATCH 05/12] refactor(network): declarative p2p capability advertisement via IP2PCapabilityResolver Replace the scattered imperative IProtocolsManager.Add/RemoveSupportedCapability calls (MergePlugin, InitializeNetwork snap-serving, SnapCapabilitySwitcher, XdcPlugin) with an IP2PCapabilityResolver interface. ProtocolsManager caches the advertised capability set and rebuilds it only when a resolver raises Changed, keeping the per-session Register path allocation-free. - MergeP2PCapabilityResolver: eth/69-71 once PoSSwitcher.TransitionFinished (registered in MergePluginModule + AuRaMergeModule only; OP/Taiko unchanged) - SnapP2PCapabilityResolver: snap/1 while serving or snap-syncing (replaces SnapCapabilitySwitcher and its sync-mode event wiring) - XdcP2PCapabilityResolver: eth/62/63/100 instead of the default eth/68 Trim AddSupportedCapability/RemoveSupportedCapability from IProtocolsManager. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Modules/PseudoNetworkModule.cs | 22 +----- .../Nethermind.Init/Modules/NetworkModule.cs | 2 +- .../Steps/InitializeNetwork.cs | 18 +---- .../Modules/TestRpcBlockchain.cs | 1 + .../Nethermind.Merge.AuRa/AuRaMergePlugin.cs | 4 ++ .../MergeP2PCapabilityResolverTests.cs | 45 ++++++++++++ .../MergePluginTests.cs | 53 -------------- .../MergeP2PCapabilityResolver.cs | 40 +++++++++++ .../Nethermind.Merge.Plugin/MergePlugin.cs | 26 ++----- .../ProtocolsManagerTests.cs | 58 +++++++++++++++ .../SnapCapabilitySwitcherTests.cs | 28 -------- .../SnapP2PCapabilityResolverTests.cs | 48 +++++++++++++ .../IP2PCapabilityResolver.cs | 26 +++++++ .../Nethermind.Network/IProtocolsManager.cs | 4 -- .../Nethermind.Network/ProtocolsManager.cs | 63 +++++++++++++---- .../SnapCapabilitySwitcher.cs | 70 ------------------- .../SnapP2PCapabilityResolver.cs | 50 +++++++++++++ .../Module/NetworkModuleTest.cs | 2 - .../E2ESyncTests.cs | 21 +++--- .../XdcP2PCapabilityResolverTests.cs | 29 ++++++++ src/Nethermind/Nethermind.Xdc/XdcModule.cs | 1 + .../XdcP2PCapabilityResolver.cs | 27 +++++++ src/Nethermind/Nethermind.Xdc/XdcPlugin.cs | 12 ---- .../Nethermind.Xdc/XdcSubnetPlugin.cs | 2 - 24 files changed, 397 insertions(+), 255 deletions(-) create mode 100644 src/Nethermind/Nethermind.Merge.Plugin.Test/MergeP2PCapabilityResolverTests.cs create mode 100644 src/Nethermind/Nethermind.Merge.Plugin/MergeP2PCapabilityResolver.cs delete mode 100644 src/Nethermind/Nethermind.Network.Test/SnapCapabilitySwitcherTests.cs create mode 100644 src/Nethermind/Nethermind.Network.Test/SnapP2PCapabilityResolverTests.cs create mode 100644 src/Nethermind/Nethermind.Network/IP2PCapabilityResolver.cs delete mode 100644 src/Nethermind/Nethermind.Network/SnapCapabilitySwitcher.cs create mode 100644 src/Nethermind/Nethermind.Network/SnapP2PCapabilityResolver.cs create mode 100644 src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs create mode 100644 src/Nethermind/Nethermind.Xdc/XdcP2PCapabilityResolver.cs diff --git a/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs b/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs index 956dcd4060a8..8421c6f14dd7 100644 --- a/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs +++ b/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs @@ -2,13 +2,10 @@ // SPDX-License-Identifier: LGPL-3.0-only using Autofac; -using Nethermind.Blockchain.Synchronization; using Nethermind.Consensus; using Nethermind.Logging; using Nethermind.Network; using Nethermind.Network.Config; -using Nethermind.Network.Contract.P2P; -using Nethermind.Stats.Model; namespace Nethermind.Core.Test.Modules; @@ -21,24 +18,7 @@ protected override void Load(ContainerBuilder builder) builder .AddSingleton(Policy.FullGossip) - // TODO: LastNStateRootTracker - - .AddAdvance(cfg => - { - cfg - .As() - .SingleInstance() - .OnActivating((m) => - { - ProtocolsManager protocolManager = m.Instance; - ISyncConfig syncConfig = m.Context.Resolve(); - - if (syncConfig.SnapServingEnabled == true || syncConfig.SnapSync) - { - protocolManager.AddSupportedCapability(new Capability(Protocol.Snap, 1)); - } - }); - }) + // Snap capability is contributed by SnapP2PCapabilityResolver, registered in the production NetworkModule. // Some config migration .AddDecorator((ctx, networkConfig) => diff --git a/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs b/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs index 94222454c1b8..90c38c3de488 100644 --- a/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs +++ b/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs @@ -67,7 +67,7 @@ protected override void Load(ContainerBuilder builder) .AddSingleton() .AddSingleton() .AddSingleton() - .AddSingleton() + .AddLast() // Handshake .AddMessageSerializer() diff --git a/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs b/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs index 164e0829fbe7..8bcfd95cbdb1 100644 --- a/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs +++ b/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs @@ -15,10 +15,8 @@ using Nethermind.Logging; using Nethermind.Network; using Nethermind.Network.Config; -using Nethermind.Network.Contract.P2P; using Nethermind.Network.Discovery; using Nethermind.Network.Rlpx; -using Nethermind.Stats.Model; using Nethermind.Synchronization; using Nethermind.Synchronization.Peers; @@ -59,7 +57,6 @@ public class InitializeNetwork : IStep private readonly IEnode _enode; private readonly INethermindPlugin[] _plugins; private readonly Lazy _protocolsManager; - private readonly Lazy _snapCapabilitySwitcher; private readonly NodeSourceToDiscV4Feeder _enrDiscoveryAppFeeder; private readonly ISyncConfig _syncConfig; @@ -84,7 +81,6 @@ public InitializeNetwork( IEnode enode, INethermindPlugin[] plugins, Lazy protocolsManager, - Lazy snapCapabilitySwitcher, INetworkConfig networkConfig, ISyncConfig syncConfig, IInitConfig initConfig, @@ -105,7 +101,6 @@ ILogManager logManager _enode = enode; _plugins = plugins; _protocolsManager = protocolsManager; - _snapCapabilitySwitcher = snapCapabilitySwitcher; _networkConfig = networkConfig; _syncConfig = syncConfig; _initConfig = initConfig; @@ -152,12 +147,6 @@ await InitPeer().ContinueWith(initPeerTask => } }); - if (_syncConfig.SnapSync && _syncConfig.SnapServingEnabled != true) - { - _snapCapabilitySwitcher.Value.EnableSnapCapabilityUntilSynced(); - } - else if (_logger.IsDebug) _logger.Debug("Skipped enabling snap capability"); - if (cancellationToken.IsCancellationRequested) { return; @@ -259,12 +248,9 @@ private Task StartSync() protected virtual async Task InitPeer() { - IProtocolsManager protocolsManager = _protocolsManager.Value; + // Force creation so the protocols manager subscribes to session events before the RLPx listener starts. + _ = _protocolsManager.Value; - if (_syncConfig.SnapServingEnabled == true) - { - protocolsManager.AddSupportedCapability(new Capability(Protocol.Snap, 1)); - } if (!_networkConfig.DisableDiscV4DnsFeeder) { // Feed some nodes into discoveryApp in case all bootnodes is faulty. diff --git a/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs b/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs index 42031d598011..6319705cd62d 100644 --- a/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs +++ b/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs @@ -233,6 +233,7 @@ await base.Build(builder => Substitute.For(), Substitute.For(), Array.Empty(), + Array.Empty(), LimboLogs.Instance ); diff --git a/src/Nethermind/Nethermind.Merge.AuRa/AuRaMergePlugin.cs b/src/Nethermind/Nethermind.Merge.AuRa/AuRaMergePlugin.cs index d66e86961909..bd543014d705 100644 --- a/src/Nethermind/Nethermind.Merge.AuRa/AuRaMergePlugin.cs +++ b/src/Nethermind/Nethermind.Merge.AuRa/AuRaMergePlugin.cs @@ -17,6 +17,7 @@ using Nethermind.Consensus.Withdrawals; using Nethermind.Config; using Nethermind.Core; +using Nethermind.Core.Container; using Nethermind.Core.Specs; using Nethermind.Evm.TransactionProcessing; using Nethermind.Logging; @@ -25,6 +26,7 @@ using Nethermind.Merge.Plugin; using Nethermind.Merge.Plugin.BlockProduction; using Nethermind.Merge.Plugin.Handlers; +using Nethermind.Network; using Nethermind.Specs.ChainSpecStyle; namespace Nethermind.Merge.AuRa @@ -80,6 +82,8 @@ public class AuRaMergeModule : Module protected override void Load(ContainerBuilder builder) => builder .AddModule(new BaseMergePluginModule()) + .AddLast() + // Aura (non merge) use `BlockProducerStarter` directly. .AddSingleton() diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/MergeP2PCapabilityResolverTests.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/MergeP2PCapabilityResolverTests.cs new file mode 100644 index 000000000000..54a7ca3430d4 --- /dev/null +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/MergeP2PCapabilityResolverTests.cs @@ -0,0 +1,45 @@ +// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System.Collections.Generic; +using Nethermind.Consensus; +using Nethermind.Network.Contract.P2P; +using Nethermind.Stats.Model; +using NSubstitute; +using NUnit.Framework; + +namespace Nethermind.Merge.Plugin.Test; + +[Parallelizable(ParallelScope.All)] +public class MergeP2PCapabilityResolverTests +{ + [TestCase(true, true)] + [TestCase(false, false)] + public void Resolve_advertises_post_merge_eth_capabilities_only_after_transition(bool transitionFinished, bool expected) + { + IPoSSwitcher poSSwitcher = Substitute.For(); + poSSwitcher.TransitionFinished.Returns(transitionFinished); + using MergeP2PCapabilityResolver resolver = new(poSSwitcher); + + HashSet capabilities = []; + resolver.Resolve(capabilities); + + Assert.That(capabilities.Contains(new Capability(Protocol.Eth, 69)), Is.EqualTo(expected)); + Assert.That(capabilities.Contains(new Capability(Protocol.Eth, 70)), Is.EqualTo(expected)); + Assert.That(capabilities.Contains(new Capability(Protocol.Eth, 71)), Is.EqualTo(expected)); + } + + [Test] + public void Raises_Changed_when_terminal_block_reached() + { + IPoSSwitcher poSSwitcher = Substitute.For(); + using MergeP2PCapabilityResolver resolver = new(poSSwitcher); + + bool changed = false; + resolver.Changed += () => changed = true; + + poSSwitcher.TerminalBlockReached += Raise.Event(); + + Assert.That(changed, Is.True); + } +} diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs index e7ca6cd6036f..7ec670896783 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/MergePluginTests.cs @@ -21,13 +21,11 @@ using Nethermind.Logging; using Nethermind.Merge.Plugin.BlockProduction; using Nethermind.Network; -using Nethermind.Network.Contract.P2P; using Nethermind.Runner.Ethereum.Modules; using Nethermind.Runner.Test.Ethereum; using Nethermind.Serialization.Json; using Nethermind.Specs.ChainSpecStyle; using Nethermind.Specs.Test.ChainSpecStyle; -using Nethermind.Stats.Model; using NUnit.Framework; using NSubstitute; @@ -198,48 +196,6 @@ public async Task Initializes_correctly() Assert.That(blockProducer, Is.InstanceOf()); } - [Test] - public async Task InitNetworkProtocol_adds_post_merge_eth_capabilities_when_transition_finished() - { - IPoSSwitcher poSSwitcher = Substitute.For(); - poSSwitcher.TransitionFinished.Returns(true); - - await using IContainer container = BuildContainer(configure: builder => builder - .RegisterInstance(poSSwitcher) - .As()); - INethermindApi api = container.Resolve(); - await _consensusPlugin!.Init(api); - await _plugin.Init(api); - - api.ProtocolsManager!.ClearReceivedCalls(); - await _plugin.InitNetworkProtocol(); - - AssertPostMergeEthCapabilitiesAdded(api); - } - - [Test] - public async Task InitNetworkProtocol_delays_post_merge_eth_capabilities_until_terminal_block() - { - IPoSSwitcher poSSwitcher = Substitute.For(); - poSSwitcher.TransitionFinished.Returns(false); - - await using IContainer container = BuildContainer(configure: builder => builder - .RegisterInstance(poSSwitcher) - .As()); - INethermindApi api = container.Resolve(); - await _consensusPlugin!.Init(api); - await _plugin.Init(api); - - api.ProtocolsManager!.ClearReceivedCalls(); - await _plugin.InitNetworkProtocol(); - - api.ProtocolsManager!.DidNotReceive().AddSupportedCapability(Arg.Any()); - - poSSwitcher.TerminalBlockReached += Raise.Event(); - - AssertPostMergeEthCapabilitiesAdded(api); - } - [Test] public async Task Init_registers_gas_limit_calculator_for_testing_rpc_module() { @@ -301,13 +257,4 @@ public async Task InitDisableJsonRpcUrlWithNoEngineUrl() Assert.That(jsonRpcConfig.EnabledModules, Is.Empty); Assert.That(jsonRpcConfig.AdditionalRpcUrls, Is.EqualTo(new[] { "http://localhost:8551|http;ws|net;eth;subscribe;web3;engine;client" })); } - - private static void AssertPostMergeEthCapabilitiesAdded(INethermindApi api) - { - IProtocolsManager protocolsManager = api.ProtocolsManager!; - - protocolsManager.Received(1).AddSupportedCapability(new Capability(Protocol.Eth, 69)); - protocolsManager.Received(1).AddSupportedCapability(new Capability(Protocol.Eth, 70)); - protocolsManager.Received(1).AddSupportedCapability(new Capability(Protocol.Eth, 71)); - } } diff --git a/src/Nethermind/Nethermind.Merge.Plugin/MergeP2PCapabilityResolver.cs b/src/Nethermind/Nethermind.Merge.Plugin/MergeP2PCapabilityResolver.cs new file mode 100644 index 000000000000..edf6c6fc4da4 --- /dev/null +++ b/src/Nethermind/Nethermind.Merge.Plugin/MergeP2PCapabilityResolver.cs @@ -0,0 +1,40 @@ +// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Collections.Generic; +using Nethermind.Consensus; +using Nethermind.Network; +using Nethermind.Network.Contract.P2P; +using Nethermind.Stats.Model; + +namespace Nethermind.Merge.Plugin; + +/// +/// Advertises the post-merge eth/69, eth/70 and eth/71 capabilities once the merge transition has finished. +/// +public class MergeP2PCapabilityResolver : IP2PCapabilityResolver, IDisposable +{ + private readonly IPoSSwitcher _poSSwitcher; + + public event Action? Changed; + + public MergeP2PCapabilityResolver(IPoSSwitcher poSSwitcher) + { + _poSSwitcher = poSSwitcher; + _poSSwitcher.TerminalBlockReached += OnTerminalBlockReached; + } + + public void Resolve(ISet capabilities) + { + if (!_poSSwitcher.TransitionFinished) return; + + capabilities.Add(new Capability(Protocol.Eth, 69)); + capabilities.Add(new Capability(Protocol.Eth, 70)); + capabilities.Add(new Capability(Protocol.Eth, 71)); + } + + private void OnTerminalBlockReached(object? sender, EventArgs e) => Changed?.Invoke(); + + public void Dispose() => _poSSwitcher.TerminalBlockReached -= OnTerminalBlockReached; +} diff --git a/src/Nethermind/Nethermind.Merge.Plugin/MergePlugin.cs b/src/Nethermind/Nethermind.Merge.Plugin/MergePlugin.cs index 5a03dc4774df..66fefcd92029 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/MergePlugin.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/MergePlugin.cs @@ -18,6 +18,7 @@ using Nethermind.Consensus.Rewards; using Nethermind.Consensus.Validators; using Nethermind.Core; +using Nethermind.Core.Container; using Nethermind.Core.Crypto; using Nethermind.Core.Specs; using Nethermind.Core.Exceptions; @@ -35,7 +36,7 @@ using Nethermind.Merge.Plugin.InvalidChainTracker; using Nethermind.Merge.Plugin.SszRest; using Nethermind.Merge.Plugin.Synchronization; -using Nethermind.Network.Contract.P2P; +using Nethermind.Network; using Nethermind.Serialization.Json; using Nethermind.Specs.ChainSpecStyle; using Nethermind.State; @@ -191,37 +192,16 @@ public Task InitNetworkProtocol() if (MergeEnabled) { ArgumentNullException.ThrowIfNull(_api.SpecProvider); - ArgumentNullException.ThrowIfNull(_api.ProtocolsManager); if (_api.BlockProductionPolicy is null) throw new ArgumentException(nameof(_api.BlockProductionPolicy)); _mergeBlockProductionPolicy = new MergeBlockProductionPolicy(_api.BlockProductionPolicy); _api.BlockProductionPolicy = _mergeBlockProductionPolicy; InitializeMergeFinalization(); - - if (_poSSwitcher.TransitionFinished) - { - AddPostMergeNetworkProtocols(); - } - else - { - if (_logger.IsDebug) _logger.Debug("Delayed adding post-merge eth/* capabilities until terminal block reached"); - _poSSwitcher.TerminalBlockReached += (_, _) => AddPostMergeNetworkProtocols(); - } } return Task.CompletedTask; } - private void AddPostMergeNetworkProtocols() - { - if (_logger.IsInfo) _logger.Info("Adding eth/69 capability"); - _api.ProtocolsManager!.AddSupportedCapability(new(Protocol.Eth, 69)); - if (_logger.IsInfo) _logger.Info("Adding eth/70 capability"); - _api.ProtocolsManager!.AddSupportedCapability(new(Protocol.Eth, 70)); - if (_logger.IsInfo) _logger.Info("Adding eth/71 capability"); - _api.ProtocolsManager!.AddSupportedCapability(new(Protocol.Eth, 71)); - } - /// /// Hook for derived plugins (e.g. AuRaMergePlugin) to set up merge-transition lifecycle /// (e.g. disposing AuRa's finalization manager at terminal block). Default: no-op. @@ -254,6 +234,8 @@ protected override void Load(ContainerBuilder builder) => builder .AddDecorator() .AddDecorator() + .AddLast() + .AddModule(new BaseMergePluginModule()); } diff --git a/src/Nethermind/Nethermind.Network.Test/ProtocolsManagerTests.cs b/src/Nethermind/Nethermind.Network.Test/ProtocolsManagerTests.cs index ecd76a07bf41..65286bd3d5f1 100644 --- a/src/Nethermind/Nethermind.Network.Test/ProtocolsManagerTests.cs +++ b/src/Nethermind/Nethermind.Network.Test/ProtocolsManagerTests.cs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: LGPL-3.0-only using System; +using System.Collections.Generic; using System.Net; using System.Numerics; using DotNetty.Transport.Channels; @@ -68,6 +69,7 @@ public void Uses_host_disconnected_event_when_unsubscribing_on_disconnect() Substitute.For(), Substitute.For(), [], + [], LimboLogs.Instance); rlpxHost.SessionCreated += Raise.EventWith(new object(), new SessionEventArgs(session)); @@ -85,6 +87,61 @@ public void Uses_host_disconnected_event_when_unsubscribing_on_disconnect() Assert.That(session.RemovedDisconnectedHandler, Is.Null); } + [Test] + public void Advertised_capabilities_apply_resolver_additions_and_removals() + { + ProtocolsManager manager = BuildManagerWithResolvers( + new FakeCapabilityResolver(caps => caps.Add(new Capability(Protocol.Eth, 69))), + new FakeCapabilityResolver(caps => caps.Remove(new Capability(Protocol.Eth, 68)))); + + // Default eth/68 removed, eth/69 added by the resolvers. + Assert.That(manager.GetHighestProtocolVersion(Protocol.Eth), Is.EqualTo(69)); + } + + [Test] + public void Advertised_capabilities_are_cached_and_rebuilt_on_resolver_change() + { + FakeCapabilityResolver resolver = new(caps => caps.Add(new Capability(Protocol.Snap, 1))); + ProtocolsManager manager = BuildManagerWithResolvers(resolver); + + Assert.That(manager.GetHighestProtocolVersion(Protocol.Snap), Is.EqualTo(1)); + Assert.That(manager.GetHighestProtocolVersion(Protocol.Snap), Is.EqualTo(1)); + Assert.That(resolver.ResolveCount, Is.EqualTo(1), "advertised capabilities should be cached across calls"); + + resolver.RaiseChanged(); + + Assert.That(manager.GetHighestProtocolVersion(Protocol.Snap), Is.EqualTo(1)); + Assert.That(resolver.ResolveCount, Is.EqualTo(2), "cache should rebuild after a resolver signals a change"); + } + + private static ProtocolsManager BuildManagerWithResolvers(params IP2PCapabilityResolver[] resolvers) => + new( + Substitute.For(), + Substitute.For(), + Substitute.For(), + Substitute.For(), + Substitute.For(), + Substitute.For(), + Substitute.For(), + [], + resolvers, + LimboLogs.Instance); + + private sealed class FakeCapabilityResolver(Action> resolve) : IP2PCapabilityResolver + { + public int ResolveCount { get; private set; } + + public void Resolve(ISet capabilities) + { + ResolveCount++; + resolve(capabilities); + } + + public event Action? Changed; + + public void RaiseChanged() => Changed?.Invoke(); + } + public class Context { private readonly int _localPort = 30312; @@ -161,6 +218,7 @@ public Context() _protocolValidator, _peerStorage, BuildProtocolHandlerFactories(), + [], LimboLogs.Instance); } diff --git a/src/Nethermind/Nethermind.Network.Test/SnapCapabilitySwitcherTests.cs b/src/Nethermind/Nethermind.Network.Test/SnapCapabilitySwitcherTests.cs deleted file mode 100644 index 8230b5c03755..000000000000 --- a/src/Nethermind/Nethermind.Network.Test/SnapCapabilitySwitcherTests.cs +++ /dev/null @@ -1,28 +0,0 @@ -// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only - -using Nethermind.Logging; -using Nethermind.Network.Contract.P2P; -using Nethermind.Stats.Model; -using Nethermind.Synchronization.ParallelSync; -using NSubstitute; -using NUnit.Framework; - -namespace Nethermind.Network.Test; - -public class SnapCapabilitySwitcherTests -{ - [Test] - public void Dispose_ShouldUnsubscribeFromSyncModeChanges() - { - IProtocolsManager protocolsManager = Substitute.For(); - ISyncModeSelector syncModeSelector = Substitute.For(); - SnapCapabilitySwitcher snapCapabilitySwitcher = new(protocolsManager, syncModeSelector, LimboLogs.Instance); - - snapCapabilitySwitcher.EnableSnapCapabilityUntilSynced(); - snapCapabilitySwitcher.Dispose(); - syncModeSelector.Changed += Raise.EventWith(this, new SyncModeChangedEventArgs(SyncMode.StateNodes, SyncMode.Full)); - - protocolsManager.Received(1).RemoveSupportedCapability(new Capability(Protocol.Snap, 1)); - } -} diff --git a/src/Nethermind/Nethermind.Network.Test/SnapP2PCapabilityResolverTests.cs b/src/Nethermind/Nethermind.Network.Test/SnapP2PCapabilityResolverTests.cs new file mode 100644 index 000000000000..3e23090aece2 --- /dev/null +++ b/src/Nethermind/Nethermind.Network.Test/SnapP2PCapabilityResolverTests.cs @@ -0,0 +1,48 @@ +// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System.Collections.Generic; +using Nethermind.Blockchain.Synchronization; +using Nethermind.Network.Contract.P2P; +using Nethermind.Stats.Model; +using Nethermind.Synchronization.ParallelSync; +using NSubstitute; +using NUnit.Framework; + +namespace Nethermind.Network.Test; + +[Parallelizable(ParallelScope.All)] +public class SnapP2PCapabilityResolverTests +{ + [TestCase(true, false, SyncMode.StateNodes, true, TestName = "Serving advertises snap regardless of sync")] + [TestCase(false, true, SyncMode.StateNodes, true, TestName = "Snap-syncing state advertises snap")] + [TestCase(false, true, SyncMode.Full, false, TestName = "Snap sync finished drops snap")] + [TestCase(false, false, SyncMode.StateNodes, false, TestName = "Neither serving nor snap-syncing")] + public void Resolve_advertises_snap_when_serving_or_syncing_state(bool snapServing, bool snapSync, SyncMode currentMode, bool expected) + { + ISyncConfig syncConfig = new SyncConfig { SnapServingEnabled = snapServing, SnapSync = snapSync }; + ISyncModeSelector syncModeSelector = Substitute.For(); + syncModeSelector.Current.Returns(currentMode); + using SnapP2PCapabilityResolver resolver = new(syncConfig, syncModeSelector); + + HashSet capabilities = []; + resolver.Resolve(capabilities); + + Assert.That(capabilities.Contains(new Capability(Protocol.Snap, 1)), Is.EqualTo(expected)); + } + + [Test] + public void Raises_Changed_on_sync_mode_change() + { + ISyncConfig syncConfig = new SyncConfig(); + ISyncModeSelector syncModeSelector = Substitute.For(); + using SnapP2PCapabilityResolver resolver = new(syncConfig, syncModeSelector); + + bool changed = false; + resolver.Changed += () => changed = true; + + syncModeSelector.Changed += Raise.EventWith(new SyncModeChangedEventArgs(SyncMode.StateNodes, SyncMode.Full)); + + Assert.That(changed, Is.True); + } +} diff --git a/src/Nethermind/Nethermind.Network/IP2PCapabilityResolver.cs b/src/Nethermind/Nethermind.Network/IP2PCapabilityResolver.cs new file mode 100644 index 000000000000..b11a185f723c --- /dev/null +++ b/src/Nethermind/Nethermind.Network/IP2PCapabilityResolver.cs @@ -0,0 +1,26 @@ +// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Collections.Generic; +using Nethermind.Stats.Model; + +namespace Nethermind.Network; + +/// +/// Contributes to the set of devp2p capabilities the node advertises in its P2P Hello message. +/// Implementations may add or remove capabilities based on node configuration and runtime state. +/// +/// +/// The advertised set is computed once into a cached array and reused for every session, so +/// is not on the per-session hot path. Implementations whose contribution depends on mutable state must raise +/// whenever it would change, so the cache is rebuilt. Static implementations never raise it. +/// +public interface IP2PCapabilityResolver +{ + /// Adds and/or removes this resolver's capabilities from the running set. + void Resolve(ISet capabilities); + + /// Raised when this resolver's contribution changes, to invalidate the cached advertised set. + event Action? Changed; +} diff --git a/src/Nethermind/Nethermind.Network/IProtocolsManager.cs b/src/Nethermind/Nethermind.Network/IProtocolsManager.cs index 0b4a4a3a3bc2..5a9fd071dec5 100644 --- a/src/Nethermind/Nethermind.Network/IProtocolsManager.cs +++ b/src/Nethermind/Nethermind.Network/IProtocolsManager.cs @@ -1,14 +1,10 @@ // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only -using Nethermind.Stats.Model; - namespace Nethermind.Network { public interface IProtocolsManager { - void AddSupportedCapability(Capability capability); - void RemoveSupportedCapability(Capability capability); int GetHighestProtocolVersion(string protocol); } } diff --git a/src/Nethermind/Nethermind.Network/ProtocolsManager.cs b/src/Nethermind/Nethermind.Network/ProtocolsManager.cs index 7e669637f0b9..aa0c2e40f948 100644 --- a/src/Nethermind/Nethermind.Network/ProtocolsManager.cs +++ b/src/Nethermind/Nethermind.Network/ProtocolsManager.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Linq; using System.Numerics; +using System.Threading; using Autofac.Features.AttributeFilters; using Nethermind.Config; using Nethermind.Core.Crypto; @@ -41,7 +42,9 @@ public class ProtocolsManager : IProtocolsManager, IProtocolRegistrar private readonly INetworkStorage _peerStorage; private readonly ILogger _logger; private readonly IProtocolHandlerFactory[] _factories; - private readonly HashSet _capabilities = DefaultCapabilities.ToHashSet(); + private readonly IP2PCapabilityResolver[] _capabilityResolvers; + private readonly Lock _capabilitiesLock = new(); + private Capability[]? _cachedCapabilities; private readonly EventHandler _onSessionCreated; private readonly EventHandler _onSessionInitialized; private readonly SessionDisconnectedEventHandler _onSessionDisconnected; @@ -55,6 +58,7 @@ public ProtocolsManager( IProtocolValidator protocolValidator, [KeyFilter(DbNames.PeersDb)] INetworkStorage peerStorage, IProtocolHandlerFactory[] factories, + IP2PCapabilityResolver[] capabilityResolvers, ILogManager logManager) { _syncPool = syncPeerPool ?? throw new ArgumentNullException(nameof(syncPeerPool)); @@ -67,6 +71,11 @@ public ProtocolsManager( // Order is already set by OrderedComponents (AddFirst/AddLast) _factories = factories; + _capabilityResolvers = capabilityResolvers; + foreach (IP2PCapabilityResolver resolver in _capabilityResolvers) + { + resolver.Changed += InvalidateCapabilities; + } _onSessionCreated = SessionCreated; _onSessionInitialized = SessionInitialized; _onSessionDisconnected = SessionDisconnected; @@ -165,9 +174,10 @@ void IProtocolRegistrar.Register(ISession session, P2PProtocolHandler handler) { session.PingSender = handler; - foreach (Capability capability in _capabilities) + Capability[] capabilities = GetAdvertisedCapabilities(); + for (int i = 0; i < capabilities.Length; i++) { - session.AddSupportedCapability(capability); + session.AddSupportedCapability(capabilities[i]); } } @@ -338,20 +348,10 @@ private void AddNodeToDiscovery(ISession session, P2PProtocolInitializedEventArg _discoveryApp.AddNodeToDiscovery(session.Node); } - public void AddSupportedCapability(Capability capability) => _capabilities.Add(capability); - - public void RemoveSupportedCapability(Capability capability) - { - if (_capabilities.Remove(capability)) - { - if (_logger.IsTrace) _logger.Trace($"Removed supported capability: {capability}"); - } - } - public int GetHighestProtocolVersion(string protocol) { int highestVersion = 0; - foreach (Capability capability in _capabilities) + foreach (Capability capability in GetAdvertisedCapabilities()) { if (capability.ProtocolCode == protocol) { @@ -361,5 +361,40 @@ public int GetHighestProtocolVersion(string protocol) return highestVersion; } + + private void InvalidateCapabilities() + { + lock (_capabilitiesLock) + { + _cachedCapabilities = null; + } + } + + /// + /// Returns the capabilities to advertise, computed from and the registered + /// s. The result is cached and only recomputed when a resolver signals a + /// change via , keeping the per-session path allocation-free. + /// + private Capability[] GetAdvertisedCapabilities() + { + Capability[]? cached = Volatile.Read(ref _cachedCapabilities); + if (cached is not null) return cached; + + lock (_capabilitiesLock) + { + if (_cachedCapabilities is null) + { + HashSet capabilities = DefaultCapabilities.ToHashSet(); + foreach (IP2PCapabilityResolver resolver in _capabilityResolvers) + { + resolver.Resolve(capabilities); + } + + Volatile.Write(ref _cachedCapabilities, capabilities.ToArray()); + } + + return _cachedCapabilities; + } + } } } diff --git a/src/Nethermind/Nethermind.Network/SnapCapabilitySwitcher.cs b/src/Nethermind/Nethermind.Network/SnapCapabilitySwitcher.cs deleted file mode 100644 index 6ef7cdb21cd9..000000000000 --- a/src/Nethermind/Nethermind.Network/SnapCapabilitySwitcher.cs +++ /dev/null @@ -1,70 +0,0 @@ -// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only -// - -using System; -using Nethermind.Logging; -using Nethermind.Network.Contract.P2P; -using Nethermind.Stats.Model; -using Nethermind.Synchronization.ParallelSync; - -namespace Nethermind.Network; - -// Temporary class used for removing snap capability after SnapSync finish. -// Will be removed after implementing missing functionality - serving data via snap protocol. -public class SnapCapabilitySwitcher : IDisposable -{ - private static readonly Capability SnapCapability = new(Protocol.Snap, 1); - - private readonly IProtocolsManager _protocolsManager; - private readonly ISyncModeSelector _syncModeSelector; - private readonly ILogger _logger; - private volatile bool _isSubscribed; - - public SnapCapabilitySwitcher(IProtocolsManager? protocolsManager, ISyncModeSelector? syncModeSelector, ILogManager? logManager) - { - ArgumentNullException.ThrowIfNull(protocolsManager); - ArgumentNullException.ThrowIfNull(syncModeSelector); - ArgumentNullException.ThrowIfNull(logManager); - - _protocolsManager = protocolsManager; - _syncModeSelector = syncModeSelector; - _logger = logManager.GetClassLogger(); - } - - /// - /// Add Snap capability if SnapSync is not finished and remove after finished. - /// - public void EnableSnapCapabilityUntilSynced() - { - if (!_isSubscribed) - { - _protocolsManager.AddSupportedCapability(SnapCapability); - _syncModeSelector.Changed += OnSyncModeChanged; - _isSubscribed = true; - } - - if (_logger.IsDebug) _logger.Debug("Enabled snap capability"); - } - - private void OnSyncModeChanged(object? sender, SyncModeChangedEventArgs syncMode) - { - if ((syncMode.Current & SyncMode.Full) != 0) - { - DisableSnapCapability(); - if (_logger.IsInfo) _logger.Info("State sync finished. Disabled snap capability."); - } - } - - public void Dispose() => DisableSnapCapability(); - - private void DisableSnapCapability() - { - if (_isSubscribed) - { - _syncModeSelector.Changed -= OnSyncModeChanged; - _protocolsManager.RemoveSupportedCapability(SnapCapability); - _isSubscribed = false; - } - } -} diff --git a/src/Nethermind/Nethermind.Network/SnapP2PCapabilityResolver.cs b/src/Nethermind/Nethermind.Network/SnapP2PCapabilityResolver.cs new file mode 100644 index 000000000000..937dbd1da42d --- /dev/null +++ b/src/Nethermind/Nethermind.Network/SnapP2PCapabilityResolver.cs @@ -0,0 +1,50 @@ +// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Collections.Generic; +using Nethermind.Blockchain.Synchronization; +using Nethermind.Network.Contract.P2P; +using Nethermind.Stats.Model; +using Nethermind.Synchronization.ParallelSync; + +namespace Nethermind.Network; + +/// +/// Advertises snap/1 while the node serves snap data or still needs to snap-sync its own state. +/// +/// +/// Replaces the former SnapCapabilitySwitcher: instead of adding the capability on start and removing it +/// when state sync reaches , the contribution is recomputed per session, so a session +/// opened after sync completes simply no longer advertises snap. +/// +public class SnapP2PCapabilityResolver : IP2PCapabilityResolver, IDisposable +{ + private static readonly Capability SnapCapability = new(Protocol.Snap, 1); + + private readonly ISyncConfig _syncConfig; + private readonly ISyncModeSelector _syncModeSelector; + + public event Action? Changed; + + public SnapP2PCapabilityResolver(ISyncConfig syncConfig, ISyncModeSelector syncModeSelector) + { + _syncConfig = syncConfig; + _syncModeSelector = syncModeSelector; + _syncModeSelector.Changed += OnSyncModeChanged; + } + + public void Resolve(ISet capabilities) + { + bool serving = _syncConfig.SnapServingEnabled == true; + bool syncingState = _syncConfig.SnapSync && (_syncModeSelector.Current & SyncMode.Full) == 0; + if (serving || syncingState) + { + capabilities.Add(SnapCapability); + } + } + + private void OnSyncModeChanged(object? sender, SyncModeChangedEventArgs e) => Changed?.Invoke(); + + public void Dispose() => _syncModeSelector.Changed -= OnSyncModeChanged; +} diff --git a/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs b/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs index fe8b758a4dd6..b9e674837e37 100644 --- a/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs +++ b/src/Nethermind/Nethermind.Runner.Test/Module/NetworkModuleTest.cs @@ -176,7 +176,6 @@ public async Task Initialize_network_registers_plugin_capabilities_before_starti await initializeNetwork.RunInitPeer(); Assert.That(callOrder, Is.EqualTo(new[] { "plugin", "rlpx" })); - protocolsManager.DidNotReceive().RemoveSupportedCapability(new Capability(Protocol.NodeData, 1)); } private IEnumerable<(Type MessageType, Type SerializerType)> FindSerializersInAssembly(Assembly assembly) @@ -250,7 +249,6 @@ private sealed class TestInitializeNetwork( Substitute.For(), plugins, new Lazy(() => protocolsManager), - new Lazy(() => null!), networkConfig, syncConfig, Substitute.For(), diff --git a/src/Nethermind/Nethermind.Synchronization.Test/E2ESyncTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/E2ESyncTests.cs index b3b1117872ec..d0b04290e2aa 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/E2ESyncTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/E2ESyncTests.cs @@ -39,6 +39,7 @@ using Nethermind.Merge.Plugin.BlockProduction; using Nethermind.Merge.Plugin.Handlers; using Nethermind.Merge.Plugin.Synchronization; +using Nethermind.Core.Container; using Nethermind.Network; using Nethermind.Network.Config; using Nethermind.Network.Contract.P2P; @@ -306,6 +307,7 @@ private async Task CreateNode( .AddModule(new TestMergeModule(configProvider.GetConfig())) .AddSingleton(timestamper) // Used by test code .AddDecorator() + .AddLast(_ => new PostMergeCapabilitiesResolver()) ; } else @@ -320,20 +322,19 @@ private async Task CreateNode( IContainer container = builder.Build(); - if (isPostMerge) - { - EnablePostMergeEthCapabilities(container.Resolve()); - } - return container; } - private static void EnablePostMergeEthCapabilities(IProtocolsManager protocolsManager) + private sealed class PostMergeCapabilitiesResolver : IP2PCapabilityResolver { - ArgumentNullException.ThrowIfNull(protocolsManager); - protocolsManager.AddSupportedCapability(new Capability(Protocol.Eth, EthVersions.Eth69)); - protocolsManager.AddSupportedCapability(new Capability(Protocol.Eth, EthVersions.Eth70)); - protocolsManager.AddSupportedCapability(new Capability(Protocol.Eth, EthVersions.Eth71)); + public event Action? Changed { add { } remove { } } + + public void Resolve(ISet capabilities) + { + capabilities.Add(new Capability(Protocol.Eth, EthVersions.Eth69)); + capabilities.Add(new Capability(Protocol.Eth, EthVersions.Eth70)); + capabilities.Add(new Capability(Protocol.Eth, EthVersions.Eth71)); + } } private static void EnableBlockAccessListsFromGenesis(ChainSpec spec) diff --git a/src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs b/src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs new file mode 100644 index 000000000000..abc6c1bb5aec --- /dev/null +++ b/src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs @@ -0,0 +1,29 @@ +// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System.Collections.Generic; +using Nethermind.Network.Contract.P2P; +using Nethermind.Stats.Model; +using NUnit.Framework; + +namespace Nethermind.Xdc.Test; + +[Parallelizable(ParallelScope.All)] +public class XdcP2PCapabilityResolverTests +{ + [Test] + public void Resolve_replaces_default_eth68_with_xdc_capabilities() + { + XdcP2PCapabilityResolver resolver = new(); + + HashSet capabilities = [new(Protocol.Eth, 68)]; + resolver.Resolve(capabilities); + + Assert.That(capabilities, Is.EquivalentTo(new[] + { + new Capability(Protocol.Eth, 62), + new Capability(Protocol.Eth, 63), + new Capability(Protocol.Eth, 100), + })); + } +} diff --git a/src/Nethermind/Nethermind.Xdc/XdcModule.cs b/src/Nethermind/Nethermind.Xdc/XdcModule.cs index ec2472272822..80c3cd70641e 100644 --- a/src/Nethermind/Nethermind.Xdc/XdcModule.cs +++ b/src/Nethermind/Nethermind.Xdc/XdcModule.cs @@ -131,6 +131,7 @@ protected override void Load(ContainerBuilder builder) .AddMessageSerializer() .AddLast() + .AddLast() .AddSingleton() // block processing diff --git a/src/Nethermind/Nethermind.Xdc/XdcP2PCapabilityResolver.cs b/src/Nethermind/Nethermind.Xdc/XdcP2PCapabilityResolver.cs new file mode 100644 index 000000000000..69bc4f9033b6 --- /dev/null +++ b/src/Nethermind/Nethermind.Xdc/XdcP2PCapabilityResolver.cs @@ -0,0 +1,27 @@ +// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Collections.Generic; +using Nethermind.Network; +using Nethermind.Network.Contract.P2P; +using Nethermind.Stats.Model; + +namespace Nethermind.Xdc; + +/// +/// XDC advertises eth/62, eth/63 and eth/100 instead of the default eth/68. +/// +public class XdcP2PCapabilityResolver : IP2PCapabilityResolver +{ + // XDC's capability set is static, so the cache never needs invalidating. + public event Action? Changed { add { } remove { } } + + public void Resolve(ISet capabilities) + { + capabilities.Remove(new Capability(Protocol.Eth, 68)); + capabilities.Add(new Capability(Protocol.Eth, 62)); + capabilities.Add(new Capability(Protocol.Eth, 63)); + capabilities.Add(new Capability(Protocol.Eth, 100)); + } +} diff --git a/src/Nethermind/Nethermind.Xdc/XdcPlugin.cs b/src/Nethermind/Nethermind.Xdc/XdcPlugin.cs index 596f3b4d5b87..eeb871624c2a 100644 --- a/src/Nethermind/Nethermind.Xdc/XdcPlugin.cs +++ b/src/Nethermind/Nethermind.Xdc/XdcPlugin.cs @@ -4,7 +4,6 @@ using Autofac.Core; using Nethermind.Api; using Nethermind.Api.Extensions; -using Nethermind.Network.Contract.P2P; using Nethermind.Specs.ChainSpecStyle; using System.Threading.Tasks; @@ -26,15 +25,4 @@ public Task Init(INethermindApi nethermindApi) _nethermindApi = nethermindApi; return Task.CompletedTask; } - - public Task InitNetworkProtocol() - { - // Remove default ETH 68 capability (XDC uses 62-63 and 100) - _nethermindApi.ProtocolsManager!.RemoveSupportedCapability(new(Protocol.Eth, 68)); - - _nethermindApi.ProtocolsManager!.AddSupportedCapability(new(Protocol.Eth, 62)); - _nethermindApi.ProtocolsManager!.AddSupportedCapability(new(Protocol.Eth, 63)); - _nethermindApi.ProtocolsManager!.AddSupportedCapability(new(Protocol.Eth, 100)); - return Task.CompletedTask; - } } diff --git a/src/Nethermind/Nethermind.Xdc/XdcSubnetPlugin.cs b/src/Nethermind/Nethermind.Xdc/XdcSubnetPlugin.cs index 5c35a6e5179f..3e006bf17ac4 100644 --- a/src/Nethermind/Nethermind.Xdc/XdcSubnetPlugin.cs +++ b/src/Nethermind/Nethermind.Xdc/XdcSubnetPlugin.cs @@ -22,6 +22,4 @@ public class XdcSubnetPlugin(ChainSpec chainSpec) : IConsensusPlugin public IModule Module => new XdcSubnetModule(); public Task Init(INethermindApi nethermindApi) => _xdcPlugin.Init(nethermindApi); - - public Task InitNetworkProtocol() => _xdcPlugin.InitNetworkProtocol(); } From 5c38d709a6266a2d1888788aa26b06ba64692375 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Sat, 20 Jun 2026 23:02:52 +0800 Subject: [PATCH 06/12] refactor(network): make the default eth/68 capability a resolver Address review: move the hardcoded DefaultCapabilities seed out of ProtocolsManager into a DefaultP2PCapabilityResolver (registered AddFirst so chain-specific resolvers still compose over it). GetAdvertisedCapabilities now runs all resolvers over an empty set. Also drop a stray comment in PseudoNetworkModule. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Modules/PseudoNetworkModule.cs | 2 -- .../Nethermind.Init/Modules/NetworkModule.cs | 1 + .../Modules/TestRpcBlockchain.cs | 2 +- .../ProtocolsManagerTests.cs | 4 +-- .../DefaultP2PCapabilityResolver.cs | 31 +++++++++++++++++++ .../P2P/P2PProtocolInfoProvider.cs | 2 +- .../Nethermind.Network/ProtocolsManager.cs | 14 +++------ 7 files changed, 41 insertions(+), 15 deletions(-) create mode 100644 src/Nethermind/Nethermind.Network/DefaultP2PCapabilityResolver.cs diff --git a/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs b/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs index 8421c6f14dd7..8109492c84e8 100644 --- a/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs +++ b/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs @@ -18,8 +18,6 @@ protected override void Load(ContainerBuilder builder) builder .AddSingleton(Policy.FullGossip) - // Snap capability is contributed by SnapP2PCapabilityResolver, registered in the production NetworkModule. - // Some config migration .AddDecorator((ctx, networkConfig) => { diff --git a/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs b/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs index 90c38c3de488..3e1cc752a42d 100644 --- a/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs +++ b/src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs @@ -67,6 +67,7 @@ protected override void Load(ContainerBuilder builder) .AddSingleton() .AddSingleton() .AddSingleton() + .AddFirst() .AddLast() // Handshake diff --git a/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs b/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs index 6319705cd62d..8d7d2b4f3ab5 100644 --- a/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs +++ b/src/Nethermind/Nethermind.JsonRpc.Test/Modules/TestRpcBlockchain.cs @@ -233,7 +233,7 @@ await base.Build(builder => Substitute.For(), Substitute.For(), Array.Empty(), - Array.Empty(), + [new DefaultP2PCapabilityResolver()], LimboLogs.Instance ); diff --git a/src/Nethermind/Nethermind.Network.Test/ProtocolsManagerTests.cs b/src/Nethermind/Nethermind.Network.Test/ProtocolsManagerTests.cs index 65286bd3d5f1..e2e020208088 100644 --- a/src/Nethermind/Nethermind.Network.Test/ProtocolsManagerTests.cs +++ b/src/Nethermind/Nethermind.Network.Test/ProtocolsManagerTests.cs @@ -124,7 +124,7 @@ private static ProtocolsManager BuildManagerWithResolvers(params IP2PCapabilityR Substitute.For(), Substitute.For(), [], - resolvers, + [new DefaultP2PCapabilityResolver(), .. resolvers], LimboLogs.Instance); private sealed class FakeCapabilityResolver(Action> resolve) : IP2PCapabilityResolver @@ -218,7 +218,7 @@ public Context() _protocolValidator, _peerStorage, BuildProtocolHandlerFactories(), - [], + [new DefaultP2PCapabilityResolver()], LimboLogs.Instance); } diff --git a/src/Nethermind/Nethermind.Network/DefaultP2PCapabilityResolver.cs b/src/Nethermind/Nethermind.Network/DefaultP2PCapabilityResolver.cs new file mode 100644 index 000000000000..9007ef82c908 --- /dev/null +++ b/src/Nethermind/Nethermind.Network/DefaultP2PCapabilityResolver.cs @@ -0,0 +1,31 @@ +// SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System; +using System.Collections.Generic; +using Nethermind.Network.Contract.P2P; +using Nethermind.Stats.Model; + +namespace Nethermind.Network; + +/// +/// Contributes the capabilities every node advertises by default (currently eth/68). +/// +/// +/// Registered first so that chain-specific resolvers (e.g. XDC) can remove or build on the default set. +/// +public class DefaultP2PCapabilityResolver : IP2PCapabilityResolver +{ + public static readonly IReadOnlyList DefaultCapabilities = [new(Protocol.Eth, 68)]; + + // The default set is static, so the cache never needs invalidating. + public event Action? Changed { add { } remove { } } + + public void Resolve(ISet capabilities) + { + for (int i = 0; i < DefaultCapabilities.Count; i++) + { + capabilities.Add(DefaultCapabilities[i]); + } + } +} diff --git a/src/Nethermind/Nethermind.Network/P2P/P2PProtocolInfoProvider.cs b/src/Nethermind/Nethermind.Network/P2P/P2PProtocolInfoProvider.cs index c44d9b9293ee..a50a1e5bdf4c 100644 --- a/src/Nethermind/Nethermind.Network/P2P/P2PProtocolInfoProvider.cs +++ b/src/Nethermind/Nethermind.Network/P2P/P2PProtocolInfoProvider.cs @@ -10,7 +10,7 @@ public static class P2PProtocolInfoProvider { public static string DefaultCapabilitiesToString() { - IEnumerable capabilities = ProtocolsManager.DefaultCapabilities + IEnumerable capabilities = DefaultP2PCapabilityResolver.DefaultCapabilities .OrderBy(static x => x.ProtocolCode).ThenByDescending(static x => x.Version) .Select(static x => $"{x.ProtocolCode}/{x.Version}"); return string.Join(",", capabilities); diff --git a/src/Nethermind/Nethermind.Network/ProtocolsManager.cs b/src/Nethermind/Nethermind.Network/ProtocolsManager.cs index aa0c2e40f948..a709b7beb4ce 100644 --- a/src/Nethermind/Nethermind.Network/ProtocolsManager.cs +++ b/src/Nethermind/Nethermind.Network/ProtocolsManager.cs @@ -26,11 +26,6 @@ namespace Nethermind.Network { public class ProtocolsManager : IProtocolsManager, IProtocolRegistrar { - public static readonly IEnumerable DefaultCapabilities = new Capability[] - { - new(Protocol.Eth, 68), - }; - private readonly ConcurrentDictionary _syncPeers = new(); private readonly ConcurrentDictionary> _hangingSatelliteProtocols = new(); private readonly ISyncPeerPool _syncPool; @@ -371,9 +366,10 @@ private void InvalidateCapabilities() } /// - /// Returns the capabilities to advertise, computed from and the registered - /// s. The result is cached and only recomputed when a resolver signals a - /// change via , keeping the per-session path allocation-free. + /// Returns the capabilities to advertise, computed by running the registered + /// s (including ) over an empty + /// set. The result is cached and only recomputed when a resolver signals a change via + /// , keeping the per-session path allocation-free. /// private Capability[] GetAdvertisedCapabilities() { @@ -384,7 +380,7 @@ private Capability[] GetAdvertisedCapabilities() { if (_cachedCapabilities is null) { - HashSet capabilities = DefaultCapabilities.ToHashSet(); + HashSet capabilities = []; foreach (IP2PCapabilityResolver resolver in _capabilityResolvers) { resolver.Resolve(capabilities); From 65e8442e6fe6b90548645dc74792fbf951f9ebed Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Mon, 22 Jun 2026 22:43:10 +0800 Subject: [PATCH 07/12] refactor(network): drop static DefaultCapabilities, log resolved caps Collapse DefaultP2PCapabilityResolver.Resolve to a single Add and remove the now-unused static DefaultCapabilities list; P2PProtocolInfoProvider derives the default set by running the resolver over an empty set. Log the resolved advertised capability set at debug in ProtocolsManager. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Nethermind.Network/DefaultP2PCapabilityResolver.cs | 10 +--------- .../Nethermind.Network/P2P/P2PProtocolInfoProvider.cs | 7 +++++-- src/Nethermind/Nethermind.Network/ProtocolsManager.cs | 4 +++- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/Nethermind/Nethermind.Network/DefaultP2PCapabilityResolver.cs b/src/Nethermind/Nethermind.Network/DefaultP2PCapabilityResolver.cs index 9007ef82c908..7d07f58e9ffc 100644 --- a/src/Nethermind/Nethermind.Network/DefaultP2PCapabilityResolver.cs +++ b/src/Nethermind/Nethermind.Network/DefaultP2PCapabilityResolver.cs @@ -16,16 +16,8 @@ namespace Nethermind.Network; /// public class DefaultP2PCapabilityResolver : IP2PCapabilityResolver { - public static readonly IReadOnlyList DefaultCapabilities = [new(Protocol.Eth, 68)]; - // The default set is static, so the cache never needs invalidating. public event Action? Changed { add { } remove { } } - public void Resolve(ISet capabilities) - { - for (int i = 0; i < DefaultCapabilities.Count; i++) - { - capabilities.Add(DefaultCapabilities[i]); - } - } + public void Resolve(ISet capabilities) => capabilities.Add(new Capability(Protocol.Eth, 68)); } diff --git a/src/Nethermind/Nethermind.Network/P2P/P2PProtocolInfoProvider.cs b/src/Nethermind/Nethermind.Network/P2P/P2PProtocolInfoProvider.cs index a50a1e5bdf4c..f02d03b4af8f 100644 --- a/src/Nethermind/Nethermind.Network/P2P/P2PProtocolInfoProvider.cs +++ b/src/Nethermind/Nethermind.Network/P2P/P2PProtocolInfoProvider.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; +using Nethermind.Stats.Model; namespace Nethermind.Network.P2P { @@ -10,10 +11,12 @@ public static class P2PProtocolInfoProvider { public static string DefaultCapabilitiesToString() { - IEnumerable capabilities = DefaultP2PCapabilityResolver.DefaultCapabilities + HashSet capabilities = []; + new DefaultP2PCapabilityResolver().Resolve(capabilities); + IEnumerable formatted = capabilities .OrderBy(static x => x.ProtocolCode).ThenByDescending(static x => x.Version) .Select(static x => $"{x.ProtocolCode}/{x.Version}"); - return string.Join(",", capabilities); + return string.Join(",", formatted); } } } diff --git a/src/Nethermind/Nethermind.Network/ProtocolsManager.cs b/src/Nethermind/Nethermind.Network/ProtocolsManager.cs index a709b7beb4ce..f31cde220769 100644 --- a/src/Nethermind/Nethermind.Network/ProtocolsManager.cs +++ b/src/Nethermind/Nethermind.Network/ProtocolsManager.cs @@ -386,7 +386,9 @@ private Capability[] GetAdvertisedCapabilities() resolver.Resolve(capabilities); } - Volatile.Write(ref _cachedCapabilities, capabilities.ToArray()); + Capability[] resolved = capabilities.ToArray(); + if (_logger.IsDebug) _logger.Debug($"Resolved advertised P2P capabilities: {string.Join(", ", resolved.Select(static c => $"{c.ProtocolCode}/{c.Version}"))}"); + Volatile.Write(ref _cachedCapabilities, resolved); } return _cachedCapabilities; From 0d4266b6f73b211bc67986f0006332361be335a7 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 23 Jun 2026 19:32:06 +0800 Subject: [PATCH 08/12] refactor(network): address review on P2P capability resolvers - SnapP2PCapabilityResolver fires Changed only when snap/1's contribution actually flips (Full-mode bit) instead of on every sync-mode transition, and logs the transition (restores the visibility SnapCapabilitySwitcher had). - ProtocolsManager: null-forgiving return for the cached array and ArgumentNullException guards for the factories/capabilityResolvers arrays, consistent with the other ctor params. - E2ESyncTests: register PostMergeCapabilitiesResolver by type instead of a factory lambda, per the DI rules. - Reword the now-stale 'capabilities must be finalized' comment in InitializeNetwork. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Steps/InitializeNetwork.cs | 2 +- .../SnapP2PCapabilityResolverTests.cs | 18 +++++++++++------- .../Nethermind.Network/ProtocolsManager.cs | 6 +++--- .../SnapP2PCapabilityResolver.cs | 19 +++++++++++++++++-- .../E2ESyncTests.cs | 2 +- 5 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs b/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs index a071ef59b2e8..24250c752367 100644 --- a/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs +++ b/src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs @@ -261,7 +261,7 @@ protected virtual async Task InitPeer() await plugin.InitNetworkProtocol(); } - // Capabilities must be finalized before the RLPx listener accepts peers. Otherwise + // Capabilities must be resolved before the RLPx listener accepts peers. Otherwise // early sessions can negotiate only the default ETH version and never upgrade. await _rlpxPeer.Init(); diff --git a/src/Nethermind/Nethermind.Network.Test/SnapP2PCapabilityResolverTests.cs b/src/Nethermind/Nethermind.Network.Test/SnapP2PCapabilityResolverTests.cs index 3e23090aece2..b451ed1820b5 100644 --- a/src/Nethermind/Nethermind.Network.Test/SnapP2PCapabilityResolverTests.cs +++ b/src/Nethermind/Nethermind.Network.Test/SnapP2PCapabilityResolverTests.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using Nethermind.Blockchain.Synchronization; +using Nethermind.Logging; using Nethermind.Network.Contract.P2P; using Nethermind.Stats.Model; using Nethermind.Synchronization.ParallelSync; @@ -23,7 +24,7 @@ public void Resolve_advertises_snap_when_serving_or_syncing_state(bool snapServi ISyncConfig syncConfig = new SyncConfig { SnapServingEnabled = snapServing, SnapSync = snapSync }; ISyncModeSelector syncModeSelector = Substitute.For(); syncModeSelector.Current.Returns(currentMode); - using SnapP2PCapabilityResolver resolver = new(syncConfig, syncModeSelector); + using SnapP2PCapabilityResolver resolver = new(syncConfig, syncModeSelector, LimboLogs.Instance); HashSet capabilities = []; resolver.Resolve(capabilities); @@ -31,18 +32,21 @@ public void Resolve_advertises_snap_when_serving_or_syncing_state(bool snapServi Assert.That(capabilities.Contains(new Capability(Protocol.Snap, 1)), Is.EqualTo(expected)); } - [Test] - public void Raises_Changed_on_sync_mode_change() + [TestCase(false, true, SyncMode.StateNodes, SyncMode.Full, true, TestName = "Snap sync finishing flips snap and fires Changed")] + [TestCase(false, true, SyncMode.StateNodes, SyncMode.FastBlocks, false, TestName = "Non-Full to non-Full leaves snap unchanged")] + [TestCase(true, true, SyncMode.StateNodes, SyncMode.Full, false, TestName = "Snap serving keeps snap constant across sync modes")] + [TestCase(false, false, SyncMode.StateNodes, SyncMode.Full, false, TestName = "No snap sync means no snap to toggle")] + public void Raises_Changed_only_when_snap_contribution_flips(bool snapServing, bool snapSync, SyncMode previous, SyncMode current, bool expectedFired) { - ISyncConfig syncConfig = new SyncConfig(); + ISyncConfig syncConfig = new SyncConfig { SnapServingEnabled = snapServing, SnapSync = snapSync }; ISyncModeSelector syncModeSelector = Substitute.For(); - using SnapP2PCapabilityResolver resolver = new(syncConfig, syncModeSelector); + using SnapP2PCapabilityResolver resolver = new(syncConfig, syncModeSelector, LimboLogs.Instance); bool changed = false; resolver.Changed += () => changed = true; - syncModeSelector.Changed += Raise.EventWith(new SyncModeChangedEventArgs(SyncMode.StateNodes, SyncMode.Full)); + syncModeSelector.Changed += Raise.EventWith(new SyncModeChangedEventArgs(previous, current)); - Assert.That(changed, Is.True); + Assert.That(changed, Is.EqualTo(expectedFired)); } } diff --git a/src/Nethermind/Nethermind.Network/ProtocolsManager.cs b/src/Nethermind/Nethermind.Network/ProtocolsManager.cs index f31cde220769..8420903d3530 100644 --- a/src/Nethermind/Nethermind.Network/ProtocolsManager.cs +++ b/src/Nethermind/Nethermind.Network/ProtocolsManager.cs @@ -65,8 +65,8 @@ public ProtocolsManager( _logger = logManager?.GetClassLogger() ?? throw new ArgumentNullException(nameof(logManager)); // Order is already set by OrderedComponents (AddFirst/AddLast) - _factories = factories; - _capabilityResolvers = capabilityResolvers; + _factories = factories ?? throw new ArgumentNullException(nameof(factories)); + _capabilityResolvers = capabilityResolvers ?? throw new ArgumentNullException(nameof(capabilityResolvers)); foreach (IP2PCapabilityResolver resolver in _capabilityResolvers) { resolver.Changed += InvalidateCapabilities; @@ -391,7 +391,7 @@ private Capability[] GetAdvertisedCapabilities() Volatile.Write(ref _cachedCapabilities, resolved); } - return _cachedCapabilities; + return _cachedCapabilities!; } } } diff --git a/src/Nethermind/Nethermind.Network/SnapP2PCapabilityResolver.cs b/src/Nethermind/Nethermind.Network/SnapP2PCapabilityResolver.cs index 937dbd1da42d..d0cdcb47eb0f 100644 --- a/src/Nethermind/Nethermind.Network/SnapP2PCapabilityResolver.cs +++ b/src/Nethermind/Nethermind.Network/SnapP2PCapabilityResolver.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using Nethermind.Blockchain.Synchronization; +using Nethermind.Logging; using Nethermind.Network.Contract.P2P; using Nethermind.Stats.Model; using Nethermind.Synchronization.ParallelSync; @@ -24,13 +25,15 @@ public class SnapP2PCapabilityResolver : IP2PCapabilityResolver, IDisposable private readonly ISyncConfig _syncConfig; private readonly ISyncModeSelector _syncModeSelector; + private readonly ILogger _logger; public event Action? Changed; - public SnapP2PCapabilityResolver(ISyncConfig syncConfig, ISyncModeSelector syncModeSelector) + public SnapP2PCapabilityResolver(ISyncConfig syncConfig, ISyncModeSelector syncModeSelector, ILogManager logManager) { _syncConfig = syncConfig; _syncModeSelector = syncModeSelector; + _logger = logManager.GetClassLogger(); _syncModeSelector.Changed += OnSyncModeChanged; } @@ -44,7 +47,19 @@ public void Resolve(ISet capabilities) } } - private void OnSyncModeChanged(object? sender, SyncModeChangedEventArgs e) => Changed?.Invoke(); + private void OnSyncModeChanged(object? sender, SyncModeChangedEventArgs e) + { + // snap/1's contribution only tracks the sync mode while we snap-sync our own state and are not also + // serving snap; in every other configuration it is constant, so the rebuild is pointless. + if (_syncConfig.SnapServingEnabled == true || !_syncConfig.SnapSync) return; + + bool wasSyncing = (e.Previous & SyncMode.Full) == 0; + bool isSyncing = (e.Current & SyncMode.Full) == 0; + if (wasSyncing == isSyncing) return; + + if (_logger.IsDebug) _logger.Debug($"State sync {(isSyncing ? "in progress" : "finished")}; snap/1 advertisement {(isSyncing ? "enabled" : "disabled")}"); + Changed?.Invoke(); + } public void Dispose() => _syncModeSelector.Changed -= OnSyncModeChanged; } diff --git a/src/Nethermind/Nethermind.Synchronization.Test/E2ESyncTests.cs b/src/Nethermind/Nethermind.Synchronization.Test/E2ESyncTests.cs index 3f2a40515504..7693bfde8c42 100644 --- a/src/Nethermind/Nethermind.Synchronization.Test/E2ESyncTests.cs +++ b/src/Nethermind/Nethermind.Synchronization.Test/E2ESyncTests.cs @@ -307,7 +307,7 @@ private async Task CreateNode( .AddModule(new TestMergeModule(configProvider.GetConfig())) .AddSingleton(timestamper) // Used by test code .AddDecorator() - .AddLast(_ => new PostMergeCapabilitiesResolver()) + .AddLast() ; } else From 6c286057d0a29a319b2b711f2bbf344ce161f3d1 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Tue, 23 Jun 2026 19:32:15 +0800 Subject: [PATCH 09/12] refactor(xdc): drop default eth/68 resolver via DI instead of removing at resolve Per review (LukaszRozmej): rather than XdcP2PCapabilityResolver imperatively removing eth/68 from the resolved set, deregister the DefaultP2PCapabilityResolver. Adds a RemoveOrderedComponents() DSL (alongside ClearOrderedComponents) backed by OrderedComponents.RemoveAll. XdcModule now drops the default resolver after registering the XDC one, so XdcP2PCapabilityResolver only contributes eth/62, eth/63 and eth/100. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Container/OrderedComponentsTests.cs | 15 +++++++++++++++ .../Container/OrderedComponents.cs | 5 ++++- ...deredComponentsContainerBuilderExtensions.cs | 17 +++++++++++++++++ .../XdcP2PCapabilityResolverTests.cs | 4 ++-- src/Nethermind/Nethermind.Xdc/XdcModule.cs | 1 + .../Nethermind.Xdc/XdcP2PCapabilityResolver.cs | 4 ++-- 6 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/Nethermind/Nethermind.Core.Test/Container/OrderedComponentsTests.cs b/src/Nethermind/Nethermind.Core.Test/Container/OrderedComponentsTests.cs index 4f490effa6a1..0ff3ba57978c 100644 --- a/src/Nethermind/Nethermind.Core.Test/Container/OrderedComponentsTests.cs +++ b/src/Nethermind/Nethermind.Core.Test/Container/OrderedComponentsTests.cs @@ -40,6 +40,20 @@ public void TestAddFirst() Assert.That(ctx.Resolve().Select(item => item.Name), Is.EqualTo(["1", "2", "3"])); } + [Test] + public void TestRemove() + { + // RemoveOrderedComponents drops every component of the given concrete type, keeping the rest in order + using IContainer ctx = new ContainerBuilder() + .AddLast(_ => new Item("1")) + .AddLast(_ => new OtherItem("2")) + .AddLast(_ => new Item("3")) + .RemoveOrderedComponents() + .Build(); + + Assert.That(ctx.Resolve().Select(item => item.Name), Is.EqualTo(["2"])); + } + [Test] public void TestDisallowIndividualRegistration() { @@ -112,6 +126,7 @@ protected override void Load(ContainerBuilder builder) => } private interface IItem { string Name { get; } } private record Item(string Name) : IItem; + private record OtherItem(string Name) : IItem; private class CompositeItem(IItem[] items) : IItem { public IItem[] Items { get; } = items; diff --git a/src/Nethermind/Nethermind.Core/Container/OrderedComponents.cs b/src/Nethermind/Nethermind.Core/Container/OrderedComponents.cs index 35d74bd14e42..d74c252f962d 100644 --- a/src/Nethermind/Nethermind.Core/Container/OrderedComponents.cs +++ b/src/Nethermind/Nethermind.Core/Container/OrderedComponents.cs @@ -1,18 +1,21 @@ // SPDX-FileCopyrightText: 2025 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only +using System; using System.Collections.Generic; namespace Nethermind.Core.Container; public class OrderedComponents { - private IList _components = []; + private readonly List _components = []; public IEnumerable Components => _components; public void AddLast(T item) => _components.Add(item); public void AddFirst(T item) => _components.Insert(0, item); + public void RemoveAll(Predicate match) => _components.RemoveAll(match); + public void Clear() => _components.Clear(); } diff --git a/src/Nethermind/Nethermind.Core/Container/OrderedComponentsContainerBuilderExtensions.cs b/src/Nethermind/Nethermind.Core/Container/OrderedComponentsContainerBuilderExtensions.cs index 8a9b10be21cf..fd003a3c441c 100644 --- a/src/Nethermind/Nethermind.Core/Container/OrderedComponentsContainerBuilderExtensions.cs +++ b/src/Nethermind/Nethermind.Core/Container/OrderedComponentsContainerBuilderExtensions.cs @@ -76,6 +76,23 @@ public static ContainerBuilder AddCompositeOrderedComponents(this return builder; } + /// + /// Remove all previously registered ordered components of type for . + /// Useful when a plugin replaces a default component instead of layering on top of it + /// (e.g., XDC dropping the default eth/68 capability resolver). + /// + /// + /// Like , this relies on the removing decorator being registered + /// after the component it targets, which holds when a plugin module loads after the core module that + /// registered the default. + /// + public static ContainerBuilder RemoveOrderedComponents(this ContainerBuilder builder) where TImpl : T => + builder.AddDecorator>((_, orderedComponents) => + { + orderedComponents.RemoveAll(static item => item is TImpl); + return orderedComponents; + }); + /// /// Clear all previously registered ordered components for . /// Useful when a plugin needs to disable all ordered policies (e.g., Hive). diff --git a/src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs b/src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs index abc6c1bb5aec..781ec569d228 100644 --- a/src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs +++ b/src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs @@ -12,11 +12,11 @@ namespace Nethermind.Xdc.Test; public class XdcP2PCapabilityResolverTests { [Test] - public void Resolve_replaces_default_eth68_with_xdc_capabilities() + public void Resolve_advertises_xdc_capabilities() { XdcP2PCapabilityResolver resolver = new(); - HashSet capabilities = [new(Protocol.Eth, 68)]; + HashSet capabilities = []; resolver.Resolve(capabilities); Assert.That(capabilities, Is.EquivalentTo(new[] diff --git a/src/Nethermind/Nethermind.Xdc/XdcModule.cs b/src/Nethermind/Nethermind.Xdc/XdcModule.cs index 80c3cd70641e..782832b23743 100644 --- a/src/Nethermind/Nethermind.Xdc/XdcModule.cs +++ b/src/Nethermind/Nethermind.Xdc/XdcModule.cs @@ -132,6 +132,7 @@ protected override void Load(ContainerBuilder builder) .AddLast() .AddLast() + .RemoveOrderedComponents() .AddSingleton() // block processing diff --git a/src/Nethermind/Nethermind.Xdc/XdcP2PCapabilityResolver.cs b/src/Nethermind/Nethermind.Xdc/XdcP2PCapabilityResolver.cs index 69bc4f9033b6..42b47a1affcd 100644 --- a/src/Nethermind/Nethermind.Xdc/XdcP2PCapabilityResolver.cs +++ b/src/Nethermind/Nethermind.Xdc/XdcP2PCapabilityResolver.cs @@ -10,7 +10,8 @@ namespace Nethermind.Xdc; /// -/// XDC advertises eth/62, eth/63 and eth/100 instead of the default eth/68. +/// XDC advertises eth/62, eth/63 and eth/100. The default eth/68 resolver is dropped at registration +/// (see XdcModule), so this resolver only contributes the XDC-specific versions. /// public class XdcP2PCapabilityResolver : IP2PCapabilityResolver { @@ -19,7 +20,6 @@ public event Action? Changed { add { } remove { } } public void Resolve(ISet capabilities) { - capabilities.Remove(new Capability(Protocol.Eth, 68)); capabilities.Add(new Capability(Protocol.Eth, 62)); capabilities.Add(new Capability(Protocol.Eth, 63)); capabilities.Add(new Capability(Protocol.Eth, 100)); From 5ab5f314da76767e924570a374315ac11d120592 Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Wed, 24 Jun 2026 10:05:06 +0800 Subject: [PATCH 10/12] fix(merge): advertise post-merge eth on terminal block, not only transition-finished On a live TTD transition, PoSSwitcher fires TerminalBlockReached while TransitionFinished is still false; the flag only flips true later in ForkchoiceUpdated, which raises no event. The resolver invalidated its cache on TerminalBlockReached but Resolve re-gated on TransitionFinished, so the rebuild still excluded eth/69-71 and they were never advertised until a restart. Gate on TransitionFinished || HasEverReachedTerminalBlock(), mirroring the pre-resolver behaviour (which added the capabilities unconditionally on TerminalBlockReached). Reported by AnkushinDaniil in review of #12093. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../MergeP2PCapabilityResolverTests.cs | 9 ++++++--- .../MergeP2PCapabilityResolver.cs | 9 +++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/MergeP2PCapabilityResolverTests.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/MergeP2PCapabilityResolverTests.cs index 54a7ca3430d4..a8c9ced70414 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/MergeP2PCapabilityResolverTests.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/MergeP2PCapabilityResolverTests.cs @@ -13,12 +13,15 @@ namespace Nethermind.Merge.Plugin.Test; [Parallelizable(ParallelScope.All)] public class MergeP2PCapabilityResolverTests { - [TestCase(true, true)] - [TestCase(false, false)] - public void Resolve_advertises_post_merge_eth_capabilities_only_after_transition(bool transitionFinished, bool expected) + [TestCase(false, false, false, TestName = "Pre-merge: not advertised")] + [TestCase(true, false, true, TestName = "Transition finished (post-merge restart): advertised")] + [TestCase(false, true, true, TestName = "Live merge: terminal block reached before transition finished: advertised")] + [TestCase(true, true, true, TestName = "Both: advertised")] + public void Resolve_advertises_post_merge_eth_capabilities_once_post_merge(bool transitionFinished, bool hasReachedTerminalBlock, bool expected) { IPoSSwitcher poSSwitcher = Substitute.For(); poSSwitcher.TransitionFinished.Returns(transitionFinished); + poSSwitcher.HasEverReachedTerminalBlock().Returns(hasReachedTerminalBlock); using MergeP2PCapabilityResolver resolver = new(poSSwitcher); HashSet capabilities = []; diff --git a/src/Nethermind/Nethermind.Merge.Plugin/MergeP2PCapabilityResolver.cs b/src/Nethermind/Nethermind.Merge.Plugin/MergeP2PCapabilityResolver.cs index edf6c6fc4da4..f5aac403231e 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/MergeP2PCapabilityResolver.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/MergeP2PCapabilityResolver.cs @@ -11,7 +11,8 @@ namespace Nethermind.Merge.Plugin; /// -/// Advertises the post-merge eth/69, eth/70 and eth/71 capabilities once the merge transition has finished. +/// Advertises the post-merge eth/69, eth/70 and eth/71 capabilities once the node is operating post-merge — +/// i.e. the merge transition has finished or the terminal PoW block has been reached. /// public class MergeP2PCapabilityResolver : IP2PCapabilityResolver, IDisposable { @@ -27,7 +28,11 @@ public MergeP2PCapabilityResolver(IPoSSwitcher poSSwitcher) public void Resolve(ISet capabilities) { - if (!_poSSwitcher.TransitionFinished) return; + // TransitionFinished alone is insufficient on a live TTD transition: it only flips true later in + // PoSSwitcher.ForkchoiceUpdated, which raises no event, so the TerminalBlockReached-driven cache + // rebuild would still see it false. HasEverReachedTerminalBlock() (set when TerminalBlockReached + // fires) mirrors the pre-resolver behaviour of advertising as soon as the terminal block is reached. + if (!_poSSwitcher.TransitionFinished && !_poSSwitcher.HasEverReachedTerminalBlock()) return; capabilities.Add(new Capability(Protocol.Eth, 69)); capabilities.Add(new Capability(Protocol.Eth, 70)); From 37dbf86d6d9b872926517b41e86404bc755e66cd Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Wed, 24 Jun 2026 10:05:06 +0800 Subject: [PATCH 11/12] test(xdc): assert DI composition drops the default eth/68 resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a container-level test mirroring NetworkModule (AddFirst default) + XdcModule (AddLast xdc, then RemoveOrderedComponents default), verifying the resolved capability set excludes eth/68 — not just the resolver in isolation. Reported by AnkushinDaniil in review of #12093. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../XdcP2PCapabilityResolverTests.cs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs b/src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs index 781ec569d228..d9dc5d2f5ba9 100644 --- a/src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs +++ b/src/Nethermind/Nethermind.Xdc.Test/XdcP2PCapabilityResolverTests.cs @@ -2,6 +2,9 @@ // SPDX-License-Identifier: LGPL-3.0-only using System.Collections.Generic; +using Autofac; +using Nethermind.Core.Container; +using Nethermind.Network; using Nethermind.Network.Contract.P2P; using Nethermind.Stats.Model; using NUnit.Framework; @@ -26,4 +29,29 @@ public void Resolve_advertises_xdc_capabilities() new Capability(Protocol.Eth, 100), })); } + + [Test] + public void Di_composition_drops_default_eth68_resolver() + { + // Mirrors NetworkModule (AddFirst default) followed by XdcModule (AddLast xdc, then deregister default). + using IContainer container = new ContainerBuilder() + .AddFirst() + .AddLast() + .RemoveOrderedComponents() + .Build(); + + IP2PCapabilityResolver[] resolvers = container.Resolve(); + Assert.That(resolvers, Has.None.TypeOf()); + + HashSet capabilities = []; + foreach (IP2PCapabilityResolver resolver in resolvers) resolver.Resolve(capabilities); + + // eth/68 (the default's contribution) is gone; only XDC's versions remain. + Assert.That(capabilities, Is.EquivalentTo(new[] + { + new Capability(Protocol.Eth, 62), + new Capability(Protocol.Eth, 63), + new Capability(Protocol.Eth, 100), + })); + } } From 26ce958a47e177f0912c8fb75352c011671a492f Mon Sep 17 00:00:00 2001 From: Amirul Ashraf Date: Wed, 24 Jun 2026 10:05:06 +0800 Subject: [PATCH 12/12] fix(lint): remove unused using in PseudoNetworkModule IDE0005: 'using Nethermind.Network' became dead when the imperative ProtocolsManager snap-capability registration was removed; NetworkDiagTracer resolves via the enclosing Nethermind.Core namespace and Network.Metrics via the Nethermind root. Fixes the 'Check code lint' CI failure. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Nethermind.Core.Test/Modules/PseudoNetworkModule.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs b/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs index 8109492c84e8..bcb0fd208b37 100644 --- a/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs +++ b/src/Nethermind/Nethermind.Core.Test/Modules/PseudoNetworkModule.cs @@ -4,7 +4,6 @@ using Autofac; using Nethermind.Consensus; using Nethermind.Logging; -using Nethermind.Network; using Nethermind.Network.Config; namespace Nethermind.Core.Test.Modules;