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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .agents/rules/di-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,8 @@ IContainer container = new ContainerBuilder()
```

Never add test-specific code to production modules. Overrides belong in `TestEnvironmentModule`, `TestBlockProcessingModule`, or a new test module passed to `Build`.

## Anti-pattern
- Using the form `.Add<IFoo>(ctx => new Foo(ctx.Resolve<Dep1>(), ctx.Resolve<Dep2>()))` is an anti-pattern. It will cause changes to the wiring when `Foo` adds new dependencies, which increases review load.
- Rather, do `.Add<IFoo, Foo>()` unless unavoidable.
- Similar case with using `IComponentContext` within a class that is not related to component wiring. Rather, pass in the dependency to the construct either directly as a type, or as a `Func<IFoo>` if multiple instantiation is needed or `Lazy<IFoo>` if lazy initialization is needed.
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.Api/IApiWithBlockchain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public interface IApiWithBlockchain : IApiWithStores
ITxSender? TxSender { get; set; }
INonceManager? NonceManager { get; set; }
ITxPool? TxPool { get; set; }
CompositeTxGossipPolicy TxGossipPolicy { get; }

ITransactionComparerProvider? TransactionComparerProvider { get; set; }

[SkipServiceCollection]
Expand Down
2 changes: 1 addition & 1 deletion src/Nethermind/Nethermind.Api/NethermindApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ ILifetimeScope Context
public IDisposableStack DisposeStack => Context.Resolve<IDisposableStack>();
public IReadOnlyList<INethermindPlugin> Plugins => _dependencies.Plugins;
public IProcessExitSource ProcessExit => _dependencies.ProcessExitSource;
public CompositeTxGossipPolicy TxGossipPolicy { get; } = new();

public ILifetimeScope Context => _dependencies.Context;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@

namespace Nethermind.Consensus.AuRa.InitializationSteps;

public class InitializeBlockchainAuRa(AuRaNethermindApi api, IChainHeadInfoProvider chainHeadInfoProvider)
: InitializeBlockchain(api, chainHeadInfoProvider)
public class InitializeBlockchainAuRa(AuRaNethermindApi api, IChainHeadInfoProvider chainHeadInfoProvider, ITxGossipPolicy txGossipPolicy)
: InitializeBlockchain(api, chainHeadInfoProvider, txGossipPolicy)
{
private INethermindApi NethermindApi => api;

Expand Down Expand Up @@ -98,7 +98,7 @@ protected override TxPool.TxPool CreateTxPool(IChainHeadInfoProvider chainHeadIn
api.TxValidator!,
api.LogManager,
CreateTxPoolTxComparer(txPriorityContract, localDataSource),
api.TxGossipPolicy,
_txGossipPolicy,
new TxFilterAdapter(api.BlockTree, txPoolFilter, api.LogManager, api.SpecProvider),
api.HeadTxValidator,
txPriorityContract is not null || localDataSource is not null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,36 @@ public void TestDisallowIndividualRegistration()
act.Should().Throw<InvalidOperationException>();
}

[Test]
public void TestCompositeResolved_EvenWhenNotLastRegistration()
{
// AddComposite is called before the last AddLast — composite should still be resolved as IItem
using IContainer ctx = new ContainerBuilder()
.AddLast<IItem>(_ => new Item("1"))
.AddCompositeOrderedComponents<IItem, CompositeItem>()
.AddLast<IItem>(_ => new Item("2"))
.Build();

IItem resolved = ctx.Resolve<IItem>();
resolved.Should().BeOfType<CompositeItem>();
((CompositeItem)resolved).Items.Select(i => i.Name).Should().BeEquivalentTo(["1", "2"]);
}

[Test]
public void TestCompositeResolved_WhenCalledBeforeAnyAddLast()
{
// AddComposite is called before any AddLast — should still work
using IContainer ctx = new ContainerBuilder()
.AddCompositeOrderedComponents<IItem, CompositeItem>()
.AddLast<IItem>(_ => new Item("1"))
.AddLast<IItem>(_ => new Item("2"))
.Build();

IItem resolved = ctx.Resolve<IItem>();
resolved.Should().BeOfType<CompositeItem>();
((CompositeItem)resolved).Items.Select(i => i.Name).Should().BeEquivalentTo(["1", "2"]);
}

private class ModuleA : Module
{
protected override void Load(ContainerBuilder builder) =>
Expand All @@ -72,5 +102,11 @@ protected override void Load(ContainerBuilder builder) =>
builder
.AddLast(_ => new Item("1"));
}
private record Item(string Name);
private interface IItem { string Name { get; } }
private record Item(string Name) : IItem;
private class CompositeItem(IItem[] items) : IItem
{
public IItem[] Items { get; } = items;
public string Name => string.Join(",", Items.Select(i => i.Name));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ protected override void Load(ContainerBuilder builder)

.AddSingleton<IProtocolValidator, ProtocolValidator>()
.AddSingleton<IGossipPolicy>(Policy.FullGossip)
.AddComposite<ITxGossipPolicy, CompositeTxGossipPolicy>()

// TODO: LastNStateRootTracker
.AddSingleton<ISnapServer, IWorldStateManager>(stateProvider => stateProvider.SnapServer!)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ protected override void Load(ContainerBuilder builder)
.AddSingleton<IBlockPreprocessorStep, RecoverSignatures>()

.AddSingleton<ITxPool, TxPool.TxPool>()
.AddSingleton<CompositeTxGossipPolicy>()
.AddSingleton<INonceManager, IChainHeadInfoProvider>((chainHeadInfoProvider) => new NonceManager(chainHeadInfoProvider.ReadOnlyStateProvider))

// Seems to be only used by block producer.
Expand Down
5 changes: 5 additions & 0 deletions src/Nethermind/Nethermind.Core/Container/OrderedComponents.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,9 @@ public void AddFirst(T item)
{
_components.Insert(0, item);
}

public void Clear()
{
_components.Clear();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ namespace Nethermind.Core.Container;
/// </summary>
public static class OrderedComponentsContainerBuilderExtensions
{
private const string OrderedMarkerPrefix = "Registered OrderedComponents For ";
private const string CompositeMarkerPrefix = "Registered OrderedComponents Composite For ";

public static ContainerBuilder AddLast<T>(this ContainerBuilder builder, Func<IComponentContext, T> factory) =>
builder
.EnsureOrderedComponents<T>()
Expand All @@ -25,6 +28,11 @@ public static ContainerBuilder AddLast<T>(this ContainerBuilder builder, Func<IC
return orderedComponents;
});

public static ContainerBuilder AddLast<T, TImpl>(this ContainerBuilder builder) where TImpl : class, T =>
builder
.AddLast<T>(ctx => ctx.Resolve<TImpl>())
.Add<TImpl>();

public static ContainerBuilder AddFirst<T>(this ContainerBuilder builder, Func<IComponentContext, T> factory) =>
builder
.EnsureOrderedComponents<T>()
Expand All @@ -34,9 +42,48 @@ public static ContainerBuilder AddFirst<T>(this ContainerBuilder builder, Func<I
return orderedComponents;
});

public static ContainerBuilder AddFirst<T, TImpl>(this ContainerBuilder builder) where TImpl : class, T =>
builder
.AddFirst<T>(ctx => ctx.Resolve<TImpl>())
.Add<TImpl>();

/// <summary>
/// Register a composite type that wraps the ordered components into a single <typeparamref name="T"/> service.
/// Unlike <see cref="ContainerBuilderExtensions.AddComposite{T, TComposite}"/> which uses Autofac's
/// <c>RegisterComposite</c> (collecting direct <typeparamref name="T"/> registrations),
/// this method registers <typeparamref name="TComposite"/> via <c>RegisterType</c> so it receives
/// <c>T[]</c> from <see cref="OrderedComponents{T}"/>. It also relaxes the ordered components
/// safety check to allow this single <typeparamref name="T"/> registration.
/// </summary>
public static ContainerBuilder AddCompositeOrderedComponents<T, TComposite>(this ContainerBuilder builder) where T : class where TComposite : class, T
{
builder.EnsureOrderedComponents<T>();

string compositeMarker = CompositeMarkerPrefix + typeof(T).Name;
if (!builder.Properties.TryAdd(compositeMarker, null))
return builder;

builder.RegisterType<TComposite>()
.As<T>()
.AsSelf();

return builder;
}

/// <summary>
/// Clear all previously registered ordered components for <typeparamref name="T"/>.
/// Useful when a plugin needs to disable all ordered policies (e.g., Hive).
/// </summary>
public static ContainerBuilder ClearOrderedComponents<T>(this ContainerBuilder builder) =>
builder.AddDecorator<OrderedComponents<T>>((_, orderedComponents) =>
{
orderedComponents.Clear();
return orderedComponents;
});

private static ContainerBuilder EnsureOrderedComponents<T>(this ContainerBuilder builder)
{
string registeredMarker = $"Registered OrderedComponents For {typeof(T).Name}";
string registeredMarker = OrderedMarkerPrefix + typeof(T).Name;
if (!builder.Properties.TryAdd(registeredMarker, null))
{
return builder;
Expand All @@ -45,7 +92,11 @@ private static ContainerBuilder EnsureOrderedComponents<T>(this ContainerBuilder
// Prevent registering separately which has no explicit ordering
builder.RegisterBuildCallback(scope =>
{
if (scope.ComponentRegistry.ServiceRegistrationsFor(new TypedService(typeof(T))).Any())
string decoratorMarker = CompositeMarkerPrefix + typeof(T).Name;
bool hasDecorator = builder.Properties.ContainsKey(decoratorMarker);
int registrationCount = scope.ComponentRegistry.ServiceRegistrationsFor(new TypedService(typeof(T))).Count();
int expectedCount = hasDecorator ? 1 : 0;
if (registrationCount > expectedCount)
{
throw new InvalidOperationException(
$"Service of type {typeof(T).Name} must only be registered with one of DSL in {nameof(OrderedComponentsContainerBuilderExtensions)}");
Expand Down
5 changes: 4 additions & 1 deletion src/Nethermind/Nethermind.Hive/HivePlugin.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
using Nethermind.Api.Extensions;
using Nethermind.Api.Steps;
using Nethermind.Core;
using Nethermind.Core.Container;
using Nethermind.TxPool;

namespace Nethermind.Hive;

Expand All @@ -26,5 +28,6 @@ public class HiveModule : Module
{
protected override void Load(ContainerBuilder builder) => builder
.AddSingleton<HiveRunner>()
.AddStep(typeof(HiveStep));
.AddStep(typeof(HiveStep))
.ClearOrderedComponents<ITxGossipPolicy>();
}
3 changes: 0 additions & 3 deletions src/Nethermind/Nethermind.Hive/HiveStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ namespace Nethermind.Hive;
[RunnerStepDependencies(typeof(InitializeNetwork), Optional = true)]
public class HiveStep(
ITxPool txPool,
CompositeTxGossipPolicy txGossipPolicy,
HiveRunner hiveRunner,
IProcessExitSource processExitSource,
ILogManager logManager
Expand All @@ -26,8 +25,6 @@ public async Task Execute(CancellationToken cancellationToken)
{
txPool!.AcceptTxWhenNotSynced = true;

txGossipPolicy.Policies.Clear();

if (_logger.IsInfo) _logger.Info("Hive is starting");

await hiveRunner.Start(processExitSource.Token);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ protected override void Load(ContainerBuilder builder)
.AddSingleton<IHeaderValidator, HeaderValidator>()
.AddSingleton<IUnclesValidator, UnclesValidator>()

.AddLast<ITxGossipPolicy, SpecDrivenTxGossipPolicy>()

// Block processing components common between rpc, validation and production
.AddScoped<ITransactionProcessor.IBlobBaseFeeCalculator, BlobBaseFeeCalculator>()
.AddScoped<ITransactionProcessor, EthereumTransactionProcessor>()
Expand Down
4 changes: 4 additions & 0 deletions src/Nethermind/Nethermind.Init/Modules/NetworkModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
using Nethermind.Network.P2P.ProtocolHandlers;
using Nethermind.Network.Rlpx;
using Nethermind.Stats;
using Nethermind.Synchronization.ParallelSync;
using Nethermind.TxPool;
using Handshake = Nethermind.Network.Rlpx.Handshake;
using P2P = Nethermind.Network.P2P.Messages;
using V62 = Nethermind.Network.P2P.Subprotocols.Eth.V62.Messages;
Expand All @@ -39,6 +41,8 @@ protected override void Load(ContainerBuilder builder)
base.Load(builder);
builder
.AddModule(new SynchronizerModule(configProvider.GetConfig<ISyncConfig>()))
.AddLast<ITxGossipPolicy, SyncedTxGossipPolicy>()
.AddCompositeOrderedComponents<ITxGossipPolicy, CompositeTxGossipPolicy>()
.AddSingleton<IIPResolver, IPResolver>()
.AddSingleton<IForkInfo, ForkInfo>()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ namespace Nethermind.Init.Steps
typeof(SetupKeyStore),
typeof(InitializePrecompiles)
)]
public class InitializeBlockchain(INethermindApi api, IChainHeadInfoProvider chainHeadInfoProvider) : IStep
public class InitializeBlockchain(INethermindApi api, IChainHeadInfoProvider chainHeadInfoProvider, ITxGossipPolicy txGossipPolicy) : IStep
{
private readonly INethermindApi _api = api;
protected readonly ITxGossipPolicy _txGossipPolicy = txGossipPolicy;

public async Task Execute(CancellationToken _)
{
Expand All @@ -52,8 +53,6 @@ protected virtual Task InitBlockchain()
blocksConfig.ExtraData :
"- binary data -");

_api.TxGossipPolicy.Policies.Add(new SpecDrivenTxGossipPolicy(chainHeadInfoProvider));

ITxPool txPool = _api.TxPool = CreateTxPool(chainHeadInfoProvider);

_api.BlockPreprocessor.AddFirst(
Expand Down Expand Up @@ -109,7 +108,7 @@ protected virtual ITxPool CreateTxPool(IChainHeadInfoProvider chainHeadInfoProvi
_api.TxValidator!,
_api.LogManager,
CreateTxPoolTxComparer(),
_api.TxGossipPolicy,
_txGossipPolicy,
null,
_api.HeadTxValidator
);
Expand Down
2 changes: 0 additions & 2 deletions src/Nethermind/Nethermind.Init/Steps/InitializeNetwork.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,6 @@ private async Task Initialize(CancellationToken cancellationToken)
int maxPeersCount = _networkConfig.ActivePeersMaxCount;
Network.Metrics.PeerLimit = maxPeersCount;

_api.TxGossipPolicy.Policies.Add(new SyncedTxGossipPolicy(_api.SyncModeSelector));

if (cancellationToken.IsCancellationRequested)
{
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public enum DisconnectReason : byte
InvalidForkId,
ProtocolInitTimeout,
TxFlooding,
InvalidTxReceived,
NoCapabilityMatched,
ClientFiltered,
AppClosing,
Expand Down Expand Up @@ -107,6 +108,7 @@ public static EthDisconnectReason ToEthDisconnectReason(this DisconnectReason di
DisconnectReason.ReceiveMessageTimeout => EthDisconnectReason.ReceiveMessageTimeout,
DisconnectReason.MultipleHeaderDependencies => EthDisconnectReason.MultipleHeaderDependencies,
DisconnectReason.ConnectionReset => EthDisconnectReason.TcpSubSystemError,
DisconnectReason.InvalidTxReceived => EthDisconnectReason.Other,
_ => EthDisconnectReason.Other,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public void Will_disconnect_on_invalid_tx()
_controller.Report(AcceptTxResult.Invalid);

_session.Received(1)
.InitiateDisconnect(DisconnectReason.Other, "invalid tx");
.InitiateDisconnect(DisconnectReason.InvalidTxReceived, "invalid tx");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public void Report(AcceptTxResult accepted)
if (accepted == AcceptTxResult.Invalid)
{
if (logger.IsDebug) logger.Debug($"Disconnecting {_protocolHandler} due to invalid tx received");
_protocolHandler.Disconnect(DisconnectReason.Other, $"invalid tx");
_protocolHandler.Disconnect(DisconnectReason.InvalidTxReceived, $"invalid tx");
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace Nethermind.Optimism;

public class InitializeBlockchainOptimism(OptimismNethermindApi api, IChainHeadInfoProvider chainHeadInfoProvider) : InitializeBlockchain(api, chainHeadInfoProvider)
public class InitializeBlockchainOptimism(OptimismNethermindApi api, IChainHeadInfoProvider chainHeadInfoProvider, ITxGossipPolicy txGossipPolicy) : InitializeBlockchain(api, chainHeadInfoProvider, txGossipPolicy)
{
protected override async Task InitBlockchain()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace Nethermind.Taiko;

public class InitializeBlockchainTaiko(TaikoNethermindApi api, IChainHeadInfoProvider chainHeadInfoProvider) : InitializeBlockchain(api, chainHeadInfoProvider)
public class InitializeBlockchainTaiko(TaikoNethermindApi api, IChainHeadInfoProvider chainHeadInfoProvider, ITxGossipPolicy txGossipPolicy) : InitializeBlockchain(api, chainHeadInfoProvider, txGossipPolicy)
{
protected override IBlockProductionPolicy CreateBlockProductionPolicy() => NeverStartBlockProductionPolicy.Instance;
}
Loading
Loading