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/error.rs

@@ -23,44 +23,46 @@ pub enum TimelockError {
     #[msg("Provided ID is invalid")]
     InvalidId,
 
-    #[msg("RBACTimelock: operation not finalized")]
+    #[msg("operation not finalized")]
     OperationNotFinalized,
 
-    #[msg("RBACTimelock: operation is already finalized")]
+    #[msg("operation is already finalized")]
     OperationAlreadyFinalized,
 
-    #[msg("RBACTimelock: too many instructions in the operation")]
+    #[msg("too many instructions in the operation")]
     TooManyInstructions,
 
     // on attempt to create PDA with the same seed(existing operation)
-    #[msg("RBACTimelock: operation already scheduled")]
+    #[msg("operation already scheduled")]
     OperationAlreadyScheduled,
 
-    #[msg("RBACTimelock: insufficient delay")]
+    #[msg("insufficient delay")]
     DelayInsufficient,
 
-    // cancel
-    #[msg("RBACTimelock: operation cannot be cancelled")]
+    #[msg("operation cannot be cancelled")]
     OperationNotCancellable,
 
     #[msg("operation is not ready")]
     OperationNotReady,
 
+    #[msg("operation is already executed")]
+    OperationAlreadyExecuted,
+
     #[msg("Predecessor operation is not found")]
     MissingDependency,
 
-    #[msg("RBACTimelock: Provided access controller is invalid")]
+    #[msg("Provided access controller is invalid")]
     InvalidAccessController,
 
-    #[msg("RBACTimelock: selector is blocked")]
+    #[msg("selector is blocked")]
     BlockedSelector,
 
-    #[msg("RBACTimelock: selector is already blocked")]
+    #[msg("selector is already blocked")]
     AlreadyBlocked,
 
-    #[msg("RBACTimelock: selector not found")]
+    #[msg("selector not found")]
     SelectorNotFound,
 
-    #[msg("RBACTimelock: maximum capacity reached for function blocker")]
+    #[msg("maximum capacity reached for function blocker")]
     MaxCapacityReached,
 }

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
         );
     }
@@ -78,8 +80,6 @@ pub fn execute_batch<'info>(
         });
     }
 
-    require!(op.is_ready(current_time), TimelockError::OperationNotReady);
-
     // all executed, update the timestamp
     op.mark_done();
 
@@ -88,10 +88,11 @@ pub fn execute_batch<'info>(
 
 /// execute operation(instructions) w/o checking predecessors and readiness
 /// bypasser_execute also need the operation to be uploaded formerly
+/// NOTE: operation should be closed after execution
 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 +167,23 @@ 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,
+        constraint = !operation.is_done() @ TimelockError::OperationAlreadyExecuted,
+        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)]