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

Specify when clients can serve block and sidecars in byRoot RPC methods #3551

Merged
merged 18 commits into from
Dec 4, 2023
Merged

Specify when clients can serve block and sidecars in byRoot RPC methods #3551

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
e664d40
Specify RPC byRoot blocks-sidecars elegibility
tbenr Nov 16, 2023
c46c4ec
fix typo
tbenr Nov 16, 2023
6bbdeb8
Update specs/phase0/p2p-interface.md
tbenr Nov 17, 2023
ccf9e34
Update specs/deneb/p2p-interface.md
tbenr Nov 17, 2023
e2ca4b3
add failed on_block condition
tbenr Nov 22, 2023
d749a49
rephrase
tbenr Nov 22, 2023
4c5e5c8
Update specs/phase0/p2p-interface.md
tbenr Nov 29, 2023
ccca449
apply suggestion
tbenr Nov 29, 2023
88b1818
Update specs/deneb/p2p-interface.md
tbenr Nov 30, 2023
b525e9e
Update specs/deneb/p2p-interface.md
tbenr Nov 30, 2023
662c6eb
remove the last consider
tbenr Nov 30, 2023
bdac932
from on_block to state_transition
tbenr Nov 30, 2023
c661e20
simplify and add a new rule
tbenr Nov 30, 2023
dad09f6
Update specs/phase0/p2p-interface.md
tbenr Dec 1, 2023
eed7a66
Update specs/deneb/p2p-interface.md
tbenr Dec 1, 2023
cf39eed
Update specs/deneb/p2p-interface.md
tbenr Dec 1, 2023
ba09761
remove gossip failure rules
tbenr Dec 1, 2023
d343b6f
Apply suggestions from code review
djrtwo Dec 4, 2023
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
7 changes: 7 additions & 0 deletions specs/deneb/p2p-interface.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,9 @@ Per `context = compute_fork_digest(fork_version, genesis_validators_root)`:

No more than `MAX_REQUEST_BLOCKS_DENEB` may be requested at a time.

*[Modified in Deneb:EIP4844]*
Clients SHOULD include a block in the response as soon as it passes the gossip validation rules.
djrtwo marked this conversation as resolved.
Show resolved Hide resolved

##### BlobSidecarsByRoot v1

**Protocol ID:** `/eth2/beacon_chain/req/blob_sidecars_by_root/1/`
Expand Down Expand Up @@ -300,6 +303,10 @@ Clients MUST support requesting sidecars since `minimum_request_epoch`, where `m
Clients MUST respond with at least one sidecar, if they have it.
Clients MAY limit the number of blocks and sidecars in the response.

Clients SHOULD include a sidecar in the response as soon as it passes the gossip validation rules.
Clients SHOULD NOT respond with sidecars related to blocks that fail gossip validation rules.
Clients SHOULD NOT include sidecars related to blocks that fail `state_transition(block)`.
Copy link
Collaborator

@mkalinin mkalinin Dec 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
Clients SHOULD NOT include sidecars related to blocks that fail `state_transition(block)`.
Clients SHOULD NOT include sidecars related to blocks that fail beacon chain state transition.

Copy link
Contributor

Choose a reason for hiding this comment

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

why do we wanna have this removed? seems reasonable to me

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, that was due to my misread, updated the suggestion!

djrtwo marked this conversation as resolved.
Show resolved Hide resolved

##### BlobSidecarsByRange v1

**Protocol ID:** `/eth2/beacon_chain/req/blob_sidecars_by_range/1/`
Expand Down
3 changes: 3 additions & 0 deletions specs/phase0/p2p-interface.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,9 @@ Clients MUST support requesting blocks since the latest finalized epoch.
Clients MUST respond with at least one block, if they have it.
Clients MAY limit the number of blocks in the response.

Clients MAY include a block in the response as soon as it passes the gossip validation rules.
Clients SHOULD NOT respond with blocks that fail beacon chain state transition.
Copy link
Contributor

Choose a reason for hiding this comment

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

we can still have this post deneb as well as all clients are verifying blocks without waiting for all blobs to show up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Deneb updates the first rule but leaves this one unchanged

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good, it would have been good to enumerate the rules so as to be super clear, but nevertheless lgtm

djrtwo marked this conversation as resolved.
Show resolved Hide resolved

`/eth2/beacon_chain/req/beacon_blocks_by_root/1/` is deprecated. Clients MAY respond with an empty list during the deprecation transition period.

##### Ping
Expand Down