Add back AttestationFromBlock

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -85,8 +85,8 @@ use execution_layer::{
 };
 use fixed_bytes::FixedBytesExtended;
 use fork_choice::{
-    ExecutionStatus, ForkChoice, ForkchoiceUpdateParameters, InvalidationOperation,
+    AttestationFromBlock, ExecutionStatus, ForkChoice, ForkchoiceUpdateParameters,
-    PayloadVerificationStatus, ResetPayloadStatuses,
+    InvalidationOperation, PayloadVerificationStatus, ResetPayloadStatuses,
 };
 use futures::channel::mpsc::Sender;
 use itertools::Itertools;
@@ -2297,7 +2297,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             .on_attestation(
                 self.slot()?,
                 verified.indexed_attestation().to_ref(),
-                false,
+                AttestationFromBlock::False,
                 &self.spec,
             )
             .map_err(Into::into)
@@ -4757,6 +4757,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             })
     }
 
+    // TODO(gloas): wrong for Gloas, needs an update
     pub fn overridden_forkchoice_update_params_or_failure_reason(
         &self,
         canonical_forkchoice_params: &ForkchoiceUpdateParameters,

beacon_node/beacon_chain/src/block_verification.rs

@@ -71,7 +71,7 @@ use bls::{PublicKey, PublicKeyBytes};
 use educe::Educe;
 use eth2::types::{BlockGossip, EventKind};
 use execution_layer::PayloadStatus;
-pub use fork_choice::PayloadVerificationStatus;
+pub use fork_choice::{AttestationFromBlock, PayloadVerificationStatus};
 use metrics::TryExt;
 use parking_lot::RwLockReadGuard;
 use proto_array::Block as ProtoBlock;
@@ -1666,7 +1666,12 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
                 .get_indexed_attestation(&state, attestation)
                 .map_err(|e| BlockError::PerBlockProcessingError(e.into_with_index(i)))?;
 
-            match fork_choice.on_attestation(current_slot, indexed_attestation, true, &chain.spec) {
+            match fork_choice.on_attestation(
+                current_slot,
+                indexed_attestation,
+                AttestationFromBlock::True,
+                &chain.spec,
+            ) {
                 Ok(()) => Ok(()),
                 // Ignore invalid attestations whilst importing attestations from a block. The
                 // block might be very old and therefore the attestations useless to fork choice.
@@ -1689,7 +1694,7 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
                 match fork_choice.on_payload_attestation(
                     current_slot,
                     indexed_payload_attestation,
-                    true,
+                    AttestationFromBlock::True,
                     &ptc.0,
                 ) {
                     Ok(()) => Ok(()),

consensus/fork_choice/src/fork_choice.rs

@@ -328,6 +328,15 @@ 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)]
 pub struct ForkchoiceUpdateParameters {
@@ -1082,7 +1091,7 @@ where
     fn validate_on_attestation(
         &self,
         indexed_attestation: IndexedAttestationRef<E>,
-        is_from_block: bool,
+        is_from_block: AttestationFromBlock,
         spec: &ChainSpec,
     ) -> Result<(), InvalidAttestation> {
         // There is no point in processing an attestation with an empty bitfield. Reject
@@ -1096,7 +1105,7 @@ where
 
         let target = indexed_attestation.data().target;
 
-        if !is_from_block {
+        if matches!(is_from_block, AttestationFromBlock::False) {
             self.validate_target_epoch_against_current_time(target.epoch)?;
         }
 
@@ -1193,7 +1202,7 @@ where
     fn validate_on_payload_attestation(
         &self,
         indexed_payload_attestation: &IndexedPayloadAttestation<E>,
-        is_from_block: bool,
+        is_from_block: AttestationFromBlock,
     ) -> Result<(), InvalidAttestation> {
         if indexed_payload_attestation.attesting_indices.is_empty() {
             return Err(InvalidAttestation::EmptyAggregationBitfield);
@@ -1215,7 +1224,7 @@ where
 
         // Gossip payload attestations must be for the current slot.
         // https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/fork-choice.md
-        if !is_from_block
+        if matches!(is_from_block, AttestationFromBlock::False)
             && indexed_payload_attestation.data.slot != self.fc_store.get_current_slot()
         {
             return Err(InvalidAttestation::PayloadAttestationNotCurrentSlot {
@@ -1227,7 +1236,7 @@ where
         // A payload attestation voting payload_present for a block in the current slot is
         // invalid: the payload cannot be known yet. This only applies to gossip attestations;
         // payload attestations from blocks have already been validated by the block producer.
-        if !is_from_block
+        if matches!(is_from_block, AttestationFromBlock::False)
             && self.fc_store.get_current_slot() == block.slot
             && indexed_payload_attestation.data.payload_present
         {
@@ -1258,7 +1267,7 @@ where
         &mut self,
         system_time_current_slot: Slot,
         attestation: IndexedAttestationRef<E>,
-        is_from_block: bool,
+        is_from_block: AttestationFromBlock,
         spec: &ChainSpec,
     ) -> Result<(), Error<T::Error>> {
         let _timer = metrics::start_timer(&metrics::FORK_CHOICE_ON_ATTESTATION_TIMES);
@@ -1319,7 +1328,7 @@ where
         &mut self,
         system_time_current_slot: Slot,
         attestation: &IndexedPayloadAttestation<E>,
-        is_from_block: bool,
+        is_from_block: AttestationFromBlock,
         ptc: &[usize],
     ) -> Result<(), Error<T::Error>> {
         self.update_time(system_time_current_slot)?;
@@ -1339,10 +1348,11 @@ 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 gossiped payload attestations are delayed one extra slot.
-        let should_process_now = if is_from_block {
+        let should_process_now = match is_from_block {
-            attestation.data.slot < processing_slot
+            AttestationFromBlock::True => attestation.data.slot < processing_slot,
-        } else {
+            AttestationFromBlock::False => {
                 attestation.data.slot.saturating_add(1_u64) < processing_slot
+            }
         };
 
         if should_process_now {

consensus/fork_choice/src/lib.rs

@@ -3,10 +3,10 @@ mod fork_choice_store;
 mod metrics;
 
 pub use crate::fork_choice::{
-    Error, ForkChoice, ForkChoiceView, ForkchoiceUpdateParameters, InvalidAttestation,
+    AttestationFromBlock, Error, ForkChoice, ForkChoiceView, ForkchoiceUpdateParameters,
-    InvalidBlock, PayloadVerificationStatus, PersistedForkChoice, PersistedForkChoiceV17,
+    InvalidAttestation, InvalidBlock, PayloadVerificationStatus, PersistedForkChoice,
-    PersistedForkChoiceV28, PersistedForkChoiceV29, QueuedAttestation, QueuedPayloadAttestation,
+    PersistedForkChoiceV17, PersistedForkChoiceV28, PersistedForkChoiceV29, QueuedAttestation,
-    ResetPayloadStatuses,
+    QueuedPayloadAttestation, ResetPayloadStatuses,
 };
 pub use fork_choice_store::ForkChoiceStore;
 pub use proto_array::{

consensus/fork_choice/tests/tests.rs

@@ -10,8 +10,8 @@ use beacon_chain::{
 use bls::AggregateSignature;
 use fixed_bytes::FixedBytesExtended;
 use fork_choice::{
-    ForkChoiceStore, InvalidAttestation, InvalidBlock, PayloadVerificationStatus,
+    AttestationFromBlock, ForkChoiceStore, InvalidAttestation, InvalidBlock,
-    QueuedAttestation, QueuedPayloadAttestation,
+    PayloadVerificationStatus, QueuedAttestation, QueuedPayloadAttestation,
 };
 use state_processing::state_advance::complete_state_advance;
 use std::fmt;
@@ -1033,7 +1033,12 @@ async fn payload_attestation_for_previous_slot_is_accepted_at_next_slot() {
     let result = chain
         .canonical_head
         .fork_choice_write_lock()
-        .on_payload_attestation(current_slot, &payload_attestation, true, ptc);
+        .on_payload_attestation(
+            current_slot,
+            &payload_attestation,
+            AttestationFromBlock::True,
+            ptc,
+        );
 
     assert!(
         result.is_ok(),
@@ -1082,7 +1087,12 @@ async fn non_block_payload_attestation_for_previous_slot_is_rejected() {
     let result = chain
         .canonical_head
         .fork_choice_write_lock()
-        .on_payload_attestation(s_plus_1, &payload_attestation, false, ptc);
+        .on_payload_attestation(
+            s_plus_1,
+            &payload_attestation,
+            AttestationFromBlock::False,
+            ptc,
+        );
     assert!(
         matches!(
             result,

consensus/proto_array/src/proto_array.rs

@@ -1217,8 +1217,6 @@ impl ProtoArray {
             return false;
         };
 
-
-
         // Skip invalid children — they aren't in store.blocks in the spec.
         let children: Vec<usize> = self
             .nodes