Repository navigation
Add eth_getLogs to the EVM-only Giga RPC - #4308
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4308 +/- ##
==========================================
- Coverage 67.49% 66.39% -1.10%
==========================================
Files 2181 2062 -119
Lines 167798 155791 -12007
==========================================
- Hits 113251 103437 -9814
+ Misses 54537 52344 -2193
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Open and tag bounds resolve to the store's indexed head; explicit bounds beyond it or below the retention floor now fail instead of returning a partial result an indexer would mistake for a complete one.
Adds an [evm-only-rpc] section to the node config with max-blocks-for-logs and max-logs-per-query (defaults 2000 / 10000) and threads them into the EVM-only RPC server.
This reverts commit 07a9a04.
| // query may cover. | ||
| maxBlocksForLogs = 2000 | ||
| // maxLogsPerQuery is the most logs one eth_getLogs query may return. | ||
| maxLogsPerQuery = 10000 |
There was a problem hiding this comment.
Left these as constant for now, given this path might refactor and unclear where we actually want the configuaration for evm-only RPC.
There was a problem hiding this comment.
✅ a const symbol is easier to parameterize than magic values. Sufficient to advance.
PR SummaryMedium Risk Overview
Reviewed by Cursor Bugbot for commit e084061. Bugbot is set up for automated code reviews on this repo. Configure here. |
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds a read-only eth_getLogs to the EVM-only Giga RPC plus a real MemoryReceiptStore.FilterLogs; the range resolution and the log-index rebase are arithmetically correct (the subtraction is exact for Giga's block-wide stored indexes and a no-op for the legacy per-tx convention), and test coverage of the new surface is good. Remaining notes are non-blocking: a pruning race that can still yield a silently partial answer, a doubled receipt read per matching transaction, a duplicated match predicate, and no test pinning the rebase against the real littidx store.
Findings: 0 blocking | 6 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] The log-index rebase depends on a convention of the real littidx store (stored
Log.Indexis block-wide under Giga, soFilterLogsaddsfirstLogIndexa second time), but every new test runs againstMemoryReceiptStore, whoseFilterLogsthis same PR wrote to mirror that convention — so the tests cannot detect a mismatch with the store actually used in production. The litt store's own tests (TestLittIdxBlockWideLogIndex) write per-transaction-relative indexes, i.e. the opposite convention, which makes the assumption worth pinning: a littidx-backed test that writes Giga-shaped receipts (block-wideLog.Index) and assertseth_getLogslogIndexequals whateth_getTransactionReceiptreports would cover it. - [suggestion]
maxBlocksForLogs/maxLogsPerQueryhardcode 2000/10000, which are exactly the defaults of the existing, operator-tunableevm.max_blocks_for_log/evm.max_log_no_blockkeys (evmrpc/config/config.go). Two copies of the same policy that can drift, and an operator running an indexer against a Giga node has no way to tune them. Sourcing them from the existing config (or at least referencing it) would keep one source of truth. - 3 suggestion(s)/nit(s) flagged inline on specific lines.
- 1 non-blocking pre-existing issue(s) listed below under pre-existing issues.
Pre-existing issues
- [suggestion]
matchLog(sei-db/ledger_db/receipt/receipt_cache.go:271) skips wildcard topic positions without requiring that the log has a topic at that position, so criteria[A, null]match a log whose only topic isA. go-ethereum'sfilterLogsrejects any log withlen(crit.Topics) > len(log.Topics). This is the predicate the littidx store applies, so the neweth_getLogsinherits the divergence on the disk-backed path.
shemnon
left a comment
shemnon
left a comment
There was a problem hiding this comment.
My notes are quasi-optional, if not fixed in this PR then there should be one or more linear issues to address them.
…gs before materializing
|
@seidroid review |
There was a problem hiding this comment.
This PR adds a read-only eth_getLogs to the EVM-only Giga RPC and implements MemoryReceiptStore.FilterLogs. The range resolution, the invalid-params errors and the log-index rebase are correct, and the new surface is well tested. None of the three points from the previous review has been addressed yet, and none of them blocks the merge.
Findings: 0 blocking | 6 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] The log-index rebase relies on a convention of the real littidx store: under Giga the stored
Log.Indexis block-wide, andFilterLogsaddsfirstLogIndexon top of it. Every new test runs againstMemoryReceiptStore, whoseFilterLogsthis PR wrote to follow that same convention, so the tests cannot catch a mismatch with the store production actually uses. A littidx-backed test would pin it: write Giga-shaped receipts and assert that thelogIndexfrometh_getLogsmatches whateth_getTransactionReceiptreports. - [suggestion]
maxBlocksForLogs(2000) andmaxLogsPerQuery(10000) repeat the defaults of the existingevm.max_blocks_for_logandevm.max_log_no_blockkeys (evmrpc/config/config.go). The configurable version was reverted in this PR, so the two copies of this policy can drift and operators cannot tune them. If hardcoding is deliberate for now, a comment pointing to the config keys would at least keep them linked. - 3 suggestion(s)/nit(s) flagged inline on specific lines.
- 1 non-blocking pre-existing issue(s) listed below under pre-existing issues.
Pre-existing issues
- [suggestion]
matchLog(sei-db/ledger_db/receipt/receipt_cache.go:271) skips wildcard topic positions without checking that the log has a topic there, so criteria[A, null]match a log whose only topic isA. go-ethereum rejects any log wherelen(crit.Topics) > len(log.Topics). The littidx path applies this predicate, so the new endpoint inherits the divergence.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…ia IterateReceipts
…memory store MatchLog and LogsForTx are exported from sei-db/ledger_db/receipt so the in-memory store filters with the same code as littidx, and MatchLog now rejects a filter with more topic positions than the log has topics, matching go-ethereum. Adds a littidx-backed test pinning eth_getLogs logIndex to the stored receipt index, and a comment tying the hardcoded getLogs caps to the legacy evm.max_blocks_for_log / evm.max_log_no_block defaults.
|
@seidroid review |
There was a problem hiding this comment.
This PR adds a read-only eth_getLogs to the EVM-only Giga RPC. The range resolution, the pruning re-check, the littidx-backed logIndex test and the shared receipt.MatchLog/LogsForTx are correct, and the earlier predicate-duplication thread is resolved. Two non-blocking points remain. Every matching transaction's receipt is still read a second time. The stricter MatchLog also changes filtering in the legacy evmrpc littidx/cache path, and the receipt package has no test for that change.
Findings: 0 blocking | 2 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
- 2 suggestion(s)/nit(s) flagged inline on specific lines.
| // offset again when it materializes range-query logs, so this is the amount to | ||
| // subtract from each returned log's Index. | ||
| func (api *filterAPI) firstLogIndex(ctx context.Context, txHash common.Hash) (uint, error) { | ||
| stored, err := api.store.GetReceipt(receiptContext(ctx), txHash) |
There was a problem hiding this comment.
[suggestion] Still open from the last two reviews. The rebase gives the right answer, but it costs a second GetReceipt and proto decode for every distinct matching transaction, even though the store already decoded that receipt inside FilterLogs. With a 10000-log budget that is up to 10000 extra point reads, issued one after another. It also leaves logIndex silently dependent on the store adding firstLogIndex. The new littidx test now pins that convention, which helps, but the cost is unchanged. Having the store report the offset, or stop applying it, would remove both the extra reads and the coupling, with the change made in one place.
There was a problem hiding this comment.
Acknowledged; deliberately deferred to a follow-up since it needs a ReceiptStore API change (store reports firstLogIndex or stops applying it) rather than an RPC-layer fix. The littidx test in this PR pins the current convention so the follow-up can change it in one place.
…a only The shared predicate also serves the legacy evmrpc littidx path, whose block-scan fallback accepts trailing wildcard positions beyond a log's topics, so the stricter go-ethereum rule now lives in filterAPI.GetLogs instead. Adds a MatchLog unit test.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0a244e6. Configure here.
… budgeting Giga opts in via receipt.WithStrictTopicCount on the request context, so littidx and memory FilterLogs reject logs with fewer topics than filter positions before budget.Reserve. Legacy evmrpc callers keep the lenient MatchLog behavior.
| // query may cover. | ||
| maxBlocksForLogs = 2000 | ||
| // maxLogsPerQuery is the most logs one eth_getLogs query may return. | ||
| maxLogsPerQuery = 10000 |
There was a problem hiding this comment.
✅ a const symbol is easier to parameterize than magic values. Sufficient to advance.
| } | ||
| if s.latestVersion >= 0 && toBlock > uint64(s.latestVersion) { //nolint:gosec // latestVersion is non-negative. | ||
| toBlock = uint64(s.latestVersion) //nolint:gosec // latestVersion is non-negative. | ||
| it, err := s.IterateReceipts(fromBlock) |
There was a problem hiding this comment.
Less data read and less code lines. Two wins in one!
|
LGTM, my prior approval is still valid. |
* main: (21 commits) Backport evmonly parse, app-hash and changeset perf fixes from giga-1 (#4345) Remove the oracle module behind a v6.8 upgrade (#4319) fix(seidb): report only the current migration boundary on the snapshot gauge (#4327) fix(flatkv): keep 10 old checkpoints instead of mirroring memIAVL's count (#4322) Backport Autobahn execute-loop and produced-tx metrics from giga-1 (#4330) Add giga.storage.receipts to toggle the Autobahn receipt store (#4333) optimize gather phase (#4326) Regenerate the Unreleased changelog as a plain PR list (#4336) Bump sei-protocol/go-ethereum to v1.15.7-sei-21 (#4332) Fail dynamic-gas precompile out-of-gas as an EVM out-of-gas call (#4318) Add dashboard and topology option for Autobahn e2e (#4167) Add Giga fetch/serve and BlockDB prune metrics (#4329) Add eth_getLogs to the EVM-only Giga RPC (#4308) Generate v6.8 precompiles (#4320) Add [giga] app.toml section and honor it on the Autobahn node (#4323) feat(evmonly): add eth_estimateGas via existing libraries (#4325) Use Pebble batch directly in SS (#4300) Fix pruning issue in SS causing huge disk spike (#4321) Make Autobahn always run the EVM-only executor, disable/remove some integration tests (#4316) reduce seal lock contention (#4314) ...
4 participants
The EVM-only Giga RPC has no
eth_getLogs, so indexers such as sei-street, which pollgetContractEventsover 2000-block ranges, cannot follow contract events through Giga even though evmonly already persists logs and blooms and opens the indexed littidx receipt backend whoseReceiptStore.FilterLogsserves range queries.This adds a
filterAPIregistered underethwith aGetLogsmethod that handles both the range form and theblockHashform and passes the full address/topic criteria through toReceiptStore.FilterLogs. Open and tag bounds resolve to the lower of the backend head and the receipt store's latest version, and theearliesttag to the store's retention floor. Explicit bounds are never clamped: littidx indexes receipts asynchronously and may trail the committed head by a block or two, so a request for[from, eth_blockNumber]answered only up to the indexed head would let an indexer advance its cursor past logs it never saw. Instead an explicit bound above the indexed head fails with a not-yet-indexed error the client can retry, and one below the retention floor fails with a pruned error; an inverted range is rejected, as is a range wider than 2000 blocks, and each query runs under areceipt.LogBudgetof 10000 logs /DefaultMaxLogBytes. Because Giga assigns block-wide log indexes at execution time and the receipt store adds the per-transactionfirstLogIndexoffset again when materialising range-query logs,GetLogsreads each distinct transaction's receipt once and subtracts its first log index sologIndexmatches whateth_getTransactionReceiptreports; it also fillsblockHashper distinct block fromBackend.Block, since stored logs carry no block hash. Rather than another index, this deliberately reuses the existing store path; the stateful filter family andeth_subscribe("logs")are out of scope.MemoryReceiptStore.FilterLogspreviously returnedErrRangeQueryNotSupportedand now implements the same query and index conventions as the disk-backed stores, which is what lets the new RPC tests run without an on-disk store. There is no consensus or state impact; the change is confined to the read-only RPC surface. Reviewers should look most closely at the range resolution and the log-index rebase. Validated with newgiga/evmonly/rpctests covering address/topic filters, theblockHashform, bound resolution against the indexed range, range limits, budget enforcement, backend errors, and an end-to-end JSON-RPC round trip, plus the existinggiga/evmonlysuite.