Skip to content

refactor(l1): move hash from Block to BlockHeader#2845

Merged
DiegoCivi merged 2 commits into
mainfrom
blockheader_refactor
May 21, 2025
Merged

refactor(l1): move hash from Block to BlockHeader#2845
DiegoCivi merged 2 commits into
mainfrom
blockheader_refactor

Conversation

@DiegoCivi

@DiegoCivi DiegoCivi commented May 19, 2025

Copy link
Copy Markdown
Contributor

Motivation

Block had the hash but the BlockHeader didn't so they had to be passed along together.

Description

Move the hash into BlockHeader, making it accesible to it and also to Block

Closes #2841

@DiegoCivi DiegoCivi changed the title Move hash from Block to BlockHeader refactor(l1): move hash from Block to BlockHeader May 19, 2025
@github-actions

github-actions Bot commented May 19, 2025

Copy link
Copy Markdown

Lines of code report

Total lines added: 9
Total lines removed: 51
Total lines changed: 60

Detailed view
+-----------------------------------------------+-------+------+
| File                                          | Lines | Diff |
+-----------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/blockchain/types.rs       | 437   | +1   |
+-----------------------------------------------+-------+------+
| ethrex/crates/blockchain/payload.rs           | 553   | +1   |
+-----------------------------------------------+-------+------+
| ethrex/crates/common/types/block.rs           | 718   | +2   |
+-----------------------------------------------+-------+------+
| ethrex/crates/common/types/genesis.rs         | 653   | +1   |
+-----------------------------------------------+-------+------+
| ethrex/crates/common/types/requests.rs        | 159   | -47  |
+-----------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/eth/mod.rs       | 165   | +1   |
+-----------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/types/block.rs   | 181   | +1   |
+-----------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/types/payload.rs | 251   | +1   |
+-----------------------------------------------+-------+------+
| ethrex/crates/storage/store.rs                | 1218  | +1   |
+-----------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs         | 637   | -1   |
+-----------------------------------------------+-------+------+
| ethrex/crates/vm/backends/revm/mod.rs         | 639   | -1   |
+-----------------------------------------------+-------+------+
| ethrex/crates/vm/errors.rs                    | 119   | -2   |
+-----------------------------------------------+-------+------+

excess_blob_gas: val.excess_blob_gas.map(|x| x.as_u64()),
parent_beacon_block_root: val.parent_beacon_block_root,
requests_hash: val.requests_hash,
..Default::default()

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.

Initializations with ..Default::default() were used instead of hash: OnceCell::new() so we wouldn't need to import once_cell and add it to the Cargo.toml as a dependency. Maybe this is to overkill and its nicer to add it anyways. Let me know!

@DiegoCivi DiegoCivi marked this pull request as ready for review May 20, 2025 21:10
@DiegoCivi DiegoCivi requested a review from a team as a code owner May 20, 2025 21:10

@JereSalo JereSalo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool.
One thing that caught my attention is that the issue #2841 says:
"This is not the case for BlockHeader, as we still need to compute the hash of a header once and then pass it along with the header itself which is not ideal."

Are there any actual cases in the code in which we are passing the BlockHeader alongside it's hash instead of just the BlockHeader now? (It's just a question, maybe there are no cases and it's fine)

@DiegoCivi

Copy link
Copy Markdown
Contributor Author

Are there any actual cases in the code in which we are passing the BlockHeader alongside it's hash instead of just the BlockHeader now?

Not right now i think. However, this issue comes from this conversation (here) where this would happen.

@DiegoCivi DiegoCivi added this pull request to the merge queue May 21, 2025
Merged via the queue into main with commit c6a54c2 May 21, 2025
22 checks passed
@DiegoCivi DiegoCivi deleted the blockheader_refactor branch May 21, 2025 18:19
github-merge-queue Bot pushed a commit that referenced this pull request May 29, 2025
…mpute_block_hash` (#2959)

**Motivation**

After #2845 we were still calculating the hash of headers every time
instead of using the `get_or_init` version.

**Description**

This PR does a couple of thing
- Make `hash` function public and `compute_block_hash` private in the
BlockHeader
- Replace all header `compute_block_hash` calls with `hash`
- On blocks, replace all `block.header.hash()` for `block.hash()`
instead given that it already delegates internally.
- Fixed an [outstanding
comment](#2658 (comment))
from #2658
- Increased the size of the DB (it was limiting Holesky syncing)

Closes Status: Open.
#2926
pedrobergamini pushed a commit to pedrobergamini/ethrex that referenced this pull request Aug 24, 2025
**Motivation**

`Block` had the hash but the `BlockHeader` didn't so they had to be
passed along together.
<!-- Why does this pull request exist? What are its goals? -->

**Description**

Move the hash into `BlockHeader`, making it accesible to it and also to
`Block`
<!-- A clear and concise general description of the changes this PR
introduces -->

<!-- Link to issues: Resolves lambdaclass#111, Resolves lambdaclass#222 -->

Closes lambdaclass#2841
pedrobergamini pushed a commit to pedrobergamini/ethrex that referenced this pull request Aug 24, 2025
…mpute_block_hash` (lambdaclass#2959)

**Motivation**

After lambdaclass#2845 we were still calculating the hash of headers every time
instead of using the `get_or_init` version.

**Description**

This PR does a couple of thing
- Make `hash` function public and `compute_block_hash` private in the
BlockHeader
- Replace all header `compute_block_hash` calls with `hash`
- On blocks, replace all `block.header.hash()` for `block.hash()`
instead given that it already delegates internally.
- Fixed an [outstanding
comment](lambdaclass#2658 (comment))
from lambdaclass#2658
- Increased the size of the DB (it was limiting Holesky syncing)

Closes Status: Open.
lambdaclass#2926
edg-l added a commit that referenced this pull request May 13, 2026
Mirrors EELS PR #2845 (tests-bal@v7.1.0, devnets/bal/7): all SELFDESTRUCT
state-gas refunds are removed from EIP-8037. Drops the
apply_same_tx_selfdestruct_state_refund pass and its call site in the
default hook; shared clamp-and-spill bookkeeping fields stay.
edg-l added a commit that referenced this pull request May 13, 2026
Mirrors EELS PR #2845 (tests-bal@v7.1.0, devnets/bal/7): all SELFDESTRUCT
state-gas refunds are removed from EIP-8037. Drops the
apply_same_tx_selfdestruct_state_refund pass and its call site in the
default hook; shared clamp-and-spill bookkeeping fields stay.
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.

L1: Move block hash cached computation from Block to BlockHeader

3 participants