solana: timelock preloading ops authority

chains/solana/contracts/programs/timelock/src/access.rs

@@ -5,56 +5,86 @@ use access_controller::AccessController;
 use crate::error::{AuthError, TimelockError};
 use crate::state::{Config, Role};
 
-// NOTE: This macro is used to check if the authority is the owner
-// or account that has access to the role
+/// A macro that checks if the authority has admin privileges OR has at least one of the specified roles.
+/// This is used as an attribute macro with #[access_control] to guard program instructions.
+///
+/// # Arguments
+/// * `$ctx` - The context containing program accounts
+/// * `$role` - One or more roles that are allowed to execute the instruction
+///
 #[macro_export]
-macro_rules! only_role_or_admin_role {
-    ($ctx:expr, $role:expr) => {
-        only_role_or_admin_role(
+macro_rules! require_role_or_admin {
+    ($ctx:expr, $($role:expr),+) => {{
+        only_role_or_admin(
             &$ctx.accounts.config,
             &$ctx.accounts.role_access_controller,
             &$ctx.accounts.authority,
-            $role,
+            &[$($role),+]
         )
-    };
+    }};
 }
 
-pub fn only_role_or_admin_role(
+/// Helper function to verify if an authority has admin rights or any of the specified roles.
+/// Returns Ok(()) if the authority is either the admin or has at least one of the roles.
+///
+/// # Arguments
+/// * `config` - Account containing program configuration including owner
+/// * `role_controller` - Account managing role-based access control
+/// * `authority` - The signer attempting to execute the instruction
+/// * `roles` - Array of roles being checked for authorization
+///
+/// # Returns
+/// * `Result<()>` - Ok if authorized
+/// * `Err(TimelockError::InvalidAccessController)` - If provided controller isn't configured for any roles
+/// * `Err(AuthError::Unauthorized)` - If authority lacks admin rights and required roles
+
+pub fn only_role_or_admin(
     config: &Account<Config>,
     role_controller: &AccountLoader<AccessController>,
     authority: &Signer,
-    role: Role,
+    roles: &[Role],
 ) -> Result<()> {
-    // early authorization check if the authority is the owner
     if authority.key() == config.owner {
         return Ok(());
     }
 
-    // check the provided access controller is the correct one
-    require_keys_eq!(
-        config.get_role_controller(&role),
-        role_controller.key(),
+    require!(
+        roles
+            .iter()
+            .any(|role| config.get_role_controller(role) == role_controller.key()),
         TimelockError::InvalidAccessController
     );
 
-    // check if the authority has access to the role
-    require!(
-        access_controller::has_access(role_controller, &authority.key())?,
-        AuthError::Unauthorized
-    );
+    let has_valid_role = roles.iter().any(|role| {
+        config.get_role_controller(role) == role_controller.key()
+            && access_controller::has_access(role_controller, &authority.key()).unwrap_or(false)
+    });
 
+    require!(has_valid_role, AuthError::Unauthorized);
     Ok(())
 }
 
-// NOTE: This macro is used to check if the authority is the owner
-// this can be in account contexts, but added also for explicit consistency with other role check
+/// A macro that checks if the authority is the admin (owner) of the program.
+/// This provides a simpler check when only admin access is required.
+///
+/// # Arguments
+/// * `$ctx` - The context containing program accounts
 #[macro_export]
-macro_rules! only_admin {
+macro_rules! require_only_admin {
     ($ctx:expr) => {
         only_admin(&$ctx.accounts.config, &$ctx.accounts.authority)
     };
 }
 
+/// Helper function to verify if an authority is the admin (owner).
+/// Returns Ok(()) if the authority is the admin, Err otherwise.
+///
+/// # Arguments
+/// * `config` - Account containing program configuration including owner
+/// * `authority` - The signer attempting to execute the instruction
+///
+/// # Returns
+/// * `Result<()>` - Ok if authorized, Err with AuthError::Unauthorized otherwise
 pub fn only_admin(config: &Account<Config>, authority: &Signer) -> Result<()> {
     require_keys_eq!(authority.key(), config.owner, AuthError::Unauthorized);
     Ok(())

chains/solana/contracts/programs/timelock/src/instructions/cancel.rs

@@ -1,4 +1,5 @@
 use anchor_lang::prelude::*;
+use anchor_lang::solana_program::keccak::HASH_BYTES;
 
 use access_controller::AccessController;
 
@@ -11,15 +12,15 @@ use crate::state::{Config, Operation};
 pub fn cancel<'info>(
     _ctx: Context<'_, '_, '_, 'info, Cancel<'info>>,
     _timelock_id: [u8; TIMELOCK_ID_PADDED],
-    id: [u8; 32],
+    id: [u8; HASH_BYTES],
 ) -> Result<()> {
     emit!(Cancelled { id });
     // NOTE: PDA is closed - is handled by anchor on exit due to the `close` attribute
     Ok(())
 }
 
 #[derive(Accounts)]
-#[instruction(timelock_id: [u8; TIMELOCK_ID_PADDED], id: [u8; 32])]
+#[instruction(timelock_id: [u8; TIMELOCK_ID_PADDED], id: [u8; HASH_BYTES])]
 pub struct Cancel<'info> {
     #[account(
         mut,
@@ -34,7 +35,7 @@ pub struct Cancel<'info> {
     #[account( seeds = [TIMELOCK_CONFIG_SEED, timelock_id.as_ref()], bump)]
     pub config: Account<'info, Config>,
 
-    // NOTE: access controller check happens in only_role_or_admin_role macro
+    // NOTE: access controller check happens in require_role_or_admin macro
     pub role_access_controller: AccountLoader<'info, AccessController>,
 
     #[account(mut)]

chains/solana/contracts/programs/timelock/src/instructions/execute.rs

@@ -19,7 +19,7 @@ use crate::state::{Config, InstructionData, Operation};
 pub fn execute_batch<'info>(
     ctx: Context<'_, '_, '_, 'info, ExecuteBatch<'info>>,
     timelock_id: [u8; TIMELOCK_ID_PADDED],
-    _id: [u8; 32],
+    _id: [u8; HASH_BYTES],
 ) -> Result<()> {
     let op = &mut ctx.accounts.operation;
 
@@ -38,8 +38,9 @@ pub fn execute_batch<'info>(
             ctx.program_id,
         );
 
-        require!(
-            ctx.accounts.predecessor_operation.key() == expected_address,
+        require_keys_eq!(
+            ctx.accounts.predecessor_operation.key(),
+            expected_address,
             TimelockError::InvalidInput
         );
 
@@ -49,8 +50,9 @@ pub fn execute_batch<'info>(
 
         require!(predecessor_acc.is_done(), TimelockError::MissingDependency);
     } else {
-        require!(
-            ctx.accounts.predecessor_operation.key() == Pubkey::zeroed(),
+        require_keys_eq!(
+            ctx.accounts.predecessor_operation.key(),
+            Pubkey::zeroed(),
             TimelockError::InvalidInput
         );
     }
@@ -91,7 +93,7 @@ pub fn execute_batch<'info>(
 pub fn bypasser_execute_batch<'info>(
     ctx: Context<'_, '_, '_, 'info, BypasserExecuteBatch<'info>>,
     timelock_id: [u8; TIMELOCK_ID_PADDED],
-    _id: [u8; 32],
+    _id: [u8; HASH_BYTES],
 ) -> Result<()> {
     let op = &mut ctx.accounts.operation;
 
@@ -166,21 +168,22 @@ pub struct ExecuteBatch<'info> {
     )]
     pub timelock_signer: UncheckedAccount<'info>,
 
-    // NOTE: access controller check happens in only_role_or_admin_role macro
+    // NOTE: access controller check happens in require_role_or_admin macro
     pub role_access_controller: AccountLoader<'info, AccessController>,
 
     #[account(mut)]
     pub authority: Signer<'info>,
 }
 
 #[derive(Accounts)]
-#[instruction(timelock_id: [u8; 32], id: [u8; 32])]
+#[instruction(timelock_id: [u8; TIMELOCK_ID_PADDED], id: [u8; HASH_BYTES])]
 pub struct BypasserExecuteBatch<'info> {
     #[account(
         mut,
         seeds = [TIMELOCK_OPERATION_SEED, timelock_id.as_ref(), id.as_ref()],
         bump,
         constraint = operation.is_finalized @ TimelockError::OperationNotFinalized,
+        close = authority, // close the operation after execution
     )]
     pub operation: Account<'info, Operation>,
 
@@ -194,7 +197,7 @@ pub struct BypasserExecuteBatch<'info> {
     )]
     pub timelock_signer: UncheckedAccount<'info>,
 
-    // NOTE: access controller check happens in only_role_or_admin_role macro
+    // NOTE: access controller check happens in require_role_or_admin macro
     pub role_access_controller: AccountLoader<'info, AccessController>,
 
     #[account(mut)]

chains/solana/contracts/programs/timelock/src/instructions/schedule.rs

@@ -1,11 +1,12 @@
 use anchor_lang::prelude::*;
+use anchor_lang::solana_program::keccak::HASH_BYTES;
 
 use access_controller::AccessController;
 
 use crate::constants::{
     ANCHOR_DISCRIMINATOR, TIMELOCK_CONFIG_SEED, TIMELOCK_ID_PADDED, TIMELOCK_OPERATION_SEED,
 };
-use crate::error::{AuthError, TimelockError};
+use crate::error::TimelockError;
 use crate::event::*;
 use crate::state::{Config, InstructionData, Operation};
 
@@ -14,7 +15,7 @@ use crate::state::{Config, InstructionData, Operation};
 pub fn schedule_batch<'info>(
     ctx: Context<'_, '_, '_, 'info, ScheduleBatch<'info>>,
     _timelock_id: [u8; TIMELOCK_ID_PADDED],
