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

modify pool-incentives submit proposal cli #2086

Merged
merged 8 commits into from
Aug 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Breaking Changes

* [#2086](https://github.com/osmosis-labs/osmosis/pull/2086) `ReplacePoolIncentivesProposal` ProposalType() returns ProposalTypeReplacePoolIncentives instead of ProposalTypeUpdatePoolIncentives
* [#2222](https://github.com/osmosis-labs/osmosis/pull/2222) Add scaling factors to MsgCreateStableswapPool
* [#1889](https://github.com/osmosis-labs/osmosis/pull/1825) Add proto responses to gamm LP messages:
* MsgJoinPoolResponse: share_out_amount and token_in fields
Expand Down Expand Up @@ -89,6 +90,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Bug Fixes

* [#2086](https://github.com/osmosis-labs/osmosis/pull/2086) `ReplacePoolIncentivesProposal` ProposalType() returns correct value of `ProposalTypeReplacePoolIncentives` instead of `ProposalTypeUpdatePoolIncentives`
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
* [1930](https://github.com/osmosis-labs/osmosis/pull/1930) Ensure you can't `JoinPoolNoSwap` tokens that are not in the pool
* [1700](https://github.com/osmosis-labs/osmosis/pull/1700) Upgrade sdk fork with missing snapshot manager fix.
* [1716](https://github.com/osmosis-labs/osmosis/pull/1716) Fix secondary over-LP shares bug with uneven swap amounts in `CalcJoinPoolShares`.
Expand Down
1 change: 1 addition & 0 deletions app/keepers/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ var AppModuleBasics = []module.AppModuleBasic{
upgradeclient.ProposalHandler,
upgradeclient.CancelProposalHandler,
poolincentivesclient.UpdatePoolIncentivesHandler,
poolincentivesclient.ReplacePoolIncentivesHandler,
ibcclientclient.UpdateClientProposalHandler,
ibcclientclient.UpgradeProposalHandler,
superfluidclient.SetSuperfluidAssetsProposalHandler,
Expand Down
32 changes: 10 additions & 22 deletions x/pool-incentives/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,13 @@ import (

"github.com/cosmos/cosmos-sdk/client"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/gov/client/cli"
govcli "github.com/cosmos/cosmos-sdk/x/gov/client/cli"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"

"github.com/osmosis-labs/osmosis/v11/osmoutils"
"github.com/osmosis-labs/osmosis/v11/x/pool-incentives/types"
)

func NewTxCmd() *cobra.Command {
Copy link
Member

Choose a reason for hiding this comment

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

hrmm is it safe to delete this / is there a reason behind deleting this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is decided in #1888 that submit proposal cmd should only have gov prefix, so these command will be registered with gov handler instead of tx cmd of incentives module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LGTM - I think we should create a changelog entry for this

Should I add this to state breaking entry or not

Copy link
Member

Choose a reason for hiding this comment

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

imo should go under breaking changes

txCmd := &cobra.Command{
Use: types.ModuleName,
Short: "pool incentives transaction subcommands",
DisableFlagParsing: true,
SuggestionsMinimumDistance: 2,
RunE: client.ValidateCmd,
}

txCmd.AddCommand(
NewCmdSubmitUpdatePoolIncentivesProposal(),
NewCmdSubmitReplacePoolIncentivesProposal(),
)

return txCmd
}

func NewCmdSubmitUpdatePoolIncentivesProposal() *cobra.Command {
cmd := &cobra.Command{
Use: "update-pool-incentives [gaugeIds] [weights]",
Expand All @@ -44,7 +27,6 @@ func NewCmdSubmitUpdatePoolIncentivesProposal() *cobra.Command {
return err
}

// TODO: Make a parse uint64 slice function
gaugeIds, err := osmoutils.ParseUint64SliceFromString(args[0], ",")
if err != nil {
return err
Expand Down Expand Up @@ -83,7 +65,7 @@ func NewCmdSubmitUpdatePoolIncentivesProposal() *cobra.Command {
return err
}

content := types.NewUpdatePoolIncentivesProposal(proposal.Title, proposal.Deposit, records)
content := types.NewUpdatePoolIncentivesProposal(proposal.Title, proposal.Description, records)

msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from)
if err != nil {
Expand All @@ -98,7 +80,10 @@ func NewCmdSubmitUpdatePoolIncentivesProposal() *cobra.Command {
},
}

cmd.Flags().String(cli.FlagProposal, "", "Proposal file path (if this path is given, other proposal flags are ignored)")
cmd.Flags().String(govcli.FlagTitle, "", "The proposal title")
cmd.Flags().String(govcli.FlagDescription, "", "The proposal description")
cmd.Flags().String(govcli.FlagDeposit, "", "The proposal deposit")
cmd.Flags().String(govcli.FlagProposal, "", "Proposal file path (if this path is given, other proposal flags are ignored)")

return cmd
}
Expand Down Expand Up @@ -167,7 +152,10 @@ func NewCmdSubmitReplacePoolIncentivesProposal() *cobra.Command {
},
}

cmd.Flags().String(cli.FlagProposal, "", "Proposal file path (if this path is given, other proposal flags are ignored)")
cmd.Flags().String(govcli.FlagTitle, "", "The proposal title")
cmd.Flags().String(govcli.FlagDescription, "", "The proposal description")
cmd.Flags().String(govcli.FlagDeposit, "", "The proposal deposit")
cmd.Flags().String(govcli.FlagProposal, "", "Proposal file path (if this path is given, other proposal flags are ignored)")

return cmd
}
1 change: 1 addition & 0 deletions x/pool-incentives/client/proposal_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ import (
)

var UpdatePoolIncentivesHandler = govclient.NewProposalHandler(cli.NewCmdSubmitUpdatePoolIncentivesProposal, rest.ProposalUpdatePoolIncentivesRESTHandler)
var ReplacePoolIncentivesHandler = govclient.NewProposalHandler(cli.NewCmdSubmitReplacePoolIncentivesProposal, rest.ProposalReplacePoolIncentivesRESTHandler)
2 changes: 1 addition & 1 deletion x/pool-incentives/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (b AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux
}

func (b AppModuleBasic) GetTxCmd() *cobra.Command {
return cli.NewTxCmd()
return nil
}

func (b AppModuleBasic) GetQueryCmd() *cobra.Command {
Expand Down
2 changes: 1 addition & 1 deletion x/pool-incentives/types/gov.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (p *ReplacePoolIncentivesProposal) ProposalRoute() string { return RouterKe

// ProposalType returns the type of the proposal
func (p *ReplacePoolIncentivesProposal) ProposalType() string {
return ProposalTypeUpdatePoolIncentives
return ProposalTypeReplacePoolIncentives
}
Comment on lines 47 to 49
Copy link
Member

Choose a reason for hiding this comment

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

ooof, would this be state breaking to backport? (I don't have a sense for where ProposalType gets used)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think they're used in routing

Copy link
Contributor

Choose a reason for hiding this comment

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

This is used in x/gov routing when executing the proposal's handler.

Copy link
Member

Choose a reason for hiding this comment

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

If it's only used for routing, I assume it's not going to be state-breaking to backport?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think If we have a ProposalTypeReplacePoolIncentives submited, this will be state-breaking


// ValidateBasic validates a governance proposal's abstract and basic contents
Expand Down