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

Track delegation not work for vesting account #8601

Closed
4 tasks
cyborgshead opened this issue Feb 17, 2021 · 4 comments · Fixed by #8865
Closed
4 tasks

Track delegation not work for vesting account #8601

cyborgshead opened this issue Feb 17, 2021 · 4 comments · Fixed by #8865
Labels
C:x/auth C:x/staking S:needs more info This bug can't be addressed until more information is provided by the reporter. T:Bug

Comments

@cyborgshead
Copy link

Summary of Bug

Delegation (and undelegation) from vesting account are not tracked. I tried to make delegation from account (in my case periodic vesting account) and found that there are no updates to delegated vesting amount.
Looking to the code, there is not SetAccount here

Note: it was worked in 0.39.0

Version

SDK 0.41.0

Steps to Reproduce

  1. Vest some account's tokens
  2. Make delegation from this account

For Admin Use

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

@litvintech Would you like to create a PR with a test?

@alexanderbez
Copy link
Contributor

Continuous and delayed accounts do track delegations and undelegations. I cannot speak for periodic account types because that was an external contribution.

@alessio
Copy link
Contributor

alessio commented Mar 10, 2021

Thank you very much for taking the time to file this issue and contribute to making Cosmos SDK better.

@litvintech dixit:

Looking to the code, there is not SetAccount here

True that, and yet addCoins() is called here:

err := k.addCoins(ctx, acc.GetAddress(), amt)

addCoins() subsequently calls setBalance():

err := k.setBalance(ctx, addr, newBalance)

Which in turn sets the new balance for the account:

accountStore.Set([]byte(balance.Denom), bz)

@litvintech can you write a test case, please? Meanwhile, I'm labeling this accordingly.

@alessio alessio added the S:needs more info This bug can't be addressed until more information is provided by the reporter. label Mar 10, 2021
@karzak
Copy link
Contributor

karzak commented Mar 10, 2021

Continuous and delayed accounts do track delegations and undelegations. I cannot speak for periodic account types because that was an external contribution.

Haven't used v0.41, but PeriodVestingAccount delegation tracking works on v0.39 and it looks to me like it should track delegations as normal here:

if err := k.trackDelegation(ctx, delegatorAddr, balances, amt); err != nil {

func (k BaseKeeper) trackDelegation(ctx sdk.Context, addr sdk.AccAddress, balance, amt sdk.Coins) error {

alessio pushed a commit that referenced this issue Apr 8, 2021
… vesting accounts (#8865)

Delegations and undelegations calculations performed during
accounting operations for vesting accounts were correct but
were not written back to the account store.

closes: #8601
closes: #8812

Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com>
Co-authored-by: Frojdi Dymylja <frojdi.dymylja@gmail.com>
Co-authored-by: Adam Bozanich <adam.boz@gmail.com>
Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
Co-authored-by: SaReN <sahithnarahari@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/auth C:x/staking S:needs more info This bug can't be addressed until more information is provided by the reporter. T:Bug
Projects
None yet
5 participants