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

Increase number of decimal places in sdk.Decimal #3039

Closed
ValarDragon opened this issue Dec 7, 2018 · 5 comments · Fixed by #3315
Closed

Increase number of decimal places in sdk.Decimal #3039

ValarDragon opened this issue Dec 7, 2018 · 5 comments · Fixed by #3315
Labels
C:x/distribution distribution module related T:Bug

Comments

@ValarDragon
Copy link
Contributor

ValarDragon commented Dec 7, 2018

Decimals are needed in fee distribution / inflation, to ensure that you still get rewards when you only have like 1 piece of a validators stake. Currently a validator's tm power could be as large as 2**63 - 1. Thus we require decimals to account for you getting 1 / (2**63 - 1) of a token of rewards. log_10(2**63 - 1) = 18.9, so we need at least 19 decimal digits, not the 10 we have now.

We need to lower this max tm power (tendermint/tendermint#2985). Per latest comment, max amount of tm power per validator would be at most 2**63 - 1 / num_validators, taking num_validators as 100, that'd require 17 decimal digits at least.

@cwgoes
Copy link
Contributor

cwgoes commented Dec 11, 2018

Isn't the effective bound on Tendermint power dependent on total supply, not purely the TM maximums?

@ValarDragon
Copy link
Contributor Author

Yes, the total supply part is independent of the decimal places of precision required though. The total supply is tracked via #2513

@cwgoes
Copy link
Contributor

cwgoes commented Dec 14, 2018

I will add some tests for this in #3099.

@jackzampolin
Copy link
Member

Is this still an issue?

@ValarDragon
Copy link
Contributor Author

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/distribution distribution module related T:Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants