ethereum / builder-specs

Specification for the external block builders.
https://ethereum.github.io/builder-specs/
Creative Commons Zero v1.0 Universal
179 stars 61 forks source link

bid: change pubkey and include relay_pubkey #30

Closed metachris closed 2 years ago

metachris commented 2 years ago

Let's add both the builder and relay pk into the BuilderBid. This is useful for many cases, including not only relying on the relay pk pre-configured in mev-boost at startup.

metachris commented 2 years ago

wip, this is meant to facilitate discussions around https://github.com/ethereum/builder-specs/issues/23

ralexstokes commented 2 years ago

in light of work on this: https://www.notion.so/Relay-API-Spec-5fb0819366954962bc02e81cb33840f5

I think we may be able to shift some of these responsibilities to transparency functions of both relays and builders, rather than add more information to the BuilderBid.

this would also lose some of its relevance I think if we move to a "private" bidding model where only the proposer sees the available bids -- if we are in the private setting then it is on the proposer and the proposer alone to raise any alarm if there is something off w/ the bid

I'd like to work on hardening the Relay API a bit more before it becomes clear that we need to change anything about the bid here. If we leave this alone then we do take more work off the "critical path" of client implementation, which I think we should generally aim for

metachris commented 2 years ago

alright, let's close it for now and revisit if needed.

lightclient commented 2 years ago

I think a source of confusion here is that the spec is overloading the builder actor in non-obvious ways. What I was hoping to achieve was the minimal interaction between a validator and someone with a block. In the 1:1 view where a validator connects directly to a builder, this terminology makes sense. However, in 1:N with mev-boost and relays it gets more confusing.

My idea was that because the relay is the last point of contact for the validator, it absolves the complexity of the relay<>builder relationship. To the validator, a relay just looks like a builder. Therefore, it would be the relay's pubkey and signature in the "builder" bid.

I think we should think very hard about a change like this and ask why do we need both keys? It feels like a leak of the validator<>builder abstraction (using builder as a term from the validators perspective as noted above). It very well could be a hard requirement, but I would like to see a more convincing argument as to why we need the validator have the original builder's signature.

Also, forging the builder's signature is trivial if a relay and builder decide to collude. Right now, I don't think relay<>builder collusion affects the builder spec because it has black boxed that whole relationship. Once we add this additional field, I think that brings a whole host of new things into scope.

I agree with Alex that we should explore what's possible with the Relay API first. If the ultimate goal is to increase transparency and accountability, I think we can build a strong solution by just querying relays and/or builders. If we find that we need validators to weigh in in some way, we should probably revisit this idea.

metachris commented 2 years ago

Thanks for the additional comments. One thing that seems confusing is the description of BuilderBid.pubkey: "BLS public key of builder"