refactor: adding check if channel exists before register counterparty (…

…#1339)

* refactor: adding check if channel exists before register. updating msg to contain PortID

* chore: changelog

* refactor: add types test and check for is fee enabled + fix test

docs/ibc/proto-docs.md

@@ -1293,6 +1293,7 @@ MsgRegisterCounterpartyAddress defines the request type for the RegisterCounterp
 | ----- | ---- | ----- | ----------- |
 | `address` | [string](#string) |  | the relayer address |
 | `counterparty_address` | [string](#string) |  | the counterparty relayer address |
+| `port_id` | [string](#string) |  | unique port identifier |
 | `channel_id` | [string](#string) |  | unique channel identifier |
 
 

modules/apps/29-fee/client/cli/tx.go

@@ -101,18 +101,18 @@ func NewPayPacketFeeAsyncTxCmd() *cobra.Command {
 // NewRegisterCounterpartyAddress returns the command to create a MsgRegisterCounterpartyAddress
 func NewRegisterCounterpartyAddress() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:     "register-counterparty [address] [counterparty-address] [channel-id]",
+		Use:     "register-counterparty [address] [counterparty-address] [channel-id] [port-id]",
 		Short:   "Register a counterparty relayer address on a given channel.",
 		Long:    strings.TrimSpace(`Register a counterparty relayer address on a given channel.`),
-		Example: fmt.Sprintf("%s tx ibc-fee register-counterparty cosmos1rsp837a4kvtgp2m4uqzdge0zzu6efqgucm0qdh osmo1v5y0tz01llxzf4c2afml8s3awue0ymju22wxx2 channel-0", version.AppName),
-		Args:    cobra.ExactArgs(3),
+		Example: fmt.Sprintf("%s tx ibc-fee register-counterparty cosmos1rsp837a4kvtgp2m4uqzdge0zzu6efqgucm0qdh osmo1v5y0tz01llxzf4c2afml8s3awue0ymju22wxx2 channel-0 transfer", version.AppName),
+		Args:    cobra.ExactArgs(4),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			clientCtx, err := client.GetClientTxContext(cmd)
 			if err != nil {
 				return err
 			}
 
-			msg := types.NewMsgRegisterCounterpartyAddress(args[0], args[1], args[2])
+			msg := types.NewMsgRegisterCounterpartyAddress(args[0], args[1], args[2], args[3])
 
 			return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
 		},

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

@@ -18,6 +18,15 @@ var _ types.MsgServer = Keeper{}
 func (k Keeper) RegisterCounterpartyAddress(goCtx context.Context, msg *types.MsgRegisterCounterpartyAddress) (*types.MsgRegisterCounterpartyAddressResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
+	// only register counterparty address if the channel exists and is fee enabled
+	if _, found := k.channelKeeper.GetChannel(ctx, msg.PortId, msg.ChannelId); !found {
+		return nil, channeltypes.ErrChannelNotFound
+	}
+
+	if !k.IsFeeEnabled(ctx, msg.PortId, msg.ChannelId) {
+		return nil, types.ErrFeeNotEnabled
+	}
+
 	k.SetCounterpartyAddress(
 		ctx, msg.Address, msg.CounterpartyAddress, msg.ChannelId,
 	)

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

@@ -13,6 +13,8 @@ func (suite *KeeperTestSuite) TestRegisterCounterpartyAddress() {
 	var (
 		sender       string
 		counterparty string
+		channelID    string
+		ctx          sdk.Context
 	)
 
 	testCases := []struct {
@@ -30,18 +32,33 @@ func (suite *KeeperTestSuite) TestRegisterCounterpartyAddress() {
 			true,
 			func() { counterparty = "arbitrary-string" },
 		},
+		{
+			"channel does not exist",
+			false,
+			func() { channelID = "channel-22" },
+		},
+		{
+			"channel is not fee enabled",
+			false,
+			func() {
+				suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(ctx, suite.path.EndpointA.ChannelConfig.PortID, channelID)
+			},
+		},
 	}
 
 	for _, tc := range testCases {
 		suite.SetupTest()
-		ctx := suite.chainA.GetContext()
+		ctx = suite.chainA.GetContext()
+		suite.coordinator.Setup(suite.path) // setup channel
 
 		sender = suite.chainA.SenderAccount.GetAddress().String()
 		counterparty = suite.chainB.SenderAccount.GetAddress().String()
+		channelID = suite.path.EndpointA.ChannelID
+
 		tc.malleate()
-		msg := types.NewMsgRegisterCounterpartyAddress(sender, counterparty, ibctesting.FirstChannelID)
 
-		_, err := suite.chainA.SendMsgs(msg)
+		msg := types.NewMsgRegisterCounterpartyAddress(sender, counterparty, suite.path.EndpointA.ChannelConfig.PortID, channelID)
+		_, err := suite.chainA.GetSimApp().IBCFeeKeeper.RegisterCounterpartyAddress(sdk.WrapSDKContext(ctx), msg)
 
 		if tc.expPass {
 			suite.Require().NoError(err) // message committed

modules/apps/29-fee/transfer_test.go

@@ -46,7 +46,7 @@ func (suite *FeeTestSuite) TestFeeTransfer() {
 	// to differentiate from the chainA.SenderAccount for checking successful relay payouts
 	relayerAddress := suite.chainB.SenderAccount.GetAddress()
 
-	msgRegister := types.NewMsgRegisterCounterpartyAddress(suite.chainB.SenderAccount.GetAddress().String(), relayerAddress.String(), ibctesting.FirstChannelID)
+	msgRegister := types.NewMsgRegisterCounterpartyAddress(suite.chainB.SenderAccount.GetAddress().String(), relayerAddress.String(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 	_, err = suite.chainB.SendMsgs(msgRegister)
 	suite.Require().NoError(err) // message committed
 

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

@@ -17,10 +17,11 @@ const (
 )
 
 // NewMsgRegisterCounterpartyAddress creates a new instance of MsgRegisterCounterpartyAddress
-func NewMsgRegisterCounterpartyAddress(address, counterpartyAddress, channelID string) *MsgRegisterCounterpartyAddress {
+func NewMsgRegisterCounterpartyAddress(address, counterpartyAddress, portID, channelID string) *MsgRegisterCounterpartyAddress {
 	return &MsgRegisterCounterpartyAddress{
 		Address:             address,
 		CounterpartyAddress: counterpartyAddress,
+		PortId:              portID,
 		ChannelId:           channelID,
 	}
 }
@@ -36,6 +37,11 @@ func (msg MsgRegisterCounterpartyAddress) ValidateBasic() error {
 		return ErrCounterpartyAddressEmpty
 	}
 
+	// validate portId
+	if err := host.PortIdentifierValidator(msg.PortId); err != nil {
+		return err
+	}
+
 	// validate channelId
 	if err := host.ChannelIdentifierValidator(msg.ChannelId); err != nil {
 		return err

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

@@ -55,10 +55,17 @@ func TestMsgRegisterCountepartyAddressValidation(t *testing.T) {
 			},
 			false,
 		},
+		{
+			"invalid portID",
+			func() {
+				msg.PortId = ""
+			},
+			false,
+		},
 	}
 
 	for i, tc := range testCases {
-		msg = types.NewMsgRegisterCounterpartyAddress(defaultAccAddress, defaultAccAddress, ibctesting.FirstChannelID)
+		msg = types.NewMsgRegisterCounterpartyAddress(defaultAccAddress, defaultAccAddress, ibctesting.MockPort, ibctesting.FirstChannelID)
 
 		tc.malleate()
 
@@ -74,7 +81,7 @@ func TestMsgRegisterCountepartyAddressValidation(t *testing.T) {
 
 func TestRegisterCountepartyAddressGetSigners(t *testing.T) {
 	accAddress := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address())
-	msg := types.NewMsgRegisterCounterpartyAddress(accAddress.String(), defaultAccAddress, ibctesting.FirstChannelID)
+	msg := types.NewMsgRegisterCounterpartyAddress(accAddress.String(), defaultAccAddress, ibctesting.MockPort, ibctesting.FirstChannelID)
 	require.Equal(t, []sdk.AccAddress{sdk.AccAddress(accAddress)}, msg.GetSigners())
 }
 

proto/ibc/applications/fee/v1/tx.proto

@@ -39,8 +39,10 @@ message MsgRegisterCounterpartyAddress {
   string address = 1;
   // the counterparty relayer address
   string counterparty_address = 2 [(gogoproto.moretags) = "yaml:\"counterparty_address\""];
+  // unique port identifier
+  string port_id = 3 [(gogoproto.moretags) = "yaml:\"port_id\""];
   // unique channel identifier
-  string channel_id = 3 [(gogoproto.moretags) = "yaml:\"channel_id\""];
+  string channel_id = 4 [(gogoproto.moretags) = "yaml:\"channel_id\""];
 }
 
 // MsgRegisterCounterpartyAddressResponse defines the response type for the RegisterCounterpartyAddress rpc