fix:ics29: WriteAck update + adding success bool to IncentivizedAck (#…

…952)

* fix: updating WriteAck & adding Success boolean to IncentivizedAcknowledgement

* feat: adding check of is fee enabled

* nit: change successful to underlying_application_success

* test: adding seperate test for fee disabled write async

* Update modules/apps/29-fee/ibc_module_test.go

Co-authored-by: Aditya <adityasripal@gmail.com>

* test: adding check to compare hash of acks

* fix: var name

Co-authored-by: Aditya <adityasripal@gmail.com>

docs/ibc/proto-docs.md

@@ -341,6 +341,7 @@ It contains the raw acknowledgement bytes, as well as the forward relayer addres
 | ----- | ---- | ----- | ----------- |
 | `result` | [bytes](#bytes) |  |  |
 | `forward_relayer_address` | [string](#string) |  |  |
+| `underlying_app_success` | [bool](#bool) |  |  |
 
 
 

modules/apps/29-fee/ibc_module.go

@@ -205,7 +205,7 @@ func (im IBCModule) OnRecvPacket(
 	// if forwardRelayer is not found we refund recv_fee
 	forwardRelayer, _ := im.keeper.GetCounterpartyAddress(ctx, relayer.String())
 
-	return types.NewIncentivizedAcknowledgement(forwardRelayer, ack.Acknowledgement())
+	return types.NewIncentivizedAcknowledgement(forwardRelayer, ack.Acknowledgement(), ack.Success())
 }
 
 // OnAcknowledgementPacket implements the IBCModule interface

modules/apps/29-fee/ibc_module_test.go

@@ -10,6 +10,7 @@ import (
 	transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
 	channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
 	host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
+	"github.com/cosmos/ibc-go/v3/modules/core/exported"
 	ibctesting "github.com/cosmos/ibc-go/v3/testing"
 	ibcmock "github.com/cosmos/ibc-go/v3/testing/mock"
 )
@@ -461,11 +462,18 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
 			true,
 		},
 		{
-			"source relayer is empty string",
+			"async write acknowledgement: ack is nil",
 			func() {
-				suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), "")
+				// setup mock callback
+				suite.chainB.GetSimApp().FeeMockModule.IBCApp.OnRecvPacket = func(
+					ctx sdk.Context,
+					packet channeltypes.Packet,
+					relayer sdk.AccAddress,
+				) exported.Acknowledgement {
+					return nil
+				}
 			},
-			false,
+			true,
 			true,
 		},
 		{
@@ -476,6 +484,14 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
 			true,
 			false,
 		},
+		{
+			"forward address is not found",
+			func() {
+				suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), "")
+			},
+			false,
+			true,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -509,19 +525,16 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
 			case !tc.feeEnabled:
 				suite.Require().Equal(ibcmock.MockAcknowledgement, result)
 
-			case tc.forwardRelayer:
-				ack := types.IncentivizedAcknowledgement{
-					Result:                ibcmock.MockAcknowledgement.Acknowledgement(),
-					ForwardRelayerAddress: suite.chainB.SenderAccount.GetAddress().String(),
-				}
-				suite.Require().Equal(ack, result)
+			case tc.forwardRelayer && result == nil:
+				suite.Require().Equal(nil, result)
 
 			case !tc.forwardRelayer:
-				ack := types.IncentivizedAcknowledgement{
+				expectedAck := types.IncentivizedAcknowledgement{
 					Result:                ibcmock.MockAcknowledgement.Acknowledgement(),
 					ForwardRelayerAddress: "",
+					UnderlyingAppSuccess:  true,
 				}
-				suite.Require().Equal(ack, result)
+				suite.Require().Equal(expectedAck, result)
 			}
 		})
 	}

modules/apps/29-fee/keeper/relay.go

