Fix validate_attestation_slot (#352)

* Fix/sync `validate_attestation_slot`

* Fix docstring

* PR feedback, thanks to djrtwo

eth2/beacon/configs.py

@@ -69,6 +69,7 @@
 class CommitteeConfig:
     def __init__(self, config: BeaconConfig):
         # Basic
+        self.GENESIS_SLOT = config.GENESIS_SLOT
         self.GENESIS_EPOCH = config.GENESIS_EPOCH
         self.SHARD_COUNT = config.SHARD_COUNT
         self.SLOTS_PER_EPOCH = config.SLOTS_PER_EPOCH

eth2/beacon/state_machines/forks/serenity/block_validation.py

@@ -310,6 +310,7 @@ def validate_attestation(state: BeaconState,
         state.slot,
         slots_per_epoch,
         min_attestation_inclusion_delay,
+        committee_config.GENESIS_SLOT,
     )
 
     validate_attestation_justified_epoch(
@@ -348,39 +349,37 @@ def validate_attestation(state: BeaconState,
 
 
 def validate_attestation_slot(attestation_data: AttestationData,
-                              current_slot: Slot,
+                              state_slot: Slot,
                               slots_per_epoch: int,
-                              min_attestation_inclusion_delay: int) -> None:
+                              min_attestation_inclusion_delay: int,
+                              genesis_slot: Slot) -> None:
     """
     Validate ``slot`` field of ``attestation_data``.
     Raise ``ValidationError`` if it's invalid.
     """
-    if attestation_data.slot > current_slot - min_attestation_inclusion_delay:
+    if attestation_data.slot < genesis_slot:
         raise ValidationError(
-            "Attestation slot is greater than the ``current_slot`` less the "
-            "``min_attestation_inclusion_delay``:\n"
-            "\tFound: %s, Needed less than or equal to %s (%s - %s)" %
-            (
-                attestation_data.slot,
-                current_slot - min_attestation_inclusion_delay,
-                current_slot,
-                min_attestation_inclusion_delay,
-            )
+            "Can't submit attestations that are too far in history (or in prehistory):\n"
+            f"\tFound attestation slot: {attestation_data.slot}, "
+            f"needed greater than or equal to `GENESIS_SLOT` ({genesis_slot})"
         )
-    if current_slot - min_attestation_inclusion_delay >= attestation_data.slot + slots_per_epoch:
+
+    if state_slot >= attestation_data.slot + slots_per_epoch:
         raise ValidationError(
-            "Attestation slot plus epoch length is too low; "
-            "must equal or exceed the ``current_slot`` less the "
-            "``min_attestation_inclusion_delay``:\n"
-            "\tFound: %s (%s + %s), Needed greater than or equal to: %s (%s - %s)" %
-            (
-                attestation_data.slot + slots_per_epoch,
-                attestation_data.slot,
-                slots_per_epoch,
-                current_slot - min_attestation_inclusion_delay,
-                current_slot,
-                min_attestation_inclusion_delay,
-            )
+            "Attestation slot plus `SLOTS_PER_EPOCH` is too low; "
+            "must exceed the current state:\n"
+            f"\tFound: {attestation_data.slot + slots_per_epoch} "
+            f"({attestation_data.slot} + {slots_per_epoch}), "
+            f"Needed greater than: {state_slot}"
+        )
+
+    if attestation_data.slot + min_attestation_inclusion_delay > state_slot:
+        raise ValidationError(
+            "Can't submit attestations too quickly; attestation slot is greater than "
+            f"current state slot ({state_slot} minus "
+            f"MIN_ATTESTATION_INCLUSION_DELAY ({min_attestation_inclusion_delay}).\n"
+            f"\tFound: {attestation_data.slot}, Needed less than or equal to "
+            f"({state_slot} - {min_attestation_inclusion_delay})"
         )
 
 

tests/eth2/beacon/state_machines/forks/test_serenity_block_attestation_validation.py

@@ -30,33 +30,37 @@
 from eth2.beacon.types.crosslink_records import CrosslinkRecord
 
 
+@pytest.mark.parametrize(
+    ('genesis_slot', 'genesis_epoch', 'slots_per_epoch', 'min_attestation_inclusion_delay'),
+    [
+        (8, 2, 4, 2),
+    ]
+)
 @pytest.mark.parametrize(
     (
         'attestation_slot,'
-        'current_slot,'
-        'slots_per_epoch,'
-        'min_attestation_inclusion_delay,'
+        'state_slot,'
         'is_valid,'
     ),
     [
         # in bounds at lower end
-        (0, 5, 5, 1, True),
+        (8, 2 + 8, True),
         # in bounds at high end
-        (0, 5, 5, 5, True),
-        # attestation_slot + min_attestation_inclusion_delay > current_slot
-        (0, 5, 5, 6, False),
-        # attestation_slot > current_slot
-        (7, 5, 10, 1, False),
-        # in bounds at lower end
-        (10, 20, 10, 2, True),
-        # attestation_slot + SLOTS_PER_EPOCH < current_slot - inclusion_delay
-        (7, 20, 10, 2, False),
+        (8, 8 + 4 - 1, True),
+        # attestation_slot < genesis_slot
+        (7, 2 + 8, False),
+        # state_slot >= attestation_data.slot + slots_per_epoch
+        (8, 8 + 4, False),
+        # attestation_data.slot + min_attestation_inclusion_delay > state_slot
+        (8, 8 - 2, False),
     ]
 )
 def test_validate_attestation_slot(sample_attestation_data_params,
                                    attestation_slot,
-                                   current_slot,
+                                   state_slot,
                                    slots_per_epoch,
+                                   genesis_slot,
+                                   genesis_epoch,
                                    min_attestation_inclusion_delay,
                                    is_valid):
     attestation_data = AttestationData(**sample_attestation_data_params).copy(
@@ -66,17 +70,19 @@ def test_validate_attestation_slot(sample_attestation_data_params,
     if is_valid:
         validate_attestation_slot(
             attestation_data,
-            current_slot,
+            state_slot,
             slots_per_epoch,
             min_attestation_inclusion_delay,
+            genesis_slot,
         )
     else:
         with pytest.raises(ValidationError):
             validate_attestation_slot(
                 attestation_data,
-                current_slot,
+                state_slot,
                 slots_per_epoch,
                 min_attestation_inclusion_delay,
+                genesis_slot,
             )