fix: skip AuRa finalization startup walk on post-merge chains

[LukaszRozmej]

Summary

• On post-merge Gnosis, AuRaBlockFinalizationManager was still doing its startup catch-up walk, accumulating every un-finalized ancestor into a local List<BlockHeader>. Heap dumps from affected nodes showed the backing array at 16,777,216 entries (~10GB retained).
• Moved the walk's trigger from the constructor into SetMainBlockBranchProcessor with an idempotency guard, and added InitializeBlockchainAuRaMerge that skips wiring the branch processor once IPoSSwitcher.HasEverReachedTerminalBlock() is true.
• AuRaMergeFinalizationManager.SetMainBlockBranchProcessor also skips forwarding post-merge as a belt-and-suspenders guard.

Test plan

• Nethermind.AuRa.Test.AuRaBlockFinalizationManagerTests — 29/29 pass, including new repeated_SetMainBlockBranchProcessor_is_idempotent.
• Nethermind.Merge.AuRa.Test.AuRaMergeFinalizationManagerTests — 5/5 pass, including new SetMainBlockBranchProcessor_forwards_only_pre_merge TestCase.
• Smoke test on a post-merge Gnosis node to confirm no longer stuck at startup and no walk-related allocations.

🤖 Generated with Claude Code

[claude]

Claude finished @LukaszRozmej's task in 10m 20s —— View job

---

PR Review

• Gather context and understand the PR
• Read changed files and rules
• Review AuRaBlockFinalizationManager.cs (core fix)
• Review InitializeBlockchainAuRa.cs and InitializeBlockchainAuRaMerge.cs
• Review AuRaMergeFinalizationManager.cs and AuRaMergePlugin.cs
• Review tests
• Post findings

---

Summary

The fix correctly addresses the root cause: Initialize() (the startup catch-up walk) was unconditionally called from the AuRaBlockFinalizationManager constructor, even on fully post-merge chains where AuRa finalization is inactive. On Gnosis with 16M+ un-finalized ancestors, this accumulated every header into a List<BlockHeader>, resulting in ~10 GB retained heap.

The approach is sound: defer the walk to SetMainBlockBranchProcessor, add an idempotency guard, and introduce InitializeBlockchainAuRaMerge to skip wiring entirely on post-merge chains.

---

Findings

Critical: 0 | High: 0 | Medium: 0 | Low: 3

Low — LastFinalizedBlockLevel = 0 window during DI resolution

(See inline comment on AuRaBlockFinalizationManager.cs:37)

Before the PR, Initialize() ran in the constructor and set LastFinalizedBlockLevel before anything else. Now it runs inside SetMainBlockBranchProcessor, which is called at the end of InitBlockchain, after base.InitBlockchain() has already resolved IAuRaValidator and created MultiValidator. MultiValidator.SetFinalizationManager reads LastFinalizedBlockLevel to pick the initial active validator set, so it will transiently see 0 and select the genesis validator instead of the current one for non-immediately-changing validator types.

No production impact in practice: StartBlockProcessor depends on InitializeBlockchain completing in full (including WireFinalizationBranchProcessor), and MultiValidator.OnBlockProcessingStart re-evaluates the validator on the first non-consecutive block, so the correct validator is always in place before any block is actually processed.

Low — silent second-call with a different processor

(See inline comment on AuRaBlockFinalizationManager.cs:39)

The idempotency guard silently drops any subsequent call, including one with a different IBranchProcessor. The current call-graph makes this impossible to hit, but throwing on mismatch would make accidental misuse obvious.

Nit — extra blank line at AuRaBlockFinalizationManager.cs:48

---

What's Good

• Step override mechanism is correct: GetStepBaseType walks up to InitializeBlockchain; EthereumStepsLoader.SelectImplementation sorts by assignability and picks InitializeBlockchainAuRaMerge over InitializeBlockchainAuRa. This follows the same pattern already used by Optimism, Taiko, and XDC.
• Step ordering is safe: InitializeNetwork (where AuRaMergeFinalizationManager is created) depends on InitializeBlockchain, so the wrapper is installed after WireFinalizationBranchProcessor() has already fired — the steps cannot race.
• Three-layer defense (skip in InitializeBlockchainAuRaMerge, guard in AuRaMergeFinalizationManager, idempotency in AuRaBlockFinalizationManager) is appropriate for such a costly operation.
• New behavior is actually slightly better for MultiValidator.OnBlocksFinalized: previously BlocksFinalized fired from the constructor when no subscribers existed; now events are subscribed first, so MultiValidator.OnBlocksFinalized is properly invoked if Initialize() discovers newly-finalizable blocks.
• Test coverage is solid: idempotency, post-merge filtering in the wrapper, terminal-block handler self-unsubscription.
• Dispose() double-call is safe: _branchProcessor will be null on post-merge chains (never set), so the second Dispose() from AuRaMergeFinalizationManager.Dispose() is a no-op.

---

Verdict: Mergeable. The critical memory regression is correctly fixed with no high/medium issues. The low-severity observations are non-blocking. ✅

[claude]

Low — LastFinalizedBlockLevel window is 0 until SetMainBlockBranchProcessor fires

Before this PR, Initialize() ran in the constructor, so LastFinalizedBlockLevel was correct by the time the DI container resolved IAuRaValidator (during base.InitBlockchain()). After this PR, the field stays at 0 until SetMainBlockBranchProcessor is called — which is after base.InitBlockchain() creates MultiValidator.

MultiValidator.SetFinalizationManager reads _blockFinalizationManager.LastFinalizedBlockLevel to call InitCurrentValidator, so it momentarily picks the wrong (genesis) validator when a non-immediately-changing validator type (e.g. Contract) has already been activated.

In practice there is no production impact: StartBlockProcessor depends on InitializeBlockchain completing, and WireFinalizationBranchProcessor (which calls SetMainBlockBranchProcessor → Initialize()) runs before the step returns. So the state is corrected before any block is ever processed. MultiValidator.OnBlockProcessingStart also re-evaluates on the first non-consecutive block regardless.

Worth being aware of if anyone adds code that reads LastFinalizedBlockLevel during the DI resolution window inside InitBlockchain.

[claude]

Nit — extra blank line

Two consecutive blank lines between SetMainBlockBranchProcessor and Initialize. One is enough per the codebase style.

[claude]

Low — silent drop when called with a different processor after initialization

The guard makes a second call with the same processor idempotent (documented by the new test). But if a caller ever passes a different IBranchProcessor instance, it is silently dropped with no diagnostic — the new processor is never subscribed and the existing one stays wired. The current call-graph makes this impossible today (only one call-site, and the merge-plugin guard prevents it from reaching here twice), but the silent failure mode is easy to introduce accidentally.

Consider either throwing InvalidOperationException when called with a different instance, or adding an ArgumentException guard:

if (_initialized)
{
    if (!ReferenceEquals(_branchProcessor, branchProcessor))
        throw new InvalidOperationException("SetMainBlockBranchProcessor called with a different processor after initialization.");
    return;
}

[claude]

Observation — this guard is not exercised in the current call flow

SetMainBlockBranchProcessor is called from InitializeBlockchainAuRa.WireFinalizationBranchProcessor() (at the end of InitBlockchain), which operates on api.FinalizationManager when it is still the raw AuRaBlockFinalizationManager. The AuRaMergeFinalizationManager wrapper is only installed later, in MergePlugin.InitNetworkProtocol (which runs inside InitializeNetwork, a step that depends on InitializeBlockchain completing).

So this guard can't be hit by the normal startup path — it's belt-and-suspenders in case something calls SetMainBlockBranchProcessor through the wrapper after startup. That's a reasonable safety net; just noting the actual code path for clarity.

[LukaszRozmej]

Thanks for the review. Addressed in a2f1397:

1. Blank line nit — fixed, now a single blank line between SetMainBlockBranchProcessor and Initialize.
2. Silent drop on different processor — added a ReferenceEquals check that throws InvalidOperationException when called with a different IBranchProcessor after initialization. Existing idempotency test uses the same instance both times so it still passes without change.
3. LastFinalizedBlockLevel = 0 window during DI resolution — accepting the risk as flagged. The only way to close the window fully is to partially re-introduce a ctor-time scan of ChainLevelInfoRepository, which on post-merge Gnosis is ~25M levels of backward metadata reads — the very thing this PR is trying to avoid. StartBlockProcessor blocks on InitializeBlockchain completing, and WireFinalizationBranchProcessor runs inside that step, so the state is corrected before any block can be processed. MultiValidator.OnBlockProcessingStart also re-evaluates on the first non-consecutive block. Worth keeping the note here for future readers.
4. Merge-wrapper guard observation — confirmed, it's defense-in-depth. The actual gate is InitializeBlockchainAuRaMerge not making the call in the first place on post-merge chains; the wrapper's guard covers any future path that reaches it.

[LukaszRozmej]

@claude re-review

[claude]

Claude finished @LukaszRozmej's task in 1m 56s —— View job

---

Re-review

• Gather context
• Read AuRaBlockFinalizationManager.cs
• Read AuRaMergeFinalizationManager.cs
• Read InitializeBlockchainAuRaMerge.cs and AuRaMergePlugin.cs
• Read tests
• Post findings

