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

Slashing penalty calculation change #1217

Merged
merged 8 commits into from
Jun 28, 2019
Merged

Slashing penalty calculation change #1217

merged 8 commits into from
Jun 28, 2019

Conversation

vbuterin
Copy link
Contributor

If the exit queue is very long, then a validator may take many months to exit. With the code as currently written, however, self-slashing is a potentially lucrative route to get one's money out faster, because one can exit in 36 days.

This PR changes it so that slashing can only extend your withdrawal time, not contract it. Also, instead of the slashed balances used to calculate one's slashing penalty being those in [withdrawal - 54 days ... withdrawal - 18 days], we now run the penalization algorithm once every 36 days that a validator is slashed but not withdrawn, so that it covers the 36-day period where the validator was actually slashed. It also moves the minimum slashing penalty to the slash_validator function so that it is only applied once.

If the exit queue is very long, then a validator may take many months to exit. With the code as currently written, however, self-slashing is a potentially lucrative route to get one's money out faster, because one can exit in 36 days.

This PR changes it so that slashing can only extend your withdrawal time, not contract it. Also, instead of the slashed balances used to calculate one's slashing penalty being those in `[withdrawal - 54 days ... withdrawal - 18 days]`, we now run the penalization algorithm once every 36 days that a validator is slashed but not withdrawn, so that it covers the 36-day period where the validator was actually slashed.  It also moves the minimum slashing penalty to the `slash_validator` function so that it is only applied once.
@JustinDrake JustinDrake added the milestone:June 30 freeze 🥶 Phase 0 spec freeze for long-lived cross-client testnet label Jun 26, 2019
@djrtwo
Copy link
Contributor

djrtwo commented Jun 26, 2019

I'm fixing tests as I review

Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

Looks good!
Fixed and added some tests

@JustinDrake
Copy link
Collaborator

I've revamped and simplified the slashing logic:

  1. Avoid the ever-growing sum in slashed_balances. Instead, simply store the per-epoch sum. This is less confusing, allows for simpler logic (simply do sum(slashed_balances)), and removes the long-term risk of slashed_balances overflow.
  2. Rename slashed_balances to slashings. Shorter, consistent with BeaconState group label, just as descriptive.
  3. whistleblowing_reward => whistleblower_reward (for consistency with proposer_reward)

Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

reasonable change to the slashing vector. That used to be the most likely thing in the state to overflow

specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
specs/core/0_beacon-chain.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
milestone:June 30 freeze 🥶 Phase 0 spec freeze for long-lived cross-client testnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants