From c88fcfed2b319b5dda4b1ff293e8cab5acd80621 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 17 Feb 2022 16:40:15 +1100 Subject: [PATCH] Implement ConsensusContext --- beacon_node/beacon_chain/src/beacon_chain.rs | 5 +- .../beacon_chain/src/block_verification.rs | 32 +++++-- beacon_node/beacon_chain/src/fork_revert.rs | 7 +- beacon_node/store/src/reconstruct.rs | 9 +- .../state_processing/src/block_replayer.rs | 6 +- .../state_processing/src/consensus_context.rs | 89 +++++++++++++++++++ consensus/state_processing/src/lib.rs | 2 + .../src/per_block_processing.rs | 20 +++-- .../src/per_block_processing/errors.rs | 24 ++++- lcli/src/transition_blocks.rs | 2 +- 10 files changed, 170 insertions(+), 26 deletions(-) create mode 100644 consensus/state_processing/src/consensus_context.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 2de12c6c3a..4eafc2271d 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -70,7 +70,7 @@ use state_processing::{ per_block_processing::{errors::AttestationValidationError, is_merge_transition_complete}, per_slot_processing, state_advance::{complete_state_advance, partial_state_advance}, - BlockSignatureStrategy, SigVerifiedOp, VerifyBlockRoot, + BlockSignatureStrategy, ConsensusContext, SigVerifiedOp, VerifyBlockRoot, }; use std::borrow::Cow; use std::cmp::Ordering; @@ -3096,12 +3096,13 @@ impl BeaconChain { } let process_timer = metrics::start_timer(&metrics::BLOCK_PRODUCTION_PROCESS_TIMES); + let mut ctxt = ConsensusContext::new(block.slot()).set_proposer_index(proposer_index); per_block_processing( &mut state, &block, - None, BlockSignatureStrategy::VerifyRandao, VerifyBlockRoot::True, + &mut ctxt, &self.spec, )?; drop(process_timer); diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 980540583e..fdd347f144 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -63,7 +63,8 @@ use state_processing::{ block_signature_verifier::{BlockSignatureVerifier, Error as BlockSignatureVerifierError}, per_block_processing, per_slot_processing, state_advance::partial_state_advance, - BlockProcessingError, BlockSignatureStrategy, SlotProcessingError, VerifyBlockRoot, + BlockProcessingError, BlockSignatureStrategy, ConsensusContext, SlotProcessingError, + VerifyBlockRoot, }; use std::borrow::Cow; use std::fs; @@ -488,10 +489,16 @@ pub fn signature_verify_chain_segment( let mut signature_verified_blocks = chain_segment .into_iter() - .map(|(block_root, block)| SignatureVerifiedBlock { - block, - block_root, - parent: None, + .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); + SignatureVerifiedBlock { + block, + block_root, + parent: None, + consensus_context, + } }) .collect::>(); @@ -509,6 +516,7 @@ pub struct GossipVerifiedBlock { pub block: SignedBeaconBlock, pub block_root: Hash256, parent: Option>, + consensus_context: ConsensusContext, } /// A wrapper around a `SignedBeaconBlock` that indicates that all signatures (except the deposit @@ -517,6 +525,7 @@ pub struct SignatureVerifiedBlock { block: SignedBeaconBlock, block_root: Hash256, parent: Option>, + consensus_context: ConsensusContext, } /// A wrapper around a `SignedBeaconBlock` that indicates that this block is fully verified and @@ -780,10 +789,16 @@ impl GossipVerifiedBlock { // Validate the block's execution_payload (if any). validate_execution_payload_for_gossip(&parent_block, block.message(), chain)?; + // Having checked the proposer index and the block root we can cache them. + let consensus_context = ConsensusContext::new(block.slot()) + .set_current_block_root(block_root) + .set_proposer_index(block.message().proposer_index()); + Ok(Self { block, block_root, parent, + consensus_context, }) } @@ -846,6 +861,8 @@ impl SignatureVerifiedBlock { if signature_verifier.verify().is_ok() { Ok(Self { + consensus_context: ConsensusContext::new(block.slot()) + .set_current_block_root(block_root), block, block_root, parent: Some(parent), @@ -895,6 +912,7 @@ impl SignatureVerifiedBlock { block, block_root: from.block_root, parent: Some(parent), + consensus_context: from.consensus_context, }) } else { Err(BlockError::InvalidSignature) @@ -930,6 +948,7 @@ impl IntoFullyVerifiedBlock for SignatureVerifiedBlock FullyVerifiedBlock<'a, T> { block: SignedBeaconBlock, block_root: Hash256, parent: PreProcessingSnapshot, + mut consensus_context: ConsensusContext, chain: &BeaconChain, ) -> Result> { // Reject any block if its parent is not known to fork choice. @@ -1172,10 +1192,10 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> { if let Err(err) = per_block_processing( &mut state, &block, - Some(block_root), // Signatures were verified earlier in this function. BlockSignatureStrategy::NoVerification, VerifyBlockRoot::True, + &mut consensus_context, &chain.spec, ) { match err { diff --git a/beacon_node/beacon_chain/src/fork_revert.rs b/beacon_node/beacon_chain/src/fork_revert.rs index 3ae3bf8a3e..e837c4fa62 100644 --- a/beacon_node/beacon_chain/src/fork_revert.rs +++ b/beacon_node/beacon_chain/src/fork_revert.rs @@ -4,7 +4,8 @@ use itertools::process_results; use slog::{info, warn, Logger}; use state_processing::state_advance::complete_state_advance; use state_processing::{ - per_block_processing, per_block_processing::BlockSignatureStrategy, VerifyBlockRoot, + per_block_processing, per_block_processing::BlockSignatureStrategy, ConsensusContext, + VerifyBlockRoot, }; use std::sync::Arc; use std::time::Duration; @@ -158,12 +159,14 @@ pub fn reset_fork_choice_to_finalization, Cold: It complete_state_advance(&mut state, None, block.slot(), spec) .map_err(|e| format!("State advance failed: {:?}", e))?; + let mut ctxt = ConsensusContext::new(block.slot()) + .set_proposer_index(block.message().proposer_index()); per_block_processing( &mut state, &block, - None, BlockSignatureStrategy::NoVerification, VerifyBlockRoot::True, + &mut ctxt, spec, ) .map_err(|e| format!("Error replaying block: {:?}", e))?; diff --git a/beacon_node/store/src/reconstruct.rs b/beacon_node/store/src/reconstruct.rs index 6b808974e7..bae051780d 100644 --- a/beacon_node/store/src/reconstruct.rs +++ b/beacon_node/store/src/reconstruct.rs @@ -4,7 +4,8 @@ use crate::{Error, ItemStore, KeyValueStore}; use itertools::{process_results, Itertools}; use slog::info; use state_processing::{ - per_block_processing, per_slot_processing, BlockSignatureStrategy, VerifyBlockRoot, + per_block_processing, per_slot_processing, BlockSignatureStrategy, ConsensusContext, + VerifyBlockRoot, }; use std::sync::Arc; use types::{EthSpec, Hash256}; @@ -87,12 +88,16 @@ where // Apply block. if let Some(block) = block { + let mut ctxt = ConsensusContext::new(block.slot()) + .set_current_block_root(block_root) + .set_proposer_index(block.message().proposer_index()); + per_block_processing( &mut state, &block, - Some(block_root), BlockSignatureStrategy::NoVerification, VerifyBlockRoot::True, + &mut ctxt, &self.spec, ) .map_err(HotColdDBError::BlockReplayBlockError)?; diff --git a/consensus/state_processing/src/block_replayer.rs b/consensus/state_processing/src/block_replayer.rs index c018a2fab0..aede0095fc 100644 --- a/consensus/state_processing/src/block_replayer.rs +++ b/consensus/state_processing/src/block_replayer.rs @@ -1,6 +1,7 @@ use crate::{ per_block_processing, per_epoch_processing::EpochProcessingSummary, per_slot_processing, - BlockProcessingError, BlockSignatureStrategy, SlotProcessingError, VerifyBlockRoot, + BlockProcessingError, BlockSignatureStrategy, ConsensusContext, SlotProcessingError, + VerifyBlockRoot, }; use std::marker::PhantomData; use types::{BeaconState, ChainSpec, EthSpec, Hash256, SignedBeaconBlock, Slot}; @@ -226,12 +227,13 @@ where VerifyBlockRoot::False } }); + let mut ctxt = ConsensusContext::new(block.slot()); per_block_processing( &mut self.state, block, - None, self.block_sig_strategy, verify_block_root, + &mut ctxt, self.spec, ) .map_err(BlockReplayError::from)?; diff --git a/consensus/state_processing/src/consensus_context.rs b/consensus/state_processing/src/consensus_context.rs new file mode 100644 index 0000000000..0d14429467 --- /dev/null +++ b/consensus/state_processing/src/consensus_context.rs @@ -0,0 +1,89 @@ +use std::marker::PhantomData; +use tree_hash::TreeHash; +use types::{BeaconState, BeaconStateError, ChainSpec, EthSpec, Hash256, SignedBeaconBlock, Slot}; + +#[derive(Debug)] +pub struct ConsensusContext { + /// Slot to act as an identifier/safeguard + slot: Slot, + /// Proposer index of the block at `slot`. + proposer_index: Option, + /// Block root of the block at `slot`. + current_block_root: Option, + _phantom: PhantomData, +} + +#[derive(Debug, PartialEq, Clone)] +pub enum ContextError { + BeaconState(BeaconStateError), + SlotMismatch { slot: Slot, expected: Slot }, +} + +impl From for ContextError { + fn from(e: BeaconStateError) -> Self { + Self::BeaconState(e) + } +} + +impl ConsensusContext { + pub fn new(slot: Slot) -> Self { + Self { + slot, + proposer_index: None, + current_block_root: None, + _phantom: PhantomData, + } + } + + pub fn set_proposer_index(mut self, proposer_index: u64) -> Self { + self.proposer_index = Some(proposer_index); + self + } + + pub fn get_proposer_index( + &mut self, + state: &BeaconState, + spec: &ChainSpec, + ) -> Result { + self.check_slot(state.slot())?; + + if let Some(proposer_index) = self.proposer_index { + return Ok(proposer_index); + } + + let proposer_index = state.get_beacon_proposer_index(self.slot, spec)? as u64; + self.proposer_index = Some(proposer_index); + Ok(proposer_index) + } + + pub fn set_current_block_root(mut self, block_root: Hash256) -> Self { + self.current_block_root = Some(block_root); + self + } + + pub fn get_current_block_root( + &mut self, + block: &SignedBeaconBlock, + ) -> Result { + self.check_slot(block.slot())?; + + if let Some(current_block_root) = self.current_block_root { + return Ok(current_block_root); + } + + let current_block_root = block.tree_hash_root(); + self.current_block_root = Some(current_block_root); + Ok(current_block_root) + } + + fn check_slot(&self, slot: Slot) -> Result<(), ContextError> { + if slot == self.slot { + Ok(()) + } else { + Err(ContextError::SlotMismatch { + slot, + expected: self.slot, + }) + } + } +} diff --git a/consensus/state_processing/src/lib.rs b/consensus/state_processing/src/lib.rs index 1c90b4355d..154e81ad7e 100644 --- a/consensus/state_processing/src/lib.rs +++ b/consensus/state_processing/src/lib.rs @@ -18,6 +18,7 @@ mod metrics; pub mod block_replayer; pub mod common; +pub mod consensus_context; pub mod genesis; pub mod per_block_processing; pub mod per_epoch_processing; @@ -27,6 +28,7 @@ pub mod upgrade; pub mod verify_operation; pub use block_replayer::{BlockReplayError, BlockReplayer}; +pub use consensus_context::{ConsensusContext, ContextError}; pub use genesis::{ eth2_genesis_time, initialize_beacon_state_from_eth1, is_valid_genesis_state, process_activations, diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 857c776332..09c5f53fd9 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -1,3 +1,4 @@ +use crate::consensus_context::ConsensusContext; use errors::{BlockOperationError, BlockProcessingError, HeaderInvalid}; use rayon::prelude::*; use safe_arith::{ArithError, SafeArith}; @@ -90,9 +91,9 @@ pub enum VerifyBlockRoot { pub fn per_block_processing( state: &mut BeaconState, signed_block: &SignedBeaconBlock, - block_root: Option, block_signature_strategy: BlockSignatureStrategy, verify_block_root: VerifyBlockRoot, + ctxt: &mut ConsensusContext, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { let block = signed_block.message(); @@ -110,6 +111,7 @@ pub fn per_block_processing( let verify_signatures = match block_signature_strategy { BlockSignatureStrategy::VerifyBulk => { // Verify all signatures in the block at once. + let block_root = Some(ctxt.get_current_block_root(signed_block)?); block_verify!( BlockSignatureVerifier::verify_entire_block( state, @@ -129,10 +131,10 @@ pub fn per_block_processing( BlockSignatureStrategy::VerifyRandao => VerifySignatures::False, }; - let proposer_index = process_block_header(state, block, verify_block_root, spec)?; + let proposer_index = process_block_header(state, block, verify_block_root, ctxt, spec)?; if verify_signatures.is_true() { - verify_block_signature(state, signed_block, block_root, spec)?; + verify_block_signature(state, signed_block, ctxt, spec)?; } let verify_randao = if let BlockSignatureStrategy::VerifyRandao = block_signature_strategy { @@ -174,6 +176,7 @@ pub fn process_block_header( state: &mut BeaconState, block: BeaconBlockRef<'_, T>, verify_block_root: VerifyBlockRoot, + ctxt: &mut ConsensusContext, spec: &ChainSpec, ) -> Result> { // Verify that the slots match @@ -192,8 +195,8 @@ pub fn process_block_header( ); // Verify that proposer index is the correct index - let proposer_index = block.proposer_index() as usize; - let state_proposer_index = state.get_beacon_proposer_index(block.slot(), spec)?; + let proposer_index = block.proposer_index(); + let state_proposer_index = ctxt.get_proposer_index(state, spec)?; verify!( proposer_index == state_proposer_index, HeaderInvalid::ProposerIndexMismatch { @@ -217,11 +220,11 @@ pub fn process_block_header( // Verify proposer is not slashed verify!( - !state.get_validator(proposer_index)?.slashed, + !state.get_validator(proposer_index as usize)?.slashed, HeaderInvalid::ProposerSlashed(proposer_index) ); - Ok(block.proposer_index()) + Ok(proposer_index) } /// Verifies the signature of a block. @@ -230,9 +233,10 @@ pub fn process_block_header( pub fn verify_block_signature( state: &BeaconState, block: &SignedBeaconBlock, - block_root: Option, + ctxt: &mut ConsensusContext, spec: &ChainSpec, ) -> Result<(), BlockOperationError> { + let block_root = Some(ctxt.get_current_block_root(block)?); verify!( block_proposal_signature_set( state, diff --git a/consensus/state_processing/src/per_block_processing/errors.rs b/consensus/state_processing/src/per_block_processing/errors.rs index 21c3024347..b001ef378f 100644 --- a/consensus/state_processing/src/per_block_processing/errors.rs +++ b/consensus/state_processing/src/per_block_processing/errors.rs @@ -1,4 +1,5 @@ use super::signature_sets::Error as SignatureSetError; +use crate::ContextError; use merkle_proof::MerkleTreeError; use safe_arith::ArithError; use types::*; @@ -70,6 +71,7 @@ pub enum BlockProcessingError { found: u64, }, ExecutionInvalid, + ConsensusContext(ContextError), #[cfg(feature = "milhouse")] MilhouseError(milhouse::Error), } @@ -104,6 +106,12 @@ impl From for BlockProcessingError { } } +impl From for BlockProcessingError { + fn from(e: ContextError) -> Self { + BlockProcessingError::ConsensusContext(e) + } +} + #[cfg(feature = "milhouse")] impl From for BlockProcessingError { fn from(e: milhouse::Error) -> Self { @@ -118,6 +126,7 @@ impl From> for BlockProcessingError { BlockOperationError::BeaconStateError(e) => BlockProcessingError::BeaconStateError(e), BlockOperationError::SignatureSetError(e) => BlockProcessingError::SignatureSetError(e), BlockOperationError::SszTypesError(e) => BlockProcessingError::SszTypesError(e), + BlockOperationError::ConsensusContext(e) => BlockProcessingError::ConsensusContext(e), BlockOperationError::ArithError(e) => BlockProcessingError::ArithError(e), } } @@ -145,6 +154,7 @@ macro_rules! impl_into_block_processing_error_with_index { BlockOperationError::BeaconStateError(e) => BlockProcessingError::BeaconStateError(e), BlockOperationError::SignatureSetError(e) => BlockProcessingError::SignatureSetError(e), BlockOperationError::SszTypesError(e) => BlockProcessingError::SszTypesError(e), + BlockOperationError::ConsensusContext(e) => BlockProcessingError::ConsensusContext(e), BlockOperationError::ArithError(e) => BlockProcessingError::ArithError(e), } } @@ -176,6 +186,7 @@ pub enum BlockOperationError { BeaconStateError(BeaconStateError), SignatureSetError(SignatureSetError), SszTypesError(ssz_types::Error), + ConsensusContext(ContextError), ArithError(ArithError), } @@ -208,6 +219,12 @@ impl From for BlockOperationError { } } +impl From for BlockOperationError { + fn from(e: ContextError) -> Self { + BlockOperationError::ConsensusContext(e) + } +} + #[derive(Debug, PartialEq, Clone)] pub enum HeaderInvalid { ProposalSignatureInvalid, @@ -217,14 +234,14 @@ pub enum HeaderInvalid { block_slot: Slot, }, ProposerIndexMismatch { - block_proposer_index: usize, - state_proposer_index: usize, + block_proposer_index: u64, + state_proposer_index: u64, }, ParentBlockRootMismatch { state: Hash256, block: Hash256, }, - ProposerSlashed(usize), + ProposerSlashed(u64), } #[derive(Debug, PartialEq, Clone)] @@ -319,6 +336,7 @@ impl From> BlockOperationError::BeaconStateError(e) => BlockOperationError::BeaconStateError(e), BlockOperationError::SignatureSetError(e) => BlockOperationError::SignatureSetError(e), BlockOperationError::SszTypesError(e) => BlockOperationError::SszTypesError(e), + BlockOperationError::ConsensusContext(e) => BlockOperationError::ConsensusContext(e), BlockOperationError::ArithError(e) => BlockOperationError::ArithError(e), } } diff --git a/lcli/src/transition_blocks.rs b/lcli/src/transition_blocks.rs index fdfce39171..9111d6d416 100644 --- a/lcli/src/transition_blocks.rs +++ b/lcli/src/transition_blocks.rs @@ -98,7 +98,7 @@ fn do_transition( &mut pre_state, &block, None, - BlockSignatureStrategy::VerifyBulk, + BlockSignatureStrategy::NoVerification, VerifyBlockRoot::True, spec, )