Make phase 0 fork choice more modular to more easily adopt for slight…

… modifications in phase 1
Zkx007 · Jan 15, 2020 · 3c07b2c · 3c07b2c
1 parent 8d0e1bd
commit 3c07b2c
Show file tree

Hide file tree

Showing 5 changed files with 133 additions and 40 deletions.
diff --git a/Makefile b/Makefile
@@ -107,7 +107,7 @@ $(PY_SPEC_PHASE_0_TARGETS): $(PY_SPEC_PHASE_0_DEPS)
 	python3 $(SCRIPT_DIR)/build_spec.py -p0 $(PHASE0_SPEC_DIR)/beacon-chain.md $(PHASE0_SPEC_DIR)/fork-choice.md $(PHASE0_SPEC_DIR)/validator.md $@
 
 $(PY_SPEC_DIR)/eth2spec/phase1/spec.py: $(PY_SPEC_PHASE_1_DEPS)
-	python3 $(SCRIPT_DIR)/build_spec.py -p1 $(PHASE0_SPEC_DIR)/beacon-chain.md $(PHASE0_SPEC_DIR)/fork-choice.md $(PHASE1_SPEC_DIR)/custody-game.md $(PHASE1_SPEC_DIR)/beacon-chain.md $(PHASE1_SPEC_DIR)/fraud-proofs.md $(PHASE1_SPEC_DIR)/phase1-fork.md $@
+	python3 $(SCRIPT_DIR)/build_spec.py -p1 $(PHASE0_SPEC_DIR)/beacon-chain.md $(PHASE0_SPEC_DIR)/fork-choice.md $(PHASE1_SPEC_DIR)/custody-game.md $(PHASE1_SPEC_DIR)/beacon-chain.md $(PHASE1_SPEC_DIR)/fraud-proofs.md $(PHASE1_SPEC_DIR)/fork-choice.md $(PHASE1_SPEC_DIR)/phase1-fork.md $@
 
 # TODO: also build validator spec and light-client-sync
 

