From cb4cafa4fd3d4356024ee27917b1208645923ecc Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Sun, 3 Mar 2019 00:19:21 +0800 Subject: [PATCH 1/2] Change `AttesationData.latest_crosslink_root` to `AttestationData.latest_crosslink` and also change the type --- .../forks/serenity/block_validation.py | 41 +++++++----- eth2/beacon/tools/builder/validator.py | 8 +-- eth2/beacon/types/attestation_data.py | 7 ++- eth2/beacon/types/crosslink_records.py | 15 +++++ tests/core/p2p-proto/bcc/test_commands.py | 14 ++--- tests/eth2/beacon/conftest.py | 4 +- ...t_serenity_block_attestation_validation.py | 63 ++++++++++++++----- .../types/test_slashable_attestation.py | 13 ---- 8 files changed, 102 insertions(+), 63 deletions(-) diff --git a/eth2/beacon/state_machines/forks/serenity/block_validation.py b/eth2/beacon/state_machines/forks/serenity/block_validation.py index e93b797e2e..81e457991a 100644 --- a/eth2/beacon/state_machines/forks/serenity/block_validation.py +++ b/eth2/beacon/state_machines/forks/serenity/block_validation.py @@ -47,6 +47,7 @@ from eth2.beacon.types.attestation_data_and_custody_bits import AttestationDataAndCustodyBit from eth2.beacon.types.attester_slashings import AttesterSlashing # noqa: F401 from eth2.beacon.types.blocks import BaseBeaconBlock # noqa: F401 +from eth2.beacon.types.crosslink_records import CrosslinkRecord from eth2.beacon.types.forks import Fork # noqa: F401 from eth2.beacon.types.proposal_signed_data import ProposalSignedData from eth2.beacon.types.slashable_attestations import SlashableAttestation # noqa: F401 @@ -330,8 +331,9 @@ def validate_attestation(state: BeaconState, ) validate_attestation_latest_crosslink_root( - attestation.data, - latest_crosslink_root=state.latest_crosslinks[attestation.data.shard].crosslink_data_root, + attestation_data=attestation.data, + state_latest_crosslink=state.latest_crosslinks[attestation.data.shard], + slots_per_epoch=slots_per_epoch, ) validate_attestation_crosslink_data_root(attestation.data) @@ -427,26 +429,31 @@ def validate_attestation_justified_block_root(attestation_data: AttestationData, def validate_attestation_latest_crosslink_root(attestation_data: AttestationData, - latest_crosslink_root: Hash32) -> None: + state_latest_crosslink: CrosslinkRecord, + slots_per_epoch: int) -> None: """ - Validate that either the attestation ``latest_crosslink_root`` or ``crosslink_data_root`` - field of ``attestation_data`` is the provided ``latest_crosslink_root``. + Validate that either the attestation ``latest_crosslink`` or ``crosslink_data_root`` + field of ``attestation_data`` is the provided ``latest_crosslink``. Raise ``ValidationError`` if it's invalid. """ - acceptable_crosslink_data_roots = { - attestation_data.latest_crosslink_root, - attestation_data.crosslink_data_root, + attestation_creating_crosslink = CrosslinkRecord( + epoch=slot_to_epoch(attestation_data.slot, slots_per_epoch), + crosslink_data_root=attestation_data.crosslink_data_root, + ) + acceptable_crosslink_data = { + # Case 1: Latest crosslink matches the one in the state + attestation_data.latest_crosslink, + # Case 2: State has already been updated, state's latest crosslink matches the crosslink + # the attestation is trying to create + attestation_creating_crosslink, } - if latest_crosslink_root not in acceptable_crosslink_data_roots: + if state_latest_crosslink not in acceptable_crosslink_data: raise ValidationError( - "Neither the attestation ``latest_crosslink_root`` nor the attestation " - "``crosslink_data_root`` are equal to the ``latest_crosslink_root``.\n" - "\tFound: %s and %s, Expected %s" % - ( - attestation_data.latest_crosslink_root, - attestation_data.crosslink_data_root, - latest_crosslink_root, - ) + f"State's latests crosslink ({state_latest_crosslink}) doesn't match " + " case 1: the `attestation_data.latest_crosslink` " + f"({attestation_data.latest_crosslink.root}) or " + "`case 2: the crosslink the attestation is trying to create " + f"({attestation_creating_crosslink})" ) diff --git a/eth2/beacon/tools/builder/validator.py b/eth2/beacon/tools/builder/validator.py index 2d34bfab30..c3f4b361fe 100644 --- a/eth2/beacon/tools/builder/validator.py +++ b/eth2/beacon/tools/builder/validator.py @@ -263,7 +263,7 @@ def create_mock_slashable_attestation(state: BeaconState, get_epoch_start_slot(state.justified_epoch, config.SLOTS_PER_EPOCH), config.LATEST_BLOCK_ROOTS_LENGTH, ) - latest_crosslink_root = state.latest_crosslinks[shard].crosslink_data_root + latest_crosslink = state.latest_crosslinks[shard] attestation_data = attestation_data = AttestationData( slot=attestation_slot, @@ -271,7 +271,7 @@ def create_mock_slashable_attestation(state: BeaconState, beacon_block_root=beacon_block_root, epoch_boundary_root=epoch_boundary_root, crosslink_data_root=ZERO_HASH32, - latest_crosslink_root=latest_crosslink_root, + latest_crosslink=latest_crosslink, justified_epoch=state.justified_epoch, justified_block_root=justified_block_root, ) @@ -493,7 +493,7 @@ def create_mock_signed_attestations_at_slot( committee, shard = crosslink_committee num_voted_attesters = int(len(committee) * voted_attesters_ratio) - latest_crosslink_root = state.latest_crosslinks[shard].crosslink_data_root + latest_crosslink = state.latest_crosslinks[shard] attestation_data = AttestationData( slot=attestation_slot, @@ -501,7 +501,7 @@ def create_mock_signed_attestations_at_slot( beacon_block_root=beacon_block_root, epoch_boundary_root=epoch_boundary_root, crosslink_data_root=ZERO_HASH32, - latest_crosslink_root=latest_crosslink_root, + latest_crosslink=latest_crosslink, justified_epoch=state.justified_epoch, justified_block_root=justified_block_root, ) diff --git a/eth2/beacon/types/attestation_data.py b/eth2/beacon/types/attestation_data.py index 4bd86d8e18..0bffcbce78 100644 --- a/eth2/beacon/types/attestation_data.py +++ b/eth2/beacon/types/attestation_data.py @@ -13,6 +13,7 @@ Slot, Shard, ) +from eth2.beacon.types.crosslink_records import CrosslinkRecord class AttestationData(ssz.Serializable): @@ -29,7 +30,7 @@ class AttestationData(ssz.Serializable): # Shard block root being attested to ('crosslink_data_root', bytes32), # Last crosslink hash - ('latest_crosslink_root', bytes32), + ('latest_crosslink', CrosslinkRecord), # epoch of the last justified beacon block ('justified_epoch', uint64), # Hash of the last justified beacon block @@ -42,7 +43,7 @@ def __init__(self, beacon_block_root: Hash32, epoch_boundary_root: Hash32, crosslink_data_root: Hash32, - latest_crosslink_root: Hash32, + latest_crosslink: CrosslinkRecord, justified_epoch: Epoch, justified_block_root: Hash32) -> None: super().__init__( @@ -51,7 +52,7 @@ def __init__(self, beacon_block_root, epoch_boundary_root, crosslink_data_root, - latest_crosslink_root, + latest_crosslink, justified_epoch, justified_block_root, ) diff --git a/eth2/beacon/types/crosslink_records.py b/eth2/beacon/types/crosslink_records.py index de4cbe862f..b9d348bfdb 100644 --- a/eth2/beacon/types/crosslink_records.py +++ b/eth2/beacon/types/crosslink_records.py @@ -8,6 +8,7 @@ bytes32, ) +from eth2.beacon._utils.hash import hash_eth2 from eth2.beacon.typing import Epoch @@ -28,3 +29,17 @@ def __init__(self, epoch=epoch, crosslink_data_root=crosslink_data_root, ) + + _hash = None + + @property + def hash(self) -> Hash32: + if self._hash is None: + self._hash = hash_eth2(ssz.encode(self)) + return self._hash + + @property + def root(self) -> Hash32: + # Alias of `hash`. + # Using flat hash, will likely use SSZ tree hash. + return self.hash diff --git a/tests/core/p2p-proto/bcc/test_commands.py b/tests/core/p2p-proto/bcc/test_commands.py index ca2d611248..be5f9b698d 100644 --- a/tests/core/p2p-proto/bcc/test_commands.py +++ b/tests/core/p2p-proto/bcc/test_commands.py @@ -2,18 +2,18 @@ import ssz +from eth.constants import ( + ZERO_HASH32, +) + from eth2.beacon.types.attestations import Attestation from eth2.beacon.types.attestation_data import AttestationData from eth2.beacon.types.blocks import ( BeaconBlock, BeaconBlockBody, ) - -from eth.constants import ( - ZERO_HASH32, -) - from eth2.beacon.types.eth1_data import Eth1Data +from eth2.beacon.types.crosslink_records import CrosslinkRecord from p2p.peer import ( MsgBuffer, @@ -167,7 +167,7 @@ async def test_send_single_attestation(request, event_loop): beacon_block_root=ZERO_HASH32, epoch_boundary_root=ZERO_HASH32, crosslink_data_root=ZERO_HASH32, - latest_crosslink_root=ZERO_HASH32, + latest_crosslink=CrosslinkRecord(SERENITY_CONFIG.GENESIS_EPOCH, ZERO_HASH32), justified_epoch=SERENITY_CONFIG.GENESIS_EPOCH, justified_block_root=ZERO_HASH32, ), @@ -194,7 +194,7 @@ async def test_send_multiple_attestations(request, event_loop): beacon_block_root=ZERO_HASH32, epoch_boundary_root=ZERO_HASH32, crosslink_data_root=ZERO_HASH32, - latest_crosslink_root=ZERO_HASH32, + latest_crosslink=CrosslinkRecord(SERENITY_CONFIG.GENESIS_EPOCH, ZERO_HASH32), justified_epoch=SERENITY_CONFIG.GENESIS_EPOCH, justified_block_root=ZERO_HASH32, ), diff --git a/tests/eth2/beacon/conftest.py b/tests/eth2/beacon/conftest.py index 01c19f2b2b..6f463d1552 100644 --- a/tests/eth2/beacon/conftest.py +++ b/tests/eth2/beacon/conftest.py @@ -100,14 +100,14 @@ def sample_attestation_params(sample_attestation_data_params): @pytest.fixture -def sample_attestation_data_params(): +def sample_attestation_data_params(sample_crosslink_record_params): return { 'slot': 10, 'shard': 12, 'beacon_block_root': b'\x11' * 32, 'epoch_boundary_root': b'\x22' * 32, 'crosslink_data_root': b'\x33' * 32, - 'latest_crosslink_root': b'\x44' * 32, + 'latest_crosslink': CrosslinkRecord(**sample_crosslink_record_params), 'justified_epoch': 0, 'justified_block_root': b'\x55' * 32, } diff --git a/tests/eth2/beacon/state_machines/forks/test_serenity_block_attestation_validation.py b/tests/eth2/beacon/state_machines/forks/test_serenity_block_attestation_validation.py index 0b62235c6c..8e08150302 100644 --- a/tests/eth2/beacon/state_machines/forks/test_serenity_block_attestation_validation.py +++ b/tests/eth2/beacon/state_machines/forks/test_serenity_block_attestation_validation.py @@ -27,6 +27,7 @@ create_mock_signed_attestation, ) from eth2.beacon.types.attestation_data import AttestationData +from eth2.beacon.types.crosslink_records import CrosslinkRecord @pytest.mark.parametrize( @@ -138,8 +139,8 @@ def test_validate_attestation_justified_epoch( 'is_valid,' ), [ - (b'\x42' * 32, b'\x35' * 32, False), # attestation.justified_block_root != justified_block_root # noqa: E501 - (b'\x42' * 32, b'\x42' * 32, True), + (b'\x33' * 32, b'\x22' * 32, False), # attestation.justified_block_root != justified_block_root # noqa: E501 + (b'\x33' * 32, b'\x33' * 32, True), ] ) def test_validate_attestation_justified_block_root(sample_attestation_data_params, @@ -165,41 +166,69 @@ def test_validate_attestation_justified_block_root(sample_attestation_data_param @pytest.mark.parametrize( ( - 'attestation_latest_crosslink_root,' + 'attestation_latest_crosslink,' 'attestation_crosslink_data_root,' - 'latest_crosslink_root,' + 'state_latest_crosslink,' 'is_valid,' ), [ - (b'\x66' * 32, b'\x42' * 32, b'\x35' * 32, False), - (b'\x42' * 32, b'\x42' * 32, b'\x66' * 32, False), - (b'\x66' * 32, b'\x42' * 32, b'\x42' * 32, True), - (b'\x42' * 32, b'\x35' * 32, b'\x42' * 32, True), - (b'\x42' * 32, b'\x42' * 32, b'\x42' * 32, True), + ( + CrosslinkRecord(0, b'\x11' * 32), + b'\x33' * 32, + CrosslinkRecord(0, b'\x22' * 32), + False, + ), + ( + CrosslinkRecord(0, b'\x33' * 32), + b'\x33' * 32, + CrosslinkRecord(0, b'\x11' * 32), + False, + ), + ( + CrosslinkRecord(0, b'\x11' * 32), + b'\x33' * 32, + CrosslinkRecord(0, b'\x33' * 32), + True, + ), + ( + CrosslinkRecord(0, b'\x33' * 32), + b'\x22' * 32, + CrosslinkRecord(0, b'\x33' * 32), + True, + ), + ( + CrosslinkRecord(0, b'\x33' * 32), + b'\x33' * 32, + CrosslinkRecord(0, b'\x33' * 32), + True, + ), ] ) def test_validate_attestation_latest_crosslink_root(sample_attestation_data_params, - attestation_latest_crosslink_root, + attestation_latest_crosslink, attestation_crosslink_data_root, - latest_crosslink_root, + state_latest_crosslink, + slots_per_epoch, is_valid): - sample_attestation_data_params['latest_crosslink_root'] = attestation_latest_crosslink_root + sample_attestation_data_params['latest_crosslink'] = attestation_latest_crosslink sample_attestation_data_params['crosslink_data_root'] = attestation_crosslink_data_root attestation_data = AttestationData(**sample_attestation_data_params).copy( - latest_crosslink_root=attestation_latest_crosslink_root, + latest_crosslink=attestation_latest_crosslink, crosslink_data_root=attestation_crosslink_data_root, ) if is_valid: validate_attestation_latest_crosslink_root( attestation_data, - latest_crosslink_root, + state_latest_crosslink, + slots_per_epoch=slots_per_epoch, ) else: with pytest.raises(ValidationError): validate_attestation_latest_crosslink_root( attestation_data, - latest_crosslink_root, + state_latest_crosslink, + slots_per_epoch=slots_per_epoch, ) @@ -210,8 +239,8 @@ def test_validate_attestation_latest_crosslink_root(sample_attestation_data_para ), [ (ZERO_HASH32, True), - (b'\x35' * 32, False), - (b'\x66' * 32, False), + (b'\x22' * 32, False), + (b'\x11' * 32, False), ] ) def test_validate_attestation_crosslink_data_root(sample_attestation_data_params, diff --git a/tests/eth2/beacon/types/test_slashable_attestation.py b/tests/eth2/beacon/types/test_slashable_attestation.py index 188e79f2d3..dc831a7adc 100644 --- a/tests/eth2/beacon/types/test_slashable_attestation.py +++ b/tests/eth2/beacon/types/test_slashable_attestation.py @@ -25,19 +25,6 @@ def test_defaults(sample_slashable_attestation_params): assert ssz.encode(slashable_attestation) -def test_hash(sample_slashable_attestation_params): - slashable_attestation = SlashableAttestation(**sample_slashable_attestation_params) - - # NOTE: this hash was simply copied from the existing implementation - # which should be the keccak-256 of the rlp serialization of `votes`. - # Given that this value will change soon (cf. ssz tree hash), we just - # do this to get the test passing for now and will need to update later - # if we expect the hash computation is not working correctly - hash_hex = "8cba2c190eae977ba16cbcc7ef1b95b32384fd12e86292ca8a91cc320eaeac9c" - - assert slashable_attestation.hash == bytes.fromhex(hash_hex) - - def test_root(sample_slashable_attestation_params): slashable_attestation = SlashableAttestation(**sample_slashable_attestation_params) From b86dbe1663a6dcdf923958fd3ceeb92a5ac65831 Mon Sep 17 00:00:00 2001 From: Hsiao-Wei Wang Date: Sun, 3 Mar 2019 02:32:45 +0800 Subject: [PATCH 2/2] Apply PR feedback, thanks to Nic --- .../test_serenity_block_attestation_validation.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/eth2/beacon/state_machines/forks/test_serenity_block_attestation_validation.py b/tests/eth2/beacon/state_machines/forks/test_serenity_block_attestation_validation.py index 8e08150302..e63e9fa8d7 100644 --- a/tests/eth2/beacon/state_machines/forks/test_serenity_block_attestation_validation.py +++ b/tests/eth2/beacon/state_machines/forks/test_serenity_block_attestation_validation.py @@ -204,12 +204,12 @@ def test_validate_attestation_justified_block_root(sample_attestation_data_param ), ] ) -def test_validate_attestation_latest_crosslink_root(sample_attestation_data_params, - attestation_latest_crosslink, - attestation_crosslink_data_root, - state_latest_crosslink, - slots_per_epoch, - is_valid): +def test_validate_attestation_latest_crosslink(sample_attestation_data_params, + attestation_latest_crosslink, + attestation_crosslink_data_root, + state_latest_crosslink, + slots_per_epoch, + is_valid): sample_attestation_data_params['latest_crosslink'] = attestation_latest_crosslink sample_attestation_data_params['crosslink_data_root'] = attestation_crosslink_data_root attestation_data = AttestationData(**sample_attestation_data_params).copy(