Param change verification (#551)

## Describe your changes and provide context
Ensure param changes are valid
## Testing performed to validate your change
Unit tests, tested on local chain

simapp/app.go

@@ -310,7 +310,7 @@ func NewSimApp(
 	//TODO: we may need to add acl gov proposal types here
 	govKeeper := govkeeper.NewKeeper(
 		appCodec, keys[govtypes.StoreKey], app.GetSubspace(govtypes.ModuleName), app.AccountKeeper, app.BankKeeper,
-		&stakingKeeper, govRouter,
+		&stakingKeeper, app.ParamsKeeper, govRouter,
 	)
 
 	app.GovKeeper = *govKeeper.SetHooks(

x/gov/keeper/keeper.go

@@ -17,8 +17,9 @@ type Keeper struct {
 	// The reference to the Paramstore to get and set gov specific params
 	paramSpace types.ParamSubspace
 
-	authKeeper types.AccountKeeper
-	bankKeeper types.BankKeeper
+	authKeeper   types.AccountKeeper
+	bankKeeper   types.BankKeeper
+	paramsKeeper types.ParamsKeeper
 
 	// The reference to the DelegationSet and ValidatorSet to get information about validators and delegators
 	sk types.StakingKeeper
@@ -45,7 +46,8 @@ type Keeper struct {
 // CONTRACT: the parameter Subspace must have the param key table already initialized
 func NewKeeper(
 	cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace types.ParamSubspace,
-	authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, sk types.StakingKeeper, rtr types.Router,
+	authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, sk types.StakingKeeper,
+	paramsKeeper types.ParamsKeeper, rtr types.Router,
 ) Keeper {
 
 	// ensure governance module account is set
@@ -66,6 +68,7 @@ func NewKeeper(
 		sk:         sk,
 		cdc:        cdc,
 		router:     rtr,
+		paramsKeeper: paramsKeeper,
 	}
 }
 

x/gov/keeper/proposal.go

@@ -2,11 +2,11 @@ package keeper
 
 import (
 	"fmt"
-
 	"github.com/cosmos/cosmos-sdk/client"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/x/gov/types"
+	"github.com/cosmos/cosmos-sdk/x/params/types/proposal"
 )
 
 // SubmitProposal create new proposal given a content
@@ -19,6 +19,25 @@ func (keeper Keeper) SubmitProposalWithExpedite(ctx sdk.Context, content types.C
 	if !keeper.router.HasRoute(content.ProposalRoute()) {
 		return types.Proposal{}, sdkerrors.Wrap(types.ErrNoProposalHandlerExists, content.ProposalRoute())
 	}
+	// Ensure that the parameter exists
+	if content.ProposalType() == proposal.ProposalTypeChange {
+		paramProposal, ok := content.(*proposal.ParameterChangeProposal)
+		if !ok {
+			return types.Proposal{}, sdkerrors.Wrap(types.ErrInvalidProposalContent, "proposal content is not a ParameterChangeProposal")
+		}
+
+		// Validate each parameter change exists
+		for _, change := range paramProposal.Changes {
+			subspace, ok := keeper.paramsKeeper.GetSubspace(change.Subspace)
+			if !ok {
+				return types.Proposal{}, sdkerrors.Wrapf(types.ErrInvalidProposalContent, "parameter %s/%s does not exist", change.Subspace, change.Key)
+			}
+			validKey := subspace.Has(ctx, []byte(change.Key))
+			if !validKey {
+				return types.Proposal{}, sdkerrors.Wrapf(types.ErrInvalidProposalContent, "parameter %s not found in subspace %s", change.Key, change.Subspace)
+			}
+		}
+	}
 
 	proposalID, err := keeper.GetProposalID(ctx)
 	if err != nil {

x/gov/keeper/proposal_test.go

@@ -3,6 +3,7 @@ package keeper_test
 import (
 	"errors"
 	"fmt"
+	"github.com/cosmos/cosmos-sdk/x/params/types/proposal"
 	"github.com/stretchr/testify/require"
 	"strings"
 	"testing"
@@ -167,3 +168,103 @@ func (suite *KeeperTestSuite) TestGetProposalsFiltered() {
 		})
 	}
 }
+
+func (suite *KeeperTestSuite) TestParamChangeProposal() {
+	// Simple parameter changes
+	validStakingChange := proposal.ParamChange{
+		Subspace: "staking",
+		Key:      "MaxValidators",
+		Value:    "100",
+	}
+
+	invalidStakingChange := proposal.ParamChange{
+		Subspace: "staking",
+		Key:      "NonExistentParam",
+		Value:    "100",
+	}
+
+	// Complex gov parameter changes
+	validVotingParamsChange := proposal.ParamChange{
+		Subspace: "gov",
+		Key:      "votingparams",
+		Value:    `{"voting_period":"172800s","expedited_voting_period":"86400s"}`,
+	}
+
+	invalidVotingParamsChange := proposal.ParamChange{
+		Subspace: "gov",
+		Key:      "invalidvotingparams",
+		Value:    `{"voting_period":"invalid"}`, // Invalid duration
+	}
+
+	testCases := map[string]struct {
+		proposal    types.Content
+		expectError bool
+	}{
+		"valid staking param change": {
+			proposal: &proposal.ParameterChangeProposal{
+				Title:       "Valid Staking Param Change",
+				Description: "Testing valid staking param changes",
+				Changes:     []proposal.ParamChange{validStakingChange},
+				IsExpedited: false,
+			},
+			expectError: false,
+		},
+		"invalid staking param change": {
+			proposal: &proposal.ParameterChangeProposal{
+				Title:       "Invalid Staking Param Change",
+				Description: "Testing invalid staking param changes",
+				Changes:     []proposal.ParamChange{invalidStakingChange},
+				IsExpedited: false,
+			},
+			expectError: true,
+		},
+		"valid voting params change": {
+			proposal: &proposal.ParameterChangeProposal{
+				Title:       "Valid Voting Params Change",
+				Description: "Testing valid voting param changes",
+				Changes:     []proposal.ParamChange{validVotingParamsChange},
+				IsExpedited: false,
+			},
+			expectError: false,
+		},
+		"invalid voting params change": {
+			proposal: &proposal.ParameterChangeProposal{
+				Title:       "Invalid Voting Params Change",
+				Description: "Testing invalid voting param changes",
+				Changes:     []proposal.ParamChange{invalidVotingParamsChange},
+				IsExpedited: false,
+			},
+			expectError: true,
+		},
+		"mixed valid and invalid params": {
+			proposal: &proposal.ParameterChangeProposal{
+				Title:       "Mixed Param Changes",
+				Description: "Testing both valid and invalid params",
+				Changes:     []proposal.ParamChange{validStakingChange, invalidStakingChange},
+				IsExpedited: false,
+			},
+			expectError: true,
+		},
+		"multiple valid params": {
+			proposal: &proposal.ParameterChangeProposal{
+				Title:       "Multiple Valid Params",
+				Description: "Testing multiple valid param changes",
+				Changes:     []proposal.ParamChange{validStakingChange, validVotingParamsChange},
+				IsExpedited: false,
+			},
+			expectError: false,
+		},
+	}
+
+	for name, tc := range testCases {
+		suite.Run(name, func() {
+			_, err := suite.app.GovKeeper.SubmitProposal(suite.ctx, tc.proposal)
+			if tc.expectError {
+				suite.Require().Error(err)
+				suite.Require().Contains(err.Error(), "parameter")
+			} else {
+				suite.Require().NoError(err)
+			}
+		})
+	}
+}

x/gov/types/expected_keepers.go

@@ -3,6 +3,7 @@ package types
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/auth/types"
+	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 )
 
@@ -61,3 +62,7 @@ type GovHooks interface {
 	AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64)                      // Must be called when proposal fails to reach min deposit
 	AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64)                     // Must be called when proposal's finishes it's voting period
 }
+
+type ParamsKeeper interface {
+	GetSubspace(s string) (paramtypes.Subspace, bool)
+}