erigontech / erigontech/erigon

Unify the temporal kv GetLatest interface — eliminate accreted duck-typed metered-getter variants

Open
#21,739 3 comments 0 reactions 0 assignees View on GitHub
tech debt reduction
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
465

Description

## Problem

The temporal `kv` read path has accreted several near-duplicate ways to do the same `GetLatest`, because each incremental need is added as an **optional duck-typed interface + type-switch** rather than by changing the `kv` interface itself. Changing the interface ripples to a large number of call sites, so it's repeatedly deferred as "too big for this PR" — and the variants accumulate.

Current state (`db/state/execctx/domain_shared.go`, `getLatestMetered`):

- plain `tx.GetLatest(domain, k)` — no metering, no txN
- `MeteredGetter.MeteredGetLatest(…, metrics, start) → (v, step, ok, err)` — adds read metering + `maxStep`
- `MeteredGetterWithTxN.MeteredGetLatestWithTxN(…) → (v, step, txN, ok, err)` — adds the read's `txN` (needed to tag BranchCache entries with their unwind watermark)

The SD getter type-switches over `tx.AggTx()` to pick the richest available variant. `MeteredGetterWithTxN` differs from `MeteredGetter` only by the extra `txN` return value.

## Proposal

Make the temporal `GetLatest` API consistent: a single `GetLatest` that returns the `txN` and is metered, with metrics threaded via `ctx` (so the signature doesn't grow a `metrics` parameter). Remove the `MeteredGetter` / `MeteredGetterWithTxN` duck-typed interfaces and the type-switches that select between them.

## Related — another take on the same problem

The lock-free, channel-fed KV-read metrics collector (#21663) threads metrics through via `ctx` rather than an interface parameter. That is effectively another take on this same underlying issue (adding read-path capabilities without an interface-param explosion). The unified `GetLatest` should adopt that ctx-carried pattern instead of passing `metrics *changeset.DomainMetrics` explicitly.

## Scope / why it keeps getting deferred

Changing the `kv` temporal interface touches many call sites, so it gets punted from feature PRs. This needs its own dedicated PR.

Surfaced in review of #21380.

Contributor guide

Open the contributing guide

Research direction

Start in db/state/execctx/domain_shared.go at getLatestMetered and trace the temporal kv GetLatest call sites and the SD getter type-switch over tx.AggTx(). Read the related ctx-based metrics approach in issue #21663. Done means one metered GetLatest returns txN, metrics flow through ctx, and the MeteredGetter variants and selecting type-switches are removed.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
database
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.