Undelegate may lose the delegator鈥檚 tokens when exceed the MaxEntries of unbonding delegations
$ git log -1
commit 95f3d322503cd79ddeebea8986261594b239aed9 (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
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
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
MaxEntriesof unbonding delegations is 7. When delegator perform the eighth undelegate, theUndelegatewill deduct the shares in thetypes.Delegation, but not put theredelegationin theUBDQueue, 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.
Thanks @alexanderbez. It uses a CacheMultiStore when run msg, so failed msg will not be written.
Most helpful comment
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.
Not true as the message will fail because
HasMaxUnbondingDelegationEntrieswill returntrueon 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.