@@ -17,7 +17,12 @@ func (k Keeper) SendPacket(ctx sdk.Context, chanCap *capabilitytypes.Capability,
 
 // WriteAcknowledgement wraps IBC ChannelKeeper's WriteAcknowledgement function
 // ICS29 WriteAcknowledgement is used for asynchronous acknowledgements
-func (k Keeper) WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, acknowledgement []byte) error {
+func (k Keeper) WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.Capability, packet ibcexported.PacketI, acknowledgement ibcexported.Acknowledgement) error {
+	if !k.IsFeeEnabled(ctx, packet.GetDestPort(), packet.GetDestChannel()) {
+		// ics4Wrapper may be core IBC or higher-level middleware
+		return k.ics4Wrapper.WriteAcknowledgement(ctx, chanCap, packet, acknowledgement)
+	}
+
 	// retrieve the forward relayer that was stored in `onRecvPacket`
 	packetId := channeltypes.NewPacketId(packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence())
 
@@ -32,7 +37,7 @@ func (k Keeper) WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.C
 
 	k.DeleteForwardRelayerAddress(ctx, packetId)
 
-	ack := types.NewIncentivizedAcknowledgement(forwardRelayer, acknowledgement)
+	ack := types.NewIncentivizedAcknowledgement(forwardRelayer, acknowledgement.Acknowledgement(), acknowledgement.Success())
 
 	// ics4Wrapper may be core IBC or higher-level middleware
 	return k.ics4Wrapper.WriteAcknowledgement(ctx, chanCap, packet, ack)

modules/apps/29-fee/keeper/relay_test.go

@@ -47,7 +47,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
 				timeoutTimestamp,
 			)
 
-			ack := []byte("ack")
+			ack := channeltypes.NewResultAcknowledgement([]byte("success"))
 			chanCap := suite.chainB.GetChannelCapability(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID)
 
 			// malleate test case
@@ -65,3 +65,31 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
 		})
 	}
 }
+
+func (suite *KeeperTestSuite) TestWriteAcknowledgementAsyncFeeDisabled() {
+	// open incentivized channel
+	suite.coordinator.Setup(suite.path)
+	suite.chainB.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainB.GetContext(), suite.path.EndpointB.ChannelConfig.PortID, "channel-0")
+
+	// build packet
+	timeoutTimestamp := ^uint64(0)
+	packet := channeltypes.NewPacket(
+		[]byte("packetData"),
+		1,
+		suite.path.EndpointA.ChannelConfig.PortID,
+		suite.path.EndpointA.ChannelID,
+		suite.path.EndpointB.ChannelConfig.PortID,
+		suite.path.EndpointB.ChannelID,
+		clienttypes.ZeroHeight(),
+		timeoutTimestamp,
+	)
+
+	ack := channeltypes.NewResultAcknowledgement([]byte("success"))
+	chanCap := suite.chainB.GetChannelCapability(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID)
+
+	err := suite.chainB.GetSimApp().IBCFeeKeeper.WriteAcknowledgement(suite.chainB.GetContext(), chanCap, packet, ack)
+	suite.Require().NoError(err)
+
+	packetAck, _ := suite.chainB.GetSimApp().GetIBCKeeper().ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), packet.DestinationPort, packet.DestinationChannel, 1)
+	suite.Require().Equal(packetAck, channeltypes.CommitAcknowledgement(ack.Acknowledgement()))
+}

modules/apps/29-fee/types/ack.go

@@ -5,18 +5,19 @@ import (
 )
 
 // NewIncentivizedAcknowledgement creates a new instance of IncentivizedAcknowledgement
-func NewIncentivizedAcknowledgement(relayer string, ack []byte) IncentivizedAcknowledgement {
+func NewIncentivizedAcknowledgement(relayer string, ack []byte, success bool) IncentivizedAcknowledgement {
 	return IncentivizedAcknowledgement{
 		Result:                ack,
 		ForwardRelayerAddress: relayer,
+		UnderlyingAppSuccess:  success,
 	}
 }
 
 // Success implements the Acknowledgement interface. The acknowledgement is
 // considered successful if the forward relayer address is empty. Otherwise it is
 // considered a failed acknowledgement.
 func (ack IncentivizedAcknowledgement) Success() bool {
-	return ack.ForwardRelayerAddress != ""
+	return ack.UnderlyingAppSuccess
 }
 
 // Acknowledgement implements the Acknowledgement interface. It returns the

proto/ibc/applications/fee/v1/ack.proto

@@ -11,4 +11,5 @@ import "gogoproto/gogo.proto";
 message IncentivizedAcknowledgement {
   bytes  result                  = 1;
   string forward_relayer_address = 2 [(gogoproto.moretags) = "yaml:\"forward_relayer_address\""];
+  bool underlying_app_success = 3 [(gogoproto.moretags) = "yaml:\"underlying_app_successl\""];
 }