Skip to content

db, execution: drop Bor snapshots and the Bor block-reader interface methods - #23494

Merged
AskAlexSharov merged 13 commits into
mainfrom
awskii/polygon-removal-db-storage
Aug 22, 2026
Merged

db, execution: drop Bor snapshots and the Bor block-reader interface methods#23494
AskAlexSharov merged 13 commits into
mainfrom
awskii/polygon-removal-db-storage

Conversation

@awskii

@awskii awskii commented Aug 22, 2026

Copy link
Copy Markdown
Member

The Bor snapshot type, its retire/merge path and the two Bor-named methods on the generic block-reader interfaces existed only to carry Heimdall data. With nothing constructing them any more, this removes them, which takes db/ off the list of packages importing polygon/. Fifth in the Polygon removal series, branched off #23492 — review the last commit only.

Changes

  • delete db/snapshotsync/freezeblocks/bor_snapshots.go
  • BlockRetire loses its Heimdall and bridge stores, BorStore(), borSnapshots(), the Bor prune metric and the Bor-data-not-ready backoff; NewBlockRetire drops two parameters
  • BlockReader loses its borSn field, BorSnapshots() and FrozenBorBlocks(); NewBlockReader takes one snapshot set instead of two
  • remove FrozenBorBlocks and BorSnapshots from dbservices.FullBlockReader, FrozenBorBlocks from the engine-facing rules.ChainHeaderReader, and BorSnapshots from snapshotsync's reader interface, along with all seven implementations
  • remotedbserver.NewKvServer drops its Bor snapshot parameter
  • db/rawdb: ReadChainConfig now rejects a stored bor section instead of decoding it
  • execution/stagedsync: drop the Bor branches from the snapshots stage, including the synchronous-indexing exception
  • db/datadir: keep removing the legacy heimdall and polygon-bridge directories, by literal name now that the DB labels are on their way out

Notes

ReadChainConfig used to rehydrate Config.Bor from the stored bor JSON. Dropping that silently would let an existing Polygon chaindata load with Bor == nil and execute under the wrong consensus rules, so it now fails with a pointer to 0xPolygon/erigon. This is the second of the three rehydration sites; txnprovider/txpool is the last and goes with chain.Config.Bor.

The heimdall and polygon-bridge datadir cleanup is deliberately kept: anyone upgrading from a Polygon datadir still wants those directories removed.

The 15 kv.Bor* tables, the HeimdallDB / PolygonBridgeDB labels and snaptype.MinBorEnum are not touched here. polygon/heimdall and polygon/bridge are their only readers and polygon/heimdall pins the snapshot enum range, so they can only go in the same PR that deletes the tree. Removing them earlier would just mean editing code that is about to disappear.

The WitnessProcessing stage stays for the same reason: it reads kv.BorWitnesses.

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

# Conflicts:
#	cmd/downloader/main.go
#	cmd/integration/commands/stages.go
#	node/eth/backend.go
# Conflicts:
#	cmd/integration/commands/stages.go
@AskAlexSharov

Copy link
Copy Markdown
Collaborator

Review of the current head. The main issue: the new ReadChainConfig rejection in db/rawdb/accessors_metadata.go is a one-sided guard — the write path and the second reader do not agree with it.

Bugs

1. Write side still produces what the read side rejectsdb/rawdb/accessors_metadata.go:63

WriteChainConfig still marshals cfg.Bor into BorJSON. Any code that commits a genesis with Config.Bor != nil (e.g. polygon/tests/helper/miner.go:56) writes a bor section into kv.ConfigTable that no later read can load — rawdb.ReadChainConfig returns the new error and db/fromdb/fromdb.go:40 turns it into a panic. The process bricks its own datadir on the first write. If a stored bor section is unsupported, the write path must refuse it too, or the marshalling block should go.

2. chain.GetConfig has no guardexecution/chain/chain_db.go:29

It decodes the same kv.ConfigTable row and returns a config with Bor == nil, BorJSON != nil. txnprovider/txpool/pool_db.go:172 calls it and then initBor (pool_db.go:138) rehydrates Bor, so the txpool happily applies Bor rules while rawdb.ReadChainConfig refuses the same row for the node. polygon/bridge/snapshot_integrity.go:29 gets the Bor == nil version with no rehydration at all. The guard belongs in the shared decode step, not in one of the two readers.

3. Stale caller guard swallows the new errorexecution/state/genesiswrite/genesis_write.go:200

storedCfg, storedErr := rawdb.ReadChainConfig(tx, storedHash)
if storedErr != nil && newCfg.Bor == nil {
	return newCfg, nil, storedErr
}

