imp(core): allow huckleberry events with a prefix

modules/core/keeper/events_test.go

@@ -0,0 +1,106 @@
+package keeper_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/cosmos/ibc-go/v8/modules/core/keeper"
+	"github.com/cosmos/ibc-go/v8/modules/core/types"
+)
+
+func TestConvertToErrorEvents(t *testing.T) {
+	var (
+		events    sdk.Events
+		expEvents sdk.Events
+	)
+
+	tc := []struct {
+		name     string
+		malleate func()
+	}{
+		{
+			"success: nil events",
+			func() {
+				events = nil
+				expEvents = nil
+			},
+		},
+		{
+			"success: empty events",
+			func() {
+				events = sdk.Events{}
+				expEvents = sdk.Events{}
+			},
+		},
+		{
+			"success: event with no attributes",
+			func() {
+				events = sdk.Events{
+					sdk.NewEvent("testevent"),
+				}
+				expEvents = sdk.Events{
+					sdk.NewEvent("testevent"),
+				}
+			},
+		},
+		{
+			"success: event with attributes",
+			func() {
+				events = sdk.Events{
+					sdk.NewEvent("testevent",
+						sdk.NewAttribute("key1", "value1"),
+						sdk.NewAttribute("key2", "value2"),
+					),
+				}
+				expEvents = sdk.Events{
+					sdk.NewEvent("testevent",
+						sdk.NewAttribute(types.ErrorAttributeKeyPrefix+"key1", "value1"),
+						sdk.NewAttribute(types.ErrorAttributeKeyPrefix+"key2", "value2"),
+					),
+				}
+			},
+		},
+		{
+			"success: multiple events with attributes",
+			func() {
+				events = sdk.Events{
+					sdk.NewEvent("testevent1",
+						sdk.NewAttribute("key1", "value1"),
+						sdk.NewAttribute("key2", "value2"),
+					),
+					sdk.NewEvent("testevent2",
+						sdk.NewAttribute("key3", "value3"),
+						sdk.NewAttribute("key4", "value4"),
+					),
+				}
+				expEvents = sdk.Events{
+					sdk.NewEvent("testevent1",
+						sdk.NewAttribute(types.ErrorAttributeKeyPrefix+"key1", "value1"),
+						sdk.NewAttribute(types.ErrorAttributeKeyPrefix+"key2", "value2"),
+					),
+					sdk.NewEvent("testevent2",
+						sdk.NewAttribute(types.ErrorAttributeKeyPrefix+"key3", "value3"),
+						sdk.NewAttribute(types.ErrorAttributeKeyPrefix+"key4", "value4"),
+					),
+				}
+			},
+		},
+	}
+
+	for _, tc := range tc {
+		t.Run(tc.name, func(t *testing.T) {
+			// initial events and expected events are reset so that the test fails if
+			// the malleate function does not set them
+			events = nil
+			expEvents = sdk.Events{}
+
+			tc.malleate()
+
+			newEvents := keeper.ConvertToErrorEvents(events)
+			require.Equal(t, expEvents, newEvents)
+		})
+	}
+}

modules/core/keeper/export_test.go

@@ -0,0 +1,9 @@
+package keeper
+
+import sdk "github.com/cosmos/cosmos-sdk/types"
+
+// ConvertToErrorEvents is a wrapper around convertToErrorEvents
+// to allow the function to be directly called in tests.
+func ConvertToErrorEvents(events sdk.Events) sdk.Events {
+	return convertToErrorEvents(events)
+}

modules/core/keeper/msg_server.go

@@ -494,6 +494,9 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke
 	if ack == nil || ack.Success() {
 		// write application state changes for asynchronous and successful acknowledgements
 		writeFn()
+	} else {
+		// Modify events in cached context to reflect unsuccessful acknowledgement
+		ctx.EventManager().EmitEvents(convertToErrorEvents(cacheCtx.EventManager().Events()))
 	}
 
 	// Set packet acknowledgement only if the acknowledgement is not nil.
@@ -1174,3 +1177,25 @@ func (k Keeper) UpdateChannelParams(goCtx context.Context, msg *channeltypes.Msg
 
 	return &channeltypes.MsgUpdateParamsResponse{}, nil
 }
+
+// convertToErrorEvents converts all events to error events by appending the
+// error attribute prefix to each event's attribute key.
+func convertToErrorEvents(events sdk.Events) sdk.Events {
+	if events == nil {
+		return nil
+	}
+
+	newEvents := make(sdk.Events, len(events))
+	for i, event := range events {
+		newAttributes := make([]sdk.Attribute, len(event.Attributes))
+		for j, attribute := range event.Attributes {
+			newAttributes[j] = sdk.NewAttribute(coretypes.ErrorAttributeKeyPrefix+attribute.Key, attribute.Value)
+		}
+
+		// no need to append the error attribute prefix to the event type because
+		// the event type is not associated to a value that can be misinterpreted
+		newEvents[i] = sdk.NewEvent(event.Type, newAttributes...)
+	}
+
+	return newEvents
+}

