From fbb09795ed3dca6e98eb9ef97c572f4e590293cf Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 27 Mar 2019 08:31:56 -0600 Subject: [PATCH] fix convert_to_indexed custody bitfield bug --- specs/core/0_beacon-chain.md | 63 +++++++++++++++---- .../test_process_attestation.py | 2 +- tests/phase0/helpers.py | 36 +++++++---- 3 files changed, 78 insertions(+), 23 deletions(-) diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index 2e2c3ad59..0bdfafb79 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -77,6 +77,7 @@ - [`generate_seed`](#generate_seed) - [`get_beacon_proposer_index`](#get_beacon_proposer_index) - [`verify_merkle_branch`](#verify_merkle_branch) + - [`get_crosslink_committee_for_attestation`](#get_crosslink_committee_for_attestation) - [`get_attestation_participants`](#get_attestation_participants) - [`int_to_bytes1`, `int_to_bytes2`, ...](#int_to_bytes1-int_to_bytes2-) - [`bytes_to_int`](#bytes_to_int) @@ -85,6 +86,7 @@ - [`get_fork_version`](#get_fork_version) - [`get_domain`](#get_domain) - [`get_bitfield_bit`](#get_bitfield_bit) + - [`set_bitfield_bit`](#set_bitfield_bit) - [`verify_bitfield`](#verify_bitfield) - [`convert_to_indexed`](#convert_to_indexed) - [`verify_indexed_attestation`](#verify_indexed_attestation) @@ -1037,6 +1039,20 @@ def verify_merkle_branch(leaf: Bytes32, proof: List[Bytes32], depth: int, index: return value == root ``` +### `get_crosslink_committee_for_attestation` + +```python +def get_crosslink_committee_for_attestation(state: BeaconState, + attestation_data: AttestationData) -> List[ValidatorIndex]: + # Find the committee in the list with the desired shard + crosslink_committees = get_crosslink_committees_at_slot(state, attestation_data.slot) + + assert attestation_data.shard in [shard for _, shard in crosslink_committees] + crosslink_committee = [committee for committee, shard in crosslink_committees if shard == attestation_data.shard][0] + + return crosslink_committee +``` + ### `get_attestation_participants` ```python @@ -1046,11 +1062,7 @@ def get_attestation_participants(state: BeaconState, """ Return the participant indices corresponding to ``attestation_data`` and ``bitfield``. """ - # Find the committee in the list with the desired shard - crosslink_committees = get_crosslink_committees_at_slot(state, attestation_data.slot) - - assert attestation_data.shard in [shard for _, shard in crosslink_committees] - crosslink_committee = [committee for committee, shard in crosslink_committees if shard == attestation_data.shard][0] + crosslink_committee = get_crosslink_committee_for_attestation(state, attestation_data) assert verify_bitfield(bitfield, len(crosslink_committee)) @@ -1060,7 +1072,7 @@ def get_attestation_participants(state: BeaconState, aggregation_bit = get_bitfield_bit(bitfield, i) if aggregation_bit == 0b1: participants.append(validator_index) - return sorted(participants) + return participants ``` ### `int_to_bytes1`, `int_to_bytes2`, ... @@ -1130,6 +1142,22 @@ def get_bitfield_bit(bitfield: bytes, i: int) -> int: return (bitfield[i // 8] >> (i % 8)) % 2 ``` +### `set_bitfield_bit` + +```python +def set_bitfield_bit(bitfield: bytes, i: int) -> int: + """ + Set the bit in ``bitfield`` at position ``i`` to ``1``. + """ + byte_index = i // 8 + bit_index = i % 8 + return ( + bitfield[:byte_index] + + bytes([bitfield[byte_index] | (1 << bit_index)]) + + bitfield[byte_index+1:] + ) +``` + ### `verify_bitfield` ```python @@ -1155,10 +1183,21 @@ def convert_to_indexed(state: BeaconState, attestation: Attestation): """ Convert an attestation to (almost) indexed-verifiable form """ + attesting_indices = get_attestation_participants(state, attestation.data, attestation.aggregation_bitfield) + + # reconstruct custody bitfield for the truncated attesting_indices + custody_bit_1_indices = get_attestation_participants(state, attestation.data, attestation.custody_bitfield) + custody_bitfield = b'\x00' * ((len(attesting_indices) + 7) // 8) + + crosslink_committee = get_crosslink_committee_for_attestation(state, attestation.data) + for i, validator_index in enumerate(crosslink_committee): + if get_bitfield_bit(attestation.custody_bitfield, i): + custody_bitfield = set_bitfield_bit(custody_bitfield, attesting_indices.index(validator_index)) + return IndexedAttestation( - validator_indices=get_attestation_participants(state, attestation.data, attestation.aggregation_bitfield), + validator_indices=attesting_indices, data=attestation.data, - custody_bitfield=attestation.custody_bitfield, + custody_bitfield=custody_bitfield, aggregate_signature=attestation.aggregate_signature ) ``` @@ -1176,9 +1215,6 @@ def verify_indexed_attestation(state: BeaconState, indexed_attestation: IndexedA if not (1 <= len(indexed_attestation.validator_indices) <= MAX_ATTESTATION_PARTICIPANTS): return False - if indexed_attestation.validator_indices != sorted(indexed_attestation.validator_indices): - return False - if not verify_bitfield(indexed_attestation.custody_bitfield, len(indexed_attestation.validator_indices)): return False @@ -2318,6 +2354,11 @@ def process_attester_slashing(state: BeaconState, is_double_vote(attestation1.data, attestation2.data) or is_surround_vote(attestation1.data, attestation2.data) ) + + # check that indices are sorted + assert attestation1.validator_indices == sorted(attestation1.validator_indices) + assert attestation2.validator_indices == sorted(attestation2.validator_indices) + assert verify_indexed_attestation(state, attestation1) assert verify_indexed_attestation(state, attestation2) slashable_indices = [ diff --git a/tests/phase0/block_processing/test_process_attestation.py b/tests/phase0/block_processing/test_process_attestation.py index 08cab11ff..ca6933ce7 100644 --- a/tests/phase0/block_processing/test_process_attestation.py +++ b/tests/phase0/block_processing/test_process_attestation.py @@ -135,7 +135,7 @@ def test_non_empty_custody_bitfield(state): attestation = get_valid_attestation(state) state.slot += spec.MIN_ATTESTATION_INCLUSION_DELAY - attestation.custody_bitfield = b'\x01' + attestation.custody_bitfield[1:] + attestation.custody_bitfield = deepcopy(attestation.aggregation_bitfield) pre_state, post_state = run_attestation_processing(state, attestation, False) diff --git a/tests/phase0/helpers.py b/tests/phase0/helpers.py index d7f4ae6e8..08ea6ca04 100644 --- a/tests/phase0/helpers.py +++ b/tests/phase0/helpers.py @@ -22,12 +22,14 @@ from build.phase0.spec import ( get_active_validator_indices, get_attestation_participants, get_block_root, + get_crosslink_committee_for_attestation, get_crosslink_committees_at_slot, get_current_epoch, get_domain, get_empty_block, get_epoch_start_slot, get_genesis_beacon_state, + slot_to_epoch, verify_merkle_branch, hash, ) @@ -248,12 +250,11 @@ def get_valid_attestation(state, slot=None): shard = state.latest_start_shard attestation_data = build_attestation_data(state, slot, shard) - crosslink_committees = get_crosslink_committees_at_slot(state, slot) - crosslink_committee = [committee for committee, _shard in crosslink_committees if _shard == attestation_data.shard][0] + crosslink_committee = get_crosslink_committee_for_attestation(state, attestation_data) committee_size = len(crosslink_committee) bitfield_length = (committee_size + 7) // 8 - aggregation_bitfield = b'\x01' + b'\x00' * (bitfield_length - 1) + aggregation_bitfield = b'\xC0' + b'\x00' * (bitfield_length - 1) custody_bitfield = b'\x00' * bitfield_length attestation = Attestation( aggregation_bitfield=aggregation_bitfield, @@ -266,23 +267,36 @@ def get_valid_attestation(state, slot=None): attestation.data, attestation.aggregation_bitfield, ) - assert len(participants) == 1 + assert len(participants) == 2 - validator_index = participants[0] - privkey = privkeys[validator_index] + signatures = [] + for validator_index in participants: + privkey = privkeys[validator_index] + signatures.append( + get_attestation_signature( + state, + attestation.data, + privkey + ) + ) + + attestation.aggregation_signature = bls.aggregate_signatures(signatures) + return attestation + + +def get_attestation_signature(state, attestation_data, privkey, custody_bit=0b0): message_hash = AttestationDataAndCustodyBit( - data=attestation.data, - custody_bit=0b0, + data=attestation_data, + custody_bit=custody_bit, ).hash_tree_root() - attestation.aggregation_signature = bls.sign( + return bls.sign( message_hash=message_hash, privkey=privkey, domain=get_domain( fork=state.fork, - epoch=get_current_epoch(state), + epoch=slot_to_epoch(attestation_data.slot), domain_type=spec.DOMAIN_ATTESTATION, ) ) - return attestation