With a Bor genesis the condition is false, the error is dropped, and storedCfg == nil wins: it logs "Found genesis block without chain config", calls WriteChainConfig(newCfg) and skips CheckCompatible. Before this change that error path was only reachable on malformed JSON; now it is reachable for well-formed data, so the newCfg.Bor == nil half should go in the same change.

4. The kept heimdall/polygon-bridge cleanup in reset is now unreachabledb/datadir/reset/reset.go:59

erigon snapshots reset on a Polygon datadir: getChainNameFromChainData (cmd/utils/app/reset-datadir.go:175) now gets the "carries a 'bor' section" error, chainNameErr is only warned, configChainName stays unset, and resetCliAction returns "chain flag not set and chain name not found in chaindata" before r.Run(). --chain bor-mainnet does not help either — snapcfg.KnownCfg then fails with "config for chain ... is not known". The PR body says anyone upgrading from a Polygon datadir still wants those directories removed, but this copy can no longer run.

5. Bor genesis silently falls through to ethashexecution/execmodule/execmoduletester/exec_module_tester.go:430

Dropping the case opt.genesis.Config.Bor != nil arm means a Bor genesis now matches TerminalTotalDifficultyPassed or the default and gets ethash.NewFaker(). The test then passes against the wrong consensus rules instead of failing — the same failure mode the ReadChainConfig change exists to prevent. An explicit error/panic here would match the fail-loud choice made in db/rawdb.

Leftovers

  • git grep erigon/polygon -- db/ still hits db/snapshotsync/freezeblocks/dump_test.go:41-42 (polygon/bor/borcfg, polygon/chain) and db/snaptype/enum_registry_test.go:27 (polygon/heimdall), so "db/ no longer imports polygon/" does not hold for the test builds — the next PR that deletes polygon/ breaks these two packages.
  • Three FrozenBorBlocks implementations survive with no declaration behind them: execution/tests/blockgen/chain_makers.go:707 (non-test file), execution/protocol/rules/merge/merge_test.go:79, and execution/vm/runtime/runtime_test.go:539 (that one has the wrong signature — no align bool — so it never satisfied the interface).
  • AllTypes() (db/snapshotsync/freezeblocks/block_reader.go:386) is now exactly Snapshots().Types() and has zero call sites — only declarations in db/dbservices/interfaces.go:108 and db/snapshotsync/snapshotsync.go:192 plus four implementations. BorSnapshots() was removed from those same interfaces for being unused; this one can go the same way. If it stays, slices.Clone is the idiomatic form and keeps the nil-vs-empty result unchanged.
  • Stale comment at db/snapshotsync/freezeblocks/block_snapshots.go:327 still documents PruneHeimdall, which this change removes from PruneAncientBlocks.
  • db/datadir/dirs.go:418 replaces dbcfg.PolygonBridgeDB/dbcfg.HeimdallDB with literals, but those constants still exist and db/datadir/dirs_test.go:57-58 still asserts against them — two sources of truth for the same directory names.

Test

The new rejection has no test. db/rawdb/accessors_metadata_test.go already covers the sibling L2 round-trip with the same Write -> Read shape, so a small TestChainConfigBorRejected would pin the error that several callers turn into a panic.

…ng' into HEAD

# Conflicts:
#	cmd/integration/commands/stages.go
#	cmd/rpcdaemon/cli/config.go
#	cmd/utils/app/init_cmd.go
#	cmd/utils/app/snapshots_cmd.go
@awskii

awskii commented Aug 22, 2026

Copy link
Copy Markdown
Member Author

Ran a 4-agent review over this PR (git diff origin/main...HEAD, 41 files) and verified @AskAlexSharov's points against the current head. Three findings of my own, all minor:

  • db/snapshotsync/freezeblocks/block_snapshots.go:327 — the PruneAncientBlocks comment still names PruneHeimdall, whose call this PR deletes along with the bordb import.
  • db/rawdb/accessors_metadata_test.go:65 — the new test's rationale says "chain.Config has no Bor field", but chain_config.go:130 still declares Bor BorConfig and WriteChainConfig still marshals it. The premise is only true two PRs later.
  • execution/tests/blockgen/chain_makers.go:707 and execution/protocol/rules/merge/merge_test.go:79FrozenBorBlocks implementations left with no interface requiring them and no caller.

On the review above, point by point:

Bugs 1, 2, 3 — confirmed. WriteChainConfig (accessors_metadata.go:63-69) marshals cfg.Bor into BorJSON while ReadChainConfig rejects any non-nil BorJSON; chain.GetConfig (execution/chain/chain_db.go) plain-unmarshals the same row with no guard and txnprovider/txpool/pool_db.go:145 rehydrates cc.Bor from it; genesis_write.go:200's storedErr != nil && newCfg.Bor == nil drops the new error for a Bor genesis.

The qualifier: all three are gone by the end of the series. At #23497 the cfg.Bor marshal block, Config.Bor, BorConfig and initBor no longer exist. So they break bisect at this commit and at #23495, not the merged result.

Bug 4 — confirmed, and this one is durable. getChainNameFromChainData (cmd/utils/app/reset-datadir.go:175) reads through rawdb.ReadChainConfig, so on a Polygon datadir it now returns the rejection error; chainNameErr is only warned at line 78, configChainName stays unset, and resetCliAction bails before r.Run(). The heimdall/polygon-bridge cleanup kept at db/datadir/reset/reset.go:59 is therefore unreachable for exactly the datadirs it exists to clean. Still true at the tip of the series. Needs a decision: let that helper fall back to the raw chainName field, or drop the cleanup as dead.

Bug 5 — confirmed here, evaporates later. Dropping the Config.Bor != nil arm does let a Bor genesis fall through to ethash.NewFaker(). From #23497 chain.Config has no Bor field, so the case stops being expressible.

Test gap — already closed. TestChainConfigRejectsStoredBorSection and TestChainConfigAcceptsNullBorSection are in accessors_metadata_test.go as of 06:09Z, 15 minutes after the review.

@awskii awskii mentioned this pull request Aug 22, 2026
@AskAlexSharov
AskAlexSharov added this pull request to the merge queue Aug 22, 2026
Merged via the queue into main with commit 06c7764 Aug 22, 2026
136 checks passed
@AskAlexSharov
AskAlexSharov deleted the awskii/polygon-removal-db-storage branch August 22, 2026 08:42
Sahil-4555 pushed a commit to Sahil-4555/erigon that referenced this pull request Aug 22, 2026
…nsensus paths (erigontech#23495)

Polygon's fork tiers reached into the EVM's precompile and opcode
tables, the signer, the base-fee formula, the ForkID and the block
executor. Every one of those sites was gated on `chainConfig.Bor !=
nil`, which no chain can satisfy since erigontech#23487 de-registered the Bor
chainspecs, so removing them changes nothing for any supported chain.
After this, **nothing outside `polygon/` imports it** — production or
test. Sixth in the Polygon removal series, branched off erigontech#23494 — review
the last commit only.

## Changes
- `execution/vm`: drop the `Napoli` and `Bhilai` precompile sets and
instruction sets, and their arms in `Precompiles`, `ActivePrecompiles`
and the interpreter's jump-table select
- `execution/chain`: drop `Config.IsAgra` / `IsNapoli` / `IsAhmedabad` /
`IsBhilai` and the matching `Rules` fields
- `execution/protocol`: drop the Bor "Rio" coinbase override in
`NewEVMBlockContext`, the synthetic `StateSyncReceipt`, and the Bor
author/error special-casing in `SysCallContract`
- `execution/protocol/misc`: `CalcBaseFee` uses the single EIP-1559
denominator again; the Delhi and Bhilai variants are gone
- `execution/types`: drop the Bhilai signer tier and the Bhilai
`setCode` gate
- `p2p/forkid`: drop the Agra, Napoli and Bhilai fork boundaries
- `txnprovider/txpool`: drop the Agra and Bhilai fork tracking, the Bor
min-fee-cap constant, and `initBor`
- drop the Bor arms in the simulated backend and the kurtosis-devnet
genesis header override
- remove the Polygon cases from the dump, forkid, enum-registry and
signer tests

## Notes
`newCancunInstructionSet` was built **on top of**
`newNapoliInstructionSet`, so EIP-1153, EIP-5656 and EIP-6780 reached
Cancun through the Polygon tier. Deleting Napoli without care would have
silently changed the Cancun opcode set on Ethereum mainnet. Those three
enables are now spelled out in `newCancunInstructionSet` directly,
leaving it byte-identical. Prague derives from Cancun, not Bhilai, so it
is unaffected. Collapsing Napoli's `validateAndFillMaxStack` into
Cancun's single call is safe: it only recomputes `maxStack` from each
op's pop/push counts.

`Rules.IsShanghai` no longer ORs in `IsAgra` and `Rules.IsPrague` no
longer ORs in `IsBhilai`. Both extra terms could only be true with a Bor
config present.

`txnprovider/txpool` was the last of the three `BorJSON` rehydration
sites; the pool now keeps the stored chain config as-is. The other two,
in `erigon init` and `db/rawdb`, reject a `bor` section outright.

`chain.Config.Bor`, the `BorConfig` interface, `chain.BorRules`,
`MimetypeBor`, the 15 `kv.Bor*` tables, `snaptype.MinBorEnum`, the
`WitnessProcessing` stage and the `remote/bor.proto` surface all remain:
`polygon/` still implements or reads them. They go in the final PR,
which is now a straight deletion.

---------

Co-authored-by: Alexey Sharov <askalexsharov@gmail.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