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.

Sign up for GitHub

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

Reward clarifications/fix #597

Merged
merged 3 commits into from
Feb 13, 2019
Merged

Reward clarifications/fix #597

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e8f9dce
simply reward logic for justified slot from previous epoch
djrtwo Feb 11, 2019
1d77c57
clarify active validator meaning for rewards/penalties
djrtwo Feb 11, 2019
8213ad6
add note about attestations on chain voting for justified
djrtwo Feb 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions specs/core/0_beacon-chain.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1781,22 +1781,19 @@ The steps below happen when `(state.slot + 1) % EPOCH_LENGTH == 0`.
* Let `current_total_balance = get_total_balance(state, get_active_validator_indices(state.validator_registry, current_epoch))`.
* Let `current_epoch_attestations = [a for a in state.latest_attestations if current_epoch == slot_to_epoch(a.data.slot)]`. (Note: this is the set of attestations of slots in the epoch `current_epoch`, _not_ attestations that got included in the chain during the epoch `current_epoch`.)
* Validators justifying the epoch boundary block at the start of the current epoch:
* Let `current_epoch_boundary_attestations = [a for a in current_epoch_attestations if a.data.epoch_boundary_root == get_block_root(state, get_epoch_start_slot(current_epoch)) and a.data.justified_epoch == state.justified_epoch]`.
* Let `current_epoch_boundary_attestations = [a for a in current_epoch_attestations if a.data.epoch_boundary_root == get_block_root(state, get_epoch_start_slot(current_epoch))]`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that the required justified epoch is contained in the block root, isn't it the case that all honest attestations with the same epoch boundary root will have the same justified epoch?

Also, however, attestations whose justified epoch is "wrong" even if their epoch boundary root is correct should not count toward the 2/3 for making a new justified block. The reason is that the safety proofs depend on the existence of a chain of "supermajority links" with 2/3 of validators sharing the same source and target.

Copy link
Contributor Author

@djrtwo djrtwo Feb 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Given that the required justified epoch is contained in the block root, isn't it the case that all honest attestations with the same epoch boundary root will have the same justified epoch?

Yes.

Also, however, attestations whose justified epoch is "wrong" even if their epoch boundary root is correct should not count toward the 2/3 for making a new justified block.

These cannot even be included on chain due to the block processing rules requiring included attestations to have the expected justified epoch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, got it. Though this seems like something that should be documented somewhere explicitly or else we might easily accidentally remove it...

