op-pool: validate_attestation_time_independent_only

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,
+    validate_attestation, validate_attestation_time_independent_only, verify_deposit, verify_exit,
-    verify_proposer_slashing, verify_transfer, verify_transfer_time_independent_only,
+    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_time_independent_only(state, &attestation, spec)?;
-        validate_attestation(state, &attestation, spec)?;
 
         let id = AttestationId::from_data(&attestation.data, state, spec);
 

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;

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 {
+    if !time_independent_only {
-        verify!(
+        verify_justified_epoch_and_root(attestation, state, spec)?;
-            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,
-            }
-        );
     }
 
     // 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.
 ///