AuRa: move finalization manager and terminal-block disposer to DI

[asdacap]

Changes

Moves AuRa's block finalization manager and its merge-transition disposer fully into dependency injection, removing two pieces of manual lifecycle wiring.

• IAuRaBlockFinalizationManager → DI singleton. Registered in AuRaModule via a factory (AuRaPlugin.cs) instead of being new-ed in InitializeBlockchainAuRa.InitBlockchain() and stashed on a mutable AuRaNethermindApi.AuRaFinalizationManager { get; set; } property served through the FallbackToFieldFromApi source.
  • AuRaNethermindApi.AuRaFinalizationManager is now a get-only computed property resolving from the container (matching ValidatorStore, TxAuRaFilterBuilders, etc.). A settable property alongside a real registration is rejected by FallbackToFieldFromApi.
  • The only genuinely cyclic part — wiring the branch processor in via SetMainBlockBranchProcessor — stays a deferred post-construction step (WireFinalizationBranchProcessor).
  • As a normal (non-ExternallyOwned) singleton, Autofac now disposes it on shutdown — an improvement over before, where non-merge AuRa never disposed it. Dispose() is idempotent.
• Removed the InitializeMergeFinalization lifecycle hook. With the finalization manager in DI, AuRaTerminalBlockDisposer's three constructor dependencies are all DI-resolvable, so the hand-rolled hook in MergePlugin was pure ceremony.
  • AuRaTerminalBlockDisposer is now registered as a singleton in AuRaMergeModule and resolved in the InitializeBlockchainAuRaMerge step (after WireFinalizationBranchProcessor) to trigger its constructor side-effect. That step runs after the head is loaded and IPoSSwitcher is initialized, but before the block processor starts, so the once-only TerminalBlockReached signal cannot be missed. Autofac owns disposal, equivalent to the removed DisposeStack.Push.
  • Deleted the AuRaMergePlugin.InitializeMergeFinalization override and the now-unused _auraApi field, plus the base virtual hook + call site in MergePlugin (no other overriders exist).
• Tests that injected a substitute by setting the old api property now register it via DI; added a DI-resolution regression test for the disposer.

Types of changes

What types of changes does your code introduce?

• Refactoring

Testing

Requires testing

• Yes

If yes, did you write tests?

• Yes

Notes on testing

Builds clean (0 warnings). Nethermind.AuRa.Test, Nethermind.Merge.AuRa.Test (incl. the new DI-resolution test), Nethermind.Merge.Plugin.Test, and Nethermind.Runner.Test (EthereumRunnerTests) pass. The one intermittent getPayloadV1_does_not_wait_for_improvement_when_block_is_not_empty failure is a pre-existing block-improvement timing flake (passes in isolation; unrelated to finalization).

Documentation

Requires documentation update

• No

Requires explanation in Release Notes

• No

[claude]

Claude finished @asdacap's task in 6m 0s —— View job

---

Code Review

• Read repository rules and context
• Read changed files and understand the DI migration
• Analyze correctness, lifecycle, and ordering concerns
• Check test coverage for the new DI wiring
• Post review findings

---

Summary

Clean, well-reasoned DI migration. The refactoring reduces manual lifecycle ceremony and improves disposal coverage (non-merge AuRa now gets proper shutdown disposal). No critical, high, or medium findings. Two low-severity observations below.

---

Low: AuRaBlockFinalizationManager.Dispose() lacks a formal idempotency guard

In AuRaMerge mode both AuRaTerminalBlockDisposer and IAuRaBlockFinalizationManager are registered as independent, Autofac-owned singletons. On container shutdown Autofac disposes them in turn, so AuRaBlockFinalizationManager.Dispose() is called twice: once indirectly through AuRaTerminalBlockDisposer.Dispose() (when the merge hasn't yet fired) and once directly as the finalization manager singleton itself.

The current implementation is safe — C# event −= on an already-unsubscribed handler is a documented no-op — and the PR description acknowledges this ("Dispose() is idempotent"). But the code:

// AuRaBlockFinalizationManager.cs
public void Dispose()
{
    if (_branchProcessor is not null)
    {
        _branchProcessor.BlockProcessed -= OnBlockProcessed;
        _branchProcessor.BlocksProcessing -= OnBlocksProcessing;
    }
}

has no _disposed guard. If Dispose() is ever extended (e.g., to null out _branchProcessor or release other resources), the double-call could silently misbehave. A one-line guard makes the intent explicit and future-safe:

private bool _disposed;
public void Dispose()
{
    if (_disposed) return;
    _disposed = true;
    if (_branchProcessor is not null)
    {
        _branchProcessor.BlockProcessed -= OnBlockProcessed;
        _branchProcessor.BlocksProcessing -= OnBlocksProcessing;
    }
}

Note: AuRaTerminalBlockDisposer.Dispose() already has this guard (preventing the direct double-call path), but the independent Autofac disposal of IAuRaBlockFinalizationManager is a separate path that bypasses it.

---

Low/Nit: Verbose inline comment on a single resolver call

InitializeBlockchainAuRaMerge.InitBlockchain() has a 6-line inline comment block above a 1-line resolver call. The timing rationale is genuinely non-obvious and worth capturing, but it reads more like a class-level contract than a local annotation. Moving it to <remarks> on the class (alongside the existing <summary>) would keep the doc colocated with the class contract and leave the method body uncluttered. Minor nit — the current form is not harmful.

---

Correctness observations (no issues found)

• Initialization ordering is preserved. IAuRaBlockFinalizationManager is resolved lazily as a DI singleton. Its first resolution is triggered inside base.InitBlockchain() when MultiValidator/ContractBasedValidator are wired up (they take it as a constructor dep). This means LoadInitialLastFinalizedBlockLevel runs before SetMainBlockBranchProcessor is called — same ordering as the old manual construction. The existing comment on that field correctly describes this invariant.
• WireFinalizationBranchProcessor() runs before the disposer is resolved. InitializeBlockchainAuRaMerge.InitBlockchain() calls await base.InitBlockchain() (which includes the wire step) before Api.Context.Resolve<AuRaTerminalBlockDisposer>(). No race with TerminalBlockReached because the block processor hasn't started at that point.
• FallbackToFieldFromApi guard. The conversion of AuRaFinalizationManager from { get; set; } to a get-only computed property is required by FallbackToFieldFromApi<AuRaNethermindApi> — having a setter alongside a real DI registration throws InvalidConfigurationException. The PR gets this right.
• Test update in AuRaMergeEngineModuleTests. Injecting the substitute via .AddSingleton<IAuRaBlockFinalizationManager>(Substitute.For<...>()) before the AddDecorator<AuRaNethermindApi> block is correct: the substitute registers as an Autofac-owned-externally instance (instance registrations are ExternallyOwned), so Autofac won't independently dispose the mock, avoiding spurious double-dispose assertions.
• EthereumRunnerTests removal. Removing the now-impossible API property set is correct. The test now exercises the real AuRaBlockFinalizationManager (registered via AuRaModule) for AuRa configs, which is more realistic.
• Non-merge AuRa disposal improvement. Before this PR, AuRaBlockFinalizationManager was never disposed for pure-AuRa (non-merge) nodes. Now Autofac owns it as a singleton and disposes it on shutdown, unsubscribing the branch processor events. This is a genuine correctness improvement.

The base branch was changed.