Repository navigation
Add gloas builder flow: produceBlockV4, builder preferences, block forwarding #630
+368
−56
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,20 @@ 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 |
||
| 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 | ||
|
|
||
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