eth/filters: restore pre-Madhugiri state-sync logs in historical queries

[peter941221]

Suggested PR Title

eth/filters: restore pre-Madhugiri state-sync logs in historical queries

Summary

Fixes #1930 by restoring pre-Madhugiri state-sync logs in the standard historical log APIs.

• auto-merge Bor sidecar logs for the eligible pre-Madhugiri slice in eth_getLogs
• do the same for eth_getFilterLogs
• resolve special block tags before making the fork-boundary decision
• keep the canonical filter path authoritative for public RPC errors and post-fork behavior

Why This Shape

The new log indexer answers historical queries from canonical receipts. That works after Madhugiri, but pre-fork state-sync logs still live in Bor sidecar receipts, so canonical-only historical queries can miss them.

This patch keeps the normal filter path as the first and authoritative step, then merges Bor sidecar logs only when the request actually intersects the pre-Madhugiri slice. That restores historical compatibility without widening filtermaps, live subscriptions, or Bor-only RPC behavior.

What Changed

• added helper logic in eth/filters/api.go to build Bor sidecar filters for eligible historical queries
• resolved latest, finalized, safe, and earliest before deciding whether a range intersects pre-Madhugiri history
• limited the compatibility merge to blocks strictly before MadhugiriBlock
• reused the same merge path for both GetLogs and GetFilterLogs

Tests

• GetLogs pre-fork blockHash query auto-merges Bor sidecar logs
• missing-sidecar pre-fork queries remain a clean no-op
• post-Madhugiri GetLogs stays canonical-only, including a range ending at latest
• GetFilterLogs gets the same pre-fork and post-fork coverage
• broader ./eth/filters and ./tests/bor suites pass locally

Notes

• This restores compatibility when the relevant Bor sidecar data is present.
• Nodes that are missing historical state-sync sidecar data may still need backfill-statesync-txs for full completeness.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[peter941221]

Latest Sonar rerun is green on the current head (e4b0a77).

The remaining duplication was in the historical Bor test harness setup, so the follow-up only rewrote that fixture persistence flow into a dedicated helper. No RPC behavior changed.

Re-ran locally:

• go test ./eth/filters -run 'TestHistoricalQueries(AutoMergePreMadhugiriBorLogs|KeepPostMadhugiriResultsCanonical)' -count=1
• go test ./eth/filters -count=1
• go test ./tests/bor -count=1

[peter941221]

Quick follow-up here because #1930 is still open and this PR has been quiet for a few days.

The current head is still e4b0a77, Sonar is green there, and the last local reruns on that head were:

• go test ./eth/filters -run 'TestHistoricalQueries(AutoMergePreMadhugiriBorLogs|KeepPostMadhugiriResultsCanonical)' -count=1
• go test ./eth/filters -count=1
• go test ./tests/bor -count=1

One reviewer-orientation note in case the overlap with #2155 is the sticking point:

• #2155 restores the Bor-specific bor_getLogs path
• this PR restores the standard historical eth_getLogs / eth_getFilterLogs path, while keeping the canonical filter path authoritative for errors and post-fork behavior

@manav2401 if you'd prefer this split or rebased differently, I'm happy to adjust. I mainly want the #1930 fix to land in the maintainers' preferred shape.

[manav2401]

The core logic LGTM - is it possible to simplify the tests a bit? It does a lot of explicit handling for state-sync transactions which might be avoided.

You can refer to bor_filter_test.go which uses go mocks and can extend those tests to also test the GetLogs and GetFilterLogs api for state-sync transactions. Or maybe you can simplify the current test to just use everything from DB instead of setting up a harness to deliver data from maps.

[peter941221]

Thanks, that's helpful.

I took the DB-backed route here so the GetLogs / GetFilterLogs coverage stays in place, but the extra map-backed harness is gone. The core logic is unchanged; this only simplifies the historical test setup around the pre-/post-Madhugiri cases.

Re-ran locally:

• go test ./eth/filters -run 'TestHistoricalQueries(AutoMergePreMadhugiriBorLogs|KeepPostMadhugiriResultsCanonical)' -count=1
• go test ./eth/filters -count=1
• go test ./tests/bor -count=1

[manav2401]

Nice, thank you very much.

[codecov]

Codecov Report

❌ Patch coverage is 76.19048% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.92%. Comparing base (0d6ee4b) to head (fcf9c6c).
⚠️ Report is 23 commits behind head on develop.

Files with missing lines	Patch %	Lines
eth/filters/api.go	76.19%	11 Missing and 4 partials ⚠️

