Governance: Remove Realm voting proposal count (#128)

* feat: Remove Realm.voting_proposal_count

* chore: Update comments

* chore: Add tests and explicit assertions for non existing accounts

Author: SebastianBor

governance/program/src/instruction.rs

@@ -269,7 +269,7 @@ pub enum GovernanceInstruction {
 
     /// Cancels Proposal by changing its state to Canceled
     ///
-    ///   0. `[writable]` Realm account
+    ///   0. `[]` Realm account
     ///   1. `[]` Governance account
     ///   2. `[writable]` Proposal account
     ///   3. `[writable]`  TokenOwnerRecord account of the  Proposal owner
@@ -282,7 +282,7 @@ pub enum GovernanceInstruction {
     /// 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]` Realm account
+    ///   0. `[]` Realm account
     ///   1. `[]` Governance account
     ///   2. `[writable]` Proposal account
     ///   3. `[signer]` Signatory account signing off the Proposal
@@ -295,7 +295,7 @@ pub enum GovernanceInstruction {
     ///  By doing so you indicate you approve or disapprove of running the Proposal set of transactions
     ///  If you tip the consensus then the transactions can begin to be run after their hold up time
     ///
-    ///   0. `[writable]` Realm account
+    ///   0. `[]` Realm account
     ///   1. `[]` Governance account
     ///   2. `[writable]` Proposal account
     ///   3. `[writable]` TokenOwnerRecord of the Proposal owner
@@ -320,7 +320,7 @@ pub enum GovernanceInstruction {
 
     /// Finalizes vote in case the Vote was not automatically tipped within max_voting_time period
     ///
-    ///   0. `[writable]` Realm account    
+    ///   0. `[]` Realm account    
     ///   1. `[]` Governance account
     ///   2. `[writable]` Proposal account
     ///   3. `[writable]` TokenOwnerRecord of the Proposal owner        
@@ -1010,7 +1010,7 @@ pub fn sign_off_proposal(
     proposal_owner_record: Option<&Pubkey>,
 ) -> Instruction {
     let mut accounts = vec![
-        AccountMeta::new(*realm, false),
+        AccountMeta::new_readonly(*realm, false),
         AccountMeta::new_readonly(*governance, false),
         AccountMeta::new(*proposal, false),
         AccountMeta::new_readonly(*signatory, true),
@@ -1055,7 +1055,7 @@ pub fn cast_vote(
         get_vote_record_address(program_id, proposal, voter_token_owner_record);
 
     let mut accounts = vec![
-        AccountMeta::new(*realm, false),
+        AccountMeta::new_readonly(*realm, false),
         AccountMeta::new_readonly(*governance, false),
         AccountMeta::new(*proposal, false),
         AccountMeta::new(*proposal_owner_record, false),
@@ -1096,7 +1096,7 @@ pub fn finalize_vote(
     max_voter_weight_record: Option<Pubkey>,
 ) -> Instruction {
     let mut accounts = vec![
-        AccountMeta::new(*realm, false),
+        AccountMeta::new_readonly(*realm, false),
         AccountMeta::new_readonly(*governance, false),
         AccountMeta::new(*proposal, false),
         AccountMeta::new(*proposal_owner_record, false),
@@ -1169,7 +1169,7 @@ pub fn cancel_proposal(
     governance_authority: &Pubkey,
 ) -> Instruction {
     let accounts = vec![
-        AccountMeta::new(*realm, false),
+        AccountMeta::new_readonly(*realm, false),
         AccountMeta::new_readonly(*governance, false),
         AccountMeta::new(*proposal, false),
         AccountMeta::new(*proposal_owner_record, false),

governance/program/src/processor/process_cancel_proposal.rs

@@ -26,7 +26,9 @@ pub fn process_cancel_proposal(program_id: &Pubkey, accounts: &[AccountInfo]) ->
 
     let clock = Clock::get()?;
 
-    let mut realm_data = get_realm_data(program_id, realm_info)?;
+    // Realm is deserialized to validate the account
+    // Note: It'll be used in the future versions to track active proposals
+    let _realm_data = get_realm_data(program_id, realm_info)?;
 
     let governance_data =
         get_governance_data_for_realm(program_id, governance_info, realm_info.key)?;
@@ -47,12 +49,6 @@ pub fn process_cancel_proposal(program_id: &Pubkey, accounts: &[AccountInfo]) ->
     proposal_owner_record_data.decrease_outstanding_proposal_count();
     proposal_owner_record_data.serialize(&mut *proposal_owner_record_info.data.borrow_mut())?;
 
-    if proposal_data.state == ProposalState::Voting {
-        // Update Realm voting_proposal_count
-        realm_data.voting_proposal_count = realm_data.voting_proposal_count.saturating_sub(1);
-        realm_data.serialize(&mut *realm_info.data.borrow_mut())?;
-    }
-
     proposal_data.state = ProposalState::Cancelled;
     proposal_data.closed_at = Some(clock.unix_timestamp);
 

governance/program/src/processor/process_cast_vote.rs

@@ -57,7 +57,7 @@ pub fn process_cast_vote(
         return Err(GovernanceError::VoteAlreadyExists.into());
     }
 
-    let mut realm_data = get_realm_data_for_governing_token_mint(
+    let realm_data = get_realm_data_for_governing_token_mint(
         program_id,
         realm_info,
         vote_governing_token_mint_info.key,
@@ -186,10 +186,6 @@ pub fn process_cast_vote(
             proposal_owner_record_data
                 .serialize(&mut *proposal_owner_record_info.data.borrow_mut())?;
         };
-
-        // Update Realm voting_proposal_count
-        realm_data.voting_proposal_count = realm_data.voting_proposal_count.saturating_sub(1);
-        realm_data.serialize(&mut *realm_info.data.borrow_mut())?;
     }
 
     let governing_token_owner = voter_token_owner_record_data.governing_token_owner;

governance/program/src/processor/process_create_realm.rs

@@ -139,7 +139,7 @@ pub fn process_create_realm(
             legacy1: 0,
             legacy2: 0,
         },
-        voting_proposal_count: 0,
+        legacy1: 0,
         reserved_v2: [0; 128],
     };
 

governance/program/src/processor/process_finalize_vote.rs

@@ -28,7 +28,7 @@ pub fn process_finalize_vote(program_id: &Pubkey, accounts: &[AccountInfo]) -> P
 
     let clock = Clock::get()?;
 
-    let mut realm_data = get_realm_data_for_governing_token_mint(
+    let realm_data = get_realm_data_for_governing_token_mint(
         program_id,
         realm_info,
         governing_token_mint_info.key,
@@ -80,9 +80,5 @@ pub fn process_finalize_vote(program_id: &Pubkey, accounts: &[AccountInfo]) -> P
 
     proposal_data.serialize(&mut *proposal_info.data.borrow_mut())?;
 
-    // Update Realm voting_proposal_count
-    realm_data.voting_proposal_count = realm_data.voting_proposal_count.saturating_sub(1);
-    realm_data.serialize(&mut *realm_info.data.borrow_mut())?;
-
     Ok(())
 }

governance/program/src/processor/process_set_governance_config.rs

@@ -30,15 +30,9 @@ pub fn process_set_governance_config(
 
     let mut governance_data = get_governance_data(program_id, governance_info)?;
 
-    // Until we have Veto implemented it's better to allow config change as the defense of last resort against governance attacks
     // Note: Config change leaves voting proposals in unpredictable state and it's DAOs responsibility
     // to ensure the changes are made when there are no proposals in voting state
     // For example changing approval quorum could accidentally make proposals to succeed which would otherwise be defeated
-    // The check wouldn't have any effect when upgrading from V1 to V2 because it was not tracked in V1
-
-    // if governance_data.voting_proposal_count > 0 {
-    //     return Err(GovernanceError::GovernanceConfigChangeNotAllowed.into());
-    // }
 
     governance_data.config = config;
 

governance/program/src/processor/process_set_realm_config.rs

@@ -39,15 +39,9 @@ pub fn process_set_realm_config(
         return Err(GovernanceError::RealmAuthorityMustSign.into());
     }
 
-    // Until we have Veto implemented it's better to allow config change as the defense of last resort against governance attacks
     // Note: Config change leaves voting proposals in unpredictable state and it's DAOs responsibility
     // to ensure the changes are made when there are no proposals in voting state
     // For example changing voter-weight or max-voter-weight addin could accidentally make proposals to succeed which would otherwise be defeated
-    // The check wouldn't have any effect when upgrading from V1 to V2 because it was not tracked in V1
-
-    // if realm_data.voting_proposal_count > 0 {
-    //     return Err(GovernanceError::RealmConfigChangeNotAllowed.into());
-    // }
 
     assert_valid_realm_config_args(&realm_config_args)?;
 

governance/program/src/processor/process_sign_off_proposal.rs

@@ -27,7 +27,8 @@ pub fn process_sign_off_proposal(program_id: &Pubkey, accounts: &[AccountInfo])
 
     let clock = Clock::get()?;
 
-    let mut realm_data = get_realm_data(program_id, realm_info)?;
+    // Realm is deserialized to validate the account
+    let _realm_data = get_realm_data(program_id, realm_info)?;
 
     // Governance account data is no longer used in the current version but we still have to load it to validate Realm -> Governance -> Proposal relationship
     // It could be replaced with PDA check but the account is going to be needed in future versions once we support mandatory signatories
@@ -89,8 +90,5 @@ pub fn process_sign_off_proposal(program_id: &Pubkey, accounts: &[AccountInfo])
 
     proposal_data.serialize(&mut *proposal_info.data.borrow_mut())?;
 
-    realm_data.voting_proposal_count = realm_data.voting_proposal_count.checked_add(1).unwrap();
-    realm_data.serialize(&mut *realm_info.data.borrow_mut())?;
-
     Ok(())
 }

governance/program/src/state/enums.rs

@@ -60,7 +60,7 @@ pub enum GovernanceAccountType {
     /// Top level aggregation for governances with Community Token (and optional Council Token)
     /// V2 adds the following fields:
     /// 1) use_community_voter_weight_addin and use_max_community_voter_weight_addin to RealmConfig
-    /// 2) voting_proposal_count
+    /// 2) voting_proposal_count / replaced with legacy1 in V3
     /// 3) extra reserved space reserved_v2
     RealmV2,
 

governance/program/src/state/governance.rs

@@ -12,11 +12,13 @@ use crate::{
 };
 use borsh::{BorshDeserialize, BorshSchema, BorshSerialize};
 use solana_program::{
-    account_info::AccountInfo, borsh::try_from_slice_unchecked, program_error::ProgramError,
-    program_pack::IsInitialized, pubkey::Pubkey,
+    account_info::AccountInfo, program_error::ProgramError, program_pack::IsInitialized,
+    pubkey::Pubkey,
 };
 use spl_governance_tools::{
-    account::{assert_is_valid_account_of_types, get_account_data, AccountMaxSize},
+    account::{
+        assert_is_valid_account_of_types, get_account_data, get_account_type, AccountMaxSize,
+    },
     error::GovernanceToolsError,
 };
 
@@ -267,12 +269,7 @@ pub fn get_governance_data(
     program_id: &Pubkey,
     governance_info: &AccountInfo,
 ) -> Result<GovernanceV2, ProgramError> {
-    if governance_info.data_is_empty() {
-        return Err(GovernanceToolsError::AccountDoesNotExist.into());
-    }
-
-    let account_type: GovernanceAccountType =
-        try_from_slice_unchecked(&governance_info.data.borrow())?;
+    let account_type: GovernanceAccountType = get_account_type(program_id, governance_info)?;
 
     // If the account is V1 version then translate to V2
     let mut governance_data = if is_governance_v1_account_type(&account_type) {