Skip to content

feat(cli): add offline block-building benchmark sub-command - #593

Closed
pablodeymo wants to merge 1 commit into
feat/cli-node-subcommandfrom
feat/block-building-benchmark-harness
Closed

feat(cli): add offline block-building benchmark sub-command#593
pablodeymo wants to merge 1 commit into
feat/cli-node-subcommandfrom
feat/block-building-benchmark-harness

Conversation

@pablodeymo

Copy link
Copy Markdown
Collaborator

🗒️ Description / Motivation

Adds ethlambda benchmark synthetic — an offline harness that 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. This is milestone M1 (mock-crypto mode) of
the design in docs/plans/block-building-benchmark.md; M2 adds real XMSS/leanVM pools
and post-build seal-phase measurement, M3 adds replay-from-datadir.

Supersedes #497. Same harness, but the CLI plumbing it needed is now split out:
the node/benchmark token dispatch is #591, which this PR stacks on, and the
non-determinism this harness uncovered in extend_proofs_greedily merged separately
as #590. What is left here is the benchmark itself. Review #591 first; the base
moves to main once it lands.

$ make bench
Block-building benchmark — synthetic workload (mock crypto)
  validators=8 warmup_slots=8 iterations=10 proofs_per_data=1 seed=42
  ...
  phase              count        min       mean        p50        p90        max
  compact               10    0.000ms    0.000ms    0.000ms    0.000ms    0.000ms
  select_payloads       10    0.001ms    0.001ms    0.001ms    0.001ms    0.001ms
  stf_simulate          10    0.011ms    0.011ms    0.011ms    0.011ms    0.011ms
  overhead              10    0.047ms    0.048ms    0.048ms    0.049ms    0.049ms
  wall                  10    0.060ms    0.061ms    0.061ms    0.062ms    0.062ms

What Changed

