sequencer: raise backfillMaxDepth to cover the reth unpersisted window - #45
Merged
Merged
Conversation
…window backfillMaxDepth bounds how far the EL head may have regressed when a parent-not-found triggers a replay. That distance comes from upstream reth configuration rather than being a fixed quantity: persistence_backpressure_threshold + memory_block_buffer_target is 16 on reth v2.4.0 (16 + 0) and 21 on reth v2.5.2 (16 + 5) by default. At 16 the walk refuses any wider gap, so on reth v2.5.2 defaults a crash that loses 17-21 blocks makes recovery a no-op, which is indistinguishable from a node that simply stopped catching up. Raise the limit to 32: it covers the v2.5.2 ceiling with margin and stays under backfillCacheCapacity. Also document the coupling the limit depends on, so the next EL tuning change does not silently disable recovery, and log the refusal explicitly. Both callers only report the error generically, and this one is a configuration mismatch rather than a transient failure they could retry. Note tendermint log.Logger has no Warn, so it is logged at Error level. TestStateV2_Backfill_DepthBoundary pins the boundary. It fails against 16, where a 21-block gap (the reth v2.5.2 ceiling) is refused.
There was a problem hiding this comment.
Claude Code Review
Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.
Tip: disable this comment in your organization's Code Review settings.
Include the memory block buffer target in the documented invariant, report the actual missing range, verify the refusal fields, and add the pending changelog entry.
tomatoishealthy
approved these changes
Sep 16, 2026
crazywriter1
pushed a commit
to crazywriter1/morph
that referenced
this pull request
Sep 17, 2026
Point every Tendermint module replacement at morph-l2/tendermint#45's merged commit and refresh the affected checksums.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
backfillMaxDepth(added in #43) bounds how far the execution layer's head may have regressed when aparent-not-foundtriggers a replay. That distance is upstream reth configuration rather than a fixed quantity — the ceiling ispersistence_backpressure_threshold + memory_block_buffer_target:At 16 the walk refuses any gap above 16, so on reth v2.5.2 defaults a crash that loses 17–21 blocks makes recovery a no-op. Nothing is applied and the only signal is a generic
Backfill failedline — indistinguishable from a node that simply stopped catching up.The constant's comment was also written against the v2.4.0 numbers ("reth buffers only a couple of unpersisted blocks by default"), which no longer hold.
Changes
backfillMaxDepth16 → 32. Covers the v2.5.2 ceiling with margin, and stays well underbackfillCacheCapacity(64).Document the coupling the limit depends on, so the next EL persistence tuning does not silently disable recovery:
Log the refusal explicitly (
Backfill refused: gap exceeds backfillMaxDepth, carrying head / oldestMissing / newestMissing / gap). Both callers only report the error generically, and this case is a configuration mismatch rather than a transient failure they could retry away. Notelibs/log.Loggerhas noWarn, so this is atErrorlevel.TestStateV2_Backfill_DepthBoundarypins the boundary: a 21-block gap (the v2.5.2 ceiling) and exactlybackfillMaxDepthsucceed,backfillMaxDepth + 1is refused. This path previously had no test coverage.Verification
go test ./sequencer/... -race -count=1— passgo vet ./sequencer/...,go build ./...— cleanThe new test is a genuine regression test: against the old value it fails with
Not addressed here
A deployment that tunes the EL above 32 (for example
--engine.persistence-backpressure-threshold 512) still disables recovery, and raisingbackfillMaxDepthalone cannot fix it because the walk is capped bybackfillCacheCapacity(64) as well. Deriving the limit from the EL at runtime would be the robust fix, but those values are not readable over the Engine API today. This PR only makes the default configuration correct and the failure visible.Follow-up to #43.