Deprecate prior and add /validator/config endpoint - #87
Conversation
| configs: | ||
| type: object | ||
| description: | | ||
| Mapping of validator public key (hex encoded with 0x prefix) to the full | ||
| `ValidatorConfig` document to set for that key. |
There was a problem hiding this comment.
Would it be possible this one to be the validator's index, rather than the public key?
In cases like DVs it's a bit better to have references of the index, rather than the public key, as the public keys the VC sees are partial keys, rather than the full key, as seen on the CL side. The current request which is with similar intent, POST /eth/v1/validator/prepare_beacon_proposer uses validator index. So my assumption is it won't be a big hurdle to use validator index here as well?
There was a problem hiding this comment.
I don't have a strong opinion here but it seems like the key manager clients that normally deal with just public keys would require a BN query to resolve the mapping from pubkey -> validator index before making the call. It would be good to know if this change would negatively impact any other software.
There was a problem hiding this comment.
You could do index for active keys but for new additions you wouldn't have an index, so if you wanted to support indices you'd still need to support pubkey as well unless you want inconsistent apis i think...
There was a problem hiding this comment.
Operationally pubkeys make much more sense for this. Node operators can pre-configure keys in batches at any time, including before they become active. With indexes, operators would need to continuously monitor the validator statuses and update their configs one-by-one or in small batches, quite impractical and unnecessarily complex.
Can't DVs work around this the same way they have worked around the current Keymanager API endpoints that are also using the pubkey?
There was a problem hiding this comment.
I agree with paul here, I think it's preferable to use public key here
There was a problem hiding this comment.
Okay, those points are valid, fine on my side to keep it as pubkeys.
Can't DVs work around this the same way they have worked around the current Keymanager API endpoints that are also using the pubkey?
They can, it would've just made the life a little bit easier. I assumed it might also be easier for key managers to go with indices as well, given the current implementation of prepare_beacon_proposer, but seems like it won't be.
51ee282 to
9f66e70
Compare
| type: string | ||
| description: | | ||
| - set: the document was validated and applied atomically | ||
| - not_found: the key is not known to the server |
There was a problem hiding this comment.
we should permit configs for keys not found i think, its not clear that if this is saying it's not set
There was a problem hiding this comment.
we should allow for preprovisioning
There was a problem hiding this comment.
added pre-provisioning and as a consequence this requires using pubkeys and not validator indices
| description: | | ||
| - set: the document was validated and applied atomically | ||
| - not_found: the key is not known to the server | ||
| - error: the document failed validation or could not be applied; no change |
There was a problem hiding this comment.
what should we do for unknown fields, silently accept ( client specific) or error here?
There was a problem hiding this comment.
removed additionalProperties: false for more leniency to match the rest of the keymanager API. Unrecognized fields should be ignored and invalid values for recognized fields should error
| description: | | ||
| Sets the block-production preferences of one or more keys. For each key in `request.configs`, | ||
| the submitted document REPLACES the key's entire configured fragment: the server MUST NOT merge | ||
| it with previously configured values. A field absent from the submitted document is unset and |
There was a problem hiding this comment.
a present builder object replaces default_config's builder entirely; inheritance applies only to absent top-level fields
right now prysm is looking at " BuilderEntry → BuilderConfig → default_config → client default" inheritance
in prysm it's whole object reading instead of field level reading . do we have to do inheritance by field level?
if builder config is set at the public key level then it replaces defaults in its entirety only if we are missing a field we fall back down the hierachy, not sure if others have feels about this
There was a problem hiding this comment.
BuilderEntry → BuilderConfig → default_config → client default
on the same page with this, I just tried disambiguating it in 8052f47
- keep pubkey as the config key (since no index exists for not-yet-active keys) - pre-provisioning: a config may be set for an unmanaged key - accept unrecognized fields to match the rest of the keymanager API - disambiguate per-builder field resolution
|
I am all for having this grab bag of options. Eth Docker may not configure every single one out of the gate; and, it's good to have it available. |
| example, a shared secret). If unset, no authentication is sent to this builder. At most | ||
| `MAX_DATA_SIZE` (4096) bytes. | ||
| example: "0x1234567890abcdef" | ||
| pubkey: |
There was a problem hiding this comment.
is this value needed? i realized in terence's poc i'ts missing this
There was a problem hiding this comment.
terence originally said
Cap bounds the "promise", pubkey control whose promises count
so the cap is: min_bid / max_execution_payment / builder_boost_factor
pubkey here is to check the originator of the promise.
arguably the signed authorization solves this already so I'm not married to it, curious others' thoughts
| "0xa99a76ed7796f7be22d5b7e85deeb7c5677e88e511e0b337618f8c4eb61349b4bf2d153f649f7b53359fe8b94a38e44c": | ||
| builder: | ||
| enabled: false | ||
| "400": |
There was a problem hiding this comment.
should we note specifics on not accepting duplicate urls just like we did on ethereum/beacon-APIs#630 ? or any other specifics
There was a problem hiding this comment.
i'm in the process of specifying that we MUST accept duplicate urls as long as the each per-key, there are no duplicate (url, auth_data) pairs
- BuilderEntry: all fields required except pubkey (optional verification
pin);
auth_data is required and opaque, compared but never parsed
- entries are identified by their (url, auth_data) pair: url as the
exact
string, auth_data as its decoded bytes; a duplicate pair rejects the
key's
entire document, never silently dropped
- config documents are all-or-nothing: a key has a complete document or
none,
{} clears it, and default_config is a read-only view of client
defaults;
per-field inheritance and per-builder fallbacks are removed
- proxy fields removed; multiple entries may share a url, one request
per entry
- nothing-agreed convention: auth_data SHOULD be the UTF-8 bytes of the
builder's advertised URL, hex encoded
- min_bid floors the bid's total payment (value plus execution_payment); builder_payload_value is defined as that total and builder_boost_factor applies only after the max_execution_payment and min_bid checks, matching beacon-APIs wording - bump the builder pubkey description above the allOf so renderers show it instead of the generic validator pubkey text
|
I have some issues with this PR. It seems to me that we're trying to accommodate / simply the UX for the most complex and niche operator that has some strange use-case where they need to mix together keys that have different builder requirements into the same validator process. Meanwhile the UX for everyone else (especially regular home stakers) has been made significantly worse because:
I'm all for standardizing things to make life easier for downstream software. But do we even know that there is anyone who needs this level of granularity? And why wouldn't they just run different validator processes? That is probably the right level of isolation anyway if you're custodying keys that have different builder policies. Also more generally, combining every setting into one endpoint means every possible setting must be specified in order to change any setting. So I actually think this makes everyone's UX worse. You already said:
But how much more verbose and error-prone is defining every setting for every builder multiplied by thousands of keys in a single request? |
The intent wasn't to make this harder for anyone! This is an API, so the assumption is it gets called programmatically. Every VC already has its own way of defining a base config, so rather than standardize that, I left the documents to whatever tooling is making the keymanager calls (fully resolved, so the API doesn't have to assume inheritance rules when matching the POST body against the base config). On having to specify everything to change one thing, that's intentional and it doesn't cost much in practice. GET returns the key's complete document, so tooling reads it, edits one field, and posts it back. setFeeRecipient is already per-key, and its description says "Specific Public keys not mapped will continue to use the default address for fee recipient in accordance to the startup of the validator client and beacon node". default_config follows that convention, I just surfaced it on GET. What gloas changes is the number of per-key options, not the model. On whether operators need this much configurability, I've been trying to get as many eyes on this as possible. I defaulted to maximal expressiveness rather than deciding for them, mostly to avoid bikeshedding. The api caller can already take a default and map it to the per-key fields. But if that's the biggest friction, letting the API write the VC's default config so you set one document instead of N would address it. Worth doing? |
|
I appreciate your work on this Jason, but I do believe we first need to agree on whether we need this at all, before we discuss how to implement this. Just because we can, doesn't mean we should.
+1 – I haven't heard of a single node operator that has asked for this with a specific reason or usecase in mind. As I've said in previous Beacon API/Discord conversations on this topic, I am not convinced per-key builder configurations are useful and I believe there are only a handful of node operators in the world that might want to use it. The vast majority of node operators will do something very simple and rational – connect all their validators to as many trustless builders as they can. The very few niche node operators that for any reason would like to have different builder configurations are likely resourceful node operators that can afford the complexity of running a separate validator client per different builder config. Existing Keymanager API endpoints have proven usecases and are used by large parts of the network. These proposed per-key builder configs? I'm not convinced they fulfil either of those. With this proposed change we'd basically be moving the maintenance and complexity burden from the niche node operators to Keymanager API maintainers & implementers. In terms of the PR itself, my first question would be why not just add another |
|
I want to reiterate that I like the idea of standardizing this endpoint and that my main issue is with making all the settings per-validator and combining every setting to one endpoint while deprecating the individual endpoints. This would be much simpler if we just had an endpoint for the builder configuration and applied it to all validators controlled by that process. |
|
From an operator perspective, the per-key config is needed.
There is also an explanation as to why multiplexing matters from an operator's perspective here: https://youtu.be/PPWwpPx4it0?t=1377 TL;DR: this sort of setup is not uncommon: any operator handling Lido keys (whether it is CSM or part of the curated set, so roughly 20%+ of the network) has to comply with a specific list of relays and options only for those specific Lido keys, it will likely be similar for builders/ePBS (https://operatorportal.lido.fi/existing-operator-portal-old-v2/ethereum-onboarding/mev-relays). It's also common to have different keys needing different requirements (if you have customers hosting US keys for instance, chances are you'll need specific options dedicated for OFAC compliance).
At scale, having a validator client per specific set of keys does not work as you want to spread your validation to cope with the occasional failure of a machine/vc etc. If all keys of a specific customer are in the same basket it makes operations/SLAs worse. |
|
I feel like the keymanager api is elegant because of it's simplicity and it works for most node operators as is. With gloas, we want to allow operators to configure builders, but the previous apis for gas limit, fee recipient and graffiti are still valid/useful and already integrated into tooling. A much less controversial approach in my opinion would be to add 3 new apis to manage builder configurations per-validator
Having the option to configure per-validator is a given imo, the keymanager api works like that since its existence. More questionable to me seem per-builder options, most likely those gonna be unused by the vast majority of operators but also complexity wise seems not too bad to support. so the above 3 apis should already allow everything this PR allows and clients could be free to store in whatever internal representation they already have for this, eg. in lodestar we have a proposer-config that is quite similar to the proposed format in this PR. There is still value in having a more standardized interchange format, similar to how we have the slashing protection interchange format but that seems like a completely separate topic. I think gloas gives us a good reason to try and standardize this too since the config becomes much more complex and it allows operators to more easily migrate to another client if they would like to without having to reconfigure all keys. |
|
I appreciate you adding that context @aimxhaisse . FYI the motivation/links you posted are partially irrelevant post-Glamsterdam – for Glamsterdam, the validator client does get configured with the builder URLs you want it to use (as opposed to today where you configure them in mev-boost/commit-boost). So if you have a good reason to split validator keys into groups with different builder configs, you can already do that much more easily in Glamsterdam by spinning up a corresponding set of VC instances. The mentioned major downside of infrastructure duplication no longer applies (you no longer need multiple mev-boost instances or CL/EL nodes, only additional lightweight VC instances). However, I can see why this still could be useful. And if it helps us get rid of the client-specific config files it makes sense to me to do it this way instead. So, add me to the support camp in terms of motivation for this PR. +1 for Nico's proposal for |
I'll make these changes tmrw:
|
better to do a separate PR for this, then we can decide which approach we prefer |
#88 has the alternative approach cc @nflaig @eth2353 @ethDreamer |
| enabled: | ||
| type: boolean | ||
| description: | | ||
| Whether blocks may be sourced from builders for this key. |
There was a problem hiding this comment.
@JasonVranek should this setting exist in #88 too? Imagine you have builder urls set globally on the vc but wanna disable sourcing builder bids for a pubkey, how would you do this otherwise?
also wondering how you'd configure a validator to only produce local blocks, since we can now source bids from p2p there will likely always be a builder bid available, I guess you can set builder_boost_factor=0, or a really high min_bid to achieve this but if local payload fails it falls back to a bid, which in general is a good thing, but it's something to consider if someone really doesn't want to select bids and only self-build, not so sure right now how to express that over the beacon-api though. The alternative I see if someone really doesn't want to use builder bids is to have a flag on the beacon-node (or api query param) to disable the p2p bid pool
There was a problem hiding this comment.
I commented in 625 on adding p2p bid config options which would likely require additional fields back in this and/or #88.
Since we added a DELETE endpoint in #88 I figured it is simpler to just clear your key's builder config -> forcibly fall back to the default, rather than have another flag to check
There was a problem hiding this comment.
Imagine you have builder urls set globally on the vc but wanna disable sourcing builder bids for a pubkey, how would you do this otherwise?
but that doesn't solve this scenario, the default will be to have builders configured globally, eg. in lodestar I plan to add a --builder.urls flag, same as mev-boost has today, and that's it, that will be sufficient for the vast majority of operators. I do wanna give users the option to disable these globally set urls for a specific pubkey though, how would I they do this with this bool flag?
There was a problem hiding this comment.
Imagine you have builder urls set globally on the vc but wanna disable sourcing builder bids for a pubkey, how would you do this otherwise?
but that doesn't solve this scenario, the default will be to have builders configured globally, eg. in lodestar I plan to add a
--builder.urlsflag, same as mev-boost has today, and that's it, that will be sufficient for the vast majority of operators. I do wanna give users the option to disable these globally set urls for a specific pubkey though, how would I they do this with this bool flag?
understood! yea I can add the flag back to 88
Add
/eth/v1/validator/config: atomic per-key management of block-production preferencesMotivation
With ePBS, proposers can talk to builders directly instead of through relay lists configured in a sidecar. The validator client becomes the owner of a per-key builder list with per-builder constraints: which builders to request bids from, the authentication data agreed with each builder, a trusted-payment cap (
max_execution_payment), bid floors, boost factors, and an optional expected bid signer. The VC must own this list because it pre-signs the request authentications that bind each request to the auth data agreed with that builder.Today that configuration only exists as static per-client config files (see the example in OffchainLabs/prysm#17124). Defining many builder entries across hundreds or thousands of keys in a static file is verbose and error-prone, and downstream software (e.g. eth-docker, dappnode, etc) would need to maintain a different config dialect per client. The keymanager API already solves this class of problem for fee recipient, gas limit and graffiti, but as three separate endpoint families, and the ePBS builder preference set is about to grow well past what per-field endpoints can sanely express.
What this PR does
/eth/v1/validator/config(GET/POST), managing a validator's full block-production preferences (fee recipient, target gas limit, graffiti, builder preferences) as one atomic per-key document.feerecipient,gas_limitandgraffitiendpoint families. This endpoint absorbs their responsibilities and replaces three partial-write surfaces with a single consistent one.The document model
Per key:
ValidatorConfig = {fee_recipient, target_gas_limit, graffiti, builder}. Thebuilderblock holdsenabledand thebuilders[]list. EachBuilderEntrycarriesurl,auth_data,max_execution_payment,min_bid,builder_boost_factor(all required), and an optionalpubkeypinning the expected bid signer.Documents are all-or-nothing. This API is called programmatically, so the caller resolves everything before submitting: a key either has a complete document, used exactly as stored, or no document, in which case it follows the client's defaults (surfaced read-only as
default_config). Nothing is inherited or merged per field, and there are no per-builder fallbacks.Entries are identified by their (
url,auth_data) pair (urlas the exact string,auth_dataas its decoded bytes). Multiple entries may share aurl, each with differentauth_data; the validator client sends one request per entry.auth_datais opaque: the keymanager only compares it for equality and never parses it. When nothing has been agreed with a builder out of band, the caller SHOULD setauth_datato the UTF-8 bytes of the builder's own advertised URL, hex encoded.Monetary values are Gwei as decimal strings.
builder_boost_factorsemantics (including the reserved 0 / 100 / 2**64 - 1 values) andmax_execution_paymentnaming follow ethereum/beacon-APIs#630 and the builder-specsBuilderPreferencesRequestV1.Example:
GET /eth/v1/validator/configEach configured key returns its complete document, exactly as submitted. A known key with no document returns
{}; its effective preferences aredefault_config, taken whole.{ "data": { "default_config": { "fee_recipient": "0x8943545177806ED17B9F23F0a21ee5948eCaa776", "target_gas_limit": "30000000", "graffiti": "example graffiti", "builder": { "enabled": true, "builders": [ { "url": "https://builder-a.example.com", "auth_data": "0x68747470733a2f2f6275696c6465722d612e6578616d706c652e636f6d", "max_execution_payment": "0", "min_bid": "0", "builder_boost_factor": "100" }, { "url": "https://builder-b.example.com", "auth_data": "0x68747470733a2f2f6275696c6465722d622e6578616d706c652e636f6d", "max_execution_payment": "0", "min_bid": "0", "builder_boost_factor": "100" } ] } }, "configs": { "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": { "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3", "target_gas_limit": "45000000", "graffiti": "example graffiti", "builder": { "enabled": true, "builders": [ { "url": "https://builder-a.example.com", "auth_data": "0x1234567890abcdef", "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a", "max_execution_payment": "250000000", "min_bid": "10000000", "builder_boost_factor": "100" }, { "url": "https://builder-a.example.com", "auth_data": "0xdeadbeef", "max_execution_payment": "0", "min_bid": "0", "builder_boost_factor": "100" } ] } }, "0xa99a76ed7796f7be22d5b7e85deeb7c5677e88e511e0b337618f8c4eb61349b4bf2d153f649f7b53359fe8b94a38e44c": { "fee_recipient": "0x8943545177806ED17B9F23F0a21ee5948eCaa776", "target_gas_limit": "30000000", "graffiti": "", "builder": { "enabled": false } }, "0xb0b1c2d3e4f5a6b7c8d9e0f1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b3c4d5e6f7": {} } } }The first key shows two entries sharing a
urlwith distinctauth_data(oneurlfronting two builders); the secondauth_datavalues demonstrate the URL-bytes convention (0x687474... is "https://builder-a.example.com" in UTF-8).Example:
POST /eth/v1/validator/configFull-replace per key. The second key demonstrates the duplicate-pair rejection (
0xdeadbeefand0xDEADBEEFdecode to the same bytes); the third shows the delete idiom:{}clears the key back to defaults, so there is no separate DELETE operation.{ "configs": { "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": { "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3", "target_gas_limit": "45000000", "graffiti": "example graffiti", "builder": { "enabled": true, "builders": [ { "url": "https://builder-a.example.com", "auth_data": "0x68747470733a2f2f6275696c6465722d612e6578616d706c652e636f6d", "max_execution_payment": "250000000", "min_bid": "10000000", "builder_boost_factor": "100" } ] } }, "0xa99a76ed7796f7be22d5b7e85deeb7c5677e88e511e0b337618f8c4eb61349b4bf2d153f649f7b53359fe8b94a38e44c": { "fee_recipient": "0x8943545177806ED17B9F23F0a21ee5948eCaa776", "target_gas_limit": "30000000", "graffiti": "", "builder": { "enabled": true, "builders": [ { "url": "https://builder-b.example.com", "auth_data": "0xdeadbeef", "max_execution_payment": "0", "min_bid": "0", "builder_boost_factor": "100" }, { "url": "https://builder-b.example.com", "auth_data": "0xDEADBEEF", "max_execution_payment": "0", "min_bid": "0", "builder_boost_factor": "100" } ] } }, "0xb0b1c2d3e4f5a6b7c8d9e0f1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b3c4d5e6f7": {} } }Response, per-key statuses keyed by the same pubkeys:
{ "data": { "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": { "status": "set" }, "0xa99a76ed7796f7be22d5b7e85deeb7c5677e88e511e0b337618f8c4eb61349b4bf2d153f649f7b53359fe8b94a38e44c": { "status": "error", "message": "builder.builders[1] duplicates the (url, auth_data) pair of builder.builders[0]" }, "0xb0b1c2d3e4f5a6b7c8d9e0f1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b1c2d3e4f5a6b7c8d9e0f1a2b3c4d5e6f7": { "status": "set" } } }Key semantics
{}(the delete idiom); anything in between fails that key.url,auth_data) pairs reject the key's entire document. The server never silently drops entries; the caller resolves and dedupes upstream.{}and followsdefault_configverbatim.default_configis read-only through this API.Relationship to other work
This PR is part of a three-PR set that defines the data flow from VC <> BN <> builders
Notes for reviewers
default_configis deliberately read-only in this version: it is the minimal static floor a bare VC needs, and management tooling owns the per-key layer. Writable defaults can be added later without breaking changes.info.versionis bumped to2.0.0-devto signal the breaking deprecation; happy to adjust to the repo's release conventions.