Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validator registration #206

Closed

Conversation

lightclient
Copy link
Member

One requirement of the Builder API is in builder_registerValidatorV1, the validator must sign a message to authenticate its registration. This is to ensure only the person who controls the validator can set its fee_recipient, even in a multi-party environment.

Registering currently entails signing over a timestamp, pubkey, and fee_recipient. The new, optional validator_registration object on prepare_beacon_proposer includes the minimal additional values (timestamp and signature) needed to satisfy the registration. If preferred, I could expand validator_registration to all required fields to verify the signature -- this would lead to duplication of fee_recipient and a new value pubkey which is already specified by validator_index.

This is my first PR to the beacon API so please let me know what I'm missing 馃檹

@james-prysm
Copy link
Contributor

I wanted to better understand the intended behavior here, what is supposed to happen with this optional set of information? for example, if I call the prepare beacon proposer api with the optional information then call it afterward without the optional information it's supposed to override as empty?
right now the keymanager APIs ( validator client frontend ) doesn't have this information so I wanted to ask what the intended behavior is.

@metachris
Copy link

metachris commented Apr 27, 2022

Would these be optional? mev-boost forwards them to the relays/builders (so they know which feeRecipient to set when building the block). See also the diagrams in the README: https://github.com/flashbots/mev-boost

(basically the payload of prepare_beacon_proposer will be used as payload for builder_registerValidatorV1)

@james-prysm
Copy link
Contributor

I'm a bit new to these apis/development, just to clarify the proposal you made. so now we would like the array of objects to include this message object ( which includes the previous fee recipient to validator index, now with added pubkey and timestamp?) and a signature of that message object? I wanted to ask what that expected timestamp represents ( maybe this is a dumb question 馃槗 sorry). I also wanted to ask I guess we would have to internally support this new signing request but also remote signers like web3signer would ideally have this new request to sign this message object too?

@ajsutton
Copy link
Contributor

It would be good to get some more detail on what the timestamp value has to be. If it's essentially just a sequence number so the message with the highest timestamp should be used we probably should just call it sequence_number and implementors can choose to use the current timestamp for the value.

If the timestamp means the message expires at some point we need to consider the load we could be putting on signer components by having to sign additional messages regularly. We've already seen issues caused by the need for all validators to sign the epoch number to determine if they're validating as it leads to a huge number of requests being sent to the signer at the start of each epoch (which can fortunately be spread out). Adding the new to also sign these validator registrations could introduce significant additional load depending on how often the need to be signed.

@metachris
Copy link

metachris commented Apr 28, 2022

The timestamp is used for replay protection, that older messages cannot override later ones (eg. that a malicious actor couldn't resend an old message and override a newer feeRecipient for a given validator). The message doesn't expire itself, it would only be replaced (by a newer one with a different feeRecipient and newer timestamp).

@djrtwo I recall you had strong opinions on using a timestamp over a sequence number here, would be great it you could you add your thoughts too 馃檹 (cc/ @ralexstokes)

@lightclient
Copy link
Member Author

the addition of pubkey was discussed and principally agreed on at the mev-boost workshop on mev.day

I definitely recall agreeing on this being included in the message for builder_registerValidator, but I don't recall strong agreement on the exact format for updating this method. I don't have a strong opinion either way, but definitely don't think we should then also include validator index in the signed message as the two are isomorphic.

I don't think these would be optional, it's a bundle of parameters that is signed, and sent to mev-boost, which forwards it to the relays/builders

I'm not sure the best way to convey this, but validator_registration here should be optional. The attributes within the object are not optional if the object is not null. I believe @paulhauner recommended it be optional as the method is currently used as a sort of heartbeat for the beacon node to know if the validator client is up and running.

so now we would like the array of objects to include this message object ... ?

Each object in the array would have it's own signature and relevant data to verify the signature (e.g. timestamp at minimum) as the signature calculated by each validator.

It would be good to get some more detail on what the timestamp value has to be. If it's essentially just a sequence number so the message with the highest timestamp should be used we probably should just call it sequence_number and implementors can choose to use the current timestamp for the value.

Right, so a reason timestamp was chosen was that in potential future where mev-boost has p2p capabilities, we'll want to minimize the amount of data it is required to store. With timestamp, it can be trivial to prune old records because messages are only propagated / utilized if they are within some delta of the current system time.

As a general note, I'm not sure how much detail to give in this endpoint on timestamp and signature. They are spec'd in the Builder API specification - should I link to that in the description? Or is there a better way?

If the timestamp means the message expires at some point we need to consider the load we could be putting on signer components by having to sign additional messages regularly.

This is why I believe validator_registration should be optional. It should really only be set i) on start up and ii) if the fee recipient is changed. IMO, a slightly more principled approach is to have a completely separate method, but in the mev-boost workshop it seemed that clients preferred overloading this endpoint.

@ralexstokes
Copy link
Member

ralexstokes commented Apr 28, 2022

I see two other free parameters in the execution payload that validators may want to be able to set (and so we will update the registration here and in the builder API)

  1. preferred/required gas limit target
  2. extraData

there is no way for a validator to signal if they have a preference on the gas limit for blocks when using remote builders, extending this registration will let them. the semantics are up in the air -- it could be a suggestion or it could be "required" with the understanding that if a builder returns a deviation they can take a reputation hit

extra data is less interesting imo as validators have the beacon block graffiti and it may still be useful for builders to put their own data into this part of the block

edit: imo this should be a separate API method and not overloaded onto an existing thing, and esp. if we add these 1 or 2 things then it will start to diverge even more

@mcdee
Copy link
Contributor

mcdee commented Apr 28, 2022

Could the validator client not overwrite the extra data and gas limit fields when it receives the execution payload (or header) from either the beacon node or the builder prior to signing it and returning the signed block proposal?

Or do these have an impact on the following state information, such that the values are required to be part of the execution of the payload?

@ralexstokes
Copy link
Member

ralexstokes commented Apr 28, 2022

they could overwrite the extra data although then I would question why we have builders sign over it in the first place. it may seem small but I'd say if we are going to have an actor commit to some data via signature then no one downstream should be able to change it. as a policy, this facilitates the implementation of fraud proof-like games which will help when we decentralize the overall architecture down the line. and this may be a reach but you could imagine extra data ends up being a vector for MEV and we don't want any agent in the system to have unilateral control over some data beyond what is absolutely required.

there are a few issues w/ the gas limit:

you don't want a validator grieving builders by lowering the gas limit just below what they have assembled (which is easily doable when the block is full) and then crying wolf that they were slighted by the relay/builder as an attack on that pair's reputation.

I also think putting the gas limit in the hands of a small cartel of builders is a bad idea as it makes it easier to coordinate to raise it against the best interests of keeping a lean, decentralized protocol -- this imo goes strongly against ethereum's values so we need a way for validators to at least signal to builders their intention. if there is a "hostile" take-over on this front, we will at least be able to provide proof of foul play from builders

edit: oh and we can almost certainly imagine the gas limit having an impact on MEV extraction so something we want to keep "constrained" in a similar sense to what I described above. each player should only have a few choices during each turn

@mcdee
Copy link
Contributor

mcdee commented Apr 28, 2022

I would question why we have builders sign over it in the first place.

From my point of view, it proves that a given builder supplied a given execution payload with expectation of funds. As to if the validator wants to sign that given payload, that's up to the validator.

I'm not sure where the control of block gas limit should be, but I'm inclined for the execution layer to handle it and as such not have the validator involved. Extra data is a different item, but feels like it fits as a field in builder_getHeaderV1 which is analogous to the graffiti field in the existing beacon API when requesting a block (and perhaps should be added there as well).

@lightclient
Copy link
Member Author

@mcdee the issue is that when using an external block builder, there is only communication between the CL and the external builder. At no point does the EL have an opportunity to chime in. Furthermore, since choosing the gas limit is on the critical path to block production (hard to build a block if you don't know how big it will be), I think it makes sense to also include a gas target in the validator registration phase of the Builder API.

@mcdee
Copy link
Contributor

mcdee commented Apr 29, 2022

@lightclient Does the builder not need to talk to the EL to understand the current block gas limit when building the block? If not, from where does it obtain this information?

It feels to me that adding the gas target in the validator registration is overloading the purpose of validator registration. I think it would make more sense for the builder or execution node to control this value separate from the validator, which also seems closer to how it works today.

@metachris
Copy link

Extra data is a different item, but feels like it fits as a field in builder_getHeaderV1

Note that builders want to start the building process already before this call is happening, to have a block ready to deliver immediately.

@mcdee
Copy link
Contributor

mcdee commented Apr 29, 2022

Note that builders want to start the building process already before this call is happening, to have a block ready to deliver immediately.

Does the lack of extra data at this time matter? As far as I can see, the builder can build the block, then slot in the extra data and sign it when it receives the builder_getHeaderV1 request?

@lightclient
Copy link
Member Author

lightclient commented Apr 29, 2022

@mcdee this diagram is a good illustration of communication paths:

image

mev-boost is a multiplexer across potentially many relays/builders (the distinction between these actors is not relevant to this point). They will of course run their own ELs to build blocks, but that is outside the purview of the validator that will propose the block. A desirable property of the gas target is that it is controllable by individual validators rather than a small number of builders.

Does the lack of extra data at this time matter?

The lack of extra data probably matters the least, because recalculating the block is relatively trivial -- just slot in the value, re-hash. The others, feeRecipient and gasTarget are less trivial as they require recomputing the state root.

@metachris
Copy link

The relay/builder setup will probably look like this, where builders submit both header and payload to the relay, which acts as escrow of sorts, and verifies that the feeRecipient received the correct value.

mev-boost-integration-overview

This also means the header must be signed as received, and can't easily be updated without another roundtrip to the builder (which might add a new layer to the issue of withholding). In this setup, the relay is trusted, and guarantees to the validator that it is already in possession of the payload.

@lightclient lightclient force-pushed the signed-validator-registration branch from f70c94b to 3c89262 Compare May 5, 2022 15:04
@lightclient
Copy link
Member Author

The two main questions that still need to be resolved:

  1. Are we okay with extending this endpoint or do we want a new one? I'm getting some conflicting thoughts here.
  2. What values should the validator respond with? Should it be i) the minimum additional elements to verify the signature or ii) should it follow the message, signature object format?

@djrtwo
Copy link
Contributor

djrtwo commented May 5, 2022

@lightclient help me remember the outputs of the conversation in devconnect. I believe that there was a desire to split this a a separate endpoint to (1) decouple it from critical operation endpoint and (2) becaus this message is likely to be on a different rhythm from the message for beacon proposer registration.

I find (1) very compelling. I dont want to have to upgrade what is a critical operation endpoint in the event that MEV boost iterates/changes. It's nice for core validator signing to be left generally unchanged as much as possible now and in future iterations.

Were there other reasons to decouple worth mentioning?

@lightclient
Copy link
Member Author

@djrtwo there was discussion about separating, but as I recall, the final decision was to not separate it and simply make validator_registration an optional value on the request.

I prefer separating though for the aforementioned reasons and have spec'd out this alternative in #209.

@lightclient
Copy link
Member Author

Closing as there has been no opposition to moving forward with #209.

@lightclient lightclient closed this May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants