fix(evmrpc): apply getLogs maxLog cap during merge instead of after (PLT-687)

[amir-deris]

Summary

eth_getLogs / sei_getLogs previously ran the k-way heap merge (mergeSortedLogs) to completion and then truncated the result to maxLog. The merge had no early exit, so the full merged result set was allocated regardless of the cap value.

This pushes the cap into mergeSortedLogs via a limit parameter: the merge stops popping the heap once limit logs are collected, and the result slice is pre-sized to min(total, limit). Output is identical to before (same maxLog, same open-ended gating) — only the wasted allocation and heap work are removed.

Scope / known limitations

This is a deliberately minimal fix matching the ticket (PLT-687). It does not fully bound peak memory for large queries — follow-up scope is tracked separately:

• Inputs are still fully materialized. The per-batch sorted slices (sortedBatches) hold all matching logs before the merge runs, so peak memory drops from ~O(2·total) to O(total + maxLog), not to O(maxLog). What this eliminates is the redundant merged-copy allocation and the wasted heap pops beyond the cap.
• Only the block-by-block fallback path is affected. The primary range-query path (tryFilterLogsRange → litt FilterLogs → filterLogsByTags) returns before this code and remains uncapped. A true memory bound requires pushing the limit into that path with ordered/windowed early termination.
• Only open-ended queries are capped (applyOpenEndedLogLimit, i.e. missing fromBlock/toBlock); bounded {fromBlock, toBlock} queries are not capped on any path. Whether to cap bounded queries — and whether to truncate silently vs. return an error — is an open product decision deferred to the follow-up.

Test plan

• Added TestMergeSortedLogs (evmrpc/filter_bounds_test.go, internal package) covering: no-limit full ordered merge; positive limit returns the globally-smallest prefix and bounds cap(res) to the limit; limit >= total; and empty batches.
• go test ./evmrpc/ -run 'TestMergeSortedLogs|Logs|GetLogs|Filter' — pass.

Note: the merge is unit-tested in isolation; there is no new end-to-end test asserting the cap over a live getLogs call, since the HTTP fixtures don't produce enough logs in a single open-ended query to trip MaxLogNoBlock. An e2e assertion belongs with the follow-up that reworks the cap semantics.

[cursor]

PR Summary

Low Risk
Behavior-preserving optimization on log merge only; open-ended cap semantics unchanged and covered by new unit tests.

Overview
Open-ended getLogs on the block-by-block fallback path no longer merges every matching log and then truncates to maxLog. The cap is passed into mergeSortedLogs, which pre-sizes the result to min(total, limit), stops heap pops once limit entries are collected, and returns the same globally sorted prefix as before.

TestMergeSortedLogs exercises full merge (limit == 0), early truncation with bounded cap(res), limits above total size, and empty batches.

^{Reviewed by Cursor Bugbot for commit 994c3ee. Bugbot is set up for automated code reviews on this repo. Configure here.}

[github-actions]

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	✅ passed	✅ passed	✅ passed	Jun 30, 2026, 1:02 AM

[seidroid]

A clean, minimal performance fix that pushes the maxLog cap into the k-way merge so it stops early instead of merging fully and then truncating; output is behaviorally identical and the change is well-tested. No blockers found, and both second-opinion passes reported nothing.

