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.

Sign up for GitHub

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

Jump to bottom

Remove UpdateSwapFee and SwapFeeGovernor #60

Merged
merged 2 commits into from
Apr 2, 2021
Merged

Remove UpdateSwapFee and SwapFeeGovernor #60

merged 2 commits into from
Apr 2, 2021

Conversation

sunnya97
Copy link
Collaborator

@sunnya97 sunnya97 commented Apr 1, 2021

closes #58

@sunnya97 sunnya97 added api-breaking Breaking changes that impact API only (not state machine). state-machine-breaking C:x/gamm Changes, features and bugs related to the gamm module. labels Apr 1, 2021
ValarDragon
ValarDragon approved these changes Apr 1, 2021
View reviewed changes
Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

LGTM. Are we going to require pools to be re-created to specify their governance procedures later?

@sunnya97
Copy link
Collaborator Author

sunnya97 commented Apr 1, 2021

I suppose so. Either these pools remain ungovernable or we use a full-chain governance proposal to decide an initial governor for them (i.e. locked LP tokens, 1 week) that they can then change.

@ValarDragon
Copy link
Member

Hrm, maybe we should add a denom for "future governance token" in the pool definition, which could also refer to its own LP token w/ a given lockup?

@ValarDragon
Copy link
Member

That we theres no need to go to chain governance when we enable that feature

@ValarDragon
Copy link
Member

Per call:

Lets add a string for "Future owner". This can be an address, or a This LP Token, lockup time pair. We then need to build a "validate owner string" function.

Then next upgrade when we implement pool-specific governance, we automatically port the governor specification from this string.

@ValarDragon ValarDragon mentioned this pull request Apr 2, 2021
Closed
@sunnya97 sunnya97 merged commit 295d3ed into main Apr 2, 2021
@ValarDragon ValarDragon deleted the sunny/remove-governor branch April 12, 2021 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ValarDragon ValarDragon ValarDragon approved these changes

@Thunnini Thunnini Awaiting requested review from Thunnini

Assignees
No one assigned
Labels
api-breaking Breaking changes that impact API only (not state machine). C:x/gamm Changes, features and bugs related to the gamm module.
Projects
Osmosis Chain Development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove SwapFee Governor
2 participants
@sunnya97 @ValarDragon