From 000f9c50035c1a855f4ef9484bc63c039d043794 Mon Sep 17 00:00:00 2001 From: Matthew Witkowski Date: Thu, 23 May 2024 10:54:44 -0400 Subject: [PATCH 1/4] Update ica simulated proposals to check which modules are instantianted. --- modules/apps/27-interchain-accounts/module.go | 4 +- .../simulation/proposals.go | 18 +++-- .../simulation/proposals_test.go | 75 +++++++++++++------ 3 files changed, 66 insertions(+), 31 deletions(-) diff --git a/modules/apps/27-interchain-accounts/module.go b/modules/apps/27-interchain-accounts/module.go index 62e2c3e0333..d3b7d00d1e1 100644 --- a/modules/apps/27-interchain-accounts/module.go +++ b/modules/apps/27-interchain-accounts/module.go @@ -195,8 +195,8 @@ func (AppModule) GenerateGenesisState(simState *module.SimulationState) { } // ProposalMsgs returns msgs used for governance proposals for simulations. -func (AppModule) ProposalMsgs(simState module.SimulationState) []simtypes.WeightedProposalMsg { - return simulation.ProposalMsgs() +func (am AppModule) ProposalMsgs(simState module.SimulationState) []simtypes.WeightedProposalMsg { + return simulation.ProposalMsgs(am.controllerKeeper, am.hostKeeper) } // WeightedOperations is unimplemented. diff --git a/modules/apps/27-interchain-accounts/simulation/proposals.go b/modules/apps/27-interchain-accounts/simulation/proposals.go index cc12948ef27..7437a2a1825 100644 --- a/modules/apps/27-interchain-accounts/simulation/proposals.go +++ b/modules/apps/27-interchain-accounts/simulation/proposals.go @@ -8,7 +8,9 @@ import ( simtypes "github.com/cosmos/cosmos-sdk/types/simulation" "github.com/cosmos/cosmos-sdk/x/simulation" + controllerkeeper "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/keeper" controllertypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types" + hostkeeper "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/keeper" "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types" ) @@ -20,19 +22,23 @@ const ( ) // ProposalMsgs defines the module weighted proposals' contents -func ProposalMsgs() []simtypes.WeightedProposalMsg { - return []simtypes.WeightedProposalMsg{ - simulation.NewWeightedProposalMsg( +func ProposalMsgs(controllerKeeper *controllerkeeper.Keeper, hostKeeper *hostkeeper.Keeper) []simtypes.WeightedProposalMsg { + msgs := make([]simtypes.WeightedProposalMsg, 0, 2) + if hostKeeper != nil { + msgs = append(msgs, simulation.NewWeightedProposalMsg( OpWeightMsgUpdateParams, DefaultWeightMsgUpdateParams, SimulateHostMsgUpdateParams, - ), - simulation.NewWeightedProposalMsg( + )) + } + if controllerKeeper != nil { + msgs = append(msgs, simulation.NewWeightedProposalMsg( OpWeightMsgUpdateParams, DefaultWeightMsgUpdateParams, SimulateControllerMsgUpdateParams, - ), + )) } + return msgs } // SimulateHostMsgUpdateParams returns a MsgUpdateParams for the host module diff --git a/modules/apps/27-interchain-accounts/simulation/proposals_test.go b/modules/apps/27-interchain-accounts/simulation/proposals_test.go index 29d8eae0ebf..216a0f3ad96 100644 --- a/modules/apps/27-interchain-accounts/simulation/proposals_test.go +++ b/modules/apps/27-interchain-accounts/simulation/proposals_test.go @@ -12,7 +12,9 @@ import ( cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" + controllerkeeper "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/keeper" controllertypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types" + hostkeeper "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/keeper" "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types" "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/simulation" ) @@ -25,32 +27,59 @@ func TestProposalMsgs(t *testing.T) { ctx := sdk.NewContext(nil, cmtproto.Header{}, true, nil) accounts := simtypes.RandomAccounts(r, 3) - // execute ProposalMsgs function - weightedProposalMsgs := simulation.ProposalMsgs() - require.Equal(t, 2, len(weightedProposalMsgs)) - w0 := weightedProposalMsgs[0] + tests := []struct { + controller *controllerkeeper.Keeper + host *hostkeeper.Keeper + proposalMsgs int + isHost []bool + }{ + { + controller: &controllerkeeper.Keeper{}, + host: &hostkeeper.Keeper{}, + proposalMsgs: 2, + isHost: []bool{true, false}, + }, + { + controller: nil, + host: nil, + proposalMsgs: 0, + }, + { + controller: &controllerkeeper.Keeper{}, + proposalMsgs: 1, + isHost: []bool{false}, + }, + { + host: &hostkeeper.Keeper{}, + proposalMsgs: 1, + isHost: []bool{true}, + }, + } - // tests w0 interface: - require.Equal(t, simulation.OpWeightMsgUpdateParams, w0.AppParamsKey()) - require.Equal(t, simulation.DefaultWeightMsgUpdateParams, w0.DefaultWeight()) + for _, test := range tests { + // execute ProposalMsgs function + weightedProposalMsgs := simulation.ProposalMsgs(test.controller, test.host) + require.Equal(t, test.proposalMsgs, len(weightedProposalMsgs)) - msg := w0.MsgSimulatorFn()(r, ctx, accounts) - msgUpdateHostParams, ok := msg.(*types.MsgUpdateParams) - require.True(t, ok) + for idx, weightedMsg := range weightedProposalMsgs { + // tests weighted interface: + require.Equal(t, simulation.OpWeightMsgUpdateParams, weightedMsg.AppParamsKey()) + require.Equal(t, simulation.DefaultWeightMsgUpdateParams, weightedMsg.DefaultWeight()) - require.Equal(t, sdk.AccAddress(address.Module("gov")).String(), msgUpdateHostParams.Signer) - require.Equal(t, msgUpdateHostParams.Params.HostEnabled, false) + msg := weightedMsg.MsgSimulatorFn()(r, ctx, accounts) + if test.isHost[idx] { + msgUpdateHostParams, ok := msg.(*types.MsgUpdateParams) + require.True(t, ok) - w1 := weightedProposalMsgs[1] + require.Equal(t, sdk.AccAddress(address.Module("gov")).String(), msgUpdateHostParams.Signer) + require.Equal(t, msgUpdateHostParams.Params.HostEnabled, false) + } else { + msgUpdateControllerParams, ok := msg.(*controllertypes.MsgUpdateParams) + require.True(t, ok) - // tests w1 interface: - require.Equal(t, simulation.OpWeightMsgUpdateParams, w1.AppParamsKey()) - require.Equal(t, simulation.DefaultWeightMsgUpdateParams, w1.DefaultWeight()) - - msg1 := w1.MsgSimulatorFn()(r, ctx, accounts) - msgUpdateControllerParams, ok := msg1.(*controllertypes.MsgUpdateParams) - require.True(t, ok) - - require.Equal(t, sdk.AccAddress(address.Module("gov")).String(), msgUpdateControllerParams.Signer) - require.Equal(t, msgUpdateControllerParams.Params.ControllerEnabled, false) + require.Equal(t, sdk.AccAddress(address.Module("gov")).String(), msgUpdateControllerParams.Signer) + require.Equal(t, msgUpdateControllerParams.Params.ControllerEnabled, false) + } + } + } } From 13640e20476fa118cda5dcf66c75101ad4b0f216 Mon Sep 17 00:00:00 2001 From: Matthew Witkowski Date: Thu, 23 May 2024 12:19:00 -0400 Subject: [PATCH 2/4] Update CHANGELOG. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9299281e19..c59e9452511 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (apps/27-interchain-accounts) [\#6377](https://github.com/cosmos/ibc-go/pull/6377) Generate ICA simtest proposals only for provided keepers. + ## [v8.3.0](https://github.com/cosmos/ibc-go/releases/tag/v8.3.0) - 2024-05-16 ### Dependencies From d7022ec7d4106329b77ffa26abe2500e3d6f0861 Mon Sep 17 00:00:00 2001 From: Matthew Witkowski Date: Thu, 6 Jun 2024 10:09:07 -0400 Subject: [PATCH 3/4] Remove isHost from tests and use reflection to determine correct branch. Add expMsgs since we know the expected messages before hand. --- .../simulation/proposals_test.go | 98 +++++++++++-------- 1 file changed, 58 insertions(+), 40 deletions(-) diff --git a/modules/apps/27-interchain-accounts/simulation/proposals_test.go b/modules/apps/27-interchain-accounts/simulation/proposals_test.go index 216a0f3ad96..0a0f650f5a4 100644 --- a/modules/apps/27-interchain-accounts/simulation/proposals_test.go +++ b/modules/apps/27-interchain-accounts/simulation/proposals_test.go @@ -2,6 +2,7 @@ package simulation_test import ( "math/rand" + "reflect" "testing" "github.com/stretchr/testify/require" @@ -15,7 +16,7 @@ import ( controllerkeeper "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/keeper" controllertypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types" hostkeeper "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/keeper" - "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types" + hosttypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types" "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/simulation" ) @@ -28,58 +29,75 @@ func TestProposalMsgs(t *testing.T) { accounts := simtypes.RandomAccounts(r, 3) tests := []struct { - controller *controllerkeeper.Keeper - host *hostkeeper.Keeper - proposalMsgs int - isHost []bool + name string + controller *controllerkeeper.Keeper + host *hostkeeper.Keeper + expMsgs []interface{} }{ { - controller: &controllerkeeper.Keeper{}, - host: &hostkeeper.Keeper{}, - proposalMsgs: 2, - isHost: []bool{true, false}, + name: "host and controller keepers are both enabled", + controller: &controllerkeeper.Keeper{}, + host: &hostkeeper.Keeper{}, + expMsgs: []interface{}{ + hosttypes.MsgUpdateParams{ + Signer: sdk.AccAddress(address.Module("gov")).String(), + Params: hosttypes.NewParams(false, []string{hosttypes.AllowAllHostMsgs}), + }, + controllertypes.MsgUpdateParams{ + Signer: sdk.AccAddress(address.Module("gov")).String(), + Params: controllertypes.NewParams(false), + }, + }, }, { - controller: nil, - host: nil, - proposalMsgs: 0, + name: "host and controller keepers are not enabled", + controller: nil, + host: nil, }, { - controller: &controllerkeeper.Keeper{}, - proposalMsgs: 1, - isHost: []bool{false}, + name: "only controller keeper is enabled", + controller: &controllerkeeper.Keeper{}, + expMsgs: []interface{}{ + controllertypes.MsgUpdateParams{ + Signer: sdk.AccAddress(address.Module("gov")).String(), + Params: controllertypes.NewParams(false), + }, + }, }, { - host: &hostkeeper.Keeper{}, - proposalMsgs: 1, - isHost: []bool{true}, + name: "only host keeper is enabled", + host: &hostkeeper.Keeper{}, + expMsgs: []interface{}{ + hosttypes.MsgUpdateParams{ + Signer: sdk.AccAddress(address.Module("gov")).String(), + Params: hosttypes.NewParams(false, []string{hosttypes.AllowAllHostMsgs}), + }, + }, }, } - for _, test := range tests { - // execute ProposalMsgs function - weightedProposalMsgs := simulation.ProposalMsgs(test.controller, test.host) - require.Equal(t, test.proposalMsgs, len(weightedProposalMsgs)) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // execute ProposalMsgs function + weightedProposalMsgs := simulation.ProposalMsgs(tc.controller, tc.host) + require.Equal(t, len(tc.expMsgs), len(weightedProposalMsgs)) - for idx, weightedMsg := range weightedProposalMsgs { - // tests weighted interface: - require.Equal(t, simulation.OpWeightMsgUpdateParams, weightedMsg.AppParamsKey()) - require.Equal(t, simulation.DefaultWeightMsgUpdateParams, weightedMsg.DefaultWeight()) + for idx, weightedMsg := range weightedProposalMsgs { + // tests weighted interface: + require.Equal(t, simulation.OpWeightMsgUpdateParams, weightedMsg.AppParamsKey()) + require.Equal(t, simulation.DefaultWeightMsgUpdateParams, weightedMsg.DefaultWeight()) - msg := weightedMsg.MsgSimulatorFn()(r, ctx, accounts) - if test.isHost[idx] { - msgUpdateHostParams, ok := msg.(*types.MsgUpdateParams) - require.True(t, ok) - - require.Equal(t, sdk.AccAddress(address.Module("gov")).String(), msgUpdateHostParams.Signer) - require.Equal(t, msgUpdateHostParams.Params.HostEnabled, false) - } else { - msgUpdateControllerParams, ok := msg.(*controllertypes.MsgUpdateParams) - require.True(t, ok) - - require.Equal(t, sdk.AccAddress(address.Module("gov")).String(), msgUpdateControllerParams.Signer) - require.Equal(t, msgUpdateControllerParams.Params.ControllerEnabled, false) + msg := weightedMsg.MsgSimulatorFn()(r, ctx, accounts) + if reflect.TypeOf(tc.expMsgs[idx]) == reflect.TypeOf(hosttypes.MsgUpdateParams{}) { + msgUpdateHostParams, ok := msg.(*hosttypes.MsgUpdateParams) + require.True(t, ok) + require.Equal(t, tc.expMsgs[idx], *msgUpdateHostParams) + } else { + msgUpdateControllerParams, ok := msg.(*controllertypes.MsgUpdateParams) + require.True(t, ok) + require.Equal(t, tc.expMsgs[idx], *msgUpdateControllerParams) + } } - } + }) } } From 6477bdd169c46c0e2f24470834857a1d0a59651c Mon Sep 17 00:00:00 2001 From: Matthew Witkowski Date: Mon, 10 Jun 2024 09:23:22 -0400 Subject: [PATCH 4/4] Update typing to be stronger in tests. --- .../simulation/proposals_test.go | 50 +++++++++---------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/modules/apps/27-interchain-accounts/simulation/proposals_test.go b/modules/apps/27-interchain-accounts/simulation/proposals_test.go index 0a0f650f5a4..c001a919765 100644 --- a/modules/apps/27-interchain-accounts/simulation/proposals_test.go +++ b/modules/apps/27-interchain-accounts/simulation/proposals_test.go @@ -2,7 +2,6 @@ package simulation_test import ( "math/rand" - "reflect" "testing" "github.com/stretchr/testify/require" @@ -32,21 +31,21 @@ func TestProposalMsgs(t *testing.T) { name string controller *controllerkeeper.Keeper host *hostkeeper.Keeper - expMsgs []interface{} + expMsgs []sdk.Msg }{ { name: "host and controller keepers are both enabled", controller: &controllerkeeper.Keeper{}, host: &hostkeeper.Keeper{}, - expMsgs: []interface{}{ - hosttypes.MsgUpdateParams{ - Signer: sdk.AccAddress(address.Module("gov")).String(), - Params: hosttypes.NewParams(false, []string{hosttypes.AllowAllHostMsgs}), - }, - controllertypes.MsgUpdateParams{ - Signer: sdk.AccAddress(address.Module("gov")).String(), - Params: controllertypes.NewParams(false), - }, + expMsgs: []sdk.Msg{ + hosttypes.NewMsgUpdateParams( + sdk.AccAddress(address.Module("gov")).String(), + hosttypes.NewParams(false, []string{hosttypes.AllowAllHostMsgs}), + ), + controllertypes.NewMsgUpdateParams( + sdk.AccAddress(address.Module("gov")).String(), + controllertypes.NewParams(false), + ), }, }, { @@ -57,21 +56,21 @@ func TestProposalMsgs(t *testing.T) { { name: "only controller keeper is enabled", controller: &controllerkeeper.Keeper{}, - expMsgs: []interface{}{ - controllertypes.MsgUpdateParams{ - Signer: sdk.AccAddress(address.Module("gov")).String(), - Params: controllertypes.NewParams(false), - }, + expMsgs: []sdk.Msg{ + controllertypes.NewMsgUpdateParams( + sdk.AccAddress(address.Module("gov")).String(), + controllertypes.NewParams(false), + ), }, }, { name: "only host keeper is enabled", host: &hostkeeper.Keeper{}, - expMsgs: []interface{}{ - hosttypes.MsgUpdateParams{ - Signer: sdk.AccAddress(address.Module("gov")).String(), - Params: hosttypes.NewParams(false, []string{hosttypes.AllowAllHostMsgs}), - }, + expMsgs: []sdk.Msg{ + hosttypes.NewMsgUpdateParams( + sdk.AccAddress(address.Module("gov")).String(), + hosttypes.NewParams(false, []string{hosttypes.AllowAllHostMsgs}), + ), }, }, } @@ -88,14 +87,13 @@ func TestProposalMsgs(t *testing.T) { require.Equal(t, simulation.DefaultWeightMsgUpdateParams, weightedMsg.DefaultWeight()) msg := weightedMsg.MsgSimulatorFn()(r, ctx, accounts) - if reflect.TypeOf(tc.expMsgs[idx]) == reflect.TypeOf(hosttypes.MsgUpdateParams{}) { - msgUpdateHostParams, ok := msg.(*hosttypes.MsgUpdateParams) - require.True(t, ok) - require.Equal(t, tc.expMsgs[idx], *msgUpdateHostParams) + + if msgUpdateHostParams, ok := msg.(*hosttypes.MsgUpdateParams); ok { + require.Equal(t, tc.expMsgs[idx], msgUpdateHostParams) } else { msgUpdateControllerParams, ok := msg.(*controllertypes.MsgUpdateParams) require.True(t, ok) - require.Equal(t, tc.expMsgs[idx], *msgUpdateControllerParams) + require.Equal(t, tc.expMsgs[idx], msgUpdateControllerParams) } } })