cerc-io/lighthouse
16
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

op-pool: fix bug in attestation_score

Browse Source 
The attestation scoring function was looking only at the previous epoch, but
should really look at whichever epoch is appropriate for a given attestation.

We also avoid including attestations that don't pay us any reward, as they
simply bloat the chain.
This commit is contained in:
Michael Sproul 2019-04-01 13:34:43 +11:00
parent 64507950dd
commit ddd9654f70
No known key found for this signature in database
GPG Key ID: 77B1309D2E54E914
1 changed files with 22 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
eth2/operation_pool/src/lib.rs
View File

@ -74,15 +74,26 @@ impl AttestationId {
/// the aggregate attestation introduces, and is proportional to the size of the reward we will
/// receive for including it in a block.
// TODO: this could be optimised with a map from validator index to whether that validator has
// attested in the *current* epoch. Alternatively, we could cache an index that allows us to
// quickly look up the attestations in the current epoch for a given shard.
fn attestation_score(attestation: &Attestation, state: &BeaconState) -> usize {
// attested in each of the current and previous epochs. Currently quadractic in number of validators.
fn attestation_score(attestation: &Attestation, state: &BeaconState, spec: &ChainSpec) -> usize {
// Bitfield of validators whose attestations are new/fresh.
let mut new_validators = attestation.aggregation_bitfield.clone();
state
.current_epoch_attestations
let attestation_epoch = attestation.data.slot.epoch(spec.slots_per_epoch);
let state_attestations = if attestation_epoch == state.current_epoch(spec) {
&state.current_epoch_attestations
} else if attestation_epoch == state.previous_epoch(spec) {
&state.previous_epoch_attestations
} else {
return 0;
};
state_attestations
.iter()
// In a single epoch, an attester should only be attesting for one shard.
// TODO: we avoid including slashable attestations in the state here,
// but maybe we should do something else with them (like construct slashings).
.filter(|current_attestation| current_attestation.data.shard == attestation.data.shard)
.for_each(|current_attestation| {
// Remove the validators who have signed the existing attestation (they are not new)
@ -176,7 +187,9 @@ impl OperationPool {
.filter(|attestation| validate_attestation(state, attestation, spec).is_ok())
// Scored by the number of new attestations they introduce (descending)
// TODO: need to consider attestations introduced in THIS block
.map(|att| (att, attestation_score(att, state)))
.map(|att| (att, attestation_score(att, state, spec)))
// Don't include any useless attestations (score 0)
.filter(|&(_, score)| score != 0)
.sorted_by_key(|&(_, score)| std::cmp::Reverse(score))
// Limited to the maximum number of attestations per block
.take(spec.max_attestations as usize)
@ -695,7 +708,7 @@ mod tests {
num_committees * (spec.slots_per_epoch * spec.target_committee_size) as usize;
let mut state_builder =
TestingBeaconStateBuilder::from_default_keypairs_file_if_exists(num_validators, spec);
let slot_offset = 100000;
let slot_offset = 1000 * spec.slots_per_epoch + spec.slots_per_epoch / 2;
let slot = spec.genesis_slot + slot_offset;
state_builder.teleport_to_slot(slot, spec);
state_builder.build_caches(spec).unwrap();
@ -726,7 +739,7 @@ mod tests {
assert_eq!(
att1.aggregation_bitfield.num_set_bits(),
attestation_score(&att1, state)
attestation_score(&att1, state, spec)
);
state
@ -735,7 +748,7 @@ mod tests {
assert_eq!(
committee.committee.len() - 2,
attestation_score(&att2, &state)
attestation_score(&att2, state, spec)
);
}
}