Skip to content
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

Revert deprecate executable feature #309

Merged
merged 2 commits into from
Mar 20, 2024
Merged
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
15 changes: 3 additions & 12 deletions cli/src/program.rs
Original file line number Diff line number Diff line change
@@ -44,7 +44,7 @@ use {
},
solana_rpc_client_nonce_utils::blockhash_query::BlockhashQuery,
solana_sdk::{
account::{is_executable, Account},
account::Account,
account_utils::StateMut,
bpf_loader, bpf_loader_deprecated,
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
@@ -1066,15 +1066,6 @@ fn get_default_program_keypair(program_location: &Option<String>) -> Keypair {
program_keypair
}

fn is_account_executable(account: &Account) -> bool {
if account.owner == bpf_loader_deprecated::id() || account.owner == bpf_loader::id() {
account.executable
} else {
let feature_set = FeatureSet::all_enabled();
is_executable(account, &feature_set)
}
}

/// Deploy program using upgradeable loader. It also can process program upgrades
#[allow(clippy::too_many_arguments)]
fn process_program_deploy(
@@ -1131,7 +1122,7 @@ fn process_program_deploy(
.into());
}

if !is_account_executable(&account) {
if !account.executable {
// Continue an initial deploy
true
} else if let Ok(UpgradeableLoaderState::Program {
@@ -2515,7 +2506,7 @@ fn complete_partial_program_init(
) -> Result<(Vec<Instruction>, u64), Box<dyn std::error::Error>> {
let mut instructions: Vec<Instruction> = vec![];
let mut balance_needed = 0;
if is_account_executable(account) {
if account.executable {
return Err("Buffer account is already executable".into());
}
if account.owner != *loader_id && !system_program::check_id(&account.owner) {
44 changes: 12 additions & 32 deletions cli/tests/program.rs
Original file line number Diff line number Diff line change
@@ -14,11 +14,9 @@ use {
solana_rpc_client::rpc_client::RpcClient,
solana_rpc_client_nonce_utils::blockhash_query::BlockhashQuery,
solana_sdk::{
account::is_executable,
account_utils::StateMut,
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
commitment_config::CommitmentConfig,
feature_set::FeatureSet,
pubkey::Pubkey,
signature::{Keypair, NullSigner, Signer},
},
@@ -102,7 +100,7 @@ fn test_cli_program_deploy_non_upgradeable() {
let account0 = rpc_client.get_account(&program_id).unwrap();
assert_eq!(account0.lamports, minimum_balance_for_program);
assert_eq!(account0.owner, bpf_loader_upgradeable::id());
assert!(is_executable(&account0, &FeatureSet::all_enabled()));
assert!(account0.executable);

let (programdata_pubkey, _) =
Pubkey::find_program_address(&[program_id.as_ref()], &bpf_loader_upgradeable::id());
@@ -112,10 +110,7 @@ fn test_cli_program_deploy_non_upgradeable() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert!(!programdata_account.executable);
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
@@ -143,7 +138,7 @@ fn test_cli_program_deploy_non_upgradeable() {
.unwrap();
assert_eq!(account1.lamports, minimum_balance_for_program);
assert_eq!(account1.owner, bpf_loader_upgradeable::id());
assert!(is_executable(&account1, &FeatureSet::all_enabled()));
assert!(account1.executable);
let (programdata_pubkey, _) = Pubkey::find_program_address(
&[custom_address_keypair.pubkey().as_ref()],
&bpf_loader_upgradeable::id(),
@@ -154,10 +149,7 @@ fn test_cli_program_deploy_non_upgradeable() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert!(!programdata_account.executable);
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
@@ -385,7 +377,7 @@ fn test_cli_program_deploy_with_authority() {
let program_account = rpc_client.get_account(&program_keypair.pubkey()).unwrap();
assert_eq!(program_account.lamports, minimum_balance_for_program);
assert_eq!(program_account.owner, bpf_loader_upgradeable::id());
assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
assert!(program_account.executable);
let (programdata_pubkey, _) = Pubkey::find_program_address(
&[program_keypair.pubkey().as_ref()],
&bpf_loader_upgradeable::id(),
@@ -396,10 +388,7 @@ fn test_cli_program_deploy_with_authority() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert!(!programdata_account.executable);
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
@@ -433,7 +422,7 @@ fn test_cli_program_deploy_with_authority() {
let program_account = rpc_client.get_account(&program_pubkey).unwrap();
assert_eq!(program_account.lamports, minimum_balance_for_program);
assert_eq!(program_account.owner, bpf_loader_upgradeable::id());
assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
assert!(program_account.executable);
let (programdata_pubkey, _) =
Pubkey::find_program_address(&[program_pubkey.as_ref()], &bpf_loader_upgradeable::id());
let programdata_account = rpc_client.get_account(&programdata_pubkey).unwrap();
@@ -442,10 +431,7 @@ fn test_cli_program_deploy_with_authority() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert!(program_account.executable);
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
@@ -470,7 +456,7 @@ fn test_cli_program_deploy_with_authority() {
let program_account = rpc_client.get_account(&program_pubkey).unwrap();
assert_eq!(program_account.lamports, minimum_balance_for_program);
assert_eq!(program_account.owner, bpf_loader_upgradeable::id());
assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
assert!(program_account.executable);
let (programdata_pubkey, _) =
Pubkey::find_program_address(&[program_pubkey.as_ref()], &bpf_loader_upgradeable::id());
let programdata_account = rpc_client.get_account(&programdata_pubkey).unwrap();
@@ -479,10 +465,7 @@ fn test_cli_program_deploy_with_authority() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert!(program_account.executable);
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
@@ -548,7 +531,7 @@ fn test_cli_program_deploy_with_authority() {
let program_account = rpc_client.get_account(&program_pubkey).unwrap();
assert_eq!(program_account.lamports, minimum_balance_for_program);
assert_eq!(program_account.owner, bpf_loader_upgradeable::id());
assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
assert!(program_account.executable);
let (programdata_pubkey, _) =
Pubkey::find_program_address(&[program_pubkey.as_ref()], &bpf_loader_upgradeable::id());
let programdata_account = rpc_client.get_account(&programdata_pubkey).unwrap();
@@ -557,10 +540,7 @@ fn test_cli_program_deploy_with_authority() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert!(program_account.executable);
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
1 change: 0 additions & 1 deletion ledger-tool/src/program.rs
Original file line number Diff line number Diff line change
@@ -540,7 +540,6 @@ pub fn program(ledger_path: &Path, matches: &ArgMatches<'_>) {
.get_current_instruction_context()
.unwrap(),
true, // copy_account_data
&invoke_context.feature_set,
)
.unwrap();

14 changes: 7 additions & 7 deletions program-runtime/src/invoke_context.rs
Original file line number Diff line number Diff line change
@@ -403,7 +403,7 @@ impl<'a> InvokeContext<'a> {
})?;
let borrowed_program_account = instruction_context
.try_borrow_instruction_account(self.transaction_context, program_account_index)?;
if !borrowed_program_account.is_executable(&self.feature_set) {
if !borrowed_program_account.is_executable() {
ic_msg!(self, "Account {} is not executable", callee_program_id);
return Err(InstructionError::AccountNotExecutable);
}
@@ -802,17 +802,17 @@ mod tests {
MockInstruction::NoopFail => return Err(InstructionError::GenericError),
MockInstruction::ModifyOwned => instruction_context
.try_borrow_instruction_account(transaction_context, 0)?
.set_data_from_slice(&[1], &invoke_context.feature_set)?,
.set_data_from_slice(&[1])?,
MockInstruction::ModifyNotOwned => instruction_context
.try_borrow_instruction_account(transaction_context, 1)?
.set_data_from_slice(&[1], &invoke_context.feature_set)?,
.set_data_from_slice(&[1])?,
MockInstruction::ModifyReadonly => instruction_context
.try_borrow_instruction_account(transaction_context, 2)?
.set_data_from_slice(&[1], &invoke_context.feature_set)?,
.set_data_from_slice(&[1])?,
MockInstruction::UnbalancedPush => {
instruction_context
.try_borrow_instruction_account(transaction_context, 0)?
.checked_add_lamports(1, &invoke_context.feature_set)?;
.checked_add_lamports(1)?;
let program_id = *transaction_context.get_key_of_account_at_index(3)?;
let metas = vec![
AccountMeta::new_readonly(
@@ -843,7 +843,7 @@ mod tests {
}
MockInstruction::UnbalancedPop => instruction_context
.try_borrow_instruction_account(transaction_context, 0)?
.checked_add_lamports(1, &invoke_context.feature_set)?,
.checked_add_lamports(1)?,
MockInstruction::ConsumeComputeUnits {
compute_units_to_consume,
desired_result,
@@ -855,7 +855,7 @@ mod tests {
}
MockInstruction::Resize { new_len } => instruction_context
.try_borrow_instruction_account(transaction_context, 0)?
.set_data(vec![0; new_len as usize], &invoke_context.feature_set)?,
.set_data(vec![0; new_len as usize])?,
}
} else {
return Err(InstructionError::InvalidInstructionData);
16 changes: 8 additions & 8 deletions program-runtime/src/message_processor.rs
Original file line number Diff line number Diff line change
@@ -221,16 +221,16 @@ mod tests {
MockSystemInstruction::TransferLamports { lamports } => {
instruction_context
.try_borrow_instruction_account(transaction_context, 0)?
.checked_sub_lamports(lamports, &invoke_context.feature_set)?;
.checked_sub_lamports(lamports)?;
instruction_context
.try_borrow_instruction_account(transaction_context, 1)?
.checked_add_lamports(lamports, &invoke_context.feature_set)?;
.checked_add_lamports(lamports)?;
Ok(())
}
MockSystemInstruction::ChangeData { data } => {
instruction_context
.try_borrow_instruction_account(transaction_context, 1)?
.set_data(vec![data], &invoke_context.feature_set)?;
.set_data(vec![data])?;
Ok(())
}
}
@@ -444,14 +444,14 @@ mod tests {
MockSystemInstruction::DoWork { lamports, data } => {
let mut dup_account = instruction_context
.try_borrow_instruction_account(transaction_context, 2)?;
dup_account.checked_sub_lamports(lamports, &invoke_context.feature_set)?;
to_account.checked_add_lamports(lamports, &invoke_context.feature_set)?;
dup_account.set_data(vec![data], &invoke_context.feature_set)?;
dup_account.checked_sub_lamports(lamports)?;
to_account.checked_add_lamports(lamports)?;
dup_account.set_data(vec![data])?;
drop(dup_account);
let mut from_account = instruction_context
.try_borrow_instruction_account(transaction_context, 0)?;
from_account.checked_sub_lamports(lamports, &invoke_context.feature_set)?;
to_account.checked_add_lamports(lamports, &invoke_context.feature_set)?;
from_account.checked_sub_lamports(lamports)?;
to_account.checked_add_lamports(lamports)?;
Ok(())
}
}
24 changes: 8 additions & 16 deletions program-test/src/lib.rs
Original file line number Diff line number Diff line change
@@ -133,7 +133,6 @@ pub fn invoke_builtin_function(
.transaction_context
.get_current_instruction_context()?,
true, // copy_account_data // There is no VM so direct mapping can not be implemented here
&invoke_context.feature_set,
)?;

// Deserialize data back into instruction params
@@ -164,25 +163,18 @@ pub fn invoke_builtin_function(
if borrowed_account.is_writable() {
if let Some(account_info) = account_info_map.get(borrowed_account.get_key()) {
if borrowed_account.get_lamports() != account_info.lamports() {
borrowed_account
.set_lamports(account_info.lamports(), &invoke_context.feature_set)?;
borrowed_account.set_lamports(account_info.lamports())?;
}

if borrowed_account
.can_data_be_resized(account_info.data_len())
.is_ok()
&& borrowed_account
.can_data_be_changed(&invoke_context.feature_set)
.is_ok()
&& borrowed_account.can_data_be_changed().is_ok()
{
borrowed_account.set_data_from_slice(
&account_info.data.borrow(),
&invoke_context.feature_set,
)?;
borrowed_account.set_data_from_slice(&account_info.data.borrow())?;
}
if borrowed_account.get_owner() != account_info.owner {
borrowed_account
.set_owner(account_info.owner.as_ref(), &invoke_context.feature_set)?;
borrowed_account.set_owner(account_info.owner.as_ref())?;
}
}
}
@@ -293,17 +285,17 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
.unwrap();
if borrowed_account.get_lamports() != account_info.lamports() {
borrowed_account
.set_lamports(account_info.lamports(), &invoke_context.feature_set)
.set_lamports(account_info.lamports())
.unwrap();
}
let account_info_data = account_info.try_borrow_data().unwrap();
// The redundant check helps to avoid the expensive data comparison if we can
match borrowed_account
.can_data_be_resized(account_info_data.len())
.and_then(|_| borrowed_account.can_data_be_changed(&invoke_context.feature_set))
.and_then(|_| borrowed_account.can_data_be_changed())
{
Ok(()) => borrowed_account
.set_data_from_slice(&account_info_data, &invoke_context.feature_set)
.set_data_from_slice(&account_info_data)
.unwrap(),
Err(err) if borrowed_account.get_data() != *account_info_data => {
panic!("{err:?}");
@@ -313,7 +305,7 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
// Change the owner at the end so that we are allowed to change the lamports and data before
if borrowed_account.get_owner() != account_info.owner {
borrowed_account
.set_owner(account_info.owner.as_ref(), &invoke_context.feature_set)
.set_owner(account_info.owner.as_ref())
.unwrap();
}
if instruction_account.is_writable {
Loading