Throw 404 for producePayloadAttestationData when no block at slot#612
Throw 404 for producePayloadAttestationData when no block at slot#612ensi321 wants to merge 5 commits into
Conversation
Per the gloas honest validator spec, a PTC member must not submit a
payload attestation when no beacon block has been seen for the assigned
slot. Without a documented "no block" response, beacon nodes have no
standard way to signal this to validator clients and clients have no
standard way to detect it, leading to attestations that vote on stale
block roots which are then ignored by peers.
Document a 404 response on /eth/v1/validator/payload_attestation_data/{slot}
to mirror the existing convention used by /eth/v2/beacon/blocks/{block_id}
for empty slots. Empty slots are common, so 404 is the expected signal
for "skip submission", not an error condition.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nflaig
left a comment
nflaig
left a comment
There was a problem hiding this comment.
using 404 seems good to me, but I'd suggest we add a note for this in the description of the endpoint to clarify it a bit more, similar to how it's done for 503
| description: "SSZ serialized `PayloadAttestationData` bytes. Use Accept header to choose this response type" | ||
| "400": | ||
| $ref: "../../beacon-node-oapi.yaml#/components/responses/InvalidRequest" | ||
| "404": |
There was a problem hiding this comment.
a tidbit to consider: a server that does not implement the beacon api at all or implements a different version without this particular request will also return 404 - it's not a great error code to base protocol semantics on, throughout
There was a problem hiding this comment.
yes, that is true, but we have the same semantics for other endpoints that can be queried by slot (eg. getBlockV2 which returns 404 as well if no block is found for slot), we can either use a different HTTP error (I don't know which one), or we can return 204 no content which might be better but deviate from existing semantics if no block for slot exists. And handling it as an error has advantages, so I am not sure 204 is even a good option. maybe you have something else in mind @arnetheduck
There was a problem hiding this comment.
204 sounds pretty good to me - ie it's not an error that things are missing in this case, it's a normal recoverable scenario that just needs to be handled in a different way - the endpoint itself is working as expected.
getBlockV2
yeah, it's been bugging me for a while. I would go as far as adressing it in v3, when it comes about.
There was a problem hiding this comment.
thinking about this more, I do like 204 over 404 here, also noticed some annoyance on the devnet, we are logging http errors as warnings by default and this endpoint is called every slot there and a missed block isn't too uncommon, using 204 would resolve this
cc @ensi321 @james-prysm @rolfyone thoughts?
There was a problem hiding this comment.
I think if we want to do 204 here, we should do it for other endpoints across the board too.
There was a problem hiding this comment.
if its part of the route, then 404 is probably better than 204, esp on a get interface like this. Im not sure i'd be a fan of using 204 on missed paths as a general rule, even if we end up going 204 in this instance...
also i really dont want incompatible changes like 'changing across the board'.
404 is still 'normal and recoverable' and either way it needs handling by the caller, so i think whether its 204 or 404 is a tiny bit academic..
There was a problem hiding this comment.
I think 404 is still fine here ... we have other occassions where 404 is used for other situations too. I think it's appropriate here
There was a problem hiding this comment.
I do think we should go with 204 here, I think it's better from an error handling hygience, I am seeing these 404's a lot on the devnet due to missed blocks and it's already causing noise in the logs.
let's vote on this, if you like 204 ( 👍 ) and if you prefer 404 ( 👎 ) this message
There was a problem hiding this comment.
I am beginning to feel more strongly about using 204 here, on glamsterdam-devnet-6, there are a lot of missed blocks right now and lodestar is emitting this as warnings (our standard request handler), I don't wanna special case this api, and this is quite a frequent error/warning being emitted. Today, this was flagged incorrectly by AI as a potential CL issue to me but it's perfectly normal behavior, so let's use a status code that indicates that, 404 does not.
| $ref: "../../beacon-node-oapi.yaml#/components/schemas/ErrorMessage" | ||
| example: | ||
| code: 404 | ||
| message: "No block found at slot. Used to signal validator to not cast any payload attestation." |
There was a problem hiding this comment.
to move on with this, we could leave it ambiguous and not mentioned anything about "canonical" or casting votes for "any block" which with the current design doesn't work
I would leave further discussion up to #619 as it's a more controversial topic while we should be able to conclude the status code discussion here, or at least I hope we can conclude this soon
also cc @jtraglia, the CL spec doesn't reflect the current behavior of clients at all, not sure it's a big deal but something we need to specify more clearly at some point
5 participants
When there is no block at slot,
producePayloadAttestationDataneed to signal validator that there is no block so validator won't panic when nothing but error was returned.This PR proposes to throw 404 to indicate such case and everything is fine.