Block headers returning incorrect public key.

[mcdee]

Describe the bug

The spec at https://ethereum.github.io/builder-specs/#/Builder/getHeader states that the pubkey field in the message struct is the public key of the builder, however the public key of the relay is being returned in this field.

Steps to reproduce

1. Request a header with /eth/v1/builder/header/{slot}/{parent_hash}/{pubkey}
2. Check the pubkey field of the response

Expected behavior

The pubkey field should be the public key of the builder, not the relay.

[metachris]

I think there was a reason why the relay key is there, I'll try to look it up and report back. Not sure if it might break something.

From an Ethereum perspective, the term "builder" in the builder-specs refers to the Builder Network in general. As in, a block coming from the builder network. The source of the block is the relay, and it seems generally okay for the relay pubkey to be in there.

What's the benefit you see on having the builder pubkey in there?

[mcdee]

The source of the block is the relay, and it seems generally okay for the relay pubkey to be in there.

The pubkey supplied by the data APIs for the call is that of the builder, not the relay. The relay public key is already provided in its definition URL, for example at the top of https://boost-relay.flashbots.net/ I don't think that "builder" is particularly ambiguous in the spec.

What's the benefit you see on having the builder pubkey in there?

It allows us to differentiate between bids from certain builders. We have seen issues where builders provided an incorrect bid value and want to filter them out, for example.

There is also an issue where, with the public key being in the response, clients are using that public key to verify the signature rather than the one in the URL. This renders the whole idea of signature verification useless, as any key could be used to sign the bid as long as the attacking public key is in place, which opens up the validators to a MiTM attack.

[ralexstokes]

+1 to metachris

it is confusing but the idea is that the builder specs refers to this "abstract" builder in some places

given that this API refers to communication between the proposer and relay (via mev-boost), the idea is that the pubkey in the response is that of the relay as the response here is signed by the relay (and follows the pattern from the consensus-specs that the signature over a message includes the public key as the last field of the message)

relays should expose the data APIs which provide bid data and can be used to correlate a particular bid with a particular builder

one reason to have bids signed by relays is to lay the groundwork for some kind of "relay fault proof", i.e. in the event of a relay failure clients could conceivably include the signed response from this getHeader call as evidence that a relay failed to release a block (for example)

[mcdee]

given that this API refers to communication between the proposer and relay (via mev-boost), the idea is that the pubkey in the response is that of the relay as the response here is signed by the relay (and follows the pattern from the consensus-specs that the signature over a message includes the public key as the last field of the message)

I'm not sure to what this is referring. As far as I'm aware, the main messages signed by validators within the consensus specs (attestations, proposals, sync committee messages, voluntary exit, ...) don't include the validator public key.

And as mentioned above, the public key should not be propagated in the same message as the signature. Verifying that signature s relates to public key p provides no useful information in itself unless there is some other knowledge about p. My impression was that this is provided in the relay URL (e.g. the URL at the top of https://boost-relay.flashbots.net/).

one reason to have bids signed by relays is to lay the groundwork for some kind of "relay fault proof", i.e. in the event of a relay failure clients could conceivably include the signed response from this getHeader call as evidence that a relay failed to release a block (for example)

I'm not arguing against this, I agree that bids should be signed by relays. What I'm saying is that the public key in the bid should not be the one used to verify the signature, because as it stands a relay could sign every single request with a different private key if it felt like it and as long as it passed the public key derived from said private key in each header the signature check would pass.

[ralexstokes]

one example is SignedVoluntaryExit, VoluntaryExit has the validator index instead (just to save space vs writing the entire public key) but same idea

--

yes, I agree the public key in the message should be verified against the expected value (e.g. from the relay public key), secure implementations of mev-boost in this context would check this

--

right, so I guess you'd like to just remove the public key from the message, or instead use it for the builder's public key

I don't think its urgent as you can still use the relay data APIs to correlate the end-to-end flow, but the specs could be changed to include the builder's public key