Repository navigation
miner: persist BidBlock revokes across restarts - #3796
Conversation
Pull Request ReviewThis PR adds asynchronous, best-effort persistence of BidBlock builder revocations to the chain database and restores active revocations when the miner restarts. It also introduces sequence-based write ordering, updates miner backends to expose the chain database, and adds regression tests for restart durability, expiration, corrupt state, failed writes, and manual clears. Sensitive ContentNo sensitive content detected. Security IssuesNo serious security issues detected. Generated by Hashdit Bot. This tool can absolutely NOT replace manual audits. |
| // a later manual clear). Losing the newest write on failure is acceptable | ||
| // (best-effort); an older snapshot overwriting a newer one is not. | ||
| func (m *BidBlockPermissionManager) persistAsync(seq uint64, snapshot map[common.Address]BidBlockRevokeRecord) { | ||
| m.persistMu.Lock() |
There was a problem hiding this comment.
Consider following the VoteJournal approach by persisting the JSON under a dedicated /bidBlockRevokeJournal/ directory instead of chaindata, since BidBlock revocations are validator-local MEV policy rather than chain data: https://github.com/bnb-chain/bsc/blob/master/core/vote/vote_journal.go.
There was a problem hiding this comment.
Agreed — moved it out of chaindata. Revokes now journal to a single atomic JSON file under the datadir (/bidblockrevokes.json, path resolved via stack.ResolvePath like the txpool journal), and the ChainDb() addition to miner.Backend is gone. I kept it a snapshot file rather than a VoteJournal-style WAL dir: the WAL fits append-heavy per-block votes, while this is a rarely-mutated small map with self-expiring (≤24h) entries — a whole-map atomic replace is simpler and crash-safe (temp file + rename). Same restore-on-restart semantics and ordering guarantees, now with file-based tests.
Pull Request ReviewThis Go-based Ethereum client PR adds file-backed JSON journaling for BidBlock builder revocations, resolving the journal path under the node datadir and restoring active lockouts at startup. Mutations are persisted asynchronously using atomic replacement and sequence-based stale-write suppression, with configuration defaults and regression tests covering restart recovery, expiry, corrupt state, ordering, and manual clears. Sensitive ContentNo sensitive content detected. Security IssuesNo serious security issues detected. Generated by Hashdit Bot. This tool can absolutely NOT replace manual audits. |
| // expired ones are dropped. | ||
| func NewBidBlockPermissionManager(journalPath string) *BidBlockPermissionManager { | ||
| m := &BidBlockPermissionManager{ | ||
| revoked: make(map[common.Address]BidBlockRevokeRecord), |
There was a problem hiding this comment.
Consider adding explicit json: tags and a version envelope to the journal.
. The journal may change format (may need to carry a per-builder revoke count as well to support new feature like dynamic reovke time computiing) , so the lockout duration can scale with the number of offenses — adding fields for that is then a clean v1→v2 migration rather than a compat branch.
Port of bnb-chain/bsc#3796. BidBlock revoke lockouts (BEP-675, validator-local MEV policy) previously lived only in memory, so a restart silently released every revoked builder. They are now mirrored to a JSON journal — a single standalone file under the datadir (bidblockrevokes.json), deliberately outside chaindata — and restored at startup, dropping any revoke whose window elapsed while the node was down. Semantics (matching go-bsc): - Versioned envelope ({version: 1, revokes: {...}}) with pinned JSON keys (revokedAt/duration/reason/blockHash/blockNum); an unknown version, corrupt file, or missing file starts empty rather than misreading the format. - Persistence is asynchronous and best-effort: mutations snapshot the whole map under the write lock and hand it to a fire-and-forget writer, so the mining path never waits on disk. - The write is atomic (temp file + rename, owner-only permissions) and the persisted sequence advances before the write regardless of outcome, so an older snapshot can never overwrite a newer one and resurrect stale state (e.g. a revoke clobbering a later manual clear). - Manual clears (mev_setBuilderBlockPermission allowed=true) persist too, so they are not resurrected on restart. - BSC_BID_BLOCK_REVOKES_JOURNAL overrides the location (a relative path resolves under the datadir); an empty value disables persistence — the go BidBlockRevokesJournal config equivalent. The journal path is registered during component build (payload service builder, the miner bootstrap point), before any consumer initializes the lazily-created global manager. Managers constructed without a journal (tests, fallback) keep the previous purely in-memory behavior. Tests mirror the go additions: restart survival with an identical status snapshot, expired-while-down revokes dropped on load, corrupt/ missing journal tolerance, unknown-version refusal, stable-key pinning, failed-newer-write-blocks-older ordering, and manual-clear persistence. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Description
BidBlock
SendBidBlockrevokes (BEP-675 Layer 2) were held only in an in-memory map inBidBlockPermissionManager, reconstructed empty on everyminer.New. A validator that revoked a builder for 24h and then restarted within that window (upgrade, crash, OOM) came back with an empty map, so the builder was immediately allowed toSendBidBlockagain — even though the permission RPC still advertisedresetAt = revokedAt + durationas a wall-clock promise.This PR persists the revoke map to the chain database and restores it on startup:
bidBlockRevokes), overwritten on every change. It is JSON-encoded — RLP is geth's codec for consensus-critical / large / frequent data, but supports neither maps nortime.Time; this state is tiny, written rarely, never on-chain, and benefits from JSON's schema tolerance and readability, matching the existingWriteChainConfigprecedent.Revoke/SetAllowed.db.Putis strictly increasing: once a newer snapshot has entered the writer, an older one is dropped even if the newer write failed — an older snapshot can never overwrite a newer one and resurrect stale state (e.g. a revoke overwriting a later manual clear).The manager keeps a nil-database path (unchanged in-memory behavior) for callers and tests that don't wire a database.
Rationale
A revoke that silently vanishes on restart lets a misbehaving builder resume
SendBidBlock(bypassing the weakerSendBidfallback) before its lockout should have ended, and makes the RPC'sresetAta promise the node doesn't keep. Durability of the lockout across restarts is what the advertised 24h window requires.Scope is deliberately limited to durability across restarts: revoke triggers, the 24h / 450s durations, the RPC semantics and the per-validator (non-shared) nature of the lockout are all unchanged.
Example
NA
Changes
Notable changes:
NA