Fix build_attestation_data and other PR feedback from Danny and

Terence

1. Rename `get_previous_slot` to `compute_previous_slot`
2. Break down `build_empty_block` into
`get_state_and_beacon_parent_root_at_slot`, use it in
`build_shard_block`
3. Set defult `slot` to `shard_state.slot + 1` in `build_shard_block`

specs/phase1/beacon-chain.md

@@ -33,7 +33,7 @@
   - [`AttestationCustodyBitWrapper`](#attestationcustodybitwrapper)
 - [Helper functions](#helper-functions)
   - [Misc](#misc-1)
-    - [`get_previous_slot`](#get_previous_slot)
+    - [`compute_previous_slot`](#compute_previous_slot)
     - [`pack_compact_validator`](#pack_compact_validator)
     - [`unpack_compact_validator`](#unpack_compact_validator)
     - [`committee_to_compact_committee`](#committee_to_compact_committee)
@@ -369,10 +369,10 @@ class AttestationCustodyBitWrapper(Container):
 
 ### Misc
 
-#### `get_previous_slot`
+#### `compute_previous_slot`
 
 ```python
-def get_previous_slot(slot: Slot) -> Slot:
+def compute_previous_slot(slot: Slot) -> Slot:
     if slot > 0:
         return Slot(slot - 1)
     else:
@@ -684,7 +684,7 @@ def validate_attestation(state: BeaconState, attestation: Attestation) -> None:
         # Correct data root count
         assert len(attestation.custody_bits_blocks) == len(get_offset_slots(state, shard))
         # Correct parent block root
-        assert data.beacon_block_root == get_block_root_at_slot(state, get_previous_slot(state.slot))
+        assert data.beacon_block_root == get_block_root_at_slot(state, compute_previous_slot(state.slot))
     # Type 2: no shard transition, no custody bits
     else:
         # Ensure delayed attestation
@@ -920,7 +920,7 @@ def process_light_client_signatures(state: BeaconState, block_body: BeaconBlockB
 
     increase_balance(state, get_beacon_proposer_index(state), Gwei(total_reward // PROPOSER_REWARD_QUOTIENT))
 
-    slot = get_previous_slot(state.slot)
+    slot = compute_previous_slot(state.slot)
     signing_root = compute_signing_root(get_block_root_at_slot(state, slot),
                                         get_domain(state, DOMAIN_LIGHT_CLIENT, compute_epoch_at_slot(slot)))
     assert bls.FastAggregateVerify(signer_pubkeys, signing_root, signature=block_body.light_client_signature)

specs/phase1/fraud-proofs.md

@@ -124,7 +124,6 @@ def is_valid_fraud_proof(beacon_state: BeaconState,
                          subkey: BLSPubkey,
                          beacon_parent_block: BeaconBlock) -> bool:
     # 1. Check if `custody_bits[offset_index][j] != generate_custody_bit(subkey, block_contents)` for any `j`.
-    shard = get_shard(beacon_state, attestation)
     custody_bits = attestation.custody_bits_blocks
     for j in range(custody_bits[offset_index]):
         if custody_bits[offset_index][j] != generate_custody_bit(subkey, block):
@@ -133,6 +132,7 @@ def is_valid_fraud_proof(beacon_state: BeaconState,
     # 2. Check if the shard state transition result is wrong between
     # `transition.shard_states[offset_index - 1]` to `transition.shard_states[offset_index]`.
     if offset_index == 0:
+        shard = get_shard(beacon_state, attestation)
         shard_state = beacon_parent_block.shard_transitions[shard].shard_states[-1]
     else:
         shard_state = transition.shard_states[offset_index - 1]  # Not doing the actual state updates here.

tests/core/pyspec/eth2spec/test/helpers/attestations.py

@@ -66,23 +66,20 @@ def build_attestation_data(spec, state, slot, index, shard_transition=None):
         source_epoch = state.current_justified_checkpoint.epoch
         source_root = state.current_justified_checkpoint.root
 
-    if spec.fork == PHASE1 and shard_transition is not None:
-        shard_transition_root = shard_transition.hash_tree_root()
-        head_shard_root = shard_transition.shard_data_roots[len(shard_transition.shard_data_roots) - 1]
-    else:
-        shard_transition_root = spec.Root()
-        head_shard_root = spec.Root()
-
-    return spec.AttestationData(
+    attestation_data = spec.AttestationData(
         slot=slot,
         index=index,
         beacon_block_root=block_root,
         source=spec.Checkpoint(epoch=source_epoch, root=source_root),
         target=spec.Checkpoint(epoch=spec.compute_epoch_at_slot(slot), root=epoch_boundary_root),
-        head_shard_root=head_shard_root,
-        shard_transition_root=shard_transition_root,
     )
 
+    if spec.fork == PHASE1 and shard_transition is not None:
+        lastest_shard_data_root_index = len(shard_transition.shard_data_roots) - 1
+        attestation_data.head_shard_root = shard_transition.shard_data_roots[lastest_shard_data_root_index]
+        attestation_data.shard_transition_root = shard_transition.hash_tree_root()
+    return attestation_data
+
 
 def convert_to_valid_on_time_attestation(spec, state, attestation, signed=False):
     shard = spec.get_shard(state, attestation)

tests/core/pyspec/eth2spec/test/helpers/block.py

@@ -71,24 +71,31 @@ def build_empty_block(spec, state, slot=None):
     """
     if slot is None:
         slot = state.slot
-    if slot < state.slot:
-        raise Exception("build_empty_block cannot build blocks for past slots")
-    if slot > state.slot:
-        # transition forward in copied state to grab relevant data from state
-        state = state.copy()
-        spec.process_slots(state, slot)
 
+    state, parent_block_root = get_state_and_beacon_parent_root_at_slot(spec, state, slot)
     empty_block = spec.BeaconBlock()
     empty_block.slot = slot
     empty_block.proposer_index = spec.get_beacon_proposer_index(state)
     empty_block.body.eth1_data.deposit_count = state.eth1_deposit_index
-    previous_block_header = state.latest_block_header.copy()
-    if previous_block_header.state_root == spec.Root():
-        previous_block_header.state_root = hash_tree_root(state)
-    empty_block.parent_root = hash_tree_root(previous_block_header)
+    empty_block.parent_root = parent_block_root
     apply_randao_reveal(spec, state, empty_block)
     return empty_block
 
 
 def build_empty_block_for_next_slot(spec, state):
     return build_empty_block(spec, state, state.slot + 1)
+
+
+def get_state_and_beacon_parent_root_at_slot(spec, state, slot):
+    if slot < state.slot:
+        raise Exception("Cannot build blocks for past slots")
+    if slot > state.slot:
+        # transition forward in copied state to grab relevant data from state
+        state = state.copy()
+        spec.process_slots(state, slot)
+
+    previous_block_header = state.latest_block_header.copy()
+    if previous_block_header.state_root == spec.Root():
+        previous_block_header.state_root = hash_tree_root(state)
+    beacon_parent_root = hash_tree_root(previous_block_header)
+    return state, beacon_parent_root

tests/core/pyspec/eth2spec/test/helpers/shard_block.py

@@ -1,3 +1,4 @@
+from eth2spec.test.helpers.block import get_state_and_beacon_parent_root_at_slot
 from eth2spec.test.helpers.keys import privkeys
 from eth2spec.utils import bls
 from eth2spec.utils.bls import only_with_bls
@@ -18,21 +19,22 @@ def sign_shard_block(spec, beacon_state, shard, block, proposer_index=None):
 def build_shard_block(spec,
                       beacon_state,
                       shard,
-                      slot,
+                      slot=None,
                       body=None,
                       signed=False):
     shard_state = beacon_state.shard_states[shard]
     if slot is None:
-        slot = shard_state.slot
+        slot = shard_state.slot + 1
 
     if body is None:
         body = []
 
     proposer_index = spec.get_shard_proposer_index(beacon_state, slot, shard)
+    beacon_state, beacon_parent_root = get_state_and_beacon_parent_root_at_slot(spec, beacon_state, slot)
 
     block = spec.ShardBlock(
         shard_parent_root=shard_state.latest_block_root,
-        beacon_parent_root=spec.get_block_root_at_slot(beacon_state, spec.get_previous_slot(slot)),
+        beacon_parent_root=beacon_parent_root,
         slot=slot,
         proposer_index=proposer_index,
         body=body,

tests/core/pyspec/eth2spec/test/phase_1/block_processing/test_process_crosslink.py

@@ -24,7 +24,7 @@ def test_basic_crosslinks(spec, state):
 
     committee_index = spec.CommitteeIndex(0)
     shard = spec.compute_shard_from_committee_index(state, committee_index, state.slot)
-    body = b'1' * spec.MAX_SHARD_BLOCK_SIZE
+    body = b'\x56' * spec.MAX_SHARD_BLOCK_SIZE
     shard_block = build_shard_block(spec, state, shard, body=body, slot=state.slot, signed=True)
     shard_blocks = [shard_block]
 
@@ -68,7 +68,7 @@ def test_multiple_offset_slots(spec, state):
 
     committee_index = spec.CommitteeIndex(0)
     shard = spec.compute_shard_from_committee_index(state, committee_index, state.slot)
-    body = b'1' * spec.MAX_SHARD_BLOCK_SIZE
+    body = b'\x56' * spec.MAX_SHARD_BLOCK_SIZE
     shard_block = build_shard_block(spec, state, shard, body=body, slot=state.slot, signed=True)
     shard_blocks = [shard_block]