From f49fd4517ab9535a68be5dd76631ff7109e9d04e Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Thu, 25 Jan 2024 10:19:12 +0000 Subject: [PATCH] Add testing for client events (#5686) --- modules/core/02-client/keeper/client_test.go | 17 +++- modules/core/02-client/keeper/events_test.go | 88 ++++++++++++++++++++ modules/core/02-client/keeper/keeper_test.go | 16 +++- modules/core/keeper/msg_server_test.go | 16 +++- testing/events.go | 39 +++++++-- 5 files changed, 166 insertions(+), 10 deletions(-) create mode 100644 modules/core/02-client/keeper/events_test.go diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index 0e666144af6..63116bdcee8 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -6,6 +6,8 @@ import ( upgradetypes "cosmossdk.io/x/upgrade/types" + sdk "github.com/cosmos/cosmos-sdk/types" + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" "github.com/cosmos/ibc-go/v8/modules/core/exported" @@ -652,16 +654,29 @@ func (suite *KeeperTestSuite) TestRecoverClient() { tc.malleate() - err = suite.chainA.App.GetIBCKeeper().ClientKeeper.RecoverClient(suite.chainA.GetContext(), subject, substitute) + ctx := suite.chainA.GetContext() + err = suite.chainA.App.GetIBCKeeper().ClientKeeper.RecoverClient(ctx, subject, substitute) expPass := tc.expErr == nil if expPass { suite.Require().NoError(err) + expectedEvents := sdk.Events{ + sdk.NewEvent( + clienttypes.EventTypeRecoverClient, + sdk.NewAttribute(clienttypes.AttributeKeySubjectClientID, subjectPath.EndpointA.ClientID), + sdk.NewAttribute(clienttypes.AttributeKeyClientType, subjectPath.EndpointA.GetClientState().ClientType()), + ), + }.ToABCIEvents() + + expectedEvents = sdk.MarkEventsToIndex(expectedEvents, map[string]struct{}{}) + ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) + // Assert that client status is now Active clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID) tmClientState := subjectPath.EndpointA.GetClientState().(*ibctm.ClientState) suite.Require().Equal(tmClientState.Status(suite.chainA.GetContext(), clientStore, suite.chainA.App.AppCodec()), exported.Active) + } else { suite.Require().Error(err) suite.Require().ErrorIs(err, tc.expErr) diff --git a/modules/core/02-client/keeper/events_test.go b/modules/core/02-client/keeper/events_test.go new file mode 100644 index 00000000000..c5d1fdd3057 --- /dev/null +++ b/modules/core/02-client/keeper/events_test.go @@ -0,0 +1,88 @@ +package keeper_test + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" + ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" + ibctesting "github.com/cosmos/ibc-go/v8/testing" +) + +func (suite *KeeperTestSuite) TestMsgCreateClientEvents() { + suite.SetupTest() + path := ibctesting.NewPath(suite.chainA, suite.chainB) + + path.EndpointA.Counterparty.Chain.NextBlock() + + tmConfig, ok := path.EndpointA.ClientConfig.(*ibctesting.TendermintConfig) + suite.Require().True(ok) + + height := path.EndpointA.Counterparty.Chain.LatestCommittedHeader.GetHeight().(clienttypes.Height) + clientState := ibctm.NewClientState( + path.EndpointA.Counterparty.Chain.ChainID, tmConfig.TrustLevel, tmConfig.TrustingPeriod, tmConfig.UnbondingPeriod, tmConfig.MaxClockDrift, + height, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) + consensusState := path.EndpointA.Counterparty.Chain.LatestCommittedHeader.ConsensusState() + + msg, err := clienttypes.NewMsgCreateClient( + clientState, consensusState, path.EndpointA.Chain.SenderAccount.GetAddress().String(), + ) + suite.Require().NoError(err) + + res, err := suite.chainA.SendMsgs(msg) + suite.Require().NoError(err) + suite.Require().NotNil(res) + + events := res.Events + expectedEvents := sdk.Events{ + sdk.NewEvent( + clienttypes.EventTypeCreateClient, + sdk.NewAttribute(clienttypes.AttributeKeyClientID, ibctesting.FirstClientID), + sdk.NewAttribute(clienttypes.AttributeKeyClientType, clientState.ClientType()), + sdk.NewAttribute(clienttypes.AttributeKeyConsensusHeight, clientState.GetLatestHeight().String()), + ), + }.ToABCIEvents() + + var indexSet map[string]struct{} + expectedEvents = sdk.MarkEventsToIndex(expectedEvents, indexSet) + ibctesting.AssertEvents(&suite.Suite, expectedEvents, events) +} + +func (suite *KeeperTestSuite) TestMsgUpdateClientEvents() { + suite.SetupTest() + path := ibctesting.NewPath(suite.chainA, suite.chainB) + + suite.Require().NoError(path.EndpointA.CreateClient()) + + suite.chainB.Coordinator.CommitBlock(suite.chainB) + + header, err := suite.chainA.ConstructUpdateTMClientHeader(suite.chainB, ibctesting.FirstClientID) + suite.Require().NoError(err) + suite.Require().NotNil(header) + + msg, err := clienttypes.NewMsgUpdateClient( + ibctesting.FirstClientID, header, + path.EndpointA.Chain.SenderAccount.GetAddress().String(), + ) + + suite.Require().NoError(err) + + res, err := suite.chainA.SendMsgs(msg) + suite.Require().NoError(err) + suite.Require().NotNil(res) + + events := res.Events + expectedEvents := sdk.Events{ + sdk.NewEvent( + clienttypes.EventTypeUpdateClient, + sdk.NewAttribute(clienttypes.AttributeKeyClientID, ibctesting.FirstClientID), + sdk.NewAttribute(clienttypes.AttributeKeyClientType, path.EndpointA.GetClientState().ClientType()), + sdk.NewAttribute(clienttypes.AttributeKeyConsensusHeight, path.EndpointA.GetClientState().GetLatestHeight().String()), + sdk.NewAttribute(clienttypes.AttributeKeyConsensusHeights, path.EndpointA.GetClientState().GetLatestHeight().String()), + ), + }.ToABCIEvents() + + var indexSet map[string]struct{} + expectedEvents = sdk.MarkEventsToIndex(expectedEvents, indexSet) + ibctesting.AssertEvents(&suite.Suite, expectedEvents, events) +} diff --git a/modules/core/02-client/keeper/keeper_test.go b/modules/core/02-client/keeper/keeper_test.go index 27b44ed7940..8f1680bf5c3 100644 --- a/modules/core/02-client/keeper/keeper_test.go +++ b/modules/core/02-client/keeper/keeper_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "fmt" "math/rand" "testing" "time" @@ -553,7 +554,8 @@ func (suite *KeeperTestSuite) TestIBCSoftwareUpgrade() { suite.Require().NoError(suite.chainA.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainA.GetContext(), oldPlan.Height, bz)) } - err := suite.chainA.App.GetIBCKeeper().ClientKeeper.ScheduleIBCSoftwareUpgrade(suite.chainA.GetContext(), plan, upgradedClientState) + ctx := suite.chainA.GetContext() + err := suite.chainA.App.GetIBCKeeper().ClientKeeper.ScheduleIBCSoftwareUpgrade(ctx, plan, upgradedClientState) if tc.expError == nil { suite.Require().NoError(err) @@ -574,6 +576,18 @@ func (suite *KeeperTestSuite) TestIBCSoftwareUpgrade() { clientState, err := types.UnmarshalClientState(suite.chainA.App.AppCodec(), storedClientState) suite.Require().NoError(err) suite.Require().Equal(upgradedClientState, clientState) + + expectedEvents := sdk.Events{ + sdk.NewEvent( + types.EventTypeScheduleIBCSoftwareUpgrade, + sdk.NewAttribute(types.AttributeKeyUpgradePlanTitle, plan.Name), + sdk.NewAttribute(types.AttributeKeyUpgradePlanHeight, fmt.Sprintf("%d", plan.Height)), + ), + }.ToABCIEvents() + + expectedEvents = sdk.MarkEventsToIndex(expectedEvents, map[string]struct{}{}) + ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) + } else { // check that the new plan wasn't stored storedPlan, err := suite.chainA.GetSimApp().UpgradeKeeper.GetUpgradePlan(suite.chainA.GetContext()) diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index b7f9a315b24..689a115549e 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -887,7 +887,8 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { tc.setup() - _, err = keeper.Keeper.UpgradeClient(*suite.chainA.App.GetIBCKeeper(), suite.chainA.GetContext(), msg) + ctx := suite.chainA.GetContext() + _, err = keeper.Keeper.UpgradeClient(*suite.chainA.App.GetIBCKeeper(), ctx, msg) if tc.expPass { suite.Require().NoError(err, "upgrade handler failed on valid case: %s", tc.name) @@ -895,6 +896,19 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { suite.Require().True(ok) newChainSpecifiedClient := newClient.ZeroCustomFields() suite.Require().Equal(upgradedClient, newChainSpecifiedClient) + + expectedEvents := sdk.Events{ + sdk.NewEvent( + clienttypes.EventTypeUpgradeClient, + sdk.NewAttribute(clienttypes.AttributeKeyClientID, ibctesting.FirstClientID), + sdk.NewAttribute(clienttypes.AttributeKeyClientType, path.EndpointA.GetClientState().ClientType()), + sdk.NewAttribute(clienttypes.AttributeKeyConsensusHeight, path.EndpointA.GetClientState().GetLatestHeight().String()), + ), + }.ToABCIEvents() + + expectedEvents = sdk.MarkEventsToIndex(expectedEvents, map[string]struct{}{}) + ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) + } else { suite.Require().Error(err, "upgrade handler passed on invalid case: %s", tc.name) } diff --git a/testing/events.go b/testing/events.go index 9a9dac0dd9f..9ebe7af9c90 100644 --- a/testing/events.go +++ b/testing/events.go @@ -207,17 +207,12 @@ func AssertEvents( for i, expectedEvent := range expected { for _, actualEvent := range actual { - // the actual event will have an extra attribute added automatically - // by Cosmos SDK since v0.50, that's why we subtract 1 when comparing - // with the number of attributes in the expected event. - if expectedEvent.Type == actualEvent.Type && (len(expectedEvent.Attributes) == len(actualEvent.Attributes)-1) { - // multiple events with the same type may be emitted, only mark the expected event as found - // if all of the attributes match + if shouldProcessEvent(expectedEvent, actualEvent) { attributeMatch := true for _, expectedAttr := range expectedEvent.Attributes { // any expected attributes that are not contained in the actual events will cause this event // not to match - attributeMatch = attributeMatch && slices.Contains(actualEvent.Attributes, expectedAttr) + attributeMatch = attributeMatch && containsAttribute(actualEvent.Attributes, expectedAttr.Key, expectedAttr.Value) } if attributeMatch { @@ -231,3 +226,33 @@ func AssertEvents( suite.Require().True(foundEvents[i], "event: %s was not found in events", expectedEvent.Type) } } + +// shouldProcessEvent returns true if the given expected event should be processed based on event type. +func shouldProcessEvent(expectedEvent abci.Event, actualEvent abci.Event) bool { + if expectedEvent.Type != actualEvent.Type { + return false + } + // the actual event will have an extra attribute added automatically + // by Cosmos SDK since v0.50, that's why we subtract 1 when comparing + // with the number of attributes in the expected event. + if containsAttributeKey(actualEvent.Attributes, "msg_index") { + return len(expectedEvent.Attributes) == len(actualEvent.Attributes)-1 + } + + return len(expectedEvent.Attributes) == len(actualEvent.Attributes) +} + +// containsAttribute returns true if the given key/value pair is contained in the given attributes. +// NOTE: this ignores the indexed field, which can be set or unset depending on how the events are retrieved. +func containsAttribute(attrs []abci.EventAttribute, key, value string) bool { + return slices.ContainsFunc(attrs, func(attr abci.EventAttribute) bool { + return attr.Key == key && attr.Value == value + }) +} + +// containsAttributeKey returns true if the given key is contained in the given attributes. +func containsAttributeKey(attrs []abci.EventAttribute, key string) bool { + return slices.ContainsFunc(attrs, func(attr abci.EventAttribute) bool { + return attr.Key == key + }) +}