From 584ce4e6b6653817cb9b6c4b264555028a8d2bff Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Wed, 11 Oct 2023 11:13:54 +0200 Subject: [PATCH 1/6] fix gov hooks --- app/provider/app.go | 5 +- testutil/keeper/mocks.go | 57 +++++++++++++++-- testutil/keeper/unit_test_helpers.go | 3 + x/ccv/provider/keeper/gov_hook.go | 93 ---------------------------- x/ccv/provider/keeper/hooks.go | 83 ++++++++++++++++++++++++- x/ccv/provider/keeper/hooks_test.go | 69 ++++++++++++++++++++- x/ccv/provider/keeper/keeper.go | 11 ++-- x/ccv/types/expected_keepers.go | 5 ++ 8 files changed, 218 insertions(+), 108 deletions(-) delete mode 100644 x/ccv/provider/keeper/gov_hook.go diff --git a/app/provider/app.go b/app/provider/app.go index 377bedd10c..f021e0797b 100644 --- a/app/provider/app.go +++ b/app/provider/app.go @@ -427,6 +427,7 @@ func New( app.EvidenceKeeper, app.DistrKeeper, app.BankKeeper, + app.GovKeeper, authtypes.FeeCollectorName, ) @@ -456,10 +457,8 @@ func New( // Set legacy router for backwards compatibility with gov v1beta1 govKeeper.SetLegacyRouter(govRouter) - - govHook := app.ProviderKeeper.GovHooks(govKeeper) app.GovKeeper = *govKeeper.SetHooks( - govtypes.NewMultiGovHooks(govHook), + govtypes.NewMultiGovHooks(app.ProviderKeeper.Hooks()), ) app.TransferKeeper = ibctransferkeeper.NewKeeper( diff --git a/testutil/keeper/mocks.go b/testutil/keeper/mocks.go index b075819cc4..308aa2c9ab 100644 --- a/testutil/keeper/mocks.go +++ b/testutil/keeper/mocks.go @@ -15,6 +15,7 @@ import ( types1 "github.com/cosmos/cosmos-sdk/x/auth/types" types2 "github.com/cosmos/cosmos-sdk/x/capability/types" types3 "github.com/cosmos/cosmos-sdk/x/evidence/types" + v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" types4 "github.com/cosmos/cosmos-sdk/x/slashing/types" types5 "github.com/cosmos/cosmos-sdk/x/staking/types" types6 "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" @@ -564,20 +565,26 @@ func (m *MockChannelKeeper) GetChannel(ctx types0.Context, srcPort, srcChan stri return ret0, ret1 } +// GetChannel indicates an expected call of GetChannel. +func (mr *MockChannelKeeperMockRecorder) GetChannel(ctx, srcPort, srcChan interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChannel", reflect.TypeOf((*MockChannelKeeper)(nil).GetChannel), ctx, srcPort, srcChan) +} + +// GetChannelConnection mocks base method. func (m *MockChannelKeeper) GetChannelConnection(ctx types0.Context, portID, channelID string) (string, exported.ConnectionI, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetChannelConnection", ctx, portID, channelID) ret0, _ := ret[0].(string) - ret1, _ := ret[0].(exported.ConnectionI) - ret2, _ := ret[1].(error) - + ret1, _ := ret[1].(exported.ConnectionI) + ret2, _ := ret[2].(error) return ret0, ret1, ret2 } -// GetChannel indicates an expected call of GetChannel. -func (mr *MockChannelKeeperMockRecorder) GetChannel(ctx, srcPort, srcChan interface{}) *gomock.Call { +// GetChannelConnection indicates an expected call of GetChannelConnection. +func (mr *MockChannelKeeperMockRecorder) GetChannelConnection(ctx, portID, channelID interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChannel", reflect.TypeOf((*MockChannelKeeper)(nil).GetChannel), ctx, srcPort, srcChan) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChannelConnection", reflect.TypeOf((*MockChannelKeeper)(nil).GetChannelConnection), ctx, portID, channelID) } // GetNextSequenceSend mocks base method. @@ -1099,3 +1106,41 @@ func (mr *MockScopedKeeperMockRecorder) GetCapability(ctx, name interface{}) *go mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCapability", reflect.TypeOf((*MockScopedKeeper)(nil).GetCapability), ctx, name) } + +// MockGovKeeper is a mock of GovKeeper interface. +type MockGovKeeper struct { + ctrl *gomock.Controller + recorder *MockGovKeeperMockRecorder +} + +// MockGovKeeperMockRecorder is the mock recorder for MockGovKeeper. +type MockGovKeeperMockRecorder struct { + mock *MockGovKeeper +} + +// NewMockGovKeeper creates a new mock instance. +func NewMockGovKeeper(ctrl *gomock.Controller) *MockGovKeeper { + mock := &MockGovKeeper{ctrl: ctrl} + mock.recorder = &MockGovKeeperMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockGovKeeper) EXPECT() *MockGovKeeperMockRecorder { + return m.recorder +} + +// GetProposal mocks base method. +func (m *MockGovKeeper) GetProposal(ctx types0.Context, proposalID uint64) (v1.Proposal, bool) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetProposal", ctx, proposalID) + ret0, _ := ret[0].(v1.Proposal) + ret1, _ := ret[1].(bool) + return ret0, ret1 +} + +// GetProposal indicates an expected call of GetProposal. +func (mr *MockGovKeeperMockRecorder) GetProposal(ctx, proposalID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProposal", reflect.TypeOf((*MockGovKeeper)(nil).GetProposal), ctx, proposalID) +} diff --git a/testutil/keeper/unit_test_helpers.go b/testutil/keeper/unit_test_helpers.go index b58d6d2471..f085a2855b 100644 --- a/testutil/keeper/unit_test_helpers.go +++ b/testutil/keeper/unit_test_helpers.go @@ -89,6 +89,7 @@ type MockedKeepers struct { *MockIBCCoreKeeper *MockEvidenceKeeper *MockDistributionKeeper + *MockGovKeeper } // NewMockedKeepers instantiates a struct with pointers to properly instantiated mocked keepers. @@ -107,6 +108,7 @@ func NewMockedKeepers(ctrl *gomock.Controller) MockedKeepers { MockIBCCoreKeeper: NewMockIBCCoreKeeper(ctrl), MockEvidenceKeeper: NewMockEvidenceKeeper(ctrl), MockDistributionKeeper: NewMockDistributionKeeper(ctrl), + MockGovKeeper: NewMockGovKeeper(ctrl), } } @@ -127,6 +129,7 @@ func NewInMemProviderKeeper(params InMemKeeperParams, mocks MockedKeepers) provi mocks.MockEvidenceKeeper, mocks.MockDistributionKeeper, mocks.MockBankKeeper, + mocks.MockGovKeeper, authtypes.FeeCollectorName, ) } diff --git a/x/ccv/provider/keeper/gov_hook.go b/x/ccv/provider/keeper/gov_hook.go deleted file mode 100644 index 125181d022..0000000000 --- a/x/ccv/provider/keeper/gov_hook.go +++ /dev/null @@ -1,93 +0,0 @@ -package keeper - -import ( - "fmt" - - "github.com/cosmos/gogoproto/proto" - - sdk "github.com/cosmos/cosmos-sdk/types" - govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper" - sdkgov "github.com/cosmos/cosmos-sdk/x/gov/types" - v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - - "github.com/cosmos/interchain-security/v3/x/ccv/provider/types" -) - -type GovHooks struct { - gk *govkeeper.Keeper - k *Keeper -} - -// Implements GovHooks interface -// see https://github.com/cosmos/cosmos-sdk/blob/release/v0.47.x/x/gov/types/hooks.go -var _ sdkgov.GovHooks = GovHooks{} - -func (k *Keeper) GovHooks(gk *govkeeper.Keeper) GovHooks { - return GovHooks{ - gk: gk, - k: k, - } -} - -// AfterProposalSubmission - call hook if registered -// After consumerAddition proposal submission, the consumer chainID is stored -func (gh GovHooks) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) { - p, ok := gh.gk.GetProposal(ctx, proposalID) - if !ok { - panic(fmt.Errorf("failed to get proposal %d in gov hook", proposalID)) - } - msgs := p.GetMessages() - - for _, msg := range msgs { - var msgLegacyContent v1.MsgExecLegacyContent - err := proto.Unmarshal(msg.Value, &msgLegacyContent) - if err != nil { - panic(fmt.Errorf("failed to unmarshal proposal content in gov hook: %w", err)) - } - - // if the consumer addition proposal cannot be unmarshaled, continue - var consAdditionProp types.ConsumerAdditionProposal - if err := proto.Unmarshal(msgLegacyContent.Content.Value, &consAdditionProp); err != nil { - continue - } - - if consAdditionProp.ProposalType() == types.ProposalTypeConsumerAddition { - gh.k.SetProposedConsumerChain(ctx, consAdditionProp.ChainId, proposalID) - } - } -} - -// AfterProposalVotingPeriodEnded - call hook if registered -// After proposal voting ends, the consumer chainID in store is deleted. -// When a proposal passes, this chainID will be available in providerKeeper.GetAllPendingConsumerAdditionProps -// or providerKeeper.GetAllConsumerChains(ctx). -func (gh GovHooks) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) { - p, ok := gh.gk.GetProposal(ctx, proposalID) - if !ok { - panic(fmt.Errorf("failed to get proposal %d in gov hook", proposalID)) - } - msgs := p.GetMessages() - - for _, msg := range msgs { - var msgLegacyContent v1.MsgExecLegacyContent - err := proto.Unmarshal(msg.Value, &msgLegacyContent) - if err != nil { - panic(fmt.Errorf("failed to unmarshal proposal content in gov hook: %w", err)) - } - - var consAdditionProp types.ConsumerAdditionProposal - // if the proposal is not ConsumerAdditionProposal, return - if err := proto.Unmarshal(msgLegacyContent.Content.Value, &consAdditionProp); err != nil { - continue - } - - if consAdditionProp.ProposalType() == types.ProposalTypeConsumerAddition { - gh.k.DeleteProposedConsumerChainInStore(ctx, proposalID) - } - } -} - -func (gh GovHooks) AfterProposalDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) { -} -func (gh GovHooks) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) {} -func (gh GovHooks) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) {} diff --git a/x/ccv/provider/keeper/hooks.go b/x/ccv/provider/keeper/hooks.go index 56c8d1e094..6ff6c69f00 100644 --- a/x/ccv/provider/keeper/hooks.go +++ b/x/ccv/provider/keeper/hooks.go @@ -1,9 +1,14 @@ package keeper import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkgov "github.com/cosmos/cosmos-sdk/x/gov/types" + v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + "github.com/cosmos/interchain-security/v3/x/ccv/provider/types" providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types" ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types" ) @@ -13,13 +18,20 @@ type Hooks struct { k *Keeper } -var _ stakingtypes.StakingHooks = Hooks{} +var ( + _ stakingtypes.StakingHooks = Hooks{} + _ sdkgov.GovHooks = Hooks{} +) // Returns new provider hooks func (k *Keeper) Hooks() Hooks { return Hooks{k} } +// +// staking hooks +// + // This stores a record of each unbonding op from staking, allowing us to track which consumer chains have unbonded func (h Hooks) AfterUnbondingInitiated(ctx sdk.Context, id uint64) error { var consumerChainIDS []string @@ -179,3 +191,72 @@ func (h Hooks) AfterValidatorBeginUnbonding(_ sdk.Context, _ sdk.ConsAddress, _ func (h Hooks) BeforeDelegationRemoved(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) error { return nil } + +// +// gov hooks +// + +// AfterProposalSubmission - call hook if registered +// After consumerAddition proposal submission, the consumer chainID is stored +func (h Hooks) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) { + p, ok := h.k.govKeeper.GetProposal(ctx, proposalID) + if !ok { + panic(fmt.Errorf("failed to get proposal %d in gov hook", proposalID)) + } + msgs := p.GetMessages() + + for _, msg := range msgs { + sdkMsg, isLegacyProposal := msg.GetCachedValue().(*v1.MsgExecLegacyContent) + if !isLegacyProposal { + continue + } + + content, err := v1.LegacyContentFromMessage(sdkMsg) + if err != nil { + panic(fmt.Errorf("failed to get legacy proposal %d content in gov hook", proposalID)) + } + + consProp, ok := content.(*types.ConsumerAdditionProposal) + if ok { + h.k.SetProposedConsumerChain(ctx, consProp.ChainId, proposalID) + } + } +} + +// AfterProposalVotingPeriodEnded - call hook if registered +// After proposal voting ends, the consumer chainID in store is deleted. +// When a proposal passes, this chainID will be available in providerKeeper.GetAllPendingConsumerAdditionProps +// or providerKeeper.GetAllConsumerChains(ctx). +func (h Hooks) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) { + p, ok := h.k.govKeeper.GetProposal(ctx, proposalID) + if !ok { + panic(fmt.Errorf("failed to get proposal %d in gov hook", proposalID)) + } + msgs := p.GetMessages() + + for _, msg := range msgs { + if sdkMsg, isLegacyProposal := msg.GetCachedValue().(*v1.MsgExecLegacyContent); isLegacyProposal { + content, err := v1.LegacyContentFromMessage(sdkMsg) + if err != nil { + panic(fmt.Errorf("failed to get legacy proposal %d content in gov hook", proposalID)) + } + + if content.ProposalType() != types.ProposalTypeConsumerAddition { + continue + } + + h.k.DeleteProposedConsumerChainInStore(ctx, proposalID) + } + + continue + } +} + +func (h Hooks) AfterProposalDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) { +} + +func (h Hooks) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) { +} + +func (h Hooks) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) { +} diff --git a/x/ccv/provider/keeper/hooks_test.go b/x/ccv/provider/keeper/hooks_test.go index 14720bd970..f73b9e5165 100644 --- a/x/ccv/provider/keeper/hooks_test.go +++ b/x/ccv/provider/keeper/hooks_test.go @@ -1,15 +1,23 @@ package keeper_test import ( + "fmt" "testing" + "time" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" + v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" cryptotestutil "github.com/cosmos/interchain-security/v3/testutil/crypto" testkeeper "github.com/cosmos/interchain-security/v3/testutil/keeper" providerkeeper "github.com/cosmos/interchain-security/v3/x/ccv/provider/keeper" + "github.com/cosmos/interchain-security/v3/x/ccv/provider/types" + ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types" ) func TestValidatorConsensusKeyInUse(t *testing.T) { @@ -63,7 +71,8 @@ func TestValidatorConsensusKeyInUse(t *testing.T) { }, } for _, tt := range tests { - k, ctx, _, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() gomock.InOrder( mocks.MockStakingKeeper.EXPECT().GetValidator(ctx, @@ -80,3 +89,61 @@ func TestValidatorConsensusKeyInUse(t *testing.T) { }) } } + +func TestAfterProposalSubmission(t *testing.T) { + // encodingConfig := appparams.MakeTestEncodingConfig() + // v1beta1.RegisterInterfaces(encodingConfig.InterfaceRegistry) + // v1.RegisterInterfaces(encodingConfig.InterfaceRegistry) + // cdc := encodingConfig.Codec + + k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + newValidator := cryptotestutil.NewCryptoIdentityFromIntSeed(0) + + content := types.NewConsumerAdditionProposal( + "chainID", + "description", + "chainID", + clienttypes.NewHeight(4, 5), + []byte("gen_hash"), + []byte("bin_hash"), + time.Now(), + ccvtypes.DefaultConsumerRedistributeFrac, + ccvtypes.DefaultBlocksPerDistributionTransmission, + "", + ccvtypes.DefaultHistoricalEntries, + ccvtypes.DefaultCCVTimeoutPeriod, + ccvtypes.DefaultTransferTimeoutPeriod, + ccvtypes.DefaultConsumerUnbondingPeriod, + ) + + propContent, err := v1.NewLegacyContent(content, authtypes.NewModuleAddress("gov").String()) + require.NoError(t, err) + + // _, err = v1.LegacyContentFromMessage(propContent) + // require.NoError(t, err) + + prop, err := v1.NewProposal( + []sdk.Msg{propContent}, + 0, + time.Now(), + time.Now(), + "", + "", + "", + // sdk.NewCoins(sdk.NewCoin("stake", math.OneInt())), + sdk.AccAddress(newValidator.SDKValOpAddress()), + ) + + require.NoError(t, err) + + gomock.InOrder( + mocks.MockGovKeeper.EXPECT().GetProposal(ctx, uint64(0)).Return(prop, true), + ) + + tHooks := k.Hooks() + tHooks.AfterProposalSubmission(ctx, 0) + + fmt.Println(len(k.GetAllProposedConsumerChainIDs(ctx))) +} diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go index f76d419b0a..0dc7c9706c 100644 --- a/x/ccv/provider/keeper/keeper.go +++ b/x/ccv/provider/keeper/keeper.go @@ -44,6 +44,7 @@ type Keeper struct { evidenceKeeper ccv.EvidenceKeeper distributionKeeper ccv.DistributionKeeper bankKeeper ccv.BankKeeper + govKeeper ccv.GovKeeper feeCollectorName string } @@ -55,7 +56,7 @@ func NewKeeper( stakingKeeper ccv.StakingKeeper, slashingKeeper ccv.SlashingKeeper, accountKeeper ccv.AccountKeeper, evidenceKeeper ccv.EvidenceKeeper, distributionKeeper ccv.DistributionKeeper, bankKeeper ccv.BankKeeper, - feeCollectorName string, + govKeeper ccv.GovKeeper, feeCollectorName string, ) Keeper { // set KeyTable if it has not already been set if !paramSpace.HasKeyTable() { @@ -77,6 +78,7 @@ func NewKeeper( evidenceKeeper: evidenceKeeper, distributionKeeper: distributionKeeper, bankKeeper: bankKeeper, + govKeeper: govKeeper, feeCollectorName: feeCollectorName, } @@ -94,8 +96,8 @@ func (k *Keeper) SetParamSpace(ctx sdk.Context, ps paramtypes.Subspace) { // non-nil values for all its fields. Otherwise this method will panic. func (k Keeper) mustValidateFields() { // Ensures no fields are missed in this validation - if reflect.ValueOf(k).NumField() != 15 { - panic("number of fields in provider keeper is not 15") + if reflect.ValueOf(k).NumField() != 16 { + panic("number of fields in provider keeper is not 16") } ccv.PanicIfZeroOrNil(k.cdc, "cdc") // 1 @@ -112,7 +114,8 @@ func (k Keeper) mustValidateFields() { ccv.PanicIfZeroOrNil(k.evidenceKeeper, "evidenceKeeper") // 12 ccv.PanicIfZeroOrNil(k.distributionKeeper, "distributionKeeper") // 13 ccv.PanicIfZeroOrNil(k.bankKeeper, "bankKeeper") // 14 - ccv.PanicIfZeroOrNil(k.feeCollectorName, "feeCollectorName") // 15 + ccv.PanicIfZeroOrNil(k.govKeeper, "govKeeper") // 15 + ccv.PanicIfZeroOrNil(k.feeCollectorName, "feeCollectorName") // 16 } // Logger returns a module-specific logger. diff --git a/x/ccv/types/expected_keepers.go b/x/ccv/types/expected_keepers.go index 2f203a47ea..070b4f2a6e 100644 --- a/x/ccv/types/expected_keepers.go +++ b/x/ccv/types/expected_keepers.go @@ -20,6 +20,7 @@ import ( stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" abci "github.com/cometbft/cometbft/abci/types" + v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" ) // StakingKeeper defines the contract expected by provider-chain ccv module from a Staking Module that will keep track @@ -145,3 +146,7 @@ type ScopedKeeper interface { AuthenticateCapability(ctx sdk.Context, cap *capabilitytypes.Capability, name string) bool ClaimCapability(ctx sdk.Context, cap *capabilitytypes.Capability, name string) error } + +type GovKeeper interface { + GetProposal(ctx sdk.Context, proposalID uint64) (v1.Proposal, bool) +} From b26b4533ac9f007254c42b6038fbc037f8fa4d1d Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Thu, 12 Oct 2023 14:00:41 +0200 Subject: [PATCH 2/6] fix bug and add tests --- x/ccv/provider/keeper/hooks.go | 82 +++++++------- x/ccv/provider/keeper/hooks_test.go | 168 +++++++++++++++++++++------- 2 files changed, 163 insertions(+), 87 deletions(-) diff --git a/x/ccv/provider/keeper/hooks.go b/x/ccv/provider/keeper/hooks.go index 6ff6c69f00..0a6f595630 100644 --- a/x/ccv/provider/keeper/hooks.go +++ b/x/ccv/provider/keeper/hooks.go @@ -197,58 +197,21 @@ func (h Hooks) BeforeDelegationRemoved(_ sdk.Context, _ sdk.AccAddress, _ sdk.Va // // AfterProposalSubmission - call hook if registered -// After consumerAddition proposal submission, the consumer chainID is stored +// After a consumerAddition proposal submission, a record is created +// that maps the proposal ID to the consumer chain ID. func (h Hooks) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) { - p, ok := h.k.govKeeper.GetProposal(ctx, proposalID) - if !ok { - panic(fmt.Errorf("failed to get proposal %d in gov hook", proposalID)) - } - msgs := p.GetMessages() - - for _, msg := range msgs { - sdkMsg, isLegacyProposal := msg.GetCachedValue().(*v1.MsgExecLegacyContent) - if !isLegacyProposal { - continue - } - - content, err := v1.LegacyContentFromMessage(sdkMsg) - if err != nil { - panic(fmt.Errorf("failed to get legacy proposal %d content in gov hook", proposalID)) - } - - consProp, ok := content.(*types.ConsumerAdditionProposal) - if ok { - h.k.SetProposedConsumerChain(ctx, consProp.ChainId, proposalID) - } + if p, ok := h.GetConsumerAdditionLegacyPropFromProp(ctx, proposalID); ok { + h.k.SetProposedConsumerChain(ctx, p.ChainId, proposalID) } } // AfterProposalVotingPeriodEnded - call hook if registered // After proposal voting ends, the consumer chainID in store is deleted. -// When a proposal passes, this chainID will be available in providerKeeper.GetAllPendingConsumerAdditionProps +// When a consumerAddition proposal passes, the consumer chainID is available in providerKeeper.GetAllPendingConsumerAdditionProps // or providerKeeper.GetAllConsumerChains(ctx). func (h Hooks) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) { - p, ok := h.k.govKeeper.GetProposal(ctx, proposalID) - if !ok { - panic(fmt.Errorf("failed to get proposal %d in gov hook", proposalID)) - } - msgs := p.GetMessages() - - for _, msg := range msgs { - if sdkMsg, isLegacyProposal := msg.GetCachedValue().(*v1.MsgExecLegacyContent); isLegacyProposal { - content, err := v1.LegacyContentFromMessage(sdkMsg) - if err != nil { - panic(fmt.Errorf("failed to get legacy proposal %d content in gov hook", proposalID)) - } - - if content.ProposalType() != types.ProposalTypeConsumerAddition { - continue - } - - h.k.DeleteProposedConsumerChainInStore(ctx, proposalID) - } - - continue + if _, ok := h.GetConsumerAdditionLegacyPropFromProp(ctx, proposalID); ok { + h.k.DeleteProposedConsumerChainInStore(ctx, proposalID) } } @@ -260,3 +223,34 @@ func (h Hooks) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr s func (h Hooks) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) { } + +func (h Hooks) GetConsumerAdditionLegacyPropFromProp( + ctx sdk.Context, + proposalID uint64, +) (types.ConsumerAdditionProposal, bool) { + p, ok := h.k.govKeeper.GetProposal(ctx, proposalID) + if !ok { + panic(fmt.Errorf("failed to get proposal %d from store", proposalID)) + } + + // Iterate over the messages in the proposal + // Note that only ONE message can contain a consumer addition proposal + for _, msg := range p.GetMessages() { + sdkMsg, isLegacyProposal := msg.GetCachedValue().(*v1.MsgExecLegacyContent) + if !isLegacyProposal { + continue + } + + content, err := v1.LegacyContentFromMessage(sdkMsg) + if err != nil { + panic(fmt.Errorf("failed to get legacy proposal %d from prop message", proposalID)) + } + + // returns if legacy prop is of ConsumerAddition proposal type + prop, ok := content.(*types.ConsumerAdditionProposal) + if ok { + return *prop, true + } + } + return types.ConsumerAdditionProposal{}, false +} diff --git a/x/ccv/provider/keeper/hooks_test.go b/x/ccv/provider/keeper/hooks_test.go index f73b9e5165..d8f30115f0 100644 --- a/x/ccv/provider/keeper/hooks_test.go +++ b/x/ccv/provider/keeper/hooks_test.go @@ -1,23 +1,24 @@ package keeper_test import ( - "fmt" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "testing" "time" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types" + "cosmossdk.io/math" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" + "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" cryptotestutil "github.com/cosmos/interchain-security/v3/testutil/crypto" testkeeper "github.com/cosmos/interchain-security/v3/testutil/keeper" providerkeeper "github.com/cosmos/interchain-security/v3/x/ccv/provider/keeper" - "github.com/cosmos/interchain-security/v3/x/ccv/provider/types" - ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types" ) func TestValidatorConsensusKeyInUse(t *testing.T) { @@ -90,60 +91,141 @@ func TestValidatorConsensusKeyInUse(t *testing.T) { } } -func TestAfterProposalSubmission(t *testing.T) { - // encodingConfig := appparams.MakeTestEncodingConfig() - // v1beta1.RegisterInterfaces(encodingConfig.InterfaceRegistry) - // v1.RegisterInterfaces(encodingConfig.InterfaceRegistry) - // cdc := encodingConfig.Codec +func TestAfterSubmissionAndAfterProposalVotingPeriodEnded(t *testing.T) { + acct := cryptotestutil.NewCryptoIdentityFromIntSeed(0) - k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) - defer ctrl.Finish() - - newValidator := cryptotestutil.NewCryptoIdentityFromIntSeed(0) - - content := types.NewConsumerAdditionProposal( - "chainID", - "description", - "chainID", - clienttypes.NewHeight(4, 5), - []byte("gen_hash"), - []byte("bin_hash"), - time.Now(), - ccvtypes.DefaultConsumerRedistributeFrac, - ccvtypes.DefaultBlocksPerDistributionTransmission, - "", - ccvtypes.DefaultHistoricalEntries, - ccvtypes.DefaultCCVTimeoutPeriod, - ccvtypes.DefaultTransferTimeoutPeriod, - ccvtypes.DefaultConsumerUnbondingPeriod, + propMsg, err := v1.NewLegacyContent( + testkeeper.GetTestConsumerAdditionProp(), + authtypes.NewModuleAddress("gov").String(), ) - - propContent, err := v1.NewLegacyContent(content, authtypes.NewModuleAddress("gov").String()) require.NoError(t, err) - // _, err = v1.LegacyContentFromMessage(propContent) - // require.NoError(t, err) - prop, err := v1.NewProposal( - []sdk.Msg{propContent}, + []sdk.Msg{propMsg}, 0, time.Now(), time.Now(), "", "", "", - // sdk.NewCoins(sdk.NewCoin("stake", math.OneInt())), - sdk.AccAddress(newValidator.SDKValOpAddress()), + sdk.AccAddress(acct.SDKValOpAddress()), ) - require.NoError(t, err) + k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + // pass the prop to the mocked gov keeper, + // which is called by both the AfterProposalVotingPeriodEnded and + // AfterProposalSubmission gov hooks gomock.InOrder( - mocks.MockGovKeeper.EXPECT().GetProposal(ctx, uint64(0)).Return(prop, true), + mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(prop, true).Times(2), ) - tHooks := k.Hooks() - tHooks.AfterProposalSubmission(ctx, 0) + k.Hooks().AfterProposalSubmission(ctx, prop.Id) + // verify that the proposal ID is created + require.NotEmpty(t, k.GetProposedConsumerChain(ctx, prop.Id)) + + k.Hooks().AfterProposalVotingPeriodEnded(ctx, prop.Id) + // verify that the proposal ID is deleted + require.Empty(t, k.GetProposedConsumerChain(ctx, prop.Id)) +} + +func TestGetConsumerAdditionLegacyPropFromProp(t *testing.T) { + acct := cryptotestutil.NewCryptoIdentityFromIntSeed(0) + anotherAcct := cryptotestutil.NewCryptoIdentityFromIntSeed(1) + + // create a dummy bank send message + dummyMsg := &banktypes.MsgSend{ + FromAddress: sdk.AccAddress(acct.SDKValOpAddress()).String(), + ToAddress: sdk.AccAddress(anotherAcct.SDKValOpAddress()).String(), + Amount: sdk.NewCoins(sdk.NewCoin("stake", math.OneInt())), + } + + testCases := map[string]struct { + propMsg func() sdk.Msg + // setup func(sdk.Context, k providerkeeper, proposalID uint64) + expPanic bool + expConsuAddProp bool + }{ + + "prop not found": { + propMsg: func() sdk.Msg { + return dummyMsg + }, + expPanic: true, + }, + "msgs in prop contain no legacy props": { + propMsg: func() sdk.Msg { + return dummyMsg + }, + expConsuAddProp: false, + }, + "msgs contain a legacy prop but not of ConsumerAdditionProposal type": { + propMsg: func() sdk.Msg { + textProp, err := v1.NewLegacyContent( + v1beta1.NewTextProposal("a title", "a legacy text prop"), + authtypes.NewModuleAddress("gov").String(), + ) + require.NoError(t, err) + + return textProp + }, + expConsuAddProp: false, + }, + "msg contains a prop of ConsumerAdditionProposal type - hook should create a new proposed chain": { + propMsg: func() sdk.Msg { + // create a dummy consumer addition prop + consuProp, err := v1.NewLegacyContent( + testkeeper.GetTestConsumerAdditionProp(), + authtypes.NewModuleAddress("gov").String(), + ) + require.NoError(t, err) + + return consuProp + }, + expConsuAddProp: true, + }, + } - fmt.Println(len(k.GetAllProposedConsumerChainIDs(ctx))) + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + // create a dummy prop for each test case + prop, err := v1.NewProposal( + []sdk.Msg{tc.propMsg()}, + 0, + time.Now(), + time.Now(), + "", + "", + "", + sdk.AccAddress(acct.SDKValOpAddress()), + ) + require.NoError(t, err) + + if !tc.expPanic { + // pass the prop to the mocked gov keeper, + // which is called by the AfterProposalSubmission gov hook + gomock.InOrder( + mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(prop, true), + ) + } else { + gomock.InOrder( + mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(v1.Proposal{}, false), + ) + defer func() { + if r := recover(); r == nil { + require.Fail(t, r.(string)) + } + }() + } + + // retrieve consumer addition proposal + _, ok := k.Hooks().GetConsumerAdditionLegacyPropFromProp(ctx, prop.Id) + require.Equal(t, tc.expConsuAddProp, ok) + }) + } } From a240ab582c9dd3003f5ffc17d27f0eac0af7beca Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Tue, 14 Nov 2023 15:10:01 +0100 Subject: [PATCH 3/6] finish unit testing of consu addition legacy prop getter --- x/ccv/provider/keeper/hooks.go | 4 +- x/ccv/provider/keeper/hooks_test.go | 95 ++++++++++++++--------------- 2 files changed, 48 insertions(+), 51 deletions(-) diff --git a/x/ccv/provider/keeper/hooks.go b/x/ccv/provider/keeper/hooks.go index 0a6f595630..fead7b06d6 100644 --- a/x/ccv/provider/keeper/hooks.go +++ b/x/ccv/provider/keeper/hooks.go @@ -224,6 +224,8 @@ func (h Hooks) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr s func (h Hooks) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) { } +// GetConsumerAdditionLegacyPropFromProp extracts a consumer addition legacy proposal from +// the proposal with given ID func (h Hooks) GetConsumerAdditionLegacyPropFromProp( ctx sdk.Context, proposalID uint64, @@ -234,7 +236,7 @@ func (h Hooks) GetConsumerAdditionLegacyPropFromProp( } // Iterate over the messages in the proposal - // Note that only ONE message can contain a consumer addition proposal + // Note that it's assumed that at most ONE message can contain a consumer addition proposal for _, msg := range p.GetMessages() { sdkMsg, isLegacyProposal := msg.GetCachedValue().(*v1.MsgExecLegacyContent) if !isLegacyProposal { diff --git a/x/ccv/provider/keeper/hooks_test.go b/x/ccv/provider/keeper/hooks_test.go index d8f30115f0..fd389e3267 100644 --- a/x/ccv/provider/keeper/hooks_test.go +++ b/x/ccv/provider/keeper/hooks_test.go @@ -91,7 +91,7 @@ func TestValidatorConsensusKeyInUse(t *testing.T) { } } -func TestAfterSubmissionAndAfterProposalVotingPeriodEnded(t *testing.T) { +func TestAfterPropSubmissionAndVotingPeriodEnded(t *testing.T) { acct := cryptotestutil.NewCryptoIdentityFromIntSeed(0) propMsg, err := v1.NewLegacyContent( @@ -142,48 +142,40 @@ func TestGetConsumerAdditionLegacyPropFromProp(t *testing.T) { Amount: sdk.NewCoins(sdk.NewCoin("stake", math.OneInt())), } + textProp, err := v1.NewLegacyContent( + v1beta1.NewTextProposal("a title", "a legacy text prop"), + authtypes.NewModuleAddress("gov").String(), + ) + require.NoError(t, err) + + consuProp, err := v1.NewLegacyContent( + testkeeper.GetTestConsumerAdditionProp(), + authtypes.NewModuleAddress("gov").String(), + ) + require.NoError(t, err) + testCases := map[string]struct { - propMsg func() sdk.Msg + propMsg sdk.Msg // setup func(sdk.Context, k providerkeeper, proposalID uint64) expPanic bool expConsuAddProp bool }{ - "prop not found": { - propMsg: func() sdk.Msg { - return dummyMsg - }, + propMsg: nil, expPanic: true, }, "msgs in prop contain no legacy props": { - propMsg: func() sdk.Msg { - return dummyMsg - }, - expConsuAddProp: false, + propMsg: dummyMsg, }, "msgs contain a legacy prop but not of ConsumerAdditionProposal type": { - propMsg: func() sdk.Msg { - textProp, err := v1.NewLegacyContent( - v1beta1.NewTextProposal("a title", "a legacy text prop"), - authtypes.NewModuleAddress("gov").String(), - ) - require.NoError(t, err) - - return textProp - }, - expConsuAddProp: false, + propMsg: textProp, + }, + "msgs contain an invalid legacy prop": { + propMsg: &v1.MsgExecLegacyContent{}, + expPanic: true, }, "msg contains a prop of ConsumerAdditionProposal type - hook should create a new proposed chain": { - propMsg: func() sdk.Msg { - // create a dummy consumer addition prop - consuProp, err := v1.NewLegacyContent( - testkeeper.GetTestConsumerAdditionProp(), - authtypes.NewModuleAddress("gov").String(), - ) - require.NoError(t, err) - - return consuProp - }, + propMsg: consuProp, expConsuAddProp: true, }, } @@ -193,30 +185,33 @@ func TestGetConsumerAdditionLegacyPropFromProp(t *testing.T) { k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() - // create a dummy prop for each test case - prop, err := v1.NewProposal( - []sdk.Msg{tc.propMsg()}, - 0, - time.Now(), - time.Now(), - "", - "", - "", - sdk.AccAddress(acct.SDKValOpAddress()), + var ( + prop v1.Proposal + propFound bool ) - require.NoError(t, err) - if !tc.expPanic { - // pass the prop to the mocked gov keeper, - // which is called by the AfterProposalSubmission gov hook - gomock.InOrder( - mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(prop, true), - ) - } else { - gomock.InOrder( - mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(v1.Proposal{}, false), + if tc.propMsg != nil { + propFound = true + prop, err = v1.NewProposal( + []sdk.Msg{tc.propMsg}, + 0, + time.Now(), + time.Now(), + "", + "", + "", + sdk.AccAddress(acct.SDKValOpAddress()), ) + require.NoError(t, err) + } + + gomock.InOrder( + mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(prop, propFound), + ) + + if tc.expPanic { defer func() { + // fail test if not panic was recovered if r := recover(); r == nil { require.Fail(t, r.(string)) } From 94fd342a846ced55ace63a84bb956b6e824db530 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Wed, 15 Nov 2023 09:49:47 +0100 Subject: [PATCH 4/6] nit --- app/provider/app.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/app/provider/app.go b/app/provider/app.go index f021e0797b..ebca50a2cc 100644 --- a/app/provider/app.go +++ b/app/provider/app.go @@ -204,7 +204,7 @@ type App struct { // nolint: golint // different fee-pool from the consumer chain ConsumerKeeper DistrKeeper distrkeeper.Keeper - GovKeeper govkeeper.Keeper + GovKeeper *govkeeper.Keeper // Gov Keeper must be a pointer in the app, so we can SetRouter on it correctly CrisisKeeper crisiskeeper.Keeper UpgradeKeeper upgradekeeper.Keeper ParamsKeeper paramskeeper.Keeper @@ -412,6 +412,17 @@ func New( app.SlashingKeeper, ) + app.GovKeeper = govkeeper.NewKeeper( + appCodec, + keys[govtypes.StoreKey], + app.AccountKeeper, + app.BankKeeper, + app.StakingKeeper, + app.MsgServiceRouter(), + govtypes.DefaultConfig(), + authtypes.NewModuleAddress(govtypes.ModuleName).String(), + ) + app.ProviderKeeper = ibcproviderkeeper.NewKeeper( appCodec, keys[providertypes.StoreKey], @@ -442,22 +453,11 @@ func New( AddRoute(ibchost.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper)). AddRoute(providertypes.RouterKey, ibcprovider.NewProviderProposalHandler(app.ProviderKeeper)). AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper)) - govConfig := govtypes.DefaultConfig() - - govKeeper := govkeeper.NewKeeper( - appCodec, - keys[govtypes.StoreKey], - app.AccountKeeper, - app.BankKeeper, - app.StakingKeeper, - app.MsgServiceRouter(), - govConfig, - authtypes.NewModuleAddress(govtypes.ModuleName).String(), - ) // Set legacy router for backwards compatibility with gov v1beta1 - govKeeper.SetLegacyRouter(govRouter) - app.GovKeeper = *govKeeper.SetHooks( + app.GovKeeper.SetLegacyRouter(govRouter) + + app.GovKeeper = app.GovKeeper.SetHooks( govtypes.NewMultiGovHooks(app.ProviderKeeper.Hooks()), ) @@ -497,7 +497,7 @@ func New( bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper, app.GetSubspace(banktypes.ModuleName)), capability.NewAppModule(appCodec, *app.CapabilityKeeper, false), crisis.NewAppModule(&app.CrisisKeeper, skipGenesisInvariants, app.GetSubspace(crisistypes.ModuleName)), - gov.NewAppModule(appCodec, &app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(govtypes.ModuleName)), + gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(govtypes.ModuleName)), mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper, nil, app.GetSubspace(minttypes.ModuleName)), slashing.NewAppModule(appCodec, app.SlashingKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GetSubspace(slashingtypes.ModuleName)), distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GetSubspace(distrtypes.ModuleName)), @@ -594,7 +594,7 @@ func New( auth.NewAppModule(appCodec, app.AccountKeeper, authsims.RandomGenesisAccounts, app.GetSubspace(authtypes.ModuleName)), bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper, app.GetSubspace(banktypes.ModuleName)), capability.NewAppModule(appCodec, *app.CapabilityKeeper, false), - gov.NewAppModule(appCodec, &app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(govtypes.ModuleName)), + gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(govtypes.ModuleName)), mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper, nil, app.GetSubspace(minttypes.ModuleName)), staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper, app.GetSubspace(stakingtypes.ModuleName)), distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper, app.GetSubspace(distrtypes.ModuleName)), From f196cc982d20f44ed9739718bc8f3d22610e98c4 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Wed, 15 Nov 2023 10:35:22 +0100 Subject: [PATCH 5/6] update changelog --- .../provider/1339-check-key-assignment-in-use.md | 3 +++ .../features/provider/1339-check-key-assignment-in-use.md | 3 +++ 2 files changed, 6 insertions(+) create mode 100644 .changelog/unreleased/consensus-breaking/provider/1339-check-key-assignment-in-use.md create mode 100644 .changelog/unreleased/features/provider/1339-check-key-assignment-in-use.md diff --git a/.changelog/unreleased/consensus-breaking/provider/1339-check-key-assignment-in-use.md b/.changelog/unreleased/consensus-breaking/provider/1339-check-key-assignment-in-use.md new file mode 100644 index 0000000000..2890582ba8 --- /dev/null +++ b/.changelog/unreleased/consensus-breaking/provider/1339-check-key-assignment-in-use.md @@ -0,0 +1,3 @@ +- Change the states by adding a consumer key for each chain that is + not yet registered meaning for which the gov proposal has not passed. + ([\#1339](https://github.com/cosmos/interchain-security/pull/1339)) \ No newline at end of file diff --git a/.changelog/unreleased/features/provider/1339-check-key-assignment-in-use.md b/.changelog/unreleased/features/provider/1339-check-key-assignment-in-use.md new file mode 100644 index 0000000000..994e637963 --- /dev/null +++ b/.changelog/unreleased/features/provider/1339-check-key-assignment-in-use.md @@ -0,0 +1,3 @@ +- Update how consumer-assigned keys are checked when a validator is + created on the provider. ([\#1339](https://github.com/cosmos/interchain- + security/pull/1339)) \ No newline at end of file From 642ca2980467fa37e03797a3fb5f17ee652185ae Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Wed, 15 Nov 2023 10:52:58 +0100 Subject: [PATCH 6/6] lint --- x/ccv/provider/keeper/hooks.go | 7 +++---- x/ccv/provider/keeper/hooks_test.go | 9 ++++----- x/ccv/types/expected_keepers.go | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/x/ccv/provider/keeper/hooks.go b/x/ccv/provider/keeper/hooks.go index 81e2f252aa..3be2be04eb 100644 --- a/x/ccv/provider/keeper/hooks.go +++ b/x/ccv/provider/keeper/hooks.go @@ -8,7 +8,6 @@ import ( v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - "github.com/cosmos/interchain-security/v3/x/ccv/provider/types" providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types" ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types" ) @@ -229,7 +228,7 @@ func (h Hooks) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) func (h Hooks) GetConsumerAdditionLegacyPropFromProp( ctx sdk.Context, proposalID uint64, -) (types.ConsumerAdditionProposal, bool) { +) (providertypes.ConsumerAdditionProposal, bool) { p, ok := h.k.govKeeper.GetProposal(ctx, proposalID) if !ok { panic(fmt.Errorf("failed to get proposal %d from store", proposalID)) @@ -249,10 +248,10 @@ func (h Hooks) GetConsumerAdditionLegacyPropFromProp( } // returns if legacy prop is of ConsumerAddition proposal type - prop, ok := content.(*types.ConsumerAdditionProposal) + prop, ok := content.(*providertypes.ConsumerAdditionProposal) if ok { return *prop, true } } - return types.ConsumerAdditionProposal{}, false + return providertypes.ConsumerAdditionProposal{}, false } diff --git a/x/ccv/provider/keeper/hooks_test.go b/x/ccv/provider/keeper/hooks_test.go index fd389e3267..0bdf9b26fa 100644 --- a/x/ccv/provider/keeper/hooks_test.go +++ b/x/ccv/provider/keeper/hooks_test.go @@ -1,18 +1,17 @@ package keeper_test import ( - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - "testing" "time" - "cosmossdk.io/math" - banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + "cosmossdk.io/math" + sdk "github.com/cosmos/cosmos-sdk/types" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" diff --git a/x/ccv/types/expected_keepers.go b/x/ccv/types/expected_keepers.go index 070b4f2a6e..825aabe920 100644 --- a/x/ccv/types/expected_keepers.go +++ b/x/ccv/types/expected_keepers.go @@ -16,11 +16,11 @@ import ( auth "github.com/cosmos/cosmos-sdk/x/auth/types" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types" + v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" abci "github.com/cometbft/cometbft/abci/types" - v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" ) // StakingKeeper defines the contract expected by provider-chain ccv module from a Staking Module that will keep track