From 1541bbb745df60b3b150ed7843c2d9e45d7421cf Mon Sep 17 00:00:00 2001
From: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com>
Date: Thu, 6 Feb 2025 12:45:15 -0500
Subject: [PATCH 1/3] fix msg server

---
 modules/apps/transfer/types/errors.go         |  1 +
 modules/apps/transfer/v2/ibc_module.go        | 26 +++++------
 modules/apps/transfer/v2/ibc_module_test.go   | 14 +++---
 .../02-client/migrations/v7/solomachine.pb.go |  2 +-
 .../core/04-channel/v2/keeper/msg_server.go   | 45 +++++++++++++------
 .../04-channel/v2/keeper/msg_server_test.go   | 10 +++--
 .../04-channel/v2/types/acknowledgement.go    |  4 ++
 modules/core/04-channel/v2/types/packet.pb.go |  5 +++
 .../06-solomachine/solomachine.pb.go          |  2 +-
 proto/ibc/core/channel/v2/packet.proto        |  5 +++
 10 files changed, 74 insertions(+), 40 deletions(-)

diff --git a/modules/apps/transfer/types/errors.go b/modules/apps/transfer/types/errors.go
index 32cd9b02b90..39b01b0e373 100644
--- a/modules/apps/transfer/types/errors.go
+++ b/modules/apps/transfer/types/errors.go
@@ -21,4 +21,5 @@ var (
 	ErrForwardedPacketFailed   = errorsmod.Register(ModuleName, 14, "forwarded packet failed")
 	ErrAbiEncoding             = errorsmod.Register(ModuleName, 15, "encoding abi failed")
 	ErrAbiDecoding             = errorsmod.Register(ModuleName, 16, "decoding abi failed")
+	ErrReceiveFailed           = errorsmod.Register(ModuleName, 17, "receive packet failed")
 )
diff --git a/modules/apps/transfer/v2/ibc_module.go b/modules/apps/transfer/v2/ibc_module.go
index 0ae4aad554f..31c0c128c7e 100644
--- a/modules/apps/transfer/v2/ibc_module.go
+++ b/modules/apps/transfer/v2/ibc_module.go
@@ -1,6 +1,7 @@
 package v2
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"strings"
@@ -87,8 +88,7 @@ func (im *IBCModule) OnRecvPacket(ctx context.Context, sourceChannel string, des
 	// rather than just the destination port
 	if payload.SourcePort != types.PortID || payload.DestinationPort != types.PortID {
 		return channeltypesv2.RecvPacketResult{
-			Status:          channeltypesv2.PacketStatus_Failure,
-			Acknowledgement: channeltypes.NewErrorAcknowledgement(errorsmod.Wrapf(channeltypesv2.ErrInvalidPacket, "payload port ID is invalid: expected %s, got sourcePort: %s destPort: %s", types.PortID, payload.SourcePort, payload.DestinationPort)).Acknowledgement(),
+			Status: channeltypesv2.PacketStatus_Failure,
 		}
 	}
 	var (
@@ -111,11 +111,9 @@ func (im *IBCModule) OnRecvPacket(ctx context.Context, sourceChannel string, des
 
 	data, ackErr = types.UnmarshalPacketData(payload.Value, payload.Version, payload.Encoding)
 	if ackErr != nil {
-		ack = channeltypes.NewErrorAcknowledgement(ackErr)
 		im.keeper.Logger.Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), sequence))
 		return channeltypesv2.RecvPacketResult{
-			Status:          channeltypesv2.PacketStatus_Failure,
-			Acknowledgement: ack.Acknowledgement(),
+			Status: channeltypesv2.PacketStatus_Failure,
 		}
 	}
 
@@ -127,11 +125,9 @@ func (im *IBCModule) OnRecvPacket(ctx context.Context, sourceChannel string, des
 		payload.DestinationPort,
 		destinationChannel,
 	); ackErr != nil {
-		ack = channeltypes.NewErrorAcknowledgement(ackErr)
 		im.keeper.Logger.Error(fmt.Sprintf("%s sequence %d", ackErr.Error(), sequence))
 		return channeltypesv2.RecvPacketResult{
-			Status:          channeltypesv2.PacketStatus_Failure,
-			Acknowledgement: ack.Acknowledgement(),
+			Status: channeltypesv2.PacketStatus_Failure,
 		}
 	}
 