-    _id: [u8; 32],
+    _id: [u8; HASH_BYTES],
     delay: u64,
 ) -> Result<()> {
     // delay should greater than min_delay
@@ -70,8 +71,8 @@ pub fn schedule_batch<'info>(
 pub fn initialize_operation<'info>(
     ctx: Context<'_, '_, '_, 'info, InitializeOperation<'info>>,
     _timelock_id: [u8; TIMELOCK_ID_PADDED],
-    id: [u8; 32],
-    predecessor: [u8; 32],
+    id: [u8; HASH_BYTES],
+    predecessor: [u8; HASH_BYTES],
     salt: [u8; 32],
     instruction_count: u32,
 ) -> Result<()> {
@@ -95,7 +96,7 @@ pub fn initialize_operation<'info>(
 pub fn append_instructions<'info>(
     ctx: Context<'_, '_, '_, 'info, AppendInstructions<'info>>,
     _timelock_id: [u8; TIMELOCK_ID_PADDED],
-    _id: [u8; 32],
+    _id: [u8; HASH_BYTES],
     instructions_batch: Vec<InstructionData>,
 ) -> Result<()> {
     let op = &mut ctx.accounts.operation;
@@ -109,7 +110,7 @@ pub fn append_instructions<'info>(
 pub fn finalize_operation<'info>(
     ctx: Context<'_, '_, '_, 'info, FinalizeOperation<'info>>,
     _timelock_id: [u8; TIMELOCK_ID_PADDED],
-    _id: [u8; 32],
+    _id: [u8; HASH_BYTES],
 ) -> Result<()> {
     let op = &mut ctx.accounts.operation;
     op.is_finalized = true;
@@ -120,15 +121,15 @@ pub fn finalize_operation<'info>(
 pub fn clear_operation(
     _ctx: Context<ClearOperation>,
     _timelock_id: [u8; TIMELOCK_ID_PADDED],
-    _id: [u8; 32],
+    _id: [u8; HASH_BYTES],
 ) -> Result<()> {
     // NOTE: ctx.accounts.operation is closed to be able to re-initialized,
     // also allow finalized operation to be cleared
     Ok(())
 }
 
 #[derive(Accounts)]
-#[instruction(timelock_id: [u8; TIMELOCK_ID_PADDED], id: [u8; 32])]
+#[instruction(timelock_id: [u8; TIMELOCK_ID_PADDED], id: [u8; HASH_BYTES])]
 pub struct ScheduleBatch<'info> {
     #[account(
         mut,
@@ -142,7 +143,7 @@ pub struct ScheduleBatch<'info> {
     #[account(seeds = [TIMELOCK_CONFIG_SEED, timelock_id.as_ref()], bump)]
     pub config: Account<'info, Config>,
 
-    // NOTE: access controller check and access happens in only_role_or_admin_role macro
+    // NOTE: access controller check and access happens in require_role_or_admin macro
     pub role_access_controller: AccountLoader<'info, AccessController>,
 
     #[account(mut)]
@@ -152,9 +153,9 @@ pub struct ScheduleBatch<'info> {
 #[derive(Accounts)]
 #[instruction(
     timelock_id: [u8; TIMELOCK_ID_PADDED],
-    id: [u8; 32],
-    predecessor: [u8; 32],
-    salt: [u8; 32],
+    id: [u8; HASH_BYTES],
+    predecessor: [u8; HASH_BYTES],
+    salt: [u8; HASH_BYTES],
     instruction_count: u32,
 )]
 pub struct InitializeOperation<'info> {
