From 607e23949c6e489c2f399bbbbaab7247cd8fc1be Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Tue, 19 May 2020 07:51:53 +0200 Subject: [PATCH 1/4] require blocks to be ordered consecutively in block range request Per the spec, if I request range 5-10, it is permissible for a client to answer with block 7, 9 - even if the blocks 5, 6 and 8 exist. Because blocks 7 and 9 cannot be validated as they arrive in such a request, it seems better to close this gap - this update adds the spec language that forbids well-behaving clients from answering this way. --- specs/phase0/p2p-interface.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/specs/phase0/p2p-interface.md b/specs/phase0/p2p-interface.md index 0e8699555..fc457c58a 100644 --- a/specs/phase0/p2p-interface.md +++ b/specs/phase0/p2p-interface.md @@ -542,12 +542,14 @@ 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 one block, if they have it and it exists in the range. Clients MAY limit the number of blocks in the response. +Clients MUST respond with at least the first block that exists in the range, if they have it. + +The following blocks MUST be ordered consecutively, with each `parent_root` matching the `hash_tree_root` of the previous block. + +Clients MAY limit the number of blocks in the response. The response MUST contain no more than `count` blocks. -Clients MUST order blocks by increasing slot number. - Clients MUST respond with blocks from their view of the current fork choice -- that is, blocks from the single chain defined by the current head. Of note, blocks from slots before the finalization MUST lead to the finalized block reported in the `Status` handshake. Clients MUST respond with blocks that are consistent from a single chain within the context of the request. After the initial block, clients MAY stop in the process of responding if their fork choice changes the view of the chain in the context of the request. From a29cbebc0e314892865667b23db8ff75f76a3983 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Wed, 20 May 2020 09:51:47 +0200 Subject: [PATCH 2/4] cover `step` parameter in stricter range request --- specs/phase0/p2p-interface.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/phase0/p2p-interface.md b/specs/phase0/p2p-interface.md index fc457c58a..787c94c34 100644 --- a/specs/phase0/p2p-interface.md +++ b/specs/phase0/p2p-interface.md @@ -544,7 +544,7 @@ Clients MUST keep a record of signed blocks seen since the since the start of th Clients MUST respond with at least the first block that exists in the range, if they have it. -The following blocks MUST be ordered consecutively, with each `parent_root` matching the `hash_tree_root` of the previous block. +The following blocks, where they exist, MUST be send in consecutive order without gaps, as selected by `step`. In particular, when `step == 1`, each `parent_root` MUST match the `hash_tree_root` of the preceding block. Clients MAY limit the number of blocks in the response. From 59a43142c2f9181b05006638019a24dfb5550a35 Mon Sep 17 00:00:00 2001 From: protolambda Date: Wed, 20 May 2020 20:39:52 +0200 Subject: [PATCH 3/4] Rebased on latest BlocksByRange spec, fix conflicts, clarify single chain, even with higher step --- specs/phase0/p2p-interface.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/specs/phase0/p2p-interface.md b/specs/phase0/p2p-interface.md index 787c94c34..e536c239a 100644 --- a/specs/phase0/p2p-interface.md +++ b/specs/phase0/p2p-interface.md @@ -544,7 +544,7 @@ Clients MUST keep a record of signed blocks seen since the since the start of th Clients MUST respond with at least the first block that exists in the range, if they have it. -The following blocks, where they exist, MUST be send in consecutive order without gaps, as selected by `step`. In particular, when `step == 1`, each `parent_root` MUST match the `hash_tree_root` of the preceding block. +The following blocks, where they exist, MUST be send in consecutive order. Clients MAY limit the number of blocks in the response. @@ -552,7 +552,10 @@ The response MUST contain no more than `count` blocks. Clients MUST respond with blocks from their view of the current fork choice -- that is, blocks from the single chain defined by the current head. Of note, blocks from slots before the finalization MUST lead to the finalized block reported in the `Status` handshake. -Clients MUST respond with blocks that are consistent from a single chain within the context of the request. After the initial block, clients MAY stop in the process of responding if their fork choice changes the view of the chain in the context of the request. +Clients MUST respond with blocks that are consistent from a single chain within the context of the request. +This applies to any `step` value. In particular when `step == 1`, each `parent_root` MUST match the `hash_tree_root` of the preceding block. + +After the initial block, clients MAY stop in the process of responding if their fork choice changes the view of the chain in the context of the request. #### BeaconBlocksByRoot From 522e34efcfca10f0c8f53e7757cdd172e119b5e7 Mon Sep 17 00:00:00 2001 From: protolambda Date: Wed, 20 May 2020 20:46:08 +0200 Subject: [PATCH 4/4] Fix markdown, use multiple lines for change-control, and add step >= 1 case --- specs/phase0/p2p-interface.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/specs/phase0/p2p-interface.md b/specs/phase0/p2p-interface.md index e536c239a..118be8839 100644 --- a/specs/phase0/p2p-interface.md +++ b/specs/phase0/p2p-interface.md @@ -532,7 +532,10 @@ Response Content: ) ``` -Requests beacon blocks in the slot range `[start_slot, start_slot + count * step)`, leading up to the current head block as selected by fork choice. `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 slots [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, …]. +Requests beacon blocks in the slot range `[start_slot, start_slot + count * step)`, leading up to the current head block as selected by fork choice. +`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 slots [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 request MUST NOT have a 0 slot increment, i.e. `step >= 1`. `BeaconBlocksByRange` is primarily used to sync historical blocks.