Governance: Add proposal and payer to ProposalDeposit account (#141)

* feat: Add proposal and payer to ProposalDeposit account

* chore: Update tests

* feat: Use referential fields to check ProposalDeposit

* chore: Make Clippy happy

* chore: Update Proposal deposit amount comments

* chore: Update deposit amount comment

Author: SebastianBor

governance/program/src/error.rs

@@ -447,9 +447,9 @@ pub enum GovernanceError {
     #[error("Cannot refund ProposalDeposit")]
     CannotRefundProposalDeposit, // 607
 
-    /// Invalid ProposalDeposit account address
-    #[error("Invalid ProposalDeposit account address")]
-    InvalidProposalDepositAccountAddress, // 608
+    ///Invalid Proposal for ProposalDeposit
+    #[error("Invalid Proposal for ProposalDeposit")]
+    InvalidProposalForProposalDeposit, // 608
 
     /// Invalid deposit_exempt_proposal_count
     #[error("Invalid deposit_exempt_proposal_count")]
@@ -458,6 +458,10 @@ pub enum GovernanceError {
     /// GoverningTokenMint not allowed to vote
     #[error("GoverningTokenMint not allowed to vote")]
     GoverningTokenMintNotAllowedToVote, // 610
+
+    ///Invalid deposit Payer for ProposalDeposit
+    #[error("Invalid deposit Payer for ProposalDeposit")]
+    InvalidDepositPayerForProposalDeposit, // 611
 }
 
 impl PrintProgramError for GovernanceError {

governance/program/src/processor/process_create_proposal.rs

@@ -189,7 +189,12 @@ pub fn process_create_proposal(
     let proposal_deposit_amount = governance_data.get_proposal_deposit_amount();
     if proposal_deposit_amount > 0 {
         let proposal_deposit_info = next_account_info(account_info_iter)?; // *10
-        let proposal_deposit_data = ProposalDeposit {};
+        let proposal_deposit_data = ProposalDeposit {
+            account_type: GovernanceAccountType::ProposalDeposit,
+            proposal: *proposal_info.key,
+            deposit_payer: *payer_info.key,
+            reserved: [0; 64],
+        };
 
         create_and_serialize_account_signed::<ProposalDeposit>(
             payer_info,

governance/program/src/processor/process_refund_proposal_deposit.rs

@@ -8,7 +8,8 @@ use solana_program::{
 use spl_governance_tools::account::dispose_account;
 
 use crate::state::{
-    proposal::get_proposal_data, proposal_deposit::assert_is_valid_proposal_deposit_account,
+    proposal::get_proposal_data,
+    proposal_deposit::get_proposal_deposit_data_for_proposal_and_deposit_payer,
 };
 
 /// Processes RefundProposalDeposit instruction
@@ -28,7 +29,7 @@ pub fn process_refund_proposal_deposit(
     proposal_data.assert_can_refund_proposal_deposit()?;
 
     // Assert we are disposing a deposit which belongs to the Proposal and the deposit payer
-    assert_is_valid_proposal_deposit_account(
+    let _proposal_deposit_data = get_proposal_deposit_data_for_proposal_and_deposit_payer(
         program_id,
         proposal_deposit_info,
         proposal_info.key,

governance/program/src/state/enums.rs

@@ -87,6 +87,9 @@ pub enum GovernanceAccountType {
     /// Proposal Signatory account
     /// V2 adds extra reserved space reserved_v2
     SignatoryRecordV2,
+
+    /// Proposal deposit account
+    ProposalDeposit,
 }
 
 impl Default for GovernanceAccountType {

governance/program/src/state/governance.rs

@@ -145,7 +145,8 @@ pub fn is_governance_v2_account_type(account_type: &GovernanceAccountType) -> bo
         | GovernanceAccountType::ProposalTransactionV2
         | GovernanceAccountType::VoteRecordV1
         | GovernanceAccountType::VoteRecordV2
-        | GovernanceAccountType::ProgramMetadata => false,
+        | GovernanceAccountType::ProgramMetadata
+        | GovernanceAccountType::ProposalDeposit => false,
     }
 }
 
@@ -178,7 +179,8 @@ pub fn try_get_governance_v2_type_for_v1(
         | GovernanceAccountType::ProposalTransactionV2
         | GovernanceAccountType::VoteRecordV1
         | GovernanceAccountType::VoteRecordV2
-        | GovernanceAccountType::ProgramMetadata => None,
+        | GovernanceAccountType::ProgramMetadata
+        | GovernanceAccountType::ProposalDeposit => None,
     }
 }
 
@@ -222,6 +224,7 @@ impl GovernanceV2 {
             | GovernanceAccountType::ProposalTransactionV2
             | GovernanceAccountType::ProposalV2
             | GovernanceAccountType::ProgramMetadata
+            | GovernanceAccountType::ProposalDeposit
             | GovernanceAccountType::RealmV2
             | GovernanceAccountType::TokenOwnerRecordV2
             | GovernanceAccountType::SignatoryRecordV2 => {
@@ -348,12 +351,20 @@ impl GovernanceV2 {
         Ok(vote_tipping)
     }
 
-    /// Returns the required deposit amount for creating a Proposal based on the number of active proposals
-    /// and the configured number of proposals exempt from the deposits  
-    /// The deposit is not payed until there are more active Proposal than the exempt amount
+    /// Returns the required deposit amount for creating Nth Proposal based on the number of active proposals
+    /// where N equals to active_proposal_count - deposit_exempt_proposal_count  
+    /// The deposit is not payed unless there are more active Proposal than the exempt amount
+    ///
+    /// Note: The exact deposit payed for Nth Proposal is N*SECURITY_DEPOSIT_BASE_LAMPORTS + min_rent_for(ProposalDeposit)
+    ///
+    /// Note: Although the deposit amount payed for Nth proposal is linear the total deposit amount required to create N proposals is sum of arithmetic series
+    /// Dn = N*r + d*N*(N+1)/2
+    // where:
+    // Dn - The total deposit amount required to create N proposals
+    // N = active_proposal_count - deposit_exempt_proposal_count
+    // d = SECURITY_DEPOSIT_BASE_LAMPORTS
+    // r = min rent amount for ProposalDeposit
     pub fn get_proposal_deposit_amount(&self) -> u64 {
-        // Charge a security deposit of n(n+1)/2 for n proposals to discourage spam
-        // where n = active_proposal_count - deposit_exempt_proposal_count
         self.active_proposal_count
             .saturating_sub(self.config.deposit_exempt_proposal_count as u64)
             .saturating_mul(SECURITY_DEPOSIT_BASE_LAMPORTS)

governance/program/src/state/legacy.rs

@@ -151,7 +151,8 @@ pub fn is_governance_v1_account_type(account_type: &GovernanceAccountType) -> bo
         | GovernanceAccountType::ProposalTransactionV2
         | GovernanceAccountType::VoteRecordV1
         | GovernanceAccountType::VoteRecordV2
-        | GovernanceAccountType::ProgramMetadata => false,
+        | GovernanceAccountType::ProgramMetadata
+        | GovernanceAccountType::ProposalDeposit => false,
     }
 }
 

governance/program/src/state/proposal_deposit.rs

@@ -1,19 +1,40 @@
 //! Proposal deposit account
 
 use borsh::{BorshDeserialize, BorshSchema, BorshSerialize};
-use solana_program::{account_info::AccountInfo, program_error::ProgramError, pubkey::Pubkey};
-use spl_governance_tools::{account::AccountMaxSize, error::GovernanceToolsError};
+use solana_program::{
+    account_info::AccountInfo, program_error::ProgramError, program_pack::IsInitialized,
+    pubkey::Pubkey,
+};
+use spl_governance_tools::account::{get_account_data, AccountMaxSize};
 
-use crate::error::GovernanceError;
+use crate::{error::GovernanceError, state::enums::GovernanceAccountType};
 
 /// Proposal deposit account
-/// The account has no data and is used to limit spam of proposals
+/// The account is used to limit spam of proposals
 #[derive(Clone, Debug, PartialEq, BorshDeserialize, BorshSerialize, BorshSchema)]
-pub struct ProposalDeposit {}
+pub struct ProposalDeposit {
+    /// Governance account type
+    pub account_type: GovernanceAccountType,
+
+    /// The Proposal the deposit belongs to
+    pub proposal: Pubkey,
+
+    /// The account which payed for the deposit
+    pub deposit_payer: Pubkey,
+
+    /// Reserved
+    pub reserved: [u8; 64],
+}
 
 impl AccountMaxSize for ProposalDeposit {
     fn get_max_size(&self) -> Option<usize> {
-        Some(0)
+        Some(1 + 32 + 32 + 64)
+    }
+}
+
+impl IsInitialized for ProposalDeposit {
+    fn is_initialized(&self) -> bool {
+        self.account_type == GovernanceAccountType::ProposalDeposit
     }
 }
 
@@ -42,23 +63,55 @@ pub fn get_proposal_deposit_address(
     .0
 }
 
-/// Asserts the given ProposalDeposit account address is derived from to the Proposal and the deposit payer
-pub fn assert_is_valid_proposal_deposit_account(
+/// Deserializes ProposalDeposit account and checks owner program and account type
+pub fn get_proposal_deposit_data(
+    program_id: &Pubkey,
+    proposal_deposit_info: &AccountInfo,
+) -> Result<ProposalDeposit, ProgramError> {
+    get_account_data::<ProposalDeposit>(program_id, proposal_deposit_info)
+}
+
+/// Deserializes ProposalDeposit account
+/// 1) Checks owner program and account type
+/// 2) Asserts it belongs to the given Proposal and deposit Payer
+pub fn get_proposal_deposit_data_for_proposal_and_deposit_payer(
     program_id: &Pubkey,
     proposal_deposit_info: &AccountInfo,
     proposal: &Pubkey,
     proposal_deposit_payer: &Pubkey,
-) -> Result<(), ProgramError> {
-    if proposal_deposit_info.owner != program_id {
-        return Err(GovernanceToolsError::InvalidAccountOwner.into());
-    }
+) -> Result<ProposalDeposit, ProgramError> {
+    let proposal_deposit_data = get_proposal_deposit_data(program_id, proposal_deposit_info)?;
 
-    let proposal_deposit_address =
-        get_proposal_deposit_address(program_id, proposal, proposal_deposit_payer);
+    if proposal_deposit_data.proposal != *proposal {
+        return Err(GovernanceError::InvalidProposalForProposalDeposit.into());
+    }
 
-    if *proposal_deposit_info.key != proposal_deposit_address {
-        return Err(GovernanceError::InvalidProposalDepositAccountAddress.into());
+    if proposal_deposit_data.deposit_payer != *proposal_deposit_payer {
+        return Err(GovernanceError::InvalidDepositPayerForProposalDeposit.into());
     }
 
-    Ok(())
+    Ok(proposal_deposit_data)
+}
+
+#[cfg(test)]
+mod test {
+
+    use super::*;
+
+    #[test]
+    fn test_max_size() {
+        // Arrange
+        let proposal_deposit_data = ProposalDeposit {
+            account_type: GovernanceAccountType::ProposalDeposit,
+            proposal: Pubkey::new_unique(),
+            deposit_payer: Pubkey::new_unique(),
+            reserved: [0; 64],
+        };
+
+        // Act
+        let size = proposal_deposit_data.try_to_vec().unwrap().len();
+
+        // Assert
+        assert_eq!(proposal_deposit_data.get_max_size(), Some(size));
+    }
 }

governance/program/src/state/realm.rs

@@ -190,7 +190,8 @@ pub fn is_realm_account_type(account_type: &GovernanceAccountType) -> bool {
         | GovernanceAccountType::ProposalTransactionV2
         | GovernanceAccountType::VoteRecordV1
         | GovernanceAccountType::VoteRecordV2
-        | GovernanceAccountType::ProgramMetadata => false,
+        | GovernanceAccountType::ProgramMetadata
+        | GovernanceAccountType::ProposalDeposit => false,
     }
 }
 

governance/program/tests/process_create_proposal.rs

@@ -11,6 +11,7 @@ use spl_governance::{
     error::GovernanceError,
     state::{enums::VoteThreshold, governance::SECURITY_DEPOSIT_BASE_LAMPORTS},
 };
+use spl_governance_tools::account::AccountMaxSize;
 
 #[tokio::test]
 async fn test_create_community_proposal() {
@@ -653,18 +654,26 @@ async fn test_create_proposal_with_security_deposit() {
 
     // Assert
 
-    let _proposal_deposit_account = governance_test
-        .get_proposal_deposit_account(&proposal_cookie.proposal_deposit)
+    let proposal_deposit_account = governance_test
+        .get_proposal_deposit_account(&proposal_cookie.proposal_deposit.address)
         .await;
 
+    assert_eq!(
+        proposal_cookie.proposal_deposit.account,
+        proposal_deposit_account
+    );
+
     let proposal_deposit_account_info = governance_test
         .bench
-        .get_account(&proposal_cookie.proposal_deposit)
+        .get_account(&proposal_cookie.proposal_deposit.address)
         .await
         .unwrap();
 
-    let expected_lamports =
-        governance_test.bench.rent.minimum_balance(0) + SECURITY_DEPOSIT_BASE_LAMPORTS;
+    let expected_lamports = governance_test
+        .bench
+        .rent
+        .minimum_balance(proposal_deposit_account.get_max_size().unwrap())
+        + SECURITY_DEPOSIT_BASE_LAMPORTS;
 
     assert_eq!(expected_lamports, proposal_deposit_account_info.lamports);
 }
@@ -715,32 +724,56 @@ async fn test_create_multiple_proposals_with_security_deposits() {
     // Assert
     let proposal_deposit_account_info1 = governance_test
         .bench
-        .get_account(&proposal_cookie1.proposal_deposit)
+        .get_account(&proposal_cookie1.proposal_deposit.address)
         .await;
 
     assert_eq!(None, proposal_deposit_account_info1);
 
     // Proposal 2
+    let proposal_deposit_account2 = governance_test
+        .get_proposal_deposit_account(&proposal_cookie2.proposal_deposit.address)
+        .await;
+
+    assert_eq!(
+        proposal_cookie2.proposal_deposit.account,
+        proposal_deposit_account2
+    );
+
     let proposal_deposit_account_info2 = governance_test
         .bench
-        .get_account(&proposal_cookie2.proposal_deposit)
+        .get_account(&proposal_cookie2.proposal_deposit.address)
         .await
         .unwrap();
 
-    let expected_lamports =
-        governance_test.bench.rent.minimum_balance(0) + SECURITY_DEPOSIT_BASE_LAMPORTS;
+    let expected_lamports = governance_test
+        .bench
+        .rent
+        .minimum_balance(proposal_deposit_account2.get_max_size().unwrap())
+        + SECURITY_DEPOSIT_BASE_LAMPORTS;
 
     assert_eq!(expected_lamports, proposal_deposit_account_info2.lamports);
 
     // Proposal 3
+    let proposal_deposit_account3 = governance_test
+        .get_proposal_deposit_account(&proposal_cookie3.proposal_deposit.address)
+        .await;
+
+    assert_eq!(
+        proposal_cookie3.proposal_deposit.account,
+        proposal_deposit_account3
+    );
+
     let proposal_deposit_account_info3 = governance_test
         .bench
-        .get_account(&proposal_cookie3.proposal_deposit)
+        .get_account(&proposal_cookie3.proposal_deposit.address)
         .await
         .unwrap();
 
-    let expected_lamports =
-        governance_test.bench.rent.minimum_balance(0) + 2 * SECURITY_DEPOSIT_BASE_LAMPORTS;
+    let expected_lamports = governance_test
+        .bench
+        .rent
+        .minimum_balance(proposal_deposit_account3.get_max_size().unwrap())
+        + 2 * SECURITY_DEPOSIT_BASE_LAMPORTS;
 
     assert_eq!(expected_lamports, proposal_deposit_account_info3.lamports);
 }