From 187dd22df5dff9ab9c652d7a83a08c8bdd506579 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Colin=20Axn=C3=A9r?=
 <25233464+colin-axner@users.noreply.github.com>
Date: Fri, 21 May 2021 14:01:58 +0200
Subject: [PATCH 1/5] improve error messages

---
 modules/core/04-channel/keeper/packet.go      |  20 ++-
 modules/core/04-channel/keeper/packet_test.go | 118 ++++++++++++++++--
 modules/core/04-channel/keeper/timeout.go     |   4 +
 .../core/04-channel/keeper/timeout_test.go    |  25 ++++
 modules/core/04-channel/types/errors.go       |  13 +-
 modules/core/23-commitment/types/merkle.go    |   4 +-
 .../07-tendermint/types/client_state.go       |   2 +-
 testing/endpoint.go                           |  28 ++++-
 8 files changed, 190 insertions(+), 24 deletions(-)

diff --git a/modules/core/04-channel/keeper/packet.go b/modules/core/04-channel/keeper/packet.go
index 72a1ff5b50c..46d7434c959 100644
--- a/modules/core/04-channel/keeper/packet.go
+++ b/modules/core/04-channel/keeper/packet.go
@@ -241,8 +241,8 @@ func (k Keeper) RecvPacket(
 		_, found := k.GetPacketReceipt(ctx, packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 		if found {
 			return sdkerrors.Wrapf(
-				types.ErrInvalidPacket,
-				"packet sequence (%d) already has been received", packet.GetSequence(),
+				types.ErrPacketReceived,
+				"packet sequence (%d)", packet.GetSequence(),
 			)
 		}
 
@@ -262,9 +262,17 @@ 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,
+			)
+		}
+
 		if packet.GetSequence() != nextSequenceRecv {
 			return sdkerrors.Wrapf(
-				types.ErrInvalidPacket,
+				types.ErrPacketReceiptOutOfOrder,
 				"packet sequence ≠ next receive sequence (%d ≠ %d)", packet.GetSequence(), nextSequenceRecv,
 			)
 		}
