From 57e80c5a1064d6d07858299b133d52e1f9ef725b Mon Sep 17 00:00:00 2001
From: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com>
Date: Wed, 4 Dec 2024 19:41:15 +0100
Subject: [PATCH 1/4] add global success to acknowledgements

---
 .../transfer/v2/keeper/msg_server_test.go     |   9 +-
 .../core/04-channel/v2/keeper/msg_server.go   |  15 ++-
 .../04-channel/v2/keeper/msg_server_test.go   |   7 +-
 .../core/04-channel/v2/keeper/packet_test.go  |   2 +
 .../core/04-channel/v2/types/commitment.go    |   5 +
 .../04-channel/v2/types/commitment_test.go    |   2 +-
 modules/core/04-channel/v2/types/packet.pb.go | 120 ++++++++++++------
 modules/core/ante/ante_test.go                |   2 +-
 proto/ibc/core/channel/v2/packet.proto        |   3 +-
 9 files changed, 113 insertions(+), 52 deletions(-)

diff --git a/modules/apps/transfer/v2/keeper/msg_server_test.go b/modules/apps/transfer/v2/keeper/msg_server_test.go
index 52ec849adc1..bd21fc05b5e 100644
--- a/modules/apps/transfer/v2/keeper/msg_server_test.go
+++ b/modules/apps/transfer/v2/keeper/msg_server_test.go
@@ -190,6 +190,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacketTransfer() {
 			"failure: receive is disabled",
 			func() {},
 			func() {
+				expectedAck.RecvSuccess = false
 				expectedAck.AppAcknowledgements[0] = channeltypes.NewErrorAcknowledgement(transfertypes.ErrReceiveDisabled).Acknowledgement()
 				suite.chainB.GetSimApp().TransferKeeperV2.SetParams(suite.chainB.GetContext(),
 					transfertypes.Params{
@@ -232,7 +233,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacketTransfer() {
 
 			// by default, we assume a successful acknowledgement will be written.
 			ackBytes := channeltypes.NewResultAcknowledgement([]byte{byte(1)}).Acknowledgement()
-			expectedAck = channeltypesv2.Acknowledgement{AppAcknowledgements: [][]byte{ackBytes}}
+			expectedAck = channeltypesv2.Acknowledgement{RecvSuccess: true, AppAcknowledgements: [][]byte{ackBytes}}
 			tc.malleate()
 
 			err = path.EndpointB.MsgRecvPacket(packet)
@@ -296,6 +297,7 @@ func (suite *KeeperTestSuite) TestMsgAckPacketTransfer() {
 		{
 			"failure: proof verification failure",
 			func() {
+				expectedAck.RecvSuccess = false
 				expectedAck.AppAcknowledgements[0] = mockv2.MockFailRecvPacketResult.Acknowledgement
 			},
 			commitmenttypes.ErrInvalidProof,
@@ -304,6 +306,7 @@ func (suite *KeeperTestSuite) TestMsgAckPacketTransfer() {
 		{
 			"failure: escrowed tokens are refunded",
 			func() {
+				expectedAck.RecvSuccess = false
 				expectedAck.AppAcknowledgements[0] = channeltypes.NewErrorAcknowledgement(transfertypes.ErrReceiveDisabled).Acknowledgement()
 			},
 			nil,
@@ -353,7 +356,7 @@ func (suite *KeeperTestSuite) TestMsgAckPacketTransfer() {
 			suite.Require().NoError(err)
 
 			ackBytes := channeltypes.NewResultAcknowledgement([]byte{byte(1)}).Acknowledgement()
-			expectedAck = channeltypesv2.Acknowledgement{AppAcknowledgements: [][]byte{ackBytes}}
+			expectedAck = channeltypesv2.Acknowledgement{RecvSuccess: true, AppAcknowledgements: [][]byte{ackBytes}}
 			tc.malleate()
 
 			err = path.EndpointA.MsgAcknowledgePacket(packet, expectedAck)
@@ -505,7 +508,7 @@ func (suite *KeeperTestSuite) TestV2RetainsFungibility() {
 	}
 
 	ackBytes := channeltypes.NewResultAcknowledgement([]byte{byte(1)}).Acknowledgement()
-	successfulAck := channeltypesv2.Acknowledgement{AppAcknowledgements: [][]byte{ackBytes}}
+	successfulAck := channeltypesv2.Acknowledgement{RecvSuccess: true, AppAcknowledgements: [][]byte{ackBytes}}
 
 	originalAmount, ok := sdkmath.NewIntFromString(ibctesting.DefaultGenesisAccBalance)
 	suite.Require().True(ok)
diff --git a/modules/core/04-channel/v2/keeper/msg_server.go b/modules/core/04-channel/v2/keeper/msg_server.go
index f4c2dc446a1..d9c3b2b4af2 100644
--- a/modules/core/04-channel/v2/keeper/msg_server.go
+++ b/modules/core/04-channel/v2/keeper/msg_server.go
@@ -131,20 +131,20 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *types.MsgRecvPacket) (*typ
 
 	// build up the recv results for each application callback.
 	ack := types.Acknowledgement{
+		RecvSuccess:         true,
 		AppAcknowledgements: [][]byte{},
 	}
 
 	var isAsync bool
+	cacheCtx, writeFn = sdkCtx.CacheContext()
 	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()
 		cb := k.Router.Route(pd.DestinationPort)
 		res := cb.OnRecvPacket(cacheCtx, msg.Packet.SourceChannel, msg.Packet.DestinationChannel, msg.Packet.Sequence, pd, signer)
 
-		if res.Status != types.PacketStatus_Failure {
-			// write application state changes for asynchronous and successful acknowledgements
-			writeFn()
-		} else {
+		if res.Status == types.PacketStatus_Failure {
+			// Set RecvSuccess to false
+			ack.RecvSuccess = false
 			// Modify events in cached context to reflect unsuccessful acknowledgement
 			sdkCtx.EventManager().EmitEvents(internalerrors.ConvertToErrorEvents(cacheCtx.EventManager().Events()))
 		}
@@ -163,6 +163,11 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *types.MsgRecvPacket) (*typ
 		ack.AppAcknowledgements = append(ack.AppAcknowledgements, res.Acknowledgement)
 	}
 
+	if ack.RecvSuccess {
+		// write application state changes for successful acknowledgements
+		writeFn()
+	}
+
 	// 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-channel", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "invalid acknowledgement results"))
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 072de3e77fb..a464a538e3a 100644
--- a/modules/core/04-channel/v2/keeper/msg_server_test.go
+++ b/modules/core/04-channel/v2/keeper/msg_server_test.go
@@ -313,7 +313,10 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() {
 			tc.malleate()
 
 			// expectedAck is derived from the expected recv result.
-			expectedAck := types.Acknowledgement{AppAcknowledgements: [][]byte{expRecvRes.Acknowledgement}}
+			expectedAck := types.Acknowledgement{
+				RecvSuccess:         expRecvRes.Status == types.PacketStatus_Success,
+				AppAcknowledgements: [][]byte{expRecvRes.Acknowledgement},
+			}
 
 			// 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 {
@@ -432,7 +435,7 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() {
 			suite.Require().NoError(err)
 
 			// Construct expected acknowledgement
-			ack = types.Acknowledgement{AppAcknowledgements: [][]byte{mockv2.MockRecvPacketResult.Acknowledgement}}
+			ack = types.Acknowledgement{RecvSuccess: true, AppAcknowledgements: [][]byte{mockv2.MockRecvPacketResult.Acknowledgement}}
 
 			tc.malleate()
 
diff --git a/modules/core/04-channel/v2/keeper/packet_test.go b/modules/core/04-channel/v2/keeper/packet_test.go
index 9f4988c240a..cc64d2a60a0 100644
--- a/modules/core/04-channel/v2/keeper/packet_test.go
+++ b/modules/core/04-channel/v2/keeper/packet_test.go
@@ -307,6 +307,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
 
 			// create standard ack that can be malleated
 			ack = types.Acknowledgement{
+				RecvSuccess:         true,
 				AppAcknowledgements: [][]byte{mockv2.MockRecvPacketResult.Acknowledgement},
 			}
 
@@ -335,6 +336,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 		packet types.Packet
 		err    error
 		ack    = types.Acknowledgement{
+			RecvSuccess:         true,
 			AppAcknowledgements: [][]byte{mockv2.MockRecvPacketResult.Acknowledgement},
 		}
 		freezeClient bool
diff --git a/modules/core/04-channel/v2/types/commitment.go b/modules/core/04-channel/v2/types/commitment.go
index 3f79cc3fad1..364a31d3270 100644
--- a/modules/core/04-channel/v2/types/commitment.go
+++ b/modules/core/04-channel/v2/types/commitment.go
@@ -59,6 +59,11 @@ func CommitAcknowledgement(acknowledgement Acknowledgement) []byte {
 		buf = append(buf, hash[:]...)
 	}
 
+	if acknowledgement.RecvSuccess {
+		buf = append([]byte{byte(1)}, buf...)
+	} else {
+		buf = append([]byte{byte(0)}, buf...)
+	}
 	buf = append([]byte{byte(2)}, buf...)
 
 	hash := sha256.Sum256(buf)
diff --git a/modules/core/04-channel/v2/types/commitment_test.go b/modules/core/04-channel/v2/types/commitment_test.go
index d469a41a298..24a51f50879 100644
--- a/modules/core/04-channel/v2/types/commitment_test.go
+++ b/modules/core/04-channel/v2/types/commitment_test.go
@@ -90,5 +90,5 @@ func TestCommitAcknowledgement(t *testing.T) {
 	}
 
 	commitment := types.CommitAcknowledgement(ack)
-	require.Equal(t, "f03b4667413e56aaf086663267913e525c442b56fa1af4fa3f3dab9f37044c5b", hex.EncodeToString(commitment))
+	require.Equal(t, "47a3b131712a356465258d5a9f50340f990a37b14e665b49ea5afa170f5e7aac", hex.EncodeToString(commitment))
 }
diff --git a/modules/core/04-channel/v2/types/packet.pb.go b/modules/core/04-channel/v2/types/packet.pb.go
index cb14c1e8880..0b9f120d124 100644
--- a/modules/core/04-channel/v2/types/packet.pb.go
+++ b/modules/core/04-channel/v2/types/packet.pb.go
@@ -227,7 +227,8 @@ func (m *Payload) GetValue() []byte {
 
 // Acknowledgement contains a list of all ack results associated with a single packet.
 type Acknowledgement struct {
-	AppAcknowledgements [][]byte `protobuf:"bytes,1,rep,name=app_acknowledgements,json=appAcknowledgements,proto3" json:"app_acknowledgements,omitempty"`
+	RecvSuccess         bool     `protobuf:"varint,1,opt,name=recv_success,json=recvSuccess,proto3" json:"recv_success,omitempty"`
+	AppAcknowledgements [][]byte `protobuf:"bytes,2,rep,name=app_acknowledgements,json=appAcknowledgements,proto3" json:"app_acknowledgements,omitempty"`
 }
 
 func (m *Acknowledgement) Reset()         { *m = Acknowledgement{} }
@@ -263,6 +264,13 @@ func (m *Acknowledgement) XXX_DiscardUnknown() {
 
 var xxx_messageInfo_Acknowledgement proto.InternalMessageInfo
 
+func (m *Acknowledgement) GetRecvSuccess() bool {
+	if m != nil {
+		return m.RecvSuccess
+	}
+	return false
+}
+
 func (m *Acknowledgement) GetAppAcknowledgements() [][]byte {
 	if m != nil {
 		return m.AppAcknowledgements
@@ -336,44 +344,45 @@ func init() {
 func init() { proto.RegisterFile("ibc/core/channel/v2/packet.proto", fileDescriptor_2f814aba9ca97169) }
 
 var fileDescriptor_2f814aba9ca97169 = []byte{
-	// 587 bytes of a gzipped FileDescriptorProto
+	// 605 bytes of a gzipped FileDescriptorProto
 	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x74, 0x93, 0xc1, 0x6e, 0xd3, 0x30,
-	0x18, 0xc7, 0xeb, 0xb5, 0xdd, 0x56, 0xaf, 0x6c, 0xc1, 0x1d, 0x52, 0xa8, 0x50, 0x17, 0x2a, 0x01,
-	0x05, 0xb4, 0x04, 0x0a, 0x97, 0x49, 0x08, 0xa9, 0xcb, 0x32, 0x69, 0x02, 0x95, 0xca, 0x69, 0x91,
-	0xe0, 0x52, 0xb9, 0xae, 0x95, 0x45, 0x4b, 0xe2, 0x10, 0x3b, 0x99, 0xf6, 0x0a, 0x3b, 0xf1, 0x02,
-	0x3b, 0x70, 0xe6, 0x45, 0x76, 0xdc, 0x91, 0x13, 0x42, 0xdb, 0x89, 0xb7, 0x40, 0x75, 0xb2, 0xaa,
-	0x1b, 0x70, 0x4a, 0xbe, 0xff, 0xf7, 0xfb, 0xdb, 0xfa, 0x7f, 0xd6, 0x07, 0x0d, 0x7f, 0x42, 0x2d,
-	0xca, 0x13, 0x66, 0xd1, 0x43, 0x12, 0x45, 0x2c, 0xb0, 0xb2, 0xae, 0x15, 0x13, 0x7a, 0xc4, 0xa4,
-	0x19, 0x27, 0x5c, 0x72, 0xd4, 0xf0, 0x27, 0xd4, 0x9c, 0x11, 0x66, 0x41, 0x98, 0x59, 0xb7, 0xb9,
-	0xe9, 0x71, 0x8f, 0xab, 0xbe, 0x35, 0xfb, 0xcb, 0xd1, 0xf6, 0x6f, 0x00, 0x97, 0x07, 0xca, 0x8b,
-	0x9a, 0x70, 0x55, 0xb0, 0x2f, 0x29, 0x8b, 0x28, 0xd3, 0x81, 0x01, 0x3a, 0x15, 0x3c, 0xaf, 0xd1,
-	0x23, 0xb8, 0x2e, 0x78, 0x9a, 0x50, 0x36, 0x2e, 0x4e, 0xd4, 0x97, 0x0c, 0xd0, 0xa9, 0xe1, 0x3b,
-	0xb9, 0x6a, 0xe7, 0x22, 0xb2, 0x60, 0x63, 0xca, 0x84, 0xf4, 0x23, 0x22, 0x7d, 0x1e, 0xcd, 0xd9,
-	0xb2, 0x62, 0xd1, 0x42, 0xeb, 0xda, 0xf0, 0x1c, 0xde, 0x95, 0x7e, 0xc8, 0x78, 0x2a, 0xc7, 0xb3,
-	0xaf, 0x90, 0x24, 0x8c, 0xf5, 0x8a, 0xba, 0x5c, 0x2b, 0x1a, 0xc3, 0x6b, 0x1d, 0xbd, 0x85, 0xab,
-	0x31, 0x39, 0x09, 0x38, 0x99, 0x0a, 0xbd, 0x6a, 0x94, 0x3b, 0x6b, 0xdd, 0x07, 0xe6, 0x3f, 0x92,
-	0x9a, 0x83, 0x1c, 0xda, 0xad, 0x9c, 0xff, 0xdc, 0x2a, 0xe1, 0xb9, 0xa7, 0xfd, 0x0d, 0xc0, 0x95,
-	0xa2, 0x87, 0xb6, 0xe0, 0x5a, 0x11, 0x28, 0xe6, 0x89, 0x54, 0x79, 0x6b, 0x18, 0xe6, 0xd2, 0x80,
-	0x27, 0x12, 0x3d, 0x85, 0xda, 0x62, 0x14, 0x45, 0xe5, 0x99, 0x37, 0x16, 0x74, 0x85, 0xea, 0x70,
-	0x25, 0x63, 0x89, 0xf0, 0x79, 0x54, 0x24, 0xbd, 0x2e, 0x67, 0x23, 0x65, 0x11, 0xe5, 0x53, 0x3f,
-	0xf2, 0x54, 0xaa, 0x1a, 0x9e, 0xd7, 0x68, 0x13, 0x56, 0x33, 0x12, 0xa4, 0x4c, 0xaf, 0x1a, 0xa0,
-	0x53, 0xc7, 0x79, 0xd1, 0xde, 0x83, 0x1b, 0x3d, 0x7a, 0x14, 0xf1, 0xe3, 0x80, 0x4d, 0x3d, 0x16,
-	0xb2, 0x48, 0xa2, 0x97, 0x70, 0x93, 0xc4, 0xf1, 0x98, 0xdc, 0x94, 0x85, 0x0e, 0x8c, 0x72, 0xa7,
-	0x8e, 0x1b, 0x24, 0x8e, 0x6f, 0x39, 0x44, 0xfb, 0x18, 0x6a, 0x98, 0xd1, 0x2c, 0x7f, 0x58, 0xcc,
-	0x44, 0x1a, 0x48, 0xb4, 0x03, 0x97, 0x85, 0x24, 0x32, 0x15, 0x2a, 0xec, 0x7a, 0xf7, 0xe1, 0x7f,
-	0x66, 0x37, 0xb3, 0xb8, 0x0a, 0xc4, 0x85, 0x01, 0x75, 0xe0, 0xc6, 0xad, 0xdb, 0xd5, 0x28, 0xea,
-	0xf8, 0xb6, 0xfc, 0xec, 0x3b, 0x80, 0xf5, 0xc5, 0x23, 0xd0, 0x13, 0x78, 0x7f, 0xd0, 0xb3, 0xdf,
-	0x39, 0xc3, 0xb1, 0x3b, 0xec, 0x0d, 0x47, 0xee, 0x78, 0xd4, 0x77, 0x07, 0x8e, 0x7d, 0xb0, 0x7f,
-	0xe0, 0xec, 0x69, 0xa5, 0xe6, 0xea, 0xe9, 0x99, 0x51, 0xe9, 0x7f, 0xe8, 0x3b, 0xe8, 0x31, 0xbc,
-	0x77, 0x13, 0x74, 0x47, 0xb6, 0xed, 0xb8, 0xae, 0x06, 0x9a, 0x6b, 0xa7, 0x67, 0xc6, 0x8a, 0x9b,
-	0x52, 0xca, 0x84, 0xf8, 0x9b, 0xdb, 0xef, 0x1d, 0xbc, 0x1f, 0x61, 0x47, 0x5b, 0xca, 0xb9, 0x7d,
-	0xe2, 0x07, 0x69, 0xc2, 0x50, 0x1b, 0x36, 0x6e, 0x72, 0x3d, 0xf7, 0x53, 0xdf, 0xd6, 0xca, 0xcd,
-	0xda, 0xe9, 0x99, 0x51, 0xed, 0x89, 0x93, 0x88, 0xee, 0x7e, 0x3c, 0xbf, 0x6c, 0x81, 0x8b, 0xcb,
-	0x16, 0xf8, 0x75, 0xd9, 0x02, 0x5f, 0xaf, 0x5a, 0xa5, 0x8b, 0xab, 0x56, 0xe9, 0xc7, 0x55, 0xab,
-	0xf4, 0xf9, 0x8d, 0xe7, 0xcb, 0xc3, 0x74, 0x62, 0x52, 0x1e, 0x5a, 0x94, 0x8b, 0x90, 0x0b, 0xcb,
-	0x9f, 0xd0, 0x6d, 0x8f, 0x5b, 0xd9, 0x8e, 0x15, 0xf2, 0x69, 0x1a, 0x30, 0x91, 0xef, 0xe0, 0x8b,
-	0xd7, 0xdb, 0x0b, 0x6b, 0x28, 0x4f, 0x62, 0x26, 0x26, 0xcb, 0x6a, 0xb7, 0x5e, 0xfd, 0x09, 0x00,
-	0x00, 0xff, 0xff, 0x23, 0xd8, 0x40, 0xe5, 0xaa, 0x03, 0x00, 0x00,
+	0x18, 0xc7, 0x9b, 0xb5, 0xdd, 0x5a, 0xb7, 0x6c, 0xc1, 0x1d, 0x52, 0xa8, 0x50, 0x97, 0x55, 0x02,
+	0x0a, 0x68, 0x09, 0x14, 0x2e, 0x93, 0x10, 0x52, 0x57, 0x3a, 0x69, 0x02, 0x95, 0xca, 0x69, 0x91,
+	0xe0, 0x52, 0xb9, 0xae, 0x95, 0x45, 0x4b, 0xe3, 0x10, 0x3b, 0x99, 0xf6, 0x0a, 0x3b, 0xf1, 0x02,
+	0x3b, 0x70, 0xe6, 0x45, 0x76, 0xdc, 0x91, 0x13, 0x42, 0xdb, 0x89, 0xb7, 0x40, 0xb1, 0xd3, 0xaa,
+	0x2b, 0x70, 0x4a, 0xbe, 0xff, 0xf7, 0xfb, 0xdb, 0xf9, 0x7f, 0xd1, 0x07, 0x4c, 0x6f, 0x42, 0x6c,
+	0xc2, 0x22, 0x6a, 0x93, 0x63, 0x1c, 0x04, 0xd4, 0xb7, 0x93, 0xb6, 0x1d, 0x62, 0x72, 0x42, 0x85,
+	0x15, 0x46, 0x4c, 0x30, 0x58, 0xf3, 0x26, 0xc4, 0x4a, 0x09, 0x2b, 0x23, 0xac, 0xa4, 0x5d, 0xdf,
+	0x76, 0x99, 0xcb, 0x64, 0xdf, 0x4e, 0xdf, 0x14, 0xda, 0xfc, 0xad, 0x81, 0xf5, 0x81, 0xf4, 0xc2,
+	0x3a, 0x28, 0x71, 0xfa, 0x25, 0xa6, 0x01, 0xa1, 0x86, 0x66, 0x6a, 0xad, 0x02, 0x5a, 0xd4, 0xf0,
+	0x21, 0xd8, 0xe4, 0x2c, 0x8e, 0x08, 0x1d, 0x67, 0x27, 0x1a, 0x6b, 0xa6, 0xd6, 0x2a, 0xa3, 0x3b,
+	0x4a, 0xed, 0x2a, 0x11, 0xda, 0xa0, 0x36, 0xa5, 0x5c, 0x78, 0x01, 0x16, 0x1e, 0x0b, 0x16, 0x6c,
+	0x5e, 0xb2, 0x70, 0xa9, 0x35, 0x37, 0x3c, 0x03, 0x77, 0x85, 0x37, 0xa3, 0x2c, 0x16, 0xe3, 0xf4,
+	0xc9, 0x05, 0x9e, 0x85, 0x46, 0x41, 0x5e, 0xae, 0x67, 0x8d, 0xe1, 0x5c, 0x87, 0x6f, 0x40, 0x29,
+	0xc4, 0x67, 0x3e, 0xc3, 0x53, 0x6e, 0x14, 0xcd, 0x7c, 0xab, 0xd2, 0x7e, 0x60, 0xfd, 0x23, 0xa9,
+	0x35, 0x50, 0xd0, 0x41, 0xe1, 0xf2, 0xe7, 0x4e, 0x0e, 0x2d, 0x3c, 0xcd, 0x6f, 0x1a, 0xd8, 0xc8,
+	0x7a, 0x70, 0x07, 0x54, 0xb2, 0x40, 0x21, 0x8b, 0x84, 0xcc, 0x5b, 0x46, 0x40, 0x49, 0x03, 0x16,
+	0x09, 0xf8, 0x04, 0xe8, 0xcb, 0x51, 0x24, 0xa5, 0x32, 0x6f, 0x2d, 0xe9, 0x12, 0x35, 0xc0, 0x46,
+	0x42, 0x23, 0xee, 0xb1, 0x20, 0x4b, 0x3a, 0x2f, 0xd3, 0x91, 0xd2, 0x80, 0xb0, 0xa9, 0x17, 0xb8,
+	0x32, 0x55, 0x19, 0x2d, 0x6a, 0xb8, 0x0d, 0x8a, 0x09, 0xf6, 0x63, 0x6a, 0x14, 0x4d, 0xad, 0x55,
+	0x45, 0xaa, 0x68, 0xba, 0x60, 0xab, 0x43, 0x4e, 0x02, 0x76, 0xea, 0xd3, 0xa9, 0x4b, 0x67, 0x34,
+	0x10, 0x70, 0x17, 0x54, 0x23, 0x4a, 0x92, 0x31, 0x8f, 0x09, 0xa1, 0x9c, 0xcb, 0x6f, 0x2d, 0xa1,
+	0x4a, 0xaa, 0x39, 0x4a, 0x82, 0x2f, 0xc0, 0x36, 0x0e, 0xc3, 0x31, 0xbe, 0xed, 0xe4, 0xc6, 0x9a,
+	0x99, 0x6f, 0x55, 0x51, 0x0d, 0x87, 0xe1, 0xca, 0xa1, 0xbc, 0x79, 0x0a, 0x74, 0x44, 0x49, 0xa2,
+	0xfe, 0x3d, 0xa2, 0x3c, 0xf6, 0x05, 0xdc, 0x07, 0xeb, 0x5c, 0x60, 0x11, 0xab, 0x3b, 0x36, 0xdb,
+	0xbb, 0xff, 0x19, 0x6f, 0x6a, 0x71, 0x24, 0x88, 0x32, 0x03, 0x6c, 0x81, 0xad, 0x95, 0xdb, 0xe5,
+	0xb4, 0xaa, 0x68, 0x55, 0x7e, 0xfa, 0x5d, 0x03, 0xd5, 0xe5, 0x23, 0xe0, 0x63, 0x70, 0x7f, 0xd0,
+	0xe9, 0xbe, 0xeb, 0x0d, 0xc7, 0xce, 0xb0, 0x33, 0x1c, 0x39, 0xe3, 0x51, 0xdf, 0x19, 0xf4, 0xba,
+	0x47, 0x87, 0x47, 0xbd, 0xb7, 0x7a, 0xae, 0x5e, 0x3a, 0xbf, 0x30, 0x0b, 0xfd, 0x0f, 0xfd, 0x1e,
+	0x7c, 0x04, 0xee, 0xdd, 0x06, 0x9d, 0x51, 0xb7, 0xdb, 0x73, 0x1c, 0x5d, 0xab, 0x57, 0xce, 0x2f,
+	0xcc, 0x8d, 0xf9, 0x34, 0xfe, 0xe2, 0x0e, 0x3b, 0x47, 0xef, 0x47, 0xa8, 0xa7, 0xaf, 0x29, 0xee,
+	0x10, 0x7b, 0x7e, 0x1c, 0x51, 0xd8, 0x04, 0xb5, 0xdb, 0x5c, 0xc7, 0xf9, 0xd4, 0xef, 0xea, 0xf9,
+	0x7a, 0xf9, 0xfc, 0xc2, 0x2c, 0x76, 0xf8, 0x59, 0x40, 0x0e, 0x3e, 0x5e, 0x5e, 0x37, 0xb4, 0xab,
+	0xeb, 0x86, 0xf6, 0xeb, 0xba, 0xa1, 0x7d, 0xbd, 0x69, 0xe4, 0xae, 0x6e, 0x1a, 0xb9, 0x1f, 0x37,
+	0x8d, 0xdc, 0xe7, 0xd7, 0xae, 0x27, 0x8e, 0xe3, 0x89, 0x45, 0xd8, 0xcc, 0x26, 0x8c, 0xcf, 0x18,
+	0xb7, 0xbd, 0x09, 0xd9, 0x73, 0x99, 0x9d, 0xec, 0xdb, 0x33, 0x36, 0x8d, 0x7d, 0xca, 0xd5, 0x9a,
+	0x3e, 0x7f, 0xb5, 0xb7, 0xb4, 0xa9, 0xe2, 0x2c, 0xa4, 0x7c, 0xb2, 0x2e, 0xd7, 0xef, 0xe5, 0x9f,
+	0x00, 0x00, 0x00, 0xff, 0xff, 0x96, 0x41, 0x25, 0xe6, 0xcd, 0x03, 0x00, 0x00,
 }
 
 func (m *Packet) Marshal() (dAtA []byte, err error) {
@@ -521,9 +530,19 @@ func (m *Acknowledgement) MarshalToSizedBuffer(dAtA []byte) (int, error) {
 			copy(dAtA[i:], m.AppAcknowledgements[iNdEx])
 			i = encodeVarintPacket(dAtA, i, uint64(len(m.AppAcknowledgements[iNdEx])))
 			i--
-			dAtA[i] = 0xa
+			dAtA[i] = 0x12
 		}
 	}
+	if m.RecvSuccess {
+		i--
+		if m.RecvSuccess {
+			dAtA[i] = 1
+		} else {
+			dAtA[i] = 0
+		}
+		i--
+		dAtA[i] = 0x8
+	}
 	return len(dAtA) - i, nil
 }
 
@@ -637,6 +656,9 @@ func (m *Acknowledgement) Size() (n int) {
 	}
 	var l int
 	_ = l
+	if m.RecvSuccess {
+		n += 2
+	}
 	if len(m.AppAcknowledgements) > 0 {
 		for _, b := range m.AppAcknowledgements {
 			l = len(b)
@@ -1096,6 +1118,26 @@ func (m *Acknowledgement) Unmarshal(dAtA []byte) error {
 		}
 		switch fieldNum {
 		case 1:
+			if wireType != 0 {
+				return fmt.Errorf("proto: wrong wireType = %d for field RecvSuccess", wireType)
+			}
+			var v int
+			for shift := uint(0); ; shift += 7 {
+				if shift >= 64 {
+					return ErrIntOverflowPacket
+				}
+				if iNdEx >= l {
+					return io.ErrUnexpectedEOF
+				}
+				b := dAtA[iNdEx]
+				iNdEx++
+				v |= int(b&0x7F) << shift
+				if b < 0x80 {
+					break
+				}
+			}
+			m.RecvSuccess = bool(v != 0)
+		case 2:
 			if wireType != 2 {
 				return fmt.Errorf("proto: wrong wireType = %d for field AppAcknowledgements", wireType)
 			}
diff --git a/modules/core/ante/ante_test.go b/modules/core/ante/ante_test.go
index a7fd7062f5e..3a5cca1d85f 100644
--- a/modules/core/ante/ante_test.go
+++ b/modules/core/ante/ante_test.go
@@ -128,7 +128,7 @@ func (suite *AnteTestSuite) createAcknowledgementMessageV2(isRedundant bool) *ch
 	err = suite.path.EndpointA.MsgRecvPacket(packet)
 	suite.Require().NoError(err)
 
-	ack := channeltypesv2.Acknowledgement{AppAcknowledgements: [][]byte{mock.MockRecvPacketResult.Acknowledgement}}
+	ack := channeltypesv2.Acknowledgement{RecvSuccess: true, AppAcknowledgements: [][]byte{mock.MockRecvPacketResult.Acknowledgement}}
 	if isRedundant {
 		err = suite.path.EndpointB.MsgAcknowledgePacket(packet, ack)
 		suite.Require().NoError(err)
diff --git a/proto/ibc/core/channel/v2/packet.proto b/proto/ibc/core/channel/v2/packet.proto
index ea14112c643..a7fb2d50d7b 100644
--- a/proto/ibc/core/channel/v2/packet.proto
+++ b/proto/ibc/core/channel/v2/packet.proto
@@ -39,7 +39,8 @@ message Payload {
 
 // Acknowledgement contains a list of all ack results associated with a single packet.
 message Acknowledgement {
-  repeated bytes app_acknowledgements = 1;
+  bool           recv_success         = 1;
+  repeated bytes app_acknowledgements = 2;
 }
 
 // PacketStatus specifies the status of a RecvPacketResult.

From 6b1858998747068d884ac848d73d16ef8fef1ddd Mon Sep 17 00:00:00 2001
From: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com>
Date: Mon, 9 Dec 2024 17:05:27 +0100
Subject: [PATCH 2/4] make changes to ack callback

---
 modules/apps/transfer/ibc_module.go               |  3 ++-
 modules/apps/transfer/internal/events/events.go   |  3 ++-
 modules/apps/transfer/types/events.go             |  1 +
 modules/apps/transfer/v2/ibc_module.go            |  8 ++++----
 modules/apps/transfer/v2/keeper/keeper.go         | 15 ++++-----------
 .../apps/transfer/v2/keeper/msg_server_test.go    |  2 +-
 modules/core/04-channel/v2/keeper/msg_server.go   |  2 +-
 .../core/04-channel/v2/keeper/msg_server_test.go  |  4 ++--
 .../core/04-channel/v2/types/commitment_test.go   |  5 +++++
 modules/core/04-channel/v2/types/packet.go        |  4 ++++
 modules/core/api/module.go                        |  1 +
 testing/mock/v2/ibc_app.go                        |  2 +-
 testing/mock/v2/ibc_module.go                     |  4 ++--
 13 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go
index 95d4681b9b9..0503f890c85 100644
--- a/modules/apps/transfer/ibc_module.go
+++ b/modules/apps/transfer/ibc_module.go
@@ -230,7 +230,8 @@ func (im IBCModule) OnAcknowledgementPacket(
 		return err
 	}
 
-	events.EmitOnAcknowledgementPacketEvent(ctx, data, ack)
+	recvSuccess := ack.Success()
+	events.EmitOnAcknowledgementPacketEvent(ctx, data, recvSuccess, ack)
 
 	return nil
 }
diff --git a/modules/apps/transfer/internal/events/events.go b/modules/apps/transfer/internal/events/events.go
index b6e48282e3c..d72a0b94e3b 100644
--- a/modules/apps/transfer/internal/events/events.go
+++ b/modules/apps/transfer/internal/events/events.go
@@ -66,7 +66,7 @@ func EmitOnRecvPacketEvent(ctx context.Context, packetData types.FungibleTokenPa
 }
 
 // EmitOnAcknowledgementPacketEvent emits a fungible token packet event in the OnAcknowledgementPacket callback
-func EmitOnAcknowledgementPacketEvent(ctx context.Context, packetData types.FungibleTokenPacketDataV2, ack channeltypes.Acknowledgement) {
+func EmitOnAcknowledgementPacketEvent(ctx context.Context, packetData types.FungibleTokenPacketDataV2, recvSuccess bool, ack channeltypes.Acknowledgement) {
 	tokensStr := mustMarshalJSON(packetData.Tokens)
 	forwardingHopsStr := mustMarshalJSON(packetData.Forwarding.Hops)
 	sdkCtx := sdk.UnwrapSDKContext(ctx) // TODO: https://github.com/cosmos/ibc-go/issues/5917
@@ -78,6 +78,7 @@ func EmitOnAcknowledgementPacketEvent(ctx context.Context, packetData types.Fung
 			sdk.NewAttribute(types.AttributeKeyTokens, tokensStr),
 			sdk.NewAttribute(types.AttributeKeyMemo, packetData.Memo),
 			sdk.NewAttribute(types.AttributeKeyForwardingHops, forwardingHopsStr),
+			sdk.NewAttribute(types.AttributeKeyRecvSuccess, strconv.FormatBool(recvSuccess)),
 			sdk.NewAttribute(types.AttributeKeyAck, ack.String()),
 		),
 		sdk.NewEvent(
diff --git a/modules/apps/transfer/types/events.go b/modules/apps/transfer/types/events.go
index f5ad7210431..064a45459e4 100644
--- a/modules/apps/transfer/types/events.go
+++ b/modules/apps/transfer/types/events.go
@@ -16,6 +16,7 @@ const (
 	AttributeKeyRefundReceiver = "refund_receiver"
 	AttributeKeyRefundTokens   = "refund_tokens"
 	AttributeKeyAckSuccess     = "success"
+	AttributeKeyRecvSuccess    = "recv_success"
 	AttributeKeyAck            = "acknowledgement"
 	AttributeKeyAckError       = "error"
 	AttributeKeyMemo           = "memo"
diff --git a/modules/apps/transfer/v2/ibc_module.go b/modules/apps/transfer/v2/ibc_module.go
index 19aa3db5292..fb5816ea420 100644
--- a/modules/apps/transfer/v2/ibc_module.go
+++ b/modules/apps/transfer/v2/ibc_module.go
@@ -113,9 +113,9 @@ func (im *IBCModule) OnTimeoutPacket(ctx context.Context, sourceChannel string,
 	return nil
 }
 
-func (im *IBCModule) OnAcknowledgementPacket(ctx context.Context, sourceChannel string, destinationChannel string, sequence uint64, acknowledgement []byte, payload types.Payload, relayer sdk.AccAddress) error {
+func (im *IBCModule) OnAcknowledgementPacket(ctx context.Context, sourceChannel string, destinationChannel string, sequence uint64, recvSuccess bool, acknowledgement []byte, payload types.Payload, relayer sdk.AccAddress) error {
 	var ack channeltypes.Acknowledgement
-	if err := transfertypes.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
+	if err := transfertypes.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil && recvSuccess {
 		return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err)
 	}
 
@@ -124,10 +124,10 @@ func (im *IBCModule) OnAcknowledgementPacket(ctx context.Context, sourceChannel
 		return err
 	}
 
-	if err := im.keeper.OnAcknowledgementPacket(ctx, payload.SourcePort, sourceChannel, data, ack); err != nil {
+	if err := im.keeper.OnAcknowledgementPacket(ctx, payload.SourcePort, sourceChannel, data, recvSuccess); err != nil {
 		return err
 	}
 
-	events.EmitOnAcknowledgementPacketEvent(ctx, data, ack)
+	events.EmitOnAcknowledgementPacketEvent(ctx, data, recvSuccess, ack)
 	return nil
 }
diff --git a/modules/apps/transfer/v2/keeper/keeper.go b/modules/apps/transfer/v2/keeper/keeper.go
index 6cf535b0cac..dd6c8cd546c 100644
--- a/modules/apps/transfer/v2/keeper/keeper.go
+++ b/modules/apps/transfer/v2/keeper/keeper.go
@@ -12,7 +12,6 @@ import (
 	"github.com/cosmos/ibc-go/v9/modules/apps/transfer/internal/events"
 	transferkeeper "github.com/cosmos/ibc-go/v9/modules/apps/transfer/keeper"
 	"github.com/cosmos/ibc-go/v9/modules/apps/transfer/types"
-	channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
 	channelkeeperv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/keeper"
 	channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types"
 	ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
@@ -186,18 +185,12 @@ func (k *Keeper) OnRecvPacket(ctx context.Context, sourceChannel, destChannel st
 	return nil
 }
 
-func (k *Keeper) OnAcknowledgementPacket(ctx context.Context, sourcePort, sourceChannel string, data types.FungibleTokenPacketDataV2, ack channeltypes.Acknowledgement) error {
-	switch ack.Response.(type) {
-	case *channeltypes.Acknowledgement_Result:
-		// the acknowledgement succeeded on the receiving chain so nothing
-		// needs to be executed and no error needs to be returned
-		return nil
-	case *channeltypes.Acknowledgement_Error:
-		// We refund the tokens from the escrow address to the sender
+func (k *Keeper) OnAcknowledgementPacket(ctx context.Context, sourcePort, sourceChannel string, data types.FungibleTokenPacketDataV2, recvSuccess bool) error {
+	if !recvSuccess {
+		// refund the tokens back to the sender
 		return k.refundPacketTokens(ctx, sourcePort, sourceChannel, data)
-	default:
-		return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected one of [%T, %T], got %T", channeltypes.Acknowledgement_Result{}, channeltypes.Acknowledgement_Error{}, ack.Response)
 	}
+	return nil
 }
 
 func (k *Keeper) OnTimeoutPacket(ctx context.Context, sourcePort, sourceChannel string, data types.FungibleTokenPacketDataV2) error {
diff --git a/modules/apps/transfer/v2/keeper/msg_server_test.go b/modules/apps/transfer/v2/keeper/msg_server_test.go
index bd21fc05b5e..13ce067b3f9 100644
--- a/modules/apps/transfer/v2/keeper/msg_server_test.go
+++ b/modules/apps/transfer/v2/keeper/msg_server_test.go
@@ -365,7 +365,7 @@ func (suite *KeeperTestSuite) TestMsgAckPacketTransfer() {
 			if expPass {
 				suite.Require().NoError(err)
 
-				if bytes.Equal(expectedAck.AppAcknowledgements[0], ackBytes) {
+				if expectedAck.RecvSuccess {
 					// tokens remain escrowed
 					for _, t := range tokens {
 						escrowedAmount := suite.chainA.GetSimApp().TransferKeeperV2.GetTotalEscrowForDenom(suite.chainA.GetContext(), t.Denom.IBCDenom())
diff --git a/modules/core/04-channel/v2/keeper/msg_server.go b/modules/core/04-channel/v2/keeper/msg_server.go
index d9c3b2b4af2..825241e05cd 100644
--- a/modules/core/04-channel/v2/keeper/msg_server.go
+++ b/modules/core/04-channel/v2/keeper/msg_server.go
@@ -217,7 +217,7 @@ func (k *Keeper) Acknowledgement(ctx context.Context, msg *types.MsgAcknowledgem
 	for i, pd := range msg.Packet.Payloads {
 		cbs := k.Router.Route(pd.SourcePort)
 		ack := msg.Acknowledgement.AppAcknowledgements[i]
-		err := cbs.OnAcknowledgementPacket(ctx, msg.Packet.SourceChannel, msg.Packet.DestinationChannel, msg.Packet.Sequence, ack, pd, relayer)
+		err := cbs.OnAcknowledgementPacket(ctx, msg.Packet.SourceChannel, msg.Packet.DestinationChannel, msg.Packet.Sequence, msg.Acknowledgement.RecvSuccess, ack, pd, relayer)
 		if err != nil {
 			return nil, errorsmod.Wrapf(err, "failed OnAcknowledgementPacket for source port %s, source channel %s, destination channel %s", pd.SourcePort, msg.Packet.SourceChannel, msg.Packet.DestinationChannel)
 		}
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 a464a538e3a..26f399c6e45 100644
--- a/modules/core/04-channel/v2/keeper/msg_server_test.go
+++ b/modules/core/04-channel/v2/keeper/msg_server_test.go
@@ -380,7 +380,7 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() {
 
 				// Modify the callback to return an error.
 				// This way, we can verify that the callback is not executed in a No-op case.
-				path.EndpointA.Chain.GetSimApp().MockModuleV2A.IBCApp.OnAcknowledgementPacket = func(context.Context, string, string, uint64, types.Payload, []byte, sdk.AccAddress) error {
+				path.EndpointA.Chain.GetSimApp().MockModuleV2A.IBCApp.OnAcknowledgementPacket = func(context.Context, string, string, uint64, types.Payload, bool, []byte, sdk.AccAddress) error {
 					return mock.MockApplicationCallbackError
 				}
 			},
@@ -388,7 +388,7 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() {
 		{
 			name: "failure: callback fails",
 			malleate: func() {
-				path.EndpointA.Chain.GetSimApp().MockModuleV2A.IBCApp.OnAcknowledgementPacket = func(context.Context, string, string, uint64, types.Payload, []byte, sdk.AccAddress) error {
+				path.EndpointA.Chain.GetSimApp().MockModuleV2A.IBCApp.OnAcknowledgementPacket = func(context.Context, string, string, uint64, types.Payload, bool, []byte, sdk.AccAddress) error {
 					return mock.MockApplicationCallbackError
 				}
 			},
diff --git a/modules/core/04-channel/v2/types/commitment_test.go b/modules/core/04-channel/v2/types/commitment_test.go
index 24a51f50879..53ab440c3f1 100644
--- a/modules/core/04-channel/v2/types/commitment_test.go
+++ b/modules/core/04-channel/v2/types/commitment_test.go
@@ -84,11 +84,16 @@ func TestCommitPacket(t *testing.T) {
 // same commitment output. But it is also useful to catch any changes in the commitment.
 func TestCommitAcknowledgement(t *testing.T) {
 	ack := types.Acknowledgement{
+		RecvSuccess: true,
 		AppAcknowledgements: [][]byte{
 			[]byte("some bytes"),
 		},
 	}
 
 	commitment := types.CommitAcknowledgement(ack)
+	require.Equal(t, "fc02a4453c297c9b65189ec354f4fc7f0c1327b72f6044a20d4dd1fac8fda9f7", hex.EncodeToString(commitment))
+
+	ack.RecvSuccess = false
+	commitment = types.CommitAcknowledgement(ack)
 	require.Equal(t, "47a3b131712a356465258d5a9f50340f990a37b14e665b49ea5afa170f5e7aac", hex.EncodeToString(commitment))
 }
diff --git a/modules/core/04-channel/v2/types/packet.go b/modules/core/04-channel/v2/types/packet.go
index 1ba7722b925..aaad88762a7 100644
--- a/modules/core/04-channel/v2/types/packet.go
+++ b/modules/core/04-channel/v2/types/packet.go
@@ -9,6 +9,10 @@ import (
 	host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
 )
 
+var (
+	SentinelAcknowledgement = []byte{byte(1)}
+)
+
 // NewPacket constructs a new packet.
 func NewPacket(sequence uint64, sourceChannel, destinationChannel string, timeoutTimestamp uint64, payloads ...Payload) Packet {
 	return Packet{
diff --git a/modules/core/api/module.go b/modules/core/api/module.go
index 4d26947f42a..58d08b863f7 100644
--- a/modules/core/api/module.go
+++ b/modules/core/api/module.go
@@ -48,6 +48,7 @@ type IBCModule interface {
 		sourceChannel string,
 		destinationChannel string,
 		sequence uint64,
+		recvSuccess bool,
 		acknowledgement []byte,
 		payload channeltypesv2.Payload,
 		relayer sdk.AccAddress,
diff --git a/testing/mock/v2/ibc_app.go b/testing/mock/v2/ibc_app.go
index 7b72f4bac30..f9ac3847e0c 100644
--- a/testing/mock/v2/ibc_app.go
+++ b/testing/mock/v2/ibc_app.go
@@ -12,5 +12,5 @@ type IBCApp struct {
 	OnSendPacket            func(goCtx context.Context, sourceChannel string, destinationChannel string, sequence uint64, payload channeltypesv2.Payload, signer sdk.AccAddress) error
 	OnRecvPacket            func(goCtx context.Context, sourceChannel string, destinationChannel string, sequence uint64, payload channeltypesv2.Payload, relayer sdk.AccAddress) channeltypesv2.RecvPacketResult
 	OnTimeoutPacket         func(goCtx context.Context, sourceChannel string, destinationChannel string, sequence uint64, payload channeltypesv2.Payload, relayer sdk.AccAddress) error
-	OnAcknowledgementPacket func(goCtx context.Context, sourceChannel string, destinationChannel string, sequence uint64, payload channeltypesv2.Payload, acknowledgement []byte, relayer sdk.AccAddress) error
+	OnAcknowledgementPacket func(goCtx context.Context, sourceChannel string, destinationChannel string, sequence uint64, payload channeltypesv2.Payload, recvSuccess bool, acknowledgement []byte, relayer sdk.AccAddress) error
 }
diff --git a/testing/mock/v2/ibc_module.go b/testing/mock/v2/ibc_module.go
index 8ea9fad704a..ff849b1aa9f 100644
--- a/testing/mock/v2/ibc_module.go
+++ b/testing/mock/v2/ibc_module.go
@@ -45,9 +45,9 @@ func (im IBCModule) OnRecvPacket(ctx context.Context, sourceChannel string, dest
 	return MockRecvPacketResult
 }
 
-func (im IBCModule) OnAcknowledgementPacket(ctx context.Context, sourceChannel string, destinationChannel string, sequence uint64, acknowledgement []byte, payload channeltypesv2.Payload, relayer sdk.AccAddress) error {
+func (im IBCModule) OnAcknowledgementPacket(ctx context.Context, sourceChannel string, destinationChannel string, sequence uint64, recvSuccess bool, acknowledgement []byte, payload channeltypesv2.Payload, relayer sdk.AccAddress) error {
 	if im.IBCApp.OnAcknowledgementPacket != nil {
-		return im.IBCApp.OnAcknowledgementPacket(ctx, sourceChannel, destinationChannel, sequence, payload, acknowledgement, relayer)
+		return im.IBCApp.OnAcknowledgementPacket(ctx, sourceChannel, destinationChannel, sequence, payload, recvSuccess, acknowledgement, relayer)
 	}
 	return nil
 }

From 0468e3c529a2209cc6a3815666eea064dff3c23f Mon Sep 17 00:00:00 2001
From: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com>
Date: Mon, 9 Dec 2024 17:12:09 +0100
Subject: [PATCH 3/4] remove unnecessary code

---
 modules/apps/transfer/v2/ibc_module.go     | 2 +-
 modules/core/04-channel/v2/types/packet.go | 4 ----
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/modules/apps/transfer/v2/ibc_module.go b/modules/apps/transfer/v2/ibc_module.go
index fb5816ea420..d1c5014e408 100644
--- a/modules/apps/transfer/v2/ibc_module.go
+++ b/modules/apps/transfer/v2/ibc_module.go
@@ -115,7 +115,7 @@ func (im *IBCModule) OnTimeoutPacket(ctx context.Context, sourceChannel string,
 
 func (im *IBCModule) OnAcknowledgementPacket(ctx context.Context, sourceChannel string, destinationChannel string, sequence uint64, recvSuccess bool, acknowledgement []byte, payload types.Payload, relayer sdk.AccAddress) error {
 	var ack channeltypes.Acknowledgement
-	if err := transfertypes.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil && recvSuccess {
+	if err := transfertypes.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
 		return errorsmod.Wrapf(ibcerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet acknowledgement: %v", err)
 	}
 
diff --git a/modules/core/04-channel/v2/types/packet.go b/modules/core/04-channel/v2/types/packet.go
index aaad88762a7..1ba7722b925 100644
--- a/modules/core/04-channel/v2/types/packet.go
+++ b/modules/core/04-channel/v2/types/packet.go
@@ -9,10 +9,6 @@ import (
 	host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
 )
 
-var (
-	SentinelAcknowledgement = []byte{byte(1)}
-)
-
 // NewPacket constructs a new packet.
 func NewPacket(sequence uint64, sourceChannel, destinationChannel string, timeoutTimestamp uint64, payloads ...Payload) Packet {
 	return Packet{

From 4418c78896e6ab8688e45784a35020a0d2873561 Mon Sep 17 00:00:00 2001
From: Aditya Sripal <14364734+AdityaSripal@users.noreply.github.com>
Date: Mon, 9 Dec 2024 17:37:27 +0100
Subject: [PATCH 4/4] ack validation

---
 .../core/04-channel/v2/keeper/msg_server.go   | 15 ++--
 modules/core/04-channel/v2/types/msgs.go      |  6 +-
 modules/core/04-channel/v2/types/msgs_test.go |  9 ++-
 modules/core/04-channel/v2/types/packet.go    | 21 ++++++
 .../core/04-channel/v2/types/packet_test.go   | 74 +++++++++++++++++++
 5 files changed, 117 insertions(+), 8 deletions(-)

diff --git a/modules/core/04-channel/v2/keeper/msg_server.go b/modules/core/04-channel/v2/keeper/msg_server.go
index 825241e05cd..ac812f3af28 100644
--- a/modules/core/04-channel/v2/keeper/msg_server.go
+++ b/modules/core/04-channel/v2/keeper/msg_server.go
@@ -157,6 +157,9 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *types.MsgRecvPacket) (*typ
 			if len(msg.Packet.Payloads) > 1 {
 				return nil, errorsmod.Wrapf(types.ErrInvalidPacket, "packet with multiple payloads cannot have async acknowledgement")
 			}
+			// break out of the loop if the acknowledgement is async
+			// to prevent the addition of nil to the acknowledgement
+			break
 		}
 
 		// append app acknowledgement to the overall acknowledgement
@@ -168,13 +171,13 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *types.MsgRecvPacket) (*typ
 		writeFn()
 	}
 
-	// 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-channel", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "invalid acknowledgement results"))
-		return &types.MsgRecvPacketResponse{Result: types.FAILURE}, errorsmod.Wrapf(err, "receive packet failed source-channel %s invalid acknowledgement results", msg.Packet.SourceChannel)
-	}
-
 	if !isAsync {
+		// 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-channel", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "invalid acknowledgement results"))
+			return &types.MsgRecvPacketResponse{Result: types.FAILURE}, errorsmod.Wrapf(err, "receive packet failed source-channel %s invalid acknowledgement results", msg.Packet.SourceChannel)
+		}
+
 		// Set packet acknowledgement only if the acknowledgement is not async.
 		// NOTE: IBC applications modules may call the WriteAcknowledgement asynchronously if the
 		// acknowledgement is async.
diff --git a/modules/core/04-channel/v2/types/msgs.go b/modules/core/04-channel/v2/types/msgs.go
index b0a8d9cd9de..c0b7772de83 100644
--- a/modules/core/04-channel/v2/types/msgs.go
+++ b/modules/core/04-channel/v2/types/msgs.go
@@ -174,7 +174,11 @@ func (msg *MsgAcknowledgement) ValidateBasic() error {
 		return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
 	}
 
-	return msg.Packet.ValidateBasic()
+	if err := msg.Packet.ValidateBasic(); err != nil {
+		return err
+	}
+
+	return msg.Acknowledgement.ValidateBasic()
 }
 
 // NewMsgTimeout creates a new MsgTimeout instance
diff --git a/modules/core/04-channel/v2/types/msgs_test.go b/modules/core/04-channel/v2/types/msgs_test.go
index 35165f0d72e..4a0a7d2a3be 100644
--- a/modules/core/04-channel/v2/types/msgs_test.go
+++ b/modules/core/04-channel/v2/types/msgs_test.go
@@ -322,12 +322,19 @@ func (s *TypesTestSuite) TestMsgAcknowledge_ValidateBasic() {
 			},
 			expError: types.ErrInvalidPacket,
 		},
+		{
+			name: "failure: invalid acknowledgement",
+			malleate: func() {
+				msg.Acknowledgement = types.Acknowledgement{}
+			},
+			expError: types.ErrInvalidAcknowledgement,
+		},
 	}
 	for _, tc := range testCases {
 		s.Run(tc.name, func() {
 			msg = types.NewMsgAcknowledgement(
 				types.NewPacket(1, ibctesting.FirstChannelID, ibctesting.SecondChannelID, s.chainA.GetTimeoutTimestamp(), mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB)),
-				types.Acknowledgement{},
+				types.Acknowledgement{RecvSuccess: true, AppAcknowledgements: [][]byte{[]byte("ack")}},
 				testProof,
 				clienttypes.ZeroHeight(),
 				s.chainA.SenderAccount.GetAddress().String(),
diff --git a/modules/core/04-channel/v2/types/packet.go b/modules/core/04-channel/v2/types/packet.go
index 1ba7722b925..93ee51f51dd 100644
--- a/modules/core/04-channel/v2/types/packet.go
+++ b/modules/core/04-channel/v2/types/packet.go
@@ -84,3 +84,24 @@ func (p Payload) ValidateBasic() error {
 func TimeoutTimestampToNanos(seconds uint64) uint64 {
 	return uint64(time.Unix(int64(seconds), 0).UnixNano())
 }
+
+// NewAcknowledgement creates a new Acknowledgement instance
+func NewAcknowledgement(recvSuccess bool, appAcknowledgements [][]byte) Acknowledgement {
+	return Acknowledgement{
+		RecvSuccess:         recvSuccess,
+		AppAcknowledgements: appAcknowledgements,
+	}
+}
+
+// ValidateBasic validates the acknowledgment
+func (a Acknowledgement) ValidateBasic() error {
+	if len(a.GetAppAcknowledgements()) == 0 {
+		return errorsmod.Wrap(ErrInvalidAcknowledgement, "acknowledgement cannot be empty")
+	}
+	for _, ack := range a.GetAppAcknowledgements() {
+		if len(ack) == 0 {
+			return errorsmod.Wrap(ErrInvalidAcknowledgement, "app acknowledgement cannot be empty")
+		}
+	}
+	return nil
+}
diff --git a/modules/core/04-channel/v2/types/packet_test.go b/modules/core/04-channel/v2/types/packet_test.go
index 4952e03671d..f134d248cbb 100644
--- a/modules/core/04-channel/v2/types/packet_test.go
+++ b/modules/core/04-channel/v2/types/packet_test.go
@@ -125,3 +125,77 @@ func TestValidateBasic(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateBasicAcknowledgment(t *testing.T) {
+	var ack types.Acknowledgement
+	testCases := []struct {
+		name     string
+		malleate func()
+		expErr   error
+	}{
+		{
+			"success",
+			func() {
+				ack = types.Acknowledgement{
+					RecvSuccess: true,
+					AppAcknowledgements: [][]byte{
+						[]byte("some bytes"),
+						[]byte("some bytes 2"),
+					},
+				}
+			},
+			nil,
+		},
+		{
+			"success with failed receive",
+			func() {
+				ack = types.Acknowledgement{
+					RecvSuccess: false,
+					AppAcknowledgements: [][]byte{
+						[]byte("some bytes"),
+						[]byte("some bytes 2"),
+					},
+				}
+			},
+			nil,
+		},
+		{
+			"failure: empty ack",
+			func() {
+				ack = types.Acknowledgement{}
+			},
+			types.ErrInvalidAcknowledgement,
+		},
+		{
+			"failure: empty success ack",
+			func() {
+				ack = types.Acknowledgement{RecvSuccess: true}
+			},
+			types.ErrInvalidAcknowledgement,
+		},
+		{
+			"failure: empty app ack",
+			func() {
+				ack = types.Acknowledgement{
+					RecvSuccess: false,
+					AppAcknowledgements: [][]byte{
+						[]byte(""), // empty app ack
+					},
+				}
+			},
+			types.ErrInvalidAcknowledgement,
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			tc.malleate()
+
+			err := ack.ValidateBasic()
+			if tc.expErr == nil {
+				require.NoError(t, err)
+			} else {
+				require.ErrorIs(t, err, tc.expErr)
+			}
+		})
+	}
+}