From 2e69f37633fa4b4ce2d3e7848c48b7b2b3348b92 Mon Sep 17 00:00:00 2001 From: Sebastian Bor Date: Sat, 29 Jan 2022 15:48:05 +0000 Subject: [PATCH] Governance: Proposals signed off by owner (#2835) * feat: allow proposal owner to sign off proposal directly * chore: add proposal by owner sign off tests * chore: add test for owner trying to sign off proposal with signatory --- governance/program/src/error.rs | 26 +-- governance/program/src/instruction.rs | 18 +- .../processor/process_sign_off_proposal.rs | 50 +++-- .../tests/process_sign_off_proposal.rs | 186 ++++++++++++++++++ governance/program/tests/program_test/mod.rs | 42 ++++ 5 files changed, 289 insertions(+), 33 deletions(-) diff --git a/governance/program/src/error.rs b/governance/program/src/error.rs index 57da84a0..dd9ab051 100644 --- a/governance/program/src/error.rs +++ b/governance/program/src/error.rs @@ -33,7 +33,7 @@ pub enum GovernanceError { /// Governing Token Owner or Delegate must sign transaction #[error("Governing Token Owner or Delegate must sign transaction")] - GoverningTokenOwnerOrDelegateMustSign, + GoverningTokenOwnerOrDelegateMustSign, // 505 /// All votes must be relinquished to withdraw governing tokens #[error("All votes must be relinquished to withdraw governing tokens")] @@ -53,47 +53,47 @@ pub enum GovernanceError { /// Invalid Proposal for ProposalTransaction, #[error("Invalid Proposal for ProposalTransaction,")] - InvalidProposalForProposalTransaction, + InvalidProposalForProposalTransaction, // 510 /// Invalid Signatory account address #[error("Invalid Signatory account address")] - InvalidSignatoryAddress, + InvalidSignatoryAddress, // 511 /// Signatory already signed off #[error("Signatory already signed off")] - SignatoryAlreadySignedOff, + SignatoryAlreadySignedOff, // 512 /// Signatory must sign #[error("Signatory must sign")] - SignatoryMustSign, + SignatoryMustSign, // 513 /// Invalid Proposal Owner #[error("Invalid Proposal Owner")] - InvalidProposalOwnerAccount, + InvalidProposalOwnerAccount, // 514 /// Invalid Proposal for VoterRecord #[error("Invalid Proposal for VoterRecord")] - InvalidProposalForVoterRecord, + InvalidProposalForVoterRecord, // 515 /// Invalid GoverningTokenOwner for VoteRecord #[error("Invalid GoverningTokenOwner for VoteRecord")] - InvalidGoverningTokenOwnerForVoteRecord, + InvalidGoverningTokenOwnerForVoteRecord, // 516 /// Invalid Governance config: Vote threshold percentage out of range" #[error("Invalid Governance config: Vote threshold percentage out of range")] - InvalidVoteThresholdPercentage, + InvalidVoteThresholdPercentage, // 517 /// Proposal for the given Governance, Governing Token Mint and index already exists #[error("Proposal for the given Governance, Governing Token Mint and index already exists")] - ProposalAlreadyExists, + ProposalAlreadyExists, // 518 /// Token Owner already voted on the Proposal #[error("Token Owner already voted on the Proposal")] - VoteAlreadyExists, + VoteAlreadyExists, // 519 /// Owner doesn't have enough governing tokens to create Proposal #[error("Owner doesn't have enough governing tokens to create Proposal")] - NotEnoughTokensToCreateProposal, + NotEnoughTokensToCreateProposal, // 520 /// Invalid State: Can't edit Signatories #[error("Invalid State: Can't edit Signatories")] @@ -132,7 +132,7 @@ pub enum GovernanceError { /// Invalid State: Can't sign off #[error("Invalid State: Can't sign off")] - InvalidStateCannotSignOff, + InvalidStateCannotSignOff, // 530 /// Invalid State: Can't vote #[error("Invalid State: Can't vote")] diff --git a/governance/program/src/instruction.rs b/governance/program/src/instruction.rs index e0a6a790..1845fa2e 100644 --- a/governance/program/src/instruction.rs +++ b/governance/program/src/instruction.rs @@ -132,7 +132,7 @@ pub enum GovernanceInstruction { /// 2. `[]` Program governed by this Governance account /// 3. `[writable]` Program Data account of the Program governed by this Governance account /// 4. `[signer]` Current Upgrade Authority account of the Program governed by this Governance account - /// 5. `[]` Governing TokenOwnerRecord account (Used only if not signed by RealmAuthority) + /// 5. `[]` Governing TokenOwnerRecord account (Used only if not signed by RealmAuthority) /// 6. `[signer]` Payer /// 7. `[]` bpf_upgradeable_loader program /// 8. `[]` System program @@ -265,12 +265,17 @@ pub enum GovernanceInstruction { CancelProposal, /// Signs off Proposal indicating the Signatory approves the Proposal - /// When the last Signatory signs the Proposal state moves to Voting state + /// When the last Signatory signs off the Proposal it enters Voting state + /// Note: Adding signatories to a Proposal is a quality and not a security gate and + /// it's entirely at the discretion of the Proposal owner + /// If Proposal owner doesn't designate any signatories then can sign off the Proposal themself /// /// 0. `[writable]` Proposal account /// 1. `[writable]` Signatory Record account - /// 2. `[signer]` Signatory account + /// 2. `[signer]` Signatory account signing off the Proposal + /// Or Proposal owner if the owner hasn't appointed any signatories /// 3. `[]` Clock sysvar + /// 4. `[]` Optional TokenOwnerRecord of the Proposal owner when self signing off the Proposal SignOffProposal, /// Uses your voter weight (deposited Community or Council tokens) to cast a vote on a Proposal @@ -982,16 +987,21 @@ pub fn sign_off_proposal( // Accounts proposal: &Pubkey, signatory: &Pubkey, + proposal_owner_record: Option<&Pubkey>, ) -> Instruction { let signatory_record_address = get_signatory_record_address(program_id, proposal, signatory); - let accounts = vec![ + let mut accounts = vec![ AccountMeta::new(*proposal, false), AccountMeta::new(signatory_record_address, false), AccountMeta::new_readonly(*signatory, true), AccountMeta::new_readonly(sysvar::clock::id(), false), ]; + if let Some(proposal_owner_record) = proposal_owner_record { + accounts.push(AccountMeta::new_readonly(*proposal_owner_record, false)) + } + let instruction = GovernanceInstruction::SignOffProposal; Instruction { diff --git a/governance/program/src/processor/process_sign_off_proposal.rs b/governance/program/src/processor/process_sign_off_proposal.rs index 475cbe95..3f61d482 100644 --- a/governance/program/src/processor/process_sign_off_proposal.rs +++ b/governance/program/src/processor/process_sign_off_proposal.rs @@ -12,6 +12,7 @@ use solana_program::{ use crate::state::{ enums::ProposalState, proposal::get_proposal_data, signatory_record::get_signatory_record_data_for_seeds, + token_owner_record::get_token_owner_record_data_for_proposal_owner, }; /// Processes SignOffProposal instruction @@ -29,26 +30,43 @@ pub fn process_sign_off_proposal(program_id: &Pubkey, accounts: &[AccountInfo]) let mut proposal_data = get_proposal_data(program_id, proposal_info)?; proposal_data.assert_can_sign_off()?; - let mut signatory_record_data = get_signatory_record_data_for_seeds( - program_id, - signatory_record_info, - proposal_info.key, - signatory_info.key, - )?; - signatory_record_data.assert_can_sign_off(signatory_info)?; + // If the owner of the proposal hasn't appointed any signatories then can sign off the proposal themself + if proposal_data.signatories_count == 0 { + let proposal_owner_record_info = next_account_info(account_info_iter)?; // 4 - signatory_record_data.signed_off = true; - signatory_record_data.serialize(&mut *signatory_record_info.data.borrow_mut())?; + let proposal_owner_record_data = get_token_owner_record_data_for_proposal_owner( + program_id, + proposal_owner_record_info, + &proposal_data.token_owner_record, + )?; + + // Proposal owner (TokenOwner) or its governance_delegate must be the signatory and sign this transaction + proposal_owner_record_data.assert_token_owner_or_delegate_is_signer(signatory_info)?; - if proposal_data.signatories_signed_off_count == 0 { proposal_data.signing_off_at = Some(clock.unix_timestamp); - proposal_data.state = ProposalState::SigningOff; - } + } else { + let mut signatory_record_data = get_signatory_record_data_for_seeds( + program_id, + signatory_record_info, + proposal_info.key, + signatory_info.key, + )?; - proposal_data.signatories_signed_off_count = proposal_data - .signatories_signed_off_count - .checked_add(1) - .unwrap(); + signatory_record_data.assert_can_sign_off(signatory_info)?; + + signatory_record_data.signed_off = true; + signatory_record_data.serialize(&mut *signatory_record_info.data.borrow_mut())?; + + if proposal_data.signatories_signed_off_count == 0 { + proposal_data.signing_off_at = Some(clock.unix_timestamp); + proposal_data.state = ProposalState::SigningOff; + } + + proposal_data.signatories_signed_off_count = proposal_data + .signatories_signed_off_count + .checked_add(1) + .unwrap(); + } // If all Signatories signed off we can start voting if proposal_data.signatories_signed_off_count == proposal_data.signatories_count { diff --git a/governance/program/tests/process_sign_off_proposal.rs b/governance/program/tests/process_sign_off_proposal.rs index 7d2b3c9c..868b17d4 100644 --- a/governance/program/tests/process_sign_off_proposal.rs +++ b/governance/program/tests/process_sign_off_proposal.rs @@ -6,6 +6,7 @@ use solana_program_test::tokio; use program_test::*; use spl_governance::{error::GovernanceError, state::enums::ProposalState}; +use spl_governance_tools::error::GovernanceToolsError; #[tokio::test] async fn test_sign_off_proposal() { @@ -113,3 +114,188 @@ async fn test_sign_off_proposal_with_signatory_must_sign_error() { // Assert assert_eq!(err, GovernanceError::SignatoryMustSign.into()); } + +#[tokio::test] +async fn test_sign_off_proposal_by_owner() { + // Arrange + let mut governance_test = GovernanceProgramTest::start_new().await; + + let realm_cookie = governance_test.with_realm().await; + let governed_account_cookie = governance_test.with_governed_account().await; + + let token_owner_record_cookie = governance_test + .with_community_token_deposit(&realm_cookie) + .await + .unwrap(); + + let mut account_governance_cookie = governance_test + .with_account_governance( + &realm_cookie, + &governed_account_cookie, + &token_owner_record_cookie, + ) + .await + .unwrap(); + + let proposal_cookie = governance_test + .with_proposal(&token_owner_record_cookie, &mut account_governance_cookie) + .await + .unwrap(); + + let clock = governance_test.bench.get_clock().await; + + // Act + governance_test + .sign_off_proposal_by_owner(&proposal_cookie, &token_owner_record_cookie) + .await + .unwrap(); + + // Assert + let proposal_account = governance_test + .get_proposal_account(&proposal_cookie.address) + .await; + + assert_eq!(0, proposal_account.signatories_count); + assert_eq!(0, proposal_account.signatories_signed_off_count); + assert_eq!(ProposalState::Voting, proposal_account.state); + assert_eq!(Some(clock.unix_timestamp), proposal_account.signing_off_at); + assert_eq!(Some(clock.unix_timestamp), proposal_account.voting_at); + assert_eq!(Some(clock.slot), proposal_account.voting_at_slot); +} + +#[tokio::test] +async fn test_sign_off_proposal_by_owner_with_owner_must_sign_error() { + // Arrange + let mut governance_test = GovernanceProgramTest::start_new().await; + + let realm_cookie = governance_test.with_realm().await; + let governed_account_cookie = governance_test.with_governed_account().await; + + let token_owner_record_cookie = governance_test + .with_community_token_deposit(&realm_cookie) + .await + .unwrap(); + + let mut account_governance_cookie = governance_test + .with_account_governance( + &realm_cookie, + &governed_account_cookie, + &token_owner_record_cookie, + ) + .await + .unwrap(); + + let proposal_cookie = governance_test + .with_proposal(&token_owner_record_cookie, &mut account_governance_cookie) + .await + .unwrap(); + + // Act + + let err = governance_test + .sign_off_proposal_by_owner_using_instruction( + &proposal_cookie, + &token_owner_record_cookie, + |i| i.accounts[2].is_signer = false, // signatory + Some(&[]), + ) + .await + .err() + .unwrap(); + + // Assert + assert_eq!( + err, + GovernanceError::GoverningTokenOwnerOrDelegateMustSign.into() + ); +} + +#[tokio::test] +async fn test_sign_off_proposal_by_owner_with_other_proposal_owner_error() { + // Arrange + let mut governance_test = GovernanceProgramTest::start_new().await; + + let realm_cookie = governance_test.with_realm().await; + let governed_account_cookie = governance_test.with_governed_account().await; + + let token_owner_record_cookie = governance_test + .with_community_token_deposit(&realm_cookie) + .await + .unwrap(); + + let mut account_governance_cookie = governance_test + .with_account_governance( + &realm_cookie, + &governed_account_cookie, + &token_owner_record_cookie, + ) + .await + .unwrap(); + + let proposal_cookie = governance_test + .with_proposal(&token_owner_record_cookie, &mut account_governance_cookie) + .await + .unwrap(); + + let token_owner_record_cookie2 = governance_test + .with_community_token_deposit(&realm_cookie) + .await + .unwrap(); + + // Act + + let err = governance_test + .sign_off_proposal_by_owner(&proposal_cookie, &token_owner_record_cookie2) + .await + .err() + .unwrap(); + + // Assert + assert_eq!(err, GovernanceError::InvalidProposalOwnerAccount.into()); +} + +#[tokio::test] +async fn test_sign_off_proposal_by_owner_with_existing_signatories_error() { + // Arrange + let mut governance_test = GovernanceProgramTest::start_new().await; + + let realm_cookie = governance_test.with_realm().await; + let governed_account_cookie = governance_test.with_governed_account().await; + + let token_owner_record_cookie = governance_test + .with_community_token_deposit(&realm_cookie) + .await + .unwrap(); + + let mut account_governance_cookie = governance_test + .with_account_governance( + &realm_cookie, + &governed_account_cookie, + &token_owner_record_cookie, + ) + .await + .unwrap(); + + let proposal_cookie = governance_test + .with_proposal(&token_owner_record_cookie, &mut account_governance_cookie) + .await + .unwrap(); + + governance_test + .with_signatory(&proposal_cookie, &token_owner_record_cookie) + .await + .unwrap(); + + // Act + + let err = governance_test + .sign_off_proposal_by_owner(&proposal_cookie, &token_owner_record_cookie) + .await + .err() + .unwrap(); + + // Assert + + // The instruction fails with AccountDoesNotExist because SignatoryRecord doesn't exist for owner + assert_eq!(err, GovernanceToolsError::AccountDoesNotExist.into()); +} diff --git a/governance/program/tests/program_test/mod.rs b/governance/program/tests/program_test/mod.rs index 0a815eb3..969eceeb 100644 --- a/governance/program/tests/program_test/mod.rs +++ b/governance/program/tests/program_test/mod.rs @@ -1885,6 +1885,47 @@ impl GovernanceProgramTest { Ok(()) } + #[allow(dead_code)] + pub async fn sign_off_proposal_by_owner( + &mut self, + proposal_cookie: &ProposalCookie, + token_owner_record_cookie: &TokenOwnerRecordCookie, + ) -> Result<(), ProgramError> { + self.sign_off_proposal_by_owner_using_instruction( + proposal_cookie, + token_owner_record_cookie, + NopOverride, + None, + ) + .await + } + + #[allow(dead_code)] + pub async fn sign_off_proposal_by_owner_using_instruction( + &mut self, + proposal_cookie: &ProposalCookie, + token_owner_record_cookie: &TokenOwnerRecordCookie, + instruction_override: F, + signers_override: Option<&[&Keypair]>, + ) -> Result<(), ProgramError> { + let mut sign_off_proposal_ix = sign_off_proposal( + &self.program_id, + &proposal_cookie.address, + &token_owner_record_cookie.account.governing_token_owner, + Some(&token_owner_record_cookie.address), + ); + + instruction_override(&mut sign_off_proposal_ix); + + let default_signers = &[&token_owner_record_cookie.token_owner]; + let signers = signers_override.unwrap_or(default_signers); + + self.bench + .process_transaction(&[sign_off_proposal_ix], Some(signers)) + .await?; + + Ok(()) + } #[allow(dead_code)] pub async fn sign_off_proposal( @@ -1913,6 +1954,7 @@ impl GovernanceProgramTest { &self.program_id, &proposal_cookie.address, &signatory_record_cookie.signatory.pubkey(), + None, ); instruction_override(&mut sign_off_proposal_ix);