-
Notifications
You must be signed in to change notification settings - Fork 141
Add SSE subscription to builder #53
Conversation
builder/service.go
Outdated
|
|
||
| beaconClient := NewBeaconClient(cfg.BeaconEndpoint, cfg.SlotsInEpoch, cfg.SecondsInSlot) | ||
| var beaconClient IBeaconClient | ||
| if cfg.BeaconEndpoint != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to run the builder service without a beacon configured?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that would break backwards compatibility. This allows the optionality to use the our old method of prysm block building during testing by not subscribing to the SSE endpoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why though?
The old code seems to always construct a beacon client
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the old code would construct the beacon client with an empty url and only fail if the local relay is used.
builder/beacon_client.go
Outdated
|
|
||
| // SubscribeToPayloadAttributesEvents subscribes to payload attributes events to validate fields such as prevrandao and withdrawals | ||
| func (b *BeaconClient) SubscribeToPayloadAttributesEvents(payloadAttrC chan types.BuilderPayloadAttributes) { | ||
| payloadAttributesResp := &struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This got to be put somewhere as a regular structure :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will add in attestant types!
builder/beacon_client.go
Outdated
|
|
||
| for { | ||
| client := sse.NewClient(eventsURL) | ||
| err := client.SubscribeRaw(func(msg *sse.Event) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better if the callback was a standalone function?
* Add handling multiple beacon clients * Initialize stop channel in builder.Builder * fix withdrawals array pointer * Build on a single head (#59) * Build on a single head * Forcibly stop building process for old sse events --------- Co-authored-by: avalonche <[email protected]>
| if b.slotCtxCancel != nil { | ||
| b.slotCtxCancel() | ||
| } | ||
| // Forcibly cancel previous building job, build on top of reorgable blocks as this is the behaviour relays expect. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please explain why "buidling on multiple tips" is no longer necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, it just doesn't work with how relays are set up. This will come back shortly after Capella.
* Add SSE subscription to builder * withdrawals marshalling * add stop channel * pr comments * Add handling multiple beacon clients (#57) * Add handling multiple beacon clients * Initialize stop channel in builder.Builder * fix withdrawals array pointer * Build on a single head (#59) * Build on a single head * Forcibly stop building process for old sse events --------- Co-authored-by: avalonche <[email protected]> * linting --------- Co-authored-by: Mateusz Morusiewicz <[email protected]>
📝 Summary
ethereum/beacon-APIs#305 Beacon API has introduced SSE subscriptions for payload attributes such that block building can be triggered from this event stream instead of a custom block trigger in the flashbots prysm fork.
📚 References
CONTRIBUTING.md