diff --git a/beacon_chain/attestation_pool.nim b/beacon_chain/attestation_pool.nim index f75ab88e3..823206609 100644 --- a/beacon_chain/attestation_pool.nim +++ b/beacon_chain/attestation_pool.nim @@ -67,11 +67,6 @@ proc validate( notice "Empty aggregation bitfield" return false - let crosslink_committee = mapIt( - filterIt(get_crosslink_committees_at_slot(state, attestation.data.slot), - it.shard == attestation.data.shard), - it.committee)[0] - ## the rest; turns into expensive NOP until then. if skipValidation notin flags: let @@ -238,8 +233,9 @@ proc add*(pool: var AttestationPool, attestation: attestation, ) -proc getAttestationsForBlock*(pool: AttestationPool, - newBlockSlot: Slot): seq[Attestation] = +proc getAttestationsForBlock*( + pool: AttestationPool, state: BeaconState, + newBlockSlot: Slot): seq[Attestation] = if newBlockSlot - GENESIS_SLOT < MIN_ATTESTATION_INCLUSION_DELAY: debug "Too early for attestations", newBlockSlot = humaneSlotNum(newBlockSlot) @@ -280,6 +276,15 @@ proc getAttestationsForBlock*(pool: AttestationPool, aggregate_signature: a.validations[0].aggregate_signature ) + # TODO what's going on here is that when producing a block, we need to + # include only such attestations that will not cause block validation + # to fail. How this interacts with voting and the acceptance of + # 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(state, attestation, {skipValidation, nextSlot}): + continue + for v in a.validations[1..^1]: if not attestation.aggregation_bitfield.overlaps( v.aggregation_bitfield): diff --git a/beacon_chain/beacon_node.nim b/beacon_chain/beacon_node.nim index ceddd18cc..6a17b4fde 100644 --- a/beacon_chain/beacon_node.nim +++ b/beacon_chain/beacon_node.nim @@ -340,7 +340,8 @@ proc proposeBlock(node: BeaconNode, blockBody = BeaconBlockBody( randao_reveal: validator.genRandaoReveal(node.state.data, slot), eth1_data: node.mainchainMonitor.getBeaconBlockRef(), - attestations: node.attestationPool.getAttestationsForBlock(slot)) + attestations: + node.attestationPool.getAttestationsForBlock(node.state.data, slot)) var newBlock = BeaconBlock( @@ -387,6 +388,12 @@ proc onAttestation(node: BeaconNode, attestation: Attestation) = attestationData = shortLog(attestation.data), signature = shortLog(attestation.aggregate_signature) + # TODO seems reasonable to use the latest head state here.. needs thinking + # though - maybe we should use the state from the block pointed to by + # the attestation for some of the check? Consider interop with block + # production! + node.blockPool.updateState(node.state, + BlockSlot(blck: node.blockPool.head, slot: node.beaconClock.now().toSlot())) node.attestationPool.add(node.state.data, attestation) proc onBeaconBlock(node: BeaconNode, blck: BeaconBlock) = diff --git a/beacon_chain/extras.nim b/beacon_chain/extras.nim index 01a3382c0..194223bae 100644 --- a/beacon_chain/extras.nim +++ b/beacon_chain/extras.nim @@ -18,5 +18,12 @@ type ## primary use case for this flag is when a proposer must propose a new ## block - in order to do so, it needs to update the state as if the block ## was valid, before it can sign it. + nextSlot ##\ + ## Perform the operation as if the next slot was being processed - this is + ## useful when using the state to verify data that will go in the next slot, + ## for example when proposing + ## TODO need to be careful here, easy to assume that slot number change is + ## enough, vs advancing the state - however, making a full state copy + ## is expensive also :/ UpdateFlags* = set[UpdateFlag] diff --git a/beacon_chain/spec/beaconstate.nim b/beacon_chain/spec/beaconstate.nim index 5fad3fa92..ca9ea9b3c 100644 --- a/beacon_chain/spec/beaconstate.nim +++ b/beacon_chain/spec/beaconstate.nim @@ -490,58 +490,62 @@ func update_validator_registry*(state: var BeaconState) = # https://github.com/ethereum/eth2.0-specs/blob/v0.5.1/specs/core/0_beacon-chain.md#attestations proc checkAttestation*( - state: var BeaconState, attestation: Attestation, flags: UpdateFlags): bool = + state: BeaconState, attestation: Attestation, flags: UpdateFlags): bool = ## 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! + let stateSlot = + if nextSlot in flags: state.slot + 1 + else: state.slot + # Can't submit attestations that are too far in history (or in prehistory) if not (attestation.data.slot >= GENESIS_SLOT): warn("Attestation predates genesis slot", attestation_slot = attestation.data.slot, - state_slot = humaneSlotNum(state.slot)) + state_slot = humaneSlotNum(stateSlot)) return - if not (state.slot <= attestation.data.slot + SLOTS_PER_EPOCH): + if not (stateSlot <= attestation.data.slot + SLOTS_PER_EPOCH): warn("Attestation too old", attestation_slot = humaneSlotNum(attestation.data.slot), - state_slot = humaneSlotNum(state.slot)) + state_slot = humaneSlotNum(stateSlot)) return # Can't submit attestations too quickly if not ( - attestation.data.slot + MIN_ATTESTATION_INCLUSION_DELAY <= state.slot): + attestation.data.slot + MIN_ATTESTATION_INCLUSION_DELAY <= stateSlot): warn("Can't submit attestations too quickly", attestation_slot = humaneSlotNum(attestation.data.slot), - state_slot = humaneSlotNum(state.slot)) + state_slot = humaneSlotNum(stateSlot)) return # # Verify that the justified epoch and root is correct - if slot_to_epoch(attestation.data.slot) >= get_current_epoch(state): + if slot_to_epoch(attestation.data.slot) >= stateSlot.slot_to_epoch(): # Case 1: current epoch attestations if not (attestation.data.source_epoch == state.current_justified_epoch): warn("Source epoch is not current justified epoch", attestation_slot = humaneSlotNum(attestation.data.slot), - state_slot = humaneSlotNum(state.slot)) + state_slot = humaneSlotNum(stateSlot)) return if not (attestation.data.source_root == state.current_justified_root): warn("Source root is not current justified root", attestation_slot = humaneSlotNum(attestation.data.slot), - state_slot = humaneSlotNum(state.slot)) + state_slot = humaneSlotNum(stateSlot)) return else: # Case 2: previous epoch attestations if not (attestation.data.source_epoch == state.previous_justified_epoch): warn("Source epoch is not previous justified epoch", attestation_slot = humaneSlotNum(attestation.data.slot), - state_slot = humaneSlotNum(state.slot)) + state_slot = humaneSlotNum(stateSlot)) return if not (attestation.data.source_root == state.previous_justified_root): warn("Source root is not previous justified root", attestation_slot = humaneSlotNum(attestation.data.slot), - state_slot = humaneSlotNum(state.slot)) + state_slot = humaneSlotNum(stateSlot)) return # Check that the crosslink data is valid @@ -641,20 +645,19 @@ proc makeAttestationData*( ## Head must be the head state during the slot that validator is ## part of committee - notably, it can't be a newer or older state (!) - # TODO update when https://github.com/ethereum/eth2.0-specs/issues/742 let epoch_start_slot = get_epoch_start_slot(slot_to_epoch(state.slot)) - epoch_boundary_root = + target_root = if epoch_start_slot == state.slot: beacon_block_root else: get_block_root(state, epoch_start_slot) - justified_slot = get_epoch_start_slot(state.current_justified_epoch) - justified_block_root = get_block_root(state, justified_slot) AttestationData( slot: state.slot, shard: shard, beacon_block_root: beacon_block_root, + target_root: target_root, crosslink_data_root: Eth2Digest(), # Stub in phase0 previous_crosslink: state.latest_crosslinks[shard], source_epoch: state.current_justified_epoch, + source_root: state.current_justified_root )