Merge branch 'feat/ibc-eureka' into gjermund/7707-handle-telemetry-in…

…-eureka

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

modules/apps/transfer/v2/ibc_module.go

@@ -1,6 +1,7 @@
 package v2
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"strings"
@@ -88,8 +89,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 (
@@ -112,11 +112,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,
 		}
 	}
 
@@ -128,11 +126,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)

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,35 +176,35 @@ 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",
 			[]string{sdk.DefaultBondDenom},
 			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",
 			[]string{sdk.DefaultBondDenom},
 			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)
 			}
 		})
 	}

modules/core/04-channel/v2/keeper/msg_server.go

@@ -1,6 +1,7 @@
 package keeper
 
 import (
+	"bytes"
 	"context"
 	"time"
 
@@ -89,18 +90,31 @@ 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()
 		cb := k.Router.Route(pd.DestinationPort)
 		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)

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 {

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}

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