From 7cd900ad2a74aae8e98106d7e7ab6c15c9daa97a Mon Sep 17 00:00:00 2001 From: MSalopek Date: Wed, 24 Apr 2024 21:34:12 +0200 Subject: [PATCH] test: move provider hooks tests to integration tests (#1816) tests: add provider gov hooks integration tests --- app/provider/app.go | 5 + tests/integration/provider_gov_hooks.go | 149 ++++++++++++++++++++++++ testutil/integration/debug_test.go | 13 +++ testutil/integration/interfaces.go | 2 +- x/ccv/provider/keeper/hooks_test.go | 138 ---------------------- 5 files changed, 168 insertions(+), 139 deletions(-) create mode 100644 tests/integration/provider_gov_hooks.go diff --git a/app/provider/app.go b/app/provider/app.go index b2042258d2..32bb17050a 100644 --- a/app/provider/app.go +++ b/app/provider/app.go @@ -923,6 +923,11 @@ func (app *App) GetTestAccountKeeper() testutil.TestAccountKeeper { return app.AccountKeeper } +// GetTestGovKeeper implements the ProviderApp interface. +func (app *App) GetTestGovKeeper() *govkeeper.Keeper { + return app.GovKeeper +} + // TestingApp functions // GetBaseApp implements the TestingApp interface. diff --git a/tests/integration/provider_gov_hooks.go b/tests/integration/provider_gov_hooks.go new file mode 100644 index 0000000000..d1970c1b72 --- /dev/null +++ b/tests/integration/provider_gov_hooks.go @@ -0,0 +1,149 @@ +package integration + +import ( + "time" + + "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" + + testkeeper "github.com/cosmos/interchain-security/v5/testutil/keeper" +) + +// tests AfterProposalSubmission and AfterProposalVotingPeriodEnded hooks +// hooks require adding a proposal in the gov module and regitering a consumer chain with the provider module +func (s *CCVTestSuite) TestAfterPropSubmissionAndVotingPeriodEnded() { + ctx := s.providerChain.GetContext() + providerKeeper := s.providerApp.GetProviderKeeper() + govKeeper := s.providerApp.GetTestGovKeeper() + proposer := s.providerChain.SenderAccount + + content := testkeeper.GetTestConsumerAdditionProp() + content.ChainId = "newchain-0" + legacyPropContent, err := v1.NewLegacyContent( + content, + authtypes.NewModuleAddress("gov").String(), + ) + s.Require().NoError(err) + + proposal, err := v1.NewProposal([]sdk.Msg{legacyPropContent}, 1, time.Now(), time.Now().Add(1*time.Hour), "metadata", "title", "summary", proposer.GetAddress(), false) + s.Require().NoError(err) + + err = govKeeper.SetProposal(ctx, proposal) + s.Require().NoError(err) + + providerKeeper.Hooks().AfterProposalSubmission(ctx, proposal.Id) + + // verify that the proposal ID is created + proposalIdOnProvider := providerKeeper.GetProposedConsumerChain(ctx, proposal.Id) + s.Require().NotEmpty(proposalIdOnProvider) + s.Require().Equal(content.ChainId, proposalIdOnProvider) + + providerKeeper.Hooks().AfterProposalVotingPeriodEnded(ctx, proposal.Id) + // verify that the proposal ID is deleted + s.Require().Empty(providerKeeper.GetProposedConsumerChain(ctx, proposal.Id)) +} + +func (s *CCVTestSuite) TestGetConsumerAdditionLegacyPropFromProp() { + ctx := s.providerChain.GetContext() + proposer := s.providerChain.SenderAccount + + // create a dummy bank send message + dummyMsg := &banktypes.MsgSend{ + FromAddress: sdk.AccAddress(proposer.GetAddress()).String(), + ToAddress: sdk.AccAddress(proposer.GetAddress()).String(), + Amount: sdk.NewCoins(sdk.NewCoin("stake", math.OneInt())), + } + + textProp, err := v1.NewLegacyContent( + v1beta1.NewTextProposal("a title", "a legacy text prop"), + authtypes.NewModuleAddress("gov").String(), + ) + s.Require().NoError(err) + + addConsumerProp, err := v1.NewLegacyContent( + testkeeper.GetTestConsumerAdditionProp(), + authtypes.NewModuleAddress("gov").String(), + ) + s.Require().NoError(err) + + testCases := []struct { + name string + propMsg sdk.Msg + expectConsumerPropFound bool + expPanic bool + }{ + { + name: "prop not found", + propMsg: nil, + expectConsumerPropFound: false, + expPanic: false, + }, + { + name: "msgs in prop contain no legacy props", + propMsg: dummyMsg, + expectConsumerPropFound: false, + expPanic: false, + }, + { + name: "msgs contain a legacy prop but not of ConsumerAdditionProposal type", + propMsg: textProp, + expectConsumerPropFound: false, + }, + { + name: "msgs contain an invalid legacy prop", + propMsg: &v1.MsgExecLegacyContent{}, + expectConsumerPropFound: false, + expPanic: true, + }, + { + name: "msg contains a prop of ConsumerAdditionProposal type - hook should create a new proposed chain", + propMsg: addConsumerProp, + expectConsumerPropFound: true, + expPanic: false, + }, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + providerKeeper := s.providerApp.GetProviderKeeper() + govKeeper := s.providerApp.GetTestGovKeeper() + + var proposal v1.Proposal + var err error + + if tc.propMsg == nil { + // cover edgecase where proposal has no messages + proposal, err = v1.NewProposal([]sdk.Msg{}, 1, time.Now(), time.Now().Add(1*time.Hour), "metadata", "title", "summary", proposer.GetAddress(), false) + s.Require().NoError(err) + } else { + // cover variolus cases where proposal has messages but only some are consumer addition proposals + proposal, err = v1.NewProposal([]sdk.Msg{tc.propMsg}, 1, time.Now(), time.Now().Add(1*time.Hour), "metadata", "title", "summary", proposer.GetAddress(), false) + s.Require().NoError(err) + } + + err = govKeeper.SetProposal(ctx, proposal) + s.Require().NoError(err) + + if tc.expPanic { + s.Require().Panics(func() { + // this panics with a nil pointer dereference because the proposal is invalid and cannot be unmarshalled + providerKeeper.Hooks().GetConsumerAdditionLegacyPropFromProp(ctx, proposal.Id) + }) + return + } + + savedProp, found := providerKeeper.Hooks().GetConsumerAdditionLegacyPropFromProp(ctx, proposal.Id) + if tc.expectConsumerPropFound { + s.Require().True(found) + s.Require().NotEmpty(savedProp, savedProp) + } else { + s.Require().False(found) + s.Require().Empty(savedProp) + } + }) + } +} diff --git a/testutil/integration/debug_test.go b/testutil/integration/debug_test.go index 1708cbf6c2..af37fbb5a1 100644 --- a/testutil/integration/debug_test.go +++ b/testutil/integration/debug_test.go @@ -281,6 +281,7 @@ func TestHandleConsumerDoubleVotingSlashesUndelegationsAndRelegations(t *testing runCCVTestByName(t, "TestHandleConsumerDoubleVotingSlashesUndelegationsAndRelegations") } +// // Throttle retry tests // @@ -291,3 +292,15 @@ func TestSlashRetries(t *testing.T) { func TestKeyAssignment(t *testing.T) { runCCVTestByName(t, "TestKeyAssignment") } + +// +// Provider gov hooks test +// + +func TestAfterPropSubmissionAndVotingPeriodEnded(t *testing.T) { + runCCVTestByName(t, "TestAfterPropSubmissionAndVotingPeriodEnded") +} + +func TestGetConsumerAdditionLegacyPropFromProp(t *testing.T) { + runCCVTestByName(t, "TestGetConsumerAdditionLegacyPropFromProp") +} diff --git a/testutil/integration/interfaces.go b/testutil/integration/interfaces.go index 3f8428f9d0..b36befe20f 100644 --- a/testutil/integration/interfaces.go +++ b/testutil/integration/interfaces.go @@ -47,7 +47,7 @@ type ProviderApp interface { // Returns an account keeper interface with more capabilities than the expected_keepers interface GetTestAccountKeeper() TestAccountKeeper - // GetTestGovKeeper() govkeeper.Keeper + GetTestGovKeeper() *govkeeper.Keeper } // The interface that any consumer app must implement to be compatible with integration tests diff --git a/x/ccv/provider/keeper/hooks_test.go b/x/ccv/provider/keeper/hooks_test.go index d7ea94c0dc..2a6d60db85 100644 --- a/x/ccv/provider/keeper/hooks_test.go +++ b/x/ccv/provider/keeper/hooks_test.go @@ -81,141 +81,3 @@ func TestValidatorConsensusKeyInUse(t *testing.T) { }) } } - -// TODO: move to integration tests -// func TestAfterPropSubmissionAndVotingPeriodEnded(t *testing.T) { -// acct := cryptotestutil.NewCryptoIdentityFromIntSeed(0) - -// propMsg, err := v1.NewLegacyContent( -// testkeeper.GetTestConsumerAdditionProp(), -// authtypes.NewModuleAddress("gov").String(), -// ) -// require.NoError(t, err) - -// prop, err := v1.NewProposal( -// []sdk.Msg{propMsg}, -// 0, -// time.Now(), -// time.Now(), -// "", -// "", -// "", -// sdk.AccAddress(acct.SDKValOpAddress()), -// false, // proposal not expedited -// ) -// require.NoError(t, err) - -// k, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) -// defer ctrl.Finish() - -// // TODO: not sure how to ho about fetching props on the mock -// // 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, prop.Id).Return(prop, true).Times(2), -// // ) - -// 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())), -// } - -// 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 sdk.Msg -// // setup func(sdk.Context, k providerkeeper, proposalID uint64) -// expPanic bool -// expConsuAddProp bool -// }{ -// "prop not found": { -// propMsg: nil, -// expPanic: true, -// }, -// "msgs in prop contain no legacy props": { -// propMsg: dummyMsg, -// }, -// "msgs contain a legacy prop but not of ConsumerAdditionProposal type": { -// 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: consuProp, -// expConsuAddProp: true, -// }, -// } - -// for name, tc := range testCases { -// t.Run(name, func(t *testing.T) { -// k, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) -// defer ctrl.Finish() - -// var ( -// prop v1.Proposal -// // propFound bool -// ) - -// if tc.propMsg != nil { -// // propFound = true -// prop, err = v1.NewProposal( -// []sdk.Msg{tc.propMsg}, -// 0, -// time.Now(), -// time.Now(), -// "", -// "", -// "", -// sdk.AccAddress(acct.SDKValOpAddress()), -// false, // proposal not expedited -// ) -// 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)) -// } -// }() -// } - -// // retrieve consumer addition proposal -// _, ok := k.Hooks().GetConsumerAdditionLegacyPropFromProp(ctx, prop.Id) -// require.Equal(t, tc.expConsuAddProp, ok) -// }) -// } -// }