Add Gloas bid selection, block production, and bid gossip processing (Gloas builder API 3/5) - #9805
Conversation
|
This pull request is part of a Mergify stack:
|
cfe5775 to
ea4ef14
Compare
Revision history
|
ea4ef14 to
46ee0fa
Compare
1ad80bd to
9b3cb1f
Compare
9b3cb1f to
e71bb39
Compare
e71bb39 to
c3501ac
Compare
c3501ac to
f95e1b9
Compare
f95e1b9 to
bd0b326
Compare
bd0b326 to
12a2374
Compare
12a2374 to
332a33e
Compare
|
This pull request has merge conflicts. Could you please resolve them @ethDreamer? 🙏 |
| // eligibility or coverage can go stale before production. Re-check against the production | ||
| // state and drop it if it would now fail `per_block_processing`, so a stale gossip bid | ||
| // can't outrank a viable candidate and sink the whole proposal. | ||
| match verify_bid_state_conditions(&gossip_bid.message, state, &self.spec) { |
There was a problem hiding this comment.
I believe this validation is still too weak. An exit in the parent payload can cause the builder to become inactive, in which case their bid is invalid.
I've opened an issue on the spec to ensure this gets added to the honest validator guide. We could definitely consider merging in the meantime.
In terms of a fix, we could apply the execution payload to the state early (seems simple), or try to infer the effect of the requests (like we do for voluntary exits).
There was a problem hiding this comment.
I think this is handled in the spec update that would prevent the bid from getting considered at all.
| fn rank_key(&self) -> (bool, bool, Uint256, bool) { | ||
| ( | ||
| self.overrides_builder(), | ||
| self.meets_min_bid(), |
There was a problem hiding this comment.
This fallback makes sense to avoid missing the block, but min_bid is currently documented as the minimum payment accepted for p2p bids and accepted from a direct builder. Should we document it as a preference rather than a hard minimum?
| /// post-state advanced to `proposal_slot` — and `parent_block_hash` / `parent_block_root` the | ||
| /// FULL/EMPTY parent the producer selected. | ||
| #[allow(clippy::too_many_arguments)] | ||
| pub fn verify_direct_bid<E: EthSpec>( |
There was a problem hiding this comment.
max_execution_payment currently only caps the value used for ranking, so a bid can still be accepted with a larger execution payment. Should we reject it during validation as required by the builder spec?
There was a problem hiding this comment.
I explicitly opted for these departures from the spec. I actually thought they had changed it but perhaps not..
https://discord.com/channels/595666850260713488/1539632415641964584/1540123275978342534
There was a problem hiding this comment.
I saw this too and actually like Mark's version better. Happy to consider the spec compliant change in a later PR, but gonna merge this one to unblock other stuff now.
cc73f1b to
6dafd2b
Compare
|
Let's get this in! |
|
@mergify queue |
Merge Queue Status
This pull request spent 30 minutes 51 seconds in the queue, with no time running CI. ReasonThe pull request conflicts with the base branch The pull request was held in the queue, conflicting with 1 pull request ahead of it. The conflict became a base branch one after #9970 merged. HintYou should update or rebase your pull request. If you want to requeue this pull request, you can post a Requeued — the merge queue status continues in this comment ↓. |
…(Gloas builder API 3/5) Third PR of the Gloas builder API stack: - beacon_chain: direct builder bid verification (spawned on the blocking executor), gossip-bid refinements, wei-domain bid selection (`BidCandidate`/`BidSource`), and Gloas block production that assembles local, gossip, and direct-builder candidates - network: process payload-bid and proposer-preference gossip, including the new `PayloadBidError` variants - client: construct the `Builders` service when the Gloas fork is scheduled and wire it into the beacon chain The HTTP API still serves `GET` produceBlockV4 at this point; the `POST` conversion and the `Eth-Builder-Url` round-trip land in the next PR. Change-Id: Ieccaee5db81bf871c19cef1b79fc48283be9dd51
6dafd2b to
74a86e2
Compare
|
@mergify queue |
Merge Queue Status
This pull request spent 30 minutes 54 seconds in the queue, including 28 minutes 47 seconds running CI. Required conditions to merge
|
Third PR of the Gloas builder API stack:
executor), gossip-bid refinements, wei-domain bid selection
(
BidCandidate/BidSource), and Gloas block production that assembleslocal, gossip, and direct-builder candidates
new
PayloadBidErrorvariantsBuildersservice when the Gloas fork is scheduledand wire it into the beacon chain
The HTTP API still serves
GETproduceBlockV4 at this point; thePOSTconversion and the
Eth-Builder-Urlround-trip land in the next PR.