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

fix: early return for packing local deposits when EIP-6110 is applied #14697

Merged

Conversation

syjn99
Copy link
Contributor

@syjn99 syjn99 commented Dec 8, 2024

What type of PR is this?

Optimization

What does this PR do? Why is it needed?

As mentioned at #14698, a block producer in post-Electra doesn't have to consider the deposit tree from the eth1 logs. All deposits will be delivered directly via execution requests, so it is expected that packDepositsAndAttestations will always return the empty list of deposit.

This PR addresses a performance issue when a block producer builds a block by returning earlier if EIP-6110 is fully enabled. For the worst case(someone spamming deposit transactions), the proposer should rebuild a deposit tree again and again from unfinalized one, delaying the propagation of the produced block. It seems finalization of deposit matters, but this PR could be the first step to ignoring the deposit cache.

Acknowledgements

  • I have read CONTRIBUTING.md.
  • I have made an appropriate entry to CHANGELOG.md.
  • I have added a description to this PR with sufficient context for reviewers to understand this PR.

@syjn99 syjn99 marked this pull request as ready for review December 8, 2024 07:29
@syjn99 syjn99 requested a review from a team as a code owner December 8, 2024 07:30
@james-prysm james-prysm self-requested a review January 6, 2025 16:02
@james-prysm
Copy link
Contributor

This seems good sorry for the late response, could you change the changelog to the changelog fragment? it follows our new guidelines.

This optimization is just a partial item to #14698 correct? you mentioned some items around the deposit cache as well.

@syjn99
Copy link
Contributor Author

syjn99 commented Jan 23, 2025

@james-prysm Sure, done at a2dc7af.

This optimization is just a partial item to #14698 correct? you mentioned some items around the deposit cache as well.

Yes, you're right.

@james-prysm james-prysm added the Electra electra hardfork label Jan 23, 2025
Copy link
Contributor

@james-prysm james-prysm left a comment

Choose a reason for hiding this comment

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

thanks for this optimization!

@james-prysm james-prysm added this pull request to the merge queue Jan 23, 2025
Merged via the queue into prysmaticlabs:develop with commit c8cb0f3 Jan 23, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Electra electra hardfork
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants