diff --git a/.circleci/config.yml b/.circleci/config.yml index 19ab1543aa..60d26e8964 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -112,6 +112,15 @@ jobs: - run: name: Check table of contents command: sudo npm install -g doctoc && make check_toc + codespell: + docker: + - image: circleci/python:3.6 + working_directory: ~/specs-repo + steps: + - checkout + - run: + name: Check codespell + command: pip install codespell --user && make codespell lint: docker: - image: circleci/python:3.6 @@ -158,6 +167,7 @@ workflows: requires: - install_pyspec_test - table_of_contents + - codespell - lint: requires: - test diff --git a/Makefile b/Makefile index 2cdb1021ff..2b165ad7d6 100644 --- a/Makefile +++ b/Makefile @@ -77,6 +77,9 @@ check_toc: $(MARKDOWN_FILES:=.toc) diff -q $* $*.tmp && \ rm $*.tmp +codespell: + codespell . --skip ./.git -I .codespell-whitelist + lint: $(PY_SPEC_ALL_TARGETS) cd $(PY_SPEC_DIR); . venv/bin/activate; \ flake8 --ignore=E252,W504,W503 --max-line-length=120 ./eth2spec \ diff --git a/scripts/build_spec.py b/scripts/build_spec.py index cca5a1bf9f..d5689b0828 100644 --- a/scripts/build_spec.py +++ b/scripts/build_spec.py @@ -252,7 +252,7 @@ def combine_ssz_objects(old_objects: Dict[str, str], new_objects: Dict[str, str] return old_objects -# inserts are handeled the same way as functions +# inserts are handled the same way as functions combine_inserts = combine_functions diff --git a/specs/core/0_beacon-chain.md b/specs/core/0_beacon-chain.md index e7c62d342b..ab1ea2c081 100644 --- a/specs/core/0_beacon-chain.md +++ b/specs/core/0_beacon-chain.md @@ -560,7 +560,7 @@ def int_to_bytes(n: uint64, length: uint64) -> bytes: ```python def bytes_to_int(data: bytes) -> uint64: """ - Return the integer deserialization of ``data`` intepreted as ``ENDIANNESS``-endian. + Return the integer deserialization of ``data`` interpreted as ``ENDIANNESS``-endian. """ return int.from_bytes(data, ENDIANNESS) ``` diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 84539713dd..91c5591738 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -53,10 +53,10 @@ It consists of four main sections: - [The discovery domain: discv5](#the-discovery-domain-discv5) - [Integration into libp2p stacks](#integration-into-libp2p-stacks) - [ENR structure](#enr-structure) + - [Attestation subnet bitfield](#attestation-subnet-bitfield) - [Interop](#interop-5) - [Mainnet](#mainnet-5) - [Topic advertisement](#topic-advertisement) - - [Interop](#interop-6) - [Mainnet](#mainnet-6) - [Design decision rationale](#design-decision-rationale) - [Transport](#transport-1) @@ -81,6 +81,7 @@ It consists of four main sections: - [How do we upgrade gossip channels (e.g. changes in encoding, compression)?](#how-do-we-upgrade-gossip-channels-eg-changes-in-encoding-compression) - [Why must all clients use the same gossip topic instead of one negotiated between each peer pair?](#why-must-all-clients-use-the-same-gossip-topic-instead-of-one-negotiated-between-each-peer-pair) - [Why are the topics strings and not hashes?](#why-are-the-topics-strings-and-not-hashes) + - [Why are we overriding the default libp2p pubsub `message-id`?](#why-are-we-overriding-the-default-libp2p-pubsub-message-id) - [Why are there `ATTESTATION_SUBNET_COUNT` attestation subnets?](#why-are-there-attestation_subnet_count-attestation-subnets) - [Why are attestations limited to be broadcast on gossip channels within `SLOTS_PER_EPOCH` slots?](#why-are-attestations-limited-to-be-broadcast-on-gossip-channels-within-slots_per_epoch-slots) - [Why are aggregate attestations broadcast to the global topic as `AggregateAndProof`s rather than just as `Attestation`s?](#why-are-aggregate-attestations-broadcast-to-the-global-topic-as-aggregateandproofs-rather-than-just-as-attestations) @@ -212,6 +213,13 @@ Topics are plain UTF-8 strings and are encoded on the wire as determined by prot Each gossipsub [message](https://github.com/libp2p/go-libp2p-pubsub/blob/master/pb/rpc.proto#L17-L24) has a maximum size of `GOSSIP_MAX_SIZE`. Clients MUST reject (fail validation) messages that are over this size limit. Likewise, clients MUST NOT emit or propagate messages larger than this limit. +The `message-id` of a gossipsub message MUST be: + +```python + message-id: base64(SHA256(message.data)) +``` +where `base64` is the [URL-safe base64 alphabet](https://tools.ietf.org/html/rfc4648#section-3.2) with padding characters omitted. + The payload is carried in the `data` field of a gossipsub message, and varies depending on the topic: | Topic | Message Type | @@ -437,6 +445,8 @@ Clients SHOULD immediately disconnect from one another following the handshake a Once the handshake completes, the client with the lower `finalized_epoch` or `head_slot` (if the clients have equal `finalized_epoch`s) SHOULD request beacon blocks from its counterparty via the `BeaconBlocksByRange` request. +*Note*: Under abnormal network condition or after some rounds of `BeaconBlocksByRange` requests, the client might need to send `Status` request again to learn if the peer has a higher head. Implementers are free to implement such behavior in their own way. + #### Goodbye **Protocol ID:** ``/eth2/beacon_chain/req/goodbye/1/`` @@ -557,6 +567,14 @@ The Ethereum Node Record (ENR) for an Ethereum 2.0 client MUST contain the follo Specifications of these parameters can be found in the [ENR Specification](http://eips.ethereum.org/EIPS/eip-778). +#### Attestation subnet bitfield + +The ENR MAY contain an entry (`attnets`) signifying the attestation subnet bitfield with the following form to more easily discover peers participating in particular attestation gossip subnets. + +| Key | Value | +|:-------------|:-------------------------------------------------| +| `attnets` | SSZ `Bitvector[ATTESTATION_SUBNET_COUNT]` | + #### Interop In the interoperability testnet, all peers will support all capabilities defined in this document (gossip, full Req/Resp suite, discovery protocol), therefore the ENR record does not need to carry Eth2 capability information, as it would be superfluous. @@ -569,13 +587,11 @@ On mainnet, ENRs MUST include a structure enumerating the capabilities offered b ### Topic advertisement -#### Interop - -This feature will not be used in the interoperability testnet. - #### Mainnet -In mainnet, we plan to use discv5’s topic advertisement feature as a rendezvous facility for peers on shards (thus subscribing to the relevant gossipsub topics). +discv5's topic advertisement feature is not expected to be ready for mainnet launch of Phase 0. + +Once this feature is built out and stable, we expect to use topic advertisement as a rendezvous facility for peers on shards. Until then, the ENR [attestation subnet bitfield](#attestation-subnet-bitfield) will be used for discovery of peers on particular subnets. # Design decision rationale @@ -738,6 +754,16 @@ No security or privacy guarantees are lost as a result of choosing plaintext top Furthermore, the Eth2 topic names are shorter than their digest equivalents (assuming SHA-256 hash), so hashing topics would bloat messages unnecessarily. +## Why are we overriding the default libp2p pubsub `message-id`? + +For our current purposes, there is no need to address messages based on source peer, and it seems likely we might even override the message `from` to obfuscate the peer. By overriding the default `message-id` to use content-addressing we can filter unnecessary duplicates before hitting the application layer. + +Some examples of where messages could be duplicated: + +* A validator client connected to multiple beacon nodes publishing duplicate gossip messages +* Attestation aggregation strategies where clients partially aggregate attestations and propagate them. Partial aggregates could be duplicated +* Clients re-publishing seen messages + ### Why are there `ATTESTATION_SUBNET_COUNT` attestation subnets? Depending on the number of validators, it may be more efficient to group shard subnets and might provide better stability for the gossipsub channel. The exact grouping will be dependent on more involved network tests. This constant allows for more flexibility in setting up the network topology for attestation aggregation (as aggregation should happen on each subnet). The value is currently set to to be equal `MAX_COMMITTEES_PER_SLOT` until network tests indicate otherwise. @@ -764,9 +790,9 @@ The prohibition of unverified-block-gossiping extends to nodes that cannot verif ### How are we going to discover peers in a gossipsub topic? -Via discv5 topics. ENRs should not be used for this purpose, as they store identity, location, and capability information, not volatile [advertisements](#topic-advertisement). +In Phase 0, peers for attestation subnets will be found using the `attnets` entry in the ENR. -In the interoperability testnet, all peers will be subscribed to all global beacon chain topics, so discovering peers in specific shard topics will be unnecessary. +Although this method will be sufficient for early phases of Eth2, we aim to use the more appropriate discv5 topics for this and other similar tasks in the future. ENRs should ultimately not be used for this purpose. They are best suited to store identity, location, and capability information, rather than more volatile advertisements. ## Req/Resp diff --git a/specs/test_formats/ssz_generic/README.md b/specs/test_formats/ssz_generic/README.md index b6faa04afd..68bdbc15f5 100644 --- a/specs/test_formats/ssz_generic/README.md +++ b/specs/test_formats/ssz_generic/README.md @@ -150,7 +150,7 @@ Template: Data: -{container name}: Any of the container names listed below (exluding the `(Container)` python super type) +{container name}: Any of the container names listed below (excluding the `(Container)` python super type) ``` ```python diff --git a/specs/validator/0_beacon-chain-validator.md b/specs/validator/0_beacon-chain-validator.md index 76bcc3b7d8..341fb8e8c2 100644 --- a/specs/validator/0_beacon-chain-validator.md +++ b/specs/validator/0_beacon-chain-validator.md @@ -197,8 +197,8 @@ The beacon chain shufflings are designed to provide a minimum of 1 epoch lookahe Specifically a validator should: * Call `get_committee_assignment(state, next_epoch, validator_index)` when checking for next epoch assignments. * Join the pubsub topic -- `committee_index{committee_index % ATTESTATION_SUBNET_COUNT}_beacon_attestation`. - * If any current peers are subscribed to the topic, the validator simply sends `subscribe` messages for the new topic. - * If no current peers are subscribed to the topic, the validator must discover new peers on this topic. If "topic discovery" is available, use topic discovery to find peers that advertise subscription to the topic. If not, "guess and check" by connecting with a number of random new peers, persisting connections with peers subscribed to the topic and (potentially) dropping the new peers otherwise. + * For any current peer subscribed to the topic, the validator simply sends a `subscribe` message for the new topic. + * If an _insufficient_ number of current peers are subscribed to the topic, the validator must discover new peers on this topic. Via the discovery protocol, find peers with an ENR containing the `attnets` entry such that `ENR["attnets"][committee_index % ATTESTATION_SUBNET_COUNT] == True`. ## Beacon chain responsibilities @@ -443,7 +443,11 @@ Where ## Phase 0 attestation subnet stability -Because Phase 0 does not have shards and thus does not have Shard Committees, there is no stable backbone to the attestation subnets (`committee_index{subnet_id}_beacon_attestation`). To provide this stability, each validator must randomly select and remain subscribed to `RANDOM_SUBNETS_PER_VALIDATOR` attestation subnets. The lifetime of each random subscription should be a random number of epochs between `EPOCHS_PER_RANDOM_SUBNET_SUBSCRIPTION` and `2 * EPOCHS_PER_RANDOM_SUBNET_SUBSCRIPTION]`. +Because Phase 0 does not have shards and thus does not have Shard Committees, there is no stable backbone to the attestation subnets (`committee_index{subnet_id}_beacon_attestation`). To provide this stability, each validator must: + +* Randomly select and remain subscribed to `RANDOM_SUBNETS_PER_VALIDATOR` attestation subnets +* Maintain advertisement of the randomly selected subnets in their node's ENR `attnets` entry by setting the randomly selected `subnet_id` bits to `True` (e.g. `ENR["attnets"][subnet_id] = True`) for all persistent attestation subnets +* Set the lifetime of each random subscription to a random number of epochs between `EPOCHS_PER_RANDOM_SUBNET_SUBSCRIPTION` and `2 * EPOCHS_PER_RANDOM_SUBNET_SUBSCRIPTION]`. At the end of life for a subscription, select a new random subnet, update subnet subscriptions, and publish an updated ENR ## How to avoid slashing diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py index 98a6e25e5f..dba48ca641 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py @@ -262,7 +262,7 @@ def test_att1_duplicate_index_normal_signed(spec, state): indices.pop(1) # remove an index, make room for the additional duplicate index. attester_slashing.attestation_1.attesting_indices = sorted(indices) - # sign it, the signature will be valid for a single occurence. If the transition accidentally ignores the duplicate. + # The signature will be valid for a single occurrence. If the transition accidentally ignores the duplicate. sign_indexed_attestation(spec, state, attester_slashing.attestation_1) indices.append(indices[0]) # add one of the indices a second time @@ -282,7 +282,7 @@ def test_att2_duplicate_index_normal_signed(spec, state): indices.pop(2) # remove an index, make room for the additional duplicate index. attester_slashing.attestation_2.attesting_indices = sorted(indices) - # sign it, the signature will be valid for a single occurence. If the transition accidentally ignores the duplicate. + # The signature will be valid for a single occurrence. If the transition accidentally ignores the duplicate. sign_indexed_attestation(spec, state, attester_slashing.attestation_2) indices.append(indices[1]) # add one of the indices a second time diff --git a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py index 7d844b63b7..b4fc46b7db 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py @@ -155,7 +155,7 @@ def test_duplicate_attestation(spec, state): next_epoch(spec, single_state) next_epoch(spec, dup_state) - # Run non-duplicate inclusion rewards for comparision. Do not yield test vectors + # Run non-duplicate inclusion rewards for comparison. Do not yield test vectors for _ in run_process_rewards_and_penalties(spec, single_state): pass diff --git a/test_libs/pyspec/eth2spec/test/sanity/test_blocks.py b/test_libs/pyspec/eth2spec/test/sanity/test_blocks.py index c2f980ba07..c32f4c5834 100644 --- a/test_libs/pyspec/eth2spec/test/sanity/test_blocks.py +++ b/test_libs/pyspec/eth2spec/test/sanity/test_blocks.py @@ -83,7 +83,7 @@ def test_invalid_state_root(spec, state): expect_assertion_error(lambda: spec.state_transition(state, signed_block)) - yield 'blocks', [block] + yield 'blocks', [signed_block] yield 'post', None @@ -91,6 +91,8 @@ def test_invalid_state_root(spec, state): @spec_state_test @always_bls def test_zero_block_sig(spec, state): + yield 'pre', state + block = build_empty_block_for_next_slot(spec, state) invalid_signed_block = spec.SignedBeaconBlock(message=block) expect_assertion_error(lambda: spec.state_transition(state, invalid_signed_block)) @@ -103,6 +105,8 @@ def test_zero_block_sig(spec, state): @spec_state_test @always_bls def test_invalid_block_sig(spec, state): + yield 'pre', state + block = build_empty_block_for_next_slot(spec, state) invalid_signed_block = spec.SignedBeaconBlock( message=block,