From b2a4369d4e7a934192bfd3b47eb9122b14ff9df4 Mon Sep 17 00:00:00 2001
From: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Date: Fri, 5 Jun 2020 08:59:16 -0400
Subject: [PATCH] ValidatorVestingAccount validation (#552)

---
 x/validator-vesting/alias.go                  |  1 -
 .../types/validator_vesting_account.go        | 17 +++++---
 .../types/validator_vesting_account_test.go   | 39 +++++++++++++------
 3 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/x/validator-vesting/alias.go b/x/validator-vesting/alias.go
index 5ddc76e1..8193ada3 100644
--- a/x/validator-vesting/alias.go
+++ b/x/validator-vesting/alias.go
@@ -67,5 +67,4 @@ type (
 	VestingProgress         = types.VestingProgress
 	CurrentPeriodProgress   = types.CurrentPeriodProgress
 	ValidatorVestingAccount = types.ValidatorVestingAccount
-	TotalCirculatingSupply  = types.TotalCirculatingSupply
 )
diff --git a/x/validator-vesting/types/validator_vesting_account.go b/x/validator-vesting/types/validator_vesting_account.go
index 297e1082..686b52c0 100644
--- a/x/validator-vesting/types/validator_vesting_account.go
+++ b/x/validator-vesting/types/validator_vesting_account.go
@@ -3,6 +3,7 @@ package types
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"time"
 
 	yaml "gopkg.in/yaml.v2"
@@ -71,11 +72,6 @@ type ValidatorVestingAccount struct {
 	DebtAfterFailedVesting sdk.Coins             `json:"debt_after_failed_vesting" yaml:"debt_after_failed_vesting"`
 }
 
-// TotalCirculatingSupply represents the total circulating supply of Kava
-type TotalCirculatingSupply struct {
-	TotalSupply uint64 `json:"total_supply" yaml:"total_supply"` // total circulating supply
-}
-
 // NewValidatorVestingAccountRaw creates a new ValidatorVestingAccount object from BaseVestingAccount
 func NewValidatorVestingAccountRaw(bva *vestingtypes.BaseVestingAccount,
 	startTime int64, periods vestingtypes.Periods, validatorAddress sdk.ConsAddress, returnAddress sdk.AccAddress, signingThreshold int64) *ValidatorVestingAccount {
@@ -95,7 +91,10 @@ func NewValidatorVestingAccountRaw(bva *vestingtypes.BaseVestingAccount,
 		ValidatorAddress:       validatorAddress,
 		ReturnAddress:          returnAddress,
 		SigningThreshold:       signingThreshold,
-		CurrentPeriodProgress:  CurrentPeriodProgress{0, 0},
+		CurrentPeriodProgress: CurrentPeriodProgress{
+			MissedBlocks: 0,
+			TotalBlocks:  0,
+		},
 		VestingPeriodProgress:  vestingPeriodProgress,
 		DebtAfterFailedVesting: sdk.NewCoins(),
 	}
@@ -193,12 +192,18 @@ func (vva *ValidatorVestingAccount) TrackDelegation(blockTime time.Time, amount
 
 // Validate checks for errors on the account fields
 func (vva ValidatorVestingAccount) Validate() error {
+	if vva.ValidatorAddress.Empty() {
+		return errors.New("validator address cannot be empty")
+	}
 	if vva.SigningThreshold > 100 || vva.SigningThreshold < 0 {
 		return errors.New("signing threshold must be between 0 and 100")
 	}
 	if vva.ReturnAddress.Equals(vva.Address) {
 		return errors.New("return address cannot be the same as the account address")
 	}
+	if !vva.DebtAfterFailedVesting.IsValid() {
+		return fmt.Errorf("invalid debt after failed vesting coins: %s", vva.DebtAfterFailedVesting)
+	}
 	return vva.PeriodicVestingAccount.Validate()
 }
 
diff --git a/x/validator-vesting/types/validator_vesting_account_test.go b/x/validator-vesting/types/validator_vesting_account_test.go
index a2562e3b..89146d11 100644
--- a/x/validator-vesting/types/validator_vesting_account_test.go
+++ b/x/validator-vesting/types/validator_vesting_account_test.go
@@ -1,7 +1,6 @@
 package types
 
 import (
-	"errors"
 	"testing"
 	"time"
 
@@ -394,36 +393,52 @@ func TestGenesisAccountValidate(t *testing.T) {
 	origCoins := sdk.Coins{sdk.NewInt64Coin(feeDenom, 1000), sdk.NewInt64Coin(stakeDenom, 100)}
 	bacc := auth.NewBaseAccountWithAddress(testAddr)
 	bacc.SetCoins(origCoins)
+
+	invAcc := NewValidatorVestingAccount(&bacc, now.Unix(), periods, testConsAddr, nil, 100)
+	invAcc.DebtAfterFailedVesting = sdk.Coins{sdk.Coin{Denom: "KAVA", Amount: sdk.OneInt()}}
+
 	tests := []struct {
-		name   string
-		acc    authexported.GenesisAccount
-		expErr error
+		name    string
+		acc     authexported.GenesisAccount
+		expPass bool
 	}{
 		{
 			"valid validator vesting account",
 			NewValidatorVestingAccount(&bacc, now.Unix(), periods, testConsAddr, nil, 100),
-			nil,
+			true,
+		},
+		{
+			"empty validator address",
+			NewValidatorVestingAccount(&bacc, now.Unix(), periods, nil, nil, 100),
+			false,
 		},
 		{
 			"invalid signing threshold",
 			NewValidatorVestingAccount(&bacc, now.Unix(), periods, testConsAddr, nil, -1),
-			errors.New("signing threshold must be between 0 and 100"),
+			false,
 		},
 		{
 			"invalid signing threshold",
 			NewValidatorVestingAccount(&bacc, now.Unix(), periods, testConsAddr, nil, 120),
-			errors.New("signing threshold must be between 0 and 100"),
+			false,
 		},
 		{
 			"invalid return address",
 			NewValidatorVestingAccount(&bacc, now.Unix(), periods, testConsAddr, testAddr, 90),
-			errors.New("return address cannot be the same as the account address"),
+			false,
+		},
+		{
+			"invalid debt after failed vesting ",
+			invAcc,
+			false,
 		},
 	}
 	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			err := tt.acc.Validate()
-			require.Equal(t, tt.expErr, err)
-		})
+		err := tt.acc.Validate()
+		if tt.expPass {
+			require.NoError(t, err, tt.name)
+		} else {
+			require.Error(t, err, tt.name)
+		}
 	}
 }