From 2e0ad2cb4d11cc9dd094e3e2c1d196d08079ef91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felf=C3=B6ldi=20Zsolt?= Date: Fri, 18 Apr 2025 13:39:11 +0200 Subject: [PATCH] core/filtermaps: only use common ancestor snapshots (#31668) This PR makes the conditions for using a map rendering snapshot stricter so that whenever a reorg happens, only a snapshot of a common ancestor block can be used. The issue fixed in https://github.com/ethereum/go-ethereum/pull/31642 originated from using a snapshot that wasn't a common ancestor. For example in the following reorg scenario: `A->B`, then `A->B2`, then `A->B2->C2`, then `A->B->C` the last reorg triggered a render from snapshot `B` saved earlier. Now this is possible under certain conditions but extra care is needed, for example if block `B` crosses a map boundary then it should not be allowed. With the latest fix the checks are sufficient but I realized I would just feel safer if we disallowed this rare and risky scenario altogether and just render from snapshot `A` after the last reorg in the example above. The performance difference if a few milliseconds and it occurs rarely (about once a day on Holesky, probably much more rare on Mainnet). Note that this PR only makes the snapshot conditions stricter and `TestIndexerRandomRange` does check that snapshots are still used whenever it's obviously possible (adding blocks after the current head without a reorg) so this change can be considered safe. Also I am running the unit tests and the fuzzer and everything seems to be fine. --- core/filtermaps/map_renderer.go | 1 + 1 file changed, 1 insertion(+) diff --git a/core/filtermaps/map_renderer.go b/core/filtermaps/map_renderer.go index cd960ad31c..23d84f0eca 100644 --- a/core/filtermaps/map_renderer.go +++ b/core/filtermaps/map_renderer.go @@ -143,6 +143,7 @@ func (f *FilterMaps) lastCanonicalSnapshotOfMap(mapIndex uint32) *renderedMap { var best *renderedMap for _, blockNumber := range f.renderSnapshots.Keys() { if cp, _ := f.renderSnapshots.Get(blockNumber); cp != nil && blockNumber < f.indexedRange.blocks.AfterLast() && + blockNumber <= f.indexedView.headNumber && f.indexedView.getBlockId(blockNumber) == cp.lastBlockId && blockNumber <= f.targetView.headNumber && f.targetView.getBlockId(blockNumber) == cp.lastBlockId && cp.mapIndex == mapIndex && (best == nil || blockNumber > best.lastBlock) { best = cp