-
Notifications
You must be signed in to change notification settings - Fork 220
Add a POST produceBlockV4 #625
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| BuilderPreferences: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder should we put this file under the directory |
||
| type: object | ||
| description: "Per-builder preferences supplied by a validator when requesting block production." | ||
| required: [url, builder_boost_factor, max_trusted_bid, min_bid] | ||
| properties: | ||
| signed_request_auth: | ||
| allOf: | ||
| - $ref: "./gloas/request_auth.yaml#/Gloas/SignedRequestAuthV1" | ||
| - description: | | ||
| Optional `SignedRequestAuthV1` object, as defined in the | ||
| [Builder API](https://ethereum.github.io/builder-specs/) specification, that the beacon | ||
| node MUST forward to the builder when requesting a bid so that the builder can | ||
| authenticate the request. If omitted, requests to this builder are sent without | ||
| authentication. | ||
| url: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this PR deviates quite a lot from what you mentioned in #620 so first of all the |
||
| type: string | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assuming this |
||
| description: | | ||
| URL that the beacon node should use to contact the builder. This URL MAY be equal to the | ||
| one contained in the `data` field of `signed_request_auth.message`, but it MAY also differ | ||
| from it. An empty string denotes the default preferences entry: these preferences are | ||
| applied to any contacted builder for which no entry with a matching `url` was supplied. | ||
| example: "https://builder.example.com" | ||
| builder_boost_factor: | ||
| allOf: | ||
| - $ref: "./primitive.yaml#/Uint64" | ||
| - description: | | ||
| Percentage multiplier to apply to this builder's bid value when choosing between a | ||
| builder bid and payload from the paired execution node. This value is only relevant | ||
| if the beacon node has a viable `ExecutionPayloadBid` available from this builder and | ||
| receives a valid response from the paired execution node. When these preconditions | ||
| are met, the server MUST act as follows: | ||
|
|
||
| * if `exec_node_payload_value >= builder_boost_factor * (builder_payload_value // 100)` | ||
| for the highest builder bid known to the beacon node (each bid boosted by the | ||
| factor applicable to the builder it originates from), then return a block | ||
| committing to the local execution node payload. | ||
| * otherwise, return a block committing to the builder bid (without execution payload, | ||
| as it is not yet available). | ||
|
|
||
| Servers must support the following values of the boost factor which encode common | ||
| preferences: | ||
|
|
||
| * `builder_boost_factor=0`: prefer the local execution node payload unless an error | ||
| makes it unviable. | ||
| * `builder_boost_factor=100`: profit maximization mode; choose whichever payload pays | ||
| more. | ||
| * `builder_boost_factor=2**64 - 1`: prefer this builder's bid unless an error or | ||
| beacon node health check makes it unviable. | ||
|
|
||
| Servers should use saturating arithmetic or another technique to ensure that large | ||
| values of the `builder_boost_factor` do not trigger overflows or errors. | ||
| max_trusted_bid: | ||
| allOf: | ||
| - $ref: "./primitive.yaml#/Gwei" | ||
| - description: | | ||
| The maximum trusted bid, in Gwei, that will be accepted from this builder. Bids with a | ||
| value exceeding this amount MUST NOT be accepted. | ||
| min_bid: | ||
| allOf: | ||
| - $ref: "./primitive.yaml#/Gwei" | ||
| - description: | | ||
| The minimum bid, in Gwei, that the validator will accept from this builder. Bids with a | ||
| value lower than this amount MUST NOT be accepted. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| Gloas: | ||
| RequestAuthV1: | ||
| type: object | ||
| description: "The `RequestAuthV1` object from the [Builder API](https://ethereum.github.io/builder-specs/) Gloas spec." | ||
| required: [data, slot] | ||
| properties: | ||
| data: | ||
| type: string | ||
| format: hex | ||
| description: "Opaque authentication data, up to `MAX_DATA_SIZE` bytes. Typically set to the URL of the builder the request is intended for." | ||
| example: "0x68747470733a2f2f6275696c6465722e6578616d706c652e636f6d" | ||
| pattern: "^0x[a-fA-F0-9]{0,8192}$" | ||
| slot: | ||
| allOf: | ||
| - $ref: "../primitive.yaml#/Uint64" | ||
| - description: "The slot for which the bid is being requested." | ||
|
|
||
| SignedRequestAuthV1: | ||
| type: object | ||
| description: "The `SignedRequestAuthV1` object from the [Builder API](https://ethereum.github.io/builder-specs/) Gloas spec." | ||
| required: [message, signature] | ||
| properties: | ||
| message: | ||
| $ref: "#/Gloas/RequestAuthV1" | ||
| signature: | ||
| $ref: "../primitive.yaml#/Signature" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,3 +47,4 @@ produceBlockV | |
| stateful | ||
| CGC | ||
| RLP | ||
| auth | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
today we use this parameter as a global flag (applied to all builders) to let the operator decide their block production strategy
eg. you can set
builder_boost_factor=0which basically says "always pick local unless the EL fails to produce the payload", orbuilder_boost_factor=90which says "pick local payload if builder payload(s) are only marginally more valuable"having
builder_boost_factorand alsomin_bidexpressed per builder works with directly connected builders via api, but how does that work with p2p builders?I think it would be good to have a way to globally apply builder boost factor and min bid
in #620 there is the following statement
this seems to suggest a way to do this? I don't see it expressed in this PR though
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.
Definitely an oversight on my side since I based 630 on 625.
To match the current conventions in beacon 630 / builder 165 / keymanager 88, p2p bids would be configurable per-key, per-builder like: "validator X accepts p2p bids from builder Y only above this min_bid, weighted by this builder_boost_factor." I'm personally not sure that's worth it. p2p bids are entirely trustless payments so a Gwei from one p2p builder is the same as a Gwei from another, and there's no trust dimension left to express per builder. The thing you might genuinely want to differentiate on is censorship but you can't read that off a bid. If the node operator's goal is to "blacklist" specific p2p builders, the builders could always get around by rotating keys. Also most builders worth configuring individually would likely also be reachable over the builder API, where the full knob set already applies.
I'm more partial to per-key p2p preferences. The pragmatic version is to bring back something like the
builder_boost_factorquery param on produceBlockV4 and treat these as per-key p2p knobs. Then either VCs hold them in their own config as a global, or we extend setBuilders here to carry them per key.Curious what others think.
Uh oh!
There was an error while loading. Please reload this page.
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.
yes I don't think it's worth it but neither is that worth it on the builder-api, this is not used today and will not be useful after gloas, builder boost factor and also min-bid is very hard to configure even just for local vs. builder payload, having this per builder doesn't seem very useful, but I rather implement that at this point and trying to debate it. But I don't see why we don't wanna support the full spectrum, all that seems to be required is add a
builder_pubkeyfield, and makeurloptional and if configured the beacon-node reaches out to the builder via api too + enforcing same policy to the bids of that builder on p2pThere 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.
Following this thread, I updated #630 and #88 to offer the same config knobs for p2p bids. tl;dr is including
builder_pubkeyand excludingurlmeans yourBuilderEntrysuppliesmin_bidandbuilder_boost_factorconstraints on the p2p bids coming frombuilder_pubkeyso the BN can apply them when determining the winning bid. Keymanager then needed to change to allow defaults to be configured.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.
was thinking about this more and getting more convinced about your point of not needing per-
BuilderEntryfor p2p builders, maybe that also keeps the configuration simpler for users if we only allow globalbuilder_boost_factorandmin_bidto affect p2p bids while for directly connected builders we keep the per-builder configs. So since you made changes to support global parameters now, we might be fine with those only and can avoid overloading the usage ofbuilder_pubkey.one of the reasons is also the following p2p rule
since
builder_boost_factorandmin_bidare local only, it would be kinda random based on how bids are propagated, so even if you boost a certain builder's bid by 100x, it might not even reach your node due to the rule above, so it might be better from a user perspective to see p2p bids as essentially a single (the highest value) bid as coming from a single builder (not sure that framing makes sense).The use cases outlined by potuz in #620 sound pretty complete to me already, so if we can cover those which I think we can now since we added the global parameters, then we are kinda getting close to the final design we want.
@JasonVranek maybe you can double check this
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.
If I remove the per-builder p2p configs in favor for just the global it would definitely simplify. Overloading
builder_pubkeyis confusing / error prone. Regarding this list, almost everything would be covered.The one exception is by dropping per-builder p2p configs, you can't blacklist a specific p2p builder by pubkey via config. But like you said the p2p rule might make this moot anyways.
I'll make the simplification across the PRs today (I think the keymanager interface can stay as-is since I think it's important for the API to be able to configure the global p2p defaults).
Uh oh!
There was an error while loading. Please reload this page.
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.
@potuz is this something we need/want? I feel like this isn't that relevant
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.
I don't see why we should have a separate object for p2p builders and direct-connection builders. In fact I kinda don't like enshrining this separation in the beacon API spec (I guess it's fine in the builder API one). There are reasons to want to have a per-builder config on the beacon API even if they are being applied to p2p builders. The reason being that specialized types of builders become viable in Gloas. Imagine an application that settles on-chain, doesn't care about latency and has purely private order flow (a chess-like game). This application may want to eventually submit full blocks on the P2P stack with a decent bid. At times of low congestion they may actually win since their POF would be superior to the regular builders. I hope many different dapps that don't have strong pressure for short inclusion may want to move to such a system and can advertise a p2p builder.
In such a case some validators may want to boost some of these apps. This is just one example of that type of builders. Another one would be a client team having an altruistic builder, purely trusted not to front-run and sending cheap bids and returning tips to participants. Some people may want to boost these types of known altruistic builders setting the "floor" on the auction.
At any rate, I dot not see any benefit in not allowing this on the API itself. A client that wants to use a global default just needs to fill all values with the same and then add a canary "default entry" that has the same values.
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.
@potuz yeah I am going back and forth on this whether it's useful, do you have good design in mind how we can express that over the api? my idea was just to overload the use of
builder_pubkeyis pubkey the right one to use, or should it be by index for p2p builders? This PR currently doesn't support per-p2p-builder preferences, the one that @JasonVranek didn't support it either, then I brought it up there and current state should support it. I am just not sure right now what is the clean design for users to configure on the keymanager and also to support it over the wireThere 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.
#630 does currently support per-builder p2p bidding configs. Might be easiest to understand the complexity/footguns via this implementer guide.
In other words, as-is, 630 checks off anything in the list at the cost of some extra complexity. I'm happy to edit it if the complexity isn't justified.
Personally I feel like today's profile of builders / node operators / apps likely won't use these knobs, but maybe something like what @potuz described could be desirable in the future. So I'd lean towards keeping our options open, especially now when it's arguably the least friction time to add this. That being said, I'm not opinionated on p2p configs and this seems more like a client dev workload question.