consensus, core, miner, params: reserved blockspace - #2302
Conversation
…+ hardfork gate (POS-3637)
…-3573) Reserved-blockspace senders (post-fork, classified via BorConfig) execute fee-free: no gas debit at buyGas, the EIP-1559 fee-cap floor is waived in preCheck, no gas refund, no producer tip, and no base-fee burn. The decision is computed once in newStateTransition so the serial and BlockSTM paths agree on the same state root. Fee fields stay readable for off-chain settlement.
Post-ReservedBlockspace, CalcBaseFee holds the reserved capacity out of the gas target and nets the parent's ReservedGasUsed out of gas used, so the public base fee tracks only normal-region demand against normal-region capacity. Anchoring the target on reserved capacity (Sigma quotas) rather than reserved used gas keeps the public market stable and stops a reserved client from moving the public fee with its own usage. Header.GasLimit is untouched (formula-only input). VerifyEIP1559Header inherits the change: its pre-Lisovo path recomputes via CalcBaseFee and its post-Lisovo path is a bounded check.
…-3570) Reserved-blockspace senders pay zero in-protocol fee, but the pool rejected and dropped their txs in several places. Add a sender-based, consensus-uniform reserved branch (the set comes from the chain config, matching the EVM and base-fee paths): - validation.go: waive the admission tip floor for reserved senders. - legacypool Pending: don't cap reserved senders' txs by the miner tip filter, so they reach the producer. - list.Add: reserved senders replace by arrival order (skip the price-bump rule) so a stuck zero-fee tx can be cancelled or replaced. The balance check needs no change: a zero-feeCap tx's Cost() is already just its value. Sender classification is gated on the ReservedBlockspace fork height.
Factor the reserved branches out of the EVM and consensus hot paths into focused helpers (reservedZeroFeeGas, calcBaseFeeBurn, verifyReservedFields). Behaviour is unchanged; this keeps buyGas, execute, and verifyHeader within the cognitive- complexity budget after the reserved-blockspace additions.
…(POS-3637) Pins the hardfork-rollout requirement that a scheduled fork prints its activation height on startup, and that an unscheduled (nil) fork stays silent.
…cks (POS-3570) The pool admitted reserved zero-fee txs but the producer dropped them: the price-and-nonce ordering rejects any tx with GasFeeCap below the base fee (ErrGasFeeCapTooLow). Carry the pool's reserved classification on LazyTransaction and exempt reserved senders from the base-fee floor in newTxWithMinerFee (zero miner tip). Caught by a two-node produce/verify integration test (tests/bor, integration tag): a reserved zero-fee tx is now mined and cross-verified, the sender pays only its call value, and a non-reserved zero-fee tx is still rejected at admission.
…ved-blockspace-block-building
…(POS-3570) Review surfaced that several comments described not-yet-implemented work as if it were live. Correct them so the foundation's deferrals read honestly; no logic change: - verifyReservedFields: quota/count value-correctness is enforced by block validation in POS-3576, not today — a producer can write any present values. - setReservedBlockspaceExtraFields: the zero ReservedTxCount/ReservedGasUsed are placeholders; the producer's reserved pass (POS-3575) sets the real values. - CalcBaseFee: the capacity-side target reduction is live while the ReservedGasUsed used-side netting stays inert until POS-3575 populates the header field, so the public base fee runs slightly hot under reserved load. Deterministic (a fee-accuracy gap, not a consensus split); resolves with POS-3575, no code change needed here. - buyGas: note the zero-fee waiver also drops a reserved blob tx's blob fee; reserved clients are not a blob use case today.
… onto the foundation branch Integrates Krishang's reserved-blockspace registry (ReservedBlockspaceRegistry.sol + Go reader/registryreader + genesis embed + txpool/miner/core wiring) alongside our consumer-side foundation (zero-fee EVM, normal-region base fee, txpool/miner admission, header fields). Clean three-way merge; whole repo builds and both the reserved-consumer and registry test suites pass. Post-merge the registry (source of truth) and our config-stub classifier still run as parallel mechanisms; unifying the consumers onto the registry is the next step. Contract still needs root()/effectiveFrom/feeMode per the finalized spec.
…kspace registry (POS-3572) Extend Krishang's ReservedBlockspaceRegistry per the finalized spec (design.md §4, §7): - feeMode (0=free, 1=routed) per client + setClientFeeMode; surfaced via getClientForAddress. - effectiveFrom block per client + setClientEffectiveFrom; Bor gates on active && effectiveFrom <= number (read at parent state, so deterministic). - configVersion + root(): a change-epoch bumped on every mutation, so Bor can cache its reserved-set snapshot keyed on root() and only rebuild when it moves (spec §4.5), avoiding a per-tx state read. Regenerate the embedded runtime bytecode (forge, solc 0.8.33 pinned), update the read-only ABI (getClientForAddress now 6 returns + root), and sync the Go reader (decode feeMode/effectiveFrom, add Root()) + registryreader.ClientLookup/Reader. 11 foundry tests and the Go registry tests pass.
Introduce registryreader.Snapshot — an immutable, pure-lookup view of the reserved set built once from the registry at a given block state (spec §4.5), so the hot classification paths (txpool admission, EVM fee-skip, base-fee capacity) never do a per-transaction state read. BuildSnapshot reads the active whitelist + per-client feeMode/effectiveFrom/quota + capacity + root(); callers cache it and rebuild only when root() moves. nil-safe (no registry = classifies nothing). Extends the Reader interface with WhitelistedAddresses/TotalReservedGas (already implemented on the concrete reader and the test harness). Snapshot unit test builds from the real contract bytecode via the harness and verifies classification, feeMode, capacity, and root.
…fig (POS-3570/3574) Repoint the pool's reserved classification off the config stub onto the registry: legacypool builds a registryreader.Snapshot at each reset from the new head's state, and isReserved / validation.go's tip-floor waiver query the snapshot (rebuilt per head, not per tx — spec §4.5). The fork-height gate stays in chain config (a legit chain param); only the reserved *set* now comes from the registry. TxPool.SetReservedRegistry propagates the reader to subpools. Reserved unit tests now drive a fake registryreader.Reader as the source; full txpool suite green.
… in execution (POS-3570/3574) Build a per-block reserved-set snapshot from the registry at parent state and inject it into the EVM block context across the serial, V1-parallel, and V2 BlockSTM execution paths plus the miner. newStateTransition now classifies reserved senders from the snapshot instead of the config stub, keeping serial and parallel execution byte-identical.
…stub CalcBaseFee is pure (config, parent) with no parent state at most call sites, so the reserved-capacity input cannot read the registry here; it arrives via a producer-stamped header field (POS-3575/3576), mirroring the inert reserved-gas-used half. Records the decision so the retained config stub is deliberate, not an oversight.
…nsensus-safe (POS-3574) A registry-initialized two-miner devnet exposed two ways reserved-blockspace classification could differ between the block producer and the verifying nodes, each a consensus split: - BuildSnapshot read the registry through the EVM (ApplyMessage), which mutates the statedb it runs against. On the execution path the caller passes the live block state, so the read leaked gas/nonce/touch changes into the block and diverged the state root. Read against a throwaway state copy. - The serial and V2 block processors and the prefetcher only hold a *HeaderChain context, which did not expose the registry reader, so ReservedSnapshotForBlock fell through its type assertion to a nil snapshot on every import path. Producers (full chain) classified reserved senders and built fee-free blocks that verifiers then rejected as underpriced. Mirror the reader onto HeaderChain so produce and verify share one source. Also gate the execution-path snapshot build on the fork height so nodes syncing from genesis skip the per-block copy and registry read before the reserved fork activates. Adds a registryreader unit test (Snapshot accessors, the effectiveFrom activation boundary, BuildSnapshot error/nil propagation) and converts the end-to-end integration test to seed the registry via real initialize()/createClient() txs, asserting a reserved zero-fee tx is admitted, mined fee-free, and accepted by the peer (cross-node parity), while a non-reserved zero-fee tx is rejected at admission.
…s (POS-3574) Multi-agent pre-PR review (/pos-code-review) surfaced fixes worth taking now; the rest are tracked as pre-activation work in the run report. - ethapi: bypass the EVM wall-clock timeout for bor-internal system-contract reads, not just the gas cap. The reserved-registry snapshot read runs through doCall, whose RPCEVMTimeout is node-local and load-dependent. A timeout on one node but not another returned a nil snapshot and flipped fee classification, diverging the state root. Both node-local limits are now bypassed together for internal consensus reads (validators, span, state receiver, registry). - params: reject scheduling ReservedBlockspaceBlock before CancunBlock (the reserved header fields only encode in the post-Cancun extra-data format, while verifyHeader requires them once the fork is active), or without a registry contract configured. Prevents a rollout height that bricks block production. - bor: add an interim header-only bound (ReservedGasUsed <= block GasUsed) in verifyReservedFields, rejecting an impossible value the base fee would otherwise silently clamp. Full value-correctness stays with POS-3576. - types: GetReservedInfo now routes through DecodeBlockExtraData instead of re-decoding the extra blob independently. - params: drop the dead config-stub classifiers (IsReservedSender/ ReservedQuotaOf — no production caller; classification is registry-sourced) and document that ReservedClients now feeds only the base-fee capacity carve-out until the producer-stamped header field lands. - registryreader: correct the Snapshot/Root/BuildSnapshot docs — the root()-keyed cross-block cache is a tracked optimization, not yet implemented on the execution path (txpool caches per head; execution rebuilds per block). Adds tests for the Cancun-order guard, the ReservedGasUsed bound, and the reserved base-fee capacity-equals-limit boundary.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (65.75%) 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 @@
## develop #2302 +/- ##
===========================================
+ Coverage 54.05% 54.20% +0.15%
===========================================
Files 907 912 +5
Lines 161982 163199 +1217
===========================================
+ Hits 87564 88470 +906
- Misses 69000 69268 +268
- Partials 5418 5461 +43
... and 23 files with indirect coverage changes
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Beyond the inline findings, this run also examined and ruled out a few adjacent concerns: blob transactions being silently dropped by the new fillTransactions (bor's txpool filter already excludes blob txs, so none are dropped by this change), SetReservedGasUsed corrupting a nil GasTarget/BaseFeeChangeDenominator into an explicit zero (the RLP optional-pointer round-trip preserves the existing nil/zero distinction), and an empty pre-seal commit bypassing writeReservedGasUsed (the write happens unconditionally at the end of fillTransactions, including for empty/interrupted builds).
Extended reasoning...
This is a record of additional areas checked beyond the two inline findings (the dead fillTransactionsOld duplication and the multi-batch BlockSTM TxDependency reset bug), not a guarantee of correctness or an instruction to skip these in future review.
|
|
@claude review |
…n and block validation Classify reserved (fee-free) transactions by per-client quota in block order via a shared registryreader.ClassifyReserved/ReservedWalk consumed by the producer, the serial and both parallel executors, and the verifier, so overflow beyond quota pays normal fees and producer and verifier agree by construction. Add block validation that recomputes and matches the header's ReservedGasUsed (enforced on the stateless path too), hard-fail per-block snapshot build post-fork on a missing registry reader or read failure, and drop the redundant per-block capacity ceiling. Adds unit matrix, integration cases, and devnet coverage.
…d-blockspace comments Comment/hygiene only, no logic change: - Remove in-house shorthand from public comments (internal iteration labels, ticket references, spec section numbers, bold-asterisk emphasis). - Reword consensus-split phrasing in favour of neutral post-state wording. - Name the parallel processor explicitly (V2 BlockSTM) instead of a bare V2. - Drop the inherited go-ethereum LGPL headers from the in-tree reserved test files, matching the rest of bor. - Add TODOs noting registry-contract/ should migrate to the genesis-contracts repo and bor should source the embedded bytecode from there.
|
@claude review once |
There was a problem hiding this comment.
Beyond the inline findings, I also checked whether the reserved blockspace registry predeploy is actually wired into the real mainnet/Amoy startup path (as opposed to only the in-repo test harness) — it is: both BorMainnetChainConfig and AmoyChainConfig in params/config.go set Bor.BlockAlloc["0"] to the registry runtime bytecode at DefaultReservedRegistryContract, and core/genesis.go's new allocWithBlockZeroAlloc merges that into genesis Alloc before hashing/committing, so the predeploy is not missing from real network genesis.
Extended reasoning...
A finder flagged that the reserved blockspace registry predeploy might be missing from the real mainnet/Amoy startup path (only present in test scaffolding). I traced this through params/config.go and core/genesis.go: both production chain configs populate Bor.BlockAlloc["0"] with the registry bytecode at the predeploy address, and Genesis.ToBlock/Commit now merge that block-zero alloc into the genesis Alloc before computing the state root, so the registry contract is genuinely present in genesis on real networks, not just in tests. This is recorded for the record only; the inline bug/nit findings from this run already convey the actionable items and don't need restating here.
|
@claude review |
borUnittestReservedConfig now co-activates Giugliano (required by the reserved-blockspace fork-order guard). That flips the worker onto the Giugliano builder path, where — under the fake engine's instant seal — an empty noempty pre-seal block wins the block-number race and the full block's mined event never fires. Run TestReservedBuild_HeaderGasUsed with noempty=true so there is no empty pre-seal to lose the race to. No production code changes.
|
@claude review |
There was a problem hiding this comment.
Beyond the inline nit found this run, I checked whether the reserved-registry predeploy needs to be mirrored into the production chain configs (internal/cli/server/chains/mainnet.go/amoy.go) used by bor server, since only the params.BorMainnetChainConfig/AmoyChainConfig presets in this PR got the registry contract address and predeploy allocation — confirmed mainnetBor/amoyBor are separate struct literals that don't reuse those presets. Since ReservedBlockspaceBlock is left nil everywhere in this PR, the fork cannot activate regardless, so this gap doesn't need to be closed here — it should land together with whichever future PR sets the activation block and wires ReservedRegistryContract into the runtime configs.
Extended reasoning...
This run's only new finding was a documentation nit (a stale doc comment on fillTransactions describing waivers as future work when this PR already implements them end-to-end), which doesn't warrant delaying approval on its own. I additionally examined a finder-raised concern that the reserved-registry predeploy (contract code + address) was only added to the params.go BorMainnetChainConfig/AmoyChainConfig presets, not to internal/cli/server/chains/mainnet.go or amoy.go — the structs actually loaded by bor server --chain mainnet/amoy. I verified independently that mainnetBor/amoyBor in chains/mainnet.go and chains/amoy.go are indeed separate, hand-written struct literals that do not reference the params.go presets, and that their BlockAlloc maps and allocs/*.json files do not contain the registry predeploy. This confirms the underlying observation is factually correct. However, since ReservedBlockspaceBlock is nil in every config touched by this PR (including the params.go presets), the fork cannot activate on any network from this change alone, so the missing predeploy in the runtime configs has no current effect and is reasonably deferred to the follow-up activation PR, which will need to set the fork block, ReservedRegistryContract, and the predeploy together across all the surfaces called out in this repo's hardfork-rollout guidance. This PR remains a large, consensus-critical, dormant-by-design feature branch that a human should review directly; I am not approving it, only leaving this as a record of what else was checked this run.
The comment described the zero-fee reserved tx waivers as future work, but this branch implements them (txpool tip-floor waiver, execution base-fee waiver) and the reserved integration test mines a zero-fee reserved tx end-to-end. Update the comment to match; no code change.
|
@claude review |
There was a problem hiding this comment.
Beyond the inline finding, I also checked whether the reserved registry predeploy is missing from production server presets or packaged genesis — it isn't: both BorMainnetChainConfig and AmoyChainConfig (params/config.go) set ReservedRegistryContract and embed the registry bytecode via BlockAlloc["0"], consistent across runtime config and packaged genesis.
Extended reasoning...
This run re-reviewed the PR after the author's latest "@claude review" ping with no new commits since the last review pass. The only new finding this round is a Nit-severity, dormant DoS gap (reserved-sender eviction immunity has no aggregate occupancy cap), which doesn't warrant blocking approval on its own given the fork is unactivated on every network. I additionally verified a candidate concern raised by a finder agent — that the registry predeploy might be missing from production presets/packaged genesis — and confirmed it is present in both mainnet and Amoy configs, so that concern is ruled out and recorded here for the record rather than re-explored in a future pass.


Summary
Reserved blockspace partitions each block into a reserved region — whitelisted senders drawn from an on-chain registry, each with a per-client gas quota, paying zero in-protocol fee — and a normal region under standard EIP-1559. This PR carries the full execution-client side of the feature (production, execution, base-fee pricing, and validation) behind a fork gate that is not activated on any network. It consolidates the earlier sequencing-only slice with the registry, txpool, EVM, base-fee, and validation slices onto one branch.
What the branch adds, per package
params— theReservedBlockspacefork gate (IsReservedBlockspace, config field, startup banner) and the genesis-embedded registry:ReservedBlockspaceRegistryCodeis allocated at0x…1002in the mainnet/Amoy presets, a predeploy like the validator set and state receiver. Activation heights are intentionally left unset.registry-contract/,consensus/bor/contract,consensus/bor/registryreader— the on-chainReservedBlockspaceRegistry(no constructor, so its runtime bytecode embeds directly in genesis) and its Go read side.registryreader.Snapshotis an immutable per-block view (active whitelist, per-client quota, fee mode, capacity, root) built once from the registry at the parent state, so hot paths never do a per-transaction state read.core/txpool— admits and keeps zero-fee transactions from reserved senders (waives the min-tip and base-fee floors; replacement by arrival order), with classification sourced from the registry snapshot rebuilt per head. Non-reserved zero-fee transactions are still rejected.core,core/vm— reserved senders execute fee-free past the fork (no gas debit, no base-fee floor, no producer tip, no burn; only value moves). The reserved set is injected into the EVM block context from the registry at the parent state, identically across the serial, parallel, and BlockSTM paths.consensus/misc/eip1559— the base fee is priced on the normal region only: reserved capacity is held out of the gas target so a zero-fee reserved client cannot move the public base fee with its own usage.core(block validation) — the header'sReservedGasUsedmust equal the gas actually used by reserved transactions, recomputed during execution and enforced before the stateless early-return, so a producer cannot stamp a value that skews the next block's base fee.miner— sequences pending transactions into commit batches (priority → one batch per reserved client → normal). Reserved selection charges each client's quota against declared gas (tx.Gas), so the set is decidable without executing anything; overflow past a client's quota diverts that sender's remaining nonces to the normal region, where they pay normal fees. Actual reserved gas used is recorded in the header.core/types—BlockExtraDatagainsReservedGasUsed *uint64(rlp:"optional"); a pointer so an explicit zero stays distinct from an absent field. Wire-neutral when unset — byte-identical 4-element encoding.Producer / verifier parity
Producer and verifier classify reserved senders through one shared pure function over the ordered block body, so both reach byte-identical results. Classification is bounded by per-client quota only — there is no cross-client ceiling: the registry guarantees that the sum of active client quotas equals capacity, so a global cap could never bind and is unnecessary. This makes classification order-independent, so parity holds by construction; a snapshot-build invariant rejects any registry whose quotas sum exceeds capacity.
Executed tests
ReservedGasUsedvalidation (match / mismatch / absent / N-1, N, N+1 boundary); miner per-client-quota sequencing and overflow; txpool zero-fee admission, pending surfacing, and replacement; reserved-aware base fee. Fullminer,core,txpool, andregistryreadersuites green under-race.tests/bor, integration tag): a reserved zero-fee tx is mined fee-free and accepted by the peer (cross-node parity); quota overflow pays normal fees; multi-client quotas partition correctly; a non-reserved zero-fee tx is rejected at admission.ReservedBlockspaceRegistry.Rollout notes
ReservedBlockspacefork, whose activation height is unset (nil) on mainnet and Amoy, soIsReservedBlockspaceis always false: every reserved path no-ops, block building and produced headers are byte-identical todevelop, and the base fee is unchanged. The registry bytecode is present in genesis but inert until the fork.