Repository navigation
Add gloas builder flow: produceBlockV4, builder preferences, block forwarding #630
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,26 @@ post: | |
| required: true | ||
| name: Eth-Consensus-Version | ||
| description: "The active consensus version to which the block being submitted belongs." | ||
| - name: Eth-Builder-Url | ||
| in: header | ||
| required: false | ||
| description: | | ||
|
nflaig marked this conversation as resolved.
|
||
| The `url` of the winning builder, as returned by `POST /eth/v4/validator/blocks/{slot}`. | ||
| When that endpoint returns this header the validator client MUST echo it here, so the beacon | ||
| node can forward the signed block to the same builder and the builder can release the | ||
| payload without waiting for gossip. The echo is what lets a beacon node that did not serve | ||
| the block-production request still forward it. Omitted for a self-built block or a block won | ||
| by a p2p bid. | ||
|
Comment on lines
+46
to
+55
Member
There was a problem hiding this comment. this can be implementation specific but we should probably not blindly call any url that is provided here, otherwise this creates a potential SSRF path this isn't that critical since generally the beacon api is not to be exposed publicly but there are providers like rescue node or in general rpc nodes that could be vulnerable to this if not handled correctly cc @jshufro might be relevant for you |
||
|
|
||
| The URL is untrusted input and a server-side request forgery (SSRF) risk: the beacon | ||
|
Member
There was a problem hiding this comment. as a side note, lodestar will only forward urls that have previously been authenticated by a proposer, I expect even in DVT setups that either |
||
| node issues only the builder-API `submitSignedBeaconBlock` request to it, over http(s), | ||
| and MUST NOT follow redirects. Deployments serving callers beyond their own validator | ||
| clients SHOULD also reject URLs that resolve to private address space, since a caller | ||
| can otherwise use the forwarded request to reach internal endpoints. | ||
| schema: | ||
| type: string | ||
| format: uri | ||
| example: "https://builder.example.com" | ||
| requestBody: | ||
| description: "The `SignedBeaconBlock` object composed of `BeaconBlock` object (produced by beacon node) and validator signature." | ||
| required: true | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| post: | ||
| operationId: "submitBuilderPreferences" | ||
| summary: Submit builder preferences | ||
| description: | | ||
| Submits per-builder preferences for one or more proposers. The request body is a flat list of | ||
| `BuilderPreferencesEntry` objects, each naming the proposer in `proposer_pubkey`; the beacon | ||
| node submits each to the builder-API `submitBuilderPreferences` endpoint at the entry's `url`. | ||
| Applicable from the Gloas fork onwards. | ||
|
|
||
| Notes: | ||
| - Each entry targets one builder `url`, to which the beacon node makes one submission. | ||
| - Entries are identified by `proposer_pubkey`, so several proposers MAY submit to the same `url`. | ||
| - The beacon node routes each entry by its `url` and submits to that builder's | ||
| `submitBuilderPreferences` for the entry's `proposer_pubkey`, forwarding the `auth` and | ||
| `max_execution_payment`. | ||
| - Validators MAY submit in the epoch prior to proposing (from `state.proposer_lookahead`), so | ||
| builders hold the preferences before the bid request arrives. | ||
| parameters: | ||
| - in: header | ||
| schema: | ||
| $ref: "../../beacon-node-oapi.yaml#/components/schemas/ConsensusVersion" | ||
| required: true | ||
| name: Eth-Consensus-Version | ||
| description: "The active consensus version to which the submitted builder preferences belong." | ||
| tags: | ||
| - Validator | ||
| requestBody: | ||
| required: true | ||
| description: "Array of `BuilderPreferencesEntry` objects to submit, one per builder per proposer." | ||
| content: | ||
|
nflaig marked this conversation as resolved.
|
||
| application/json: | ||
| schema: | ||
| type: array | ||
| maxItems: 4096 # MAX_BUILDER_ENTRIES * (MIN_SEED_LOOKAHEAD + 1) * SLOTS_PER_EPOCH | ||
| items: | ||
| $ref: "../../beacon-node-oapi.yaml#/components/schemas/Gloas.BuilderPreferencesEntry" | ||
| application/octet-stream: | ||
| schema: | ||
| description: "SSZ serialized `List[BuilderPreferencesEntry, MAX_BUILDER_ENTRIES * (MIN_SEED_LOOKAHEAD + 1) * SLOTS_PER_EPOCH]` bytes. Use Content-Type header to indicate that SSZ data is contained in the request body." | ||
| responses: | ||
| "200": | ||
| description: "Every entry was submitted and its builder accepted it" | ||
| "400": | ||
| description: | | ||
| One or more entries failed; the others were still submitted. Each failure is reported by the | ||
| entry's index via `IndexedErrorMessage`, proxying the builder's error for that entry. | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "../../beacon-node-oapi.yaml#/components/schemas/IndexedErrorMessage" | ||
| "415": | ||
| $ref: "../../beacon-node-oapi.yaml#/components/responses/UnsupportedMediaType" | ||
| "500": | ||
| $ref: "../../beacon-node-oapi.yaml#/components/responses/InternalError" | ||
| "503": | ||
| $ref: "../../beacon-node-oapi.yaml#/components/responses/CurrentlySyncing" | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 we really need this header? if I understand correctly the sole purpose of this is so that the beacon node knows where to submit it via
submitSignedBeaconBlockbut the beacon node should know this anyways, since it got the bid from that builder via apithe only case where the beacon node doesn't know this is if it's a different node (eg. in a multi-node setup) if the primary node that sourced the bid from the builder via api went offline during the proposal flow but this seems like an edge case and even then, publishing only via p2p is a fine fallback behavior that is sufficient anyways most of the time
that said, we can keep the header if people wanna have it, just questionable to me if it's useful enough
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.
this was solely for the multi-node support, not opinionated on it but it seemed like DVT teams wanted. Will leave for now and address most of the fat trimming comments
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 do not feel strongly about removing it, also I think when I commented there was in addition a
Eth-Proxy-Urlheader which is now gone, so this single header, if useful for DVT teams, might be worth keepingThere 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.
+1 questioning the usefulness of this header, both here and in the produceBlockV4 response. I'd personally prefer removing it unless there is a good reason to keep it.
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.
after implementing it, the header makes it easier on the submission side on the beacon node, avoids having to implement a lookup cache to "remember" to which builder to submit the bid to, it also acts as a signal whether the beacon node should forward the beacon block to a builder