rpctest: implement positional topic invariant in EthGetLogsInvariants

[lupin012]

Closes #16840.

Topics are now checked per-position with a single batched query per position (at most 4 queries per block) instead of one query per topic. The filter uses topics: [null, ..., [topics_at_pos]] so only the target position is constrained, matching the semantics of the existing address batch check.

[Copilot]

Pull request overview

This PR optimizes rpctest’s EthGetLogsInvariants topic checks by switching from per-topic queries to positional topic batching (up to 4 queries per block, one per topic position), aligning the semantics with how address batching is already handled.

Changes:

• Add RequestGenerator.getLogsForTopics to build positional eth_getLogs topic filters using null wildcards for unconstrained positions.
• Update EthGetLogsInvariants to collect unique topics per position (0–3) and validate indexing via batched per-position RPC calls.
• Add filterLogsByTopicAtPos helper to validate per-topic presence and duplicate constraints from the batched response.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
cmd/rpctest/rpctest/request_generator.go	Adds a request builder for positional topics filters in eth_getLogs.
cmd/rpctest/rpctest/bench_ethgetlogs.go	Implements positional topic invariants using per-position topic batching and adds filtering helper.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yperbasis]

Review summary

Re-enables the topic invariant in invariantsEthGetLogs using one batched positional query per topic position (topics: [null, …, [topics_at_pos]]), mirroring the existing address batching. Sound approach and well-scoped — closes #16840.

I confirmed the package builds on the PR head, the "0x%x" hash formatting matches getLogs1/getLogsForAddresses (no double 0x), and both earlier Copilot comments were addressed (single-pass logsByTopic grouping; continue on the failing call).

Requesting changes on one correctness issue, plus one lower-severity hygiene point — details inline:

1. [Medium] Stale resp.Error leaks across the reused resp. resp is reused for every Erigon(..., &resp) call and post() never resets it; a successful JSON-RPC response omits "error", so a previously-set resp.Error survives json.Unmarshal. With failFast=false, one position's RPC error makes every later position falsely log an error and continue past its check.

2. [Low] topicsByPos is built from the address-filtered response rather than the unfiltered baseline. Better to populate it in the same loop that builds sawAddr, off the original getLogsNoFilters result.

Optional nit (non-blocking): getLogsForTopics is a pure string builder and would be cheap to pin with a unit test asserting the null-wildcard serialization for nil vs. non-nil positions.

[lupin012]

@yperbasis fixed comments

[yperbasis]

The two earlier comments are addressed correctly — thanks. One follow-up that the new reset introduces:

Address-check loop needs a skip-on-error guard.

The resp = EthGetLogs{} reset at bench_ethgetlogs.go:214 is right, but with failFast=false (the default) a failed getLogsForAddresses now falls through to for k := range sawAddr (:231) with an empty resp.Result — post() skips json.Unmarshal on a failed call — so filterLogsByAddr returns nothing for every address and logs a false "addr not indexed" for every address in the block. Before the reset this was a vacuous pass off the stale baseline; the reset flips it from false-negative to false-positive.

continue won't work here (straight-line code in the eg.Go closure, not a loop), so guard the loop the way the topic loop is guarded by its continue:

res = reqGen.Erigon("eth_getLogs", reqGen.getLogsForAddresses(bn, bn, slices.Collect(maps.Keys(sawAddr))), &resp)
addrOK := true
if res.Err != nil {
    if failFast {
        return fmt.Errorf("could not get modified accounts (Erigon): %v", res.Err)
    }
    log.Error("[ethGetLogsInvariants] could not get modified accounts (Erigon)", "blockNum", bn, "error", res.Err.Error())
    addrOK = false
}
if resp.Error != nil {
    if failFast {
        return fmt.Errorf("error getting modified accounts (Erigon): %d %s", resp.Error.Code, resp.Error.Message)
    }
    log.Error("[ethGetLogsInvariants] error getting modified accounts (Erigon)", "blockNum", bn, "error", resp.Error.Code, "message", resp.Error.Message)
    addrOK = false
}
if addrOK {
    for k := range sawAddr {
        // ... existing checks ...
    }
}

(The baseline block at :180 has the same missing guard but is harmless — resp starts fresh there, so a failure just yields empty sawAddr/topicsByPos and skips downstream checks. Worth aligning for consistency.)

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.