erigontech / erigontech/erigon
db/kv: converge DomainProgress and DomainVisibleEnd — one exact bound, one explicitly-approximate reporting number
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 455
Description
## Motivation
`kv.TemporalDebugTx` now exposes two similar-looking per-domain progress numbers with silently different boundary semantics:
- `DomainProgress` — `max(filesEnd, lastTxNumInDB)`: an **exclusive** files end mixed with an **inclusive** DB txNum, so the result is ±1 depending on which component wins. For `HistoryDisabled` domains it falls back to a step-granular guess (its own comment: "this is not accurate, okay for reporting").
- `DomainVisibleEnd` (added in #22444) — `max(filesEnd, lastTxNumInDB+1)`: a uniformly **exclusive** bound (a view with frontier N sees txNums < N), or an honest `ok=false` when no exact bound exists (`HistoryDisabled`, remote).
This is a standing trap. The near-miss already happened once: the old cache fill path stamped entries with `DomainProgress`, and #22444 exists partly because that number was ±1-fuzzy exactly where fuzziness was fatal. Someone will reach for `DomainProgress` in a correctness context again.
Note the two are **not mergeable into one method**:
1. `DomainProgress` is not derivable from `DomainVisibleEnd`: when files dominate, Progress equals VisibleEnd; when the DB dominates, it is VisibleEnd−1. Its semantics are accidental, which argues for migrating callers off it — and also means migration is a per-call-site semantic review, not a mechanical refactor.
2. The `HistoryDisabled` fallback is load-bearing: reporting/validator callers need *some* number for domains where an exact bound honestly does not exist.
## Scope
1. Audit each `DomainProgress` call site and migrate the ones that want an exact bound to `DomainVisibleEnd`, with per-site ±1 review:
- `db/integrity/`: `rcache_no_duplicates.go`, `rcache_receipt_root.go`, `receipts_no_duplicates.go` (incl. `ValidateDomainProgress`)
- `db/state/squeeze.go` (rebuild resume: `FindBlockNum(domainTxNum)`)
- `execution/stagedsync/stage_custom_trace.go` (`progressOfDomains`, resume points, finish logging)
- `execution/commitment/backtester/backtester.go`
- `cmd/integration/commands/commitment.go`, `cmd/integration/commands/reset_state.go`, `cmd/utils/app/snapshots_cmd.go`
2. Keep `DomainProgress` only for reporting / step-granular uses; consider a name that says so, or at minimum keep the doc fence added in #22444.
3. Decide the `HistoryDisabled` story explicitly: which callers genuinely need the step-granular guess, and whether they should receive it under a clearly-approximate name.
Not blocking #22444 — this is follow-up hygiene so the exact and the approximate number stop looking interchangeable.
Contributor guide
Assessment
This issue has not been assessed yet.