From 4937fa9b5860c145b74519b88065e6db97aeac27 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Sun, 8 Sep 2019 22:55:55 +1000 Subject: [PATCH 01/31] Network specification update --- specs/networking/p2p-interface.md | 53 +++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index c9bab8406..64f8a3ce8 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -113,7 +113,7 @@ This section outlines constants that are used in this spec. | Name | Value | Description | |---|---|---| -| `REQ_RESP_MAX_SIZE` | `TODO` | The maximum size of uncompressed req/resp messages that clients will allow. | +| `REQ_RESP_MAX_SIZE` | `2**22` (4194304, 4 MiB) | The maximum size of uncompressed req/resp messages that clients will allow. | | `SSZ_MAX_LIST_SIZE` | `TODO` | The maximum size of SSZ-encoded variable lists. | | `GOSSIP_MAX_SIZE` | `2**20` (= 1048576, 1 MiB) | The maximum size of uncompressed gossip messages. | | `SHARD_SUBNET_COUNT` | `TODO` | The number of shard subnets used in the gossipsub protocol. | @@ -301,7 +301,7 @@ Here, `result` represents the 1-byte response code. The token of the negotiated protocol ID specifies the type of encoding to be used for the req/resp interaction. Two values are possible at this time: -- `ssz`: the contents are [SSZ-encoded](#ssz-encoding). This encoding type MUST be supported by all clients. For objects containing a single field, only the field is SSZ-encoded not a container with a single field. For example, the `BeaconBlocks` response would be an SSZ-encoded list of `BeaconBlock`s. All SSZ-Lists in the Req/Resp domain will have a maximum list size of `SSZ_MAX_LIST_SIZE`. +- `ssz`: the contents are [SSZ-encoded](../simple-serialize.md). This encoding type MUST be supported by all clients. For objects containing a single field, only the field is SSZ-encoded not a container with a single field. For example, the `BeaconBlocksByRange` response would be an SSZ-encoded list of `BeaconBlock`s. All SSZ-Lists in the Req/Resp domain will have a maximum list size of `SSZ_MAX_LIST_SIZE`. - `ssz_snappy`: The contents are SSZ-encoded and then compressed with [Snappy](https://github.com/google/snappy). MAY be supported in the interoperability testnet; MUST be supported in mainnet. #### SSZ-encoding strategy (with or without Snappy) @@ -318,10 +318,10 @@ The [SimpleSerialize (SSZ) specification](../simple-serialize.md) outlines how o **Protocol ID:** ``/eth2/beacon_chain/req/hello/1/`` -**Content**: +Request, Response Content: ``` ( - fork_version: bytes4 + head_fork_version: bytes4 finalized_root: bytes32 finalized_epoch: uint64 head_root: bytes32 @@ -330,26 +330,28 @@ The [SimpleSerialize (SSZ) specification](../simple-serialize.md) outlines how o ``` The fields are: -- `fork_version`: The beacon_state `Fork` version. +- `head_fork_version`: The beacon_state `Fork` version. - `finalized_root`: The latest finalized root the node knows about. - `finalized_epoch`: The latest finalized epoch the node knows about. - `head_root`: The block hash tree root corresponding to the head of the chain as seen by the sending node. - `head_slot`: The slot corresponding to the `head_root`. -Clients exchange hello messages upon connection, forming a two-phase handshake. The first message the initiating client sends MUST be the hello message. In response, the receiving client MUST respond with its own hello message. +The dialing client MUST send a `Hello` request upon connection. + +This should be encoded as an SSZ-container. Clients SHOULD immediately disconnect from one another following the handshake above under the following conditions: -1. If `fork_version` doesn’t match the local fork version, since the client’s chain is on another fork. `fork_version` can also be used to segregate testnets. +1. If `head_fork_version` doesn’t match the expected fork version at the epoch of the `head_slot`, since the client’s chain is on another fork. `head_fork_version` can also be used to segregate testnets. 2. If the (`finalized_root`, `finalized_epoch`) shared by the peer is not in the client's chain at the expected epoch. For example, if Peer 1 sends (root, epoch) of (A, 5) and Peer 2 sends (B, 3) but Peer 1 has root C at epoch 3, then Peer 1 would disconnect because it knows that their chains are irreparably disjoint. -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 `BeaconBlocks` request. +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. #### Goodbye **Protocol ID:** ``/eth2/beacon_chain/req/goodbye/1/`` -**Content:** +Request, Response Content: ``` ( reason: uint64 @@ -365,11 +367,13 @@ Clients MAY use reason codes above `128` to indicate alternative, erroneous requ The range `[4, 127]` is RESERVED for future usage. -#### BeaconBlocks +This should not be encoded as an SSZ-container. -**Protocol ID:** `/eth2/beacon_chain/req/beacon_blocks/1/` +#### BeaconBlocksByRange -Request Content +**Protocol ID:** `/eth2/beacon_chain/req/beacon_blocks_by_range/1/` + +Request Content: ``` ( head_block_root: HashTreeRoot @@ -388,15 +392,24 @@ Response Content: Requests count beacon blocks from the peer starting from `start_slot` on the chain defined by `head_block_root`. The response MUST contain no more than count blocks. `step` defines the slot increment between blocks. For example, requesting blocks starting at `start_slot` 2 with a step value of 2 would return the blocks at [2, 4, 6, …]. In cases where a slot is empty for a given slot number, no block is returned. For example, if slot 4 were empty in the previous example, the returned array would contain [2, 6, …]. A step value of 1 returns all blocks on the range `[start_slot, start_slot + count)`. -`BeaconBlocks` is primarily used to sync historical blocks. +The request is encoded as an SSZ-container, the response is not encoded as an +SSZ container. + +`BeaconBlocksByRange` is primarily used to sync historical blocks. Clients MUST support requesting blocks since the start of the weak subjectivity period and up to the given `head_block_root`. Clients MUST support `head_block_root` values since the latest finalized epoch. -#### RecentBeaconBlocks +Clients MUST respond with at least one block, if they have it. -**Protocol ID:** `/eth2/beacon_chain/req/recent_beacon_blocks/1/` +Clients MUST order blocks by increasing slot number. + +Clients MAY respond with fewer blocks than requested, for example when the size of the response would exceed `REQ_RESP_MAX_SIZE` or `SSZ_MAX_LIST_SIZE`. + +#### BeaconBlocksByRoot + +**Protocol ID:** `/eth2/beacon_chain/req/beacon_blocks_by_root/1/` Request Content: @@ -414,12 +427,18 @@ Response Content: ) ``` -Requests blocks by their block roots. The response is a list of `BeaconBlock` with the same length as the request. Blocks are returned in order of the request and any missing/unknown blocks are left empty (SSZ null `BeaconBlock`). +Requests blocks by their block roots. The response is a list of `BeaconBlock` whose length is less or equal to the number of requested blocks. It may be less in the case that the responding peer is missing blocks. -`RecentBeaconBlocks` is primarily used to recover recent blocks (ex. when receiving a block or attestation whose parent is unknown). +`BeaconBlocksByRoot` is primarily used to recover recent blocks (ex. when receiving a block or attestation whose parent is unknown). + +Both the request and the response should not be encoded as an SSZ-container. Clients MUST support requesting blocks since the latest finalized epoch. +Clients MUST respond with at least one block, if they have it. + +Clients MAY respond with fewer blocks than requested, for example when the size of the response would exceed `REQ_RESP_MAX_SIZE` or `SSZ_MAX_LIST_SIZE`. + ## The discovery domain: discv5 Discovery Version 5 ([discv5](https://github.com/ethereum/devp2p/blob/master/discv5/discv5.md)) is used for peer discovery, both in the interoperability testnet and mainnet. From 28da0a07b8abd395e3b1903da2b79ec4cb604800 Mon Sep 17 00:00:00 2001 From: protolambda Date: Sun, 8 Sep 2019 14:36:09 -0400 Subject: [PATCH 02/31] fix BLS tests name length --- test_generators/bls/main.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test_generators/bls/main.py b/test_generators/bls/main.py index a74397e77..587b3adc0 100644 --- a/test_generators/bls/main.py +++ b/test_generators/bls/main.py @@ -11,6 +11,12 @@ from eth_utils import ( from gen_base import gen_runner, gen_typing from py_ecc import bls +from hashlib import sha256 + + +def hash(x): + return sha256(x).digest() + F2Q_COEFF_LEN = 48 G2_COMPRESSED_Z_LEN = 48 @@ -122,7 +128,8 @@ def case04_sign_messages(): for message in MESSAGES: for domain in DOMAINS: sig = bls.sign(message, privkey, domain) - yield f'sign_msg_{int_to_hex(privkey)}_{encode_hex(message)}_{encode_hex(domain)}', { + full_name = f'{int_to_hex(privkey)}_{encode_hex(message)}_{encode_hex(domain)}' + yield f'sign_msg_case_{(hash(bytes(full_name, "utf-8"))[:8]).hex()}', { 'input': { 'privkey': int_to_hex(privkey), 'message': encode_hex(message), From 3a79ad53637be372a4dc021c61e1069948ef3bd1 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Mon, 9 Sep 2019 05:38:06 +1000 Subject: [PATCH 03/31] Adds chunked responses to the RPC --- specs/networking/p2p-interface.md | 36 +++++++++++++++++++------------ 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 64f8a3ce8..a72843ba6 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -231,15 +231,16 @@ Request/response messages MUST adhere to the encoding specified in the protocol ``` request ::= | -response ::= | | +response ::= + +response_chunk ::= | | result ::= “0” | “1” | “2” | [“128” ... ”255”] ``` The encoding-dependent header may carry metadata or assertions such as the encoded payload length, for integrity and attack proofing purposes. Because req/resp streams are single-use and stream closures implicitly delimit the boundaries, it is not strictly necessary to length-prefix payloads; however, certain encodings like SSZ do, for added security. -`encoded-payload` has a maximum byte size of `REQ_RESP_MAX_SIZE`. +A `response` is formed by one or more `response_chunk`. The exact request determines whether a response consists of a single `response_chunk` or many. Responses that consist of a single SSZ-list of objects (such as `BlocksByRange` and `BlocksByRoot`) will send each list item as a `response_chunk`. The total `response` has a maximum uncompressed byte size of `REQ_RESP_MAX_SIZE`. -Clients MUST ensure the payload size is less than or equal to `REQ_RESP_MAX_SIZE`; if not, they SHOULD reset the stream immediately. Clients tracking peer reputation MAY decrement the score of the misbehaving peer under this circumstance. +Clients MUST ensure the total `response` is less than or equal to `REQ_RESP_MAX_SIZE`; if not, they SHOULD reset the stream immediately. Clients tracking peer reputation MAY decrement the score of the misbehaving peer under this circumstance. #### Requesting side @@ -247,7 +248,7 @@ Once a new stream with the protocol ID for the request type has been negotiated, The requester MUST close the write side of the stream once it finishes writing the request message—at this point, the stream will be half-closed. -The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester will allow further `RESP_TIMEOUT` to receive the full response. +The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester will allow further `RESP_TIMEOUT` to receive the full response. For requests consisting of many `response_chunk` the requester should read from the stream until either; a) An error is received by one of the chunks, b) The responder closes the stream or c) `REQ_RESP_MAX_SIZE` bytes have been read. Requests that have a single `response_chunk` and a length-prefix, requesters can read the exact number of bytes defined by the length-prefix before closing the stream. If any of these timeouts fire, the requester SHOULD reset the stream and deem the req/resp operation to have failed. @@ -260,14 +261,14 @@ The responder MUST: 1. Use the encoding strategy to read the optional header. 2. If there are any length assertions for length `N`, it should read exactly `N` bytes from the stream, at which point an EOF should arise (no more bytes). Should this not be the case, it should be treated as a failure. 3. Deserialize the expected type, and process the request. -4. Write the response (result, optional header, payload). +4. Write the response which may consist of one or many `response_chunks` (result, optional header, payload). 5. Close their write side of the stream. At this point, the stream will be fully closed. If steps (1), (2), or (3) fail due to invalid, malformed, or inconsistent data, the responder MUST respond in error. Clients tracking peer reputation MAY record such failures, as well as unexpected events, e.g. early stream resets. The entire request should be read in no more than `RESP_TIMEOUT`. Upon a timeout, the responder SHOULD reset the stream. -The responder SHOULD send a response promptly, starting with a **single-byte** response code which determines the contents of the response (`result` particle in the BNF grammar above). +The responder SHOULD send a response promptly, starting with a **single-byte** response code which determines the contents of the `response_chunk` (`result` particle in the BNF grammar above). It can have one of the following values, encoded as a single unsigned byte: @@ -289,7 +290,8 @@ The `ErrorMessage` schema is: *Note*: The String type is encoded as UTF-8 bytes without NULL terminator when SSZ-encoded. As the `ErrorMessage` is not an SSZ-container, only the UTF-8 bytes will be sent when SSZ-encoded. -A response therefore has the form: +A response therefore has the form of 1 or more `response_chunk` which look +like: ``` +--------+--------+--------+--------+--------+--------+ | result | header (opt) | encoded_response | @@ -310,7 +312,11 @@ The [SimpleSerialize (SSZ) specification](../simple-serialize.md) outlines how o **Encoding-dependent header:** Req/Resp protocols using the `ssz` or `ssz_snappy` encoding strategies MUST prefix all encoded and compressed (if applicable) payloads with an unsigned [protobuf varint](https://developers.google.com/protocol-buffers/docs/encoding#varints). -*Note*: Parameters defined as `[]VariableName` are SSZ-encoded containerless vectors. +All messages that have a single field, are not encoded as SSZ containers. + +Responses that are SSZ list objects (for example `[]BeaconBlocks`) send their +constituents individually as `response_chunk`. For example, the +`[]BeaconBlocks` response send many `response_chunk`s with each payload being a `BeaconBlock`. ### Messages @@ -338,7 +344,8 @@ The fields are: The dialing client MUST send a `Hello` request upon connection. -This should be encoded as an SSZ-container. +This should be encoded as an SSZ-container. The response consists of a single +`response_chunk`. Clients SHOULD immediately disconnect from one another following the handshake above under the following conditions: @@ -367,7 +374,8 @@ Clients MAY use reason codes above `128` to indicate alternative, erroneous requ The range `[4, 127]` is RESERVED for future usage. -This should not be encoded as an SSZ-container. +This should not be encoded as an SSZ-container. The response consists of a +single `response_chunk`. #### BeaconBlocksByRange @@ -392,8 +400,8 @@ Response Content: Requests count beacon blocks from the peer starting from `start_slot` on the chain defined by `head_block_root`. The response MUST contain no more than count blocks. `step` defines the slot increment between blocks. For example, requesting blocks starting at `start_slot` 2 with a step value of 2 would return the blocks at [2, 4, 6, …]. In cases where a slot is empty for a given slot number, no block is returned. For example, if slot 4 were empty in the previous example, the returned array would contain [2, 6, …]. A step value of 1 returns all blocks on the range `[start_slot, start_slot + count)`. -The request is encoded as an SSZ-container, the response is not encoded as an -SSZ container. +The request is encoded as an SSZ-container. The response is sent as many +`response_chunk` with each chunk consisting of a single `BeaconBlock`. `BeaconBlocksByRange` is primarily used to sync historical blocks. @@ -431,13 +439,13 @@ Requests blocks by their block roots. The response is a list of `BeaconBlock` wh `BeaconBlocksByRoot` is primarily used to recover recent blocks (ex. when receiving a block or attestation whose parent is unknown). -Both the request and the response should not be encoded as an SSZ-container. +The request is not encoded as an SSZ-container. The response is sent as many `response_chunk` with each chunk consisting of a single `BeaconBlock`. Clients MUST support requesting blocks since the latest finalized epoch. Clients MUST respond with at least one block, if they have it. -Clients MAY respond with fewer blocks than requested, for example when the size of the response would exceed `REQ_RESP_MAX_SIZE` or `SSZ_MAX_LIST_SIZE`. +Clients MAY respond with fewer blocks than requested, for example when the size of the uncompressed response would exceed `REQ_RESP_MAX_SIZE` or `SSZ_MAX_LIST_SIZE`. ## The discovery domain: discv5 From acb86e881791e8fe7e180adabd77195068c5475b Mon Sep 17 00:00:00 2001 From: Age Manning Date: Mon, 9 Sep 2019 05:45:42 +1000 Subject: [PATCH 04/31] Apply Danny's suggestions --- specs/networking/p2p-interface.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index a72843ba6..c51320b5a 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -113,7 +113,7 @@ This section outlines constants that are used in this spec. | Name | Value | Description | |---|---|---| -| `REQ_RESP_MAX_SIZE` | `2**22` (4194304, 4 MiB) | The maximum size of uncompressed req/resp messages that clients will allow. | +| `REQ_RESP_MAX_SIZE` | `2**20` (1048576, 1 MiB) | The maximum size of uncompressed req/resp messages that clients will allow. | | `SSZ_MAX_LIST_SIZE` | `TODO` | The maximum size of SSZ-encoded variable lists. | | `GOSSIP_MAX_SIZE` | `2**20` (= 1048576, 1 MiB) | The maximum size of uncompressed gossip messages. | | `SHARD_SUBNET_COUNT` | `TODO` | The number of shard subnets used in the gossipsub protocol. | @@ -244,7 +244,7 @@ Clients MUST ensure the total `response` is less than or equal to `REQ_RESP_MAX_ #### Requesting side -Once a new stream with the protocol ID for the request type has been negotiated, the full request message should be sent immediately. It should be encoded according to the encoding strategy. +Once a new stream with the protocol ID for the request type has been negotiated, the full request message should be sent immediately. It MUST be encoded according to the encoding strategy. The requester MUST close the write side of the stream once it finishes writing the request message—at this point, the stream will be half-closed. @@ -344,7 +344,7 @@ The fields are: The dialing client MUST send a `Hello` request upon connection. -This should be encoded as an SSZ-container. The response consists of a single +This MUST be encoded as an SSZ-container. The response consists of a single `response_chunk`. Clients SHOULD immediately disconnect from one another following the handshake above under the following conditions: @@ -374,7 +374,7 @@ Clients MAY use reason codes above `128` to indicate alternative, erroneous requ The range `[4, 127]` is RESERVED for future usage. -This should not be encoded as an SSZ-container. The response consists of a +This MUST be encoded as a single SSZ-field. The response consists of a single `response_chunk`. #### BeaconBlocksByRange @@ -400,7 +400,7 @@ Response Content: Requests count beacon blocks from the peer starting from `start_slot` on the chain defined by `head_block_root`. The response MUST contain no more than count blocks. `step` defines the slot increment between blocks. For example, requesting blocks starting at `start_slot` 2 with a step value of 2 would return the blocks at [2, 4, 6, …]. In cases where a slot is empty for a given slot number, no block is returned. For example, if slot 4 were empty in the previous example, the returned array would contain [2, 6, …]. A step value of 1 returns all blocks on the range `[start_slot, start_slot + count)`. -The request is encoded as an SSZ-container. The response is sent as many +The request MUST be encoded as an SSZ-container. The response is sent as many `response_chunk` with each chunk consisting of a single `BeaconBlock`. `BeaconBlocksByRange` is primarily used to sync historical blocks. @@ -439,7 +439,7 @@ Requests blocks by their block roots. The response is a list of `BeaconBlock` wh `BeaconBlocksByRoot` is primarily used to recover recent blocks (ex. when receiving a block or attestation whose parent is unknown). -The request is not encoded as an SSZ-container. The response is sent as many `response_chunk` with each chunk consisting of a single `BeaconBlock`. +The request MUST be encoded as an SSZ-field. The response is sent as many `response_chunk` with each chunk consisting of a single `BeaconBlock`. Clients MUST support requesting blocks since the latest finalized epoch. From cc12e29b25dab10e45a43fd0441e0a0abe71979a Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Sun, 8 Sep 2019 14:57:53 -0600 Subject: [PATCH 05/31] cleanup response_chunk refactor --- specs/networking/p2p-interface.md | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index c51320b5a..45c2361ca 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -248,7 +248,7 @@ Once a new stream with the protocol ID for the request type has been negotiated, The requester MUST close the write side of the stream once it finishes writing the request message—at this point, the stream will be half-closed. -The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester will allow further `RESP_TIMEOUT` to receive the full response. For requests consisting of many `response_chunk` the requester should read from the stream until either; a) An error is received by one of the chunks, b) The responder closes the stream or c) `REQ_RESP_MAX_SIZE` bytes have been read. Requests that have a single `response_chunk` and a length-prefix, requesters can read the exact number of bytes defined by the length-prefix before closing the stream. +The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester allows a further `RESP_TIMEOUT` to receive the full response. For requests consisting of many `response_chunk` the requester SHOULD read from the stream until either; a) An error is received in one of the chunks, b) The responder closes the stream or c) `REQ_RESP_MAX_SIZE` bytes have been read. For requests consisting of a single `response_chunk` and a length-prefix, the requester should read the exact number of bytes defined by the length-prefix before closing the stream. If any of these timeouts fire, the requester SHOULD reset the stream and deem the req/resp operation to have failed. @@ -261,7 +261,7 @@ The responder MUST: 1. Use the encoding strategy to read the optional header. 2. If there are any length assertions for length `N`, it should read exactly `N` bytes from the stream, at which point an EOF should arise (no more bytes). Should this not be the case, it should be treated as a failure. 3. Deserialize the expected type, and process the request. -4. Write the response which may consist of one or many `response_chunks` (result, optional header, payload). +4. Write the response which may consist of one or many `response_chunk` (result, optional header, payload). 5. Close their write side of the stream. At this point, the stream will be fully closed. If steps (1), (2), or (3) fail due to invalid, malformed, or inconsistent data, the responder MUST respond in error. Clients tracking peer reputation MAY record such failures, as well as unexpected events, e.g. early stream resets. @@ -290,8 +290,7 @@ The `ErrorMessage` schema is: *Note*: The String type is encoded as UTF-8 bytes without NULL terminator when SSZ-encoded. As the `ErrorMessage` is not an SSZ-container, only the UTF-8 bytes will be sent when SSZ-encoded. -A response therefore has the form of 1 or more `response_chunk` which look -like: +A response therefore has the form of one or more `response_chunk`, each structured as follows: ``` +--------+--------+--------+--------+--------+--------+ | result | header (opt) | encoded_response | @@ -312,11 +311,11 @@ The [SimpleSerialize (SSZ) specification](../simple-serialize.md) outlines how o **Encoding-dependent header:** Req/Resp protocols using the `ssz` or `ssz_snappy` encoding strategies MUST prefix all encoded and compressed (if applicable) payloads with an unsigned [protobuf varint](https://developers.google.com/protocol-buffers/docs/encoding#varints). -All messages that have a single field, are not encoded as SSZ containers. +All messages that contain only a single field MUST be encoded directly as the type of that field and MUST NOT be encoded as an SSZ container. Responses that are SSZ list objects (for example `[]BeaconBlocks`) send their constituents individually as `response_chunk`. For example, the -`[]BeaconBlocks` response send many `response_chunk`s with each payload being a `BeaconBlock`. +`[]BeaconBlocks` response type sends one or more `response_chunk`s. Each successful `response_chunk` has a single `BeaconBlock` payload. ### Messages @@ -344,8 +343,9 @@ The fields are: The dialing client MUST send a `Hello` request upon connection. -This MUST be encoded as an SSZ-container. The response consists of a single -`response_chunk`. +The request/response MUST be encoded as an SSZ-container. + +The response MUST consist of a single `response_chunk`. Clients SHOULD immediately disconnect from one another following the handshake above under the following conditions: @@ -374,8 +374,9 @@ Clients MAY use reason codes above `128` to indicate alternative, erroneous requ The range `[4, 127]` is RESERVED for future usage. -This MUST be encoded as a single SSZ-field. The response consists of a -single `response_chunk`. +The request/response MUST be encoded as a single SSZ-field. + +The response MUST consist of a single `response_chunk`. #### BeaconBlocksByRange @@ -400,8 +401,9 @@ Response Content: Requests count beacon blocks from the peer starting from `start_slot` on the chain defined by `head_block_root`. The response MUST contain no more than count blocks. `step` defines the slot increment between blocks. For example, requesting blocks starting at `start_slot` 2 with a step value of 2 would return the blocks at [2, 4, 6, …]. In cases where a slot is empty for a given slot number, no block is returned. For example, if slot 4 were empty in the previous example, the returned array would contain [2, 6, …]. A step value of 1 returns all blocks on the range `[start_slot, start_slot + count)`. -The request MUST be encoded as an SSZ-container. The response is sent as many -`response_chunk` with each chunk consisting of a single `BeaconBlock`. +The request MUST be encoded as an SSZ-container. + +The response MUST consist of at least one `response_chunk` and MAY consist of many. Each successful `response_chunk` MUST contain a single `BeaconBlock` payload. `BeaconBlocksByRange` is primarily used to sync historical blocks. @@ -439,7 +441,9 @@ Requests blocks by their block roots. The response is a list of `BeaconBlock` wh `BeaconBlocksByRoot` is primarily used to recover recent blocks (ex. when receiving a block or attestation whose parent is unknown). -The request MUST be encoded as an SSZ-field. The response is sent as many `response_chunk` with each chunk consisting of a single `BeaconBlock`. +The request MUST be encoded as an SSZ-field. + +The response MUST consist of at least one `response_chunk` and MAY consist of many. Each successful `response_chunk` MUST contain a single `BeaconBlock` payload. Clients MUST support requesting blocks since the latest finalized epoch. From b743deb0612c741fc4546c12db52cfcc5338bc87 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Sun, 8 Sep 2019 15:03:25 -0600 Subject: [PATCH 06/31] cleanup max size vars --- specs/networking/p2p-interface.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 45c2361ca..90b9b16a0 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -113,9 +113,9 @@ This section outlines constants that are used in this spec. | Name | Value | Description | |---|---|---| -| `REQ_RESP_MAX_SIZE` | `2**20` (1048576, 1 MiB) | The maximum size of uncompressed req/resp messages that clients will allow. | +| `GOSSIP_MAX_SIZE` | `2**20` (= 1048576, 1 MiB) | The maximum allowed size of uncompressed gossip messages. | +| `REQ_RESP_MAX_SIZE` | `2**20` (1048576, 1 MiB) | The maximum allowed size of uncompressed req/resp messages. | | `SSZ_MAX_LIST_SIZE` | `TODO` | The maximum size of SSZ-encoded variable lists. | -| `GOSSIP_MAX_SIZE` | `2**20` (= 1048576, 1 MiB) | The maximum size of uncompressed gossip messages. | | `SHARD_SUBNET_COUNT` | `TODO` | The number of shard subnets used in the gossipsub protocol. | | `TTFB_TIMEOUT` | `5s` | The maximum time to wait for first byte of request response (time-to-first-byte). | | `RESP_TIMEOUT` | `10s` | The maximum time for complete response transfer. | From 3ead8981099d56633e14a40922991d5960342a1d Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Sun, 8 Sep 2019 15:31:22 -0600 Subject: [PATCH 07/31] p2p spec copy cleanups --- specs/networking/p2p-interface.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 90b9b16a0..40228708e 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -238,17 +238,17 @@ result ::= “0” | “1” | “2” | [“128” ... ”255”] The encoding-dependent header may carry metadata or assertions such as the encoded payload length, for integrity and attack proofing purposes. Because req/resp streams are single-use and stream closures implicitly delimit the boundaries, it is not strictly necessary to length-prefix payloads; however, certain encodings like SSZ do, for added security. -A `response` is formed by one or more `response_chunk`. The exact request determines whether a response consists of a single `response_chunk` or many. Responses that consist of a single SSZ-list of objects (such as `BlocksByRange` and `BlocksByRoot`) will send each list item as a `response_chunk`. The total `response` has a maximum uncompressed byte size of `REQ_RESP_MAX_SIZE`. +A `response` is formed by one or more `response_chunk`s. The exact request determines whether a response consists of a single `response_chunk` or possibly many. Responses that consist of a single SSZ-list (such as `BlocksByRange` and `BlocksByRoot`) send each list item as a `response_chunk`. All other response types (non-Lists) send a single `response_chunk`. The total `response` has a maximum uncompressed byte size of `REQ_RESP_MAX_SIZE`. Clients MUST ensure the total `response` is less than or equal to `REQ_RESP_MAX_SIZE`; if not, they SHOULD reset the stream immediately. Clients tracking peer reputation MAY decrement the score of the misbehaving peer under this circumstance. #### Requesting side -Once a new stream with the protocol ID for the request type has been negotiated, the full request message should be sent immediately. It MUST be encoded according to the encoding strategy. +Once a new stream with the protocol ID for the request type has been negotiated, the full request message SHOULD be sent immediately. The request MUST be encoded according to the encoding strategy. -The requester MUST close the write side of the stream once it finishes writing the request message—at this point, the stream will be half-closed. +The requester MUST close the write side of the stream once it finishes writing the request message. At this point, the stream will be half-closed. -The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester allows a further `RESP_TIMEOUT` to receive the full response. For requests consisting of many `response_chunk` the requester SHOULD read from the stream until either; a) An error is received in one of the chunks, b) The responder closes the stream or c) `REQ_RESP_MAX_SIZE` bytes have been read. For requests consisting of a single `response_chunk` and a length-prefix, the requester should read the exact number of bytes defined by the length-prefix before closing the stream. +The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester allows a further `RESP_TIMEOUT` to receive the full response. For responses consisting of potentially many `response_chunk`s (an SSZ-list) the requester SHOULD read from the stream until either; a) An error result is received in one of the chunks, b) The responder closes the stream or c) `REQ_RESP_MAX_SIZE` bytes have been read. For requests consisting of a single `response_chunk` and a length-prefix, the requester should read the exact number of bytes defined by the length-prefix before closing the stream. If any of these timeouts fire, the requester SHOULD reset the stream and deem the req/resp operation to have failed. @@ -261,7 +261,7 @@ The responder MUST: 1. Use the encoding strategy to read the optional header. 2. If there are any length assertions for length `N`, it should read exactly `N` bytes from the stream, at which point an EOF should arise (no more bytes). Should this not be the case, it should be treated as a failure. 3. Deserialize the expected type, and process the request. -4. Write the response which may consist of one or many `response_chunk` (result, optional header, payload). +4. Write the response which may consist of one or more `response_chunk`s (result, optional header, payload). 5. Close their write side of the stream. At this point, the stream will be fully closed. If steps (1), (2), or (3) fail due to invalid, malformed, or inconsistent data, the responder MUST respond in error. Clients tracking peer reputation MAY record such failures, as well as unexpected events, e.g. early stream resets. @@ -290,7 +290,7 @@ The `ErrorMessage` schema is: *Note*: The String type is encoded as UTF-8 bytes without NULL terminator when SSZ-encoded. As the `ErrorMessage` is not an SSZ-container, only the UTF-8 bytes will be sent when SSZ-encoded. -A response therefore has the form of one or more `response_chunk`, each structured as follows: +A response therefore has the form of one or more `response_chunk`s, each structured as follows: ``` +--------+--------+--------+--------+--------+--------+ | result | header (opt) | encoded_response | @@ -313,9 +313,9 @@ The [SimpleSerialize (SSZ) specification](../simple-serialize.md) outlines how o All messages that contain only a single field MUST be encoded directly as the type of that field and MUST NOT be encoded as an SSZ container. -Responses that are SSZ list objects (for example `[]BeaconBlocks`) send their -constituents individually as `response_chunk`. For example, the -`[]BeaconBlocks` response type sends one or more `response_chunk`s. Each successful `response_chunk` has a single `BeaconBlock` payload. +Responses that are SSZ-lists (for example `[]BeaconBlocks`) send their +constituents individually as `response_chunk`s. For example, the +`[]BeaconBlocks` response type sends one or more `response_chunk`s. Each _successful_ `response_chunk` contains a single `BeaconBlock` payload. ### Messages @@ -403,7 +403,7 @@ Requests count beacon blocks from the peer starting from `start_slot` on the cha The request MUST be encoded as an SSZ-container. -The response MUST consist of at least one `response_chunk` and MAY consist of many. Each successful `response_chunk` MUST contain a single `BeaconBlock` payload. +The response MUST consist of at least one `response_chunk` and MAY consist of many. Each _successful_ `response_chunk` MUST contain a single `BeaconBlock` payload. `BeaconBlocksByRange` is primarily used to sync historical blocks. @@ -437,13 +437,13 @@ Response Content: ) ``` -Requests blocks by their block roots. The response is a list of `BeaconBlock` whose length is less or equal to the number of requested blocks. It may be less in the case that the responding peer is missing blocks. +Requests blocks by their block roots. The response is a list of `BeaconBlock` whose length is less than or equal to the number of requested blocks. It may be less in the case that the responding peer is missing blocks. -`BeaconBlocksByRoot` is primarily used to recover recent blocks (ex. when receiving a block or attestation whose parent is unknown). +`BeaconBlocksByRoot` is primarily used to recover recent blocks (e.g. when receiving a block or attestation whose parent is unknown). The request MUST be encoded as an SSZ-field. -The response MUST consist of at least one `response_chunk` and MAY consist of many. Each successful `response_chunk` MUST contain a single `BeaconBlock` payload. +The response MUST consist of at least one `response_chunk` and MAY consist of many. Each _successful_ `response_chunk` MUST contain a single `BeaconBlock` payload. Clients MUST support requesting blocks since the latest finalized epoch. From 4a7d8a4e480786f27390ea7cb8f580977338bb33 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Wed, 11 Sep 2019 06:06:22 +1000 Subject: [PATCH 08/31] Applies github suggestions --- specs/networking/p2p-interface.md | 39 ++++++++++++++++--------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 40228708e..d97be2b24 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -114,8 +114,9 @@ This section outlines constants that are used in this spec. | Name | Value | Description | |---|---|---| | `GOSSIP_MAX_SIZE` | `2**20` (= 1048576, 1 MiB) | The maximum allowed size of uncompressed gossip messages. | -| `REQ_RESP_MAX_SIZE` | `2**20` (1048576, 1 MiB) | The maximum allowed size of uncompressed req/resp messages. | -| `SSZ_MAX_LIST_SIZE` | `TODO` | The maximum size of SSZ-encoded variable lists. | +| `REQ_RESP_MAX_SIZE` | `2**20` (1048576, 1 MiB) | The maximum allowed size of uncompressed req/resp chunked responses. | +| `MAX_REQUESTED_BLOCKS` | `20` | The maximum allowed number of blocks that can +be requested. | | `SHARD_SUBNET_COUNT` | `TODO` | The number of shard subnets used in the gossipsub protocol. | | `TTFB_TIMEOUT` | `5s` | The maximum time to wait for first byte of request response (time-to-first-byte). | | `RESP_TIMEOUT` | `10s` | The maximum time for complete response transfer. | @@ -238,9 +239,9 @@ result ::= “0” | “1” | “2” | [“128” ... ”255”] The encoding-dependent header may carry metadata or assertions such as the encoded payload length, for integrity and attack proofing purposes. Because req/resp streams are single-use and stream closures implicitly delimit the boundaries, it is not strictly necessary to length-prefix payloads; however, certain encodings like SSZ do, for added security. -A `response` is formed by one or more `response_chunk`s. The exact request determines whether a response consists of a single `response_chunk` or possibly many. Responses that consist of a single SSZ-list (such as `BlocksByRange` and `BlocksByRoot`) send each list item as a `response_chunk`. All other response types (non-Lists) send a single `response_chunk`. The total `response` has a maximum uncompressed byte size of `REQ_RESP_MAX_SIZE`. +A `response` is formed by one or more `response_chunk`s. The exact request determines whether a response consists of a single `response_chunk` or possibly many. Responses that consist of a single SSZ-list (such as `BlocksByRange` and `BlocksByRoot`) send each list item as a `response_chunk`. All other response types (non-Lists) send a single `response_chunk`. The encoded-payload of a `response_chunk` has a maximum uncompressed byte size of `REQ_RESP_MAX_SIZE`. -Clients MUST ensure the total `response` is less than or equal to `REQ_RESP_MAX_SIZE`; if not, they SHOULD reset the stream immediately. Clients tracking peer reputation MAY decrement the score of the misbehaving peer under this circumstance. +Clients MUST ensure the each encoded payload of a `response_chunk` is less than or equal to `REQ_RESP_MAX_SIZE`; if not, they SHOULD reset the stream immediately. Clients tracking peer reputation MAY decrement the score of the misbehaving peer under this circumstance. #### Requesting side @@ -248,7 +249,7 @@ Once a new stream with the protocol ID for the request type has been negotiated, The requester MUST close the write side of the stream once it finishes writing the request message. At this point, the stream will be half-closed. -The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester allows a further `RESP_TIMEOUT` to receive the full response. For responses consisting of potentially many `response_chunk`s (an SSZ-list) the requester SHOULD read from the stream until either; a) An error result is received in one of the chunks, b) The responder closes the stream or c) `REQ_RESP_MAX_SIZE` bytes have been read. For requests consisting of a single `response_chunk` and a length-prefix, the requester should read the exact number of bytes defined by the length-prefix before closing the stream. +The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester allows a further `RESP_TIMEOUT` to receive the full response. For responses consisting of potentially many `response_chunk`s (an SSZ-list) the requester SHOULD read from the stream until either; a) An error result is received in one of the chunks, b) The responder closes the stream, c) More than `REQ_RESP_MAX_SIZE` bytes have been read for a single `response_chunk` payload or d) More than the maximum number of requested chunks are read. For requests consisting of a single `response_chunk` and a length-prefix, the requester should read the exact number of bytes defined by the length-prefix before closing the stream. If any of these timeouts fire, the requester SHOULD reset the stream and deem the req/resp operation to have failed. @@ -268,9 +269,9 @@ If steps (1), (2), or (3) fail due to invalid, malformed, or inconsistent data, The entire request should be read in no more than `RESP_TIMEOUT`. Upon a timeout, the responder SHOULD reset the stream. -The responder SHOULD send a response promptly, starting with a **single-byte** response code which determines the contents of the `response_chunk` (`result` particle in the BNF grammar above). +The responder SHOULD send a `response_chunk` promptly. Chunks start with a **single-byte** response code which determines the contents of the `response_chunk` (`result` particle in the BNF grammar above). -It can have one of the following values, encoded as a single unsigned byte: +The response code can have one of the following values, encoded as a single unsigned byte: - 0: **Success** -- a normal response follows, with contents matching the expected message schema and encoding specified in the request. - 1: **InvalidRequest** -- the contents of the request are semantically invalid, or the payload is malformed, or could not be understood. The response payload adheres to the `ErrorMessage` schema (described below). @@ -302,7 +303,7 @@ Here, `result` represents the 1-byte response code. The token of the negotiated protocol ID specifies the type of encoding to be used for the req/resp interaction. Two values are possible at this time: -- `ssz`: the contents are [SSZ-encoded](../simple-serialize.md). This encoding type MUST be supported by all clients. For objects containing a single field, only the field is SSZ-encoded not a container with a single field. For example, the `BeaconBlocksByRange` response would be an SSZ-encoded list of `BeaconBlock`s. All SSZ-Lists in the Req/Resp domain will have a maximum list size of `SSZ_MAX_LIST_SIZE`. +- `ssz`: the contents are [SSZ-encoded](../simple-serialize.md). This encoding type MUST be supported by all clients. For objects containing a single field, only the field is SSZ-encoded not a container with a single field. For example, the `BeaconBlocksByRoot` request is an SSZ-encoded list of `HashTreeRoots`'s. - `ssz_snappy`: The contents are SSZ-encoded and then compressed with [Snappy](https://github.com/google/snappy). MAY be supported in the interoperability testnet; MUST be supported in mainnet. #### SSZ-encoding strategy (with or without Snappy) @@ -319,9 +320,9 @@ constituents individually as `response_chunk`s. For example, the ### Messages -#### Hello +#### Status -**Protocol ID:** ``/eth2/beacon_chain/req/hello/1/`` +**Protocol ID:** ``/eth2/beacon_chain/req/status/1/`` Request, Response Content: ``` @@ -341,7 +342,7 @@ The fields are: - `head_root`: The block hash tree root corresponding to the head of the chain as seen by the sending node. - `head_slot`: The slot corresponding to the `head_root`. -The dialing client MUST send a `Hello` request upon connection. +The dialing client MUST send a `Status` request upon connection. The request/response MUST be encoded as an SSZ-container. @@ -349,7 +350,7 @@ The response MUST consist of a single `response_chunk`. Clients SHOULD immediately disconnect from one another following the handshake above under the following conditions: -1. If `head_fork_version` doesn’t match the expected fork version at the epoch of the `head_slot`, since the client’s chain is on another fork. `head_fork_version` can also be used to segregate testnets. +1. If `head_fork_version` does not match the expected fork version at the epoch of the `head_slot`, since the client’s chain is on another fork. `head_fork_version` can also be used to segregate testnets. 2. If the (`finalized_root`, `finalized_epoch`) shared by the peer is not in the client's chain at the expected epoch. For example, if Peer 1 sends (root, epoch) of (A, 5) and Peer 2 sends (B, 3) but Peer 1 has root C at epoch 3, then Peer 1 would disconnect because it knows that their chains are irreparably disjoint. 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. @@ -361,7 +362,7 @@ Once the handshake completes, the client with the lower `finalized_epoch` or `he Request, Response Content: ``` ( - reason: uint64 + uint64 ) ``` Client MAY send goodbye messages upon disconnection. The reason field MAY be one of the following values: @@ -395,7 +396,7 @@ Request Content: Response Content: ``` ( - blocks: []BeaconBlock + []BeaconBlock ) ``` @@ -403,6 +404,8 @@ Requests count beacon blocks from the peer starting from `start_slot` on the cha The request MUST be encoded as an SSZ-container. +The `count` MUST not exceed MAX_REQUESTED_BLOCKS. + The response MUST consist of at least one `response_chunk` and MAY consist of many. Each _successful_ `response_chunk` MUST contain a single `BeaconBlock` payload. `BeaconBlocksByRange` is primarily used to sync historical blocks. @@ -415,8 +418,6 @@ Clients MUST respond with at least one block, if they have it. Clients MUST order blocks by increasing slot number. -Clients MAY respond with fewer blocks than requested, for example when the size of the response would exceed `REQ_RESP_MAX_SIZE` or `SSZ_MAX_LIST_SIZE`. - #### BeaconBlocksByRoot **Protocol ID:** `/eth2/beacon_chain/req/beacon_blocks_by_root/1/` @@ -441,7 +442,9 @@ Requests blocks by their block roots. The response is a list of `BeaconBlock` wh `BeaconBlocksByRoot` is primarily used to recover recent blocks (e.g. when receiving a block or attestation whose parent is unknown). -The request MUST be encoded as an SSZ-field. +The length of `block_roots` MUST not exceed MAX_REQUESTED_BLOCKS. + +The request MUST be encoded as an SSZ-field. The response MUST consist of at least one `response_chunk` and MAY consist of many. Each _successful_ `response_chunk` MUST contain a single `BeaconBlock` payload. @@ -449,8 +452,6 @@ Clients MUST support requesting blocks since the latest finalized epoch. Clients MUST respond with at least one block, if they have it. -Clients MAY respond with fewer blocks than requested, for example when the size of the uncompressed response would exceed `REQ_RESP_MAX_SIZE` or `SSZ_MAX_LIST_SIZE`. - ## The discovery domain: discv5 Discovery Version 5 ([discv5](https://github.com/ethereum/devp2p/blob/master/discv5/discv5.md)) is used for peer discovery, both in the interoperability testnet and mainnet. From 9fa720b994668df2a9e886295cb0a6b55f71e858 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Wed, 11 Sep 2019 23:23:14 +1000 Subject: [PATCH 09/31] Removes a max chunk count and corrects timeout for chunked responses --- specs/networking/p2p-interface.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index d97be2b24..e33026be3 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -115,8 +115,6 @@ This section outlines constants that are used in this spec. |---|---|---| | `GOSSIP_MAX_SIZE` | `2**20` (= 1048576, 1 MiB) | The maximum allowed size of uncompressed gossip messages. | | `REQ_RESP_MAX_SIZE` | `2**20` (1048576, 1 MiB) | The maximum allowed size of uncompressed req/resp chunked responses. | -| `MAX_REQUESTED_BLOCKS` | `20` | The maximum allowed number of blocks that can -be requested. | | `SHARD_SUBNET_COUNT` | `TODO` | The number of shard subnets used in the gossipsub protocol. | | `TTFB_TIMEOUT` | `5s` | The maximum time to wait for first byte of request response (time-to-first-byte). | | `RESP_TIMEOUT` | `10s` | The maximum time for complete response transfer. | @@ -249,7 +247,7 @@ Once a new stream with the protocol ID for the request type has been negotiated, The requester MUST close the write side of the stream once it finishes writing the request message. At this point, the stream will be half-closed. -The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester allows a further `RESP_TIMEOUT` to receive the full response. For responses consisting of potentially many `response_chunk`s (an SSZ-list) the requester SHOULD read from the stream until either; a) An error result is received in one of the chunks, b) The responder closes the stream, c) More than `REQ_RESP_MAX_SIZE` bytes have been read for a single `response_chunk` payload or d) More than the maximum number of requested chunks are read. For requests consisting of a single `response_chunk` and a length-prefix, the requester should read the exact number of bytes defined by the length-prefix before closing the stream. +The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester allows a further `RESP_TIMEOUT` for each subsequent `response_chunk` received. For responses consisting of potentially many `response_chunk`s (an SSZ-list) the requester SHOULD read from the stream until either; a) An error result is received in one of the chunks, b) The responder closes the stream, c) More than `REQ_RESP_MAX_SIZE` bytes have been read for a single `response_chunk` payload or d) More than the maximum number of requested chunks are read. For requests consisting of a single `response_chunk` and a length-prefix, the requester should read the exact number of bytes defined by the length-prefix before closing the stream. If any of these timeouts fire, the requester SHOULD reset the stream and deem the req/resp operation to have failed. @@ -404,8 +402,6 @@ Requests count beacon blocks from the peer starting from `start_slot` on the cha The request MUST be encoded as an SSZ-container. -The `count` MUST not exceed MAX_REQUESTED_BLOCKS. - The response MUST consist of at least one `response_chunk` and MAY consist of many. Each _successful_ `response_chunk` MUST contain a single `BeaconBlock` payload. `BeaconBlocksByRange` is primarily used to sync historical blocks. @@ -442,8 +438,6 @@ Requests blocks by their block roots. The response is a list of `BeaconBlock` wh `BeaconBlocksByRoot` is primarily used to recover recent blocks (e.g. when receiving a block or attestation whose parent is unknown). -The length of `block_roots` MUST not exceed MAX_REQUESTED_BLOCKS. - The request MUST be encoded as an SSZ-field. The response MUST consist of at least one `response_chunk` and MAY consist of many. Each _successful_ `response_chunk` MUST contain a single `BeaconBlock` payload. From 8bb9354e65590c1e7f7df9d90d985d5e1c31e06b Mon Sep 17 00:00:00 2001 From: Age Manning Date: Fri, 13 Sep 2019 02:20:59 +1000 Subject: [PATCH 10/31] Renames REQ_RESP_MAX_SIZE to MAX_CHUNK_SIZE --- specs/networking/p2p-interface.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index e33026be3..46663fff1 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -114,7 +114,7 @@ This section outlines constants that are used in this spec. | Name | Value | Description | |---|---|---| | `GOSSIP_MAX_SIZE` | `2**20` (= 1048576, 1 MiB) | The maximum allowed size of uncompressed gossip messages. | -| `REQ_RESP_MAX_SIZE` | `2**20` (1048576, 1 MiB) | The maximum allowed size of uncompressed req/resp chunked responses. | +| `MAX_CHUNK_SIZE` | `2**20` (1048576, 1 MiB) | The maximum allowed size of uncompressed req/resp chunked responses. | | `SHARD_SUBNET_COUNT` | `TODO` | The number of shard subnets used in the gossipsub protocol. | | `TTFB_TIMEOUT` | `5s` | The maximum time to wait for first byte of request response (time-to-first-byte). | | `RESP_TIMEOUT` | `10s` | The maximum time for complete response transfer. | @@ -237,9 +237,9 @@ result ::= “0” | “1” | “2” | [“128” ... ”255”] The encoding-dependent header may carry metadata or assertions such as the encoded payload length, for integrity and attack proofing purposes. Because req/resp streams are single-use and stream closures implicitly delimit the boundaries, it is not strictly necessary to length-prefix payloads; however, certain encodings like SSZ do, for added security. -A `response` is formed by one or more `response_chunk`s. The exact request determines whether a response consists of a single `response_chunk` or possibly many. Responses that consist of a single SSZ-list (such as `BlocksByRange` and `BlocksByRoot`) send each list item as a `response_chunk`. All other response types (non-Lists) send a single `response_chunk`. The encoded-payload of a `response_chunk` has a maximum uncompressed byte size of `REQ_RESP_MAX_SIZE`. +A `response` is formed by one or more `response_chunk`s. The exact request determines whether a response consists of a single `response_chunk` or possibly many. Responses that consist of a single SSZ-list (such as `BlocksByRange` and `BlocksByRoot`) send each list item as a `response_chunk`. All other response types (non-Lists) send a single `response_chunk`. The encoded-payload of a `response_chunk` has a maximum uncompressed byte size of `MAX_CHUNK_SIZE`. -Clients MUST ensure the each encoded payload of a `response_chunk` is less than or equal to `REQ_RESP_MAX_SIZE`; if not, they SHOULD reset the stream immediately. Clients tracking peer reputation MAY decrement the score of the misbehaving peer under this circumstance. +Clients MUST ensure the each encoded payload of a `response_chunk` is less than or equal to `MAX_CHUNK_SIZE`; if not, they SHOULD reset the stream immediately. Clients tracking peer reputation MAY decrement the score of the misbehaving peer under this circumstance. #### Requesting side @@ -247,7 +247,7 @@ Once a new stream with the protocol ID for the request type has been negotiated, The requester MUST close the write side of the stream once it finishes writing the request message. At this point, the stream will be half-closed. -The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester allows a further `RESP_TIMEOUT` for each subsequent `response_chunk` received. For responses consisting of potentially many `response_chunk`s (an SSZ-list) the requester SHOULD read from the stream until either; a) An error result is received in one of the chunks, b) The responder closes the stream, c) More than `REQ_RESP_MAX_SIZE` bytes have been read for a single `response_chunk` payload or d) More than the maximum number of requested chunks are read. For requests consisting of a single `response_chunk` and a length-prefix, the requester should read the exact number of bytes defined by the length-prefix before closing the stream. +The requester MUST wait a maximum of `TTFB_TIMEOUT` for the first response byte to arrive (time to first byte—or TTFB—timeout). On that happening, the requester allows a further `RESP_TIMEOUT` for each subsequent `response_chunk` received. For responses consisting of potentially many `response_chunk`s (an SSZ-list) the requester SHOULD read from the stream until either; a) An error result is received in one of the chunks, b) The responder closes the stream, c) More than `MAX_CHUNK_SIZE` bytes have been read for a single `response_chunk` payload or d) More than the maximum number of requested chunks are read. For requests consisting of a single `response_chunk` and a length-prefix, the requester should read the exact number of bytes defined by the length-prefix before closing the stream. If any of these timeouts fire, the requester SHOULD reset the stream and deem the req/resp operation to have failed. @@ -267,7 +267,7 @@ If steps (1), (2), or (3) fail due to invalid, malformed, or inconsistent data, The entire request should be read in no more than `RESP_TIMEOUT`. Upon a timeout, the responder SHOULD reset the stream. -The responder SHOULD send a `response_chunk` promptly. Chunks start with a **single-byte** response code which determines the contents of the `response_chunk` (`result` particle in the BNF grammar above). +The responder SHOULD send a `response_chunk` promptly. Chunks start with a **single-byte** response code which determines the contents of the `response_chunk` (`result` particle in the BNF grammar above). For multiple chunks, only the last chunk is allowed to have a non-zero error code (i.e. The chunk stream is terminated once an error occurs). The response code can have one of the following values, encoded as a single unsigned byte: @@ -708,7 +708,7 @@ Disadvantages include: * Harder to stream as length must be known up-front * Additional code path required to verify length -In some protocols, adding a length prefix serves as a form of DoS protection against very long messages, allowing the client to abort if an overlong message is about to be sent. In this protocol, we are globally limiting message sizes using `REQ_RESP_MAX_SIZE`, thus the length prefix does not afford any additional protection. +In some protocols, adding a length prefix serves as a form of DoS protection against very long messages, allowing the client to abort if an overlong message is about to be sent. In this protocol, we are globally limiting message sizes using `MAX_CHUNK_SIZE`, thus the length prefix does not afford any additional protection. [Protobuf varint](https://developers.google.com/protocol-buffers/docs/encoding#varints) is an efficient technique to encode variable-length ints. Instead of reserving a fixed-size field of as many bytes as necessary to convey the maximum possible value, this field is elastic in exchange for 1-bit overhead per byte. From 4b2596dbad4a68bd09e4b844637102eb347c0ec3 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Mon, 16 Sep 2019 08:59:04 -0500 Subject: [PATCH 11/31] ensure BeaconBlocksByRoot requests are lists rather than containers --- specs/networking/p2p-interface.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/networking/p2p-interface.md b/specs/networking/p2p-interface.md index 46663fff1..4f71ed6d9 100644 --- a/specs/networking/p2p-interface.md +++ b/specs/networking/p2p-interface.md @@ -422,7 +422,7 @@ Request Content: ``` ( - block_roots: []HashTreeRoot + []HashTreeRoot ) ``` @@ -430,7 +430,7 @@ Response Content: ``` ( - blocks: []BeaconBlock + []BeaconBlock ) ``` From 687b262f0d96f3b331758b35520ec27597585d15 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Fri, 20 Sep 2019 11:27:42 -0500 Subject: [PATCH 12/31] add test case for crosslink tie breaking between epochs --- .../eth2spec/test/helpers/attestations.py | 5 +- .../test_process_crosslinks.py | 52 +++++++++++++++++-- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/helpers/attestations.py b/test_libs/pyspec/eth2spec/test/helpers/attestations.py index 868517018..43d9bf44f 100644 --- a/test_libs/pyspec/eth2spec/test/helpers/attestations.py +++ b/test_libs/pyspec/eth2spec/test/helpers/attestations.py @@ -138,10 +138,11 @@ def fill_aggregate_attestation(spec, state, attestation): attestation.aggregation_bits[i] = True -def add_attestation_to_state(spec, state, attestation, slot): +def add_attestations_to_state(spec, state, attestations, slot): block = build_empty_block_for_next_slot(spec, state) block.slot = slot - block.body.attestations.append(attestation) + for attestation in attestations: + block.body.attestations.append(attestation) spec.process_slots(state, block.slot) sign_block(spec, state, block) spec.state_transition(state, block) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py index 41d784c50..a335896df 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py @@ -7,7 +7,7 @@ from eth2spec.test.helpers.state import ( ) from eth2spec.test.helpers.block import apply_empty_block from eth2spec.test.helpers.attestations import ( - add_attestation_to_state, + add_attestations_to_state, fill_aggregate_attestation, get_valid_attestation, sign_attestation, @@ -36,7 +36,7 @@ def test_single_crosslink_update_from_current_epoch(spec, state): attestation = get_valid_attestation(spec, state, signed=True) fill_aggregate_attestation(spec, state, attestation) - add_attestation_to_state(spec, state, attestation, state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) + add_attestations_to_state(spec, state, [attestation], state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) assert len(state.current_epoch_attestations) == 1 @@ -57,7 +57,7 @@ def test_single_crosslink_update_from_previous_epoch(spec, state): attestation = get_valid_attestation(spec, state, signed=True) fill_aggregate_attestation(spec, state, attestation) - add_attestation_to_state(spec, state, attestation, state.slot + spec.SLOTS_PER_EPOCH) + add_attestations_to_state(spec, state, [attestation], state.slot + spec.SLOTS_PER_EPOCH) assert len(state.previous_epoch_attestations) == 1 @@ -95,7 +95,7 @@ def test_double_late_crosslink(spec, state): # add attestation_1 to next epoch next_epoch(spec, state) - add_attestation_to_state(spec, state, attestation_1, state.slot + 1) + add_attestations_to_state(spec, state, [attestation_1], state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) for _ in range(spec.SLOTS_PER_EPOCH): attestation_2 = get_valid_attestation(spec, state) @@ -110,7 +110,7 @@ def test_double_late_crosslink(spec, state): # add attestation_2 in the next epoch after attestation_1 has # already updated the relevant crosslink next_epoch(spec, state) - add_attestation_to_state(spec, state, attestation_2, state.slot + 1) + add_attestations_to_state(spec, state, [attestation_2], state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) assert len(state.previous_epoch_attestations) == 1 assert len(state.current_epoch_attestations) == 0 @@ -130,3 +130,45 @@ def test_double_late_crosslink(spec, state): attestation_2.data.crosslink.shard): assert crosslink_deltas[0][index] == 0 assert crosslink_deltas[1][index] > 0 + + +@with_all_phases +@spec_state_test +def test_tied_crosslink_between_epochs(spec, state): + """ + Addresses scenario found at Interop described by this test case + https://github.com/djrtwo/interop-test-cases/tree/master/tests/night_one_16_crosslinks + + Ensure that ties on crosslinks between epochs are broken by previous epoch. + """ + prev_attestation = get_valid_attestation(spec, state, signed=True) + fill_aggregate_attestation(spec, state, prev_attestation) + + # add attestation at start of next epoch + next_epoch(spec, state) + add_attestations_to_state(spec, state, [prev_attestation], state.slot) + + # create attestation from current epoch for same shard + for _ in range(spec.SLOTS_PER_EPOCH): + cur_attestation = get_valid_attestation(spec, state) + if cur_attestation.data.crosslink.shard == prev_attestation.data.crosslink.shard: + sign_attestation(spec, state, cur_attestation) + break + next_slot(spec, state) + fill_aggregate_attestation(spec, state, cur_attestation) + + add_attestations_to_state(spec, state, [cur_attestation], state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) + + shard = prev_attestation.data.crosslink.shard + pre_crosslink = deepcopy(state.current_crosslinks[shard]) + + assert prev_attestation.data.crosslink != cur_attestation.data.crosslink + assert state.current_crosslinks[shard] == spec.Crosslink() + assert len(state.previous_epoch_attestations) == 1 + assert len(state.current_epoch_attestations) == 1 + + yield from run_process_crosslinks(spec, state) + + assert state.previous_crosslinks[shard] != state.current_crosslinks[shard] + assert pre_crosslink != state.current_crosslinks[shard] + assert state.current_crosslinks[shard] == prev_attestation.data.crosslink From ad4da4cd14aecd95424a5c38da735051e1056940 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Fri, 20 Sep 2019 12:45:46 -0500 Subject: [PATCH 13/31] rewards test for duplicate attestation --- .../test_process_rewards_and_penalties.py | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py 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 new file mode 100644 index 000000000..8b79e8d68 --- /dev/null +++ b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_rewards_and_penalties.py @@ -0,0 +1,96 @@ +from copy import deepcopy + +from eth2spec.test.context import spec_state_test, with_all_phases +from eth2spec.test.helpers.state import ( + next_epoch, + next_slot, +) +from eth2spec.test.helpers.attestations import ( + add_attestations_to_state, + fill_aggregate_attestation, + get_valid_attestation, + sign_attestation, +) +from eth2spec.test.phase_0.epoch_processing.run_epoch_process_base import run_epoch_processing_with + + +def run_process_rewards_and_penalties(spec, state): + yield from run_epoch_processing_with(spec, state, 'process_rewards_and_penalties') + + +@with_all_phases +@spec_state_test +def test_genesis_epoch_no_attestations_no_penalties(spec, state): + pre_state = deepcopy(state) + + assert spec.compute_epoch_of_slot(state.slot) == spec.GENESIS_EPOCH + + yield from run_process_rewards_and_penalties(spec, state) + + for index in range(len(pre_state.validators)): + assert state.balances[index] == pre_state.balances[index] + + +@with_all_phases +@spec_state_test +def test_genesis_epoch_full_attestations_no_rewards(spec, state): + attestations = [] + for slot in range(spec.SLOTS_PER_EPOCH - spec.MIN_ATTESTATION_INCLUSION_DELAY - 1): + attestation = get_valid_attestation(spec, state, signed=True) + fill_aggregate_attestation(spec, state, attestation) + attestations.append(attestation) + next_slot(spec, state) + add_attestations_to_state(spec, state, attestations, state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) + + assert spec.compute_epoch_of_slot(state.slot) == spec.GENESIS_EPOCH + + pre_state = deepcopy(state) + + yield from run_process_rewards_and_penalties(spec, state) + + for index in range(len(pre_state.validators)): + assert state.balances[index] == pre_state.balances[index] + + +@with_all_phases +@spec_state_test +def test_no_attestations_all_penalties(spec, state): + next_epoch(spec, state) + pre_state = deepcopy(state) + + yield from run_process_rewards_and_penalties(spec, state) + + for index in range(len(pre_state.validators)): + assert state.balances[index] < pre_state.balances[index] + + +@with_all_phases +@spec_state_test +def test_duplicate_attestation(spec, state): + attestation = get_valid_attestation(spec, state, signed=True) + fill_aggregate_attestation(spec, state, attestation) + + indexed_attestation = spec.get_indexed_attestation(state, attestation) + participants = indexed_attestation.custody_bit_0_indices + indexed_attestation.custody_bit_1_indices + + assert len(participants) > 0 + + single_state = deepcopy(state) + dup_state = deepcopy(state) + + inclusion_slot = state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY + add_attestations_to_state(spec, single_state, [attestation], inclusion_slot) + add_attestations_to_state(spec, dup_state, [attestation, attestation], inclusion_slot) + + next_epoch(spec, single_state) + next_epoch(spec, dup_state) + + # Run non-duplicate inclusion rewards for comparision. Do not yield test vectors + pre, post = run_process_rewards_and_penalties(spec, single_state) + + # Output duplicate inclusion to test vectors + yield from run_process_rewards_and_penalties(spec, dup_state) + + for index in participants: + assert state.balances[index] < single_state.balances[index] + assert single_state.balances[index] == dup_state.balances[index] From b3f7dd9dae4cb3bd7a5e004ebe64b44427e8bf8c Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Fri, 20 Sep 2019 16:05:10 -0500 Subject: [PATCH 14/31] fix up rewards/penalties test signatures --- .../eth2spec/test/helpers/attestations.py | 5 +++- .../test_process_crosslinks.py | 24 ++++++++----------- .../test_process_rewards_and_penalties.py | 9 ++++--- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/helpers/attestations.py b/test_libs/pyspec/eth2spec/test/helpers/attestations.py index 43d9bf44f..394579413 100644 --- a/test_libs/pyspec/eth2spec/test/helpers/attestations.py +++ b/test_libs/pyspec/eth2spec/test/helpers/attestations.py @@ -128,7 +128,7 @@ def get_attestation_signature(spec, state, attestation_data, privkey, custody_bi ) -def fill_aggregate_attestation(spec, state, attestation): +def fill_aggregate_attestation(spec, state, attestation, signed=False): crosslink_committee = spec.get_crosslink_committee( state, attestation.data.target.epoch, @@ -137,6 +137,9 @@ def fill_aggregate_attestation(spec, state, attestation): for i in range(len(crosslink_committee)): attestation.aggregation_bits[i] = True + if signed: + sign_attestation(spec, state, attestation) + def add_attestations_to_state(spec, state, attestations, slot): block = build_empty_block_for_next_slot(spec, state) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py index a335896df..83e2bb017 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py @@ -10,7 +10,6 @@ from eth2spec.test.helpers.attestations import ( add_attestations_to_state, fill_aggregate_attestation, get_valid_attestation, - sign_attestation, ) from eth2spec.test.phase_0.epoch_processing.run_epoch_process_base import run_epoch_processing_with @@ -33,9 +32,9 @@ def test_no_attestations(spec, state): def test_single_crosslink_update_from_current_epoch(spec, state): next_epoch(spec, state) - attestation = get_valid_attestation(spec, state, signed=True) + attestation = get_valid_attestation(spec, state) - fill_aggregate_attestation(spec, state, attestation) + fill_aggregate_attestation(spec, state, attestation, signed=True) add_attestations_to_state(spec, state, [attestation], state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) assert len(state.current_epoch_attestations) == 1 @@ -54,9 +53,9 @@ def test_single_crosslink_update_from_current_epoch(spec, state): def test_single_crosslink_update_from_previous_epoch(spec, state): next_epoch(spec, state) - attestation = get_valid_attestation(spec, state, signed=True) + attestation = get_valid_attestation(spec, state) - fill_aggregate_attestation(spec, state, attestation) + fill_aggregate_attestation(spec, state, attestation, signed=True) add_attestations_to_state(spec, state, [attestation], state.slot + spec.SLOTS_PER_EPOCH) assert len(state.previous_epoch_attestations) == 1 @@ -90,8 +89,8 @@ def test_double_late_crosslink(spec, state): next_epoch(spec, state) state.slot += 4 - attestation_1 = get_valid_attestation(spec, state, signed=True) - fill_aggregate_attestation(spec, state, attestation_1) + attestation_1 = get_valid_attestation(spec, state) + fill_aggregate_attestation(spec, state, attestation_1, signed=True) # add attestation_1 to next epoch next_epoch(spec, state) @@ -100,13 +99,11 @@ def test_double_late_crosslink(spec, state): for _ in range(spec.SLOTS_PER_EPOCH): attestation_2 = get_valid_attestation(spec, state) if attestation_2.data.crosslink.shard == attestation_1.data.crosslink.shard: - sign_attestation(spec, state, attestation_2) + fill_aggregate_attestation(spec, state, attestation_2, signed=True) break next_slot(spec, state) apply_empty_block(spec, state) - fill_aggregate_attestation(spec, state, attestation_2) - # add attestation_2 in the next epoch after attestation_1 has # already updated the relevant crosslink next_epoch(spec, state) @@ -141,8 +138,8 @@ def test_tied_crosslink_between_epochs(spec, state): Ensure that ties on crosslinks between epochs are broken by previous epoch. """ - prev_attestation = get_valid_attestation(spec, state, signed=True) - fill_aggregate_attestation(spec, state, prev_attestation) + prev_attestation = get_valid_attestation(spec, state) + fill_aggregate_attestation(spec, state, prev_attestation, signed=True) # add attestation at start of next epoch next_epoch(spec, state) @@ -152,10 +149,9 @@ def test_tied_crosslink_between_epochs(spec, state): for _ in range(spec.SLOTS_PER_EPOCH): cur_attestation = get_valid_attestation(spec, state) if cur_attestation.data.crosslink.shard == prev_attestation.data.crosslink.shard: - sign_attestation(spec, state, cur_attestation) + fill_aggregate_attestation(spec, state, cur_attestation, signed=True) break next_slot(spec, state) - fill_aggregate_attestation(spec, state, cur_attestation) add_attestations_to_state(spec, state, [cur_attestation], state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) 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 8b79e8d68..e42092941 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 @@ -9,7 +9,6 @@ from eth2spec.test.helpers.attestations import ( add_attestations_to_state, fill_aggregate_attestation, get_valid_attestation, - sign_attestation, ) from eth2spec.test.phase_0.epoch_processing.run_epoch_process_base import run_epoch_processing_with @@ -36,8 +35,8 @@ def test_genesis_epoch_no_attestations_no_penalties(spec, state): def test_genesis_epoch_full_attestations_no_rewards(spec, state): attestations = [] for slot in range(spec.SLOTS_PER_EPOCH - spec.MIN_ATTESTATION_INCLUSION_DELAY - 1): - attestation = get_valid_attestation(spec, state, signed=True) - fill_aggregate_attestation(spec, state, attestation) + attestation = get_valid_attestation(spec, state) + fill_aggregate_attestation(spec, state, attestation, signed=True) attestations.append(attestation) next_slot(spec, state) add_attestations_to_state(spec, state, attestations, state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) @@ -67,8 +66,8 @@ def test_no_attestations_all_penalties(spec, state): @with_all_phases @spec_state_test def test_duplicate_attestation(spec, state): - attestation = get_valid_attestation(spec, state, signed=True) - fill_aggregate_attestation(spec, state, attestation) + attestation = get_valid_attestation(spec, state) + fill_aggregate_attestation(spec, state, attestation, signed=True) indexed_attestation = spec.get_indexed_attestation(state, attestation) participants = indexed_attestation.custody_bit_0_indices + indexed_attestation.custody_bit_1_indices From cf1323b79e5d1786a5d81888dd428c45f95b62c0 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Sun, 22 Sep 2019 09:35:18 -0500 Subject: [PATCH 15/31] add rewards/penalties test for full epoch of attestations --- .../test_process_rewards_and_penalties.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) 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 e42092941..f0cde1a73 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 @@ -51,12 +51,37 @@ def test_genesis_epoch_full_attestations_no_rewards(spec, state): assert state.balances[index] == pre_state.balances[index] +@with_all_phases +@spec_state_test +def test_full_attestations(spec, state): + attestations = [] + for slot in range(spec.SLOTS_PER_EPOCH - spec.MIN_ATTESTATION_INCLUSION_DELAY): + attestation = get_valid_attestation(spec, state) + fill_aggregate_attestation(spec, state, attestation, signed=True) + attestations.append(attestation) + next_slot(spec, state) + add_attestations_to_state(spec, state, attestations, state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) + + assert spec.compute_epoch_of_slot(state.slot) == spec.GENESIS_EPOCH + 1 + + pre_state = deepcopy(state) + + yield from run_process_rewards_and_penalties(spec, state) + + attesting_indices = spec.get_unslashed_attesting_indices(state, attestations) + assert len(attesting_indices) > 0 + for index in attesting_indices: + assert state.balances[index] > pre_state.balances[index] + + @with_all_phases @spec_state_test def test_no_attestations_all_penalties(spec, state): next_epoch(spec, state) pre_state = deepcopy(state) + assert spec.compute_epoch_of_slot(state.slot) == spec.GENESIS_EPOCH + 1 + yield from run_process_rewards_and_penalties(spec, state) for index in range(len(pre_state.validators)): @@ -66,6 +91,12 @@ def test_no_attestations_all_penalties(spec, state): @with_all_phases @spec_state_test def test_duplicate_attestation(spec, state): + """ + Although duplicate attestations can be included on-chain, they should only + be rewarded for once. + This test addresses this issue found at Interop + https://github.com/djrtwo/interop-test-cases/tree/master/tests/prysm_16_duplicate_attestation_rewards + """ attestation = get_valid_attestation(spec, state) fill_aggregate_attestation(spec, state, attestation, signed=True) From 1aa12034e55867554739b0f941aecb65b83edf27 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Sun, 22 Sep 2019 09:51:12 -0500 Subject: [PATCH 16/31] make full_attestation reward test better --- .../test_process_rewards_and_penalties.py | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) 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 f0cde1a73..114898abf 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 @@ -55,14 +55,20 @@ def test_genesis_epoch_full_attestations_no_rewards(spec, state): @spec_state_test def test_full_attestations(spec, state): attestations = [] - for slot in range(spec.SLOTS_PER_EPOCH - spec.MIN_ATTESTATION_INCLUSION_DELAY): - attestation = get_valid_attestation(spec, state) - fill_aggregate_attestation(spec, state, attestation, signed=True) - attestations.append(attestation) + for slot in range(spec.SLOTS_PER_EPOCH + spec.MIN_ATTESTATION_INCLUSION_DELAY): + # create an attestation for each slot in epoch + if slot < spec.SLOTS_PER_EPOCH: + attestation = get_valid_attestation(spec, state) + fill_aggregate_attestation(spec, state, attestation, signed=True) + attestations.append(attestation) + # fill each created slot in state after inclusion delay + if slot - spec.MIN_ATTESTATION_INCLUSION_DELAY >= 0: + include_att = attestations[slot - spec.MIN_ATTESTATION_INCLUSION_DELAY] + add_attestations_to_state(spec, state, [include_att], state.slot) next_slot(spec, state) - add_attestations_to_state(spec, state, attestations, state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) assert spec.compute_epoch_of_slot(state.slot) == spec.GENESIS_EPOCH + 1 + assert len(state.previous_epoch_attestations) == spec.SLOTS_PER_EPOCH pre_state = deepcopy(state) @@ -70,8 +76,11 @@ def test_full_attestations(spec, state): attesting_indices = spec.get_unslashed_attesting_indices(state, attestations) assert len(attesting_indices) > 0 - for index in attesting_indices: - assert state.balances[index] > pre_state.balances[index] + for index in range(len(pre_state.validators)): + if index in attesting_indices: + assert state.balances[index] > pre_state.balances[index] + else: + assert state.balances[index] < pre_state.balances[index] @with_all_phases From 16887215541b416a74edf8f5fcb5b809091352e8 Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Sun, 22 Sep 2019 09:54:17 -0500 Subject: [PATCH 17/31] fix up attesation reward tests --- .../test_process_rewards_and_penalties.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) 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 114898abf..40f6d781e 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 @@ -34,13 +34,19 @@ def test_genesis_epoch_no_attestations_no_penalties(spec, state): @spec_state_test def test_genesis_epoch_full_attestations_no_rewards(spec, state): attestations = [] - for slot in range(spec.SLOTS_PER_EPOCH - spec.MIN_ATTESTATION_INCLUSION_DELAY - 1): - attestation = get_valid_attestation(spec, state) - fill_aggregate_attestation(spec, state, attestation, signed=True) - attestations.append(attestation) + for slot in range(spec.SLOTS_PER_EPOCH - 1): + # create an attestation for each slot + if slot < spec.SLOTS_PER_EPOCH: + attestation = get_valid_attestation(spec, state) + fill_aggregate_attestation(spec, state, attestation, signed=True) + attestations.append(attestation) + # fill each created slot in state after inclusion delay + if slot - spec.MIN_ATTESTATION_INCLUSION_DELAY >= 0: + include_att = attestations[slot - spec.MIN_ATTESTATION_INCLUSION_DELAY] + add_attestations_to_state(spec, state, [include_att], state.slot) next_slot(spec, state) - add_attestations_to_state(spec, state, attestations, state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) + # ensure has not cross the epoch boundary assert spec.compute_epoch_of_slot(state.slot) == spec.GENESIS_EPOCH pre_state = deepcopy(state) From a6e543fd37110eaaaba9bb1521e9c28982159e86 Mon Sep 17 00:00:00 2001 From: protolambda Date: Tue, 24 Sep 2019 12:24:20 +0900 Subject: [PATCH 18/31] just signing, attestations are already filled by get_valid_attestation --- .../test_process_crosslinks.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py index 83e2bb017..18dd3ce81 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_crosslinks.py @@ -8,9 +8,8 @@ from eth2spec.test.helpers.state import ( from eth2spec.test.helpers.block import apply_empty_block from eth2spec.test.helpers.attestations import ( add_attestations_to_state, - fill_aggregate_attestation, get_valid_attestation, -) + sign_attestation) from eth2spec.test.phase_0.epoch_processing.run_epoch_process_base import run_epoch_processing_with @@ -32,9 +31,8 @@ def test_no_attestations(spec, state): def test_single_crosslink_update_from_current_epoch(spec, state): next_epoch(spec, state) - attestation = get_valid_attestation(spec, state) + attestation = get_valid_attestation(spec, state, signed=True) - fill_aggregate_attestation(spec, state, attestation, signed=True) add_attestations_to_state(spec, state, [attestation], state.slot + spec.MIN_ATTESTATION_INCLUSION_DELAY) assert len(state.current_epoch_attestations) == 1 @@ -53,9 +51,8 @@ def test_single_crosslink_update_from_current_epoch(spec, state): def test_single_crosslink_update_from_previous_epoch(spec, state): next_epoch(spec, state) - attestation = get_valid_attestation(spec, state) + attestation = get_valid_attestation(spec, state, signed=True) - fill_aggregate_attestation(spec, state, attestation, signed=True) add_attestations_to_state(spec, state, [attestation], state.slot + spec.SLOTS_PER_EPOCH) assert len(state.previous_epoch_attestations) == 1 @@ -89,8 +86,7 @@ def test_double_late_crosslink(spec, state): next_epoch(spec, state) state.slot += 4 - attestation_1 = get_valid_attestation(spec, state) - fill_aggregate_attestation(spec, state, attestation_1, signed=True) + attestation_1 = get_valid_attestation(spec, state, signed=True) # add attestation_1 to next epoch next_epoch(spec, state) @@ -99,7 +95,7 @@ def test_double_late_crosslink(spec, state): for _ in range(spec.SLOTS_PER_EPOCH): attestation_2 = get_valid_attestation(spec, state) if attestation_2.data.crosslink.shard == attestation_1.data.crosslink.shard: - fill_aggregate_attestation(spec, state, attestation_2, signed=True) + sign_attestation(spec, state, attestation_2) break next_slot(spec, state) apply_empty_block(spec, state) @@ -139,7 +135,7 @@ def test_tied_crosslink_between_epochs(spec, state): Ensure that ties on crosslinks between epochs are broken by previous epoch. """ prev_attestation = get_valid_attestation(spec, state) - fill_aggregate_attestation(spec, state, prev_attestation, signed=True) + sign_attestation(spec, state, prev_attestation) # add attestation at start of next epoch next_epoch(spec, state) @@ -149,7 +145,7 @@ def test_tied_crosslink_between_epochs(spec, state): for _ in range(spec.SLOTS_PER_EPOCH): cur_attestation = get_valid_attestation(spec, state) if cur_attestation.data.crosslink.shard == prev_attestation.data.crosslink.shard: - fill_aggregate_attestation(spec, state, cur_attestation, signed=True) + sign_attestation(spec, state, cur_attestation) break next_slot(spec, state) From 525d7330334435c3ff05cd9c1a7a9a58398218be Mon Sep 17 00:00:00 2001 From: protolambda Date: Tue, 24 Sep 2019 13:56:29 +0900 Subject: [PATCH 19/31] rewards testing now with cleaner attestation signing --- .../test_process_rewards_and_penalties.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) 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 40f6d781e..0d2547436 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 @@ -7,7 +7,6 @@ from eth2spec.test.helpers.state import ( ) from eth2spec.test.helpers.attestations import ( add_attestations_to_state, - fill_aggregate_attestation, get_valid_attestation, ) from eth2spec.test.phase_0.epoch_processing.run_epoch_process_base import run_epoch_processing_with @@ -37,8 +36,7 @@ def test_genesis_epoch_full_attestations_no_rewards(spec, state): for slot in range(spec.SLOTS_PER_EPOCH - 1): # create an attestation for each slot if slot < spec.SLOTS_PER_EPOCH: - attestation = get_valid_attestation(spec, state) - fill_aggregate_attestation(spec, state, attestation, signed=True) + attestation = get_valid_attestation(spec, state, signed=True) attestations.append(attestation) # fill each created slot in state after inclusion delay if slot - spec.MIN_ATTESTATION_INCLUSION_DELAY >= 0: @@ -64,8 +62,7 @@ def test_full_attestations(spec, state): for slot in range(spec.SLOTS_PER_EPOCH + spec.MIN_ATTESTATION_INCLUSION_DELAY): # create an attestation for each slot in epoch if slot < spec.SLOTS_PER_EPOCH: - attestation = get_valid_attestation(spec, state) - fill_aggregate_attestation(spec, state, attestation, signed=True) + attestation = get_valid_attestation(spec, state, signed=True) attestations.append(attestation) # fill each created slot in state after inclusion delay if slot - spec.MIN_ATTESTATION_INCLUSION_DELAY >= 0: @@ -112,8 +109,7 @@ def test_duplicate_attestation(spec, state): This test addresses this issue found at Interop https://github.com/djrtwo/interop-test-cases/tree/master/tests/prysm_16_duplicate_attestation_rewards """ - attestation = get_valid_attestation(spec, state) - fill_aggregate_attestation(spec, state, attestation, signed=True) + attestation = get_valid_attestation(spec, state, signed=True) indexed_attestation = spec.get_indexed_attestation(state, attestation) participants = indexed_attestation.custody_bit_0_indices + indexed_attestation.custody_bit_1_indices @@ -131,7 +127,8 @@ def test_duplicate_attestation(spec, state): next_epoch(spec, dup_state) # Run non-duplicate inclusion rewards for comparision. Do not yield test vectors - pre, post = run_process_rewards_and_penalties(spec, single_state) + for _ in run_process_rewards_and_penalties(spec, single_state): + pass # Output duplicate inclusion to test vectors yield from run_process_rewards_and_penalties(spec, dup_state) From d50ffa5f3e55123bf1b10c4d62ac6b235ff77c3c Mon Sep 17 00:00:00 2001 From: Denis Bogdanas Date: Mon, 30 Sep 2019 12:49:32 +0300 Subject: [PATCH 20/31] Added generator for rewards_and_penalties --- test_generators/epoch_processing/main.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test_generators/epoch_processing/main.py b/test_generators/epoch_processing/main.py index f0505ee94..0b6c49fb0 100644 --- a/test_generators/epoch_processing/main.py +++ b/test_generators/epoch_processing/main.py @@ -7,6 +7,7 @@ from eth2spec.test.phase_0.epoch_processing import ( test_process_final_updates, test_process_justification_and_finalization, test_process_registry_updates, + test_process_rewards_and_penalties, test_process_slashings ) from gen_base import gen_runner, gen_typing @@ -43,6 +44,8 @@ if __name__ == "__main__": create_provider('justification_and_finalization', test_process_justification_and_finalization, 'mainnet'), create_provider('registry_updates', test_process_registry_updates, 'minimal'), create_provider('registry_updates', test_process_registry_updates, 'mainnet'), + create_provider('rewards_and_penalties', test_process_rewards_and_penalties, 'minimal'), + create_provider('rewards_and_penalties', test_process_rewards_and_penalties, 'mainnet'), create_provider('slashings', test_process_slashings, 'minimal'), create_provider('slashings', test_process_slashings, 'mainnet'), ]) From f47e023bf0f651f1058c4fc942a722d919ce3bfa Mon Sep 17 00:00:00 2001 From: Denis Bogdanas Date: Sat, 21 Sep 2019 20:18:32 +0300 Subject: [PATCH 21/31] Test case for get_matching_target_attestations() with some real filtering going on on line `if a.data.target.root == get_block_root(state, epoch)`. Discovered by K coverage tool. --- ..._process_justification_and_finalization.py | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_justification_and_finalization.py b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_justification_and_finalization.py index 7dcdb42a4..0f2ab2ff7 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_justification_and_finalization.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_justification_and_finalization.py @@ -16,7 +16,7 @@ def get_shards_for_slot(spec, state, slot): return [shard + i for i in range(committees_per_slot)] -def add_mock_attestations(spec, state, epoch, source, target, sufficient_support=False): +def add_mock_attestations(spec, state, epoch, source, target, sufficient_support=False, messed_up_target=False): # we must be at the end of the epoch assert (state.slot + 1) % spec.SLOTS_PER_EPOCH == 0 @@ -67,6 +67,8 @@ def add_mock_attestations(spec, state, epoch, source, target, sufficient_support ), inclusion_delay=1, )) + if messed_up_target: + attestations[len(attestations) - 1].data.target.root = b'\x99' * 32 def get_checkpoints(spec, epoch): @@ -196,7 +198,7 @@ def finalize_on_123(spec, state, epoch, sufficient_support): assert state.finalized_checkpoint == old_finalized # no new finalized -def finalize_on_12(spec, state, epoch, sufficient_support): +def finalize_on_12(spec, state, epoch, sufficient_support, messed_up_target): assert epoch > 2 state.slot = (spec.SLOTS_PER_EPOCH * epoch) - 1 # skip ahead to just before epoch @@ -218,13 +220,13 @@ def finalize_on_12(spec, state, epoch, sufficient_support): epoch=epoch - 1, source=c2, target=c1, - sufficient_support=sufficient_support) + sufficient_support=sufficient_support, messed_up_target=messed_up_target) # process! yield from run_process_just_and_fin(spec, state) assert state.previous_justified_checkpoint == c2 # changed to old current - if sufficient_support: + if sufficient_support and not messed_up_target: assert state.current_justified_checkpoint == c1 # changed to 1st latest assert state.finalized_checkpoint == c2 # finalized previous justified epoch else: @@ -271,10 +273,16 @@ def test_123_poor_support(spec, state): @with_all_phases @spec_state_test def test_12_ok_support(spec, state): - yield from finalize_on_12(spec, state, 3, True) + yield from finalize_on_12(spec, state, 3, True, False) + + +@with_all_phases +@spec_state_test +def test_12_ok_support_messed_target(spec, state): + yield from finalize_on_12(spec, state, 3, True, True) @with_all_phases @spec_state_test def test_12_poor_support(spec, state): - yield from finalize_on_12(spec, state, 3, False) + yield from finalize_on_12(spec, state, 3, False, False) From 82d41db1b4d5ef8fa7f30e22246cc5647e0d9e24 Mon Sep 17 00:00:00 2001 From: Denis Bogdanas Date: Thu, 26 Sep 2019 22:55:58 +0300 Subject: [PATCH 22/31] Test case for get_beacon_proposer_index(), loop with multiple iterations. --- test_libs/pyspec/eth2spec/test/context.py | 18 +++++++++++++++++- .../pyspec/eth2spec/test/helpers/genesis.py | 6 +++--- .../test_process_attestation.py | 13 ++++++++++++- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/context.py b/test_libs/pyspec/eth2spec/test/context.py index 5a0ddb59d..05e3706e6 100644 --- a/test_libs/pyspec/eth2spec/test/context.py +++ b/test_libs/pyspec/eth2spec/test/context.py @@ -10,7 +10,19 @@ from .utils import vector_test, with_meta_tags def with_state(fn): def entry(*args, **kw): try: - kw['state'] = create_genesis_state(spec=kw['spec'], num_validators=spec_phase0.SLOTS_PER_EPOCH * 8) + kw['state'] = create_genesis_state(spec=kw['spec'], num_validators=spec_phase0.SLOTS_PER_EPOCH * 8, + validator_balance=spec_phase0.MAX_EFFECTIVE_BALANCE) + except KeyError: + raise TypeError('Spec decorator must come within state decorator to inject spec into state.') + return fn(*args, **kw) + return entry + + +def with_state_low_balance(fn): + def entry(*args, **kw): + try: + kw['state'] = create_genesis_state(spec=kw['spec'], num_validators=spec_phase0.SLOTS_PER_EPOCH * 8, + validator_balance=18 * 10**9) except KeyError: raise TypeError('Spec decorator must come within state decorator to inject spec into state.') return fn(*args, **kw) @@ -41,6 +53,10 @@ def spec_state_test(fn): return spec_test(with_state(fn)) +def spec_state_low_balance_test(fn): + return spec_test(with_state_low_balance(fn)) + + def expect_assertion_error(fn): bad = False try: diff --git a/test_libs/pyspec/eth2spec/test/helpers/genesis.py b/test_libs/pyspec/eth2spec/test/helpers/genesis.py index 11ab76b79..19fd65829 100644 --- a/test_libs/pyspec/eth2spec/test/helpers/genesis.py +++ b/test_libs/pyspec/eth2spec/test/helpers/genesis.py @@ -18,7 +18,7 @@ def build_mock_validator(spec, i: int, balance: int): ) -def create_genesis_state(spec, num_validators): +def create_genesis_state(spec, num_validators, validator_balance): deposit_root = b'\x42' * 32 state = spec.BeaconState( @@ -34,12 +34,12 @@ def create_genesis_state(spec, num_validators): # We "hack" in the initial validators, # as it is much faster than creating and processing genesis deposits for every single test case. - state.balances = [spec.MAX_EFFECTIVE_BALANCE] * num_validators + state.balances = [validator_balance] * num_validators state.validators = [build_mock_validator(spec, i, state.balances[i]) for i in range(num_validators)] # Process genesis activations for validator in state.validators: - if validator.effective_balance >= spec.MAX_EFFECTIVE_BALANCE: + if validator.effective_balance >= validator_balance: validator.activation_eligibility_epoch = spec.GENESIS_EPOCH validator.activation_epoch = spec.GENESIS_EPOCH diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py index 24ffd940b..a13db1815 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py @@ -1,4 +1,5 @@ -from eth2spec.test.context import spec_state_test, expect_assertion_error, always_bls, with_all_phases, with_phases +from eth2spec.test.context import spec_state_test, spec_state_low_balance_test, expect_assertion_error, always_bls, \ + with_all_phases, with_phases from eth2spec.test.helpers.attestations import ( get_valid_attestation, sign_aggregate_attestation, @@ -56,6 +57,16 @@ def test_success(spec, state): yield from run_attestation_processing(spec, state, attestation) +@with_all_phases +@spec_state_low_balance_test +def test_success_multi_proposer_index_iterations(spec, state): + state.slot += spec.SLOTS_PER_EPOCH * 2 + attestation = get_valid_attestation(spec, state, signed=True) + state.slot += spec.MIN_ATTESTATION_INCLUSION_DELAY + + yield from run_attestation_processing(spec, state, attestation) + + @with_all_phases @spec_state_test def test_success_previous_epoch(spec, state): From c108d1a3563e97e8b5f28f5645f66c4ddc3fdb6e Mon Sep 17 00:00:00 2001 From: Denis Bogdanas Date: Sun, 6 Oct 2019 14:59:13 +0300 Subject: [PATCH 23/31] test for initialize_beacon_state_from_eth1, case when some small deposits don't contribute to active balance. --- .../test/genesis/test_initialization.py | 39 ++++++++++++++++++- .../eth2spec/test/genesis/test_validity.py | 6 +-- .../pyspec/eth2spec/test/helpers/deposits.py | 7 ++-- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py b/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py index 2ff57be74..6c9a46ed3 100644 --- a/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py +++ b/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py @@ -8,7 +8,7 @@ from eth2spec.test.helpers.deposits import ( @spec_test def test_initialize_beacon_state_from_eth1(spec): deposit_count = spec.MIN_GENESIS_ACTIVE_VALIDATOR_COUNT - deposits, deposit_root = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True) + deposits, deposit_root, _ = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True) eth1_block_hash = b'\x12' * 32 eth1_timestamp = spec.MIN_GENESIS_TIME @@ -25,6 +25,43 @@ def test_initialize_beacon_state_from_eth1(spec): assert state.eth1_data.deposit_root == deposit_root assert state.eth1_data.deposit_count == deposit_count assert state.eth1_data.block_hash == eth1_block_hash + assert spec.get_total_active_balance(state) == deposit_count * spec.MAX_EFFECTIVE_BALANCE # yield state yield 'state', state + + +@with_phases(['phase0']) +@spec_test +def test_initialize_beacon_state_some_small_balances(spec): + main_deposit_count = spec.MIN_GENESIS_ACTIVE_VALIDATOR_COUNT + main_deposits, _, deposit_data_list = prepare_genesis_deposits(spec, main_deposit_count, + spec.MAX_EFFECTIVE_BALANCE, signed=True) + # For deposits above, and for another deposit_count, add a balance of EFFECTIVE_BALANCE_INCREMENT + small_deposit_count = main_deposit_count * 2 + small_deposits, deposit_root, _ = prepare_genesis_deposits(spec, small_deposit_count, + spec.MIN_DEPOSIT_AMOUNT, + signed=True, + deposit_data_list=deposit_data_list) + deposits = main_deposits + small_deposits + + eth1_block_hash = b'\x12' * 32 + eth1_timestamp = spec.MIN_GENESIS_TIME + + yield 'eth1_block_hash', eth1_block_hash + yield 'eth1_timestamp', eth1_timestamp + yield 'deposits', deposits + + # initialize beacon_state + state = spec.initialize_beacon_state_from_eth1(eth1_block_hash, eth1_timestamp, deposits) + + assert state.genesis_time == eth1_timestamp - eth1_timestamp % spec.SECONDS_PER_DAY + 2 * spec.SECONDS_PER_DAY + assert len(state.validators) == small_deposit_count + assert state.eth1_data.deposit_root == deposit_root + assert state.eth1_data.deposit_count == len(deposits) + assert state.eth1_data.block_hash == eth1_block_hash + # only main deposits participate to the active balance + assert spec.get_total_active_balance(state) == main_deposit_count * spec.MAX_EFFECTIVE_BALANCE + + # yield state + yield 'state', state \ No newline at end of file diff --git a/test_libs/pyspec/eth2spec/test/genesis/test_validity.py b/test_libs/pyspec/eth2spec/test/genesis/test_validity.py index 07ad3a73c..a003938e7 100644 --- a/test_libs/pyspec/eth2spec/test/genesis/test_validity.py +++ b/test_libs/pyspec/eth2spec/test/genesis/test_validity.py @@ -6,7 +6,7 @@ from eth2spec.test.helpers.deposits import ( def create_valid_beacon_state(spec): deposit_count = spec.MIN_GENESIS_ACTIVE_VALIDATOR_COUNT - deposits, _ = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True) + deposits, _, _ = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True) eth1_block_hash = b'\x12' * 32 eth1_timestamp = spec.MIN_GENESIS_TIME @@ -65,7 +65,7 @@ def test_is_valid_genesis_state_true_more_balance(spec): @spec_test def test_is_valid_genesis_state_true_one_more_validator(spec): deposit_count = spec.MIN_GENESIS_ACTIVE_VALIDATOR_COUNT + 1 - deposits, _ = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True) + deposits, _, _ = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True) eth1_block_hash = b'\x12' * 32 eth1_timestamp = spec.MIN_GENESIS_TIME @@ -78,7 +78,7 @@ def test_is_valid_genesis_state_true_one_more_validator(spec): @spec_test def test_is_valid_genesis_state_false_not_enough_validator(spec): deposit_count = spec.MIN_GENESIS_ACTIVE_VALIDATOR_COUNT - 1 - deposits, _ = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True) + deposits, _, _ = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True) eth1_block_hash = b'\x12' * 32 eth1_timestamp = spec.MIN_GENESIS_TIME diff --git a/test_libs/pyspec/eth2spec/test/helpers/deposits.py b/test_libs/pyspec/eth2spec/test/helpers/deposits.py index 8dc6b3b58..b3d230eaf 100644 --- a/test_libs/pyspec/eth2spec/test/helpers/deposits.py +++ b/test_libs/pyspec/eth2spec/test/helpers/deposits.py @@ -55,8 +55,9 @@ def build_deposit(spec, return deposit, root, deposit_data_list -def prepare_genesis_deposits(spec, genesis_validator_count, amount, signed=False): - deposit_data_list = [] +def prepare_genesis_deposits(spec, genesis_validator_count, amount, signed=False, deposit_data_list=None): + if deposit_data_list is None: + deposit_data_list = [] genesis_deposits = [] for validator_index in range(genesis_validator_count): pubkey = pubkeys[validator_index] @@ -75,7 +76,7 @@ def prepare_genesis_deposits(spec, genesis_validator_count, amount, signed=False ) genesis_deposits.append(deposit) - return genesis_deposits, root + return genesis_deposits, root, deposit_data_list def prepare_state_and_deposit(spec, state, validator_index, amount, withdrawal_credentials=None, signed=False): From 1a65570c9b8a1f9847d42b70c738b4f4180f148f Mon Sep 17 00:00:00 2001 From: Denis Bogdanas Date: Sun, 6 Oct 2019 19:55:27 +0300 Subject: [PATCH 24/31] test_process_rewards_and_penalties.py: test for case when eligible_validator_indices in get_attestation_deltas() != state.validators. In this test some validators were just never active. --- test_libs/pyspec/eth2spec/test/context.py | 18 ++++++++- .../test/genesis/test_initialization.py | 4 +- .../pyspec/eth2spec/test/helpers/genesis.py | 37 +++++++++++++++++++ .../test_process_rewards_and_penalties.py | 36 +++++++++++++++++- 4 files changed, 91 insertions(+), 4 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/context.py b/test_libs/pyspec/eth2spec/test/context.py index 05e3706e6..c1c085b16 100644 --- a/test_libs/pyspec/eth2spec/test/context.py +++ b/test_libs/pyspec/eth2spec/test/context.py @@ -2,7 +2,7 @@ from eth2spec.phase0 import spec as spec_phase0 from eth2spec.phase1 import spec as spec_phase1 from eth2spec.utils import bls -from .helpers.genesis import create_genesis_state +from .helpers.genesis import create_genesis_state, create_genesis_state_misc_balances from .utils import vector_test, with_meta_tags @@ -29,6 +29,18 @@ def with_state_low_balance(fn): return entry +def with_state_misc_balances(fn): + def entry(*args, **kw): + try: + validator_balances = [spec_phase0.MAX_EFFECTIVE_BALANCE] * (spec_phase0.SLOTS_PER_EPOCH * 8) + [ + spec_phase0.MIN_DEPOSIT_AMOUNT] * spec_phase0.SLOTS_PER_EPOCH + kw['state'] = create_genesis_state_misc_balances(spec=kw['spec'], validator_balances=validator_balances) + except KeyError: + raise TypeError('Spec decorator must come within state decorator to inject spec into state.') + return fn(*args, **kw) + return entry + + # BLS is turned off by default *for performance purposes during TESTING*. # The runner of the test can indicate the preferred setting (test generators prefer BLS to be ON). # - Some tests are marked as BLS-requiring, and ignore this setting. @@ -57,6 +69,10 @@ def spec_state_low_balance_test(fn): return spec_test(with_state_low_balance(fn)) +def spec_state_misc_balances_test(fn): + return spec_test(with_state_misc_balances(fn)) + + def expect_assertion_error(fn): bad = False try: diff --git a/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py b/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py index 6c9a46ed3..462065bb9 100644 --- a/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py +++ b/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py @@ -36,7 +36,7 @@ def test_initialize_beacon_state_from_eth1(spec): def test_initialize_beacon_state_some_small_balances(spec): main_deposit_count = spec.MIN_GENESIS_ACTIVE_VALIDATOR_COUNT main_deposits, _, deposit_data_list = prepare_genesis_deposits(spec, main_deposit_count, - spec.MAX_EFFECTIVE_BALANCE, signed=True) + spec.MAX_EFFECTIVE_BALANCE, signed=True) # For deposits above, and for another deposit_count, add a balance of EFFECTIVE_BALANCE_INCREMENT small_deposit_count = main_deposit_count * 2 small_deposits, deposit_root, _ = prepare_genesis_deposits(spec, small_deposit_count, @@ -64,4 +64,4 @@ def test_initialize_beacon_state_some_small_balances(spec): assert spec.get_total_active_balance(state) == main_deposit_count * spec.MAX_EFFECTIVE_BALANCE # yield state - yield 'state', state \ No newline at end of file + yield 'state', state diff --git a/test_libs/pyspec/eth2spec/test/helpers/genesis.py b/test_libs/pyspec/eth2spec/test/helpers/genesis.py index 19fd65829..06322d3a1 100644 --- a/test_libs/pyspec/eth2spec/test/helpers/genesis.py +++ b/test_libs/pyspec/eth2spec/test/helpers/genesis.py @@ -1,6 +1,7 @@ from eth2spec.test.helpers.keys import pubkeys from eth2spec.utils.ssz.ssz_impl import hash_tree_root from eth2spec.utils.ssz.ssz_typing import List +import copy def build_mock_validator(spec, i: int, balance: int): @@ -52,3 +53,39 @@ def create_genesis_state(spec, num_validators, validator_balance): state.compact_committees_roots[index] = genesis_compact_committees_root return state + + +def create_genesis_state_misc_balances(spec, validator_balances): + deposit_root = b'\x42' * 32 + + state = spec.BeaconState( + genesis_time=0, + eth1_deposit_index=len(validator_balances), + eth1_data=spec.Eth1Data( + deposit_root=deposit_root, + deposit_count=len(validator_balances), + block_hash=spec.Hash(), + ), + latest_block_header=spec.BeaconBlockHeader(body_root=spec.hash_tree_root(spec.BeaconBlockBody())), + ) + + # We "hack" in the initial validators, + # as it is much faster than creating and processing genesis deposits for every single test case. + state.balances = copy.deepcopy(validator_balances) + state.validators = [build_mock_validator(spec, i, state.balances[i]) for i in range(len(validator_balances))] + + # Process genesis activations + for validator in state.validators: + if validator.effective_balance > spec.EJECTION_BALANCE: + validator.activation_eligibility_epoch = spec.GENESIS_EPOCH + validator.activation_epoch = spec.GENESIS_EPOCH + + genesis_active_index_root = hash_tree_root(List[spec.ValidatorIndex, spec.VALIDATOR_REGISTRY_LIMIT]( + spec.get_active_validator_indices(state, spec.GENESIS_EPOCH))) + genesis_compact_committees_root = hash_tree_root(List[spec.ValidatorIndex, spec.VALIDATOR_REGISTRY_LIMIT]( + spec.get_active_validator_indices(state, spec.GENESIS_EPOCH))) + for index in range(spec.EPOCHS_PER_HISTORICAL_VECTOR): + state.active_index_roots[index] = genesis_active_index_root + state.compact_committees_roots[index] = genesis_compact_committees_root + + return state 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 0d2547436..857ad8bb0 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 @@ -1,6 +1,6 @@ from copy import deepcopy -from eth2spec.test.context import spec_state_test, with_all_phases +from eth2spec.test.context import spec_state_test, spec_state_misc_balances_test, with_all_phases from eth2spec.test.helpers.state import ( next_epoch, next_slot, @@ -86,6 +86,40 @@ def test_full_attestations(spec, state): assert state.balances[index] < pre_state.balances[index] +@with_all_phases +@spec_state_misc_balances_test +def test_full_attestations_misc_balances(spec, state): + attestations = [] + for slot in range(spec.SLOTS_PER_EPOCH + spec.MIN_ATTESTATION_INCLUSION_DELAY): + # create an attestation for each slot in epoch + if slot < spec.SLOTS_PER_EPOCH: + attestation = get_valid_attestation(spec, state, signed=True) + attestations.append(attestation) + # fill each created slot in state after inclusion delay + if slot - spec.MIN_ATTESTATION_INCLUSION_DELAY >= 0: + include_att = attestations[slot - spec.MIN_ATTESTATION_INCLUSION_DELAY] + add_attestations_to_state(spec, state, [include_att], state.slot) + next_slot(spec, state) + + assert spec.compute_epoch_of_slot(state.slot) == spec.GENESIS_EPOCH + 1 + assert len(state.previous_epoch_attestations) == spec.SLOTS_PER_EPOCH + + pre_state = deepcopy(state) + + yield from run_process_rewards_and_penalties(spec, state) + + attesting_indices = spec.get_unslashed_attesting_indices(state, attestations) + assert len(attesting_indices) > 0 + assert len(attesting_indices) != len(pre_state.validators) + for index in range(len(pre_state.validators)): + if index in attesting_indices: + assert state.balances[index] > pre_state.balances[index] + elif spec.is_active_validator(pre_state.validators[index], spec.compute_epoch_of_slot(state.slot)): + assert state.balances[index] < pre_state.balances[index] + else: + assert state.balances[index] == pre_state.balances[index] + + @with_all_phases @spec_state_test def test_no_attestations_all_penalties(spec, state): From baded822472c45ba136a0c9e8df2c219601f1c16 Mon Sep 17 00:00:00 2001 From: Denis Bogdanas Date: Mon, 7 Oct 2019 13:50:24 +0300 Subject: [PATCH 25/31] test for process_rewards_and_penalties: Case when some eligible attestations are slashed. Modifies attesting_balance and consequently rewards/penalties. --- .../test_process_rewards_and_penalties.py | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) 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 857ad8bb0..09f4b9eb0 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 @@ -170,3 +170,42 @@ def test_duplicate_attestation(spec, state): for index in participants: assert state.balances[index] < single_state.balances[index] assert single_state.balances[index] == dup_state.balances[index] + + +@with_all_phases +@spec_state_test +# Case when some eligible attestations are slashed. Modifies attesting_balance and consequently rewards/penalties. +def test_attestations_some_slashed(spec, state): + attestations = [] + for slot in range(spec.SLOTS_PER_EPOCH + spec.MIN_ATTESTATION_INCLUSION_DELAY): + # create an attestation for each slot in epoch + if slot < spec.SLOTS_PER_EPOCH: + attestation = get_valid_attestation(spec, state, signed=True) + attestations.append(attestation) + # fill each created slot in state after inclusion delay + if slot - spec.MIN_ATTESTATION_INCLUSION_DELAY >= 0: + include_att = attestations[slot - spec.MIN_ATTESTATION_INCLUSION_DELAY] + add_attestations_to_state(spec, state, [include_att], state.slot) + next_slot(spec, state) + + attesting_indices_before_slashings = spec.get_unslashed_attesting_indices(state, attestations) + + # Slash maximum amount of validators allowed per epoch. + for i in range(spec.MIN_PER_EPOCH_CHURN_LIMIT): + spec.slash_validator(state, list(attesting_indices_before_slashings)[i]) + + assert spec.compute_epoch_of_slot(state.slot) == spec.GENESIS_EPOCH + 1 + assert len(state.previous_epoch_attestations) == spec.SLOTS_PER_EPOCH + + pre_state = deepcopy(state) + + yield from run_process_rewards_and_penalties(spec, state) + + attesting_indices = spec.get_unslashed_attesting_indices(state, attestations) + assert len(attesting_indices) > 0 + assert len(attesting_indices_before_slashings) - len(attesting_indices) == spec.MIN_PER_EPOCH_CHURN_LIMIT + for index in range(len(pre_state.validators)): + if index in attesting_indices: + assert state.balances[index] > pre_state.balances[index] + else: + assert state.balances[index] < pre_state.balances[index] From e8a3eac55e493bdc0f693af3ec2e8cf73efd7d7f Mon Sep 17 00:00:00 2001 From: protolambda Date: Wed, 23 Oct 2019 16:33:48 +0800 Subject: [PATCH 26/31] cleanup code duplication, and build new context util for state customization --- test_libs/pyspec/eth2spec/test/context.py | 87 +++++++++++-------- .../pyspec/eth2spec/test/helpers/genesis.py | 40 +-------- .../test_process_attestation.py | 7 +- .../test_process_rewards_and_penalties.py | 33 +++---- 4 files changed, 69 insertions(+), 98 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/context.py b/test_libs/pyspec/eth2spec/test/context.py index c1c085b16..462c50685 100644 --- a/test_libs/pyspec/eth2spec/test/context.py +++ b/test_libs/pyspec/eth2spec/test/context.py @@ -2,43 +2,62 @@ from eth2spec.phase0 import spec as spec_phase0 from eth2spec.phase1 import spec as spec_phase1 from eth2spec.utils import bls -from .helpers.genesis import create_genesis_state, create_genesis_state_misc_balances +from .helpers.genesis import create_genesis_state from .utils import vector_test, with_meta_tags - -def with_state(fn): - def entry(*args, **kw): - try: - kw['state'] = create_genesis_state(spec=kw['spec'], num_validators=spec_phase0.SLOTS_PER_EPOCH * 8, - validator_balance=spec_phase0.MAX_EFFECTIVE_BALANCE) - except KeyError: - raise TypeError('Spec decorator must come within state decorator to inject spec into state.') - return fn(*args, **kw) - return entry +from typing import Any, Callable, Sequence -def with_state_low_balance(fn): - def entry(*args, **kw): - try: - kw['state'] = create_genesis_state(spec=kw['spec'], num_validators=spec_phase0.SLOTS_PER_EPOCH * 8, - validator_balance=18 * 10**9) - except KeyError: - raise TypeError('Spec decorator must come within state decorator to inject spec into state.') - return fn(*args, **kw) - return entry +def with_custom_state(balances_fn: Callable[[Any], Sequence[int]], + threshold_fn: Callable[[Any], int]): + def deco(fn): + def entry(*args, **kw): + try: + spec = kw['spec'] + + balances = balances_fn(spec) + activation_threshold = threshold_fn(spec) + + kw['state'] = create_genesis_state(spec=spec, validator_balances=balances, + activation_threshold=activation_threshold) + except KeyError: + raise TypeError('Spec decorator must come within state decorator to inject spec into state.') + return fn(*args, **kw) + return entry + return deco -def with_state_misc_balances(fn): - def entry(*args, **kw): - try: - validator_balances = [spec_phase0.MAX_EFFECTIVE_BALANCE] * (spec_phase0.SLOTS_PER_EPOCH * 8) + [ - spec_phase0.MIN_DEPOSIT_AMOUNT] * spec_phase0.SLOTS_PER_EPOCH - kw['state'] = create_genesis_state_misc_balances(spec=kw['spec'], validator_balances=validator_balances) - except KeyError: - raise TypeError('Spec decorator must come within state decorator to inject spec into state.') - return fn(*args, **kw) - return entry +def default_activation_threshold(spec): + return spec.MAX_EFFECTIVE_BALANCE + + +def default_balances(spec): + num_validators = spec.SLOTS_PER_EPOCH * 8 + return [spec.MAX_EFFECTIVE_BALANCE] * num_validators + + +with_state = with_custom_state(default_balances, default_activation_threshold) + + +def low_balances(spec): + """ + Helper method to create a series of low balances. Usage: `@with_validator_balances(low_balances)` + """ + num_validators = spec.SLOTS_PER_EPOCH * 8 + # Technically the balances cannot be this low starting from genesis, but it is useful for testing + low_balance = 18 * 10 ** 9 + return [low_balance] * num_validators + + +def misc_balances(spec): + """ + Helper method to create a series of balances that includes some misc. balances. + Usage: `@with_validator_balances(misc_balances)` + """ + num_validators = spec.SLOTS_PER_EPOCH * 8 + num_misc_validators = spec.SLOTS_PER_EPOCH + return [spec.MAX_EFFECTIVE_BALANCE] * num_validators + [spec.MIN_DEPOSIT_AMOUNT] * num_misc_validators # BLS is turned off by default *for performance purposes during TESTING*. @@ -65,14 +84,6 @@ def spec_state_test(fn): return spec_test(with_state(fn)) -def spec_state_low_balance_test(fn): - return spec_test(with_state_low_balance(fn)) - - -def spec_state_misc_balances_test(fn): - return spec_test(with_state_misc_balances(fn)) - - def expect_assertion_error(fn): bad = False try: diff --git a/test_libs/pyspec/eth2spec/test/helpers/genesis.py b/test_libs/pyspec/eth2spec/test/helpers/genesis.py index 06322d3a1..65d7efc4f 100644 --- a/test_libs/pyspec/eth2spec/test/helpers/genesis.py +++ b/test_libs/pyspec/eth2spec/test/helpers/genesis.py @@ -19,43 +19,7 @@ def build_mock_validator(spec, i: int, balance: int): ) -def create_genesis_state(spec, num_validators, validator_balance): - deposit_root = b'\x42' * 32 - - state = spec.BeaconState( - genesis_time=0, - eth1_deposit_index=num_validators, - eth1_data=spec.Eth1Data( - deposit_root=deposit_root, - deposit_count=num_validators, - block_hash=spec.Hash(), - ), - latest_block_header=spec.BeaconBlockHeader(body_root=spec.hash_tree_root(spec.BeaconBlockBody())), - ) - - # We "hack" in the initial validators, - # as it is much faster than creating and processing genesis deposits for every single test case. - state.balances = [validator_balance] * num_validators - state.validators = [build_mock_validator(spec, i, state.balances[i]) for i in range(num_validators)] - - # Process genesis activations - for validator in state.validators: - if validator.effective_balance >= validator_balance: - validator.activation_eligibility_epoch = spec.GENESIS_EPOCH - validator.activation_epoch = spec.GENESIS_EPOCH - - genesis_active_index_root = hash_tree_root(List[spec.ValidatorIndex, spec.VALIDATOR_REGISTRY_LIMIT]( - spec.get_active_validator_indices(state, spec.GENESIS_EPOCH))) - genesis_compact_committees_root = hash_tree_root(List[spec.ValidatorIndex, spec.VALIDATOR_REGISTRY_LIMIT]( - spec.get_active_validator_indices(state, spec.GENESIS_EPOCH))) - for index in range(spec.EPOCHS_PER_HISTORICAL_VECTOR): - state.active_index_roots[index] = genesis_active_index_root - state.compact_committees_roots[index] = genesis_compact_committees_root - - return state - - -def create_genesis_state_misc_balances(spec, validator_balances): +def create_genesis_state(spec, validator_balances, activation_threshold): deposit_root = b'\x42' * 32 state = spec.BeaconState( @@ -76,7 +40,7 @@ def create_genesis_state_misc_balances(spec, validator_balances): # Process genesis activations for validator in state.validators: - if validator.effective_balance > spec.EJECTION_BALANCE: + if validator.effective_balance >= activation_threshold: validator.activation_eligibility_epoch = spec.GENESIS_EPOCH validator.activation_epoch = spec.GENESIS_EPOCH diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py index a13db1815..0445d763e 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py @@ -1,5 +1,5 @@ -from eth2spec.test.context import spec_state_test, spec_state_low_balance_test, expect_assertion_error, always_bls, \ - with_all_phases, with_phases +from eth2spec.test.context import spec_state_test, expect_assertion_error, always_bls, \ + with_all_phases, with_phases, spec_test, low_balances, with_custom_state from eth2spec.test.helpers.attestations import ( get_valid_attestation, sign_aggregate_attestation, @@ -58,7 +58,8 @@ def test_success(spec, state): @with_all_phases -@spec_state_low_balance_test +@spec_test +@with_custom_state(balances_fn=low_balances, threshold_fn=lambda spec: spec.EJECTION_BALANCE) def test_success_multi_proposer_index_iterations(spec, state): state.slot += spec.SLOTS_PER_EPOCH * 2 attestation = get_valid_attestation(spec, state, signed=True) 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 09f4b9eb0..e8be79aaf 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 @@ -1,6 +1,7 @@ from copy import deepcopy -from eth2spec.test.context import spec_state_test, spec_state_misc_balances_test, with_all_phases +from eth2spec.test.context import spec_state_test, with_all_phases, spec_test, \ + misc_balances, with_custom_state, default_activation_threshold from eth2spec.test.helpers.state import ( next_epoch, next_slot, @@ -55,9 +56,7 @@ def test_genesis_epoch_full_attestations_no_rewards(spec, state): assert state.balances[index] == pre_state.balances[index] -@with_all_phases -@spec_state_test -def test_full_attestations(spec, state): +def prepare_state_with_full_attestations(spec, state): attestations = [] for slot in range(spec.SLOTS_PER_EPOCH + spec.MIN_ATTESTATION_INCLUSION_DELAY): # create an attestation for each slot in epoch @@ -73,6 +72,14 @@ def test_full_attestations(spec, state): assert spec.compute_epoch_of_slot(state.slot) == spec.GENESIS_EPOCH + 1 assert len(state.previous_epoch_attestations) == spec.SLOTS_PER_EPOCH + return attestations + + +@with_all_phases +@spec_state_test +def test_full_attestations(spec, state): + attestations = prepare_state_with_full_attestations(spec, state) + pre_state = deepcopy(state) yield from run_process_rewards_and_penalties(spec, state) @@ -87,22 +94,10 @@ def test_full_attestations(spec, state): @with_all_phases -@spec_state_misc_balances_test +@spec_test +@with_custom_state(balances_fn=misc_balances, threshold_fn=default_activation_threshold) def test_full_attestations_misc_balances(spec, state): - attestations = [] - for slot in range(spec.SLOTS_PER_EPOCH + spec.MIN_ATTESTATION_INCLUSION_DELAY): - # create an attestation for each slot in epoch - if slot < spec.SLOTS_PER_EPOCH: - attestation = get_valid_attestation(spec, state, signed=True) - attestations.append(attestation) - # fill each created slot in state after inclusion delay - if slot - spec.MIN_ATTESTATION_INCLUSION_DELAY >= 0: - include_att = attestations[slot - spec.MIN_ATTESTATION_INCLUSION_DELAY] - add_attestations_to_state(spec, state, [include_att], state.slot) - next_slot(spec, state) - - assert spec.compute_epoch_of_slot(state.slot) == spec.GENESIS_EPOCH + 1 - assert len(state.previous_epoch_attestations) == spec.SLOTS_PER_EPOCH + attestations = prepare_state_with_full_attestations(spec, state) pre_state = deepcopy(state) From 9deda149da900da8196dd1271dccb205840fe0e7 Mon Sep 17 00:00:00 2001 From: protolambda Date: Thu, 24 Oct 2019 00:00:27 +0800 Subject: [PATCH 27/31] fix list lookup --- .../epoch_processing/test_process_rewards_and_penalties.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 e8be79aaf..b4bcc7425 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 @@ -183,11 +183,11 @@ def test_attestations_some_slashed(spec, state): add_attestations_to_state(spec, state, [include_att], state.slot) next_slot(spec, state) - attesting_indices_before_slashings = spec.get_unslashed_attesting_indices(state, attestations) + attesting_indices_before_slashings = list(spec.get_unslashed_attesting_indices(state, attestations)) # Slash maximum amount of validators allowed per epoch. for i in range(spec.MIN_PER_EPOCH_CHURN_LIMIT): - spec.slash_validator(state, list(attesting_indices_before_slashings)[i]) + spec.slash_validator(state, attesting_indices_before_slashings[i]) assert spec.compute_epoch_of_slot(state.slot) == spec.GENESIS_EPOCH + 1 assert len(state.previous_epoch_attestations) == spec.SLOTS_PER_EPOCH From 473c42b994ec88ab80584de4efbf1e860c979597 Mon Sep 17 00:00:00 2001 From: protolambda Date: Thu, 24 Oct 2019 00:44:00 +0800 Subject: [PATCH 28/31] temporarily disable a few mainnet tests in generator; too long --- test_generators/epoch_processing/main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test_generators/epoch_processing/main.py b/test_generators/epoch_processing/main.py index 0b6c49fb0..d28cb4832 100644 --- a/test_generators/epoch_processing/main.py +++ b/test_generators/epoch_processing/main.py @@ -45,7 +45,8 @@ if __name__ == "__main__": create_provider('registry_updates', test_process_registry_updates, 'minimal'), create_provider('registry_updates', test_process_registry_updates, 'mainnet'), create_provider('rewards_and_penalties', test_process_rewards_and_penalties, 'minimal'), - create_provider('rewards_and_penalties', test_process_rewards_and_penalties, 'mainnet'), + # runs full epochs filled with data, with uncached ssz. Disabled for now. + # create_provider('rewards_and_penalties', test_process_rewards_and_penalties, 'mainnet'), create_provider('slashings', test_process_slashings, 'minimal'), create_provider('slashings', test_process_slashings, 'mainnet'), ]) From 691c25ab38ab7b1bf3a21ba1400ad3aaaa448866 Mon Sep 17 00:00:00 2001 From: gnattishness <1620192+gnattishness@users.noreply.github.com> Date: Tue, 8 Oct 2019 15:44:07 +1100 Subject: [PATCH 29/31] Update config_helper requirements. Fixes build errors for python 3.8. --- test_libs/config_helpers/requirements.txt | 2 +- test_libs/config_helpers/setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test_libs/config_helpers/requirements.txt b/test_libs/config_helpers/requirements.txt index f2f208c3f..6c7334268 100644 --- a/test_libs/config_helpers/requirements.txt +++ b/test_libs/config_helpers/requirements.txt @@ -1 +1 @@ -ruamel.yaml==0.15.96 +ruamel.yaml==0.16.5 diff --git a/test_libs/config_helpers/setup.py b/test_libs/config_helpers/setup.py index 9f0ea0641..3f893f3d4 100644 --- a/test_libs/config_helpers/setup.py +++ b/test_libs/config_helpers/setup.py @@ -4,6 +4,6 @@ setup( name='config_helpers', packages=['preset_loader'], install_requires=[ - "ruamel.yaml==0.15.96" + "ruamel.yaml==0.16.5" ] ) From b2ad6069d439683ebb855d211128b8823249a49f Mon Sep 17 00:00:00 2001 From: Danny Ryan Date: Thu, 24 Oct 2019 14:55:56 +0800 Subject: [PATCH 30/31] minor nitpick to PR --- .../test_process_justification_and_finalization.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_justification_and_finalization.py b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_justification_and_finalization.py index 0f2ab2ff7..175ff54c9 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_justification_and_finalization.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/epoch_processing/test_process_justification_and_finalization.py @@ -220,7 +220,8 @@ def finalize_on_12(spec, state, epoch, sufficient_support, messed_up_target): epoch=epoch - 1, source=c2, target=c1, - sufficient_support=sufficient_support, messed_up_target=messed_up_target) + sufficient_support=sufficient_support, + messed_up_target=messed_up_target) # process! yield from run_process_just_and_fin(spec, state) From 0cc50725edcd514f954ce7830dfa23979ec25068 Mon Sep 17 00:00:00 2001 From: protolambda Date: Thu, 24 Oct 2019 15:31:43 +0800 Subject: [PATCH 31/31] py docs fixes --- test_libs/pyspec/eth2spec/test/context.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/context.py b/test_libs/pyspec/eth2spec/test/context.py index 462c50685..33171c4e1 100644 --- a/test_libs/pyspec/eth2spec/test/context.py +++ b/test_libs/pyspec/eth2spec/test/context.py @@ -29,10 +29,18 @@ def with_custom_state(balances_fn: Callable[[Any], Sequence[int]], def default_activation_threshold(spec): + """ + Helper method to use the default balance activation threshold for state creation for tests. + Usage: `@with_custom_state(threshold_fn=default_activation_threshold, ...)` + """ return spec.MAX_EFFECTIVE_BALANCE def default_balances(spec): + """ + Helper method to create a series of default balances. + Usage: `@with_custom_state(balances_fn=default_balances, ...)` + """ num_validators = spec.SLOTS_PER_EPOCH * 8 return [spec.MAX_EFFECTIVE_BALANCE] * num_validators @@ -42,7 +50,8 @@ with_state = with_custom_state(default_balances, default_activation_threshold) def low_balances(spec): """ - Helper method to create a series of low balances. Usage: `@with_validator_balances(low_balances)` + Helper method to create a series of low balances. + Usage: `@with_custom_state(balances_fn=low_balances, ...)` """ num_validators = spec.SLOTS_PER_EPOCH * 8 # Technically the balances cannot be this low starting from genesis, but it is useful for testing @@ -53,7 +62,7 @@ def low_balances(spec): def misc_balances(spec): """ Helper method to create a series of balances that includes some misc. balances. - Usage: `@with_validator_balances(misc_balances)` + Usage: `@with_custom_state(balances_fn=misc_balances, ...)` """ num_validators = spec.SLOTS_PER_EPOCH * 8 num_misc_validators = spec.SLOTS_PER_EPOCH