From c5fcbefe192d0df49ade3f02b511e5dbb3180502 Mon Sep 17 00:00:00 2001 From: Sebastian Bor Date: Fri, 18 Nov 2022 15:49:04 +0000 Subject: [PATCH] Governance: Remove stats only fields to make room for more Config values (#3828) * fix: Remove voting_proposal_count * chore: add governance size tests * chore: fix chat tests compilation --- .../chat/program/tests/program_test/mod.rs | 2 +- governance/program/src/instruction.rs | 16 ++-- .../src/processor/process_cancel_proposal.rs | 7 +- .../src/processor/process_cast_vote.rs | 7 +- .../processor/process_create_governance.rs | 2 +- .../process_create_mint_governance.rs | 1 - .../process_create_program_governance.rs | 1 - .../process_create_token_governance.rs | 1 - .../src/processor/process_finalize_vote.rs | 6 +- .../processor/process_sign_off_proposal.rs | 8 +- governance/program/src/state/governance.rs | 92 ++++++++++++++++--- governance/program/src/state/legacy.rs | 8 -- governance/program/src/state/proposal.rs | 2 +- .../program/tests/process_cancel_proposal.rs | 12 --- governance/program/tests/process_cast_vote.rs | 18 ---- .../program/tests/process_finalize_vote.rs | 6 -- .../tests/process_sign_off_proposal.rs | 6 -- governance/program/tests/program_test/mod.rs | 6 +- governance/program/tests/use_veto_vote.rs | 6 -- 19 files changed, 94 insertions(+), 113 deletions(-) diff --git a/governance/chat/program/tests/program_test/mod.rs b/governance/chat/program/tests/program_test/mod.rs index fa730385..ff177d0b 100644 --- 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 index 9c49a120..19f69135 100644 --- 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 index 046cfc27..051cc2da 100644 --- 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 index b8434d3c..043efa4f 100644 --- 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 index 552cf370..bc91854a 100644 --- 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 index 67592320..4816b7a6 100644 --- 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 index 0bfbbc2f..945fef70 100644 --- 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 index 8bd77ea3..18170ba4 100644 --- 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 index 5d79130f..76f3b746 100644 --- 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 index bf15e2e8..7d529bb9 100644 --- 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 index fdfb9294..ef7e5c5a 100644 --- 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 index 19f87d6c..6e1d78a5 100644 --- 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 index 0230a3a7..b0f15823 100644 --- 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 index 460a3549..dbc35351 100644 --- a/governance/program/tests/process_cancel_proposal.rs +++ b/governance/program/tests/process_cancel_proposal.rs @@ -61,12 +61,6 @@ async fn test_cancel_proposal() { .await; assert_eq!(0, realm_account.voting_proposal_count); - - let governance_account = governance_test - .get_governance_account(&governance_cookie.address) - .await; - - assert_eq!(0, governance_account.voting_proposal_count); } #[tokio::test] @@ -262,10 +256,4 @@ async fn test_cancel_proposal_in_voting_state() { .await; assert_eq!(0, realm_account.voting_proposal_count); - - let governance_account = governance_test - .get_governance_account(&governance_cookie.address) - .await; - - assert_eq!(0, governance_account.voting_proposal_count); } diff --git a/governance/program/tests/process_cast_vote.rs b/governance/program/tests/process_cast_vote.rs index db068a06..b16d111e 100644 --- a/governance/program/tests/process_cast_vote.rs +++ b/governance/program/tests/process_cast_vote.rs @@ -93,12 +93,6 @@ async fn test_cast_vote() { .await; assert_eq!(0, realm_account.voting_proposal_count); - - let governance_account = governance_test - .get_governance_account(&governance_cookie.address) - .await; - - assert_eq!(0, governance_account.voting_proposal_count); } #[tokio::test] @@ -438,12 +432,6 @@ async fn test_cast_vote_with_strict_vote_tipped_to_succeeded() { .await; assert_eq!(0, realm_account.voting_proposal_count); - - let governance_account = governance_test - .get_governance_account(&governance_cookie.address) - .await; - - assert_eq!(0, governance_account.voting_proposal_count); } #[tokio::test] @@ -874,12 +862,6 @@ async fn test_cast_vote_with_threshold_below_50_and_vote_not_tipped() { .await; assert_eq!(1, realm_account.voting_proposal_count); - - let governance_account = governance_test - .get_governance_account(&governance_cookie.address) - .await; - - assert_eq!(1, governance_account.voting_proposal_count); } #[tokio::test] diff --git a/governance/program/tests/process_finalize_vote.rs b/governance/program/tests/process_finalize_vote.rs index ae0aa37f..57889866 100644 --- a/governance/program/tests/process_finalize_vote.rs +++ b/governance/program/tests/process_finalize_vote.rs @@ -104,12 +104,6 @@ async fn test_finalize_vote_to_succeeded() { .await; assert_eq!(0, realm_account.voting_proposal_count); - - let governance_account = governance_test - .get_governance_account(&governance_cookie.address) - .await; - - assert_eq!(0, governance_account.voting_proposal_count); } #[tokio::test] diff --git a/governance/program/tests/process_sign_off_proposal.rs b/governance/program/tests/process_sign_off_proposal.rs index 36a6643f..d94d614a 100644 --- a/governance/program/tests/process_sign_off_proposal.rs +++ b/governance/program/tests/process_sign_off_proposal.rs @@ -70,12 +70,6 @@ async fn test_sign_off_proposal() { .await; assert_eq!(1, realm_account.voting_proposal_count); - - let governance_account = governance_test - .get_governance_account(&governance_cookie.address) - .await; - - assert_eq!(1, governance_account.voting_proposal_count); } #[tokio::test] diff --git a/governance/program/tests/program_test/mod.rs b/governance/program/tests/program_test/mod.rs index 42b97923..d5404b32 100644 --- a/governance/program/tests/program_test/mod.rs +++ b/governance/program/tests/program_test/mod.rs @@ -1415,7 +1415,7 @@ impl GovernanceProgramTest { council_veto_vote_threshold: VoteThreshold::YesVotePercentage(55), council_vote_tipping: spl_governance::state::enums::VoteTipping::Strict, community_veto_vote_threshold: VoteThreshold::YesVotePercentage(80), - reserved: [0; 3], + reserved: [0; 5], } } @@ -1490,7 +1490,6 @@ impl GovernanceProgramTest { governed_account: governed_account_cookie.address, config: governance_config.clone(), proposals_count: 0, - voting_proposal_count: 0, reserved_v2: [0; 128], }; @@ -1660,7 +1659,6 @@ impl GovernanceProgramTest { governed_account: governed_program_cookie.address, config, proposals_count: 0, - voting_proposal_count: 0, reserved_v2: [0; 128], }; @@ -1781,7 +1779,6 @@ impl GovernanceProgramTest { governed_account: governed_mint_cookie.address, config: governance_config.clone(), proposals_count: 0, - voting_proposal_count: 0, reserved_v2: [0; 128], }; @@ -1862,7 +1859,6 @@ impl GovernanceProgramTest { governed_account: governed_token_cookie.address, config, proposals_count: 0, - voting_proposal_count: 0, reserved_v2: [0; 128], }; diff --git a/governance/program/tests/use_veto_vote.rs b/governance/program/tests/use_veto_vote.rs index 55b0d52b..05257b77 100644 --- a/governance/program/tests/use_veto_vote.rs +++ b/governance/program/tests/use_veto_vote.rs @@ -102,12 +102,6 @@ async fn test_cast_council_veto_vote() { .await; assert_eq!(0, realm_account.voting_proposal_count); - - let governance_account = governance_test - .get_governance_account(&governance_cookie.address) - .await; - - assert_eq!(0, governance_account.voting_proposal_count); } #[tokio::test]