Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/discriminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ pub enum DlpDiscriminator {
UndelegateConfinedAccount = 18,
/// See [crate::processor::process_delegate_with_any_validator] for docs.
DelegateWithAnyValidator = 19,
/// See [crate::processor::process_call_handler_v2] for docs.
CallHandlerV2 = 20,
}

impl DlpDiscriminator {
Expand Down
3 changes: 3 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ use num_enum::IntoPrimitive;
use solana_program::program_error::ProgramError;
use thiserror::Error;

pub const INVALID_ESCROW_PDA: &str = "invalid escrow pda in CallHandler";
pub const INVALID_ESCROW_OWNER: &str = "escrow can not be delegated in CallHandler";

#[derive(Debug, Error, Clone, Copy, PartialEq, Eq, IntoPrimitive)]
#[repr(u32)]
pub enum DlpError {
Expand Down
1 change: 1 addition & 0 deletions src/instruction_builder/call_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use solana_program::{instruction::AccountMeta, pubkey::Pubkey};

/// Builds a call handler instruction.
/// See [crate::processor::call_handler] for docs.
#[deprecated(since = "1.1.4", note = "Use `call_handler_v2` instead")]
pub fn call_handler(
validator: Pubkey,
destination_program: Pubkey,
Expand Down
64 changes: 64 additions & 0 deletions src/instruction_builder/call_handler_v2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
use crate::args::CallHandlerArgs;
use crate::discriminator::DlpDiscriminator;
use crate::pda::{ephemeral_balance_pda_from_payer, validator_fees_vault_pda_from_validator};
use crate::{total_size_budget, AccountSizeClass, DLP_PROGRAM_DATA_SIZE_CLASS};
use borsh::to_vec;
use solana_program::instruction::Instruction;
use solana_program::{instruction::AccountMeta, pubkey::Pubkey};

/// Builds a call handler v2 instruction.
/// See [crate::processor::call_handler_v2] for docs.
pub fn call_handler_v2(
validator: Pubkey,
destination_program: Pubkey,
source_program: Pubkey,
escrow_authority: Pubkey,
other_accounts: Vec<AccountMeta>,
args: CallHandlerArgs,
) -> Instruction {
let validator_fees_vault_pda = validator_fees_vault_pda_from_validator(&validator);

// handler accounts
let escrow_account = ephemeral_balance_pda_from_payer(&escrow_authority, args.escrow_index);
let mut accounts = vec![
AccountMeta::new(validator, true),
AccountMeta::new(validator_fees_vault_pda, false),
AccountMeta::new_readonly(destination_program, false),
AccountMeta::new_readonly(source_program, false),
AccountMeta::new(escrow_authority, false),
AccountMeta::new(escrow_account, false),
];
// append other accounts at the end
accounts.extend(other_accounts);

Instruction {
program_id: crate::id(),
accounts,
data: [
DlpDiscriminator::CallHandlerV2.to_vec(),
to_vec(&args).unwrap(),
]
.concat(),
}
}

///
/// Returns accounts-data-size budget for call_handler_v2 instruction.
///
/// This value can be used with ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit
///
pub fn call_handler_v2_size_budget(
destination_program: AccountSizeClass,
source_program: AccountSizeClass,
other_accounts: u32,
) -> u32 {
total_size_budget(&[
DLP_PROGRAM_DATA_SIZE_CLASS,
AccountSizeClass::Tiny, // validator
AccountSizeClass::Tiny, // validator_fees_vault_pda
destination_program,
source_program,
AccountSizeClass::Tiny, // escrow_authority
AccountSizeClass::Tiny, // escrow_account
]) + other_accounts
}
2 changes: 2 additions & 0 deletions src/instruction_builder/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod call_handler;
mod call_handler_v2;
mod close_ephemeral_balance;
mod close_validator_fees_vault;
mod commit_diff;
Expand All @@ -18,6 +19,7 @@ mod validator_claim_fees;
mod whitelist_validator_for_program;

pub use call_handler::*;
pub use call_handler_v2::*;
pub use close_ephemeral_balance::*;
pub use close_validator_fees_vault::*;
pub use commit_diff::*;
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ pub fn slow_process_instruction(
DlpDiscriminator::CallHandler => {
processor::process_call_handler(program_id, accounts, data)?
}
DlpDiscriminator::CallHandlerV2 => {
processor::process_call_handler_v2(program_id, accounts, data)?
}
_ => {
#[cfg(feature = "logging")]
msg!("PANIC: Instruction must be processed by fast_process_instruction");
Expand Down
18 changes: 2 additions & 16 deletions src/processor/call_handler.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::args::CallHandlerArgs;
use crate::ephemeral_balance_seeds_from_payer;
use crate::error::{INVALID_ESCROW_OWNER, INVALID_ESCROW_PDA};
use crate::processor::utils::loaders::{
load_initialized_validator_fees_vault, load_owned_pda, load_pda, load_signer,
};
Expand All @@ -11,10 +12,7 @@ use solana_program::instruction::{AccountMeta, Instruction};
use solana_program::program::invoke_signed;
use solana_program::program_error::ProgramError;
use solana_program::pubkey::Pubkey;
use solana_program::{msg, system_program};

pub const INVALID_ESCROW_PDA: &str = "invalid escrow pda in CallHandler";
pub const INVALID_ESCROW_OWNER: &str = "escrow can not be delegated in CallHandler";
use solana_program::system_program;

/// Calls a handler on user specified program
///
Expand Down Expand Up @@ -50,10 +48,6 @@ pub fn process_call_handler(
) -> ProgramResult {
const OTHER_ACCOUNTS_OFFSET: usize = 5;

if accounts.len() < OTHER_ACCOUNTS_OFFSET {
return Err(ProgramError::NotEnoughAccountKeys);
}

let (
[validator, validator_fees_vault, destination_program, escrow_authority_account, escrow_account],
other_accounts,
Expand All @@ -68,14 +62,6 @@ pub fn process_call_handler(
load_signer(validator, "validator")?;
// verify signer is a registered validator
load_initialized_validator_fees_vault(validator, validator_fees_vault, true)?;
// Check if destination program is executable
if !destination_program.executable {
msg!(
"{} program is not executable: destination program",
destination_program.key
);
return Err(ProgramError::InvalidAccountData);
}

// verify passed escrow_account derived from escrow authority
let escrow_seeds: &[&[u8]] =
Expand Down
110 changes: 110 additions & 0 deletions src/processor/call_handler_v2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
use crate::args::CallHandlerArgs;
use crate::ephemeral_balance_seeds_from_payer;
use crate::error::{INVALID_ESCROW_OWNER, INVALID_ESCROW_PDA};
use crate::processor::utils::loaders::{
load_initialized_validator_fees_vault, load_owned_pda, load_pda, load_signer,
};

use borsh::BorshDeserialize;
use solana_program::account_info::AccountInfo;
use solana_program::entrypoint::ProgramResult;
use solana_program::instruction::{AccountMeta, Instruction};
use solana_program::program::invoke_signed;
use solana_program::program_error::ProgramError;
use solana_program::pubkey::Pubkey;
use solana_program::system_program;

/// Calls a handler on user specified program
///
/// Accounts:
/// 0: `[signer]` validator
/// 1: `[]` validator fee vault to verify its registration
/// 2: `[]` destination program of an action
/// 3: `[]` source program of an action
/// 4: `[]` escrow authority account which created escrow account
/// 5: `[writable]` non delegated escrow PDA created from escrow_authority (index 4)
/// 6: `[readonly/writable]` other accounts needed for action
/// 7: `[readonly/writable]` other accounts needed for action
/// 8: ...
///
/// Requirements:
///
/// - escrow account initialized
/// - escrow account not delegated
/// - validator as a caller
///
/// Steps:
/// 1. Verify that signer is a valid registered validator
/// 2. Verify escrow pda exists and not delegated
/// 3. Invoke signed on behalf of escrow pda user specified action
///
/// Usage:
///
/// This instruction is meant to be called via CPI with the owning program signing for the
/// delegated account.
pub fn process_call_handler_v2(
_program_id: &Pubkey,
accounts: &[AccountInfo],
data: &[u8],
) -> ProgramResult {
const OTHER_ACCOUNTS_OFFSET: usize = 6;

let (
[validator, validator_fees_vault, destination_program, source_program, escrow_authority_account, escrow_account],
other_accounts,
) = accounts.split_at(OTHER_ACCOUNTS_OFFSET)
else {
return Err(ProgramError::NotEnoughAccountKeys);
};

let args = CallHandlerArgs::try_from_slice(data)?;

// verify account is a signer
load_signer(validator, "validator")?;
// verify signer is a registered validator
load_initialized_validator_fees_vault(validator, validator_fees_vault, true)?;

// verify passed escrow_account derived from escrow authority
let escrow_seeds: &[&[u8]] =
ephemeral_balance_seeds_from_payer!(escrow_authority_account.key, args.escrow_index);
let escrow_bump = load_pda(
escrow_account,
escrow_seeds,
&crate::id(),
true,
INVALID_ESCROW_PDA,
)?;
load_owned_pda(escrow_account, &system_program::id(), INVALID_ESCROW_OWNER)?;
Comment on lines +67 to +77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

source_program executability is never verified.

source_program is passed as a plain account in the CPI (not the program_id), so the Solana runtime will not enforce that it is executable. Without an explicit check, any arbitrary data account can be supplied as source_program, giving the destination program a misleading "source" key — which directly undermines the feature's stated goal ("users must be able to get source program from where the action was scheduled").

A prior code review on this PR observed that source_program was checked for executability ("source_program is checked for executability, correctly, since it's passed as a plain account to CPI and the runtime won't validate it"), implying the check may have been removed in a later iteration.

🛡️ Proposed fix
     // verify passed escrow_account derived from escrow authority
+    if !source_program.executable {
+        return Err(ProgramError::InvalidAccountData);
+    }
     let escrow_seeds: &[&[u8]] =
         ephemeral_balance_seeds_from_payer!(escrow_authority_account.key, args.escrow_index);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/processor/call_handler_v2.rs` around lines 67 - 77, The code never
verifies that the plain account passed as source_program is executable, so add
an explicit executability check for source_program (e.g., right after the PDA
checks for escrow_account) and return a clear error (use or add
INVALID_SOURCE_PROGRAM) if source_program.executable is false; implement this by
testing source_program.executable (or call the project helper like
require_executable if available) and returning the appropriate
ProgramError/early Err from the function before performing the CPI so an
arbitrary data account cannot be supplied as the source.


// deduce necessary accounts for CPI
let (accounts_meta, handler_accounts): (Vec<AccountMeta>, Vec<AccountInfo>) = other_accounts
.iter()
.chain([source_program, escrow_authority_account, escrow_account])
.filter(|account| account.key != validator.key)
.map(|account| {
(
// We enable only escrow to be a signer
AccountMeta {
pubkey: *account.key,
is_writable: account.is_writable,
is_signer: account.key == escrow_account.key,
},
account.clone(),
)
})
.collect();

let handler_instruction = Instruction {
program_id: *destination_program.key,
data: args.data,
accounts: accounts_meta,
};
let bump_slice = &[escrow_bump];
let escrow_signer_seeds = [escrow_seeds, &[bump_slice]].concat();

invoke_signed(
&handler_instruction,
&handler_accounts,
&[&escrow_signer_seeds],
)
}
2 changes: 2 additions & 0 deletions src/processor/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod call_handler;
mod call_handler_v2;
mod close_ephemeral_balance;
mod close_validator_fees_vault;
mod delegate_ephemeral_balance;
Expand All @@ -13,6 +14,7 @@ mod whitelist_validator_for_program;
pub mod fast;

pub use call_handler::*;
pub use call_handler_v2::*;
pub use close_ephemeral_balance::*;
pub use close_validator_fees_vault::*;
pub use delegate_ephemeral_balance::*;
Expand Down
Binary file modified tests/buffers/test_delegation.so
Binary file not shown.
70 changes: 70 additions & 0 deletions tests/integration/programs/test-delegation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,48 @@ pub mod test_delegation {

Ok(())
}

/// Delegation program call handler v2 (commit variant)
#[instruction(discriminator = [1, 0, 3, 0])]
pub fn commit_base_action_handler_v2(
ctx: Context<CommitBaseActionHandlerV2>,
amount: u64,
) -> Result<()> {
msg!("commit_base_action_handler_v2!");
let transfer_ctx = CpiContext::new(
ctx.accounts.system_program.to_account_info(),
Transfer {
from: ctx.accounts.escrow_account.to_account_info(),
to: ctx.accounts.destination_account.to_account_info(),
},
);

transfer(transfer_ctx, amount)
}

/// Delegation program call handler v2 (undelegate variant)
#[instruction(discriminator = [1, 0, 4, 0])]
pub fn undelegate_base_action_handler_v2(
ctx: Context<UndelegateBaseActionHandlerV2>,
amount: u64,
) -> Result<()> {
msg!("undelegate_base_action_handler_v2");
let transfer_ctx = CpiContext::new(
ctx.accounts.system_program.to_account_info(),
Transfer {
from: ctx.accounts.escrow_account.to_account_info(),
to: ctx.accounts.destination_account.to_account_info(),
},
);
transfer(transfer_ctx, amount)?;

let counter_data = &mut ctx.accounts.counter.try_borrow_mut_data()?;
let mut counter = Counter::try_from_slice(&counter_data)?;
counter.count += 1;
counter_data.copy_from_slice(&counter.try_to_vec()?);

Ok(())
}
}

pub fn transfer_from_undelegated(
Expand Down Expand Up @@ -210,6 +252,34 @@ pub struct UndelegateBaseActionHandler<'info> {
pub escrow_account: Signer<'info>,
}

#[derive(Accounts)]
pub struct CommitBaseActionHandlerV2<'info> {
/// CHECK: The destination account to transfer lamports to
#[account(mut)]
pub destination_account: AccountInfo<'info>,
pub system_program: Program<'info, System>,
/// CHECK: The source program passed through by call_handler_v2
pub source_program: UncheckedAccount<'info>,
/// CHECK: The authority that owns the escrow account
pub escrow_authority: UncheckedAccount<'info>,
pub escrow_account: Signer<'info>,
}

#[derive(Accounts)]
pub struct UndelegateBaseActionHandlerV2<'info> {
/// CHECK: The destination account to transfer lamports to
#[account(mut)]
pub destination_account: AccountInfo<'info>,
/// CHECK: fails in finalize stage due to ownership by dlp
pub counter: UncheckedAccount<'info>,
pub system_program: Program<'info, System>,
/// CHECK: The source program passed through by call_handler_v2
pub source_program: UncheckedAccount<'info>,
/// CHECK: The authority that owns the escrow account
pub escrow_authority: UncheckedAccount<'info>,
pub escrow_account: Signer<'info>,
}

#[account]
pub struct Counter {
pub count: u64,
Expand Down
Loading