Cosmos-sdk: Possible inconsistency in LooseTokens

Created on 29 Jul 2018  路  9Comments  路  Source: cosmos/cosmos-sdk

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.

https://github.com/cosmos/cosmos-sdk/blob/c9936b310af96c5eb519cd7142f3ad22adce7a7c/x/stake/keeper/delegation.go#L331

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

https://github.com/cosmos/cosmos-sdk/blob/c9936b310af96c5eb519cd7142f3ad22adce7a7c/x/stake/types/validator.go#L398-L404

https://github.com/cosmos/cosmos-sdk/blob/c9936b310af96c5eb519cd7142f3ad22adce7a7c/x/stake/types/pool.go#L73

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


For Admin Use

  • [x] Not duplicate issue
  • [x] Appropriate labels applied
  • [x] Appropriate contributors tagged
  • [x] Contributor assigned/self-assigned
bug staking

Most helpful comment

rounding down seems fine for now...

All 9 comments

Wow, great catch! ref #1471

/cc @rigelrozanski , @cwgoes

Yup, this needs to be fixed - ref https://github.com/cosmos/cosmos-sdk/issues/1807.

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

We should verify this once staking refactor has been completed.

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.

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.

rounding down seems fine for now...

rounding down seems fine for now...

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

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

Was this page helpful?
0 / 5 - 0 ratings