From 8a5e5334d6ffca9d01d3aaace0ed04786a2f36f5 Mon Sep 17 00:00:00 2001 From: Dustin Brody Date: Tue, 2 Jul 2019 23:14:55 +0200 Subject: [PATCH] re-implement attestation checking in 0.8.0 form and rename relevant function from checkAttestation(...) to spec's process_attestation(...), though there's a slight difference in how they're factored; remove a couple of pointless type conversion warnings; rename validate_indexed_attestation(...) to is_valid_indexed_attestation(...); remove verify_bitfield(...) not in 0.8.0; update AttestationData to 0.8.0 by using Checkpoint data structure; rename MAX_CROSSLINK_EPOCHS to MAX_EPOCHS_PER_CROSSLINK --- beacon_chain/attestation_pool.nim | 2 +- beacon_chain/spec/beaconstate.nim | 126 +++++++++++-------- beacon_chain/spec/bitfield.nim | 13 -- beacon_chain/spec/datatypes.nim | 12 +- beacon_chain/spec/presets/mainnet.nim | 2 +- beacon_chain/spec/presets/minimal.nim | 2 +- beacon_chain/spec/state_transition_block.nim | 16 +-- beacon_chain/spec/state_transition_epoch.nim | 2 +- 8 files changed, 89 insertions(+), 86 deletions(-) diff --git a/beacon_chain/attestation_pool.nim b/beacon_chain/attestation_pool.nim index e873b94f7..f3f7c36bc 100644 --- a/beacon_chain/attestation_pool.nim +++ b/beacon_chain/attestation_pool.nim @@ -297,7 +297,7 @@ proc getAttestationsForBlock*( # attestations into the pool in general is an open question that needs # revisiting - for example, when attestations are added, against which # state should they be validated, if at all? - if not checkAttestation( + if not process_attestation( state, attestation, {skipValidation, nextSlot}, cache): continue diff --git a/beacon_chain/spec/beaconstate.nim b/beacon_chain/spec/beaconstate.nim index 4e75fa281..991b522cc 100644 --- a/beacon_chain/spec/beaconstate.nim +++ b/beacon_chain/spec/beaconstate.nim @@ -146,7 +146,7 @@ func initiate_validator_exit*(state: var BeaconState, # Set validator exit epoch and withdrawable epoch validator.exit_epoch = exit_queue_epoch validator.withdrawable_epoch = - (validator.exit_epoch + MIN_VALIDATOR_WITHDRAWABILITY_DELAY).Epoch + validator.exit_epoch + MIN_VALIDATOR_WITHDRAWABILITY_DELAY # https://github.com/ethereum/eth2.0-specs/blob/v0.7.1/specs/core/0_beacon-chain.md#slash_validator func slash_validator*(state: var BeaconState, slashed_index: ValidatorIndex, @@ -259,16 +259,16 @@ func get_attestation_data_slot*(state: BeaconState, # Return the slot corresponding to the attestation ``data``. let offset = (data.crosslink.shard + SHARD_COUNT - - get_start_shard(state, data.target_epoch)) mod SHARD_COUNT + get_start_shard(state, data.target.epoch)) mod SHARD_COUNT - (compute_start_slot_of_epoch(data.target_epoch) + offset div - (committee_count div SLOTS_PER_EPOCH)).Slot + compute_start_slot_of_epoch(data.target.epoch) + offset div + (committee_count div SLOTS_PER_EPOCH) # This is the slower (O(n)), spec-compatible signature. func get_attestation_data_slot*(state: BeaconState, data: AttestationData): Slot = get_attestation_data_slot( - state, data, get_committee_count(state, data.target_epoch)) + state, data, get_committee_count(state, data.target.epoch)) # https://github.com/ethereum/eth2.0-specs/blob/v0.8.0/specs/core/0_beacon-chain.md#get_block_root_at_slot func get_block_root_at_slot*(state: BeaconState, @@ -332,8 +332,8 @@ func process_registry_updates*(state: var BeaconState) = validator.activation_epoch = get_delayed_activation_exit_epoch(get_current_epoch(state)) -# https://github.com/ethereum/eth2.0-specs/blob/v0.7.1/specs/core/0_beacon-chain.md#validate_indexed_attestation -func validate_indexed_attestation*( +# https://github.com/ethereum/eth2.0-specs/blob/v0.7.1/specs/core/0_beacon-chain.md#is_valid_indexed_attestation +func is_valid_indexed_attestation*( state: BeaconState, indexed_attestation: IndexedAttestation): bool = # Verify validity of ``indexed_attestation`` fields. @@ -379,7 +379,7 @@ func validate_indexed_attestation*( get_domain( state, DOMAIN_ATTESTATION, - indexed_attestation.data.target_epoch + indexed_attestation.data.target.epoch ), ) @@ -398,9 +398,8 @@ func get_attesting_indices*(state: BeaconState, result = initSet[ValidatorIndex]() let committee = get_crosslink_committee( - state, attestation_data.target_epoch, attestation_data.crosslink.shard, + state, attestation_data.target.epoch, attestation_data.crosslink.shard, stateCache) - doAssert verify_bitfield(bitfield, len(committee)) for i, index in committee: if get_bitfield_bit(bitfield, i): result.incl index @@ -462,11 +461,10 @@ func get_indexed_attestation(state: BeaconState, attestation: Attestation, signature: attestation.signature, ) -# https://github.com/ethereum/eth2.0-specs/blob/v0.6.3/specs/core/0_beacon-chain.md#attestations -proc checkAttestation*( +# https://github.com/ethereum/eth2.0-specs/blob/v0.8.0/specs/core/0_beacon-chain.md#attestations +proc process_attestation*( state: BeaconState, attestation: Attestation, flags: UpdateFlags, stateCache: var StateCache): bool = - ## Process ``Attestation`` operation. ## Check that an attestation follows the rules of being included in the state ## at the current slot. When acting as a proposer, the same rules need to ## be followed! @@ -475,9 +473,19 @@ proc checkAttestation*( if nextSlot in flags: state.slot + 1 else: state.slot - let - data = attestation.data - attestation_slot = get_attestation_data_slot(state, attestation.data) + let data = attestation.data + + if not (data.crosslink.shard < SHARD_COUNT): + warn("Attestation shard too high", + attestation_shard = data.crosslink.shard) + return + + if not (data.target.epoch == get_previous_epoch(state) or + data.target.epoch == get_current_epoch(state)): + warn("Target epoch not current or previous epoch") + return + + let attestation_slot = get_attestation_data_slot(state, attestation.data) if not (attestation_slot + MIN_ATTESTATION_INCLUSION_DELAY <= stateSlot): warn("Attestation too new", @@ -498,25 +506,19 @@ proc checkAttestation*( proposer_index: get_beacon_proposer_index(state, stateCache), ) - # Check target epoch, source epoch, source root, and source crosslink - if not (data.target_epoch == get_previous_epoch(state) or - data.target_epoch == get_current_epoch(state)): - warn("Target epoch not current or previous epoch") - return - # Check FFG data, crosslink data, and signature - let ffg_check_data = (data.source_epoch, data.source_root, data.target_epoch) + let ffg_check_data = (data.source.epoch, data.source.root, data.target.epoch) - if data.target_epoch == get_current_epoch(state): + if data.target.epoch == get_current_epoch(state): if not (ffg_check_data == (state.current_justified_epoch, state.current_justified_root, get_current_epoch(state))): warn("FFG data not matching current justified epoch") return - #if not (data.crosslink.parent_root == - # hash_tree_root(state.current_crosslinks[data.crosslink.shard])): - # warn("Crosslink shard's current crosslinks not matching crosslink parent root") - # return + if not (data.crosslink.parent_root == + hash_tree_root(state.current_crosslinks[data.crosslink.shard])): + warn("Crosslink shard's current crosslinks not matching crosslink parent root") + return #state.current_epoch_attestations.add(pending_attestation) else: @@ -525,39 +527,49 @@ proc checkAttestation*( warn("FFG data not matching current justified epoch") return - #if not (data.crosslink.parent_root == - # hash_tree_root(state.previous_crosslinks[data.crosslink.shard])): - # warn("Crosslink shard's previous crosslinks not matching crosslink parent root") - # return + if not (data.crosslink.parent_root == + hash_tree_root(state.previous_crosslinks[data.crosslink.shard])): + warn("Crosslink shard's previous crosslinks not matching crosslink parent root") + return #state.previous_epoch_attestations.add(pending_attestation) - # TODO un-comment when changed Crosslink to 0.7 structure - #if not (data.crosslink.start_epoch == parent_crosslink.end_epoch): - # warn("Crosslink start and end epochs not the same") - # return + let parent_crosslink = if data.target.epoch == get_current_epoch(state): + state.current_crosslinks[data.crosslink.shard] + else: + state.previous_crosslinks[data.crosslink.shard] - #if not (data.crosslink.end_epoch == min(data.target_epoch, parent_crosslink.end_epoch + MAX_EPOCHS_PER_CROSSLINK)): - # warn("Crosslink end epoch incorrect") - # return + if not (data.crosslink.parent_root == hash_tree_root(parent_crosslink)): + warn("Crosslink parent root doesn't match parent crosslink's root") + return - # Simlarly, these depend on 0.7 data structures - #assert data.crosslink.parent_root == hash_tree_root(parent_crosslink) + if not (data.crosslink.start_epoch == parent_crosslink.end_epoch): + warn("Crosslink start and end epochs not the same") + return - #if not (data.crosslink.data_root == ZERO_HASH): # [to be removed in phase 1] - # warn("Crosslink data root not zero") - # return + if not (data.crosslink.end_epoch == min( + data.target.epoch, + parent_crosslink.end_epoch + MAX_EPOCHS_PER_CROSSLINK)): + warn("Crosslink end epoch incorrect", + crosslink_end_epoch = data.crosslink.end_epoch, + parent_crosslink_end_epoch = parent_crosslink.end_epoch, + target_epoch = data.target.epoch) + return + + if not (data.crosslink.data_root == ZERO_HASH): # [to be removed in phase 1] + warn("Crosslink data root not zero") + return # Check signature and bitfields - if not validate_indexed_attestation( + if not is_valid_indexed_attestation( state, get_indexed_attestation(state, attestation, stateCache)): - warn("checkAttestation: signature or bitfields incorrect") + warn("process_attestation: signature or bitfields incorrect") return true proc makeAttestationData*( - state: BeaconState, shard: uint64, + state: BeaconState, shard_offset: uint64, beacon_block_root: Eth2Digest): AttestationData = ## Fine points: ## Head must be the head state during the slot that validator is @@ -568,17 +580,23 @@ proc makeAttestationData*( target_root = if epoch_start_slot == state.slot: beacon_block_root else: get_block_root_at_slot(state, epoch_start_slot) + shard = (shard_offset + get_start_shard(state, + compute_epoch_of_slot(state.slot))) mod SHARD_COUNT + target_epoch = compute_epoch_of_slot(state.slot) AttestationData( beacon_block_root: beacon_block_root, - target_root: target_root, - source_epoch: state.current_justified_epoch, - source_root: state.current_justified_root, - target_epoch: compute_epoch_of_slot(state.slot), + source: Checkpoint( + epoch: state.current_justified_epoch, + root: state.current_justified_root + ), + target: Checkpoint( + root: target_root, + epoch: target_epoch + ), crosslink: Crosslink( - # Alternative is to put this offset into all callers - shard: shard + get_start_shard(state, compute_epoch_of_slot(state.slot)), + shard: shard, parent_root: hash_tree_root(state.current_crosslinks[shard]), - data_root: Eth2Digest(), # Stub in phase0 + end_epoch: target_epoch, ) ) diff --git a/beacon_chain/spec/bitfield.nim b/beacon_chain/spec/bitfield.nim index 906f1db12..963590d25 100644 --- a/beacon_chain/spec/bitfield.nim +++ b/beacon_chain/spec/bitfield.nim @@ -24,19 +24,6 @@ func get_bitfield_bit*(bitfield: BitField, i: int): bool = doAssert i div 8 < bitfield.bits.len, "i: " & $i & " i div 8: " & $(i div 8) ((bitfield.bits[i div 8] shr (i mod 8)) mod 2) > 0'u8 -# https://github.com/ethereum/eth2.0-specs/blob/v0.7.1/specs/core/0_beacon-chain.md#verify_bitfield -func verify_bitfield*(bitfield: BitField, committee_size: int): bool = - # Verify ``bitfield`` against the ``committee_size``. - if len(bitfield.bits) != (committee_size + 7) div 8: - return false - - # Check `bitfield` is padded with zero bits only - for i in committee_size ..< (len(bitfield.bits) * 8): - if get_bitfield_bit(bitfield, i): - return false - - true - # TODO spec candidatidates below, though they're used only indirectly there.. func set_bitfield_bit*(bitfield: var BitField, i: int) = bitfield.bits[i div 8] = bitfield.bits[i div 8] or 1'u8 shl (i mod 8) diff --git a/beacon_chain/spec/datatypes.nim b/beacon_chain/spec/datatypes.nim index ca0a7e08d..17f1d6598 100644 --- a/beacon_chain/spec/datatypes.nim +++ b/beacon_chain/spec/datatypes.nim @@ -123,16 +123,14 @@ type epoch*: Epoch root*: Eth2Digest - # https://github.com/ethereum/eth2.0-specs/blob/v0.7.1/specs/core/0_beacon-chain.md#attestationdata + # https://github.com/ethereum/eth2.0-specs/blob/v0.8.0/specs/core/0_beacon-chain.md#AttestationData AttestationData* = object # LMD GHOST vote beacon_block_root*: Eth2Digest # FFG vote - source_epoch*: Epoch - source_root*: Eth2Digest - target_epoch*: Epoch - target_root*: Eth2Digest + source*: Checkpoint + target*: Checkpoint # Crosslink vote crosslink*: Crosslink @@ -456,8 +454,8 @@ func shortLog*(v: BeaconBlock): tuple[ func shortLog*(v: AttestationData): auto = ( shortLog(v.beacon_block_root), - humaneEpochNum(v.source_epoch), shortLog(v.target_root), - shortLog(v.source_root), + humaneEpochNum(v.source.epoch), shortLog(v.target.root), + shortLog(v.source.root), v.crosslink ) diff --git a/beacon_chain/spec/presets/mainnet.nim b/beacon_chain/spec/presets/mainnet.nim index cba3f0733..241c7f1a6 100644 --- a/beacon_chain/spec/presets/mainnet.nim +++ b/beacon_chain/spec/presets/mainnet.nim @@ -128,7 +128,7 @@ const PERSISTENT_COMMITTEE_PERIOD* = 2'u64^11 ##\ ## epochs (9 days) - MAX_CROSSLINK_EPOCHS* = 2'u64^6 ##\ + MAX_EPOCHS_PER_CROSSLINK* = 2'u64^6 ##\ ## epochs (~7 hours) MIN_EPOCHS_TO_INACTIVITY_PENALTY* = 2'u64^2 ##\ diff --git a/beacon_chain/spec/presets/minimal.nim b/beacon_chain/spec/presets/minimal.nim index 4364d9773..600377dfb 100644 --- a/beacon_chain/spec/presets/minimal.nim +++ b/beacon_chain/spec/presets/minimal.nim @@ -93,7 +93,7 @@ const # Unchanged MIN_VALIDATOR_WITHDRAWABILITY_DELAY* = 2'u64^8 PERSISTENT_COMMITTEE_PERIOD* = 2'u64^11 - MAX_CROSSLINK_EPOCHS* = 2'u64^6 + MAX_EPOCHS_PER_CROSSLINK* = 2'u64^6 MIN_EPOCHS_TO_INACTIVITY_PENALTY* = 2'u64^2 # State list lengths diff --git a/beacon_chain/spec/state_transition_block.nim b/beacon_chain/spec/state_transition_block.nim index 44e3ea96d..7c22b6265 100644 --- a/beacon_chain/spec/state_transition_block.nim +++ b/beacon_chain/spec/state_transition_block.nim @@ -184,10 +184,10 @@ func is_slashable_attestation_data( ## rules. # Double vote - (data_1 != data_2 and data_1.target_epoch == data_2.target_epoch) or + (data_1 != data_2 and data_1.target.epoch == data_2.target.epoch) or # Surround vote - (data_1.source_epoch < data_2.source_epoch and - data_2.target_epoch < data_1.target_epoch) + (data_1.source.epoch < data_2.source.epoch and + data_2.target.epoch < data_1.target.epoch) # https://github.com/ethereum/eth2.0-specs/blob/v0.7.1/specs/core/0_beacon-chain.md#attester-slashings proc processAttesterSlashings(state: var BeaconState, blck: BeaconBlock, @@ -208,11 +208,11 @@ proc processAttesterSlashings(state: var BeaconState, blck: BeaconBlock, notice "CaspSlash: surround or double vote check failed" return false - if not validate_indexed_attestation(state, attestation_1): + if not is_valid_indexed_attestation(state, attestation_1): notice "CaspSlash: invalid votes 1" return false - if not validate_indexed_attestation(state, attestation_2): + if not is_valid_indexed_attestation(state, attestation_2): notice "CaspSlash: invalid votes 2" return false @@ -247,7 +247,7 @@ proc processAttestations( notice "Attestation: too many!", attestations = blck.body.attestations.len return false - if not blck.body.attestations.allIt(checkAttestation(state, it, flags, stateCache)): + if not blck.body.attestations.allIt(process_attestation(state, it, flags, stateCache)): return false # All checks passed - update state @@ -257,7 +257,7 @@ proc processAttestations( for attestation in blck.body.attestations: # Caching let - epoch = attestation.data.target_epoch + epoch = attestation.data.target.epoch committee_count = if epoch in committee_count_cache: committee_count_cache[epoch] else: @@ -274,7 +274,7 @@ proc processAttestations( proposer_index: get_beacon_proposer_index(state, stateCache), ) - if attestation.data.target_epoch == get_current_epoch(state): + if attestation.data.target.epoch == get_current_epoch(state): state.current_epoch_attestations.add(pending_attestation) else: state.previous_epoch_attestations.add(pending_attestation) diff --git a/beacon_chain/spec/state_transition_epoch.nim b/beacon_chain/spec/state_transition_epoch.nim index 1938058c3..fcd6fb7e9 100644 --- a/beacon_chain/spec/state_transition_epoch.nim +++ b/beacon_chain/spec/state_transition_epoch.nim @@ -55,7 +55,7 @@ func get_matching_target_attestations(state: BeaconState, epoch: Epoch): seq[PendingAttestation] = filterIt( get_matching_source_attestations(state, epoch), - it.data.target_root == get_block_root(state, epoch) + it.data.target.root == get_block_root(state, epoch) ) func get_matching_head_attestations(state: BeaconState, epoch: Epoch):