Skip to content

Commit 042ea18

Browse files
authored
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
1 parent 4214e66 commit 042ea18

File tree

5 files changed

+133
-5
lines changed

5 files changed

+133
-5
lines changed

simapp/app.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ func NewSimApp(
310310
//TODO: we may need to add acl gov proposal types here
311311
govKeeper := govkeeper.NewKeeper(
312312
appCodec, keys[govtypes.StoreKey], app.GetSubspace(govtypes.ModuleName), app.AccountKeeper, app.BankKeeper,
313-
&stakingKeeper, govRouter,
313+
&stakingKeeper, app.ParamsKeeper, govRouter,
314314
)
315315

316316
app.GovKeeper = *govKeeper.SetHooks(

x/gov/keeper/keeper.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@ type Keeper struct {
1717
// The reference to the Paramstore to get and set gov specific params
1818
paramSpace types.ParamSubspace
1919

20-
authKeeper types.AccountKeeper
21-
bankKeeper types.BankKeeper
20+
authKeeper types.AccountKeeper
21+
bankKeeper types.BankKeeper
22+
paramsKeeper types.ParamsKeeper
2223

2324
// The reference to the DelegationSet and ValidatorSet to get information about validators and delegators
2425
sk types.StakingKeeper
@@ -45,7 +46,8 @@ type Keeper struct {
4546
// CONTRACT: the parameter Subspace must have the param key table already initialized
4647
func NewKeeper(
4748
cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace types.ParamSubspace,
48-
authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, sk types.StakingKeeper, rtr types.Router,
49+
authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, sk types.StakingKeeper,
50+
paramsKeeper types.ParamsKeeper, rtr types.Router,
4951
) Keeper {
5052

5153
// ensure governance module account is set
@@ -66,6 +68,7 @@ func NewKeeper(
6668
sk: sk,
6769
cdc: cdc,
6870
router: rtr,
71+
paramsKeeper: paramsKeeper,
6972
}
7073
}
7174

x/gov/keeper/proposal.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ package keeper
22

33
import (
44
"fmt"
5-
65
"github.com/cosmos/cosmos-sdk/client"
76
sdk "github.com/cosmos/cosmos-sdk/types"
87
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
98
"github.com/cosmos/cosmos-sdk/x/gov/types"
9+
"github.com/cosmos/cosmos-sdk/x/params/types/proposal"
1010
)
1111

1212
// SubmitProposal create new proposal given a content
@@ -19,6 +19,25 @@ func (keeper Keeper) SubmitProposalWithExpedite(ctx sdk.Context, content types.C
1919
if !keeper.router.HasRoute(content.ProposalRoute()) {
2020
return types.Proposal{}, sdkerrors.Wrap(types.ErrNoProposalHandlerExists, content.ProposalRoute())
2121
}
22+
// Ensure that the parameter exists
23+
if content.ProposalType() == proposal.ProposalTypeChange {
24+
paramProposal, ok := content.(*proposal.ParameterChangeProposal)
25+
if !ok {
26+
return types.Proposal{}, sdkerrors.Wrap(types.ErrInvalidProposalContent, "proposal content is not a ParameterChangeProposal")
27+
}
28+
29+
// Validate each parameter change exists
30+
for _, change := range paramProposal.Changes {
31+
subspace, ok := keeper.paramsKeeper.GetSubspace(change.Subspace)
32+
if !ok {
33+
return types.Proposal{}, sdkerrors.Wrapf(types.ErrInvalidProposalContent, "parameter %s/%s does not exist", change.Subspace, change.Key)
34+
}
35+
validKey := subspace.Has(ctx, []byte(change.Key))
36+
if !validKey {
37+
return types.Proposal{}, sdkerrors.Wrapf(types.ErrInvalidProposalContent, "parameter %s not found in subspace %s", change.Key, change.Subspace)
38+
}
39+
}
40+
}
2241

2342
proposalID, err := keeper.GetProposalID(ctx)
2443
if err != nil {

x/gov/keeper/proposal_test.go

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package keeper_test
33
import (
44
"errors"
55
"fmt"
6+
"github.com/cosmos/cosmos-sdk/x/params/types/proposal"
67
"github.com/stretchr/testify/require"
78
"strings"
89
"testing"
@@ -167,3 +168,103 @@ func (suite *KeeperTestSuite) TestGetProposalsFiltered() {
167168
})
168169
}
169170
}
171+
172+
func (suite *KeeperTestSuite) TestParamChangeProposal() {
173+
// Simple parameter changes
174+
validStakingChange := proposal.ParamChange{
175+
Subspace: "staking",
176+
Key: "MaxValidators",
177+
Value: "100",
178+
}
179+
180+
invalidStakingChange := proposal.ParamChange{
181+
Subspace: "staking",
182+
Key: "NonExistentParam",
183+
Value: "100",
184+
}
185+
186+
// Complex gov parameter changes
187+
validVotingParamsChange := proposal.ParamChange{
188+
Subspace: "gov",
189+
Key: "votingparams",
190+
Value: `{"voting_period":"172800s","expedited_voting_period":"86400s"}`,
191+
}
192+
193+
invalidVotingParamsChange := proposal.ParamChange{
194+
Subspace: "gov",
195+
Key: "invalidvotingparams",
196+
Value: `{"voting_period":"invalid"}`, // Invalid duration
197+
}
198+
199+
testCases := map[string]struct {
200+
proposal types.Content
201+
expectError bool
202+
}{
203+
"valid staking param change": {
204+
proposal: &proposal.ParameterChangeProposal{
205+
Title: "Valid Staking Param Change",
206+
Description: "Testing valid staking param changes",
207+
Changes: []proposal.ParamChange{validStakingChange},
208+
IsExpedited: false,
209+
},
210+
expectError: false,
211+
},
212+
"invalid staking param change": {
213+
proposal: &proposal.ParameterChangeProposal{
214+
Title: "Invalid Staking Param Change",
215+
Description: "Testing invalid staking param changes",
216+
Changes: []proposal.ParamChange{invalidStakingChange},
217+
IsExpedited: false,
218+
},
219+
expectError: true,
220+
},
221+
"valid voting params change": {
222+
proposal: &proposal.ParameterChangeProposal{
223+
Title: "Valid Voting Params Change",
224+
Description: "Testing valid voting param changes",
225+
Changes: []proposal.ParamChange{validVotingParamsChange},
226+
IsExpedited: false,
227+
},
228+
expectError: false,
229+
},
230+
"invalid voting params change": {
231+
proposal: &proposal.ParameterChangeProposal{
232+
Title: "Invalid Voting Params Change",
233+
Description: "Testing invalid voting param changes",
234+
Changes: []proposal.ParamChange{invalidVotingParamsChange},
235+
IsExpedited: false,
236+
},
237+
expectError: true,
238+
},
239+
"mixed valid and invalid params": {
240+
proposal: &proposal.ParameterChangeProposal{
241+
Title: "Mixed Param Changes",
242+
Description: "Testing both valid and invalid params",
243+
Changes: []proposal.ParamChange{validStakingChange, invalidStakingChange},
244+
IsExpedited: false,
245+
},
246+
expectError: true,
247+
},
248+
"multiple valid params": {
249+
proposal: &proposal.ParameterChangeProposal{
250+
Title: "Multiple Valid Params",
251+
Description: "Testing multiple valid param changes",
252+
Changes: []proposal.ParamChange{validStakingChange, validVotingParamsChange},
253+
IsExpedited: false,
254+
},
255+
expectError: false,
256+
},
257+
}
258+
259+
for name, tc := range testCases {
260+
suite.Run(name, func() {
261+
_, err := suite.app.GovKeeper.SubmitProposal(suite.ctx, tc.proposal)
262+
if tc.expectError {
263+
suite.Require().Error(err)
264+
suite.Require().Contains(err.Error(), "parameter")
265+
} else {
266+
suite.Require().NoError(err)
267+
}
268+
})
269+
}
270+
}

x/gov/types/expected_keepers.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package types
33
import (
44
sdk "github.com/cosmos/cosmos-sdk/types"
55
"github.com/cosmos/cosmos-sdk/x/auth/types"
6+
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
67
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
78
)
89

@@ -61,3 +62,7 @@ type GovHooks interface {
6162
AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) // Must be called when proposal fails to reach min deposit
6263
AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) // Must be called when proposal's finishes it's voting period
6364
}
65+
66+
type ParamsKeeper interface {
67+
GetSubspace(s string) (paramtypes.Subspace, bool)
68+
}

0 commit comments

Comments
 (0)