---

All three low findings from the initial review have been addressed. Verification:

Blank line nit — Fixed. SetMainBlockBranchProcessor now has a single blank line between the event subscriptions and Initialize().

Silent drop on different processor — Fixed correctly. Lines 41–43 do a ReferenceEquals check and throw InvalidOperationException with a descriptive message. Ordering is correct: _initialized = true and _branchProcessor = branchProcessor are both set before the guard could be re-entered (single-threaded init path), so there is no window where _initialized is true but _branchProcessor is still null.

LastFinalizedBlockLevel = 0 window — Rationale accepted. The explanation is accurate: the only alternative (a partial ctor-time backward scan of ChainLevelInfoRepository) is exactly the cost this PR exists to avoid. The structural guarantee that StartBlockProcessor depends on InitializeBlockchain completing before any block is processed means the field is always corrected in time.

No new issues found. The three-layer defense (skip in InitializeBlockchainAuRaMerge, guard in AuRaMergeFinalizationManager.SetMainBlockBranchProcessor, throw-on-mismatch in AuRaBlockFinalizationManager.SetMainBlockBranchProcessor) is consistent and correct throughout. The Dispose() double-call on post-merge chains is safe because _branchProcessor remains null (never assigned). Both test suites cover the new behaviors well.

Critical: 0 | High: 0 | Medium: 0 | Verdict: Mergeable.

[LukaszRozmej]

@claude re-review

[claude]

Claude finished @LukaszRozmej's task in 3m 37s —— View job

---

Re-review

• Gather context
• Read changed files
• Check new commits (4f9ddef, 530d790) since prior re-review
• Post findings

---

Two new commits since the prior re-review. Both address regressions discovered in testing against the Gnosis archive case; both are clean.

---

4f9ddef — eager LastFinalizedBlockLevel load

This commit directly resolves the "Low — LastFinalizedBlockLevel = 0 window" finding from the initial review. The field is now initialized via a static field initializer (LoadInitialLastFinalizedBlockLevel), so MultiValidator.SetFinalizationManager sees the correct value during DI resolution before SetMainBlockBranchProcessor is ever called.

Key correctness points verified:

• -1 vs 0 on fresh chain: the original code set LastFinalizedBlockLevel = -1 via the property setter, but the "only-increase" guard (if (field < value)) prevented -1 from overwriting the C# default 0 — so the old behavior was actually wrong. The new field initializer bypasses the setter for construction and correctly stores -1 when no finalized blocks exist. Subsequent calls go through the setter as before.
• Bounded scan: LoadInitialLastFinalizedBlockLevel only reads ChainLevelInfo metadata; it does not allocate BlockHeader objects. On post-merge Gnosis with a fully-synced DB it walks backward until it hits the first finalized level, which is near the top — not 25M levels deep.
• FinalizeBlocks still deferred: the catch-up walk (FinalizeBlocks(head)) stays in SetMainBlockBranchProcessor, so the original memory fix is fully preserved.

---

530d790 — head-based IsHeadPostMerge guard

Replaces HasEverReachedTerminalBlock() at all three check sites (AuRaMergeFinalizationManager constructor, SetMainBlockBranchProcessor, InitializeBlockchainAuRaMerge.WireFinalizationBranchProcessor).

Root cause: PoSSwitcher.Initialize sets _hasEverReachedTerminalDifficulty = true whenever _terminalBlockNumber is not null || _finalTotalDifficulty is not null. On Gnosis, FinalTotalDifficulty is in config, so this flag is true even on a fresh archive DB at genesis — causing wiring to be incorrectly skipped and block 1301 to fail with InvalidStateRoot.

Correctness of the replacement:

Scenario	HasEverReachedTerminalBlock()	IsHeadPostMerge()	Correct?
Fresh archive, head=genesis	true (FTD in config)	false (genesis TD < TTD)	✅ now correct
Running node, head post-merge	true	true	✅ unchanged
Mid-sync restart, head pre-merge	true or false	false	✅ now correct
Null head (empty DB)	N/A	false (null guard)	✅ correct

The IsPostMerge(header) path for genesis: TotalDifficulty < TTD → isPostMerge = false. Correct.

Regression test Fresh_archive_with_FinalTotalDifficulty_in_config_still_wires_pre_merge_finalization directly covers the Gnosis-archive scenario with HasEverReachedTerminalBlock=true and genesis head. The SetHead helper cleanly separates mock setup from test logic.

No new issues found. The fix is layered correctly across all three guard sites.

---

Critical: 0 | High: 0 | Medium: 0 | Verdict: Mergeable. ✅