Deneb: Produce Block V3 - adding consensus block value #12948
Conversation
james-prysm
added
API
| // We want to run several block processing functions that update the proposer's balance. | ||
| // This will allow us to calculate proposer rewards for each operation (atts, slashings etc). | ||
| // To do this, we replay the state up to the block's slot, but before processing the block. |
There was a problem hiding this comment.
This comment should be placed before the call to this function in BlockRewards because it is accurate only in that function context.
There was a problem hiding this comment.
I think the produceblockv3 needs to do the same , it should be accurate to the context of the block passed in as an argument right?
| type BlockRewardsFetcher interface { | ||
| GetBlockRewardsData(context.Context, interfaces.ReadOnlySignedBeaconBlock) (*BlockRewards, *http2.DefaultErrorJson) | ||
| GetStateForRewards(ctx context.Context, blk interfaces.ReadOnlySignedBeaconBlock) (state.BeaconState, *http2.DefaultErrorJson) | ||
| } |
There was a problem hiding this comment.
nitpick: one function has named params, the other one doesn't
| http2 "github.com/prysmaticlabs/prysm/v4/network/http" | ||
| ) | ||
|
|
||
| // BlockRewardsFetcher retrieves the Consensus Payload ( aka block rewards) of the passed in block |
There was a problem hiding this comment.
I suggest making this comment more generic because GetStateForRewards doesn't retrieve the consensus payload
| switch b := i.(type) { | ||
| case *eth.GenericBeaconBlock_Phase0: | ||
| //ignore for phase0 | ||
| return nil, nil |
There was a problem hiding this comment.
Instead of returning nil here and then checking for nil outside, I think it makes more sense to support all types here and skip the function call when we have a phase0 block. This will make the function more usable and someone reading produceBlockV3 will understand the intent better, without having to navigate here to figure out why the result can be nil.
There was a problem hiding this comment.
i changed it to its own function and made it more apparent.
| } | ||
|
|
||
| // GetBlockRewardsData returns the BlockRewards Object which is used for the BlockRewardsResponse and ProduceBlockV3 | ||
| func (rs *BlockRewardService) GetBlockRewardsData(ctx context.Context, blk interfaces.ReadOnlySignedBeaconBlock) (*BlockRewards, *http2.DefaultErrorJson) { |
There was a problem hiding this comment.
I think we should place the interface, the service struct and its methods in a new file.
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
james-prysm
changed the title
What type of PR is this?
Feature
What does this PR do? Why is it needed?
This PR adds the consensus block value to the produce block v3
/eth/v3/validator/blocks/{slot}which is the same value as thetotalfield from the/eth/v1/beacon/rewards/blocks/{block_id}endpoint. To do this I have introduced a new interface and service to isolate testing on reward responses and the produce block v3 setup.Which issues(s) does this PR fix?
Fixes # ethereum/beacon-APIs#358 for prysm