Repository: cosmos/relayer
Pull request #788 (2022-06-08 19:59:02)

Remove the need for importing the `flags` and `errors` packages from Cosmos SDK
We were importing the `flags` and `errors` packages from the Cosmos SDK  just to import some flag names and to wrap some errors. We can eliminate the need for importing these packages by simply utilizing `fmt` and including our own flag names inside the `cmd` package.

Additionally, I made the receiver names consistent for methods on type `*Config`


Patch:
diff --git a/cmd/config.go b/cmd/config.go
index c4fcf1a..168c949 100644
--- a/cmd/config.go
+++ b/cmd/config.go
@@ -29,8 +29,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/cosmos/cosmos-sdk/client/flags"
-	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/relayer/v2/relayer"
 	"github.com/cosmos/relayer/v2/relayer/provider"
 	"github.com/cosmos/relayer/v2/relayer/provider/cosmos"
@@ -67,7 +65,7 @@ func configShowCmd(a *appState) *cobra.Command {
 $ %s config show --home %s
 $ %s cfg list`, appName, defaultHome, appName)),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			home, err := cmd.Flags().GetString(flags.FlagHome)
+			home, err := cmd.Flags().GetString(flagHome)
 			if err != nil {
 				return err
 			}
@@ -123,7 +121,7 @@ func configInitCmd() *cobra.Command {
 $ %s config init --home %s
 $ %s cfg i`, appName, defaultHome, appName)),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			home, err := cmd.Flags().GetString(flags.FlagHome)
+			home, err := cmd.Flags().GetString(flagHome)
 			if err != nil {
 				return err
 			}
@@ -305,16 +303,16 @@ func addPathsFromDirectory(ctx context.Context, stderr io.Writer, a *appState, d
 }
 
 // Wrapped converts the Config struct into a ConfigOutputWrapper struct
