mirror of
https://github.com/sigp/lighthouse.git
synced 2026-04-25 00:38:22 +00:00
vote sanity and genesis epoch fix
This commit is contained in:
@@ -85,7 +85,7 @@ use execution_layer::{
|
||||
};
|
||||
use fixed_bytes::FixedBytesExtended;
|
||||
use fork_choice::{
|
||||
AttestationFromBlock, ExecutionStatus, ForkChoice, ForkchoiceUpdateParameters,
|
||||
ExecutionStatus, ForkChoice, ForkchoiceUpdateParameters,
|
||||
InvalidationOperation, PayloadVerificationStatus, ResetPayloadStatuses,
|
||||
};
|
||||
use futures::channel::mpsc::Sender;
|
||||
@@ -2262,7 +2262,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
|
||||
.on_attestation(
|
||||
self.slot()?,
|
||||
verified.indexed_attestation().to_ref(),
|
||||
AttestationFromBlock::False,
|
||||
false,
|
||||
&self.spec,
|
||||
)
|
||||
.map_err(Into::into)
|
||||
|
||||
@@ -71,7 +71,7 @@ use bls::{PublicKey, PublicKeyBytes};
|
||||
use educe::Educe;
|
||||
use eth2::types::{BlockGossip, EventKind};
|
||||
use execution_layer::PayloadStatus;
|
||||
pub use fork_choice::{AttestationFromBlock, PayloadVerificationStatus};
|
||||
pub use fork_choice::PayloadVerificationStatus;
|
||||
use metrics::TryExt;
|
||||
use parking_lot::RwLockReadGuard;
|
||||
use proto_array::Block as ProtoBlock;
|
||||
@@ -1664,7 +1664,7 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
|
||||
match fork_choice.on_attestation(
|
||||
current_slot,
|
||||
indexed_attestation,
|
||||
AttestationFromBlock::True,
|
||||
true,
|
||||
&chain.spec,
|
||||
) {
|
||||
Ok(()) => Ok(()),
|
||||
@@ -1685,7 +1685,7 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
|
||||
match fork_choice.on_payload_attestation(
|
||||
current_slot,
|
||||
indexed_payload_attestation,
|
||||
AttestationFromBlock::True,
|
||||
true,
|
||||
) {
|
||||
Ok(()) => Ok(()),
|
||||
// Ignore invalid payload attestations whilst importing from a block.
|
||||
|
||||
@@ -312,14 +312,6 @@ fn dequeue_payload_attestations(
|
||||
}
|
||||
|
||||
/// Denotes whether an attestation we are processing was received from a block or from gossip.
|
||||
/// Equivalent to the `is_from_block` `bool` in:
|
||||
///
|
||||
/// https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#validate_on_attestation
|
||||
#[derive(Clone, Copy)]
|
||||
pub enum AttestationFromBlock {
|
||||
True,
|
||||
False,
|
||||
}
|
||||
|
||||
/// Parameters which are cached between calls to `ForkChoice::get_head`.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
@@ -1056,7 +1048,7 @@ where
|
||||
fn validate_on_attestation(
|
||||
&self,
|
||||
indexed_attestation: IndexedAttestationRef<E>,
|
||||
is_from_block: AttestationFromBlock,
|
||||
is_from_block: bool,
|
||||
spec: &ChainSpec,
|
||||
) -> Result<(), InvalidAttestation> {
|
||||
// There is no point in processing an attestation with an empty bitfield. Reject
|
||||
@@ -1070,7 +1062,7 @@ where
|
||||
|
||||
let target = indexed_attestation.data().target;
|
||||
|
||||
if matches!(is_from_block, AttestationFromBlock::False) {
|
||||
if !is_from_block {
|
||||
self.validate_target_epoch_against_current_time(target.epoch)?;
|
||||
}
|
||||
|
||||
@@ -1148,7 +1140,7 @@ where
|
||||
fn validate_on_payload_attestation(
|
||||
&self,
|
||||
indexed_payload_attestation: &IndexedPayloadAttestation<E>,
|
||||
_is_from_block: AttestationFromBlock,
|
||||
_is_from_block: bool,
|
||||
) -> Result<(), InvalidAttestation> {
|
||||
if indexed_payload_attestation.attesting_indices.is_empty() {
|
||||
return Err(InvalidAttestation::EmptyAggregationBitfield);
|
||||
@@ -1198,7 +1190,7 @@ where
|
||||
&mut self,
|
||||
system_time_current_slot: Slot,
|
||||
attestation: IndexedAttestationRef<E>,
|
||||
is_from_block: AttestationFromBlock,
|
||||
is_from_block: bool,
|
||||
spec: &ChainSpec,
|
||||
) -> Result<(), Error<T::Error>> {
|
||||
let _timer = metrics::start_timer(&metrics::FORK_CHOICE_ON_ATTESTATION_TIMES);
|
||||
@@ -1251,7 +1243,7 @@ where
|
||||
&mut self,
|
||||
system_time_current_slot: Slot,
|
||||
attestation: &IndexedPayloadAttestation<E>,
|
||||
is_from_block: AttestationFromBlock,
|
||||
is_from_block: bool,
|
||||
) -> Result<(), Error<T::Error>> {
|
||||
self.update_time(system_time_current_slot)?;
|
||||
|
||||
@@ -1264,9 +1256,10 @@ where
|
||||
let processing_slot = self.fc_store.get_current_slot();
|
||||
// Payload attestations from blocks can be applied in the next slot (S+1 for data.slot=S),
|
||||
// while non-block payload attestations are delayed one extra slot.
|
||||
let should_process_now = match is_from_block {
|
||||
AttestationFromBlock::True => attestation.data.slot < processing_slot,
|
||||
AttestationFromBlock::False => attestation.data.slot + 1_u64 < processing_slot,
|
||||
let should_process_now = if is_from_block {
|
||||
attestation.data.slot < processing_slot
|
||||
} else {
|
||||
attestation.data.slot.saturating_add(1_u64) < processing_slot
|
||||
};
|
||||
|
||||
if should_process_now {
|
||||
|
||||
@@ -3,7 +3,7 @@ mod fork_choice_store;
|
||||
mod metrics;
|
||||
|
||||
pub use crate::fork_choice::{
|
||||
AttestationFromBlock, Error, ForkChoice, ForkChoiceView, ForkchoiceUpdateParameters,
|
||||
Error, ForkChoice, ForkChoiceView, ForkchoiceUpdateParameters,
|
||||
InvalidAttestation, InvalidBlock, PayloadVerificationStatus, PersistedForkChoice,
|
||||
PersistedForkChoiceV17, PersistedForkChoiceV28, PersistedForkChoiceV29, QueuedAttestation,
|
||||
QueuedPayloadAttestation, ResetPayloadStatuses,
|
||||
|
||||
@@ -10,7 +10,7 @@ use beacon_chain::{
|
||||
use bls::AggregateSignature;
|
||||
use fixed_bytes::FixedBytesExtended;
|
||||
use fork_choice::{
|
||||
AttestationFromBlock, ForkChoiceStore, InvalidAttestation, InvalidBlock,
|
||||
ForkChoiceStore, InvalidAttestation, InvalidBlock,
|
||||
PayloadVerificationStatus, QueuedAttestation, QueuedPayloadAttestation,
|
||||
};
|
||||
use state_processing::state_advance::complete_state_advance;
|
||||
@@ -1033,7 +1033,7 @@ async fn payload_attestation_for_previous_slot_is_accepted_at_next_slot() {
|
||||
.on_payload_attestation(
|
||||
current_slot,
|
||||
&payload_attestation,
|
||||
AttestationFromBlock::True,
|
||||
true,
|
||||
);
|
||||
|
||||
assert!(
|
||||
@@ -1082,7 +1082,7 @@ async fn non_block_payload_attestation_at_next_slot_is_delayed() {
|
||||
let result = chain
|
||||
.canonical_head
|
||||
.fork_choice_write_lock()
|
||||
.on_payload_attestation(s_plus_1, &payload_attestation, AttestationFromBlock::False);
|
||||
.on_payload_attestation(s_plus_1, &payload_attestation, false);
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"payload attestation should be accepted for queueing"
|
||||
|
||||
@@ -339,7 +339,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition {
|
||||
execution_payload_block_hash: None,
|
||||
});
|
||||
|
||||
// Ensure that 5 becomes the head.
|
||||
// Ensure that 5 is filtered out and the head stays at 4.
|
||||
//
|
||||
// 0
|
||||
// / \
|
||||
@@ -347,9 +347,9 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition {
|
||||
// |
|
||||
// 3
|
||||
// |
|
||||
// 4
|
||||
// 4 <- head
|
||||
// /
|
||||
// head-> 5
|
||||
// 5
|
||||
ops.push(Operation::FindHead {
|
||||
justified_checkpoint: Checkpoint {
|
||||
epoch: Epoch::new(1),
|
||||
@@ -360,7 +360,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition {
|
||||
root: get_root(0),
|
||||
},
|
||||
justified_state_balances: balances.clone(),
|
||||
expected_head: get_root(5),
|
||||
expected_head: get_root(4),
|
||||
});
|
||||
|
||||
// Add block 6, which has a justified epoch of 0.
|
||||
@@ -476,8 +476,8 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition {
|
||||
execution_payload_block_hash: None,
|
||||
});
|
||||
|
||||
// Ensure that 9 is the head. The branch rooted at 5 remains viable and its best descendant
|
||||
// is selected.
|
||||
// Ensure that 6 is the head, even though 5 has all the votes. This is testing to ensure
|
||||
// that 5 is filtered out due to a differing justified epoch.
|
||||
//
|
||||
// 0
|
||||
// / \
|
||||
@@ -487,13 +487,13 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition {
|
||||
// |
|
||||
// 4
|
||||
// / \
|
||||
// 5 6
|
||||
// 5 6 <- head
|
||||
// |
|
||||
// 7
|
||||
// |
|
||||
// 8
|
||||
// /
|
||||
// head-> 9
|
||||
// 9
|
||||
ops.push(Operation::FindHead {
|
||||
justified_checkpoint: Checkpoint {
|
||||
epoch: Epoch::new(1),
|
||||
@@ -504,7 +504,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition {
|
||||
root: get_root(0),
|
||||
},
|
||||
justified_state_balances: balances.clone(),
|
||||
expected_head: get_root(9),
|
||||
expected_head: get_root(6),
|
||||
});
|
||||
|
||||
// Change fork-choice justified epoch to 1, and the start block to 5 and ensure that 9 is
|
||||
|
||||
@@ -130,7 +130,6 @@ pub struct ProtoNode {
|
||||
#[superstruct(only(V29), partial_getter(copy))]
|
||||
pub execution_payload_block_hash: ExecutionBlockHash,
|
||||
/// Tiebreaker for payload preference when full_payload_weight == empty_payload_weight.
|
||||
/// Per spec: prefer Full if block was timely and data is available; otherwise prefer Empty.
|
||||
#[superstruct(only(V29), partial_getter(copy))]
|
||||
pub payload_tiebreak: PayloadTiebreak,
|
||||
}
|
||||
@@ -1152,7 +1151,7 @@ impl ProtoArray {
|
||||
return false;
|
||||
}
|
||||
|
||||
let genesis_epoch = Epoch::new(1);
|
||||
let genesis_epoch = Epoch::new(0);
|
||||
let current_epoch = current_slot.epoch(E::slots_per_epoch());
|
||||
let node_epoch = node.slot().epoch(E::slots_per_epoch());
|
||||
let node_justified_checkpoint = node.justified_checkpoint();
|
||||
|
||||
@@ -530,6 +530,8 @@ impl ProtoArrayForkChoice {
|
||||
if attestation_slot > vote.next_slot || *vote == VoteTracker::default() {
|
||||
vote.next_root = block_root;
|
||||
vote.next_slot = attestation_slot;
|
||||
vote.next_payload_present = false;
|
||||
vote.next_blob_data_available = false;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
|
||||
Reference in New Issue
Block a user