diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 14fee72e59..d8e15e644d 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -491,9 +491,11 @@ pub fn signature_verify_chain_segment( 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, diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs index 330f2fe6ca..9cd1ea799d 100644 --- a/consensus/types/src/beacon_state.rs +++ b/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 BeaconState { /// dictated by `self.slot()`. pub fn fork_name(&self, spec: &ChainSpec) -> Result { 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 BeaconState { } } + /// 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 { diff --git a/consensus/types/src/beacon_state/diff.rs b/consensus/types/src/beacon_state/diff.rs index b87b7daf42..f1962e4201 100644 --- a/consensus/types/src/beacon_state/diff.rs +++ b/consensus/types/src/beacon_state/diff.rs @@ -84,6 +84,8 @@ pub struct BeaconStateDiff { // 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>, } +#[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 { + 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 Diff for BeaconStateDiff { type Target = BeaconState; type Error = Error; - // FIXME(sproul): proc macro fn compute_diff(orig: &Self::Target, other: &Self::Target) -> Result { - // 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 Diff for BeaconStateDiff { 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 Diff for BeaconStateDiff { 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(()) } } diff --git a/consensus/types/src/execution_payload.rs b/consensus/types/src/execution_payload.rs index 0b77beeb9a..3a9f95fa9c 100644 --- a/consensus/types/src/execution_payload.rs +++ b/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 = VariableList; diff --git a/consensus/types/src/execution_payload_header.rs b/consensus/types/src/execution_payload_header.rs index 70ce75db59..d01e0d89a5 100644 --- a/consensus/types/src/execution_payload_header.rs +++ b/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))]