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.

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

EIP-4844: blobs / blocks by root edge case #3113

Open
dapplion opened this issue Nov 18, 2022 · 4 comments
Open

EIP-4844: blobs / blocks by root edge case #3113

dapplion opened this issue Nov 18, 2022 · 4 comments
Labels
Deneb was called: eip-4844

Comments

@dapplion
Copy link
Collaborator

dapplion commented Nov 18, 2022

When receiving a block with slot close and after EIP4844_FORK_EPOCH with an unknown parent you don't know if the parent will be pre or post EIP4844_FORK_EPOCH. So, should you:

  • Use beacon_block_and_blobs_sidecar_by_root:
    • Parent is pre EIP4844: The responder will either error or return empty, spec is not specified. If empty it's unclear if the responder does not have the block or if the block is pre EIP4844. However if it errors, how to handle a mix case where a request of multiple roots, some are pre EIP4844 and some post? If we use a per response_chunk error code for that case, at least Lodestar's logic aborts the entire response stream on the first status != success
    • Parent is post EIP4844: Ok
  • Use beacon_blocks_by_root:
    • Parent is pre EIP4844: Ok
    • Parent is post EIP4844: According to the spec After EIP4844_FORK_EPOCH, BeaconBlocksByRootV2 is replaced by BeaconBlockAndBlobsSidecarByRootV1. How should the client respond for this case, and in the case of mixed roots per and post EIP4844? Arguments from first case apply here too.

In Lodestar implementation we currently race both request with 1 root max per request. However would be good to have a well defined interaction between EIP4844_FORK_EPOCH and finalizing EIP4844_FORK_EPOCH.

@tbenr
Copy link
Contributor

tbenr commented Nov 19, 2022

Shouldn't you rename the issue "... by root edge case"?
My gut says we should allow the coupled request to retrieve pre 4844. Clients should respond with a zero sidecar. But it might create ssz containers issues (a pre 4844 block into a 4844 blockandsidecar container).

@dapplion dapplion changed the title EIP-4844: blobs / block by range edge case EIP-4844: blobs / blocks by root edge case Nov 21, 2022
@tbenr
Copy link
Contributor

tbenr commented Nov 22, 2022

Another option is let beacon_block_and_blobs_sidecar_by_root return a SignedBeaconBlock instead of BlobsAndBlobsSidecar when block is pre 4844.

@divagant-martian
Copy link

divagant-martian commented Nov 29, 2022

Another option is let beacon_block_and_blobs_sidecar_by_root return a SignedBeaconBlock instead of BlobsAndBlobsSidecar when block is pre 4844.

This seems the best option to me. It makes it clear what the peer responding is claiming about the block: either a pre 4844, or post along with it's blob, and allows us to do parent requests in a single rpc call

@realbigsean
Copy link
Contributor

Just to put it in clear terms: beacon_block_and_blobs_sidecar_by_root could always return Union[SignedBeaconBlockAndBlobsSidecar, SignedBeaconBlock]. This seems like a pretty reasonable option to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deneb was called: eip-4844
Projects
None yet
Development

No branches or pull requests

7 participants
@realbigsean @hwwhww @tbenr @divagant-martian @dapplion and others