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

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

Merged
merged 13 commits into from
Nov 26, 2022
2 changes: 1 addition & 1 deletion app/keepers/keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,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 {
Expand Down
1 change: 1 addition & 0 deletions app/keepers/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ var AppModuleBasics = []module.AppModuleBasic{
ibcclientclient.UpgradeProposalHandler,
superfluidclient.SetSuperfluidAssetsProposalHandler,
superfluidclient.RemoveSuperfluidAssetsProposalHandler,
superfluidclient.UpdateUnpoolWhitelistProposalHandler,
)...,
),
params.AppModuleBasic{},
Expand Down
14 changes: 14 additions & 0 deletions osmoutils/store_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

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

Can be changed later / can discuss in an issue, should not block this PR. Perhaps this should be called HasWithType ?

e.g. if we were masters of the store interface

store.HasProtoMsg("myKey", types.Pool)

Has to indicate its a bool (rather than assuming you'd get the object back)

OR is proto.Message here actually mutated? If so lets update the comment, but this doesn't actually make sense to me as an API.

Why don't just make a method called Get thats the same as MustGet, but returns an error instead of panics?

b := store.Get(key)
if b == nil {
return false, nil
}
if err := proto.Unmarshal(b, result); err != nil {
return true, err
}
return true, nil
}
92 changes: 92 additions & 0 deletions osmoutils/store_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
13 changes: 13 additions & 0 deletions proto/osmosis/superfluid/v1beta1/gov.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
2 changes: 2 additions & 0 deletions x/superfluid/client/cli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ package cli
// Proposal flags.
const (
FlagSuperfluidAssets = "superfluid-assets"
FlagPoolIds = "pool-ids"
FlagOverwrite = "is-overwrite"
)
94 changes: 93 additions & 1 deletion x/superfluid/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
1 change: 1 addition & 0 deletions x/superfluid/client/proposal_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
16 changes: 9 additions & 7 deletions x/superfluid/client/rest/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,25 @@ import (
func ProposalSetSuperfluidAssetsRESTHandler(clientCtx client.Context) govrest.ProposalRESTHandler {
return govrest.ProposalRESTHandler{
SubRoute: "set-superfluid-assets",
Handler: newSetSuperfluidAssetsHandler(clientCtx),
Copy link
Member

@ValarDragon ValarDragon Nov 25, 2022

Choose a reason for hiding this comment

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

idk whats going on here, but IIRC this REST folder doesn't matter for anything / should be deleted, so not blocking

Copy link
Member Author

Choose a reason for hiding this comment

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

For each of these, the handler is the same - no-op. I just made a shared no-op function for all of these but yes it's unimportant

Copy link
Member

@ValarDragon ValarDragon Nov 25, 2022

Choose a reason for hiding this comment

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

ah that makes sense! Nice :)

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) {
}
}
44 changes: 44 additions & 0 deletions x/superfluid/keeper/gov/gov.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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
}
Loading