osmosis
osmosis copied to clipboard
modify pool-incentives submit proposal cli
Closes: #1888
Brief Changelog
- add proposal-specific flags to NewCmdSubmitUpdatePoolIncentivesProposal and NewCmdSubmitReplacePoolIncentivesProposal
- disable poolincetives tx commands since these should be only used with a gov prefix
- register NewCmdSubmitReplacePoolIncentivesProposal handler on the gov module
Testing and Verifying
- Manually verified
Documentation and Release Note
- Does this pull request introduce a new feature or user-facing behavior changes? yes
- Is a relevant changelog entry added to the
Unreleased
section inCHANGELOG.md
? yes - How is the feature or change documented? not documented
Sorry, I'm actually generally confused about whats going on in these governance proposal routing.
My understanding is that in the today code, replace incentives proposal works on mainnet. Looking into this, looks like the handler is registered here? https://github.com/osmosis-labs/osmosis/blob/main/app/keepers/keepers.go#L366
Which then splits execution based on type https://github.com/osmosis-labs/osmosis/blob/fd23552f94caf1df9dbeb6fb4784e9d0efded2a4/x/pool-incentives/handler.go#L12-L24
So the routes being equivalent may be correct?
(this is also needlessly complex, and we should simplify it)
Sorry, I'm actually generally confused about whats going on in these governance proposal routing.
My understanding is that in the today code, replace incentives proposal works on mainnet. Looking into this, looks like the handler is registered here? https://github.com/osmosis-labs/osmosis/blob/main/app/keepers/keepers.go#L366
Which then splits execution based on type
https://github.com/osmosis-labs/osmosis/blob/fd23552f94caf1df9dbeb6fb4784e9d0efded2a4/x/pool-incentives/handler.go#L12-L24
So the routes being equivalent may be correct?
I think that the linked handler registration is focused on routing. On the other hand, this PR is primarily about fixing the client bootstrapping / cli.
Currently, pool-incentives gov cli is broken due to incorrectly setting up the cli logic and registering some flags twice. I've verified that this should help fix the original problem in #1888.
We expose 2 ways to submit pool-incentives proposals:
-
osmosisd tx poolincentives update-pool-incentives
-
osmosisd tx gov submit-proposal update-pool-incentives
Only the latter is correct because it properly gets registered on the gov module. The original bug is stemming from attempting to support both ways and, as a result, registering some CLI flags twice which breaks both commands.
The change related to the proposal type seems to be a pass-through change. However, It does seem like a valid change that we should keep because we were returning an invalid proposal type.
The claim that ProposalType()
is used in routing does seem to be correct. Although this is client-side (rest) routing.
Based on my investigation, the changes in this PR do not affect the state-breaking routing we are concerned about.
I think we need the changes in this PR to fix the CLI and client (REST) routing. However, it might make sense to spend more time manually testing to ensure that everything is sound
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!
Hi @catShaark . I just would like to confirm that you've gotten a chance to manually test that both pool incentives proposals are functioning as expected.
Please let me know
I think it would be great to have an e2e test for submitting these pool-incentive proposals. Would you be interested in adding it?