-func (config *Config) Wrapped() *ConfigOutputWrapper {
+func (c *Config) Wrapped() *ConfigOutputWrapper {
 	providers := make(ProviderConfigs)
-	for _, chain := range config.Chains {
+	for _, chain := range c.Chains {
 		pcfgw := &ProviderConfigWrapper{
 			Type:  chain.ChainProvider.Type(),
 			Value: chain.ChainProvider.ProviderConfig(),
 		}
 		providers[chain.ChainProvider.ChainName()] = pcfgw
 	}
-	return &ConfigOutputWrapper{Global: config.Global, ProviderConfigs: providers, Paths: config.Paths}
+	return &ConfigOutputWrapper{Global: c.Global, ProviderConfigs: providers, Paths: c.Paths}
 }
 
 // Config represents the config file for the relayer
@@ -552,7 +550,7 @@ func validateConfig(c *Config) error {
 
 // initConfig reads in config file and ENV variables if set.
 func initConfig(cmd *cobra.Command, a *appState) error {
-	home, err := cmd.PersistentFlags().GetString(flags.FlagHome)
+	home, err := cmd.PersistentFlags().GetString(flagHome)
 	if err != nil {
 		return err
 	}
@@ -617,10 +615,10 @@ func initConfig(cmd *cobra.Command, a *appState) error {
 // ValidatePath checks that a path is valid
 func (c *Config) ValidatePath(ctx context.Context, stderr io.Writer, p *relayer.Path) (err error) {
 	if err = c.ValidatePathEnd(ctx, stderr, p.Src); err != nil {
-		return sdkerrors.Wrapf(err, "chain %s failed path validation", p.Src.ChainID)
+		return fmt.Errorf("chain %s failed path validation: %w", p.Src.ChainID, err)
 	}
 	if err = c.ValidatePathEnd(ctx, stderr, p.Dst); err != nil {
-		return sdkerrors.Wrapf(err, "chain %s failed path validation", p.Dst.ChainID)
+		return fmt.Errorf("chain %s failed path validation: %w", p.Dst.ChainID, err)
 	}
 	return nil
 }
diff --git a/cmd/flags.go b/cmd/flags.go
index 1295e94..876a220 100644
--- a/cmd/flags.go
+++ b/cmd/flags.go
@@ -1,14 +1,15 @@
 package cmd
 
 import (
+	"fmt"
 	"time"
 
-	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 )
 
 const (
+	flagHome                    = "home"
 	flagURL                     = "url"
 	flagSkip                    = "skip"
 	flagTimeout                 = "timeout"
@@ -31,6 +32,13 @@ const (
 	flagOrder                   = "order"
 	flagVersion                 = "version"
 	flagDebugAddr               = "debug-addr"
+	flagOffset                  = "offset"
+	flagLimit                   = "limit"
+	flagHeight                  = "height"
+	flagPage                    = "page"
+	flagPageKey                 = "page-key"
+	flagCountTotal              = "count-total"
+	flagReverse                 = "reverse"
 )
 
 const (
@@ -48,20 +56,37 @@ func ibcDenomFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command {
 }
 
 func heightFlag(v *viper.Viper, cmd *cobra.Command) *cobra.Command {
-	cmd.Flags().Int64(flags.FlagHeight, 0, "Height of headers to fetch")
-	if err := v.BindPFlag(flags.FlagHeight, cmd.Flags().Lookup(flags.FlagHeight)); err != nil {
+	cmd.Flags().Int64(flagHeight, 0, "Height of headers to fetch")
+	if err := v.BindPFlag(flagHeight, cmd.Flags().Lookup(flagHeight)); err != nil {
 		panic(err)
 	}
 	return cmd
 }
 
-func paginationFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command {
-	cmd.Flags().Uint64P(flags.FlagOffset, "o", 0, "pagination offset for query")
-	cmd.Flags().Uint64P(flags.FlagLimit, "l", 10, "pagination limit for query")
-	if err := v.BindPFlag(flags.FlagOffset, cmd.Flags().Lookup(flags.FlagOffset)); err != nil {
+func paginationFlags(v *viper.Viper, cmd *cobra.Command, query string) *cobra.Command {
+	cmd.Flags().Uint64(flagPage, 1, fmt.Sprintf("pagination page of %s to query for. This sets offset to a multiple of limit", query))
+	cmd.Flags().String(flagPageKey, "", fmt.Sprintf("pagination page-key of %s to query for", query))
+	cmd.Flags().Uint64(flagOffset, 0, fmt.Sprintf("pagination offset of %s to query for", query))
+	cmd.Flags().Uint64(flagLimit, 100, fmt.Sprintf("pagination limit of %s to query for", query))
+	cmd.Flags().Bool(flagCountTotal, false, fmt.Sprintf("count total number of records in %s to query for", query))
+	cmd.Flags().Bool(flagReverse, false, "results are sorted in descending order")
+
+	if err := v.BindPFlag(flagPage, cmd.Flags().Lookup(flagPage)); err != nil {
+		panic(err)
+	}
+	if err := v.BindPFlag(flagPageKey, cmd.Flags().Lookup(flagPageKey)); err != nil {
+		panic(err)
+	}
+	if err := v.BindPFlag(flagOffset, cmd.Flags().Lookup(flagOffset)); err != nil {
+		panic(err)
+	}
+	if err := v.BindPFlag(flagLimit, cmd.Flags().Lookup(flagLimit)); err != nil {
+		panic(err)
+	}
+	if err := v.BindPFlag(flagCountTotal, cmd.Flags().Lookup(flagCountTotal)); err != nil {
 		panic(err)
 	}
-	if err := v.BindPFlag(flags.FlagLimit, cmd.Flags().Lookup(flags.FlagLimit)); err != nil {
+	if err := v.BindPFlag(flagReverse, cmd.Flags().Lookup(flagReverse)); err != nil {
 		panic(err)
 	}
 	return cmd
diff --git a/cmd/query.go b/cmd/query.go
index b5c63ef..f1c7664 100644
--- a/cmd/query.go
+++ b/cmd/query.go
@@ -6,7 +6,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/cosmos/cosmos-sdk/client/flags"
 	ibcexported "github.com/cosmos/ibc-go/v3/modules/core/exported"
 	"github.com/cosmos/relayer/v2/helpers"
 	"github.com/cosmos/relayer/v2/relayer"
@@ -172,12 +171,12 @@ $ %s q txs ibc-0 "message.action=transfer"`,
 				return errChainNotFound(args[0])
 			}
 
-			offset, err := cmd.Flags().GetUint64(flags.FlagOffset)
+			offset, err := cmd.Flags().GetUint64(flagOffset)
 			if err != nil {
 				return err
 			}
 
-			limit, err := cmd.Flags().GetUint64(flags.FlagLimit)
+			limit, err := cmd.Flags().GetUint64(flagLimit)
 			if err != nil {
 				return err
 			}
@@ -197,7 +196,7 @@ $ %s q txs ibc-0 "message.action=transfer"`,
 		},
 	}
 
-	return paginationFlags(a.Viper, cmd)
+	return paginationFlags(a.Viper, cmd, "txs")
 }
 
 //func queryAccountCmd() *cobra.Command {
@@ -406,7 +405,7 @@ $ %s query client ibc-0 ibczeroclient --height 1205`,
 				return errChainNotFound(args[0])
 			}
 
-			height, err := cmd.Flags().GetInt64(flags.FlagHeight)
+			height, err := cmd.Flags().GetInt64(flagHeight)
 			if err != nil {
 				return err
 			}
@@ -483,8 +482,7 @@ $ %s query clients ibc-2 --offset 2 --limit 30`,
 		},
 	}
 
-	flags.AddPaginationFlagsToCmd(cmd, "client states")
-	return cmd
+	return paginationFlags(a.Viper, cmd, "client states")
 }
 
 //func queryValSetAtHeightCmd() *cobra.Command {
@@ -565,8 +563,7 @@ $ %s q conns ibc-1`,
 		},
 	}
 
-	flags.AddPaginationFlagsToCmd(cmd, "connections on a network")
-	return cmd
+	return paginationFlags(a.Viper, cmd, "connections on a network")
 }
 
 func queryConnectionsUsingClient(a *appState) *cobra.Command {
@@ -591,7 +588,7 @@ $ %s query client-connections ibc-0 ibczeroclient --height 1205`,
 				return err
 			}
 
-			height, err := cmd.Flags().GetInt64(flags.FlagHeight)
+			height, err := cmd.Flags().GetInt64(flagHeight)
 			if err != nil {
 				return err
 			}
@@ -712,8 +709,7 @@ $ %s query connection-channels ibc-2 ibcconnection2 --offset 2 --limit 30`,
 		},
 	}
 
-	flags.AddPaginationFlagsToCmd(cmd, "channels associated with a connection")
-	return cmd
+	return paginationFlags(a.Viper, cmd, "channels associated with a connection")
 }
 
 func queryChannel(a *appState) *cobra.Command {
@@ -738,7 +734,7 @@ $ %s query channel ibc-2 ibctwochannel transfer --height 1205`,
 				return err
 			}
 
-			height, err := cmd.Flags().GetInt64(flags.FlagHeight)
+			height, err := cmd.Flags().GetInt64(flagHeight)
 			if err != nil {
 				return err
 			}
@@ -810,8 +806,7 @@ $ %s query channels ibc-2 --offset 2 --limit 30`,
 		},
 	}
 
-	flags.AddPaginationFlagsToCmd(cmd, "channels on a network")
-	return cmd
+	return paginationFlags(a.Viper, cmd, "channels on a network")
 }
 
 func queryPacketCommitment(a *appState) *cobra.Command {
diff --git a/cmd/root.go b/cmd/root.go
index ff02315..b5ac3e7 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -29,7 +29,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/cosmos/cosmos-sdk/client/flags"
 	zaplogfmt "github.com/jsternberg/zap-logfmt"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -100,8 +99,8 @@ func NewRootCmd(log *zap.Logger) *cobra.Command {
 	}
 
 	// Register --home flag
-	rootCmd.PersistentFlags().StringVar(&a.HomePath, flags.FlagHome, defaultHome, "set home directory")
-	if err := a.Viper.BindPFlag(flags.FlagHome, rootCmd.PersistentFlags().Lookup(flags.FlagHome)); err != nil {
+	rootCmd.PersistentFlags().StringVar(&a.HomePath, flagHome, defaultHome, "set home directory")
+	if err := a.Viper.BindPFlag(flagHome, rootCmd.PersistentFlags().Lookup(flagHome)); err != nil {
 		panic(err)
 	}
 
@@ -222,15 +221,10 @@ func readLine(in io.Reader) (string, error) {
 	return strings.TrimSpace(str), err
 }
 
-// lineBreakCommand returns a new instance of a command to be used as a line break
-// in a command's help output.
-//
-// This is not a plain reference to flags.LineBreak,
-// because that is a global value that will be modified by concurrent tests,
-// causing a data race.
+// lineBreakCommand returns a new instance of the lineBreakCommand every time to avoid
+// data races in concurrent tests exercising commands.
 func lineBreakCommand() *cobra.Command {
-	var cmd cobra.Command = *flags.LineBreak
-	return &cmd
+	return &cobra.Command{Run: func(*cobra.Command, []string) {}}
 }
 
 // withUsage wraps a PositionalArgs to display usage only when the PositionalArgs
diff --git a/cmd/tx.go b/cmd/tx.go
index e672a61..dd810d5 100644
--- a/cmd/tx.go
+++ b/cmd/tx.go
@@ -6,7 +6,6 @@ import (
 	"time"
 
 	"github.com/avast/retry-go/v4"
-	"github.com/cosmos/cosmos-sdk/client/flags"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	chantypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
 	"github.com/cosmos/ibc-go/v3/modules/core/exported"
@@ -300,7 +299,7 @@ func upgradeClientsCmd(a *appState) *cobra.Command {
 				return err
 			}
 
-			height, err := cmd.Flags().GetInt64(flags.FlagHeight)
+			height, err := cmd.Flags().GetInt64(flagHeight)
 			if err != nil {
 				return err
 			}


Test patch:
diff --git a/cmd/flags_test.go b/cmd/flags_test.go
new file mode 100644
index 0000000..a5a2916
--- /dev/null
+++ b/cmd/flags_test.go
@@ -0,0 +1,22 @@
+package cmd
+
+import (
+	"testing"
+
+	"github.com/cosmos/cosmos-sdk/client/flags"
+	"github.com/stretchr/testify/require"
+)
+
+// TestFlagEqualityAgainstSDK makes assertions against our local flags and the corresponding flags from
+// the Cosmos SDK to ensure that the CLI experience is consistent across both the SDK and the relayer.
+// This allows us to avoid directly depending on the `sdk/client/flags` package inside of our cmd package.
+func TestFlagEqualityAgainstSDK(t *testing.T) {
+	require.Equal(t, flagHome, flags.FlagHome)
+	require.Equal(t, flagOffset, flags.FlagOffset)
+	require.Equal(t, flagLimit, flags.FlagLimit)
+	require.Equal(t, flagHeight, flags.FlagHeight)
+	require.Equal(t, flagPage, flags.FlagPage)
+	require.Equal(t, flagPageKey, flags.FlagPageKey)
+	require.Equal(t, flagCountTotal, flags.FlagCountTotal)
+	require.Equal(t, flagReverse, flags.FlagReverse)
+}