* Let `current_epoch_boundary_attester_indices` be the union of the [validator](#dfn-validator) index sets given by `[get_attestation_participants(state, a.data, a.aggregation_bitfield) for a in current_epoch_boundary_attestations]`.
* Let `current_epoch_boundary_attesting_balance = get_total_balance(state, current_epoch_boundary_attester_indices)`.

[Validators](#dfn-Validator) attesting during the previous epoch:

* Let `previous_total_balance = get_total_balance(state, get_active_validator_indices(state.validator_registry, previous_epoch))`.
* Validators that made an attestation during the previous epoch:
* Validators that made an attestation during the previous epoch, targeting the previous justified slot:
* Let `previous_epoch_attestations = [a for a in state.latest_attestations if previous_epoch == slot_to_epoch(a.data.slot)]`.
* Let `previous_epoch_attester_indices` be the union of the validator index sets given by `[get_attestation_participants(state, a.data, a.aggregation_bitfield) for a in previous_epoch_attestations]`.
* Validators targeting the previous justified slot:
* Let `previous_epoch_justified_attestations = [a for a in current_epoch_attestations + previous_epoch_attestations if a.data.justified_epoch == state.previous_justified_epoch]`.
* Let `previous_epoch_justified_attester_indices` be the union of the validator index sets given by `[get_attestation_participants(state, a.data, a.aggregation_bitfield) for a in previous_epoch_justified_attestations]`.
* Let `previous_epoch_justified_attesting_balance = get_total_balance(state, previous_epoch_justified_attester_indices)`.
* Let `previous_epoch_attesting_balance = get_total_balance(state, previous_epoch_attester_indices)`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't understand why we're removong the matching justified epoch requirement here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any attestation that is in latest_attestations is guaranteed to have the expected justified block/epoch because of the inclusion rules in the block.attestations section.
Keeping it in is strictly redundant logic

* Validators justifying the epoch boundary block at the start of the previous epoch:
* Let `previous_epoch_boundary_attestations = [a for a in previous_epoch_justified_attestations if a.data.epoch_boundary_root == get_block_root(state, get_epoch_start_slot(previous_epoch))]`.
* Let `previous_epoch_boundary_attestations = [a for a in previous_epoch_attestations if a.data.epoch_boundary_root == get_block_root(state, get_epoch_start_slot(previous_epoch))]`.
* Let `previous_epoch_boundary_attester_indices` be the union of the validator index sets given by `[get_attestation_participants(state, a.data, a.aggregation_bitfield) for a in previous_epoch_boundary_attestations]`.
* Let `previous_epoch_boundary_attesting_balance = get_total_balance(state, previous_epoch_boundary_attester_indices)`.
* Validators attesting to the expected beacon chain head during the previous epoch:
Expand Down Expand Up @@ -1861,17 +1858,19 @@ First, we define some additional helpers:
* Let `base_reward(state, index) = get_effective_balance(state, index) // base_reward_quotient // 5` for any validator with the given `index`.
* Let `inactivity_penalty(state, index, epochs_since_finality) = base_reward(state, index) + get_effective_balance(state, index) * epochs_since_finality // INACTIVITY_PENALTY_QUOTIENT // 2` for any validator with the given `index`.

Note: When applying penalties in the following balance recalculations implementers should make sure the `uint64` does not underflow.

##### Justification and finalization

Note: When applying penalties in the following balance recalculations implementers should make sure the `uint64` does not underflow.
Note: Rewards and penalties are for participation in the previous epoch, so the "active validator" set is drawn from `get_active_validator_indices(state.validator_registry, previous_epoch)`.

* Let `epochs_since_finality = next_epoch - state.finalized_epoch`.

Case 1: `epochs_since_finality <= 4`:

* Expected FFG source:
* Any [validator](#dfn-validator) `index` in `previous_epoch_justified_attester_indices` gains `base_reward(state, index) * previous_epoch_justified_attesting_balance // previous_total_balance`.
* Any [active validator](#dfn-active-validator) `index` not in `previous_epoch_justified_attester_indices` loses `base_reward(state, index)`.
* Any [validator](#dfn-validator) `index` in `previous_epoch_attester_indices` gains `base_reward(state, index) * previous_epoch_attesting_balance // previous_total_balance`.
* Any [active validator](#dfn-active-validator) `index` not in `previous_epoch_attester_indices` loses `base_reward(state, index)`.
* Expected FFG target:
* Any [validator](#dfn-validator) `index` in `previous_epoch_boundary_attester_indices` gains `base_reward(state, index) * previous_epoch_boundary_attesting_balance // previous_total_balance`.
* Any [active validator](#dfn-active-validator) `index` not in `previous_epoch_boundary_attester_indices` loses `base_reward(state, index)`.
Expand All @@ -1883,10 +1882,10 @@ Case 1: `epochs_since_finality <= 4`:

Case 2: `epochs_since_finality > 4`:

* Any [active validator](#dfn-active-validator) `index` not in `previous_epoch_justified_attester_indices`, loses `inactivity_penalty(state, index, epochs_since_finality)`.
* Any [active validator](#dfn-active-validator) `index` not in `previous_epoch_attester_indices`, loses `inactivity_penalty(state, index, epochs_since_finality)`.
* Any [active validator](#dfn-active-validator) `index` not in `previous_epoch_boundary_attester_indices`, loses `inactivity_penalty(state, index, epochs_since_finality)`.
* Any [active validator](#dfn-active-validator) `index` not in `previous_epoch_head_attester_indices`, loses `base_reward(state, index)`.
* Any [active_validator](#dfn-active-validator) `index` with `validator.penalized_epoch <= current_epoch`, loses `2 * inactivity_penalty(state, index, epochs_since_finality) + base_reward(state, index)`.
* Any [active validator](#dfn-active-validator) `index` with `validator.penalized_epoch <= current_epoch`, loses `2 * inactivity_penalty(state, index, epochs_since_finality) + base_reward(state, index)`.
* Any [validator](#dfn-validator) `index` in `previous_epoch_attester_indices` loses `base_reward(state, index) - base_reward(state, index) * MIN_ATTESTATION_INCLUSION_DELAY // inclusion_distance(state, index)`

##### Attestation inclusion
Expand Down