feat: whitelist addresses param for setting fee tokens (#7855) (#7908)

* whitelist address for setting fee tokens

* register subspace and codec

* changelog

* add params to genesis, add test case for SenderValidationSetFeeTokens

* add genesis test and TestMsgSetFeeTokens

* add DAO address to upgrade handler

* use osmoutils helper

(cherry picked from commit f3e311a)

Co-authored-by: Adam Tucker <[email protected]>

CHANGELOG.md

@@ -79,6 +79,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#7833](https://github.com/osmosis-labs/osmosis/pull/7883) Bump max gas wanted per tx to 6 mil
 * [#7839](https://github.com/osmosis-labs/osmosis/pull/7839) Add ICA controller
 * [#7527](https://github.com/osmosis-labs/osmosis/pull/7527) Add 30M gas limit to CW pool contract calls
+* [#7855](https://github.com/osmosis-labs/osmosis/pull/7855) Whitelist address parameter for setting fee tokens
 * [#7857](https://github.com/osmosis-labs/osmosis/pull/7857) SuperfluidDelegationsByValidatorDenom query now returns equivalent staked amount
 
 ### SDK
@@ -90,6 +91,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#514](https://github.com/osmosis-labs/cosmos-sdk/pull/514) Let gov hooks return an error
 * [#580](https://github.com/osmosis-labs/cosmos-sdk/pull/580) Less time intensive slashing migration
 
+### CometBFT
+
+* [#5](https://github.com/osmosis-labs/cometbft/pull/5) Batch verification
+* [#11](https://github.com/osmosis-labs/cometbft/pull/11) Skip verification of commit sigs
+* [#13](https://github.com/osmosis-labs/cometbft/pull/13) Avoid double-saving ABCI responses
+* [#20](https://github.com/osmosis-labs/cometbft/pull/20) Fix the rollback command
+
 ## v23.0.8-iavl-v1 & v23.0.8
 
 * [#7769](https://github.com/osmosis-labs/osmosis/pull/7769) Set and default timeout commit to 3s. Add flag to prevent custom overrides if not desired.

app/keepers/keepers.go

@@ -439,6 +439,7 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
 		appKeepers.DistrKeeper,
 		appKeepers.ConsensusParamsKeeper,
 		dataDir,
+		appKeepers.GetSubspace(txfeestypes.ModuleName),
 	)
 	appKeepers.TxFeesKeeper = &txFeesKeeper
 
@@ -749,6 +750,7 @@ func (appKeepers *AppKeepers) initParamsKeeper(appCodec codec.BinaryCodec, legac
 	paramsKeeper.Subspace(packetforwardtypes.ModuleName).WithKeyTable(packetforwardtypes.ParamKeyTable())
 	paramsKeeper.Subspace(cosmwasmpooltypes.ModuleName)
 	paramsKeeper.Subspace(ibchookstypes.ModuleName)
+	paramsKeeper.Subspace(txfeestypes.ModuleName)
 
 	return paramsKeeper
 }

app/upgrades/v24/constants.go

@@ -3,6 +3,7 @@ package v24
 import (
 	ibcwasmtypes "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types"
 	icacontrollertypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/types"
+
 	"github.com/osmosis-labs/osmosis/v24/app/upgrades"
 
 	store "github.com/cosmos/cosmos-sdk/store/types"
@@ -20,6 +21,8 @@ var Upgrade = upgrades.Upgrade{
 	},
 }
 
+var WhitelistedFeeTokenSetters = []string{"osmo17eqe9dpglajwd48r65lasq3mftra5q4uxce525htyvjdp0q037vqpurhve"}
+
 var FinalIncentiveAccumulatorPoolIDsToMigrate = map[uint64]struct{}{
 	// token0 ibc/E7905742CE2EA4EA5D592527DC89220C59B617DE803939FE7293805A64B484D7
 	// token1 uosmo

app/upgrades/v24/upgrades.go

@@ -14,6 +14,7 @@ import (
 	concentratedliquidity "github.com/osmosis-labs/osmosis/v24/x/concentrated-liquidity"
 	cwpooltypes "github.com/osmosis-labs/osmosis/v24/x/cosmwasmpool/types"
 	incentivestypes "github.com/osmosis-labs/osmosis/v24/x/incentives/types"
+	txfeestypes "github.com/osmosis-labs/osmosis/v24/x/txfees/types"
 )
 
 const (
@@ -98,6 +99,8 @@ func CreateUpgradeHandler(
 			keepers.CosmwasmPoolKeeper.SetPool(ctx, cwPool)
 		}
 
+		// Set whitelistedFeeTokenSetters param as per https://forum.osmosis.zone/t/temperature-check-add-a-permissioned-address-to-manage-the-fee-token-whitelist/2604
+		keepers.TxFeesKeeper.SetParam(ctx, txfeestypes.KeyWhitelistedFeeTokenSetters, WhitelistedFeeTokenSetters)
 		return migrations, nil
 	}
 }

app/upgrades/v24/upgrades_test.go

@@ -256,6 +256,14 @@ func (s *UpgradeTestSuite) TestUpgrade() {
 
 	// Test that the white whale pools have been updated
 	s.requirePoolsHaveCodeId(whiteWhalePoolIds, 572)
+
+	// TXFEES Tests
+	//
+
+	// Check that the whitelisted fee token address has been set
+	whitelistedFeeTokenSetters := s.App.TxFeesKeeper.GetParams(s.Ctx).WhitelistedFeeTokenSetters
+	s.Require().Len(whitelistedFeeTokenSetters, 1)
+	s.Require().Equal(whitelistedFeeTokenSetters, v24.WhitelistedFeeTokenSetters)
 }
 
 func (s *UpgradeTestSuite) requirePoolsHaveCodeId(pools []uint64, codeId uint64) {

proto/osmosis/txfees/v1beta1/genesis.proto

@@ -4,6 +4,7 @@ package osmosis.txfees.v1beta1;
 import "gogoproto/gogo.proto";
 import "osmosis/txfees/v1beta1/feetoken.proto";
 import "cosmos/base/v1beta1/coin.proto";
+import "osmosis/txfees/v1beta1/params.proto";
 
 option go_package = "github.com/osmosis-labs/osmosis/v24/x/txfees/types";
 
@@ -16,4 +17,7 @@ message GenesisState {
   // TxFeesTracker txFeesTracker = 3;
   reserved 3;
   reserved "txFeesTracker";
+
+  // params is the container of txfees parameters.
+  Params params = 4 [ (gogoproto.nullable) = false ];
 }

proto/osmosis/txfees/v1beta1/params.proto

@@ -0,0 +1,14 @@
+syntax = "proto3";
+package osmosis.txfees.v1beta1;
+
+import "gogoproto/gogo.proto";
+
+option go_package = "github.com/osmosis-labs/osmosis/v24/x/txfees/types";
+
+// Params holds parameters for the txfees module
+message Params {
+  repeated string whitelisted_fee_token_setters = 1 [
+    (gogoproto.moretags) = "yaml:\"whitelisted_fee_token_setters\"",
+    (gogoproto.nullable) = false
+  ];
+}

proto/osmosis/txfees/v1beta1/tx.proto

@@ -0,0 +1,25 @@
+syntax = "proto3";
+package osmosis.txfees.v1beta1;
+
+import "gogoproto/gogo.proto";
+import "amino/amino.proto";
+import "osmosis/txfees/v1beta1/feetoken.proto";
+
+option go_package = "github.com/osmosis-labs/osmosis/v24/x/txfees/types";
+
+service Msg {
+  rpc SetFeeTokens(MsgSetFeeTokens) returns (MsgSetFeeTokensResponse);
+}
+
+// ===================== MsgSetFeeTokens
+message MsgSetFeeTokens {
+  option (amino.name) = "osmosis/set-fee-tokens";
+
+  repeated FeeToken fee_tokens = 1 [
+    (gogoproto.moretags) = "yaml:\"fee_tokens\"",
+    (gogoproto.nullable) = false
+  ];
+  string sender = 2 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
+}
+
+message MsgSetFeeTokensResponse {}

x/txfees/client/cli/query.go

@@ -4,6 +4,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/osmosis-labs/osmosis/osmoutils/osmocli"
+	"github.com/osmosis-labs/osmosis/v24/x/twap/client/queryproto"
 	"github.com/osmosis-labs/osmosis/v24/x/txfees/types"
 )
 
@@ -15,6 +16,8 @@ func GetQueryCmd() *cobra.Command {
 		GetCmdFeeTokens(),
 		GetCmdDenomPoolID(),
 		GetCmdBaseDenom(),
+		osmocli.GetParams[*queryproto.ParamsRequest](
+			types.ModuleName, queryproto.NewQueryClient),
 	)
 
 	osmocli.AddQueryCmd(cmd, types.NewQueryClient, GetCmdQueryBaseFee)

x/txfees/keeper/feetokens.go

@@ -9,6 +9,8 @@ import (
 	"github.com/osmosis-labs/osmosis/v24/x/txfees/types"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	osmoutils "github.com/osmosis-labs/osmosis/osmoutils"
 )
 
 // ConvertToBaseToken converts a fee amount in a whitelisted fee token to the base fee token amount.
@@ -186,3 +188,17 @@ func (k Keeper) SetFeeTokens(ctx sdk.Context, feetokens []types.FeeToken) error
 	}
 	return nil
 }
+
+// SenderValidationSetFeeTokens first checks to see if the sender is whitelisted to set fee tokens.
+// If the sender is whitelisted, it sets the fee tokens.
+// If the sender is not whitelisted, it returns an error.
+func (k Keeper) SenderValidationSetFeeTokens(ctx sdk.Context, sender string, feetokens []types.FeeToken) error {
+	whitelistedAddresses := k.GetParams(ctx).WhitelistedFeeTokenSetters
+
+	isWhitelisted := osmoutils.Contains(whitelistedAddresses, sender)
+	if !isWhitelisted {
+		return errorsmod.Wrapf(types.ErrNotWhitelistedFeeTokenSetter, "%s", sender)
+	}
+
+	return k.SetFeeTokens(ctx, feetokens)
+}

x/txfees/keeper/feetokens_test.go

@@ -1,6 +1,8 @@
 package keeper_test
 
 import (
+	"fmt"
+
 	"github.com/osmosis-labs/osmosis/osmomath"
 	"github.com/osmosis-labs/osmosis/v24/x/txfees/types"
 
@@ -281,3 +283,98 @@ func (s *KeeperTestSuite) TestFeeTokenConversions() {
 		})
 	}
 }
+
+func (s *KeeperTestSuite) TestSenderValidationSetFeeTokens() {
+	s.SetupTest(false)
+
+	baseDenom, _ := s.App.TxFeesKeeper.GetBaseDenom(s.Ctx)
+
+	tests := []struct {
+		name              string
+		isWhitelistedAddr bool
+		prepareFeePools   bool
+		feeTokensToSet    []types.FeeToken
+		expectedError     error
+	}{
+		{
+			name: "Set multiple fee tokens with whitelisted address",
+			feeTokensToSet: []types.FeeToken{
+				{Denom: "foo", PoolID: 1},
+				{Denom: "bar", PoolID: 2},
+			},
+			prepareFeePools:   true,
+			isWhitelistedAddr: true,
+		},
+		{
+			name: "Set single fee token with whitelisted address",
+			feeTokensToSet: []types.FeeToken{
+				{Denom: "foo", PoolID: 1},
+			},
+			prepareFeePools:   true,
+			isWhitelistedAddr: true,
+		},
+		{
+			name: "Error: Set multiple fee tokens with non-whitelisted address",
+			feeTokensToSet: []types.FeeToken{
+				{Denom: "foo", PoolID: 1},
+				{Denom: "bar", PoolID: 2},
+			},
+			prepareFeePools:   true,
+			isWhitelistedAddr: false,
+			expectedError:     types.ErrNotWhitelistedFeeTokenSetter,
+		},
+		{
+			name: "Error: Set single fee token with non-whitelisted address",
+			feeTokensToSet: []types.FeeToken{
+				{Denom: "foo", PoolID: 1},
+			},
+			prepareFeePools:   true,
+			isWhitelistedAddr: false,
+			expectedError:     types.ErrNotWhitelistedFeeTokenSetter,
+		},
+		{
+			name: "Error: Set single fee token with whitelisted address with fee pool not set",
+			feeTokensToSet: []types.FeeToken{
+				{Denom: "foo", PoolID: 1},
+			},
+			prepareFeePools:   false,
+			isWhitelistedAddr: true,
+			expectedError:     fmt.Errorf("failed to find route for pool id (1)"),
+		},
+	}
+
+	for _, tc := range tests {
+		s.SetupTest(false)
+
+		s.Run(tc.name, func() {
+			if tc.prepareFeePools {
+				for _, feeToken := range tc.feeTokensToSet {
+					s.PrepareBalancerPoolWithCoins(sdk.NewInt64Coin(baseDenom, 100), sdk.NewInt64Coin(feeToken.Denom, 100))
+				}
+			}
+
+			if tc.isWhitelistedAddr {
+				s.App.TxFeesKeeper.SetParam(s.Ctx, types.KeyWhitelistedFeeTokenSetters, []string{s.TestAccs[0].String()})
+			}
+
+			// Retrieve fee tokens before setting
+			feeTokensBefore := s.App.TxFeesKeeper.GetFeeTokens(s.Ctx)
+
+			err := s.App.TxFeesKeeper.SenderValidationSetFeeTokens(s.Ctx, s.TestAccs[0].String(), tc.feeTokensToSet)
+
+			// Retrieve fee tokens after setting
+			feeTokensAfter := s.App.TxFeesKeeper.GetFeeTokens(s.Ctx)
+
+			if tc.expectedError != nil {
+				s.Require().Error(err)
+				s.Require().ErrorContains(err, tc.expectedError.Error())
+				// Ensure that the fee tokens are the same
+				s.Require().Equal(len(feeTokensAfter), len(feeTokensBefore))
+			} else {
+				s.Require().NoError(err)
+				// Ensure that the fee tokens now include the new fee tokens
+				s.Require().Equal(len(feeTokensAfter), len(feeTokensBefore)+len(tc.feeTokensToSet))
+			}
+		})
+	}
+}

x/txfees/keeper/genesis.go

@@ -17,12 +17,14 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState types.GenesisState) {
 	if err != nil {
 		panic(err)
 	}
+	k.SetParams(ctx, genState.Params)
 }
 
 // ExportGenesis returns the txfees module's exported genesis.
 func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 	genesis := types.DefaultGenesis()
 	genesis.Basedenom, _ = k.GetBaseDenom(ctx)
 	genesis.Feetokens = k.GetFeeTokens(ctx)
+	genesis.Params = k.GetParams(ctx)
 	return genesis
 }

x/txfees/keeper/genesis_test.go

@@ -18,6 +18,7 @@ var (
 			PoolID: 2,
 		},
 	}
+	testWhitelistAddrs = []string{"osmo106x8q2nv7xsg7qrec2zgdf3vvq0t3gn49zvaha", "osmo105l5r3rjtynn7lg362r2m9hkpfvmgmjtkglsn9"}
 )
 
 func (s *KeeperTestSuite) TestInitGenesis() {
@@ -28,13 +29,18 @@ func (s *KeeperTestSuite) TestInitGenesis() {
 	s.App.TxFeesKeeper.InitGenesis(s.Ctx, types.GenesisState{
 		Basedenom: testBaseDenom,
 		Feetokens: testFeeTokens,
+		Params: types.Params{
+			WhitelistedFeeTokenSetters: testWhitelistAddrs,
+		},
 	})
 
 	actualBaseDenom, err := s.App.TxFeesKeeper.GetBaseDenom(s.Ctx)
 	s.Require().NoError(err)
-
 	s.Require().Equal(testBaseDenom, actualBaseDenom)
 	s.Require().Equal(testFeeTokens, s.App.TxFeesKeeper.GetFeeTokens(s.Ctx))
+
+	actualParams := s.App.TxFeesKeeper.GetParams(s.Ctx)
+	s.Require().Equal(testWhitelistAddrs, actualParams.WhitelistedFeeTokenSetters)
 }
 
 func (s *KeeperTestSuite) TestExportGenesis() {
@@ -45,9 +51,13 @@ func (s *KeeperTestSuite) TestExportGenesis() {
 	s.App.TxFeesKeeper.InitGenesis(s.Ctx, types.GenesisState{
 		Basedenom: testBaseDenom,
 		Feetokens: testFeeTokens,
+		Params: types.Params{
+			WhitelistedFeeTokenSetters: testWhitelistAddrs,
+		},
 	})
 
 	genesis := s.App.TxFeesKeeper.ExportGenesis(s.Ctx)
 	s.Require().Equal(testBaseDenom, genesis.Basedenom)
 	s.Require().Equal(testFeeTokens, genesis.Feetokens)
+	s.Require().Equal(testWhitelistAddrs, genesis.Params.WhitelistedFeeTokenSetters)
 }