Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BeaconBlocksByRange v3 #3845

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
+57 −0
Open

Add BeaconBlocksByRange v3 #3845

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b8556f
Add BeaconBlocksByRange v3
dapplion Jul 15, 2024
06fb7a0
Fix block_root definition
dapplion Jul 23, 2024
a3ef2d7
Add BlobSidecarsByRange v2
dapplion Jul 25, 2024
f782b27
Update specs/electra/p2p-interface.md
dapplion Jul 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions specs/electra/p2p-interface.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ The specification of these changes continues in the same format as the network s
- [Global topics](#global-topics)
- [`beacon_aggregate_and_proof`](#beacon_aggregate_and_proof)
- [`beacon_attestation_{subnet_id}`](#beacon_attestation_subnet_id)
- [The Req/Resp domain](#the-reqresp-domain)
- [Messages](#messages)
- [BeaconBlocksByRange v3](#beaconblocksbyrange-v3)
- [BlobSidecarsByRange v2](#blobsidecarsbyrange-v2)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->
<!-- /TOC -->
Expand Down Expand Up @@ -57,3 +61,56 @@ The following convenience variables are re-defined
The following validations are added:
* [REJECT] `len(committee_indices) == 1`, where `committee_indices = get_committee_indices(attestation)`.
* [REJECT] `attestation.data.index == 0`

## The Req/Resp domain

### Messages

#### BeaconBlocksByRange v3

**Protocol ID:** `/eth2/beacon_chain/req/beacon_blocks_by_range/3/`

Request Content:
```
(
block_root: Root
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intended to be the last block in the segment (start_slot + count)? Or just a block anywhere in the segment

Copy link
Contributor

@jimmygchen jimmygchen Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this is intended to be the target block to sync to?
which could be either greater, equal or less than (start_slot + count) but greater than or equal to the start_slot

e.g. target block_root 0xabcde (at slot 50)

  • start_slot: 0, count: 32: valid and returns block for slots 0-31
  • start_slot: 32, count: 32: valid and returns block for slots 32-50
  • start_slot: 64, count: 32: invalid request

In the 2nd case above, the block 0xabcde is the last block in the segment.

If the above is correct, perhaps this can be renamed to target_block_root?

Copy link
Collaborator Author

@dapplion dapplion Jul 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the definition for block_root to refer to a block that's at the tip or descendant of the request slot range 06fb7a0 for the block range to be uniquely identifiable block_root must not be less than start_slot + count, that was an oversight on my end.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jimmygchen I like the suggestion to use a more descriptive name than block_root. However target is overloaded with the CasperFFG checkpoint meaning.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking that maybe we should just start from the end slot and go back to the ancestors rather than starting from the start slot and go down to the descendants. How about changing the entire request content to the following?

(
  block_root: Root
  end_slot: Slot
  count: uint64
)

The request will be valid only if block_root is in the slot end_slot.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The request will be valid only if block_root is in the slot end_slot.

I feel it's an unnecessary departure to how by_range requests are done today

start_slot: Slot
count: uint64
)
```

Response Content:
```
(
List[SignedBeaconBlock, MAX_REQUEST_BLOCKS]
)
```

Extends behaviour of BeaconBlocksByRange v2 as defined in [the altair p2p spec](../altair/p2p-interface.md).

Requests beacon blocks in the slot range `[start_slot, start_slot + count)`, leading up to `block_root`. If the block with `block_root` is unknown the responder MUST respond with `3: ResourceUnavailable`. If the slot of the block with `block_root` is less than `start_slot + count` the responder MUST respond with `1: InvalidRequest`.

#### BlobSidecarsByRange v2

**Protocol ID:** `/eth2/beacon_chain/req/blob_sidecars_by_range/2/`

Request Content:
```
(
block_root: Root
start_slot: Slot
count: uint64
)
```

Response Content:
```
(
List[BlobSidecar, MAX_REQUEST_BLOB_SIDECARS]
)
```

Extends behaviour of BlobSidecarByRange v2 as defined in [the deneb p2p spec](../deneb/p2p-interface.md).

Requests blob sidecars in the slot range `[start_slot, start_slot + count)`, leading up to `block_root`. If the block with `block_root` is unknown the responder MUST respond with `3: ResourceUnavailable`. If the slot of the block with `block_root` is less than `start_slot + count` the responder MUST respond with `1: InvalidRequest`.