Skip to content

db/version: enforce upper-bound file version check#20722

Merged
AskAlexSharov merged 11 commits into
mainfrom
alex/upper_bound_version_chk_35
Apr 22, 2026
Merged

db/version: enforce upper-bound file version check#20722
AskAlexSharov merged 11 commits into
mainfrom
alex/upper_bound_version_chk_35

Conversation

@AskAlexSharov

@AskAlexSharov AskAlexSharov commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator

Problem

Snapshot file-version checks only rejected files that were too old. Files that were too new passed the check and got read with the wrong parser.

Example: binary supports up to v2.1, directory contains v3.0-storage.0-32.efi — opened silently, no error.

Solution

Check both bounds in Versions.MustSupport and fail fast:

  • Too old → suggests erigon snapshots reset.
  • Too new → suggests upgrading Erigon, or erigon snapshots reset to align.

Logged once via sync.Once (parallel goroutines don't duplicate the banner), then panics. Callsites in dirty_files.go and block_types.go migrated; VersionTooLowPanic and its local twin deleted. Unit tests cover in-range, too-low, and too-high.

`openDirtyFiles` only checked `fileVer >= MinSupported` so files written
by a newer binary (e.g. v3.0 .efi from a branch that bumped the current
version) were silently accepted by older code that didn't understand the
new format.

Add `Versions.MustSupport(ver, filename)` as the single check point for
both bounds: panics with actionable guidance when the file is too old
(reset snapshots) or too new (upgrade erigon).  Replace all 10 manual
lower-bound-only checks in dirty_files.go and block_types.go with a
call to MustSupport.  Remove the now-unused local helpers and exported
VersionTooLowPanic.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens snapshot file compatibility checks by enforcing both the minimum supported and maximum/current supported snapshot file versions, preventing older binaries from silently opening files written by newer binaries.

Changes:

  • Added Versions.MustSupport(ver, filename) in db/version as the canonical bounds check for snapshot file versions.
  • Replaced lower-bound-only checks in state dirty-file open paths and snaptype2 indexing code with MustSupport.
  • Removed now-unused “version too low” panic helper(s).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
db/version/file_version.go Adds MustSupport() (upper + lower bound enforcement) and removes VersionTooLowPanic.
db/state/domain.go Removes local versionTooLowPanic helper no longer used.
db/state/dirty_files.go Switches dirty file opening logic to call MustSupport() for version checks.
db/snaptype2/block_types.go Switches indexing-time file version validation to MustSupport() (now enforces upper bound too).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread db/version/file_version.go
@AskAlexSharov AskAlexSharov changed the title [wip] db/version, db/state: enforce upper-bound file version check db/version, db/state: enforce upper-bound file version check Apr 22, 2026
@AskAlexSharov AskAlexSharov requested a review from JkLondon April 22, 2026 07:05
@AskAlexSharov AskAlexSharov changed the title db/version, db/state: enforce upper-bound file version check db/version: enforce upper-bound file version check Apr 22, 2026
@AskAlexSharov AskAlexSharov enabled auto-merge April 22, 2026 07:40
@AskAlexSharov AskAlexSharov added this pull request to the merge queue Apr 22, 2026
Merged via the queue into main with commit 91088e6 Apr 22, 2026
36 checks passed
@AskAlexSharov AskAlexSharov deleted the alex/upper_bound_version_chk_35 branch April 22, 2026 10:11
AskAlexSharov added a commit that referenced this pull request Apr 23, 2026
AskAlexSharov pushed a commit that referenced this pull request May 11, 2026
…21103)

- This reverts commit 91088e6.
- we need to regenerate receipts before re-adding this check back.
- should fix:

```
INFO[05-08|23:29:49.236] adjusting receipt current version to v1.1
EROR[05-08|23:29:49.476] Snapshot file is newer than this Erigon build supports: file=v3.0-receipt.187416-187420.v, highest_supported=<v3.0. To fix, either upgrade Erigon to a newer release, or align snapshots by command: `erigon snapshots reset --datadir $DATADIR --chain $CHAIN`
panic: Snapshot file is newer than this Erigon build supports: file=v3.0-receipt.187416-187420.v, highest_supported=<v3.0. To fix, either upgrade Erigon to a newer release, or align snapshots by command: `erigon snapshots reset --datadir $DATADIR --chain $CHAIN`

goroutine 136 [running]:
github.com/erigontech/erigon/db/version.Versions.MustSupport({{0x1, 0x1}, {0x1, 0x0}}, {0x3, 0x0}, {0xc001c91edf, 0x1c})
        
```

