search

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

Clarify signing routines #14

Closed ralexstokes closed 2 years ago

ralexstokes commented 2 years ago

The signing routines for the Builder API messages are under-specified.

The spec currently says to compute a message to sign over using compute_signing_root which takes the SSZ data and a Domain. Rather than specify a Domain, the spec says to simply use the DomainType 0x00000001.

There are two degrees of freedom to go from a DomainType to a Domain which we should decide how to handle:

1) Fork version 2) Genesis validators root

The basic question to answer is if we want to version messages across instances of the protocol or not. The degrees of freedom here are to allow for messages (like deposits) where the relevant information either is not known or we don't want to restrict a message to a given fork.

If we follow the current layout of the builder spec here, we should version (so use state.genesis_validators_root and the appropriate fork_version) the builder bid types but not version (so use "default" values as defined in the spec) the validator registration types. Moreover, this maps to the "deposits vs. other protocol messages" usage in the consensus specs.

lightclient commented 2 years ago

I have to say that I don't have as much experience with the Domain situation in the consensus spec, so all of my opinions here are weakly held.

With that said, it makes the most sense to me to compute the domain for an application using compute_domain(APP_DOMAIN_TYPE, fork_version=None, genesis_validators_root=None), because I don't think application should need any external data to compute / verify a signature.

ralexstokes commented 2 years ago

given that the builder bid messages aren't slashable at the moment it is less important to sign w/ the right data

if we want to bias towards implementation speed then I agree just using the default values compute_domain(DOMAIN_TYPE) is fine

we can always extend this signing method down the line if it becomes more important (e.g. gossiped messages)

mcdee commented 2 years ago

Note that deposits are versioned in the consensus specs, using the genesis validators root.

ralexstokes commented 2 years ago

deposits have a "default" genesis validators root bc before genesis you don't know what the root is (and need to allow deposits to get there!)

they do have the GENESIS_FORK_VERSION

I was suggesting we do something similar to deposits up above for the validator registrations

realbigsean commented 2 years ago

If we don't add something to give us chain context we are exposed to replaying messages cross-context. IMO we need genesis root and fork for both. If builders want to operate on multiple forks and on testnets with the same identity, they'd risk signing messages that would tank their reputation in another context. If a validator registers a fee recipient in one context it could be replayed in another (testnet vs mainnet, eth2 vs eth2 classic).