From e512f7c0e1cc0d836e1aac2adcea4bc5b2b7af23 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Wed, 20 Mar 2019 16:52:58 +1100 Subject: [PATCH] op-pool: validate_attestation_time_independent_only --- eth2/operation_pool/src/lib.rs | 8 +- .../src/per_block_processing.rs | 5 +- .../validate_attestation.rs | 108 +++++++++++------- 3 files changed, 75 insertions(+), 46 deletions(-) diff --git a/eth2/operation_pool/src/lib.rs b/eth2/operation_pool/src/lib.rs index c6af777e6..6d276600a 100644 --- a/eth2/operation_pool/src/lib.rs +++ b/eth2/operation_pool/src/lib.rs @@ -6,8 +6,9 @@ use state_processing::per_block_processing::errors::{ ProposerSlashingValidationError, TransferValidationError, }; use state_processing::per_block_processing::{ - validate_attestation, verify_deposit, verify_exit, verify_exit_time_independent_only, - verify_proposer_slashing, verify_transfer, verify_transfer_time_independent_only, + validate_attestation, validate_attestation_time_independent_only, verify_deposit, verify_exit, + verify_exit_time_independent_only, verify_proposer_slashing, verify_transfer, + verify_transfer_time_independent_only, }; use std::collections::{btree_map::Entry, hash_map, BTreeMap, HashMap, HashSet}; use types::chain_spec::Domain; @@ -113,8 +114,7 @@ impl OperationPool { spec: &ChainSpec, ) -> Result<(), AttestationValidationError> { // Check that attestation signatures are valid. - // FIXME: should disable the time-dependent checks. - validate_attestation(state, &attestation, spec)?; + validate_attestation_time_independent_only(state, &attestation, spec)?; let id = AttestationId::from_data(&attestation.data, state, spec); diff --git a/eth2/state_processing/src/per_block_processing.rs b/eth2/state_processing/src/per_block_processing.rs index 55e2c29d0..031b919c1 100644 --- a/eth2/state_processing/src/per_block_processing.rs +++ b/eth2/state_processing/src/per_block_processing.rs @@ -8,7 +8,10 @@ pub use self::verify_attester_slashing::{ gather_attester_slashing_indices, verify_attester_slashing, }; pub use self::verify_proposer_slashing::verify_proposer_slashing; -pub use validate_attestation::{validate_attestation, validate_attestation_without_signature}; +pub use validate_attestation::{ + validate_attestation, validate_attestation_time_independent_only, + validate_attestation_without_signature, +}; pub use verify_deposit::{get_existing_validator_index, verify_deposit, verify_deposit_index}; pub use verify_exit::{verify_exit, verify_exit_time_independent_only}; pub use verify_slashable_attestation::verify_slashable_attestation; diff --git a/eth2/state_processing/src/per_block_processing/validate_attestation.rs b/eth2/state_processing/src/per_block_processing/validate_attestation.rs index 2143988a4..3b89bec99 100644 --- a/eth2/state_processing/src/per_block_processing/validate_attestation.rs +++ b/eth2/state_processing/src/per_block_processing/validate_attestation.rs @@ -14,7 +14,16 @@ pub fn validate_attestation( attestation: &Attestation, spec: &ChainSpec, ) -> Result<(), Error> { - validate_attestation_signature_optional(state, attestation, spec, true) + validate_attestation_parametric(state, attestation, spec, true, false) +} + +/// Like `validate_attestation` but doesn't run checks which may become true in future states. +pub fn validate_attestation_time_independent_only( + state: &BeaconState, + attestation: &Attestation, + spec: &ChainSpec, +) -> Result<(), Error> { + validate_attestation_parametric(state, attestation, spec, true, true) } /// Indicates if an `Attestation` is valid to be included in a block in the current epoch of the @@ -28,7 +37,7 @@ pub fn validate_attestation_without_signature( attestation: &Attestation, spec: &ChainSpec, ) -> Result<(), Error> { - validate_attestation_signature_optional(state, attestation, spec, false) + validate_attestation_parametric(state, attestation, spec, false, false) } /// Indicates if an `Attestation` is valid to be included in a block in the current epoch of the @@ -36,15 +45,13 @@ pub fn validate_attestation_without_signature( /// /// /// Spec v0.5.0 -fn validate_attestation_signature_optional( +fn validate_attestation_parametric( state: &BeaconState, attestation: &Attestation, spec: &ChainSpec, verify_signature: bool, + time_independent_only: bool, ) -> Result<(), Error> { - let state_epoch = state.slot.epoch(spec.slots_per_epoch); - let attestation_epoch = attestation.data.slot.epoch(spec.slots_per_epoch); - // Can't submit pre-historic attestations. verify!( attestation.data.slot >= spec.genesis_slot, @@ -65,7 +72,8 @@ fn validate_attestation_signature_optional( // Can't submit attestation too quickly. verify!( - attestation.data.slot + spec.min_attestation_inclusion_delay <= state.slot, + time_independent_only + || attestation.data.slot + spec.min_attestation_inclusion_delay <= state.slot, Invalid::IncludedTooEarly { state: state.slot, delay: spec.min_attestation_inclusion_delay, @@ -74,40 +82,8 @@ fn validate_attestation_signature_optional( ); // Verify the justified epoch and root is correct. - if attestation_epoch >= state_epoch { - verify!( - attestation.data.source_epoch == state.current_justified_epoch, - Invalid::WrongJustifiedEpoch { - state: state.current_justified_epoch, - attestation: attestation.data.source_epoch, - is_current: true, - } - ); - verify!( - attestation.data.source_root == state.current_justified_root, - Invalid::WrongJustifiedRoot { - state: state.current_justified_root, - attestation: attestation.data.source_root, - is_current: true, - } - ); - } else { - verify!( - attestation.data.source_epoch == state.previous_justified_epoch, - Invalid::WrongJustifiedEpoch { - state: state.previous_justified_epoch, - attestation: attestation.data.source_epoch, - is_current: false, - } - ); - verify!( - attestation.data.source_root == state.previous_justified_root, - Invalid::WrongJustifiedRoot { - state: state.previous_justified_root, - attestation: attestation.data.source_root, - is_current: true, - } - ); + if !time_independent_only { + verify_justified_epoch_and_root(attestation, state, spec)?; } // Check that the crosslink data is valid. @@ -188,6 +164,56 @@ fn validate_attestation_signature_optional( Ok(()) } +/// Verify that the `source_epoch` and `source_root` of an `Attestation` correctly +/// match the current (or previous) justified epoch and root from the state. +/// +/// Spec v0.5.0 +fn verify_justified_epoch_and_root( + attestation: &Attestation, + state: &BeaconState, + spec: &ChainSpec, +) -> Result<(), Error> { + let state_epoch = state.slot.epoch(spec.slots_per_epoch); + let attestation_epoch = attestation.data.slot.epoch(spec.slots_per_epoch); + + if attestation_epoch >= state_epoch { + verify!( + attestation.data.source_epoch == state.current_justified_epoch, + Invalid::WrongJustifiedEpoch { + state: state.current_justified_epoch, + attestation: attestation.data.source_epoch, + is_current: true, + } + ); + verify!( + attestation.data.source_root == state.current_justified_root, + Invalid::WrongJustifiedRoot { + state: state.current_justified_root, + attestation: attestation.data.source_root, + is_current: true, + } + ); + } else { + verify!( + attestation.data.source_epoch == state.previous_justified_epoch, + Invalid::WrongJustifiedEpoch { + state: state.previous_justified_epoch, + attestation: attestation.data.source_epoch, + is_current: false, + } + ); + verify!( + attestation.data.source_root == state.previous_justified_root, + Invalid::WrongJustifiedRoot { + state: state.previous_justified_root, + attestation: attestation.data.source_root, + is_current: true, + } + ); + } + Ok(()) +} + /// Verifies an aggregate signature for some given `AttestationData`, returning `true` if the /// `aggregate_signature` is valid. ///