From a4f8a77c027766af6cbc12f9f5cddc08fe0b85bc Mon Sep 17 00:00:00 2001 From: Martin Lundfall Date: Mon, 16 Dec 2019 12:55:18 +0100 Subject: [PATCH 01/11] Fix spelling errors found by codespell --- Makefile | 3 +++ scripts/build_spec.py | 2 +- specs/core/0_beacon-chain.md | 2 +- specs/test_formats/ssz_generic/README.md | 2 +- .../block_processing/test_process_attester_slashing.py | 4 ++-- .../epoch_processing/test_process_rewards_and_penalties.py | 2 +- 6 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 2cdb1021f..2bd87aed8 100644 --- a/Makefile +++ b/Makefile @@ -77,6 +77,9 @@ check_toc: $(MARKDOWN_FILES:=.toc) diff -q $* $*.tmp && \ rm $*.tmp +codespell: + ! codespell . --skip ./.git | grep -v 'disabled' + 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 cca5a1bf9..d5689b082 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 e7c62d342..ab1ea2c08 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/test_formats/ssz_generic/README.md b/specs/test_formats/ssz_generic/README.md index b6faa04af..68bdbc15f 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/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 98a6e25e5..19fdd04a2 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. + # sign it, 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. + # sign it, 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 7d844b63b..b4fc46b7d 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 From 5234e431ec71643e57da90825728d9a5db963054 Mon Sep 17 00:00:00 2001 From: Martin Lundfall Date: Mon, 16 Dec 2019 12:55:51 +0100 Subject: [PATCH 02/11] Add codespell to ci --- .circleci/config.yml | 10 ++++++++++ Makefile | 1 + 2 files changed, 11 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 19ab1543a..60d26e896 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 2bd87aed8..61fc34a52 100644 --- a/Makefile +++ b/Makefile @@ -78,6 +78,7 @@ check_toc: $(MARKDOWN_FILES:=.toc) rm $*.tmp codespell: + # Check codespell for errors, but disregard entries in .codespell-whitelist ! codespell . --skip ./.git | grep -v 'disabled' lint: $(PY_SPEC_ALL_TARGETS) From 21c8c58cb18c717b59b715c089a588501b27a5d4 Mon Sep 17 00:00:00 2001 From: Martin Lundfall Date: Mon, 16 Dec 2019 13:22:08 +0100 Subject: [PATCH 03/11] edit comment to make line shorter --- .../block_processing/test_process_attester_slashing.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 19fdd04a2..dba48ca64 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 occurrence. 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 occurrence. 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 From 798fadc3cb432ac6c260ecf456e58d3ca6ebbbe4 Mon Sep 17 00:00:00 2001 From: Martin Lundfall Date: Mon, 16 Dec 2019 14:11:05 +0100 Subject: [PATCH 04/11] Makefile: Use codespell as intended --- Makefile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 61fc34a52..2b165ad7d 100644 --- a/Makefile +++ b/Makefile @@ -78,8 +78,7 @@ check_toc: $(MARKDOWN_FILES:=.toc) rm $*.tmp codespell: - # Check codespell for errors, but disregard entries in .codespell-whitelist - ! codespell . --skip ./.git | grep -v 'disabled' + codespell . --skip ./.git -I .codespell-whitelist lint: $(PY_SPEC_ALL_TARGETS) cd $(PY_SPEC_DIR); . venv/bin/activate; \ From fa916323f0e520208d0f51f5153d73482904c763 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 17 Dec 2019 10:56:52 -0700 Subject: [PATCH 05/11] add basics for ENR bitfield --- specs/networking/p2p-interface.md | 9 +++++++++ specs/validator/0_beacon-chain-validator.md | 8 ++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 84539713d..8487bd3ba 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -53,6 +53,7 @@ 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) + - [Shard bitfield](#shard-bitfield) - [Interop](#interop-5) - [Mainnet](#mainnet-5) - [Topic advertisement](#topic-advertisement) @@ -557,6 +558,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). +#### Shard bitfield + +The ENR MAY contain an entry signifying the shard subnet bitfield with the following form to more easily discover peers participating in particular shard gossip subnets. + +| Key | Value | +|:-------------|:-------------------------------------------------| +| `shards` | SSZ `Bitvector[MAX_COMMITTEES_PER_SLOT]` | + #### 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. diff --git a/specs/validator/0_beacon-chain-validator.md b/specs/validator/0_beacon-chain-validator.md index 76bcc3b7d..f2a666dce 100644 --- a/specs/validator/0_beacon-chain-validator.md +++ b/specs/validator/0_beacon-chain-validator.md @@ -198,7 +198,7 @@ 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. + * If no current peers are subscribed to the topic, the validator must discover new peers on this topic. Find peers via the discovery protocol with an ENR containing the `shards` entry such that `ENR["shards"][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 `shards` entry by setting the randomly selected `subnet_id` bits to `True` (e.g. `ENR["shards"][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 From caffe8d720b5987ac6d794f9b00182a9439666e0 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Tue, 17 Dec 2019 16:25:30 -0700 Subject: [PATCH 06/11] update ENR to use attesation subnets instead of shards --- specs/networking/p2p-interface.md | 21 +++++++++------------ specs/validator/0_beacon-chain-validator.md | 8 ++++---- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 8487bd3ba..38e9b366b 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -53,11 +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) - - [Shard bitfield](#shard-bitfield) + - [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) @@ -558,13 +557,13 @@ 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). -#### Shard bitfield +#### Attestation subnet bitfield -The ENR MAY contain an entry signifying the shard subnet bitfield with the following form to more easily discover peers participating in particular shard gossip subnets. +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 | |:-------------|:-------------------------------------------------| -| `shards` | SSZ `Bitvector[MAX_COMMITTEES_PER_SLOT]` | +| `attnets` | SSZ `Bitvector[ATTESTATION_SUBNET_COUNT]` | #### Interop @@ -578,13 +577,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 @@ -773,9 +770,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/validator/0_beacon-chain-validator.md b/specs/validator/0_beacon-chain-validator.md index f2a666dce..341fb8e8c 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. Find peers via the discovery protocol with an ENR containing the `shards` entry such that `ENR["shards"][committee_index % ATTESTATION_SUBNET_COUNT] == True`. + * 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,10 +443,10 @@ 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 +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 `shards` entry by setting the randomly selected `subnet_id` bits to `True` (e.g. `ENR["shards"][subnet_id] = True`) for all persistent 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 From 8a03e6d291caad8e4adcd37b4ae2b935768310ba Mon Sep 17 00:00:00 2001 From: Age Manning Date: Wed, 18 Dec 2019 10:08:52 +1100 Subject: [PATCH 07/11] Add gossipsub message-id specification --- specs/networking/p2p-interface.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 38e9b366b..2220740d9 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -212,6 +212,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 | From 31d90ea7990ac90342a83df7b2fe154d57802f40 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Wed, 18 Dec 2019 14:43:13 -0700 Subject: [PATCH 08/11] add p2p faq on why message-id override --- specs/networking/p2p-interface.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 2220740d9..7305a8b59 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -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,7 +213,7 @@ 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: +The `message-id` of a gossipsub message MUST be: ```python message-id: base64(SHA256(message.data)) @@ -751,6 +752,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. From c4b23590d402f4e1bae811601a779359cf10e14e Mon Sep 17 00:00:00 2001 From: Chih Cheng Liang Date: Thu, 19 Dec 2019 18:46:15 +0800 Subject: [PATCH 09/11] Add a note on requesting STATUS again --- specs/networking/p2p-interface.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 38e9b366b..9bcfe6aae 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -437,6 +437,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 bad 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/`` From 2de5119cfe64eacc5310d9c512607c62499ab383 Mon Sep 17 00:00:00 2001 From: protolambda Date: Thu, 19 Dec 2019 17:31:58 +0100 Subject: [PATCH 10/11] fix two missing pre-states, and fix unsigned block --- test_libs/pyspec/eth2spec/test/sanity/test_blocks.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test_libs/pyspec/eth2spec/test/sanity/test_blocks.py b/test_libs/pyspec/eth2spec/test/sanity/test_blocks.py index c2f980ba0..c32f4c583 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, From e34d22e4f81e7dbcdaacebdf25a05e0a691428d6 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Thu, 19 Dec 2019 14:04:45 -0700 Subject: [PATCH 11/11] bad -> abnormal --- specs/networking/p2p-interface.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 9bcfe6aae..6bdcc1233 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -437,7 +437,7 @@ 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 bad 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. +*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