Resolve merge conflicts

consensus/fork_choice/Cargo.toml

@@ -8,10 +8,12 @@ edition = { workspace = true }
 [dependencies]
 ethereum_ssz = { workspace = true }
 ethereum_ssz_derive = { workspace = true }
+fixed_bytes = { workspace = true }
 logging = { workspace = true }
 metrics = { workspace = true }
 proto_array = { workspace = true }
 state_processing = { workspace = true }
+superstruct = { workspace = true }
 tracing = { workspace = true }
 types = { workspace = true }
 

consensus/fork_choice/src/fork_choice.rs

@@ -1,10 +1,12 @@
 use crate::metrics::{self, scrape_for_metrics};
 use crate::{ForkChoiceStore, InvalidationOperation};
+use fixed_bytes::FixedBytesExtended;
 use logging::crit;
 use proto_array::{
-    Block as ProtoBlock, DisallowedReOrgOffsets, ExecutionStatus, ProposerHeadError,
-    ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold,
+    Block as ProtoBlock, DisallowedReOrgOffsets, ExecutionStatus, JustifiedBalances,
+    ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold,
 };
+use ssz::{Decode, Encode};
 use ssz_derive::{Decode, Encode};
 use state_processing::{
     per_block_processing::errors::AttesterSlashingValidationError, per_epoch_processing,
@@ -13,12 +15,13 @@ use std::cmp::Ordering;
 use std::collections::BTreeSet;
 use std::marker::PhantomData;
 use std::time::Duration;
-use tracing::{debug, warn};
+use superstruct::superstruct;
+use tracing::{debug, instrument, warn};
 use types::{
-    consts::bellatrix::INTERVALS_PER_SLOT, AbstractExecPayload, AttestationShufflingId,
-    AttesterSlashingRef, BeaconBlockRef, BeaconState, BeaconStateError, ChainSpec, Checkpoint,
-    Epoch, EthSpec, ExecPayload, ExecutionBlockHash, FixedBytesExtended, Hash256,
-    IndexedAttestationRef, RelativeEpoch, SignedBeaconBlock, Slot,
+    AbstractExecPayload, AttestationShufflingId, AttesterSlashingRef, BeaconBlockRef, BeaconState,
+    BeaconStateError, ChainSpec, Checkpoint, Epoch, EthSpec, ExecPayload, ExecutionBlockHash,
+    Hash256, IndexedAttestationRef, RelativeEpoch, SignedBeaconBlock, Slot,
+    consts::bellatrix::INTERVALS_PER_SLOT,
 };
 
 #[derive(Debug)]
@@ -473,6 +476,7 @@ where
     /// Is equivalent to:
     ///
     /// https://github.com/ethereum/eth2.0-specs/blob/v0.12.1/specs/phase0/fork-choice.md#get_head
+    #[instrument(skip_all, level = "debug")]
     pub fn get_head(
         &mut self,
         system_time_current_slot: Slot,
@@ -521,6 +525,7 @@ where
     ///
     /// You *must* call `get_head` for the proposal slot prior to calling this function and pass
     /// in the result of `get_head` as `canonical_head`.
+    #[instrument(level = "debug", skip_all)]
     pub fn get_proposer_head(
         &self,
         current_slot: Slot,
@@ -624,7 +629,7 @@ where
         op: &InvalidationOperation,
     ) -> Result<(), Error<T::Error>> {
         self.proto_array
-            .process_execution_payload_invalidation::<E>(op)
+            .process_execution_payload_invalidation::<E>(op, self.finalized_checkpoint())
             .map_err(Error::FailedToProcessInvalidExecutionPayload)
     }
 
@@ -653,6 +658,12 @@ where
     /// The supplied block **must** pass the `state_transition` function as it will not be run
     /// here.
     #[allow(clippy::too_many_arguments)]
+    #[instrument(
+        name = "fork_choice_on_block",
+        skip_all,
+        fields(
+            fork_choice_block_delay = ?block_delay
+        ))]
     pub fn on_block<Payload: AbstractExecPayload<E>>(
         &mut self,
         system_time_current_slot: Slot,
@@ -736,6 +747,11 @@ where
         self.update_checkpoints(
             state.current_justified_checkpoint(),
             state.finalized_checkpoint(),
+            || {
+                state
+                    .get_state_root_at_epoch_start(state.current_justified_checkpoint().epoch)
+                    .map_err(Into::into)
+            },
         )?;
 
         // Update unrealized justified/finalized checkpoints.
@@ -795,8 +811,15 @@ where
         if unrealized_justified_checkpoint.epoch
             > self.fc_store.unrealized_justified_checkpoint().epoch
         {
-            self.fc_store
-                .set_unrealized_justified_checkpoint(unrealized_justified_checkpoint);
+            // Justification has recently updated therefore the justified state root should be in
+            // range of the head state's `state_roots` vector.
+            let unrealized_justified_state_root =
+                state.get_state_root_at_epoch_start(unrealized_justified_checkpoint.epoch)?;
+
+            self.fc_store.set_unrealized_justified_checkpoint(
+                unrealized_justified_checkpoint,
+                unrealized_justified_state_root,
+            );
         }
         if unrealized_finalized_checkpoint.epoch
             > self.fc_store.unrealized_finalized_checkpoint().epoch
@@ -810,6 +833,13 @@ where
             self.pull_up_store_checkpoints(
                 unrealized_justified_checkpoint,
                 unrealized_finalized_checkpoint,
+                || {
+                    // In the case where we actually update justification, it must be that the
+                    // unrealized justification is recent and in range of the `state_roots` vector.
+                    state
+                        .get_state_root_at_epoch_start(unrealized_justified_checkpoint.epoch)
+                        .map_err(Into::into)
+                },
             )?;
         }
 
