Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify per-block attestation processing #753

Closed
JustinDrake opened this issue Mar 10, 2019 · 2 comments
Closed

Simplify per-block attestation processing #753

JustinDrake opened this issue Mar 10, 2019 · 2 comments
Labels
general:presentation Presentation (as opposed to content)

Comments

@JustinDrake
Copy link
Collaborator

JustinDrake commented Mar 10, 2019

Below is a simplified version of process_attestation (the only hard-to-read transaction processing function). It incorporates #698 and #752. It has miscellaneous cleanups and simplifications. It also avoids untriggerable asserts (which goes against the goal of 100% coverage in the executable spec) such as assert get_bitfield_bit(attestation.custody_bitfield, i) == 0b0. I did this mostly for myself but happy to create a PR if people like it.

def process_attestation(state: BeaconState, attestation: Attestation) -> None:
    """
    Process ``Attestation`` transaction.
    Note that this function mutates ``state``.
    """
    assert max(GENESIS_SLOT, state.slot - SLOTS_PER_EPOCH) <= attestation.data.slot
    assert attestation.data.slot <= state.slot - MIN_ATTESTATION_INCLUSION_DELAY

    # Check source epoch and root
    assert (slot_to_epoch(attestation.data.slot), attestation.data.source_epoch) in {
        (get_current_epoch(state), state.current_justified_epoch),   # Case 1: current epoch attestation
        (get_previous_epoch(state), state.previous_justified_epoch)  # Case 2: previous epoch attestation
    }
    assert attestation.data.source_root == get_block_root(state, get_epoch_start_slot(attestation.data.source_epoch))

    # Check crosslink data
    assert attestation.data.crosslink_data_root == ZERO_HASH  # [to be removed in phase 1]
    assert state.latest_crosslinks[attestation.data.shard] in {
        attestation.data.previous_crosslink,  # Case 1: crosslink in state matches previous crosslink
        Crosslink(                            # Case 2: crosslink in state matches current crosslink
            crosslink_data_root=attestation.data.crosslink_data_root,
            epoch=slot_to_epoch(attestation.data.slot)
        )
    }

    # Check custody bits [to be generalised in phase 1]
    assert attestation.custody_bitfield == b'\x00' * len(attestation.custody_bitfield)

    # Check aggregate signature [to be generalised in phase 1]
    participants = get_attestation_participants(state, attestation.data, attestation.aggregation_bitfield)
    assert len(participants) != 0
    assert bls_verify(
        pubkey=bls_aggregate_pubkeys([state.validator_registry[i].pubkey for i in participants]),
        message_hash=hash_tree_root(AttestationDataAndCustodyBit(data=attestation.data, custody_bit=0b0)),
        signature=attestation.aggregate_signature,
        domain=get_domain(state.fork, slot_to_epoch(attestation.data.slot), DOMAIN_ATTESTATION),
    )

    # Cache pending attestation
    pending_attestation = PendingAttestation(
        data=attestation.data,
        aggregation_bitfield=attestation.aggregation_bitfield,
        custody_bitfield=attestation.custody_bitfield,
        inclusion_slot=state.slot
    )
    if slot_to_epoch(attestation.data.slot) == get_current_epoch(state):
        state.current_epoch_attestations.append(pending_attestation)
    else:
        state.previous_epoch_attestations.append(pending_attestation)
@JustinDrake JustinDrake added the general:presentation Presentation (as opposed to content) label Mar 11, 2019
@vbuterin
Copy link
Contributor

Definitely looks like an improvement to me!

@cemozerr
Copy link

This also seems to fix the issue I just mentioned in #775

JustinDrake added a commit that referenced this issue Mar 15, 2019
Improve readability and testability (by avoiding untriggerable `assert`). Fix #753.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general:presentation Presentation (as opposed to content)
Projects
None yet
Development

No branches or pull requests

3 participants