Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add EIP: Double speed for hash_tree_root #8995

Merged
merged 6 commits into from
Nov 3, 2024

Conversation

etan-status
Copy link
Contributor

No description provided.

@eth-bot
Copy link
Collaborator

eth-bot commented Oct 23, 2024

✅ All reviewers have approved.

@eth-bot eth-bot added e-consensus Waiting on editor consensus e-review Waiting on editor to review labels Oct 23, 2024
@github-actions github-actions bot added the w-ci Waiting on CI to pass label Oct 23, 2024
@etan-status etan-status marked this pull request as ready for review October 23, 2024 16:35
Copy link

The commit dc94183 (as a parent of 76bcc83) contains errors.
Please inspect the Run Summary for details.

EIPS/eip-####.md Outdated Show resolved Hide resolved
EIPS/eip-####.md Outdated Show resolved Hide resolved
etan-status and others added 3 commits October 24, 2024 11:00
Co-authored-by: Andrew B Coathup <28278242+abcoathup@users.noreply.github.com>
Co-authored-by: Andrew B Coathup <28278242+abcoathup@users.noreply.github.com>
@github-actions github-actions bot added c-new Creates a brand new proposal s-draft This EIP is a Draft t-core and removed w-ci Waiting on CI to pass labels Oct 24, 2024
@g11tech
Copy link
Contributor

g11tech commented Oct 30, 2024

@twoeths could you help review this new EIP PR having work considerably on ssz sha256 internals

@twoeths
Copy link

twoeths commented Oct 31, 2024

the change makes a lot of sense to me. For lodestar on holesky, it takes ~530ms for hash_tree_root on epoch transition with wasm sha256 and going to be 130ms - 150ms with hashtree so this helps a lot. I'm curious to know the hash_tree_root time at epoch transition for other clients, do you have this statistic for Nimbus @etan-status ?

@etan-status
Copy link
Contributor Author

Per block, the main effort is for the epoch participation flags. On epoch transition, the balance changes increase the load. Both of these scale with number of validators. I don't have exact timings for Nimbus, but if necessary we should create a reference state with 2 epochs worth of blocks for proper measurement. I have added this as an open item to the Ethereum Magicians thread.

@Giulio2002
Copy link
Contributor

Giulio2002 commented Oct 31, 2024

I think we need to evaluate this in a post-MaxEB world. I suspect, it is not going to be a significant improvement when less hashes change in the merkle tree. It is quite an invasive change and benefit is not super obvious imo. we could also make use of different hash functions such as blake3 which is also consistently faster than sha256 (14x)

Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm for draft

@eth-bot eth-bot enabled auto-merge (squash) November 3, 2024 04:20
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

@eth-bot eth-bot merged commit c5646bf into ethereum:master Nov 3, 2024
10 checks passed
Copy link

@Deluaney1 Deluaney1 left a comment

Choose a reason for hiding this comment

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

Deposit

@etan-status etan-status deleted the sz-htr2x branch November 4, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-new Creates a brand new proposal e-consensus Waiting on editor consensus e-review Waiting on editor to review s-draft This EIP is a Draft t-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants