Merge pull request #238 from terra-money/fix/donate-all-function

fix: donate all vesting token to work with dust delegations

Author: emidev98

.github/workflows/lint.yml

@@ -23,4 +23,4 @@ jobs:
         uses: golangci/golangci-lint-action@v3
         with:
           # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
-          version: v1.49.0
+          version: v1.51.2

.github/workflows/test-race.yml

@@ -48,6 +48,9 @@ jobs:
             **/**.go
             **/go.mod
             **/go.sum
+      - name: Update deps
+        run: go mod tidy
+
       - name: Build
         run: GOARCH=${{ matrix.go-arch }} LEDGER_ENABLED=false make build
 
@@ -105,6 +108,8 @@ jobs:
         with:
           name: "${{ github.sha }}-${{ matrix.part }}"
         if: env.GIT_DIFF
+      - name: Update deps
+        run: go mod tidy
       - name: test & coverage report creation
         run: |
           xargs --arg-file=pkgs.txt.part.${{ matrix.part }} go test -mod=readonly -timeout 30m -race -tags='cgo ledger test_ledger_mock'

.github/workflows/test.yml

@@ -35,6 +35,9 @@ jobs:
             **/**.go
             **/go.mod
             **/go.sum
+      - name: Update deps
+        run: go mod tidy
+
       - name: Build
         run: GOARCH=${{ matrix.go-arch }} LEDGER_ENABLED=false make build
 
@@ -56,6 +59,8 @@ jobs:
             **/**.go
             go.mod
             go.sum
+      - name: Update deps
+        run: go mod tidy
       - name: Run submodule tests and create test coverage profile.
         # GIT_DIFF is passed to the scripts
         run: bash scripts/module-tests.sh
@@ -116,6 +121,8 @@ jobs:
         with:
           name: "${{ github.sha }}-${{ matrix.part }}"
         if: env.GIT_DIFF
+      - name: update deps
+        run: go mod tidy
       - name: test & coverage report creation
         run: |
           cat pkgs.txt.part.${{ matrix.part }} | xargs go test -mod=readonly -timeout 30m -coverprofile=${{ matrix.part }}profile.out -covermode=atomic -tags='norace ledger test_ledger_mock'
@@ -189,6 +196,8 @@ jobs:
             **/**.go
             go.mod
             go.sum
+      - name: Update deps
+        run: go mod tidy
       - name: test rosetta
         run: |
           make test-rosetta
@@ -209,6 +218,8 @@ jobs:
             **/**.go
             go.mod
             go.sum
+      - name: Update deps
+        run: go mod tidy
       - name: start localnet
         run: |
           make clean localnet-start
@@ -250,6 +261,8 @@ jobs:
             **/**.go
             go.mod
             go.sum
+      - name: Update deps
+        run: go mod tidy
       - uses: actions/cache@v3
         with:
           path: ~/go/bin

.golangci.yml

@@ -41,8 +41,7 @@ issues:
     - text: "should be written without leading space as"
       linters:
         - nolintlint
-    - path: "migrations"
-      text: "SA1019:"
+    - text: "SA1019:"
       linters:
         - staticcheck
 

Makefile

@@ -329,7 +329,7 @@ benchmark:
 ###############################################################################
 
 golangci_lint_cmd=golangci-lint
-golangci_version=v1.49.0
+golangci_version=v1.51.2
 
 lint:
 	@echo "--> Running linter"

go.mod

