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 50 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
42 changes: 42 additions & 0 deletions x/auth/keeper/migrations.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package keeper

import (
v043 "github.com/cosmos/cosmos-sdk/x/auth/legacy/v043"
"github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/gogo/protobuf/grpc"

sdk "github.com/cosmos/cosmos-sdk/types"
)

// Migrator is a struct for handling in-place store migrations.
type Migrator struct {
keeper AccountKeeper
queryServer grpc.Server
}

// NewMigrator returns a new Migrator.
func NewMigrator(keeper AccountKeeper, queryServer grpc.Server) Migrator {
return Migrator{keeper: keeper, queryServer: queryServer}
}

// Migrate1to2 migrates from version 1 to 2.
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
func (m Migrator) Migrate1to2(ctx sdk.Context) error {
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
var iterErr error

m.keeper.IterateAccounts(ctx, func(account types.AccountI) (stop bool) {
wb, err := v043.MigrateStore(ctx, account, m.queryServer)
gsora marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
iterErr = err
return true
}

if wb == nil {
return false
}

m.keeper.SetAccount(ctx, wb)
return false
})

return iterErr
}
268 changes: 268 additions & 0 deletions x/auth/legacy/v043/store.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,268 @@
package v043

import (
"errors"
"fmt"

"github.com/cosmos/cosmos-sdk/baseapp"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/cosmos/cosmos-sdk/x/auth/vesting/exported"
vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
"github.com/gogo/protobuf/grpc"
"github.com/gogo/protobuf/proto"
abci "github.com/tendermint/tendermint/abci/types"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

const (
delegatorDelegationPath = "/cosmos.staking.v1beta1.Query/DelegatorDelegations"
stakingParamsPath = "/cosmos.staking.v1beta1.Query/Params"
delegatorUnbondingDelegationsPath = "/cosmos.staking.v1beta1.Query/DelegatorUnbondingDelegations"
balancesPath = "/cosmos.bank.v1beta1.Query/AllBalances"
gsora marked this conversation as resolved.
Show resolved Hide resolved
)

func migrateVestingAccounts(ctx sdk.Context, account types.AccountI, queryServer grpc.Server) (types.AccountI, error) {
bondDenom, err := getBondDenom(ctx, queryServer)

if err != nil {
return nil, err
}

asVesting, ok := account.(exported.VestingAccount)
if !ok {
return nil, nil
}

addr := account.GetAddress().String()
balance, err := getBalance(
ctx,
addr,
queryServer,
)

if err != nil {
return nil, err
}

delegations, err := getDelegatorDelegationsSum(
ctx,
addr,
queryServer,
)

if err != nil {
return nil, err
}

unbondingDelegations, err := getDelegatorUnbondingDelegationsSum(
ctx,
addr,
bondDenom,
queryServer,
)

if err != nil {
return nil, err
}

delegations = delegations.Add(unbondingDelegations...)

asVesting, ok = resetVestingDelegatedBalances(asVesting)
if !ok {
return nil, nil
}

// balance before any delegation includes balance of delegation
for _, coin := range delegations {
balance = balance.Add(coin)
}

asVesting.TrackDelegation(ctx.BlockTime(), balance, delegations)

return asVesting.(types.AccountI), nil
}

func resetVestingDelegatedBalances(evacct exported.VestingAccount) (exported.VestingAccount, bool) {
// reset `DelegatedVesting` and `DelegatedFree` to zero
df := sdk.NewCoins()
dv := sdk.NewCoins()

switch vacct := evacct.(type) {
case *vestingtypes.ContinuousVestingAccount:
vacct.DelegatedVesting = dv
vacct.DelegatedFree = df
return vacct, true
case *vestingtypes.DelayedVestingAccount:
vacct.DelegatedVesting = dv
vacct.DelegatedFree = df
return vacct, true
case *vestingtypes.PeriodicVestingAccount:
vacct.DelegatedVesting = dv
vacct.DelegatedFree = df
return vacct, true
gsora marked this conversation as resolved.
Show resolved Hide resolved
default:
return nil, false
}
}

func getDelegatorDelegationsSum(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.Coins, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to include unbonding delegations.

DelegatorDelegations returns items stored with the GetDelegationsKey prefix

delStore := prefix.NewStore(store, types.GetDelegationsKey(delAddr))

An Undelegate message will Unbond which subtracts from or deletes an entry under that same prefix

store.Delete(types.GetDelegationKey(delegatorAddress, delegation.GetValidatorAddr()))

It then creates an unbonding entry

ubd := k.SetUnbondingDelegationEntry(ctx, delAddr, valAddr, ctx.BlockHeight(), completionTime, returnAmount)
k.InsertUBDQueue(ctx, ubd, completionTime)

When the unbonding is complete, the originally problematic method in bank keeper is called

if err := k.bankKeeper.UndelegateCoinsFromModuleToAccount(

which eventually calls TrackUndelegation

vacc.TrackUndelegation(amt)

So, if this doesn't look at unbonding delegations, there would be a situation where

  1. user unbonds with bug in place
  2. migration runs, resets vesting account fields, does not account for unbonding delegation
  3. unbond completes, TrackUndelegation is called for which there was no corresponding TrackDelegation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just about to push support for unbonding delegations!

I'll request review from you as soon as I'm done.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It seams this is still not done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been taken into account already in fef2d00.

querier, ok := queryServer.(*baseapp.GRPCQueryRouter)
if !ok {
return nil, fmt.Errorf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer)
}

queryFn := querier.Route(delegatorDelegationPath)

q := &stakingtypes.QueryDelegatorDelegationsRequest{
DelegatorAddr: address,
}

b, err := proto.Marshal(q)
if err != nil {
return nil, fmt.Errorf("cannot marshal staking type query request, %w", err)
}
req := abci.RequestQuery{
Data: b,
Path: delegatorDelegationPath,
}
resp, err := queryFn(ctx, req)
gsora marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
e, ok := status.FromError(err)
if ok && e.Code() == codes.NotFound {
return nil, nil
}
return nil, fmt.Errorf("staking query error, %w", err)
}

balance := new(stakingtypes.QueryDelegatorDelegationsResponse)
if err := proto.Unmarshal(resp.Value, balance); err != nil {
return nil, fmt.Errorf("unable to unmarshal delegator query delegations: %w", err)
}

res := sdk.NewCoins()
for _, i := range balance.DelegationResponses {
res = res.Add(i.Balance)
}

return res, nil
}

