erigontech / erigontech/erigon

db/kv: converge DomainProgress and DomainVisibleEnd — one exact bound, one explicitly-approximate reporting number

Open
#23,004 1 comment 0 reactions 2 assignees Claimed by @AskAlexSharov View on GitHub
ErigonDB
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.