-
Notifications
You must be signed in to change notification settings - Fork 998
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
Revamp balances and incentivisation #949
Conversation
Cosmetic changes related to `get_justification_and_finalization_deltas`: * Review naming of misc helper functions and variables * Abstract away common logic and rework for readability * Add `MAX_FINALITY_LOOKBACK` and `BASE_REWARDS_PER_EPOCH` constants * Rescale `INACTIVITY_PENALTY_QUOTIENT` Substantive changes: * Make logic relative to `previous_epoch` throughout (as opposed to mixing `current_epoch` and `previous_epoch`) * Replace inclusion delay bonus by an inclusion delay penalty
@JustinDrake could you give a summary of how exactly the rewards/penalty change through this? I would like to read it with this info side by side :) |
See the "substantive changes" section (which I fleshed out a bit) in the PR description. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a pass.
The main concern is the use of get_beacon_proposer_index
outside of the current epoch during get_attestation_deltas
. get_beacon_proposer_index
is not guaranteed to return the correct value outside of the current epoch.
Co-Authored-By: JustinDrake <drakefjustin@gmail.com>
| `INACTIVITY_PENALTY_QUOTIENT` | `2**24` (= 16,777,216) | | ||
| `MIN_PENALTY_QUOTIENT` | `2**5` (= 32) | | ||
| `INACTIVITY_PENALTY_QUOTIENT` | `2**25` (= 33,554,432) | | ||
| `MIN_SLASHING_PENALTY_QUOTIENT` | `2**5` (= 32) | | ||
|
||
* **The `BASE_REWARD_QUOTIENT` is NOT final. Once all other protocol details are finalized it will be adjusted, to target a theoretical maximum total issuance of `2**21` ETH per year if `2**27` ETH is validating (and therefore `2**20` per year if `2**25` ETH is validating, etc etc)** | ||
* The `INACTIVITY_PENALTY_QUOTIENT` equals `INVERSE_SQRT_E_DROP_TIME**2` where `INVERSE_SQRT_E_DROP_TIME := 2**12 epochs` (~18 days) is the time it takes the inactivity penalty to reduce the balance of non-participating [validators](#dfn-validator) to about `1/sqrt(e) ~= 60.6%`. Indeed, the balance retained by offline [validators](#dfn-validator) after `n` epochs is about `(1 - 1/INACTIVITY_PENALTY_QUOTIENT)**(n**2/2)` so after `INVERSE_SQRT_E_DROP_TIME` epochs it is roughly `(1 - 1/INACTIVITY_PENALTY_QUOTIENT)**(INACTIVITY_PENALTY_QUOTIENT/2) ~= 1/sqrt(e)`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If INACTIVITY_PENALTY_QUOTIENT
changes from 2**25
to 2**24
, then INACTIVITY_PENALTY_QUOTIENT != INVERSE_SQRT_E_DROP_TIME**2
now. This sentence needs to be updated or we change INACTIVITY_PENALTY_QUOTIENT
back to 2**24
with INACTIVITY_PENALTY_QUOTIENT // 2
in formula.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this sentence should go away. We should have a separate document explaining design decisions/rationales. (This will be included in the Transparent Paper, whenever I get round to writing it.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's also mentioned in @vbuterin's https://notes.ethereum.org/s/rkhCgQteN#Slashing-and-anti-correlation-penalties. We could (i) update rationales doc (ii) remove this sentence from the spec and link to rationale doc for now.
Co-Authored-By: JustinDrake <drakefjustin@gmail.com>
Substantive changes:
validator.effective_balance
) throughout beacon chain consensusvalidator.effective_balance
once per epochPendingAttestation.proposer_index
for attestation proposer rewardsCosmetic changes (mostly related to
get_attestation_deltas
):MIN_EPOCHS_TO_INACTIVITY_PENALTY
andBASE_REWARDS_PER_EPOCH
constantsINACTIVITY_PENALTY_QUOTIENT