Governance: Use mint authority to revoke membership tokens (#3498)

* chore: fix process_revoke_governing_tokens feature

* feat: Use governing token mint authority to authorise token revocation

* chore: Fix authority spelling

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),
     ];

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)?;
 

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()
+    );
+}

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,

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