Repository navigation
feat(builder): ramp a pre-run's gas limit down; fill: false for a ramp-only pre-run - #349
Conversation
…ut tests A pre-run's gas_limit could only be reached from below: bumpGasLimit skipped whenever the head was already at or above it. Below the head it now ramps down, sending payload attribute targetGasLimit (the CL's gas limit target since Amsterdam) on each block. Before Amsterdam there is no such attribute and the filler keeps rising toward its miner ceiling, so a ramp down fails on the first block that does not fall rather than after gas_bump_max_blocks. A pre-run with no tests is now valid and skips the EEST checkout and the fill: a pure gas ramp (plus funding, if any), recorded as a bundle like any other pre-run. Together these let a released pre-run's 1T head be walked down to a devnet's limit and replayed by every client. Amsterdam slot numbers continue the parent's when it has one. Every existing flow starts from a pre-Amsterdam snapshot, where that equals the old per-client counter, so their bundles are unchanged; a pre-run on top of an Amsterdam chain no longer restarts at slot 1.
…fill: false (review) - The non-predeploy pre-run builds its funding block after the bump. Clearing targetGasLimit when the ramp returned sent that block back toward the filler's 1T miner ceiling, ~1/1024 above the target. A ramp down now leaves the target set for every later block this client builds. - An empty tests list cannot opt out of the fill: ResolveTarget re-inherits config.tests. A pre-run that only ramps the gas limit is now explicit, fill: false, hoisted like the other fields; tests stays required otherwise.
…hain An in-place (schelk) target restores the baseline before it boots, so a second target cannot continue the first one's scratch. base_bundle replays a recorded bundle (e.g. a release's pre-run) before the gas bump; the target's own bundle holds only the blocks it built, replayed after the base.
This comment has been minimized.
This comment has been minimized.
A release's pre-run bundle is 49.7 GB, which readRequestLines held in memory several times over. Replays now read line by line. A replay target can take base_bundle as well (base, then replay_from), so the other clients continue a release + ramp without a concatenated copy of the release.
There was a problem hiding this comment.
Summary
This PR makes the builder's gas bump bidirectional (via the Amsterdam targetGasLimit payload attribute), adds fill: false for a ramp-only pre-run, streams replay bundles instead of loading them whole, and lets replay targets take a base_bundle. The streaming and fill: false work look sound and the prior two findings (tests required, fill undocumented) are fixed; I found one functional gap in the ramp-down check for predeploy targets and one documentation omission.
Issues
- 🟡
pkg/builder/engine.go:673— ramp-down aborts on a predeploy target's first pre-fork bump block — see the thread on that line - 🟢
docs/configuration.md— new base_bundle option is undocumented in the configuration reference —base_bundleis a new per-target option (pkg/config/pre_runs.go:188) used by both the fill and replay paths, but thebuilder.pre_runs.targets[]table in docs/configuration.md still lists onlyreplay_from; onlyconfig.example.yamlmentions it. A row describing the pre-run it continues and the replay order (base, then the target's own bundle /replay_from) belongs next toreplay_from.
Reviewed @ 2a66455c
"The most dangerous phrase in the language is 'we've always done it this way.'" — Grace Hopper
| return built, fmt.Errorf("building gas-bump block %d: %w", built+1, buildErr) | ||
| } | ||
|
|
||
| if down && built == 0 && gl >= gasLimit { |
There was a problem hiding this comment.
🟡 ramp-down aborts on a predeploy target's first pre-fork bump block
With predeploy, the funding/deploy blocks are built pre-fork and the gas bump is what crosses into Amsterdam (forkAt returns preFork until activationTS). If the head is above gas_limit and the first bump block is still pre-fork, buildBlock sends no targetGasLimit, the limit rises, and this check returns an error on block 1 — even though the later Amsterdam blocks would ramp it down. The pre-fork bump is the expected case when activation is more than one block after the deploy block, so this combination cannot use ramp-down.
…filler fails the target The 15-minute stop window was granted only to schelk_options.promote targets, and the target still passed when the filler was SIGKILLed. A pre-run's datadir is always its product (promoted, exported or read as output_dir): on the release-snapshot e2e, geth v1.17.7 wrote its ramp journal in 32 ms and was then killed at 30 s while waiting on its history indexer, with pebble unclosed, and the target was reported OK.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…ed cleanly
nethermind (dotnet) returns 130 and besu (java) 143 after the signal a
container stop sends, having logged a complete shutdown ("All DBs closed").
Reading graceful as exit 0 logged every nethermind pre-run as not stopped
cleanly, and with the previous commit would have failed it.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
This PR adds a downward gas-limit ramp (payload attribute targetGasLimit), a fill:false mode for gas-ramp-only pre-runs, base_bundle chain continuation, streaming bundle replay, and clean-stop enforcement. The newest commit (adabbd8) lets a ramp-down cross a configured Amsterdam fork instead of aborting on the first pre-fork bump block; I traced that the blockTS/fork computation matches buildBlock and that geth v1.17.7 really does expose and honour targetGasLimit in testing_buildBlockV1/worker.go and returns slotNumber from eth_getBlockByNumber, so the mechanism is sound. Re-checking the prior open findings: fill:false now makes tests optional and skips the EEST checkout, the option is documented in docs/configuration.md and config.example.yaml, and the predeploy first-block abort is fixed by the waitingForAmsterdam guard. I found nothing worth reporting.
Reviewed 9 changed file(s) @ adabbd88 — no blocking issues found.
"The most dangerous phrase in the language is 'we've always done it this way.'" — Grace Hopper
A released benchmark snapshot can have a much higher block gas limit than the shadowfork needs. This lets a pre-run lower that limit—for example, from 1T to 200M—and record the new blocks for other clients to replay.
targetGasLimitto ramp down and hold the requested limit. A configured fork transition can complete before ramp-down starts; the block limit still applies.fill: falseto skip EEST setup andbase_bundleto continue from an existing recorded chain. The new bundle contains only the additional blocks, so consumers replay the base first.Validation: builder/config tests, fork-transition regressions, real Geth gas-limit checks, and replay of the 49.7 GB release bundle. CI passes.
Used by benchmarkoor-tests #44.