-
Notifications
You must be signed in to change notification settings - Fork 275
allow background relay requests in getPayloadV2 #850
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
allow background relay requests in getPayloadV2 #850
Conversation
server/get_payload.go
Outdated
| // Create a context with a timeout as configured in the http client | ||
| requestCtx, requestCtxCancel := context.WithTimeout(context.Background(), m.httpClientGetPayload.Timeout) | ||
| defer requestCtxCancel() | ||
| originalVersionToUse := versionToUse |
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.
don't use an originalVersionToUse here. Each relay request is a separate go-routine and we need to handle each separately.
I think we just don't call requestCtxCancel at this point and let the other requests run.
08b7326 to
6011a60
Compare
server/get_payload.go
Outdated
| // falling back to the V1 API, falling back to the V1 API in the case of any error | ||
| // can be beneficial to the proposer to avoid a missed slot. | ||
| if resp.StatusCode >= http.StatusBadRequest && url == relay.GetURI(params.PathGetPayloadV2) { | ||
| log.Warn("relay may not support V2 API, Retrying with V1 API") |
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.
The capitalized "Retrying" word should be lowercase.
| log.Warn("relay may not support V2 API, Retrying with V1 API") | |
| log.Warn("relay may not support V2 endpoint, retrying with V1 endpoint") |
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.
Or this if you prefer:
| log.Warn("relay may not support V2 API, Retrying with V1 API") | |
| log.Warn("relay may not support getPayloadV2, retrying with getPayloadV1") |
| // can be beneficial to the proposer to avoid a missed slot. | ||
| if resp.StatusCode >= http.StatusBadRequest && url == relay.GetURI(params.PathGetPayloadV2) { | ||
| log.Warn("relay may not support V2 API, Retrying with V1 API") | ||
| // retry with v1 api |
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.
Just a nit, you say "V1 API" (uppercase) in the comments above. A bit consistent.
jtraglia
left a comment
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.
LGTM, but maybe fix the little warning message nit.
📝 Summary
This PR allows sending
getPayloadV2endpoint to all relays by allowing relay requests to continue in the background after receiving the first successful response.This PR also involves improving the logging around MEV-Boost:
✅ I have run these commands
make lintmake test-racego mod tidy