Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: decouple x/params from simapp #12259

Merged
merged 6 commits into from
Jun 15, 2022
Merged

feat: decouple x/params from simapp #12259

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
263fa5e
feat: decouple `x/params` from simapp
julienrbrt Jun 14, 2022
5c3e288
updates
julienrbrt Jun 14, 2022
3ec8249
updates
julienrbrt Jun 14, 2022
5aabf9c
Merge branch 'main' into julien/params-decouple-simapp
julienrbrt Jun 15, 2022
ad0a1db
use staking keeper
julienrbrt Jun 15, 2022
295b617
Merge branch 'main' into julien/params-decouple-simapp
tac0turtle Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion simapp/params/weights.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const (

DefaultWeightCommunitySpendProposal int = 5
DefaultWeightTextProposal int = 5
DefaultWeightParamChangeProposal int = 5

// feegrant
DefaultWeightGrantAllowance int = 100
Expand Down
29 changes: 29 additions & 0 deletions testutil/sims/weights.go
View file
Open in desktop
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
)
4 changes: 2 additions & 2 deletions x/nft/testutil/app.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ modules:

app_name: NFTApp

begin_blockers: [mint, staking, auth, bank, mint, genutil, nft, params]
end_blockers: [mint, staking, auth, bank, mint, genutil, nft, params]
begin_blockers: [staking, auth, bank, mint, genutil, nft, params]
end_blockers: [staking, auth, bank, mint, genutil, nft, params]
init_genesis: [auth, bank, staking, mint, genutil, nft, params]

- name: auth
Expand Down
9 changes: 6 additions & 3 deletions x/params/client/testutil/cli_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@ package testutil
import (
"testing"

"github.com/cosmos/cosmos-sdk/testutil/network"

"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

"github.com/cosmos/cosmos-sdk/testutil/network"
"github.com/cosmos/cosmos-sdk/x/params/testutil"
)

func TestIntegrationTestSuite(t *testing.T) {
cfg := network.DefaultConfig()
cfg, err := network.DefaultConfigWithAppConfig(testutil.AppConfig)
require.NoError(t, err)
cfg.NumValidators = 1
suite.Run(t, NewIntegrationTestSuite(cfg))
}
15 changes: 10 additions & 5 deletions x/params/keeper/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,25 @@ package keeper_test

import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/simapp"
"github.com/cosmos/cosmos-sdk/depinject"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
"github.com/cosmos/cosmos-sdk/testutil"
sdktestutil "github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
paramskeeper "github.com/cosmos/cosmos-sdk/x/params/keeper"
"github.com/cosmos/cosmos-sdk/x/params/testutil"
)