@@ -142,10 +138,8 @@ func (im *IBCModule) OnRecvPacket(ctx context.Context, sourceChannel string, des
 
 	if data.HasForwarding() {
 		// we are now sending from the forward escrow address to the final receiver address.
-		ack = channeltypes.NewErrorAcknowledgement(fmt.Errorf("forwarding not yet supported"))
 		return channeltypesv2.RecvPacketResult{
-			Status:          channeltypesv2.PacketStatus_Failure,
-			Acknowledgement: ack.Acknowledgement(),
+			Status: channeltypesv2.PacketStatus_Failure,
 		}
 		// TODO: handle forwarding
 		// TODO: inside this version of the function, we should fetch the packet that was stored in IBC core in order to set it for forwarding.
@@ -181,8 +175,14 @@ func (im *IBCModule) OnTimeoutPacket(ctx context.Context, sourceChannel string,
 
 func (im *IBCModule) OnAcknowledgementPacket(ctx context.Context, sourceChannel string, destinationChannel string, sequence uint64, acknowledgement []byte, payload channeltypesv2.Payload, relayer sdk.AccAddress) error {
 	var ack channeltypes.Acknowledgement
-	if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
-		return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err)
+	// construct an error acknowledgement if the acknowledgement bytes are the sentinel error acknowledgement so we can use the shared transfer logic
+	if bytes.Equal(acknowledgement, channeltypesv2.ErrorAcknowledgement[:]) {
+		// the specific error does not matter
+		ack = channeltypes.NewErrorAcknowledgement(types.ErrReceiveFailed)
+	} else {
+		if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
+			return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err)
+		}
 	}
 
 	data, err := types.UnmarshalPacketData(payload.Value, payload.Version, payload.Encoding)
diff --git a/modules/apps/transfer/v2/ibc_module_test.go b/modules/apps/transfer/v2/ibc_module_test.go
index 4c410dc3773..13995e855cd 100644
--- a/modules/apps/transfer/v2/ibc_module_test.go
+++ b/modules/apps/transfer/v2/ibc_module_test.go
@@ -6,7 +6,6 @@ import (
 
 	testifysuite "github.com/stretchr/testify/suite"
 
-	errorsmod "cosmossdk.io/errors"
 	sdkmath "cosmossdk.io/math"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -177,19 +176,19 @@ func (suite *TransferTestSuite) TestOnRecvPacket() {
 		name                   string
 		sourceDenomsToTransfer []string
 		malleate               func()
-		expErrAck              []byte
+		expErr                 bool
 	}{
 		{
 			"transfer single denom",
 			[]string{sdk.DefaultBondDenom},
 			func() {},
-			nil,
+			false,
 		},
 		{
 			"transfer multiple denoms",
 			[]string{sdk.DefaultBondDenom, ibctesting.SecondaryDenom},
 			func() {},
-			nil,
+			false,
 		},
 		{
 			"transfer with invalid source port",
@@ -197,7 +196,7 @@ func (suite *TransferTestSuite) TestOnRecvPacket() {
 			func() {
 				payload.SourcePort = invalidPortID
 			},
-			channeltypes.NewErrorAcknowledgement(errorsmod.Wrapf(channeltypesv2.ErrInvalidPacket, "payload port ID is invalid: expected %s, got sourcePort: %s destPort: %s", types.PortID, payload.SourcePort, payload.DestinationPort)).Acknowledgement(),
+			true,
 		},
 		{
 			"transfer with invalid dest port",
@@ -205,7 +204,7 @@ func (suite *TransferTestSuite) TestOnRecvPacket() {
 			func() {
 				payload.DestinationPort = invalidPortID
 			},
-			channeltypes.NewErrorAcknowledgement(errorsmod.Wrapf(channeltypesv2.ErrInvalidPacket, "payload port ID is invalid: expected %s, got sourcePort: %s destPort: %s", types.PortID, payload.SourcePort, payload.DestinationPort)).Acknowledgement(),
+			true,
 		},
 	}
 
@@ -270,7 +269,7 @@ func (suite *TransferTestSuite) TestOnRecvPacket() {
 				1, payload, suite.chainB.SenderAccount.GetAddress(),
 			)
 
-			if tc.expErrAck == nil {
+			if !tc.expErr {
 
 				suite.Require().Equal(channeltypesv2.PacketStatus_Success, recvResult.Status)
 				suite.Require().Equal(channeltypes.NewResultAcknowledgement([]byte{byte(1)}).Acknowledgement(), recvResult.Acknowledgement)
@@ -296,7 +295,6 @@ func (suite *TransferTestSuite) TestOnRecvPacket() {
 				}
 			} else {
 				suite.Require().Equal(channeltypesv2.PacketStatus_Failure, recvResult.Status)
-				suite.Require().Equal(tc.expErrAck, recvResult.Acknowledgement)
 			}
 		})
 	}
diff --git a/modules/core/02-client/migrations/v7/solomachine.pb.go b/modules/core/02-client/migrations/v7/solomachine.pb.go
index 58f14a1f163..3de8b27ea4a 100644
--- a/modules/core/02-client/migrations/v7/solomachine.pb.go
+++ b/modules/core/02-client/migrations/v7/solomachine.pb.go
@@ -139,7 +139,7 @@ var xxx_messageInfo_ClientState proto.InternalMessageInfo
 type ConsensusState struct {
 	// public key of the solo machine
 	PublicKey *any.Any `protobuf:"bytes,1,opt,name=public_key,json=publicKey,proto3" json:"public_key,omitempty"`
-	// diversifier allows the same public key to be re-used across different solo
+	// diversifier allows the same public key to be reused across different solo
 	// machine clients (potentially on different chains) without being considered
 	// misbehaviour.
 	Diversifier string `protobuf:"bytes,2,opt,name=diversifier,proto3" json:"diversifier,omitempty"`
diff --git a/modules/core/04-channel/v2/keeper/msg_server.go b/modules/core/04-channel/v2/keeper/msg_server.go
index 7ce4f51a30a..cb77dda971d 100644
--- a/modules/core/04-channel/v2/keeper/msg_server.go
+++ b/modules/core/04-channel/v2/keeper/msg_server.go
@@ -1,6 +1,7 @@
 package keeper
 
 import (
+	"bytes"
 	"context"
 	"time"
 
@@ -89,6 +90,7 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *types.MsgRecvPacket) (*typ
 	}
 
 	var isAsync bool
+	isSuccess := true
 	for _, pd := range msg.Packet.Payloads {
 		// Cache context so that we may discard state changes from callback if the acknowledgement is unsuccessful.
 		cacheCtx, writeFn = sdkCtx.CacheContext()
@@ -96,11 +98,23 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *types.MsgRecvPacket) (*typ
 		res := cb.OnRecvPacket(cacheCtx, msg.Packet.SourceClient, msg.Packet.DestinationClient, msg.Packet.Sequence, pd, signer)
 
 		if res.Status != types.PacketStatus_Failure {
+			// successful app acknowledgement cannot equal sentinel error acknowledgement
+			if bytes.Equal(res.GetAcknowledgement(), types.ErrorAcknowledgement[:]) {
+				return nil, errorsmod.Wrapf(types.ErrInvalidAcknowledgement, "application acknowledgement cannot be sentinel error acknowledgement")
+			}
 			// write application state changes for asynchronous and successful acknowledgements
 			writeFn()
+			// append app acknowledgement to the overall acknowledgement
+			ack.AppAcknowledgements = append(ack.AppAcknowledgements, res.Acknowledgement)
 		} else {
+			isSuccess = false
+			// construct acknowledgement with single app acknowledgement that is the sentinel error acknowledgement
+			ack = types.Acknowledgement{
+				AppAcknowledgements: [][]byte{types.ErrorAcknowledgement[:]},
+			}
 			// Modify events in cached context to reflect unsuccessful acknowledgement
 			sdkCtx.EventManager().EmitEvents(internalerrors.ConvertToErrorEvents(cacheCtx.EventManager().Events()))
+			break
 		}
 
 		if res.Status == types.PacketStatus_Async {
@@ -112,22 +126,16 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *types.MsgRecvPacket) (*typ
 				return nil, errorsmod.Wrapf(types.ErrInvalidPacket, "packet with multiple payloads cannot have async acknowledgement")
 			}
 		}
-
-		// append app acknowledgement to the overall acknowledgement
-		ack.AppAcknowledgements = append(ack.AppAcknowledgements, res.Acknowledgement)
-	}
-
-	if len(ack.AppAcknowledgements) != len(msg.Packet.Payloads) {
-		return nil, errorsmod.Wrapf(types.ErrInvalidAcknowledgement, "length of app acknowledgement %d does not match length of app payload %d", len(ack.AppAcknowledgements), len(msg.Packet.Payloads))
-	}
-
-	// note this should never happen as the payload would have had to be empty.
-	if len(ack.AppAcknowledgements) == 0 {
-		sdkCtx.Logger().Error("receive packet failed", "source-client", msg.Packet.SourceClient, "error", errorsmod.Wrap(err, "invalid acknowledgement results"))
-		return &types.MsgRecvPacketResponse{Result: types.FAILURE}, errorsmod.Wrapf(err, "receive packet failed source-client %s invalid acknowledgement results", msg.Packet.SourceClient)
 	}
 
 	if !isAsync {
+		// If the application callback was successful, the acknowledgement must have the same number of app acknowledgements as the packet payloads.
+		if isSuccess {
+			if len(ack.AppAcknowledgements) != len(msg.Packet.Payloads) {
+				return nil, errorsmod.Wrapf(types.ErrInvalidAcknowledgement, "length of app acknowledgement %d does not match length of app payload %d", len(ack.AppAcknowledgements), len(msg.Packet.Payloads))
+			}
+		}
+
 		// Validate ack before forwarding to WriteAcknowledgement.
 		if err := ack.Validate(); err != nil {
 			return nil, err
@@ -173,9 +181,18 @@ func (k *Keeper) Acknowledgement(ctx context.Context, msg *types.MsgAcknowledgem
 		return nil, errorsmod.Wrap(err, "acknowledge packet verification failed")
 	}
 
+	recvSuccess := !bytes.Equal(msg.Acknowledgement.AppAcknowledgements[0], types.ErrorAcknowledgement[:])
 	for i, pd := range msg.Packet.Payloads {
 		cbs := k.Router.Route(pd.SourcePort)
-		ack := msg.Acknowledgement.AppAcknowledgements[i]
+		var ack []byte
+		// if recv was successful, each payload should have its own acknowledgement so we send each individual acknowledgment to the application
+		// otherwise, the acknowledgement only contains the sentinel error acknowledgement which we send to the application. The application is responsible
+		// for knowing that this is an error acknowledgement and executing the appropriate logic.
+		if recvSuccess {
+			ack = msg.Acknowledgement.AppAcknowledgements[i]
+		} else {
+			ack = types.ErrorAcknowledgement[:]
+		}
 		err := cbs.OnAcknowledgementPacket(ctx, msg.Packet.SourceClient, msg.Packet.DestinationClient, msg.Packet.Sequence, ack, pd, relayer)
 		if err != nil {
 			return nil, errorsmod.Wrapf(err, "failed OnAcknowledgementPacket for source port %s, source client %s, destination client %s", pd.SourcePort, msg.Packet.SourceClient, msg.Packet.DestinationClient)
diff --git a/modules/core/04-channel/v2/keeper/msg_server_test.go b/modules/core/04-channel/v2/keeper/msg_server_test.go
index 0c5e2d8342d..7a99344c73f 100644
--- a/modules/core/04-channel/v2/keeper/msg_server_test.go
+++ b/modules/core/04-channel/v2/keeper/msg_server_test.go
@@ -165,8 +165,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() {
 			name: "success: failed recv result",
 			malleate: func() {
 				expRecvRes = types.RecvPacketResult{
-					Status:          types.PacketStatus_Failure,
-					Acknowledgement: mock.MockFailPacketData,
+					Status: types.PacketStatus_Failure,
 				}
 			},
 			expError:      nil,
@@ -240,7 +239,12 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() {
 			tc.malleate()
 
 			// expectedAck is derived from the expected recv result.
-			expectedAck := types.Acknowledgement{AppAcknowledgements: [][]byte{expRecvRes.Acknowledgement}}
+			var expectedAck types.Acknowledgement
+			if expRecvRes.Status == types.PacketStatus_Success {
+				expectedAck = types.Acknowledgement{AppAcknowledgements: [][]byte{expRecvRes.Acknowledgement}}
+			} else {
+				expectedAck = types.Acknowledgement{AppAcknowledgements: [][]byte{types.ErrorAcknowledgement[:]}}
+			}
 
 			// modify the callback to return the expected recv result.
 			path.EndpointB.Chain.GetSimApp().MockModuleV2B.IBCApp.OnRecvPacket = func(ctx context.Context, sourceChannel string, destinationChannel string, sequence uint64, data types.Payload, relayer sdk.AccAddress) types.RecvPacketResult {
diff --git a/modules/core/04-channel/v2/types/acknowledgement.go b/modules/core/04-channel/v2/types/acknowledgement.go
index f5d50e20394..c0290d8f47d 100644
--- a/modules/core/04-channel/v2/types/acknowledgement.go
+++ b/modules/core/04-channel/v2/types/acknowledgement.go
@@ -1,9 +1,13 @@
 package types
 
 import (
+	"crypto/sha256"
+
 	errorsmod "cosmossdk.io/errors"
 )
 
+var ErrorAcknowledgement = sha256.Sum256([]byte("UNIVERSAL ERROR ACKNOWLEDGEMENT"))
+
 // NewAcknowledgement creates a new Acknowledgement containing the provided app acknowledgements.
 func NewAcknowledgement(appAcknowledgements ...[]byte) Acknowledgement {
 	return Acknowledgement{AppAcknowledgements: appAcknowledgements}
diff --git a/modules/core/04-channel/v2/types/packet.pb.go b/modules/core/04-channel/v2/types/packet.pb.go
index 2c52ed19bf9..cfd2df014fe 100644
--- a/modules/core/04-channel/v2/types/packet.pb.go
+++ b/modules/core/04-channel/v2/types/packet.pb.go
@@ -226,6 +226,11 @@ func (m *Payload) GetValue() []byte {
 }
 
 // Acknowledgement contains a list of all ack results associated with a single packet.
+// In the case of a successful receive, the acknowledgement will contain an app acknowledgement
+// for each application that received a payload in the same order that the payloads were sent
+// in the packet.
+// If the receive is not successful, the acknowledgement will contain a single app acknowledgment
+// which will be a constant error acknowledgment as defined by the IBC v2 protocol.
 type Acknowledgement struct {
 	AppAcknowledgements [][]byte `protobuf:"bytes,1,rep,name=app_acknowledgements,json=appAcknowledgements,proto3" json:"app_acknowledgements,omitempty"`
 }
diff --git a/modules/light-clients/06-solomachine/solomachine.pb.go b/modules/light-clients/06-solomachine/solomachine.pb.go
index 6ed1f01f3fa..44a5407d3f3 100644
--- a/modules/light-clients/06-solomachine/solomachine.pb.go
+++ b/modules/light-clients/06-solomachine/solomachine.pb.go
@@ -73,7 +73,7 @@ var xxx_messageInfo_ClientState proto.InternalMessageInfo
 type ConsensusState struct {
 	// public key of the solo machine
 	PublicKey *any.Any `protobuf:"bytes,1,opt,name=public_key,json=publicKey,proto3" json:"public_key,omitempty"`
-	// diversifier allows the same public key to be re-used across different solo
+	// diversifier allows the same public key to be reused across different solo
 	// machine clients (potentially on different chains) without being considered
 	// misbehaviour.
 	Diversifier string `protobuf:"bytes,2,opt,name=diversifier,proto3" json:"diversifier,omitempty"`
diff --git a/proto/ibc/core/channel/v2/packet.proto b/proto/ibc/core/channel/v2/packet.proto
index 8a311967237..e7284bae0c2 100644
--- a/proto/ibc/core/channel/v2/packet.proto
+++ b/proto/ibc/core/channel/v2/packet.proto
@@ -38,6 +38,11 @@ message Payload {
 }
 
 // Acknowledgement contains a list of all ack results associated with a single packet.
+// In the case of a successful receive, the acknowledgement will contain an app acknowledgement
+// for each application that received a payload in the same order that the payloads were sent
+// in the packet.
+// If the receive is not successful, the acknowledgement will contain a single app acknowledgment
+// which will be a constant error acknowledgment as defined by the IBC v2 protocol.
 message Acknowledgement {
   repeated bytes app_acknowledgements = 1;
 }

From 43755e9143995a2267e9439fa597c1fac3150fc8 Mon Sep 17 00:00:00 2001
From: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com>
Date: Thu, 6 Feb 2025 13:57:22 -0500
Subject: [PATCH 2/3] add tests

---
 .../04-channel/v2/keeper/msg_server_test.go   | 19 ++++++++++++++++---
 .../core/04-channel/v2/keeper/packet_test.go  |  2 +-
 .../04-channel/v2/types/acknowledgement.go    |  2 +-
 .../v2/types/acknowledgement_test.go          |  5 +++++
 .../04-channel/v2/types/commitment_test.go    |  9 +++++++++
 testing/mock/v2/ibc_module.go                 | 11 ++++++++---
 testing/mock/v2/mock.go                       | 14 ++++++++++----
 7 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/modules/core/04-channel/v2/keeper/msg_server_test.go b/modules/core/04-channel/v2/keeper/msg_server_test.go
index 7a99344c73f..e90895287e8 100644
--- a/modules/core/04-channel/v2/keeper/msg_server_test.go
+++ b/modules/core/04-channel/v2/keeper/msg_server_test.go
@@ -175,8 +175,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() {
 			name: "success: async recv result",
 			malleate: func() {
 				expRecvRes = types.RecvPacketResult{
-					Status:          types.PacketStatus_Async,
-					Acknowledgement: nil,
+					Status: types.PacketStatus_Async,
 				}
 			},
 			expError:      nil,
@@ -295,11 +294,13 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() {
 	testCases := []struct {
 		name     string
 		malleate func()
+		payload  types.Payload
 		expError error
 	}{
 		{
 			name:     "success",
 			malleate: func() {},
+			payload:  mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB),
 		},
 		{
 			name: "success: NoOp",
@@ -312,6 +313,14 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() {
 					return mock.MockApplicationCallbackError
 				}
 			},
+			payload: mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB),
+		},
+		{
+			name: "success: failed ack result",
+			malleate: func() {
+				ack.AppAcknowledgements[0] = types.ErrorAcknowledgement[:]
+			},
+			payload: mockv2.NewErrorMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB),
 		},
 		{
 			name: "failure: callback fails",
@@ -320,6 +329,7 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() {
 					return mock.MockApplicationCallbackError
 				}
 			},
+			payload:  mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB),
 			expError: mock.MockApplicationCallbackError,
 		},
 		{
@@ -328,6 +338,7 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() {
 				// change the source id to a non-existent channel.
 				packet.SourceClient = "not-existent-channel"
 			},
+			payload:  mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB),
 			expError: clienttypes.ErrCounterpartyNotFound,
 		},
 		{
@@ -335,6 +346,7 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() {
 			malleate: func() {
 				suite.chainA.App.GetIBCKeeper().ChannelKeeperV2.SetPacketCommitment(suite.chainA.GetContext(), packet.SourceClient, packet.Sequence, []byte("foo"))
 			},
+			payload:  mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB),
 			expError: types.ErrInvalidPacket,
 		},
 		{
@@ -342,6 +354,7 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() {
 			malleate: func() {
 				ack.AppAcknowledgements[0] = mock.MockFailPacketData
 			},
+			payload:  mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB),
 			expError: errors.New("failed packet acknowledgement verification"),
 		},
 	}
@@ -356,7 +369,7 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() {
 
 			var err error
 			// Send packet from A to B
-			packet, err = path.EndpointA.MsgSendPacket(timeoutTimestamp, mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB))
+			packet, err = path.EndpointA.MsgSendPacket(timeoutTimestamp, tc.payload)
 			suite.Require().NoError(err)
 
 			err = path.EndpointB.MsgRecvPacket(packet)
diff --git a/modules/core/04-channel/v2/keeper/packet_test.go b/modules/core/04-channel/v2/keeper/packet_test.go
index 094cc1b85e1..4e2c3221967 100644
--- a/modules/core/04-channel/v2/keeper/packet_test.go
+++ b/modules/core/04-channel/v2/keeper/packet_test.go
@@ -394,7 +394,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 		{
 			"failure: verify membership fails",
 			func() {
-				ack.AppAcknowledgements[0] = mockv2.MockFailRecvPacketResult.Acknowledgement
+				ack.AppAcknowledgements[0] = types.ErrorAcknowledgement[:]
 			},
 			commitmenttypes.ErrInvalidProof,
 		},
diff --git a/modules/core/04-channel/v2/types/acknowledgement.go b/modules/core/04-channel/v2/types/acknowledgement.go
index c0290d8f47d..ef59f221f77 100644
--- a/modules/core/04-channel/v2/types/acknowledgement.go
+++ b/modules/core/04-channel/v2/types/acknowledgement.go
@@ -6,7 +6,7 @@ import (
 	errorsmod "cosmossdk.io/errors"
 )
 
-var ErrorAcknowledgement = sha256.Sum256([]byte("UNIVERSAL ERROR ACKNOWLEDGEMENT"))
+var ErrorAcknowledgement = sha256.Sum256([]byte("UNIVERSAL_ERROR_ACKNOWLEDGEMENT"))
 
 // NewAcknowledgement creates a new Acknowledgement containing the provided app acknowledgements.
 func NewAcknowledgement(appAcknowledgements ...[]byte) Acknowledgement {
diff --git a/modules/core/04-channel/v2/types/acknowledgement_test.go b/modules/core/04-channel/v2/types/acknowledgement_test.go
index f06c6e89a50..3c6f8f64061 100644
--- a/modules/core/04-channel/v2/types/acknowledgement_test.go
+++ b/modules/core/04-channel/v2/types/acknowledgement_test.go
@@ -16,6 +16,11 @@ func (s *TypesTestSuite) Test_ValidateAcknowledgement() {
 			types.NewAcknowledgement([]byte("appAck1")),
 			nil,
 		},
+		{
+			"success: valid failed ack",
+			types.NewAcknowledgement(types.ErrorAcknowledgement[:]),
+			nil,
+		},
 		{
 			"failure: more than one app acknowledgements",
 			types.NewAcknowledgement([]byte("appAck1"), []byte("appAck2")),
diff --git a/modules/core/04-channel/v2/types/commitment_test.go b/modules/core/04-channel/v2/types/commitment_test.go
index 1de9f25d205..637c0ade235 100644
--- a/modules/core/04-channel/v2/types/commitment_test.go
+++ b/modules/core/04-channel/v2/types/commitment_test.go
@@ -89,4 +89,13 @@ func TestCommitAcknowledgement(t *testing.T) {
 
 	commitment := types.CommitAcknowledgement(ack)
 	require.Equal(t, "f03b4667413e56aaf086663267913e525c442b56fa1af4fa3f3dab9f37044c5b", hex.EncodeToString(commitment))
+
+	failedAck := types.Acknowledgement{
+		AppAcknowledgements: [][]byte{
+			types.ErrorAcknowledgement[:],
+		},
+	}
+
+	failedAckCommitment := types.CommitAcknowledgement(failedAck)
+	require.Equal(t, "e2fb30dfbf7abdeaca82d426534d2b3a9d5444dd2a87fa16d38b77ba1a13ced7", hex.EncodeToString(failedAckCommitment))
 }
diff --git a/testing/mock/v2/ibc_module.go b/testing/mock/v2/ibc_module.go
index 4aa53ee8d34..20a63bc2b83 100644
--- a/testing/mock/v2/ibc_module.go
+++ b/testing/mock/v2/ibc_module.go
@@ -1,12 +1,14 @@
 package mock
 
 import (
+	"bytes"
 	"context"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types"
 	"github.com/cosmos/ibc-go/v9/modules/core/api"
+	mockv1 "github.com/cosmos/ibc-go/v9/testing/mock"
 )
 
 var _ api.IBCModule = (*IBCModule)(nil)
@@ -42,11 +44,14 @@ func (im IBCModule) OnSendPacket(ctx context.Context, sourceChannel string, dest
 	return nil
 }
 
-func (im IBCModule) OnRecvPacket(ctx context.Context, sourceChannel string, destinationChannel string, sequence uint64, data channeltypesv2.Payload, relayer sdk.AccAddress) channeltypesv2.RecvPacketResult {
+func (im IBCModule) OnRecvPacket(ctx context.Context, sourceChannel string, destinationChannel string, sequence uint64, payload channeltypesv2.Payload, relayer sdk.AccAddress) channeltypesv2.RecvPacketResult {
 	if im.IBCApp.OnRecvPacket != nil {
-		return im.IBCApp.OnRecvPacket(ctx, sourceChannel, destinationChannel, sequence, data, relayer)
+		return im.IBCApp.OnRecvPacket(ctx, sourceChannel, destinationChannel, sequence, payload, relayer)
 	}
-	return MockRecvPacketResult
+	if bytes.Equal(payload.Value, mockv1.MockPacketData) {
+		return MockRecvPacketResult
+	}
+	return channeltypesv2.RecvPacketResult{Status: channeltypesv2.PacketStatus_Failure}
 }
 
 func (im IBCModule) OnAcknowledgementPacket(ctx context.Context, sourceChannel string, destinationChannel string, sequence uint64, acknowledgement []byte, payload channeltypesv2.Payload, relayer sdk.AccAddress) error {
diff --git a/testing/mock/v2/mock.go b/testing/mock/v2/mock.go
index 3d2a0bd8201..3094a9b0e25 100644
--- a/testing/mock/v2/mock.go
+++ b/testing/mock/v2/mock.go
@@ -15,10 +15,6 @@ var (
 		Status:          channeltypesv2.PacketStatus_Success,
 		Acknowledgement: mockv1.MockAcknowledgement.Acknowledgement(),
 	}
-	MockFailRecvPacketResult = channeltypesv2.RecvPacketResult{
-		Status:          channeltypesv2.PacketStatus_Success,
-		Acknowledgement: mockv1.MockFailAcknowledgement.Acknowledgement(),
-	}
 )
 
 func NewMockPayload(sourcePort, destPort string) channeltypesv2.Payload {
@@ -30,3 +26,13 @@ func NewMockPayload(sourcePort, destPort string) channeltypesv2.Payload {
 		Version:         mockv1.Version,
 	}
 }
+
+func NewErrorMockPayload(sourcePort, destPort string) channeltypesv2.Payload {
+	return channeltypesv2.Payload{
+		SourcePort:      sourcePort,
+		DestinationPort: destPort,
+		Encoding:        transfertypes.EncodingProtobuf,
+		Value:           mockv1.MockFailPacketData,
+		Version:         mockv1.Version,
+	}
+}

From af78be6f39567324df83de675db94ab0c9e7f5af Mon Sep 17 00:00:00 2001
From: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com>
Date: Thu, 6 Feb 2025 14:07:45 -0500
Subject: [PATCH 3/3] harden transfer ack

---
 modules/apps/transfer/v2/ibc_module.go      |  3 +++
 modules/apps/transfer/v2/ibc_module_test.go | 11 +++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/modules/apps/transfer/v2/ibc_module.go b/modules/apps/transfer/v2/ibc_module.go
index 31c0c128c7e..88122d74dc7 100644
--- a/modules/apps/transfer/v2/ibc_module.go
+++ b/modules/apps/transfer/v2/ibc_module.go
@@ -183,6 +183,9 @@ func (im *IBCModule) OnAcknowledgementPacket(ctx context.Context, sourceChannel
 		if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
 			return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err)
 		}
+		if !ack.Success() {
+			return errorsmod.Wrapf(ibcerrors.ErrInvalidRequest, "cannot pass in a custom error acknowledgement with IBC v2")
+		}
 	}
 
 	data, err := types.UnmarshalPacketData(payload.Value, payload.Version, payload.Encoding)
diff --git a/modules/apps/transfer/v2/ibc_module_test.go b/modules/apps/transfer/v2/ibc_module_test.go
index 13995e855cd..bd5b02106dc 100644
--- a/modules/apps/transfer/v2/ibc_module_test.go
+++ b/modules/apps/transfer/v2/ibc_module_test.go
@@ -388,13 +388,20 @@ func (suite *TransferTestSuite) TestOnAckPacket() {
 				suite.Require().Equal(coin, chainAEscrowBalance)
 			}
 
-			// create error ack and replay the callback to check that funds are returned to sender
-			// we can replay for simplicity here since replay protection is done in the core handlers
+			// create a custom error ack and replay the callback to ensure it fails with IBC v2 callbacks
 			errAck := channeltypes.NewErrorAcknowledgement(types.ErrInvalidAmount)
 			err = cbs.OnAcknowledgementPacket(
 				ctx, suite.pathAToB.EndpointA.ClientID, suite.pathAToB.EndpointB.ClientID,
 				1, errAck.Acknowledgement(), payload, suite.chainA.SenderAccount.GetAddress(),
 			)
+			suite.Require().Error(err)
+
+			// create the sentinel error ack and replay the callback to ensure the tokens are correctly refunded
+			// we can replay the callback here because the replay protection is handled in the IBC handler
+			err = cbs.OnAcknowledgementPacket(
+				ctx, suite.pathAToB.EndpointA.ClientID, suite.pathAToB.EndpointB.ClientID,
+				1, channeltypesv2.ErrorAcknowledgement[:], payload, suite.chainA.SenderAccount.GetAddress(),
+			)
 			suite.Require().NoError(err)
 
 			// on error ack, the tokens sent in packets should be returned to sender