refactor: adding check if channel exists before register counterparty

CHANGELOG.md

@@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (modules/app/29-fee) [\#1339](https://github.com/cosmos/ibc-go/pull/1339) The `RegisterCounterpartyAddress` gRPC endpoint now checks if the channel exists before registering.  
 * (modules/core/keeper) [\#1284](https://github.com/cosmos/ibc-go/pull/1284) Add sanity check for the keepers passed into `ibckeeper.NewKeeper`. `ibckeeper.NewKeeper` now panics if any of the keepers passed in is empty.
 * (middleware) [\#1022](https://github.com/cosmos/ibc-go/pull/1022) Add `GetAppVersion` to the ICS4Wrapper interface. This function should be used by IBC applications to obtain their own version since the version set in the channel structure may be wrapped many times by middleware. 
 * (modules/core/04-channel) [\#1160](https://github.com/cosmos/ibc-go/pull/1160) Improve `uint64 -> string` performance in `Logger`.

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

@@ -17,6 +17,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

@@ -12,6 +12,8 @@ func (suite *KeeperTestSuite) TestRegisterCounterpartyAddress() {
 	var (
 		sender       string
 		counterparty string
+		channelID    string
+		ctx          sdk.Context
 	)
 
 	testCases := []struct {
@@ -29,18 +31,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
 
@@ -68,5 +68,4 @@ func (suite *FeeTestSuite) TestFeeTransfer() {
 		fee.AckFee.Add(fee.TimeoutFee...), // ack fee paid, timeout fee refunded
 		sdk.NewCoins(suite.chainA.GetSimApp().BankKeeper.GetBalance(suite.chainA.GetContext(), suite.chainA.SenderAccount.GetAddress(), ibctesting.TestCoin.Denom)).Sub(originalChainASenderAccountBalance),
 	)
-
 }

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