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

Make lightclient patch pass the linter #2133

Merged
merged 10 commits into from
Nov 17, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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 .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ eth2.0-spec-tests/
# Dynamically built from Markdown spec
tests/core/pyspec/eth2spec/phase0/
tests/core/pyspec/eth2spec/phase1/
tests/core/pyspec/eth2spec/lightclient/

# coverage reports
.htmlcov
Expand Down
5 changes: 3 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ GENERATOR_VENVS = $(patsubst $(GENERATOR_DIR)/%, $(GENERATOR_DIR)/%venv, $(GENER
# To check generator matching:
#$(info $$GENERATOR_TARGETS is [${GENERATOR_TARGETS}])

MARKDOWN_FILES = $(wildcard $(SPEC_DIR)/phase0/*.md) $(wildcard $(SPEC_DIR)/phase1/*.md) $(wildcard $(SSZ_DIR)/*.md) $(wildcard $(SPEC_DIR)/networking/*.md) $(wildcard $(SPEC_DIR)/validator/*.md)
MARKDOWN_FILES = $(wildcard $(SPEC_DIR)/phase0/*.md) $(wildcard $(SPEC_DIR)/phase1/*.md) $(wildcard $(SPEC_DIR)/lightclient/*.md) $(wildcard $(SSZ_DIR)/*.md) $(wildcard $(SPEC_DIR)/networking/*.md) $(wildcard $(SPEC_DIR)/validator/*.md)

COV_HTML_OUT=.htmlcov
COV_INDEX_FILE=$(PY_SPEC_DIR)/$(COV_HTML_OUT)/index.html
Expand Down Expand Up @@ -49,6 +49,7 @@ partial_clean:
rm -rf $(DEPOSIT_CONTRACT_TESTER_DIR)/.pytest_cache
rm -rf $(PY_SPEC_DIR)/phase0
rm -rf $(PY_SPEC_DIR)/phase1
rm -rf $(PY_SPEC_DIR)/lightclient
rm -rf $(PY_SPEC_DIR)/$(COV_HTML_OUT)
rm -rf $(PY_SPEC_DIR)/.coverage
rm -rf $(PY_SPEC_DIR)/test-reports
Expand Down Expand Up @@ -112,7 +113,7 @@ codespell:
lint: pyspec
. venv/bin/activate; cd $(PY_SPEC_DIR); \
flake8 --config $(LINTER_CONFIG_FILE) ./eth2spec \
&& mypy --config-file $(LINTER_CONFIG_FILE) -p eth2spec.phase0 -p eth2spec.phase1
&& mypy --config-file $(LINTER_CONFIG_FILE) -p eth2spec.phase0 -p eth2spec.phase1 -p eth2spec.lightclient

lint_generators: pyspec
. venv/bin/activate; cd $(TEST_GENERATORS_DIR); \
Expand Down
43 changes: 43 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,40 @@ def get_spec(file_name: str) -> SpecObject:

CONFIG_NAME = 'mainnet'
'''
LIGHTCLIENT_IMPORT = '''from eth2spec.phase0 import spec as phase0
from eth2spec.config.config_util import apply_constants_config
from typing import (
Any, Dict, Set, Sequence, NewType, Tuple, TypeVar, Callable, Optional
)

from dataclasses import (
dataclass,
field,
)

from lru import LRU

from eth2spec.utils.ssz.ssz_impl import hash_tree_root, copy, uint_to_bytes
from eth2spec.utils.ssz.ssz_typing import (
View, boolean, Container, List, Vector, uint8, uint32, uint64,
Bytes1, Bytes4, Bytes32, Bytes48, Bytes96, Bitlist, Bitvector,
)
from eth2spec.utils import bls

from eth2spec.utils.hash_function import hash

# Whenever lightclient is loaded, make sure we have the latest phase0
from importlib import reload
reload(phase0)


SSZVariableName = str
GeneralizedIndex = NewType('GeneralizedIndex', int)
SSZObject = TypeVar('SSZObject', bound=View)

CONFIG_NAME = 'mainnet'
'''

SUNDRY_CONSTANTS_FUNCTIONS = '''
def ceillog2(x: int) -> uint64:
if x < 1:
Expand Down Expand Up @@ -351,6 +385,7 @@ def combine_spec_objects(spec0: SpecObject, spec1: SpecObject) -> SpecObject:
fork_imports = {
'phase0': PHASE0_IMPORTS,
'phase1': PHASE1_IMPORTS,
'lightclient': LIGHTCLIENT_IMPORT,
}


Expand Down Expand Up @@ -417,6 +452,14 @@ def finalize_options(self):
specs/phase1/shard-fork-choice.md
specs/phase1/validator.md
"""
elif self.spec_fork == "lightclient":
self.md_doc_paths = """
specs/phase0/beacon-chain.md
specs/phase0/fork-choice.md
specs/phase0/validator.md
specs/phase0/weak-subjectivity.md
specs/lightclient/beacon-chain.md
"""
else:
raise Exception('no markdown files specified, and spec fork "%s" is unknown', self.spec_fork)

Expand Down
56 changes: 52 additions & 4 deletions specs/lightclient/beacon-chain.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Table of contents

<!-- TOC -->
<!-- START doctoc generated TOC please keep comment here to allow auto update -->
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
**Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)*

- [Introduction](#introduction)
- [Custom types](#custom-types)
- [Constants](#constants)
Expand All @@ -24,8 +29,11 @@
- [`get_sync_committee`](#get_sync_committee)
- [Block processing](#block-processing)
- [Sync committee processing](#sync-committee-processing)
- [Epoch processing](#epoch-transition)
- [Final updates](#updates-updates)
- [Epoch processing](#epoch-processing)
- [Final updates](#final-updates)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->
<!-- /TOC -->

## Introduction

Expand Down Expand Up @@ -70,15 +78,55 @@ This is a standalone beacon chain patch adding light client support via sync com
#### `BeaconBlockBody`

```python
class BeaconBlockBody(phase0.BeaconBlockBody):
Copy link
Collaborator

Choose a reason for hiding this comment

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

class BeaconBlockBody(phase0.BeaconBlockBody): is so much cleaner than repeating all the properties!

@protolambda: Can we do some Python magic here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

With python we can do a lot, but the question is if we should. Being implicit would mean we need to spec appending of fields. And then sometimes appending is not the approach we want. Or functionality is not sequential, and we remove a field.

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason why I did the inheriting from phase0 in the original PR instead of repeating properties is not just to cut down the number of lines of code, it's also to make the patch to the spec truly not sequential (ie. if it's implemented after sharding it could still be included as is). This would make it easier for us to work on light client stuff, the merge and sharding in parallel. So it would be really nice if we can make the inheritance work!

Copy link
Collaborator

Choose a reason for hiding this comment

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

That sounds great, but then do what we want some diamond structure? I.e. phase 0 type inherited by various phase 1 types, and then recombined in the complete phase 1 type? This becomes "the diamond problem" once we start overriding things in different types, which we then need to reconsolidate.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I don't support the concept of "phases" anymore, so... 😄

Realistically, once some patch gets implemented, we could just quickly rewrite the other patches to point to the previous patch as a base (eg. s/phase0./lightclient_fork./g), and when writing each individual patch make sure that it is valid under any combination of the other patches.

This does of course assume that the absolute number of patches that are treated this way (as opposed to just being PRs to existing spec as all the beacon chain changes pre-launch were) needs to be very small.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, implemented it and opened a PR to my SSZ library that we use here in the spec, with support for all this (inheritance, mixins, diamonds): protolambda/remerkleable#8

Please give it a review, and let's discuss on next spec call. It seems small, but it's quite a step away from bare struct definitions.

class BeaconBlockBody(Container):
randao_reveal: BLSSignature
eth1_data: Eth1Data # Eth1 data vote
graffiti: Bytes32 # Arbitrary data
# Operations
proposer_slashings: List[ProposerSlashing, MAX_PROPOSER_SLASHINGS]
attester_slashings: List[AttesterSlashing, MAX_ATTESTER_SLASHINGS]
attestations: List[Attestation, MAX_ATTESTATIONS]
deposits: List[Deposit, MAX_DEPOSITS]
voluntary_exits: List[SignedVoluntaryExit, MAX_VOLUNTARY_EXITS]
# Light client
sync_committee_bits: Bitlist[MAX_SYNC_COMMITTEE_SIZE]
sync_committee_signature: BLSSignature
```

#### `BeaconState`

```python
class BeaconState(phase0.BeaconState):
class BeaconState(Container):
# Versioning
genesis_time: uint64
genesis_validators_root: Root
slot: Slot
fork: Fork
# History
latest_block_header: BeaconBlockHeader
block_roots: Vector[Root, SLOTS_PER_HISTORICAL_ROOT]
state_roots: Vector[Root, SLOTS_PER_HISTORICAL_ROOT]
historical_roots: List[Root, HISTORICAL_ROOTS_LIMIT]
# Eth1
eth1_data: Eth1Data
eth1_data_votes: List[Eth1Data, EPOCHS_PER_ETH1_VOTING_PERIOD * SLOTS_PER_EPOCH]
eth1_deposit_index: uint64
# Registry
validators: List[Validator, VALIDATOR_REGISTRY_LIMIT]
balances: List[Gwei, VALIDATOR_REGISTRY_LIMIT]
# Randomness
randao_mixes: Vector[Bytes32, EPOCHS_PER_HISTORICAL_VECTOR]
# Slashings
slashings: Vector[Gwei, EPOCHS_PER_SLASHINGS_VECTOR] # Per-epoch sums of slashed effective balances
# Attestations
previous_epoch_attestations: List[PendingAttestation, MAX_ATTESTATIONS * SLOTS_PER_EPOCH]
current_epoch_attestations: List[PendingAttestation, MAX_ATTESTATIONS * SLOTS_PER_EPOCH]
# Finality
justification_bits: Bitvector[JUSTIFICATION_BITS_LENGTH] # Bit set for every recent justified epoch
previous_justified_checkpoint: Checkpoint # Previous epoch snapshot
current_justified_checkpoint: Checkpoint
finalized_checkpoint: Checkpoint
# Light client
current_sync_committee: SyncCommittee
next_sync_committee: SyncCommittee
```
Expand Down