Prysm V4: SSE api adding payload_attributes#12102
Conversation
| Version: version.String(headState.Version()), | ||
| Data: ðpb.EventPayloadAttributeV1_BasePayloadAttribute{ | ||
| ProposerIndex: headBlock.Block().ProposerIndex(), | ||
| ProposalSlot: headState.Slot(), |
There was a problem hiding this comment.
I've seen this as s.CurrentSlot +1 on others but since the headState is saved in this case i think it's headState.Slot()
There was a problem hiding this comment.
headState.Slot() feels a little useless to me. I think it should be +1 but best to check with others
EDIT: this is wrong
|
|
||
| nextSlot := s.CurrentSlot() + 1 // Cache payload ID for next slot proposer. | ||
| hasAttr, attr, proposerId := s.getPayloadAttribute(ctx, arg.headState, nextSlot) | ||
|
|
There was a problem hiding this comment.
Why remove these lines?
There was a problem hiding this comment.
will add it back in, was reverting some changes removed extra lines
| } | ||
| } | ||
|
|
||
| // notifyPayloadAttributesStream on successful FCU notify the event stream that a payload was sent |
There was a problem hiding this comment.
Probably want to update this comment
| return streamData(stream, PayloadAttributesTopic, ðpb.EventPayloadAttributeV1{ | ||
| Version: version.String(headState.Version()), | ||
| Data: ðpb.EventPayloadAttributeV1_BasePayloadAttribute{ | ||
| ProposerIndex: headBlock.Block().ProposerIndex(), |
There was a problem hiding this comment.
I think this should be the proposer index of headState.slot + 1. We want to advance the state by 1 and use BeaconProposerIndex
There was a problem hiding this comment.
I checked the API again. This is what it says
- `proposal_slot`: the slot at which a block using these payload attributes may be built.
Based on that, I'm taking it as headState.slot. Your current implementation is right
| Version: version.String(headState.Version()), | ||
| Data: ðpb.EventPayloadAttributeV1_BasePayloadAttribute{ | ||
| ProposerIndex: headBlock.Block().ProposerIndex(), | ||
| ProposalSlot: headState.Slot(), |
There was a problem hiding this comment.
headState.Slot() feels a little useless to me. I think it should be +1 but best to check with others
EDIT: this is wrong
terencechain
left a comment
There was a problem hiding this comment.
Lgtm! Nice work @james-prysm !
What type of PR is this?
Feature
What does this PR do? Why is it needed?
This PR adds the new
payload_attributestopic to the EventStream, There are 2 versions of the payload at this time based on hardfork ( "bellatrix" and "capella"). The endpoint is intended to be used by builders and relays especially to retrieve withdrawal information. The event is triggered by new head events which should happen on every slot that has a canonical block. The results are not limited to validators you own but will show the information of every block proposer added as a new head.Which issues(s) does this PR fix?
related #11841
Implements ethereum/beacon-APIs#305
Fixes ethereum/beacon-APIs#244