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

Default to "zero" Eth1Data instead of current state #2501

Closed
wants to merge 3 commits into from

Conversation

ajsutton
Copy link
Contributor

@ajsutton ajsutton commented Jul 1, 2021

Addresses #2227 by changing the default vote to be zeroed out - zero block hash, deposit root and zero deposits. The state transition is updated to explicitly disallow the zero vote from becoming the current Eth1Data in the spec.

The zero vote is automatically ignored by get_eth1_vote because it won't be in the set of known blocks so doesn't lead validators to following a vote which doesn't progress the chain like defaulting to the current state.eth1_data does.

The alternative approach would be for validators to vote for a random block hash which has the advantage of not needing to update the state transition rules as it's exceedingly unlikely 50% of validators will choose the same random data, but the downside of making it harder to identify default votes. Differentiating default votes from votes for unknown blocks is particularly useful when evaluating whether a beacon node is tracking the eth1 chain correctly.

Note: Python likely needs some tweaking but hopefully gets the idea across.

@@ -1732,8 +1732,9 @@ def process_randao(state: BeaconState, body: BeaconBlockBody) -> None:
```python
def process_eth1_data(state: BeaconState, body: BeaconBlockBody) -> None:
state.eth1_data_votes.append(body.eth1_data)
if state.eth1_data_votes.count(body.eth1_data) * 2 > EPOCHS_PER_ETH1_VOTING_PERIOD * SLOTS_PER_EPOCH:
state.eth1_data = body.eth1_data
if body.eth1_data.block_hash != Bytes32():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regardless of the change to default volte, should we have a check in here that the deposit count can't go backwards? It can only happen if enough validators don't follow the honest validator spec but would definitely be a very odd case to allow.

@ajsutton
Copy link
Contributor Author

ajsutton commented Jul 1, 2021

Also worth noting that this would have to be rolled out as a soft fork - first deploy the state transition change, then change the default vote. Seems safe but is perhaps a reason to just use a random vote instead and deal with not being able to distinguish default votes from unknown blocks.

@ajsutton
Copy link
Contributor Author

ajsutton commented Jan 4, 2024

At this point I think we can just give up on ever actually making this change. Eth1Data is working fine and the next time we change it should be to get rid of it entirely.

@ajsutton ajsutton closed this Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants