Staked Builder API for Glamsterdam - #138
Conversation
01bb9e6 to
0cf7078
Compare
|
Open questions:
|
|
Spellcheck fixes are trivial and shuoldn't block any feedback. |
|
Note: We want the response to the get the bid to be signed by the validator on the builder pub key and slot. This is so that the builder won't get spammed by other builders to reveal their bid. |
1a37a61 to
9bd69ae
Compare
| def verify_registration_signature(state: BeaconState, signed_registration: SignedValidatorRegistrationV2) -> bool: | ||
| validator = state.validators[signed_registration.message.validator_index] | ||
| pubkey = validator.pubkey | ||
| domain = compute_domain(DOMAIN_APPLICATION_BUILDER) |
There was a problem hiding this comment.
Do we need to update the domain as we are introducing a new type of registration?
|
|
||
| ```python | ||
| class BuilderPreferences(Container): | ||
| execution_payment_accepted: boolean |
There was a problem hiding this comment.
Can we allow validators to express arbitrary preferences here? That might be useful for proposers and builders alike. But the proposer and builders will have to agree on the scheme for preferences ahead of time. And on the other hand, hard coding preferences into BuilderPreferences will have to involve a co-ordinated rollout amongst clients.
There was a problem hiding this comment.
What arbitrary preferences are we talking about here? My feeling is that we don't necessarily need that. If proposers want builders to perform arbitrary logic, they just shouldn't connect to any other builder.
| assert signed_bid.prev_randao == get_randao_mix(state, get_current_epoch(state)) | ||
| assert is_builder(state, builder.pubkey) | ||
|
|
||
| if signed_bid.value > 0: |
There was a problem hiding this comment.
it is not necessary for a builder to use the execution_payment field to pay the bid even through the builder-api.
|
|
||
| ```python | ||
| def is_eligible_for_bid(state: BeaconState, | ||
| registrations: Dict[BLSPubkey, ValidatorRegistrationV2], |
There was a problem hiding this comment.
I guess it shouldn't be relevant to use whether registrations are a Dict from BLSPubKey -> ValidatorRegistrationV2 or ValidatorIndex -> ValidatorRegistrationV2?
| description: Root of the beacon block the proposer will build on. | ||
| schema: | ||
| $ref: "../../builder-oapi.yaml#/components/schemas/Root" | ||
| - name: proposer_index |
There was a problem hiding this comment.
should we stick with the proposer pubkey or use proposer index?
There was a problem hiding this comment.
we could just stick with proposer_index. its much easier to read.
There was a problem hiding this comment.
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).
Thus, pubkey might be preferable if it doesn't add any complexity.
There was a problem hiding this comment.
I believe we use pubkey in the existing getPayloadHeader so maybe for consistency we should stick to pubkey?
There was a problem hiding this comment.
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.
In favour of simplifying client implementation by sticking to pubkeys
There was a problem hiding this comment.
|
|
||
| ```python | ||
| class ValidatorRegistrationV2(Container): | ||
| builder_index: BuilderIndex |
There was a problem hiding this comment.
do we have to specify the builder_index in validator registrations? The main issue it addresses is that, it disallows malicious builders to send registrations to other builders to whom the proposer doesn't intend to fetch a block from.
This could also be a list of builder indices instead of just a builder index because a builder could have multiple corresponding indices and i don't think we will specify preferences at the builder index level.
c31ff96 to
154b1fe
Compare
| schema: | ||
| type: array | ||
| items: | ||
| $ref: "../../beacon-apis/types/gloas/bid_request_auth.yaml#/Gloas/SignedBidRequestAuth" |
There was a problem hiding this comment.
We send a SignedBidRequestAuth for each builder index registered by the builder. This could become large if a builder has registered multiple builder indices for themselves. But I am unsure as to why they would do that.
There was a problem hiding this comment.
We could send a list of indices per endpoint. The builder only needs to check that his index is in the list. In fact this is better for the CL as well: a validator can keep the full list of builders with the same builder preferences and send one signature to all. Replaying would become a race condition but given that the timeout is relatively large, I think this may not be a problem
| enum: [ gloas ] | ||
| example: "gloas" | ||
| data: | ||
| type: array |
There was a problem hiding this comment.
We get back a bid corresponding to each builder index. The builder may not choose to return bids for all builder indices.
There was a problem hiding this comment.
For e.g, The builder could choose to just return one bid from one builder index.
154b1fe to
fd4266f
Compare
| @@ -0,0 +1,38 @@ | |||
| Gloas: | |||
There was a problem hiding this comment.
We should move these to beacon-apis repo after the builder-api has been approved.
| #### `ValidatorRegistrationV2` | ||
|
|
||
| ```python | ||
| class ValidatorRegistrationV2(Container): |
There was a problem hiding this comment.
We are not specifying the builder_index in the ValidatorRegistrationV2 object because the preferences should ideally be per builder url instead of per builder index. I am unsure how preferences can change based on the builder index.
2bbb09d to
59c3194
Compare
|
|
||
| ```python | ||
| class BidRequestAuth(Container): | ||
| builder_index: BuilderIndex |
There was a problem hiding this comment.
Do we want to specify the builder_index here? Or could we do without it? Do we want to auth per builder_index or url?
|
|
||
| ### New Containers | ||
|
|
||
| #### `BidRequestAuth` |
There was a problem hiding this comment.
We could probably have a better name here?
* updates * remove header to communicate max_execution_payment * use proposer_pubkey instead of index * some spec updates * update is_eligible_for_bid * update error code * don't encapsulate the beacon block under a "data" field * add an identity api * reword sentence on block and payload decoupling * update request auth to use builder url * update error codes * add eth consensus version header * suffix containers with v1 * run make lint * fix spellcheck * minor spec updates * builder can choose to not serve bid if preferences not seen
| def is_eligible_for_bid( | ||
| state: BeaconState, | ||
| proposer_preferences: Dict[ValidatorIndex, ProposerPreferences], | ||
| slot: Slot, | ||
| parent_hash: Hash32, | ||
| # [New in Gloas] | ||
| parent_root: Root, | ||
| # [New in Gloas] | ||
| proposer_pubkey: BLSPubkey, | ||
| ): |
There was a problem hiding this comment.
Not all parameter changes here are documented. Like proposer_preferences is new too.
builder-specs/specs/bellatrix/builder.md
Lines 279 to 285 in 1bf8f49
Should document what's removed as well. In consensus-specs, we'd do:
# [Modified in Gloas:EIP7732]
# Removed `registrations`
# [New in Gloas:EIP7732]
proposer_preferences: Dict[ValidatorIndex, ProposerPreferences],
...| # Verify that proposer preferences have been received via the gossip topic | ||
| assert validator_index in proposer_preferences.keys() |
There was a problem hiding this comment.
This is modified but not marked as such.
There was a problem hiding this comment.
This would be marked with # [New in Gloas:EIP7732] cause the check with proposer preferences is pretty new.
| # Verify parent root | ||
| # [Modified in Gloas:EIP7732] | ||
| assert parent_root == hash_tree_root(state.latest_block_header) |
There was a problem hiding this comment.
This is completely new, not modified.
| [`BuilderPreferencesRequestV1`](#builderpreferencesrequestv1) to each builder | ||
| via the [`submitBuilderPreferences`][submit-builder-preferences-api] API call in | ||
| the epoch prior to the epoch in which they will be proposing, as determined from | ||
| `state.lookahead`. This ensures builders have the preferences before the bid |
There was a problem hiding this comment.
It should be state.proposer_lookahead.
| the builder's staked collateral on-chain even when `bid.execution_payment` is | ||
| also set. | ||
|
|
||
| ## Constructing a `SignedExecutionPayloadEnvelope` |
There was a problem hiding this comment.
Not sure if you want to specify this (because I do not want to in consensus-specs) but this would be an appropriate place to talk about when a builder should reveal their payload with some certainty that it will be included onchain, within some risk tolerance.
There was a problem hiding this comment.
I am unsure if there is value in specifying it. I would assume builders would be sophisticated enough to have their own strategies here. We can revisit this in the future when builders start adapting their infra and there is value in adding it here.
| #### `BuilderPreferencesV1` | ||
|
|
||
| `BuilderPreferencesV1` communicates a proposer's per-builder preferences to a | ||
| specific builder ahead of the bid request. | ||
|
|
||
| ```python | ||
| class BuilderPreferencesV1(Container): | ||
| max_execution_payment: Gwei | ||
| ``` |
There was a problem hiding this comment.
I sort of suggest simplifying this to just be PreferencesV1. Because to me it sounds like these are the builder's preferences, not the preferences for each builder. Might just be me.
There was a problem hiding this comment.
Yeah the intention is that, these are the per builder preferences of the validator. I am fine with PreferencesV1 too, will check what ppl think.
|
|
||
| Validators MAY communicate their per-builder preferences ahead of the bid | ||
| request by calling the | ||
| [`submitBuilderPreferences`][submit-builder-preferences-api] API in the epoch |
There was a problem hiding this comment.
I might suggest calling this something like prepareExecutionPayloadBid instead. Builders would use this as a signal to start working on a bid. Right now the name doesn't make this exactly clear.
There was a problem hiding this comment.
I am not sure if prepareExecutionPayloadBid is right here. Builders start working on a bid for proposers ideally every slot based on the payload attributes SSE event.
This endpoint is for validators to be able to submit their preferences. The builder would build their bid based on the latest preference they have seen for the proposer. It is not an indication to start building their bids.
| type: integer | ||
| format: int64 | ||
| example: 10000 |
There was a problem hiding this comment.
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...
Not sure if that's a problem here specifically, but figured its worth talking about
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
this is already part of current getHeader
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
| builder is responsible for publishing the execution payload envelope. The proposer | ||
| MAY observe PTC votes to monitor the timely reveal of the execution payload envelope. |
There was a problem hiding this comment.
The proposer MAY observe PTC votes to monitor the timely reveal of the execution payload envelope.
why is that note here, is there any action you are suggesting the proposer should do, or clients should implement?
There was a problem hiding this comment.
Nope that can be removed, that was a old line that shouldn't be there! this was back when we were thinking about payload reorg based on PTC. But regardless this shouldn't be there.
There was a problem hiding this comment.
we are doing payload reorgs based on PTC, so builders, if you read this, watch out
| builder, authenticated via a `SignedRequestAuthV1`. | ||
|
|
||
| Validators MAY call this endpoint in the epoch prior to the epoch in | ||
| which they will be proposing, as determined from `state.lookahead`, so that |
There was a problem hiding this comment.
this should be state.proposer_lookahead
| - in: header | ||
| schema: | ||
| $ref: "../../builder-oapi.yaml#/components/schemas/ConsensusVersion" | ||
| required: false |
There was a problem hiding this comment.
we should make this required, or how exactly do you determine the consensus type otherwise?
There was a problem hiding this comment.
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.
| $ref: "../../beacon-apis/types/primitive.yaml#/Pubkey" | ||
| - name: Eth-Consensus-Version | ||
| in: header | ||
| required: false |
There was a problem hiding this comment.
same here, we should make this header required
| (`Content-Type: application/octet-stream`). | ||
|
|
||
| The proposer's `max_execution_payment` is communicated exclusively via the | ||
| [`submitBuilderPreferences`][submit-builder-preferences-api] endpoint. If no |
There was a problem hiding this comment.
[submit-builder-preferences-api] is this a placeholder?
There was a problem hiding this comment.
Shouldn't have added it here. Removing it!
| example: 10000 | ||
| - name: Eth-Consensus-Version | ||
| in: header | ||
| required: false |
There was a problem hiding this comment.
I noted this before, I think this header should be required
There was a problem hiding this comment.
yeah clients should it send it regardless of SSZ or JSON encoding, it seems useful to have in general
| $ref: "../../types/gloas/request_auth.yaml#/Gloas/SignedRequestAuthV1" | ||
| application/octet-stream: | ||
| schema: | ||
| description: "SSZ serialized `SignedRequestAuthV1` 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." |
There was a problem hiding this comment.
| description: "SSZ serialized `SignedRequestAuthV1` 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." | |
| description: "SSZ serialized `SignedRequestAuthV1` bytes. Use Content-Type header to indicate that SSZ data is contained in the request body." |
we should always sent the version header
| $ref: "./apis/builder/header.yaml" | ||
| /eth/v1/builder/execution_payload_bid/{slot}/{parent_hash}/{parent_root}/{proposer_pubkey}: | ||
| $ref: "./apis/builder/execution_payload_bid.yaml" | ||
| /eth/v1/builder/beacon_block: |
There was a problem hiding this comment.
pretty sure plural is more correct here, see previous /eth/v2/builder/blinded_blocks
| /eth/v1/builder/beacon_block: | |
| /eth/v1/builder/beacon_blocks: |
There was a problem hiding this comment.
Not too opinionated, we only send back 1 beacon block hahaha. But I am fine with this change
There was a problem hiding this comment.
but this is common REST naming convention, we also call it /eth/v2/beacon/blocks on the beacon-api
There was a problem hiding this comment.
usually we also have a ssz example, see https://github.com/ethereum/builder-specs/tree/main/examples/bellatrix
There was a problem hiding this comment.
Yes I will add the SSZ examples. I was considering adding them in a follow up PR.
There was a problem hiding this comment.
Claude was able to get the ssz examples from the json correctly without any issues!
| application/octet-stream: | ||
| schema: | ||
| description: "SSZ serialized `SignedRequestAuthV1` 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." |
There was a problem hiding this comment.
did anyone ssz encode a SignedRequestAuthV1 already and made sure it can be decoded by the receiver? I might be wrong, but arbitrary strings like urls seem problematic
There was a problem hiding this comment.
I have a unit test in buildoor on a sort of round trip with request auth here: https://github.com/ethpandaops/buildoor/pull/81/changes#diff-9e15279d13b1890a9f040f159e5d3431a84f9a3406480a4e304b1bd8d75b55e3R58
We basically marshall it into SSZ, unmarshal it, get its hash tree root, sign it and verify the signature
There was a problem hiding this comment.
This is very similar to extra data: https://github.com/ethereum/consensus-specs/blob/6ebb2216ca8d7fdac7c108fcfddb74d0e56c42ab/specs/bellatrix/beacon-chain.md#L174
which is also a ByteList[MAX_DATA_BYTES] and can be any arbitrary string.
There was a problem hiding this comment.
if we are fine with data being hex encoded in the json and not human readable, then this works. worth pointing out that depending on what character encoding you use to convert an arbitrary string to bytes it may produce different results, but I am assuming everyone is using utf-8 and for urls, this is mostly irrelevant even with other encodings like ascii
) This PR adds the protobuf types for the Gloas builder API: request auth and builder preferences plus the `DOMAIN_REQUEST_AUTH` signature domain and the `max_execution_payment` / `request_auth` fields on the keymanager config. Definitions only ethereum/builder-specs#138
This PR begins work and discussions on the Builder-API for ePBS for staked builders.
ePBS introduces the following protocol upgrades:
BeaconBlockvalidation andExecutionPayloadvalidation.We introduce 3 new APIs. We ideally want to have new APIs to make the transition from Fulu to Gloas easier:
getExecutionPayloadBid: POST/eth/v1/builder/execution_payload_bid/{slot}/{parent_hash}/{parent_root}/{proposer_pubkey}Called by the proposer to the builder/relay. The builder/relay returns the best execution payload bid available to it. Per-request validator inputs are communicated as follows:
SignedRequestAuthV1request body (optional): authenticates the request. The body MAY be encoded as JSON (Content-Type: application/json) or SSZ (Content-Type: application/octet-stream); when SSZ is used, theEth-Consensus-Versionheader MUST also be set. If the body is omitted, the builder MAY still serve a bid.submitBuilderPreferences: POST/eth/v1/builder/builder_preferences/{validator_pubkey}Called by the proposer in the epoch prior to the epoch in which they will be proposing. The proposer sends a
BuilderPreferencesRequestV1containing aBuilderPreferencesV1(withmax_execution_payment) and aSignedRequestAuthV1to authenticate the request. The builder MUST check thatauth.message.datamatches its own URL and MUST verify the BLS signature against thevalidator_pubkeypath parameter to ensure that the preference came from the intended proposer.submitSignedBeaconBlock: POST/eth/v1/builder/beacon_blockCalled by the proposer when it commits to the bid received from
getExecutionPayloadBid. The proposer sends theSignedBeaconBlockwith the execution payload bid embedded. The builder is then responsible for constructing and broadcasting the correspondingSignedExecutionPayloadEnvelopeto the PTC committee.ValidatorRegistrationV1is deprecated in favour ofProposerPreferencesfrom the consensus specs. General validator preferences (fee_recipient,gas_limit, etc.) are now communicated via theproposer_preferencesgossip topic defined in the consensus specs. Validators broadcastSignedProposerPreferencesmessages at the beginning of each epoch for their proposal slots in the next epoch, and builders SHOULD subscribe to this gossip topic.We introduce
RequestAuthV1which contains the following fields:data: MUST be set to the URL of the builder the request is intended for.slot: The slot for which the request is being sent.SignedRequestAuthV1is a container holding aRequestAuthV1message and a BLS signature over it. The goal is to allow the proposer to authenticate to a specific builder for a given slot, preventing DDOS and replay attacks from other parties. Sending theSignedRequestAuthV1withgetExecutionPayloadBidis optional; builders MAY use it for authentication purposes.We also introduce
BuilderPreferencesV1, which carriesmax_execution_payment— the maximum execution layer payment (in Gwei) the proposer will accept from this builder. If no preferences have been submitted, the builder MUST treatmax_execution_paymentas0and MUST NOT include an execution layer payment in its bid.Closes: #137