Return HTTP 404 for pruned blob requests #6331
Conversation
michaelsproul
added
ready-for-review
| .get_blobs(&root) | ||
| .map_err(|e| warp_utils::reject::beacon_chain_error(e.into()))? | ||
| .ok_or_else(|| { | ||
| warp_utils::reject::custom_not_found(format!( |
There was a problem hiding this comment.
This looks like a good change to me.
I'm not sure if we want to introduce new status codes, but there's also 410 Gone which would allow us to differentiate between block not found and expired blobs, without having to parse response message - but I'm also happy with 404.
In PeerDAS we'd also need a error response for nodes that are not capable of serving blobs data (custody columns < 64), and I think we might be returning 404 too? 😅
There was a problem hiding this comment.
410 sounds good, but we would need to add it to the spec: https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Beacon/getBlobSidecars.
Maybe we can advocate for that, while returning 404 in the meantime?
jimmygchen
added
ready-for-merge
|
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at e3d2d38 |
* Return HTTP 404 for pruned blob requests
Issue Addressed
Closes:
Proposed Changes
This resolves both cases of ambiguity where we currently return an empty list of blobs. Now, the empty blobs response is reserved exclusively for post-Deneb blocks that actually do not have any blobs.
This is motivated in part by the above issue, but also by the growing desire to allow importing/exporting blobs. In this case, some extra checks on the responses from the database are prudent in case a partial set of blobs is imported (something which may be quite desirable).
This is a breaking change to Lighthouse's behaviour, but IMO is still in keeping with the spec, which allows 404s for blocks that are not found.
Additional Info
I've added 2 new tests to confirm the new behaviour.
One oddity is that we will return
[]for blocks without blobs even if the blobs in that range have been pruned. E.g. if we make a request atslot == 12andoldest_blob_slot == 32and the block from slot 12 is post-Deneb with 0 blobs, then we will return[]. As long as we have the block stored we are capable of responding correctly, so we may as well. The alternative would be to artificially limit empty blob responses to just slots>= oldest_blob_slot.