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

Document why staking small numbers of coins fails #4656

Closed
4 tasks
rhuairahrighairidh opened this issue Jul 1, 2019 · 8 comments
Closed
4 tasks

Document why staking small numbers of coins fails #4656

rhuairahrighairidh opened this issue Jul 1, 2019 · 8 comments
Labels
C:x/staking T:Docs Changes and features related to documentation.

Comments

@rhuairahrighairidh
Copy link
Contributor

Summary of Bug

Creating a validator with < 1M coins fails without errors.
Same for unjailing a validator with < 1M coins.

Not sure if this is a bug or a feature, but it caused some problems in our recent testnet launch.
Some validators were slashed early on, leaving them without enough tokens to become a validator again after unjailing.

In the staking endBlocker, the total delegations are used to calculate the tendermint power by dividing by 10^6. When the total delegations are < 10^6 the tendermint power is rounded to 0 and the validator doesn't become a tendermint validator.

Code in question:

// PowerReduction is the amount of staking tokens required for 1 unit of consensus-engine power
var PowerReduction = NewIntFromBigInt(new(big.Int).Exp(big.NewInt(10), big.NewInt(6), nil))
// TokensToConsensusPower - convert input tokens to potential consensus-engine power
func TokensToConsensusPower(tokens Int) int64 {
return (tokens.Quo(PowerReduction)).Int64()
}

Problems:

  • Sets an undocumented(?) minimum staking requirement for being a validator
  • May cause problems for chains using different subunit amounts to the Hub.

@karzak, @kincaidoneil can comment more on subunit considerations. They're experience in cross chain work revealed subtle problems in this area

Same issue mentioned on the forum: https://forum.cosmos.network/t/why-is-my-newly-created-validator-unbonded/1841

Version

v0.34.7 and version currently used by gaia (c898dac)

Steps to Reproduce

Initialise a normal single validator testnet, but provide <1M starting stake. The chain won't start as no validators are created on launch.

  • gaiacli keys add someName
  • gaiad init someName
  • gaiad add-genesis-account $(gaiacli keys show someName -a) 100stake
  • gaiad gentx --name someName --amount 100stake
  • gaiad collect-gentxs
  • gaiad start

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

Thanks for creating an issue @rhuairahrighairidh. Indeed this would get truncated to zero and leave a validator powerless. I'm not totally sure if this case was considered during implementation. It seems at the very least we should have this documented. Furthermore, if the tokens are less than 10^6, I wonder if we can simply use that power without dividing.

/cc @rigelrozanski

@karzak
Copy link
Contributor

karzak commented Jul 1, 2019

👍 To Ruaridh’s comments. As far as units go, it’s usually not possible/desirable to try and program with the expectation that you can support any units that users might input. I would probably start by documenting the tested/expected range so end users can adjust accordingly.

@rigelrozanski
Copy link
Contributor

This is intentional design, in order to stay well within bounds of ABCI requirements the Tendermint/ABCI power (int64) is 10^-6 x number of uatom. Hence one is not allowed to create a validator with less than a single atom.

you have to remember 10^6 uatom = 1 atom.

More discussion on original design: #2513

May cause problems for chains using different subunit amounts to the Hub.

Another blockchain can simply reassign the value of PowerReduction within staking as it's a global exposed variable. Hence this shouldn't be a problem.


all and all I agree we need better documentation surrounding these nuances however.

@rigelrozanski rigelrozanski added the T:Docs Changes and features related to documentation. label Jul 1, 2019
@rigelrozanski rigelrozanski changed the title Staking small numbers of coins fails Document why staking small numbers of coins fails Jul 1, 2019
@aaronc
Copy link
Member

aaronc commented Jul 18, 2019

Okay, so we know how to change the PowerReduction in code. Should this actually be a param? @rigelrozanski @alexanderbez

@joe-bowman
Copy link
Contributor

I agree @aaronc, it feels like power reduction should be a parameter. Just because cosmoshub desires the ability to have divisible denominations, doesn't mean that every project will, and this should be specified on a per project basis.

@aaronc
Copy link
Member

aaronc commented Jul 18, 2019

And if it's a parameter it's something that could be updated by governance because the scenario we're in with our testnet, we simply can't do anything about it without doing an upgrade (which we do plan to do and maybe should use this as an opportunity to test).

@alexanderbez
Copy link
Contributor

Assuming there is a single staking token, I don't immediately see the advantage of having it as a parameter because I don't understand when it would ever change once a chain is live? This of course assumes that the application developers know about this variable and that should change it if necessary.

@rigelrozanski
Copy link
Contributor

Power Reduction should not be a param as it shouldn't be changing with any real frequency (ideally not at all once the blockchain is defined). If for some reason it needed to change there would have to be a special migration which would require a hardfork, hence it couldn't even be structured as a parameter change proposal - a whole new proposal type would need to be created for this to happen on a live chain... seems like a low priority for typical blockchains

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/staking T:Docs Changes and features related to documentation.
Projects
None yet
Development

No branches or pull requests

6 participants