@@ -461,6 +469,10 @@ func (k Keeper) AcknowledgePacket(
 
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
+	if len(commitment) == 0 {
+		return sdkerrors.Wrapf(types.ErrPacketAcknowledged, "packet commitment does not exist")
+	}
+
 	packetCommitment := types.CommitPacket(k.cdc, packet)
 
 	// verify we sent the packet and haven't cleared it out yet
@@ -472,7 +484,7 @@ func (k Keeper) AcknowledgePacket(
 		ctx, connectionEnd, proofHeight, proof, packet.GetDestPort(), packet.GetDestChannel(),
 		packet.GetSequence(), acknowledgement,
 	); err != nil {
-		return sdkerrors.Wrap(err, "packet acknowledgement verification failed")
+		return err
 	}
 
 	// assert packets acknowledged in order
diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go
index 4916ae9d995..af05b176de3 100644
--- a/modules/core/04-channel/keeper/packet_test.go
+++ b/modules/core/04-channel/keeper/packet_test.go
@@ -235,6 +235,29 @@ 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() {
+			path.SetChannelOrdered()
+			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)
+			err := path.EndpointA.SendPacket(packet)
+			suite.Require().NoError(err)
+			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
+
+			err = path.EndpointB.RecvPacket(packet.(types.Packet))
+			suite.Require().NoError(err)
+		}, false},
+		{"packet already relayed UNORDERED channel", func() {
+			// setup uses an UNORDERED channel
+			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)
+			err := path.EndpointA.SendPacket(packet)
+			suite.Require().NoError(err)
+			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
+
+			err = path.EndpointB.RecvPacket(packet.(types.Packet))
+			suite.Require().NoError(err)
+		}, false},
 		{"out of order packet failure with ORDERED channel", func() {
 			path.SetChannelOrdered()
 			suite.coordinator.Setup(path)
@@ -285,6 +308,8 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"connection not found", func() {
+			suite.coordinator.Setup(path)
+
 			// pass channel check
 			suite.chainB.App.GetIBCKeeper().ChannelKeeper.SetChannel(
 				suite.chainB.GetContext(),
@@ -323,9 +348,11 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"next receive sequence is not found", func() {
-			path := ibctesting.NewPath(suite.chainA, suite.chainB)
 			suite.coordinator.SetupConnections(path)
 
+			path.EndpointA.ChannelID = ibctesting.FirstChannelID
+			path.EndpointB.ChannelID = ibctesting.FirstChannelID
+
 			// manually creating channel prevents next recv sequence from being set
 			suite.chainB.App.GetIBCKeeper().ChannelKeeper.SetChannel(
 				suite.chainB.GetContext(),
@@ -336,10 +363,13 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
 
 			// manually set packet commitment
-			suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, packet.GetSequence(), ibctesting.MockPacketData)
+			suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, packet.GetSequence(), types.CommitPacket(suite.chainA.App.AppCodec(), packet))
 			suite.chainB.CreateChannelCapability(suite.chainB.GetSimApp().ScopedIBCMockKeeper, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
+
+			path.EndpointA.UpdateClient()
+			path.EndpointB.UpdateClient()
 		}, false},
 		{"receipt already stored", func() {
 			suite.coordinator.Setup(path)
@@ -366,7 +396,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 
 			// get proof of packet commitment from chainA
 			packetKey := host.PacketCommitmentKey(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
-			proof, proofHeight := suite.chainA.QueryProof(packetKey)
+			proof, proofHeight := path.EndpointA.QueryProof(packetKey)
 
 			err := suite.chainB.App.GetIBCKeeper().ChannelKeeper.RecvPacket(suite.chainB.GetContext(), channelCap, packet, proof, proofHeight)
 
@@ -520,6 +550,41 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 		}, true},
+		{"packet already acknowledged ordered channel", func() {
+			path.SetChannelOrdered()
+			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)
+			// create packet commitment
+			err := path.EndpointA.SendPacket(packet)
+			suite.Require().NoError(err)
+
+			// create packet receipt and acknowledgement
+			err = path.EndpointB.RecvPacket(packet)
+			suite.Require().NoError(err)
+
+			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+
+			err = path.EndpointA.AcknowledgePacket(packet, ack.Acknowledgement())
+			suite.Require().NoError(err)
+		}, false},
+		{"packet already acknowledged unordered channel", func() {
+			// setup uses an UNORDERED channel
+			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)
+
+			// create packet commitment
+			err := path.EndpointA.SendPacket(packet)
+			suite.Require().NoError(err)
+
+			// create packet receipt and acknowledgement
+			err = path.EndpointB.RecvPacket(packet)
+			suite.Require().NoError(err)
+
+			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+
+			err = path.EndpointA.AcknowledgePacket(packet, ack.Acknowledgement())
+			suite.Require().NoError(err)
+		}, false},
 		{"channel not found", func() {
 			// use wrong channel naming
 			suite.coordinator.Setup(path)
@@ -560,11 +625,13 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 		}, false},
 		{"connection not found", func() {
+			suite.coordinator.Setup(path)
+
 			// pass channel check
-			suite.chainB.App.GetIBCKeeper().ChannelKeeper.SetChannel(
-				suite.chainB.GetContext(),
-				path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID,
-				types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID), []string{connIDB}, path.EndpointB.ChannelConfig.Version),
+			suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetChannel(
+				suite.chainA.GetContext(),
+				path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID,
+				types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID), []string{"connection-1000"}, path.EndpointA.ChannelConfig.Version),
 			)
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
 			suite.chainA.CreateChannelCapability(suite.chainA.GetSimApp().ScopedIBCMockKeeper, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
@@ -601,23 +668,50 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			path.EndpointA.SendPacket(packet)
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 		}, false},
+		{"packet commitment bytes do not match", func() {
+			// setup uses an UNORDERED channel
+			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)
+
+			// create packet commitment
+			err := path.EndpointA.SendPacket(packet)
+			suite.Require().NoError(err)
+
+			// create packet receipt and acknowledgement
+			err = path.EndpointB.RecvPacket(packet)
+			suite.Require().NoError(err)
+
+			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+
+			packet.Data = []byte("invalid packet commitment")
+		}, false},
 		{"next ack sequence not found", func() {
-			path := ibctesting.NewPath(suite.chainA, suite.chainB)
 			suite.coordinator.SetupConnections(path)
-			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
+
+			path.EndpointA.ChannelID = ibctesting.FirstChannelID
+			path.EndpointB.ChannelID = ibctesting.FirstChannelID
+
 			// manually creating channel prevents next sequence acknowledgement from being set
 			suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetChannel(
 				suite.chainA.GetContext(),
 				path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID,
 				types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID), []string{path.EndpointA.ConnectionID}, path.EndpointA.ChannelConfig.Version),
 			)
+
+			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
 			// manually set packet commitment
-			suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, packet.GetSequence(), ibctesting.MockPacketData)
+			suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, packet.GetSequence(), types.CommitPacket(suite.chainA.App.AppCodec(), packet))
 
 			// manually set packet acknowledgement and capability
-			suite.chainB.App.GetIBCKeeper().ChannelKeeper.SetPacketAcknowledgement(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, packet.GetSequence(), ibctesting.MockAcknowledgement)
+			suite.chainB.App.GetIBCKeeper().ChannelKeeper.SetPacketAcknowledgement(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, packet.GetSequence(), types.CommitAcknowledgement(ack.Acknowledgement()))
+
 			suite.chainA.CreateChannelCapability(suite.chainA.GetSimApp().ScopedIBCMockKeeper, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+
+			suite.coordinator.CommitBlock(path.EndpointA.Chain, path.EndpointB.Chain)
+
+			path.EndpointA.UpdateClient()
+			path.EndpointB.UpdateClient()
 		}, false},
 		{"next ack sequence mismatch ORDERED", func() {
 			path.SetChannelOrdered()
@@ -646,7 +740,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			tc.malleate()
 
 			packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
-			proof, proofHeight := suite.chainB.QueryProof(packetKey)
+			proof, proofHeight := path.EndpointB.QueryProof(packetKey)
 
 			err := suite.chainA.App.GetIBCKeeper().ChannelKeeper.AcknowledgePacket(suite.chainA.GetContext(), channelCap, packet, ack.Acknowledgement(), proof, proofHeight)
 			pc := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetPacketCommitment(suite.chainA.GetContext(), packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
diff --git a/modules/core/04-channel/keeper/timeout.go b/modules/core/04-channel/keeper/timeout.go
index 1f6357d34d3..6342145870b 100644
--- a/modules/core/04-channel/keeper/timeout.go
+++ b/modules/core/04-channel/keeper/timeout.go
@@ -80,6 +80,10 @@ func (k Keeper) TimeoutPacket(
 
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
+	if len(commitment) == 0 {
+		return sdkerrors.Wrapf(types.ErrPacketTimedOut, "packet sequence (%d)", packet.GetSequence())
+	}
+
 	packetCommitment := types.CommitPacket(k.cdc, packet)
 
 	// verify we sent the packet and haven't cleared it out yet
diff --git a/modules/core/04-channel/keeper/timeout_test.go b/modules/core/04-channel/keeper/timeout_test.go
index ab6c4e49d2c..bdbb3349713 100644
--- a/modules/core/04-channel/keeper/timeout_test.go
+++ b/modules/core/04-channel/keeper/timeout_test.go
@@ -42,6 +42,31 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			// need to update chainA's client representing chainB to prove missing ack
 			path.EndpointA.UpdateClient()
 		}, true},
+		{"packet already timed out: ORDERED", func() {
+			ordered = true
+			path.SetChannelOrdered()
+
+			suite.coordinator.Setup(path)
+			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), uint64(suite.chainB.GetContext().BlockTime().UnixNano()))
+			path.EndpointA.SendPacket(packet)
+			// need to update chainA's client representing chainB to prove missing ack
+			path.EndpointA.UpdateClient()
+
+			err := path.EndpointA.TimeoutPacket(packet)
+			suite.Require().NoError(err)
+		}, false},
+		{"packet already timed out: UNORDERED", func() {
+			ordered = false
+
+			suite.coordinator.Setup(path)
+			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp)
+			path.EndpointA.SendPacket(packet)
+			// need to update chainA's client representing chainB to prove missing ack
+			path.EndpointA.UpdateClient()
+
+			err := path.EndpointA.TimeoutPacket(packet)
+			suite.Require().NoError(err)
+		}, false},
 		{"channel not found", func() {
 			// use wrong channel naming
 			suite.coordinator.Setup(path)
diff --git a/modules/core/04-channel/types/errors.go b/modules/core/04-channel/types/errors.go
index 82cf773057d..547524717a6 100644
--- a/modules/core/04-channel/types/errors.go
+++ b/modules/core/04-channel/types/errors.go
@@ -22,7 +22,14 @@ var (
 	ErrTooManyConnectionHops     = sdkerrors.Register(SubModuleName, 15, "too many connection hops")
 	ErrInvalidAcknowledgement    = sdkerrors.Register(SubModuleName, 16, "invalid acknowledgement")
 	ErrPacketCommitmentNotFound  = sdkerrors.Register(SubModuleName, 17, "packet commitment not found")
-	ErrPacketReceived            = sdkerrors.Register(SubModuleName, 18, "packet already received")
-	ErrAcknowledgementExists     = sdkerrors.Register(SubModuleName, 19, "acknowledgement for packet already exists")
-	ErrInvalidChannelIdentifier  = sdkerrors.Register(SubModuleName, 20, "invalid channel identifier")
+	ErrAcknowledgementExists     = sdkerrors.Register(SubModuleName, 18, "acknowledgement for packet already exists")
+	ErrInvalidChannelIdentifier  = sdkerrors.Register(SubModuleName, 19, "invalid channel identifier")
+
+	// packets already relayed errors
+	ErrPacketReceived     = sdkerrors.Register(SubModuleName, 20, "packet already received")
+	ErrPacketAcknowledged = sdkerrors.Register(SubModuleName, 21, "packet acknowledgement already processed")
+	ErrPacketTimedOut     = sdkerrors.Register(SubModuleName, 22, "packet already timed out")
+
+	// ORDERED channel error
+	ErrPacketReceiptOutOfOrder = sdkerrors.Register(SubModuleName, 23, "packet cannot be received out of order")
 )
diff --git a/modules/core/23-commitment/types/merkle.go b/modules/core/23-commitment/types/merkle.go
index 597a1ac9983..72b3de47b4f 100644
--- a/modules/core/23-commitment/types/merkle.go
+++ b/modules/core/23-commitment/types/merkle.go
@@ -253,8 +253,8 @@ func verifyChainedMembershipProof(root []byte, specs []*ics23.ProofSpec, proofs
 			value = subroot
 		case *ics23.CommitmentProof_Nonexist:
 			return sdkerrors.Wrapf(ErrInvalidProof,
-				"chained membership proof contains nonexistence proof at index %d. If this is unexpected, please ensure that proof was queried from the height that contained the value in store and was queried with the correct key.",
-				i)
+				"chained membership proof contains nonexistence proof at index %d. If this is unexpected, please ensure that proof was queried from a height that contained the value in store and was queried with the correct key. The key used: %s",
+				i, keys)
 		default:
 			return sdkerrors.Wrapf(ErrInvalidProof,
 				"expected proof type: %T, got: %T", &ics23.CommitmentProof_Exist{}, proofs[i].Proof)
diff --git a/modules/light-clients/07-tendermint/types/client_state.go b/modules/light-clients/07-tendermint/types/client_state.go
index 4928a6819aa..5e302436897 100644
--- a/modules/light-clients/07-tendermint/types/client_state.go
+++ b/modules/light-clients/07-tendermint/types/client_state.go
@@ -522,7 +522,7 @@ func produceVerificationArgs(
 	if cs.GetLatestHeight().LT(height) {
 		return commitmenttypes.MerkleProof{}, nil, sdkerrors.Wrapf(
 			sdkerrors.ErrInvalidHeight,
-			"client state height < proof height (%d < %d)", cs.GetLatestHeight(), height,
+			"client state height < proof height (%d < %d), please ensure the client has been updated", cs.GetLatestHeight(), height,
 		)
 	}
 
diff --git a/testing/endpoint.go b/testing/endpoint.go
index e32d0cdfbf7..bc03c5482e6 100644
--- a/testing/endpoint.go
+++ b/testing/endpoint.go
@@ -403,8 +403,6 @@ func (endpoint *Endpoint) WriteAcknowledgement(ack exported.Acknowledgement, pac
 }
 
 // AcknowledgePacket sends a MsgAcknowledgement to the channel associated with the endpoint.
-// TODO: add a query for the acknowledgement by events
-// - https://github.com/cosmos/cosmos-sdk/issues/6509
 func (endpoint *Endpoint) AcknowledgePacket(packet channeltypes.Packet, ack []byte) error {
 	// get proof of acknowledgement on counterparty
 	packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
@@ -415,6 +413,32 @@ func (endpoint *Endpoint) AcknowledgePacket(packet channeltypes.Packet, ack []by
 	return endpoint.Chain.sendMsgs(ackMsg)
 }
 
+// TimeoutPacket sends a MsgTimeout to the channel associated with the endpoint.
+func (endpoint *Endpoint) TimeoutPacket(packet channeltypes.Packet) error {
+	// get proof for timeout based on channel order
+	var packetKey []byte
+
+	switch endpoint.ChannelConfig.Order {
+	case channeltypes.ORDERED:
+		packetKey = host.NextSequenceRecvKey(packet.GetDestPort(), packet.GetDestChannel())
+	case channeltypes.UNORDERED:
+		packetKey = host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
+	default:
+		return fmt.Errorf("unsupported order type %s", endpoint.ChannelConfig.Order)
+	}
+
+	proof, proofHeight := endpoint.Counterparty.QueryProof(packetKey)
+	nextSeqRecv, found := endpoint.Counterparty.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextSequenceRecv(endpoint.Counterparty.Chain.GetContext(), endpoint.ChannelConfig.PortID, endpoint.ChannelID)
+	require.True(endpoint.Chain.t, found)
+
+	timeoutMsg := channeltypes.NewMsgTimeout(
+		packet, nextSeqRecv,
+		proof, proofHeight, endpoint.Chain.SenderAccount.GetAddress().String(),
+	)
+
+	return endpoint.Chain.sendMsgs(timeoutMsg)
+}
+
 // SetChannelClosed sets a channel state to CLOSED.
 func (endpoint *Endpoint) SetChannelClosed() error {
 	channel := endpoint.GetChannel()

From 79c9c5b2e0e882db73f6c7cc9c26c7919b691403 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Colin=20Axn=C3=A9r?=
 <25233464+colin-axner@users.noreply.github.com>
Date: Fri, 21 May 2021 14:11:27 +0200
Subject: [PATCH 2/5] changelog

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

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8119e1019a7..cc2dbd2adb8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -65,6 +65,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (modules/core) [\#184](https://github.com/cosmos/ibc-go/pull/184) Improve error messages. Uses unique error codes to indicate already relayed packets.
 * (modules/core/04-channel) [\#7949](https://github.com/cosmos/cosmos-sdk/issues/7949) Standardized channel `Acknowledgement` moved to its own file. Codec registration redundancy removed.
 * (modules/core/04-channel) [\#144](https://github.com/cosmos/ibc-go/pull/144) Introduced a `packet_data_hex` attribute to emit the hex-encoded packet data in events. This allows for raw binary (proto-encoded message) to be sent over events and decoded correctly on relayer. Original `packet_data` is DEPRECATED. All relayers and IBC event consumers are encouraged to switch to `packet_data_hex` as soon as possible.
 * (modules/light-clients/07-tendermint) [\#125](https://github.com/cosmos/ibc-go/pull/125) Implement efficient iteration of consensus states and pruning of earliest expired consensus state on UpdateClient.

From b9757eb664acbe6bf8d0629f798e7478ccee0f32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Colin=20Axn=C3=A9r?=
 <25233464+colin-axner@users.noreply.github.com>
Date: Wed, 26 May 2021 13:36:38 +0200
Subject: [PATCH 3/5] update recvpacket errors and test

---
 modules/core/04-channel/keeper/packet.go      |  2 +-
 modules/core/04-channel/keeper/packet_test.go | 39 +++++++++++++++++++
 modules/core/04-channel/keeper/timeout.go     |  2 +-
 modules/core/04-channel/types/errors.go       | 10 ++---
 4 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/modules/core/04-channel/keeper/packet.go b/modules/core/04-channel/keeper/packet.go
index 46d7434c959..2156e9a5118 100644
--- a/modules/core/04-channel/keeper/packet.go
+++ b/modules/core/04-channel/keeper/packet.go
@@ -470,7 +470,7 @@ func (k Keeper) AcknowledgePacket(
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
 	if len(commitment) == 0 {
-		return sdkerrors.Wrapf(types.ErrPacketAcknowledged, "packet commitment does not exist")
+		return sdkerrors.Wrapf(types.ErrPacketCommitmentNotFound, "packet with sequence (%d) has been acknowledged, or timed out. In rare cases the packet was never sent or the packet sequence is incorrect", packet.GetSequence())
 	}
 
 	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 af05b176de3..5eb9f708ec0 100644
--- a/modules/core/04-channel/keeper/packet_test.go
+++ b/modules/core/04-channel/keeper/packet_test.go
@@ -1,10 +1,14 @@
 package keeper_test
 
 import (
+	"errors"
 	"fmt"
 
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
+
 	clienttypes "github.com/cosmos/ibc-go/modules/core/02-client/types"
+	connectiontypes "github.com/cosmos/ibc-go/modules/core/03-connection/types"
 	"github.com/cosmos/ibc-go/modules/core/04-channel/types"
 	host "github.com/cosmos/ibc-go/modules/core/24-host"
 	"github.com/cosmos/ibc-go/modules/core/exported"
@@ -200,6 +204,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 		path       *ibctesting.Path
 		packet     exported.PacketI
 		channelCap *capabilitytypes.Capability
+		expError   *sdkerrors.Error
 	)
 
 	testCases := []testCase{
@@ -236,6 +241,8 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, true},
 		{"packet already relayed ORDERED channel", func() {
+			expError = types.ErrPacketReceived
+
 			path.SetChannelOrdered()
 			suite.coordinator.Setup(path)
 
@@ -248,6 +255,8 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			suite.Require().NoError(err)
 		}, false},
 		{"packet already relayed UNORDERED channel", func() {
+			expError = types.ErrPacketReceived
+
 			// setup uses an UNORDERED channel
 			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)
@@ -259,6 +268,8 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			suite.Require().NoError(err)
 		}, false},
 		{"out of order packet failure with ORDERED channel", func() {
+			expError = types.ErrPacketReceiptOutOfOrder
+
 			path.SetChannelOrdered()
 			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)
@@ -274,12 +285,16 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"channel not found", func() {
+			expError = types.ErrChannelNotFound
+
 			// use wrong channel naming
 			suite.coordinator.Setup(path)
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, ibctesting.InvalidID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp)
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"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)
 
@@ -288,26 +303,34 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"capability cannot authenticate ORDERED", func() {
+			expError = types.ErrInvalidChannelCapability
+
 			path.SetChannelOrdered()
 			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)
 			err := path.EndpointA.SendPacket(packet)
 			suite.Require().NoError(err)
 			channelCap = capabilitytypes.NewCapability(3)
 		}, false},
 		{"packet source port ≠ channel counterparty port", func() {
+			expError = types.ErrInvalidPacket
 			suite.coordinator.Setup(path)
+
 			// use wrong port for dest
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, ibctesting.InvalidID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"packet source channel ID ≠ channel counterparty channel ID", func() {
+			expError = types.ErrInvalidPacket
 			suite.coordinator.Setup(path)
+
 			// use wrong port for dest
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, ibctesting.InvalidID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"connection not found", func() {
+			expError = connectiontypes.ErrConnectionNotFound
 			suite.coordinator.Setup(path)
 
 			// pass channel check
@@ -321,6 +344,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"connection not OPEN", func() {
+			expError = connectiontypes.ErrInvalidConnectionState
 			suite.coordinator.SetupClients(path)
 
 			// connection on chainB is in INIT
@@ -338,16 +362,21 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"timeout height passed", func() {
+			expError = types.ErrPacketTimeout
 			suite.coordinator.Setup(path)
+
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp)
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"timeout timestamp passed", func() {
+			expError = types.ErrPacketTimeout
 			suite.coordinator.Setup(path)
+
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, disabledTimeoutHeight, uint64(suite.chainB.GetContext().BlockTime().UnixNano()))
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"next receive sequence is not found", func() {
+			expError = types.ErrSequenceReceiveNotFound
 			suite.coordinator.SetupConnections(path)
 
 			path.EndpointA.ChannelID = ibctesting.FirstChannelID
@@ -372,13 +401,17 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			path.EndpointB.UpdateClient()
 		}, false},
 		{"receipt already stored", func() {
+			expError = types.ErrPacketReceived
 			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)
 			path.EndpointA.SendPacket(packet)
 			suite.chainB.App.GetIBCKeeper().ChannelKeeper.SetPacketReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, 1)
 			channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 		}, false},
 		{"validation failed", func() {
+			// skip error code check, downstream error code is used from light-client implementations
+
 			// packet commitment not set resulting in invalid proof
 			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)
@@ -390,6 +423,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 		tc := tc
 		suite.Run(fmt.Sprintf("Case %s, %d/%d tests", tc.msg, i, len(testCases)), func() {
 			suite.SetupTest() // reset
+			expError = nil    // must explicitly set for failed cases
 			path = ibctesting.NewPath(suite.chainA, suite.chainB)
 
 			tc.malleate()
@@ -418,6 +452,11 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 				}
 			} else {
 				suite.Require().Error(err)
+
+				// only check if expError is set, since not all error codes can be known
+				if expError != nil {
+					suite.Require().True(errors.Is(err, expError))
+				}
 			}
 		})
 	}
diff --git a/modules/core/04-channel/keeper/timeout.go b/modules/core/04-channel/keeper/timeout.go
index 6342145870b..004e9d211ef 100644
--- a/modules/core/04-channel/keeper/timeout.go
+++ b/modules/core/04-channel/keeper/timeout.go
@@ -81,7 +81,7 @@ func (k Keeper) TimeoutPacket(
 	commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 
 	if len(commitment) == 0 {
-		return sdkerrors.Wrapf(types.ErrPacketTimedOut, "packet sequence (%d)", packet.GetSequence())
+		return sdkerrors.Wrapf(types.ErrPacketCommitmentNotFound, "packet with sequence (%d) has been acknowledged or timed out. In rare cases the packet was never sent or the packet sequence is incorrect", packet.GetSequence())
 	}
 
 	packetCommitment := types.CommitPacket(k.cdc, packet)
diff --git a/modules/core/04-channel/types/errors.go b/modules/core/04-channel/types/errors.go
index 547524717a6..23b3bdf5988 100644
--- a/modules/core/04-channel/types/errors.go
+++ b/modules/core/04-channel/types/errors.go
@@ -21,14 +21,12 @@ var (
 	ErrPacketTimeout             = sdkerrors.Register(SubModuleName, 14, "packet timeout")
 	ErrTooManyConnectionHops     = sdkerrors.Register(SubModuleName, 15, "too many connection hops")
 	ErrInvalidAcknowledgement    = sdkerrors.Register(SubModuleName, 16, "invalid acknowledgement")
-	ErrPacketCommitmentNotFound  = sdkerrors.Register(SubModuleName, 17, "packet commitment not found")
-	ErrAcknowledgementExists     = sdkerrors.Register(SubModuleName, 18, "acknowledgement for packet already exists")
-	ErrInvalidChannelIdentifier  = sdkerrors.Register(SubModuleName, 19, "invalid channel identifier")
+	ErrAcknowledgementExists     = sdkerrors.Register(SubModuleName, 17, "acknowledgement for packet already exists")
+	ErrInvalidChannelIdentifier  = sdkerrors.Register(SubModuleName, 18, "invalid channel identifier")
 
 	// packets already relayed errors
-	ErrPacketReceived     = sdkerrors.Register(SubModuleName, 20, "packet already received")
-	ErrPacketAcknowledged = sdkerrors.Register(SubModuleName, 21, "packet acknowledgement already processed")
-	ErrPacketTimedOut     = sdkerrors.Register(SubModuleName, 22, "packet already timed out")
+	ErrPacketReceived           = sdkerrors.Register(SubModuleName, 19, "packet already received")
+	ErrPacketCommitmentNotFound = sdkerrors.Register(SubModuleName, 20, "packet commitment not found") // may occur for already received acknowledgements or timeouts and in rare cases for packets never sent
 
 	// ORDERED channel error
 	ErrPacketReceiptOutOfOrder = sdkerrors.Register(SubModuleName, 23, "packet cannot be received out of order")

From 7044cd8daa84872c178e045003a86e9bc6525629 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Colin=20Axn=C3=A9r?=
 <25233464+colin-axner@users.noreply.github.com>
Date: Wed, 26 May 2021 15:52:07 +0200
Subject: [PATCH 4/5] add acknowledge packet tests

---
 modules/core/04-channel/keeper/packet.go      |  4 +--
 modules/core/04-channel/keeper/packet_test.go | 33 ++++++++++++++++++-
 modules/core/04-channel/types/errors.go       |  2 +-
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/modules/core/04-channel/keeper/packet.go b/modules/core/04-channel/keeper/packet.go
index 2156e9a5118..32ecb14e654 100644
--- a/modules/core/04-channel/keeper/packet.go
+++ b/modules/core/04-channel/keeper/packet.go
@@ -272,7 +272,7 @@ func (k Keeper) RecvPacket(
 
 		if packet.GetSequence() != nextSequenceRecv {
 			return sdkerrors.Wrapf(
-				types.ErrPacketReceiptOutOfOrder,
+				types.ErrPacketSequenceOutOfOrder,
 				"packet sequence ≠ next receive sequence (%d ≠ %d)", packet.GetSequence(), nextSequenceRecv,
 			)
 		}
@@ -499,7 +499,7 @@ func (k Keeper) AcknowledgePacket(
 
 		if packet.GetSequence() != nextSequenceAck {
 			return sdkerrors.Wrapf(
-				sdkerrors.ErrInvalidSequence,
+				types.ErrPacketSequenceOutOfOrder,
 				"packet sequence ≠ next ack sequence (%d ≠ %d)", packet.GetSequence(), nextSequenceAck,
 			)
 		}
diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go
index 5eb9f708ec0..91ddc7d15d6 100644
--- a/modules/core/04-channel/keeper/packet_test.go
+++ b/modules/core/04-channel/keeper/packet_test.go
@@ -268,7 +268,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			suite.Require().NoError(err)
 		}, false},
 		{"out of order packet failure with ORDERED channel", func() {
-			expError = types.ErrPacketReceiptOutOfOrder
+			expError = types.ErrPacketSequenceOutOfOrder
 
 			path.SetChannelOrdered()
 			suite.coordinator.Setup(path)
@@ -557,6 +557,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 		ack    = ibcmock.MockAcknowledgement
 
 		channelCap *capabilitytypes.Capability
+		expError   *sdkerrors.Error
 	)
 
 	testCases := []testCase{
@@ -590,6 +591,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
+
 			path.SetChannelOrdered()
 			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)
@@ -607,6 +610,8 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			suite.Require().NoError(err)
 		}, false},
 		{"packet already acknowledged unordered channel", func() {
+			expError = types.ErrPacketCommitmentNotFound
+
 			// setup uses an UNORDERED channel
 			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)
@@ -625,11 +630,15 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			suite.Require().NoError(err)
 		}, false},
 		{"channel not found", func() {
+			expError = types.ErrChannelNotFound
+
 			// use wrong channel naming
 			suite.coordinator.Setup(path)
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
 		}, false},
 		{"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)
 
@@ -638,8 +647,11 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 		}, false},
 		{"capability authentication failed ORDERED", func() {
+			expError = types.ErrInvalidChannelCapability
+
 			path.SetChannelOrdered()
 			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)
 			// create packet commitment
 			err := path.EndpointA.SendPacket(packet)
@@ -652,18 +664,23 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			channelCap = capabilitytypes.NewCapability(3)
 		}, false},
 		{"packet destination port ≠ channel counterparty port", func() {
+			expError = types.ErrInvalidPacket
 			suite.coordinator.Setup(path)
+
 			// use wrong port for dest
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, ibctesting.InvalidID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 		}, false},
 		{"packet destination channel ID ≠ channel counterparty channel ID", func() {
+			expError = types.ErrInvalidPacket
 			suite.coordinator.Setup(path)
+
 			// use wrong channel for dest
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp)
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 		}, false},
 		{"connection not found", func() {
+			expError = connectiontypes.ErrConnectionNotFound
 			suite.coordinator.Setup(path)
 
 			// pass channel check
@@ -677,6 +694,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 		}, false},
 		{"connection not OPEN", func() {
+			expError = connectiontypes.ErrInvalidConnectionState
 			suite.coordinator.SetupClients(path)
 			// connection on chainA is in INIT
 			err := path.EndpointA.ConnOpenInit()
@@ -693,12 +711,16 @@ 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
+
 			// packet commitment never written
 			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)
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 		}, false},
 		{"packet ack verification failed", func() {
+			// skip error code check since error occurs in light-clients
+
 			// ack never written
 			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)
@@ -708,6 +730,8 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
 		}, false},
 		{"packet commitment bytes do not match", func() {
+			expError = types.ErrInvalidPacket
+
 			// setup uses an UNORDERED channel
 			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)
@@ -725,6 +749,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			packet.Data = []byte("invalid packet commitment")
 		}, false},
 		{"next ack sequence not found", func() {
+			expError = types.ErrSequenceAckNotFound
 			suite.coordinator.SetupConnections(path)
 
 			path.EndpointA.ChannelID = ibctesting.FirstChannelID
@@ -753,6 +778,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			path.EndpointB.UpdateClient()
 		}, false},
 		{"next ack sequence mismatch ORDERED", func() {
+			expError = types.ErrPacketSequenceOutOfOrder
 			path.SetChannelOrdered()
 			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)
@@ -774,6 +800,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 		tc := tc
 		suite.Run(fmt.Sprintf("Case %s, %d/%d tests", tc.msg, i, len(testCases)), func() {
 			suite.SetupTest() // reset
+			expError = nil    // must explcitly set error for failed cases
 			path = ibctesting.NewPath(suite.chainA, suite.chainB)
 
 			tc.malleate()
@@ -798,6 +825,10 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 				}
 			} else {
 				suite.Error(err)
+				// only check if expError is set, since not all error codes can be known
+				if expError != nil {
+					suite.Require().True(errors.Is(err, expError))
+				}
 			}
 		})
 	}
diff --git a/modules/core/04-channel/types/errors.go b/modules/core/04-channel/types/errors.go
index 23b3bdf5988..30293eddf07 100644
--- a/modules/core/04-channel/types/errors.go
+++ b/modules/core/04-channel/types/errors.go
@@ -29,5 +29,5 @@ var (
 	ErrPacketCommitmentNotFound = sdkerrors.Register(SubModuleName, 20, "packet commitment not found") // may occur for already received acknowledgements or timeouts and in rare cases for packets never sent
 
 	// ORDERED channel error
-	ErrPacketReceiptOutOfOrder = sdkerrors.Register(SubModuleName, 23, "packet cannot be received out of order")
+	ErrPacketSequenceOutOfOrder = sdkerrors.Register(SubModuleName, 21, "packet sequence is out of order")
 )

From ef95d451d90e1f14ffbca18746e16c07e9ba5126 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Colin=20Axn=C3=A9r?=
 <25233464+colin-axner@users.noreply.github.com>
Date: Wed, 26 May 2021 16:01:51 +0200
Subject: [PATCH 5/5] update timeout tests

---
 modules/core/04-channel/keeper/timeout.go     |  2 +-
 .../core/04-channel/keeper/timeout_test.go    | 25 +++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/modules/core/04-channel/keeper/timeout.go b/modules/core/04-channel/keeper/timeout.go
index 004e9d211ef..08d10f83612 100644
--- a/modules/core/04-channel/keeper/timeout.go
+++ b/modules/core/04-channel/keeper/timeout.go
@@ -96,7 +96,7 @@ func (k Keeper) TimeoutPacket(
 		// check that packet has not been received
 		if nextSequenceRecv > packet.GetSequence() {
 			return sdkerrors.Wrapf(
-				types.ErrInvalidPacket,
+				types.ErrPacketReceived,
 				"packet already received, next sequence receive > packet sequence (%d > %d)", nextSequenceRecv, packet.GetSequence(),
 			)
 		}
diff --git a/modules/core/04-channel/keeper/timeout_test.go b/modules/core/04-channel/keeper/timeout_test.go
index bdbb3349713..460e6097632 100644
--- a/modules/core/04-channel/keeper/timeout_test.go
+++ b/modules/core/04-channel/keeper/timeout_test.go
@@ -1,10 +1,14 @@
 package keeper_test
 
 import (
+	"errors"
 	"fmt"
 
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
+
 	clienttypes "github.com/cosmos/ibc-go/modules/core/02-client/types"
+	connectiontypes "github.com/cosmos/ibc-go/modules/core/03-connection/types"
 	"github.com/cosmos/ibc-go/modules/core/04-channel/types"
 	host "github.com/cosmos/ibc-go/modules/core/24-host"
 	"github.com/cosmos/ibc-go/modules/core/exported"
@@ -20,6 +24,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 		packet      types.Packet
 		nextSeqRecv uint64
 		ordered     bool
+		expError    *sdkerrors.Error
 	)
 
 	testCases := []testCase{
@@ -43,6 +48,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			path.EndpointA.UpdateClient()
 		}, true},
 		{"packet already timed out: ORDERED", func() {
+			expError = types.ErrInvalidChannelState
 			ordered = true
 			path.SetChannelOrdered()
 
@@ -56,6 +62,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			suite.Require().NoError(err)
 		}, false},
 		{"packet already timed out: UNORDERED", func() {
+			expError = types.ErrPacketCommitmentNotFound
 			ordered = false
 
 			suite.coordinator.Setup(path)
@@ -68,11 +75,13 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			suite.Require().NoError(err)
 		}, false},
 		{"channel not found", func() {
+			expError = types.ErrChannelNotFound
 			// use wrong channel naming
 			suite.coordinator.Setup(path)
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, ibctesting.InvalidID, ibctesting.InvalidID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
 		}, false},
 		{"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)
 
@@ -80,16 +89,19 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			suite.Require().NoError(err)
 		}, false},
 		{"packet destination port ≠ channel counterparty port", func() {
+			expError = types.ErrInvalidPacket
 			suite.coordinator.Setup(path)
 			// use wrong port for dest
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, ibctesting.InvalidID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
 		}, false},
 		{"packet destination channel ID ≠ channel counterparty channel ID", func() {
+			expError = types.ErrInvalidPacket
 			suite.coordinator.Setup(path)
 			// use wrong channel for dest
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, ibctesting.InvalidID, timeoutHeight, disabledTimeoutTimestamp)
 		}, false},
 		{"connection not found", func() {
+			expError = connectiontypes.ErrConnectionNotFound
 			// pass channel check
 			suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetChannel(
 				suite.chainA.GetContext(),
@@ -99,12 +111,14 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
 		}, false},
 		{"timeout", func() {
+			expError = types.ErrPacketTimeout
 			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)
 			path.EndpointA.SendPacket(packet)
 			path.EndpointA.UpdateClient()
 		}, false},
 		{"packet already received ", func() {
+			expError = types.ErrPacketReceived
 			ordered = true
 			path.SetChannelOrdered()
 
@@ -116,6 +130,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			path.EndpointA.UpdateClient()
 		}, false},
 		{"packet hasn't been sent", func() {
+			expError = types.ErrPacketCommitmentNotFound
 			ordered = true
 			path.SetChannelOrdered()
 
@@ -124,6 +139,8 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			path.EndpointA.UpdateClient()
 		}, false},
 		{"next seq receive verification failed", func() {
+			// skip error check, error occurs in light-clients
+
 			// set ordered to false resulting in wrong proof provided
 			ordered = false
 
@@ -135,6 +152,8 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			path.EndpointA.UpdateClient()
 		}, false},
 		{"packet ack verification failed", func() {
+			// skip error check, error occurs in light-clients
+
 			// set ordered to true resulting in wrong proof provided
 			ordered = true
 
@@ -154,6 +173,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 			)
 
 			suite.SetupTest() // reset
+			expError = nil    // must be expliticly changed by failed cases
 			nextSeqRecv = 1   // must be explicitly changed
 			path = ibctesting.NewPath(suite.chainA, suite.chainB)
 
@@ -174,6 +194,11 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
 				suite.Require().NoError(err)
 			} else {
 				suite.Require().Error(err)
+				// only check if expError is set, since not all error codes can be known
+				if expError != nil {
+					suite.Require().True(errors.Is(err, expError))
+				}
+
 			}
 		})
 	}