Possible inconsistency in LooseTokens

[hendrikhofstadt]

Summary of Bug

When a validator is slashed and an individual share is is now a floating point number like 1.6 a delegator will get 2 steaks back when he unrevokes because of bankers rounding.

cosmos-sdk/x/stake/keeper/delegation.go

Line 331 in c9936b3

balance := sdk.Coin{params.BondDenom, returnAmount.RoundInt()}

However LooseTokens will only be incremented by the Rat value of 1.6

cosmos-sdk/x/stake/types/validator.go

Lines 398 to 404 in c9936b3

	issuedTokens := v.DelegatorShareExRate().Mul(delShares)
	v.Tokens = v.Tokens.Sub(issuedTokens)
	v.DelegatorShares = v.DelegatorShares.Sub(delShares)
	if v.Status == sdk.Bonded {
	pool = pool.bondedTokensToLoose(issuedTokens)
	}

cosmos-sdk/x/stake/types/pool.go

Line 73 in c9936b3

p.LooseTokens = p.LooseTokens.Add(bondedTokens)

This will lead to an inconsistency between the real staking tokens in circulation and LooseTokens.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[ValarDragon]

Wow, great catch! ref #1471

/cc @rigelrozanski , @cwgoes

[cwgoes]

Yup, this needs to be fixed - ref #1807.

[ValarDragon]

I guess we just need to double check whether or not this is still a concern with the decimal implementation.

[alexanderbez]

We should verify this once staking refactor has been completed.

[cwgoes]

We should verify this once staking refactor has been completed.

This is still an issue; the staking refactor did not change the delegation/undelegation logic.

[cwgoes]

I think we need to change the approach here altogether, this seems like an attack vector - if it's possible to get "extra" tokens by rounding, there's likely some way to create such delegations en masse and mint currency. Either we need to round down instead, or we need to restrict unbonding to whole-token amounts.

[jaekwon]

rounding down seems fine for now...

[cwgoes]

rounding down seems fine for now...

OK, let's also update the pool accordingly so supply tracking remains consistent.

[rigelrozanski]

I think rounding down is fine because this is going to be such a fractionally small amount of tokens (10^-8 of an atom) should be okay