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

Fair proposer selection probability #772

Merged
merged 5 commits into from Mar 26, 2019
Merged

Fair proposer selection probability #772

merged 5 commits into from Mar 26, 2019

Conversation

vbuterin
Copy link
Contributor

@vbuterin vbuterin commented Mar 14, 2019

Makes a validator's probability of being selected as a proposer proportional to a validator's balance. This ensures fairness between validators of different deposit sizes, making proposer manipulation attacks more expensive and ensuring that there is no advantage to be gained by concentrating one's ETH in more vs fewer validator slots. This allows us to widen the gap between the min and max deposit size.

Note that as a side effect, proposer selection becomes less predictable, but I don't feel like this is a large downside. Specifically, the proposer selection does not have a guaranteed 1 epoch lookahead like the committees do. This change is noted in the validator guide within this PR.

vbuterin and others added 2 commits Mar 14, 2019
Note that as a side effect, proposer selection becomes less predictable, but I don't feel like this is a large downside.
@JustinDrake
Copy link
Collaborator

JustinDrake commented Mar 14, 2019

I refactored the logic to remove the line return first_committee[epoch % len(first_committee)] because it would never get triggered in practice (not friendly to testing), and it's also simpler :)

Assuming `epoch % i` is a bug, and you meant `epoch + i`. @vbuterin
@vbuterin
Copy link
Contributor Author

vbuterin commented Mar 15, 2019

OK that works. I had thought you might favor a constant bound on runtime, but I suppose the probability of it actually going longer than ~100 rounds is cryptographically securely low in any case.

@JustinDrake JustinDrake self-requested a review Mar 15, 2019
@djrtwo
Copy link
Contributor

djrtwo commented Mar 21, 2019

Can you write a note to implementers on why this is needed/desirable?

@djrtwo
Copy link
Contributor

djrtwo commented Mar 21, 2019

Note that as a side effect, proposer selection becomes less predictable, but I don't feel like this is a large downside.

Ah, this means there is no longer a guaranteed 1 epoch lookahead. You can know your committee but don't know if you are the proposer until start of epoch because of potential changes in balance. We need to make note of this in V-guide -- you cannot tell if proposer until epoch-of even if you used lookahead on your committee/slot

@vbuterin
Copy link
Contributor Author

vbuterin commented Mar 22, 2019

Can you write a note to implementers on why this is needed/desirable?

Done.

Ah, this means there is no longer a guaranteed 1 epoch lookahead. You can know your committee but don't know if you are the proposer until start of epoch because of potential changes in balance. We need to make note of this in V-guide -- you cannot tell if proposer until epoch-of even if you used lookahead on your committee/slot

Correct. There's a guaranteed 1-epoch lookahead for which committee you will be in, but not proposer status. Although we expect that ~99.99%+ of the time, the proposer will nevertheless be the same in two different branches, because balances don't change that much.

@djrtwo
Copy link
Contributor

djrtwo commented Mar 26, 2019

Fixed a couple of minor bugs and modified the validator guide to not have lookahead on proposing. Just on committee

@djrtwo djrtwo merged commit db63b3e into dev Mar 26, 2019
1 check passed
@djrtwo djrtwo deleted the vbuterin-patch-13 branch Mar 26, 2019
sorpaas added a commit to paritytech/shasper that referenced this pull request May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants