feat(cli): add offline block-building benchmark sub-command - #595
feat(cli): add offline block-building benchmark sub-command#595pablodeymo wants to merge 1 commit into
Conversation
🤖 Kimi Code ReviewThis is a well-structured benchmarking PR that correctly isolates the block-building path. The code is clean, properly documented, and follows Rust idioms. Below are specific observations. Code Correctness & Safety
store.insert_new_aggregated_payloads_batch(entries);The batch insert assumes the store's internal ordering is deterministic. Ensure
eyre::ensure!(
options.proofs_per_data as usize <= NEW_PAYLOAD_CAP,
"--proofs-per-data {} exceeds the pending-pool capacity..."
);Good defensive check. However,
let total_slots = options
.warmup_slots
.checked_add(common.iterations)
.ok_or_else(|| eyre::eyre!("--warmup-slots plus --iterations overflows u64"))?;Correct overflow handling.
proof: MultiMessageAggregate::default(),In mock-crypto mode this is fine, but verify that Consensus & Cryptographic Considerations
const GENESIS_TIME: u64 = 1_700_000_000;Hardcoding genesis time is correct for deterministic benchmarking. Ensure this doesn't conflict with any time-based fork logic if the benchmark is later extended to simulate forks across long timespans.
let validators = (0..self.num_validators)
.map(|index| Validator {
attestation_pubkey: synthetic_pubkey(&mut rng_state),
proposal_pubkey: synthetic_pubkey(&mut rng_state),
index,
})The deterministic RNG is appropriate here. Verify that
on_block_without_verification(&mut store, signed_block)Documented why verification is skipped (self-generated blocks). Ensure this function doesn't skip state-transition validation that could corrupt the store for subsequent iterations. The comment says it imports "outside the measured span," which is correct methodology. Performance & Memory
.flat_map(|family| family.get_metric())
.filter_map(|metric| { ... })This iterates all metrics in the global registry. If the registry grows large, consider caching the histogram lookup by name, though for a benchmark tool this is negligible.
fn splitmix64(state: &mut u64) -> u64 {
*state = state.wrapping_add(0x9e37_79b9_7f4a_7c15);
...
}Consider marking this Error Handling & Robustness
eyre::ensure!(
store.new_aggregated_payloads_count() > 0,
"seeded attestations were evicted from the pending pool at slot {slot}..."
);Excellent check. Silent eviction would produce misleading benchmark results (empty blocks).
eyre::ensure!(
observations == 1,
"phase '{phase}' was observed {observations} times during one build (expected 1)..."
);The assumption of single-threaded execution is valid now but fragile. Add a comment warning that this check will fail if CLI & UX
args.drain(..2);
let argv = std::iter::once(OsString::from("ethlambda benchmark")).chain(args);Clever handling to get clap to render
Testing
Minor Suggestions
Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code Review
I didn’t find a production consensus/security regression in the new proposer-path benchmark wiring or the Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
🤖 Claude Code ReviewReview:
|
233cc94 to
428174e
Compare
db70dbf to
99adf46
Compare
`ethlambda benchmark synthetic` measures block building exactly as executed when the node proposes, against a reproducible synthetic workload, with no devnet required. "Optimize block building" (#465) is the top roadmap item, but the only observability today is Prometheus histograms on a live devnet: noisy, not reproducible, and unable to compare an optimization against a baseline. The harness drives the production proposer entry point (`produce_block_with_signatures`) over a seeded in-memory chain, seeding the pending pool per slot and letting the proposal tick promote it, as on a live node. Per-phase timings come from the existing `lean_block_proposal_attestation_build_phase_seconds` histogram: the per-label sample sums are deltaed between iterations, so attribution is exact and the hot path is untouched. Each iteration reports its block root, so a baseline-vs-optimized diff proves an optimization changed only speed and not attestation selection. `benchmark` is a second clap sub-command alongside `node`, so clap lists it in `--help` and names it in its own usage lines, and the harness arguments live in their own `Args` group rather than in CliOptions. `main` becomes synchronous and only the node path enters the tokio runtime: the benchmark is synchronous CPU-bound work and would otherwise park a worker thread for its whole run. Its logs go to stderr so the report owns stdout. NEW_PAYLOAD_CAP becomes public so the harness can reject a --proofs-per-data batch the pending pool would evict whole. `make bench` runs it. Reports one row per measured iteration. Aggregate statistics, build provenance and machine-readable output follow separately, as does the real-crypto workload — see docs/plans/block-building-benchmark.md for the milestones. Laid out to be read in one pass: `SyntheticCorpus` builds the chain and seeds the pool, `build_one_slot` is one slot's work end to end (seed, time the build, import), `PhaseTimer` turns two histogram readings into per-phase durations, `run_synthetic` is the loop over slots, and report.rs formats. Warmup and measured slots run the same code path; only whether the sample is kept differs.
99adf46 to
c2da08d
Compare
🗒️ Description / Motivation
Adds
ethlambda benchmark synthetic— an offline harness that measures block buildingexactly as executed when the node proposes, against a reproducible synthetic
workload, with no devnet required.
Second of three (design doc → this → comparable reports). What lands here is the
smallest thing that runs: the real proposer path, driven deterministically, reporting one
row per measured iteration. Aggregate statistics, build provenance and machine-readable
output follow in the next PR, so this one can be reviewed for what it measures rather
than how it formats.
How to read this
In this order — each piece is understandable without the next:
corpus.rs— the workload. Deterministic validators, a genesis store overInMemoryBackend, andseed_pool, which fills the pending pool for one slot andreports how many entries the next build will see.
build_one_slot(mod.rs) — one slot end to end: seed, time the build, import theblock. Warmup and measured slots run this same path; only whether the sample is kept
differs, so there is no "am I warming up?" branching inside.
PhaseTimer(mod.rs) —start()before the build,finish()after. Tworeadings of the existing phase histogram; the difference between their sample sums is
the build's phase time, so nothing is added to the hot path.
run_synthetic(mod.rs) — validate, set up, loop over slots, report. 46 lines.report.rs— the types and the per-iteration table.What Changed
bin/ethlambda/src/benchmark/mod.rsrun_synthetic's slot loop,build_one_slotfor one slot's work, andPhaseTimerfor per-phase attributionbin/ethlambda/src/benchmark/corpus.rsInMemoryBackend, deterministic pubkeys via splitmix64, and per-slot pool seeding in fixed insertion order, which also rejects a batch the pool would evict wholebin/ethlambda/src/benchmark/report.rsbin/ethlambda/src/command.rsbenchmarkjoinsnodeas a second clap sub-command, so clap lists it in--helpand names it in its own usage lines. The node payload becomesBox<CliOptions>now that a much smaller variant sits beside itbin/ethlambda/src/main.rsmainbecomes synchronous and dispatches; only the node path enters the tokio runtime (run_nodecarries the#[tokio::main]attributes). Benchmark logs go to stderr at WARN so the report owns stdoutcrates/storage/{lib,store}.rsNEW_PAYLOAD_CAPso the harness rejects a--proofs-per-databatch the pending pool would evict wholeMakefilemake bench(overrideBENCH_ARGSto customize)Correctness / Behavior Guarantees
cli.rsis not touched by this PR and the node runtime is unchanged. The harnessarguments live in their own
Argsgroup; the node's stay plainPathBuf/String, soclap keeps emitting its own missing-argument errors.
produce_block_with_signatures, the same functionBlockChainServer::propose_blockcalls, and seeds the pending pool so the proposal tick promotes it exactly as on a
live node.
across repeated runs; the roots are printed so a baseline-vs-optimized diff proves an
optimization changed only speed, not attestation selection. The harness never reads the
wall clock into results.
select_payloads/compact/stf_simulatecome from the sample sums of the existinglean_block_proposal_attestation_build_phase_secondshistogram, deltaed betweeniterations, with a per-phase assertion that the count advanced by exactly one.
overheadis the clamped remainder of wall minus the phases.duration of a CPU-bound run.
Tests Added / Run
corpus.rs: participant groups partition every validator; synthetic pubkeys aredeterministic for a seed.
command.rs: the benchmark token parses with no node argument, rejects node flags, andits usage line names the sub-command.
make bench; identical block-root sequences across two runs at thesame seed;
ethlambda --genesis config.yamlstill failing with clap's ownmissing-argument list.
make fmt,make lint,make test(576 tests, 30 suites) — all clean.Related Issues / PRs
nodesub-command for running the node #591; design doc in the accompanying docs PR✅ Verification Checklist
make fmt— cleanmake lint(clippy with-D warnings) — cleanmake test(cargo test --workspace --profile release-fast) — all passing