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

Write back account changes after tracking delegation/undelegation for vesting accounts #8865

Merged
merged 57 commits into from
Apr 8, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
a77b0c3
write back account changes after tracking delegation/undelegation
gsora Mar 12, 2021
af5b275
add test case for delegated vesting amount
gsora Mar 12, 2021
58c1b72
remove whitespace
gsora Mar 12, 2021
6cfba52
Merge branch 'master' into gsora/track-delegation-vesting-accounts
Mar 12, 2021
9357165
Merge branch 'master' into gsora/track-delegation-vesting-accounts
Mar 14, 2021
bbe8232
Merge branch 'master' into gsora/track-delegation-vesting-accounts
jgimeno Mar 15, 2021
d4a4640
Merge branch 'master' into gsora/track-delegation-vesting-accounts
Mar 15, 2021
a58c500
check that undelegate updates the delegatedvesting amount
gsora Mar 15, 2021
6e5d442
Merge branch 'master' into gsora/track-delegation-vesting-accounts
jgimeno Mar 15, 2021
c974a2e
Merge branch 'master' into gsora/track-delegation-vesting-accounts
Mar 17, 2021
fcb93b3
temp commit
jgimeno Mar 17, 2021
3f979a8
add migrator code to auth module
gsora Mar 17, 2021
bcc5198
add: query way
fdymylja Mar 17, 2021
313dc6d
Merge branch 'master' into gsora/track-delegation-vesting-accounts
Mar 17, 2021
c7d5798
implement first iteration of migration handling for vesting accounts …
gsora Mar 18, 2021
0bbbc0a
add test
jgimeno Mar 18, 2021
520a94a
next test iteration
jgimeno Mar 18, 2021
a9501d3
account for the case where delegations are greater than the original …
gsora Mar 18, 2021
aaa7a9c
delayed vesting test cases, first test case for continuous vesting
gsora Mar 18, 2021
c3fdb8a
add more tests
jgimeno Mar 18, 2021
17e5c5a
prototype of periodic vesting account test
gsora Mar 18, 2021
f20a862
Merge branch 'master' into gsora/track-delegation-vesting-accounts
jgimeno Mar 19, 2021
61ea15a
better function naming
gsora Mar 19, 2021
204db7a
Merge remote-tracking branch 'origin/gsora/track-delegation-vesting-a…
gsora Mar 19, 2021
6920bb9
correct func name
gsora Mar 19, 2021
e533951
handle error
gsora Mar 19, 2021
0f03346
added remaining test for periodic vesting account
gsora Mar 19, 2021
c0cb1d2
Merge branch 'master' into gsora/track-delegation-vesting-accounts
Mar 20, 2021
8d2cc6c
x/auth/keeper: add test for periodic vesting
boz Mar 22, 2021
80335ad
x/auth: add migrations module
boz Mar 22, 2021
9a1c74a
x/auth/migrations: fix lint
boz Mar 22, 2021
93b2ec9
x/auth/migrations: fixes
boz Mar 22, 2021
c10b189
x/auth/migrations: fix tests
boz Mar 22, 2021
b82d06f
Merge pull request #8946 from ovrclk/boz/track-delegation-vesting-acc…
gsora Mar 23, 2021
3b70dbc
Merge remote-tracking branch 'origin/gsora/track-delegation-vesting-a…
gsora Mar 23, 2021
d99c22e
meaningful comment for Migrate1to2 function
gsora Mar 23, 2021
327277d
re-introduced periodic vesting tests
gsora Mar 23, 2021
e8f906b
refactor auth migration to use the same directory tree/call style as …
gsora Mar 23, 2021
2b7e14e
Merge branch 'master' into gsora/track-delegation-vesting-accounts
Mar 23, 2021
b40333f
check test Delegate() error
gsora Mar 24, 2021
3c6489d
init test validator properly
gsora Mar 24, 2021
60c41cc
remove vesting() function
gsora Mar 24, 2021
d9f55cd
import sorting
gsora Mar 24, 2021
fef2d00
properly support unbonding delegations
gsora Mar 24, 2021
bfc1860
more tests!
gsora Mar 24, 2021
08926b1
Merge branch 'master' into gsora/track-delegation-vesting-accounts
jgimeno Mar 26, 2021
c44fe38
use account iterator instead of a big slice of accounts
gsora Mar 26, 2021
5fdda3d
call auth keeper migrations in tests
gsora Mar 26, 2021
f6a602b
check delegations queries errors
gsora Mar 26, 2021
0b946d6
Merge branch 'master' into gsora/track-delegation-vesting-accounts
jgimeno Mar 29, 2021
f928380
Merge branch 'master' into gsora/track-delegation-vesting-accounts
amaury1093 Apr 2, 2021
289e4c6
Merge branch 'master' into gsora/track-delegation-vesting-accounts
sahith-narahari Apr 3, 2021
5d9d4f9
fix linter warnings
Apr 4, 2021
226ca8c
Merge branch 'master' into gsora/track-delegation-vesting-accounts
sahith-narahari Apr 6, 2021
85b79b2
Merge branch 'master' into gsora/track-delegation-vesting-accounts
jgimeno Apr 6, 2021
f342589
rename migrate function to MigrateAccount
gsora Apr 6, 2021
34b7961
Merge branch 'master' into gsora/track-delegation-vesting-accounts
Apr 7, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions x/bank/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ func (k BaseKeeper) trackDelegation(ctx sdk.Context, addr sdk.AccAddress, balanc
if ok {
// TODO: return error on account.TrackDelegation
vacc.TrackDelegation(ctx.BlockHeader().Time, balance, amt)
k.ak.SetAccount(ctx, acc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little unlcear how type assertions work - is vacc a copy of acc? If so, I believe you want

Suggested change
k.ak.SetAccount(ctx, acc)
k.ak.SetAccount(ctx, vacc)

As the call to TrackUndelegation alters the state of vacc not acc.

Same would be true on L449.

Copy link
Contributor

@alexanderbez alexanderbez Mar 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same underlying account in memory, so I do not believe it matters.

In fact, this is more or less the same as the original code: https://github.com/cosmos/cosmos-sdk/blob/v0.39.2/x/bank/internal/keeper/keeper.go#L78-L82

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.

}

return nil
Expand All @@ -445,6 +446,7 @@ func (k BaseKeeper) trackUndelegation(ctx sdk.Context, addr sdk.AccAddress, amt
if ok {
// TODO: return error on account.TrackUndelegation
vacc.TrackUndelegation(amt)
k.ak.SetAccount(ctx, acc)
}

return nil
Expand Down
13 changes: 13 additions & 0 deletions x/bank/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"
"time"

"github.com/cosmos/cosmos-sdk/x/auth/vesting/exported"
minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"

"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -860,6 +861,12 @@ func (suite *IntegrationTestSuite) TestDelegateCoins() {
// require the ability for a vesting account to delegate
suite.Require().NoError(app.BankKeeper.DelegateCoins(ctx, addr1, addrModule, delCoins))
suite.Require().Equal(delCoins, app.BankKeeper.GetAllBalances(ctx, addr1))

// require that delegated vesting amount is equal to what was delegated with DelegateCoins
acc = app.AccountKeeper.GetAccount(ctx, addr1)
vestingAcc, ok := acc.(exported.VestingAccount)
suite.Require().True(ok)
suite.Require().Equal(delCoins, vestingAcc.GetDelegatedVesting())
}

func (suite *IntegrationTestSuite) TestDelegateCoins_Invalid() {
Expand Down Expand Up @@ -934,6 +941,12 @@ func (suite *IntegrationTestSuite) TestUndelegateCoins() {

suite.Require().Equal(origCoins, app.BankKeeper.GetAllBalances(ctx, addr1))
suite.Require().True(app.BankKeeper.GetAllBalances(ctx, addrModule).Empty())

// require that delegated vesting amount is completely empty, since they were completely undelegated
acc = app.AccountKeeper.GetAccount(ctx, addr1)
vestingAcc, ok := acc.(exported.VestingAccount)
suite.Require().True(ok)
suite.Require().Empty(vestingAcc.GetDelegatedVesting())
}

func (suite *IntegrationTestSuite) TestUndelegateCoins_Invalid() {
Expand Down