fix(archiver): always advance L1-to-L2 messages syncpoint to current L1 block (backport #22154)#22625
Closed
spalladino wants to merge 4 commits into
Closed
fix(archiver): always advance L1-to-L2 messages syncpoint to current L1 block (backport #22154)#22625spalladino wants to merge 4 commits into
spalladino wants to merge 4 commits into
Conversation
…L1 block (backport #22154) The messages syncpoint used to track the L1 block of the last downloaded message, causing unnecessary re-scans when no messages were sent for a long time. Now it always advances to currentL1BlockNumber, with the rolling hash check and rollback logic providing reorg protection. Also moves setInboxTreeInProgress to after message insertion to prevent concurrent reads from seeing a checkpoint as sealed before its messages are available, and fixes a pre-existing division by zero in metrics when a batch has no messages. NOTE: Cherry-picked with conflicts still present; resolved in following commit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
v4 has no setInboxTreeInProgress/treeInProgress persistence concept, so: - simplify setMessageSyncState to take only l1Block (drop treeInProgress param) - drop the tree-in-progress guard tests that don't apply to v4 - keep the improved fake_l1_state getState that computes treeInProgress from checkpoint/message visibility (InboxContractState.treeInProgress is still returned by the real inbox contract in v4) - drop getInboxTreeInProgress from message_store Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use remote messages state in rollbackL1ToL2Messages. On v4 the syncpoint does not need treeInProgress, so only destructure messagesRollingHash. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Backport of #22154 to
v4.Motivation
The L1-to-L2 messages syncpoint tracked the L1 block of the last downloaded message. If no messages were sent for a long time, the syncpoint got stuck and the next non-empty sync iteration re-scanned old block ranges. The rolling hash check and rollback logic already handle L1 reorgs, making this conservative syncpoint advancement redundant.
Approach
The syncpoint now always advances to
currentL1BlockNumberon success. After downloading, local state is verified against L1; on mismatch the syncpoint rolls back and the operation retries (up to 3 times within the same L1 sync iteration).retrieveL1ToL2Messagenow scans a single L1 block (the one recorded on the local message) rather than a large range.Changes
handleL1ToL2Messagesto always advance the syncpoint on success, with rollback-and-retry on mismatch. Re-throws unexpected (non-MessageStoreError) exceptions instead of swallowing them. Updated README to document the new sync logic.fake_l1_stateto correctly filter messages by block number ingetStateand supportgetMessageSentEventByHash(msgHash, l1BlockHash).InboxContract.getMessageSentEventByHashsignature to take(msgHash, l1BlockHash)instead of(hash, fromBlock, toBlock).retryTimesutility (likeretryUntilbut with a retry count instead of a timeout).Notes on the backport
v4does not have thesetInboxTreeInProgress/ tree-in-progress persistence concept, so this backport differs from the original PR in the following ways:setMessageSyncStateis simplified to take onlyl1Block(thetreeInProgressparameter is dropped, since there is nothing to persist inv4).v4).fake_l1_state.getStateimprovements (computingtreeInProgressfrom checkpoint/message visibility) are kept as-is —InboxContractState.treeInProgressis still returned by the real inbox contract inv4.getInboxTreeInProgresson the message store is not added.The two commits are preserved (cherry-pick with conflicts + resolution) so the adaptations are easier to review — PR should merge with
ci-no-squash.