feat: fallback to v1 getPayload api if v2 getPayload api fails #840
Conversation
server/get_payload.go
Outdated
| // retry with v1 api | ||
| url = relay.GetURI(params.PathGetPayload) | ||
| versionToUse = GetPayloadV1 | ||
| return nil, fmt.Errorf("relay may not support V2 API, Retrying with v1 API") |
There was a problem hiding this comment.
We leverage the existing retry loop to call the v1 API in the upcoming retry.
| result.success = true | ||
|
|
||
| if version == GetPayloadV1 { | ||
| if versionToUse == GetPayloadV1 { |
There was a problem hiding this comment.
We should definitely be validating the returned block by the relay even if we fall back to the v1 API.
There was a problem hiding this comment.
Pull Request Overview
This PR implements a fallback mechanism to use the v1 getPayload API when the v2 getPayload API fails with a 404 error, preventing missed slots when relays don't support the newer v2 API.
- Modified return types of getPayload functions to support fallback logic
- Added error handling to detect v2 API unavailability and retry with v1 API
- Updated function signatures and callers to work with new payloadResult structure
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| server/service.go | Updated function calls to handle new payloadResult return type and added error constant |
| server/get_payload.go | Implemented v2-to-v1 fallback logic and modified function signatures to return payloadResult |
Comments suppressed due to low confidence (2)
server/get_payload.go:1
- Line 244 sets the URL to the V2 endpoint, but it should be set to the V1 endpoint. This should be
url = relay.GetURI(params.PathGetPayload)to match the fallback logic.
package server
server/get_payload.go:1
- The fallback logic only modifies local variables and returns an error, but the retry mechanism will use the same URL and version on the next attempt. The fallback should be implemented at a higher level in the retry loop or the variables should be properly updated for the retry to work correctly.
package server
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
bharath-123
force-pushed
the
add-fallback-for-v2-api
branch
from
d27e060 to
8fcadcd
Compare
server/service.go
Outdated
| errInvalidPubkey = errors.New("invalid pubkey") | ||
| errNoSuccessfulRelayResponse = errors.New("no successful relay response") | ||
| errServerAlreadyRunning = errors.New("server already running") | ||
| errRetryWithV1API = errors.New("relay may not support V2 API, retrying with v1 API") |
server/get_payload.go
Outdated
|
|
||
| for _, relay := range m.relays { | ||
| go func(relay types.RelayEntry) { | ||
| var versionToUse GetPayloadVersion |
There was a problem hiding this comment.
we can reuse version and update it where needed, but i think versionToUse sounds more mutable since now it can also be updated. Why dont we rename version to versionToUse in the innerGetPayload args and then use it everywhere
There was a problem hiding this comment.
sounds good let me do that
server/get_payload.go
Outdated
| RecordRelayStatusCode(strconv.Itoa(statusCode), endpoint, relay.String()) | ||
| // Check that the response was successful | ||
|
|
||
| if resp.StatusCode == http.StatusNotFound && url == relay.GetURI(params.PathGetPayloadV2) { |
There was a problem hiding this comment.
maybe more robust to do this on any 4xx status code, instead of only not found
There was a problem hiding this comment.
that is not a bad idea. There is no harm falling back to the v1 api eitherways.
There was a problem hiding this comment.
we could just fallback if the status code >= 400 in general i believe
| // retry with v1 api | ||
| url = relay.GetURI(params.PathGetPayload) | ||
| versionToUse = GetPayloadV1 | ||
| return nil, errRetryWithV1API |
There was a problem hiding this comment.
where is errRetryWithV1API used? where is the actual retry with v1 triggered and executed?
There was a problem hiding this comment.
errRetryWithV1API is not used anywhere. so this logic of calling the API is encapsulated in the retry method which retries if the function passed to it returns an error. errRetryWithV1API is an error that is returned when the V2 API is not found or so.
3 participants
📝 Summary
We want to fallback calling the v1 getPayload API if the v2 getPayload API fails.
ethereum/builder-specs#123 introduced a new v2 getPayload API for the relays. This essentially doesn't return the entire signed beacon block response which can really large with more blobs.
If in case, relays do not implement this API. The getPayload v2 API will return a 404 not found and the slot will be missed. It would be better to fallback to the v1 API in such a case to avoid a missed slot.
✅ I have run these commands
make lintmake test-racego mod tidy