-
Notifications
You must be signed in to change notification settings - Fork 25
add per-key builders endpoints #88
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: master
Are you sure you want to change the base?
Changes from all commits
157a370
db7dc28
eeec060
576ec6a
52ce114
e2213e6
b647dd9
655d731
a29a3f7
8da225a
d5b54fc
831b257
be0f46e
4c8db99
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,156 @@ | ||||||||||||||
| get: | ||||||||||||||
| operationId: getBuilders | ||||||||||||||
| summary: Get Builders | ||||||||||||||
| description: | | ||||||||||||||
| Get the builder configuration in effect for an individual validator public key, with omitted | ||||||||||||||
| values resolved: an entry that omits a field is returned with the value that will be used for | ||||||||||||||
| it, and a key with no configuration of its own is returned with the validator client's own | ||||||||||||||
| configuration. A returned configuration is therefore fully resolved, so re-submitting it with | ||||||||||||||
| `POST` fixes those values rather than leaving them to follow the defaults. | ||||||||||||||
| security: | ||||||||||||||
| - bearerAuth: [] | ||||||||||||||
| tags: | ||||||||||||||
| - Builders | ||||||||||||||
| parameters: | ||||||||||||||
| - in: path | ||||||||||||||
| name: pubkey | ||||||||||||||
| schema: | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/schemas/Pubkey" | ||||||||||||||
| required: true | ||||||||||||||
| responses: | ||||||||||||||
| "200": | ||||||||||||||
| description: success response | ||||||||||||||
| content: | ||||||||||||||
| application/json: | ||||||||||||||
| schema: | ||||||||||||||
| title: GetBuildersResponse | ||||||||||||||
| type: object | ||||||||||||||
| required: [data] | ||||||||||||||
| properties: | ||||||||||||||
| data: | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/schemas/BuilderConfig" | ||||||||||||||
|
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. worth pointing out that keymanager-APIs/types/fee_recipient.yaml Lines 2 to 7 in beecfa8
I am not sure how useful that is though, the reason for adding this would be purely for consistency |
||||||||||||||
| examples: | ||||||||||||||
| BuilderConfig: | ||||||||||||||
| value: | ||||||||||||||
| data: | ||||||||||||||
| min_bid: "10000000" | ||||||||||||||
| builder_boost_factor: "100" | ||||||||||||||
| builders: | ||||||||||||||
| - url: "https://builder-a.example.com" | ||||||||||||||
| auth_data: "0x68747470733a2f2f6275696c6465722d612e6578616d706c652e636f6d" | ||||||||||||||
| builder_pubkeys: [] | ||||||||||||||
| max_execution_payment: "0" | ||||||||||||||
| min_bid: "20000000" | ||||||||||||||
| builder_boost_factor: "100" | ||||||||||||||
| - url: "https://builder-b.example.com" | ||||||||||||||
| auth_data: "0x1234567890abcdef" | ||||||||||||||
| builder_pubkeys: ["0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a"] | ||||||||||||||
| max_execution_payment: "250000000" | ||||||||||||||
| min_bid: "10000000" | ||||||||||||||
| builder_boost_factor: "100" | ||||||||||||||
| "400": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/BadRequest" | ||||||||||||||
| "401": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/Unauthorized" | ||||||||||||||
| "403": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/Forbidden" | ||||||||||||||
| "404": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/NotFound" | ||||||||||||||
| "500": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/InternalError" | ||||||||||||||
|
|
||||||||||||||
| post: | ||||||||||||||
| operationId: setBuilders | ||||||||||||||
| summary: Set Builders | ||||||||||||||
| description: | | ||||||||||||||
| Set the builder configuration for an individual validator public key. The submission replaces | ||||||||||||||
| the key's stored configuration in full; the server MUST NOT merge it with what was stored | ||||||||||||||
| before. | ||||||||||||||
|
|
||||||||||||||
| Each entry in the `builders` array MUST contain a `url`, and no two entries may share both | ||||||||||||||
| their `url` and their `auth_data`. The server MUST reject a submission that violates either | ||||||||||||||
| rule with a 400 rather than storing a partial list or silently dropping an entry. | ||||||||||||||
|
|
||||||||||||||
| `POST` always leaves the key configured; use `DELETE` to remove the configuration so the key | ||||||||||||||
| follows the validator client's own configuration again. An empty `builders` array is a | ||||||||||||||
| configuration that names no builders, not a removal. | ||||||||||||||
| security: | ||||||||||||||
| - bearerAuth: [] | ||||||||||||||
| tags: | ||||||||||||||
| - Builders | ||||||||||||||
| parameters: | ||||||||||||||
| - in: path | ||||||||||||||
| name: pubkey | ||||||||||||||
| schema: | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/schemas/Pubkey" | ||||||||||||||
| required: true | ||||||||||||||
| requestBody: | ||||||||||||||
| required: true | ||||||||||||||
| content: | ||||||||||||||
| application/json: | ||||||||||||||
| schema: | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/schemas/BuilderConfig" | ||||||||||||||
| examples: | ||||||||||||||
| BuilderConfig: | ||||||||||||||
| value: | ||||||||||||||
| min_bid: "10000000" | ||||||||||||||
| builder_boost_factor: "100" | ||||||||||||||
| builders: | ||||||||||||||
| - url: "https://builder-a.example.com" | ||||||||||||||
| max_execution_payment: "0" | ||||||||||||||
| - url: "https://builder-b.example.com" | ||||||||||||||
| builder_pubkeys: ["0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a"] | ||||||||||||||
| max_execution_payment: "0" | ||||||||||||||
| builder_boost_factor: "120" | ||||||||||||||
| responses: | ||||||||||||||
| "202": | ||||||||||||||
| description: successfully updated | ||||||||||||||
| "400": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/BadRequest" | ||||||||||||||
| "401": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/Unauthorized" | ||||||||||||||
| "403": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/Forbidden" | ||||||||||||||
| "404": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/NotFound" | ||||||||||||||
| "500": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/InternalError" | ||||||||||||||
|
|
||||||||||||||
| delete: | ||||||||||||||
| operationId: deleteBuilders | ||||||||||||||
| summary: Delete Configured Builders | ||||||||||||||
| description: | | ||||||||||||||
| Remove this public key's builder configuration entirely. The key then follows the validator | ||||||||||||||
| client's own configuration, in accordance with the startup of the validator client, exactly as | ||||||||||||||
| a key that was never configured does. | ||||||||||||||
|
|
||||||||||||||
| This is not the same as an empty `builders` list, which is a stored configuration saying this | ||||||||||||||
| key sources no builder-API bids. To stop using one builder while keeping the others, `POST` the | ||||||||||||||
| remaining entries of the `builders` array instead. | ||||||||||||||
| security: | ||||||||||||||
| - bearerAuth: [] | ||||||||||||||
| tags: | ||||||||||||||
| - Builders | ||||||||||||||
| parameters: | ||||||||||||||
| - in: path | ||||||||||||||
| name: pubkey | ||||||||||||||
| schema: | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/schemas/Pubkey" | ||||||||||||||
| required: true | ||||||||||||||
| responses: | ||||||||||||||
| "204": | ||||||||||||||
| description: Successfully removed the builder configuration, or there was none set for the requested public key. | ||||||||||||||
| "400": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/BadRequest" | ||||||||||||||
| "401": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/Unauthorized" | ||||||||||||||
| "403": | ||||||||||||||
| description: A builder configuration was found, but cannot be removed. This may be because it was in configuration files that cannot be updated. | ||||||||||||||
| content: | ||||||||||||||
| application/json: | ||||||||||||||
| schema: | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/schemas/ErrorResponse" | ||||||||||||||
| "404": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/NotFound" | ||||||||||||||
| "500": | ||||||||||||||
| $ref: "../keymanager-oapi.yaml#/components/responses/InternalError" | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,8 @@ servers: | |
| default: "http://localhost/" | ||
|
|
||
| tags: | ||
| - name: Builders | ||
| description: Set of endpoints for management of builders. | ||
| - name: Fee Recipient | ||
| description: Set of endpoints for management of fee recipient. | ||
| - name: Gas Limit | ||
|
|
@@ -52,6 +54,8 @@ paths: | |
| $ref: './apis/local_keystores.yaml' | ||
| /eth/v1/remotekeys: | ||
| $ref: './apis/remote_keystores.yaml' | ||
| /eth/v1/validator/{pubkey}/builders: | ||
|
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. maybe if we do allow a default config per pubkey that applies to all builders we should rename this to |
||
| $ref: './apis/builders.yaml' | ||
| /eth/v1/validator/{pubkey}/feerecipient: | ||
| $ref: './apis/fee_recipient.yaml' | ||
| /eth/v1/validator/{pubkey}/gas_limit: | ||
|
|
@@ -75,6 +79,10 @@ components: | |
| $ref: './types/eth_address.yaml' | ||
| Keystore: | ||
| $ref: './types/keystore.yaml' | ||
| BuilderConfig: | ||
| $ref: './types/builder_entry.yaml#/BuilderConfig' | ||
| BuilderEntry: | ||
| $ref: './types/builder_entry.yaml#/BuilderEntry' | ||
| FeeRecipient: | ||
| $ref: './types/fee_recipient.yaml' | ||
| GasLimit: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| BuilderConfig: | ||
| type: object | ||
| description: | | ||
| How a validator public key sources blocks from builders. | ||
|
|
||
| An empty `builders` list requests bids from no builder, overriding any builders the validator | ||
| client is configured with globally; with a `builder_boost_factor` of `0` the key also prefers | ||
| its local build. This differs from having no configuration, which follows the validator client | ||
| in full. | ||
|
|
||
| `BuilderConfig` carries a `min_bid` and a `builder_boost_factor` that are this key's defaults. | ||
| An entry that omits its own takes the default, so one value covers every builder without being | ||
| repeated on each entry, and the defaults also apply to p2p bids. | ||
|
|
||
| The validator client resolves every entry before it reaches the beacon node. An omitted | ||
| `min_bid` or `builder_boost_factor` takes this key's default, and then the validator client's | ||
|
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. let's assume we have |
||
| own configuration; an omitted `max_execution_payment` or `auth_data` takes the validator | ||
| client's configuration; an omitted `builder_pubkeys` resolves to the empty list. | ||
| required: [] | ||
| properties: | ||
| min_bid: | ||
| allOf: | ||
| - $ref: "../keymanager-oapi.yaml#/components/schemas/Uint64" | ||
| - description: | | ||
| This key's default `min_bid`, in Gwei. Applies to any entry that omits its own, and to | ||
| p2p bids. | ||
| example: "10000000" | ||
|
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. the |
||
| builder_boost_factor: | ||
| allOf: | ||
|
nflaig marked this conversation as resolved.
|
||
| - $ref: "../keymanager-oapi.yaml#/components/schemas/Uint64" | ||
| - description: | | ||
| This key's default `builder_boost_factor`. Applies to any entry that omits its own, and | ||
| to p2p bids. | ||
|
Comment on lines
+32
to
+33
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. it's a bit redundant but I would add more details here how it applies, similar to below |
||
|
|
||
| * `0`: prefer the local payload unless an error makes it unviable. | ||
| * `100`: profit maximization; choose whichever pays more. | ||
| * `2**64 - 1`: prefer the bid unless an error or health check makes it unviable. | ||
| example: "100" | ||
| builders: | ||
| type: array | ||
| maxItems: 64 # MAX_BUILDER_ENTRIES | ||
| description: | | ||
| The builders this key sources bids from, replacing any the validator client is configured | ||
| with. Omit this field to use the validator client's builders instead. Send an empty array to | ||
| use none, so no bid is requested from any builder and p2p bids remain this key's only | ||
| source. | ||
| items: | ||
| $ref: "../keymanager-oapi.yaml#/components/schemas/BuilderEntry" | ||
|
|
||
| BuilderEntry: | ||
| type: object | ||
| description: | | ||
| One builder a validator public key may source blocks from. | ||
|
|
||
| No two entries may share both their `url` and their `auth_data`. `url` is compared as the | ||
| exact string and `auth_data` as the decoded bytes, so hex case does not distinguish entries, | ||
| and an omitted `auth_data` is compared as the value the validator client would derive for it. | ||
| One request is made per entry, so several entries MAY share a `url` with different `auth_data`, | ||
| since several builders may be reachable at one `url`. | ||
|
|
||
| See [produceBlockV4](https://ethereum.github.io/beacon-APIs/#/Validator/produceBlockV4) for the | ||
| selection rule. | ||
| required: [url] | ||
| properties: | ||
| 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 configuration seems to be fully tailored towards builders connected via builder-api, based on the original intend from ethereum/beacon-APIs#620, this doesn't seem to match up, I left a related comment here ethereum/beacon-APIs#625 (comment) we also lose functionality we currently have, eg. the maybe @potuz, @terence, or @james-prysm can clarify this, I thought this follows the prysm implementation but it doesn't at all match what potuz documented in ethereum/beacon-APIs#620 which I do prefer since it at least supported the full spectrum of builders, and not just the builder-api |
||
| type: string | ||
| format: uri | ||
| minLength: 1 | ||
| maxLength: 2048 # MAX_BUILDER_URL_SIZE | ||
| description: | | ||
| The URL this entry's requests are sent to. | ||
| example: "https://builder.example.com" | ||
| auth_data: | ||
| type: string | ||
| pattern: "^0x(?:[a-fA-F0-9]{2}){1,4096}$" | ||
| description: | | ||
| Opaque data used to authenticate this validator's requests to the builder. Agreed with the | ||
| builder out of band and placed verbatim in the `RequestAuth.data` the validator signs. The | ||
| builder checks the signature and the exact bytes. Its meaning is left to the two parties | ||
| (for example, a shared secret). At most `MAX_DATA_SIZE` (4096) bytes. | ||
|
|
||
| The keymanager treats this value as opaque bytes: it only ever compares `auth_data` values | ||
| for equality, and never parses or inspects them. When no value has been agreed with the | ||
| builder out of band, the caller SHOULD set `auth_data` to the UTF-8 bytes of the builder's | ||
| own advertised URL, exactly as advertised, hex encoded: it is deterministic, distinct per | ||
| builder, and requires no coordination between the signers of a distributed validator. That | ||
| is purely a convention about what tooling writes into the field. | ||
|
Comment on lines
+82
to
+87
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. Wouldn't it be simpler/better to just treat this
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 thinking was if the key manager API required fully resolved fields, it simplified everything downstream since the request auth could be required by beacon api -> builder api and remove the need to reason about the cases where it's absent. If everyone used the default auth data they could just supply some motivation was talking with @james-prysm here where it was hard to reason about how to resolve configs when VCs have different defaults + keymanager fields were all optional
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 does
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 it isn't in the config, how could a node operator set their
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. operators can still set the
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. It definitely doesn't need to be required. I'm happy to make it and all of the fields that can as optional. It was just simpler if the key manager API supplied the VC with fully resolved values derived from the caller's config, so we don't have to reason about how missing fields in the key manager request body have to be resolved differently across all the different VC implementations
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 do see your point but most users are just fine with the default that validator clients provide and each client will have their own defaults, eg. we currently don't force users to provide a gas limit in their config, and most don't and just go with whatever the client sets as default. so the vc can easily fill missing fields with default and supply all fields to the beacon node via api. but when it comes to configuring the validator client from a user perspective, I would prefer if users only have to configure the values they are interested in instead of having to provide the full set of options
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. From your comment here, if we use I'm pretty in favor of this. Not too sure who will want to set per-key, per-builder p2p bid settings but it is elegant that we can support both p2p bids / builder api bids symmetrically. (will build towards this if it sounds good)
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.
if we want the mutually exclusive it could even be a config field like
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. Agreed separate fields over What I'm thinking is optional fields in keymanager and VC has to fully resolve before sending to BN. The one thing is we'll still need per-key
So thinking to also add a way for the keymanager api to express the per-key p2p defaults. |
||
|
|
||
| Omitting it leaves the validator client to derive it the same way, since the builder | ||
| requires authentication on every bid request. A zero-length `auth_data` is invalid. | ||
| example: "0x68747470733a2f2f6275696c6465722e6578616d706c652e636f6d" | ||
| builder_pubkeys: | ||
| type: array | ||
| maxItems: 64 # MAX_BUILDER_PUBKEYS | ||
| description: | | ||
| The builder BLS public keys this entry accepts bids from. Empty or omitted accepts any | ||
| builder; otherwise a bid not signed by one of them MUST NOT be accepted. | ||
| items: | ||
| allOf: | ||
| - $ref: "../keymanager-oapi.yaml#/components/schemas/Pubkey" | ||
| - description: "A builder's BLS public key. _48-bytes, hex encoded with 0x prefix, case insensitive._" | ||
| max_execution_payment: | ||
| allOf: | ||
| - $ref: "../keymanager-oapi.yaml#/components/schemas/Uint64" | ||
| - description: | | ||
| A ceiling, in Gwei, on the trusted execution payment accepted from this builder. The | ||
| remainder of its payment is expected in the trustless (in-protocol) form. | ||
|
|
||
| `0` accepts no execution payments from this builder, requiring the whole payment to be | ||
| trustless. `MAX_EXECUTION_PAYMENT` (`2**64 - 1`) accepts any amount. | ||
| example: "250000000" | ||
| min_bid: | ||
| allOf: | ||
| - $ref: "../keymanager-oapi.yaml#/components/schemas/Uint64" | ||
| - description: | | ||
| A floor, in Gwei, on the total payment accepted from this builder, counting a bid's | ||
| `value` plus its `execution_payment`. | ||
| example: "10000000" | ||
| builder_boost_factor: | ||
| allOf: | ||
| - $ref: "../keymanager-oapi.yaml#/components/schemas/Uint64" | ||
| - description: | | ||
| Percentage multiplier weighting this builder's bid against the local payload. Below | ||
| `100` favors the local payload, above `100` favors this builder. | ||
|
|
||
| Values that encode common preferences: | ||
|
|
||
| * `0`: prefer the local payload unless an error makes it unviable. | ||
| * `100`: profit maximization; choose whichever pays more. | ||
| * `2**64 - 1`: prefer this builder unless an error or health check makes it unviable. | ||
| example: "100" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,3 +19,8 @@ VPN | |
| BLS | ||
| oapi | ||
| Vero | ||
| Gwei | ||
| uri | ||
| UTF | ||
| RequestAuth | ||
| produceBlockV | ||
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.
what about changing file name to:
builders.yamlI don't think the keymanager will ever be used to configure builders, that is likely gonna be a different api if this is needed in the future, so "validator" here is redundant