gloas proposer preference apis#608
Conversation
| properties: | ||
| version: | ||
| type: string | ||
| enum: [gloas] |
There was a problem hiding this comment.
maybe fulu should be available too?
There was a problem hiding this comment.
is there any reason not to include fulu?
There was a problem hiding this comment.
the proposer preferences concept doesn't exist or make sense for Fulu, does it?
There was a problem hiding this comment.
There is no fulu type for SignedProposerPreferences, so I'd just keep gloas here
There was a problem hiding this comment.
yeah was just thinking from perspective of having to submit proposer preferences while still in fulu and having the ability to get what you submitted?
There was a problem hiding this comment.
while the clock slot is still fulu, the proposal_slot would be in gloas, we would use the message slot anyways to determine the fork? I guess the same question exists in gossip, which uses the gloas fork digest even if messages are broadcast before the fork (at least in lodestar)
the same question exists if we wanna extend the proposer preferences in the future but I think we should always use the message slot
|
Awesome! I ll close my PR! |
| From the Gloas fork onwards, beacon nodes use the set of validators that have submitted | ||
| subscriptions via this endpoint to identify which validators are using the node for duties, | ||
| and adjust the node's custody group count (CGC) accordingly. Validator clients SHOULD |
There was a problem hiding this comment.
historically CGC was up only, this implies it might be able to be reduced now?
There was a problem hiding this comment.
No I can try to reword it. I'm just implying that we should use this as a replacement to prepare beacon proposer endpoint for cgc purposes.
| - For validators served by the beacon node, if no signed proposer preferences are available | ||
| for a given proposal slot, the beacon node MAY fall back to its configured default fee | ||
| recipient and target gas limit when constructing or accepting bids for that validator. | ||
| Bids whose fee recipient or gas limit do not match a validator's preferences will be | ||
| rejected at gossip validation, so applying sensible local defaults reduces missed | ||
| proposals during the transition. |
There was a problem hiding this comment.
should (vs may) fall back? what would be the alternative?
There was a problem hiding this comment.
zeroed out i guess, in a sense that is also a local default so i'll change it to should
| produceBlockV | ||
| stateful | ||
| CGC | ||
| dedup |
There was a problem hiding this comment.
dedup is a horrible horrible word, can we just spell de-duplicate out instead?
| @@ -0,0 +1,47 @@ | |||
| get: | |||
| operationId: getPoolProposerPreferences | |||
There was a problem hiding this comment.
A bit pedantic of me, but I find it strange that there is a pool API for a message type that is not included in blocks. In my mind the /pool APIs are for messages that can be packed into blocks like attestations, payload attestations, sync committee messages, etc.
There was a problem hiding this comment.
Separately, why do beacon nodes have to store these messages at all? My understanding is that they exist mainly for the sake of builders, so there is limited value in a beacon node keeping track of them after validation (aside from the minimal amount of data that is required to be tracked to detect duplicates).
There was a problem hiding this comment.
I guess builders mostly use the SSE event, but also load from this pool to catch up on any events that happened in the recent past prior to their subscription starting?
There was a problem hiding this comment.
Yeah the SSE event is the most important one. I guess its not so important to catch up on any events cause the preferences are gossiped every epoch. So in the worse case a builder would have to wait 2 epochs before building their blocks which I think is fine?
There was a problem hiding this comment.
should i remove it from the pool namespace? i can do that. i just didn't think it should be under validator, the original proposal from bharath was under pool.
waiting for some more opinions here I can remove the endpoint in general if not very useful
There was a problem hiding this comment.
Separately, why do beacon nodes have to store these messages at all?
we do need to store them to be able to validate bids against target gas limit and fee recipient of the proposer
A bit pedantic of me, but I find it strange that there is a pool API for a message type that is not included in blocks.
in lodestar we currently call this ProposerPreferencesPool, but we can think about renaming this if we wanna keep that notion to only use pool for messages that are included in blocks
There was a problem hiding this comment.
for now i removed it from the pools namespace as suggested
There was a problem hiding this comment.
I'm not sure I see the utility of it either.
nflaig
left a comment
There was a problem hiding this comment.
just some initial comments, haven't done an in-depth review yet
| properties: | ||
| version: | ||
| type: string | ||
| enum: [gloas] |
There was a problem hiding this comment.
There is no fulu type for SignedProposerPreferences, so I'd just keep gloas here
| $ref: "../../../beacon-node-oapi.yaml#/components/schemas/Gloas.SignedProposerPreferences" | ||
| application/octet-stream: | ||
| schema: | ||
| description: "SSZ serialized `List[SignedProposerPreferences]` bytes. Use Accept header to choose this response type" |
There was a problem hiding this comment.
List requires a limit, need to think more about what the right limit here is, technically you can have proposer preferences from different branches, so 2 * SLOTS_PER_EPOCH might not be sufficient
| @@ -0,0 +1,47 @@ | |||
| get: | |||
| operationId: getPoolProposerPreferences | |||
There was a problem hiding this comment.
Separately, why do beacon nodes have to store these messages at all?
we do need to store them to be able to validate bids against target gas limit and fee recipient of the proposer
A bit pedantic of me, but I find it strange that there is a pool API for a message type that is not included in blocks.
in lodestar we currently call this ProposerPreferencesPool, but we can think about renaming this if we wanna keep that notion to only use pool for messages that are included in blocks
| Deprecated from Gloas onwards in favor of | ||
| `POST /eth/v1/validator/proposer_preferences`, which supersedes both fee recipient and | ||
| gas limit preferences. Beacon node support for this endpoint pre-Gloas is REQUIRED; | ||
| post-Gloas support is OPTIONAL and the endpoint MAY be removed. |
There was a problem hiding this comment.
I don't think we should keep supporting this endpoint going forward, ideally, we wanna get rid of it in hegota
There was a problem hiding this comment.
should i reword this or is this wording fine
| @@ -0,0 +1,56 @@ | |||
| post: | |||
| operationId: submitSignedProposerPreferences | |||
There was a problem hiding this comment.
do we want the "signed" part in the operation id?
There was a problem hiding this comment.
good question will double check other ids
There was a problem hiding this comment.
they don't good catch i will remove it
…n-APIs into proposer-preferences-apis
|
updated wording to fall back to parent gas limit |
| parameters: | ||
| - in: header | ||
| schema: | ||
| $ref: "../../beacon-node-oapi.yaml#/components/schemas/ConsensusVersion" | ||
| required: true | ||
| name: Eth-Consensus-Version | ||
| description: "The active consensus version to which the signed proposer preferences being submitted belongs." |
There was a problem hiding this comment.
Do we need to enforce the consensus version here?
There was a problem hiding this comment.
that's required to make the endpoint forward compatible, otherwise if SignedProposerPreferences format changes in the future (eg. in heze) you cannot signal to the receiver what's the data format you are sending (required by receiver to decode the data), the receiver could always compute it from the current clock slot in the case here, but since proposer preferences should be send 1 epoch before the fork (due to epoch lookahead), that's not a good option either.
Supersedes #593 with ethereum/consensus-specs#5196, ethereum/consensus-specs#5236 added
Adds the proposer preferences API for Gloas, deprecates the legacy
fee-recipient/gas-limit endpoints, and tightens
beacon_committee_subscriptionsforCGC bookkeeping.
Added
POST /eth/v1/validator/proposer_preferences— VC submission of signedpreferences and publishes them on the
proposer_preferencesgossipsub topic.GET /eth/v1/beacon/proposer_preferences— retrieval, matching theconvention used by
payload_attestations,proposer_slashings, etc.proposer_preferencesSSE event for messages passing gossip validation.Gloas.ProposerPreferences/Gloas.SignedProposerPreferencestypes, includingdependent_root(required for gossip dedup and signature anchoring against thecheckpoint state at
epoch(proposal_slot) - 1).Deprecated
POST /eth/v1/validator/prepare_beacon_proposer— superseded by signed proposerpreferences. Pre-Gloas support is REQUIRED; post-Gloas the endpoint MAY be a no-op
or removed.
POST /eth/v1/validator/register_validator— the external builder relay flow isreplaced by the in-protocol builder market; fee recipient and gas limit move to
signed proposer preferences. Same pre/post-Gloas support rules apply.
Updated
POST /eth/v1/validator/beacon_committee_subscriptions— from Gloas onwards, BNsuse the subscription set to identify which validators are using the node and to
size the node's custody group count (CGC). VCs SHOULD submit one entry per
attached active validator per attestation duty, including when multiple
validators share the same
(slot, committee_index), so the BN tracks everyattached validator rather than only one per subnet.
Fixes #570