Governance: Make realm authority check optional (#2827)

* feat: support unchecked SetRealmAuthority

* chore: add test for unchecked set realm authority

Co-authored-by: Jon Cinque

Author: SebastianBor

governance/program/src/instruction.rs

@@ -11,7 +11,10 @@ use crate::{
         program_metadata::get_program_metadata_address,
         proposal::{get_proposal_address, VoteType},
         proposal_instruction::{get_proposal_instruction_address, InstructionData},
-        realm::{get_governing_token_holding_address, get_realm_address, RealmConfigArgs},
+        realm::{
+            get_governing_token_holding_address, get_realm_address, RealmConfigArgs,
+            SetRealmAuthorityAction,
+        },
         realm_config::get_realm_config_address,
         signatory_record::get_signatory_record_address,
         token_owner_record::get_token_owner_record_address,
@@ -417,8 +420,8 @@ pub enum GovernanceInstruction {
     ///   2. `[]` New realm authority. Must be one of the realm governances when set
     SetRealmAuthority {
         #[allow(dead_code)]
-        /// Indicates whether the realm authority should be removed and set to None
-        remove_authority: bool,
+        /// Set action ( SetUnchecked, SetChecked, Remove)
+        action: SetRealmAuthorityAction,
     },
 
     /// Sets realm config
@@ -1306,22 +1309,26 @@ pub fn set_realm_authority(
     // Accounts
     realm: &Pubkey,
     realm_authority: &Pubkey,
-    new_realm_authority: &Option<Pubkey>,
+    new_realm_authority: Option<&Pubkey>,
     // Args
+    action: SetRealmAuthorityAction,
 ) -> Instruction {
     let mut accounts = vec![
         AccountMeta::new(*realm, false),
         AccountMeta::new_readonly(*realm_authority, true),
     ];
 
-    let remove_authority = if let Some(new_realm_authority) = new_realm_authority {
-        accounts.push(AccountMeta::new_readonly(*new_realm_authority, false));
-        false
-    } else {
-        true
-    };
+    match action {
+        SetRealmAuthorityAction::SetChecked | SetRealmAuthorityAction::SetUnchecked => {
+            accounts.push(AccountMeta::new_readonly(
+                *new_realm_authority.unwrap(),
+                false,
+            ));
+        }
+        SetRealmAuthorityAction::Remove => {}
+    }
 
-    let instruction = GovernanceInstruction::SetRealmAuthority { remove_authority };
+    let instruction = GovernanceInstruction::SetRealmAuthority { action };
 
     Instruction {
         program_id: *program_id,

governance/program/src/processor/mod.rs

@@ -198,8 +198,8 @@ pub fn process_instruction(
         GovernanceInstruction::FlagInstructionError {} => {
             process_flag_instruction_error(program_id, accounts)
         }
-        GovernanceInstruction::SetRealmAuthority { remove_authority } => {
-            process_set_realm_authority(program_id, accounts, remove_authority)
+        GovernanceInstruction::SetRealmAuthority { action } => {
+            process_set_realm_authority(program_id, accounts, action)
         }
         GovernanceInstruction::SetRealmConfig { config_args } => {
             process_set_realm_config(program_id, accounts, config_args)

governance/program/src/processor/process_set_realm_authority.rs

@@ -9,14 +9,17 @@ use solana_program::{
 
 use crate::{
     error::GovernanceError,
-    state::{governance::assert_governance_for_realm, realm::get_realm_data_for_authority},
+    state::{
+        governance::assert_governance_for_realm,
+        realm::{get_realm_data_for_authority, SetRealmAuthorityAction},
+    },
 };
 
 /// Processes SetRealmAuthority instruction
 pub fn process_set_realm_authority(
     program_id: &Pubkey,
     accounts: &[AccountInfo],
-    remove_authority: bool,
+    action: SetRealmAuthorityAction,
 ) -> ProgramResult {
     let account_info_iter = &mut accounts.iter();
 
@@ -30,16 +33,18 @@ pub fn process_set_realm_authority(
         return Err(GovernanceError::RealmAuthorityMustSign.into());
     }
 
-    let new_realm_authority = if remove_authority {
-        None
-    } else {
-        // Ensure the new realm authority is one of the governances from the realm
-        // Note: This is not a security feature because governance creation is only gated with min_community_tokens_to_create_governance
-        //       The check is done to prevent scenarios where the authority could be accidentally set to a wrong or none existing account
-        let new_realm_authority_info = next_account_info(account_info_iter)?; // 2
-        assert_governance_for_realm(program_id, new_realm_authority_info, realm_info.key)?;
+    let new_realm_authority = match action {
+        SetRealmAuthorityAction::SetUnchecked | SetRealmAuthorityAction::SetChecked => {
+            let new_realm_authority_info = next_account_info(account_info_iter)?; // 2
 
-        Some(*new_realm_authority_info.key)
+            if action == SetRealmAuthorityAction::SetChecked {
+                // Ensure the new realm authority is one of the governances from the realm
+                assert_governance_for_realm(program_id, new_realm_authority_info, realm_info.key)?;
+            }
+
+            Some(*new_realm_authority_info.key)
+        }
+        SetRealmAuthorityAction::Remove => None,
     };
 
     realm_data.authority = new_realm_authority;

governance/program/src/state/realm.rs

@@ -36,6 +36,22 @@ pub struct RealmConfigArgs {
     pub use_max_community_voter_weight_addin: bool,
 }
 
+/// SetRealmAuthority instruction action
+#[derive(Clone, Debug, PartialEq, BorshDeserialize, BorshSerialize, BorshSchema)]
+pub enum SetRealmAuthorityAction {
+    /// Sets realm authority without any checks
+    /// Uncheck option allows to set the realm authority to non governance accounts
+    SetUnchecked,
+
+    /// Sets realm authority and checks the new new authority is one of the realm's governances
+    // Note: This is not a security feature because governance creation is only gated with min_community_tokens_to_create_governance
+    //       The check is done to prevent scenarios where the authority could be accidentally set to a wrong or none existing account
+    SetChecked,
+
+    /// Removes realm authority
+    Remove,
+}
+
 /// Realm Config defining Realm parameters.
 #[repr(C)]
 #[derive(Clone, Debug, PartialEq, BorshDeserialize, BorshSerialize, BorshSchema)]

governance/program/tests/process_set_realm_authority.rs

@@ -36,7 +36,7 @@ async fn test_set_realm_authority() {
 
     // Act
     governance_test
-        .set_realm_authority(&realm_cookie, &Some(new_realm_authority))
+        .set_realm_authority(&realm_cookie, Some(&new_realm_authority))
         .await
         .unwrap();
 
@@ -59,7 +59,7 @@ async fn test_set_realm_authority_with_non_existing_new_authority_error() {
 
     // Act
     let err = governance_test
-        .set_realm_authority(&realm_cookie, &Some(new_realm_authority))
+        .set_realm_authority(&realm_cookie, Some(&new_realm_authority))
         .await
         .err()
         .unwrap();
@@ -77,7 +77,7 @@ async fn test_set_realm_authority_to_none() {
 
     // Act
     governance_test
-        .set_realm_authority(&realm_cookie, &None)
+        .set_realm_authority(&realm_cookie, None)
         .await
         .unwrap();
 
@@ -89,6 +89,29 @@ async fn test_set_realm_authority_to_none() {
     assert_eq!(realm_account.authority, None);
 }
 
+#[tokio::test]
+async fn test_set_realm_authority_unchecked() {
+    // Arrange
+    let mut governance_test = GovernanceProgramTest::start_new().await;
+
+    let realm_cookie = governance_test.with_realm().await;
+
+    let new_realm_authority = Pubkey::new_unique();
+
+    // Act
+    governance_test
+        .set_realm_authority_impl(&realm_cookie, Some(&new_realm_authority), false)
+        .await
+        .unwrap();
+
+    // Assert
+    let realm_account = governance_test
+        .get_realm_account(&realm_cookie.address)
+        .await;
+
+    assert_eq!(realm_account.authority, Some(new_realm_authority));
+}
+
 #[tokio::test]
 async fn test_set_realm_authority_with_no_authority_error() {
     // Arrange
@@ -97,15 +120,15 @@ async fn test_set_realm_authority_with_no_authority_error() {
     let realm_cookie = governance_test.with_realm().await;
 
     governance_test
-        .set_realm_authority(&realm_cookie, &None)
+        .set_realm_authority(&realm_cookie, None)
         .await
         .unwrap();
 
     let new_realm_authority = Pubkey::new_unique();
 
     // Act
     let err = governance_test
-        .set_realm_authority(&realm_cookie, &Some(new_realm_authority))
+        .set_realm_authority(&realm_cookie, Some(&new_realm_authority))
         .await
         .err()
         .unwrap();
@@ -129,7 +152,7 @@ async fn test_set_realm_authority_with_invalid_authority_error() {
 
     // Act
     let err = governance_test
-        .set_realm_authority(&realm_cookie, &Some(new_realm_authority))
+        .set_realm_authority(&realm_cookie, Some(&new_realm_authority))
         .await
         .err()
         .unwrap();
@@ -151,7 +174,8 @@ async fn test_set_realm_authority_with_authority_must_sign_error() {
     let err = governance_test
         .set_realm_authority_using_instruction(
             &realm_cookie,
-            &Some(new_realm_authority),
+            Some(&new_realm_authority),
+            true,
             |i| i.accounts[1].is_signer = false, // realm_authority
             Some(&[]),
         )
@@ -193,7 +217,7 @@ async fn test_set_realm_authority_with_governance_from_other_realm_error() {
 
     // Act
     let err = governance_test
-        .set_realm_authority(&realm_cookie, &Some(new_realm_authority))
+        .set_realm_authority(&realm_cookie, Some(&new_realm_authority))
         .await
         .err()
         .unwrap();

governance/program/tests/process_set_realm_config.rs

@@ -112,7 +112,7 @@ async fn test_set_realm_config_with_no_authority_error() {
     };
 
     governance_test
-        .set_realm_authority(&realm_cookie, &None)
+        .set_realm_authority(&realm_cookie, None)
         .await
         .unwrap();
 

governance/program/tests/program_test/mod.rs

@@ -43,7 +43,7 @@ use spl_governance::{
         },
         realm::{
             get_governing_token_holding_address, get_realm_address, Realm, RealmConfig,
-            RealmConfigArgs,
+            RealmConfigArgs, SetRealmAuthorityAction,
         },
         realm_config::{get_realm_config_address, RealmConfigAccount},
         signatory_record::{get_signatory_record_address, SignatoryRecord},
@@ -857,11 +857,29 @@ impl GovernanceProgramTest {
     pub async fn set_realm_authority(
         &mut self,
         realm_cookie: &RealmCookie,
-        new_realm_authority: &Option<Pubkey>,
+        new_realm_authority: Option<&Pubkey>,
     ) -> Result<(), ProgramError> {
         self.set_realm_authority_using_instruction(
             realm_cookie,
             new_realm_authority,
+            true,
+            NopOverride,
+            None,
+        )
+        .await
+    }
+
+    #[allow(dead_code)]
+    pub async fn set_realm_authority_impl(
+        &mut self,
+        realm_cookie: &RealmCookie,
+        new_realm_authority: Option<&Pubkey>,
+        check_authority: bool,
+    ) -> Result<(), ProgramError> {
+        self.set_realm_authority_using_instruction(
+            realm_cookie,
+            new_realm_authority,
+            check_authority,
             NopOverride,
             None,
         )
@@ -872,15 +890,27 @@ impl GovernanceProgramTest {
     pub async fn set_realm_authority_using_instruction<F: Fn(&mut Instruction)>(
         &mut self,
         realm_cookie: &RealmCookie,
-        new_realm_authority: &Option<Pubkey>,
+        new_realm_authority: Option<&Pubkey>,
+        check_authority: bool,
         instruction_override: F,
         signers_override: Option<&[&Keypair]>,
     ) -> Result<(), ProgramError> {
+        let action = if new_realm_authority.is_some() {
+            if check_authority {
+                SetRealmAuthorityAction::SetChecked
+            } else {
+                SetRealmAuthorityAction::SetUnchecked
+            }
+        } else {
+            SetRealmAuthorityAction::Remove
+        };
+
         let mut set_realm_authority_ix = set_realm_authority(
             &self.program_id,
             &realm_cookie.address,
             &realm_cookie.realm_authority.as_ref().unwrap().pubkey(),
             new_realm_authority,
+            action,
         );
 
         instruction_override(&mut set_realm_authority_ix);