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.

Sign up for GitHub

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

Non-substantive typing clean-up #2036

Merged
merged 1 commit into from
Sep 10, 2020
Merged

Non-substantive typing clean-up #2036

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def get_spec(file_name: str) -> SpecObject:
from typing import (
Any, Dict, Set, Sequence, NewType, Tuple, TypeVar, Callable, Optional
)
from typing import List as PyList

from dataclasses import (
dataclass,
Expand Down
6 changes: 3 additions & 3 deletions specs/phase0/beacon-chain.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ def compute_committee(indices: Sequence[ValidatorIndex],
Return the committee corresponding to ``indices``, ``seed``, ``index``, and committee ``count``.
"""
start = (len(indices) * index) // count
end = (len(indices) * (index + 1)) // count
end = (len(indices) * uint64(index + 1)) // count
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this wouldn't be necessary anymore with the changes to remerkleable? uint64 + int -> uint64?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message (#1707) was error: Need type annotation for 'end'. I think mypy couldn't determine the type here with static checks.

I can change it to end: uint64 = (len(indices) * (index + 1)) // count alternatively.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it works when you uint64(len(indices)) instead? I find it weird to coerce the result, instead of the inputs.

return [indices[compute_shuffled_index(uint64(i), uint64(len(indices)), seed)] for i in range(start, end)]
```

Expand Down Expand Up @@ -1454,7 +1454,7 @@ def get_inclusion_delay_deltas(state: BeaconState) -> Tuple[Sequence[Gwei], Sequ
if index in get_attesting_indices(state, a.data, a.aggregation_bits)
], key=lambda a: a.inclusion_delay)
rewards[attestation.proposer_index] += get_proposer_reward(state, index)
max_attester_reward = get_base_reward(state, index) - get_proposer_reward(state, index)
max_attester_reward = Gwei(get_base_reward(state, index) - get_proposer_reward(state, index))
rewards[index] += Gwei(max_attester_reward // attestation.inclusion_delay)

# No penalties associated with inclusion delay
Expand Down Expand Up @@ -1574,7 +1574,7 @@ def process_final_updates(state: BeaconState) -> None:
# Update effective balances with hysteresis
for index, validator in enumerate(state.validators):
balance = state.balances[index]
HYSTERESIS_INCREMENT = EFFECTIVE_BALANCE_INCREMENT // HYSTERESIS_QUOTIENT
HYSTERESIS_INCREMENT = uint64(EFFECTIVE_BALANCE_INCREMENT // HYSTERESIS_QUOTIENT)
DOWNWARD_THRESHOLD = HYSTERESIS_INCREMENT * HYSTERESIS_DOWNWARD_MULTIPLIER
UPWARD_THRESHOLD = HYSTERESIS_INCREMENT * HYSTERESIS_UPWARD_MULTIPLIER
if (
Expand Down
4 changes: 2 additions & 2 deletions specs/phase0/validator.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def get_committee_assignment(state: BeaconState,
* ``assignment[2]`` is the slot at which the committee is assigned
Return None if no assignment.
"""
next_epoch = get_current_epoch(state) + 1
next_epoch = Epoch(get_current_epoch(state) + 1)
assert epoch <= next_epoch

start_slot = compute_start_slot_at_epoch(epoch)
Expand Down Expand Up @@ -460,7 +460,7 @@ def compute_subnet_for_attestation(committees_per_slot: uint64, slot: Slot, comm
Compute the correct subnet for an attestation for Phase 0.
Note, this mimics expected Phase 1 behavior where attestations will be mapped to their shard subnet.
"""
slots_since_epoch_start = slot % SLOTS_PER_EPOCH
slots_since_epoch_start = uint64(slot % SLOTS_PER_EPOCH)
committees_since_epoch_start = committees_per_slot * slots_since_epoch_start

return uint64((committees_since_epoch_start + committee_index) % ATTESTATION_SUBNET_COUNT)
Expand Down
24 changes: 12 additions & 12 deletions specs/phase1/beacon-chain.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,20 +105,20 @@ Configuration is not namespaced. Instead it is strictly an extension;

| Name | Value |
| - | - |
| `MAX_SHARDS` | `2**10` (= 1024) |
| `INITIAL_ACTIVE_SHARDS` | `2**6` (= 64) |
| `LIGHT_CLIENT_COMMITTEE_SIZE` | `2**7` (= 128) |
| `GASPRICE_ADJUSTMENT_COEFFICIENT` | `2**3` (= 8) |
| `MAX_SHARDS` | `uint64(2**10)` (= 1024) |
| `INITIAL_ACTIVE_SHARDS` | `uint64(2**6)` (= 64) |
| `LIGHT_CLIENT_COMMITTEE_SIZE` | `uint64(2**7)` (= 128) |
| `GASPRICE_ADJUSTMENT_COEFFICIENT` | `uint64(2**3)` (= 8) |

### Shard block configs

| Name | Value | Unit |
| - | - | - |
| `MAX_SHARD_BLOCK_SIZE` | `2**20` (= 1,048,576) | bytes |
| `TARGET_SHARD_BLOCK_SIZE` | `2**18` (= 262,144) | bytes |
| `SHARD_BLOCK_OFFSETS` | `[1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233]` | - |
| `MAX_SHARD_BLOCK_SIZE` | `uint64(2**20)` (= 1,048,576) | bytes |
| `TARGET_SHARD_BLOCK_SIZE` | `uint64(2**18)` (= 262,144) | bytes |
| `SHARD_BLOCK_OFFSETS` | `List[uint64, 12]([1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233])` | - |
| `MAX_SHARD_BLOCKS_PER_ATTESTATION` | `len(SHARD_BLOCK_OFFSETS)` | - |
| `BYTES_PER_CUSTODY_CHUNK` | `2**12` (= 4,096) | bytes |
| `BYTES_PER_CUSTODY_CHUNK` | `uint64(2**12)` (= 4,096) | bytes |
| `CUSTODY_RESPONSE_DEPTH` | `ceillog2(MAX_SHARD_BLOCK_SIZE // BYTES_PER_CUSTODY_CHUNK)` | - |

### Gwei values
Expand Down Expand Up @@ -504,7 +504,7 @@ def compute_committee_source_epoch(epoch: Epoch, period: uint64) -> Epoch:
"""
Return the source epoch for computing the committee.
"""
source_epoch = epoch - epoch % period
source_epoch = Epoch(epoch - epoch % period)
if source_epoch >= period:
source_epoch -= period # `period` epochs lookahead
return source_epoch
Expand Down Expand Up @@ -575,7 +575,7 @@ def get_light_client_committee(beacon_state: BeaconState, epoch: Epoch) -> Seque
return compute_committee(
indices=active_validator_indices,
seed=seed,
index=0,
index=uint64(0),
count=get_active_shard_count(beacon_state),
)[:LIGHT_CLIENT_COMMITTEE_SIZE]
```
Expand All @@ -601,10 +601,10 @@ def get_committee_count_delta(state: BeaconState, start_slot: Slot, stop_slot: S
"""
Return the sum of committee counts in range ``[start_slot, stop_slot)``.
"""
return sum(
return uint64(sum(
get_committee_count_per_slot(state, compute_epoch_at_slot(Slot(slot)))
for slot in range(start_slot, stop_slot)
)
))
```

#### `get_start_shard`
Expand Down
42 changes: 21 additions & 21 deletions specs/phase1/custody-game.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,40 +56,40 @@ This document details the beacon chain additions and changes in Phase 1 of Ether

| Name | Value | Unit |
| - | - | - |
| `CUSTODY_PRIME` | `2 ** 256 - 189` | - |
| `CUSTODY_SECRETS` | `3` | - |
| `BYTES_PER_CUSTODY_ATOM` | `32` | bytes |
| `CUSTODY_PROBABILITY_EXPONENT` | `10` | - |
| `CUSTODY_PRIME` | `int(2 ** 256 - 189)` | - |
| `CUSTODY_SECRETS` | `uint64(3)` | - |
| `BYTES_PER_CUSTODY_ATOM` | `uint64(32)` | bytes |
| `CUSTODY_PROBABILITY_EXPONENT` | `uint64(10)` | - |

## Configuration

### Time parameters

| Name | Value | Unit | Duration |
| - | - | :-: | :-: |
| `RANDAO_PENALTY_EPOCHS` | `2**1` (= 2) | epochs | 12.8 minutes |
| `EARLY_DERIVED_SECRET_PENALTY_MAX_FUTURE_EPOCHS` | `2**15` (= 32,768) | epochs | ~146 days |
| `EPOCHS_PER_CUSTODY_PERIOD` | `2**14` (= 16,384) | epochs | ~73 days |
| `CUSTODY_PERIOD_TO_RANDAO_PADDING` | `2**11` (= 2,048) | epochs | ~9 days |
| `MAX_CHUNK_CHALLENGE_DELAY` | `2**15` (= 32,768) | epochs | ~146 days |
| `RANDAO_PENALTY_EPOCHS` | `uint64(2**1)` (= 2) | epochs | 12.8 minutes |
| `EARLY_DERIVED_SECRET_PENALTY_MAX_FUTURE_EPOCHS` | `uint64(2**15)` (= 32,768) | epochs | ~146 days |
| `EPOCHS_PER_CUSTODY_PERIOD` | `uint64(2**14)` (= 16,384) | epochs | ~73 days |
| `CUSTODY_PERIOD_TO_RANDAO_PADDING` | `uint64(2**11)` (= 2,048) | epochs | ~9 days |
| `MAX_CHUNK_CHALLENGE_DELAY` | `uint64(2**15)` (= 32,768) | epochs | ~146 days |

### Max operations per block

| Name | Value |
| - | - |
| `MAX_CUSTODY_CHUNK_CHALLENGE_RECORDS` | `2**20` (= 1,048,576) |
| `MAX_CUSTODY_KEY_REVEALS` | `2**8` (= 256) |
| `MAX_EARLY_DERIVED_SECRET_REVEALS` | `2**0` (= 1) |
| `MAX_CUSTODY_CHUNK_CHALLENGES` | `2**2` (= 4) |
| `MAX_CUSTODY_CHUNK_CHALLENGE_RESPONSES` | `2**4` (= 16) |
| `MAX_CUSTODY_SLASHINGS` | `2**0` (= 1) |
| `MAX_CUSTODY_CHUNK_CHALLENGE_RECORDS` | `uint64(2**20)` (= 1,048,576) |
| `MAX_CUSTODY_KEY_REVEALS` | `uint64(2**8)` (= 256) |
| `MAX_EARLY_DERIVED_SECRET_REVEALS` | `uint64(2**0)` (= 1) |
| `MAX_CUSTODY_CHUNK_CHALLENGES` | `uint64(2**2)` (= 4) |
| `MAX_CUSTODY_CHUNK_CHALLENGE_RESPONSES` | `uint64(2**4)` (= 16) |
| `MAX_CUSTODY_SLASHINGS` | `uint64(2**0)` (= 1) |

### Reward and penalty quotients

| Name | Value |
| - | - |
| `EARLY_DERIVED_SECRET_REVEAL_SLOT_REWARD_MULTIPLE` | `2**1` (= 2) |
| `MINOR_REWARD_QUOTIENT` | `2**8` (= 256) |
| `EARLY_DERIVED_SECRET_REVEAL_SLOT_REWARD_MULTIPLE` | `uint64(2**1)` (= 2) |
| `MINOR_REWARD_QUOTIENT` | `uint64(2**8)` (= 256) |

## Data structures

Expand Down Expand Up @@ -265,12 +265,12 @@ def universal_hash_function(data_chunks: Sequence[bytes], secrets: Sequence[int]
### `compute_custody_bit`

```python
def compute_custody_bit(key: BLSSignature, data: ByteList[MAX_SHARD_BLOCK_SIZE]) -> bit:
def compute_custody_bit(key: BLSSignature, data: ByteList) -> bit:
custody_atoms = get_custody_atoms(data)
secrets = get_custody_secrets(key)
uhf = universal_hash_function(custody_atoms, secrets)
legendre_bits = [legendre_bit(uhf + secrets[0] + i, CUSTODY_PRIME) for i in range(CUSTODY_PROBABILITY_EXPONENT)]
return all(legendre_bits)
return bit(all(legendre_bits))
```

### `get_randao_epoch_for_custody_period`
Expand Down Expand Up @@ -316,7 +316,7 @@ def process_chunk_challenge(state: BeaconState, challenge: CustodyChunkChallenge
# Verify the attestation
assert is_valid_indexed_attestation(state, get_indexed_attestation(state, challenge.attestation))
# Verify it is not too late to challenge the attestation
max_attestation_challenge_epoch = challenge.attestation.data.target.epoch + MAX_CHUNK_CHALLENGE_DELAY
max_attestation_challenge_epoch = Epoch(challenge.attestation.data.target.epoch + MAX_CHUNK_CHALLENGE_DELAY)
assert get_current_epoch(state) <= max_attestation_challenge_epoch
# Verify it is not too late to challenge the responder
responder = state.validators[challenge.responder_index]
Expand Down Expand Up @@ -438,7 +438,7 @@ def process_early_derived_secret_reveal(state: BeaconState, reveal: EarlyDerived
Note that this function mutates ``state``.
"""
revealed_validator = state.validators[reveal.revealed_index]
derived_secret_location = reveal.epoch % EARLY_DERIVED_SECRET_PENALTY_MAX_FUTURE_EPOCHS
derived_secret_location = uint64(reveal.epoch % EARLY_DERIVED_SECRET_PENALTY_MAX_FUTURE_EPOCHS)

assert reveal.epoch >= get_current_epoch(state) + RANDAO_PENALTY_EPOCHS
assert reveal.epoch < get_current_epoch(state) + EARLY_DERIVED_SECRET_PENALTY_MAX_FUTURE_EPOCHS
Expand Down
8 changes: 4 additions & 4 deletions specs/phase1/validator.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,9 @@ def get_shard_transition_fields(
shard: Shard,
shard_blocks: Sequence[SignedShardBlock],
) -> Tuple[Sequence[uint64], Sequence[Root], Sequence[ShardState]]:
shard_states = []
shard_data_roots = []
shard_block_lengths = []
shard_block_lengths = [] # type: PyList[uint64]
shard_data_roots = [] # type: PyList[Root]
shard_states = [] # type: PyList[ShardState]
Comment on lines +284 to +286
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyList was not meant to make it into the spec. More type safety is nice, but I am hesitant to introduce this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hesitant too, and intentionally used type comments instead of shard_block_lengths: PyList[uint64] = []. 😅 But I understand your concerns.

A "PyList is okay" argument is that we already have Python Set, Dict, and Tuple in the specs.

Would renaming PyList to Array be better?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyList is okay then I guess. And sorry for nitpicking, it's just a style thing that we tried to avoid previously, but typing is more important.


shard_state = beacon_state.shard_states[shard]
shard_block_slots = [shard_block.message.slot for shard_block in shard_blocks]
Expand All @@ -301,7 +301,7 @@ def get_shard_transition_fields(
shard_state = shard_state.copy()
process_shard_block(shard_state, shard_block.message)
shard_states.append(shard_state)
shard_block_lengths.append(len(shard_block.message.body))
shard_block_lengths.append(uint64(len(shard_block.message.body)))

return shard_block_lengths, shard_data_roots, shard_states
```
Expand Down