Code cleanup & Tests (#36)

* chore: extract get_vote_record_address

* chore: add reserved space to NftVoteRecord

* chore: test_relinquish_nft_vote_for_proposal_in_voting_state_and_vote_record_exists_error

* chore: log version

Author: SebastianBor

programs/nft-voter/src/instructions/cast_nft_vote.rs

@@ -75,6 +75,7 @@ pub fn cast_nft_vote<'a,'b,'c,'info>(ctx: Context<'a,'b,'c,'info,CastNftVote<'in
             proposal,
             nft_mint,
             governing_token_owner:*governing_token_owner,
+            reserved: [0; 8],
         };
 
         // Anchor doesn't natively support dynamic account creation using remaining_accounts

programs/nft-voter/src/instructions/relinquish_nft_vote.rs

@@ -1,8 +1,8 @@
 use anchor_lang::prelude::*;
-use spl_governance::state::{vote_record,token_owner_record};
 use spl_governance::state::{enums::ProposalState, governance, proposal};
 use spl_governance_tools::account::dispose_account;
 use crate::error::NftVoterError;
+use crate::tools::governance::get_vote_record_address;
 use crate::{state::*};
 
 use crate::state::{get_nft_vote_record_data_for_proposal_and_token_owner, Registrar};
@@ -71,16 +71,12 @@ pub fn relinquish_nft_vote(ctx: Context<RelinquishNftVote>) -> Result<()> {
         let vote_record_info = &ctx.accounts.vote_record.to_account_info();
 
         // Ensure the given VoteRecord address matches the expected PDA
-        let token_owner_record_key = token_owner_record::get_token_owner_record_address(
+        let vote_record_key = get_vote_record_address(
             &registrar.governance_program_id,
-            &registrar.realm,
+            &registrar.realm, 
             &registrar.governing_token_mint,
-            &ctx.accounts.governing_token_owner.key());
-
-        let vote_record_key = vote_record::get_vote_record_address(
-            &registrar.governance_program_id,
-            &ctx.accounts.proposal.key(),
-            &token_owner_record_key);
+            &ctx.accounts.governing_token_owner.key(),
+            &ctx.accounts.proposal.key());
 
         require!(
             vote_record_key == vote_record_info.key(),

programs/nft-voter/src/lib.rs

@@ -18,21 +18,25 @@ pub mod nft_voter {
 
     use super::*;
     pub fn create_registrar(ctx: Context<CreateRegistrar>, max_collections: u8) -> Result<()> {
+        log_version();
         instructions::create_registrar(ctx, max_collections)
     }
     pub fn create_voter_weight_record(
         ctx: Context<CreateVoterWeightRecord>,
         governing_token_owner: Pubkey,
     ) -> Result<()> {
+        log_version();
         instructions::create_voter_weight_record(ctx, governing_token_owner)
     }
     pub fn create_max_voter_weight_record(ctx: Context<CreateMaxVoterWeightRecord>) -> Result<()> {
+        log_version();
         instructions::create_max_voter_weight_record(ctx)
     }
     pub fn update_voter_weight_record(
         ctx: Context<UpdateVoterWeightRecord>,
         voter_weight_action: VoterWeightAction,
     ) -> Result<()> {
+        log_version();
         instructions::update_voter_weight_record(ctx, voter_weight_action)
     }
     pub fn relinquish_nft_vote(ctx: Context<RelinquishNftVote>) -> Result<()> {
@@ -43,13 +47,20 @@ pub mod nft_voter {
         weight: u64,
         size: u32,
     ) -> Result<()> {
+        log_version();
         instructions::configure_collection(ctx, weight, size)
     }
 
     pub fn cast_nft_vote<'a, 'b, 'c, 'info>(
         ctx: Context<'a, 'b, 'c, 'info, CastNftVote<'info>>,
         proposal: Pubkey,
     ) -> Result<()> {
+        log_version();
         instructions::cast_nft_vote(ctx, proposal)
     }
 }
+
+fn log_version() {
+    // TODO: Check if Anchor allows to log it before instruction is deserialized
+    msg!("VERSION:{:?}", env!("CARGO_PKG_VERSION"));
+}

programs/nft-voter/src/state/nft_vote_record.rs

@@ -27,6 +27,9 @@ pub struct NftVoteRecord {
     /// The voter who casted this vote
     /// It's a Realm member pubkey corresponding to TokenOwnerRecord.governing_token_owner
     pub governing_token_owner: Pubkey,
+
+    /// Reserved for future upgrades
+    pub reserved: [u8; 8],
 }
 
 impl NftVoteRecord {

programs/nft-voter/src/tools/governance.rs

@@ -1,5 +1,6 @@
 use crate::id;
 use anchor_lang::prelude::Pubkey;
+use spl_governance::state::{token_owner_record, vote_record};
 
 /// A macro is exposed so that we can embed the program ID.
 #[macro_export]
@@ -187,3 +188,20 @@ pub fn get_max_voter_weight_record_address(
     )
     .0
 }
+
+pub fn get_vote_record_address(
+    program_id: &Pubkey,
+    realm: &Pubkey,
+    governing_token_mint: &Pubkey,
+    governing_token_owner: &Pubkey,
+    proposal: &Pubkey,
+) -> Pubkey {
+    let token_owner_record_key = token_owner_record::get_token_owner_record_address(
+        program_id,
+        realm,
+        governing_token_mint,
+        governing_token_owner,
+    );
+
+    vote_record::get_vote_record_address(program_id, proposal, &token_owner_record_key)
+}

programs/nft-voter/tests/program_test/nft_voter_test.rs

@@ -459,7 +459,7 @@ impl NftVoterTest {
         Ok(CollectionConfigCookie { collection_config })
     }
 
-    /// Casts NFT vote
+    /// Casts NFT Vote and spl-gov Vote
     #[allow(dead_code)]
     pub async fn cast_nft_vote(
         &mut self,
@@ -501,6 +501,7 @@ impl NftVoterTest {
                 nft_mint: nft_cookie.mint_cookie.address,
                 governing_token_owner: voter_weight_record_cookie.account.governing_token_owner,
                 account_discriminator: NftVoteRecord::ACCOUNT_DISCRIMINATOR,
+                reserved: [0; 8],
             };
 
             nft_vote_record_cookies.push(NftVoteRecordCookie {

programs/nft-voter/tests/relinquish_nft_vote.rs

@@ -1,5 +1,7 @@
 use crate::program_test::nft_voter_test::ConfigureCollectionArgs;
+use gpl_nft_voter::error::NftVoterError;
 use program_test::nft_voter_test::NftVoterTest;
+use program_test::tools::assert_nft_voter_err;
 use solana_program_test::*;
 use solana_sdk::transport::TransportError;
 
@@ -194,3 +196,82 @@ async fn test_relinquish_nft_vote_for_proposal_in_voting_state() -> Result<(), T
 
     Ok(())
 }
+
+#[tokio::test]
+async fn test_relinquish_nft_vote_for_proposal_in_voting_state_and_vote_record_exists_error(
+) -> Result<(), TransportError> {
+    // Arrange
+    let mut nft_voter_test = NftVoterTest::start_new().await;
+
+    let realm_cookie = nft_voter_test.governance.with_realm().await?;
+
+    let registrar_cookie = nft_voter_test.with_registrar(&realm_cookie).await?;
+
+    let nft_collection_cookie = nft_voter_test.token_metadata.with_nft_collection().await?;
+
+    let max_voter_weight_record_cookie = nft_voter_test
+        .with_max_voter_weight_record(&registrar_cookie)
+        .await?;
+
+    nft_voter_test
+        .with_collection(
+            &registrar_cookie,
+            &nft_collection_cookie,
+            &max_voter_weight_record_cookie,
+            None,
+        )
+        .await?;
+
+    let voter_cookie = nft_voter_test.bench.with_wallet().await;
+
+    let voter_token_owner_record_cookie = nft_voter_test
+        .governance
+        .with_token_owner_record(&realm_cookie, &voter_cookie)
+        .await?;
+
+    let voter_weight_record_cookie = nft_voter_test
+        .with_voter_weight_record(&registrar_cookie, &voter_cookie)
+        .await?;
+
+    let proposal_cookie = nft_voter_test
+        .governance
+        .with_proposal(&realm_cookie)
+        .await?;
+
+    let nft_cookie1 = nft_voter_test
+        .token_metadata
+        .with_nft_v2(&nft_collection_cookie, &voter_cookie, None)
+        .await?;
+
+    let nft_vote_record_cookies = nft_voter_test
+        .cast_nft_vote(
+            &registrar_cookie,
+            &voter_weight_record_cookie,
+            &max_voter_weight_record_cookie,
+            &proposal_cookie,
+            &voter_cookie,
+            &voter_token_owner_record_cookie,
+            &[&nft_cookie1],
+        )
+        .await?;
+
+    // Act
+
+    let err = nft_voter_test
+        .relinquish_nft_vote(
+            &registrar_cookie,
+            &voter_weight_record_cookie,
+            &proposal_cookie,
+            &voter_cookie,
+            &voter_token_owner_record_cookie,
+            &nft_vote_record_cookies,
+        )
+        .await
+        .err()
+        .unwrap();
+
+    // Assert
+    assert_nft_voter_err(err, NftVoterError::VoteRecordMustBeWithdrawn);
+
+    Ok(())
+}