Decouple builder request auth from the dial URL - #17373
Conversation
92490e3 to
18823f2
Compare
18823f2 to
ba183cc
Compare
… auth data from the dial URL
ba183cc to
2817290
Compare
| urls = append(urls, url) | ||
| id := entryIdentity{url: e.GetUrl(), data: string(e.GetAuth().GetMessage().GetData())} | ||
| if seen[id] { | ||
| continue |
There was a problem hiding this comment.
if the boost faster/builder pubkeys min bid are different this will just take the first one and drop, is it worth adding a debug log for this or is first seen fine?
| return | ||
| } | ||
| bid, err := c.GetExecutionPayloadBid(ctx, slot, parentHash, parentRoot, proposerPubkey, byURL[url]) | ||
| bid, err := c.GetExecutionPayloadBid(ctx, slot, parentHash, parentRoot, proposerPubkey, e.GetAuth()) |
There was a problem hiding this comment.
I don't believe we do any validation on the auth like on slot, if they end up differing we would get a 400 error
There was a problem hiding this comment.
The beacon deliberately doesn't introspect the auth: the spec requires forwarding message and signature byte for byte, and the data/slot inside are the builder's to verify against what it agreed with the validator
| @@ -45,4 +45,30 @@ message BuilderPreferencesRequest { | |||
| message SubmitBuilderPreferencesRequest { | |||
| bytes proposer_pubkey = 1; | |||
| BuilderPreferencesRequest request = 2; | |||
There was a problem hiding this comment.
I think we should change this to match 630 and have it as builder preferences entry + have it batched, right now
we can then change
func (v *validator) submitBuilderPreferenceRequests(ctx context.Context, reqs []*ethpb.SubmitBuilderPreferencesRequest) {
for _, req := range reqs {
if _, err := v.validatorClient.SubmitBuilderPreferences(ctx, req); err != nil {
log.WithError(err).Warn("Failed to submit builder preferences")
}
}
}
to match 630
| "github.com/OffchainLabs/prysm/v7/consensus-types/primitives.Gwei" | ||
| ]; | ||
| uint64 builder_boost_factor = 2; | ||
| repeated BuilderEntry builders = 3; |
There was a problem hiding this comment.
small nit shouldn't this be builder_entries or entries?
There was a problem hiding this comment.
It mirrors beacon-APIs 630, BuilderConfig.builders is the spec field name, so I keep it for 1:1 mapping with the REST
| func (v *validator) signRequestAuthCached(ctx context.Context, km keymanager.IKeymanager, pk pubkey, relay string, slot primitives.Slot) (*ethpb.SignedRequestAuth, error) { | ||
| key := requestAuthKey{pk: pk, slot: slot, relay: relay} | ||
| func (v *validator) signRequestAuthCached(ctx context.Context, km keymanager.IKeymanager, pk pubkey, authData []byte, slot primitives.Slot) (*ethpb.SignedRequestAuth, error) { | ||
| key := requestAuthKey{pk: pk, slot: slot, data: string(authData)} |
There was a problem hiding this comment.
is data: string(authData) ok or does it need to be like a hex
There was a problem hiding this comment.
Yes, it's fine. A Go string is just an immutable byte container. Hex would be an extra allocation for the same uniqueness
| Preferences: ðpb.BuilderPreferences{MaxExecutionPayment: e.MaxExecutionPayment}, | ||
| Auth: e.Auth, | ||
| } | ||
| if err := vs.BlockBuilder.SubmitBuilderPreferences(ctx, pubkey, e.Url, breq); err != nil { |
There was a problem hiding this comment.
since these are unique urls, maybe we should be spinning up these in their own go routines and submitting that way one doesn't block others? not sure if there are any bad tradeoffs though, as a thought.
…7374) - Builder-API bids are checked against their own entry's max execution payment, min bid, boost factor, and builder pubkeys - P2P bids use the config-level min bid and boost, payments are trusted only from entries naming the builder key - Removes the per-pubkey `maxExecutionPayments` map, the preferences push now sends each builder its own cap Last of the stack replacing OffchainLabs#17124, on top of OffchainLabs#17373.
2 participants
builder_request_authsonBlockRequestwith aBuilderConfigcarrying resolvedBuilderEntrys (url,auth,builder_pubkeys,max_execution_payment,min_bid,builder_boost_factor), shaped to beacon-APIs#630url) instead of the URL string, so the signed identity is decoupled from the HTTP dial target and a sidecar/proxy can front a builderurland forwards the auth byte-for-byte; entries may share aurlwith distinct auth data, one request per entrySubmitBuilderPreferencesRequestcarries the dialurlseparately from the opaque auth datamax_execution_payment(enforcement is the next PR in the stack)Second of the stack replacing #17124, on top of #17372.