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

[spec test]: Ensure duplicate deposits are handled successfully during electra epoch processing #4021

Closed
ralexstokes opened this issue Nov 25, 2024 · 3 comments · Fixed by #4024
Labels

Comments

@ralexstokes
Copy link
Member

ralexstokes commented Nov 25, 2024

We may already have a test case for this, but we should verify and write one if we don't.

This issue is coming from a bug on mekong where three clients incorrectly handled the following scenario:

Two deposits to the same validator in an epoch (the mekong setting had a deposit in one block and then another two blocks later to the same validator). The first deposit was fresh (aka the validator was not on-chain beforehand), and the second would be considered a "top-up".

During epoch processing, we had two separate issues:

  1. The two deposits were treated as one (e.g. for two deposits of 32 ETH, the resulting state only had one validator of 32 ETH, instead of 64 ETH).
  2. The same validator was added to the state's registry twice, one for each deposit (rather than one validator with the balance summing the amount in each deposit).
@0xshikhar
Copy link

Hey @ralexstokes , would like to work on this task - can you assign me that. Could you clarify the remaining tasks for this issue? Any additional context would be appreciated. Thanks!

@ralexstokes
Copy link
Member Author

hi @0xshikhar I think @mkalinin is looking at this, but if you want to make a pass at electra spec tests broadly, that would be very helpful

@mkalinin
Copy link
Contributor

I think we need more tests to cover epoch processing in a way that would help revealing such bugs before testnets, more detailed description is here #4025. @0xshikhar this is something worth looking into as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants