Allow closing upgradeable program accounts (#19319)

This commit is contained in:
Jack May
2021-08-24 10:05:54 -07:00
committed by GitHub
parent 27a41c5954
commit a89f180145
9 changed files with 481 additions and 138 deletions

View File

@ -30,13 +30,16 @@ use solana_sdk::{
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
clock::Clock,
entrypoint::{HEAP_LENGTH, SUCCESS},
feature_set::{add_missing_program_error_mappings, stop_verify_mul64_imm_nonzero},
feature_set::{
add_missing_program_error_mappings, close_upgradeable_program_accounts,
stop_verify_mul64_imm_nonzero,
},
ic_logger_msg, ic_msg,
instruction::InstructionError,
keyed_account::{from_keyed_account, keyed_account_at_index},
keyed_account::{from_keyed_account, keyed_account_at_index, KeyedAccount},
loader_instruction::LoaderInstruction,
loader_upgradeable_instruction::UpgradeableLoaderInstruction,
process_instruction::{stable_log, ComputeMeter, Executor, InvokeContext},
process_instruction::{stable_log, ComputeMeter, Executor, InvokeContext, Logger},
program_error::{ACCOUNT_NOT_RENT_EXEMPT, BORSH_IO_ERROR},
program_utils::limited_deserialize,
pubkey::Pubkey,
@ -198,6 +201,16 @@ fn process_instruction_common(
ic_logger_msg!(logger, "Wrong ProgramData account for this Program account");
return Err(InstructionError::InvalidArgument);
}
if !matches!(
programdata.state()?,
UpgradeableLoaderState::ProgramData {
slot: _,
upgrade_authority_address: _,
}
) {
ic_logger_msg!(logger, "Program has been closed");
return Err(InstructionError::InvalidAccountData);
}
invoke_context.remove_first_keyed_account()?;
UpgradeableLoaderState::programdata_data_offset()?
} else {
@ -620,46 +633,143 @@ fn process_loader_upgradeable_instruction(
UpgradeableLoaderInstruction::Close => {
let close_account = keyed_account_at_index(keyed_accounts, 0)?;
let recipient_account = keyed_account_at_index(keyed_accounts, 1)?;
let authority = keyed_account_at_index(keyed_accounts, 2)?;
if !invoke_context.is_feature_active(&close_upgradeable_program_accounts::id()) {
let _ = keyed_account_at_index(keyed_accounts, 2)?;
}
if close_account.unsigned_key() == recipient_account.unsigned_key() {
ic_logger_msg!(logger, "Recipient is the same as the account being closed");
return Err(InstructionError::InvalidArgument);
}
if let UpgradeableLoaderState::Buffer { authority_address } = close_account.state()? {
if authority_address.is_none() {
ic_logger_msg!(logger, "Buffer is immutable");
return Err(InstructionError::Immutable);
}
if authority_address != Some(*authority.unsigned_key()) {
ic_logger_msg!(logger, "Incorrect buffer authority provided");
return Err(InstructionError::IncorrectAuthority);
}
if authority.signer_key().is_none() {
ic_logger_msg!(logger, "Buffer authority did not sign");
return Err(InstructionError::MissingRequiredSignature);
}
recipient_account
.try_account_ref_mut()?
.checked_add_lamports(close_account.lamports()?)?;
close_account.try_account_ref_mut()?.set_lamports(0);
for elt in close_account.try_account_ref_mut()?.data_as_mut_slice() {
*elt = 0;
}
} else {
if !invoke_context.is_feature_active(&close_upgradeable_program_accounts::id())
&& !matches!(
close_account.state()?,
UpgradeableLoaderState::Buffer {
authority_address: _,
}
)
{
ic_logger_msg!(logger, "Account does not support closing");
return Err(InstructionError::InvalidArgument);
}
ic_logger_msg!(logger, "Closed {}", close_account.unsigned_key());
match close_account.state()? {
UpgradeableLoaderState::Uninitialized => {
recipient_account
.try_account_ref_mut()?
.checked_add_lamports(close_account.lamports()?)?;
close_account.try_account_ref_mut()?.set_lamports(0);
ic_logger_msg!(
logger,
"Closed Uninitialized {}",
close_account.unsigned_key()
);
}
UpgradeableLoaderState::Buffer { authority_address } => {
let authority = keyed_account_at_index(keyed_accounts, 2)?;
common_close_account(
&authority_address,
authority,
close_account,
recipient_account,
logger.clone(),
!invoke_context
.is_feature_active(&close_upgradeable_program_accounts::id()),
)?;
ic_logger_msg!(logger, "Closed Buffer {}", close_account.unsigned_key());
}
UpgradeableLoaderState::ProgramData {
slot: _,
upgrade_authority_address: authority_address,
} => {
let program_account = keyed_account_at_index(keyed_accounts, 3)?;
if !program_account.is_writable() {
ic_logger_msg!(logger, "Program account is not writable");
return Err(InstructionError::InvalidArgument);
}
match program_account.state()? {
UpgradeableLoaderState::Program {
programdata_address,
} => {
if programdata_address != *close_account.unsigned_key() {
ic_logger_msg!(
logger,
"ProgramData account does not match ProgramData account"
);
return Err(InstructionError::InvalidArgument);
}
let authority = keyed_account_at_index(keyed_accounts, 2)?;
common_close_account(
&authority_address,
authority,
close_account,
recipient_account,
logger.clone(),
!invoke_context
.is_feature_active(&close_upgradeable_program_accounts::id()),
)?;
}
_ => {
ic_logger_msg!(logger, "Invalid Program account");
return Err(InstructionError::InvalidArgument);
}
}
ic_logger_msg!(logger, "Closed Program {}", program_account.unsigned_key());
}
_ => {
ic_logger_msg!(logger, "Account does not support closing");
return Err(InstructionError::InvalidArgument);
}
}
}
}
Ok(())
}
fn common_close_account(
authority_address: &Option<Pubkey>,
authority_account: &KeyedAccount,
close_account: &KeyedAccount,
recipient_account: &KeyedAccount,
logger: Rc<RefCell<dyn Logger>>,
do_clear_data: bool,
) -> Result<(), InstructionError> {
if authority_address.is_none() {
ic_logger_msg!(logger, "Account is immutable");
return Err(InstructionError::Immutable);
}
if *authority_address != Some(*authority_account.unsigned_key()) {
ic_logger_msg!(logger, "Incorrect authority provided");
return Err(InstructionError::IncorrectAuthority);
}
if authority_account.signer_key().is_none() {
ic_logger_msg!(logger, "Authority did not sign");
return Err(InstructionError::MissingRequiredSignature);
}
recipient_account
.try_account_ref_mut()?
.checked_add_lamports(close_account.lamports()?)?;
close_account.try_account_ref_mut()?.set_lamports(0);
if do_clear_data {
for elt in close_account.try_account_ref_mut()?.data_as_mut_slice() {
*elt = 0;
}
} else {
close_account.set_state(&UpgradeableLoaderState::Uninitialized)?;
}
Ok(())
}
fn process_loader_instruction(
program_id: &Pubkey,
instruction_data: &[u8],
@ -3234,11 +3344,8 @@ mod tests {
);
assert_eq!(0, buffer_account.borrow().lamports());
assert_eq!(2, recipient_account.borrow().lamports());
assert!(buffer_account
.borrow()
.data()
.iter()
.all(|&value| value == 0));
let state: UpgradeableLoaderState = buffer_account.borrow().state().unwrap();
assert_eq!(state, UpgradeableLoaderState::Uninitialized);
// Case: close with wrong authority
buffer_account
@ -3262,26 +3369,95 @@ mod tests {
)
);
// Case: close but not a buffer account
buffer_account
// Case: close an uninitialized account
let uninitialized_address = Pubkey::new_unique();
let uninitialized_account = AccountSharedData::new_ref(
1,
UpgradeableLoaderState::programdata_len(0).unwrap(),
&bpf_loader_upgradeable::id(),
);
uninitialized_account
.borrow_mut()
.set_state(&UpgradeableLoaderState::Program {
programdata_address: Pubkey::new_unique(),
})
.set_state(&UpgradeableLoaderState::Uninitialized)
.unwrap();
let recipient_account = AccountSharedData::new_ref(1, 0, &Pubkey::new_unique());
let keyed_accounts = vec![
KeyedAccount::new(&buffer_address, false, &buffer_account),
KeyedAccount::new(&uninitialized_address, false, &uninitialized_account),
KeyedAccount::new(&recipient_address, false, &recipient_account),
KeyedAccount::new_readonly(&incorrect_authority_address, true, &authority_account),
];
assert_eq!(
Err(InstructionError::InvalidArgument),
Ok(()),
process_instruction(
&bpf_loader_upgradeable::id(),
&instruction,
&mut MockInvokeContext::new(keyed_accounts),
)
);
assert_eq!(0, uninitialized_account.borrow().lamports());
assert_eq!(2, recipient_account.borrow().lamports());
let state: UpgradeableLoaderState = uninitialized_account.borrow().state().unwrap();
assert_eq!(state, UpgradeableLoaderState::Uninitialized);
// Case: close a program account
let programdata_address = Pubkey::new_unique();
let programdata_account = AccountSharedData::new_ref(
1,
UpgradeableLoaderState::programdata_len(0).unwrap(),
&bpf_loader_upgradeable::id(),
);
programdata_account
.borrow_mut()
.set_state(&UpgradeableLoaderState::ProgramData {
slot: 0,
upgrade_authority_address: Some(authority_address),
})
.unwrap();
let program_address = Pubkey::new_unique();
let program_account = AccountSharedData::new_ref(
1,
UpgradeableLoaderState::program_len().unwrap(),
&bpf_loader_upgradeable::id(),
);
program_account.borrow_mut().set_executable(true);
program_account
.borrow_mut()
.set_state(&UpgradeableLoaderState::Program {
programdata_address,
})
.unwrap();
let recipient_account = AccountSharedData::new_ref(1, 0, &Pubkey::new_unique());
let keyed_accounts = vec![
KeyedAccount::new(&programdata_address, false, &programdata_account),
KeyedAccount::new(&recipient_address, false, &recipient_account),
KeyedAccount::new_readonly(&authority_address, true, &authority_account),
KeyedAccount::new(&program_address, false, &program_account),
];
assert_eq!(
Ok(()),
process_instruction(
&bpf_loader_upgradeable::id(),
&instruction,
&mut MockInvokeContext::new(keyed_accounts),
)
);
assert_eq!(0, programdata_account.borrow().lamports());
assert_eq!(2, recipient_account.borrow().lamports());
let state: UpgradeableLoaderState = programdata_account.borrow().state().unwrap();
assert_eq!(state, UpgradeableLoaderState::Uninitialized);
// Try to invoke closed account
let keyed_accounts = vec![
KeyedAccount::new(&program_address, false, &program_account),
KeyedAccount::new(&programdata_address, false, &programdata_account),
];
assert_eq!(
Err(InstructionError::InvalidAccountData),
process_instruction(
&program_address,
&instruction,
&mut MockInvokeContext::new(keyed_accounts),
)
);
}
/// fuzzing utility function

View File

@ -21,9 +21,9 @@ use solana_sdk::{
entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS},
epoch_schedule::EpochSchedule,
feature_set::{
blake3_syscall_enabled, disable_fees_sysvar, enforce_aligned_host_addrs,
libsecp256k1_0_5_upgrade_enabled, mem_overlap_fix, memory_ops_syscalls,
secp256k1_recover_syscall_enabled,
blake3_syscall_enabled, close_upgradeable_program_accounts, disable_fees_sysvar,
enforce_aligned_host_addrs, libsecp256k1_0_5_upgrade_enabled, mem_overlap_fix,
memory_ops_syscalls, secp256k1_recover_syscall_enabled,
},
hash::{Hasher, HASH_BYTES},
ic_msg,
@ -2236,13 +2236,16 @@ fn check_account_infos(
fn check_authorized_program(
program_id: &Pubkey,
instruction_data: &[u8],
close_upgradeable_program_accounts: bool,
) -> Result<(), EbpfError<BpfError>> {
if native_loader::check_id(program_id)
|| bpf_loader::check_id(program_id)
|| bpf_loader_deprecated::check_id(program_id)
|| (bpf_loader_upgradeable::check_id(program_id)
&& !(bpf_loader_upgradeable::is_upgrade_instruction(instruction_data)
|| bpf_loader_upgradeable::is_set_authority_instruction(instruction_data)))
|| bpf_loader_upgradeable::is_set_authority_instruction(instruction_data)
|| (close_upgradeable_program_accounts
&& bpf_loader_upgradeable::is_close_instruction(instruction_data))))
{
return Err(SyscallError::ProgramNotSupported(*program_id).into());
}
@ -2350,7 +2353,11 @@ fn call<'a>(
}
})
.collect::<Vec<bool>>();
check_authorized_program(&callee_program_id, &instruction.data)?;
check_authorized_program(
&callee_program_id,
&instruction.data,
invoke_context.is_feature_active(&close_upgradeable_program_accounts::id()),
)?;
let (accounts, account_refs) = syscall.translate_accounts(
&message.account_keys,
callee_program_id_index,