Skip to content

ci(l1,l2): add 'build block' benchmark to PR checks#2827

Merged
ilitteri merged 42 commits into
mainfrom
run_build_block_bench_in_ci
Jun 24, 2025
Merged

ci(l1,l2): add 'build block' benchmark to PR checks#2827
ilitteri merged 42 commits into
mainfrom
run_build_block_bench_in_ci

Conversation

@fkrause98

@fkrause98 fkrause98 commented May 16, 2025

Copy link
Copy Markdown
Contributor

Motivation

Make the "build block" benchmark run in the CI.

@github-actions

Copy link
Copy Markdown

Lines of code report

Total lines added: 0
Total lines removed: 1
Total lines changed: 1

Detailed view
+--------------------------------------------------+-------+------+
| File                                             | Lines | Diff |
+--------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/bench/build_block_benchmark.rs | 154   | -1   |
+--------------------------------------------------+-------+------+

@github-actions

Copy link
Copy Markdown

Benchmark for d95a859

Click to view benchmark
Test Base PR %
block payload building bench 171.5±1.87ms 169.9±1.71ms -0.93%

@github-actions

Copy link
Copy Markdown

Benchmark for 44ebeb1

Click to view benchmark
Test Base PR %
block payload building bench 173.4±2.73ms 171.5±1.72ms -1.10%

@@ -0,0 +1,29 @@
name: '"Build block" benchmark'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look into the names of other workflows. They use perf nomenclature in naming

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it seems this could be added as another job inside Benchmark Block execution instead of new workflow?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! About merging the workflows: I think it's best to do it this way since I'm using another github action to post the results and the diff.


on:
pull_request:
branches: ["**"]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to run this on every pr or just the one that has performance label?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed it to only run to if a PR has the perf label

@Arkenan

Arkenan commented Jun 16, 2025

Copy link
Copy Markdown
Collaborator

I think this PR is generally good, but I agree with Martin's comments. I'm in favor of merging once those are solved and the upstream branch is merged as well.

Base automatically changed from build_block_benchmark to main June 17, 2025 16:11
@fkrause98 fkrause98 changed the title ci: add 'build block' benchmark to PR checks ci(l1,l2): add 'build block' benchmark to PR checks Jun 17, 2025

@Arkenan Arkenan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread .github/workflows/pr_perf_build_block_bench.yml Outdated

@mpaulucci mpaulucci left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with comment

Co-authored-by: Martin Paulucci <martin.c.paulucci@gmail.com>
@ilitteri ilitteri enabled auto-merge June 24, 2025 13:38
@ilitteri ilitteri added this pull request to the merge queue Jun 24, 2025
Merged via the queue into main with commit b135beb Jun 24, 2025
22 checks passed
@ilitteri ilitteri deleted the run_build_block_bench_in_ci branch June 24, 2025 14:24
pedrobergamini pushed a commit to pedrobergamini/ethrex that referenced this pull request Aug 24, 2025
**Motivation**

Make the "build block" benchmark run in the CI.

---------

Co-authored-by: Martin Paulucci <martin.c.paulucci@gmail.com>
edg-l added a commit that referenced this pull request May 11, 2026
The zkevm@v0.3.3 fixture bundle (the only bundle that ships
executionWitness, used by test-stateless-zkevm) is filled against an
older bal spec and disagrees with bal@v7.0.0 gas accounting:
storage_set/new_account/cpsb constants pre-recalibration plus pre-
EELS-#2815/#2816/#2823/#2827/#2828 refund-channel semantics.

Skips the 21 remaining gas mismatches in the eip8025_optional_proofs
filter (witness_codes_*, witness_state_*, validation_state_*),
analogous to the bal@v5.6.1 block already at the top of the list.
Re-enable once the zkevm bundle is regenerated against bal-7.
edg-l added a commit that referenced this pull request May 13, 2026
The zkevm@v0.3.3 fixture bundle (the only bundle that ships
executionWitness, used by test-stateless-zkevm) is filled against an
older bal spec and disagrees with bal@v7.0.0 gas accounting:
storage_set/new_account/cpsb constants pre-recalibration plus pre-
EELS-#2815/#2816/#2823/#2827/#2828 refund-channel semantics.

Skips the 21 remaining gas mismatches in the eip8025_optional_proofs
filter (witness_codes_*, witness_state_*, validation_state_*),
analogous to the bal@v5.6.1 block already at the top of the list.
Re-enable once the zkevm bundle is regenerated against bal-7.
edg-l added a commit that referenced this pull request May 13, 2026
The zkevm@v0.3.3 fixture bundle (the only bundle that ships
executionWitness, used by test-stateless-zkevm) is filled against an
older bal spec and disagrees with bal@v7.0.0 gas accounting:
storage_set/new_account/cpsb constants pre-recalibration plus pre-
EELS-#2815/#2816/#2823/#2827/#2828 refund-channel semantics.

Skips the 21 remaining gas mismatches in the eip8025_optional_proofs
filter (witness_codes_*, witness_state_*, validation_state_*),
analogous to the bal@v5.6.1 block already at the top of the list.
Re-enable once the zkevm bundle is regenerated against bal-7.
akshay-ap pushed a commit to akshay-ap/ethrex that referenced this pull request May 19, 2026
**Motivation**

Bring ethrex up to bal-devnet-7 (BAL fixtures `bal@v7.1.1`). Stacked
on top of #bal-devnet-6-pr (now in main).

**Description**

Aligns EIP-8037 state-gas accounting with bal-devnet-7 spec progression
(EELS PRs lambdaclass#2815 / lambdaclass#2816 / lambdaclass#2823 / lambdaclass#2827 / lambdaclass#2828 / lambdaclass#2836 / lambdaclass#2845 /
lambdaclass#2848),
bumps Amsterdam fixtures from `snobal-devnet-6@v1.1.0` to `bal@v7.1.1`,
and bumps the pinned hive version past the ethrex `--http.api` fix.

Main changes:

- EIP-8037 state-gas alignment with bal-devnet-7:
  - System-call state-gas reservoir.
  - Halt refunds spilled state gas (Policy A).
  - Tx-level CREATE failure refunds intrinsic `NEW_ACCOUNT`;
    `intrinsic_state_gas_charged` preserved across the failure path.
  - Tx-CREATE collision refund with regular-gas burn; billing matches
    EELS.
  - Cross-frame revert leaks inline credits.
  - Cross-frame revert reservoir formula fix.
  - Block-level `state_gas_used` subtracts `state_refund`.
- Remove same-tx SELFDESTRUCT state-gas refund (EELS PR lambdaclass#2845, v7.1.0).
- EIP-7702:
  - `set_delegation` refund via dedicated `state_refund` channel.
  - `set_delegation` refunds `AUTH_BASE` on existing delegation
    (EELS PR lambdaclass#2836).
  - `set_delegation` refunds `AUTH_BASE` on delegation clear
    (EELS PR lambdaclass#2848, v7.1.1).
- levm fixes pulled from main:
  - `revert` doesn't unmark the account as existing (lambdaclass#6592).
  - Account erroneously considered as existing after zero-value transfer
    (lambdaclass#6591).
- Tooling / tests:
  - Per-tx gas-dimension dump on block `gas_used` mismatch.
  - Bump Amsterdam fixtures to `bal@v7.1.1`.
  - Annotate BAL balance-mismatch errors with gas-equivalent diff and
    recognised state-gas constant multiples.
  - Unskip 74 bal-devnet-6 Amsterdam fixtures now passing.
  - Skip 21 stale EIP-8025 fixtures pinned at `bal@v5.7.0`
    (zkevm@v0.3.3 bundle, pre-bal-7).
  - Drop stale bal-devnet-6 known-issues entries from
    `docs/known_issues.md` and hive `KNOWN_EXCLUDED_TESTS`.
- CI:
  - Bump pinned hive version past the ethrex `--http.api` flag
    feature-detect fix (`c4d839b3`, hive lambdaclass#1485). Without this, hive
    starts ethrex with the default HTTP namespace allowlist
    (`eth,net,web3`) and tests touching `admin_*`/`debug_*`/`txpool_*`
    fail.

**Local test run**

`./run_test.sh` against `tests-bal@v7.1.1`: 2,145 / 2,145 pass.
`cargo test -p ethrex-test --tests`: 453 / 453 pass.

**Checklist**

- [ ] Updated `STORE_SCHEMA_VERSION` (crates/storage/lib.rs) if the PR
  includes breaking changes to the `Store` requiring a re-sync.

---------

Co-authored-by: Lucas Fiegl <iovoid@users.noreply.github.com>
Co-authored-by: Ivan Litteri <67517699+ilitteri@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.

4 participants