Skip to content

fix: fixed undelegation of ephemeral balance #76

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
9 changes: 8 additions & 1 deletion src/discriminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ pub enum DlpDiscriminator {
CommitStateFromBuffer = 13,
/// See [crate::processor::process_close_validator_fees_vault] for docs.
CloseValidatorFeesVault = 14,
/// See [crate::processor::process_undelegate_ephemeral_balance] for docs.
UndelegateEphemeralBalance = 15,
/// See [crate::processor::process_close_ephemeral_balance_v1] for docs.
CloseEphemeralBalanceV1 = 16
}

impl DlpDiscriminator {
Expand All @@ -45,7 +49,8 @@ impl DlpDiscriminator {
impl TryFrom<[u8; 8]> for DlpDiscriminator {
type Error = ProgramError;
fn try_from(bytes: [u8; 8]) -> Result<Self, Self::Error> {
match bytes[0] {
let discriminator = u64::from_le_bytes(bytes);

Choose a reason for hiding this comment

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

Isn't this more CU-intense without benefits (until the program reaches 255 instructions)?

match discriminator {
0x0 => Ok(DlpDiscriminator::Delegate),
0x1 => Ok(DlpDiscriminator::CommitState),
0x2 => Ok(DlpDiscriminator::Finalize),
Expand All @@ -60,6 +65,8 @@ impl TryFrom<[u8; 8]> for DlpDiscriminator {
0xc => Ok(DlpDiscriminator::ProtocolClaimFees),
0xd => Ok(DlpDiscriminator::CommitStateFromBuffer),
0xe => Ok(DlpDiscriminator::CloseValidatorFeesVault),
0xf => Ok(DlpDiscriminator::UndelegateEphemeralBalance),
0x10 => Ok(DlpDiscriminator::CloseEphemeralBalanceV1),
_ => Err(ProgramError::InvalidInstructionData),
}
}
Expand Down
8 changes: 5 additions & 3 deletions src/instruction_builder/close_ephemeral_balance.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
use solana_program::instruction::Instruction;
use solana_program::{instruction::AccountMeta, pubkey::Pubkey};
use solana_program::{instruction::AccountMeta, pubkey::Pubkey, system_program};

use crate::discriminator::DlpDiscriminator;
use crate::pda::ephemeral_balance_pda_from_payer;

/// Creates instruction to close an ephemeral balance account
/// See [crate::processor::process_close_ephemeral_balance] for docs.
/// See [crate::processor::process_close_ephemeral_balance_v1] for docs.
/// [crate::processor::process_close_ephemeral_balance] now deprecated

Choose a reason for hiding this comment

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

Should probably annotate process_close_ephemeral_balance with #[deprecated(since="...", note="...")]

pub fn close_ephemeral_balance(payer: Pubkey, index: u8) -> Instruction {
let ephemeral_balance_pda = ephemeral_balance_pda_from_payer(&payer, index);
Instruction {
program_id: crate::id(),
accounts: vec![
AccountMeta::new(payer, true),
AccountMeta::new(ephemeral_balance_pda, false),
AccountMeta::new_readonly(system_program::id(), false),
],
data: [
DlpDiscriminator::CloseEphemeralBalance.to_vec(),
DlpDiscriminator::CloseEphemeralBalanceV1.to_vec(),
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not currently used, so we can avoid the "V1" as it's not gonna be a breaking change

vec![index],
]
.concat(),
Expand Down
2 changes: 2 additions & 0 deletions src/instruction_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ mod init_validator_fees_vault;
mod protocol_claim_fees;
mod top_up_ephemeral_balance;
mod undelegate;
mod undelegate_ephemeral_balance;
mod validator_claim_fees;
mod whitelist_validator_for_program;

Expand All @@ -26,5 +27,6 @@ pub use init_validator_fees_vault::*;
pub use protocol_claim_fees::*;
pub use top_up_ephemeral_balance::*;
pub use undelegate::*;
pub use undelegate_ephemeral_balance::*;
pub use validator_claim_fees::*;
pub use whitelist_validator_for_program::*;
17 changes: 17 additions & 0 deletions src/instruction_builder/undelegate_ephemeral_balance.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
use crate::discriminator::DlpDiscriminator;
use crate::instruction_builder::undelegate;
use solana_program::instruction::Instruction;
use solana_program::pubkey::Pubkey;

/// Builds an undelegate instruction for ephemeral balance.
/// See [crate::processor::process_undelegate_ephemeral_balance] for docs.
pub fn undelegate_ephemeral_balance(
validator: Pubkey,
delegated_account: Pubkey,
rent_reimbursement: Pubkey,
) -> Instruction {
let mut ix = undelegate(validator, delegated_account, crate::ID, rent_reimbursement);
ix.data = DlpDiscriminator::UndelegateEphemeralBalance.to_vec();

ix
}
6 changes: 6 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ pub fn process_instruction(
discriminator::DlpDiscriminator::CloseValidatorFeesVault => {
processor::process_close_validator_fees_vault(program_id, accounts, data)?
}
discriminator::DlpDiscriminator::UndelegateEphemeralBalance => {
processor::process_undelegate_ephemeral_balance(program_id, accounts, data)?
}
discriminator::DlpDiscriminator::CloseEphemeralBalanceV1 => {
processor::process_close_ephemeral_balance_v1(program_id, accounts, data)?
}
}
Ok(())
}
76 changes: 76 additions & 0 deletions src/processor/close_ephemeral_balance_v1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
use crate::ephemeral_balance_seeds_from_payer;
use crate::processor::utils::loaders::{load_pda, load_signer};
use solana_program::msg;
use solana_program::program::invoke_signed;
use solana_program::program_error::ProgramError;
use solana_program::system_instruction::transfer;
use solana_program::{
account_info::AccountInfo, entrypoint::ProgramResult, pubkey::Pubkey, system_program,
};

/// Process the closing of an ephemeral balance account
///
/// Accounts:
///
/// 0: `[signer]` payer to pay for the transaction and receive the refund
/// 1: `[writable]` ephemeral balance account we are closing
/// 2: `[]` the system program
///
/// Requirements:
///
/// - ephemeral balance account is initialized
///
/// Steps:
///
/// 1. Closes the ephemeral balance account and refunds the payer with the
/// escrowed lamports
pub fn process_close_ephemeral_balance_v1(
_program_id: &Pubkey,
accounts: &[AccountInfo],
data: &[u8],
) -> ProgramResult {
let index = *data.first().ok_or(ProgramError::InvalidInstructionData)?;

// Load Accounts
let [payer, ephemeral_balance_account, system_program] = accounts else {
return Err(ProgramError::NotEnoughAccountKeys);
};

load_signer(payer, "payer")?;

let ephemeral_balance_seeds: &[&[u8]] = ephemeral_balance_seeds_from_payer!(payer.key, index);
let ephemeral_balance_bump = load_pda(
ephemeral_balance_account,
ephemeral_balance_seeds,
&crate::id(),
true,
"ephemeral balance",
)?;
if ephemeral_balance_account.owner != &system_program::id() {
msg!(
"ephemeral balance expected to be owned by system program: {}",
system_program::id()

Choose a reason for hiding this comment

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

I think it would be more interesting to log ephemeral_balance_account.owner rather than the well-known system program. It might be even better to give a static message to avoid the CU cost of formatting strings

);
return Err(ProgramError::InvalidAccountOwner);
}

let amount = ephemeral_balance_account.lamports();
if amount == 0 {
return Ok(());
}

let ephemeral_balance_bump_slice: &[u8] = &[ephemeral_balance_bump];

Choose a reason for hiding this comment

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

Shouldn't it use close_pda at some point like the old process_close_ephemeral_balance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I understand this practically closes account, since it doesn't have lamports and data

let ephemeral_balance_signer_seeds =
[ephemeral_balance_seeds, &[ephemeral_balance_bump_slice]].concat();
invoke_signed(
&transfer(ephemeral_balance_account.key, payer.key, amount),
&[
ephemeral_balance_account.clone(),
payer.clone(),
system_program.clone(),
],
&[&ephemeral_balance_signer_seeds],
)?;

Ok(())
}
4 changes: 4 additions & 0 deletions src/processor/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod close_ephemeral_balance;
mod close_ephemeral_balance_v1;
mod close_validator_fees_vault;
mod commit_state;
mod commit_state_from_buffer;
Expand All @@ -10,11 +11,13 @@ mod init_validator_fees_vault;
mod protocol_claim_fees;
mod top_up_ephemeral_balance;
mod undelegate;
mod undelegate_ephemeral_balance;
mod utils;
mod validator_claim_fees;
mod whitelist_validator_for_program;

pub use close_ephemeral_balance::*;
pub use close_ephemeral_balance_v1::*;
pub use close_validator_fees_vault::*;
pub use commit_state::*;
pub use commit_state_from_buffer::*;
Expand All @@ -26,5 +29,6 @@ pub use init_validator_fees_vault::*;
pub use protocol_claim_fees::*;
pub use top_up_ephemeral_balance::*;
pub use undelegate::*;
pub use undelegate_ephemeral_balance::*;
pub use validator_claim_fees::*;
pub use whitelist_validator_for_program::*;
2 changes: 1 addition & 1 deletion src/processor/top_up_ephemeral_balance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub fn process_top_up_ephemeral_balance(
create_pda(
ephemeral_balance_account,
&system_program::id(),
8,
0,

Choose a reason for hiding this comment

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

I don't understand what causes this change in this PR (and the subsequent changes in tests), I'm guessing it's the core of the fix for something introduced in another PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's 2 points to this

  1. It fixes undelegation. Otherwise we try to call non-existing callback
  2. I reckoned discriminator isn't needed. It is not set anyway. the account just stores lamports, the same as on curve addresses don't have any discriminator. The only thing that @GabrielePicco mentioned is that it might complicate filtering, but the address of this pda shall suffice for filtering

Copy link
Contributor

Choose a reason for hiding this comment

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

It was there mainly for consistency (discriminator should be set on creation) to allow queries such as getAllEscrowAccounts that are not otherwise possible.

Copy link
Contributor Author

@taco-paco taco-paco May 7, 2025

Choose a reason for hiding this comment

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

Ah I see now what you meant

ephemeral_balance_seeds_from_payer!(pubkey.key, args.index),
bump_ephemeral_balance,
system_program,
Expand Down
74 changes: 74 additions & 0 deletions src/processor/undelegate_ephemeral_balance.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
use crate::instruction_builder::undelegate;
use solana_program::msg;
use solana_program::program::invoke;
use solana_program::program_error::ProgramError;
use solana_program::{
account_info::AccountInfo, entrypoint::ProgramResult, pubkey::Pubkey, system_program,
};

/// Undelegate ephemeral balance
///
/// Accounts:
///
/// 0: `[signer]` the validator account
/// 1: `[writable]` the delegated account
/// 2: `[]` the owner program of the delegated account
/// 3: `[writable]` the undelegate buffer PDA we use to store the data temporarily
/// 4: `[]` the commit state PDA
/// 5: `[]` the commit record PDA
/// 6: `[writable]` the delegation record PDA
/// 7: `[writable]` the delegation metadata PDA
/// 8: `[]` the rent reimbursement account
/// 9: `[writable]` the protocol fees vault account
/// 10: `[writable]` the validator fees vault account
/// 11: `[]` the system program
///
/// Requirements:
///
/// - delegated account is owned by delegation program
/// - delegation record is initialized
/// - delegation metadata is initialized
/// - protocol fees vault is initialized
/// - validator fees vault is initialized
/// - commit state is uninitialized
/// - commit record is uninitialized
/// - delegated account is NOT undelegatable
/// - owner program account matches the owner in the delegation record
/// - rent reimbursement account matches the rent payer in the delegation metadata
///
/// Steps:
///
/// - Undelegate using CPI into [`crate::processor::undelegate`]
/// - Assigns ownership back to system program
pub fn process_undelegate_ephemeral_balance(
_program_id: &Pubkey,
accounts: &[AccountInfo],
_data: &[u8],
) -> ProgramResult {
let [validator, delegated_account, owner_program, _, _, _, _, _, rent_reimbursement, _, _, _] =
accounts
else {
return Err(ProgramError::NotEnoughAccountKeys);
};

if owner_program.key != &crate::ID {
msg!(
"Unexpected owner program. expected dlp, got: {}",
owner_program.key
);
return Err(ProgramError::IncorrectProgramId);
}

// Propagate to undelegate which also runs all necessary checks.
let undelegate_ix = undelegate(
*validator.key,
*delegated_account.key,
*owner_program.key,
*rent_reimbursement.key,
);
invoke(&undelegate_ix, accounts)?;

// Assign ownership back to system_program
delegated_account.assign(&system_program::ID);
Ok(())
}
Loading