Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Rpc: introduce get_inflation_reward rpc call #16278

Merged
merged 22 commits into from
Apr 7, 2021

Conversation

oJshua
Copy link
Contributor

@oJshua oJshua commented Mar 31, 2021

Problem

There is currently no way in the RPC to get the reward for a given epoch.

Summary of Changes

  • introduce rpc call for fetching reward for a single epoch
  • if epoch is omitted, the most recent reward is fetched

Fixes #16077

@oJshua oJshua force-pushed the rpc/getinflationreward branch 4 times, most recently from d08d592 to a574cd9 Compare March 31, 2021 23:36
@oJshua oJshua changed the title RPC: introduce get_inflation_reward rpc call Rpc: introduce get_inflation_reward rpc call Mar 31, 2021
@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

Merging #16278 (2858528) into master (92f4018) will decrease coverage by 0.0%.
The diff coverage is 0.0%.

@@            Coverage Diff            @@
##           master   #16278     +/-   ##
=========================================
- Coverage    80.1%    80.0%   -0.1%     
=========================================
  Files         413      413             
  Lines      110490   110564     +74     
=========================================
- Hits        88558    88553      -5     
- Misses      21932    22011     +79     

core/src/rpc.rs Show resolved Hide resolved
core/src/rpc.rs Outdated Show resolved Hide resolved
client/src/rpc_response.rs Outdated Show resolved Hide resolved
core/src/rpc.rs Outdated Show resolved Hide resolved
core/src/rpc.rs Outdated Show resolved Hide resolved
core/src/rpc.rs Outdated Show resolved Hide resolved
core/src/rpc.rs Outdated Show resolved Hide resolved
core/src/rpc.rs Outdated Show resolved Hide resolved
@oJshua oJshua force-pushed the rpc/getinflationreward branch 2 times, most recently from f178076 to 3ea4247 Compare April 1, 2021 21:12
@oJshua
Copy link
Contributor Author

oJshua commented Apr 1, 2021

@CriesofCarrots was waiting for this to pass CI, but it might be good for another glance. Thanks much!

@oJshua oJshua requested a review from CriesofCarrots April 1, 2021 21:40
core/src/rpc.rs Outdated Show resolved Hide resolved
core/src/rpc.rs Show resolved Hide resolved
CriesofCarrots
CriesofCarrots previously approved these changes Apr 1, 2021
Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit, otherwise lgtm! Thanks for all the polish

@mergify mergify bot dismissed CriesofCarrots’s stale review April 2, 2021 15:54

Pull request has been modified.

@mvines
Copy link
Contributor

mvines commented Apr 2, 2021

Can you please also add a section in the JSON RPC API docs, as well as support in RpcClient for the new method

@oJshua
Copy link
Contributor Author

oJshua commented Apr 2, 2021

@mvines on it.

@mvines
Copy link
Contributor

mvines commented Apr 2, 2021

Oh as a follow-up PR, can you please use this new method in the cli as well. To avoid backwards compatibility issues for now, the cli changes could only go into master and 1.6

@CriesofCarrots
Copy link
Contributor

Oh as a follow-up PR, can you please use this new method in the cli as well. To avoid backwards compatibility issues for now, the cli changes could only go into master and 1.6

What do you think about limiting the cli to some number of previous epochs? We could add a separate command or arg to include complete rewards history. But it would be rather nice if solana stake-account could return quicker for users who just want to check the current status of an account.

@mvines
Copy link
Contributor

mvines commented Apr 2, 2021

Oh as a follow-up PR, can you please use this new method in the cli as well. To avoid backwards compatibility issues for now, the cli changes could only go into master and 1.6

What do you think about limiting the cli to some number of previous epochs? We could add a separate command or arg to include complete rewards history. But it would be rather nice if solana stake-account could return quicker for users who just want to check the current status of an account.

Yes. I hate so much how slow it is. It worked great for the first couple epochs after pico-inflation was enabled though.

How about if on solana stake-account and solana vote-account, there

  1. were no rewards displayed by default
  2. --rewards by default shows the the latest epoch with an optional EPOCH arg to --rewards to display a previous epoch. No ability

Alternatively move rewards to solana inflation-rewards? solana inflation-rewards --epoch X address1 address2 ...

core/src/rpc.rs Outdated Show resolved Hide resolved
@CriesofCarrots
Copy link
Contributor

How about if on solana stake-account and solana vote-account, there

1. were no rewards displayed by default

2. `--rewards` by default shows the the latest epoch with an optional `EPOCH` arg to `--rewards` to display a previous epoch.  No ability

Alternatively move rewards to solana inflation-rewards? solana inflation-rewards --epoch X address1 address2 ...

How about both? Too much? I can see different use-cases for each.

@mvines
Copy link
Contributor

mvines commented Apr 2, 2021

How about both? Too much? I can see different use-cases for each.

Yeah I think probably both too

@oJshua oJshua requested a review from CriesofCarrots April 2, 2021 18:44
client/src/rpc_response.rs Outdated Show resolved Hide resolved
@oJshua oJshua force-pushed the rpc/getinflationreward branch from a8e4b9b to 503f7b7 Compare April 6, 2021 16:39
@oJshua oJshua force-pushed the rpc/getinflationreward branch from a10ca8e to 8cdd0b3 Compare April 6, 2021 20:43
client/src/rpc_client.rs Outdated Show resolved Hide resolved
core/src/rpc.rs Outdated Show resolved Hide resolved
core/src/rpc.rs Outdated Show resolved Hide resolved
docs/src/developing/clients/jsonrpc-api.md Outdated Show resolved Hide resolved
@oJshua oJshua merged commit e501fa5 into solana-labs:master Apr 7, 2021
mergify bot pushed a commit that referenced this pull request Apr 7, 2021
* feat: introduce get_inflation_reward rpc call

