ethereum / keymanager-APIs

Collection of RESTful APIs provided by Ethereum consensus keymanagers
https://ethereum.github.io/keymanager-APIs/
Creative Commons Zero v1.0 Universal
37 stars 19 forks source link

One single api endpoint to update proposer data #49

Closed g11tech closed 1 year ago

g11tech commented 2 years ago

Right now, there seem to be different endpoints to fetch/update/delete fee recipient, I propose there be just a single endpoint to update the proposer information of a validator pubkey, we should also include graffiti and builder info as part of proposer data update

  graffiti: 'default graffiti'
  strict_fee_recipient_check: true
  fee_recipient: '0xcccccccccccccccccccccccccccccccccccccccc'
  builder:
    enabled: true
    gas_limit: "30000000"

cc @james-prysm If you feel this is a good way to go, I can do a PR to update the api section

dapplion commented 2 years ago

thinking about the UX of this some sample cases:

So to achieve the above all properties should be also, and clients only update what's provided? Also I would prefer to not nest properties.

g11tech commented 2 years ago

I agree, it could just be a key,value pairs (all/any optional) that are posted to one endpoint, without constraining/relating to update of other properties (for e.g. gasLimit update would/should be independent to builder status).

Also agree that a flattened structure would be good for the api update and clients can then internally map to their corresponding structures, so we can have builder.enabled, builder.gas_limit as the properties which are flat and still decorate namespace.

Open to other formats as well.

ralexstokes commented 2 years ago

strict_fee_recipient_check: true

is this something implemented in all clients?

g11tech commented 2 years ago

strict_fee_recipient_check: true

is this something implemented in all clients?

I don't think so and I don't feel strongly for including this, but lodestar has this flag for validator.

rolfyone commented 2 years ago

Graffiti is a whole thing we've not really designed in this way, so from a teku perspective at least that's likely to be a big thing...

I'm not sure why it matters to have a single api endpoint when a UI is calling it anyway, unless it needs to be a single transaction where everything changes....

Is there some context in what you expect from strict_fee_recipient?

Builder configuration has never really leaked into keymanager api before, and it's not going to be appropriate for some implementers, I'm wondering why that's needed?

Builder configuration is also likely to cause issues or have to be ignored if the rest of the builder configuration underlying in the client isn't already setup, so i'm not sure it's quite as simple as allowing it to be turned on here...

rolfyone commented 2 years ago

My initial suggestion is

if an endpoint like this were to exist, i'd probably suggest all fields are mandatory, and if only one was wanting to be updated, that the specific endpoint should be used. That would allow better overall UX when something fails, rather than 'this update failed, bits may have been applied' kind of responses...

From the teku perspective we currently set graffiti in a very different way to this, and that's likely to be non trivial to implement it if it's actually needed... it'd basically be out of scope to set graffiti if other clients are configured similar to how teku is configured with graffiti...

james-prysm commented 2 years ago

I think going off paul's responses we had this discussion on discord and felt that even now I'm a bit afraid of committing to a specific spec as custom builder-related things haven't really solidified in my opinion and it'll be bad to coordinate breaking changes a lot all dependent on one api. @g11tech

strict_fee_recipient_check: true

is this something implemented in all clients?

at least for prysm right now we don't have this feature... not sure if we need it yet

Is there some context in what you expect from strict_fee_recipient?

If the flag --strict-fee-recipient is set in the validator client, Lighthouse will refuse to sign any block whose fee_recipient does not match the suggested_fee_recipient sent by this validator. This applies to both the normal block proposal flow and block proposals through the builder API.

this is what I saw lighthouse publish @rolfyone

Will continue to watch the thread for further development. My personal opinion is if we want to include builder-related settings I'd like it to be after I see more users using it in the current way to see how much value this API brings.