- added issue for adding this check back -
#21104
domiwei pushed a commit to domiwei/erigon that referenced this pull request May 12, 2026
Fixes erigontech#20560.

Problem: 
bloatnet 3B keys .efi file building: FuseFIlter used `t2hash=29GB` and
`reverseOrder=26GB`
we can't mmap FuseFilter without forking it. But maybe it's okey. Maybe
it's better to shard FuseFilter rather than mmap large thing. Maybe
sharding will improve data-locality.

--
Solution: 
shard `fusefilter` by first byte of `keyHash`. it will reduce building
allocation buffers 256x times. And it will not add reading overhead.
(`keyHash>>56` is not same as `byte(keyHash)`. it's `hi/lo` bytes.)

- Writer: re-using 1 `xorfilter.BuildBinaryFuse` to build all shards
- `ShardedReader` is a `[256]Reader`. Lookup:
`shards[keyHash>>56].ContainsHash(hash)`

Depends on: erigontech#20722

---------

Co-authored-by: JkLondon <me@ilyamikheev.com>
JkLondon pushed a commit that referenced this pull request May 13, 2026
Fixes #20560.

Problem: 
bloatnet 3B keys .efi file building: FuseFIlter used `t2hash=29GB` and
`reverseOrder=26GB`
we can't mmap FuseFilter without forking it. But maybe it's okey. Maybe
it's better to shard FuseFilter rather than mmap large thing. Maybe
sharding will improve data-locality.

--
Solution: 
shard `fusefilter` by first byte of `keyHash`. it will reduce building
allocation buffers 256x times. And it will not add reading overhead.
(`keyHash>>56` is not same as `byte(keyHash)`. it's `hi/lo` bytes.)

- Writer: re-using 1 `xorfilter.BuildBinaryFuse` to build all shards
- `ShardedReader` is a `[256]Reader`. Lookup:
`shards[keyHash>>56].ContainsHash(hash)`

Depends on: #20722

---------

Co-authored-by: JkLondon <me@ilyamikheev.com>
AskAlexSharov added a commit that referenced this pull request May 13, 2026
Cherry-pick of
[be36d4f](be36d4f)
(original PR #20644) onto `performance`.

## Summary
- Shards `fusefilter` by `keyHash>>56` (256 shards) to cut FuseFilter
build-time RAM ~256× on large `.efi` files (3B-key bloatnet case).
- Writer reuses one `xorfilter.BuildBinaryFuse` to build all shards;
`ShardedReader` is a `[256]Reader` with lookup
`shards[keyHash>>56].ContainsHash(hash)`.
- Adds `ExistenceFilterVersion = 2` (sharded format), keeps v1
(monolithic) and v0 (legacy) intact. `recsplit.RecSplitArgs.Version` is
now `version.DataStructureVersion` (was `uint8`).
- Wires `snaptype.BuildIndex` to pick `recsplit.ExistenceFilterVersion`
whenever the index version is not `v1.0` (v1.0 keeps inner v=0 for
compat).
- Brings the full new test set: sharded writer/reader tests, corruption
tests, fuzz tests, real-file tests, and benchmarks +
`scripts/fusefilter-bench/`.

## Cherry-pick status
Clean cherry-pick. No manual conflict resolutions required —
`performance` already has all the prerequisites the original commit
assumed:
- `xorfilter v0.5.1` (provides `MakeBinaryFuseBuilder` /
`BuildBinaryFuse`)
- Package-scope `headerSize` in
`db/datastruct/fusefilter/fusefilter_reader.go`
- `offsetFile *os.File` / `offsetWriter *bufio.Writer` in
`recsplit.RecSplit` (from PR #20722)

Auto-merge handled minor textual deltas in `.gitignore`,
`fusefilter_reader.go`, `recsplit/index.go`, and `db/state/domain.go`
without conflicts.

## Test plan
- [ ] `go build` of affected packages: passes locally
(`db/recsplit/...`, `db/datastruct/fusefilter/...`, `db/snaptype/...`)
- [ ] `go test ./db/datastruct/fusefilter/... ./db/recsplit/...`
- [ ] CI on the branch

> Note: `./db/state/statecfg/...` fails to build with `undefined:
InitSchemas` on `origin/performance` independently of this PR —
pre-existing issue on the base branch, unrelated to the cherry-pick.

## References
- Original PR: #20644
- Original issue: #20560

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Alex Sharov <AskAlexSharov@gmail.com>
Co-authored-by: JkLondon <me@ilyamikheev.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants