From 4f42f63e4e6cb8dbc0c7237dbfe0e4fee8b660c1 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 5 Nov 2019 11:35:58 -0700 Subject: [PATCH 1/3] only allow attestatiosn to be considered from current and previous epoch --- specs/core/0_fork-choice.md | 3 ++ .../test/fork_choice/test_on_attestation.py | 52 +++++++++++++++++-- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/specs/core/0_fork-choice.md b/specs/core/0_fork-choice.md index e08ea09b68..09c2fd31c4 100644 --- a/specs/core/0_fork-choice.md +++ b/specs/core/0_fork-choice.md @@ -224,6 +224,9 @@ def on_block(store: Store, block: BeaconBlock) -> None: def 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)) + assert target.epoch in [current_epoch, current_epoch - 1 if current_epoch > GENESIS_EPOCH else GENESIS_EPOCH] # Cannot calculate the current shuffling if have not seen the target assert target.root in store.blocks diff --git a/test_libs/pyspec/eth2spec/test/fork_choice/test_on_attestation.py b/test_libs/pyspec/eth2spec/test/fork_choice/test_on_attestation.py index 70375ef271..3a513cf2f2 100644 --- a/test_libs/pyspec/eth2spec/test/fork_choice/test_on_attestation.py +++ b/test_libs/pyspec/eth2spec/test/fork_choice/test_on_attestation.py @@ -29,10 +29,9 @@ def run_on_attestation(spec, state, store, attestation, valid=True): @with_all_phases @spec_state_test -def test_on_attestation(spec, state): +def test_on_attestation_current_epoch(spec, state): store = spec.get_genesis_store(state) - time = 100 - spec.on_tick(store, time) + spec.on_tick(store, store.time + spec.SECONDS_PER_SLOT * 2) block = build_empty_block_for_next_slot(spec, state) state_transition_and_sign_block(spec, state, block) @@ -41,9 +40,53 @@ def test_on_attestation(spec, state): spec.on_block(store, block) attestation = get_valid_attestation(spec, state, slot=block.slot) + assert attestation.data.target.epoch == spec.GENESIS_EPOCH + assert spec.compute_epoch_at_slot(spec.get_current_slot(store)) == spec.GENESIS_EPOCH + run_on_attestation(spec, state, store, attestation) +@with_all_phases +@spec_state_test +def test_on_attestation_previous_epoch(spec, state): + store = spec.get_genesis_store(state) + spec.on_tick(store, store.time + spec.SECONDS_PER_SLOT * spec.SLOTS_PER_EPOCH) + + block = build_empty_block_for_next_slot(spec, state) + state_transition_and_sign_block(spec, state, block) + + # store block in store + spec.on_block(store, block) + + attestation = get_valid_attestation(spec, state, slot=block.slot) + assert attestation.data.target.epoch == spec.GENESIS_EPOCH + assert spec.compute_epoch_at_slot(spec.get_current_slot(store)) == spec.GENESIS_EPOCH + 1 + + run_on_attestation(spec, state, store, attestation) + + +@with_all_phases +@spec_state_test +def test_on_attestation_past_epoch(spec, state): + store = spec.get_genesis_store(state) + + # move time forward 2 epochs + time = store.time + 2 * spec.SECONDS_PER_SLOT * spec.SLOTS_PER_EPOCH + spec.on_tick(store, time) + + # create and store block from 3 epochs ago + block = build_empty_block_for_next_slot(spec, state) + state_transition_and_sign_block(spec, state, block) + spec.on_block(store, block) + + # create attestation for past block + attestation = get_valid_attestation(spec, state, slot=state.slot) + assert attestation.data.target.epoch == spec.GENESIS_EPOCH + assert spec.compute_epoch_at_slot(spec.get_current_slot(store)) == spec.GENESIS_EPOCH + 2 + + run_on_attestation(spec, state, store, attestation, False) + + @with_all_phases @spec_state_test def test_on_attestation_target_not_in_store(spec, state): @@ -77,8 +120,7 @@ def test_on_attestation_future_epoch(spec, state): spec.on_block(store, block) # move state forward but not store - attestation_slot = block.slot + spec.SLOTS_PER_EPOCH - state.slot = attestation_slot + state.slot = block.slot + spec.SLOTS_PER_EPOCH attestation = get_valid_attestation(spec, state, slot=state.slot) run_on_attestation(spec, state, store, attestation, False) From a28c02794319d3b7cf6a1c96f541f53b647b151b Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 6 Nov 2019 17:26:06 -0700 Subject: [PATCH 2/3] be explicit about use of genesis epoch for previous epoch in fork choice on_block --- specs/core/0_fork-choice.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/specs/core/0_fork-choice.md b/specs/core/0_fork-choice.md index 34948fbce1..68e681fc7d 100644 --- a/specs/core/0_fork-choice.md +++ b/specs/core/0_fork-choice.md @@ -233,9 +233,11 @@ def on_block(store: Store, block: BeaconBlock) -> None: def on_attestation(store: Store, attestation: Attestation) -> None: target = attestation.data.target - # Attestations must be from the current or previous epoch + # Attestations must be from the current or previous epoch current_epoch = compute_epoch_at_slot(get_current_slot(store)) - assert target.epoch in [current_epoch, current_epoch - 1 if current_epoch > GENESIS_EPOCH else GENESIS_EPOCH] + # 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] # Cannot calculate the current shuffling if have not seen the target assert target.root in store.blocks From 9b21c0db933679eb381aa5ab4105f4445a9607ff Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Thu, 7 Nov 2019 12:06:23 -0700 Subject: [PATCH 3/3] add note aboutgenesis attestations --- specs/validator/0_beacon-chain-validator.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specs/validator/0_beacon-chain-validator.md b/specs/validator/0_beacon-chain-validator.md index 166534031c..fa7fce8ea1 100644 --- a/specs/validator/0_beacon-chain-validator.md +++ b/specs/validator/0_beacon-chain-validator.md @@ -291,6 +291,8 @@ A validator is expected to create, sign, and broadcast an attestation during eac A validator should create and broadcast the `attestation` to the associated attestation subnet one-third of the way through the `slot` during which the validator is assigned―that is, `SECONDS_PER_SLOT / 3` seconds after the start of `slot`. +*Note*: Although attestations during `GENESIS_EPOCH` do not count toward FFG finality, these initial attestations do give weight to the fork choice, are rewarded fork, and should be made. + #### Attestation data First, the validator should construct `attestation_data`, an [`AttestationData`](../core/0_beacon-chain.md#attestationdata) object based upon the state at the assigned slot.