Updates for almost v0.8 #714
Conversation
ralexstokes
changed the title
|
|
ralexstokes
changed the title
ralexstokes
force-pushed
the
validator-spec-updates
branch
3 times, most recently
from
faf19b2 to
e0fbe7d
Compare
| ) | ||
|
|
||
| if config: |
There was a problem hiding this comment.
[Not in this PR]: What do you think creating a classmethod that takes config as argument? So we can keep the init function clean
@classmethod
def from_config(config, **kwargs):
...
return cls(**kwargs) There was a problem hiding this comment.
my view is that an instance of BeaconState is not valid until this __init__ function returns w/ these defaults as given.
the only reason this is done this way is because the linter complains if we have a function call in a kwargs argument.
the thing consistent w/ what you are suggesting is to remove all default arguments but that goes against the broader intention to have valuable default objects.
i'll send notes soon about the rationale here on the "default objects"
| from typing import Any # noqa: F401 | ||
|
|
||
|
|
||
| default_bls_pubkey = BLSPubkey(b'\x00' * 48) |
There was a problem hiding this comment.
What do you think moving this to eth2/beacon/constants.py and next to EMPTY_SIGNATURE = BLSSignature(b'\x00' * 96)?
There was a problem hiding this comment.
the intention is to have these default_type types live internal to the types package so that the only thing a user of this package sees is an abstract type Type()
the pattern to achieve this is that for any default type in types/defaults, it should not escape that package.... i think i have done this succeessfully :)
if you see something that falls outside of this pattern or what to change it, lets talk :)
There was a problem hiding this comment.
This PR is using both default_* and constants.ZERO_HASH32/constants.EMPTY_SIGNATURE. If being nitpicky, it's better to make them all in default_* - but I think it's not necessary for now. 😄
| # This can be done by providing either a ``block`` *or* a ``future_slot``. | ||
| # We enforce this invariant with the assertion on ``target_slot``. | ||
| target_slot = block.slot if block else future_slot | ||
| assert target_slot is not None |
There was a problem hiding this comment.
[Not in this PR] It looks like the interface requires some checks to ensure the input has exactly a block or a future slot. Maybe make it two functions like the old apply_state_transition and apply_state_transition_without_block?
There was a problem hiding this comment.
yeah we could go either way on this -- the spec functions handle either so i think it makes sense to have a unified implementation.
the question then becomes: is the interface unified or not?
in this PR, the interface is the union, but we could roll it back how we had it....
i changed it this way bc as a user of this method i like just having to remember one symbol apply_state_transition and then i can feed it whichever data i need to accomplish that goal -- it seems to always be clear from context which one i need to do
this all being said, i'm not opposed to having two distinct interfaces -- they would just both call the existing implementation w/ the appropriate target_slot (and optional block)
ralexstokes
force-pushed
the
validator-spec-updates
branch
from
ecaa160 to
3ea9081
Compare
| block_class: Type[BaseBeaconBlock]) -> BaseBeaconBlock: | ||
| return block_class.create_empty_block(genesis_slot).copy( | ||
| state_root=genesis_state_root, | ||
| def is_genesis_trigger(deposits: Sequence[Deposit], timestamp: int, config: Eth2Config) -> bool: |
There was a problem hiding this comment.
Note: the procedure has been updated: https://github.com/ethereum/eth2.0-specs/blob/dev/specs/core/0_beacon-chain.md#genesis-state
The main difference is now we use SSZ List hash tree root of deposit_data_list as the deposit_root.
|
|
||
| class AttestationData(ssz.Serializable): | ||
|
|
||
| fields = [ | ||
| # LMD GHOST vote | ||
| ('slot', uint64), | ||
| ('beacon_block_root', bytes32), | ||
|
|
||
| # FFG vote | ||
| ('source_epoch', uint64), | ||
| ('source_root', bytes32), |
|
|
||
| class Attestation(ssz.Serializable): | ||
|
|
||
| fields = [ | ||
| # Attester aggregation bitfield | ||
| ('aggregation_bitfield', byte_list), |
There was a problem hiding this comment.
TODO: rename to aggregation_bits and use Bitlist (aggregation_bits: Bitlist[MAX_VALIDATORS_PER_COMMITTEE])
| ('data', AttestationData), | ||
| # Proof of custody bitfield | ||
| ('custody_bitfield', byte_list), |
There was a problem hiding this comment.
TODO: custody_bits: Bitlist[MAX_VALIDATORS_PER_COMMITTEE]
| default_gwei = Gwei(0) | ||
| default_timestamp = Timestamp(0) | ||
| default_second = Second(0) | ||
| default_bitfield = Bitfield(b'') |
There was a problem hiding this comment.
Do you think it makes sense to rename all these default_* variable to capital DEFAULT_*? (not strongly opinion :) )
| ('active_index_roots', Vector(bytes32, 1)), | ||
|
|
||
| # Slashings | ||
| ('slashed_balances', Vector(uint64, 1)), # Balances slashed at every withdrawal period # noqa: E501 |
There was a problem hiding this comment.
Rename to slashings.
| ('slashed_balances', Vector(uint64, 1)), # Balances slashed at every withdrawal period # noqa: E501 | |
| ('slashings', Vector(uint64, 1)), # Balances slashed at every withdrawal period # noqa: E501 |
| ('previous_justified_root', bytes32), | ||
| ('current_justified_epoch', uint64), | ||
| ('current_justified_root', bytes32), |
| ('current_justified_root', bytes32), | ||
| # Note: justification_bitfield is meant to be defined as an integer type, | ||
| # so its bit operation is in Python and is easier to specify and implement. | ||
| ('justification_bitfield', uint64), | ||
|
|
||
| # Finality | ||
| ('finalized_epoch', uint64), | ||
| ('finalized_root', bytes32), |
| previous_justified_root=previous_justified_root, | ||
| current_justified_epoch=current_justified_epoch, | ||
| current_justified_root=current_justified_root, | ||
| justification_bitfield=justification_bitfield, |
There was a problem hiding this comment.
TODO:
justification_bits: Bitvector[JUSTIFICATION_BITS_LENGTH]
| ) | ||
|
|
||
|
|
||
| def _round_down_to_previous_multiple(amount: int, increment: int) -> int: |
There was a problem hiding this comment.
| def _round_down_to_previous_multiple(amount: int, increment: int) -> int: | |
| def _round_down_to_previous_multiple(amount: Gwei, increment: Gwei) -> Gwei: |
| ('MAX_BALANCE_CHURN_QUOTIENT', int), | ||
| ('MAX_INDICES_PER_SLASHABLE_VOTE', int), | ||
| ('MAX_EXIT_DEQUEUES_PER_EPOCH', int), | ||
| ('MAX_INDICES_PER_ATTESTATION', int), |
There was a problem hiding this comment.
Rename to MAX_VALIDATORS_PER_COMMITTEE.
| ('MAX_INDICES_PER_ATTESTATION', int), | |
| ('MAX_VALIDATORS_PER_COMMITTEE', int), |
|
|
||
| def validate_indexed_attestation(state: BeaconState, | ||
| indexed_attestation: IndexedAttestation, | ||
| max_indices_per_attestation: int, |
There was a problem hiding this comment.
Rename to MAX_VALIDATORS_PER_COMMITTEE.
| max_indices_per_attestation: int, | |
| max_validators_per_committee: int, |
| f"Expected no custody bit 1 validators (cf. {bit_1_indices})." | ||
| ) | ||
|
|
||
| if len(bit_0_indices) + len(bit_1_indices) > max_indices_per_attestation: |
There was a problem hiding this comment.
| if len(bit_0_indices) + len(bit_1_indices) > max_indices_per_attestation: | |
| if len(bit_0_indices) + len(bit_1_indices) > max_validators_per_committee: |
|
|
||
| if len(bit_0_indices) + len(bit_1_indices) > max_indices_per_attestation: | ||
| raise ValidationError( | ||
| f"Require no more than {max_indices_per_attestation} validators per attestation," |
There was a problem hiding this comment.
| f"Require no more than {max_indices_per_attestation} validators per attestation," | |
| f"Require no more than {max_validators_per_committee} validators per attestation," |
| MAX_BALANCE_CHURN_QUOTIENT=2**5, # (= 32) | ||
| MAX_INDICES_PER_SLASHABLE_VOTE=2**12, # (= 4,096) votes | ||
| MAX_EXIT_DEQUEUES_PER_EPOCH=2**2, # (= 4) | ||
| MAX_INDICES_PER_ATTESTATION=2**12, # (= 4,096) votes |
There was a problem hiding this comment.
| MAX_INDICES_PER_ATTESTATION=2**12, # (= 4,096) votes | |
| MAX_VALIDATORS_PER_COMMITTEE =2**12, # (= 4,096) votes |
| # Max operations per block | ||
| ('MAX_PROPOSER_SLASHINGS', int), | ||
| ('MAX_ATTESTER_SLASHINGS', int), | ||
| ('MAX_ATTESTATIONS', int), | ||
| ('MAX_DEPOSITS', int), | ||
| ('MAX_VOLUNTARY_EXITS', int), | ||
| ('MAX_TRANSFERS', int), | ||
| # Genesis | ||
| ('GENESIS_ACTIVE_VALIDATOR_COUNT', int), |
There was a problem hiding this comment.
Missing:
| ('GENESIS_ACTIVE_VALIDATOR_COUNT', int), | |
| ('MIN_GENESIS_TIME', int), |
| ('WHISTLEBLOWER_REWARD_QUOTIENT', int), | ||
| ('ATTESTATION_INCLUSION_REWARD_QUOTIENT', int), | ||
| ('EPOCHS_PER_HISTORICAL_VECTOR', int), | ||
| ('EPOCHS_PER_SLASHED_BALANCES_VECTOR', int), |
There was a problem hiding this comment.
Missing: HISTORICAL_ROOTS_LIMIT and VALIDATOR_REGISTRY_LIMIT
| self.LATEST_RANDAO_MIXES_LENGTH = config.LATEST_RANDAO_MIXES_LENGTH | ||
| self.EPOCHS_PER_HISTORICAL_VECTOR = config.EPOCHS_PER_HISTORICAL_VECTOR | ||
| self.EPOCHS_PER_HISTORICAL_VECTOR = config.EPOCHS_PER_HISTORICAL_VECTOR | ||
|
|
| BLS_WITHDRAWAL_PREFIX_BYTE=b'\x00', | ||
| GENESIS_SLOT=Slot(0), | ||
| GENESIS_EPOCH=Epoch(0), | ||
| BLS_WITHDRAWAL_PREFIX=0, |
There was a problem hiding this comment.
It's now in byte (again).
| BLS_WITHDRAWAL_PREFIX=0, | |
| BLS_WITHDRAWAL_PREFIX=b'\x00', |
| # `FAR_FUTURE_EPOCH`, `EMPTY_SIGNATURE` `ZERO_HASH (ZERO_HASH32)` | ||
| # are defined in constants.py | ||
| ('BLS_WITHDRAWAL_PREFIX_BYTE', bytes), | ||
| ('BLS_WITHDRAWAL_PREFIX', int), |
There was a problem hiding this comment.
It's now in byte (again).
| ('BLS_WITHDRAWAL_PREFIX', int), | |
| ('BLS_WITHDRAWAL_PREFIX', bytes), |
| shuffling_epoch=state.previous_shuffling_epoch, | ||
| shuffling_start_shard=state.previous_shuffling_start_shard, | ||
| ) | ||
| def get_epoch_start_shard(state: BeaconState, |
| committee_config: CommitteeConfig, | ||
| registry_change: bool=False) -> Iterable[Tuple[Sequence[ValidatorIndex], Shard]]: | ||
| def get_beacon_proposer_index(state: BeaconState, | ||
| committee_config: CommitteeConfig) -> ValidatorIndex: |
There was a problem hiding this comment.
| committee_config: CommitteeConfig) -> ValidatorIndex: | |
| config: CommitteeConfig) -> ValidatorIndex: |
| state: 'BeaconState', | ||
| attestation_data: 'AttestationData', | ||
| committee_config: CommitteeConfig) -> Iterable[ValidatorIndex]: | ||
| def _get_shuffled_index(index: int, |
There was a problem hiding this comment.
Rename to compute_shuffled_index?
| state: 'BeaconState', | ||
| attestation_data: 'AttestationData', | ||
| committee_config: CommitteeConfig) -> Iterable[ValidatorIndex]: | ||
| def _get_shuffled_index(index: int, |
There was a problem hiding this comment.
| def _get_shuffled_index(index: int, | |
| def _get_shuffled_index(index: ValidatorIndex, |
| attestation_data.shard, | ||
| ) | ||
| f"The given `index_count` ({index_count}) should be equal to or less than " | ||
| f"`MAX_INDEX_COUNT` ({MAX_INDEX_COUNT}" | ||
| ) |
There was a problem hiding this comment.
This assertion has been removed from the spec.
| ) | ||
| byte = h[(hash_pos % 256) // 8] | ||
| bit = (byte >> (hash_pos % 8)) % 2 | ||
| new_index = flip if bit else new_index |
There was a problem hiding this comment.
Some spec renames nitpick:
hash_pos->positionh->source
ralexstokes
force-pushed
the
validator-spec-updates
branch
from
6400927 to
8f45a0d
Compare
There are some tests that expect them to exist. This commit is a temporary stop gap until a cleaner design is implemented.
The rest of the cache can cheaply be derived from the data that is now persisted on disk.
remove attestation.slot
ralexstokes
force-pushed
the
validator-spec-updates
branch
from
8f45a0d to
5750a2e
Compare
What was wrong?
This PR contains the updates for almost v0.8. Given that we are skipping over v0.6 and v0.7, there are quite a few changes here.
This PR targets the spec repo as of this commit: ethereum/consensus-specs@b007d5a
I'll complete the work on this branch and then think about about how to make it more digestible for review.
How was it fixed?
Updating all the code according to the latest spec.
To-Do
Cute Animal Picture