File Change
bin/ethlambda/src/benchmark/{mod,corpus,report}.rs New harness: seeded synthetic chain over InMemoryBackend, per-slot pool seeding into the pending pool (promoted by the proposal tick, as in production), iteration loop driving produce_block_with_signatures + block import; stats + human/JSON report
bin/ethlambda/src/command.rs benchmark joins node as a dispatched token, parsed by a clap::Parser of its own so harness arguments never enter CliOptions; argv[0] is rewritten to ethlambda benchmark so usage lines name the sub-command that owns them
bin/ethlambda/src/main.rs main is synchronous: it dispatches, and only the node path enters the tokio runtime (run_node carries the #[tokio::main] attributes). Benchmark logs go to stderr at WARN so the report owns stdout
crates/storage/{lib,store}.rs Export NEW_PAYLOAD_CAP so the harness rejects --proofs-per-data batches the pending pool would evict whole
bin/ethlambda/build.rs Embed the resolved leanSig and leanVM revisions from Cargo.lock into reports (leansig tracks the moving devnet4 branch; leanVM does the aggregation, so its pinned rev moves the measured crypto too). The per-[[package]] parse collects name and source before extracting the rev, so it does not depend on TOML field order
Makefile, .github/workflows/ci.yml make bench; seconds-fast mock smoke step in the Test job validating the JSON contract
docs/plans/block-building-benchmark.md Design doc and milestone roadmap

Correctness / Behavior Guarantees

  • cli.rs is not touched, and the node runtime is unchanged. The seven node-required
    arguments stay plain PathBuf/String, so clap keeps emitting its own
    missing-argument errors and there is no Option<T> to unwrap on the node path. That
    replaces feat(cli): add offline block-building benchmark subcommand #497's subcommand_negates_reqs approach, which needed both.
  • Determinism: same seed + params → identical per-iteration block roots (recorded in
    the JSON, so a baseline-vs-optimized diff proves an optimization changed only speed,
    not attestation selection). Verified across repeated runs. The harness never reads the
    wall clock into results.
  • Exact phase attribution with zero hot-path changes: per-iteration
    select_payloads/compact/stf_simulate come from the sample sums of the existing
    lean_block_proposal_attestation_build_phase_seconds histogram, deltaed between
    iterations; overhead is the clamped remainder of wall minus the phases.
  • The benchmark never starts the tokio runtime, so it cannot park a worker thread for the
    duration of a CPU-bound run.

Tests Added / Run

  • command.rs unit tests cover the benchmark token: it parses with no node argument, it
    rejects node flags, and its usage line names the sub-command.
  • Verified by hand on this branch: make bench; the CI smoke assertion
    (jq -e '.schema_version == 1 and (.samples | length == 3)'); identical block-root
    sequences across two runs at the same seed; and ethlambda --genesis config.yaml still
    failing with clap's own missing-argument list.
  • make fmt, make lint, make test (582 tests, 30 suites) — all clean.

Related Issues / PRs

✅ Verification Checklist

  • Ran make fmt — clean
  • Ran make lint (clippy with -D warnings) — clean
  • Ran make test (cargo test --workspace --profile release-fast) — all passing

`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. Phases 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 records its block
root, so a baseline-vs-optimized diff proves an optimization changed only
speed and not attestation selection.

This is milestone M1 (mock crypto) of docs/plans/block-building-benchmark.md;
M2 adds real XMSS/leanVM pools and the seal phase, M3 replay-from-datadir.

Dispatch goes through the `benchmark` token that command.rs already
strips, parsed by a `clap::Parser` of its own, so the harness arguments
never enter CliOptions and the node parser keeps the exact shape it has
today. That replaces the earlier `subcommand_negates_reqs` approach,
which forced all seven node-required arguments to `Option<T>` and an
unwrap helper on the node path for an invariant clap already enforced.

`main` is now synchronous: it dispatches, 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. Benchmark logs go to
stderr at WARN so the report owns stdout and stays pipe-clean for
`--format json | jq`.

NEW_PAYLOAD_CAP becomes public so the harness can reject a
--proofs-per-data batch the pending pool would evict whole. build.rs
embeds the resolved leansig and leanVM revisions from Cargo.lock into
reports, since both move the measured crypto. `make bench` runs it, and
CI adds a seconds-fast mock smoke step asserting the JSON contract.
@github-actions

Copy link
Copy Markdown

🤖 Kimi Code Review

Overall Assessment: Well-structured, secure benchmark harness with proper isolation between node and benchmark code paths. No critical consensus bugs or security vulnerabilities. Minor maintainability issues noted below.


build.rs (bin/ethlambda)

Issue: Fragile manual TOML parsing of Cargo.lock.

  • Line 63-78: Parsing relies on split("[[package]]") and line prefixes. This breaks if Cargo changes lockfile formatting (e.g., adds inline tables, different quoting) or if package names contain escaped characters.
  • Line 43: Path construction join("../../Cargo.lock") assumes the crate is always two levels deep from workspace root. If the crate moves, builds fail or embed "unknown" silently.

Recommendation: Use the cargo-lock crate (already in ecosystem) or toml crate to parse the lockfile robustly. If avoiding deps, add a build.rs test that fails CI if ETHLAMBDA_LEANSIG_REV is "unknown" when it shouldn't be.


benchmark/corpus.rs

Issue: Hardcoded pubkey size may drift from type definition.

  • Line 88: let mut bytes = [0u8; 52]; assumes ValidatorPubkeyBytes is 52 bytes. If the type changes (e.g., XMSS parameter upgrade), this compiles but fails at SSZ serialization boundaries or crypto ops.

Recommendation: Use ValidatorPubkeyBytes::size() or std::mem::size_of::<ValidatorPubkeyBytes>() to keep the mock data aligned with the real type.


benchmark/mod.rs

Issue: Hardcoded metric name coupling.

  • Line 226: const PHASE_HISTOGRAM: &str = "lean_block_proposal_attestation_build_phase_seconds";
    If the metric name changes in ethlambda-blockchain, the benchmark compiles but reports all phases as zero/missing at runtime.

Recommendation: Export the metric name constant from ethlambda-blockchain (e.g., pub const BLOCK_PROPOSAL_PHASE_HISTOGRAM: &str = ...) and import it here.

Issue: Potential silent precision loss in timing.

  • Line 197: overhead_seconds clamps negative values to 0.0 due to floating-point rounding. While commented, if overhead consistently measures as 0.0 due to phase sums exceeding wall time, it masks measurement errors.

Recommendation: Log a warning if overhead_seconds == 0.0 and wall time > 1ms, or if the unattributed time exceeds 5% of wall time (stricter than the 2% comment suggests).


benchmark/report.rs

Issue: Population vs. sample standard deviation.

  • Line 277: Variance divides by count (population). For benchmark samples, sample standard deviation (divide by count - 1) is statistically more appropriate for small iteration counts, though the difference is negligible for N=10.

Note: Document that CV uses population stddev if this is intentional.


command.rs

Code Quality: Excellent backward compatibility handling. The args.drain(..2) manipulation for benchmark subcommand argv[0] rewriting is correct and preserves clap's usage strings properly.


main.rs

Security/Isolation: Good separation of concerns.

  • Line 73-82: Benchmark runs synchronously on main thread without Tokio runtime, eliminating scheduling noise for CPU-bound measurements. Node path retains async runtime.
  • Line 93-102: Benchmark logging directed to stderr with WARN level keeps stdout pipe-clean for JSON reports. Correct use of tracing_subscriber.

Consensus & State Transition Correctness

Validations:

  1. Attestation partitioning: participant_groups (corpus.rs:98) correctly creates disjoint validator sets using modulo indexing, accurately simulating committee aggregation.
  2. Proposer rotation: slot % options.num_validators (mod.rs:164) matches the round-robin proposer selection in is_proposer.
  3. State advancement: Importing built blocks via on_block_without_verification (mod.rs:188) ensures process_slots costs remain constant across iterations rather than growing linearly with slot number.
  4. Pool capacity guard: The check at mod.rs:135 prevents silent eviction of seeded attestations when proofs_per_data > NEW_PAYLOAD_CAP.

Note on on_block_without_verification: Appropriate for this context since the block was produced by the same process immediately before import; no untrusted input is processed.


Security

  • No unsafe code introduced in diff.
  • Mock crypto isolation: The mock_crypto flag correctly conflicts with enable_proposer_aggregation (mod.rs:74) to prevent undefined behavior from empty proof aggregation.
  • Determinism: splitmix64 PRNG (corpus.rs:108) is cryptographically insecure but appropriate for deterministic test data generation. Seeded state ensures reproducible benchmarks.

CI Integration (ci.yml)

Line 78-84: The smoke test validates JSON schema version and sample count. Consider adding a determinism check:

# Run twice with same seed and compare block roots
 cargo run ... --format json > run1.json
 cargo run ... --format json > run2.json
 jq -e '.samples[].block_root' run


---
*Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt*

@github-actions

Copy link
Copy Markdown

🤖 Codex Code Review

  1. The new benchmark’s determinism guarantee is not actually enforced. SyntheticOptions promises identical block roots for the same seed, and seed_pool says insertion order pins proof choice, but extend_proofs_greedily still keeps candidate indices in a HashSet and selects with max_by_key on coverage only. When two proofs add the same marginal coverage, tie-breaking depends on randomized hash iteration order, so repeated runs can produce different selected proofs and different block_roots. That breaks the benchmark’s core regression signal. Add an explicit stable tie-break, e.g. lowest original index.

  2. The reported percentiles are off by one for many sample sizes. percentile is documented as “nearest-rank”, but round((len - 1) * q) is not nearest-rank. For example, with 10 samples, p50 becomes the 6th sample instead of the 5th; with 4 samples, the current test explicitly locks in the upper median behavior (report.rs). Since these summaries are the main benchmark output, this will mislead performance comparisons. Use a true nearest-rank formula such as ceil(len * q).max(1) - 1, or rename/document the current convention if intentional.

Aside from those benchmark/reporting issues, I did not see a new consensus-rule regression in fork choice, attestation validation, STF, XMSS, or SSZ handling from this PR’s direct code changes.

I could not run cargo test end-to-end here because the environment cannot fetch the repo’s git dependencies and the default cargo cache is read-only.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@pablodeymo

Copy link
Copy Markdown
Collaborator Author

Split into three smaller PRs, each with a goal of its own:

#595 + #596 reconstruct what was reviewed here, plus a per-iteration table (#595 would otherwise collect the phase breakdown without ever showing it). Each of the three was verified independently: make fmt, make lint, make test, and the benchmark run end-to-end.

Closing in favour of that stack.

@pablodeymo pablodeymo closed this Aug 26, 2026
@pablodeymo
pablodeymo deleted the feat/block-building-benchmark-harness branch August 26, 2026 19:54
Sahilgill24 pushed a commit to Sahilgill24/ethlambda that referenced this pull request Sep 1, 2026
…ass#595)

## 🗒️ Description / Motivation

Adds `ethlambda benchmark synthetic` — an offline harness that measures
block building
**exactly 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.

> **Stacked on lambdaclass#591**, which adds the `node`/`benchmark` token dispatch
this uses. Review
> that first; the base moves to `main` once it lands.

```
$ make bench
Block-building benchmark — synthetic workload (mock crypto)
  validators=8 warmup_slots=8 iterations=10 proofs_per_data=1 seed=42
  enable_proposer_aggregation=false max_attestations_per_block=3
  ethlambda/v0.1.0/aarch64-apple-darwin/rustc-v1.97.1 os=macos arch=aarch64 threads=14

  iter           compact  select_payloads     stf_simulate   overhead       wall         root
  1              0.000ms          0.002ms          0.015ms    0.068ms    0.085ms   0x7282cc99
  2              0.000ms          0.002ms          0.015ms    0.066ms    0.083ms   0xb9065af0
  3              0.000ms          0.002ms          0.015ms    0.064ms    0.081ms   0x303f6b0f
```

## How to read this

In this order — each piece is understandable without the next:

1. **`corpus.rs`** — the workload. Deterministic validators, a genesis
store over
`InMemoryBackend`, and `seed_pool`, which fills the pending pool for one
slot and
   reports how many entries the next build will see.
2. **`build_one_slot`** (`mod.rs`) — one slot end to end: seed, time the
build, import the
block. 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.
3. **`PhaseTimer`** (`mod.rs`) — `start()` before the build, `finish()`
after. Two
readings 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.
4. **`run_synthetic`** (`mod.rs`) — validate, set up, loop over slots,
report. 46 lines.
5. **`report.rs`** — the types and the per-iteration table.

## What Changed

| File | Change |
|------|--------|
| `bin/ethlambda/src/benchmark/mod.rs` | Harness driver: clap options +
validation, `run_synthetic`'s slot loop, `build_one_slot` for one slot's
work, and `PhaseTimer` for per-phase attribution |
| `bin/ethlambda/src/benchmark/corpus.rs` | Seeded synthetic corpus:
genesis store over `InMemoryBackend`, deterministic pubkeys via
splitmix64, and per-slot pool seeding in fixed insertion order, which
also rejects a batch the pool would evict whole |
| `bin/ethlambda/src/benchmark/report.rs` | Params/Environment/Sample
types and the human-readable per-iteration table |
| `bin/ethlambda/src/command.rs` | `benchmark` joins `node` as a second
clap sub-command, so clap lists it in `--help` and names it in its own
usage lines. The node payload becomes `Box<CliOptions>` now that a much
smaller variant sits beside it |
| `bin/ethlambda/src/main.rs` | `main` becomes synchronous and
dispatches; only the node path enters the tokio runtime (`run_node`
carries the `#[tokio::main]` attributes). Benchmark logs go to stderr at
WARN so the report owns stdout |
| `crates/storage/{lib,store}.rs` | Export `NEW_PAYLOAD_CAP` so the
harness rejects a `--proofs-per-data` batch the pending pool would evict
whole |
| `Makefile` | `make bench` (override `BENCH_ARGS` to customize) |

## Correctness / Behavior Guarantees

- **`cli.rs` is not touched by this PR and the node runtime is
unchanged.** The harness
arguments live in their own `Args` group; the node's stay plain
`PathBuf`/`String`, so
  clap keeps emitting its own missing-argument errors.
- **It measures the production path**, not a copy: the harness enters
through
`produce_block_with_signatures`, the same function
`BlockChainServer::propose_block`
calls, and seeds the *pending* pool so the proposal tick promotes it
exactly as on a
  live node.
- **Determinism:** same seed + params → identical per-iteration block
roots. Verified
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.
- **Exact phase attribution with zero hot-path changes:** per-iteration
`select_payloads`/`compact`/`stf_simulate` come from the sample sums of
the existing
`lean_block_proposal_attestation_build_phase_seconds` histogram, deltaed
between
iterations, with a per-phase assertion that the count advanced by
exactly one.
  `overhead` is the clamped remainder of wall minus the phases.
- The benchmark never starts the tokio runtime, so it cannot park a
worker thread for the
  duration of a CPU-bound run.

## Tests Added / Run

- `corpus.rs`: participant groups partition every validator; synthetic
pubkeys are
  deterministic for a seed.
- `command.rs`: the benchmark token parses with no node argument,
rejects node flags, and
  its usage line names the sub-command.
- Verified by hand: `make bench`; identical block-root sequences across
two runs at the
same seed; `ethlambda --genesis config.yaml` still failing with clap's
own
  missing-argument list.
- `make fmt`, `make lint`, `make test` (576 tests, 30 suites) — all
clean.

## Related Issues / PRs

- Stacked on lambdaclass#591; design doc in the accompanying docs PR
- Followed by the comparable-reports PR
- Splits the now-closed lambdaclass#497 / lambdaclass#593
- Related to lambdaclass#465

## ✅ Verification Checklist

- [x] Ran `make fmt` — clean
- [x] Ran `make lint` (clippy with `-D warnings`) — clean
- [x] Ran `make test` (`cargo test --workspace --profile release-fast`)
— all passing
Sahilgill24 pushed a commit to Sahilgill24/ethlambda that referenced this pull request Sep 1, 2026
…lambdaclass#596)

## 🗒️ Description / Motivation

Per-iteration rows show what one build cost. Comparing an optimization
against a baseline
needs three more things, and this adds them: aggregate statistics, build
provenance, and
machine-readable output.

Third of three (design doc → harness → **this**).

> **Stacked on the harness PR.** Its diff here is additive — the
per-iteration rows stay,
> the summary is appended below them.

```
  iter           compact  select_payloads     stf_simulate   overhead       wall         root
  1              0.000ms          0.002ms          0.015ms    0.068ms    0.085ms   0x7282cc99
  2              0.000ms          0.002ms          0.015ms    0.066ms    0.083ms   0xb9065af0
  3              0.000ms          0.002ms          0.015ms    0.064ms    0.081ms   0x303f6b0f

  phase              count        min       mean        p50        p90        max
  compact                3    0.000ms    0.000ms    0.000ms    0.000ms    0.000ms
  select_payloads        3    0.002ms    0.002ms    0.002ms    0.002ms    0.002ms
  stf_simulate           3    0.015ms    0.015ms    0.015ms    0.015ms    0.015ms
  overhead               3    0.064ms    0.066ms    0.066ms    0.068ms    0.068ms
  wall                   3    0.081ms    0.083ms    0.083ms    0.085ms    0.085ms
```

## What Changed

| File | Change |
|------|--------|
| `bin/ethlambda/src/benchmark/report.rs` | `Stats`/`Summary` plus
`stats()`, `percentile()` and the aggregate table: count, min, mean,
p50, p90, max per phase, and a CV flagged above 10%. `schema_version` +
`to_json()`. `Environment` gains the two resolved crypto revisions |
| `bin/ethlambda/build.rs` | Resolve the leansig and leanVM revisions
from `Cargo.lock` into `rustc-env` vars. The per-`[[package]]` parse
collects `name` and `source` before extracting the rev, so it does not
depend on TOML field order |
| `bin/ethlambda/src/benchmark/mod.rs` | `--format human\|json` and
`--output <path>` |
| `bin/ethlambda/Cargo.toml`, `Cargo.lock` | `serde_json` |
| `.github/workflows/ci.yml` | Seconds-fast mock smoke step in the Test
job asserting the JSON contract |

## Correctness / Behavior Guarantees

- **Nearest-rank percentiles, no interpolation.** Sample counts are
small, so an exact
  observed value beats a blend of two neighbours.
- **Outliers are never discarded** and the raw per-iteration rows stay
above the summary,
so a heavy tail stays visible instead of being averaged away. A CV above
10% is flagged
  so a noisy run is not read as a result.
- **Provenance is a comparability guard, not decoration.** leansig is
pinned to a moving
branch and leanVM does the signature aggregation, so either revision
moving moves the
measured crypto. Two reports that disagree on them are not comparable,
and without this
  the report cannot say so.
- **The JSON shape is pinned by CI**, so a change to the report contract
cannot land
unnoticed. Logs already go to stderr, so the JSON pipes straight into
`jq`.
- Node behavior is untouched; `build.rs` only adds env vars consumed by
the report.

## Tests Added / Run

- `report.rs`: percentile on a single sample and on odd/even lengths;
`stats()` against a
known set whose population stddev gives CV = 0.4; `stats()` on empty
input is zeroed.
- Verified by hand: the CI assertion
`jq -e '.schema_version == 1 and (.samples | length == 3)'` passes, and
reports carry
  both resolved revisions.
- `make fmt`, `make lint`, `make test` (580 tests, 30 suites) — all
clean.

## Related Issues / PRs

- Stacked on the harness PR; design doc in the accompanying docs PR
- Splits the now-closed lambdaclass#497 / lambdaclass#593
- Related to lambdaclass#465

## ✅ Verification Checklist

- [x] Ran `make fmt` — clean
- [x] Ran `make lint` (clippy with `-D warnings`) — clean
- [x] Ran `make test` (`cargo test --workspace --profile release-fast`)
— all passing

---------

Co-authored-by: Tomás Grüner <47506558+MegaRedHand@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant