Governance: Remove stats only fields to make room for more Config values (#3828)

SebastianBor · web-flow · commit c5fcbefe192d · 2022-11-18T15:49:04.000Z
* fix: Remove voting_proposal_count

* chore: add governance size tests

* chore: fix chat tests compilation
diff --git a/governance/chat/program/tests/program_test/mod.rs b/governance/chat/program/tests/program_test/mod.rs
@@ -196,7 +196,7 @@ impl GovernanceChatProgramTest {
             council_veto_vote_threshold: VoteThreshold::YesVotePercentage(50),
             council_vote_tipping: spl_governance::state::enums::VoteTipping::Strict,
             community_veto_vote_threshold: VoteThreshold::YesVotePercentage(55),
-            reserved: [0; 3],
+            reserved: [0; 5],
         };
 
         let token_owner_record_address = get_token_owner_record_address(
diff --git a/governance/program/src/instruction.rs b/governance/program/src/instruction.rs
@@ -266,7 +266,7 @@ pub enum GovernanceInstruction {
     /// Cancels Proposal by changing its state to Canceled
     ///
     ///   0. `[writable]` Realm account
-    ///   1. `[writable]` Governance account
+    ///   1. `[]` Governance account
     ///   2. `[writable]` Proposal account
     ///   3. `[writable]`  TokenOwnerRecord account of the  Proposal owner
     ///   4. `[signer]` Governance Authority (Token Owner or Governance Delegate)
@@ -279,7 +279,7 @@ pub enum GovernanceInstruction {
     /// If Proposal owner doesn't designate any signatories then can sign off the Proposal themself
     ///
     ///   0. `[writable]` Realm account
-    ///   1. `[writable]` Governance account
+    ///   1. `[]` Governance account
     ///   2. `[writable]` Proposal account
     ///   3. `[signer]` Signatory account signing off the Proposal
     ///       Or Proposal owner if the owner hasn't appointed any signatories
@@ -292,7 +292,7 @@ pub enum GovernanceInstruction {
     ///  If you tip the consensus then the transactions can begin to be run after their hold up time
     ///
     ///   0. `[writable]` Realm account
-    ///   1. `[writable]` Governance account
+    ///   1. `[]` Governance account
     ///   2. `[writable]` Proposal account
     ///   3. `[writable]` TokenOwnerRecord of the Proposal owner
     ///   4. `[writable]` TokenOwnerRecord of the voter. PDA seeds: ['governance',realm, vote_governing_token_mint, governing_token_owner]
@@ -317,7 +317,7 @@ pub enum GovernanceInstruction {
     /// Finalizes vote in case the Vote was not automatically tipped within max_voting_time period
     ///
     ///   0. `[writable]` Realm account    
-    ///   1. `[writable]` Governance account
+    ///   1. `[]` Governance account
     ///   2. `[writable]` Proposal account
     ///   3. `[writable]` TokenOwnerRecord of the Proposal owner        
     ///   4. `[]` Governing Token Mint
@@ -1010,7 +1010,7 @@ pub fn sign_off_proposal(
 ) -> Instruction {
     let mut accounts = vec![
         AccountMeta::new(*realm, false),
-        AccountMeta::new(*governance, false),
+        AccountMeta::new_readonly(*governance, false),
         AccountMeta::new(*proposal, false),
         AccountMeta::new_readonly(*signatory, true),
     ];
@@ -1055,7 +1055,7 @@ pub fn cast_vote(
 
     let mut accounts = vec![
         AccountMeta::new(*realm, false),
-        AccountMeta::new(*governance, false),
+        AccountMeta::new_readonly(*governance, false),
         AccountMeta::new(*proposal, false),
         AccountMeta::new(*proposal_owner_record, false),
         AccountMeta::new(*voter_token_owner_record, false),
@@ -1096,7 +1096,7 @@ pub fn finalize_vote(
 ) -> Instruction {
     let mut accounts = vec![
         AccountMeta::new(*realm, false),
-        AccountMeta::new(*governance, false),
+        AccountMeta::new_readonly(*governance, false),
         AccountMeta::new(*proposal, false),
         AccountMeta::new(*proposal_owner_record, false),
         AccountMeta::new_readonly(*governing_token_mint, false),
@@ -1169,7 +1169,7 @@ pub fn cancel_proposal(
 ) -> Instruction {
     let accounts = vec![
         AccountMeta::new(*realm, false),
-        AccountMeta::new(*governance, false),
+        AccountMeta::new_readonly(*governance, false),
         AccountMeta::new(*proposal, false),
         AccountMeta::new(*proposal_owner_record, false),
         AccountMeta::new_readonly(*governance_authority, true),
diff --git a/governance/program/src/processor/process_cancel_proposal.rs b/governance/program/src/processor/process_cancel_proposal.rs
@@ -28,7 +28,7 @@ pub fn process_cancel_proposal(program_id: &Pubkey, accounts: &[AccountInfo]) ->
 
     let mut realm_data = get_realm_data(program_id, realm_info)?;
 
-    let mut governance_data =
+    let governance_data =
         get_governance_data_for_realm(program_id, governance_info, realm_info.key)?;
 
     let mut proposal_data =
@@ -51,11 +51,6 @@ pub fn process_cancel_proposal(program_id: &Pubkey, accounts: &[AccountInfo]) ->
         // 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())?;
-
-        // Update  Governance voting_proposal_count
-        governance_data.voting_proposal_count =
-            governance_data.voting_proposal_count.saturating_sub(1);
-        governance_data.serialize(&mut *governance_info.data.borrow_mut())?;
     }
 
     proposal_data.state = ProposalState::Cancelled;
diff --git a/governance/program/src/processor/process_cast_vote.rs b/governance/program/src/processor/process_cast_vote.rs
@@ -63,7 +63,7 @@ pub fn process_cast_vote(
         vote_governing_token_mint_info.key,
     )?;
 
-    let mut governance_data =
+    let governance_data =
         get_governance_data_for_realm(program_id, governance_info, realm_info.key)?;
 
     let vote_kind = get_vote_kind(&vote);
@@ -190,11 +190,6 @@ pub fn process_cast_vote(
         // 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())?;
-
-        // Update  Governance voting_proposal_count
-        governance_data.voting_proposal_count =
-            governance_data.voting_proposal_count.saturating_sub(1);
-        governance_data.serialize(&mut *governance_info.data.borrow_mut())?;
     }
 
     let governing_token_owner = voter_token_owner_record_data.governing_token_owner;
diff --git a/governance/program/src/processor/process_create_governance.rs b/governance/program/src/processor/process_create_governance.rs
@@ -57,7 +57,7 @@ pub fn process_create_governance(
         governed_account: *governed_account_info.key,
         config,
         proposals_count: 0,
-        voting_proposal_count: 0,
+
         reserved_v2: [0; 128],
     };
 
diff --git a/governance/program/src/processor/process_create_mint_governance.rs b/governance/program/src/processor/process_create_mint_governance.rs
@@ -69,7 +69,6 @@ pub fn process_create_mint_governance(
         governed_account: *governed_mint_info.key,
         config,
         proposals_count: 0,
-        voting_proposal_count: 0,
         reserved_v2: [0; 128],
     };
 
diff --git a/governance/program/src/processor/process_create_program_governance.rs b/governance/program/src/processor/process_create_program_governance.rs
@@ -69,7 +69,6 @@ pub fn process_create_program_governance(
         governed_account: *governed_program_info.key,
         config,
         proposals_count: 0,
-        voting_proposal_count: 0,
         reserved_v2: [0; 128],
     };
 
diff --git a/governance/program/src/processor/process_create_token_governance.rs b/governance/program/src/processor/process_create_token_governance.rs
@@ -67,7 +67,6 @@ pub fn process_create_token_governance(
         governed_account: *governed_token_info.key,
         config,
         proposals_count: 0,
-        voting_proposal_count: 0,
         reserved_v2: [0; 128],
     };
 
diff --git a/governance/program/src/processor/process_finalize_vote.rs b/governance/program/src/processor/process_finalize_vote.rs
@@ -33,7 +33,7 @@ pub fn process_finalize_vote(program_id: &Pubkey, accounts: &[AccountInfo]) -> P
         realm_info,
         governing_token_mint_info.key,
     )?;
-    let mut governance_data =
+    let governance_data =
         get_governance_data_for_realm(program_id, governance_info, realm_info.key)?;
 
     let mut proposal_data = get_proposal_data_for_governance_and_governing_mint(
@@ -84,9 +84,5 @@ pub fn process_finalize_vote(program_id: &Pubkey, accounts: &[AccountInfo]) -> P
     realm_data.voting_proposal_count = realm_data.voting_proposal_count.saturating_sub(1);
     realm_data.serialize(&mut *realm_info.data.borrow_mut())?;
 
-    // Update  Governance voting_proposal_count
-    governance_data.voting_proposal_count = governance_data.voting_proposal_count.saturating_sub(1);
-    governance_data.serialize(&mut *governance_info.data.borrow_mut())?;
-
     Ok(())
 }
diff --git a/governance/program/src/processor/process_sign_off_proposal.rs b/governance/program/src/processor/process_sign_off_proposal.rs
@@ -29,7 +29,7 @@ pub fn process_sign_off_proposal(program_id: &Pubkey, accounts: &[AccountInfo])
 
     let mut realm_data = get_realm_data(program_id, realm_info)?;
 
-    let mut governance_data =
+    let _governance_data =
         get_governance_data_for_realm(program_id, governance_info, realm_info.key)?;
 
     let mut proposal_data =
@@ -89,11 +89,5 @@ pub fn process_sign_off_proposal(program_id: &Pubkey, accounts: &[AccountInfo])
     realm_data.voting_proposal_count = realm_data.voting_proposal_count.checked_add(1).unwrap();
     realm_data.serialize(&mut *realm_info.data.borrow_mut())?;
 
-    governance_data.voting_proposal_count = governance_data
-        .voting_proposal_count
-        .checked_add(1)
-        .unwrap();
-    governance_data.serialize(&mut *governance_info.data.borrow_mut())?;
-
     Ok(())
 }
diff --git a/governance/program/src/state/governance.rs b/governance/program/src/state/governance.rs
@@ -56,7 +56,7 @@ pub struct GovernanceConfig {
     pub community_veto_vote_threshold: VoteThreshold,
 
     /// Reserved space for future versions
-    pub reserved: [u8; 3],
+    pub reserved: [u8; 5],
 }
 
 /// Governance Account
@@ -85,9 +85,6 @@ pub struct GovernanceV2 {
     /// Governance config
     pub config: GovernanceConfig,
 
-    /// The number of proposals in voting state in the Governance
-    pub voting_proposal_count: u16,
-
     /// Reserved space for versions v2 and onwards
     /// Note: This space won't be available to v1 accounts until runtime supports resizing
     pub reserved_v2: [u8; 128],
@@ -192,8 +189,6 @@ impl GovernanceV2 {
                 governed_account: self.governed_account,
                 proposals_count: self.proposals_count,
                 config: self.config,
-                reserved: [0; 6],
-                voting_proposal_count: self.voting_proposal_count,
             };
 
             BorshSerialize::serialize(&governance_data_v1, writer)?;
@@ -284,8 +279,6 @@ pub fn get_governance_data(
             governed_account: governance_data_v1.governed_account,
             proposals_count: governance_data_v1.proposals_count,
             config: governance_data_v1.config,
-            voting_proposal_count: governance_data_v1.voting_proposal_count,
-
             // Add the extra reserved_v2 padding
             reserved_v2: [0; 128],
         }
@@ -313,8 +306,11 @@ pub fn get_governance_data(
         governance_data.config.council_vote_tipping =
             governance_data.config.community_vote_tipping.clone();
 
-        // For legacy accoutns set the community Veto threshold to Disabled
+        // For legacy accounts set the community Veto threshold to Disabled
         governance_data.config.community_veto_vote_threshold = VoteThreshold::Disabled;
+
+        // Reset reserved space previously used for voting_proposal_count
+        governance_data.config.reserved = [0; 5];
     }
 
     Ok(governance_data)
@@ -508,6 +504,67 @@ mod test {
 
     use super::*;
 
+    fn create_test_governance_config() -> GovernanceConfig {
+        GovernanceConfig {
+            min_community_weight_to_create_proposal: 5,
+            min_council_weight_to_create_proposal: 1,
+            min_transaction_hold_up_time: 10,
+            max_voting_time: 5,
+            community_vote_threshold: VoteThreshold::YesVotePercentage(60),
+            community_vote_tipping: VoteTipping::Strict,
+            council_vote_threshold: VoteThreshold::YesVotePercentage(60),
+            council_veto_vote_threshold: VoteThreshold::YesVotePercentage(50),
+            council_vote_tipping: VoteTipping::Strict,
+            community_veto_vote_threshold: VoteThreshold::YesVotePercentage(40),
+            reserved: [0; 5],
+        }
+    }
+
+    fn create_test_governance() -> GovernanceV2 {
+        GovernanceV2 {
+            account_type: GovernanceAccountType::GovernanceV2,
+            realm: Pubkey::new_unique(),
+            governed_account: Pubkey::new_unique(),
+            proposals_count: 10,
+            config: create_test_governance_config(),
+            reserved_v2: [0; 128],
+        }
+    }
+
+    fn create_test_v1_governance() -> GovernanceV1 {
+        GovernanceV1 {
+            account_type: GovernanceAccountType::GovernanceV1,
+            realm: Pubkey::new_unique(),
+            governed_account: Pubkey::new_unique(),
+            proposals_count: 10,
+            config: create_test_governance_config(),
+        }
+    }
+
+    #[test]
+    fn test_governance_size() {
+        // Arrange
+        let governance = create_test_governance();
+
+        // Act
+        let size = governance.try_to_vec().unwrap().len();
+
+        // Assert
+        assert_eq!(236, size);
+    }
+
+    #[test]
+    fn test_v1_governance_size() {
+        // Arrange
+        let governance = create_test_v1_governance();
+
+        // Act
+        let size = governance.try_to_vec().unwrap().len();
+
+        // Assert
+        assert_eq!(108, size);
+    }
+
     #[test]
     fn test_deserialize_legacy_governance_account_without_council_vote_thresholds() {
         // Arrange
@@ -558,6 +615,13 @@ mod test {
             governance.config.council_vote_threshold,
             governance.config.council_veto_vote_threshold
         );
+
+        assert_eq!(
+            governance.config.council_vote_tipping,
+            governance.config.community_vote_tipping
+        );
+
+        assert_eq!(governance.config.reserved, [0; 5]);
     }
 
     #[test]
@@ -574,7 +638,7 @@ mod test {
             min_council_weight_to_create_proposal: 1,
             council_vote_tipping: VoteTipping::Strict,
             community_veto_vote_threshold: VoteThreshold::YesVotePercentage(1),
-            reserved: [0; 3],
+            reserved: [0; 5],
         };
 
         // Act
@@ -600,7 +664,7 @@ mod test {
             min_council_weight_to_create_proposal: 1,
             council_vote_tipping: VoteTipping::Strict,
             community_veto_vote_threshold: VoteThreshold::YesVotePercentage(1),
-            reserved: [0; 3],
+            reserved: [0; 5],
         };
 
         // Act
@@ -626,7 +690,7 @@ mod test {
             min_council_weight_to_create_proposal: 1,
             council_vote_tipping: VoteTipping::Strict,
             community_veto_vote_threshold: VoteThreshold::YesVotePercentage(1),
-            reserved: [0; 3],
+            reserved: [0; 5],
         };
 
         // Act
@@ -652,7 +716,7 @@ mod test {
             min_council_weight_to_create_proposal: 1,
             council_vote_tipping: VoteTipping::Strict,
             community_veto_vote_threshold: VoteThreshold::YesVotePercentage(1),
-            reserved: [0; 3],
+            reserved: [0; 5],
         };
 
         // Act
@@ -678,7 +742,7 @@ mod test {
             min_council_weight_to_create_proposal: 1,
             community_veto_vote_threshold: VoteThreshold::YesVotePercentage(0),
             community_vote_tipping: VoteTipping::Strict,
-            reserved: [0; 3],
+            reserved: [0; 5],
         };
 
         // Act
diff --git a/governance/program/src/state/legacy.rs b/governance/program/src/state/legacy.rs
@@ -125,14 +125,6 @@ pub struct GovernanceV1 {
 
     /// Governance config
     pub config: GovernanceConfig,
-
-    /// Reserved space for future versions
-    pub reserved: [u8; 6],
-
-    /// The number of proposals in voting state in the Governance
-    /// Note: This is field introduced in V2 but it took space from reserved
-    /// and we have preserve it for V1 serialization roundtrip
-    pub voting_proposal_count: u16,
 }
 
 /// Checks if the given account type is one of the Governance V1 account types
diff --git a/governance/program/src/state/proposal.rs b/governance/program/src/state/proposal.rs
@@ -1177,7 +1177,7 @@ mod test {
             council_veto_vote_threshold: VoteThreshold::YesVotePercentage(50),
             council_vote_tipping: VoteTipping::Strict,
             community_veto_vote_threshold: VoteThreshold::YesVotePercentage(40),
-            reserved: [0; 3],
+            reserved: [0; 5],
         }
     }
 
diff --git a/governance/program/tests/process_cancel_proposal.rs b/governance/program/tests/process_cancel_proposal.rs
diff --git a/governance/program/tests/process_cast_vote.rs b/governance/program/tests/process_cast_vote.rs
diff --git a/governance/program/tests/process_finalize_vote.rs b/governance/program/tests/process_finalize_vote.rs
diff --git a/governance/program/tests/process_sign_off_proposal.rs b/governance/program/tests/process_sign_off_proposal.rs
diff --git a/governance/program/tests/program_test/mod.rs b/governance/program/tests/program_test/mod.rs
diff --git a/governance/program/tests/use_veto_vote.rs b/governance/program/tests/use_veto_vote.rs
