Clarifications on builder spec

[JasonVranek]

There have been some ambiguities when reading the builder specs around slot numbers, URL encodings, request auth signing/verifying, plus some broken links. This PR aims to clarify them.

[shane-moore]

From the SSV side, the proposal-slot clarification and reuse of one auth for the AOT and JIT channels are compatible with our pre-signing design. I think one signing-contract issue remains.

RequestAuthV1.data contains bytes, but canonicalize operates on URL text. The current code passes the byte-valued request_auth.data directly to a string function, which raises a type error. Conversely, the canonical URL returned by that function must be ASCII-encoded before it can populate the SSZ ByteList.

More importantly for SSV, normalizing inside verification authenticates a reconstructed message rather than the exact signed_request_auth.message. If operators do not derive the same exact message and signing root, their partial signatures cannot be reconstructed.

Could we instead canonicalize the configured builder URL before constructing RequestAuthV1, then have the builder compare the received message.data with its own canonical URL bytes?

This maps onto the existing spec structure: steps 1 and 2 belong in specs/gloas/validator.md under "Constructing the RequestAuthV1" and "URL canonicalization"; step 3 belongs in specs/gloas/builder.md under "Signing"; and steps 4 and 5 belong in that file's existing request-validation text. types/gloas/request_auth.yaml should then mirror the resulting byte-level meaning of data.

1. Canonicalize and validate configured URL text.
2. ASCII-encode it and construct RequestAuthV1(data=canonical_bytes, slot=proposal_slot).
3. Sign compute_signing_root(request_auth, domain) without transforming it.
4. On the builder, require message.data to equal the builder's canonical identity bytes, then verify compute_signing_root(message, domain) unchanged.
5. Reject noncanonical wire data, even if normalizing it would identify the same builder.

[JasonVranek]

5078eab decouples the request authorization data from the builder's URL. The motivation is that request authorizations require data unique to the builder. A builder's URL is just one instance of this, so rather than lock into one format, the idea is that the authorization data is just opaque bytes. These can be pre-negotiated between the builder and proposer, or very well just be the builder's URL. This change just makes it more flexible.

Related to the conversation here ethereum/beacon-APIs/pull/625

[gd-0]

Thanks @JasonVranek !

Very supportive of changes that:

1. include ahead-of-time proposer preferences
2. natively support sidecars and their future extensions

[potuz]

I think all explicit mentions to a proxy should be completely removed from the spec. This is out of protocol and unnecessary to be mentioned.

We just need to have two fields, one is the url to send the request, and another is arbitrary data. The latter is part of the signed auth, the former is not.

These two fields are already enough to enable proxying if people want to use that.

[JasonVranek]

Agreed, will remove explicit mentions of proxy across the PRs. The proxy was included because of the change to make auth data opaque bytes and not explicitly carry the builder's url. For a proxy to exist under this format, they must set url to their own and require data to carry the builder's URL to route the request. An easily fixable issue is that the constraint that per-key, URLs must be unique should be relaxed to (url, data) pairs should be unique. This would allow for example, a relay to receive N requests to different builders that carry real auth data.

For example the config would need to support:

{
      "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": {
        "fee_recipient": "0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3",
        "target_gas_limit": "45000000",
        "builder": {
          "enabled": true,
          "builders": [
            {
              "url": "https://proxy.example.com",
              "data": "https://builder-a.example.com",
              "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a",
              "max_execution_payment": "250000000",
              "min_bid": "10000000",
              "builder_boost_factor": "100"
            }
          ],
          "max_execution_payment": "500000000"
        },
        {
        "url": "https://proxy.example.com", // this should be allowed (will update PRs)
        "data": "https://builder-b.example.com",
        ...
        }
      }

[nflaig]

related to ethereum/beacon-APIs#630 (comment), maybe there was some discussion around this but I don't really understand why we wanna sent a single request per pubkey, maybe someone can explain this

[JasonVranek]

I don't think I understood the question the first time. Is the assumption if a node operator has more than one proposer in the lookahead they can save on a call if batching was supported?

[nflaig]

yes, you only do a single call no matter how many validators are proposers in the lookahead, that matches how the proposer preferences api works

as far as I am aware all apis support batching, this one seems like an exception that decided to do a request per pubkey, but I don't see why it needs to be

to be clear, I don't feel strongly about this, on mainnet that will not make a large difference, but it would be kinda nice if the builder preferences work alongside the proposer preferences more closely unless there is a good reason to have a different api design

[nflaig]

after thinking about this more and reading up on the current spec, although I haven't found that mentioned as an explicit argument during the initial spec design, I believe it makes sense due to signed auth requests since we allow pubkeys to sign over different auth, so there can be partial failures, however per the spec here this should return a 401 which is communicated back via http status code which doesn't really allow reporting partial failures. It seems favorable to send individual requests per-pubkey to me.

[nflaig]

generally looks good to me, just some comments and few self-reminders

[nflaig]

I guess another reason for SlotMismatch, or actually the more accurate reason would be if the validator is not proposer of the given slot?

also compared to ProposerPreferences, the BuilderPreferencesRequest doesn't contain the dependent_root so in case the builder and proposer have a different view of the chain, it's not trivial for them to validate it

but this might be fine for this message since it's not propagated via gossip and builders can decide to keep BuilderPreferencesRequest even if it doesn't match their current view of the shuffling

[nflaig]

to be clear on this, so the CL client is expected to always send this, but a builder may choose to ignore it, right?

[nflaig]

this whole section seems redundant to me, but fine with keeping it in case someone only looks at the .md file(s)

[nflaig]

all of these links should probably be fixed later by using a tag/release instead of master as reference

[nflaig]

since we are renaming these, I think it would be good to use less generic names, what about the following

• RequestAuth / SignedRequestAuth -> BuilderRequestAuth / SignedBuilderRequestAuth
• DOMAIN_REQUEST_AUTH -> DOMAIN_BUILDER_REQUEST_AUTH
• MAX_DATA_SIZE -> MAX_BUILDER_AUTH_DATA_SIZE
• get_/verify_request_auth_signature -> get_/verify_builder_request_auth_signature

when reading the builder-specs itself, the naming is fine, but in client code it's a bit too generic

cc @markolazic01 who brought this up

[nflaig]

The validator does not call builders directly.

I don't know why we have such details here in the spec, in case of vero/vouch they might directly talk to builders, in any case, this note isn't harmful, I am mostly questioning this whole section here, not just this specific line

[nflaig]

min_bid is missing here, might be something we can add in a follow-up if desired, or it's fine as is since it's a local selection rule, and not related to bid validity

[markolazic01]

LGTM, thanks for the renames @nflaig @JasonVranek