Repository navigation
Staked Builder API for Glamsterdam #138
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 49 commits
74b53c1
0c55646
278df30
1577ba9
a3db7f7
df52176
6e1a777
80974bc
a242d3b
4ab5a0f
9901fdc
6959fda
e752694
d5773ac
68c1f21
373493a
b0d9542
77be521
56a5ec9
dd644e5
80dbeb0
3a9640a
8962cc3
2cdc29c
5ab4d6d
f4a9e04
35fb5a7
e33dfed
8aaa3b5
3625f54
66c15dc
31dc1d4
0644e07
b02106f
dbce4d6
b0e3d0c
e7d4321
37c8e87
3b5735b
19c272f
18ca2ed
c1f2af3
48c6ad8
3d53d8d
d125dea
5a618ae
6429ef9
9d4a18d
fbc4747
7bd4ea1
75ea726
8a51c81
9ec931e
1e26b71
8bf1bee
1ac301a
b573714
cd89786
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,50 @@ | ||
| post: | ||
| operationId: "submitSignedBeaconBlock" | ||
| summary: Submit a signed beacon block with the execution payload bid. | ||
| description: | | ||
| Submits a `SignedBeaconBlock` to the builder, binding the proposer to the block. | ||
|
|
||
| A success response (200) indicates that the signed beacon block was | ||
| valid. If the signed beacon block was invalid, then the builder | ||
| must return an error response (400) with a description of the validation | ||
| failure. | ||
| tags: | ||
| - Builder | ||
| parameters: | ||
| - in: header | ||
| schema: | ||
| $ref: "../../builder-oapi.yaml#/components/schemas/ConsensusVersion" | ||
| required: false | ||
|
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. we should make this required, or how exactly do you determine the consensus type otherwise?
Contributor
Author
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. yes agreed, i was not sure if we should make it required because we don't do it in the previous apis. But I see no harm in making it required. A builder can use the header when they need to. |
||
| name: Eth-Consensus-Version | ||
| description: "The active consensus version to which the block being submitted belongs. Required if request is SSZ encoded." | ||
| requestBody: | ||
| description: A `SignedBeaconBlock`. | ||
| required: true | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| required: [data] | ||
| properties: | ||
| data: | ||
|
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. we usually don't use a
Contributor
Author
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 ll remove that! |
||
| $ref: "../../beacon-apis/types/gloas/block.yaml#/Gloas/SignedBeaconBlock" | ||
| description: "The signed beacon block." | ||
| application/octet-stream: | ||
| schema: | ||
| description: "SSZ serialized `SignedBeaconBlock` bytes. Use content type header to indicate that SSZ data is contained in the request body." | ||
| responses: | ||
| "202": | ||
| description: Success response. | ||
| "400": | ||
| description: Error response. | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "../../builder-oapi.yaml#/components/schemas/ErrorMessage" | ||
| example: | ||
| code: 400 | ||
| message: "Invalid signed beacon block: missing signature" | ||
| "415": | ||
| $ref: "../../builder-oapi.yaml#/components/responses/UnsupportedMediaType" | ||
| "500": | ||
| $ref: "../../builder-oapi.yaml#/components/responses/InternalError" | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,188 @@ | ||||||||||||||||||||||||||
| post: | ||||||||||||||||||||||||||
|
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. what is the reason of using
Contributor
Author
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 used a post cause we are sending the request auth in the body. I put the params in the path segments cause it was what we are doing previously. Let me think more on whether we should put the params in path segments or request body. This point was raised by a builder too 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. can we just pass the signature in the request header? because builder must know its own pubkey and
Contributor
Author
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. If in the future, we update the request auth the builder would probably need to try multiple structs to verify the signature. Just sending the full body with the signature seems much easier to me to avoid this issue? |
||||||||||||||||||||||||||
| operationId: "getExecutionPayloadBid" | ||||||||||||||||||||||||||
| summary: Get an execution payload bid. | ||||||||||||||||||||||||||
| description: | | ||||||||||||||||||||||||||
| Requests a builder node to produce a valid execution payload bid, which | ||||||||||||||||||||||||||
| can be integrated into a beacon block and signed. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| The proposer sends a POST request to the builder with the following information: | ||||||||||||||||||||||||||
| - The slot for which the block should be proposed. | ||||||||||||||||||||||||||
| - The hash of the execution layer block the proposer will build on. | ||||||||||||||||||||||||||
| - The root of the beacon block the proposer will build on. | ||||||||||||||||||||||||||
| - The index of the proposer. | ||||||||||||||||||||||||||
| - The `X-Eth-Max-Trusted-Bid` header carrying the proposer's | ||||||||||||||||||||||||||
|
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'm not a big fan of the
Contributor
Author
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. Yes I agree with not having the header. If the validator has not submitted the preference, I think we should just assume max trusted bid is 0 and all payments are trustless? But I have a PR to remove the header here: https://github.com/ethereum/builder-specs/pull/161/changes#diff-a186f4bbbe96e38c7699013ced6e58643ff9a03dddbc4fd7584c817920f5ef41L99 |
||||||||||||||||||||||||||
| `max_trusted_bid` (decimal `uint64`, in Gwei) for this request. | ||||||||||||||||||||||||||
| - Optionally, a `SignedRequestAuth` in the request body that | ||||||||||||||||||||||||||
| authenticates the request. The body MAY be encoded as JSON | ||||||||||||||||||||||||||
| (`Content-Type: application/json`) or SSZ | ||||||||||||||||||||||||||
| (`Content-Type: application/octet-stream`). | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| The `X-Eth-Max-Trusted-Bid` header is required. If it is missing or | ||||||||||||||||||||||||||
|
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. Why is this a header rather than just another field in the body?
Contributor
Author
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 am not too opinianated on this. We can revisit these aspects. |
||||||||||||||||||||||||||
| malformed, the builder MUST NOT serve a bid for the proposer and MUST | ||||||||||||||||||||||||||
| return a 400 response. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| The `SignedRequestAuth` body is optional. If it is present but malformed | ||||||||||||||||||||||||||
| or fails signature verification, the builder MAY return a 400 response. | ||||||||||||||||||||||||||
| If it is absent, the builder MAY still serve a bid, but builders MAY | ||||||||||||||||||||||||||
| use the presence and validity of the `SignedRequestAuth` to apply | ||||||||||||||||||||||||||
| per-validator policy (e.g. rate-limiting, prioritization, or refusing | ||||||||||||||||||||||||||
| unauthenticated requests). | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| The builder responds with a 200 response containing an execution payload bid if it can provide one. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| If the builder is unable to produce a valid execution payload bid, then | ||||||||||||||||||||||||||
| the builder MUST return a 204 response. If the request is invalid, then the | ||||||||||||||||||||||||||
| builder MUST return an error response (400) with a description of the | ||||||||||||||||||||||||||
| validation failure. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| This API is applicable from Gloas fork onwards. | ||||||||||||||||||||||||||
| tags: | ||||||||||||||||||||||||||
| - Builder | ||||||||||||||||||||||||||
| parameters: | ||||||||||||||||||||||||||
|
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. In the existing api the query params make sense as it's a GET, but this new one is a POST with a mix of query params, a body, and headers that contain application data rather than just meta data. Shouldn't it all just be in the body if it's a POST?
Contributor
Author
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. Yeah I am unopinianated on this. We can discuss this again. |
||||||||||||||||||||||||||
| - name: slot | ||||||||||||||||||||||||||
| in: path | ||||||||||||||||||||||||||
| required: true | ||||||||||||||||||||||||||
| description: The slot for which the block should be proposed. | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| $ref: "../../builder-oapi.yaml#/components/schemas/Uint64" | ||||||||||||||||||||||||||
| - name: parent_hash | ||||||||||||||||||||||||||
| in: path | ||||||||||||||||||||||||||
| required: true | ||||||||||||||||||||||||||
| description: Hash of execution layer block the proposer will build on. | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| $ref: "../../builder-oapi.yaml#/components/schemas/Root" | ||||||||||||||||||||||||||
| - name: parent_root | ||||||||||||||||||||||||||
| in: path | ||||||||||||||||||||||||||
| required: true | ||||||||||||||||||||||||||
| description: Root of the beacon block the proposer will build on. | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| $ref: "../../builder-oapi.yaml#/components/schemas/Root" | ||||||||||||||||||||||||||
| - name: proposer_index | ||||||||||||||||||||||||||
|
Contributor
Author
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. should we stick with the proposer pubkey or use proposer index?
Contributor
Author
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. we could just stick with proposer_index. its much easier to read. 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. There are ideas around reusing the proposer index, whereas the proposer pubkey is more in the custody of the proposer (rather than easily changeable with a protocol change). 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 believe we use pubkey in the existing 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 call is in the hot path I don't see any reason to send extra bytes. The slot is signed so index reuse is irrelevant. But this is a soft take. I think getting the index is always cheaper than the pubkey at the point of the call, but I empathize with @eserilev's comment that perhaps the caller code needs to be changed less if we stick to pubkey 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. In favour of simplifying client implementation by sticking to pubkeys
Contributor
Author
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. |
||||||||||||||||||||||||||
| in: path | ||||||||||||||||||||||||||
| required: true | ||||||||||||||||||||||||||
| description: Index of the proposer. | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| $ref: "../../builder-oapi.yaml#/components/schemas/Uint64" | ||||||||||||||||||||||||||
| - name: Date-Milliseconds | ||||||||||||||||||||||||||
| in: header | ||||||||||||||||||||||||||
| required: false | ||||||||||||||||||||||||||
| description: | | ||||||||||||||||||||||||||
| Optional header containing a Unix timestamp in milliseconds representing | ||||||||||||||||||||||||||
| the point-in-time the request was sent. This header can be used to measure | ||||||||||||||||||||||||||
| latency. | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| type: integer | ||||||||||||||||||||||||||
| format: int64 | ||||||||||||||||||||||||||
| example: 1710338135000 | ||||||||||||||||||||||||||
| - name: X-Timeout-Ms | ||||||||||||||||||||||||||
|
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. It would be nice if clients agreed NOT to make this configurable.
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. What's the advantage in not making it configurable? Would it be incentive compatible?
Contributor
Author
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. We ideally don't want people to be able to abuse the
Contributor
Author
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. refer to https://ethpandaops.io/posts/fusaka-more-blobs-less-votes/ for an analysis on the relation between timing games and attestation performance degradation as blob count increases with BPOs.
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. In my view the tradeoff is that if something is not incentive compatible for the proposer the outcome is just that a subset of "sophisticated" proposers will fork clients, which is undesirable 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. If it was configurable there'd want to be strict sanity limits imo, but i think it really does open up some degenerate behaviour to have it available potentially... Probably just not having it settable is the better option than the guards it'd need...
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.
Tend to agree with this. |
||||||||||||||||||||||||||
| in: header | ||||||||||||||||||||||||||
| required: false | ||||||||||||||||||||||||||
| description: | | ||||||||||||||||||||||||||
| Optional header containing the proposer's timeout for the request in milliseconds. | ||||||||||||||||||||||||||
| Builders should use this header to adjust the amount of time by which they delay the | ||||||||||||||||||||||||||
| requests to maximise block rewards. Otherwise, requests will timeout and the proposer | ||||||||||||||||||||||||||
|
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. why is this even specified?
Contributor
Author
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 was something added to MEV-Boost by Obol here: flashbots/mev-boost#817 , it made sense to specify it in the builder-spec. We specify it today too: https://github.com/ethereum/builder-specs/blob/main/apis/builder/header.yaml#L46 This is also a completely optional header which clients don't have to send. |
||||||||||||||||||||||||||
| will not receive the header in time. | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| type: integer | ||||||||||||||||||||||||||
| format: int64 | ||||||||||||||||||||||||||
| example: 10000 | ||||||||||||||||||||||||||
|
Comment on lines
+80
to
+82
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. int64 would need to be a string format because of limitations over 54? bits in javascirpt iirc.. @nflaig knows better than me on this though... its the reason most of beacon-api numbers being contained in string anyway...
Contributor
Author
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. The header is sent as ASCII bytes over the wire anyways. I think specifying it as int64 here is good to clarify the type of the header value for the servers parsing the header.
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 is already part of current builder-specs/apis/builder/header.yaml Lines 46 to 57 in 1bf8f49
I don't think it matters as @bharath-123 already noted as this is not transmitted via json |
||||||||||||||||||||||||||
| - name: Eth-Consensus-Version | ||||||||||||||||||||||||||
| in: header | ||||||||||||||||||||||||||
| required: false | ||||||||||||||||||||||||||
|
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. why is this header not required? if we change
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. I noted this before, I think this header should be required
Contributor
Author
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. yeah clients should it send it regardless of SSZ or JSON encoding, it seems useful to have in general
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. yes please 🙏 |
||||||||||||||||||||||||||
| description: | | ||||||||||||||||||||||||||
| The active consensus version to which the `SignedRequestAuth` in the | ||||||||||||||||||||||||||
| request body belongs. Required if the request body is SSZ encoded. | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| $ref: "../../builder-oapi.yaml#/components/schemas/ConsensusVersion" | ||||||||||||||||||||||||||
| - name: X-Eth-Max-Trusted-Bid | ||||||||||||||||||||||||||
|
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. should this rather be a query param, usually we only sent metadata in headers, could also consider making this optional, and omitting it means only trustless bids are accepted, same as setting to 0 |
||||||||||||||||||||||||||
| in: header | ||||||||||||||||||||||||||
| required: true | ||||||||||||||||||||||||||
| description: | | ||||||||||||||||||||||||||
| Decimal `uint64` (in Gwei) carrying the proposer's `max_trusted_bid` | ||||||||||||||||||||||||||
| for this request. `bid.execution_payment` MUST NOT exceed this value. | ||||||||||||||||||||||||||
| A value of `0` indicates that the proposer does not accept any | ||||||||||||||||||||||||||
| trusted payments from this builder. A value of `2**64 - 1` | ||||||||||||||||||||||||||
| (`MAX_TRUSTED_BID`) indicates that the proposer accepts any trusted | ||||||||||||||||||||||||||
| payment amount. If the header is missing or malformed, the builder | ||||||||||||||||||||||||||
| MUST return a 400 response. | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| type: integer | ||||||||||||||||||||||||||
| format: uint64 | ||||||||||||||||||||||||||
| example: 1000000000 | ||||||||||||||||||||||||||
| requestBody: | ||||||||||||||||||||||||||
| description: | | ||||||||||||||||||||||||||
| Optional `SignedRequestAuth` authenticating the request. If provided, | ||||||||||||||||||||||||||
|
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. sorry for this dumb question, how can a proposer learn about
Contributor
Author
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. Not at all a dumb question. A builder should advertise their 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. what if there is an endpoint say
Contributor
Author
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. An identity endpoint makes a lot of sense and is better than embedding this info into the status endpoint. I will spec this endpoint. 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. cool! when will we have a last call to get this PR sort out? It's been a while now, I'm still hesitate to implement since there is still stuff moving around
Contributor
Author
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. have a spec here: https://github.com/ethereum/builder-specs/pull/161/changes#diff-c48bd203974d43dd649472eb1c21dc906548942662b5f1fb998cf65f2990aaefR1 I think this might make sense?
Contributor
Author
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 am not sure yet about the last call for this PR haha, will raise it in ACDC this week!
Contributor
Author
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. yeah I wouldn't recommend to start implementation on this as of now yet. |
||||||||||||||||||||||||||
| the builder MAY verify the BLS signature against the validator pubkey | ||||||||||||||||||||||||||
| resolved from the `proposer_index` path parameter, and check that | ||||||||||||||||||||||||||
| `builder_pubkey` matches its own identity and that `slot` matches the | ||||||||||||||||||||||||||
| requested slot. If absent, the builder MAY still serve a bid subject | ||||||||||||||||||||||||||
| to its own policy. | ||||||||||||||||||||||||||
| required: false | ||||||||||||||||||||||||||
| content: | ||||||||||||||||||||||||||
| application/json: | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| $ref: "../../types/gloas/request_auth.yaml#/Gloas/SignedRequestAuth" | ||||||||||||||||||||||||||
| application/octet-stream: | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| description: "SSZ serialized `SignedRequestAuth` bytes. Use Content-Type header to indicate that SSZ data is contained in the request body. The `Eth-Consensus-Version` header MUST also be set." | ||||||||||||||||||||||||||
| responses: | ||||||||||||||||||||||||||
| "200": | ||||||||||||||||||||||||||
| description: Success response. | ||||||||||||||||||||||||||
| headers: | ||||||||||||||||||||||||||
| Eth-Consensus-Version: | ||||||||||||||||||||||||||
| $ref: "../../builder-oapi.yaml#/components/headers/Eth-Consensus-Version" | ||||||||||||||||||||||||||
| required: false | ||||||||||||||||||||||||||
| content: | ||||||||||||||||||||||||||
| application/json: | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| title: GetExecutionPayloadBidResponse | ||||||||||||||||||||||||||
| type: object | ||||||||||||||||||||||||||
| required: [version, data] | ||||||||||||||||||||||||||
| properties: | ||||||||||||||||||||||||||
| version: | ||||||||||||||||||||||||||
| type: string | ||||||||||||||||||||||||||
| enum: [ gloas ] | ||||||||||||||||||||||||||
| example: "gloas" | ||||||||||||||||||||||||||
| data: | ||||||||||||||||||||||||||
| $ref: "../../beacon-apis/types/gloas/execution_payload_bid.yaml#/Gloas/SignedExecutionPayloadBid" | ||||||||||||||||||||||||||
| application/octet-stream: | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| description: "SSZ serialized `SignedExecutionPayloadBid` bytes. Use Accept header to choose this response type" | ||||||||||||||||||||||||||
| "204": | ||||||||||||||||||||||||||
| description: No bid is available. | ||||||||||||||||||||||||||
| "400": | ||||||||||||||||||||||||||
| description: Error response. | ||||||||||||||||||||||||||
| content: | ||||||||||||||||||||||||||
| application/json: | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| $ref: "../../builder-oapi.yaml#/components/schemas/ErrorMessage" | ||||||||||||||||||||||||||
| examples: | ||||||||||||||||||||||||||
| InvalidHash: | ||||||||||||||||||||||||||
| value: | ||||||||||||||||||||||||||
| code: 400 | ||||||||||||||||||||||||||
| message: "Unknown hash: missing parent hash" | ||||||||||||||||||||||||||
| InvalidAuth: | ||||||||||||||||||||||||||
| value: | ||||||||||||||||||||||||||
| code: 400 | ||||||||||||||||||||||||||
| message: "Invalid SignedRequestAuth: signature verification failed" | ||||||||||||||||||||||||||
|
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 should probably be 401 as well no?
Contributor
Author
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. 401 is if the authorization fails or is not valid. I think this makes sense to be a 401. |
||||||||||||||||||||||||||
| MissingMaxTrustedBid: | ||||||||||||||||||||||||||
| value: | ||||||||||||||||||||||||||
| code: 400 | ||||||||||||||||||||||||||
| message: "Missing X-Eth-Max-Trusted-Bid header" | ||||||||||||||||||||||||||
| "401": | ||||||||||||||||||||||||||
| description: Authentication required. | ||||||||||||||||||||||||||
| content: | ||||||||||||||||||||||||||
| application/json: | ||||||||||||||||||||||||||
| schema: | ||||||||||||||||||||||||||
| $ref: "../../builder-oapi.yaml#/components/schemas/ErrorMessage" | ||||||||||||||||||||||||||
| examples: | ||||||||||||||||||||||||||
| MissingAuth: | ||||||||||||||||||||||||||
| value: | ||||||||||||||||||||||||||
| code: 401 | ||||||||||||||||||||||||||
| message: "Missing SignedRequestAuth: this builder requires authenticated requests" | ||||||||||||||||||||||||||
|
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. if 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. My understanding is that it is optional for the proposer to send the request with a Shall we also include a 403 error for the case where the signature was provided, but the signature is invalid, and the builder requires auth?
Contributor
Author
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. Some builders might require it, in that case they could send an error back instead of sending a bid back. 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. What about an error code to explicitly state that the signature is invalid?
Contributor
Author
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. that does make sense to have! I will add that.
Contributor
Author
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. We already have an error code for an invalid signature but I made it a 401 here: https://github.com/ethereum/builder-specs/pull/161/changes#diff-a186f4bbbe96e38c7699013ced6e58643ff9a03dddbc4fd7584c817920f5ef41R160 |
||||||||||||||||||||||||||
| "406": | ||||||||||||||||||||||||||
| $ref: "../../builder-oapi.yaml#/components/responses/NotAcceptable" | ||||||||||||||||||||||||||
| "415": | ||||||||||||||||||||||||||
| $ref: "../../builder-oapi.yaml#/components/responses/UnsupportedMediaType" | ||||||||||||||||||||||||||
| "500": | ||||||||||||||||||||||||||
| $ref: "../../builder-oapi.yaml#/components/responses/InternalError" | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -53,6 +53,10 @@ paths: | |||||
| $ref: "./apis/builder/validators.yaml" | ||||||
| /eth/v1/builder/header/{slot}/{parent_hash}/{pubkey}: | ||||||
| $ref: "./apis/builder/header.yaml" | ||||||
| /eth/v1/builder/execution_payload_bid/{slot}/{parent_hash}/{parent_root}/{proposer_index}: | ||||||
| $ref: "./apis/builder/execution_payload_bid.yaml" | ||||||
| /eth/v1/builder/beacon_block: | ||||||
|
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. pretty sure plural is more correct here, see previous
Suggested change
Contributor
Author
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. Not too opinionated, we only send back 1 beacon block hahaha. But I am fine with this change
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. but this is common REST naming convention, we also call it |
||||||
| $ref: "./apis/builder/beacon_block.yaml" | ||||||
| /eth/v1/builder/blinded_blocks: | ||||||
| $ref: "./apis/builder/blinded_blocks.yaml" | ||||||
| /eth/v2/builder/blinded_blocks: | ||||||
|
|
@@ -72,7 +76,7 @@ components: | |||||
| $ref: "./beacon-apis/types/http.yaml#/ErrorMessage" | ||||||
| ConsensusVersion: | ||||||
| $ref: "./beacon-apis/beacon-node-oapi.yaml#/components/schemas/ConsensusVersion" | ||||||
| enum: [bellatrix, capella, deneb, electra, fulu] | ||||||
| enum: [bellatrix, capella, deneb, electra, fulu, gloas] | ||||||
| example: "bellatrix" | ||||||
| Bellatrix.ExecutionPayload: | ||||||
| $ref: "./beacon-apis/types/bellatrix/execution_payload.yaml#/Bellatrix/ExecutionPayload" | ||||||
|
|
@@ -102,7 +106,11 @@ components: | |||||
| $ref: "./types/fulu/blobs_bundle.yaml#/Fulu/BlobsBundle" | ||||||
| Fulu.ExecutionPayloadAndBlobsBundle: | ||||||
| $ref: "./types/fulu/execution_payload_and_blobs_bundle.yaml#/Fulu/ExecutionPayloadAndBlobsBundle" | ||||||
|
|
||||||
| Gloas.RequestAuth: | ||||||
| $ref: "./types/gloas/request_auth.yaml#/Gloas/RequestAuth" | ||||||
| Gloas.SignedRequestAuth: | ||||||
| $ref: "./types/gloas/request_auth.yaml#/Gloas/SignedRequestAuth" | ||||||
|
|
||||||
| responses: | ||||||
| InternalError: | ||||||
| $ref: "./types/http.yaml#/InternalError" | ||||||
|
|
@@ -151,3 +159,5 @@ components: | |||||
| $ref: "./examples/fulu/signed_blinded_beacon_block.json" | ||||||
| Fulu.SignedBuilderBid: | ||||||
| $ref: "./examples/fulu/signed_builder_bid.json" | ||||||
| Gloas.SignedRequestAuth: | ||||||
| $ref: "./examples/gloas/signed_request_auth.json" | ||||||
|
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. usually we also have a ssz example, see https://github.com/ethereum/builder-specs/tree/main/examples/bellatrix
Contributor
Author
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. Yes I will add the SSZ examples. I was considering adding them in a follow up PR.
Contributor
Author
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. Claude was able to get the ssz examples from the json correctly without any issues! |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "value": { | ||
|
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. Why is this "value" needed? Shouldn't is just be message and signature?
Contributor
Author
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. That seems to be the common pattern in writing examples in the builder-specs. To encapsulate them in |
||
| "message": { | ||
| "builder_pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a", | ||
| "slot": "1" | ||
| }, | ||
| "signature": "0x1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505cc411d61252fb6cb3fa0017b679f8bb2305b26a285fa2737f175668d0dff91cc1b66ac1fb663c9bc59509846d6ec05345bd908eda73e670af888da41af171505" | ||
| } | ||
| } | ||
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.
Below in responses, it says 202
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.
Yeah my bad, i ll fix this