perf(verifier): cache the unfinalized range instead of re-reading it every poll - #1466
Closed
KodeyThomas wants to merge 2 commits into
Closed
KodeyThomas wants to merge 2 commits into
KodeyThomas wants to merge 2 commits into
Conversation
KodeyThomas
force-pushed
the
perf/ccip-evm-unfinalized-range-cache
branch
from
September 18, 2026 13:06
6ef7676 to
4dc1176
Compare
…every poll The source reader re-queried logs for the entire finalized..latest window on every poll, because that re-query was also the reorg-detection mechanism. On chains with fast blocks and slow finality the window is thousands of blocks, so a 2s poll issued ~40 eth_getLogs calls to re-scan blocks whose contents could not have changed. The EVM reader now caches the hash-linked headers from finalized up to the head. Because every stored header's ParentHash equals the stored header below it, one link check at the head proves the whole range is unchanged - a block hash commits to its entire ancestry, so a reorg at any depth breaks the link from the new head down to the anchor, including one that re-extends between two polls. The verifier reads only new blocks while the link holds, and falls back to the full range on a reorg, a tracker error, or a reader without the capability. Exposed as the optional chainaccess.UnfinalizedRangeTracker so the linkage assumption stays in the EVM package; other chain families keep today's behaviour. SourceReaderUnwrapper is needed alongside it because observedSourceReader embeds the SourceReader interface, which would otherwise hide the capability from the verifier and silently disable the cache. Adds verifier_source_reader_unfinalized_range_rereads. Break-even is a reread rate near one poll in five, above which this costs more RPC than it saves, so the rate needs watching per chain on rollout.
KodeyThomas
force-pushed
the
perf/ccip-evm-unfinalized-range-cache
branch
from
September 18, 2026 13:11
4dc1176 to
3152a9a
Compare
Member
Author
|
recreating with anvil -b 1 --slots-in-an-epoch 900 gives a 1800 block unfinlized window so still way less than abstract saw a drop of 94% from 558/min o 30/min |
carte7000
reviewed
Sep 18, 2026
| // Record how far logs have actually been read, which lags the tracked range when a chunk | ||
| // fails. The next cycle re-reads from here so a partial read never leaves a gap. | ||
| if lastQueried > r.logHighWater.Load() { | ||
| r.logHighWater.Store(lastQueried) |
Contributor
There was a problem hiding this comment.
If there is a reorg do we need to decrement the logHighWater so we can query again from a lower point that we reached?
Member
Author
There was a problem hiding this comment.
ah! yeah i think so. nice catch
carte7000
reviewed
Sep 18, 2026
| if stored, ok := t.at(finalized.Number); ok && stored.Hash != finalized.Hash { | ||
| t.lggr.Errorw("Finalized block hash disagrees with the cached tail, dropping tail", | ||
| "blockNumber", finalized.Number, "storedHash", stored.Hash, "finalizedHash", finalized.Hash) | ||
| t.headers = nil |
Contributor
There was a problem hiding this comment.
Should we be defensive here? Maybe we can return something that indicates the finality violation or return changed in the re-query?
Member
Author
There was a problem hiding this comment.
we have a seperate component that handles finality violations. would rather keep that seperate and not duplicate logic
The read extent was guarded by `if lastQueried > logHighWater`, making it monotonic. A reorg that rewinds the head re-mines blocks at heights already read, and those heights stay below the old mark, so every later poll computed `from = max(logHighWater+1, checkpoint)` above them and skipped them. Messages in the re-mined blocks were never discovered until the head passed the old head, which is what TestE2EReorg asserts against: it reverts to a snapshot and re-sends messages at the same heights in swapped order. The guard was a no-op on every non-reorg path - `from` is always `max(lastRead+1, checkpoint)`, so a successful read always ends above the mark. It only ever bit after a reorg, where moving backwards is the correct behaviour. Store the extent unconditionally and rename the field to lastReadBlock, since it is not a high water mark.
|
Code coverage report:
Files added (in
|
KodeyThomas
marked this pull request as ready for review
September 18, 2026 14:25
carte7000
reviewed
Sep 18, 2026
|
|
||
| // Only return early when no progress was made | ||
| if lastQueriedBlock.Cmp(fromBlock) == 0 { | ||
| if lastQueried == queryFrom { |
Contributor
There was a problem hiding this comment.
Do we need to reset here as well? If we return here could we still have skipped the lastQueried block
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.
The source reader re-queried logs for the entire
finalized..latestwindow on every poll, because that re-query was also the reorg-detection mechanism.On chains with fast blocks and slow finality the window is thousands of blocks, so a 2s poll issued ~40 eth_getLogs calls to re-scan blocks whose contents could not have changed.
The EVM reader now caches the hash-linked headers from finalized up to the head. Because every stored header's
ParentHashequals the stored header below it, one link check at the head proves the whole range is unchanged and a block hash commits to its entire ancestry, so a reorg at any depth breaks the link from the new head down to the finalized block.The verifier reads only new blocks while the link holds, and falls back to the full range on a reorg, a tracker error, or a reader without the capability.
Exposed as the optional
chainaccess.UnfinalizedRangeTrackerso the linkage assumption stays in the EVM package and other chain families keep today's behaviour.SourceReaderUnwrapperis needed alongside it because observedSourceReader embeds the SourceReader interface, which would otherwise hide the capability from the verifier and silently disable the cache.Additionally adds
verifier_source_reader_unfinalized_range_rereadsbreak-even in terms of performance here is a reread rate near 1/5 above which this costs more RPC than it saves, so the rate needs watching per chain on rollout.