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

Disallow single Eth1 block in two voting periods #1757

Closed
wants to merge 1 commit into from
Closed

Disallow single Eth1 block in two voting periods #1757

wants to merge 1 commit into from

Conversation

mcdee
Copy link
Contributor

@mcdee mcdee commented Apr 26, 2020

The definition of a candidate block in is_candidate_block allows for an Eth1 block whose timestamp is exactly aligned with the start of an Eth1 voting period to be considered in two voting periods.

This patch ensures that such a block can only be included as a candidate for a single voting period.

@hwwhww hwwhww added the scope:v-guide Validator guide label Apr 27, 2020
@hwwhww
Copy link
Contributor

hwwhww commented Apr 27, 2020

I think it doesn't help for "a block can only be included as a candidate for a single voting period." since ETH1 block time is not matching ETH2 period, so ETH1_FOLLOW_DISTANCE * SECONDS_PER_ETH1_BLOCK != EPOCHS_PER_ETH1_VOTING_PERIOD * SLOTS_PER_EPOCH * SECONDS_PER_SLOT.

Illustration:
voting_period 001

overlapping time = ~0.6 hour

But it's interesting to think more about how to mitigate the overlapping!

@djrtwo
Copy link
Contributor

djrtwo commented May 14, 2020

yeah, you're right @hwwhww. What we probably want to ensure is that we don't consider any blocks that are older than state.eth1data. Because Eth1Data doesn't have a timestamp, we can use deposit_count as a proxy.

maybe the following:

votes_to_consider = [
    get_eth1_data(block) for block in eth1_chain if (
        is_candidate_block(block, period_start)
        and get_eth1_data(block).deposit_count > state.eth1data.deposit_count
]

@djrtwo
Copy link
Contributor

djrtwo commented May 20, 2020

close in favor of #1836

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:v-guide Validator guide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants