From 0ffbcecb103e55cf783434cd52a8b00c934cc371 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Colin=20Axn=C3=A9r?=
 <25233464+colin-axner@users.noreply.github.com>
Date: Thu, 15 Jul 2021 17:26:53 +0200
Subject: [PATCH 1/6] create initial changes for delivertx handling

---
 modules/core/04-channel/keeper/packet.go  | 23 +++++++-----
 modules/core/04-channel/keeper/timeout.go | 14 ++++++-
 modules/core/keeper/msg_server.go         | 43 +++++++++++++++++++---
 modules/core/keeper/msg_server_test.go    | 45 +++++++++++++----------
 4 files changed, 89 insertions(+), 36 deletions(-)

diff --git a/modules/core/04-channel/keeper/packet.go b/modules/core/04-channel/keeper/packet.go
index 571b7ae3b53..dce5ed0a6e0 100644
--- a/modules/core/04-channel/keeper/packet.go
+++ b/modules/core/04-channel/keeper/packet.go
@@ -249,10 +249,10 @@ func (k Keeper) RecvPacket(
 		// check if the packet receipt has been received already for unordered channels
 		_, found := k.GetPacketReceipt(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 		if found {
-			return sdkerrors.Wrapf(
-				types.ErrPacketReceived,
-				"packet sequence (%d)", packet.GetSequence(),
-			)
+			// This error indicates that the packet has already been relayed. Core IBC will
+			// treat this error as a no-op in order to prevent an entire relay transaction
+			// from failing and consuming unnecessary fees.
+			return types.ErrPacketReceived
 		}
 
 		// All verification complete, update state
@@ -271,12 +271,11 @@ func (k Keeper) RecvPacket(
 			)
 		}
 
-		// helpful error message for relayers
 		if packet.GetSequence() < nextSequenceRecv {
-			return sdkerrors.Wrapf(
-				types.ErrPacketReceived,
-				"packet sequence (%d), next sequence receive (%d)", packet.GetSequence(), nextSequenceRecv,
-			)
+			// This error indicates that the packet has already been relayed. Core IBC will
+			// treat this error as a no-op in order to prevent an entire relay transaction
+			// from failing and consuming unnecessary fees.
+			return types.ErrPacketReceived
 		}
 
 		if packet.GetSequence() != nextSequenceRecv {
@@ -480,7 +479,11 @@ func (k Keeper) AcknowledgePacket(
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
 	if len(commitment) == 0 {
-		return sdkerrors.Wrapf(types.ErrPacketCommitmentNotFound, "packet with sequence (%d) has been acknowledged, or timed out. In rare cases, the packet referenced was never sent, likely due to the relayer being misconfigured", packet.GetSequence())
+		// This error indicates that the acknowledgement has already been relayed
+		// or there is a misconfigured relayer attempting to prove an acknowledgement
+		// for a packet never sent. Core IBC will treat this error as a no-op in order to
+		// prevent an entire relay transaction from failing and consuming unnecessary fees.
+		return types.ErrPacketCommitmentNotFound
 	}
 
 	packetCommitment := types.CommitPacket(k.cdc, packet)
diff --git a/modules/core/04-channel/keeper/timeout.go b/modules/core/04-channel/keeper/timeout.go
index caf4e037cf8..e18e4613c14 100644
--- a/modules/core/04-channel/keeper/timeout.go
+++ b/modules/core/04-channel/keeper/timeout.go
@@ -81,7 +81,11 @@ func (k Keeper) TimeoutPacket(
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
 	if len(commitment) == 0 {
-		return sdkerrors.Wrapf(types.ErrPacketCommitmentNotFound, "packet with sequence (%d) has been acknowledged or timed out. In rare cases, the packet referenced was never sent, likely due to the relayer being misconfigured", packet.GetSequence())
+		// This error indicates that the timeout has already been relayed
+		// or there is a misconfigured relayer attempting to prove a timeout
+		// for a packet never sent. Core IBC will treat this error as a no-op in order to
+		// prevent an entire relay transaction from failing and consuming unnecessary fees.
+		return types.ErrPacketCommitmentNotFound
 	}
 
 	packetCommitment := types.CommitPacket(k.cdc, packet)
@@ -222,6 +226,14 @@ func (k Keeper) TimeoutOnClose(
 
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
+	if len(commitment) == 0 {
+		// This error indicates that the timeout has already been relayed
+		// or there is a misconfigured relayer attempting to prove a timeout
+		// for a packet never sent. Core IBC will treat this error as a no-op in order to
+		// prevent an entire relay transaction from failing and consuming unnecessary fees.
+		return types.ErrPacketCommitmentNotFound
+	}
+
 	packetCommitment := types.CommitPacket(k.cdc, packet)
 
 	// verify we sent the packet and haven't cleared it out yet
diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go
index ab9e583385c..73a093bcb1c 100644
--- a/modules/core/keeper/msg_server.go
+++ b/modules/core/keeper/msg_server.go
@@ -492,13 +492,21 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke
 	}
 
 	// Perform TAO verification
-	if err := k.ChannelKeeper.RecvPacket(ctx, cap, msg.Packet, msg.ProofCommitment, msg.ProofHeight); err != nil {
+	// If the packet was already received we want to perform a no-op. We use a cached context to prevent
+	// accidental state changes
+	cacheCtx, writeFn := ctx.CacheContext()
+	if err := k.ChannelKeeper.RecvPacket(cacheCtx, cap, msg.Packet, msg.ProofCommitment, msg.ProofHeight); err != nil {
+		// packet already received
+		if err == channeltypes.ErrPacketReceived {
+			return &channeltypes.MsgRecvPacketResponse{}, nil // no-op
+		}
 		return nil, sdkerrors.Wrap(err, "receive packet verification failed")
 	}
+	writeFn()
 
 	// Perform application logic callback
 	// Cache context so that we may discard state changes from callback if the acknowledgement is unsuccessful.
-	cacheCtx, writeFn := ctx.CacheContext()
+	cacheCtx, writeFn = ctx.CacheContext()
 	ack := cbs.OnRecvPacket(cacheCtx, msg.Packet, relayer)
 	// This doesn't cause duplicate events to be emitted.
 	// NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context.
@@ -556,9 +564,18 @@ func (k Keeper) Timeout(goCtx context.Context, msg *channeltypes.MsgTimeout) (*c
 	}
 
 	// Perform TAO verification
-	if err := k.ChannelKeeper.TimeoutPacket(ctx, msg.Packet, msg.ProofUnreceived, msg.ProofHeight, msg.NextSequenceRecv); err != nil {
+	// If the timeout was already received we want to perform a no-op. We use a cached context to prevent
+	// accidental state changes
+	cacheCtx, writeFn := ctx.CacheContext()
+	if err := k.ChannelKeeper.TimeoutPacket(cacheCtx, msg.Packet, msg.ProofUnreceived, msg.ProofHeight, msg.NextSequenceRecv); err != nil {
+		// timeout already received
+		// NOTE: ordered channels will error on a closed channel upon replay
+		if err == channeltypes.ErrPacketCommitmentNotFound {
+			return &channeltypes.MsgTimeoutResponse{}, nil // no-op
+		}
 		return nil, sdkerrors.Wrap(err, "timeout packet verification failed")
 	}
+	writeFn()
 
 	// Perform application logic callback
 	err = cbs.OnTimeoutPacket(ctx, msg.Packet, relayer)
@@ -610,9 +627,17 @@ func (k Keeper) TimeoutOnClose(goCtx context.Context, msg *channeltypes.MsgTimeo
 	}
 
 	// Perform TAO verification
-	if err := k.ChannelKeeper.TimeoutOnClose(ctx, cap, msg.Packet, msg.ProofUnreceived, msg.ProofClose, msg.ProofHeight, msg.NextSequenceRecv); err != nil {
+	// If the timeout was already received we want to perform a no-op. We use a cached context to prevent
+	// accidental state changes
+	cacheCtx, writeFn := ctx.CacheContext()
+	if err := k.ChannelKeeper.TimeoutOnClose(cacheCtx, cap, msg.Packet, msg.ProofUnreceived, msg.ProofClose, msg.ProofHeight, msg.NextSequenceRecv); err != nil {
+		// timeout already received
+		if err == channeltypes.ErrPacketCommitmentNotFound {
+			return &channeltypes.MsgTimeoutOnCloseResponse{}, nil // no-op
+		}
 		return nil, sdkerrors.Wrap(err, "timeout on close packet verification failed")
 	}
+	writeFn()
 
 	// Perform application logic callback
 	// NOTE: MsgTimeout and MsgTimeoutOnClose use the same "OnTimeoutPacket"
@@ -666,9 +691,17 @@ func (k Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAckn
 	}
 
 	// Perform TAO verification
-	if err := k.ChannelKeeper.AcknowledgePacket(ctx, cap, msg.Packet, msg.Acknowledgement, msg.ProofAcked, msg.ProofHeight); err != nil {
+	// If the acknowledgement was already received we want to perform a no-op. We use a cached context to prevent
+	// accidental state changes
+	cacheCtx, writeFn := ctx.CacheContext()
+	if err := k.ChannelKeeper.AcknowledgePacket(cacheCtx, cap, msg.Packet, msg.Acknowledgement, msg.ProofAcked, msg.ProofHeight); err != nil {
+		// acknowledgement already received
+		if err == channeltypes.ErrPacketCommitmentNotFound {
+			return &channeltypes.MsgAcknowledgementResponse{}, nil // no-op
+		}
 		return nil, sdkerrors.Wrap(err, "acknowledge packet verification failed")
 	}
+	writeFn()
 
 	// Perform application logic callback
 	err = cbs.OnAcknowledgementPacket(ctx, msg.Packet, msg.Acknowledgement, relayer)
diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go
index a337c1fb323..94c262e413d 100644
--- a/modules/core/keeper/msg_server_test.go
+++ b/modules/core/keeper/msg_server_test.go
@@ -141,7 +141,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 			suite.coordinator.Setup(path)
 			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
 		}, false, false},
-		{"ORDERED: packet already received (replay)", func() {
+		{"successful no-op: ORDERED - packet already received (replay)", func() {
 			path.SetChannelOrdered()
 			suite.coordinator.Setup(path)
 			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
@@ -151,8 +151,8 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 
 			err = path.EndpointB.RecvPacket(packet)
 			suite.Require().NoError(err)
-		}, false, false},
-		{"UNORDERED: packet already received (replay)", func() {
+		}, true, true},
+		{"successful no-op: UNORDERED - packet already received (replay)", func() {
 			suite.coordinator.Setup(path)
 			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
 
@@ -161,7 +161,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 
 			err = path.EndpointB.RecvPacket(packet)
 			suite.Require().NoError(err)
-		}, false, false},
+		}, true, true},
 	}
 
 	for _, tc := range testCases {
@@ -185,9 +185,9 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 			if tc.expPass {
 				suite.Require().NoError(err)
 
-				// replay should fail since state changes occur
+				// replay should not fail since it will be treated as a no-op
 				_, err := keeper.Keeper.RecvPacket(*suite.chainB.App.GetIBCKeeper(), sdk.WrapSDKContext(suite.chainB.GetContext()), msg)
-				suite.Require().Error(err)
+				suite.Require().NoError(err)
 
 				// check that callback state was handled correctly
 				_, exists := suite.chainB.GetSimApp().ScopedIBCMockKeeper.GetCapability(suite.chainB.GetContext(), ibctesting.MockCanaryCapabilityName)
@@ -293,7 +293,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 			err := path.EndpointA.SendPacket(packet)
 			suite.Require().NoError(err)
 		}, false},
-		{"ORDERED: packet already acknowledged (replay)", func() {
+		{"successful no-op: ORDERED - packet already acknowledged (replay)", func() {
 			suite.coordinator.Setup(path)
 			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
 
@@ -305,8 +305,8 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 
 			err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement)
 			suite.Require().NoError(err)
-		}, false},
-		{"UNORDERED: packet already acknowledged (replay)", func() {
+		}, true},
+		{"successful no-op: UNORDERED - packet already acknowledged (replay)", func() {
 			suite.coordinator.Setup(path)
 
 			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
@@ -319,7 +319,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 
 			err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement)
 			suite.Require().NoError(err)
-		}, false},
+		}, true},
 	}
 
 	for _, tc := range testCases {
@@ -341,9 +341,9 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 			if tc.expPass {
 				suite.Require().NoError(err)
 
-				// replay should an error
+				// replay should not error as it is treated as a no-op
 				_, err := keeper.Keeper.Acknowledgement(*suite.chainA.App.GetIBCKeeper(), sdk.WrapSDKContext(suite.chainA.GetContext()), msg)
-				suite.Require().Error(err)
+				suite.Require().NoError(err)
 
 				// verify packet commitment was deleted on source chain
 				has := suite.chainA.App.GetIBCKeeper().ChannelKeeper.HasPacketCommitment(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
@@ -441,11 +441,11 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() {
 
 			packetKey = host.NextSequenceRecvKey(packet.GetDestPort(), packet.GetDestChannel())
 		}, false},
-		{"UNORDERED: packet not sent", func() {
+		{"successful no-op: UNORDERED - packet not sent", func() {
 			suite.coordinator.Setup(path)
 			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
 			packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
-		}, false},
+		}, true},
 	}
 
 	for _, tc := range testCases {
@@ -466,9 +466,14 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() {
 			if tc.expPass {
 				suite.Require().NoError(err)
 
-				// replay should return an error
+				if path.EndpointA.ChannelConfig.Order == channeltypes.ORDERED {
+					// replay should return an error as replayed transaction error on the channel being closed
+					_, err := keeper.Keeper.Timeout(*suite.chainA.App.GetIBCKeeper(), sdk.WrapSDKContext(suite.chainA.GetContext()), msg)
+					suite.Require().Error(err)
+				}
+				// replay should not return an error as it is treated as a no-op
 				_, err := keeper.Keeper.Timeout(*suite.chainA.App.GetIBCKeeper(), sdk.WrapSDKContext(suite.chainA.GetContext()), msg)
-				suite.Require().Error(err)
+				suite.Require().NoError(err)
 
 				// verify packet commitment was deleted on source chain
 				has := suite.chainA.App.GetIBCKeeper().ChannelKeeper.HasPacketCommitment(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
@@ -577,14 +582,14 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() {
 
 			packetKey = host.NextSequenceRecvKey(packet.GetDestPort(), packet.GetDestChannel())
 		}, false},
-		{"UNORDERED: packet not sent", func() {
+		{"successful no-op: UNORDERED - packet not sent", func() {
 			suite.coordinator.Setup(path)
 			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
 			packetKey = host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 
 			// close counterparty channel
 			path.EndpointB.SetChannelClosed()
-		}, false},
+		}, true},
 		{"ORDERED: channel not closed", func() {
 			path.SetChannelOrdered()
 			suite.coordinator.Setup(path)
@@ -622,9 +627,9 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() {
 			if tc.expPass {
 				suite.Require().NoError(err)
 
-				// replay should return an error
+				// replay should not return an error as it will be treated as a no-op
 				_, err := keeper.Keeper.TimeoutOnClose(*suite.chainA.App.GetIBCKeeper(), sdk.WrapSDKContext(suite.chainA.GetContext()), msg)
-				suite.Require().Error(err)
+				suite.Require().NoError(err)
 
 				// verify packet commitment was deleted on source chain
 				has := suite.chainA.App.GetIBCKeeper().ChannelKeeper.HasPacketCommitment(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())

From 990d6a8945fd1212d8fd7baba134cc1c79605b29 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Colin=20Axn=C3=A9r?=
 <25233464+colin-axner@users.noreply.github.com>
Date: Mon, 19 Jul 2021 13:47:33 +0200
Subject: [PATCH 2/6] handle closed channel no-ops, fix tests

---
 modules/core/04-channel/keeper/packet.go      |  6 +--
 modules/core/04-channel/keeper/packet_test.go | 20 ++++----
 modules/core/04-channel/keeper/timeout.go     | 18 +++----
 .../core/04-channel/keeper/timeout_test.go    | 24 +++++----
 modules/core/04-channel/types/errors.go       |  3 ++
 modules/core/keeper/msg_server.go             | 31 +++++++-----
 modules/core/keeper/msg_server_test.go        | 49 ++++++++++++-------
 testing/mock/mock.go                          | 40 +++++++++++----
 testing/values.go                             | 14 ++++--
 9 files changed, 131 insertions(+), 74 deletions(-)

diff --git a/modules/core/04-channel/keeper/packet.go b/modules/core/04-channel/keeper/packet.go
index dce5ed0a6e0..02843c6b408 100644
--- a/modules/core/04-channel/keeper/packet.go
+++ b/modules/core/04-channel/keeper/packet.go
@@ -252,7 +252,7 @@ func (k Keeper) RecvPacket(
 			// This error indicates that the packet has already been relayed. Core IBC will
 			// treat this error as a no-op in order to prevent an entire relay transaction
 			// from failing and consuming unnecessary fees.
-			return types.ErrPacketReceived
+			return types.ErrNoOpMsg
 		}
 
 		// All verification complete, update state
@@ -275,7 +275,7 @@ func (k Keeper) RecvPacket(
 			// This error indicates that the packet has already been relayed. Core IBC will
 			// treat this error as a no-op in order to prevent an entire relay transaction
 			// from failing and consuming unnecessary fees.
-			return types.ErrPacketReceived
+			return types.ErrNoOpMsg
 		}
 
 		if packet.GetSequence() != nextSequenceRecv {
@@ -483,7 +483,7 @@ func (k Keeper) AcknowledgePacket(
 		// or there is a misconfigured relayer attempting to prove an acknowledgement
 		// for a packet never sent. Core IBC will treat this error as a no-op in order to
 		// prevent an entire relay transaction from failing and consuming unnecessary fees.
-		return types.ErrPacketCommitmentNotFound
+		return types.ErrNoOpMsg
 	}
 
 	packetCommitment := types.CommitPacket(k.cdc, packet)
diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go
index d1cb11370c3..9ef7798140d 100644
--- a/modules/core/04-channel/keeper/packet_test.go
+++ b/modules/core/04-channel/keeper/packet_test.go
@@ -267,8 +267,8 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			// attempts to receive packet 2 without receiving packet 1
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, true},
-		{"packet already relayed ORDERED channel", func() {
-			expError = types.ErrPacketReceived
+		{"packet already relayed ORDERED channel (no-op)", func() {
+			expError = types.ErrNoOpMsg
 
 			path.SetChannelOrdered()
 			suite.coordinator.Setup(path)
@@ -281,8 +281,8 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			err = path.EndpointB.RecvPacket(packet.(types.Packet))
 			suite.Require().NoError(err)
 		}, false},
-		{"packet already relayed UNORDERED channel", func() {
-			expError = types.ErrPacketReceived
+		{"packet already relayed UNORDERED channel (no-op)", func() {
+			expError = types.ErrNoOpMsg
 
 			// setup uses an UNORDERED channel
 			suite.coordinator.Setup(path)
@@ -428,7 +428,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			path.EndpointB.UpdateClient()
 		}, false},
 		{"receipt already stored", func() {
-			expError = types.ErrPacketReceived
+			expError = types.ErrNoOpMsg
 			suite.coordinator.Setup(path)
 
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
@@ -617,8 +617,8 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 		}, true},
-		{"packet already acknowledged ordered channel", func() {
-			expError = types.ErrPacketCommitmentNotFound
+		{"packet already acknowledged ordered channel (no-op)", func() {
+			expError = types.ErrNoOpMsg
 
 			path.SetChannelOrdered()
 			suite.coordinator.Setup(path)
@@ -636,8 +636,8 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			err = path.EndpointA.AcknowledgePacket(packet, ack.Acknowledgement())
 			suite.Require().NoError(err)
 		}, false},
-		{"packet already acknowledged unordered channel", func() {
-			expError = types.ErrPacketCommitmentNotFound
+		{"packet already acknowledged unordered channel (no-op)", func() {
+			expError = types.ErrNoOpMsg
 
 			// setup uses an UNORDERED channel
 			suite.coordinator.Setup(path)
@@ -738,7 +738,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 		}, false},
 		{"packet hasn't been sent", func() {
-			expError = types.ErrPacketCommitmentNotFound
+			expError = types.ErrNoOpMsg
 
 			// packet commitment never written
 			suite.coordinator.Setup(path)
diff --git a/modules/core/04-channel/keeper/timeout.go b/modules/core/04-channel/keeper/timeout.go
index e18e4613c14..181441d45a9 100644
--- a/modules/core/04-channel/keeper/timeout.go
+++ b/modules/core/04-channel/keeper/timeout.go
@@ -34,13 +34,6 @@ func (k Keeper) TimeoutPacket(
 		)
 	}
 
-	if channel.State != types.OPEN {
-		return sdkerrors.Wrapf(
-			types.ErrInvalidChannelState,
-			"channel state is not OPEN (got %s)", channel.State.String(),
-		)
-	}
-
 	// NOTE: TimeoutPacket is called by the AnteHandler which acts upon the packet.Route(),
 	// so the capability authentication can be omitted here
 
@@ -85,7 +78,14 @@ func (k Keeper) TimeoutPacket(
 		// or there is a misconfigured relayer attempting to prove a timeout
 		// for a packet never sent. Core IBC will treat this error as a no-op in order to
 		// prevent an entire relay transaction from failing and consuming unnecessary fees.
-		return types.ErrPacketCommitmentNotFound
+		return types.ErrNoOpMsg
+	}
+
+	if channel.State != types.OPEN {
+		return sdkerrors.Wrapf(
+			types.ErrInvalidChannelState,
+			"channel state is not OPEN (got %s)", channel.State.String(),
+		)
 	}
 
 	packetCommitment := types.CommitPacket(k.cdc, packet)
@@ -231,7 +231,7 @@ func (k Keeper) TimeoutOnClose(
 		// or there is a misconfigured relayer attempting to prove a timeout
 		// for a packet never sent. Core IBC will treat this error as a no-op in order to
 		// prevent an entire relay transaction from failing and consuming unnecessary fees.
-		return types.ErrPacketCommitmentNotFound
+		return types.ErrNoOpMsg
 	}
 
 	packetCommitment := types.CommitPacket(k.cdc, packet)
diff --git a/modules/core/04-channel/keeper/timeout_test.go b/modules/core/04-channel/keeper/timeout_test.go
index 460e6097632..7cdb88cdc05 100644
--- a/modules/core/04-channel/keeper/timeout_test.go
+++ b/modules/core/04-channel/keeper/timeout_test.go
@@ -48,7 +48,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			path.EndpointA.UpdateClient()
 		}, true},
 		{"packet already timed out: ORDERED", func() {
-			expError = types.ErrInvalidChannelState
+			expError = types.ErrNoOpMsg
 			ordered = true
 			path.SetChannelOrdered()
 
@@ -62,7 +62,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			suite.Require().NoError(err)
 		}, false},
 		{"packet already timed out: UNORDERED", func() {
-			expError = types.ErrPacketCommitmentNotFound
+			expError = types.ErrNoOpMsg
 			ordered = false
 
 			suite.coordinator.Setup(path)
@@ -83,9 +83,13 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 		{"channel not open", func() {
 			expError = types.ErrInvalidChannelState
 			suite.coordinator.Setup(path)
-			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
+			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.GetClientState().GetLatestHeight().Increment().(clienttypes.Height), disabledTimeoutTimestamp)
+			err := path.EndpointA.SendPacket(packet)
+			suite.Require().NoError(err)
+			// need to update chainA's client representing chainB to prove missing ack
+			path.EndpointA.UpdateClient()
 
-			err := path.EndpointA.SetChannelClosed()
+			err = path.EndpointA.SetChannelClosed()
 			suite.Require().NoError(err)
 		}, false},
 		{"packet destination port ≠ channel counterparty port", func() {
@@ -130,7 +134,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			path.EndpointA.UpdateClient()
 		}, false},
 		{"packet hasn't been sent", func() {
-			expError = types.ErrPacketCommitmentNotFound
+			expError = types.ErrNoOpMsg
 			ordered = true
 			path.SetChannelOrdered()
 
@@ -182,10 +186,12 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			orderedPacketKey := host.NextSequenceRecvKey(packet.GetDestPort(), packet.GetDestChannel())
 			unorderedPacketKey := host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 
-			if ordered {
-				proof, proofHeight = suite.chainB.QueryProof(orderedPacketKey)
-			} else {
-				proof, proofHeight = suite.chainB.QueryProof(unorderedPacketKey)
+			if path.EndpointB.ConnectionID != "" {
+				if ordered {
+					proof, proofHeight = path.EndpointB.QueryProof(orderedPacketKey)
+				} else {
+					proof, proofHeight = path.EndpointB.QueryProof(unorderedPacketKey)
+				}
 			}
 
 			err := suite.chainA.App.GetIBCKeeper().ChannelKeeper.TimeoutPacket(suite.chainA.GetContext(), packet, proof, proofHeight, nextSeqRecv)
diff --git a/modules/core/04-channel/types/errors.go b/modules/core/04-channel/types/errors.go
index 30293eddf07..326e8af7f46 100644
--- a/modules/core/04-channel/types/errors.go
+++ b/modules/core/04-channel/types/errors.go
@@ -30,4 +30,7 @@ var (
 
 	// ORDERED channel error
 	ErrPacketSequenceOutOfOrder = sdkerrors.Register(SubModuleName, 21, "packet sequence is out of order")
+
+	// Perform a no-op on the current Msg
+	ErrNoOpMsg = sdkerrors.Register(SubModuleName, 22, "message is redundant, perform no-op")
 )
diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go
index 73a093bcb1c..a112019aab9 100644
--- a/modules/core/keeper/msg_server.go
+++ b/modules/core/keeper/msg_server.go
@@ -492,12 +492,13 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke
 	}
 
 	// Perform TAO verification
-	// If the packet was already received we want to perform a no-op. We use a cached context to prevent
-	// accidental state changes
+	//
+	// If the packet was already received we want to perform a no-op
+	// Use a cached context to prevent accidental state changes
 	cacheCtx, writeFn := ctx.CacheContext()
 	if err := k.ChannelKeeper.RecvPacket(cacheCtx, cap, msg.Packet, msg.ProofCommitment, msg.ProofHeight); err != nil {
 		// packet already received
-		if err == channeltypes.ErrPacketReceived {
+		if err == channeltypes.ErrNoOpMsg {
 			return &channeltypes.MsgRecvPacketResponse{}, nil // no-op
 		}
 		return nil, sdkerrors.Wrap(err, "receive packet verification failed")
@@ -505,6 +506,7 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke
 	writeFn()
 
 	// Perform application logic callback
+	//
 	// Cache context so that we may discard state changes from callback if the acknowledgement is unsuccessful.
 	cacheCtx, writeFn = ctx.CacheContext()
 	ack := cbs.OnRecvPacket(cacheCtx, msg.Packet, relayer)
@@ -564,13 +566,13 @@ func (k Keeper) Timeout(goCtx context.Context, msg *channeltypes.MsgTimeout) (*c
 	}
 
 	// Perform TAO verification
-	// If the timeout was already received we want to perform a no-op. We use a cached context to prevent
-	// accidental state changes
+	//
+	// If the timeout was already received, perform a no-op
+	// Use a cached context to prevent accidental state changes
 	cacheCtx, writeFn := ctx.CacheContext()
 	if err := k.ChannelKeeper.TimeoutPacket(cacheCtx, msg.Packet, msg.ProofUnreceived, msg.ProofHeight, msg.NextSequenceRecv); err != nil {
 		// timeout already received
-		// NOTE: ordered channels will error on a closed channel upon replay
-		if err == channeltypes.ErrPacketCommitmentNotFound {
+		if err == channeltypes.ErrNoOpMsg {
 			return &channeltypes.MsgTimeoutResponse{}, nil // no-op
 		}
 		return nil, sdkerrors.Wrap(err, "timeout packet verification failed")
@@ -627,12 +629,13 @@ func (k Keeper) TimeoutOnClose(goCtx context.Context, msg *channeltypes.MsgTimeo
 	}
 
 	// Perform TAO verification
-	// If the timeout was already received we want to perform a no-op. We use a cached context to prevent
-	// accidental state changes
+	//
+	// If the timeout was already received, perform a no-op
+	// Use a cached context to prevent accidental state changes
 	cacheCtx, writeFn := ctx.CacheContext()
 	if err := k.ChannelKeeper.TimeoutOnClose(cacheCtx, cap, msg.Packet, msg.ProofUnreceived, msg.ProofClose, msg.ProofHeight, msg.NextSequenceRecv); err != nil {
 		// timeout already received
-		if err == channeltypes.ErrPacketCommitmentNotFound {
+		if err == channeltypes.ErrNoOpMsg {
 			return &channeltypes.MsgTimeoutOnCloseResponse{}, nil // no-op
 		}
 		return nil, sdkerrors.Wrap(err, "timeout on close packet verification failed")
@@ -640,6 +643,7 @@ func (k Keeper) TimeoutOnClose(goCtx context.Context, msg *channeltypes.MsgTimeo
 	writeFn()
 
 	// Perform application logic callback
+	//
 	// NOTE: MsgTimeout and MsgTimeoutOnClose use the same "OnTimeoutPacket"
 	// application logic callback.
 	err = cbs.OnTimeoutPacket(ctx, msg.Packet, relayer)
@@ -691,12 +695,13 @@ func (k Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAckn
 	}
 
 	// Perform TAO verification
-	// If the acknowledgement was already received we want to perform a no-op. We use a cached context to prevent
-	// accidental state changes
+	//
+	// If the acknowledgement was already received we want to perform a no-op
+	// Use a cached context to prevent accidental state changes
 	cacheCtx, writeFn := ctx.CacheContext()
 	if err := k.ChannelKeeper.AcknowledgePacket(cacheCtx, cap, msg.Packet, msg.Acknowledgement, msg.ProofAcked, msg.ProofHeight); err != nil {
 		// acknowledgement already received
-		if err == channeltypes.ErrPacketCommitmentNotFound {
+		if err == channeltypes.ErrNoOpMsg {
 			return &channeltypes.MsgAcknowledgementResponse{}, nil // no-op
 		}
 		return nil, sdkerrors.Wrap(err, "acknowledge packet verification failed")
diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go
index 94c262e413d..8945c710edc 100644
--- a/modules/core/keeper/msg_server_test.go
+++ b/modules/core/keeper/msg_server_test.go
@@ -142,6 +142,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
 		}, false, false},
 		{"successful no-op: ORDERED - packet already received (replay)", func() {
+			// mock will panic if application callback is called twice on the same packet
 			path.SetChannelOrdered()
 			suite.coordinator.Setup(path)
 			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
@@ -151,8 +152,9 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 
 			err = path.EndpointB.RecvPacket(packet)
 			suite.Require().NoError(err)
-		}, true, true},
+		}, true, false},
 		{"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)
 			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
 
@@ -161,7 +163,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 
 			err = path.EndpointB.RecvPacket(packet)
 			suite.Require().NoError(err)
-		}, true, true},
+		}, true, false},
 	}
 
 	for _, tc := range testCases {
@@ -174,9 +176,16 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 
 			tc.malleate()
 
+			var (
+				proof       []byte
+				proofHeight clienttypes.Height
+			)
+
 			// get proof of packet commitment from chainA
 			packetKey := host.PacketCommitmentKey(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
-			proof, proofHeight := suite.chainA.QueryProof(packetKey)
+			if path.EndpointA.ChannelID != "" {
+				proof, proofHeight = path.EndpointA.QueryProof(packetKey)
+			}
 
 			msg := channeltypes.NewMsgRecvPacket(packet, proof, proofHeight, suite.chainB.SenderAccount.GetAddress().String())
 
@@ -190,7 +199,7 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {
 				suite.Require().NoError(err)
 
 				// check that callback state was handled correctly
-				_, exists := suite.chainB.GetSimApp().ScopedIBCMockKeeper.GetCapability(suite.chainB.GetContext(), ibctesting.MockCanaryCapabilityName)
+				_, exists := suite.chainB.GetSimApp().ScopedIBCMockKeeper.GetCapability(suite.chainB.GetContext(), ibctesting.GetMockRecvCanaryCapabilityName(packet))
 				if tc.expRevert {
 					suite.Require().False(exists, "capability exists in store even after callback reverted")
 				} else {
@@ -331,8 +340,14 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 
 			tc.malleate()
 
+			var (
+				proof       []byte
+				proofHeight clienttypes.Height
+			)
 			packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
-			proof, proofHeight := suite.chainB.QueryProof(packetKey)
+			if path.EndpointA.ChannelID != "" {
+				proof, proofHeight = path.EndpointB.QueryProof(packetKey)
+			}
 
 			msg := channeltypes.NewMsgAcknowledgement(packet, ibcmock.MockAcknowledgement.Acknowledgement(), proof, proofHeight, suite.chainA.SenderAccount.GetAddress().String())
 
@@ -341,14 +356,13 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 			if tc.expPass {
 				suite.Require().NoError(err)
 
-				// replay should not error as it is treated as a no-op
-				_, err := keeper.Keeper.Acknowledgement(*suite.chainA.App.GetIBCKeeper(), sdk.WrapSDKContext(suite.chainA.GetContext()), msg)
-				suite.Require().NoError(err)
-
 				// verify packet commitment was deleted on source chain
 				has := suite.chainA.App.GetIBCKeeper().ChannelKeeper.HasPacketCommitment(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 				suite.Require().False(has)
 
+				// replay should not error as it is treated as a no-op
+				_, err := keeper.Keeper.Acknowledgement(*suite.chainA.App.GetIBCKeeper(), sdk.WrapSDKContext(suite.chainA.GetContext()), msg)
+				suite.Require().NoError(err)
 			} else {
 				suite.Require().Error(err)
 			}
@@ -443,7 +457,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() {
 		}, false},
 		{"successful no-op: UNORDERED - packet not sent", func() {
 			suite.coordinator.Setup(path)
-			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
+			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},
 	}
@@ -457,7 +471,13 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() {
 
 			tc.malleate()
 
-			proof, proofHeight := suite.chainB.QueryProof(packetKey)
+			var (
+				proof       []byte
+				proofHeight clienttypes.Height
+			)
+			if path.EndpointB.ChannelID != "" {
+				proof, proofHeight = path.EndpointB.QueryProof(packetKey)
+			}
 
 			msg := channeltypes.NewMsgTimeout(packet, 1, proof, proofHeight, suite.chainA.SenderAccount.GetAddress().String())
 
@@ -466,11 +486,6 @@ func (suite *KeeperTestSuite) TestHandleTimeoutPacket() {
 			if tc.expPass {
 				suite.Require().NoError(err)
 
-				if path.EndpointA.ChannelConfig.Order == channeltypes.ORDERED {
-					// replay should return an error as replayed transaction error on the channel being closed
-					_, err := keeper.Keeper.Timeout(*suite.chainA.App.GetIBCKeeper(), sdk.WrapSDKContext(suite.chainA.GetContext()), msg)
-					suite.Require().Error(err)
-				}
 				// replay should not return an error as it is treated as a no-op
 				_, err := keeper.Keeper.Timeout(*suite.chainA.App.GetIBCKeeper(), sdk.WrapSDKContext(suite.chainA.GetContext()), msg)
 				suite.Require().NoError(err)
@@ -584,7 +599,7 @@ func (suite *KeeperTestSuite) TestHandleTimeoutOnClosePacket() {
 		}, false},
 		{"successful no-op: UNORDERED - packet not sent", func() {
 			suite.coordinator.Setup(path)
-			packet = channeltypes.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, 0)
+			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.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 
 			// close counterparty channel
diff --git a/testing/mock/mock.go b/testing/mock/mock.go
index 9edf5d3536a..0e527255432 100644
--- a/testing/mock/mock.go
+++ b/testing/mock/mock.go
@@ -3,6 +3,7 @@ package mock
 import (
 	"bytes"
 	"encoding/json"
+	"strconv"
 
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/codec"
@@ -27,12 +28,14 @@ const (
 )
 
 var (
-	MockAcknowledgement      = channeltypes.NewResultAcknowledgement([]byte("mock acknowledgement"))
-	MockFailAcknowledgement  = channeltypes.NewErrorAcknowledgement("mock failed acknowledgement")
-	MockPacketData           = []byte("mock packet data")
-	MockFailPacketData       = []byte("mock failed packet data")
-	MockAsyncPacketData      = []byte("mock async packet data")
-	MockCanaryCapabilityName = "mock canary capability name"
+	MockAcknowledgement             = channeltypes.NewResultAcknowledgement([]byte("mock acknowledgement"))
+	MockFailAcknowledgement         = channeltypes.NewErrorAcknowledgement("mock failed acknowledgement")
+	MockPacketData                  = []byte("mock packet data")
+	MockFailPacketData              = []byte("mock failed packet data")
+	MockAsyncPacketData             = []byte("mock async packet data")
+	MockRecvCanaryCapabilityName    = "mock recveive canary capability name"
+	MockAckCanaryCapabilityName     = "mock acknowledgement canary capability name"
+	MockTimeoutCanaryCapabilityName = "mock timeout canary capability name"
 )
 
 var _ porttypes.IBCModule = AppModule{}
@@ -194,7 +197,12 @@ func (am AppModule) OnChanCloseConfirm(sdk.Context, string, string) error {
 // OnRecvPacket implements the IBCModule interface.
 func (am AppModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, relayer sdk.AccAddress) exported.Acknowledgement {
 	// set state by claiming capability to check if revert happens return
-	am.scopedKeeper.NewCapability(ctx, MockCanaryCapabilityName)
+	_, err := am.scopedKeeper.NewCapability(ctx, MockRecvCanaryCapabilityName+strconv.Itoa(int(packet.GetSequence())))
+	if err != nil {
+		// application callback called twice on same packet sequence
+		// must never occur
+		panic(err)
+	}
 	if bytes.Equal(MockPacketData, packet.GetData()) {
 		return MockAcknowledgement
 	} else if bytes.Equal(MockAsyncPacketData, packet.GetData()) {
@@ -205,11 +213,25 @@ func (am AppModule) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet, re
 }
 
 // OnAcknowledgementPacket implements the IBCModule interface.
-func (am AppModule) OnAcknowledgementPacket(sdk.Context, channeltypes.Packet, []byte, sdk.AccAddress) error {
+func (am AppModule) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Packet, _ []byte, _ sdk.AccAddress) error {
+	_, err := am.scopedKeeper.NewCapability(ctx, MockAckCanaryCapabilityName+strconv.Itoa(int(packet.GetSequence())))
+	if err != nil {
+		// application callback called twice on same packet sequence
+		// must never occur
+		panic(err)
+	}
+
 	return nil
 }
 
 // OnTimeoutPacket implements the IBCModule interface.
-func (am AppModule) OnTimeoutPacket(sdk.Context, channeltypes.Packet, sdk.AccAddress) error {
+func (am AppModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, _ sdk.AccAddress) error {
+	_, err := am.scopedKeeper.NewCapability(ctx, MockTimeoutCanaryCapabilityName+strconv.Itoa(int(packet.GetSequence())))
+	if err != nil {
+		// application callback called twice on same packet sequence
+		// must never occur
+		panic(err)
+	}
+
 	return nil
 }
diff --git a/testing/values.go b/testing/values.go
index 5ba3ab1dd29..7a66b683826 100644
--- a/testing/values.go
+++ b/testing/values.go
@@ -5,12 +5,14 @@
 package ibctesting
 
 import (
+	"strconv"
 	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	ibctransfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types"
 	connectiontypes "github.com/cosmos/ibc-go/modules/core/03-connection/types"
+	channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types"
 	commitmenttypes "github.com/cosmos/ibc-go/modules/core/23-commitment/types"
 	ibctmtypes "github.com/cosmos/ibc-go/modules/light-clients/07-tendermint/types"
 	"github.com/cosmos/ibc-go/testing/mock"
@@ -50,10 +52,14 @@ var (
 
 	ConnectionVersion = connectiontypes.ExportedVersionsToProto(connectiontypes.GetCompatibleVersions())[0]
 
-	MockAcknowledgement      = mock.MockAcknowledgement.Acknowledgement()
-	MockPacketData           = mock.MockPacketData
-	MockFailPacketData       = mock.MockFailPacketData
-	MockCanaryCapabilityName = mock.MockCanaryCapabilityName
+	MockAcknowledgement          = mock.MockAcknowledgement.Acknowledgement()
+	MockPacketData               = mock.MockPacketData
+	MockFailPacketData           = mock.MockFailPacketData
+	MockRecvCanaryCapabilityName = mock.MockRecvCanaryCapabilityName
 
 	prefix = commitmenttypes.NewMerklePrefix([]byte("ibc"))
 )
+
+func GetMockRecvCanaryCapabilityName(packet channeltypes.Packet) string {
+	return MockRecvCanaryCapabilityName + strconv.Itoa(int(packet.GetSequence()))
+}

From 59ad1801f0d19c44c6cdfeb8a116ff4355a18ec8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Colin=20Axn=C3=A9r?=
 <25233464+colin-axner@users.noreply.github.com>
Date: Mon, 19 Jul 2021 17:33:00 +0200
Subject: [PATCH 3/6] self review nits

---
 modules/core/04-channel/types/errors.go | 2 +-
 modules/core/keeper/msg_server.go       | 4 ++--
 modules/core/keeper/msg_server_test.go  | 2 +-
 testing/mock/mock.go                    | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/modules/core/04-channel/types/errors.go b/modules/core/04-channel/types/errors.go
index 326e8af7f46..16b2214a1f4 100644
--- a/modules/core/04-channel/types/errors.go
+++ b/modules/core/04-channel/types/errors.go
@@ -32,5 +32,5 @@ var (
 	ErrPacketSequenceOutOfOrder = sdkerrors.Register(SubModuleName, 21, "packet sequence is out of order")
 
 	// Perform a no-op on the current Msg
-	ErrNoOpMsg = sdkerrors.Register(SubModuleName, 22, "message is redundant, perform no-op")
+	ErrNoOpMsg = sdkerrors.Register(SubModuleName, 22, "message is redundant, no-op will be performed")
 )
diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go
index a112019aab9..96a7c1f105a 100644
--- a/modules/core/keeper/msg_server.go
+++ b/modules/core/keeper/msg_server.go
@@ -493,7 +493,7 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke
 
 	// Perform TAO verification
 	//
-	// If the packet was already received we want to perform a no-op
+	// If the packet was already received, perform a no-op
 	// Use a cached context to prevent accidental state changes
 	cacheCtx, writeFn := ctx.CacheContext()
 	if err := k.ChannelKeeper.RecvPacket(cacheCtx, cap, msg.Packet, msg.ProofCommitment, msg.ProofHeight); err != nil {
@@ -696,7 +696,7 @@ func (k Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAckn
 
 	// Perform TAO verification
 	//
-	// If the acknowledgement was already received we want to perform a no-op
+	// If the acknowledgement was already received, perform a no-op
 	// Use a cached context to prevent accidental state changes
 	cacheCtx, writeFn := ctx.CacheContext()
 	if err := k.ChannelKeeper.AcknowledgePacket(cacheCtx, cap, msg.Packet, msg.Acknowledgement, msg.ProofAcked, msg.ProofHeight); err != nil {
diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go
index 8945c710edc..b28dc263689 100644
--- a/modules/core/keeper/msg_server_test.go
+++ b/modules/core/keeper/msg_server_test.go
@@ -345,7 +345,7 @@ func (suite *KeeperTestSuite) TestHandleAcknowledgePacket() {
 				proofHeight clienttypes.Height
 			)
 			packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
-			if path.EndpointA.ChannelID != "" {
+			if path.EndpointB.ChannelID != "" {
 				proof, proofHeight = path.EndpointB.QueryProof(packetKey)
 			}
 
diff --git a/testing/mock/mock.go b/testing/mock/mock.go
index 0e527255432..fe527e1c22d 100644
--- a/testing/mock/mock.go
+++ b/testing/mock/mock.go
@@ -33,7 +33,7 @@ var (
 	MockPacketData                  = []byte("mock packet data")
 	MockFailPacketData              = []byte("mock failed packet data")
 	MockAsyncPacketData             = []byte("mock async packet data")
-	MockRecvCanaryCapabilityName    = "mock recveive canary capability name"
+	MockRecvCanaryCapabilityName    = "mock receive canary capability name"
 	MockAckCanaryCapabilityName     = "mock acknowledgement canary capability name"
 	MockTimeoutCanaryCapabilityName = "mock timeout canary capability name"
 )

From 31a55f4c7eafee1da8b5e4d9d4e4632bb944b5f7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Colin=20Axn=C3=A9r?=
 <25233464+colin-axner@users.noreply.github.com>
Date: Mon, 19 Jul 2021 17:40:15 +0200
Subject: [PATCH 4/6] add changelog

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4c2de271a62..092b19cab12 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -37,6 +37,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ## [Unreleased]
 
 * (core) [\#227](https://github.com/cosmos/ibc-go/pull/227) Remove sdk.Result from application callbacks
+* (core) [\#268](https://github.com/cosmos/ibc-go/pull/268) Perform a no-op on redundant relay messages. Previous behaviour returned an error. Now no state change will occur and no error will be returned.
 
 
 ## [v1.0.0-rc0](https://github.com/cosmos/ibc-go/releases/tag/v1.0.0-rc0) - 2021-07-07

From 560ad17ca0815d23666939027887b4662e1a6f2f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Colin=20Axn=C3=A9r?=
 <25233464+colin-axner@users.noreply.github.com>
Date: Tue, 20 Jul 2021 12:33:53 +0200
Subject: [PATCH 5/6] add events for no op messages

---
 modules/core/04-channel/keeper/events.go  | 86 +++++++++++++++++++++++
 modules/core/04-channel/keeper/packet.go  | 48 ++-----------
 modules/core/04-channel/keeper/timeout.go | 20 +-----
 modules/core/keeper/msg_server.go         | 69 +++++++++++-------
 4 files changed, 138 insertions(+), 85 deletions(-)
 create mode 100644 modules/core/04-channel/keeper/events.go

diff --git a/modules/core/04-channel/keeper/events.go b/modules/core/04-channel/keeper/events.go
new file mode 100644
index 00000000000..d63f3c57eba
--- /dev/null
+++ b/modules/core/04-channel/keeper/events.go
@@ -0,0 +1,86 @@
+package keeper
+
+import (
+	"encoding/hex"
+	"fmt"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/cosmos/ibc-go/modules/core/04-channel/types"
+	"github.com/cosmos/ibc-go/modules/core/exported"
+)
+
+// EmitRecvPacketEvent emits a receive packet event. It will be emitted both the first time a packet
+// is received for a certain sequence and for all duplicate receives.
+func EmitRecvPacketEvent(ctx sdk.Context, packet exported.PacketI, channel types.Channel) {
+	ctx.EventManager().EmitEvents(sdk.Events{
+		sdk.NewEvent(
+			types.EventTypeRecvPacket,
+			sdk.NewAttribute(types.AttributeKeyData, string(packet.GetData())), // DEPRECATED
+			sdk.NewAttribute(types.AttributeKeyDataHex, hex.EncodeToString(packet.GetData())),
+			sdk.NewAttribute(types.AttributeKeyTimeoutHeight, packet.GetTimeoutHeight().String()),
+			sdk.NewAttribute(types.AttributeKeyTimeoutTimestamp, fmt.Sprintf("%d", packet.GetTimeoutTimestamp())),
+			sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
+			sdk.NewAttribute(types.AttributeKeySrcPort, packet.GetSourcePort()),
+			sdk.NewAttribute(types.AttributeKeySrcChannel, packet.GetSourceChannel()),
+			sdk.NewAttribute(types.AttributeKeyDstPort, packet.GetDestPort()),
+			sdk.NewAttribute(types.AttributeKeyDstChannel, packet.GetDestChannel()),
+			sdk.NewAttribute(types.AttributeKeyChannelOrdering, channel.Ordering.String()),
+			// we only support 1-hop packets now, and that is the most important hop for a relayer
+			// (is it going to a chain I am connected to)
+			sdk.NewAttribute(types.AttributeKeyConnection, channel.ConnectionHops[0]),
+		),
+		sdk.NewEvent(
+			sdk.EventTypeMessage,
+			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
+		),
+	})
+}
+
+// EmitAcknowledgePacketEvent emits an acknowledge packet event. It will be emitted both the first time
+// a packet is acknowledged for a certain sequence and for all duplicate acknowledgements.
+func EmitAcknowledgePacketEvent(ctx sdk.Context, packet exported.PacketI, channel types.Channel) {
+	// emit an event marking that we have processed the acknowledgement
+	ctx.EventManager().EmitEvents(sdk.Events{
+		sdk.NewEvent(
+			types.EventTypeAcknowledgePacket,
+			sdk.NewAttribute(types.AttributeKeyTimeoutHeight, packet.GetTimeoutHeight().String()),
+			sdk.NewAttribute(types.AttributeKeyTimeoutTimestamp, fmt.Sprintf("%d", packet.GetTimeoutTimestamp())),
+			sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
+			sdk.NewAttribute(types.AttributeKeySrcPort, packet.GetSourcePort()),
+			sdk.NewAttribute(types.AttributeKeySrcChannel, packet.GetSourceChannel()),
+			sdk.NewAttribute(types.AttributeKeyDstPort, packet.GetDestPort()),
+			sdk.NewAttribute(types.AttributeKeyDstChannel, packet.GetDestChannel()),
+			sdk.NewAttribute(types.AttributeKeyChannelOrdering, channel.Ordering.String()),
+			// we only support 1-hop packets now, and that is the most important hop for a relayer
+			// (is it going to a chain I am connected to)
+			sdk.NewAttribute(types.AttributeKeyConnection, channel.ConnectionHops[0]),
+		),
+		sdk.NewEvent(
+			sdk.EventTypeMessage,
+			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
+		),
+	})
+}
+
+// EmitTimeoutPacketEvent emits a timeout packet event. It will be emitted both the first time a packet
+// is timed out for a certain sequence and for all duplicate timeouts.
+func EmitTimeoutPacketEvent(ctx sdk.Context, packet exported.PacketI, channel types.Channel) {
+	ctx.EventManager().EmitEvents(sdk.Events{
+		sdk.NewEvent(
+			types.EventTypeTimeoutPacket,
+			sdk.NewAttribute(types.AttributeKeyTimeoutHeight, packet.GetTimeoutHeight().String()),
+			sdk.NewAttribute(types.AttributeKeyTimeoutTimestamp, fmt.Sprintf("%d", packet.GetTimeoutTimestamp())),
+			sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
+			sdk.NewAttribute(types.AttributeKeySrcPort, packet.GetSourcePort()),
+			sdk.NewAttribute(types.AttributeKeySrcChannel, packet.GetSourceChannel()),
+			sdk.NewAttribute(types.AttributeKeyDstPort, packet.GetDestPort()),
+			sdk.NewAttribute(types.AttributeKeyDstChannel, packet.GetDestChannel()),
+			sdk.NewAttribute(types.AttributeKeyChannelOrdering, channel.Ordering.String()),
+		),
+		sdk.NewEvent(
+			sdk.EventTypeMessage,
+			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
+		),
+	})
+}
diff --git a/modules/core/04-channel/keeper/packet.go b/modules/core/04-channel/keeper/packet.go
index 02843c6b408..1a7ea41b887 100644
--- a/modules/core/04-channel/keeper/packet.go
+++ b/modules/core/04-channel/keeper/packet.go
@@ -249,6 +249,7 @@ func (k Keeper) RecvPacket(
 		// check if the packet receipt has been received already for unordered channels
 		_, found := k.GetPacketReceipt(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 		if found {
+			EmitRecvPacketEvent(ctx, packet, channel)
 			// This error indicates that the packet has already been relayed. Core IBC will
 			// treat this error as a no-op in order to prevent an entire relay transaction
 			// from failing and consuming unnecessary fees.
@@ -272,6 +273,7 @@ func (k Keeper) RecvPacket(
 		}
 
 		if packet.GetSequence() < nextSequenceRecv {
+			EmitRecvPacketEvent(ctx, packet, channel)
 			// This error indicates that the packet has already been relayed. Core IBC will
 			// treat this error as a no-op in order to prevent an entire relay transaction
 			// from failing and consuming unnecessary fees.
@@ -299,28 +301,7 @@ func (k Keeper) RecvPacket(
 	k.Logger(ctx).Info("packet received", "packet", fmt.Sprintf("%v", packet))
 
 	// emit an event that the relayer can query for
-	ctx.EventManager().EmitEvents(sdk.Events{
-		sdk.NewEvent(
-			types.EventTypeRecvPacket,
-			sdk.NewAttribute(types.AttributeKeyData, string(packet.GetData())), // DEPRECATED
-			sdk.NewAttribute(types.AttributeKeyDataHex, hex.EncodeToString(packet.GetData())),
-			sdk.NewAttribute(types.AttributeKeyTimeoutHeight, packet.GetTimeoutHeight().String()),
-			sdk.NewAttribute(types.AttributeKeyTimeoutTimestamp, fmt.Sprintf("%d", packet.GetTimeoutTimestamp())),
-			sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
-			sdk.NewAttribute(types.AttributeKeySrcPort, packet.GetSourcePort()),
-			sdk.NewAttribute(types.AttributeKeySrcChannel, packet.GetSourceChannel()),
-			sdk.NewAttribute(types.AttributeKeyDstPort, packet.GetDestPort()),
-			sdk.NewAttribute(types.AttributeKeyDstChannel, packet.GetDestChannel()),
-			sdk.NewAttribute(types.AttributeKeyChannelOrdering, channel.Ordering.String()),
-			// we only support 1-hop packets now, and that is the most important hop for a relayer
-			// (is it going to a chain I am connected to)
-			sdk.NewAttribute(types.AttributeKeyConnection, channel.ConnectionHops[0]),
-		),
-		sdk.NewEvent(
-			sdk.EventTypeMessage,
-			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
-		),
-	})
+	EmitRecvPacketEvent(ctx, packet, channel)
 
 	return nil
 }
@@ -479,6 +460,7 @@ func (k Keeper) AcknowledgePacket(
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
 	if len(commitment) == 0 {
+		EmitAcknowledgePacketEvent(ctx, packet, channel)
 		// This error indicates that the acknowledgement has already been relayed
 		// or there is a misconfigured relayer attempting to prove an acknowledgement
 		// for a packet never sent. Core IBC will treat this error as a no-op in order to
@@ -532,27 +514,7 @@ func (k Keeper) AcknowledgePacket(
 	// log that a packet has been acknowledged
 	k.Logger(ctx).Info("packet acknowledged", "packet", fmt.Sprintf("%v", packet))
 
-	// emit an event marking that we have processed the acknowledgement
-	ctx.EventManager().EmitEvents(sdk.Events{
-		sdk.NewEvent(
-			types.EventTypeAcknowledgePacket,
-			sdk.NewAttribute(types.AttributeKeyTimeoutHeight, packet.GetTimeoutHeight().String()),
-			sdk.NewAttribute(types.AttributeKeyTimeoutTimestamp, fmt.Sprintf("%d", packet.GetTimeoutTimestamp())),
-			sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
-			sdk.NewAttribute(types.AttributeKeySrcPort, packet.GetSourcePort()),
-			sdk.NewAttribute(types.AttributeKeySrcChannel, packet.GetSourceChannel()),
-			sdk.NewAttribute(types.AttributeKeyDstPort, packet.GetDestPort()),
-			sdk.NewAttribute(types.AttributeKeyDstChannel, packet.GetDestChannel()),
-			sdk.NewAttribute(types.AttributeKeyChannelOrdering, channel.Ordering.String()),
-			// we only support 1-hop packets now, and that is the most important hop for a relayer
-			// (is it going to a chain I am connected to)
-			sdk.NewAttribute(types.AttributeKeyConnection, channel.ConnectionHops[0]),
-		),
-		sdk.NewEvent(
-			sdk.EventTypeMessage,
-			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
-		),
-	})
+	EmitAcknowledgePacketEvent(ctx, packet, channel)
 
 	return nil
 }
diff --git a/modules/core/04-channel/keeper/timeout.go b/modules/core/04-channel/keeper/timeout.go
index 181441d45a9..93fa9eca4fe 100644
--- a/modules/core/04-channel/keeper/timeout.go
+++ b/modules/core/04-channel/keeper/timeout.go
@@ -74,6 +74,7 @@ func (k Keeper) TimeoutPacket(
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
 	if len(commitment) == 0 {
+		EmitTimeoutPacketEvent(ctx, packet, channel)
 		// This error indicates that the timeout has already been relayed
 		// or there is a misconfigured relayer attempting to prove a timeout
 		// for a packet never sent. Core IBC will treat this error as a no-op in order to
@@ -159,23 +160,7 @@ func (k Keeper) TimeoutExecuted(
 	k.Logger(ctx).Info("packet timed-out", "packet", fmt.Sprintf("%v", packet))
 
 	// emit an event marking that we have processed the timeout
-	ctx.EventManager().EmitEvents(sdk.Events{
-		sdk.NewEvent(
-			types.EventTypeTimeoutPacket,
-			sdk.NewAttribute(types.AttributeKeyTimeoutHeight, packet.GetTimeoutHeight().String()),
-			sdk.NewAttribute(types.AttributeKeyTimeoutTimestamp, fmt.Sprintf("%d", packet.GetTimeoutTimestamp())),
-			sdk.NewAttribute(types.AttributeKeySequence, fmt.Sprintf("%d", packet.GetSequence())),
-			sdk.NewAttribute(types.AttributeKeySrcPort, packet.GetSourcePort()),
-			sdk.NewAttribute(types.AttributeKeySrcChannel, packet.GetSourceChannel()),
-			sdk.NewAttribute(types.AttributeKeyDstPort, packet.GetDestPort()),
-			sdk.NewAttribute(types.AttributeKeyDstChannel, packet.GetDestChannel()),
-			sdk.NewAttribute(types.AttributeKeyChannelOrdering, channel.Ordering.String()),
-		),
-		sdk.NewEvent(
-			sdk.EventTypeMessage,
-			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
-		),
-	})
+	EmitTimeoutPacketEvent(ctx, packet, channel)
 
 	return nil
 }
@@ -227,6 +212,7 @@ func (k Keeper) TimeoutOnClose(
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
 	if len(commitment) == 0 {
+		EmitTimeoutPacketEvent(ctx, packet, channel)
 		// This error indicates that the timeout has already been relayed
 		// or there is a misconfigured relayer attempting to prove a timeout
 		// for a packet never sent. Core IBC will treat this error as a no-op in order to
diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go
index 96a7c1f105a..69bde98b687 100644
--- a/modules/core/keeper/msg_server.go
+++ b/modules/core/keeper/msg_server.go
@@ -496,21 +496,25 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke
 	// If the packet was already received, perform a no-op
 	// Use a cached context to prevent accidental state changes
 	cacheCtx, writeFn := ctx.CacheContext()
-	if err := k.ChannelKeeper.RecvPacket(cacheCtx, cap, msg.Packet, msg.ProofCommitment, msg.ProofHeight); err != nil {
-		// packet already received
-		if err == channeltypes.ErrNoOpMsg {
-			return &channeltypes.MsgRecvPacketResponse{}, nil // no-op
-		}
+	err = k.ChannelKeeper.RecvPacket(cacheCtx, cap, msg.Packet, msg.ProofCommitment, msg.ProofHeight)
+
+	// NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context.
+	ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events())
+
+	switch err {
+	case nil:
+		writeFn()
+	case channeltypes.ErrNoOpMsg:
+		return &channeltypes.MsgRecvPacketResponse{}, nil // no-op
+	default:
 		return nil, sdkerrors.Wrap(err, "receive packet verification failed")
 	}
-	writeFn()
 
 	// Perform application logic callback
 	//
 	// Cache context so that we may discard state changes from callback if the acknowledgement is unsuccessful.
 	cacheCtx, writeFn = ctx.CacheContext()
 	ack := cbs.OnRecvPacket(cacheCtx, msg.Packet, relayer)
-	// This doesn't cause duplicate events to be emitted.
 	// NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context.
 	// Events from callback are emitted regardless of acknowledgement success
 	ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events())
@@ -570,14 +574,19 @@ func (k Keeper) Timeout(goCtx context.Context, msg *channeltypes.MsgTimeout) (*c
 	// If the timeout was already received, perform a no-op
 	// Use a cached context to prevent accidental state changes
 	cacheCtx, writeFn := ctx.CacheContext()
-	if err := k.ChannelKeeper.TimeoutPacket(cacheCtx, msg.Packet, msg.ProofUnreceived, msg.ProofHeight, msg.NextSequenceRecv); err != nil {
-		// timeout already received
-		if err == channeltypes.ErrNoOpMsg {
-			return &channeltypes.MsgTimeoutResponse{}, nil // no-op
-		}
+	err = k.ChannelKeeper.TimeoutPacket(cacheCtx, msg.Packet, msg.ProofUnreceived, msg.ProofHeight, msg.NextSequenceRecv)
+
+	// NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context.
+	ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events())
+
+	switch err {
+	case nil:
+		writeFn()
+	case channeltypes.ErrNoOpMsg:
+		return &channeltypes.MsgTimeoutResponse{}, nil // no-op
+	default:
 		return nil, sdkerrors.Wrap(err, "timeout packet verification failed")
 	}
-	writeFn()
 
 	// Perform application logic callback
 	err = cbs.OnTimeoutPacket(ctx, msg.Packet, relayer)
@@ -633,14 +642,19 @@ func (k Keeper) TimeoutOnClose(goCtx context.Context, msg *channeltypes.MsgTimeo
 	// If the timeout was already received, perform a no-op
 	// Use a cached context to prevent accidental state changes
 	cacheCtx, writeFn := ctx.CacheContext()
-	if err := k.ChannelKeeper.TimeoutOnClose(cacheCtx, cap, msg.Packet, msg.ProofUnreceived, msg.ProofClose, msg.ProofHeight, msg.NextSequenceRecv); err != nil {
-		// timeout already received
-		if err == channeltypes.ErrNoOpMsg {
-			return &channeltypes.MsgTimeoutOnCloseResponse{}, nil // no-op
-		}
+	err = k.ChannelKeeper.TimeoutOnClose(cacheCtx, cap, msg.Packet, msg.ProofUnreceived, msg.ProofClose, msg.ProofHeight, msg.NextSequenceRecv)
+
+	// NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context.
+	ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events())
+
+	switch err {
+	case nil:
+		writeFn()
+	case channeltypes.ErrNoOpMsg:
+		return &channeltypes.MsgTimeoutOnCloseResponse{}, nil // no-op
+	default:
 		return nil, sdkerrors.Wrap(err, "timeout on close packet verification failed")
 	}
-	writeFn()
 
 	// Perform application logic callback
 	//
@@ -699,14 +713,19 @@ func (k Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAckn
 	// If the acknowledgement was already received, perform a no-op
 	// Use a cached context to prevent accidental state changes
 	cacheCtx, writeFn := ctx.CacheContext()
-	if err := k.ChannelKeeper.AcknowledgePacket(cacheCtx, cap, msg.Packet, msg.Acknowledgement, msg.ProofAcked, msg.ProofHeight); err != nil {
-		// acknowledgement already received
-		if err == channeltypes.ErrNoOpMsg {
-			return &channeltypes.MsgAcknowledgementResponse{}, nil // no-op
-		}
+	err = k.ChannelKeeper.AcknowledgePacket(cacheCtx, cap, msg.Packet, msg.Acknowledgement, msg.ProofAcked, msg.ProofHeight)
+
+	// NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context.
+	ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events())
+
+	switch err {
+	case nil:
+		writeFn()
+	case channeltypes.ErrNoOpMsg:
+		return &channeltypes.MsgAcknowledgementResponse{}, nil // no-op
+	default:
 		return nil, sdkerrors.Wrap(err, "acknowledge packet verification failed")
 	}
-	writeFn()
 
 	// Perform application logic callback
 	err = cbs.OnAcknowledgementPacket(ctx, msg.Packet, msg.Acknowledgement, relayer)

From c23ae390f3df1f7baf6a311bf3f3642626f69c24 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Colin=20Axn=C3=A9r?=
 <25233464+colin-axner@users.noreply.github.com>
Date: Tue, 20 Jul 2021 12:36:05 +0200
Subject: [PATCH 6/6] add back comment

---
 modules/core/04-channel/keeper/events.go | 1 -
 modules/core/04-channel/keeper/packet.go | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/core/04-channel/keeper/events.go b/modules/core/04-channel/keeper/events.go
index d63f3c57eba..13c92246f02 100644
--- a/modules/core/04-channel/keeper/events.go
+++ b/modules/core/04-channel/keeper/events.go
@@ -40,7 +40,6 @@ func EmitRecvPacketEvent(ctx sdk.Context, packet exported.PacketI, channel types
 // EmitAcknowledgePacketEvent emits an acknowledge packet event. It will be emitted both the first time
 // a packet is acknowledged for a certain sequence and for all duplicate acknowledgements.
 func EmitAcknowledgePacketEvent(ctx sdk.Context, packet exported.PacketI, channel types.Channel) {
-	// emit an event marking that we have processed the acknowledgement
 	ctx.EventManager().EmitEvents(sdk.Events{
 		sdk.NewEvent(
 			types.EventTypeAcknowledgePacket,
diff --git a/modules/core/04-channel/keeper/packet.go b/modules/core/04-channel/keeper/packet.go
index 1a7ea41b887..5cfc7e3c14f 100644
--- a/modules/core/04-channel/keeper/packet.go
+++ b/modules/core/04-channel/keeper/packet.go
@@ -514,6 +514,7 @@ func (k Keeper) AcknowledgePacket(
 	// log that a packet has been acknowledged
 	k.Logger(ctx).Info("packet acknowledged", "packet", fmt.Sprintf("%v", packet))
 
+	// emit an event marking that we have processed the acknowledgement
 	EmitAcknowledgePacketEvent(ctx, packet, channel)
 
 	return nil