-
Notifications
You must be signed in to change notification settings - Fork 76
Optimistic governance #396
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
base: develop
Are you sure you want to change the base?
Conversation
…e proposal has already passed but not been finalized yet
R-K-H
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only other thing is I don't think another proposal should be able to be created while there exists an unexecuted OGP.
| match self.dao.amm.state { | ||
| PoolState::Spot { spot: _ } => {} | ||
| _ => { | ||
| return Err(FutarchyError::PoolNotInSpotState.into()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment with the matches!
| pub squads_multisig_vault: UncheckedAccount<'info>, | ||
|
|
||
| #[account(seeds = [squads_multisig_program::SEED_PREFIX, squads_multisig.key().as_ref(), squads_multisig_program::SEED_SPENDING_LIMIT, dao.key().as_ref()], bump, seeds::program = squads_program)] | ||
| pub squads_spending_limit: Account<'info, squads_multisig_program::SpendingLimit>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure this is always one account
- add a test for updating spending limit that optimistic governance still works
| optimistic_proposal: None, | ||
| is_optimistic_governance_enabled: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't we need to add some update account instructions again here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
theoretically this could be a separate PR but I prefer it in this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update_dao handles updating already. We decided to go with false as a default on creation as we don't want it on when a new DAO gets created. We can add a param to creation if you want to.
| pub fn validate(&self) -> Result<()> { | ||
| // If we're trying to challenge an optimistic proposal that has already passed due to age, we should error | ||
| // This is because the optimistic proposal's Squads proposal will eventually have to be executed | ||
| match self.dao.optimistic_proposal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think an if let would be better here
|
|
||
| /// CHECK: The squads multisig vault that executes the transaction | ||
| #[account(seeds = [squads_multisig_program::SEED_PREFIX, squads_multisig.key().as_ref(), squads_multisig_program::SEED_VAULT, 0_u8.to_le_bytes().as_ref()], bump, seeds::program = squads_program)] | ||
| pub squads_multisig_vault: UncheckedAccount<'info>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't have this be unchecked unless necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note - Squads Multisig Vaults don't have any data, they are just derived addresses.
Disregarding this comment for now.
| match self.dao.amm.state { | ||
| PoolState::Spot { spot: _ } => {} | ||
| _ => { | ||
| return Err(FutarchyError::PoolNotInSpotState.into()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matches!
| match self.dao.optimistic_proposal { | ||
| Some(ref optimistic_proposal) => { | ||
| require_gte!( | ||
| Clock::get()?.unix_timestamp, | ||
| optimistic_proposal.enqueued_timestamp + self.dao.seconds_per_proposal as i64, | ||
| FutarchyError::ProposalDurationTooShort | ||
| ); | ||
| } | ||
| None => {} | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like ever enqueuing an optimistic proposal while another one is still there, I'd rather add an instruction to remove the optimistic proposal
|
|
||
| // Amount must be less than or equal to 3 times the spending limit | ||
| require_gte!( | ||
| self.squads_spending_limit.amount.checked_mul(3).unwrap(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checked_mul unwrap is equivalent to just doing multiplication because anchor has the overflow checks
| self.proposal.squads_proposal | ||
| ); | ||
|
|
||
| // The optimistic proposal must be younger than seconds_per_proposal, otherwise it is considered passed and must be finalized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also check that it's unpassed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually applies in general, so the check will be outside this condition.
| proposal.timestamp_enqueued = clock.unix_timestamp; | ||
|
|
||
| // Update the DAO state | ||
| if dao.optimistic_proposal.is_some() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to do this, can just do
dao.optimistic_proposal = None and clear out any active proposal
| // Team cannot sponsor a challenge to an optimistic proposal | ||
| if let Some(optimistic_proposal) = &self.dao.optimistic_proposal { | ||
| require_keys_neq!( | ||
| optimistic_proposal.squads_proposal, | ||
| self.proposal.squads_proposal, | ||
| FutarchyError::CannotSponsorOptimisticProposalChallenge | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk if we need this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After our conversation, we don't.
|
After talking to @metaproph3t we came to the following conclusion: Converting an optimistic proposal to a regular market proposal (by means of using |
|
LGTM aside from the account resizing |
…AOproject/programs into pileks/met-5-optimistic-governance
This PR introduces optimistic governance, an alternative governance mode that allows the designated team address to propose treasury spends that automatically pass after a time delay unless challenged.
Overview
Optimistic governance enables faster treasury operations for routine spends while preserving the ability for token holders to challenge and force a full futarchy vote when needed.
Mechanics
Initiating an Optimistic Proposal
The team address (stored in
dao.team_address) can callinitiate_vault_spend_optimistic_proposalto create a token transfer proposal. This instruction:is_optimistic_governance_enabledto betrueon the DAOdao.optimistic_proposalChallenge Window
Once enqueued, the optimistic proposal enters a challenge window equal to
seconds_per_proposal. During this period, anyone can challenge the proposal by initializing, staking to, and launching a futarchy proposal with the same Squads proposal, which:dao.optimistic_proposalFinalization
After
seconds_per_proposalelapses without a challenge, anyone can callfinalize_optimistic_proposalto:dao.optimistic_proposalSafety Checks
seconds_per_proposalelapsesinitialize_proposal(returnsOptimisticProposalAlreadyPassed)launch_proposalalso checks that the optimistic proposal hasn't already passed, preventing edge cases where initialization succeeds but launch failsteam_sponsored_pass_threshold_bpswhen a challenge goes to a decision market.State Changes
The
Daoaccount gains two new fields:optimistic_proposal: Option<OptimisticProposal>- tracks the active optimistic proposal if anyis_optimistic_governance_enabled: bool- feature flag (defaults tofalse)OptimisticProposalcontains:squads_proposal: Pubkey- the Squads proposal that will be approved on finalizationenqueued_timestamp: i64- when the optimistic proposal was createdNew Instructions
initiate_vault_spend_optimistic_proposal- team-only, creates a transfer proposalfinalize_optimistic_proposal- permissionless, approves the Squads proposal after time delayRefactoring
The
compile_transaction_messagehelper was extracted tosrc/squads.rsascompile_squads_transaction_messagefor reuse across instructions that create Squads vault transactions.