Undelegate may lose the delegator’s tokens when exceed the MaxEntries of unbonding delegations

[helldealer]

Summary of Bug

Undelegate may lose the delegator’s tokens when exceed the MaxEntries of unbonding delegations

Version

$ git log -1
commit 95f3d32 (HEAD -> master, origin/master, origin/HEAD)
Author: colin axner colinaxner@berkeley.edu
Date: Thu Jun 13 06:54:17 2019 -0700

Merge PR #4536: Return account queries with height

Steps to Reproduce

Suppose the MaxEntries of unbonding delegations is 7. When delegator perform the eighth undelegate, the Undelegate will deduct the shares in the types.Delegation, but not put the redelegation in the UBDQueue, which will cause the reduce of the delegator shares, and the corresponding account balance will not receive the reduced token, Supplementing TestUnbondingDelegationsMaxEntries with delegation and account balance check can be exposed to this problem

---

For Admin Use

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

[alexanderbez]

Thanks for opening up an issue @helldealer. However, this isn't technically a bug. If a validator/delegator pair has max entries, then the message will fail and no state will be written. i.e. the order of operations doesn't really matter -- no shares will be deducted.

Suppose the MaxEntries of unbonding delegations is 7. When delegator perform the eighth undelegate, the Undelegate will deduct the shares in the types.Delegation, but not put the redelegation in the UBDQueue, which will cause the reduce of the delegator shares, and the corresponding account balance will not receive the reduced token

Not true as the message will fail because HasMaxUnbondingDelegationEntries will return true on the 8th attempt.

However, I do agree that the code and logic reads better by having the max entries check first! But again, no bug.

[helldealer]

Thanks @alexanderbez. It uses a CacheMultiStore when run msg, so failed msg will not be written.