-
Notifications
You must be signed in to change notification settings - Fork 740
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
Check data availability boundary in rpc request #3891
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Thanks for getting some clarity around this behavior
This is an interesting question.. I think this would need to be spec'd out as a specific response code if we wanted to convey "we vouch that this was available at some point" But this is sort of a "trust me bro" type of statement from a peer so I'm not sure it's actually worth much. I think |
862deff
to
3543b9a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! LGTM overall. Just a few nits
Co-authored-by: realbigsean <seananderson33@GMAIL.com>
Co-authored-by: realbigsean <seananderson33@GMAIL.com>
Ok so,
|
So it looks like the latest commits combine some parts of blob
In
This would map to our Confusingly,
I tried to implement the ResourceUnavailable handling more precisely here: #3912 Building on top of this commit e145504 |
closed in favor of #3912 |
Issue Addressed
Addressing this closed PR ethereum/consensus-specs#3211.
Proposed Changes
Check data availability boundary differently for ByRange and ByRoot request and serve client error message with regard to data availability boundary.
Additional Info
For blobs by root, should we do smthg here with the
BeaconBlobOrphan
column eventually, like responding "data was available"? @michaelsproulAs in the issues you mention here in the last two paragraphs #3852 (review)