Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem: Undecided future of x/supply module #716

Closed
devashishdxt opened this issue Mar 3, 2022 · 6 comments
Closed

Problem: Undecided future of x/supply module #716

devashishdxt opened this issue Mar 3, 2022 · 6 comments

Comments

@devashishdxt
Copy link
Collaborator

The current x/supply module tracks liquid supply of a given token using the following formula:

liquid_supply = total_supply - (unvested_supply + module_account_balance)

where,

  • total_supply: Total supply of a denom which is obtained from x/bank module.
  • unvested_supply: The sum of tokens locked in vesting accounts (x/supply maintains a static list of vesting accounts configured in genesis.json, it does not support adding/removing vesting accounts).
  • module_account_balance: The sum of tokens locked in module accounts of different modules (x/supply maintains a static list of module accounts that it uses to fetch total tokens locked in module accounts)

Current module account list:

// ModuleAccounts defines the module accounts which will be queried to get liquid supply
ModuleAccounts = []string{
	authtypes.FeeCollectorName,
	distrtypes.ModuleName,
	stakingtypes.BondedPoolName,
	stakingtypes.NotBondedPoolName,
	minttypes.ModuleName,
	govtypes.ModuleName,
}

To accurately calculate liquid_supply, x/supply module needs updated list of all the vesting accounts and module accounts. Also, for all the vesting accounts and module accounts, it loops over them and fetches their balance one-by-one (which'll not be efficient if there are a lot of vesting accounts).

There are following available options:

  • Deprecate and remove x/supply and rely on the solution developed under Ability to query total liquid supply in the system cosmos/cosmos-sdk#7774. For the short-term, we can add ability to calculate liquid supply in explorer.
  • Add ability to modify the list of modules and vesting accounts that are used to calculate liquid supply (this is do-able for module accounts, but, there can be a lot of vesting accounts in the network it can become highly inefficient to loop over all of them to calculate locked tokens).
@tomtau
Copy link
Contributor

tomtau commented Mar 3, 2022

Having this tracked in the explorer sounds like a good idea. @calvinaco @ysong42 what do you think?

@yihuang
Copy link
Collaborator

yihuang commented Mar 3, 2022

Currently, the supply module only tracks the vesting accounts in genesis, which can be done by the client already.
The extra issue is to track vesting accounts created at runtime, which could be done by indexing service too, but maybe better to be able to do that in cosmos-sdk.

@leejw51crypto
Copy link
Contributor

leejw51crypto commented Mar 3, 2022

this is heavy task in the chain-itself, how about

  1. keep fetch account inside the core
  2. aggregate the information in client level such as indexer.

like elastic search, for an example

once all balance is updated in client level, sum all values can be done in database level.

@calvinaco
Copy link
Contributor

calvinaco commented Mar 4, 2022

Having this tracked in the explorer sounds like a good idea. @calvinaco @ysong42 what do you think?

Yes, I think it make senses to index on explorer. The indexer is capable and designed to keep track of all on-chain events, even if that would means heavy computation at runtime.

Having been said, this is also a commonly needed features on Cosmos SDK chains, so I saw in the issue on Cosmos SDK it is considered a wanted feature too.

While this can be done on the indexer, we can see if it still brings a huge benefits if it can be done inside the node software.

@devashishdxt
Copy link
Collaborator Author

@calvinaco The solution to do this in indexer is only temporary. Eventually, this should become available directly in Cosmos SDK's x/bank module once cosmos/cosmos-sdk#7774 is closed.

@devashishdxt
Copy link
Collaborator Author

Closing this issue in favour of #720 and crypto-com/chain-indexing#700.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants