-
Notifications
You must be signed in to change notification settings - Fork 4
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should probably annotate |
||
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(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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], | ||
taco-paco marked this conversation as resolved.
Show resolved
Hide resolved
|
||
] | ||
.concat(), | ||
|
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); | ||
taco-paco marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ix.data = DlpDiscriminator::UndelegateEphemeralBalance.to_vec(); | ||
|
||
ix | ||
} |
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() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be more interesting to log |
||
); | ||
return Err(ProgramError::InvalidAccountOwner); | ||
} | ||
|
||
let amount = ephemeral_balance_account.lamports(); | ||
if amount == 0 { | ||
return Ok(()); | ||
} | ||
|
||
let ephemeral_balance_bump_slice: &[u8] = &[ephemeral_balance_bump]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't it use There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,7 +56,7 @@ pub fn process_top_up_ephemeral_balance( | |
create_pda( | ||
ephemeral_balance_account, | ||
&system_program::id(), | ||
8, | ||
0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's 2 points to this
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
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(()) | ||
} |
There was a problem hiding this comment.
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)?