From 3583743042e1675a0bd2632ebf585a51d1d62768 Mon Sep 17 00:00:00 2001 From: jesteban <129153821+joanestebanr@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:23:37 +0200 Subject: [PATCH] fix(l2gersync): adapt removal-event scan to RPC eth_getLogs block-range cap isGERRemovedFromL2 scanned for the GER removal event from the insert block (which can be arbitrarily far behind the head) to "latest" in one open-ended eth_getLogs call. Once the chain advanced past the RPC provider's block-range cap, that call failed with "query exceeds max block range N" on every single appender retry, logging an ERROR forever and never actually recovering a genuinely-removed GER. scanRemovedGERs now parses that error via ParseMaxRangeFromError and retries chunked - the same adaptive, config-free pattern already used by L2EVMGERReader.GetRemovedGERsForRange and AgglayerBridgeL2Reader's fetchUnsetClaimsWithFallbackChunking/getUnsetClaimsInChunks - caching the learned cap (removalScanMaxRange) so later retries skip the doomed unbounded call, and recursing per-chunk (fetchRemovedGERsChunk) so a chunk that is itself still too large keeps adapting. Closes #1812 Co-Authored-By: Claude Sonnet 5 --- l2gersync/evm_downloader_sovereign.go | 94 +++++++++++++- l2gersync/evm_downloader_sovereign_test.go | 141 +++++++++++++++++++++ 2 files changed, 234 insertions(+), 1 deletion(-) diff --git a/l2gersync/evm_downloader_sovereign.go b/l2gersync/evm_downloader_sovereign.go index 4bd5c6ffa..7e1d6cf1a 100644 --- a/l2gersync/evm_downloader_sovereign.go +++ b/l2gersync/evm_downloader_sovereign.go @@ -7,6 +7,8 @@ import ( "github.com/0xPolygon/cdk-contracts-tooling/contracts/aggchain-multisig/agglayerger" "github.com/0xPolygon/cdk-contracts-tooling/contracts/aggchain-multisig/agglayergerl2" + agglayertypes "github.com/agglayer/aggkit/agglayer/types" + aggkitcommon "github.com/agglayer/aggkit/common" "github.com/agglayer/aggkit/log" "github.com/agglayer/aggkit/sync" aggkittypes "github.com/agglayer/aggkit/types" @@ -32,6 +34,13 @@ type downloaderSovereign struct { l1GERManager *agglayerger.Agglayerger rh *sync.RetryHandler syncBlockChunkSize uint64 + + // removalScanMaxRange caches the eth_getLogs block-range cap learned from an RPC "query exceeds max + // block range" error (see scanRemovedGERs). Zero means "not learned yet". Once learned, subsequent + // removal scans go straight to the chunked query instead of repeating the unbounded call, which + // would otherwise fail (and log an ERROR) again with the exact same error on every retry. Only + // touched from the single Download() goroutine, so a plain field is safe. + removalScanMaxRange uint64 } func newDownloaderSovereign( @@ -197,7 +206,7 @@ func (d *downloaderSovereign) buildAppender( // non-zero). Any read/scan error is treated as "not removed" (never dereferences a nil value), so the // caller keeps retrying rather than skipping a stale insert incorrectly. func (d *downloaderSovereign) isGERRemovedFromL2(ctx context.Context, fromBlock uint64, ger common.Hash) bool { - removedEvents, err := filterRemovedGERs(ctx, d.l2GERManager, fromBlock, nil, [][common.HashLength]byte{ger}) + removedEvents, err := d.scanRemovedGERs(ctx, fromBlock, ger) if err != nil { log.Errorf("failed to scan for GER %s removal events from block %d: %v", ger.Hex(), fromBlock, err) return false @@ -214,3 +223,86 @@ func (d *downloaderSovereign) isGERRemovedFromL2(ctx context.Context, fromBlock return timestampL2.Cmp(common.Big0) == 0 } + +// scanRemovedGERs scans for UpdateRemovalHashChainValue events matching ger from fromBlock to the +// current head. The scan is open-ended (toBlock == nil, i.e. "latest") because the insert block can be +// arbitrarily far behind the head. It adapts to the RPC provider's eth_getLogs block-range cap purely by +// parsing the "query exceeds max block range" error via aggkitcommon.ParseMaxRangeFromError - no config +// parameter involved - the same pattern used by L2EVMGERReader.GetRemovedGERsForRange and +// AgglayerBridgeL2Reader.fetchUnsetClaimsWithFallbackChunking/getUnsetClaimsInChunks. +// +// The one deviation from those siblings: this is invoked on every appender retry while a GER stays +// unresolved, and fromBlock keeps being just as far behind an ever-growing head on every retry, so their +// "always try the full range first, chunk on error" approach would repeat the doomed unbounded call - +// and its ERROR log - forever. d.removalScanMaxRange caches the learned cap across calls so that, once +// learned, later calls skip straight to the chunked path in fetchRemovedGERsChunk below. +func (d *downloaderSovereign) scanRemovedGERs( + ctx context.Context, fromBlock uint64, ger common.Hash, +) ([]*agglayertypes.RemovedGER, error) { + gers := [][common.HashLength]byte{ger} + + if d.removalScanMaxRange == 0 { + removedEvents, err := filterRemovedGERs(ctx, d.l2GERManager, fromBlock, nil, gers) + if err == nil { + return removedEvents, nil + } + + maxRange, isMaxRangeErr := aggkitcommon.ParseMaxRangeFromError(err.Error()) + if !isMaxRangeErr { + return nil, err + } + d.removalScanMaxRange = maxRange + } + + toBlock, headErr := d.GetLastFinalizedBlock(ctx) + if headErr != nil { + return nil, fmt.Errorf("failed to resolve chain head for chunked removal scan: %w", headErr) + } + if fromBlock > toBlock { + // fromBlock (the insert block) is already ahead of the resolved head; nothing to scan yet. + return nil, nil + } + + log.Debugf("scanning for GER %s removal in chunks of max %d blocks over range [%d, %d]", + ger.Hex(), d.removalScanMaxRange, fromBlock, toBlock) + + return aggkitcommon.ChunkedRangeQuery(ctx, fromBlock, toBlock, d.removalScanMaxRange, + func(ctx context.Context, from, to uint64) ([]*agglayertypes.RemovedGER, error) { + return d.fetchRemovedGERsChunk(ctx, from, to, gers) + }, + func(all, chunk []*agglayertypes.RemovedGER) []*agglayertypes.RemovedGER { + return append(all, chunk...) + }, + []*agglayertypes.RemovedGER{}, + ) +} + +// fetchRemovedGERsChunk fetches one [fromBlock, toBlock] chunk directly and, on a "range too large" +// error, re-learns the cap and recurses through ChunkedRangeQuery - mirroring +// AgglayerBridgeL2Reader.fetchUnsetClaimsWithFallbackChunking/getUnsetClaimsInChunks - so a chunk that is +// itself still too large (e.g. the provider's cap shrank since it was first learned) keeps adapting +// instead of failing the whole scan outright. +func (d *downloaderSovereign) fetchRemovedGERsChunk( + ctx context.Context, fromBlock, toBlock uint64, gers [][common.HashLength]byte, +) ([]*agglayertypes.RemovedGER, error) { + removedEvents, err := filterRemovedGERs(ctx, d.l2GERManager, fromBlock, &toBlock, gers) + if err == nil { + return removedEvents, nil + } + + maxRange, isMaxRangeErr := aggkitcommon.ParseMaxRangeFromError(err.Error()) + if !isMaxRangeErr { + return nil, err + } + d.removalScanMaxRange = maxRange + + return aggkitcommon.ChunkedRangeQuery(ctx, fromBlock, toBlock, maxRange, + func(ctx context.Context, from, to uint64) ([]*agglayertypes.RemovedGER, error) { + return d.fetchRemovedGERsChunk(ctx, from, to, gers) + }, + func(all, chunk []*agglayertypes.RemovedGER) []*agglayertypes.RemovedGER { + return append(all, chunk...) + }, + []*agglayertypes.RemovedGER{}, + ) +} diff --git a/l2gersync/evm_downloader_sovereign_test.go b/l2gersync/evm_downloader_sovereign_test.go index db9c841b4..aed81273f 100644 --- a/l2gersync/evm_downloader_sovereign_test.go +++ b/l2gersync/evm_downloader_sovereign_test.go @@ -339,3 +339,144 @@ func TestDownloaderSovereign_GetInfoByGlobalExitRootErrorHandlingInAppender(t *t }) } } + +// TestDownloaderSovereign_IsGERRemovedFromL2_RecoversFromMaxBlockRangeError is a regression test for a +// production incident: isGERRemovedFromL2 scans for the (S-log) removal event from fromBlock (the +// insert block, which can be arbitrarily far behind the head) to "latest" (bind.FilterOpts.End == nil). +// Some RPC providers cap eth_getLogs to a maximum block range and reject that open-ended query with e.g. +// "query exceeds max block range 100000" once fromBlock is more than that many blocks behind the head. +// Before the fix, this error was just logged and treated as "not removed" forever, so the recovery path +// could never unstick a stale insert once the chain had advanced past the provider's range cap. The fix +// (scanRemovedGERs) detects that specific error, resolves the current head, and retries chunked - +// mirroring L2EVMGERReader.GetRemovedGERsForRange. +func TestDownloaderSovereign_IsGERRemovedFromL2_RecoversFromMaxBlockRangeError(t *testing.T) { + t.Parallel() + + fromBlock := uint64(5) + latestBlock := uint64(250) + l2GERAddr := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678") + testGER := common.HexToHash("0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef") + testHashChainValue := common.HexToHash("0xabcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890") + + mockL2Client := aggkittypesmocks.NewBaseEthereumClienter(t) + mockL1Client := aggkittypesmocks.NewBaseEthereumClienter(t) + mockL1InfoTreeSync := l2gersyncmocks.NewL1InfoTreeQuerier(t) + rh := &sync.RetryHandler{ + MaxRetryAttemptsAfterError: 5, + RetryAfterErrorPeriod: time.Millisecond, + } + + // 1st attempt: the open-ended (fromBlock -> latest) scan is rejected by the provider. + mockL2Client.EXPECT().FilterLogs(mock.Anything, mock.Anything). + Return(nil, fmt.Errorf("query exceeds max block range 100")).Once() + + // The current head is resolved so the scan can be retried with an explicit, chunkable toBlock. + mockL2Client.EXPECT().CustomHeaderByNumber(mock.Anything, &aggkittypes.LatestBlock). + Return(&aggkittypes.BlockHeader{Number: latestBlock}, nil).Once() + + // Chunk 1 [5,104]: no removal event. + mockL2Client.EXPECT().FilterLogs(mock.Anything, mock.Anything).Return([]ethtypes.Log{}, nil).Once() + // Chunk 2 [105,204]: the removal event lives here, proving results across chunks are combined + // rather than only the first or last chunk being considered. + removalLog := ethtypes.Log{ + Address: l2GERAddr, + Topics: []common.Hash{removeGEREventSignature, testGER, testHashChainValue}, + Data: []byte{}, + BlockNumber: 150, + TxHash: common.HexToHash("0x222"), + TxIndex: 0, + BlockHash: common.HexToHash("0xdef456"), + Index: 1, + } + mockL2Client.EXPECT().FilterLogs(mock.Anything, mock.Anything).Return([]ethtypes.Log{removalLog}, nil).Once() + // Chunk 3 [205,250]: no removal event. + mockL2Client.EXPECT().FilterLogs(mock.Anything, mock.Anything).Return([]ethtypes.Log{}, nil).Once() + + // (S-map) L2 globalExitRootMap reads 0, so the AND of S-log and S-map confirms the GER is removed. + mockL2Client.EXPECT().CallContract(mock.Anything, mock.Anything, mock.Anything). + Return(make([]byte, 32), nil).Once() + + downloader, err := newDownloaderSovereign( + mockL2Client, + l2GERAddr, + mockL1InfoTreeSync, + mockL1Client, + common.HexToAddress("0x0000000000000000000000000000000000000001"), // l1GERAddr + rh, + aggkittypes.LatestBlock, + time.Millisecond*10, + uint64(10), // syncBlockChunkSize (unrelated to the removal-scan chunking under test) + ) + require.NoError(t, err) + + removed := downloader.isGERRemovedFromL2(context.Background(), fromBlock, testGER) + require.True(t, removed, "GER must be reported removed once the chunked scan finds the removal event") + + mockL2Client.AssertExpectations(t) + mockL1Client.AssertExpectations(t) + mockL1InfoTreeSync.AssertExpectations(t) +} + +// TestDownloaderSovereign_IsGERRemovedFromL2_CachesLearnedMaxRangeAcrossCalls proves the fix for the +// noisy follow-up to the max-range bug: isGERRemovedFromL2 runs on every appender retry while a GER stays +// unresolved, so without caching the learned range cap, the doomed open-ended scan (and its ERROR log) +// would repeat on every single retry forever, even though the recovery path itself already works. Once +// the cap is learned from a first "query exceeds max block range" error, a second call (any ger/fromBlock) +// must skip straight to the chunked scan: only mocking the head lookup + one chunked FilterLogs call +// (and no error-returning "wide open" call, which is not even stubbed here) proves it never retries the +// doomed unbounded query again. +func TestDownloaderSovereign_IsGERRemovedFromL2_CachesLearnedMaxRangeAcrossCalls(t *testing.T) { + t.Parallel() + + l2GERAddr := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678") + firstGER := common.HexToHash("0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef") + secondGER := common.HexToHash("0xabcdef1234567890abcdef1234567890abcdef1234567890abcdef12345678ab") + + mockL2Client := aggkittypesmocks.NewBaseEthereumClienter(t) + mockL1Client := aggkittypesmocks.NewBaseEthereumClienter(t) + mockL1InfoTreeSync := l2gersyncmocks.NewL1InfoTreeQuerier(t) + rh := &sync.RetryHandler{ + MaxRetryAttemptsAfterError: 5, + RetryAfterErrorPeriod: time.Millisecond, + } + + // --- 1st call: learns the range cap the same way as the recovery test above. --- + firstLatestBlock := uint64(150) + mockL2Client.EXPECT().FilterLogs(mock.Anything, mock.Anything). + Return(nil, fmt.Errorf("query exceeds max block range 1000")).Once() + mockL2Client.EXPECT().CustomHeaderByNumber(mock.Anything, &aggkittypes.LatestBlock). + Return(&aggkittypes.BlockHeader{Number: firstLatestBlock}, nil).Once() + mockL2Client.EXPECT().FilterLogs(mock.Anything, mock.Anything).Return([]ethtypes.Log{}, nil).Once() + + downloader, err := newDownloaderSovereign( + mockL2Client, + l2GERAddr, + mockL1InfoTreeSync, + mockL1Client, + common.HexToAddress("0x0000000000000000000000000000000000000001"), // l1GERAddr + rh, + aggkittypes.LatestBlock, + time.Millisecond*10, // waitForNewBlocksPeriod + uint64(10), // syncBlockChunkSize (unrelated to the removal-scan chunking under test) + ) + require.NoError(t, err) + + removed := downloader.isGERRemovedFromL2(context.Background(), uint64(5), firstGER) + require.False(t, removed, "no removal event found on the (single, cap-fitting) chunk") + require.Equal(t, uint64(1000), downloader.removalScanMaxRange, "the learned cap must be cached") + + // --- 2nd call (different GER/block): must go straight to the chunked path. Only the head lookup and + // one chunked FilterLogs call are stubbed; if the code repeated the unbounded call first, testify + // would panic on an unexpected FilterLogs invocation instead of matching one of these. --- + secondLatestBlock := uint64(300) + mockL2Client.EXPECT().CustomHeaderByNumber(mock.Anything, &aggkittypes.LatestBlock). + Return(&aggkittypes.BlockHeader{Number: secondLatestBlock}, nil).Once() + mockL2Client.EXPECT().FilterLogs(mock.Anything, mock.Anything).Return([]ethtypes.Log{}, nil).Once() + + removed = downloader.isGERRemovedFromL2(context.Background(), uint64(20), secondGER) + require.False(t, removed, "no removal event found on the (single, cap-fitting) chunk") + + mockL2Client.AssertExpectations(t) + mockL1Client.AssertExpectations(t) + mockL1InfoTreeSync.AssertExpectations(t) +}