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

Adjust Advisory Committee voting ratios #975

Merged
merged 7 commits into from Feb 23, 2023

Conversation

maltekliemann
Copy link
Member

@maltekliemann maltekliemann commented Feb 15, 2023

Context. We want to introduce community members to the committee and add a prime member to allow the committee to reach higher voting thresholds without too much delay, even if members are AFK.

The following changes are made to the Advisory Committee's voting:

  • approve_market requires more than 33.3...% of approval
  • reject_market requires more than 66.6...% of approval
  • request_edit requires more than 33.3...% of approval

This gives the following thresholds for low numbers of advisors. The numbers in parentheses specify the expected number of FT advisors and community advisors. The numbers are chosen so that the community can make autonomous decisions when there's at least three community members on the board (provided that the count of FT advisors doesn't change). Note that the number of votes required for approval/request of edit plus the number of votes required for rejection is larger than the number of advisors to prevent races.

Number of advisors approve (>33.3...%) reject_market (>66.6...%) requests_edit (>33.3...%)
4 2 3 2
5 (4+1) 2 4 2
6 (4+2) 3 5 3
7 (4+3) 3 5 3
8 (4+4) 3 6 3

The motion duration remains at three days. I think that's at the low end of the pain threshold. Anything higher than four days means that it takes the prime member too long to exercise their power. Anything below three days and the prime member is hard pressed to keep an eye on possible motions over weekends.

Edit. This market also denies the AC the ability to close market early, see the discussion below.

@maltekliemann maltekliemann added the s:in-progress The pull requests is currently being worked on label Feb 15, 2023
@maltekliemann maltekliemann added this to the v0.3.9 milestone Feb 15, 2023
@maltekliemann maltekliemann self-assigned this Feb 15, 2023
@maltekliemann maltekliemann added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on labels Feb 20, 2023
Copy link
Member

@sea212 sea212 left a comment

Choose a reason for hiding this comment

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

LGTM.

I addition to the mentioned changes, only root can close markets now, whereas before the advisory committee also had this power. Does this make sense? How would we deal with scenarios such as sudden change of information now? For example, we have a market that wants to know who is responsible for something. Duration is 2 months. After 1 month a person publicly makes a confession that he is responsible for that. Before this change, the advisory committee could close and resolve the market (should they even do that?), now the information shock leads to a massive withdrawal of liquidity and liquidity providers profits.

@sea212
Copy link
Member

sea212 commented Feb 21, 2023

I think it might make sense to make LPs aware of that such that they can "soft" close a market by being alert and withdrawing liquidity in that case. They are incentivized to do so, I assume therefore it should be fine.
What's your opinion?

@maltekliemann
Copy link
Member Author

Sorry about that omission. I had already forgotten about that again. I'm ok with giving the AC that power, but a couple of comments on the matter:

  • I never really considered this to be an adequate power for the AC because of the admin_* prefix.
  • If the AC has control over closing a market, shouldn't that power be restricted to advised markets? Otherwise, you're dealing with something that is almost as powerful as destroying a market and can applied to permissionless markets.
  • Shouldn't the other admin_* functions be filtered on the mainnet? I think as tools for fixing human error these are pretty awkward and aren't even the full toolbox. Fox example, we currently don't have the tools to move stuck markets out of the grace period.

@maltekliemann
Copy link
Member Author

I think it might make sense to make LPs aware of that such that they can "soft" close a market by being alert and withdrawing liquidity in that case. They are incentivized to do so, I assume therefore it should be fine.
What's your opinion?

This guide should explain the concept of soft-closing a market: https://docs-7od61utlc-zeitgeistpm.vercel.app/docs/learn/betting-strategy. It's currently still in review, but generally LPs are aware they're allowed to pull their liquidity if they don't like the bet anymore. I honestly think that closing the market should, if at all, be the perogative of the market creator. Actually, Bob and I are thinking about a feature which allows the market creator to close the market early in situations like the one you describe above.

@maltekliemann maltekliemann added s:in-progress The pull requests is currently being worked on and removed s:review-needed The pull request requires reviews labels Feb 21, 2023
@sea212
Copy link
Member

sea212 commented Feb 21, 2023

I like the idea of soft closing the more I think about it.
Giving the market creator the power to close a market seems pretty centralized and I assume will introduce different problems.

@maltekliemann
Copy link
Member Author

I like the idea of soft closing the more I think about it.
Giving the market creator the power to close a market seems pretty centralized and I assume will introduce different problems.

It will speed up the resolution in some cases (like the one you mentioned) and also allow us to run open ended markets.

@maltekliemann
Copy link
Member Author

What do you think about filtering the admin_* calls on the mainnet?

@sea212
Copy link
Member

sea212 commented Feb 21, 2023

I think we should do it. Those functions introduce way to many risks and also assigns too much power to the advisory committee. Are any of those functions still used?
I think we'll have to find other solutions for those on the mainnet. The soft close for markets were the result is available early is fine for now from my perspective. In regards to destroying markets I'll shortly provide an alternative proposal.

@maltekliemann
Copy link
Member Author

I'll leave this PR as it is. The AC has never closed a market, but we may want to keep the option to destroy markets around until you've implemented the new approach.

@maltekliemann maltekliemann added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on labels Feb 21, 2023
sea212
sea212 previously approved these changes Feb 21, 2023
vivekvpandya
vivekvpandya previously approved these changes Feb 22, 2023
Copy link
Contributor

@vivekvpandya vivekvpandya left a comment

Choose a reason for hiding this comment

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

LGTM!
In future it will be better if we add test for theses kind of things too. May be in TS side or whatever is easy to maintain.

@vivekvpandya vivekvpandya added s:accepted This pull request is ready for merge and removed s:review-needed The pull request requires reviews labels Feb 22, 2023
@maltekliemann
Copy link
Member Author

In future it will be better if we add test for theses kind of things too. May be in TS side or whatever is easy to maintain.

I think that's a very good idea. I was actually thinking the same about our RPC queries. I think we should start working on the integration-tests/ folder in this repository.

@maltekliemann maltekliemann added s:in-progress The pull requests is currently being worked on and removed s:accepted This pull request is ready for merge labels Feb 22, 2023
@maltekliemann
Copy link
Member Author

Sorry, guys ☹️ - do over! I miscalculated the thresholds in the PR description. Should be correct now. I'll ping reviews as soon as CI is done. Shows how important testing is here.

@maltekliemann maltekliemann merged commit 1f7ea61 into main Feb 23, 2023
@maltekliemann maltekliemann deleted the mkl-advisory-committee-reorg branch February 23, 2023 11:49
@sea212 sea212 added s:accepted This pull request is ready for merge and removed s:in-progress The pull requests is currently being worked on labels Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s:accepted This pull request is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants