From a89f180145fdc746378cb7c0077bb3478ae972a1 Mon Sep 17 00:00:00 2001 From: Jack May Date: Tue, 24 Aug 2021 10:05:54 -0700 Subject: [PATCH] Allow closing upgradeable program accounts (#19319) --- cli-output/src/cli_output.rs | 23 ++ cli/src/program.rs | 192 +++++++------ cli/tests/program.rs | 84 +++++- programs/bpf_loader/src/lib.rs | 256 +++++++++++++++--- programs/bpf_loader/src/syscalls.rs | 17 +- sdk/cargo-build-bpf/src/main.rs | 2 + sdk/program/src/bpf_loader_upgradeable.rs | 30 +- .../src/loader_upgradeable_instruction.rs | 8 +- sdk/src/feature_set.rs | 7 +- 9 files changed, 481 insertions(+), 138 deletions(-) diff --git a/cli-output/src/cli_output.rs b/cli-output/src/cli_output.rs index 31d26c664..4937b6f6a 100644 --- a/cli-output/src/cli_output.rs +++ b/cli-output/src/cli_output.rs @@ -1954,6 +1954,29 @@ impl fmt::Display for CliUpgradeableProgram { } } +#[derive(Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct CliUpgradeableProgramClosed { + pub program_id: String, + pub lamports: u64, + #[serde(skip_serializing)] + pub use_lamports_unit: bool, +} +impl QuietDisplay for CliUpgradeableProgramClosed {} +impl VerboseDisplay for CliUpgradeableProgramClosed {} +impl fmt::Display for CliUpgradeableProgramClosed { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + writeln!(f)?; + writeln!( + f, + "Closed Program Id {}, {} reclaimed", + &self.program_id, + &build_balance_message(self.lamports, self.use_lamports_unit, true) + )?; + Ok(()) + } +} + #[derive(Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CliUpgradeableBuffer { diff --git a/cli/src/program.rs b/cli/src/program.rs index 3518ce48b..ad675868c 100644 --- a/cli/src/program.rs +++ b/cli/src/program.rs @@ -14,7 +14,7 @@ use solana_clap_utils::{self, input_parsers::*, input_validators::*, keypair::*} use solana_cli_output::{ display::new_spinner_progress_bar, CliProgram, CliProgramAccountType, CliProgramAuthority, CliProgramBuffer, CliProgramId, CliUpgradeableBuffer, CliUpgradeableBuffers, - CliUpgradeableProgram, + CliUpgradeableProgram, CliUpgradeableProgramClosed, }; use solana_client::{ client_error::ClientErrorKind, @@ -333,29 +333,31 @@ impl ProgramSubCommands for App<'_, '_> { ) .subcommand( SubCommand::with_name("close") - .about("Close an account and withdraw all lamports") + .about("Close a program or buffer account and withdraw all lamports") .arg( Arg::with_name("account") .index(1) - .value_name("BUFFER_ACCOUNT_ADDRESS") + .value_name("ACCOUNT_ADDRESS") .takes_value(true) - .help("Address of the buffer account to close"), + .help("Address of the program or buffer account to close"), ) .arg( Arg::with_name("buffers") .long("buffers") .conflicts_with("account") .required_unless("account") - .help("Close every buffer accounts that match the authority") + .help("Close all buffer accounts that match the authority") ) .arg( - Arg::with_name("buffer_authority") - .long("buffer-authority") + Arg::with_name("authority") + .long("authority") + .alias("buffer-authority") .value_name("AUTHORITY_SIGNER") .takes_value(true) .validator(is_valid_signer) - .help("Authority [default: the default configured keypair]") + .help("Upgrade or buffer authority [default: the default configured keypair]") ) + .arg( pubkey!(Arg::with_name("recipient_account") .long("recipient") @@ -626,7 +628,7 @@ pub fn parse_program_subcommand( }; let (authority_signer, authority_pubkey) = - signer_of(matches, "buffer_authority", wallet_manager)?; + signer_of(matches, "authority", wallet_manager)?; let signer_info = default_signer.generate_unique_signers( vec![ @@ -860,15 +862,17 @@ fn process_program_deploy( false } else { return Err(format!( - "{} is not an upgradeable loader ProgramData account", - programdata_address + "Program {} has been closed, use a new Program Id", + program_pubkey ) .into()); } } else { - return Err( - format!("ProgramData account {} does not exist", programdata_address).into(), - ); + return Err(format!( + "Program {} has been closed, use a new Program Id", + program_pubkey + ) + .into()); } } else { return Err(format!("{} is not an upgradeable program", program_pubkey).into()); @@ -1196,17 +1200,10 @@ fn process_show( - UpgradeableLoaderState::programdata_data_offset()?, })) } else { - Err(format!("Invalid associated ProgramData account {} found for the program {}", - programdata_address, account_pubkey) - .into(), - ) + Err(format!("Program {} has been closed", account_pubkey).into()) } } else { - Err(format!( - "Failed to find associated ProgramData account {} for the program {}", - programdata_address, account_pubkey - ) - .into()) + Err(format!("Program {} has been closed", account_pubkey).into()) } } else if let Ok(UpgradeableLoaderState::Buffer { authority_address }) = account.state() @@ -1298,18 +1295,10 @@ fn process_dump( f.write_all(program_data)?; Ok(format!("Wrote program to {}", output_location)) } else { - Err( - format!("Invalid associated ProgramData account {} found for the program {}", - programdata_address, account_pubkey) - .into(), - ) + Err(format!("Program {} has been closed", account_pubkey).into()) } } else { - Err(format!( - "Failed to find associated ProgramData account {} for the program {}", - programdata_address, account_pubkey - ) - .into()) + Err(format!("Program {} has been closed", account_pubkey).into()) } } else if let Ok(UpgradeableLoaderState::Buffer { .. }) = account.state() { let offset = UpgradeableLoaderState::buffer_data_offset().unwrap_or(0); @@ -1341,14 +1330,16 @@ fn close( account_pubkey: &Pubkey, recipient_pubkey: &Pubkey, authority_signer: &dyn Signer, + program_pubkey: Option<&Pubkey>, ) -> Result<(), Box> { let blockhash = rpc_client.get_latest_blockhash()?; let mut tx = Transaction::new_unsigned(Message::new( - &[bpf_loader_upgradeable::close( + &[bpf_loader_upgradeable::close_any( account_pubkey, recipient_pubkey, - &authority_signer.pubkey(), + Some(&authority_signer.pubkey()), + program_pubkey, )], Some(&config.signers[0].pubkey()), )); @@ -1393,64 +1384,92 @@ fn process_close( .get_account_with_commitment(&account_pubkey, config.commitment)? .value { - if let Ok(UpgradeableLoaderState::Buffer { authority_address }) = account.state() { - if authority_address != Some(authority_signer.pubkey()) { - return Err(format!( - "Buffer account authority {:?} does not match {:?}", - authority_address, - Some(authority_signer.pubkey()) - ) - .into()); - } else { - close( - rpc_client, - config, - &account_pubkey, - &recipient_pubkey, - authority_signer, - )?; + match account.state() { + Ok(UpgradeableLoaderState::Buffer { authority_address }) => { + if authority_address != Some(authority_signer.pubkey()) { + return Err(format!( + "Buffer account authority {:?} does not match {:?}", + authority_address, + Some(authority_signer.pubkey()) + ) + .into()); + } else { + close( + rpc_client, + config, + &account_pubkey, + &recipient_pubkey, + authority_signer, + None, + )?; - buffers.push(CliUpgradeableBuffer { - address: account_pubkey.to_string(), - authority: authority_address - .map(|pubkey| pubkey.to_string()) - .unwrap_or_else(|| "none".to_string()), - data_len: 0, - lamports: account.lamports, - use_lamports_unit, - }); + buffers.push(CliUpgradeableBuffer { + address: account_pubkey.to_string(), + authority: authority_address + .map(|pubkey| pubkey.to_string()) + .unwrap_or_else(|| "none".to_string()), + data_len: 0, + lamports: account.lamports, + use_lamports_unit, + }); + } + } + Ok(UpgradeableLoaderState::Program { + programdata_address: programdata_pubkey, + }) => { + if let Some(account) = rpc_client + .get_account_with_commitment(&programdata_pubkey, config.commitment)? + .value + { + if let Ok(UpgradeableLoaderState::ProgramData { + slot: _, + upgrade_authority_address: authority_pubkey, + }) = account.state() + { + if authority_pubkey != Some(authority_signer.pubkey()) { + return Err(format!( + "Program authority {:?} does not match {:?}", + authority_pubkey, + Some(authority_signer.pubkey()) + ) + .into()); + } else { + close( + rpc_client, + config, + &programdata_pubkey, + &recipient_pubkey, + authority_signer, + Some(&account_pubkey), + )?; + return Ok(config.output_format.formatted_string( + &CliUpgradeableProgramClosed { + program_id: account_pubkey.to_string(), + lamports: account.lamports, + use_lamports_unit, + }, + )); + } + } else { + return Err( + format!("Program {} has been closed", account_pubkey).into() + ); + } + } else { + return Err(format!("Program {} has been closed", account_pubkey).into()); + } + } + _ => { + return Err( + format!("{} is not a Program or Buffer account", account_pubkey).into(), + ); } - } else { - return Err(format!( - "{} is not an upgradeble loader buffer account", - account_pubkey - ) - .into()); } } else { return Err(format!("Unable to find the account {}", account_pubkey).into()); } } else { - let mut bytes = vec![1, 0, 0, 0, 1]; - bytes.extend_from_slice(authority_signer.pubkey().as_ref()); - let length = bytes.len(); - - let results = rpc_client.get_program_accounts_with_config( - &bpf_loader_upgradeable::id(), - RpcProgramAccountsConfig { - filters: Some(vec![RpcFilterType::Memcmp(Memcmp { - offset: 0, - bytes: MemcmpEncodedBytes::Binary(bs58::encode(bytes).into_string()), - encoding: None, - })]), - account_config: RpcAccountInfoConfig { - encoding: Some(UiAccountEncoding::Base64), - data_slice: Some(UiDataSliceConfig { offset: 0, length }), - ..RpcAccountInfoConfig::default() - }, - ..RpcProgramAccountsConfig::default() - }, - )?; + let results = get_buffers(rpc_client, Some(authority_signer.pubkey()))?; for (address, account) in results.iter() { if close( @@ -1459,6 +1478,7 @@ fn process_close( address, &recipient_pubkey, authority_signer, + None, ) .is_ok() { diff --git a/cli/tests/program.rs b/cli/tests/program.rs index 1772bec05..647e31b65 100644 --- a/cli/tests/program.rs +++ b/cli/tests/program.rs @@ -525,7 +525,7 @@ fn test_cli_program_deploy_with_authority() { { assert_eq!(upgrade_authority_address, None); } else { - panic!("not a buffer account"); + panic!("not a ProgramData account"); } // Get buffer authority @@ -548,6 +548,88 @@ fn test_cli_program_deploy_with_authority() { assert_eq!("none", authority_pubkey_str); } +#[test] +fn test_cli_program_close_program() { + solana_logger::setup(); + + let mut pathbuf = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + pathbuf.push("tests"); + pathbuf.push("fixtures"); + pathbuf.push("noop"); + pathbuf.set_extension("so"); + + let mint_keypair = Keypair::new(); + let mint_pubkey = mint_keypair.pubkey(); + let faucet_addr = run_local_faucet(mint_keypair, None); + let test_validator = + TestValidator::with_no_fees(mint_pubkey, Some(faucet_addr), SocketAddrSpace::Unspecified); + + let rpc_client = + RpcClient::new_with_commitment(test_validator.rpc_url(), CommitmentConfig::processed()); + + let mut file = File::open(pathbuf.to_str().unwrap()).unwrap(); + let mut program_data = Vec::new(); + file.read_to_end(&mut program_data).unwrap(); + let max_len = program_data.len(); + let minimum_balance_for_programdata = rpc_client + .get_minimum_balance_for_rent_exemption( + UpgradeableLoaderState::programdata_len(max_len).unwrap(), + ) + .unwrap(); + let minimum_balance_for_program = rpc_client + .get_minimum_balance_for_rent_exemption(UpgradeableLoaderState::program_len().unwrap()) + .unwrap(); + let upgrade_authority = Keypair::new(); + + let mut config = CliConfig::recent_for_tests(); + let keypair = Keypair::new(); + config.json_rpc_url = test_validator.rpc_url(); + config.signers = vec![&keypair]; + config.command = CliCommand::Airdrop { + pubkey: None, + lamports: 100 * minimum_balance_for_programdata + minimum_balance_for_program, + }; + process_command(&config).unwrap(); + + // Deploy the upgradeable program + let program_keypair = Keypair::new(); + config.signers = vec![&keypair, &upgrade_authority, &program_keypair]; + config.command = CliCommand::Program(ProgramCliCommand::Deploy { + program_location: Some(pathbuf.to_str().unwrap().to_string()), + program_signer_index: Some(2), + program_pubkey: Some(program_keypair.pubkey()), + buffer_signer_index: None, + buffer_pubkey: None, + allow_excessive_balance: false, + upgrade_authority_signer_index: 1, + is_final: false, + max_len: Some(max_len), + }); + config.output_format = OutputFormat::JsonCompact; + process_command(&config).unwrap(); + + let (programdata_pubkey, _) = Pubkey::find_program_address( + &[program_keypair.pubkey().as_ref()], + &bpf_loader_upgradeable::id(), + ); + + // Close program + let close_account = rpc_client.get_account(&programdata_pubkey).unwrap(); + let programdata_lamports = close_account.lamports; + let recipient_pubkey = Pubkey::new_unique(); + config.signers = vec![&keypair, &upgrade_authority]; + config.command = CliCommand::Program(ProgramCliCommand::Close { + account_pubkey: Some(program_keypair.pubkey()), + recipient_pubkey, + authority_index: 1, + use_lamports_unit: false, + }); + process_command(&config).unwrap(); + rpc_client.get_account(&programdata_pubkey).unwrap_err(); + let recipient_account = rpc_client.get_account(&recipient_pubkey).unwrap(); + assert_eq!(programdata_lamports, recipient_account.lamports); +} + #[test] fn test_cli_program_write_buffer() { solana_logger::setup(); diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index b10841db4..da23270d0 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -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, + authority_account: &KeyedAccount, + close_account: &KeyedAccount, + recipient_account: &KeyedAccount, + logger: Rc>, + 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 diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 3757c604f..de951300c 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -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> { 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::>(); - 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, diff --git a/sdk/cargo-build-bpf/src/main.rs b/sdk/cargo-build-bpf/src/main.rs index ba2a15638..07ce2df19 100644 --- a/sdk/cargo-build-bpf/src/main.rs +++ b/sdk/cargo-build-bpf/src/main.rs @@ -589,6 +589,8 @@ fn build_bpf_package(config: &Config, target_directory: &Path, package: &cargo_m println!(); println!("To deploy this program:"); println!(" $ solana program deploy {}", program_so.display()); + println!("The program address will default to this keypair (override with --program-id):"); + println!(" {}", program_keypair.display()); } else if config.dump { println!("Note: --dump is only available for crates with a cdylib target"); } diff --git a/sdk/program/src/bpf_loader_upgradeable.rs b/sdk/program/src/bpf_loader_upgradeable.rs index 2ff747ba7..298790e4a 100644 --- a/sdk/program/src/bpf_loader_upgradeable.rs +++ b/sdk/program/src/bpf_loader_upgradeable.rs @@ -196,6 +196,10 @@ pub fn is_set_authority_instruction(instruction_data: &[u8]) -> bool { !instruction_data.is_empty() && 4 == instruction_data[0] } +pub fn is_close_instruction(instruction_data: &[u8]) -> bool { + !instruction_data.is_empty() && 5 == instruction_data[0] +} + /// Returns the instructions required to set a buffers's authority. pub fn set_buffer_authority( buffer_address: &Pubkey, @@ -231,17 +235,37 @@ pub fn set_upgrade_authority( Instruction::new_with_bincode(id(), &UpgradeableLoaderInstruction::SetAuthority, metas) } -/// Returns the instructions required to close an account +/// Returns the instructions required to close a buffer account pub fn close( close_address: &Pubkey, recipient_address: &Pubkey, authority_address: &Pubkey, ) -> Instruction { - let metas = vec![ + close_any( + close_address, + recipient_address, + Some(authority_address), + None, + ) +} + +/// Returns the instructions required to close program, buffer, or uninitialized account +pub fn close_any( + close_address: &Pubkey, + recipient_address: &Pubkey, + authority_address: Option<&Pubkey>, + program_address: Option<&Pubkey>, +) -> Instruction { + let mut metas = vec![ AccountMeta::new(*close_address, false), AccountMeta::new(*recipient_address, false), - AccountMeta::new_readonly(*authority_address, true), ]; + if let Some(authority_address) = authority_address { + metas.push(AccountMeta::new(*authority_address, true)); + } + if let Some(program_address) = program_address { + metas.push(AccountMeta::new(*program_address, false)); + } Instruction::new_with_bincode(id(), &UpgradeableLoaderInstruction::Close, metas) } diff --git a/sdk/program/src/loader_upgradeable_instruction.rs b/sdk/program/src/loader_upgradeable_instruction.rs index 76d199363..e7f088d10 100644 --- a/sdk/program/src/loader_upgradeable_instruction.rs +++ b/sdk/program/src/loader_upgradeable_instruction.rs @@ -117,8 +117,12 @@ pub enum UpgradeableLoaderInstruction { /// withdraws all the lamports /// /// # Account references - /// 0. `[writable]` The account to close. + /// 0. `[writable]` The account to close, if closing a program must be the + /// ProgramData account. /// 1. `[writable]` The account to deposit the closed account's lamports. - /// 2. `[signer]` The account's authority. + /// 2. `[signer]` The account's authority, Optional, required for + /// initialized accounts. + /// 3. `[writable]` The associated Program account if the account to close + /// is a ProgramData account. Close, } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 0d0cf203a..a6bdac9c8 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -199,6 +199,10 @@ pub mod libsecp256k1_fail_on_bad_count { solana_sdk::declare_id!("8aXvSuopd1PUj7UhehfXJRg6619RHp8ZvwTyyJHdUYsj"); } +pub mod close_upgradeable_program_accounts { + solana_sdk::declare_id!("EQMtCuSAkMVF9ZdhGuABtgvyXJLtSRF5AQKv1RNsrhj7"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -241,7 +245,8 @@ lazy_static! { (gate_large_block::id(), "validator checks block cost against max limit in realtime, reject if exceeds."), (mem_overlap_fix::id(), "memory overlap fix"), (versioned_tx_message_enabled::id(), "enable versioned transaction message processing"), - (libsecp256k1_fail_on_bad_count::id(), "Fail libsec256k1_verify if count appears wrong") + (libsecp256k1_fail_on_bad_count::id(), "Fail libsec256k1_verify if count appears wrong"), + (close_upgradeable_program_accounts::id(), "enable closing upgradeable program accounts"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()