Add builders endpoints under /eth/v1/beacon/states/{state_id}#610
Conversation
| get: | ||
| operationId: "getStateBuilder" | ||
| summary: "Get builder from state by id" | ||
| description: "Returns builder specified by state and id or public key along with status and balance." |
There was a problem hiding this comment.
nit
| description: "Returns builder specified by state and id or public key along with status and balance." | |
| description: "Returns builder specified by state and builder id." |
| - name: state_id | ||
| in: path | ||
| $ref: '../../../beacon-node-oapi.yaml#/components/parameters/StateId' | ||
| - name: id |
There was a problem hiding this comment.
getStateBuilder calls it builder_id maybe we should call it that
nflaig
left a comment
There was a problem hiding this comment.
I don't think we need to mirror the validator apis 1:1, especially not the validator apis we know have a bad design (eg. using non spec containers).
There is a big difference in how builders work compared to validators and how consumers will use the api.
- we don't expect builders to be nearly as many as validators, so fetching all builders always will likely be fine
- even if we don't wanna fetch all builders, having
getStateBuildersas defined in this PR would be sufficient as it allows to pass thepubkey - in contrast to validators, there should not be the use case to fetch thousands of records, it will likely just be a few at most, so returning the full
Builderobject should always be fine
There is likely more, but I think we can have a single api here which allows consumers to fetch all builders, or filter them by pubkey (or index) to just get the builders they care about.
| $ref: "./primitive.yaml#/Gwei" | ||
| description: "Current builder balance in gwei." | ||
| status: | ||
| $ref: "#/BuilderStatus" |
There was a problem hiding this comment.
I am definitely against introducing a custom type like this with a status field, especially because the status in case of builders it much simpler compared to validators
There was a problem hiding this comment.
maybe if we can conclude #601 this is fine, but I really like to avoid having another endpoint that cannot support ssz
the argument for the status in the response is to have all computation done on the beacon node side which is also valid and simplifies clients consuming the response
| @@ -0,0 +1,143 @@ | |||
| get: | |||
| operationId: "getStateBuilderBalances" | |||
There was a problem hiding this comment.
why do we need a separate api for balances? seems overkill
There was a problem hiding this comment.
im not against being easily able to get balances but we can probably just retain the 'post', and remove the get...
There was a problem hiding this comment.
but why does it need to be a separate endpoint? we can just get that by accessing builder.balance
this is in contrast to validators, as balances are separatly stored in the state via state.balances which is different from validator.effective_balance (actual balance vs. effective balance)
| @@ -0,0 +1,74 @@ | |||
| post: | |||
| operationId: "postStateBuilderIdentities" | |||
There was a problem hiding this comment.
I feel like this shouldn't be required, what's the most amount of builders a consumer needs to query? returning the full Builder spec object seems fine to me
| @@ -0,0 +1,60 @@ | |||
| get: | |||
| operationId: "getStateBuilder" | |||
There was a problem hiding this comment.
why do we need this? I feel like this was already a mistake in the validators api, can't we just have getStateBuilders?
| @@ -0,0 +1,168 @@ | |||
| get: | |||
| operationId: "getStateBuilders" | |||
There was a problem hiding this comment.
having both the get and post endpoint is too much, we should decide on whether we need post here, but if we wanna be on the safe side, likely best to just go with post here
|
I recommend closing this in favor of #614 |
Add builder endpoints similar to the validator endpoints. This is useful for builder clients to track and resolve their builder indices, status, balances etc when preparing bids.