core, txpool, miner, eth/gasprice, internal/ethapi: reserved blockspace follow-ups - #2376
Merged
marcello33 merged 25 commits intoSep 2, 2026
Conversation
…m the parent header (POS-3669)
… for unspent gas (POS-3671)
… reserved receipts (POS-3670)
# Conflicts: # cmd/keeper/go.mod
…or a clean compare
…ool occupancy Reserved-blockspace senders (POS-3674) are eviction-immune in the txpool with no aggregate ceiling: a client with enough whitelisted addresses could occupy unbounded pending+queued slots, starving normal fee-paying senders network-wide since the reserved set is registry-derived and therefore consensus-uniform. Adds a combined pending+queued reservedOccupancy counter on LegacyPool, capped by config.ReservedMaxOccupancyPercent (default 50% of GlobalSlots+GlobalQueue). Two-layered by design: an incremental O(1) counter updated at every mutation site, plus a periodic from-scratch recompute in reset() that self-heals any drift every reorg cycle. Admission is gated with a new ErrReservedOccupancyExceeded; a reorg-time backstop trims the largest reserved account via the same prque idiom truncatePending already uses. The new config knob is wired through internal/cli/server like its sibling pool-size settings. POS-3681.
…or a clean compare
…or a clean compare
… suggestions and expose normal-region gas-used ratio
…s with per-client quota utilization
…ge at the fork boundary
…or a clean compare # Conflicts: # consensus/bor/bor.go # consensus/bor/bor_test.go # core/reserved_validation_test.go # core/types/block.go # core/types/block_test.go # miner/worker.go
…or a clean compare # Conflicts: # core/blockchain.go # core/blockchain_test.go # eth/api_debug.go # miner/worker.go
…alidation) for a clean compare
The still-in-pool invariant was judged against both nodes' pools while mined-ness came only from node0's canonical chain. The producing node drops a tx from its pool the moment its own head includes it, which can be several hundred ms before node0 imports that block, so the assertion raced with block propagation (and with tip-fork reinjection windows). A tx now counts as healthy on a node if it is pending in that node's pool or canonical on that node's own chain, and a drop only fails the test after persisting across consecutive polls. A genuine balance eviction is permanent, so it still trips the threshold.
…he reserved set The registry defines feeMode 1 (routed: fee paid, credited to the producer) as reserved for a future external-block-producer world; the spec's zero-fee handling applies to feeMode 0 only. The reader carried FeeMode as metadata but never consulted it, so a routed client's senders inherited the free-mode waiver end to end and mined at effectiveGasPrice 0, identically to feeMode 0. Resolve fee mode at snapshot build, the single choke point every consumer derives from: non-free clients stay out of the effective set, so their senders pay standard fees, their quotas leave EffectiveCapacity, and no downstream surface (EVM waiver, txpool, sequencing, header stamping) needs its own gate. The Snapshot's now-meaningless FeeMode plumbing is removed. registrytest gains a CreateClient helper and caller-provided state (the latter also serves the witness-completeness tests in the next commit) to pin the exclusion against the real registry bytecode.
…d witnesses registryreader.BuildSnapshot read the registry against a throwaway statedb.Copy(), and Copy deep-clones the attached witness: every trie node the read touched landed in the discarded clone and never reached the witness shipped to peers. A block's witness only carried the registry's storage when the block's own transactions happened to touch it, so the first transaction-free block after a registry shape change (an ordinary createClient) permanently wedged every stateless node with "missing trie node" for the registry account; with stateless validators holding voting power, span rotation lost quorum and the whole chain halted. Extract the copy-read-collect protocol the span and state-sync reads already inlined twice in consensus/bor into state.StateDB.ReadIsolated, and route all three call sites through it: run the reads on a reset copy, record them into the live witness (StartPrefetcher + IntermediateRoot), and re-register them on the live state so they enter its FlatDiff read surface (PropagateReadsTo). The shared helper also stops the copy's prefetcher on error paths (previously leaked at the bor.go sites), skips the witness machinery when none is being produced, and detaches the witness while copying instead of cloning it just to replace the clone. A registrytest regression test pins the contract end to end: a witness produced during a snapshot build must let a consumer rebuild the identical snapshot from the witness's node set alone, exactly as a stateless verifier does.
Five pipeline tests reassigned engine, exitCh, or speculativeWorkCh on a live worker whose background goroutines read those fields concurrently, failing go test -race deterministically. Stop the worker's goroutines first (idempotently, so the fixture cleanup can still run) or build a bare worker directly with the wrapped engine where no goroutines are needed. The closed-exitCh swaps become the real thing: a stopped worker's exitCh is genuinely closed.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (77.22%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## reserved-blockspace-block-building #2376 +/- ##
======================================================================
+ Coverage 55.33% 55.56% +0.23%
======================================================================
Files 917 919 +2
Lines 167127 167993 +866
======================================================================
+ Hits 92480 93351 +871
+ Misses 69124 69082 -42
- Partials 5523 5560 +37
... and 24 files with indirect coverage changes
🚀 New features to boost your workflow:
|
marcello33
reviewed
Aug 28, 2026
| return nil, err | ||
| } | ||
| if !c.Active || c.EffectiveFrom > effectiveAt { | ||
| if !c.Active || c.EffectiveFrom > effectiveAt || c.FeeMode != FeeModeFree { |
Collaborator
There was a problem hiding this comment.
looks like non-free (feeMode!=0) clients still get read from the registry every single block before getting filtered out. Not a big deal today since it's gated behind whoever controls the whitelist, but if that list grows with a bunch of non-free addresses, seems like unnecessary per-block work for addresses that can never be reserved anyway. Maybe we should filter earlier in the read path? Not sure how much this matters in practice tho
Member
Author
There was a problem hiding this comment.
don't think this is fixable in a meaningful way - you need the read to know the fee mode in the first place, so there's no way to filter earlier than right after it comes back. cost's the same for every whitelisted address regardless of mode, a non-free client doesn't add anything extra, it just doesn't make it into the map.
Collaborator
|
Shall we improve diffguard? Also worth merging develop back to unlock the kurtosis related tests |
…ng' into kamuikatsurgi/reserved-blockspace-followups # Conflicts: # miner/pipeline_session_test.go # miner/pipeline_test.go
reservedOccupancy bumped by a flat 1 per transaction at every mutation site, but reservedOccupancyCap is computed in the pool's own slot unit (GlobalSlots+GlobalQueue), the same one pool.all.Slots() uses for real fullness. A handful of large-calldata reserved transactions could occupy far more of the pool's actual capacity than the cap was meant to admit - counting as "1" each while consuming several slots - undermining the exact aggregate-starvation defense ReservedMaxOccupancyPercent exists to provide, just via few big transactions instead of many small ones. Weight every mutation site (the admission gate, promotion/removal loss, and the bulk fairness/demotion/truncation paths) by numSlots(tx) instead of a flat 1, and rename reservedCount to reservedSlots to match. reservedSlots now reads a new totalslots field on list, maintained incrementally in Add/subTotals the same way totalcost/totalvalue already are, instead of summing list.Flatten() on every call - Flatten nonce-sorts and copies the whole list once its cache is invalidated, which the reorg-time occupancy backstop would otherwise pay on every single eviction iteration (removeTx invalidates the cache it just read). Slot-weighting also exposed a real cap bypass in a pre-existing assumption: isNewReservedSlot treated any same-nonce replacement as occupancy-neutral, which was true under flat per-transaction counting (1 out, 1 in) but is false once transactions can differ in slot count. A reserved sender could fill their cap with minimal transactions, then replace each at a trivial fallback-fee cost with a maximal-size (txMaxSize) transaction at the same nonce - every replacement skipping the cap check entirely, since "same nonce" short-circuited it - ending up occupying several times the intended share while reservedOccupancy kept reporting exactly the cap. Replaced isNewReservedSlot with reservedSlotAt, which returns the incumbent transaction (if any) so both the admission gate and the two replacement call sites (add's pending-replace branch, enqueueTx's queue-replace branch) can compute and apply the real numSlots(new)-numSlots(old) delta, gating and costing a replacement exactly like any other admission. New regression tests pin both the capacity-header-style slot-weighting fix and the replacement-delta fix: a single sender's 3-slot transactions must get rejected once their combined slot count would exceed the cap well before their transaction count does, an oversized same-nonce replacement must be rejected against the cap rather than skipped outright, and a successful size-changing replacement must update occupancy by the real delta rather than leaving it stale. Confirmed all three fail on the pre-fix code and pass with the fix. Shared test fixtures (bigZeroFeeTx, bigReplacementTx) reuse zeroFeeTxWithGasAndData rather than duplicating the transaction- building literal.
Member
Author
|
merged develop into both branches, pushed - should pick up the kurtosis-pos pin bump and unblock e2e-tests. on diffguard - agree it's worth doing, but I'd rather run it as its own pass. it's currently flagging 17 survived mutants across 8 files, that's a bigger job than a comment-response round. |
marcello33
merged commit Sep 2, 2026
7e1ffa8
into
reserved-blockspace-block-building
17 of 19 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up hardening on top of the core Reserved Blockspace protocol change (#2302): value-only-balance txpool admission and fallback-fee handling (POS-3671), an aggregate reserved-sender occupancy cap to prevent normal-sender starvation (POS-3681), gas-price-oracle/
eth_feeHistoryexclusion of fee-free reserved transactions plus a newnormalGasUsedRatiofield (POS-3675/3676),chain/reserved/*andworker/reserved/*observability metrics (POS-3679), and execution-path produce/import determinism coverage at the fork boundary across the serial and both BlockSTM processors (POS-3672). Also carries the wire-format reconciliation needed when the Austin hard fork changed the header extra-data shape mid-implementation, and two correctness fixes found via live devnet testing (see below). Also fixes a txpool-only correctness/security gap found during a same-day security review of the occupancy-cap change: a same-nonce transaction replacement could grow totxMaxSizewhile skipping the reserved-occupancy cap check entirely, letting a reserved sender occupy several times their intended pool share (ad75b96ad).A combined delivery-and-test-status primer for both this branch and the base protocol branch, written for the still-open production go/no-go decision:
Reserved Blockspace - Delivery & Test Status
Executed tests
-racesuites across every touched package, and all 9tests/borreserved integration tests green.registryreader.BuildSnapshotread the registry against a state copy whose witness never reached the real per-block witness -cf9f3e82c). Also foundfeeMode=1clients were mining fee-free identically tofeeMode=0, contradicting the contract's own documented semantics (124087f55).Rollout notes
Consensus-affecting (gated behind
Bor.ReservedBlockspaceBlock, same as the base branch). No operator-facing config changes beyond the existingReservedMaxOccupancyPercenttxpool knob (default 50%, CLI-wired). Both devnet-found issues above are fixed on this branch; production readiness still depends on the smart-contract team's audited registry contract and genesis-contracts parity, neither of which is part of this PR - see the primer's "still outstanding" section.Node upgrade ordering: this PR adds a new header field (
ReservedCapacity, alongside the existingReservedGasUsed) toBlockExtraData/BlockExtraDataPostAustin. Like any hard-fork field addition, every node needs the upgraded binary before any of them start producing past the fork block - a pre-upgrade node attempting to parse a post-fork header carrying this field will fail. Nothing is live on any network yet, so this is purely a reminder for whoever runs the next devnet round or upgrade rehearsal: roll out the binary to all nodes first, in line with the existingBor.ReservedBlockspaceBlockhard-fork gate.