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

Rounded effective balance #862

Closed
wants to merge 3 commits into from
Closed

Rounded effective balance #862

wants to merge 3 commits into from

Conversation

JustinDrake
Copy link
Collaborator

@JustinDrake JustinDrake commented Mar 31, 2019

Use the rounded balance for the effective balance to homogenise the on-onchain finality and crosslink gadgets with the fork choice rule and the light client logic.

Minor cleanups:

  • Use increase_balance and decrease_balance in apply_rewards instead of set_balance.
  • Use get_effective_balance for activations and ejections.
  • Rename high_balance to rounded_balance (we no longer have high_balance and low_balance).

Use the rounded balance for the effective balance to homogenise the on-onchain finality and crosslink gadgets with the fork choice rule and the light client logic.

Minor cleanups:

* Use `increase_balance` and `decrease_balance` in `apply_rewards` instead of `set_balance`.
* Use `get_effective_balance` for activations and ejections.
* Rename `high_balance` to `rounded_balance` (we no longer have `high_balance` and `low_balance`).
@@ -1091,7 +1088,7 @@ def get_effective_balance(state: BeaconState, index: ValidatorIndex) -> Gwei:
"""
Return the effective balance (also known as "balance at stake") for a validator with the given ``index``.
"""
return min(get_balance(state, index), MAX_DEPOSIT_AMOUNT)
return min(state.validator_registry[index].rounded_balance, MAX_DEPOSIT_AMOUNT)
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is used for reward/penalty calculations. Do we want to use rounded balance for reward/penalties?

Copy link
Collaborator Author

@JustinDrake JustinDrake Apr 3, 2019

Choose a reason for hiding this comment

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

I'd argue so. The balance at stake is rounded so rewards/penalties should also be. Basically I'd argue use rounded everywhere for consensus stuff.

Copy link
Contributor

Choose a reason for hiding this comment

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

This does effect a validator's effective rate of return as rounded_balance <= actual_balance. In the event that a validator has >= 32ETH and has always had so, then this doesn't matter. In the event that a validator's balance drops to 16ETH, however, by this makes the difference between having 64 ETH and 65 ETH after validating for a long time.

If we allow balances to grow exponentially as per #730, then this always matters. See graphs attached below:

econ_test_init_bal16
econ_test_init_bal32

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right. I guess a downside of using rounded balance for rewards is that it creates a misalignment between capital costs (not rounded) and rewards (rounded).

@JustinDrake
Copy link
Collaborator Author

Close in favour of #949

@djrtwo djrtwo deleted the JustinDrake-patch-7 branch September 7, 2019 20:33
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