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
This commit is contained in:
Sebastian Bor 2022-01-29 15:48:05 +00:00 committed by GitHub
parent aefbebe3ae
commit 2e69f37633
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 289 additions and 33 deletions

View File

@ -33,7 +33,7 @@ pub enum GovernanceError {
/// Governing Token Owner or Delegate must sign transaction /// Governing Token Owner or Delegate must sign transaction
#[error("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 /// All votes must be relinquished to withdraw governing tokens
#[error("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, /// Invalid Proposal for ProposalTransaction,
#[error("Invalid Proposal for ProposalTransaction,")] #[error("Invalid Proposal for ProposalTransaction,")]
InvalidProposalForProposalTransaction, InvalidProposalForProposalTransaction, // 510
/// Invalid Signatory account address /// Invalid Signatory account address
#[error("Invalid Signatory account address")] #[error("Invalid Signatory account address")]
InvalidSignatoryAddress, InvalidSignatoryAddress, // 511
/// Signatory already signed off /// Signatory already signed off
#[error("Signatory already signed off")] #[error("Signatory already signed off")]
SignatoryAlreadySignedOff, SignatoryAlreadySignedOff, // 512
/// Signatory must sign /// Signatory must sign
#[error("Signatory must sign")] #[error("Signatory must sign")]
SignatoryMustSign, SignatoryMustSign, // 513
/// Invalid Proposal Owner /// Invalid Proposal Owner
#[error("Invalid Proposal Owner")] #[error("Invalid Proposal Owner")]
InvalidProposalOwnerAccount, InvalidProposalOwnerAccount, // 514
/// Invalid Proposal for VoterRecord /// Invalid Proposal for VoterRecord
#[error("Invalid Proposal for VoterRecord")] #[error("Invalid Proposal for VoterRecord")]
InvalidProposalForVoterRecord, InvalidProposalForVoterRecord, // 515
/// Invalid GoverningTokenOwner for VoteRecord /// Invalid GoverningTokenOwner for VoteRecord
#[error("Invalid GoverningTokenOwner for VoteRecord")] #[error("Invalid GoverningTokenOwner for VoteRecord")]
InvalidGoverningTokenOwnerForVoteRecord, InvalidGoverningTokenOwnerForVoteRecord, // 516
/// Invalid Governance config: Vote threshold percentage out of range" /// Invalid Governance config: Vote threshold percentage out of range"
#[error("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 /// 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")] #[error("Proposal for the given Governance, Governing Token Mint and index already exists")]
ProposalAlreadyExists, ProposalAlreadyExists, // 518
/// Token Owner already voted on the Proposal /// Token Owner already voted on the Proposal
#[error("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 /// Owner doesn't have enough governing tokens to create Proposal
#[error("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 /// Invalid State: Can't edit Signatories
#[error("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 /// Invalid State: Can't sign off
#[error("Invalid State: Can't sign off")] #[error("Invalid State: Can't sign off")]
InvalidStateCannotSignOff, InvalidStateCannotSignOff, // 530
/// Invalid State: Can't vote /// Invalid State: Can't vote
#[error("Invalid State: Can't vote")] #[error("Invalid State: Can't vote")]

View File

@ -132,7 +132,7 @@ pub enum GovernanceInstruction {
/// 2. `[]` Program governed by this Governance account /// 2. `[]` Program governed by this Governance account
/// 3. `[writable]` Program Data account of the 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 /// 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 /// 6. `[signer]` Payer
/// 7. `[]` bpf_upgradeable_loader program /// 7. `[]` bpf_upgradeable_loader program
/// 8. `[]` System program /// 8. `[]` System program
@ -265,12 +265,17 @@ pub enum GovernanceInstruction {
CancelProposal, CancelProposal,
/// Signs off Proposal indicating the Signatory approves the Proposal /// 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 /// 0. `[writable]` Proposal account
/// 1. `[writable]` Signatory Record 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 /// 3. `[]` Clock sysvar
/// 4. `[]` Optional TokenOwnerRecord of the Proposal owner when self signing off the Proposal
SignOffProposal, SignOffProposal,
/// Uses your voter weight (deposited Community or Council tokens) to cast a vote on a Proposal /// 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 // Accounts
proposal: &Pubkey, proposal: &Pubkey,
signatory: &Pubkey, signatory: &Pubkey,
proposal_owner_record: Option<&Pubkey>,
) -> Instruction { ) -> Instruction {
let signatory_record_address = get_signatory_record_address(program_id, proposal, signatory); 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(*proposal, false),
AccountMeta::new(signatory_record_address, false), AccountMeta::new(signatory_record_address, false),
AccountMeta::new_readonly(*signatory, true), AccountMeta::new_readonly(*signatory, true),
AccountMeta::new_readonly(sysvar::clock::id(), false), 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; let instruction = GovernanceInstruction::SignOffProposal;
Instruction { Instruction {

View File

@ -12,6 +12,7 @@ use solana_program::{
use crate::state::{ use crate::state::{
enums::ProposalState, proposal::get_proposal_data, enums::ProposalState, proposal::get_proposal_data,
signatory_record::get_signatory_record_data_for_seeds, signatory_record::get_signatory_record_data_for_seeds,
token_owner_record::get_token_owner_record_data_for_proposal_owner,
}; };
/// Processes SignOffProposal instruction /// 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)?; let mut proposal_data = get_proposal_data(program_id, proposal_info)?;
proposal_data.assert_can_sign_off()?; proposal_data.assert_can_sign_off()?;
let mut signatory_record_data = get_signatory_record_data_for_seeds( // If the owner of the proposal hasn't appointed any signatories then can sign off the proposal themself
program_id, if proposal_data.signatories_count == 0 {
signatory_record_info, let proposal_owner_record_info = next_account_info(account_info_iter)?; // 4
proposal_info.key,
signatory_info.key,
)?;
signatory_record_data.assert_can_sign_off(signatory_info)?;
signatory_record_data.signed_off = true; let proposal_owner_record_data = get_token_owner_record_data_for_proposal_owner(
signatory_record_data.serialize(&mut *signatory_record_info.data.borrow_mut())?; 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.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 signatory_record_data.assert_can_sign_off(signatory_info)?;
.signatories_signed_off_count
.checked_add(1) signatory_record_data.signed_off = true;
.unwrap(); 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 all Signatories signed off we can start voting
if proposal_data.signatories_signed_off_count == proposal_data.signatories_count { if proposal_data.signatories_signed_off_count == proposal_data.signatories_count {

View File

@ -6,6 +6,7 @@ use solana_program_test::tokio;
use program_test::*; use program_test::*;
use spl_governance::{error::GovernanceError, state::enums::ProposalState}; use spl_governance::{error::GovernanceError, state::enums::ProposalState};
use spl_governance_tools::error::GovernanceToolsError;
#[tokio::test] #[tokio::test]
async fn test_sign_off_proposal() { async fn test_sign_off_proposal() {
@ -113,3 +114,188 @@ async fn test_sign_off_proposal_with_signatory_must_sign_error() {
// Assert // Assert
assert_eq!(err, GovernanceError::SignatoryMustSign.into()); 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());
}

View File

@ -1885,6 +1885,47 @@ impl GovernanceProgramTest {
Ok(()) 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<F: Fn(&mut 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)] #[allow(dead_code)]
pub async fn sign_off_proposal( pub async fn sign_off_proposal(
@ -1913,6 +1954,7 @@ impl GovernanceProgramTest {
&self.program_id, &self.program_id,
&proposal_cookie.address, &proposal_cookie.address,
&signatory_record_cookie.signatory.pubkey(), &signatory_record_cookie.signatory.pubkey(),
None,
); );
instruction_override(&mut sign_off_proposal_ix); instruction_override(&mut sign_off_proposal_ix);