func getDelegatorUnbondingDelegationsSum(ctx sdk.Context, address, bondDenom string, queryServer grpc.Server) (sdk.Coins, error) {
querier, ok := queryServer.(*baseapp.GRPCQueryRouter)
if !ok {
return nil, fmt.Errorf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer)
}

queryFn := querier.Route(delegatorUnbondingDelegationsPath)

q := &stakingtypes.QueryDelegatorUnbondingDelegationsRequest{
DelegatorAddr: address,
}

b, err := proto.Marshal(q)
if err != nil {
return nil, fmt.Errorf("cannot marshal staking type query request, %w", err)
}
req := abci.RequestQuery{
Data: b,
Path: delegatorUnbondingDelegationsPath,
}
resp, err := queryFn(ctx, req)
if err != nil && !errors.Is(err, sdkerrors.ErrNotFound) {
e, ok := status.FromError(err)
if ok && e.Code() == codes.NotFound {
return nil, nil
}
return nil, fmt.Errorf("staking query error, %w", err)
}

balance := new(stakingtypes.QueryDelegatorUnbondingDelegationsResponse)
if err := proto.Unmarshal(resp.Value, balance); err != nil {
return nil, fmt.Errorf("unable to unmarshal delegator query delegations: %w", err)
}

res := sdk.NewCoins()
for _, i := range balance.UnbondingResponses {
for _, r := range i.Entries {
res = res.Add(sdk.NewCoin(bondDenom, r.Balance))
}
}

return res, nil
}

func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.Coins, error) {
querier, ok := queryServer.(*baseapp.GRPCQueryRouter)
if !ok {
return nil, fmt.Errorf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer)
}

queryFn := querier.Route(balancesPath)

q := &banktypes.QueryAllBalancesRequest{
Address: address,
Pagination: nil,
}
b, err := proto.Marshal(q)
if err != nil {
return nil, fmt.Errorf("cannot marshal bank type query request, %w", err)
}

req := abci.RequestQuery{
Data: b,
Path: balancesPath,
}
resp, err := queryFn(ctx, req)
if err != nil {
return nil, fmt.Errorf("bank query error, %w", err)
}
balance := new(banktypes.QueryAllBalancesResponse)
if err := proto.Unmarshal(resp.Value, balance); err != nil {
return nil, fmt.Errorf("unable to unmarshal bank balance response: %w", err)
}
return balance.Balances, nil
}

func getBondDenom(ctx sdk.Context, queryServer grpc.Server) (string, error) {
querier, ok := queryServer.(*baseapp.GRPCQueryRouter)
if !ok {
return "", fmt.Errorf("unexpected type: %T wanted *baseapp.GRPCQueryRouter", queryServer)
}

queryFn := querier.Route(stakingParamsPath)

q := &stakingtypes.QueryParamsRequest{}

b, err := proto.Marshal(q)
if err != nil {
return "", fmt.Errorf("cannot marshal staking params query request, %w", err)
}
req := abci.RequestQuery{
Data: b,
Path: stakingParamsPath,
}

resp, err := queryFn(ctx, req)
if err != nil {
return "", fmt.Errorf("staking query error, %w", err)
}

params := new(stakingtypes.QueryParamsResponse)
if err := proto.Unmarshal(resp.Value, params); err != nil {
return "", fmt.Errorf("unable to unmarshal delegator query delegations: %w", err)
}

return params.Params.BondDenom, nil
}

// MigrateStore migrates vesting account to make the DelegatedVesting and DelegatedFree fields correctly
// track delegations.
// References: https://github.com/cosmos/cosmos-sdk/issues/8601, https://github.com/cosmos/cosmos-sdk/issues/8812
func MigrateStore(ctx sdk.Context, account types.AccountI, queryServer grpc.Server) (types.AccountI, error) {
return migrateVestingAccounts(ctx, account, queryServer)
}
Loading