@@ -849,7 +879,7 @@ where
                             block_slot: block.slot(),
                             block_root,
                             payload_verification_status,
-                        })
+                        });
                     }
                 }
             }
@@ -886,6 +916,8 @@ where
                 unrealized_finalized_checkpoint: Some(unrealized_finalized_checkpoint),
             },
             current_slot,
+            self.justified_checkpoint(),
+            self.finalized_checkpoint(),
         )?;
 
         Ok(())
@@ -896,11 +928,13 @@ where
         &mut self,
         justified_checkpoint: Checkpoint,
         finalized_checkpoint: Checkpoint,
+        justified_state_root_producer: impl FnOnce() -> Result<Hash256, Error<T::Error>>,
     ) -> Result<(), Error<T::Error>> {
         // Update justified checkpoint.
         if justified_checkpoint.epoch > self.fc_store.justified_checkpoint().epoch {
+            let justified_state_root = justified_state_root_producer()?;
             self.fc_store
-                .set_justified_checkpoint(justified_checkpoint)
+                .set_justified_checkpoint(justified_checkpoint, justified_state_root)
                 .map_err(Error::UnableToSetJustifiedCheckpoint)?;
         }
 
@@ -1166,10 +1200,12 @@ where
         // Update the justified/finalized checkpoints based upon the
         // best-observed unrealized justification/finality.
         let unrealized_justified_checkpoint = *self.fc_store.unrealized_justified_checkpoint();
+        let unrealized_justified_state_root = self.fc_store.unrealized_justified_state_root();
         let unrealized_finalized_checkpoint = *self.fc_store.unrealized_finalized_checkpoint();
         self.pull_up_store_checkpoints(
             unrealized_justified_checkpoint,
             unrealized_finalized_checkpoint,
+            || Ok(unrealized_justified_state_root),
         )?;
 
         Ok(())
@@ -1179,10 +1215,12 @@ where
         &mut self,
         unrealized_justified_checkpoint: Checkpoint,
         unrealized_finalized_checkpoint: Checkpoint,
+        unrealized_justified_state_root_producer: impl FnOnce() -> Result<Hash256, Error<T::Error>>,
     ) -> Result<(), Error<T::Error>> {
         self.update_checkpoints(
             unrealized_justified_checkpoint,
             unrealized_finalized_checkpoint,
+            unrealized_justified_state_root_producer,
         )
     }
 
