diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index e71039a3730..7c5a203ed7f 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2199,12 +2199,14 @@ impl BeaconChain { &self, exit: SignedVoluntaryExit, ) -> Result, Error> { - // NOTE: this could be more efficient if it avoided cloning the head state - let wall_clock_state = self.wall_clock_state()?; + let head_snapshot = self.head().snapshot; + let head_state = &head_snapshot.beacon_state; + let wall_clock_epoch = self.epoch()?; + Ok(self .observed_voluntary_exits .lock() - .verify_and_observe(exit, &wall_clock_state, &self.spec) + .verify_and_observe_at(exit, wall_clock_epoch, head_state, &self.spec) .map(|exit| { // this method is called for both API and gossip exits, so this covers all exit events if let Some(event_handler) = self.event_handler.as_ref() { diff --git a/beacon_node/beacon_chain/src/observed_operations.rs b/beacon_node/beacon_chain/src/observed_operations.rs index 6e53373939a..4121111b3ee 100644 --- a/beacon_node/beacon_chain/src/observed_operations.rs +++ b/beacon_node/beacon_chain/src/observed_operations.rs @@ -1,11 +1,11 @@ use derivative::Derivative; use smallvec::{smallvec, SmallVec}; use ssz::{Decode, Encode}; -use state_processing::{SigVerifiedOp, VerifyOperation}; +use state_processing::{SigVerifiedOp, VerifyOperation, VerifyOperationAt}; use std::collections::HashSet; use std::marker::PhantomData; use types::{ - AttesterSlashing, BeaconState, ChainSpec, EthSpec, ForkName, ProposerSlashing, + AttesterSlashing, BeaconState, ChainSpec, Epoch, EthSpec, ForkName, ProposerSlashing, SignedBlsToExecutionChange, SignedVoluntaryExit, Slot, }; @@ -87,12 +87,16 @@ impl ObservableOperation for SignedBlsToExecutionChange { } impl, E: EthSpec> ObservedOperations { - pub fn verify_and_observe( + pub fn verify_and_observe_parametric( &mut self, op: T, + validate: F, head_state: &BeaconState, spec: &ChainSpec, - ) -> Result, T::Error> { + ) -> Result, T::Error> + where + F: Fn(T) -> Result, T::Error>, + { self.reset_at_fork_boundary(head_state.slot(), spec); let observed_validator_indices = &mut self.observed_validator_indices; @@ -112,7 +116,7 @@ impl, E: EthSpec> ObservedOperations { } // Validate the op using operation-specific logic (`verify_attester_slashing`, etc). - let verified_op = op.validate(head_state, spec)?; + let verified_op = validate(op)?; // Add the relevant indices to the set of known indices to prevent processing of duplicates // in the future. @@ -121,6 +125,16 @@ impl, E: EthSpec> ObservedOperations { Ok(ObservationOutcome::New(verified_op)) } + pub fn verify_and_observe( + &mut self, + op: T, + head_state: &BeaconState, + spec: &ChainSpec, + ) -> Result, T::Error> { + let validate = |op: T| op.validate(head_state, spec); + self.verify_and_observe_parametric(op, validate, head_state, spec) + } + /// Reset the cache when crossing a fork boundary. /// /// This prevents an attacker from crafting a self-slashing which is only valid before the fork @@ -140,3 +154,16 @@ impl, E: EthSpec> ObservedOperations { } } } + +impl + VerifyOperationAt, E: EthSpec> ObservedOperations { + pub fn verify_and_observe_at( + &mut self, + op: T, + verify_at_epoch: Epoch, + head_state: &BeaconState, + spec: &ChainSpec, + ) -> Result, T::Error> { + let validate = |op: T| op.validate_at(head_state, verify_at_epoch, spec); + self.verify_and_observe_parametric(op, validate, head_state, spec) + } +} diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index d401deb8968..6b98d1dbcec 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -497,7 +497,8 @@ impl OperationPool { |exit| { filter(exit.as_inner()) && exit.signature_is_still_valid(&state.fork()) - && verify_exit(state, exit.as_inner(), VerifySignatures::False, spec).is_ok() + && verify_exit(state, None, exit.as_inner(), VerifySignatures::False, spec) + .is_ok() }, |exit| exit.as_inner().clone(), T::MaxVoluntaryExits::to_usize(), diff --git a/consensus/state_processing/src/lib.rs b/consensus/state_processing/src/lib.rs index e4f36bedd8c..9641e8f96ec 100644 --- a/consensus/state_processing/src/lib.rs +++ b/consensus/state_processing/src/lib.rs @@ -41,4 +41,4 @@ pub use per_epoch_processing::{ errors::EpochProcessingError, process_epoch as per_epoch_processing, }; pub use per_slot_processing::{per_slot_processing, Error as SlotProcessingError}; -pub use verify_operation::{SigVerifiedOp, VerifyOperation}; +pub use verify_operation::{SigVerifiedOp, VerifyOperation, VerifyOperationAt}; diff --git a/consensus/state_processing/src/per_block_processing/process_operations.rs b/consensus/state_processing/src/per_block_processing/process_operations.rs index cbfc2bd465e..3802d049132 100644 --- a/consensus/state_processing/src/per_block_processing/process_operations.rs +++ b/consensus/state_processing/src/per_block_processing/process_operations.rs @@ -284,7 +284,8 @@ pub fn process_exits( // Verify and apply each exit in series. We iterate in series because higher-index exits may // become invalid due to the application of lower-index ones. for (i, exit) in voluntary_exits.iter().enumerate() { - verify_exit(state, exit, verify_signatures, spec).map_err(|e| e.into_with_index(i))?; + verify_exit(state, None, exit, verify_signatures, spec) + .map_err(|e| e.into_with_index(i))?; initiate_validator_exit(state, exit.message.validator_index as usize, spec)?; } diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index db0b91d03c1..63f71915fed 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -978,8 +978,14 @@ async fn fork_spanning_exit() { let head = harness.chain.canonical_head.cached_head(); let head_state = &head.snapshot.beacon_state; assert!(head_state.current_epoch() < spec.altair_fork_epoch.unwrap()); - verify_exit(head_state, &signed_exit, VerifySignatures::True, &spec) - .expect("phase0 exit verifies against phase0 state"); + verify_exit( + head_state, + None, + &signed_exit, + VerifySignatures::True, + &spec, + ) + .expect("phase0 exit verifies against phase0 state"); /* * Ensure the exit verifies after Altair. @@ -992,8 +998,14 @@ async fn fork_spanning_exit() { let head_state = &head.snapshot.beacon_state; assert!(head_state.current_epoch() >= spec.altair_fork_epoch.unwrap()); assert!(head_state.current_epoch() < spec.bellatrix_fork_epoch.unwrap()); - verify_exit(head_state, &signed_exit, VerifySignatures::True, &spec) - .expect("phase0 exit verifies against altair state"); + verify_exit( + head_state, + None, + &signed_exit, + VerifySignatures::True, + &spec, + ) + .expect("phase0 exit verifies against altair state"); /* * Ensure the exit no longer verifies after Bellatrix. @@ -1009,6 +1021,12 @@ async fn fork_spanning_exit() { let head = harness.chain.canonical_head.cached_head(); let head_state = &head.snapshot.beacon_state; assert!(head_state.current_epoch() >= spec.bellatrix_fork_epoch.unwrap()); - verify_exit(head_state, &signed_exit, VerifySignatures::True, &spec) - .expect_err("phase0 exit does not verify against bellatrix state"); + verify_exit( + head_state, + None, + &signed_exit, + VerifySignatures::True, + &spec, + ) + .expect_err("phase0 exit does not verify against bellatrix state"); } diff --git a/consensus/state_processing/src/per_block_processing/verify_exit.rs b/consensus/state_processing/src/per_block_processing/verify_exit.rs index f17e5fcd230..9e9282912de 100644 --- a/consensus/state_processing/src/per_block_processing/verify_exit.rs +++ b/consensus/state_processing/src/per_block_processing/verify_exit.rs @@ -20,10 +20,12 @@ fn error(reason: ExitInvalid) -> BlockOperationError { /// Spec v0.12.1 pub fn verify_exit( state: &BeaconState, + current_epoch: Option, signed_exit: &SignedVoluntaryExit, verify_signatures: VerifySignatures, spec: &ChainSpec, ) -> Result<()> { + let current_epoch = current_epoch.unwrap_or(state.current_epoch()); let exit = &signed_exit.message; let validator = state @@ -33,7 +35,7 @@ pub fn verify_exit( // Verify the validator is active. verify!( - validator.is_active_at(state.current_epoch()), + validator.is_active_at(current_epoch), ExitInvalid::NotActive(exit.validator_index) ); @@ -45,9 +47,9 @@ pub fn verify_exit( // Exits must specify an epoch when they become valid; they are not valid before then. verify!( - state.current_epoch() >= exit.epoch, + current_epoch >= exit.epoch, ExitInvalid::FutureEpoch { - state: state.current_epoch(), + state: current_epoch, exit: exit.epoch } ); @@ -57,9 +59,9 @@ pub fn verify_exit( .activation_epoch .safe_add(spec.shard_committee_period)?; verify!( - state.current_epoch() >= earliest_exit_epoch, + current_epoch >= earliest_exit_epoch, ExitInvalid::TooYoungToExit { - current_epoch: state.current_epoch(), + current_epoch, earliest_exit_epoch, } ); diff --git a/consensus/state_processing/src/verify_operation.rs b/consensus/state_processing/src/verify_operation.rs index 50ac2ff3de5..864844080fb 100644 --- a/consensus/state_processing/src/verify_operation.rs +++ b/consensus/state_processing/src/verify_operation.rs @@ -134,7 +134,7 @@ impl VerifyOperation for SignedVoluntaryExit { state: &BeaconState, spec: &ChainSpec, ) -> Result, Self::Error> { - verify_exit(state, &self, VerifySignatures::True, spec)?; + verify_exit(state, None, &self, VerifySignatures::True, spec)?; Ok(SigVerifiedOp::new(self, state)) } @@ -205,3 +205,35 @@ impl VerifyOperation for SignedBlsToExecutionChange { smallvec![] } } + +/// Trait for operations that can be verified and transformed into a +/// `SigVerifiedOp`. +/// +/// The `At` suffix indicates that we can specify a particular epoch at which to +/// verify the operation. +pub trait VerifyOperationAt: VerifyOperation + Sized { + fn validate_at( + self, + state: &BeaconState, + validate_at_epoch: Epoch, + spec: &ChainSpec, + ) -> Result, Self::Error>; +} + +impl VerifyOperationAt for SignedVoluntaryExit { + fn validate_at( + self, + state: &BeaconState, + validate_at_epoch: Epoch, + spec: &ChainSpec, + ) -> Result, Self::Error> { + verify_exit( + state, + Some(validate_at_epoch), + &self, + VerifySignatures::True, + spec, + )?; + Ok(SigVerifiedOp::new(self, state)) + } +}