Findings: 0 blocking | 3 non-blocking | 0 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Codex and Cursor second-opinion passes both returned no material findings (Cursor file was empty; Codex noted it could not run the new test because the sandbox couldn't download the Go 1.25.6 toolchain).
• As the PR description candidly notes, this does not bound peak memory: sortedBatches still fully materializes all matching logs before the merge, and the primary range-query path remains uncapped. This is acknowledged as deliberate follow-up scope (PLT-687), not a defect in this PR.
• Consider an end-to-end test asserting the cap over a live getLogs call in the tracked follow-up, since this PR only unit-tests the merge in isolation (also acknowledged in the description).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.21%. Comparing base (b8c3929) to head (994c3ee).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3666      +/-   ##
==========================================
- Coverage   59.17%   58.21%   -0.96%     
==========================================
  Files        2262     2178      -84     
  Lines      187055   177435    -9620     
==========================================
- Hits       110690   103294    -7396     
+ Misses      66427    64995    -1432     
+ Partials     9938     9146     -792     

Flag	Coverage Δ
sei-chain-pr	68.71% <100.00%> (?)
sei-db	70.41% <ø> (ø)
sei-db-state-db	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
evmrpc/filter.go	66.86% <100.00%> (+1.17%)	⬆️

... and 110 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.

[claude]

LGTM, behavior-preserving optimization that pushes the maxLog cap into the k-way merge to skip wasted work.

Extended reasoning...

Overview

This PR modifies evmrpc/filter.go's mergeSortedLogs to accept a limit parameter and early-exit the heap pop loop once limit logs are collected, plus pre-sizes the result slice to min(totalSize, limit). The caller (GetLogsByFilters) passes filterConfig.maxLog when applyOpenEndedLogLimit is true, else 0 (no cap, preserving prior behavior). A unit test TestMergeSortedLogs is added covering no-limit, positive limit (global prefix + bounded cap), limit ≥ total, and empty batches.

Security risks

None apparent. This is purely an internal optimization — no new input is parsed, no auth/permission boundary is touched, and the cap was already enforced by the slice truncation that this replaces. Output is identical to before (same globally-smallest prefix, same maxLog ceiling).

Level of scrutiny

Low. The change is a few-line refactor of an internal merge helper with the same semantics as the prior res[:maxLog] truncation — limit == 0 means no cap, matching the prior 'apply only when applyOpenEndedLogLimit' gate. The unit test verifies both ordering (global prefix from a min-heap) and the allocation bound. The PR description is unusually explicit about scope (range-query path still uncapped, bounded queries still uncapped, inputs still fully materialized), which makes the diff easy to evaluate.

Other factors

Bug hunter found nothing. PR is two files, ~30 production lines, with the only behavioral risk being whether the early-break preserves the globally-smallest prefix — which it does, because a min-heap pop always yields the smallest remaining item. The 'non-app-hash-breaking' label is appropriate (RPC-only).

[seidroid]

A minimal, correct performance fix that pushes the eth_getLogs/sei_getLogs maxLog cap into the k-way heap merge, stopping early and pre-sizing the result to min(total, maxLog) instead of merging everything and truncating after. Output is behaviorally identical to before and the change is well-tested.

Findings: 0 blocking | 4 non-blocking | 0 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Output equivalence verified: applyOpenEndedLogLimit requires maxLog > 0 (filter.go:1197) and maxLog is defaulted positive (filter.go:286-287), so limit is always > 0 when capping applies — the limit == 0 no-cap branch is never hit on the capped path, and results match the old post-merge truncation exactly.
• As the PR description itself notes, this is a deliberately partial fix: inputs (sortedBatches) are still fully materialized, the primary tryFilterLogsRange path remains uncapped, and bounded {fromBlock,toBlock} queries are not capped. Peak memory drops to O(total + maxLog), not O(maxLog). These are acknowledged follow-ups, not defects in this PR.
• Second-opinion passes produced no material findings: codex-review.md reported none (with a note it couldn't run the new test due to a Go 1.25.6 toolchain mismatch in its sandbox), and cursor-review.md was empty. REVIEW_GUIDELINES.md was also empty/missing, so no repo-specific standards were applied beyond the general assessment.
• Consider a follow-up end-to-end test asserting the cap over a live getLogs call; the PR notes existing HTTP fixtures don't generate enough logs to trip MaxLogNoBlock, so only the isolated unit test exists today.

[claude]

LGTM — small, behavior-preserving optimization with thorough unit tests.

Extended reasoning...

Overview

This PR modifies the existing LogFetcher.mergeSortedLogs k-way heap merge in evmrpc/filter.go to accept a limit parameter. The cap is now applied during the merge (heap pop loop terminates early; result slice is pre-sized to min(total, limit)) rather than after a full merge followed by a slice truncation. A new TestMergeSortedLogs covers the no-limit case, positive limit (asserting both the global-smallest prefix and bounded cap(res)), limit >= total, and empty batches.

Security risks

None apparent. The change is purely an allocation/CPU optimization on an internal merge step. No auth, crypto, input-parsing, or permission surface is touched. The visible behavior (which logs are returned, in what order, with what cap) is unchanged — the only difference is that work and memory beyond the cap are avoided.

Level of scrutiny

Low. The diff is ~30 LOC of straightforward refactor in a single function, the cap logic is gated by limit > 0 (a zero value preserves the prior unbounded behavior), and there are no call-site changes outside the one caller in GetLogsByFilters, which simply forwards maxLog (or 0) based on the existing applyOpenEndedLogLimit flag. The semantics are identical to the pre-existing res = res[:int(f.filterConfig.maxLog)] truncation.

Other factors

The bug hunting system found no issues. Codecov reports 100% coverage on the modified lines. The PR description is explicit about the scope and known limitations (inputs still materialized, range-query path untouched, bounded queries still uncapped) — all of those are pre-existing characteristics that this PR does not alter. Cursor Bugbot independently rated this Low Risk.