Skip to content

feat: Improvements from audit feedback #72

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 2, 2025
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
7 changes: 0 additions & 7 deletions src/consts.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use solana_program::pubkey;
use solana_program::pubkey::Pubkey;

/// The delegation session fees (extracted in percentage from the delegation PDAs rent on closure).
Expand All @@ -12,9 +11,3 @@ pub const EXTERNAL_UNDELEGATE_DISCRIMINATOR: [u8; 8] = [196, 28, 41, 206, 48, 37

/// The program ID of the delegation program.
pub const DELEGATION_PROGRAM_ID: Pubkey = crate::id();

/// The admin pubkey of the authority allowed to whitelist validators.
#[cfg(feature = "unit_test_config")]
pub const ADMIN_PUBKEY: Pubkey = pubkey!("tEsT3eV6RFCWs1BZ7AXTzasHqTtMnMLCB2tjQ42TDXD");
#[cfg(not(feature = "unit_test_config"))]
pub const ADMIN_PUBKEY: Pubkey = pubkey!("3FwNxjbCqdD7G6MkrAdwTd5Zf6R3tHoapam4Pv1X2KBB");
5 changes: 4 additions & 1 deletion src/instruction_builder/close_validator_fees_vault.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use solana_program::instruction::Instruction;
use solana_program::{instruction::AccountMeta, pubkey::Pubkey};
use solana_program::{bpf_loader_upgradeable, instruction::AccountMeta, pubkey::Pubkey};

use crate::discriminator::DlpDiscriminator;
use crate::pda::validator_fees_vault_pda_from_validator;
Expand All @@ -12,11 +12,14 @@ pub fn close_validator_fees_vault(
validator_identity: Pubkey,
) -> Instruction {
let validator_fees_vault_pda = validator_fees_vault_pda_from_validator(&validator_identity);
let delegation_program_data =
Pubkey::find_program_address(&[crate::ID.as_ref()], &bpf_loader_upgradeable::id()).0;
Instruction {
program_id: crate::id(),
accounts: vec![
AccountMeta::new(payer, true),
AccountMeta::new(admin, true),
AccountMeta::new_readonly(delegation_program_data, false),
AccountMeta::new(validator_identity, false),
AccountMeta::new(validator_fees_vault_pda, false),
],
Expand Down
5 changes: 4 additions & 1 deletion src/instruction_builder/init_validator_fees_vault.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use solana_program::instruction::Instruction;
use solana_program::system_program;
use solana_program::{bpf_loader_upgradeable, system_program};
use solana_program::{instruction::AccountMeta, pubkey::Pubkey};

use crate::discriminator::DlpDiscriminator;
Expand All @@ -13,11 +13,14 @@ pub fn init_validator_fees_vault(
validator_identity: Pubkey,
) -> Instruction {
let validator_fees_vault_pda = validator_fees_vault_pda_from_validator(&validator_identity);
let delegation_program_data =
Pubkey::find_program_address(&[crate::ID.as_ref()], &bpf_loader_upgradeable::id()).0;
Instruction {
program_id: crate::id(),
accounts: vec![
AccountMeta::new(payer, true),
AccountMeta::new(admin, true),
AccountMeta::new_readonly(delegation_program_data, false),
AccountMeta::new(validator_identity, false),
AccountMeta::new(validator_fees_vault_pda, false),
AccountMeta::new_readonly(system_program::id(), false),
Expand Down
5 changes: 4 additions & 1 deletion src/instruction_builder/protocol_claim_fees.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use solana_program::instruction::Instruction;
use solana_program::{instruction::AccountMeta, pubkey::Pubkey};
use solana_program::{bpf_loader_upgradeable, instruction::AccountMeta, pubkey::Pubkey};

use crate::discriminator::DlpDiscriminator;
use crate::pda::fees_vault_pda;
Expand All @@ -8,11 +8,14 @@ use crate::pda::fees_vault_pda;
/// See [crate::processor::process_protocol_claim_fees] for docs.
pub fn protocol_claim_fees(admin: Pubkey) -> Instruction {
let fees_vault_pda = fees_vault_pda();
let delegation_program_data =
Pubkey::find_program_address(&[crate::ID.as_ref()], &bpf_loader_upgradeable::id()).0;
Instruction {
program_id: crate::id(),
accounts: vec![
AccountMeta::new(admin, true),
AccountMeta::new(fees_vault_pda, false),
AccountMeta::new_readonly(delegation_program_data, false),
],
data: DlpDiscriminator::ProtocolClaimFees.to_vec(),
}
Expand Down
3 changes: 3 additions & 0 deletions src/instruction_builder/whitelist_validator_for_program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ pub fn whitelist_validator_for_program(
let args = WhitelistValidatorForProgramArgs { insert };
let program_data =
Pubkey::find_program_address(&[program.as_ref()], &bpf_loader_upgradeable::id()).0;
let delegation_program_data =
Pubkey::find_program_address(&[crate::ID.as_ref()], &bpf_loader_upgradeable::id()).0;
let program_config_pda = program_config_from_program_id(&program);
Instruction {
program_id: crate::id(),
Expand All @@ -28,6 +30,7 @@ pub fn whitelist_validator_for_program(
AccountMeta::new_readonly(validator_identity, false),
AccountMeta::new_readonly(program, false),
AccountMeta::new_readonly(program_data, false),
AccountMeta::new_readonly(delegation_program_data, false),
AccountMeta::new(program_config_pda, false),
AccountMeta::new_readonly(system_program::id(), false),
],
Expand Down
15 changes: 10 additions & 5 deletions src/processor/close_validator_fees_vault.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ use solana_program::msg;
use solana_program::program_error::ProgramError;
use solana_program::{account_info::AccountInfo, entrypoint::ProgramResult, pubkey::Pubkey};

use crate::consts::ADMIN_PUBKEY;
use crate::error::DlpError::Unauthorized;
use crate::processor::utils::loaders::{load_initialized_pda, load_signer};
use crate::processor::utils::loaders::{
load_initialized_pda, load_program_upgrade_authority, load_signer,
};
use crate::processor::utils::pda::close_pda;
use crate::validator_fees_vault_seeds_from_validator;

Expand All @@ -30,7 +31,9 @@ pub fn process_close_validator_fees_vault(
_data: &[u8],
) -> ProgramResult {
// Load Accounts
let [payer, admin, validator_identity, validator_fees_vault] = accounts else {
let [payer, admin, delegation_program_data, validator_identity, validator_fees_vault] =
accounts
else {
return Err(ProgramError::NotEnoughAccountKeys);
};

Expand All @@ -39,10 +42,12 @@ pub fn process_close_validator_fees_vault(
load_signer(admin, "admin")?;

// Check if the admin is the correct one
if !admin.key.eq(&ADMIN_PUBKEY) {
let admin_pubkey =
load_program_upgrade_authority(&crate::ID, delegation_program_data)?.ok_or(Unauthorized)?;
if !admin.key.eq(&admin_pubkey) {
msg!(
"Expected admin pubkey: {} but got {}",
ADMIN_PUBKEY,
admin_pubkey,
admin.key
);
return Err(Unauthorized.into());
Expand Down
10 changes: 1 addition & 9 deletions src/processor/finalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::processor::utils::loaders::{
load_initialized_validator_fees_vault, load_owned_pda, load_program, load_signer,
};
use crate::processor::utils::pda::close_pda;
use crate::processor::utils::verify::verify_state;
use crate::state::{CommitRecord, DelegationMetadata, DelegationRecord};
use solana_program::program_error::ProgramError;
use solana_program::{
Expand Down Expand Up @@ -42,7 +41,7 @@ use solana_program::{
///
/// Steps:
///
/// 1. Validate the new state
/// 1. Validate the new state (currently state is valid if committed from a whitelisted validator)
/// 2. If the state is valid, copy the committed state to the delegated account
/// 3. Close the state diff account
/// 4. Close the commit state record
Expand Down Expand Up @@ -94,13 +93,6 @@ pub fn process_finalize(
let commit_record_data = commit_record_account.try_borrow_data()?;
let commit_record = CommitRecord::try_from_bytes_with_discriminator(&commit_record_data)?;

verify_state(
validator,
delegation_record,
commit_record,
commit_state_account,
)?;

// Check that the commit record is the right one
if !commit_record.account.eq(delegated_account.key) {
return Err(DlpError::InvalidDelegatedAccount.into());
Expand Down
2 changes: 2 additions & 0 deletions src/processor/init_protocol_fees_vault.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ use crate::processor::utils::pda::create_pda;
///
/// - fees vault is uninitialized
///
/// NOTE: this operation is permisionless and can be done by anyone
///
/// Steps:
///
/// 1. Create the protocol fees vault PDA
Expand Down
15 changes: 10 additions & 5 deletions src/processor/init_validator_fees_vault.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ use solana_program::{
account_info::AccountInfo, entrypoint::ProgramResult, pubkey::Pubkey, system_program,
};

use crate::consts::ADMIN_PUBKEY;
use crate::error::DlpError::Unauthorized;
use crate::processor::utils::loaders::{load_program, load_signer, load_uninitialized_pda};
use crate::processor::utils::loaders::{
load_program, load_program_upgrade_authority, load_signer, load_uninitialized_pda,
};
use crate::processor::utils::pda::create_pda;
use crate::validator_fees_vault_seeds_from_validator;

Expand Down Expand Up @@ -35,7 +36,9 @@ pub fn process_init_validator_fees_vault(
_data: &[u8],
) -> ProgramResult {
// Load Accounts
let [payer, admin, validator_identity, validator_fees_vault, system_program] = accounts else {
let [payer, admin, delegation_program_data, validator_identity, validator_fees_vault, system_program] =
accounts
else {
return Err(ProgramError::NotEnoughAccountKeys);
};

Expand All @@ -45,10 +48,12 @@ pub fn process_init_validator_fees_vault(
load_program(system_program, system_program::id(), "system program")?;

// Check if the admin is the correct one
if !admin.key.eq(&ADMIN_PUBKEY) {
let admin_pubkey =
load_program_upgrade_authority(&crate::ID, delegation_program_data)?.ok_or(Unauthorized)?;
if !admin.key.eq(&admin_pubkey) {
msg!(
"Expected admin pubkey: {} but got {}",
ADMIN_PUBKEY,
admin_pubkey,
admin.key
);
return Err(Unauthorized.into());
Expand Down
13 changes: 8 additions & 5 deletions src/processor/protocol_claim_fees.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::consts::ADMIN_PUBKEY;
use crate::error::DlpError::Unauthorized;
use crate::processor::utils::loaders::{load_initialized_protocol_fees_vault, load_signer};
use crate::processor::utils::loaders::{
load_initialized_protocol_fees_vault, load_program_upgrade_authority, load_signer,
};
use solana_program::msg;
use solana_program::program_error::ProgramError;
use solana_program::rent::Rent;
Expand All @@ -27,7 +28,7 @@ pub fn process_protocol_claim_fees(
_data: &[u8],
) -> ProgramResult {
// Load Accounts
let [admin, fees_vault] = accounts else {
let [admin, fees_vault, delegation_program_data] = accounts else {
return Err(ProgramError::NotEnoughAccountKeys);
};

Expand All @@ -36,10 +37,12 @@ pub fn process_protocol_claim_fees(
load_initialized_protocol_fees_vault(fees_vault, true)?;

// Check if the admin is the correct one
if !admin.key.eq(&ADMIN_PUBKEY) {
let admin_pubkey =
load_program_upgrade_authority(&crate::ID, delegation_program_data)?.ok_or(Unauthorized)?;
if !admin.key.eq(&admin_pubkey) {
msg!(
"Expected admin pubkey: {} but got {}",
ADMIN_PUBKEY,
admin_pubkey,
admin.key
);
return Err(Unauthorized.into());
Expand Down
45 changes: 43 additions & 2 deletions src/processor/utils/loaders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ use crate::{
delegation_record_seeds_from_delegated_account, fees_vault_seeds,
program_config_seeds_from_program_id, validator_fees_vault_seeds_from_validator,
};
use solana_program::bpf_loader_upgradeable::UpgradeableLoaderState;
use solana_program::{
account_info::AccountInfo, msg, program_error::ProgramError, pubkey::Pubkey, system_program,
sysvar,
account_info::AccountInfo, bpf_loader_upgradeable, msg, program_error::ProgramError,
pubkey::Pubkey, system_program, sysvar,
};

/// Errors if:
Expand Down Expand Up @@ -193,6 +194,46 @@ pub fn load_program(info: &AccountInfo, key: Pubkey, label: &str) -> Result<(),
Ok(())
}

/// Get the program upgrade authority for a given program
pub fn load_program_upgrade_authority(
program: &Pubkey,
program_data: &AccountInfo,
) -> Result<Option<Pubkey>, ProgramError> {
let program_data_address =
Pubkey::find_program_address(&[program.as_ref()], &bpf_loader_upgradeable::id()).0;

// During tests, the upgrade authority is a test pubkey
#[cfg(feature = "unit_test_config")]
if program.eq(&crate::ID) {
return Ok(Some(solana_program::pubkey!(
"tEsT3eV6RFCWs1BZ7AXTzasHqTtMnMLCB2tjQ42TDXD"
)));
}

if !program_data_address.eq(program_data.key) {
msg!(
"Expected program data address to be {}, but got {}",
program_data_address,
program_data.key
);
return Err(ProgramError::InvalidAccountData);
}

let program_account_data = program_data.try_borrow_data()?;
if let UpgradeableLoaderState::ProgramData {
upgrade_authority_address,
..
} = bincode::deserialize(&program_account_data).map_err(|_| {
msg!("Unable to deserialize ProgramData {}", program);
ProgramError::InvalidAccountData
})? {
Ok(upgrade_authority_address)
} else {
msg!("Expected program account {} to hold ProgramData", program);
Err(ProgramError::InvalidAccountData)
}
}

/// Load fee vault PDA
/// - Protocol fees vault PDA
pub fn load_initialized_protocol_fees_vault(
Expand Down
1 change: 0 additions & 1 deletion src/processor/utils/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
pub(crate) mod curve;
pub(crate) mod loaders;
pub(crate) mod pda;
pub(crate) mod verify;
16 changes: 0 additions & 16 deletions src/processor/utils/verify.rs

This file was deleted.

Loading