feat: decouple x/params from simapp
#12259
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| package sims | ||
|
|
||
| // Default simulation operation weights for messages and gov proposals | ||
| const ( | ||
| DefaultWeightMsgSend int = 100 | ||
| DefaultWeightMsgMultiSend int = 10 | ||
| DefaultWeightMsgSetWithdrawAddress int = 50 | ||
| DefaultWeightMsgWithdrawDelegationReward int = 50 | ||
| DefaultWeightMsgWithdrawValidatorCommission int = 50 | ||
| DefaultWeightMsgFundCommunityPool int = 50 | ||
| DefaultWeightMsgDeposit int = 100 | ||
| DefaultWeightMsgVote int = 67 | ||
| DefaultWeightMsgVoteWeighted int = 33 | ||
| DefaultWeightMsgUnjail int = 100 | ||
| DefaultWeightMsgCreateValidator int = 100 | ||
| DefaultWeightMsgEditValidator int = 5 | ||
| DefaultWeightMsgDelegate int = 100 | ||
| DefaultWeightMsgUndelegate int = 100 | ||
| DefaultWeightMsgBeginRedelegate int = 100 | ||
| DefaultWeightMsgCancelUnbondingDelegation int = 100 | ||
|
|
||
| DefaultWeightCommunitySpendProposal int = 5 | ||
| DefaultWeightTextProposal int = 5 | ||
| DefaultWeightParamChangeProposal int = 5 | ||
|
|
||
| // feegrant | ||
| DefaultWeightGrantAllowance int = 100 | ||
| DefaultWeightRevokeAllowance int = 100 | ||
| ) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,28 +7,36 @@ import ( | |
|
|
||
| tmproto "github.com/tendermint/tendermint/proto/tendermint/types" | ||
|
|
||
| simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" | ||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||
| govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" | ||
| "github.com/cosmos/cosmos-sdk/x/params" | ||
| "github.com/cosmos/cosmos-sdk/x/params/keeper" | ||
| "github.com/cosmos/cosmos-sdk/x/params/testutil" | ||
| "github.com/cosmos/cosmos-sdk/x/params/types/proposal" | ||
| stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper" | ||
| stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" | ||
| ) | ||
|
|
||
| type HandlerTestSuite struct { | ||
| suite.Suite | ||
|
|
||
| ctx sdk.Context | ||
| govHandler govv1beta1.Handler | ||
| stakingKeeper *stakingkeeper.Keeper | ||
| } | ||
|
|
||
| func (suite *HandlerTestSuite) SetupTest() { | ||
| var paramsKeeper keeper.Keeper | ||
| app, err := simtestutil.Setup( | ||
| testutil.AppConfig, | ||
| ¶msKeeper, | ||
| &suite.stakingKeeper, | ||
| ) | ||
| suite.Require().NoError(err) | ||
|
|
||
| suite.ctx = app.BaseApp.NewContext(false, tmproto.Header{}) | ||
| suite.govHandler = params.NewParamChangeProposalHandler(paramsKeeper) | ||
| } | ||
|
|
||
| func TestHandlerTestSuite(t *testing.T) { | ||
|
|
@@ -50,7 +58,7 @@ func (suite *HandlerTestSuite) TestProposalHandler() { | |
| "all fields", | ||
| testProposal(proposal.NewParamChange(stakingtypes.ModuleName, string(stakingtypes.KeyMaxValidators), "1")), | ||
| func() { | ||
| maxVals := suite.stakingKeeper.MaxValidators(suite.ctx) | ||
| suite.Require().Equal(uint32(1), maxVals) | ||
| }, | ||
| false, | ||
|
|
@@ -61,23 +69,23 @@ func (suite *HandlerTestSuite) TestProposalHandler() { | |
| func() {}, | ||
| true, | ||
| }, | ||
| // { | ||
|
There was a problem hiding this comment. @aaronc Shouldn't we use another There was a problem hiding this comment. Not really following. Can you explain more? There was a problem hiding this comment. What I meant is from my understanding the app.yaml of a module should be minimalistic. That means only requiring modules that are dependencies or strictly required for running the module app. Here we need to add as well the gov module which isn't a dependency of params. There was a problem hiding this comment. That makes sense but seems like the tests were written in a tightly coupled way so they depend on gov... We'll need to do some refactoring of tests probably |
||
| // "omit empty fields", | ||
| // testProposal(proposal.ParamChange{ | ||
| // Subspace: govtypes.ModuleName, | ||
| // Key: string(govv1.ParamStoreKeyDepositParams), | ||
| // Value: `{"min_deposit": [{"denom": "uatom","amount": "64000000"}], "max_deposit_period": "172800000000000"}`, | ||
| // }), | ||
| // func() { | ||
| // depositParams := suite.app.GovKeeper.GetDepositParams(suite.ctx) | ||
| // defaultPeriod := govv1.DefaultPeriod | ||
| // suite.Require().Equal(govv1.DepositParams{ | ||
| // MinDeposit: sdk.NewCoins(sdk.NewCoin("uatom", sdk.NewInt(64000000))), | ||
| // MaxDepositPeriod: &defaultPeriod, | ||
| // }, depositParams) | ||
| // }, | ||
| // false, | ||
| // }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
|
|
||
| # App Wiring | ||
|
|
||
| The minimal app-wiring configuration for `x/params` is as follows: | ||
|
|
||
| +++ https://github.com/cosmos/cosmos-sdk/blob/main/x/params/testutil/app.yaml |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| modules: | ||
| - name: runtime | ||
| config: | ||
| "@type": cosmos.app.runtime.v1alpha1.Module | ||
|
|
||
| app_name: ParamsApp | ||
|
|
||
| begin_blockers: [staking, auth, bank, genutil, params] | ||
| end_blockers: [staking, auth, bank, genutil, params] | ||
| init_genesis: [auth, bank, staking, genutil, params] | ||
|
|
||
| - name: auth | ||
| config: | ||
| "@type": cosmos.auth.module.v1.Module | ||
| bech32_prefix: cosmos | ||
| module_account_permissions: | ||
| - account: fee_collector | ||
| - account: bonded_tokens_pool | ||
| permissions: [burner, staking] | ||
| - account: not_bonded_tokens_pool | ||
| permissions: [burner, staking] | ||
|
|
||
| - name: bank | ||
| config: | ||
| "@type": cosmos.bank.module.v1.Module | ||
|
|
||
| - name: params | ||
| config: | ||
| "@type": cosmos.params.module.v1.Module | ||
|
|
||
| - name: tx | ||
| config: | ||
| "@type": cosmos.tx.module.v1.Module | ||
|
|
||
| - name: staking | ||
| config: | ||
| "@type": cosmos.staking.module.v1.Module | ||
|
|
||
| - name: genutil | ||
| config: | ||
| "@type": cosmos.genutil.module.v1.Module |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im a bit worried about these direct dependencies. It could make versioning harder for us and users. Not sure alternative solutions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There shouldn't be a problem using an expected keeper interface instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think sometimes with expected keepers we still need to import the types. I am not sure it solve that concern then.
It seems it does here tho as the input is only a context for staking, so I will fix that.
However for gov it won't work as we import types as well.