Skip to content
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

Reflect relays statuses in status endpoint #169

Closed
tbenr opened this issue Jun 24, 2022 · 9 comments · Fixed by #172
Closed

Reflect relays statuses in status endpoint #169

tbenr opened this issue Jun 24, 2022 · 9 comments · Fixed by #172
Labels
brainstorming Currently in discussion enhancement New feature or request

Comments

@tbenr
Copy link

tbenr commented Jun 24, 2022

I think mev-boost status endpoint should call status endpoints for each relays and return OK if at least one returned OK. Otherwise it should return KO (503?).

@metachris
Copy link
Collaborator

I think that could make sense. Would love to hear inputs from the other CL clients on this too!

@metachris metachris added enhancement New feature or request brainstorming Currently in discussion labels Jun 25, 2022
@metachris
Copy link
Collaborator

I see a problem with this approach, namely that mev-boost uses it's internal request timeout, waiting for the slowest relay. But it has no notion about the BN timeout for this call. Thus even if a relay already responded okay, boost would still wait for further relay responses.

Either we should allow the BN to pass the request timeout as argument, or we should revert this change. Any thoughts?

@metachris metachris reopened this Jul 3, 2022
@tbenr
Copy link
Author

tbenr commented Jul 3, 2022

I think mev-boost should respond immediately as soon as the first relay responds ok. (So it requires calling status in parallel against all relays).

Another approach could be to detach the calls. Mev-boost can do it's own polling to relays and maintain its own view of the situation, so the mev-boost can respond immediately with its current view.

@metachris
Copy link
Collaborator

Good ideas! Thanks 👍

@tbenr
Copy link
Author

tbenr commented Jul 3, 2022

The second approach adds a bit of latency but saves from the need of coordinating the timeouts, so slightly prefer that over the first option :)

@metachris
Copy link
Collaborator

Implemented the first option in v0.7.2, moving to option 2 in the future

@tbenr
Copy link
Author

tbenr commented Jul 7, 2022

What timeout are you applying on the relays call?

@metachris
Copy link
Collaborator

The default timeout used for all requests. But perhaps that's not good enough and we need to define them per request... See also #181

@metachris
Copy link
Collaborator

finished

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brainstorming Currently in discussion enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants