Merge PR #2450: Add ValidateGenesis to staking, add more tests

PENDING.md

@@ -144,6 +144,7 @@ IMPROVEMENTS
     * [x/stake] Improve speed of GetValidator, which was shown to be a performance bottleneck. [#2046](https://github.com/tendermint/tendermint/pull/2200)
     * [x/stake] \#2435 Improve memory efficiency of getting the various store keys
     * [genesis] \#2229 Ensure that there are no duplicate accounts or validators in the genesis state.
+    * [genesis] \#2450 Validate staking genesis parameters.
     * Add SDK validation to `config.toml` (namely disabling `create_empty_blocks`) \#1571
     * \#1941(https://github.com/cosmos/cosmos-sdk/issues/1941) Version is now inferred via `git describe --tags`.
     * [x/distribution] \#1671 add distribution types and tests

cmd/gaia/app/genesis.go

@@ -13,7 +13,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/auth"
 	"github.com/cosmos/cosmos-sdk/x/gov"
 	"github.com/cosmos/cosmos-sdk/x/stake"
-	stakeTypes "github.com/cosmos/cosmos-sdk/x/stake/types"
 
 	"github.com/spf13/pflag"
 
@@ -242,29 +241,13 @@ func GaiaValidateGenesisState(genesisState GenesisState) (err error) {
 	if err != nil {
 		return
 	}
-	err = validateGenesisStateValidators(genesisState.StakeData.Validators)
+	err = stake.ValidateGenesis(genesisState.StakeData)
 	if err != nil {
 		return
 	}
 	return
 }
 
-func validateGenesisStateValidators(validators []stakeTypes.Validator) (err error) {
-	addrMap := make(map[string]bool, len(validators))
-	for i := 0; i < len(validators); i++ {
-		val := validators[i]
-		strKey := string(val.ConsPubKey.Bytes())
-		if _, ok := addrMap[strKey]; ok {
-			return fmt.Errorf("Duplicate validator in genesis state: moniker %v, Address %v", val.Description.Moniker, val.ConsAddress())
-		}
-		if val.Jailed && val.Status == sdk.Bonded {
-			return fmt.Errorf("Validator is bonded and jailed in genesis state: moniker %v, Address %v", val.Description.Moniker, val.ConsAddress())
-		}
-		addrMap[strKey] = true
-	}
-	return
-}
-
 // Ensures that there are no duplicate accounts in the genesis state,
 func validateGenesisStateAccounts(accs []GenesisAccount) (err error) {
 	addrMap := make(map[string]bool, len(accs))

x/stake/genesis.go

@@ -1,6 +1,8 @@
 package stake
 
 import (
+	"fmt"
+
 	abci "github.com/tendermint/tendermint/abci/types"
 	tmtypes "github.com/tendermint/tendermint/types"
 
@@ -75,3 +77,58 @@ func WriteValidators(ctx sdk.Context, keeper Keeper) (vals []tmtypes.GenesisVali
 
 	return
 }
+
+// ValidateGenesis validates the provided staking genesis state to ensure the
+// expected invariants holds. (i.e. params in correct bounds, no duplicate validators)
+func ValidateGenesis(data types.GenesisState) error {
+	err := validateGenesisStateValidators(data.Validators)
+	if err != nil {
+		return err
+	}
+	err = validateParams(data.Params)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func validateParams(params types.Params) error {
+	if params.GoalBonded.LTE(sdk.ZeroDec()) {
+		bondedPercent := params.GoalBonded.MulInt(sdk.NewInt(100)).String()
+		return fmt.Errorf("staking parameter GoalBonded should be positive, instead got %s percent", bondedPercent)
+	}
+	if params.GoalBonded.GT(sdk.OneDec()) {
+		bondedPercent := params.GoalBonded.MulInt(sdk.NewInt(100)).String()
+		return fmt.Errorf("staking parameter GoalBonded should be less than 100 percent, instead got %s percent", bondedPercent)
+	}
+	if params.BondDenom == "" {
+		return fmt.Errorf("staking parameter BondDenom can't be an empty string")
+	}
+	if params.InflationMax.LT(params.InflationMin) {
+		return fmt.Errorf("staking parameter Max inflation must be greater than or equal to min inflation")
+	}
+	return nil
+}
+
+func validateGenesisStateValidators(validators []types.Validator) (err error) {
+	addrMap := make(map[string]bool, len(validators))
+	for i := 0; i < len(validators); i++ {
+		val := validators[i]
+		strKey := string(val.ConsPubKey.Bytes())
+		if _, ok := addrMap[strKey]; ok {
+			return fmt.Errorf("duplicate validator in genesis state: moniker %v, Address %v", val.Description.Moniker, val.ConsAddress())
+		}
+		if val.Jailed && val.Status == sdk.Bonded {
+			return fmt.Errorf("validator is bonded and jailed in genesis state: moniker %v, Address %v", val.Description.Moniker, val.ConsAddress())
+		}
+		if val.Tokens.IsZero() {
+			return fmt.Errorf("genesis validator cannot have zero pool shares, validator: %v", val)
+		}
+		if val.DelegatorShares.IsZero() {
+			return fmt.Errorf("genesis validator cannot have zero delegator shares, validator: %v", val)
+		}
+		addrMap[strKey] = true
+	}
+	return
+}

x/stake/genesis_test.go

@@ -4,13 +4,15 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/crypto/ed25519"
 
-	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/stretchr/testify/assert"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	keep "github.com/cosmos/cosmos-sdk/x/stake/keeper"
 	"github.com/cosmos/cosmos-sdk/x/stake/types"
+	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/tendermint/abci/types"
 )
 
 func TestInitGenesis(t *testing.T) {
@@ -105,3 +107,59 @@ func TestInitGenesisLargeValidatorSet(t *testing.T) {
 
 	require.Equal(t, abcivals, vals)
 }
+
+func TestValidateGenesis(t *testing.T) {
+	genValidators1 := make([]types.Validator, 1, 5)
+	pk := ed25519.GenPrivKey().PubKey()
+	genValidators1[0] = types.NewValidator(sdk.ValAddress(pk.Address()), pk, types.NewDescription("", "", "", ""))
+	genValidators1[0].Tokens = sdk.OneDec()
+	genValidators1[0].DelegatorShares = sdk.OneDec()
+
+	tests := []struct {
+		name    string
+		mutate  func(*types.GenesisState)
+		wantErr bool
+	}{
+		{"default", func(*types.GenesisState) {}, false},
+		// validate params
+		{"200% goalbonded", func(data *types.GenesisState) { (*data).Params.GoalBonded = sdk.OneDec().Add(sdk.OneDec()) }, true},
+		{"-67% goalbonded", func(data *types.GenesisState) { (*data).Params.GoalBonded = sdk.OneDec().Neg() }, true},
+		{"no bond denom", func(data *types.GenesisState) { (*data).Params.BondDenom = "" }, true},
+		{"min inflation > max inflation", func(data *types.GenesisState) {
+			(*data).Params.InflationMin = (*data).Params.InflationMax.Add(sdk.OneDec())
+		}, true},
+		{"min inflation = max inflation", func(data *types.GenesisState) {
+			(*data).Params.InflationMax = (*data).Params.InflationMin
+		}, false},
+		// validate genesis validators
+		{"duplicate validator", func(data *types.GenesisState) {
+			(*data).Validators = genValidators1
+			(*data).Validators = append((*data).Validators, genValidators1[0])
+		}, true},
+		{"no pool shares", func(data *types.GenesisState) {
+			(*data).Validators = genValidators1
+			(*data).Validators[0].Tokens = sdk.ZeroDec()
+		}, true},
+		{"no delegator shares", func(data *types.GenesisState) {
+			(*data).Validators = genValidators1
+			(*data).Validators[0].DelegatorShares = sdk.ZeroDec()
+		}, true},
+		{"jailed and bonded validator", func(data *types.GenesisState) {
+			(*data).Validators = genValidators1
+			(*data).Validators[0].Jailed = true
+			(*data).Validators[0].Status = sdk.Bonded
+		}, true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			genesisState := types.DefaultGenesisState()
+			tt.mutate(&genesisState)
+			if tt.wantErr {
+				assert.Error(t, ValidateGenesis(genesisState))
+			} else {
+				assert.NoError(t, ValidateGenesis(genesisState))
+			}
+		})
+	}
+}