* fix: style suggestions

* fix: more style changes and match how other rpc functions are defined

* feat: get reward for a single epoch

* feat: default to the most recent epoch

* fix: don't factor out get_confirmed_block

* style: introduce from impl for RpcEncodingConfigWrapper

* style: bring commitment into variable

* feat: support multiple pubkeys for get_inflation_reward

* feat: add get_inflation_reward to rpc client

* feat: return rewards in order

* fix: rename pubkeys to addresses

* docs: introduce jsonrpc docs for get_inflation_reward

* style: early return in map (not sure which is more idiomatic)

* fix: call the rpc client function args addresses as well

* fix: style

* fix: filter out only addresses we care about

* style: make this more idiomatic

* fix: change rpc client epoch to optional and include some docs edits

* feat: filter out rent rewards in get_inflation_reward

* feat: add option epoch config param to get_inflation_reward

* feat: rpc client get_inflation_reward takes epoch instead of config and some filter staking and voting rewards

(cherry picked from commit e501fa5)

# Conflicts:
#	client/src/rpc_client.rs
#	core/src/rpc.rs
mergify bot pushed a commit that referenced this pull request Apr 7, 2021
* feat: introduce get_inflation_reward rpc call

* fix: style suggestions

* fix: more style changes and match how other rpc functions are defined

* feat: get reward for a single epoch

* feat: default to the most recent epoch

* fix: don't factor out get_confirmed_block

* style: introduce from impl for RpcEncodingConfigWrapper

* style: bring commitment into variable

* feat: support multiple pubkeys for get_inflation_reward

* feat: add get_inflation_reward to rpc client

* feat: return rewards in order

* fix: rename pubkeys to addresses

* docs: introduce jsonrpc docs for get_inflation_reward

* style: early return in map (not sure which is more idiomatic)

* fix: call the rpc client function args addresses as well

* fix: style

* fix: filter out only addresses we care about

* style: make this more idiomatic

* fix: change rpc client epoch to optional and include some docs edits

* feat: filter out rent rewards in get_inflation_reward

* feat: add option epoch config param to get_inflation_reward

* feat: rpc client get_inflation_reward takes epoch instead of config and some filter staking and voting rewards

(cherry picked from commit e501fa5)
mergify bot added a commit that referenced this pull request Apr 7, 2021
* feat: introduce get_inflation_reward rpc call

* fix: style suggestions

* fix: more style changes and match how other rpc functions are defined

* feat: get reward for a single epoch

* feat: default to the most recent epoch

* fix: don't factor out get_confirmed_block

* style: introduce from impl for RpcEncodingConfigWrapper

* style: bring commitment into variable

* feat: support multiple pubkeys for get_inflation_reward

* feat: add get_inflation_reward to rpc client

* feat: return rewards in order

* fix: rename pubkeys to addresses

* docs: introduce jsonrpc docs for get_inflation_reward

* style: early return in map (not sure which is more idiomatic)

* fix: call the rpc client function args addresses as well

* fix: style

* fix: filter out only addresses we care about

* style: make this more idiomatic

* fix: change rpc client epoch to optional and include some docs edits

* feat: filter out rent rewards in get_inflation_reward

* feat: add option epoch config param to get_inflation_reward

* feat: rpc client get_inflation_reward takes epoch instead of config and some filter staking and voting rewards

(cherry picked from commit e501fa5)

Co-authored-by: Josh <josh.hundley@gmail.com>
mergify bot added a commit that referenced this pull request Apr 7, 2021
* Rpc: introduce get_inflation_reward rpc call (#16278)

* feat: introduce get_inflation_reward rpc call

* fix: style suggestions

* fix: more style changes and match how other rpc functions are defined

* feat: get reward for a single epoch

* feat: default to the most recent epoch

* fix: don't factor out get_confirmed_block

* style: introduce from impl for RpcEncodingConfigWrapper

* style: bring commitment into variable

* feat: support multiple pubkeys for get_inflation_reward

* feat: add get_inflation_reward to rpc client

* feat: return rewards in order

* fix: rename pubkeys to addresses

* docs: introduce jsonrpc docs for get_inflation_reward

* style: early return in map (not sure which is more idiomatic)

* fix: call the rpc client function args addresses as well

* fix: style

* fix: filter out only addresses we care about

* style: make this more idiomatic

* fix: change rpc client epoch to optional and include some docs edits

* feat: filter out rent rewards in get_inflation_reward

* feat: add option epoch config param to get_inflation_reward

* feat: rpc client get_inflation_reward takes epoch instead of config and some filter staking and voting rewards

(cherry picked from commit e501fa5)

# Conflicts:
#	client/src/rpc_client.rs
#	core/src/rpc.rs

* fix: resolve cherry-pick conflicts

* fix: change bool_to_option filter_map to filter + map

* style: use filter_map in place of filter + map

Co-authored-by: Josh <josh.hundley@gmail.com>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rpc: introduce getInflationReward RPC call for total rewards
3 participants