feat(devnet): snapshot-backed L1-free devnet + benchmark harness - #4708
feat(devnet): snapshot-backed L1-free devnet + benchmark harness#4708meyer9 wants to merge 1 commit into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| stack.stop_sequencer().await?; | ||
| let client_rpc = stack.client_rpc_url()?; | ||
| let validator_metrics = | ||
| PrometheusBlockCollector::start(client_rpc.clone(), stack.builder_metrics_url()?) |
There was a problem hiding this comment.
Bug: The validator's PrometheusBlockCollector is started with stack.builder_metrics_url()? (the builder's Prometheus endpoint) but polls client_rpc for block number advancement. This means the collector will scrape the builder's Reth metrics while the client is the node actually executing the replayed blocks. The resulting validator_blocks[*].prometheus_metrics will contain the builder's stale metrics (sequencing has stopped), not the client's execution metrics.
The client InProcessClient currently doesn't expose a metrics port/URL. To fix this, the client would need a metrics_port configuration + metrics_url() method (mirroring the builder's), and SnapshotL2Stack would need a client_metrics_url() accessor.
e834429 to
692aae0
Compare
dce771d to
7cb88e1
Compare
| })?; | ||
| let sequencer_gas_per_second = result.blocks.iter().map(|block| block.gas_used).sum::<u64>() | ||
| as f64 | ||
| / (result.blocks.len() as f64 * block_seconds); |
There was a problem hiding this comment.
The write_visualizer_bundle path can reach a division by zero when result.blocks is empty. The guards at lines 242-260 check total_confirmed, total_gas, and measurement_block_count, but the block-count check at line 256 is inside if let Some(expected_blocks) — so when measurement_blocks is None in the config (e.g. a duration-only YAML), write_visualizer_bundle proceeds with result.blocks.len() == 0, producing 0.0 / 0.0 = NaN for sequencer_gas_per_second and validator_gas_per_second.
Consider guarding against empty blocks before calling write_visualizer_bundle, or at least at the top of the method:
if result.blocks.is_empty() || result.validator_blocks.is_empty() {
eyre::bail!("cannot write visualizer bundle without measured blocks");
}
|
|
||
| /// Fetches every canonical block in the measured window from the builder. | ||
| pub async fn collect_block_metrics( | ||
| builder_rpc: &url::Url, |
There was a problem hiding this comment.
The collect_block_metrics function parameter is named builder_rpc but it is called with both builder_rpc and client_rpc URLs:
Self::collect_block_metrics(&builder_rpc, &summary, &sequencer_metrics),
Self::collect_block_metrics(&client_rpc, &summary, &validator_metrics),Consider renaming to rpc_url to avoid confusion about which node is being queried.
| eyre.workspace = true | ||
| nanoid.workspace = true | ||
| reqwest.workspace = true | ||
| chrono.workspace = true |
There was a problem hiding this comment.
Nit: chrono is now listed as both a [dependencies] entry (here) and a [dev-dependencies] entry (line 153). Since it's already a regular dependency, the [dev-dependencies] entry is redundant and should be removed.
692aae0 to
9a4b3ba
Compare
7cb88e1 to
4e3d8d0
Compare
9a4b3ba to
cc9e219
Compare
4e3d8d0 to
c54cdb9
Compare
Add the etc/systems snapshot devnet: an in-process standalone-consensus L2 stack booted from a chain snapshot, plus benchmark and devnet CLIs (`base-bench`, `base-devnet`), Prometheus metrics, and a snapshot devnet integration test. Depends on the L1-free standalone sequencer (consensus) and the load-test measurement-blocks window. Final part of the split from #4180.
cc9e219 to
86b5513
Compare
c54cdb9 to
0c5428a
Compare
| Self { | ||
| l1_chain_id: 1337, | ||
| l2_chain_id: 84_538_453, | ||
| l1_slot_duration: 2, |
There was a problem hiding this comment.
Bug: The old DEFAULT_SLOT_DURATION was 1 (one second), used to minimize L1-dominated startup time in system tests. This new standard() default sets l1_slot_duration: 2, doubling the L1 slot duration for every existing SystemTestStackBuilder::new().build() caller that doesn't explicitly set with_slot_duration() — which is all of them in the current test suite.
This will make system tests that wait for L1 confirmations take roughly twice as long. Was this change intentional? If standard() is meant to match the docker-compose devnet config (which uses 2s slots), consider adding with_slot_duration(1) in SystemTestStackBuilder::build() to preserve the fast-path default for programmatic system tests.
| .map(|value| StandalonePrefund { address: value.address, amount: value.amount }); | ||
| let block_interval = config.snapshot.block_interval; | ||
| let first_block_timestamp = SystemTime::now() | ||
| .duration_since(UNIX_EPOCH) |
There was a problem hiding this comment.
The first_block_timestamp is computed from SystemTime::now() plus a 10-second lead. If the system clock is significantly behind the snapshot's boundary.head.timestamp (e.g. on a machine with clock skew), the guard at line 104 catches it. However, there's a subtler issue: anchored_rollup_config at line 107 computes config.genesis.l2_time = first_block_timestamp - legacy_elapsed, and legacy_elapsed is blocks_since_genesis * block_time (i.e. ~30M * 2 = ~60M seconds for mainnet). If first_block_timestamp is less than ~60M seconds (which can't happen for real Unix timestamps), the subtraction underflows — but the checked_sub guards handle it.
More practically: if the snapshot head's block_time assumption in the canonical rollup config doesn't match historical production cadence (e.g. missed blocks), the derived l2_time won't produce exact timestamp alignment for historical blocks — but that's fine since only the first descendant needs to be correct, which is verified at line 115.
Review SummaryPR 3/3 adds snapshot-backed devnet infrastructure ( FindingsPreviously identified (4 inline comments from prior review):
New finding (1 inline comment): Architecture Notes
|
|
Caution This PR may regress performance. 5 benchmark(s) slower by more than 10% beyond the noise band: Benchmark results (advisory)Median time on the PR head versus the base branch, measured on the same host. Wall-clock, so a change is only flagged when it clears ±10% and the confidence intervals do not overlap. Only benchmarks past the ±10% threshold (plus new or dropped ones) are listed. This check never blocks a merge.
43 benchmark(s) within ±10% omitted. |
1 participant
Summary
Adds the etc/systems snapshot devnet: an in-process standalone-consensus L2 stack booted from a chain snapshot, plus benchmark and devnet CLIs (
base-bench,base-devnet), Prometheus metrics, and a snapshot devnet integration test.This is PR 3 of 3 (top of stack) splitting the combined PR #4180 into focused, independently-reviewable pieces.
Stack
base: split/loadtests-measurement-window· snapshot devnet + benchmark harnessChanges
base-system-tests(20 files, +3042/-183):etc/systems/src/{benchmark_cli.rs, devnet_cli.rs, prometheus_metrics.rs, lib.rs, smoke.rs, system_config.rs, Cargo.toml, README.md},src/bin/{base_bench.rs, base_devnet.rs}(new[[bin]]targets), 8src/l2/*files,tests/snapshot_devnet.rs, and a 2-lineCargo.lockdelta (base-common-chains,reth-ethereum-forks).Verification
cargo clippy -p base-system-tests --all-targets --all-features -- -D warnings→ cleancargo +nightly fmt --all -- --check→ clean ·cargo metadata --locked→ OKgit diff <#4180-tree> HEADis empty)Split from #4180 (kept open for reference).