fix: ics29: switch source with destintion for chan/port IDs (#961)

* fix: switch source with destintion for chan/port IDs

* fix: blunder

* test: adding tests in case of incorrect channel/port id

* test: moving check to WriteAcknowledgement

* add test case for Get/Set counterparty address

* nit: path name

* Update modules/apps/29-fee/keeper/msg_server_test.go

modules/apps/29-fee/fee_test.go

@@ -19,14 +19,17 @@ type FeeTestSuite struct {
 
 	chainA *ibctesting.TestChain
 	chainB *ibctesting.TestChain
+	chainC *ibctesting.TestChain
 
-	path *ibctesting.Path
+	path  *ibctesting.Path
+	pathAToC *ibctesting.Path
 }
 
 func (suite *FeeTestSuite) SetupTest() {
-	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)
+	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 3)
 	suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
 	suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))
+	suite.chainC = suite.coordinator.GetChain(ibctesting.GetChainID(3))
 
 	path := ibctesting.NewPath(suite.chainA, suite.chainB)
 	mockFeeVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: ibcmock.Version}))
@@ -35,6 +38,13 @@ func (suite *FeeTestSuite) SetupTest() {
 	path.EndpointA.ChannelConfig.PortID = ibctesting.MockFeePort
 	path.EndpointB.ChannelConfig.PortID = ibctesting.MockFeePort
 	suite.path = path
+
+	path = ibctesting.NewPath(suite.chainA, suite.chainC)
+	path.EndpointA.ChannelConfig.Version = mockFeeVersion
+	path.EndpointB.ChannelConfig.Version = mockFeeVersion
+	path.EndpointA.ChannelConfig.PortID = ibctesting.MockFeePort
+	path.EndpointB.ChannelConfig.PortID = ibctesting.MockFeePort
+	suite.pathAToC = path
 }
 
 func TestIBCFeeTestSuite(t *testing.T) {

modules/apps/29-fee/ibc_module.go

@@ -198,12 +198,12 @@ func (im IBCModule) OnRecvPacket(
 
 	// incase of async aknowledgement (ack == nil) store the relayer address for use later during async WriteAcknowledgement
 	if ack == nil {
-		im.keeper.SetRelayerAddressForAsyncAck(ctx, channeltypes.NewPacketId(packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence()), relayer.String())
+		im.keeper.SetRelayerAddressForAsyncAck(ctx, channeltypes.NewPacketId(packet.GetDestChannel(), packet.GetDestPort(), packet.GetSequence()), relayer.String())
 		return nil
 	}
 
 	// if forwardRelayer is not found we refund recv_fee
-	forwardRelayer, _ := im.keeper.GetCounterpartyAddress(ctx, relayer.String(), packet.GetSourceChannel())
+	forwardRelayer, _ := im.keeper.GetCounterpartyAddress(ctx, relayer.String(), packet.GetDestChannel())
 
 	return types.NewIncentivizedAcknowledgement(forwardRelayer, ack.Acknowledgement(), ack.Success())
 }

modules/apps/29-fee/ibc_module_test.go

@@ -498,12 +498,12 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
 		tc := tc
 		suite.Run(tc.name, func() {
 			suite.SetupTest()
+			// setup pathAToC (chainA -> chainC) first in order to have different channel IDs for chainA & chainB
+			suite.coordinator.Setup(suite.pathAToC)
+			// setup path for chainA -> chainB
 			suite.coordinator.Setup(suite.path)
 
-			// set up a different channel to make sure that the test will error if the destination channel of the packet is not fee enabled
-			suite.path.EndpointB.ChannelID = "channel-1"
 			suite.chainB.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainB.GetContext(), suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID)
-			suite.chainB.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainB.GetContext(), suite.path.EndpointB.ChannelConfig.PortID, "channel-0")
 
 			packet := suite.CreateMockPacket()
 
@@ -522,11 +522,26 @@ func (suite *FeeTestSuite) TestOnRecvPacket() {
 			result := cbs.OnRecvPacket(suite.chainB.GetContext(), packet, suite.chainA.SenderAccount.GetAddress())
 
 			switch {
+			case tc.name == "success":
+				forwardAddr, _ := suite.chainB.GetSimApp().IBCFeeKeeper.GetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.path.EndpointB.ChannelID)
+
+				expectedAck := types.IncentivizedAcknowledgement{
+					Result:                ibcmock.MockAcknowledgement.Acknowledgement(),
+					ForwardRelayerAddress: forwardAddr,
+					UnderlyingAppSuccess:  true,
+				}
+				suite.Require().Equal(expectedAck, result)
+
 			case !tc.feeEnabled:
 				suite.Require().Equal(ibcmock.MockAcknowledgement, result)
 
 			case tc.forwardRelayer && result == nil:
 				suite.Require().Equal(nil, result)
+				packetId := channeltypes.NewPacketId(packet.GetDestChannel(), packet.GetDestPort(), packet.GetSequence())
+
+				// retrieve the forward relayer that was stored in `onRecvPacket`
+				relayer, _ := suite.chainB.GetSimApp().IBCFeeKeeper.GetRelayerAddressForAsyncAck(suite.chainB.GetContext(), packetId)
+				suite.Require().Equal(relayer, suite.chainA.SenderAccount.GetAddress().String())
 
 			case !tc.forwardRelayer:
 				expectedAck := types.IncentivizedAcknowledgement{

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

@@ -28,15 +28,19 @@ type KeeperTestSuite struct {
 	// testing chains used for convenience and readability
 	chainA *ibctesting.TestChain
 	chainB *ibctesting.TestChain
+	chainC *ibctesting.TestChain
+
+	path     *ibctesting.Path
+	pathAToC *ibctesting.Path
 
-	path        *ibctesting.Path
 	queryClient types.QueryClient
 }
 
 func (suite *KeeperTestSuite) SetupTest() {
-	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)
+	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 3)
 	suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
 	suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))
+	suite.chainC = suite.coordinator.GetChain(ibctesting.GetChainID(3))
 
 	path := ibctesting.NewPath(suite.chainA, suite.chainB)
 	mockFeeVersion := string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: ibcmock.Version}))
@@ -46,6 +50,13 @@ func (suite *KeeperTestSuite) SetupTest() {
 	path.EndpointB.ChannelConfig.PortID = ibctesting.MockFeePort
 	suite.path = path
 
+	path = ibctesting.NewPath(suite.chainA, suite.chainC)
+	path.EndpointA.ChannelConfig.Version = mockFeeVersion
+	path.EndpointB.ChannelConfig.Version = mockFeeVersion
+	path.EndpointA.ChannelConfig.PortID = ibctesting.MockFeePort
+	path.EndpointB.ChannelConfig.PortID = ibctesting.MockFeePort
+	suite.pathAToC = path
+
 	queryHelper := baseapp.NewQueryServerTestHelper(suite.chainA.GetContext(), suite.chainA.GetSimApp().InterfaceRegistry())
 	types.RegisterQueryServer(queryHelper, suite.chainA.GetSimApp().IBCFeeKeeper)
 	suite.queryClient = types.NewQueryClient(queryHelper)

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

@@ -23,7 +23,7 @@ func (suite *KeeperTestSuite) TestRegisterCounterpartyAddress() {
 			func() {},
 		},
 		{
-			"success",
+			"counterparty is an arbitrary string",
 			true,
 			func() { counterparty = "arbitrary-string" },
 		},

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

@@ -23,17 +23,17 @@ func (k Keeper) WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.C
 		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())
+	packetId := channeltypes.NewPacketId(packet.GetDestChannel(), packet.GetDestPort(), packet.GetSequence())
 
+	// retrieve the forward relayer that was stored in `onRecvPacket`
 	relayer, found := k.GetRelayerAddressForAsyncAck(ctx, packetId)
 	if !found {
 		return sdkerrors.Wrapf(types.ErrRelayerNotFoundForAsyncAck, "no relayer address stored for async acknowledgement for packet with portID: %s, channelID: %s, sequence: %d", packetId.PortId, packetId.ChannelId, packetId.Sequence)
 	}
 
 	// it is possible that a relayer has not registered a counterparty address.
 	// if there is no registered counterparty address then write acknowledgement with empty relayer address and refund recv_fee.
-	forwardRelayer, _ := k.GetCounterpartyAddress(ctx, relayer, packet.GetSourceChannel())
+	forwardRelayer, _ := k.GetCounterpartyAddress(ctx, relayer, packet.GetDestChannel())
 
 	ack := types.NewIncentivizedAcknowledgement(forwardRelayer, acknowledgement.Acknowledgement(), acknowledgement.Success())
 

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

@@ -1,6 +1,7 @@
 package keeper_test
 
 import (
+	"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types"
 	clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
 )
@@ -14,8 +15,8 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
 		{
 			"success",
 			func() {
-				suite.chainB.GetSimApp().IBCFeeKeeper.SetRelayerAddressForAsyncAck(suite.chainB.GetContext(), channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, 1), suite.chainA.SenderAccount.GetAddress().String())
-				suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.path.EndpointA.ChannelID)
+				suite.chainB.GetSimApp().IBCFeeKeeper.SetRelayerAddressForAsyncAck(suite.chainB.GetContext(), channeltypes.NewPacketId(suite.path.EndpointB.ChannelID, suite.path.EndpointB.ChannelConfig.PortID, 1), suite.chainA.SenderAccount.GetAddress().String())
+				suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.path.EndpointB.ChannelID)
 			},
 			true,
 		},
@@ -31,7 +32,10 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
 		suite.Run(tc.name, func() {
 			suite.SetupTest()
 
-			// open incentivized channel
+			// open incentivized channels
+			// setup pathAToC (chainA -> chainC) first in order to have different channel IDs for chainA & chainB
+			suite.coordinator.Setup(suite.pathAToC)
+			// setup path for chainA -> chainB
 			suite.coordinator.Setup(suite.path)
 
 			// build packet
@@ -59,6 +63,10 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
 				suite.Require().NoError(err)
 				_, found := suite.chainB.GetSimApp().IBCFeeKeeper.GetRelayerAddressForAsyncAck(suite.chainB.GetContext(), channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, 1))
 				suite.Require().False(found)
+
+				expectedAck := types.NewIncentivizedAcknowledgement(suite.chainB.SenderAccount.GetAddress().String(), ack.Acknowledgement(), ack.Success())
+				commitedAck, _ := suite.chainB.GetSimApp().GetIBCKeeper().ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), packet.DestinationPort, packet.DestinationChannel, 1)
+				suite.Require().Equal(commitedAck, channeltypes.CommitAcknowledgement(expectedAck.Acknowledgement()))
 			} else {
 				suite.Require().Error(err)
 			}