feat: multiple fee tokens gov prop (#5261)

* multiple fee tokens gov prop

* update changelog

* add cli example line

---------

Co-authored-by: Roman <[email protected]>

CHANGELOG.md

@@ -69,8 +69,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   * [#5129](https://github.com/osmosis-labs/osmosis/pull/5129) Relax twap record validation in init genesis to allow one of the spot prices to be non-zero when twap error is observed.
   * [#5165](https://github.com/osmosis-labs/osmosis/pull/5165) Improve error message when fully exiting from a pool.
   * [#5187](https://github.com/osmosis-labs/osmosis/pull/5187) Expand `IncentivizedPools` query to include internally incentivized CL pools.
-  * [#5239](https://github.com/osmosis-labs/osmosis/pull/5239) Implement `GetTotalPoolShares` public keeper function for GAMM. 
-
+  * [#5239](https://github.com/osmosis-labs/osmosis/pull/5239) Implement `GetTotalPoolShares` public keeper function for GAMM.
+  * [#5261](https://github.com/osmosis-labs/osmosis/pull/5261) Allows `UpdateFeeTokenProposal` to take in multiple fee tokens instead of just one.
+
 ### API breaks
 
 * [#4336](https://github.com/osmosis-labs/osmosis/pull/4336) Move epochs module into its own go.mod

proto/osmosis/txfees/v1beta1/gov.proto

@@ -8,11 +8,11 @@ import "osmosis/txfees/v1beta1/feetoken.proto";
 
 option go_package = "github.com/osmosis-labs/osmosis/v15/x/txfees/types";
 
-// UpdateFeeTokenProposal is a gov Content type for adding a new whitelisted fee
-// token. It must specify a denom along with gamm pool ID to use as a spot price
-// calculator. It can be used to add a new denom to the whitelist It can also be
-// used to update the Pool to associate with the denom. If Pool ID is set to 0,
-// it will remove the denom from the whitelisted set.
+// UpdateFeeTokenProposal is a gov Content type for adding new whitelisted fee
+// token(s). It must specify a denom along with gamm pool ID to use as a spot
+// price calculator. It can be used to add new denoms to the whitelist. It can
+// also be used to update the Pool to associate with the denom. If Pool ID is
+// set to 0, it will remove the denom from the whitelisted set.
 message UpdateFeeTokenProposal {
   option (gogoproto.equal) = true;
   option (gogoproto.goproto_getters) = false;
@@ -22,8 +22,8 @@ message UpdateFeeTokenProposal {
 
   string title = 1 [ (gogoproto.moretags) = "yaml:\"title\"" ];
   string description = 2 [ (gogoproto.moretags) = "yaml:\"description\"" ];
-  FeeToken feetoken = 3 [
-    (gogoproto.moretags) = "yaml:\"fee_token\"",
+  repeated FeeToken feetokens = 3 [
+    (gogoproto.moretags) = "yaml:\"fee_tokens\"",
     (gogoproto.nullable) = false
   ];
 }

x/txfees/client/cli/query_test.go

@@ -32,9 +32,11 @@ func (s *QueryTestSuite) SetupSuite() {
 	upgradeProp := types.NewUpdateFeeTokenProposal(
 		"Test Proposal",
 		"test",
-		types.FeeToken{
-			Denom:  "uosmo",
-			PoolID: 1,
+		[]types.FeeToken{
+			{
+				Denom:  "uosmo",
+				PoolID: 1,
+			},
 		},
 	)
 	err := s.App.TxFeesKeeper.HandleUpdateFeeTokenProposal(s.Ctx, &upgradeProp)

x/txfees/client/cli/tx.go

@@ -1,14 +1,15 @@
 package cli
 
 import (
+	"errors"
 	"strconv"
+	"strings"
 
 	"github.com/cosmos/cosmos-sdk/client/tx"
 
 	"github.com/spf13/cobra"
 
 	"github.com/cosmos/cosmos-sdk/client"
-	"github.com/cosmos/cosmos-sdk/client/flags"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/gov/client/cli"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
@@ -17,6 +18,8 @@ import (
 	"github.com/osmosis-labs/osmosis/v15/x/txfees/types"
 )
 
+const FlagFeeTokens = "fee-tokens"
+
 func NewTxCmd() *cobra.Command {
 	txCmd := osmocli.TxIndexCmd(types.ModuleName)
 	txCmd.AddCommand(
@@ -27,32 +30,22 @@ func NewTxCmd() *cobra.Command {
 
 func NewCmdSubmitUpdateFeeTokenProposal() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "update-fee-token [denom] [poolId]",
-		Args:  cobra.ExactArgs(2),
-		Short: "Submit an update to a fee token to be usable for tx fees",
-		RunE: func(cmd *cobra.Command, args []string) error {
-			clientCtx, err := client.GetClientTxContext(cmd)
-			if err != nil {
-				return err
-			}
+		Use:     "update-fee-token [flags]",
+		Args:    cobra.ExactArgs(0),
+		Example: "update-fee-token --fee-tokens uosmo,1,uion,2,ufoo,0 --from val --chain-id osmosis-1",
+		Short:   "Submit a update fee token record proposal",
+		Long: strings.TrimSpace(`Submit a update fee token record proposal.
 
-			denom := args[0]
-			pool_id, err := strconv.ParseUint(args[1], 10, 64)
-			if err != nil {
-				return err
-			}
+Passing in denom,poolID pairs separated by commas would be parsed automatically to pairs of fee token records.
+Ex) uosmo,1,uion,2,ufoo,0 -> [Adds uosmo<>pool1, uion<>pool2, Removes ufoo as a fee token]
 
-			feeToken := types.FeeToken{
-				Denom:  denom,
-				PoolID: pool_id,
-			}
-
-			title, err := cmd.Flags().GetString(cli.FlagTitle)
+		`),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			clientCtx, err := client.GetClientTxContext(cmd)
 			if err != nil {
 				return err
 			}
-
-			description, err := cmd.Flags().GetString(cli.FlagDescription)
+			content, err := parseFeeTokenRecordsArgsToContent(cmd)
 			if err != nil {
 				return err
 			}
@@ -68,9 +61,7 @@ func NewCmdSubmitUpdateFeeTokenProposal() *cobra.Command {
 				return err
 			}
 
-			content := types.NewUpdateFeeTokenProposal(title, description, feeToken)
-
-			msg, err := govtypes.NewMsgSubmitProposal(&content, deposit, from)
+			msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from)
 			if err != nil {
 				return err
 			}
@@ -86,9 +77,66 @@ func NewCmdSubmitUpdateFeeTokenProposal() *cobra.Command {
 	cmd.Flags().String(cli.FlagTitle, "", "title of proposal")
 	cmd.Flags().String(cli.FlagDescription, "", "description of proposal")
 	cmd.Flags().String(cli.FlagDeposit, "", "deposit of proposal")
-	flags.AddTxFlagsToCmd(cmd)
-	_ = cmd.MarkFlagRequired(cli.FlagTitle)
-	_ = cmd.MarkFlagRequired(cli.FlagDescription)
+	cmd.Flags().Bool(cli.FlagIsExpedited, false, "If true, makes the proposal an expedited one")
+	cmd.Flags().String(cli.FlagProposal, "", "Proposal file path (if this path is given, other proposal flags are ignored)")
+	cmd.Flags().String(FlagFeeTokens, "", "The fee token records array")
 
 	return cmd
 }
+
+func parseFeeTokenRecords(cmd *cobra.Command) ([]types.FeeToken, error) {
+	feeTokensStr, err := cmd.Flags().GetString(FlagFeeTokens)
+	if err != nil {
+		return nil, err
+	}
+
+	feeTokens := strings.Split(feeTokensStr, ",")
+
+	if len(feeTokens)%2 != 0 {
+		return nil, errors.New("fee denom records should be a comma separated list of denom and poolId pairs")
+	}
+
+	feeTokenRecords := []types.FeeToken{}
+	i := 0
+	for i < len(feeTokens) {
+		denom := feeTokens[i]
+		poolId, err := strconv.Atoi(feeTokens[i+1])
+		if err != nil {
+			return nil, err
+		}
+
+		feeTokenRecords = append(feeTokenRecords, types.FeeToken{
+			Denom:  denom,
+			PoolID: uint64(poolId),
+		})
+
+		// increase counter by the next 2
+		i = i + 2
+	}
+
+	return feeTokenRecords, nil
+}
+
+func parseFeeTokenRecordsArgsToContent(cmd *cobra.Command) (govtypes.Content, error) {
+	title, err := cmd.Flags().GetString(cli.FlagTitle)
+	if err != nil {
+		return nil, err
+	}
+
+	description, err := cmd.Flags().GetString(cli.FlagDescription)
+	if err != nil {
+		return nil, err
+	}
+
+	feeTokenRecords, err := parseFeeTokenRecords(cmd)
+	if err != nil {
+		return nil, err
+	}
+
+	content := &types.UpdateFeeTokenProposal{
+		Title:       title,
+		Description: description,
+		Feetokens:   feeTokenRecords,
+	}
+	return content, nil
+}

x/txfees/keeper/gov.go

@@ -8,5 +8,10 @@ import (
 
 func (k Keeper) HandleUpdateFeeTokenProposal(ctx sdk.Context, p *types.UpdateFeeTokenProposal) error {
 	// setFeeToken internally calls ValidateFeeToken
-	return k.setFeeToken(ctx, p.Feetoken)
+	for _, feeToken := range p.Feetokens {
+		if err := k.setFeeToken(ctx, feeToken); err != nil {
+			return err
+		}
+	}
+	return nil
 }

x/txfees/keeper/keeper_test.go

@@ -55,9 +55,11 @@ func (s *KeeperTestSuite) ExecuteUpgradeFeeTokenProposal(feeToken string, poolId
 	upgradeProp := types.NewUpdateFeeTokenProposal(
 		"Test Proposal",
 		"test",
-		types.FeeToken{
-			Denom:  feeToken,
-			PoolID: poolId,
+		[]types.FeeToken{
+			{
+				Denom:  feeToken,
+				PoolID: poolId,
+			},
 		},
 	)
 	return s.App.TxFeesKeeper.HandleUpdateFeeTokenProposal(s.Ctx, &upgradeProp)

x/txfees/types/gov.go

@@ -19,11 +19,11 @@ func init() {
 
 var _ govtypes.Content = &UpdateFeeTokenProposal{}
 
-func NewUpdateFeeTokenProposal(title, description string, feeToken FeeToken) UpdateFeeTokenProposal {
+func NewUpdateFeeTokenProposal(title, description string, feeTokens []FeeToken) UpdateFeeTokenProposal {
 	return UpdateFeeTokenProposal{
 		Title:       title,
 		Description: description,
-		Feetoken:    feeToken,
+		Feetokens:   feeTokens,
 	}
 }
 
@@ -43,15 +43,25 @@ func (p *UpdateFeeTokenProposal) ValidateBasic() error {
 		return err
 	}
 
-	return sdk.ValidateDenom(p.Feetoken.Denom)
+	for _, feeToken := range p.Feetokens {
+		if err := sdk.ValidateDenom(feeToken.Denom); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 func (p UpdateFeeTokenProposal) String() string {
+	recordsStr := ""
+	for _, feeToken := range p.Feetokens {
+		recordsStr = recordsStr + fmt.Sprintf("(Denom: %s, PoolID: %d) ", feeToken.Denom, feeToken.PoolID)
+	}
+
 	var b strings.Builder
 	b.WriteString(fmt.Sprintf(`Update Fee Token Proposal:
   Title:       %s
   Description: %s
   Records:     %s
-`, p.Title, p.Description, p.Feetoken.String()))
+`, p.Title, p.Description, recordsStr))
 	return b.String()
 }