func testComponents() (*codec.LegacyAmino, sdk.Context, storetypes.StoreKey, storetypes.StoreKey, paramskeeper.Keeper) {
marshaler := simapp.MakeTestEncodingConfig().Codec
var cdc codec.Codec
if err := depinject.Inject(testutil.AppConfig, &cdc); err != nil {
panic(err)
}

legacyAmino := createTestCodec()
mkey := sdk.NewKVStoreKey("test")
tkey := sdk.NewTransientStoreKey("transient_test")
ctx := testutil.DefaultContext(mkey, tkey)
keeper := paramskeeper.NewKeeper(marshaler, legacyAmino, mkey, tkey)
ctx := sdktestutil.DefaultContext(mkey, tkey)
keeper := paramskeeper.NewKeeper(cdc, legacyAmino, mkey, tkey)

return legacyAmino, ctx, mkey, tkey, keeper
}
Expand Down
2 changes: 1 addition & 1 deletion x/params/keeper/grpc_query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (suite *KeeperTestSuite) TestGRPCQueryParams() {
{
"success",
func() {
space = suite.app.ParamsKeeper.Subspace("test").
space = suite.paramsKeeper.Subspace("test").
WithKeyTable(types.NewKeyTable(types.NewParamSetPair(key, paramJSON{}, validateNoOp)))
req = &proposal.QueryParamsRequest{Subspace: "test", Key: "key"}
expValue = ""
Expand Down
26 changes: 17 additions & 9 deletions x/params/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,36 @@ import (
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/simapp"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/store/prefix"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/params/keeper"
"github.com/cosmos/cosmos-sdk/x/params/testutil"
"github.com/cosmos/cosmos-sdk/x/params/types"
"github.com/cosmos/cosmos-sdk/x/params/types/proposal"
)

type KeeperTestSuite struct {
suite.Suite

app *simapp.SimApp
ctx sdk.Context

queryClient proposal.QueryClient
ctx sdk.Context
paramsKeeper keeper.Keeper
queryClient proposal.QueryClient
}

func (suite *KeeperTestSuite) SetupTest() {
suite.app = simapp.Setup(suite.T(), false)
suite.ctx = suite.app.BaseApp.NewContext(false, tmproto.Header{})
var interfaceRegistry codectypes.InterfaceRegistry

app, err := simtestutil.Setup(
testutil.AppConfig,
&suite.paramsKeeper,
)
suite.Require().NoError(err)

queryHelper := baseapp.NewQueryServerTestHelper(suite.ctx, suite.app.InterfaceRegistry())
proposal.RegisterQueryServer(queryHelper, suite.app.ParamsKeeper)
suite.ctx = app.BaseApp.NewContext(false, tmproto.Header{})
queryHelper := baseapp.NewQueryServerTestHelper(suite.ctx, interfaceRegistry)
proposal.RegisterQueryServer(queryHelper, suite.paramsKeeper)
suite.queryClient = proposal.NewQueryClient(queryHelper)
}

Expand Down
35 changes: 26 additions & 9 deletions x/params/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ func (AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.Validato
return []abci.ValidatorUpdate{}
}

//
// New App Wiring Setup
//

func init() {
appmodule.Register(&modulev1.Module{},
appmodule.Provide(
Expand All @@ -168,18 +172,31 @@ func provideModuleBasic() runtime.AppModuleBasicWrapper {
return runtime.WrapAppModuleBasic(AppModuleBasic{})
}

func provideModule(
kvStoreKey *store.KVStoreKey,
transientStoreKey *store.TransientStoreKey,
cdc codec.Codec,
amino *codec.LegacyAmino,
) (keeper.Keeper, runtime.AppModuleWrapper, runtime.BaseAppOption) {
k := keeper.NewKeeper(cdc, amino, kvStoreKey, transientStoreKey)
m := NewAppModule(k)
type paramsInputs struct {
depinject.In

KvStoreKey *store.KVStoreKey
TransientStoreKey *store.TransientStoreKey
Cdc codec.Codec
LegacyAmino *codec.LegacyAmino
}

type paramsOutputs struct {
depinject.Out

ParamsKeeper keeper.Keeper
BaseAppOption runtime.BaseAppOption
Module runtime.AppModuleWrapper
}

func provideModule(in paramsInputs) paramsOutputs {
k := keeper.NewKeeper(in.Cdc, in.LegacyAmino, in.KvStoreKey, in.TransientStoreKey)
baseappOpt := func(app *baseapp.BaseApp) {
app.SetParamStore(k.Subspace(baseapp.Paramspace).WithKeyTable(types.ConsensusParamsKeyTable()))
}
return k, runtime.WrapAppModule(m), baseappOpt
m := runtime.WrapAppModule(NewAppModule(k))

return paramsOutputs{ParamsKeeper: k, BaseAppOption: baseappOpt, Module: m}
}

func provideSubSpace(key depinject.ModuleKey, k keeper.Keeper) types.Subspace {
Expand Down
66 changes: 39 additions & 27 deletions x/params/proposal_handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,40 @@ import (

tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

"github.com/cosmos/cosmos-sdk/simapp"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
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"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

// StakingKeeper defines the expected staking keeper
type StakingKeeper interface {
MaxValidators(ctx sdk.Context) (res uint32)
}

type HandlerTestSuite struct {
suite.Suite

app *simapp.SimApp
ctx sdk.Context
govHandler govv1beta1.Handler
ctx sdk.Context
govHandler govv1beta1.Handler
stakingKeeper StakingKeeper
}

func (suite *HandlerTestSuite) SetupTest() {
suite.app = simapp.Setup(suite.T(), false)
suite.ctx = suite.app.BaseApp.NewContext(false, tmproto.Header{})
suite.govHandler = params.NewParamChangeProposalHandler(suite.app.ParamsKeeper)
var paramsKeeper keeper.Keeper
app, err := simtestutil.Setup(
testutil.AppConfig,
&paramsKeeper,
&suite.stakingKeeper,
)
suite.Require().NoError(err)

suite.ctx = app.BaseApp.NewContext(false, tmproto.Header{})
suite.govHandler = params.NewParamChangeProposalHandler(paramsKeeper)
}

func TestHandlerTestSuite(t *testing.T) {
Expand All @@ -50,7 +62,7 @@ func (suite *HandlerTestSuite) TestProposalHandler() {
"all fields",
testProposal(proposal.NewParamChange(stakingtypes.ModuleName, string(stakingtypes.KeyMaxValidators), "1")),
func() {
maxVals := suite.app.StakingKeeper.MaxValidators(suite.ctx)
maxVals := suite.stakingKeeper.MaxValidators(suite.ctx)
suite.Require().Equal(uint32(1), maxVals)
},
false,
Expand All @@ -61,23 +73,23 @@ func (suite *HandlerTestSuite) TestProposalHandler() {
func() {},
true,
},
{
"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,
},
// {
Copy link
Member Author

@julienrbrt julienrbrt Jun 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaronc Shouldn't we use another app.yaml for this specific test? I feel like the default app.yaml from testutil should contain the minimum modules required for the tested specific module. Here, gov does not fall into this category.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really following. Can you explain more?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {
Expand Down
4 changes: 2 additions & 2 deletions x/params/simulation/proposals.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package simulation

import (
simappparams "github.com/cosmos/cosmos-sdk/simapp/params"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
"github.com/cosmos/cosmos-sdk/x/simulation"
)
Expand All @@ -14,7 +14,7 @@ func ProposalContents(paramChanges []simtypes.ParamChange) []simtypes.WeightedPr
return []simtypes.WeightedProposalContent{
simulation.NewWeightedProposalContent(
OpWeightSubmitParamChangeProposal,
simappparams.DefaultWeightParamChangeProposal,
simtestutil.DefaultWeightParamChangeProposal,
SimulateParamChangeProposalContent(paramChanges),
),
}
Expand Down
4 changes: 2 additions & 2 deletions x/params/simulation/proposals_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/stretchr/testify/require"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

simappparams "github.com/cosmos/cosmos-sdk/simapp/params"
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims"
sdk "github.com/cosmos/cosmos-sdk/types"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
"github.com/cosmos/cosmos-sdk/x/params/simulation"
Expand All @@ -32,7 +32,7 @@ func TestProposalContents(t *testing.T) {

// tests w0 interface:
require.Equal(t, simulation.OpWeightSubmitParamChangeProposal, w0.AppParamsKey())
require.Equal(t, simappparams.DefaultWeightParamChangeProposal, w0.DefaultWeight())
require.Equal(t, simtestutil.DefaultWeightParamChangeProposal, w0.DefaultWeight())

content := w0.ContentSimulatorFn()(r, ctx, accounts)

Expand Down
6 changes: 6 additions & 0 deletions x/params/spec/03_app_wiring.md
View file
Open in desktop
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
1 change: 1 addition & 0 deletions x/params/spec/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ The following contents explains how to use params module for master and user mod
* [Key](02_subspace.md#key)
* [KeyTable](02_subspace.md#keytable)
* [ParamSet](02_subspace.md#paramset)
3. * **[App-Wiring](03_app_wiring.md)**
41 changes: 41 additions & 0 deletions x/params/testutil/app.yaml
View file
Open in desktop
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
Loading