-
Notifications
You must be signed in to change notification settings - Fork 272
Add timing feature for bid optimization #839
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
base: develop
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR adds a timing games feature for bid optimization that allows sending multiple getHeader requests within a slot to maximize the chance of receiving higher bids. The feature is configurable via YAML configuration files and remains backwards compatible with existing relay configuration methods.
Key changes:
- Introduces
RelayConfigstruct to wrapRelayEntrywith timing games parameters - Implements timing games logic with configurable delays and request frequencies
- Adds YAML configuration support for relay timing settings
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/types/relay_entry.go | Adds RelayConfig struct with timing games parameters |
| server/service.go | Updates BoostService to use RelayConfig instead of RelayEntry |
| server/get_header.go | Implements timing games logic with multiple timed requests |
| cli/main.go | Adds relay config file loading and merging functionality |
| cli/flags.go | Adds --relay-config flag for YAML configuration |
| cli/config.go | Implements YAML config parsing and relay merging logic |
| examples/relay_config.yml | Provides example configuration file |
| go.mod | Promotes yaml.v3 from indirect to direct dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Doesn't this feature require CL client implementation changes to not timeout on the request on the CL side? |
I think the CL clients sends an optional |
|
CLs have a hardcoded timeout for the return of the header from the call to MeV-Boost, I do not see how this works without modifying the CL |
That is okay too since we also have |
|
What I mean is that the CL has a hardcoded timeout that is not configurable. |
Yeah i understand what you are saying "the CL clients have hardcoded timeouts (e.g: 1000ms) that isnt configurable". |
|
This can already be (and is) done at the relay level, I still don't see the importance of having this at the middleware level. Anyway hopefully Gloas minimizes this to at most 200-300ms |
|
|
||
| type RelayConfigYAML struct { | ||
| URL string `yaml:"url"` | ||
| ID string `yaml:"id"` |
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.
Do relays need an ID outside the URL?
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.
optionally if you want. URLs can be too long when being read in metrics so you can essentially give an alias
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.
in other places, just the hostname was a good enough ID and short one. seems better to use only the hostname for metrics too?
I'm pretty sure this is just on the mev-boost side. I don't think any CLs send this header to the sidecar, nor does the sidecar take the header from that request and re-use it upstream. |
|
Why does the ID field in the config exist? I think it may not be needed, and MEV-Boost should simply use the full hostname as short version, as it does in other places, not the full URL. |
|
@potuz we had multiple conversations with node operators, which all require such a feature to be present to not switch to other implementations that have this (such as commit boost). |
|
@metachris I guess it's fine, I don't care much cause hopefully this becomes irrelevant in the next fork, but what I am pointing out is that the relays can (and already do) this, and adding this to mev boost itself is kinda irrelevant in this sense, my reading of this is that mev boost will force a relay that does not do timing games to actually do it. I would be worried that adding a delay in the middleware and misscomunication with the relay adding an extra one may cause missed blocks because of breaking the deadline in the CL. I hope these changes get actually tested in deployed environment with real relays and not some mock flashbots builder run by pandaOps. |
| } | ||
|
|
||
| timeoutGetHeaderMs := config.TimeoutGetHeaderMs | ||
| if timeoutGetHeaderMs == 0 { |
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.
neeed to clearly specify this in a documentaton on what the default value is. you can do it in the config.yaml
|
|
||
| lateInSlotTimeMs := config.LateInSlotTimeMs | ||
| if lateInSlotTimeMs == 0 { | ||
| lateInSlotTimeMs = 1000 |
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.
same for this.
| @@ -0,0 +1,24 @@ | |||
| # Example configuration for mev-boost | |||
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.
would recommend to create a config.example.yaml
📝 Summary
Allows sending multiple getHeader requests within a slot and returns the freshest successful header. This helps improving bid while respecting deadlines. It maximizes chance of a higher bid since later requests can return improved headers. It achieves this via YAML config file which can be passed via
--relay-config, allowing delayed first requests viaTargetFirstRequestMsand multiple subsequent requests at intervals viaFrequencyGetHeaderMs. This feature is only activated ifEnableTimingGamesis set to true in the config file for a particular relay.This PR also allows setting relays via YAML file where timing games parameters can be set. This is also backwards compatible if someone doesnt want to set relays via YAML file and want to use the default method of setting relays via
--relayflagexample config
Closes #830
⛱ Motivation and Context
Timing games around bids are a double-edged sword. They can maximize yield for involved parties but also cause more latency (later block proposal within the slot -> more time to include TXs) for the network. Due to sophistication of builders and relays they have already become the norm in the PBS pipeline.
This PR simply provides feature parity with other sidecar implementations and it's still up to proposers whether to opt-in to these advanced features.
📚 References
✅ I have run these commands
make lintmake test-racego mod tidy