feat(builder): add bid_included event - #9875
Conversation
| /** The node has received a block (from P2P or API) that passes validation rules of the `beacon_block` topic */ | ||
| blockGossip = "block_gossip", | ||
| /** The node has received a gloas block (from P2P or API) that includes an execution payload bid */ | ||
| bidIncluded = "bid_included", |
There was a problem hiding this comment.
was wondering, do we have a better name for this? I haven't really spent to much thought on it yet
There was a problem hiding this comment.
Maybe block_bid?
There was a problem hiding this comment.
having hard time on this one to come up with a good name 😅
There was a problem hiding this comment.
maybe we can ask the keeper to give us some ideas
| // TODO: What happens if this handler throws? Does it break the other chain.emitter listeners? | ||
|
|
||
| onEvent({type: topic, message: data}); | ||
| onEvent({type: topic, message: data} as routes.events.BeaconEvent); |
There was a problem hiding this comment.
There's an error which shows up without explicit typing:
packages/beacon-node check-types$ tsc
│ src/api/impl/events/index.ts(17,19): error TS2345: Argument of type '{ type: routes.events.Even…
│ Type '{ type: routes.events.EventType; message: any; }' is not assignable to type '{ type: Ev…
│ Types of property 'type' are incompatible.
│ Type 'EventType' is not assignable to type 'EventType.fastConfirmation'.
│ Found 1 error in src/api/impl/events/index.ts:17
└─ Failed in 2s at /Users/lqzic/lodestar-my/lodestar/packages/beacon-node
Though this has nothing to do with fastConfirmation, it is the last event in the list so it gets 'called out'.
There was a problem hiding this comment.
but this is strange, why does this new event trigger this type error?
There was a problem hiding this comment.
I am not completely sure, will dig more
| [EventType.bidIncluded]: { | ||
| slot: Slot; | ||
| block: RootHex; | ||
| blockHash: RootHex; | ||
| builderIndex: BuilderIndex; | ||
| }; |
There was a problem hiding this comment.
would be simpler to just emit the signed bid object?
There was a problem hiding this comment.
yes, I can switch to that, I was following the discussion on what it should look like
switching
| slot: 10, | ||
| block: "0x9a2fefd2fdb57f74993c7780ea5b9030d2897b615b89f808011ca5aebed54eaf", | ||
| signedBid: ssz.gloas.SignedExecutionPayloadBid.fromJson({ | ||
| message: { | ||
| parent_block_hash: "0x9a2fefd2fdb57f74993c7780ea5b9030d2897b615b89f808011ca5aebed54eaf", | ||
| parent_block_root: "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", |
There was a problem hiding this comment.
so the slot is duplicated now, need to think about it a bit more what we wanna emit in the event, if we emit a spec container objects, the event itself should also have a version/data container schema
There was a problem hiding this comment.
can I introduce versioning then and remove the slot?
There was a problem hiding this comment.
I am wondering if we should keep the block or not, it would be nice to have a clean spec object emitted
There was a problem hiding this comment.
can we just introduce another location where the execution payload bid is emitted then? if we remove the 'block' then 'bid_included' and 'executionPayloadBid' have the same payload. It makes sense to have a name difference there but maybe it seems weird when payloads are the same.
There was a problem hiding this comment.
but essentially we wanna emit it for every block we import successfully if it has a bid from a builder included (ie. exclude self-build blocks), but yes more or less it's just emitted together with the block event
## Motivation Split out from the block / builder event PRs (#9854, #9875, #9876) as a standalone change, as suggested by @markolazic01. The `eventstream` handler in `getEventsApi` forwards every emitter event through a single `onEvent({type: topic, message: data})` call, where `topic` is the full `EventType` union and `data` (and therefore `message`) is `any`. That object literal only type-checks via TypeScript's discriminated-union distribution path (`typeRelatedToDiscriminatedType`), which bails out once the number of source discriminant combinations exceeds 25. `EventType` currently has exactly **25** members, so it compiles today. Adding a **26th** event tips it over the cap and fails with: ``` TS2345: Argument of type '{ type: EventType; message: any; }' is not assignable to parameter of type 'BeaconEvent'. Types of property 'type' are incompatible. Type 'EventType' is not assignable to type 'EventType.<lastMember>'. ``` which is why each new-event PR currently has to add this cast. Landing it once here unblocks those PRs without each carrying the change. ## Description Assert `routes.events.BeaconEvent` at the `onEvent` call. The cast only makes explicit the type erasure that already exists at this `chain.emitter` boundary — `message` is `any`, so the topic/message pairing was never verified by the compiler regardless. The only cast-free alternative would be to construct the event per-topic instead of funneling every topic through one union-typed `onEvent({type, message})` call, which is a larger refactor not worth it here. **No runtime or current-compile behavior change** on `unstable` (25 `EventType` members). --- 🤖 Generated with AI assistance --------- Co-authored-by: lodekeeper <lodekeeper@users.noreply.github.com>
| executionOptimistic: blockSummary != null && isOptimisticBlock(blockSummary), | ||
| }); | ||
| } | ||
| if (this.emitter.listenerCount(routes.events.EventType.bidIncluded) && isGloasBeaconBlock(block.message)) { |
There was a problem hiding this comment.
Exclusion of self built blocks is missing here, reminder if we decide to go with this version.
|
Final shape: block(rootHex) + SignedExecutionPayloadBid |
Motivation
Introduce a new
bid_includedevent which carriesblock_hashandbuilder_indexbid fields (PoC).Description
Introduces a new event, opposing the implementation in #9854 where the
blockevent is extended.Addition of the event to EventType surfaced a type mismatch bug which was fixed with explicit typing here:
https://github.com/markolazic01/lodestar/blob/db528d682aec8480a6a81e12988a628d1ca004d5/packages/beacon-node/src/api/impl/events/index.ts#L17
Update: switched from
block_hashandbuilder_indextosigned_bid(SignedExecutionPayloadBid)AI Assistance Disclosure
Used Claude to inspect the typing bug and audit the changes.