erigontech / erigontech/erigon
rpc/rpchelper, rpc/jsonrpc: retire the filters param on GetBlockNumber — the tx you pass is the view you get
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 455
Description
Follow-up to #22533 (the RPC piece of the #21293 split). That PR made head-sensitive RPC paths coherent by splitting them into overlay-aware and committed-view resolution, and documented the resulting contract on `rpchelper.GetBlockNumber`. The contract works, but it leaves three view-selection conventions coexisting at `GetBlockNumber`/`GetCanonicalBlockNumber` call sites:
1. pass `api.filters` — the helper wraps the tx in the block overlay internally (a fresh pin per call) and resolves `"pending"` via `filters.LastPendingBlock()`;
2. pass `nil` filters + a caller-pinned `filters.WithOverlay(tx)` — overlay view, pinned once for the whole request;
3. pass `nil` filters + a plain tx — committed view.
The docstring #22533 adds is documenting accidental complexity: the `filters` parameter does two unrelated jobs (view selection and pending-block resolution), and the correct choice at each call site is invisible in the signature.
## Proposal
Remove the `filters *Filters` parameter from `GetBlockNumber` / `GetCanonicalBlockNumber` / `_GetBlockNumber` (and the resolution step of `CreateStateReader`). The view is determined solely by the tx the caller passes:
- plain tx → committed view;
- `filters.WithOverlay(tx)` → overlay view, pinned once and reused for every dependent read.
`"pending"` resolves to the latest executed block inside the helper. Endpoints that genuinely serve the in-memory pending block do an explicit pre-check via `filters.LastPendingBlock()` — the pattern #22533 establishes in `debug_getRawHeader` and that `eth_getBlockTransactionCountByNumber` already uses.
## Why
- **The nil-vs-non-nil contract disappears.** "The tx you pass is the view you get" cannot be misused, and the view a call site reads is visible at the call site.
- **Per-call double-pins become pin-once.** `BaseAPI.headerByNumber`/`headerByHash` currently pin the overlay twice (once inside `_GetBlockNumber`, once for the header read). Today that is safe only because `PublishOverlay(nil)` strictly follows the commit, so the plain-tx fallback already contains the head; pin-once removes the reliance on that ordering.
- **Pending handling becomes explicit and reviewable per endpoint.** Buried in the resolver, it can feed a block number that exists nowhere in the DB into DB-reading code — e.g. `CreateStateReader` with `"pending"` resolves the pending block and then builds a history reader at `pendingBlock+1`, whose txnum lookup cannot succeed. Hoisting pending out of the resolver turns this from a latent trap into a per-endpoint decision.
## Scope
Mechanical but broad: dozens of `GetBlockNumber`/`GetCanonicalBlockNumber`/`CreateStateReader` call sites across `rpc/jsonrpc` and `rpc/rpchelper`. Best done as a dedicated PR after #22533 lands, and coordinated with the in-flight overlay/semaphore rework, which touches some of the same call sites.
Contributor guide
Assessment
This issue has not been assessed yet.