Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cli to submit gov proposal to upgrade multiple channels #5548

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion modules/core/04-channel/client/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ func NewTxCmd() *cobra.Command {
RunE: client.ValidateCmd,
}

txCmd.AddCommand()
txCmd.AddCommand(
newUpgradeChannelsCmd(),
)

return txCmd
}
160 changes: 160 additions & 0 deletions modules/core/04-channel/client/cli/tx.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
package cli

import (
"fmt"
"github.com/spf13/cobra"
"regexp"
"slices"
"strings"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/tx"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/version"
govtypesv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"

"github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
ibcexported "github.com/cosmos/ibc-go/v8/modules/core/exported"
)

const (
flagMetadata = "metadata"
flagSummary = "summary"
flagTitle = "title"
flagJSON = "json"
flagPortPattern = "port-pattern"
flagExpedited = "expedited"
flagChannelIDs = "channel-ids"
)

func newUpgradeChannelsCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "upgrade-channels",
Short: "Upgrade IBC channels",
Long: `Submit a governance proposal to upgrade all open channels whose port matches a specified pattern
(the default is transfer), optionally, specific an exact list of channel IDs with a comma separated list.`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
(the default is transfer), optionally, specific an exact list of channel IDs with a comma separated list.`,
(the default is transfer), optionally, an exact list of comma separated channel IDs may be specified.`,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we specify in this docstring that if channel IDs are specified then it must match the specified port? or leave that for the longer documentation?

Args: cobra.ExactArgs(2),
Example: fmt.Sprintf(`%s tx %s %s upgrade-channels 10stake "{\"fee_version\":\"ics29-1\",\"app_version\":\"ics20-1\"}"`, version.AppName, ibcexported.ModuleName, types.SubModuleName),
RunE: func(cmd *cobra.Command, args []string) error {
depositStr := args[0]
versionStr := args[1]

metadata, err := cmd.Flags().GetString(flagMetadata)
if err != nil {
return err
}

summary, err := cmd.Flags().GetString(flagSummary)
if err != nil {
return err
}

title, err := cmd.Flags().GetString(flagTitle)
if err != nil {
return err
}

portPattern, err := cmd.Flags().GetString(flagPortPattern)
if err != nil {
return err
}

commaSeparatedChannelIDs, err := cmd.Flags().GetString(flagChannelIDs)
if err != nil {
return err
}

displayJSON, err := cmd.Flags().GetBool(flagJSON)
if err != nil {
return err
}

expidited, err := cmd.Flags().GetBool(flagExpedited)
if err != nil {
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can maybe simplify all this code about the gov flags with a call ReadGovPropFlags like here, including the deposit parameter. And that function will return us a govv1.MsgSubmitProposal where we can add the messages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, could also add AddGovPropFlagsToCmd though we do set a sensible default here so fine either way (maybe add and override default value?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great suggestions, will implement these.


clientCtx, err := client.GetClientQueryContext(cmd)
if err != nil {
return err
}

queryClient := types.NewQueryClient(clientCtx)

resp, err := queryClient.Channels(cmd.Context(), &types.QueryChannelsRequest{})
if err != nil {
return err
}

channelIDs := strings.Split(commaSeparatedChannelIDs, ",")

var msgs []sdk.Msg
pattern := regexp.MustCompile(portPattern)

for _, ch := range resp.Channels {

if !channelShouldBeUpgraded(*ch, pattern, channelIDs) {
continue
}

// construct a MsgChannelUpgradeInit which will upgrade the specified channel to a specific version.
msgUpgradeInit := types.NewMsgChannelUpgradeInit(ch.PortId, ch.ChannelId, types.NewUpgradeFields(ch.Ordering, ch.ConnectionHops, versionStr), clientCtx.GetFromAddress().String())
msgs = append(msgs, msgUpgradeInit)
Comment on lines +112 to +113
Copy link
Contributor Author

@chatton chatton Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left ordering and connection hops unchanged, and only the version string changes. Technically we can allow these fields to be updated, but I think the version string will be all that we need 99% of the time. As far as bulk creating MsgChanUpgradeInits is concerned

}

if len(msgs) == 0 {
return fmt.Errorf("no channels would be upgraded with pattern %s", portPattern)
}

deposit, err := sdk.ParseCoinsNormalized(depositStr)
if err != nil {
return err
}

msgSubmitProposal, err := govtypesv1.NewMsgSubmitProposal(msgs, deposit, clientCtx.GetFromAddress().String(), metadata, title, summary, expidited)
if err != nil {
return fmt.Errorf("invalid message: %w", err)
}

dryRun, _ := cmd.Flags().GetBool(flags.FlagDryRun)
if displayJSON || dryRun {
out, err := clientCtx.Codec.MarshalJSON(msgSubmitProposal)
if err != nil {
return err
}
return clientCtx.PrintBytes(out)
}

return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msgSubmitProposal)
},
}

flags.AddTxFlagsToCmd(cmd)
cmd.Flags().String(flagSummary, "Upgrading open channels", "The summary for the gov proposal which will upgrade existing open channels.")
cmd.Flags().String(flagTitle, "Channel upgrades", "The title for the gov proposal which will upgrade existing open channels.")
cmd.Flags().String(flagMetadata, "", "Metadata for the gov proposal which will upgrade existing open channels.")
cmd.Flags().String(flagPortPattern, "transfer", "The pattern to use to match port ids.")
cmd.Flags().Bool(flagExpedited, false, "set the expedited value for the governance proposal.")
cmd.Flags().Bool(flagJSON, false, "specify true to output valid proposal.json contents, instead of submitting a governance proposal.")
cmd.Flags().String(flagChannelIDs, "", "a comma separated list of channel IDs to upgrade.")

return cmd
}

// channelShouldBeUpgraded returns a boolean indicated whether or not the given channel should be upgraded based
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// channelShouldBeUpgraded returns a boolean indicated whether or not the given channel should be upgraded based
// channelShouldBeUpgraded returns a boolean indicating whether or not the given channel should be upgraded based

// on either the provided regex pattern or list of desired channel IDs.
func channelShouldBeUpgraded(channel types.IdentifiedChannel, pattern *regexp.Regexp, channelIDs []string) bool {
// skip any channel that is not open
if channel.State != types.OPEN {
return false
}

// if specified, the channel ID must exactly match.
if len(channelIDs) > 0 {
return pattern.MatchString(channel.PortId) && slices.Contains(channelIDs, channel.ChannelId)
}

// otherwise we only need the port pattern to match.
return pattern.MatchString(channel.PortId)
}