feat(superfluid/gov): configure unpool whitelist via gov prop (#3512) (…

…#3531)

* feat(superfluid/gov): configure unpool whitelist via gov prop

* cli and amino registrateions

* linter

* Update x/superfluid/keeper/gov/suite_test.go

* Update x/superfluid/keeper/gov/suite_test.go

* add tests for GetIfFound (#3145)

* add tests for GetIfFound

* refactor

* cli fix

* cli updates

* remove message not activated

* Update x/superfluid/types/gov.go

Co-authored-by: Dev Ojha <[email protected]>

* correct CLI wording

* GetIfFound godoc

Co-authored-by: Hieu Vu <[email protected]>
Co-authored-by: Dev Ojha <[email protected]>
(cherry picked from commit 404cf9b)

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

app/keepers/keepers.go

@@ -362,7 +362,7 @@ func (appKeepers *AppKeepers) InitNormalKeepers(
 		AddRoute(ibchost.RouterKey, ibcclient.NewClientProposalHandler(appKeepers.IBCKeeper.ClientKeeper)).
 		AddRoute(poolincentivestypes.RouterKey, poolincentives.NewPoolIncentivesProposalHandler(*appKeepers.PoolIncentivesKeeper)).
 		AddRoute(txfeestypes.RouterKey, txfees.NewUpdateFeeTokenProposalHandler(*appKeepers.TxFeesKeeper)).
-		AddRoute(superfluidtypes.RouterKey, superfluid.NewSuperfluidProposalHandler(*appKeepers.SuperfluidKeeper, *appKeepers.EpochsKeeper))
+		AddRoute(superfluidtypes.RouterKey, superfluid.NewSuperfluidProposalHandler(*appKeepers.SuperfluidKeeper, *appKeepers.EpochsKeeper, *appKeepers.GAMMKeeper))
 
 	// The gov proposal types can be individually enabled
 	if len(wasmEnabledProposals) != 0 {

app/keepers/modules.go

@@ -66,6 +66,7 @@ var AppModuleBasics = []module.AppModuleBasic{
 			ibcclientclient.UpgradeProposalHandler,
 			superfluidclient.SetSuperfluidAssetsProposalHandler,
 			superfluidclient.RemoveSuperfluidAssetsProposalHandler,
+			superfluidclient.UpdateUnpoolWhitelistProposalHandler,
 		)...,
 	),
 	params.AppModuleBasic{},

osmoutils/store_helper.go

@@ -137,3 +137,17 @@ func MustGetDec(store store.KVStore, key []byte) sdk.Dec {
 	MustGet(store, key, result)
 	return result.Dec
 }
+
+// GetIfFound returns a value at key by mutating the result parameter. Returns true if the value was found and the
+// result mutated correctly. False otherwise. Returns error only when database or serialization errors occur
+// returns a boolean indicating whether value exists for the given key and error
+func GetIfFound(store store.KVStore, key []byte, result proto.Message) (found bool, err error) {
+	b := store.Get(key)
+	if b == nil {
+		return false, nil
+	}
+	if err := proto.Unmarshal(b, result); err != nil {
+		return true, err
+	}
+	return true, nil
+}

osmoutils/store_helper_test.go

@@ -811,6 +811,98 @@ func (s *TestSuite) TestMustGet() {
 	}
 }
 
+// TestMustGet tests that GetIfFound returns a boolean indicating 
+// whether value exists for the given key and error
+func (s *TestSuite) TestGetIfFound() {
+	tests := map[string]struct {
+		// keys and values to preset
+		preSetKeyValues map[string]proto.Message
+
+		// keys and values to attempt to get and validate
+		expectedGetKeyValues map[string]proto.Message
+
+		actualResultProto proto.Message
+
+		expectFound bool
+
+		expectErr bool
+	}{
+		"basic valid test": {
+			preSetKeyValues: map[string]proto.Message{
+				keyA: &sdk.DecProto{Dec: sdk.OneDec()},
+				keyB: &sdk.DecProto{Dec: sdk.OneDec().Add(sdk.OneDec())},
+				keyC: &sdk.DecProto{Dec: sdk.OneDec().Add(sdk.OneDec())},
+			},
+
+			expectedGetKeyValues: map[string]proto.Message{
+				keyA: &sdk.DecProto{Dec: sdk.OneDec()},
+				keyB: &sdk.DecProto{Dec: sdk.OneDec().Add(sdk.OneDec())},
+				keyC: &sdk.DecProto{Dec: sdk.OneDec().Add(sdk.OneDec())},
+			},
+
+			actualResultProto: &sdk.DecProto{},
+
+			expectFound: true,
+		},
+		"attempt to get non-existent key - not found & no err return": {
+			preSetKeyValues: map[string]proto.Message{
+				keyA: &sdk.DecProto{Dec: sdk.OneDec()},
+				keyC: &sdk.DecProto{Dec: sdk.OneDec().Add(sdk.OneDec())},
+			},
+
+			expectedGetKeyValues: map[string]proto.Message{
+				keyB: &sdk.DecProto{Dec: sdk.OneDec().Add(sdk.OneDec())},
+			},
+
+			actualResultProto: &sdk.DecProto{},
+
+			expectFound: false,
+
+			expectErr: false,
+		},
+		"invalid proto Dec vs TwapRecord - found but Unmarshal err": {
+			preSetKeyValues: map[string]proto.Message{
+				keyA: &sdk.DecProto{Dec: sdk.OneDec()},
+			},
+
+			expectedGetKeyValues: map[string]proto.Message{
+				keyA: &sdk.DecProto{Dec: sdk.OneDec()},
+			},
+
+			actualResultProto: &twaptypes.TwapRecord{},
+
+			expectFound: true,
+
+			expectErr: true,
+		},
+	}
+
+	for name, tc := range tests {
+		s.Run(name, func() {
+			s.SetupStoreWithBasePrefix()
+
+			// Setup
+			for key, value := range tc.preSetKeyValues {
+				osmoutils.MustSet(s.store, []byte(key), value)
+			}
+
+			for key, expectedValue := range tc.expectedGetKeyValues {
+				// System under test.
+				found, err := osmoutils.GetIfFound(s.store, []byte(key), tc.actualResultProto)
+				// Assertions.
+				s.Require().Equal(found, tc.expectFound)
+				if tc.expectErr {
+					s.Require().Error(err)
+				} 
+				// make sure found by key & Unmarshal successfully
+				if !tc.expectErr && tc.expectFound {
+					s.Require().Equal(expectedValue.String(), tc.actualResultProto.String())
+				}
+			}
+		})
+	}
+}
+
 // TestMustSet tests that MustSet updates the store correctly
 // and panics if an error is encountered.
 func (s *TestSuite) TestMustSet() {

proto/osmosis/superfluid/v1beta1/gov.proto

@@ -29,3 +29,16 @@ message RemoveSuperfluidAssetsProposal {
   string description = 2;
   repeated string superfluid_asset_denoms = 3;
 }
+
+// UpdateUnpoolWhiteListProposal is a gov Content type to update the
+// allowed list of pool ids.
+message UpdateUnpoolWhiteListProposal {
+  option (gogoproto.equal) = true;
+  option (gogoproto.goproto_getters) = false;
+  option (gogoproto.goproto_stringer) = false;
+
+  string title = 1;
+  string description = 2;
+  repeated uint64 ids = 3;
+  bool is_overwrite = 4;
+}

x/superfluid/client/cli/flags.go

@@ -3,4 +3,6 @@ package cli
 // Proposal flags.
 const (
 	FlagSuperfluidAssets = "superfluid-assets"
+	FlagPoolIds          = "pool-ids"
+	FlagOverwrite        = "is-overwrite"
 )

x/superfluid/client/cli/tx.go

@@ -5,8 +5,11 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/osmosis-labs/osmosis/v13/x/superfluid/types"
 	"github.com/spf13/cobra"
+	flag "github.com/spf13/pflag"
+
+	"github.com/osmosis-labs/osmosis/v13/osmoutils"
+	"github.com/osmosis-labs/osmosis/v13/x/superfluid/types"
 
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/flags"
@@ -402,3 +405,92 @@ func NewCmdUnPoolWhitelistedPool() *cobra.Command {
 	flags.AddTxFlagsToCmd(cmd)
 	return cmd
 }
+
+// NewCmdUpdateUnpoolWhitelistProposal defines the command to create a new update unpool whitelist proposal command.
+func NewCmdUpdateUnpoolWhitelistProposal() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:   "update-unpool-whitelist [flags]",
+		Args:  cobra.ExactArgs(0),
+		Short: "Update unpool whitelist proposal",
+		Long: "This proposal will update the unpool whitelist if passed. " +
+			"Every pool id must be valid. If the pool id is invalid, the proposal will not be submitted. " +
+			"If the flag to overwrite is set, the whitelist is completely overridden. Otherwise, it is appended to the existing whitelist, having all duplicates removed.",
+		Example: "osmosisd tx gov submit-proposal update-unpool-whitelist --pool-ids \"1, 2, 3\" --title \"Title\" --description \"Description\"",
+		RunE: func(cmd *cobra.Command, args []string) error {
+			clientCtx, err := client.GetClientTxContext(cmd)
+			if err != nil {
+				return err
+			}
+			content, err := parseUpdateUnpoolWhitelistArgsToContent(cmd.Flags())
+			if err != nil {
+				return err
+			}
+
+			from := clientCtx.GetFromAddress()
+
+			depositStr, err := cmd.Flags().GetString(govcli.FlagDeposit)
+			if err != nil {
+				return err
+			}
+
+			deposit, err := sdk.ParseCoinsNormalized(depositStr)
+			if err != nil {
+				return err
+			}
+
+			msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from)
+			if err != nil {
+				return err
+			}
+
+			if err = msg.ValidateBasic(); err != nil {
+				return err
+			}
+
+			return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
+		},
+	}
+
+	cmd.Flags().String(govcli.FlagTitle, "", "title of proposal")
+	cmd.Flags().String(govcli.FlagDescription, "", "description of proposal")
+	cmd.Flags().String(govcli.FlagDeposit, "", "deposit of proposal")
+	cmd.Flags().String(FlagPoolIds, "", "The new pool id whitelist to set")
+	cmd.Flags().Bool(FlagOverwrite, false, "The flag indicating whether to overwrite the whitelist or append to it")
+
+	return cmd
+}
+
+func parseUpdateUnpoolWhitelistArgsToContent(flags *flag.FlagSet) (govtypes.Content, error) {
+	title, err := flags.GetString(govcli.FlagTitle)
+	if err != nil {
+		return nil, err
+	}
+
+	description, err := flags.GetString(govcli.FlagDescription)
+	if err != nil {
+		return nil, err
+	}
+
+	poolIdsStr, err := flags.GetString(FlagPoolIds)
+	if err != nil {
+		return nil, err
+	}
+
+	poolIds, err := osmoutils.ParseUint64SliceFromString(poolIdsStr, ",")
+	if err != nil {
+		return nil, err
+	}
+
+	isOverwrite, err := flags.GetBool(FlagOverwrite)
+	if err != nil {
+		return nil, err
+	}
+
+	content := &types.UpdateUnpoolWhiteListProposal{
+		Title:       title,
+		Description: description,
+		Ids:         poolIds,
+		IsOverwrite: isOverwrite,
+	}
+	return content, nil
+}

x/superfluid/client/proposal_handler.go

@@ -10,4 +10,5 @@ import (
 var (
 	SetSuperfluidAssetsProposalHandler    = govclient.NewProposalHandler(cli.NewCmdSubmitSetSuperfluidAssetsProposal, rest.ProposalSetSuperfluidAssetsRESTHandler)
 	RemoveSuperfluidAssetsProposalHandler = govclient.NewProposalHandler(cli.NewCmdSubmitRemoveSuperfluidAssetsProposal, rest.ProposalRemoveSuperfluidAssetsRESTHandler)
+	UpdateUnpoolWhitelistProposalHandler  = govclient.NewProposalHandler(cli.NewCmdUpdateUnpoolWhitelistProposal, rest.ProposalUpdateUnpoolWhitelistProposal)
 )

x/superfluid/client/rest/tx.go

@@ -10,23 +10,25 @@ import (
 func ProposalSetSuperfluidAssetsRESTHandler(clientCtx client.Context) govrest.ProposalRESTHandler {
 	return govrest.ProposalRESTHandler{
 		SubRoute: "set-superfluid-assets",
-		Handler:  newSetSuperfluidAssetsHandler(clientCtx),
+		Handler:  emptyHandler(clientCtx),
 	}
 }
 
-func newSetSuperfluidAssetsHandler(clientCtx client.Context) http.HandlerFunc {
-	return func(w http.ResponseWriter, r *http.Request) {
+func ProposalRemoveSuperfluidAssetsRESTHandler(clientCtx client.Context) govrest.ProposalRESTHandler {
+	return govrest.ProposalRESTHandler{
+		SubRoute: "remove-superfluid-assets",
+		Handler:  emptyHandler(clientCtx),
 	}
 }
 
-func ProposalRemoveSuperfluidAssetsRESTHandler(clientCtx client.Context) govrest.ProposalRESTHandler {
+func ProposalUpdateUnpoolWhitelistProposal(clientCtx client.Context) govrest.ProposalRESTHandler {
 	return govrest.ProposalRESTHandler{
-		SubRoute: "remove-superfluid-assets",
-		Handler:  newRemoveSuperfluidAssetsHandler(clientCtx),
+		SubRoute: "update-unpool-whitelist",
+		Handler:  emptyHandler(clientCtx),
 	}
 }
 
-func newRemoveSuperfluidAssetsHandler(clientCtx client.Context) http.HandlerFunc {
+func emptyHandler(clientCtx client.Context) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 	}
 }

x/superfluid/keeper/gov/gov.go

@@ -1,7 +1,9 @@
 package gov
 
 import (
+	"errors"
 	"fmt"
+	"sort"
 
 	"github.com/osmosis-labs/osmosis/v13/x/superfluid/keeper"
 	"github.com/osmosis-labs/osmosis/v13/x/superfluid/keeper/internal/events"
@@ -32,3 +34,45 @@ func HandleRemoveSuperfluidAssetsProposal(ctx sdk.Context, k keeper.Keeper, p *t
 	}
 	return nil
 }
+
+// HandleUnpoolWhiteListChange handles the unpool whitelist change proposal. It validates that every new pool id exists. Fails if not.
+// If IsOverwrite flag is set, the whitelist is completely overridden. Otherwise, it is merged with pre-existing whitelisted pool ids.
+// Any duplicates are removed and the pool ids are sorted prior to being written to state.
+// Returns nil on success, error on failure.
+func HandleUnpoolWhiteListChange(ctx sdk.Context, k keeper.Keeper, gammKeeper types.GammKeeper, p *types.UpdateUnpoolWhiteListProposal) error {
+	allPoolIds := make([]uint64, 0, len(p.Ids))
+
+	// if overwrite flag is not set, we merge the old white list with the
+	// newly added pool ids.
+	if !p.IsOverwrite {
+		allPoolIds = append(allPoolIds, k.GetUnpoolAllowedPools(ctx)...)
+	}
+
+	for _, newId := range p.Ids {
+		if newId == 0 {
+			return errors.New("pool id 0 is not allowed. Pool ids start from 0")
+		}
+
+		if _, err := gammKeeper.GetPoolAndPoke(ctx, newId); err != nil {
+			return fmt.Errorf("failed to get pool with id (%d), likely does not exist: %w", newId, err)
+		}
+		allPoolIds = append(allPoolIds, newId)
+	}
+
+	// Sort
+	sort.Slice(allPoolIds, func(i, j int) bool {
+		return allPoolIds[i] < allPoolIds[j]
+	})
+
+	// Remove duplicates, if any
+	duplicatesRemovedIds := make([]uint64, 0, len(allPoolIds))
+	for i, curId := range allPoolIds {
+		if i < len(allPoolIds)-1 && curId == allPoolIds[i+1] {
+			continue
+		}
+		duplicatesRemovedIds = append(duplicatesRemovedIds, curId)
+	}
+
+	k.SetUnpoolAllowedPools(ctx, duplicatesRemovedIds)
+	return nil
+}