@@ -171,14 +172,17 @@ pub struct InitializeOperation<'info> {
     #[account(seeds = [TIMELOCK_CONFIG_SEED, timelock_id.as_ref()], bump)]
     pub config: Account<'info, Config>,
 
+    // NOTE: access controller check and access happens in require_role_or_admin macro
+    pub role_access_controller: AccountLoader<'info, AccessController>,
+
     #[account(mut)]
     pub authority: Signer<'info>,
 
     pub system_program: Program<'info, System>,
 }
 
 #[derive(Accounts)]
-#[instruction(timelock_id: [u8; TIMELOCK_ID_PADDED], id: [u8; 32], instructions_batch: Vec<InstructionData>)]
+#[instruction(timelock_id: [u8; TIMELOCK_ID_PADDED], id: [u8; HASH_BYTES], instructions_batch: Vec<InstructionData>)]
 pub struct AppendInstructions<'info> {
     #[account(
         mut,
@@ -203,20 +207,17 @@ pub struct AppendInstructions<'info> {
     #[account(seeds = [TIMELOCK_CONFIG_SEED, timelock_id.as_ref()], bump)]
     pub config: Account<'info, Config>,
 
-    #[account(
-        mut,
-        constraint = (
-            operation.authority == authority.key() ||
-            config.owner == authority.key()
-        ) @ AuthError::Unauthorized
-    )]
+    // NOTE: access controller check and access happens in require_role_or_admin macro
+    pub role_access_controller: AccountLoader<'info, AccessController>,
+
+    #[account(mut)]
     pub authority: Signer<'info>,
 
     pub system_program: Program<'info, System>,
 }
 
 #[derive(Accounts)]
-#[instruction(timelock_id: [u8; TIMELOCK_ID_PADDED], id: [u8; 32])]
+#[instruction(timelock_id: [u8; TIMELOCK_ID_PADDED], id: [u8; HASH_BYTES])]
 pub struct FinalizeOperation<'info> {
     #[account(
         mut,
@@ -232,18 +233,15 @@ pub struct FinalizeOperation<'info> {
     #[account(seeds = [TIMELOCK_CONFIG_SEED, timelock_id.as_ref()], bump)]
     pub config: Account<'info, Config>,
 
-    #[account(
-        mut,
-        constraint = (
-            operation.authority == authority.key() ||
-            config.owner == authority.key()
-        ) @ AuthError::Unauthorized
-    )]
+    // NOTE: access controller check and access happens in require_role_or_admin macro
+    pub role_access_controller: AccountLoader<'info, AccessController>,
+
+    #[account(mut)]
     pub authority: Signer<'info>,
 }
 
 #[derive(Accounts)]
-#[instruction(timelock_id: [u8; TIMELOCK_ID_PADDED], id: [u8; 32])]
+#[instruction(timelock_id: [u8; TIMELOCK_ID_PADDED], id: [u8; HASH_BYTES])]
 pub struct ClearOperation<'info> {
     #[account(
         mut,
@@ -257,12 +255,9 @@ pub struct ClearOperation<'info> {
     #[account(seeds = [TIMELOCK_CONFIG_SEED, timelock_id.as_ref()], bump)]
     pub config: Account<'info, Config>,
 
-    #[account(
-        mut,
-        constraint = (
-            operation.authority == authority.key() ||
-            config.owner == authority.key()
-        ) @ AuthError::Unauthorized
-    )]
+    // NOTE: access controller check and access happens in require_role_or_admin macro
+    pub role_access_controller: AccountLoader<'info, AccessController>,
+
+    #[account(mut)]
     pub authority: Signer<'info>,
 }