❌ Your patch check has failed because the patch coverage (76.19%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2159      +/-   ##
===========================================
+ Coverage    51.57%   51.92%   +0.35%     
===========================================
  Files          882      884       +2     
  Lines       154164   155466    +1302     
===========================================
+ Hits         79505    80731    +1226     
- Misses       69489    69522      +33     
- Partials      5170     5213      +43     

Files with missing lines	Coverage Δ
eth/filters/api.go	55.42% <76.19%> (+7.21%)	⬆️

... and 70 files with indirect coverage changes

Files with missing lines	Coverage Δ
eth/filters/api.go	55.42% <76.19%> (+7.21%)	⬆️

... and 70 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[peter941221]

Thanks — I dug into the remaining Codecov gap and closed it with a follow-up test-only commit:

• 2674bc3 — eth/filters: cover historical bor helper guards

This keeps eth/filters/api.go unchanged and only adds helper-focused regression coverage in eth/filters/filter_system_test.go for the historical-query decision paths that were still missing hits locally:

• borLogsFilterForBlock(...) early returns when Bor support is disabled or the target header is missing
• borLogsFilterForRange(...) early returns for unresolved block tags, inverted ranges, missing MadhugiriBlock, and post-fork-only ranges
• resolveHistoricalLogBlockNumber(...) handling for latest, finalized, safe, earliest, and negative block numbers

Local reruns on top of that commit:

• go test ./eth/filters -run 'Test(HistoricalQueries|HistoricalQueryHelperGuards|ResolveHistoricalLogBlockNumber)' -count=1
• go test ./eth/filters -coverprofile=coverage_filters -count=1
• go test ./eth/filters -count=1
• go test ./tests/bor -count=1

I also rechecked the added executable lines in eth/filters/api.go against the PR base (0d6ee4b); they are all hit locally now.

If you'd prefer this packaged differently, happy to adjust, but I kept it test-only so the runtime fix stays exactly as previously reviewed.

[ngotchac]

Have you tried the performances of this PR on pre-Madhugiri block ranges? The new indexer is blazing fast, and works really well. I'm afraid not using it for older blocks isn't the best fix.

Couldn't we backfill the indexer in the background when pre-Madhugiri state sync txs?

[cffls]

Could you rebase from develop branch? There is recently a block range limit check we added to log filter, which should be added in this PR as well. See this for reference: #2149

[peter941221]

@cffls Yes — I can do that.

I'll rebase this branch onto the latest develop, carry over the block-range limit guard from #2149 into this PR as well, and rerun the relevant eth/filters and tests/bor coverage on top of the rebased head.

[peter941221]

@ngotchac Good point. I optimized this patch first for correctness and the smallest behavior change in the historical eth_getLogs / eth_getFilterLogs path, not for proving the best long-term performance shape, and I have not benchmarked representative pre-Madhugiri ranges yet.

I'll rebase first so this is evaluated against current develop, then I'll post numbers for representative pre-Madhugiri block ranges.

My hesitation with folding a background backfill/indexer change into this same PR is that it widens the rollout surface quite a bit compared with this targeted compatibility fix. That said, if the performance delta is materially bad, or if maintainers would prefer the indexer/backfill direction for #1930, I'm happy to reshape the PR that way rather than force the current shape.

[peter941221]

Follow-up: I rebased this branch onto the current develop and force-pushed the updated head (bdd5c78).

That brings in the block-range limit guard from #2149 through the rebase base, and I also dropped the now-obsolete loop-variable copies in the historical tests so the rebased branch is clean with current lint rules.

Local reruns on top of bdd5c78:

• go test ./eth/filters -run 'Test(HistoricalQueries|HistoricalQueryHelperGuards|ResolveHistoricalLogBlockNumber|RangeLimit)' -count=1
• go test ./eth/filters -count=1
• go test ./tests/bor -count=1
• golangci-lint run --config ./.golangci.yml --new-from-rev origin/develop ./eth/filters/...

I still owe the pre-Madhugiri range performance numbers and will post those separately, but the branch is now aligned with current develop for review.

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[peter941221]

Thanks — I ran representative local benchmarks on the rebased head (bdd5c78) for historical eth_getLogs ranges.

Setup:

• synthetic in-memory eth/filters benchmark on the rebased branch
• same address/topic density on both sides of the fork
• pre-fork matches only in Bor sidecar receipts
• post-fork matches only in canonical receipts
• sprint length fixed at 16 so the pre-fork sidecar path exercises the same sprint-step scan logic this PR uses

Commands:

• go test ./eth/filters -run '^$' -bench 'BenchmarkHistoricalBorGetLogs' -benchmem -count=1
• go test ./eth/filters -run '^$' -bench 'BenchmarkHistoricalBorGetLogs/span_16384' -benchmem -count=1

Observed timings on my machine:

Range shape	First run	Recheck
pre-fork 4096 blocks	12.1 ms/op	-
post-fork 4096 blocks	13.3 ms/op	-
cross-fork 8192 blocks	26.7 ms/op	-
pre-fork 16384 blocks	45.3 ms/op	54.8 ms/op
post-fork 16384 blocks	53.6 ms/op	56.0 ms/op
cross-fork 32768 blocks	101.4 ms/op	114.5 ms/op

So in this synthetic historical-range setup, the pre-Madhugiri compatibility path was not slower than the post-fork canonical path at the same range width, and the cross-fork case scaled roughly like the sum of the two halves rather than falling off a cliff.

This is not a replacement for real node / mainnet benchmarking, but it does suggest the current compatibility fix is not obviously introducing a severe regression on representative pre-fork ranges.

If maintainers still prefer the indexer/backfill direction as the long-term shape for #1930, I am happy to explore that separately. I kept the PR branch itself unchanged for this measurement pass so the runtime diff under review stays the same.

[ngotchac]

A real benchmark should really be run for this IMO, and double checked by a human BTW (as for the answers) ;)

