Builder: update to new Constraints API #241
Conversation
Co-authored-by: merklefruit <[email protected]>
thedevbirb
force-pushed
the
naman/feat/flashbot-builder
branch
from
1db9a3b to
b511df6
Compare
…e sse constraints auth Co-authored-by: merklefruit <[email protected]>
…preconfirmed and non-preconfirmed txs from same user
thedevbirb
force-pushed
the
naman/feat/flashbot-builder
branch
from
b511df6 to
a1d3397
Compare
chore(sidecar): update constraint api
|
Will not support the |
Co-authored-by: merklefruit <[email protected]>
…e sse constraints auth Co-authored-by: merklefruit <[email protected]>
…preconfirmed and non-preconfirmed txs from same user
…olt into naman/feat/flashbot-builder
There was a problem hiding this comment.
Great work @mempirate, the changes you made are clear to me. Just two minor questions
| // GetConstraintsDomain returns the constraints domain used to sign constraints-API related messages. | ||
| // | ||
| // The builder signing domain is built as follows: | ||
| // - We build a ForkData ssz container with the fork version and the genesis validators root. In the builder domain, this is an empty root. |
There was a problem hiding this comment.
What do you mean with "In the builder domain, this is an empty root"?
There was a problem hiding this comment.
Across all implementations, the builder signing domain is built with an empty genesis validators root. I don't know why. In contrast, the beacon signing domain always contains a valid gvr.
There was a problem hiding this comment.
If it can be helpful, here is more context on the decision:
ethereum/builder-specs#14 (comment)
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.
There was a problem hiding this comment.
The builder domain does use the fork_version field though. I think for us that also makes sense to ensure signatures are only valid for the fork they're running on. Wdyt?
This PR does the steps outlined in #209 to support the new constraints API
To do: ping Kubi when this is merged