Skip to content

Commit

Permalink
Deprecate account meta executable read/update in bpf loaders (solana-…
Browse files Browse the repository at this point in the history 
…labs#34194)

* use PROGRAM_OWNER + program data for account executable

mock account data with executable_meta in precompiled program and update
test_bank_hash_consistency test

pr: return const slice and add comments

pr: use ReadableAccount

use const to get rid of magic number

add featuregate disable_bpf_loader_instructions to disable bpf loader management instructions, and deprecate_executable_meta_update_in_bpf_loader to deprecate executable flag update in bpf loader

deprecate usage of executable in Account

fix a test

fix sbp bench

fix sbf program tests

add feature gate to account and borrowed account apis

fix tests

more test fixes

* restore bpf_loader v2 tests

---------

Co-authored-by: HaoranYi <[email protected]>
  • Loading branch information
@HaoranYi
HaoranYi and HaoranYi authored Jan 3, 2024
1 parent 1d93732 commit 5a3a10e
Show file tree
Hide file tree
Showing 14 changed files with 596 additions and 84 deletions.
15 changes: 12 additions & 3 deletions cli/src/program.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ use {
},
solana_rpc_client_nonce_utils::blockhash_query::BlockhashQuery,
solana_sdk::{
account::Account,
account::{is_executable, Account},
account_utils::StateMut,
bpf_loader, bpf_loader_deprecated,
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
Expand Down Expand Up @@ -1036,6 +1036,15 @@ 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(
Expand Down Expand Up @@ -1092,7 +1101,7 @@ fn process_program_deploy(
.into());
}

if !account.executable {
if !is_account_executable(&account) {
// Continue an initial deploy
true
} else if let Ok(UpgradeableLoaderState::Program {
Expand Down Expand Up @@ -2444,7 +2453,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 account.executable {
if is_account_executable(account) {
return Err("Buffer account is already executable".into());
}
if account.owner != *loader_id && !system_program::check_id(&account.owner) {
Expand Down
45 changes: 33 additions & 12 deletions cli/tests/program.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ 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},
},
Expand Down Expand Up @@ -100,7 +102,8 @@ 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!(account0.executable);
assert!(is_executable(&account0, &FeatureSet::all_enabled()));

let (programdata_pubkey, _) =
Pubkey::find_program_address(&[program_id.as_ref()], &bpf_loader_upgradeable::id());
let programdata_account = rpc_client.get_account(&programdata_pubkey).unwrap();
Expand All @@ -109,7 +112,10 @@ fn test_cli_program_deploy_non_upgradeable() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!programdata_account.executable);
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
Expand Down Expand Up @@ -137,7 +143,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!(account1.executable);
assert!(is_executable(&account1, &FeatureSet::all_enabled()));
let (programdata_pubkey, _) = Pubkey::find_program_address(
&[custom_address_keypair.pubkey().as_ref()],
&bpf_loader_upgradeable::id(),
Expand All @@ -148,7 +154,10 @@ fn test_cli_program_deploy_non_upgradeable() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!programdata_account.executable);
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
Expand Down Expand Up @@ -376,7 +385,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!(program_account.executable);
assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
let (programdata_pubkey, _) = Pubkey::find_program_address(
&[program_keypair.pubkey().as_ref()],
&bpf_loader_upgradeable::id(),
Expand All @@ -387,7 +396,10 @@ fn test_cli_program_deploy_with_authority() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!programdata_account.executable);
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
Expand Down Expand Up @@ -421,7 +433,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!(program_account.executable);
assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
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();
Expand All @@ -430,7 +442,10 @@ fn test_cli_program_deploy_with_authority() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!programdata_account.executable);
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
Expand All @@ -455,7 +470,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!(program_account.executable);
assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
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();
Expand All @@ -464,7 +479,10 @@ fn test_cli_program_deploy_with_authority() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!programdata_account.executable);
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
Expand Down Expand Up @@ -511,7 +529,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!(program_account.executable);
assert!(is_executable(&program_account, &FeatureSet::all_enabled()));
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();
Expand All @@ -520,7 +538,10 @@ fn test_cli_program_deploy_with_authority() {
minimum_balance_for_programdata
);
assert_eq!(programdata_account.owner, bpf_loader_upgradeable::id());
assert!(!programdata_account.executable);
assert!(!is_executable(
&programdata_account,
&FeatureSet::all_enabled()
));
assert_eq!(
programdata_account.data[UpgradeableLoaderState::size_of_programdata_metadata()..],
program_data[..]
Expand Down
6 changes: 3 additions & 3 deletions program-runtime/src/invoke_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -962,8 +962,8 @@ mod tests {
let owned_account = AccountSharedData::new(42, 1, &callee_program_id);
let not_owned_account = AccountSharedData::new(84, 1, &solana_sdk::pubkey::new_rand());
let readonly_account = AccountSharedData::new(168, 1, &solana_sdk::pubkey::new_rand());
let loader_account = AccountSharedData::new(0, 0, &native_loader::id());
let mut program_account = AccountSharedData::new(1, 0, &native_loader::id());
let loader_account = AccountSharedData::new(0, 1, &native_loader::id());
let mut program_account = AccountSharedData::new(1, 1, &native_loader::id());
program_account.set_executable(true);
let transaction_accounts = vec![
(solana_sdk::pubkey::new_rand(), owned_account),
Expand All @@ -990,7 +990,7 @@ mod tests {
let mut programs_loaded_for_tx_batch = LoadedProgramsForTxBatch::default();
programs_loaded_for_tx_batch.replenish(
callee_program_id,
Arc::new(LoadedProgram::new_builtin(0, 0, MockBuiltin::vm)),
Arc::new(LoadedProgram::new_builtin(0, 1, MockBuiltin::vm)),
);
invoke_context.programs_loaded_for_tx_batch = &programs_loaded_for_tx_batch;

Expand Down
49 changes: 46 additions & 3 deletions programs/bpf_loader/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ use {
clock::Slot,
entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS},
feature_set::{
bpf_account_data_direct_mapping, enable_bpf_loader_extend_program_ix,
bpf_account_data_direct_mapping, deprecate_executable_meta_update_in_bpf_loader,
disable_bpf_loader_instructions, enable_bpf_loader_extend_program_ix,
enable_bpf_loader_set_authority_checked_ix, native_programs_consume_cu,
remove_bpf_loader_incorrect_program_id, FeatureSet,
},
Expand Down Expand Up @@ -785,7 +786,15 @@ fn process_loader_upgradeable_instruction(
},
&invoke_context.feature_set,
)?;
program.set_executable(true)?;

// Skip writing true to executable meta after bpf program deployment when
// `deprecate_executable_meta_update_in_bpf_loader` feature is activated.
if !invoke_context
.feature_set
.is_active(&deprecate_executable_meta_update_in_bpf_loader::id())
{
program.set_executable(true)?;
}
drop(program);

ic_logger_msg!(log_collector, "Deployed program {:?}", new_program_id);
Expand Down Expand Up @@ -1469,6 +1478,20 @@ fn process_loader_instruction(invoke_context: &mut InvokeContext) -> Result<(),
);
return Err(InstructionError::IncorrectProgramId);
}

// Return `UnsupportedProgramId` error for bpf_loader when
// `disable_bpf_loader_instruction` feature is activated.
if invoke_context
.feature_set
.is_active(&disable_bpf_loader_instructions::id())
{
ic_msg!(
invoke_context,
"BPF loader management instructions are no longer supported"
);
return Err(InstructionError::UnsupportedProgramId);
}

let is_program_signer = program.is_signer();
match limited_deserialize(instruction_data)? {
LoaderInstruction::Write { offset, bytes } => {
Expand All @@ -1493,6 +1516,13 @@ fn process_loader_instruction(invoke_context: &mut InvokeContext) -> Result<(),
{},
program.get_data(),
);

// `deprecate_executable_meta_update_in_bpf_loader` feature doesn't
// apply to bpf_loader v2. Instead, the deployment by bpf_loader
// will be deprecated by its own feature
// `disable_bpf_loader_instructions`. Before we activate
// deprecate_executable_meta_update_in_bpf_loader, we should
// activate `disable_bpf_loader_instructions` first.
program.set_executable(true)?;
ic_msg!(invoke_context, "Finalized account {:?}", program.get_key());
}
Expand Down Expand Up @@ -1779,6 +1809,10 @@ mod tests {
expected_result,
Entrypoint::vm,
|invoke_context| {
let mut features = FeatureSet::all_enabled();
features.deactivate(&disable_bpf_loader_instructions::id());
features.deactivate(&deprecate_executable_meta_update_in_bpf_loader::id());
invoke_context.feature_set = Arc::new(features);
test_utils::load_all_invoked_programs(invoke_context);
},
|_invoke_context| {},
Expand Down Expand Up @@ -1998,6 +2032,10 @@ mod tests {
Err(InstructionError::ProgramFailedToComplete),
Entrypoint::vm,
|invoke_context| {
let mut features = FeatureSet::all_enabled();
features.deactivate(&disable_bpf_loader_instructions::id());
features.deactivate(&deprecate_executable_meta_update_in_bpf_loader::id());
invoke_context.feature_set = Arc::new(features);
invoke_context.mock_set_remaining(0);
test_utils::load_all_invoked_programs(invoke_context);
},
Expand Down Expand Up @@ -2543,7 +2581,12 @@ mod tests {
instruction_accounts,
expected_result,
Entrypoint::vm,
|_invoke_context| {},
|invoke_context| {
let mut features = FeatureSet::all_enabled();
features.deactivate(&disable_bpf_loader_instructions::id());
features.deactivate(&deprecate_executable_meta_update_in_bpf_loader::id());
invoke_context.feature_set = Arc::new(features);
},
|_invoke_context| {},
)
}
Expand Down
4 changes: 2 additions & 2 deletions programs/bpf_loader/src/serialization.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@ mod tests {
assert_eq!(account.lamports(), account_info.lamports());
assert_eq!(account.data(), &account_info.data.borrow()[..]);
assert_eq!(account.owner(), account_info.owner);
assert_eq!(account.executable(), account_info.executable);
assert!(account_info.executable);
assert_eq!(account.rent_epoch(), account_info.rent_epoch);

assert_eq!(
Expand Down Expand Up @@ -1023,7 +1023,7 @@ mod tests {
assert_eq!(account.lamports(), account_info.lamports());
assert_eq!(account.data(), &account_info.data.borrow()[..]);
assert_eq!(account.owner(), account_info.owner);
assert_eq!(account.executable(), account_info.executable);
assert!(account_info.executable);
assert_eq!(account.rent_epoch(), account_info.rent_epoch);
}

Expand Down
1 change: 1 addition & 0 deletions programs/loader-v4/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ pub fn process_instruction_retract(
let transaction_context = &invoke_context.transaction_context;
let instruction_context = transaction_context.get_current_instruction_context()?;
let mut program = instruction_context.try_borrow_instruction_account(transaction_context, 0)?;

let authority_address = instruction_context
.get_index_of_instruction_account_in_transaction(1)
.and_then(|index| transaction_context.get_key_of_account_at_index(index))?;
Expand Down
15 changes: 13 additions & 2 deletions programs/sbf/benches/bpf_loader.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use {
bpf_loader,
client::SyncClient,
entrypoint::SUCCESS,
feature_set::FeatureSet,
feature_set::{self, FeatureSet},
instruction::{AccountMeta, Instruction},
message::Message,
native_loader,
Expand Down Expand Up @@ -185,10 +185,21 @@ fn bench_program_alu(bencher: &mut Bencher) {
#[bench]
fn bench_program_execute_noop(bencher: &mut Bencher) {
let GenesisConfigInfo {
genesis_config,
mut genesis_config,
mint_keypair,
..
} = create_genesis_config(50);

// deactivate `disable_bpf_loader_instructions` feature so that the program
// can be loaded, finalized and benched.
genesis_config
.accounts
.remove(&feature_set::disable_bpf_loader_instructions::id());

genesis_config
.accounts
.remove(&feature_set::deprecate_executable_meta_update_in_bpf_loader::id());

let bank = Bank::new_for_benches(&genesis_config);
let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests();
let mut bank_client = BankClient::new_shared(bank.clone());
Expand Down
Loading

0 comments on commit 5a3a10e

Please sign in to comment.