modules/core/keeper/msg_server_test.go

@@ -39,6 +39,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 		packet channeltypes.Packet
 		path   *ibctesting.Path
 		async  bool // indicate no ack written
+		replay bool // indicate replay (no-op)
 	)
 
 	testCases := []struct {
@@ -127,6 +128,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 			// mock will panic if application callback is called twice on the same packet
 			path.SetChannelOrdered()
 			suite.coordinator.Setup(path)
+			replay = true
 
 			sequence, err := path.EndpointA.SendPacket(timeoutHeight, 0, ibctesting.MockPacketData)
 			suite.Require().NoError(err)
@@ -138,6 +140,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 		{"successful no-op: UNORDERED - packet already received (replay)", func() {
 			// mock will panic if application callback is called twice on the same packet
 			suite.coordinator.Setup(path)
+			replay = true
 
 			sequence, err := path.EndpointA.SendPacket(timeoutHeight, 0, ibctesting.MockPacketData)
 			suite.Require().NoError(err)
@@ -154,6 +157,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 		suite.Run(tc.name, func() {
 			suite.SetupTest() // reset
 			async = false     // reset
+			replay = false    // reset
 			path = ibctesting.NewPath(suite.chainA, suite.chainB)
 
 			tc.malleate()
@@ -171,7 +175,10 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 
 			msg := channeltypes.NewMsgRecvPacket(packet, proof, proofHeight, suite.chainB.SenderAccount.GetAddress().String())
 
-			_, err := keeper.Keeper.RecvPacket(*suite.chainB.App.GetIBCKeeper(), suite.chainB.GetContext(), msg)
+			ctx := suite.chainB.GetContext()
+			_, err := keeper.Keeper.RecvPacket(*suite.chainB.App.GetIBCKeeper(), ctx, msg)
+
+			events := ctx.EventManager().Events()
 
 			if tc.expPass {
 				suite.Require().NoError(err)
@@ -184,8 +191,20 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 				_, exists := suite.chainB.GetSimApp().ScopedIBCMockKeeper.GetCapability(suite.chainB.GetContext(), ibcmock.GetMockRecvCanaryCapabilityName(packet))
 				if tc.expRevert {
 					suite.Require().False(exists, "capability exists in store even after callback reverted")
+
+					// context events should contain error events
+					suite.Require().Contains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0])
 				} else {
 					suite.Require().True(exists, "callback state not persisted when revert is false")
+
+					if replay {
+						// context should not contain application events
+						suite.Require().NotContains(events, ibcmock.NewMockRecvPacketEvent())
+						suite.Require().NotContains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0])
+					} else {
+						// context events should contain application events
+						suite.Require().Contains(events, ibcmock.NewMockRecvPacketEvent())
+					}
 				}
 
 				// verify if ack was written
@@ -291,6 +310,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 	var (
 		packet channeltypes.Packet
 		path   *ibctesting.Path
+		replay bool // indicate replay (no-op)
 	)
 
 	testCases := []struct {
@@ -361,6 +381,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 		}, false},
 		{"successful no-op: ORDERED - packet already acknowledged (replay)", func() {
 			suite.coordinator.Setup(path)
+			replay = true
 
 			sequence, err := path.EndpointA.SendPacket(timeoutHeight, 0, ibctesting.MockPacketData)
 			suite.Require().NoError(err)
@@ -374,6 +395,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 		}, true},
 		{"successful no-op: UNORDERED - packet already acknowledged (replay)", func() {
 			suite.coordinator.Setup(path)
+			replay = true
 
 			sequence, err := path.EndpointA.SendPacket(timeoutHeight, 0, ibctesting.MockPacketData)
 			suite.Require().NoError(err)
@@ -392,6 +414,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 
 		suite.Run(tc.name, func() {
 			suite.SetupTest() // reset
+			replay = false    // reset
 			path = ibctesting.NewPath(suite.chainA, suite.chainB)
 
 			tc.malleate()
@@ -407,7 +430,10 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 
 			msg := channeltypes.NewMsgAcknowledgement(packet, ibcmock.MockAcknowledgement.Acknowledgement(), proof, proofHeight, suite.chainA.SenderAccount.GetAddress().String())
 
-			_, err := keeper.Keeper.Acknowledgement(*suite.chainA.App.GetIBCKeeper(), suite.chainA.GetContext(), msg)
+			ctx := suite.chainA.GetContext()
+			_, err := keeper.Keeper.Acknowledgement(*suite.chainA.App.GetIBCKeeper(), ctx, msg)
+
+			events := ctx.EventManager().Events()
 
 			if tc.expPass {
 				suite.Require().NoError(err)
@@ -419,6 +445,14 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 				// replay should not error as it is treated as a no-op
 				_, err := keeper.Keeper.Acknowledgement(*suite.chainA.App.GetIBCKeeper(), suite.chainA.GetContext(), msg)
 				suite.Require().NoError(err)
+
+				if replay {
+					// context should not contain application events
+					suite.Require().NotContains(events, ibcmock.NewMockAckPacketEvent())
+				} else {
+					// context events should contain application events
+					suite.Require().Contains(events, ibcmock.NewMockAckPacketEvent())
+				}
 			} else {
 				suite.Require().Error(err)
 			}
@@ -436,6 +470,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() {
 		packet    channeltypes.Packet
 		packetKey []byte
 		path      *ibctesting.Path
+		noop      bool // indicate no-op
 	)
 
 	testCases := []struct {
@@ -528,6 +563,8 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() {
 		}, false},
 		{"successful no-op: UNORDERED - packet not sent", func() {
 			suite.coordinator.Setup(path)
+			noop = true
+
 			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(0, 1), 0)
 			packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 		}, true},
@@ -538,6 +575,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() {
 
 		suite.Run(tc.name, func() {
 			suite.SetupTest() // reset
+			noop = false      // reset
 			path = ibctesting.NewPath(suite.chainA, suite.chainB)
 
 			tc.malleate()
@@ -552,7 +590,10 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() {
 
 			msg := channeltypes.NewMsgTimeout(packet, 1, proof, proofHeight, suite.chainA.SenderAccount.GetAddress().String())
 
-			_, err := keeper.Keeper.Timeout(*suite.chainA.App.GetIBCKeeper(), suite.chainA.GetContext(), msg)
+			ctx := suite.chainA.GetContext()
+			_, err := keeper.Keeper.Timeout(*suite.chainA.App.GetIBCKeeper(), ctx, msg)
+
+			events := ctx.EventManager().Events()
 
 			if tc.expPass {
 				suite.Require().NoError(err)
@@ -565,6 +606,14 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() {
 				has := suite.chainA.App.GetIBCKeeper().ChannelKeeper.HasPacketCommitment(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 				suite.Require().False(has)
 
+				if noop {
+					// context should not contain application events
+					suite.Require().NotContains(events, ibcmock.NewMockTimeoutPacketEvent())
+				} else {
+					// context should contain application events
+					suite.Require().Contains(events, ibcmock.NewMockTimeoutPacketEvent())
+				}
+
 			} else {
 				suite.Require().Error(err)
 			}

modules/core/types/events.go

@@ -0,0 +1,3 @@
+package types
+
+const ErrorAttributeKeyPrefix = "ibccallbackerror-"

testing/mock/events.go

@@ -0,0 +1,39 @@
+package mock
+
+import sdk "github.com/cosmos/cosmos-sdk/types"
+
+const (
+	MockEventTypeRecvPacket    = "mock-recv-packet"
+	MockEventTypeAckPacket     = "mock-ack-packet"
+	MockEventTypeTimeoutPacket = "mock-timeout"
+
+	MockAttributeKey1 = "mock-attribute-key-1"
+	MockAttributeKey2 = "mock-attribute-key-2"
+
+	MockAttributeValue1 = "mock-attribute-value-1"
+	MockAttributeValue2 = "mock-attribute-value-2"
+)
+
+// NewMockRecvPacketEvent returns a mock receive packet event
+func NewMockRecvPacketEvent() sdk.Event {
+	return newMockEvent(MockEventTypeRecvPacket)
+}
+
+// NewMockAckPacketEvent returns a mock acknowledgement packet event
+func NewMockAckPacketEvent() sdk.Event {
+	return newMockEvent(MockEventTypeAckPacket)
+}
+
+// NewMockTimeoutPacketEvent emits a mock timeout packet event
+func NewMockTimeoutPacketEvent() sdk.Event {
+	return newMockEvent(MockEventTypeTimeoutPacket)
+}
+
+// emitMockEvent returns a mock event with the given event type
+func newMockEvent(eventType string) sdk.Event {
+	return sdk.NewEvent(
+		eventType,
+		sdk.NewAttribute(MockAttributeKey1, MockAttributeValue1),
+		sdk.NewAttribute(MockAttributeKey2, MockAttributeValue2),
+	)
+}

testing/mock/ibc_module.go

@@ -136,6 +136,8 @@ func (im IBCModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, re
 		panic(err)
 	}
 
+	ctx.EventManager().EmitEvent(NewMockRecvPacketEvent())
+
 	if bytes.Equal(MockPacketData, packet.GetData()) {
 		return MockAcknowledgement
 	} else if bytes.Equal(MockAsyncPacketData, packet.GetData()) {
@@ -158,6 +160,8 @@ func (im IBCModule) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes
 		panic(err)
 	}
 
+	ctx.EventManager().EmitEvent(NewMockAckPacketEvent())
+
 	return nil
 }
 
@@ -174,6 +178,8 @@ func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet,
 		panic(err)
 	}
 
+	ctx.EventManager().EmitEvent(NewMockTimeoutPacketEvent())
+
 	return nil
 }