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.

Sign up for GitHub

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

Demote write locks on transaction program ids (backport #19593) #19633

Merged
merged 2 commits into from
Sep 4, 2021
Merged

Demote write locks on transaction program ids (backport #19593) #19633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d40bb38
Demote write locks on transaction program ids (#19593)
CriesofCarrots Sep 4, 2021
1c238a1
Fix conflicts
Sep 4, 2021
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
2 changes: 1 addition & 1 deletion cli-output/src/display.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ fn format_account_mode(message: &Message, index: usize) -> String {
} else {
"-"
},
if message.is_writable(index) {
if message.is_writable(index, /*demote_program_write_locks=*/ true) {
"w" // comment for consistent rust fmt (no joking; lol)
} else {
"-"
Expand Down
7 changes: 5 additions & 2 deletions program-test/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use {
clock::{Clock, Slot},
entrypoint::{ProgramResult, SUCCESS},
epoch_schedule::EpochSchedule,
feature_set::demote_program_write_locks,
fee_calculator::{FeeCalculator, FeeRateGovernor},
genesis_config::{ClusterType, GenesisConfig},
hash::Hash,
Expand Down Expand Up @@ -258,12 +259,14 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
}
panic!("Program id {} wasn't found in account_infos", program_id);
};
let demote_program_write_locks =
invoke_context.is_feature_active(&demote_program_write_locks::id());
// TODO don't have the caller's keyed_accounts so can't validate writer or signer escalation or deescalation yet
let caller_privileges = message
.account_keys
.iter()
.enumerate()
.map(|(i, _)| message.is_writable(i))
.map(|(i, _)| message.is_writable(i, demote_program_write_locks))
.collect::<Vec<bool>>();

stable_log::program_invoke(&logger, &program_id, invoke_context.invoke_depth());
Expand Down Expand Up @@ -334,7 +337,7 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {

// Copy writeable account modifications back into the caller's AccountInfos
for (i, (pubkey, account)) in accounts.iter().enumerate().take(message.account_keys.len()) {
if !message.is_writable(i) {
if !message.is_writable(i, demote_program_write_locks) {
continue;
}
for account_info in account_infos {
Expand Down
97 changes: 4 additions & 93 deletions programs/bpf/tests/programs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1782,7 +1782,7 @@ fn test_program_bpf_upgrade_and_invoke_in_same_tx() {
"solana_bpf_rust_panic",
);

// Invoke, then upgrade the program, and then invoke again in same tx
// Attempt to invoke, then upgrade the program in same tx
let message = Message::new(
&[
invoke_instruction.clone(),
Expand All @@ -1801,10 +1801,12 @@ fn test_program_bpf_upgrade_and_invoke_in_same_tx() {
message.clone(),
bank.last_blockhash(),
);
// program_id is automatically demoted to readonly, preventing the upgrade, which requires
// writeability
let (result, _) = process_transaction_and_record_inner(&bank, tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InstructionError(2, InstructionError::ProgramFailedToComplete)
TransactionError::InstructionError(1, InstructionError::InvalidArgument)
);
}

Expand Down Expand Up @@ -2103,97 +2105,6 @@ fn test_program_bpf_upgrade_via_cpi() {
assert_ne!(programdata, original_programdata);
}

#[cfg(feature = "bpf_rust")]
#[test]
fn test_program_bpf_upgrade_self_via_cpi() {
solana_logger::setup();

let GenesisConfigInfo {
genesis_config,
mint_keypair,
..
} = create_genesis_config(50);
let mut bank = Bank::new(&genesis_config);
let (name, id, entrypoint) = solana_bpf_loader_program!();
bank.add_builtin(&name, id, entrypoint);
let (name, id, entrypoint) = solana_bpf_loader_upgradeable_program!();
bank.add_builtin(&name, id, entrypoint);
let bank = Arc::new(bank);
let bank_client = BankClient::new_shared(&bank);
let noop_program_id = load_bpf_program(
&bank_client,
&bpf_loader::id(),
&mint_keypair,
"solana_bpf_rust_noop",
);

// Deploy upgradeable program
let buffer_keypair = Keypair::new();
let program_keypair = Keypair::new();
let program_id = program_keypair.pubkey();
let authority_keypair = Keypair::new();
load_upgradeable_bpf_program(
&bank_client,
&mint_keypair,
&buffer_keypair,
&program_keypair,
&authority_keypair,
"solana_bpf_rust_invoke_and_return",
);

let mut invoke_instruction = Instruction::new_with_bytes(
program_id,
&[0],
vec![
AccountMeta::new(noop_program_id, false),
AccountMeta::new(noop_program_id, false),
AccountMeta::new(clock::id(), false),
AccountMeta::new(fees::id(), false),
],
);

// Call the upgraded program
invoke_instruction.data[0] += 1;
let result =
bank_client.send_and_confirm_instruction(&mint_keypair, invoke_instruction.clone());
assert!(result.is_ok());

// Prepare for upgrade
let buffer_keypair = Keypair::new();
load_upgradeable_buffer(
&bank_client,
&mint_keypair,
&buffer_keypair,
&authority_keypair,
"solana_bpf_rust_panic",
);

// Invoke, then upgrade the program, and then invoke again in same tx
let message = Message::new(
&[
invoke_instruction.clone(),
bpf_loader_upgradeable::upgrade(
&program_id,
&buffer_keypair.pubkey(),
&authority_keypair.pubkey(),
&mint_keypair.pubkey(),
),
invoke_instruction,
],
Some(&mint_keypair.pubkey()),
);
let tx = Transaction::new(
&[&mint_keypair, &authority_keypair],
message.clone(),
bank.last_blockhash(),
);
let (result, _) = process_transaction_and_record_inner(&bank, tx);
assert_eq!(
result.unwrap_err(),
TransactionError::InstructionError(2, InstructionError::ProgramFailedToComplete)
);
}

#[cfg(feature = "bpf_rust")]
#[test]
fn test_program_bpf_set_upgrade_authority_via_cpi() {
Expand Down
18 changes: 13 additions & 5 deletions programs/bpf_loader/src/syscalls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ use solana_sdk::{
entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS},
epoch_schedule::EpochSchedule,
feature_set::{
close_upgradeable_program_accounts, cpi_data_cost, enforce_aligned_host_addrs,
keccak256_syscall_enabled, libsecp256k1_0_5_upgrade_enabled, mem_overlap_fix,
memory_ops_syscalls, secp256k1_recover_syscall_enabled,
close_upgradeable_program_accounts, cpi_data_cost, demote_program_write_locks,
enforce_aligned_host_addrs, keccak256_syscall_enabled, libsecp256k1_0_5_upgrade_enabled,
mem_overlap_fix, memory_ops_syscalls, secp256k1_recover_syscall_enabled,
set_upgrade_authority_via_cpi_enabled, sysvar_via_syscall, update_data_on_realloc,
},
hash::{Hasher, HASH_BYTES},
Expand Down Expand Up @@ -2244,7 +2244,14 @@ fn call<'a>(
signers_seeds_len: u64,
memory_mapping: &MemoryMapping,
) -> Result<u64, EbpfError<BpfError>> {
let (message, executables, accounts, account_refs, caller_write_privileges) = {
let (
message,
executables,
accounts,
account_refs,
caller_write_privileges,
demote_program_write_locks,
) = {
let invoke_context = syscall.get_context()?;

invoke_context
Expand Down Expand Up @@ -2335,6 +2342,7 @@ fn call<'a>(
accounts,
account_refs,
caller_write_privileges,
invoke_context.is_feature_active(&demote_program_write_locks::id()),
)
};

Expand All @@ -2360,7 +2368,7 @@ fn call<'a>(
for (i, ((_key, account), account_ref)) in accounts.iter().zip(account_refs).enumerate() {
let account = account.borrow();
if let Some(mut account_ref) = account_ref {
if message.is_writable(i) && !account.executable() {
if message.is_writable(i, demote_program_write_locks) && !account.executable() {
*account_ref.lamports = account.lamports();
*account_ref.owner = *account.owner();
if account_ref.data.len() != account.data().len() {
Expand Down
5 changes: 3 additions & 2 deletions rpc/src/transaction_status_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ impl TransactionStatusService {
})
.expect("FeeCalculator must exist");
let fee = fee_calculator.calculate_fee(transaction.message());
let (writable_keys, readonly_keys) =
transaction.message.get_account_keys_by_lock_type();
let (writable_keys, readonly_keys) = transaction
.message
.get_account_keys_by_lock_type(bank.demote_program_write_locks());

let inner_instructions = inner_instructions.map(|inner_instructions| {
inner_instructions
Expand Down
Loading