From 055d5e018db6095eaf1d36ed6e119cdf71d8db99 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Tue, 28 Apr 2020 11:24:14 +0800 Subject: [PATCH] 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 | 10 +++---- specs/phase1/fraud-proofs.md | 2 +- .../eth2spec/test/helpers/attestations.py | 17 +++++------- .../pyspec/eth2spec/test/helpers/block.py | 27 ++++++++++++------- .../eth2spec/test/helpers/shard_block.py | 8 +++--- .../test_process_crosslink.py | 4 +-- 6 files changed, 37 insertions(+), 31 deletions(-) diff --git a/specs/phase1/beacon-chain.md b/specs/phase1/beacon-chain.md index fc9687545a..b7ab772453 100644 --- a/specs/phase1/beacon-chain.md +++ b/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) diff --git a/specs/phase1/fraud-proofs.md b/specs/phase1/fraud-proofs.md index a5068cd07a..a61c282aac 100644 --- a/specs/phase1/fraud-proofs.md +++ b/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. diff --git a/tests/core/pyspec/eth2spec/test/helpers/attestations.py b/tests/core/pyspec/eth2spec/test/helpers/attestations.py index c203b737a5..86544b3de4 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/attestations.py +++ b/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) diff --git a/tests/core/pyspec/eth2spec/test/helpers/block.py b/tests/core/pyspec/eth2spec/test/helpers/block.py index 96cc30e35d..e40d5e7d8d 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/block.py +++ b/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 diff --git a/tests/core/pyspec/eth2spec/test/helpers/shard_block.py b/tests/core/pyspec/eth2spec/test/helpers/shard_block.py index 25d868b65e..daf2a7574c 100644 --- a/tests/core/pyspec/eth2spec/test/helpers/shard_block.py +++ b/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, diff --git a/tests/core/pyspec/eth2spec/test/phase_1/block_processing/test_process_crosslink.py b/tests/core/pyspec/eth2spec/test/phase_1/block_processing/test_process_crosslink.py index 6d1e65678b..33e9b32e33 100644 --- a/tests/core/pyspec/eth2spec/test/phase_1/block_processing/test_process_crosslink.py +++ b/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]