@@ -1260,7 +1298,7 @@ where
     /// Return `true` if `block_root` is equal to the finalized checkpoint, or a known descendant of it.
     pub fn is_finalized_checkpoint_or_descendant(&self, block_root: Hash256) -> bool {
         self.proto_array
-            .is_finalized_checkpoint_or_descendant::<E>(block_root)
+            .is_finalized_checkpoint_or_descendant::<E>(block_root, self.finalized_checkpoint())
     }
 
     pub fn is_descendant(&self, ancestor_root: Hash256, descendant_root: Hash256) -> bool {
@@ -1375,12 +1413,16 @@ where
     /// Instantiate `Self` from some `PersistedForkChoice` generated by a earlier call to
     /// `Self::to_persisted`.
     pub fn proto_array_from_persisted(
-        persisted: &PersistedForkChoice,
+        persisted_proto_array: proto_array::core::SszContainer,
+        justified_balances: JustifiedBalances,
         reset_payload_statuses: ResetPayloadStatuses,
         spec: &ChainSpec,
     ) -> Result<ProtoArrayForkChoice, Error<T::Error>> {
-        let mut proto_array = ProtoArrayForkChoice::from_bytes(&persisted.proto_array_bytes)
-            .map_err(Error::InvalidProtoArrayBytes)?;
+        let mut proto_array = ProtoArrayForkChoice::from_container(
+            persisted_proto_array.clone(),
+            justified_balances.clone(),
+        )
+        .map_err(Error::InvalidProtoArrayBytes)?;
         let contains_invalid_payloads = proto_array.contains_invalid_payloads();
 
         debug!(
@@ -1408,7 +1450,7 @@ where
                 info = "please report this error",
                 "Failed to reset payload statuses"
             );
-            ProtoArrayForkChoice::from_bytes(&persisted.proto_array_bytes)
+            ProtoArrayForkChoice::from_container(persisted_proto_array, justified_balances)
                 .map_err(Error::InvalidProtoArrayBytes)
         } else {
             debug!("Successfully reset all payload statuses");
@@ -1424,8 +1466,13 @@ where
         fc_store: T,
         spec: &ChainSpec,
     ) -> Result<Self, Error<T::Error>> {
-        let proto_array =
-            Self::proto_array_from_persisted(&persisted, reset_payload_statuses, spec)?;
+        let justified_balances = fc_store.justified_balances().clone();
+        let proto_array = Self::proto_array_from_persisted(
+            persisted.proto_array,
+            justified_balances,
+            reset_payload_statuses,
+            spec,
+        )?;
 
         let current_slot = fc_store.get_current_slot();
 
@@ -1471,7 +1518,9 @@ where
     /// be instantiated again later.
     pub fn to_persisted(&self) -> PersistedForkChoice {
         PersistedForkChoice {
-            proto_array_bytes: self.proto_array().as_bytes(),
+            proto_array: self
+                .proto_array()
+                .as_ssz_container(self.justified_checkpoint(), self.finalized_checkpoint()),
             queued_attestations: self.queued_attestations().to_vec(),
         }
     }
@@ -1485,10 +1534,46 @@ where
 /// Helper struct that is used to encode/decode the state of the `ForkChoice` as SSZ bytes.
 ///
 /// This is used when persisting the state of the fork choice to disk.
-#[derive(Encode, Decode, Clone)]
+#[superstruct(
+    variants(V17, V28),
+    variant_attributes(derive(Encode, Decode, Clone)),
+    no_enum
+)]
 pub struct PersistedForkChoice {
+    #[superstruct(only(V17))]
     pub proto_array_bytes: Vec<u8>,
-    queued_attestations: Vec<QueuedAttestation>,
+    #[superstruct(only(V28))]
+    pub proto_array: proto_array::core::SszContainerV28,
+    pub queued_attestations: Vec<QueuedAttestation>,
+}
+
+pub type PersistedForkChoice = PersistedForkChoiceV28;
+
+impl TryFrom<PersistedForkChoiceV17> for PersistedForkChoiceV28 {
+    type Error = ssz::DecodeError;
+
+    fn try_from(v17: PersistedForkChoiceV17) -> Result<Self, Self::Error> {
+        let container_v17 =
+            proto_array::core::SszContainerV17::from_ssz_bytes(&v17.proto_array_bytes)?;
+        let container_v28 = container_v17.into();
+
+        Ok(Self {
+            proto_array: container_v28,
+            queued_attestations: v17.queued_attestations,
+        })
+    }
+}
+
+impl From<(PersistedForkChoiceV28, JustifiedBalances)> for PersistedForkChoiceV17 {
+    fn from((v28, balances): (PersistedForkChoiceV28, JustifiedBalances)) -> Self {
+        let container_v17 = proto_array::core::SszContainerV17::from((v28.proto_array, balances));
+        let proto_array_bytes = container_v17.as_ssz_bytes();
+
+        Self {
+            proto_array_bytes,
+            queued_attestations: v28.queued_attestations,
+        }
+    }
 }
 
 #[cfg(test)]

consensus/fork_choice/src/fork_choice_store.rs

