From 56309342c05d324e359591b902abc355fba82fb1 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Thu, 21 May 2020 15:33:47 +0200 Subject: [PATCH 1/2] Use `Bytes32` for `error_message` `ErrorMessage.error_message` is a leftover from an older version of SSZ that was able to encode unbounded lists. This is no longer the case - all collection types now have a fixed upper bound on length. In general, the `error_message`, just like the `graffitti` field, should not be interpreted in any particular way except for debugging and vanity - as such, using the same type, a `Bytes32`, seems reasonable. An alternative would be `List[byte, 256]` which maybe could be "reasonably backwards compatible" with whatever clients are are doing now - depending on how they are dealing with this field type that no longer exists in the SSZ spec :) It would however be the only place where `List[uintN, N]` is used in the current spec. As an exercise, this could be considered a security issue since it's essentially unbounded and undefined behaviour. --- specs/phase0/p2p-interface.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specs/phase0/p2p-interface.md b/specs/phase0/p2p-interface.md index 118be8839..84db361c9 100644 --- a/specs/phase0/p2p-interface.md +++ b/specs/phase0/p2p-interface.md @@ -391,11 +391,11 @@ The `ErrorMessage` schema is: ``` ( - error_message: String + error_message: Bytes32 ) ``` -*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. +*Note*: By convention, the `error_message` is a sequence of bytes that can be interpreted as a UTF-8 string up to 32 bytes - a 0 byte shortens the string in this interpretation. Clients MUST treat as valid any bytes. ### Encoding strategies From 33e115f8c65407357852f1605399bfaf79f933cb Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Sat, 30 May 2020 20:59:12 +0200 Subject: [PATCH 2/2] Clean up remaining `[]` List syntax ..and use List[byte, 256] for error message --- specs/phase0/p2p-interface.md | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/specs/phase0/p2p-interface.md b/specs/phase0/p2p-interface.md index 84db361c9..a77f1851a 100644 --- a/specs/phase0/p2p-interface.md +++ b/specs/phase0/p2p-interface.md @@ -150,6 +150,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. | +| `MAX_REQUEST_BLOCKS` | `2**10` (= 1024) | Maximum number of blocks in a single request | | `MAX_CHUNK_SIZE` | `2**20` (1048576, 1 MiB) | The maximum allowed size of uncompressed req/resp chunked responses. | | `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. | @@ -391,11 +392,11 @@ The `ErrorMessage` schema is: ``` ( - error_message: Bytes32 + error_message: List[byte, 256] ) ``` -*Note*: By convention, the `error_message` is a sequence of bytes that can be interpreted as a UTF-8 string up to 32 bytes - a 0 byte shortens the string in this interpretation. Clients MUST treat as valid any bytes. +*Note*: By convention, the `error_message` is a sequence of bytes that MAY be interpreted as a UTF-8 string (for debugging purposes). Clients MUST treat as valid any byte sequences. ### Encoding strategies @@ -443,9 +444,9 @@ In case of an invalid input (header or payload), a reader MUST: 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-lists (for example `[]SignedBeaconBlock`) send their +Responses that are SSZ-lists (for example `List[SignedBeaconBlock, ...]`) send their constituents individually as `response_chunk`s. For example, the -`[]SignedBeaconBlock` response type sends zero or more `response_chunk`s. Each _successful_ `response_chunk` contains a single `SignedBeaconBlock` payload. +`List[SignedBeaconBlock, ...]` response type sends zero or more `response_chunk`s. Each _successful_ `response_chunk` contains a single `SignedBeaconBlock` payload. ### Messages @@ -528,7 +529,7 @@ Request Content: Response Content: ``` ( - []SignedBeaconBlock + List[SignedBeaconBlock, MAX_REQUEST_BLOCKS] ) ``` @@ -545,7 +546,7 @@ The response MUST consist of zero or more `response_chunk`. Each _successful_ `r Clients MUST keep a record of signed blocks seen since the since the start of the weak subjectivity period and MUST support serving requests of blocks up to their own `head_block_root`. -Clients MUST respond with at least the first block that exists in the range, if they have it. +Clients MUST respond with at least the first block that exists in the range, if they have it, and no more than `MAX_REQUEST_BLOCKS` blocks. The following blocks, where they exist, MUST be send in consecutive order. @@ -568,7 +569,7 @@ Request Content: ``` ( - []Root + List[Root, MAX_REQUEST_BLOCKS] ) ``` @@ -576,12 +577,14 @@ Response Content: ``` ( - []SignedBeaconBlock + List[SignedBeaconBlock, MAX_REQUEST_BLOCKS] ) ``` Requests blocks by block root (= `hash_tree_root(SignedBeaconBlock.message)`). The response is a list of `SignedBeaconBlock` 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. +No more than `MAX_REQUEST_BLOCKS` may be requested at a time. + `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.