diff --git a/scripts/build_spec.py b/scripts/build_spec.py
@@ -230,6 +230,7 @@ def build_phase1_spec(phase0_beacon_sourcefile: str,
                       phase1_custody_sourcefile: str,
                       phase1_beacon_sourcefile: str,
                       phase1_fraud_sourcefile: str,
+                      phase1_fork_choice_sourcefile: str,
                       phase1_fork_sourcefile: str,
                       outfile: str=None) -> Optional[str]:
     all_sourcefiles = (
@@ -238,6 +239,7 @@ def build_phase1_spec(phase0_beacon_sourcefile: str,
         phase1_custody_sourcefile,
         phase1_beacon_sourcefile,
         phase1_fraud_sourcefile,
+        phase1_fork_choice_sourcefile,
         phase1_fork_sourcefile,
     )
     all_spescs = [get_spec(spec) for spec in all_sourcefiles]
@@ -267,8 +269,9 @@ def build_phase1_spec(phase0_beacon_sourcefile: str,
     3rd argument is input phase1/custody-game.md
     4th argument is input phase1/beacon-chain.md
     5th argument is input phase1/fraud-proofs.md
-    6th argument is input phase1/phase1-fork.md
-    7th argument is output spec.py
+    6th argument is input phase1/fork-choice.md
+    7th argument is input phase1/phase1-fork.md
+    8th argument is output spec.py
 '''
     parser = ArgumentParser(description=description)
     parser.add_argument("-p", "--phase", dest="phase", type=int, default=0, help="Build for phase #")
@@ -281,13 +284,13 @@ def build_phase1_spec(phase0_beacon_sourcefile: str,
         else:
             print(" Phase 0 requires spec, forkchoice, and v-guide inputs as well as an output file.")
     elif args.phase == 1:
-        if len(args.files) == 7:
+        if len(args.files) == 8:
             build_phase1_spec(*args.files)
         else:
             print(
                 " Phase 1 requires input files as well as an output file:\n"
                 "\t phase0: (beacon-chain.md, fork-choice.md)\n"
-                "\t phase1: (custody-game.md, beacon-chain.md, fraud-proofs.md, phase1-fork.md)\n"
+                "\t phase1: (custody-game.md, beacon-chain.md, fraud-proofs.md, fork-choice.md, phase1-fork.md)\n"
                 "\t and output.py"
             )
     else:

diff --git a/specs/phase0/fork-choice.md b/specs/phase0/fork-choice.md
@@ -24,6 +24,10 @@
     - [`get_filtered_block_tree`](#get_filtered_block_tree)
     - [`get_head`](#get_head)
     - [`should_update_justified_checkpoint`](#should_update_justified_checkpoint)
+    - [`on_attestation` helpers](#on_attestation-helpers)
+      - [`validate_on_attestation`](#validate_on_attestation)
+      - [`store_target_checkpoint_state`](#store_target_checkpoint_state)
+      - [`update_latest_messages`](#update_latest_messages)
   - [Handlers](#handlers)
     - [`on_tick`](#on_tick)
     - [`on_block`](#on_block)
@@ -257,6 +261,59 @@ def should_update_justified_checkpoint(store: Store, new_justified_checkpoint: C
     return True
 ```
 
+#### `on_attestation` helpers
+
+##### `validate_on_attestation`
+
+```python
+def validate_on_attestation(store: Store, attestation: Attestation) -> None:
+    target = attestation.data.target
+
+    # Attestations must be from the current or previous epoch
+    current_epoch = compute_epoch_at_slot(get_current_slot(store))
+    # Use GENESIS_EPOCH for previous when genesis to avoid underflow
+    previous_epoch = current_epoch - 1 if current_epoch > GENESIS_EPOCH else GENESIS_EPOCH
+    assert target.epoch in [current_epoch, previous_epoch]
+    assert target.epoch == compute_epoch_at_slot(attestation.data.slot)
+
+    # Attestations target be for a known block. If target block is unknown, delay consideration until the block is found
+    assert target.root in store.blocks
+    # Attestations cannot be from future epochs. If they are, delay consideration until the epoch arrives
+    assert get_current_slot(store) >= compute_start_slot_at_epoch(target.epoch)
+
+    # Attestations must be for a known block. If block is unknown, delay consideration until the block is found
+    assert attestation.data.beacon_block_root in store.blocks
+    # Attestations must not be for blocks in the future. If not, the attestation should not be considered
+    assert store.blocks[attestation.data.beacon_block_root].slot <= attestation.data.slot
+
+    # Attestations can only affect the fork choice of subsequent slots.
+    # Delay consideration in the fork choice until their slot is in the past.
+    assert get_current_slot(store) >= attestation.data.slot + 1
+```
+
+##### `store_target_checkpoint_state`
+
+```python
+def store_target_checkpoint_state(store: Store, target: Checkpoint) -> None:
+    # Store target checkpoint state if not yet seen
+    if target not in store.checkpoint_states:
+        base_state = store.block_states[target.root].copy()
+        process_slots(base_state, compute_start_slot_at_epoch(target.epoch))
+        store.checkpoint_states[target] = base_state
+```
+
+##### `update_latest_messages`
+
+```python
+def update_latest_messages(store: Store, attesting_indices: Sequence[ValidatorIndex], attestation: Attestation) -> None:
+    target = attestation.data.target
+    beacon_block_root = attestation.data.beacon_block_root
+    for i in attesting_indices:
+        if i not in store.latest_messages or target.epoch > store.latest_messages[i].epoch:
+            store.latest_messages[i] = LatestMessage(epoch=target.epoch, root=beacon_block_root)
+```
+
+
 ### Handlers
 
 #### `on_tick`
@@ -323,42 +380,14 @@ def on_attestation(store: Store, attestation: Attestation) -> None:
     An ``attestation`` that is asserted as invalid may be valid at a later time,
     consider scheduling it for later processing in such case.
     """
-    target = attestation.data.target
-
-    # Attestations must be from the current or previous epoch 
-    current_epoch = compute_epoch_at_slot(get_current_slot(store))
-    # Use GENESIS_EPOCH for previous when genesis to avoid underflow
-    previous_epoch = current_epoch - 1 if current_epoch > GENESIS_EPOCH else GENESIS_EPOCH
-    assert target.epoch in [current_epoch, previous_epoch]
-    assert target.epoch == compute_epoch_at_slot(attestation.data.slot)
-
-    # Attestations target be for a known block. If target block is unknown, delay consideration until the block is found
-    assert target.root in store.blocks
-    # Attestations cannot be from future epochs. If they are, delay consideration until the epoch arrives
-    base_state = store.block_states[target.root].copy()
-    assert get_current_slot(store) >= compute_start_slot_at_epoch(target.epoch)
+    validate_on_attestation(store, attestation)
+    store_target_checkpoint_state(store, attestation.data.target)
 
-    # Attestations must be for a known block. If block is unknown, delay consideration until the block is found
-    assert attestation.data.beacon_block_root in store.blocks
-    # Attestations must not be for blocks in the future. If not, the attestation should not be considered
-    assert store.blocks[attestation.data.beacon_block_root].slot <= attestation.data.slot
-
-    # Store target checkpoint state if not yet seen
-    if target not in store.checkpoint_states:
-        process_slots(base_state, compute_start_slot_at_epoch(target.epoch))
-        store.checkpoint_states[target] = base_state
-    target_state = store.checkpoint_states[target]
-
-    # Attestations can only affect the fork choice of subsequent slots.
-    # Delay consideration in the fork choice until their slot is in the past.
-    assert get_current_slot(store) >= attestation.data.slot + 1
-
-    # Get state at the `target` to validate attestation and calculate the committees
+    # Get state at the `target` to fully validate attestation
+    target_state = store.checkpoint_states[attestation.data.target]
     indexed_attestation = get_indexed_attestation(target_state, attestation)
     assert is_valid_indexed_attestation(target_state, indexed_attestation)
 
-    # Update latest messages
-    for i in indexed_attestation.attesting_indices:
-        if i not in store.latest_messages or target.epoch > store.latest_messages[i].epoch:
-            store.latest_messages[i] = LatestMessage(epoch=target.epoch, root=attestation.data.beacon_block_root)
+    # Update latest messages for attesting indices
+    update_latest_messages(store, indexed_attestation.attesting_indices, attestation)
 ```
diff --git a/specs/phase1/fork-choice.md b/specs/phase1/fork-choice.md
@@ -0,0 +1,52 @@
+# Ethereum 2.0 Phase 1 -- Beacon Chain Fork Choice
+
+**Notice**: This document is a work-in-progress for researchers and implementers.
+
+## Table of contents
+<!-- TOC -->
+<!-- START doctoc generated TOC please keep comment here to allow auto update -->
+<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
+
+
+- [Introduction](#introduction)
+- [Fork choice](#fork-choice)
+  - [Handlers](#handlers)
+
+<!-- END doctoc generated TOC please keep comment here to allow auto update -->
+<!-- /TOC -->
+
+## Introduction
+
+This document is the beacon chain fork choice spec for part of Ethereum 2.0 Phase 1.
+
+## Fork choice
+
+Due to the changes in the structure of `IndexedAttestation` in Phase 1, `on_attestation` must be re-specified to handle this. The bulk of `on_attestation` has been moved out into a few helpers to reduce code duplication where possible.
+
+The rest of the fork choice remains stable.
+
+### Handlers
+
+```python
+def on_attestation(store: Store, attestation: Attestation) -> None:
+    """
+    Run ``on_attestation`` upon receiving a new ``attestation`` from either within a block or directly on the wire.
+
+    An ``attestation`` that is asserted as invalid may be valid at a later time,
+    consider scheduling it for later processing in such case.
+    """
+    validate_on_attestation(store, attestation)
+    store_target_checkpoint_state(store, attestation.data.target)
+
+    # Get state at the `target` to fully validate attestation
+    target_state = store.checkpoint_states[attestation.data.target]
+    indexed_attestation = get_indexed_attestation(target_state, attestation)
+    assert is_valid_indexed_attestation(target_state, indexed_attestation)
+
+    # Update latest messages for attesting indices
+    attesting_indices = [
+        index for i, index in enumerate(indexed_attestation.committee)
+        if attestation.aggregation_bits[i]
+    ]
+    update_latest_messages(store, attesting_indices, attestation)
+```
diff --git a/tests/core/pyspec/eth2spec/test/fork_choice/test_on_attestation.py b/tests/core/pyspec/eth2spec/test/fork_choice/test_on_attestation.py
@@ -15,8 +15,17 @@ def run_on_attestation(spec, state, store, attestation, valid=True):
 
     indexed_attestation = spec.get_indexed_attestation(state, attestation)
     spec.on_attestation(store, attestation)
+
+    if spec.version == 'phase0':
+        sample_index = indexed_attestation.attesting_indices[0]
+    else:
+        attesting_indices = [
+            index for i, index in enumerate(indexed_attestation.committee)
+            if attestation.aggregation_bits[i]
+        ]
+        sample_index = attesting_indices[0]
     assert (
-        store.latest_messages[indexed_attestation.attesting_indices[0]] ==
+        store.latest_messages[sample_index] ==
         spec.LatestMessage(
             epoch=attestation.data.target.epoch,
             root=attestation.data.beacon_block_root,