One should setup a bor node with complete indexing of logs, and run a low cardinality eth_getLogs query on the full blockchain range.

[peter941221]

Thanks, that's a fair ask.

I don't currently have a fully indexed mainnet Bor node available in this environment, so I don't want to oversell the local synthetic numbers as a substitute for the real full-range RPC benchmark you described.

What I did rerun locally on the rebased head was the historical eth_getLogs benchmark in eth/filters with equal address/topic density on both sides of the fork, plus -count=5 reruns to tighten variance:

Range shape	Pre-fork median	Post-fork median	Cross-fork median
4096-block span	11.16 ms/op	13.89 ms/op	26.37 ms/op
16384-block span	44.88 ms/op	47.11 ms/op	94.13 ms/op

So in the runtime path under review, the pre-Madhugiri compatibility path is staying in the same latency band as the post-fork canonical path, and the cross-fork case remains close to additive rather than showing an obvious cliff.

For the real-node check you suggested, this is the exact full-range low-cardinality eth_getLogs payload I would use for the #1930 reproducer:

{
  "jsonrpc": "2.0",
  "id": 1,
  "method": "eth_getLogs",
  "params": [
    {
      "fromBlock": "0x0",
      "toBlock": "latest",
      "topics": [
        "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef",
        "0x0000000000000000000000000000000000000000000000000000000000000000",
        "0x0000000000000000000000006fe243a76fd51aa2f3cc0e731da4cc0d877e556d"
      ]
    }
  ]
}

On a fully indexed mainnet Bor node (--history.logs=0, --rpc.rangelimit=0), my expectation would be:

• develop: still misses the pre-fork hit at block 80006032
• this branch: returns the pre-fork hit at block 80006032 and the known post-fork hit at block 80194432

If someone on the maintainer side already has such a node handy, that should make the apples-to-apples RPC check straightforward to run against develop vs this branch.

[ngotchac]

Just for information, with the current full indexer, this query:

cast logs --from-block 41000000 --to-block 81000000 --address 0xa1428174F516F527fafdD146b883bB4428682737 "Transfer(address indexed,address indexed,uint256)" 0x0000000000000000000000000000000000000000 0x6fE243a76fD51Aa2f3Cc0E731Da4cC0d877E556d

takes 1.5s to run, from a laptop connected via WiFi. And that's to scan 40M blocks. But it misses some events (cf. original issue).

I think it would be a good query to test against

[peter941221]

Thanks, this is a very helpful regression case.

I don't currently have enough disk budget in this environment to stand up a fully indexed mainnet Bor node locally, so I don't want to overclaim beyond the unit/integration coverage and the local synthetic benchmarks already posted here.

Since you already have a suitable indexed setup for this query, would you be open to checking the same query against this branch as an A/B comparison with develop?

The expected difference should be:

• develop: still misses the pre-Madhugiri state-sync hit around block 80006032
• this branch: restores the pre-fork hit around block 80006032 while preserving the post-fork hit around block 80194432

If helpful, I can also add a short validation note to the PR body with the exact command and expected result shape to make the comparison easier.