diff --git a/governance/program/src/instruction.rs b/governance/program/src/instruction.rs index eb8beadf..d95a3c1b 100644 --- a/governance/program/src/instruction.rs +++ b/governance/program/src/instruction.rs @@ -78,7 +78,7 @@ pub enum GovernanceInstruction { /// Tokens will be transferred or minted to the Holding account /// 3. `[signer]` Governing Token Owner account /// 4. `[signer]` Governing Token Source account authority - /// It should be owner for TokenAccount and mint_auhtority for MintAccount + /// It should be owner for TokenAccount and mint_authority for MintAccount /// 5. `[writable]` TokenOwnerRecord account. PDA seeds: ['governance',realm, governing_token_mint, governing_token_owner] /// 6. `[signer]` Payer /// 7. `[]` System @@ -494,10 +494,10 @@ pub enum GovernanceInstruction { /// Note: If there are active votes for the TokenOwner then the vote weights won't be updated automatically /// /// 0. `[]` Realm account - /// 1. `[signer]` Realm authority - /// 2. `[writable]` Governing Token Holding account. PDA seeds: ['governance',realm, governing_token_mint] - /// 3. `[writable]` TokenOwnerRecord account. PDA seeds: ['governance',realm, governing_token_mint, governing_token_owner] - /// 4. `[writable]` GoverningTokenMint + /// 1. `[writable]` Governing Token Holding account. PDA seeds: ['governance',realm, governing_token_mint] + /// 2. `[writable]` TokenOwnerRecord account. PDA seeds: ['governance',realm, governing_token_mint, governing_token_owner] + /// 3. `[writable]` GoverningTokenMint + /// 4. `[signer]` GoverningTokenMint mint_authority /// 5. `[]` RealmConfig account. PDA seeds: ['realm-config', realm] /// 6. `[]` SPL Token program RevokeGoverningTokens { @@ -1545,9 +1545,9 @@ pub fn revoke_governing_tokens( program_id: &Pubkey, // Accounts realm: &Pubkey, - realm_authority: &Pubkey, governing_token_owner: &Pubkey, governing_token_mint: &Pubkey, + governing_token_mint_authority: &Pubkey, // Args amount: u64, ) -> Instruction { @@ -1565,10 +1565,10 @@ pub fn revoke_governing_tokens( let accounts = vec![ AccountMeta::new_readonly(*realm, false), - AccountMeta::new_readonly(*realm_authority, true), AccountMeta::new(governing_token_holding_address, false), AccountMeta::new(token_owner_record_address, false), AccountMeta::new(*governing_token_mint, false), + AccountMeta::new_readonly(*governing_token_mint_authority, true), AccountMeta::new_readonly(realm_config_address, false), AccountMeta::new_readonly(spl_token::id(), false), ]; diff --git a/governance/program/src/processor/process_revoke_governing_tokens.rs b/governance/program/src/processor/process_revoke_governing_tokens.rs index de517c5f..fe4774fb 100644 --- a/governance/program/src/processor/process_revoke_governing_tokens.rs +++ b/governance/program/src/processor/process_revoke_governing_tokens.rs @@ -9,11 +9,11 @@ use solana_program::{ use crate::{ error::GovernanceError, state::{ - realm::{get_realm_address_seeds, get_realm_data_for_authority}, + realm::{get_realm_address_seeds, get_realm_data}, realm_config::get_realm_config_data_for_realm, token_owner_record::get_token_owner_record_data_for_realm_and_governing_mint, }, - tools::spl_token::burn_spl_tokens_signed, + tools::spl_token::{assert_spl_token_mint_authority_is_signer, burn_spl_tokens_signed}, }; /// Processes RevokeGoverningTokens instruction @@ -25,21 +25,18 @@ pub fn process_revoke_governing_tokens( let account_info_iter = &mut accounts.iter(); let realm_info = next_account_info(account_info_iter)?; // 0 - let realm_authority_info = next_account_info(account_info_iter)?; // 1 - let governing_token_holding_info = next_account_info(account_info_iter)?; // 2 - let token_owner_record_info = next_account_info(account_info_iter)?; // 3 - let governing_token_mint_info = next_account_info(account_info_iter)?; // 4 + let governing_token_holding_info = next_account_info(account_info_iter)?; // 1 + let token_owner_record_info = next_account_info(account_info_iter)?; // 2 + + let governing_token_mint_info = next_account_info(account_info_iter)?; // 3 + let governing_token_mint_authority_info = next_account_info(account_info_iter)?; // 4 + let realm_config_info = next_account_info(account_info_iter)?; // 5 let spl_token_info = next_account_info(account_info_iter)?; // 6 - let realm_data = - get_realm_data_for_authority(program_id, realm_info, realm_authority_info.key)?; - - if !realm_authority_info.is_signer { - return Err(GovernanceError::RealmAuthorityMustSign.into()); - } + let realm_data = get_realm_data(program_id, realm_info)?; realm_data.assert_is_valid_governing_token_mint_and_holding( program_id, @@ -48,6 +45,12 @@ pub fn process_revoke_governing_tokens( governing_token_holding_info.key, )?; + // The governing_token_mint_authority must sign the transaction to revoke Membership tokens + assert_spl_token_mint_authority_is_signer( + governing_token_mint_info, + governing_token_mint_authority_info, + )?; + let realm_config_data = get_realm_config_data_for_realm(program_id, realm_config_info, realm_info.key)?; diff --git a/governance/program/tests/process_revoke_governing_tokens.rs b/governance/program/tests/process_revoke_governing_tokens.rs index 8114da1e..ac7d16dd 100644 --- a/governance/program/tests/process_revoke_governing_tokens.rs +++ b/governance/program/tests/process_revoke_governing_tokens.rs @@ -1,4 +1,4 @@ -#![cfg(feature = "test-bpf")] +#![cfg(feature = "test-sbf")] use solana_program::pubkey::Pubkey; use solana_program_test::*; @@ -11,7 +11,7 @@ use spl_governance::{ error::GovernanceError, state::{realm::get_governing_token_holding_address, realm_config::GoverningTokenType}, }; -use spl_governance_test_sdk::tools::NopOverride; +use spl_governance_test_sdk::tools::{clone_keypair, NopOverride}; use crate::program_test::args::RealmSetupArgs; use solana_sdk::signature::{Keypair, Signer}; @@ -149,7 +149,7 @@ async fn test_revoke_community_tokens_with_cannot_revoke_dormant_token_error() { } #[tokio::test] -async fn test_revoke_council_tokens_with_realm_authority_must_sign_error() { +async fn test_revoke_council_tokens_with_mint_authority_must_sign_error() { // Arrange let mut governance_test = GovernanceProgramTest::start_new().await; @@ -172,7 +172,7 @@ async fn test_revoke_council_tokens_with_realm_authority_must_sign_error() { &token_owner_record_cookie, &realm_cookie.account.config.council_mint.unwrap(), 1, - |i| i.accounts[1].is_signer = false, // realm_authority + |i| i.accounts[4].is_signer = false, // mint_authority Some(&[]), ) .await @@ -181,11 +181,11 @@ async fn test_revoke_council_tokens_with_realm_authority_must_sign_error() { // Assert - assert_eq!(err, GovernanceError::RealmAuthorityMustSign.into()); + assert_eq!(err, GovernanceError::MintAuthorityMustSign.into()); } #[tokio::test] -async fn test_revoke_council_tokens_with_invalid_realm_authority_error() { +async fn test_revoke_council_tokens_with_invalid_mint_authority_error() { // Arrange let mut governance_test = GovernanceProgramTest::start_new().await; @@ -201,8 +201,8 @@ async fn test_revoke_council_tokens_with_invalid_realm_authority_error() { .await .unwrap(); - // Try to use fake auhtority - let realm_authority = Keypair::new(); + // Try to use fake authority + let mint_authority = Keypair::new(); // Act let err = governance_test @@ -211,8 +211,8 @@ async fn test_revoke_council_tokens_with_invalid_realm_authority_error() { &token_owner_record_cookie, &realm_cookie.account.config.council_mint.unwrap(), 1, - |i| i.accounts[1].pubkey = realm_authority.pubkey(), // realm_authority - Some(&[&realm_authority]), + |i| i.accounts[4].pubkey = mint_authority.pubkey(), // mint_authority + Some(&[&mint_authority]), ) .await .err() @@ -220,7 +220,7 @@ async fn test_revoke_council_tokens_with_invalid_realm_authority_error() { // Assert - assert_eq!(err, GovernanceError::InvalidAuthorityForRealm.into()); + assert_eq!(err, GovernanceError::InvalidMintAuthority.into()); } #[tokio::test] @@ -254,7 +254,7 @@ async fn test_revoke_council_tokens_with_invalid_token_holding_error() { &token_owner_record_cookie, &realm_cookie.account.config.council_mint.unwrap(), 1, - |i| i.accounts[2].pubkey = governing_token_holding_address, // governing_token_holding_address + |i| i.accounts[1].pubkey = governing_token_holding_address, // governing_token_holding_address None, ) .await @@ -377,7 +377,7 @@ async fn test_revoke_council_tokens_with_token_owner_record_for_different_mint_e &token_owner_record_cookie, &realm_cookie.account.config.council_mint.unwrap(), 1, - |i| i.accounts[3].pubkey = token_owner_record_cookie2.address, // token_owner_record_address + |i| i.accounts[2].pubkey = token_owner_record_cookie2.address, // token_owner_record_address None, ) .await @@ -466,3 +466,98 @@ async fn test_revoke_council_tokens_with_partial_revoke_amount() { assert_eq!(token_owner_record.governing_token_deposit_amount, 95); } + +#[tokio::test] +async fn test_revoke_council_tokens_with_community_mint_error() { + // Arrange + let mut governance_test = GovernanceProgramTest::start_new().await; + + let mut realm_config_args = RealmSetupArgs::default(); + realm_config_args.council_token_config_args.token_type = GoverningTokenType::Membership; + + let realm_cookie = governance_test + .with_realm_using_args(&realm_config_args) + .await; + + let token_owner_record_cookie = governance_test + .with_council_token_deposit(&realm_cookie) + .await + .unwrap(); + + // Try to use mint and authority for Community to revoke Council token + let governing_token_mint = realm_cookie.account.community_mint.clone(); + let governing_token_mint_authority = clone_keypair(&realm_cookie.community_mint_authority); + let governing_token_holding_address = get_governing_token_holding_address( + &governance_test.program_id, + &realm_cookie.address, + &governing_token_mint, + ); + + // Act + let err = governance_test + .revoke_governing_tokens_using_instruction( + &realm_cookie, + &token_owner_record_cookie, + &realm_cookie.account.config.council_mint.unwrap(), + 1, + |i| { + i.accounts[1].pubkey = governing_token_holding_address; + i.accounts[3].pubkey = governing_token_mint; + i.accounts[4].pubkey = governing_token_mint_authority.pubkey(); + }, // mint_authority + Some(&[&governing_token_mint_authority]), + ) + .await + .err() + .unwrap(); + + // Assert + + assert_eq!(err, GovernanceError::CannotRevokeGoverningTokens.into()); +} + +#[tokio::test] +async fn test_revoke_council_tokens_with_not_matching_mint_and_authority_error() { + // Arrange + let mut governance_test = GovernanceProgramTest::start_new().await; + + let mut realm_config_args = RealmSetupArgs::default(); + realm_config_args.council_token_config_args.token_type = GoverningTokenType::Membership; + + let realm_cookie = governance_test + .with_realm_using_args(&realm_config_args) + .await; + + let token_owner_record_cookie = governance_test + .with_council_token_deposit(&realm_cookie) + .await + .unwrap(); + + // Try to use a valid mint and authority not matching the Council mint + let governing_token_mint = realm_cookie.account.community_mint.clone(); + let governing_token_mint_authority = clone_keypair(&realm_cookie.community_mint_authority); + + // Act + let err = governance_test + .revoke_governing_tokens_using_instruction( + &realm_cookie, + &token_owner_record_cookie, + &realm_cookie.account.config.council_mint.unwrap(), + 1, + |i| { + i.accounts[3].pubkey = governing_token_mint; + i.accounts[4].pubkey = governing_token_mint_authority.pubkey(); + }, // mint_authority + Some(&[&governing_token_mint_authority]), + ) + .await + .err() + .unwrap(); + + // Assert + + assert_eq!( + err, + GovernanceError::InvalidGoverningTokenHoldingAccount.into() + ); +} diff --git a/governance/program/tests/program_test/cookies.rs b/governance/program/tests/program_test/cookies.rs index 875d5a4d..560fcc2a 100644 --- a/governance/program/tests/program_test/cookies.rs +++ b/governance/program/tests/program_test/cookies.rs @@ -35,6 +35,18 @@ pub struct RealmCookie { pub realm_config: RealmConfigCookie, } +impl RealmCookie { + pub fn get_mint_authority(&self, governing_token_mint: &Pubkey) -> &Keypair { + if *governing_token_mint == self.account.community_mint { + &self.community_mint_authority + } else if Some(*governing_token_mint) == self.account.config.council_mint { + &self.council_mint_authority.as_ref().unwrap() + } else { + panic!("Invalid governing_token_mint") + } + } +} + #[derive(Debug)] pub struct RealmConfigCookie { pub address: Pubkey, diff --git a/governance/program/tests/program_test/mod.rs b/governance/program/tests/program_test/mod.rs index 13138792..0aa2f868 100644 --- a/governance/program/tests/program_test/mod.rs +++ b/governance/program/tests/program_test/mod.rs @@ -1303,18 +1303,20 @@ impl GovernanceProgramTest { instruction_override: F, signers_override: Option<&[&Keypair]>, ) -> Result<(), ProgramError> { + let governing_token_mint_authority = realm_cookie.get_mint_authority(governing_token_mint); + let mut revoke_governing_tokens_ix = revoke_governing_tokens( &self.program_id, &realm_cookie.address, - &realm_cookie.account.authority.unwrap(), &token_owner_record_cookie.account.governing_token_owner, governing_token_mint, + &governing_token_mint_authority.pubkey(), amount, ); instruction_override(&mut revoke_governing_tokens_ix); - let default_signers = &[realm_cookie.realm_authority.as_ref().unwrap()]; + let default_signers = &[governing_token_mint_authority]; let signers = signers_override.unwrap_or(default_signers); self.bench