From 65b615a4d4cf75a50b29d25c53f1bc5422770ae5 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Fri, 1 Nov 2019 21:02:53 -0600 Subject: [PATCH] remove custody_bits from attestation --- specs/core/0_beacon-chain.md | 33 +++-------- specs/core/0_fork-choice.md | 2 +- .../test/fork_choice/test_on_attestation.py | 13 ++-- .../eth2spec/test/helpers/attestations.py | 4 +- .../test_process_attestation.py | 58 ------------------ .../test_process_attester_slashing.py | 59 +++++-------------- .../test_process_rewards_and_penalties.py | 2 +- .../eth2spec/test/sanity/test_blocks.py | 3 +- 8 files changed, 35 insertions(+), 139 deletions(-) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index 9f0f21435..632c4963a 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -320,8 +320,7 @@ class AttestationDataAndCustodyBit(Container): ```python class IndexedAttestation(Container): - custody_bit_0_indices: List[ValidatorIndex, MAX_VALIDATORS_PER_COMMITTEE] # Indices with custody bit equal to 0 - custody_bit_1_indices: List[ValidatorIndex, MAX_VALIDATORS_PER_COMMITTEE] # Indices with custody bit equal to 1 + attesting_indices: List[ValidatorIndex, MAX_VALIDATORS_PER_COMMITTEE] data: AttestationData signature: BLSSignature ``` @@ -399,7 +398,6 @@ class AttesterSlashing(Container): class Attestation(Container): aggregation_bits: Bitlist[MAX_VALIDATORS_PER_COMMITTEE] data: AttestationData - custody_bits: Bitlist[MAX_VALIDATORS_PER_COMMITTEE] signature: BLSSignature ``` @@ -605,30 +603,21 @@ def is_valid_indexed_attestation(state: BeaconState, indexed_attestation: Indexe """ Check if ``indexed_attestation`` has valid indices and signature. """ - bit_0_indices = indexed_attestation.custody_bit_0_indices - bit_1_indices = indexed_attestation.custody_bit_1_indices + indices = indexed_attestation.attesting_indices - # Verify no index has custody bit equal to 1 [to be removed in phase 1] - if not len(bit_1_indices) == 0: # [to be removed in phase 1] - return False # [to be removed in phase 1] # Verify max number of indices - if not len(bit_0_indices) + len(bit_1_indices) <= MAX_VALIDATORS_PER_COMMITTEE: - return False - # Verify index sets are disjoint - if not len(set(bit_0_indices).intersection(bit_1_indices)) == 0: + if not len(indices) <= MAX_VALIDATORS_PER_COMMITTEE: return False # Verify indices are sorted - if not (bit_0_indices == sorted(bit_0_indices) and bit_1_indices == sorted(bit_1_indices)): + if not indices == sorted(indices): return False # Verify aggregate signature if not bls_verify_multiple( pubkeys=[ - bls_aggregate_pubkeys([state.validators[i].pubkey for i in bit_0_indices]), - bls_aggregate_pubkeys([state.validators[i].pubkey for i in bit_1_indices]), + bls_aggregate_pubkeys([state.validators[i].pubkey for i in indices]), ], message_hashes=[ hash_tree_root(AttestationDataAndCustodyBit(data=indexed_attestation.data, custody_bit=0b0)), - hash_tree_root(AttestationDataAndCustodyBit(data=indexed_attestation.data, custody_bit=0b1)), ], signature=indexed_attestation.signature, domain=get_domain(state, DOMAIN_BEACON_ATTESTER, indexed_attestation.data.target.epoch), @@ -922,13 +911,9 @@ def get_indexed_attestation(state: BeaconState, attestation: Attestation) -> Ind Return the indexed attestation corresponding to ``attestation``. """ attesting_indices = get_attesting_indices(state, attestation.data, attestation.aggregation_bits) - custody_bit_1_indices = get_attesting_indices(state, attestation.data, attestation.custody_bits) - assert custody_bit_1_indices.issubset(attesting_indices) - custody_bit_0_indices = attesting_indices.difference(custody_bit_1_indices) return IndexedAttestation( - custody_bit_0_indices=sorted(custody_bit_0_indices), - custody_bit_1_indices=sorted(custody_bit_1_indices), + attesting_indices=sorted(attesting_indices), data=attestation.data, signature=attestation.signature, ) @@ -1460,8 +1445,8 @@ def process_attester_slashing(state: BeaconState, attester_slashing: AttesterSla assert is_valid_indexed_attestation(state, attestation_2) slashed_any = False - attesting_indices_1 = attestation_1.custody_bit_0_indices + attestation_1.custody_bit_1_indices - attesting_indices_2 = attestation_2.custody_bit_0_indices + attestation_2.custody_bit_1_indices + attesting_indices_1 = attestation_1.attesting_indices + attesting_indices_2 = attestation_2.attesting_indices for index in sorted(set(attesting_indices_1).intersection(attesting_indices_2)): if is_slashable_validator(state.validators[index], get_current_epoch(state)): slash_validator(state, index) @@ -1479,7 +1464,7 @@ def process_attestation(state: BeaconState, attestation: Attestation) -> None: assert data.slot + MIN_ATTESTATION_INCLUSION_DELAY <= state.slot <= data.slot + SLOTS_PER_EPOCH committee = get_beacon_committee(state, data.slot, data.index) - assert len(attestation.aggregation_bits) == len(attestation.custody_bits) == len(committee) + assert len(attestation.aggregation_bits) == len(committee) pending_attestation = PendingAttestation( data=data, diff --git a/specs/core/0_fork-choice.md b/specs/core/0_fork-choice.md index b909ce732..bf9688db7 100644 --- a/specs/core/0_fork-choice.md +++ b/specs/core/0_fork-choice.md @@ -199,7 +199,7 @@ def on_attestation(store: Store, attestation: Attestation) -> None: assert is_valid_indexed_attestation(target_state, indexed_attestation) # Update latest messages - for i in indexed_attestation.custody_bit_0_indices + indexed_attestation.custody_bit_1_indices: + for i in indexed_attestation.attesting_indices: if i not in store.latest_messages or target.epoch > store.latest_messages[i].epoch: store.latest_messages[i] = LatestMessage(epoch=target.epoch, root=attestation.data.beacon_block_root) ``` 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 70375ef27..39c179b59 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 @@ -1,7 +1,5 @@ -from eth2spec.test.context import with_all_phases, spec_state_test, with_phases - - +from eth2spec.test.context import with_all_phases, spec_state_test from eth2spec.test.helpers.block import build_empty_block_for_next_slot from eth2spec.test.helpers.attestations import get_valid_attestation from eth2spec.test.helpers.state import state_transition_and_sign_block @@ -19,7 +17,7 @@ def run_on_attestation(spec, state, store, attestation, valid=True): indexed_attestation = spec.get_indexed_attestation(state, attestation) spec.on_attestation(store, attestation) assert ( - store.latest_messages[indexed_attestation.custody_bit_0_indices[0]] == + store.latest_messages[indexed_attestation.attesting_indices[0]] == spec.LatestMessage( epoch=attestation.data.target.epoch, root=attestation.data.beacon_block_root, @@ -100,7 +98,7 @@ def test_on_attestation_same_slot(spec, state): run_on_attestation(spec, state, store, attestation, False) -@with_phases(['phase0']) +@with_all_phases @spec_state_test def test_on_attestation_invalid_attestation(spec, state): store = spec.get_genesis_store(state) @@ -113,6 +111,7 @@ def test_on_attestation_invalid_attestation(spec, state): spec.on_block(store, block) attestation = get_valid_attestation(spec, state, slot=block.slot) - # make attestation invalid by setting a phase1-only custody bit - attestation.custody_bits[0] = 1 + # make invalid by using an invalid committee index + attestation.data.index = spec.MAX_COMMITTEES_PER_SLOT * spec.SLOTS_PER_EPOCH + run_on_attestation(spec, state, store, attestation, False) diff --git a/test_libs/pyspec/eth2spec/test/helpers/attestations.py b/test_libs/pyspec/eth2spec/test/helpers/attestations.py index d36e62ace..d00a4cc96 100644 --- a/test_libs/pyspec/eth2spec/test/helpers/attestations.py +++ b/test_libs/pyspec/eth2spec/test/helpers/attestations.py @@ -54,11 +54,9 @@ def get_valid_attestation(spec, state, slot=None, index=None, signed=False): committee_size = len(beacon_committee) aggregation_bits = Bitlist[spec.MAX_VALIDATORS_PER_COMMITTEE](*([0] * committee_size)) - custody_bits = Bitlist[spec.MAX_VALIDATORS_PER_COMMITTEE](*([0] * committee_size)) attestation = spec.Attestation( aggregation_bits=aggregation_bits, data=attestation_data, - custody_bits=custody_bits, ) fill_aggregate_attestation(spec, state, attestation) if signed: @@ -83,7 +81,7 @@ def sign_aggregate_attestation(spec, state, attestation_data, participants: List def sign_indexed_attestation(spec, state, indexed_attestation): - participants = indexed_attestation.custody_bit_0_indices + indexed_attestation.custody_bit_1_indices + participants = indexed_attestation.attesting_indices indexed_attestation.signature = sign_aggregate_attestation(spec, state, indexed_attestation.data, participants) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py index 04fa880e0..554d77a00 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py @@ -274,35 +274,6 @@ def test_bad_source_root(spec, state): yield from run_attestation_processing(spec, state, attestation, False) -@with_all_phases -@spec_state_test -def test_inconsistent_bits(spec, state): - attestation = get_valid_attestation(spec, state) - state.slot += spec.MIN_ATTESTATION_INCLUSION_DELAY - - custody_bits = attestation.custody_bits[:] - custody_bits.append(False) - - attestation.custody_bits = custody_bits - - sign_attestation(spec, state, attestation) - - yield from run_attestation_processing(spec, state, attestation, False) - - -@with_phases(['phase0']) -@spec_state_test -def test_non_empty_custody_bits(spec, state): - attestation = get_valid_attestation(spec, state) - state.slot += spec.MIN_ATTESTATION_INCLUSION_DELAY - - attestation.custody_bits = attestation.aggregation_bits[:] - - sign_attestation(spec, state, attestation) - - yield from run_attestation_processing(spec, state, attestation, False) - - @with_all_phases @spec_state_test def test_empty_aggregation_bits(spec, state): @@ -344,32 +315,3 @@ def test_too_few_aggregation_bits(spec, state): attestation.aggregation_bits = attestation.aggregation_bits[:-1] yield from run_attestation_processing(spec, state, attestation, False) - - -@with_all_phases -@spec_state_test -def test_too_many_custody_bits(spec, state): - attestation = get_valid_attestation(spec, state, signed=True) - state.slot += spec.MIN_ATTESTATION_INCLUSION_DELAY - - # one too many bits - attestation.custody_bits.append(0b0) - - yield from run_attestation_processing(spec, state, attestation, False) - - -@with_all_phases -@spec_state_test -def test_too_few_custody_bits(spec, state): - attestation = get_valid_attestation(spec, state) - state.slot += spec.MIN_ATTESTATION_INCLUSION_DELAY - - attestation.custody_bits = Bitlist[spec.MAX_VALIDATORS_PER_COMMITTEE]( - *([0b1] + [0b0] * (len(attestation.custody_bits) - 1))) - - sign_attestation(spec, state, attestation) - - # one too few bits - attestation.custody_bits = attestation.custody_bits[:-1] - - yield from run_attestation_processing(spec, state, attestation, False) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py index 20a510648..85e807ec0 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py @@ -25,10 +25,7 @@ def run_attester_slashing_processing(spec, state, attester_slashing, valid=True) yield 'post', None return - slashed_indices = ( - attester_slashing.attestation_1.custody_bit_0_indices - + attester_slashing.attestation_1.custody_bit_1_indices - ) + slashed_indices = attester_slashing.attestation_1.attesting_indices proposer_index = spec.get_beacon_proposer_index(state) pre_proposer_balance = get_balance(state, proposer_index) @@ -112,10 +109,7 @@ def test_success_surround(spec, state): @always_bls def test_success_already_exited_recent(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) - slashed_indices = ( - attester_slashing.attestation_1.custody_bit_0_indices - + attester_slashing.attestation_1.custody_bit_1_indices - ) + slashed_indices = attester_slashing.attestation_1.attesting_indices for index in slashed_indices: spec.initiate_validator_exit(state, index) @@ -127,10 +121,7 @@ def test_success_already_exited_recent(spec, state): @always_bls def test_success_already_exited_long_ago(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) - slashed_indices = ( - attester_slashing.attestation_1.custody_bit_0_indices - + attester_slashing.attestation_1.custody_bit_1_indices - ) + slashed_indices = attester_slashing.attestation_1.attesting_indices for index in slashed_indices: spec.initiate_validator_exit(state, index) state.validators[index].withdrawable_epoch = spec.get_current_epoch(state) + 2 @@ -190,38 +181,23 @@ def test_participants_already_slashed(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) # set all indices to slashed - attestation_1 = attester_slashing.attestation_1 - validator_indices = attestation_1.custody_bit_0_indices + attestation_1.custody_bit_1_indices + validator_indices = attester_slashing.attestation_1.attesting_indices for index in validator_indices: state.validators[index].slashed = True yield from run_attester_slashing_processing(spec, state, attester_slashing, False) -@with_all_phases -@spec_state_test -def test_custody_bit_0_and_1_intersect(spec, state): - attester_slashing = get_valid_attester_slashing(spec, state, signed_1=False, signed_2=True) - - attester_slashing.attestation_1.custody_bit_1_indices.append( - attester_slashing.attestation_1.custody_bit_0_indices[0] - ) - - sign_indexed_attestation(spec, state, attester_slashing.attestation_1) - - yield from run_attester_slashing_processing(spec, state, attester_slashing, False) - - @with_all_phases @spec_state_test @always_bls def test_att1_bad_extra_index(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) - indices = attester_slashing.attestation_1.custody_bit_0_indices + indices = attester_slashing.attestation_1.attesting_indices options = list(set(range(len(state.validators))) - set(indices)) indices.append(options[len(options) // 2]) # add random index, not previously in attestation. - attester_slashing.attestation_1.custody_bit_0_indices = sorted(indices) + attester_slashing.attestation_1.attesting_indices = sorted(indices) # Do not sign the modified attestation (it's ok to slash if attester signed, not if they did not), # see if the bad extra index is spotted, and slashing is aborted. @@ -234,10 +210,10 @@ def test_att1_bad_extra_index(spec, state): def test_att1_bad_replaced_index(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) - indices = attester_slashing.attestation_1.custody_bit_0_indices + indices = attester_slashing.attestation_1.attesting_indices options = list(set(range(len(state.validators))) - set(indices)) indices[3] = options[len(options) // 2] # replace with random index, not previously in attestation. - attester_slashing.attestation_1.custody_bit_0_indices = sorted(indices) + attester_slashing.attestation_1.attesting_indices = sorted(indices) # Do not sign the modified attestation (it's ok to slash if attester signed, not if they did not), # see if the bad replaced index is spotted, and slashing is aborted. @@ -250,10 +226,10 @@ def test_att1_bad_replaced_index(spec, state): def test_att2_bad_extra_index(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) - indices = attester_slashing.attestation_2.custody_bit_0_indices + indices = attester_slashing.attestation_2.attesting_indices options = list(set(range(len(state.validators))) - set(indices)) indices.append(options[len(options) // 2]) # add random index, not previously in attestation. - attester_slashing.attestation_2.custody_bit_0_indices = sorted(indices) + attester_slashing.attestation_2.attesting_indices = sorted(indices) # Do not sign the modified attestation (it's ok to slash if attester signed, not if they did not), # see if the bad extra index is spotted, and slashing is aborted. @@ -266,10 +242,10 @@ def test_att2_bad_extra_index(spec, state): def test_att2_bad_replaced_index(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) - indices = attester_slashing.attestation_2.custody_bit_0_indices + indices = attester_slashing.attestation_2.attesting_indices options = list(set(range(len(state.validators))) - set(indices)) indices[3] = options[len(options) // 2] # replace with random index, not previously in attestation. - attester_slashing.attestation_2.custody_bit_0_indices = sorted(indices) + attester_slashing.attestation_2.attesting_indices = sorted(indices) # Do not sign the modified attestation (it's ok to slash if attester signed, not if they did not), # see if the bad replaced index is spotted, and slashing is aborted. @@ -278,10 +254,10 @@ def test_att2_bad_replaced_index(spec, state): @with_all_phases @spec_state_test -def test_unsorted_att_1_bit0(spec, state): +def test_unsorted_att_1(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=False, signed_2=True) - indices = attester_slashing.attestation_1.custody_bit_0_indices + indices = attester_slashing.attestation_1.attesting_indices assert len(indices) >= 3 indices[1], indices[2] = indices[2], indices[1] # unsort second and third index sign_indexed_attestation(spec, state, attester_slashing.attestation_1) @@ -291,15 +267,12 @@ def test_unsorted_att_1_bit0(spec, state): @with_all_phases @spec_state_test -def test_unsorted_att_2_bit0(spec, state): +def test_unsorted_att_2(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=False) - indices = attester_slashing.attestation_2.custody_bit_0_indices + indices = attester_slashing.attestation_2.attesting_indices assert len(indices) >= 3 indices[1], indices[2] = indices[2], indices[1] # unsort second and third index sign_indexed_attestation(spec, state, attester_slashing.attestation_2) yield from run_attester_slashing_processing(spec, state, attester_slashing, False) - - -# note: unsorted indices for custody bit 0 are to be introduced in phase 1 testing. diff --git a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py index 99ba9d284..7d844b63b 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py @@ -141,7 +141,7 @@ def test_duplicate_attestation(spec, state): attestation = get_valid_attestation(spec, state, signed=True) indexed_attestation = spec.get_indexed_attestation(state, attestation) - participants = indexed_attestation.custody_bit_0_indices + indexed_attestation.custody_bit_1_indices + participants = indexed_attestation.attesting_indices assert len(participants) > 0 diff --git a/test_libs/pyspec/eth2spec/test/sanity/test_blocks.py b/test_libs/pyspec/eth2spec/test/sanity/test_blocks.py index bffb1c1fc..bbc4bba3a 100644 --- a/test_libs/pyspec/eth2spec/test/sanity/test_blocks.py +++ b/test_libs/pyspec/eth2spec/test/sanity/test_blocks.py @@ -188,8 +188,7 @@ def test_attester_slashing(spec, state): pre_state = deepcopy(state) attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) - validator_index = (attester_slashing.attestation_1.custody_bit_0_indices - + attester_slashing.attestation_1.custody_bit_1_indices)[0] + validator_index = attester_slashing.attestation_1.attesting_indices[0] assert not state.validators[validator_index].slashed