@@ -44,6 +44,9 @@ pub trait ForkChoiceStore<E: EthSpec>: Sized {
     /// Returns the `justified_checkpoint`.
     fn justified_checkpoint(&self) -> &Checkpoint;
 
+    /// Returns the state root of the justified checkpoint.
+    fn justified_state_root(&self) -> Hash256;
+
     /// Returns balances from the `state` identified by `justified_checkpoint.root`.
     fn justified_balances(&self) -> &JustifiedBalances;
 
@@ -53,6 +56,9 @@ pub trait ForkChoiceStore<E: EthSpec>: Sized {
     /// Returns the `unrealized_justified_checkpoint`.
     fn unrealized_justified_checkpoint(&self) -> &Checkpoint;
 
+    /// Returns the state root of the unrealized justified checkpoint.
+    fn unrealized_justified_state_root(&self) -> Hash256;
+
     /// Returns the `unrealized_finalized_checkpoint`.
     fn unrealized_finalized_checkpoint(&self) -> &Checkpoint;
 
@@ -63,10 +69,14 @@ pub trait ForkChoiceStore<E: EthSpec>: Sized {
     fn set_finalized_checkpoint(&mut self, checkpoint: Checkpoint);
 
     /// Sets the `justified_checkpoint`.
-    fn set_justified_checkpoint(&mut self, checkpoint: Checkpoint) -> Result<(), Self::Error>;
+    fn set_justified_checkpoint(
+        &mut self,
+        checkpoint: Checkpoint,
+        state_root: Hash256,
+    ) -> Result<(), Self::Error>;
 
     /// Sets the `unrealized_justified_checkpoint`.
-    fn set_unrealized_justified_checkpoint(&mut self, checkpoint: Checkpoint);
+    fn set_unrealized_justified_checkpoint(&mut self, checkpoint: Checkpoint, state_root: Hash256);
 
     /// Sets the `unrealized_finalized_checkpoint`.
     fn set_unrealized_finalized_checkpoint(&mut self, checkpoint: Checkpoint);

consensus/fork_choice/src/lib.rs

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

consensus/fork_choice/tests/tests.rs

@@ -7,6 +7,7 @@ use beacon_chain::{
     BeaconChain, BeaconChainError, BeaconForkChoiceStore, ChainConfig, ForkChoiceError,
     StateSkipConfig, WhenSlotSkipped,
 };
+use fixed_bytes::FixedBytesExtended;
 use fork_choice::{
     ForkChoiceStore, InvalidAttestation, InvalidBlock, PayloadVerificationStatus, QueuedAttestation,
 };
@@ -15,10 +16,11 @@ use std::fmt;
 use std::sync::Mutex;
 use std::time::Duration;
 use store::MemoryStore;
+use types::SingleAttestation;
 use types::{
-    test_utils::generate_deterministic_keypair, BeaconBlockRef, BeaconState, ChainSpec, Checkpoint,
-    Epoch, EthSpec, FixedBytesExtended, ForkName, Hash256, IndexedAttestation, MainnetEthSpec,
-    RelativeEpoch, SignedBeaconBlock, Slot, SubnetId,
+    BeaconBlockRef, BeaconState, ChainSpec, Checkpoint, Epoch, EthSpec, ForkName, Hash256,
+    IndexedAttestation, MainnetEthSpec, RelativeEpoch, SignedBeaconBlock, Slot, SubnetId,
+    test_utils::generate_deterministic_keypair,
 };
 
 pub type E = MainnetEthSpec;
@@ -463,10 +465,17 @@ impl ForkChoiceTest {
             )
             .expect("should sign attestation");
 
+        let single_attestation = SingleAttestation {
+            attester_index: validator_index as u64,
+            committee_index: validator_committee_index as u64,
+            data: attestation.data().clone(),
+            signature: attestation.signature().clone(),
+        };
+
         let mut verified_attestation = self
             .harness
             .chain
-            .verify_unaggregated_attestation_for_gossip(&attestation, Some(subnet_id))
+            .verify_unaggregated_attestation_for_gossip(&single_attestation, Some(subnet_id))
             .expect("precondition: should gossip verify attestation");
 
         if let MutationDelay::Blocks(slots) = delay {
@@ -744,11 +753,11 @@ async fn invalid_attestation_empty_bitfield() {
         .apply_attestation_to_chain(
             MutationDelay::NoDelay,
             |attestation, _| match attestation {
-                IndexedAttestation::Base(ref mut att) => {
-                    att.attesting_indices = vec![].into();
+                IndexedAttestation::Base(att) => {
+                    att.attesting_indices = vec![].try_into().unwrap();
                 }
-                IndexedAttestation::Electra(ref mut att) => {
-                    att.attesting_indices = vec![].into();
+                IndexedAttestation::Electra(att) => {
+                    att.attesting_indices = vec![].try_into().unwrap();
                 }
             },
             |result| {