Block rewards API endpoint #12020
Block rewards API endpoint #12020
Conversation
# Conflicts: # beacon-chain/rpc/eth/beacon/blocks.go # testing/spectest/shared/altair/operations/sync_committee.go # testing/spectest/shared/bellatrix/operations/sync_committee.go # testing/spectest/shared/capella/operations/sync_committee.go
rkapka
added
Ready For Review
# Conflicts: # beacon-chain/core/altair/block.go # beacon-chain/core/altair/block_test.go # beacon-chain/rpc/eth/beacon/blocks_test.go
api/gateway/gateway.go
Outdated
| g := &Gateway{ | ||
| ctx: ctx, | ||
| cfg: &config{ | ||
| router: mux.NewRouter(), | ||
| router: router, |
There was a problem hiding this comment.
Could we make this an Option? Then after all for loop that applies the Options runs, we can check if the router field is nil, and if so give it a default value of mux.NewRouter. That way tests don't need to specify it when they don't want to customize the router.
There could be Options that want to rely on the router, which would make ordering tricky, but it's probably best for Options to not reach down into other fields anyway, since that creates fragile ordering assumptions. So Options like that would have to make sure to specify a router and do any route registration or other changes to the router before passing it as an Option to New.
beacon-chain/server/main.go
Outdated
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
|
|
||
| r := mux.NewRouter() | ||
| r.HandleFunc("/swagger/", gateway.SwaggerServer()) | ||
| r.HandleFunc("/healthz", healthzServer(gw)) | ||
| gw.SetRouter(r) |
There was a problem hiding this comment.
You are already setting the router to this value in gateway.New so I assume you do not need to use SetRouter here.
| return | ||
| } | ||
| attsReward := newBalance - oldBalance | ||
| oldBalance = newBalance |
There was a problem hiding this comment.
This old/new balance variable juggling is a little tricky, I wonder if it could be easier to read if you used a separate variable for the balance at each step, then at the end you could have:
Attestations: attBalance - initBalance, and so on
wdyt?
| Finalized bool `json:"finalized"` | ||
| } | ||
|
|
||
| type BlockRewards struct { |
There was a problem hiding this comment.
Why do we need this type - do we never need to read unmarshal a value into BlockRewards? Could this be a bit simpler if the handler used BlockRewardsJson and explicitly converted values to strings, so the value can just be passed to standard json.Marshal?
| "github.com/prysmaticlabs/prysm/v4/testing/require" | ||
| ) | ||
|
|
||
| func TestMarshalBlockRewards(t *testing.T) { |
There was a problem hiding this comment.
You could basically delete this entire file of tests if the handler can directly use BlockRewardsJson.
| } | ||
|
|
||
| // BeaconDbBlocker is an implementation of Blocker. It retrieves blocks from the beacon chain database. | ||
| type BeaconDbBlocker struct { |
There was a problem hiding this comment.
Nice job on this refactor, looks great!
| State(ctx context.Context, stateId []byte) (state.BeaconState, error) | ||
| StateRoot(ctx context.Context, stateId []byte) ([]byte, error) | ||
| // Stater is responsible for retrieving states. | ||
| type Stater interface { |
| blks := make([]interfaces.ReadOnlySignedBeaconBlock, count) | ||
| blkContainers := make([]*ethpbalpha.BeaconBlockContainer, count) | ||
| for i := primitives.Slot(0); i < count; i++ { | ||
| b := util.NewBeaconBlock() |
There was a problem hiding this comment.
Noticing that this makes phase0 blocks but your rewards code wants > phase0.
There was a problem hiding this comment.
this has nothing to do with rewards, it's used only in TestGetBlock
Co-authored-by: terencechain <terence@prysmaticlabs.com> Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: terencechain <terence@prysmaticlabs.com> Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
What type of PR is this?
Feature
What does this PR do? Why is it needed?
Implements https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Beacon/getBlockRewards
Notes:
blockfetcherequivalent ofstatefercherfor code reuse.api/gateway/gateway.gotobeacon-chain/server/main.go, so that the same router can be used for pure http endpoints and gprc-gateway endpoints .Which issues(s) does this PR fix?
Part of #11952