@@ -161,6 +161,7 @@ require (
 replace (
 	// use cosmos fork of keyring
 	github.com/99designs/keyring => github.com/cosmos/keyring v1.2.0
+	github.com/ChainSafe/go-schnorrkel => github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d
 	// dgrijalva/jwt-go is deprecated and doesn't receive security updates.
 	// TODO: remove it: https://github.com/cosmos/cosmos-sdk/issues/13134
 	github.com/dgrijalva/jwt-go => github.com/golang-jwt/jwt/v4 v4.4.2

x/auth/vesting/handler_test.go

@@ -1,7 +1,9 @@
 package vesting_test
 
 import (
+	stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/suite"
 	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
@@ -108,8 +110,22 @@ func (suite *HandlerTestSuite) TestMsgDonateVestingToken() {
 	addr1 := sdk.AccAddress([]byte("addr1_______________"))
 	addr2 := sdk.AccAddress([]byte("addr2_______________"))
 	addr3 := sdk.AccAddress([]byte("addr3_______________"))
+	addr4 := sdk.AccAddress([]byte("addr4_______________"))
 
 	valAddr := sdk.ValAddress([]byte("validator___________"))
+	suite.app.StakingKeeper.SetValidator(ctx, stakingtypes.Validator{
+		OperatorAddress:   valAddr.String(),
+		ConsensusPubkey:   nil,
+		Jailed:            false,
+		Status:            0,
+		Tokens:            sdk.NewInt(2),
+		DelegatorShares:   sdk.MustNewDecFromStr("1.1"),
+		Description:       stakingtypes.Description{},
+		UnbondingHeight:   0,
+		UnbondingTime:     time.Time{},
+		Commission:        stakingtypes.Commission{},
+		MinSelfDelegation: sdk.NewInt(1),
+	})
 
 	acc1 := suite.app.AccountKeeper.NewAccountWithAddress(ctx, addr1)
 	suite.app.AccountKeeper.SetAccount(ctx, acc1)
@@ -133,6 +149,18 @@ func (suite *HandlerTestSuite) TestMsgDonateVestingToken() {
 	suite.app.AccountKeeper.SetAccount(ctx, acc3)
 	suite.Require().NoError(simapp.FundAccount(suite.app.BankKeeper, ctx, addr3, balances))
 
+	acc4 := types.NewPermanentLockedAccount(
+		suite.app.AccountKeeper.NewAccountWithAddress(ctx, addr4).(*authtypes.BaseAccount), balances,
+	)
+	acc4.DelegatedVesting = balances
+	suite.app.AccountKeeper.SetAccount(ctx, acc4)
+	suite.app.StakingKeeper.SetDelegation(ctx, stakingtypes.Delegation{
+		DelegatorAddress: addr4.String(),
+		ValidatorAddress: valAddr.String(),
+		Shares:           sdk.MustNewDecFromStr("0.1"),
+	})
+	suite.Require().NoError(simapp.FundAccount(suite.app.BankKeeper, ctx, addr4, balances))
+
 	testCases := []struct {
 		name      string
 		msg       *types.MsgDonateAllVestingTokens
@@ -153,12 +181,19 @@ func (suite *HandlerTestSuite) TestMsgDonateVestingToken() {
 			msg:       types.NewMsgDonateAllVestingTokens(addr3),
 			expectErr: false,
 		},
+		{
+			name:      "donate from vesting account with dust delegation",
+			msg:       types.NewMsgDonateAllVestingTokens(addr4),
+			expectErr: false,
+		},
 	}
 
 	for _, tc := range testCases {
 		tc := tc
 
 		suite.Run(tc.name, func() {
+			// Rollback context after every test case
+			ctx, _ := ctx.CacheContext()
 			res, err := suite.handler(ctx, tc.msg)
 			if tc.expectErr {
 				suite.Require().Error(err)
@@ -178,6 +213,9 @@ func (suite *HandlerTestSuite) TestMsgDonateVestingToken() {
 				suite.Require().True(ok)
 				balance := suite.app.BankKeeper.GetAllBalances(ctx, fromAddr)
 				suite.Require().Empty(balance)
+
+				_, broken := stakingkeeper.DelegatorSharesInvariant(suite.app.StakingKeeper)(ctx)
+				suite.Require().False(broken)
 			}
 		})
 	}

x/auth/vesting/msg_server.go

@@ -2,6 +2,8 @@ package vesting
 
 import (
 	"context"
+	"fmt"
+	"math"
 
 	"github.com/armon/go-metrics"
 
@@ -239,9 +241,33 @@ func (s msgServer) DonateAllVestingTokens(goCtx context.Context, msg *types.MsgD
 		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "account %s not exists", msg.FromAddress)
 	}
 
-	// check whether an account has any type of staking entry
-	if len(sk.GetDelegatorDelegations(ctx, acc.GetAddress(), 1)) != 0 ||
-		len(sk.GetUnbondingDelegations(ctx, acc.GetAddress(), 1)) != 0 ||
+	// get all delegations of an account and undust those that have less than 1 uluna
+	delegations := sk.GetDelegatorDelegations(ctx, acc.GetAddress(), math.MaxUint16)
+	for _, delegation := range delegations {
+		validatorAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress)
+		if err != nil {
+			return nil, err
+		}
+		validator, found := sk.GetValidator(ctx, validatorAddr)
+		if !found {
+			return nil, fmt.Errorf("validator not found")
+		}
+		// Try to delete the dust delegation
+		_, removedTokens := sk.RemoveValidatorTokensAndShares(ctx, validator, delegation.Shares)
+		// If the delegation is not dust, return an error and stop the donation flow
+		if !removedTokens.IsZero() {
+			return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "account %s has a non-zero staking entry", msg.FromAddress)
+		}
+
+		// Remove the dust delegation shares from the validator
+		err = sk.RemoveDelegation(ctx, delegation)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	// check whether an account has any other type of staking entries
+	if len(sk.GetUnbondingDelegations(ctx, acc.GetAddress(), 1)) != 0 ||
 		len(sk.GetRedelegations(ctx, acc.GetAddress(), 1)) != 0 {
 		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "account %s has staking entry", msg.FromAddress)
 	}

x/auth/vesting/types/expected_keepers.go

@@ -1,6 +1,7 @@
 package types
 
 import (
+	"cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 )
@@ -23,4 +24,7 @@ type StakingKeeper interface {
 	GetDelegatorDelegations(ctx sdk.Context, delegator sdk.AccAddress, maxRetrieve uint16) (delegations []stakingtypes.Delegation)
 	GetUnbondingDelegations(ctx sdk.Context, delegator sdk.AccAddress, maxRetrieve uint16) (unbondingDelegations []stakingtypes.UnbondingDelegation)
 	GetRedelegations(ctx sdk.Context, delegator sdk.AccAddress, maxRetrieve uint16) (redelegations []stakingtypes.Redelegation)
+	RemoveValidatorTokensAndShares(ctx sdk.Context, validator stakingtypes.Validator, sharesToRemove sdk.Dec) (valOut stakingtypes.Validator, removedTokens math.Int)
+	GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator stakingtypes.Validator, found bool)
+	RemoveDelegation(ctx sdk.Context, delegation stakingtypes.Delegation) error
 }