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: eth1data votes error on block propose #1307

Merged
merged 7 commits into from
Sep 27, 2024
Merged

Conversation

rodrigo-o
Copy link
Collaborator

@rodrigo-o rodrigo-o commented Sep 24, 2024

Motivation

This errors were encountered on a long running session after passing the ChainSpec.get("SECONDS_PER_ETH1_BLOCK") * ChainSpec.get("ETH1_FOLLOW_DISTANCE") slots threshold.

Description
This PR solved 4 issues, from lower to higher priority:

  • The has_majority?/2 calculation was not updating the votes past half of period, effectively this wasn't an issue, worst case another vote has also half of the votes totalling to the 100%, but it was pretty useful when debugging, adding it doesn't modify functionality.
  • There were an old line of code trying to do a List.last/1 on a MapSet, this was part of some previous refactor.
  • The implementation of getting the first vote had some difference with the spec, it wasn't taking into account the previous eth1data when the valid_votes was empty, and it lacked a comparison with the deposit count of the previous vote.
  • There was an issue related to finalization of the empty deposit tree, it was solved following what was seen in the empty snapshots, also here is an issue in Teku that targeted the same problem: Fix incorrect DepositTree finalization when 0 finalized deposits Consensys/teku#7628

Resolves #1304

@rodrigo-o
Copy link
Collaborator Author

Here is a minimal spec run long past the 2048 slot bug due to eth1data. I even modified the checks for eth1data to have it early (every 32 slots) and update the deposit tree, and everything appears to work as expected, we don't have the errors anymore, and when we update eth1data, other nodes pick it up and do the same (in this scenario we were the main set of valdiators)

image

@rodrigo-o rodrigo-o marked this pull request as ready for review September 26, 2024 18:39
@rodrigo-o rodrigo-o requested a review from a team as a code owner September 26, 2024 18:39
Copy link
Collaborator

@Arkenan Arkenan left a comment

Choose a reason for hiding this comment

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

LGTM!

@rodrigo-o rodrigo-o merged commit e10c578 into main Sep 27, 2024
13 checks passed
@rodrigo-o rodrigo-o deleted the eth1data-votes-error-on-tie branch September 27, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Eth1Data fetching error on ties
2 participants