Skip to content

chore(l1): make hash public in BlockHeader and use instead of compute_block_hash#2959

Merged
fmoletta merged 9 commits into
mainfrom
make-header-hash-public
May 29, 2025
Merged

chore(l1): make hash public in BlockHeader and use instead of compute_block_hash#2959
fmoletta merged 9 commits into
mainfrom
make-header-hash-public

Conversation

@rodrigo-o

@rodrigo-o rodrigo-o commented May 28, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 from fix(l1): validate received blocks from peers #2658
  • Increased the size of the DB (it was limiting Holesky syncing)

Closes: #2926

@rodrigo-o rodrigo-o self-assigned this May 28, 2025
@rodrigo-o rodrigo-o requested a review from a team as a code owner May 28, 2025 19:44
@rodrigo-o rodrigo-o changed the title Make header hash public chore(l1): make hash public in BlockHeader and use instead of compute_block_hash May 28, 2025
@github-actions

github-actions Bot commented May 28, 2025 •
edited
Loading

Copy link
Copy Markdown

Lines of code report

Total lines added: 35
Total lines removed: 14
Total lines changed: 49

Detailed view 
+----------------------------------------------------+-------+------+
| File                                               | Lines | Diff |
+----------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/utils.rs                 | 61    | -4   |
+----------------------------------------------------+-------+------+
| ethrex/crates/l2/prover/bench/src/rpc/db.rs        | 477   | +32  |
+----------------------------------------------------+-------+------+
| ethrex/crates/l2/utils/prover/db.rs                | 198   | -1   |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/peer_handler.rs       | 645   | -2   |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/sync.rs               | 568   | +2   |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/fork_choice.rs | 352   | -4   |
+----------------------------------------------------+-------+------+
| ethrex/crates/storage/store.rs                     | 1232  | -1   |
+----------------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/in_memory.rs        | 599   | -2   |
+----------------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/libmdbx.rs          | 1357  | +1   |
+----------------------------------------------------+-------+------+

fmoletta
fmoletta approved these changes May 29, 2025
View reviewed changes

@fmoletta fmoletta 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.

🚀

@fmoletta fmoletta added this pull request to the merge queue May 29, 2025
Merged via the queue into main with commit 43d31da May 29, 2025
27 checks passed
@fmoletta fmoletta deleted the make-header-hash-public branch May 29, 2025 13:31
pedrobergamini pushed a commit to pedrobergamini/ethrex that referenced this pull request Aug 24, 2025
@rodrigo-o
chore(l1): make hash public in BlockHeader and use instead of `co…
67ad0e7 
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@fmoletta fmoletta fmoletta approved these changes

@tomip01 tomip01 tomip01 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rodrigo-o rodrigo-o

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use hash instead of compute_block_hash from headers

3 participants

@rodrigo-o @fmoletta @tomip01