Reward clarifications/fix #597
Conversation
| @@ -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))]`. | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Right, got it. Though this seems like something that should be documented somewhere explicitly or else we might easily accidentally remove it...
| * 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)`. |
There was a problem hiding this comment.
Don't understand why we're removong the matching justified epoch requirement here...
There was a problem hiding this comment.
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
|
@djrtwo any reason for the phase 1 label on this one? It feels phase 0 specific. |
Mistake! adding an explicit note about the attestation inclusion matching justified hash now |
|
@djrtwo: I still find most of the "Per-epoch processing" section daunting and hard to read. I intend to sit down to review the section carefully to weed out further bugs, and possibly refactor things for readability. I've "approved" the PR to keep the ball rolling but haven't really wrapped my head around all the subtle behaviour. |
Note: this does not affect justification/finalization as those were already bound to the
epoch_boundary_attestationswhich split them by epoch