Diff total active balance, fix low hanging fruit

beacon_node/beacon_chain/src/block_verification.rs

@@ -491,9 +491,11 @@ pub fn signature_verify_chain_segment<T: BeaconChainTypes>(
     let mut signature_verified_blocks = chain_segment
         .into_iter()
         .map(|(block_root, block)| {
-            // FIXME(sproul): safe to include proposer index here?
-            let consensus_context =
-                ConsensusContext::new(block.slot()).set_current_block_root(block_root);
+            // It's safe to include the proposer index here as the signature check also checks that
+            // the block is signed by the correct proposer (see `block_proposal_signature_set`).
+            let consensus_context = ConsensusContext::new(block.slot())
+                .set_current_block_root(block_root)
+                .set_proposer_index(block.message().proposer_index());
             SignatureVerifiedBlock {
                 block,
                 block_root,

consensus/types/src/beacon_state.rs

@@ -143,6 +143,11 @@ pub enum Error {
     CommitteeCacheDiffUninitialized {
         expected_epoch: Epoch,
     },
+    DiffAcrossFork {
+        prev_fork: ForkName,
+        current_fork: ForkName,
+    },
+    TotalActiveBalanceDiffUninitialized,
 }
 
 /// Control whether an epoch-indexed field can be indexed at the next epoch or not.
@@ -419,11 +424,7 @@ impl<T: EthSpec> BeaconState<T> {
     /// dictated by `self.slot()`.
     pub fn fork_name(&self, spec: &ChainSpec) -> Result<ForkName, InconsistentFork> {
         let fork_at_slot = spec.fork_name_at_epoch(self.current_epoch());
-        let object_fork = match self {
-            BeaconState::Base { .. } => ForkName::Base,
-            BeaconState::Altair { .. } => ForkName::Altair,
-            BeaconState::Merge { .. } => ForkName::Merge,
-        };
+        let object_fork = self.fork_name_unchecked();
 
         if fork_at_slot == object_fork {
             Ok(object_fork)
@@ -435,6 +436,18 @@ impl<T: EthSpec> BeaconState<T> {
         }
     }
 
+    /// Returns the name of the fork pertaining to `self`.
+    ///
+    /// This is not checked for consistency with respect to the actual fork epochs, see `fork_name`
+    /// for a safer function.
+    pub fn fork_name_unchecked(&self) -> ForkName {
+        match self {
+            BeaconState::Base { .. } => ForkName::Base,
+            BeaconState::Altair { .. } => ForkName::Altair,
+            BeaconState::Merge { .. } => ForkName::Merge,
+        }
+    }
+
     /// Specialised deserialisation method that uses the `ChainSpec` as context.
     #[allow(clippy::integer_arithmetic)]
     pub fn from_ssz_bytes(bytes: &[u8], spec: &ChainSpec) -> Result<Self, ssz::DecodeError> {

consensus/types/src/beacon_state/diff.rs

@@ -84,6 +84,8 @@ pub struct BeaconStateDiff<T: EthSpec> {
 
     // Committee caches
     committee_caches: CommitteeCachesDiff,
+    // Total active balance cache
+    total_active_balance: TotalActiveBalanceDiff,
 }
 
 /// Zero to three committee caches which update a `BeaconState`'s stored committee caches.
@@ -96,6 +98,12 @@ pub struct CommitteeCachesDiff {
     caches: Vec<Arc<CommitteeCache>>,
 }
 
+#[derive(Debug, PartialEq, Encode, Decode)]
+pub struct TotalActiveBalanceDiff {
+    current_epoch: Epoch,
+    balance: u64,
+}
+
 fn optional_field_diff<
     T: EthSpec,
     X,
@@ -217,13 +225,39 @@ impl Diff for CommitteeCachesDiff {
     }
 }
 
+impl Diff for TotalActiveBalanceDiff {
+    type Target = Option<(Epoch, u64)>;
+    type Error = Error;
+
+    fn compute_diff(_: &Self::Target, other: &Self::Target) -> Result<Self, Error> {
+        let (current_epoch, balance) = other.ok_or(Error::TotalActiveBalanceDiffUninitialized)?;
+        Ok(Self {
+            current_epoch,
+            balance,
+        })
+    }
+
+    fn apply_diff(self, target: &mut Self::Target) -> Result<(), Error> {
+        *target = Some((self.current_epoch, self.balance));
+        Ok(())
+    }
+}
+
 impl<T: EthSpec> Diff for BeaconStateDiff<T> {
     type Target = BeaconState<T>;
     type Error = Error;
 
-    // FIXME(sproul): proc macro
     fn compute_diff(orig: &Self::Target, other: &Self::Target) -> Result<Self, Error> {
-        // FIXME(sproul): consider cross-variant diffs
+        // We don't support diffs across forks. A full state should be stored on the fork boundary
+        // instead.
+        let prev_fork = orig.fork_name_unchecked();
+        let current_fork = other.fork_name_unchecked();
+        if prev_fork != current_fork {
+            return Err(Error::DiffAcrossFork {
+                prev_fork,
+                current_fork,
+            });
+        }
 
         // Compute committee caches diff.
         let prev_current_epoch = orig.current_epoch();
@@ -314,6 +348,10 @@ impl<T: EthSpec> Diff for BeaconStateDiff<T> {
                 BeaconState::latest_execution_payload_header,
             )?,
             committee_caches,
+            total_active_balance: TotalActiveBalanceDiff::compute_diff(
+                orig.total_active_balance(),
+                other.total_active_balance(),
+            )?,
         })
     }
 
@@ -380,6 +418,10 @@ impl<T: EthSpec> Diff for BeaconStateDiff<T> {
         self.committee_caches.apply_diff(&mut committee_caches)?;
         *target.committee_caches_mut() = committee_caches.1;
 
+        // Apply total active balance diff.
+        self.total_active_balance
+            .apply_diff(target.total_active_balance_mut())?;
+
         Ok(())
     }
 }

consensus/types/src/execution_payload.rs

@@ -6,7 +6,6 @@ use ssz_derive::{Decode, Encode};
 use test_random_derive::TestRandom;
 use tree_hash_derive::TreeHash;
 
-// FIXME(sproul): tree-ify the payload types
 use ssz_types::{FixedVector, VariableList};
 
 pub type Transaction<T> = VariableList<u8, T>;

consensus/types/src/execution_payload_header.rs

@@ -4,7 +4,6 @@ use ssz_derive::{Decode, Encode};
 use test_random_derive::TestRandom;
 use tree_hash_derive::TreeHash;
 
-// FIXME(sproul): tree-ify the payload types
 use ssz_types::{FixedVector, VariableList};
 
 #[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))]