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

Rpc: Add getCirculatingSupply endpoint #9934

Closed

Conversation

CriesofCarrots
Copy link
Contributor

@CriesofCarrots CriesofCarrots commented May 8, 2020

Problem

Users want to know the circulating supply in the cluster; even though we know this approximate value, currently we have no way to calculate it.

Summary of Changes

  • Add update_supply module to calculate circulating and non-circulating supply on epoch boundaries
  • Plumb getCirculatingSupply into rpc, which returns circulating and non-circulating supplies, and the list of non-circulating accounts. Note: these values are static, even though total supply may fall across an epoch, since supply is updated once at the beginning of the epoch.
  • On mainnet-beta, returns circulating supply: ◎19,681,955.661301762

Fixes #9433
Closes #9541

@CriesofCarrots
Copy link
Contributor Author

CriesofCarrots commented May 8, 2020

@mvines , I wasn't able to put the non-circulating accounts list in the genesis crate and also add the computation to get_entered_epoch_callback() in genesis_programs, due to circular dependency. Alternatives to packing it all in genesis_programs (as I did) would be putting it in a separate crate or something lower-level (like runtime). Thoughts?

Also, should I change the endpoint name to getSupply since it also returns the non-circulating balance?

@CriesofCarrots
Copy link
Contributor Author

One other note: currently this code will only populate Bank::supply on the next epoch boundary. If we want it to populate sooner, we could plug in a particular slot for mainnet-beta/testnet, which should keep the data consistent across nodes.

@codecov
Copy link

codecov bot commented May 8, 2020

Codecov Report

Merging #9934 into master will decrease coverage by 0.0%.
The diff coverage is 86.5%.

@@           Coverage Diff            @@
##           master   #9934     +/-   ##
========================================
- Coverage    80.4%   80.4%   -0.1%     
========================================
  Files         283     285      +2     
  Lines       65951   66026     +75     
========================================
+ Hits        53073   53120     +47     
- Misses      12878   12906     +28     

@mvines
Copy link
Contributor

mvines commented May 8, 2020

When a node boots from a snapshot, it looks like it won't have a valid bank supply until the next epoch? Call update_supply() after snapshot deserialization in core/src/validator.rs?

@mvines
Copy link
Contributor

mvines commented May 8, 2020

@mvines , I wasn't able to put the non-circulating accounts list in the genesis crate and also add the computation to get_entered_epoch_callback() in genesis_programs, due to circular dependency. Alternatives to packing it all in genesis_programs (as I did) would be putting it in a separate crate or something lower-level (like runtime). Thoughts?

The current location in genesis_programs seems not bad at all

Also, should I change the endpoint name to getSupply since it also returns the non-circulating balance?

Yeah I like it, which makes me want to also deprecate getTotalSupply and add a new total field in getSupply as well

@@ -0,0 +1,30 @@
pub(crate) const NON_CIRCULATING_ACCOUNTS: [&str; 25] = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Some brief comments in this file that mark these accounts as mainnet-beta accounts would be nice.

"GK2zqSsXLA2rwVZk347RYhh6jJpRsCA69FjLW93ZGi3B",
"HCV5dGFJXRrJ3jhDYA4DCeb9TEDTwGGYXtT3wHksu2Zr",
"25odAafVXnd63L6Hq5Cx6xGmhKqkhE2y6UrLVuqUfWZj",
"14FUT96s9swbmH7ZjpDvfEDywnAYy9zaNhv4xvezySGu",
Copy link
Contributor

Choose a reason for hiding this comment

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

Try solana_sdk::declare_id!() to produce a Pubkey at compile time

Copy link
Contributor Author

@CriesofCarrots CriesofCarrots May 8, 2020

Choose a reason for hiding this comment

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

Hmm, can you point me at an example that would work for this case?
I think I would need a different macro that can handle multiple keys to produce an non_circulating_accounts::ids() method...?

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, it won't work out of the box. A pubkey!("25odAafVXnd63L6Hq5Cx6xGmhKqkhE2y6UrLVuqUfWZj") macro that produces a Pubkey struct at compile time?

@CriesofCarrots
Copy link
Contributor Author

When a node boots from a snapshot, it looks like it won't have a valid bank supply until the next epoch? Call update_supply() after snapshot deserialization in core/src/validator.rs?

True. How to handle the fact that nodes booting from snapshots at different slots will have different circulating-supply values? Perhaps it's enough to add a slot_when_calculated field to the Supply struct?

@mvines
Copy link
Contributor

mvines commented May 8, 2020

True. How to handle the fact that nodes booting from snapshots at different slots will have different circulating-supply values? Perhaps it's enough to add a slot_when_calculated field to the Supply struct?

Sure, that works. effective: Slot, updated: Slot, _.

@CriesofCarrots
Copy link
Contributor Author

Yeah I like it, which makes me want to also deprecate getTotalSupply and add a new total field in getSupply as well

Right now, total supply drops constantly as fees/rent are burned. If we add total supply to this response, it seems like it probably makes the most sense to calculate/cache it once at the beginning of the epoch and hold it constant like the other values. Agree?

@mvines
Copy link
Contributor

mvines commented May 8, 2020

Yeah I like it, which makes me want to also deprecate getTotalSupply and add a new total field in getSupply as well

Right now, total supply drops constantly as fees/rent are burned. If we add total supply to this response, it seems like it probably makes the most sense to calculate/cache it once at the beginning of the epoch and hold it constant like the other values. Agree?

Oh wait. What if we just removed all the bank-level integration, and calculated everything in the getSupply RPC implementation? That would give us the current supply for all fields and keep runtime/ out of it entirely

@CriesofCarrots
Copy link
Contributor Author

Oh wait. What if we just removed all the bank-level integration, and calculated everything in the getSupply RPC implementation? That would give us the current supply for all fields and keep runtime/ out of it entirely

Do you mean abandon the epoch timing altogether, and calculate everything on every request?
We could do that. It is just a little bit expensive, since we have to iterate through all the stake accounts.

Another option is to calculate/cache the non-circulating account addresses on the epoch boundary, but get balances to calculate supply totals on rpc request.

@mvines
Copy link
Contributor

mvines commented May 8, 2020

Do you mean abandon the epoch timing altogether, and calculate everything on every request?
We could do that. It is just a little bit expensive, since we have to iterate through all the stake accounts.

Yes. We can also cache the value within RPC too for some number of slots as well.

@CriesofCarrots
Copy link
Contributor Author

Yes. We can also cache the value within RPC too for some number of slots as well.

Sort of like #9541 ? 😆

Okay, I think I'll scrap this, and PR:

  • deprecate getTotalSupply and return as part of one getSupply method, wrapped in slot context
  • all computation in RPC on request, no caching for now. We could add caching as a later optimization, if it seems necessary

@CriesofCarrots CriesofCarrots deleted the revamp-circ-supply branch May 26, 2020 23:30
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: add getCirculatingSupply API
2 participants