Create tests for constant error acknowledgement

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)

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

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,20 +338,23 @@ 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,
 		},
 		{
 			name: "failure: invalid commitment",
 			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,
 		},
 		{
 			name: "failure: failed membership verification",
 			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)

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,
 		},

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 {

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")),

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))
 }

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 {

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,
+	}
+}