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

adjust hysteresis to avoid initial over-deposit incentive #1627

Merged
merged 3 commits into from Mar 3, 2020
Merged

Conversation

djrtwo
Copy link
Contributor

@djrtwo djrtwo commented Feb 18, 2020

Address #1609

Make hysteresis calculations configurable.
Change default increment/decrement thresholds to x-0.25 and x+1.25

@djrtwo djrtwo requested a review from protolambda Feb 18, 2020
@protolambda
Copy link
Collaborator

protolambda commented Feb 18, 2020

From chat earlier, we need to consider that if we do not offset the actual effective balance, we:

  • increase allowance for mistakes by 0.25
  • increase punishment by requiring another 0.25 extra
Previously:
Downwards hysteresis width: 32-32 = 0
Upwards hysteresis width: 32.5-31 = 1.5
Now:
Downwards hysteresis width: 32-31.75 = 0.25
Upwards hysteresis width: 32.75 - 31 = 1.75

I think the system is closer to original intention if we change it to:

  • A small margin of e.g. 0.05 for initially offline validators to not immediately lose 1 effective balance increment because of some small inactivity.
  • The old 1.5 upwards width.

@mcdee
Copy link
Contributor

mcdee commented Feb 19, 2020

What is the rationale behind x+0.75 rather than x+0.25? A +/-0.25 would retain the current gap of requiring 0.5 Ether in rewards (or additional deposit) prior to an uptick in effective balance, and solves the issue of newly on-boarded deposits not suffering an immediate decrease in effective balance if they miss their first attestation.

@vbuterin
Copy link
Contributor

vbuterin commented Feb 28, 2020

Yeah, it should be -0.25 and +1.25, not +1.75.

Downwards hysteresis width: 32-31.75 = 0.25
Upwards hysteresis width: 32.75 - 31 = 1.75

The error here is that as implemeneted the "downwards hysteresis width" (wrong term arguably) is actually negative 0.25, and not 0.25. So preserving the 1.5 width would require an upwards threshold of 1.25.

@protolambda
Copy link
Collaborator

protolambda commented Feb 28, 2020

Yeah the term could have been better. I care more about the hysteresis numbers than the names though, the leap needed to get back to 32 is affected significantly.

@djrtwo
Copy link
Contributor Author

djrtwo commented Mar 2, 2020

modded to be 0.25/1.25

Ready for review and merge @protolambda

@protolambda
Copy link
Collaborator

protolambda commented Mar 2, 2020

With experimentation and review these numbers may change again, I am thinking we could just make it configurable, and avoid further spec changes.

@djrtwo
Copy link
Contributor Author

djrtwo commented Mar 3, 2020

okay, now configurable @protolambda

Copy link
Collaborator

@protolambda protolambda left a comment

LGTM

# 1 (minus 0.25)
HYSTERESIS_DOWNWARD_MULTIPLIER: 1
# 5 (plus 1.25)
HYSTERESIS_UPWARD_MULTIPLIER: 5
Copy link
Collaborator

@protolambda protolambda Mar 3, 2020

Choose a reason for hiding this comment

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

So previous behavior is, for context:

HYSTERESIS_QUOTIENT: 2
HYSTERESIS_DOWNWARD_MULTIPLIER: 0
HYSTERESIS_UPWARD_MULTIPLIER: 3

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

4 participants