Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/More proper use of DI for Synchronizer #7500

Open
wants to merge 42 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
4df93ed
Seems nice
asdacap Sep 24, 2024
4b3cc69
Snap provider
asdacap Sep 24, 2024
479a3f8
Sync dispatcher
asdacap Sep 24, 2024
0a52825
Dedup
asdacap Sep 24, 2024
3144f3b
Multi sync mode selector
asdacap Sep 25, 2024
c85f3e8
Fast and full sync
asdacap Sep 25, 2024
c126f95
The merge synchronizer
asdacap Sep 25, 2024
5d21826
Use reflection
asdacap Sep 25, 2024
3d90236
Fic test
asdacap Sep 25, 2024
b9dac97
Minor cleanup
asdacap Sep 25, 2024
0f5343d
Remote block downloader
asdacap Sep 25, 2024
1181068
Reduce constructor parameter
asdacap Sep 25, 2024
54facc1
Add unit tests
asdacap Sep 25, 2024
67fe004
Cleanup
asdacap Sep 25, 2024
aecaea1
Whitespace
asdacap Sep 25, 2024
867f1f3
Pass through the servide collection
asdacap Sep 27, 2024
cb82a56
Use a need to wait for header config
asdacap Sep 27, 2024
7b2b7ec
Remove more manual construction
asdacap Sep 27, 2024
13e167f
Whitespace
asdacap Sep 27, 2024
7ba6845
Merge remote-tracking branch 'origin/master' into feature/minor-di
asdacap Sep 27, 2024
9b0cf3d
Do this more properly
asdacap Sep 27, 2024
b06b200
Set sync mode selector from container directly
asdacap Sep 27, 2024
8034622
Get component directly from container
asdacap Sep 27, 2024
915047e
Remove unnecessary code
asdacap Sep 27, 2024
e4b3baa
I thought I specifically fixed this before
asdacap Sep 27, 2024
fd8845e
Merge remote-tracking branch 'origin/master' into feature/minor-di-pr…
asdacap Sep 27, 2024
1c54c1d
Well.. might as well make everything autofac
asdacap Sep 27, 2024
e7abfba
Fix test
asdacap Sep 27, 2024
0b58c9c
Cleanup
asdacap Sep 27, 2024
a90fb3f
Why did you not fail earlier?
asdacap Sep 27, 2024
c14ae8b
Use module. Module nice.
asdacap Sep 28, 2024
0f73b6b
Fix fast sync being loaded in tests
asdacap Sep 30, 2024
dd015d3
Merge remote-tracking branch 'origin/master' into feature/minor-di-pr…
asdacap Sep 30, 2024
ee4cf91
Fic build
asdacap Sep 30, 2024
69bcc78
Merge branch 'fix/build-on-nodeloaders-test' into feature/minor-di-pr…
asdacap Sep 30, 2024
dfd0f34
Merge remote-tracking branch 'origin/master' into feature/minor-di-pr…
asdacap Sep 30, 2024
f8612f4
Merge remote-tracking branch 'origin/master' into feature/minor-di-pr…
asdacap Oct 1, 2024
399fe76
Update src/Nethermind/Nethermind.Api/IApiWithNetwork.cs
asdacap Oct 1, 2024
2083c73
Update src/Nethermind/Nethermind.Db/IDbProvider.cs
asdacap Oct 1, 2024
101b779
Update src/Nethermind/Nethermind.Db/IDbProvider.cs
asdacap Oct 1, 2024
54680c7
Address comment
asdacap Oct 3, 2024
c0dd797
Update src/Nethermind/Nethermind.Api/IApiWithBlockchain.cs
asdacap Oct 3, 2024
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
3 changes: 3 additions & 0 deletions src/Nethermind/Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
<PackageVersion Include="AspNetCore.HealthChecks.UI" Version="8.0.1" />
<PackageVersion Include="AspNetCore.HealthChecks.UI.Client" Version="8.0.1" />
<PackageVersion Include="AspNetCore.HealthChecks.UI.InMemory.Storage" Version="8.0.1" />
<PackageVersion Include="Autofac" Version="8.1.0" />
Copy link
Contributor

@alexb5dh alexb5dh Oct 3, 2024

Choose a reason for hiding this comment

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

Why is Autofac needed on top of Microsoft.Extensions.DependencyInjection? If because of modules - can it be replicated by using separate IServiceBuilders instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

This would be my question as well. I believe Autofac provides named registrations while Microsoft.Extensions.DependencyInjection may lack it. Not sure if this is the case.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was wrong. ServiceKey etc are the way to go.

Copy link
Contributor

Choose a reason for hiding this comment

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

For named registrations, there's IKeyedServiceProvider.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same reason as outlined here #6483.

I originally wanted to stick with Microsoft.Extensions.DependencyInjection, but can't make it work. See #7485.

<PackageVersion Include="Autofac.Extensions.DependencyInjection" Version="10.0.0" />
<PackageVersion Include="BenchmarkDotNet" Version="0.13.12" />
<PackageVersion Include="BenchmarkDotNet.Diagnostics.Windows" Version="0.13.12" />
<PackageVersion Include="BouncyCastle.Cryptography" Version="2.4.0" />
Expand Down Expand Up @@ -36,6 +38,7 @@
<PackageVersion Include="Microsoft.ClearScript.V8.Native.osx-x64" Version="7.4.5" />
<PackageVersion Include="Microsoft.ClearScript.V8.Native.win-x64" Version="7.4.5" />
<PackageVersion Include="McMaster.Extensions.CommandLineUtils" Version="4.1.1" />
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />
<PackageVersion Include="Microsoft.Extensions.Logging" Version="8.0.0" />
<PackageVersion Include="Microsoft.Extensions.Logging.Abstractions" Version="8.0.1" />
<PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="8.0.0" />
Expand Down
8 changes: 8 additions & 0 deletions src/Nethermind/Nethermind.Api/IApiWithBlockchain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: LGPL-3.0-only

#nullable enable
using Autofac;
using Nethermind.Blockchain;
using Nethermind.Blockchain.Filters;
using Nethermind.Blockchain.FullPruning;
Expand Down Expand Up @@ -99,5 +100,12 @@ public interface IApiWithBlockchain : IApiWithStores, IBlockchainBridgeFactory
INodeStorageFactory NodeStorageFactory { get; set; }
BackgroundTaskScheduler BackgroundTaskScheduler { get; set; }
CensorshipDetector CensorshipDetector { get; set; }

public ContainerBuilder ConfigureContainerBuilderFromApiWithBlockchain(ContainerBuilder builder)
{
return ConfigureContainerBuilderFromApiWithStores(builder)
.AddPropertiesFrom<IApiWithBlockchain>(this)
.AddSingleton<INodeStorage>(NodeStorageFactory.WrapKeyValueStore(DbProvider!.StateDb));
}
}
}
15 changes: 14 additions & 1 deletion src/Nethermind/Nethermind.Api/IApiWithNetwork.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// SPDX-License-Identifier: LGPL-3.0-only

using System.Collections.Generic;
using Autofac;
using Nethermind.Consensus;
using Nethermind.Core;
using Nethermind.Core.PubSub;
using Nethermind.Grpc;
using Nethermind.JsonRpc;
Expand All @@ -16,6 +18,7 @@
using Nethermind.Synchronization;
using Nethermind.Synchronization.Peers;
using Nethermind.Sockets;
using Nethermind.Synchronization.ParallelSync;

namespace Nethermind.Api
{
Expand All @@ -41,12 +44,22 @@ public interface IApiWithNetwork : IApiWithBlockchain
IJsonRpcLocalStats? JsonRpcLocalStats { get; set; }
ISessionMonitor? SessionMonitor { get; set; }
IStaticNodesManager? StaticNodesManager { get; set; }
ISynchronizer? Synchronizer { get; set; }
ISynchronizer? Synchronizer { get; }
ISyncModeSelector SyncModeSelector { get; }
ISyncProgressResolver? SyncProgressResolver { get; }
IPivot? Pivot { get; set; }
ISyncPeerPool? SyncPeerPool { get; set; }
IPeerDifficultyRefreshPool? PeerDifficultyRefreshPool { get; set; }
ISyncServer? SyncServer { get; set; }
IWebSocketsManager WebSocketsManager { get; set; }
ISubscriptionFactory? SubscriptionFactory { get; set; }

IContainer? ApiWithNetworkServiceContainer { get; set; }

public ContainerBuilder ConfigureContainerBuilderFromApiWithNetwork(ContainerBuilder builder)
{
return ConfigureContainerBuilderFromApiWithBlockchain(builder)
.AddPropertiesFrom<IApiWithNetwork>(this);
}
}
}
8 changes: 8 additions & 0 deletions src/Nethermind/Nethermind.Api/IApiWithStores.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only

using Autofac;
using Nethermind.Blockchain;
using Nethermind.Blockchain.Blocks;
using Nethermind.Blockchain.Find;
using Nethermind.Blockchain.Receipts;
using Nethermind.Consensus;
using Nethermind.Core;
using Nethermind.Crypto;
using Nethermind.Db.Blooms;
using Nethermind.State.Repositories;
Expand All @@ -29,5 +31,11 @@ public interface IApiWithStores : IBasicApi
IReceiptMonitor? ReceiptMonitor { get; set; }
IWallet? Wallet { get; set; }
IBlockStore? BadBlocksStore { get; set; }

public ContainerBuilder ConfigureContainerBuilderFromApiWithStores(ContainerBuilder builder)
{
return ConfigureContainerBuilderFromBasicApi(builder)
.AddPropertiesFrom<IApiWithStores>(this);
}
}
}
18 changes: 15 additions & 3 deletions src/Nethermind/Nethermind.Api/IBasicApi.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only

using System;
using System.Collections.Generic;
using System.IO.Abstractions;
using System.Linq;
using Autofac;
using Nethermind.Abi;
using Nethermind.Api.Extensions;
using Nethermind.Blockchain.Synchronization;
using Nethermind.Config;
using Nethermind.Core;
using Nethermind.Core.Specs;
Expand All @@ -17,7 +20,6 @@
using Nethermind.Serialization.Json;
using Nethermind.Specs.ChainSpecStyle;
using Nethermind.Synchronization;
using Nethermind.Synchronization.ParallelSync;

namespace Nethermind.Api
{
Expand All @@ -38,10 +40,9 @@ public interface IBasicApi
ILogManager LogManager { get; set; }
ProtectedPrivateKey? OriginalSignerKey { get; set; }
IReadOnlyList<INethermindPlugin> Plugins { get; }
[SkipServiceCollection]
string SealEngineType { get; set; }
ISpecProvider? SpecProvider { get; set; }
ISyncModeSelector SyncModeSelector { get; set; }
ISyncProgressResolver? SyncProgressResolver { get; set; }
IBetterPeerStrategy? BetterPeerStrategy { get; set; }
ITimestamper Timestamper { get; }
ITimerFactory TimerFactory { get; }
Expand All @@ -57,5 +58,16 @@ public IEnumerable<IConsensusWrapperPlugin> GetConsensusWrapperPlugins() =>

public IEnumerable<ISynchronizationPlugin> GetSynchronizationPlugins() =>
Plugins.OfType<ISynchronizationPlugin>();

public ContainerBuilder ConfigureContainerBuilderFromBasicApi(ContainerBuilder builder)
{
builder
.AddPropertiesFrom<IBasicApi>(this)
.AddSingleton(ConfigProvider.GetConfig<ISyncConfig>());

DbProvider!.ConfigureServiceCollection(builder);

return builder;
}
}
}
9 changes: 6 additions & 3 deletions src/Nethermind/Nethermind.Api/NethermindApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.IO.Abstractions;
using Autofac;
using Nethermind.Abi;
using Nethermind.Api.Extensions;
using Nethermind.Blockchain;
Expand Down Expand Up @@ -185,14 +186,14 @@ public ISealEngine SealEngine
public ISessionMonitor? SessionMonitor { get; set; }
public ISpecProvider? SpecProvider { get; set; }
public IPoSSwitcher PoSSwitcher { get; set; } = NoPoS.Instance;
public ISyncModeSelector SyncModeSelector { get; set; } = null!;
public ISyncModeSelector SyncModeSelector => ApiWithNetworkServiceContainer?.Resolve<ISyncModeSelector>()!;

public ISyncProgressResolver? SyncProgressResolver { get; set; }
public ISyncProgressResolver? SyncProgressResolver => ApiWithNetworkServiceContainer?.Resolve<ISyncProgressResolver>();
public IBetterPeerStrategy? BetterPeerStrategy { get; set; }
public IPivot? Pivot { get; set; }
public ISyncPeerPool? SyncPeerPool { get; set; }
public IPeerDifficultyRefreshPool? PeerDifficultyRefreshPool { get; set; }
public ISynchronizer? Synchronizer { get; set; }
public ISynchronizer? Synchronizer => ApiWithNetworkServiceContainer?.Resolve<ISynchronizer>();
public ISyncServer? SyncServer { get; set; }
public IWorldState? WorldState { get; set; }
public IReadOnlyStateProvider? ChainHeadStateProvider { get; set; }
Expand Down Expand Up @@ -241,5 +242,7 @@ public ISealEngine SealEngine
public CompositePruningTrigger PruningTrigger { get; } = new();
public IProcessExitSource? ProcessExit { get; set; }
public CompositeTxGossipPolicy TxGossipPolicy { get; } = new();

public IContainer? ApiWithNetworkServiceContainer { get; set; }
asdacap marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -143,4 +143,7 @@ public interface ISyncConfig : IConfig

[ConfigItem(Description = "_Technical._ MultiSyncModeSelector sync mode timer loop interval. Used for testing.", DefaultValue = "1000", HiddenFromDocs = true)]
int MultiSyncModeSelectorLoopTimerMs { get; set; }

[ConfigItem(Description = "_Technical._ MultiSyncModeSelector will wait for header to completely sync first.", DefaultValue = "false", HiddenFromDocs = true)]
bool NeedToWaitForHeader { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public string? PivotHash
public int MallocTrimIntervalSec { get; set; } = 300;
public bool? SnapServingEnabled { get; set; } = null;
public int MultiSyncModeSelectorLoopTimerMs { get; set; } = 1000;
public bool NeedToWaitForHeader { get; set; }
public bool TrieHealing { get; set; } = true;

public override string ToString()
Expand Down
113 changes: 113 additions & 0 deletions src/Nethermind/Nethermind.Core.Test/ContainerBuilderExtensionsTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
// SPDX-FileCopyrightText: 2024 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only

using System;
using Autofac;
using FluentAssertions;
using NUnit.Framework;

namespace Nethermind.Core.Test;

public class ContainerBuilderExtensionsTests
{
[Test]
public void AddPropertiesFrom_CanAddProperties()
{
ITestInterface interfaceImplementation = new InterfaceImplementation();
IContainer sp = new ContainerBuilder()
.AddPropertiesFrom(interfaceImplementation)
.Build();

sp.ResolveOptional<DeclaredService>().Should().NotBeNull();
sp.ResolveOptional<DeclaredInBase>().Should().BeNull();
sp.ResolveOptional<Ignored>().Should().BeNull();
sp.ResolveOptional<DeclaredButNullService>().Should().BeNull();
}

[Test]
public void TestRegisterNamedComponent()
{
IContainer sp = new ContainerBuilder()
.AddScoped<MainComponent>()
.AddScoped<MainComponentDependency>()
.RegisterNamedComponentInItsOwnLifetime<MainComponent>("custom", cfg =>
{
// Override it in custom
cfg.AddScoped<MainComponentDependency, MainComponentDependencySubClass>();
})
.Build();

using (ILifetimeScope scope = sp.BeginLifetimeScope())
{
scope.Resolve<MainComponent>().Property.Should().BeOfType<MainComponentDependency>();
}

MainComponentDependency customMainComponentDependency = sp.ResolveNamed<MainComponent>("custom").Property;
sp.ResolveNamed<MainComponent>("custom").Property.Should().BeOfType<MainComponentDependencySubClass>();

sp.Dispose();

customMainComponentDependency.WasDisposed.Should().BeTrue();
}

private class MainComponent(MainComponentDependency mainComponentDependency, ILifetimeScope scope) : IDisposable
{
public MainComponentDependency Property => mainComponentDependency;

public void Dispose()
{
scope.Dispose();
}
}

private class MainComponentDependency : IDisposable
{
public bool WasDisposed { get; set; }

public void Dispose()
{
WasDisposed = true;
}
}

private class MainComponentDependencySubClass : MainComponentDependency
{
}

private class InterfaceImplementation : ITestInterface
{
public DeclaredService TheService { get; set; } = new DeclaredService();
public DeclaredButNullService? NullService { get; set; } = null;
public Ignored IgnoredService { get; set; } = new Ignored();
public DeclaredInBase BaseService { get; set; } = new DeclaredInBase();
}

private interface ITestInterface : ITestInterfaceBase
{
DeclaredService TheService { get; set; }
DeclaredButNullService? NullService { get; set; }

[SkipServiceCollection]
Ignored IgnoredService { get; set; }
}

private interface ITestInterfaceBase
{
DeclaredInBase BaseService { get; set; }
}

private class DeclaredInBase { }
private class DeclaredService { }
private class DeclaredButNullService { }
private class Ignored { }

private class DisposableService : IDisposable
{
public bool WasDisposed { get; set; } = false;

public void Dispose()
{
WasDisposed = true;
}
}
}
Loading