gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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

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

* Demote write locks on transaction program ids (#19593)

* Add feature

* Demote write lock on program ids

* Fixup bpf tests

* Update MappedMessage::is_writable

* Comma nit

* Review comments

(cherry picked from commit decec3cd8b)

# Conflicts:
#	core/src/banking_stage.rs
#	core/src/cost_model.rs
#	core/src/cost_tracker.rs
#	ledger-tool/src/main.rs
#	program-runtime/src/instruction_processor.rs
#	programs/bpf/tests/programs.rs
#	programs/bpf_loader/src/syscalls.rs
#	rpc/src/transaction_status_service.rs
#	runtime/src/accounts.rs
#	runtime/src/bank.rs
#	runtime/src/message_processor.rs
#	sdk/benches/serialize_instructions.rs
#	sdk/program/src/message/mapped.rs
#	sdk/program/src/message/sanitized.rs
#	sdk/src/transaction/sanitized.rs

* Fix conflicts

Co-authored-by: Tyera Eulberg <teulberg@gmail.com>
Co-authored-by: Tyera Eulberg <tyera@solana.com>
(cherry picked from commit fcda5d4a7d)

# Conflicts:
#	cli-output/src/display.rs
#	core/src/transaction_status_service.rs
#	program-test/src/lib.rs
#	programs/bpf_loader/src/syscalls.rs
#	runtime/src/accounts.rs
#	runtime/src/bank.rs
#	runtime/src/message_processor.rs
#	sdk/benches/serialize_instructions.rs
#	sdk/program/src/message.rs
#	sdk/src/feature_set.rs
#	transaction-status/src/parse_accounts.rs

* Replace feature and fix conflicts

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Tyera Eulberg <tyera@solana.com>
This commit is contained in:
mergify[bot]
2021-09-04 09:10:57 +00:00
committed by GitHub
parent b4fdce9443
commit 53f8e58300
12 changed files with 198 additions and 241 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
cli-output/src/display.rs
View File

@ -140,7 +140,7 @@ fn format_account_mode(message: &Message, index: usize) -> String {
} else { } else {
"-" "-"
}, },
if message.is_writable(index, /*demote_sysvar_write_locks=*/ true) { if message.is_writable(index, /*demote_program_write_locks=*/ true) {
"w" // comment for consistent rust fmt (no joking; lol) "w" // comment for consistent rust fmt (no joking; lol)
} else { } else {
"-" "-"

2
core/src/transaction_status_service.rs
View File

@ -111,7 +111,7 @@ impl TransactionStatusService {
let fee = fee_calculator.calculate_fee(transaction.message()); let fee = fee_calculator.calculate_fee(transaction.message());
let (writable_keys, readonly_keys) = transaction let (writable_keys, readonly_keys) = transaction
.message .message
.get_account_keys_by_lock_type(bank.demote_sysvar_write_locks()); .get_account_keys_by_lock_type(bank.demote_program_write_locks());
let inner_instructions = inner_instructions.map(|inner_instructions| { let inner_instructions = inner_instructions.map(|inner_instructions| {
inner_instructions inner_instructions

10
program-test/src/lib.rs
View File

@ -19,7 +19,7 @@ use {
clock::{Clock, Slot}, clock::{Clock, Slot},
entrypoint::{ProgramResult, SUCCESS}, entrypoint::{ProgramResult, SUCCESS},
epoch_schedule::EpochSchedule, epoch_schedule::EpochSchedule,
feature_set::demote_sysvar_write_locks, feature_set::demote_program_write_locks,
fee_calculator::{FeeCalculator, FeeRateGovernor}, fee_calculator::{FeeCalculator, FeeRateGovernor},
genesis_config::{ClusterType, GenesisConfig}, genesis_config::{ClusterType, GenesisConfig},
hash::Hash, hash::Hash,
@ -256,14 +256,14 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
} }
panic!("Program id {} wasn't found in account_infos", program_id); panic!("Program id {} wasn't found in account_infos", program_id);
}; };
let demote_sysvar_write_locks = let demote_program_write_locks =
invoke_context.is_feature_active(&demote_sysvar_write_locks::id()); 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 // TODO don't have the caller's keyed_accounts so can't validate writer or signer escalation or deescalation yet
let caller_privileges = message let caller_privileges = message
.account_keys .account_keys
.iter() .iter()
.enumerate() .enumerate()
.map(|(i, _)| message.is_writable(i, demote_sysvar_write_locks)) .map(|(i, _)| message.is_writable(i, demote_program_write_locks))
.collect::<Vec<bool>>(); .collect::<Vec<bool>>();
stable_log::program_invoke(&logger, &program_id, invoke_context.invoke_depth()); stable_log::program_invoke(&logger, &program_id, invoke_context.invoke_depth());
@ -334,7 +334,7 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
// Copy writeable account modifications back into the caller's AccountInfos // Copy writeable account modifications back into the caller's AccountInfos
for (i, account_pubkey) in message.account_keys.iter().enumerate() { for (i, account_pubkey) in message.account_keys.iter().enumerate() {
if !message.is_writable(i, true) { if !message.is_writable(i, demote_program_write_locks) {
continue; continue;
} }

97
programs/bpf/tests/programs.rs
View File

@ -1773,7 +1773,7 @@ fn test_program_bpf_upgrade_and_invoke_in_same_tx() {
"solana_bpf_rust_panic", "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( let message = Message::new(
&[ &[
invoke_instruction.clone(), invoke_instruction.clone(),
@ -1792,10 +1792,12 @@ fn test_program_bpf_upgrade_and_invoke_in_same_tx() {
message.clone(), message.clone(),
bank.last_blockhash(), 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); let (result, _) = process_transaction_and_record_inner(&bank, tx);
assert_eq!( assert_eq!(
result.unwrap_err(), result.unwrap_err(),
TransactionError::InstructionError(2, InstructionError::ProgramFailedToComplete) TransactionError::InstructionError(1, InstructionError::InvalidArgument)
); );
} }
@ -2076,97 +2078,6 @@ fn test_program_bpf_upgrade_via_cpi() {
); );
} }
#[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")] #[cfg(feature = "bpf_rust")]
#[test] #[test]
fn test_program_bpf_set_upgrade_authority_via_cpi() { fn test_program_bpf_set_upgrade_authority_via_cpi() {

12
programs/bpf_loader/src/syscalls.rs
View File

@ -19,7 +19,7 @@ use solana_sdk::{
entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS}, entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS},
epoch_schedule::EpochSchedule, epoch_schedule::EpochSchedule,
feature_set::{ feature_set::{
cpi_data_cost, cpi_share_ro_and_exec_accounts, demote_sysvar_write_locks, cpi_data_cost, cpi_share_ro_and_exec_accounts, demote_program_write_locks,
enforce_aligned_host_addrs, keccak256_syscall_enabled, memory_ops_syscalls, enforce_aligned_host_addrs, keccak256_syscall_enabled, memory_ops_syscalls,
set_upgrade_authority_via_cpi_enabled, sysvar_via_syscall, update_data_on_realloc, set_upgrade_authority_via_cpi_enabled, sysvar_via_syscall, update_data_on_realloc,
}, },
@ -2146,7 +2146,7 @@ fn call<'a>(
accounts, accounts,
account_refs, account_refs,
caller_write_privileges, caller_write_privileges,
demote_sysvar_write_locks, demote_program_write_locks,
) = { ) = {
let invoke_context = syscall.get_context()?; let invoke_context = syscall.get_context()?;
@ -2237,7 +2237,7 @@ fn call<'a>(
accounts, accounts,
account_refs, account_refs,
caller_write_privileges, caller_write_privileges,
invoke_context.is_feature_active(&demote_sysvar_write_locks::id()), invoke_context.is_feature_active(&demote_program_write_locks::id()),
) )
}; };
@ -2263,9 +2263,9 @@ fn call<'a>(
for (i, (account, account_ref)) in accounts.iter().zip(account_refs).enumerate() { for (i, (account, account_ref)) in accounts.iter().zip(account_refs).enumerate() {
let account = account.borrow(); let account = account.borrow();
if let Some(mut account_ref) = account_ref { if let Some(mut account_ref) = account_ref {
if message.is_writable(i, demote_sysvar_write_locks) && !account.executable { if message.is_writable(i, demote_program_write_locks) && !account.executable() {
*account_ref.lamports = account.lamports; *account_ref.lamports = account.lamports();
*account_ref.owner = account.owner; *account_ref.owner = *account.owner();
if account_ref.data.len() != account.data().len() { if account_ref.data.len() != account.data().len() {
if !account_ref.data.is_empty() { if !account_ref.data.is_empty() {
// Only support for `CreateAccount` at this time. // Only support for `CreateAccount` at this time.

162
runtime/src/accounts.rs
View File

@ -172,9 +172,9 @@ impl Accounts {
fn construct_instructions_account( fn construct_instructions_account(
message: &Message, message: &Message,
demote_sysvar_write_locks: bool, demote_program_write_locks: bool,
) -> AccountSharedData { ) -> AccountSharedData {
let mut data = message.serialize_instructions(demote_sysvar_write_locks); let mut data = message.serialize_instructions(demote_program_write_locks);
// add room for current instruction index. // add room for current instruction index.
data.resize(data.len() + 2, 0); data.resize(data.len() + 2, 0);
AccountSharedData::from(Account { AccountSharedData::from(Account {
@ -203,9 +203,9 @@ impl Accounts {
let mut tx_rent: TransactionRent = 0; let mut tx_rent: TransactionRent = 0;
let mut accounts = Vec::with_capacity(message.account_keys.len()); let mut accounts = Vec::with_capacity(message.account_keys.len());
let mut account_deps = Vec::with_capacity(message.account_keys.len()); let mut account_deps = Vec::with_capacity(message.account_keys.len());
let demote_sysvar_write_locks =
feature_set.is_active(&feature_set::demote_sysvar_write_locks::id());
let mut rent_debits = RentDebits::default(); let mut rent_debits = RentDebits::default();
let demote_program_write_locks =
feature_set.is_active(&feature_set::demote_program_write_locks::id());
for (i, key) in message.account_keys.iter().enumerate() { for (i, key) in message.account_keys.iter().enumerate() {
let account = if message.is_non_loader_key(key, i) { let account = if message.is_non_loader_key(key, i) {
@ -216,16 +216,16 @@ impl Accounts {
if solana_sdk::sysvar::instructions::check_id(key) if solana_sdk::sysvar::instructions::check_id(key)
&& feature_set.is_active(&feature_set::instructions_sysvar_enabled::id()) && feature_set.is_active(&feature_set::instructions_sysvar_enabled::id())
{ {
if message.is_writable(i, demote_sysvar_write_locks) { if message.is_writable(i, demote_program_write_locks) {
return Err(TransactionError::InvalidAccountIndex); return Err(TransactionError::InvalidAccountIndex);
} }
Self::construct_instructions_account(message, demote_sysvar_write_locks) Self::construct_instructions_account(message, demote_program_write_locks)
} else { } else {
let (account, rent) = self let (account, rent) = self
.accounts_db .accounts_db
.load(ancestors, key) .load(ancestors, key)
.map(|(mut account, _)| { .map(|(mut account, _)| {
if message.is_writable(i, demote_sysvar_write_locks) { if message.is_writable(i, demote_program_write_locks) {
let rent_due = rent_collector let rent_due = rent_collector
.collect_from_existing_account(&key, &mut account); .collect_from_existing_account(&key, &mut account);
(account, rent_due) (account, rent_due)
@ -776,7 +776,7 @@ impl Accounts {
tx: &Transaction, tx: &Transaction,
result: &Result<()>, result: &Result<()>,
locks: &mut AccountLocks, locks: &mut AccountLocks,
demote_sysvar_write_locks: bool, demote_program_write_locks: bool,
) { ) {
match result { match result {
Err(TransactionError::AccountInUse) => (), Err(TransactionError::AccountInUse) => (),
@ -785,7 +785,7 @@ impl Accounts {
_ => { _ => {
let (writable_keys, readonly_keys) = &tx let (writable_keys, readonly_keys) = &tx
.message() .message()
.get_account_keys_by_lock_type(demote_sysvar_write_locks); .get_account_keys_by_lock_type(demote_program_write_locks);
for k in writable_keys { for k in writable_keys {
locks.unlock_write(k); locks.unlock_write(k);
} }
@ -817,7 +817,7 @@ impl Accounts {
pub fn lock_accounts<'a>( pub fn lock_accounts<'a>(
&self, &self,
txs: impl Iterator<Item = &'a Transaction>, txs: impl Iterator<Item = &'a Transaction>,
demote_sysvar_write_locks: bool, demote_program_write_locks: bool,
) -> Vec<Result<()>> { ) -> Vec<Result<()>> {
use solana_sdk::sanitize::Sanitize; use solana_sdk::sanitize::Sanitize;
let keys: Vec<Result<_>> = txs let keys: Vec<Result<_>> = txs
@ -830,7 +830,7 @@ impl Accounts {
Ok(tx Ok(tx
.message() .message()
.get_account_keys_by_lock_type(demote_sysvar_write_locks)) .get_account_keys_by_lock_type(demote_program_write_locks))
}) })
.collect(); .collect();
let mut account_locks = &mut self.account_locks.lock().unwrap(); let mut account_locks = &mut self.account_locks.lock().unwrap();
@ -849,7 +849,7 @@ impl Accounts {
&self, &self,
txs: impl Iterator<Item = &'a Transaction>, txs: impl Iterator<Item = &'a Transaction>,
results: &[Result<()>], results: &[Result<()>],
demote_sysvar_write_locks: bool, demote_program_write_locks: bool,
) { ) {
let mut account_locks = self.account_locks.lock().unwrap(); let mut account_locks = self.account_locks.lock().unwrap();
debug!("bank unlock accounts"); debug!("bank unlock accounts");
@ -858,7 +858,7 @@ impl Accounts {
tx, tx,
lock_result, lock_result,
&mut account_locks, &mut account_locks,
demote_sysvar_write_locks, demote_program_write_locks,
); );
} }
} }
@ -875,8 +875,8 @@ impl Accounts {
rent_collector: &RentCollector, rent_collector: &RentCollector,
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
fix_recent_blockhashes_sysvar_delay: bool, fix_recent_blockhashes_sysvar_delay: bool,
demote_sysvar_write_locks: bool,
merge_nonce_error_into_system_error: bool, merge_nonce_error_into_system_error: bool,
demote_program_write_locks: bool,
) { ) {
let accounts_to_store = self.collect_accounts_to_store( let accounts_to_store = self.collect_accounts_to_store(
txs, txs,
@ -885,8 +885,8 @@ impl Accounts {
rent_collector, rent_collector,
last_blockhash_with_fee_calculator, last_blockhash_with_fee_calculator,
fix_recent_blockhashes_sysvar_delay, fix_recent_blockhashes_sysvar_delay,
demote_sysvar_write_locks,
merge_nonce_error_into_system_error, merge_nonce_error_into_system_error,
demote_program_write_locks,
); );
self.accounts_db.store_cached(slot, &accounts_to_store); self.accounts_db.store_cached(slot, &accounts_to_store);
} }
@ -902,6 +902,7 @@ impl Accounts {
self.accounts_db.add_root(slot) self.accounts_db.add_root(slot)
} }
#[allow(clippy::too_many_arguments)]
fn collect_accounts_to_store<'a>( fn collect_accounts_to_store<'a>(
&self, &self,
txs: impl Iterator<Item = &'a Transaction>, txs: impl Iterator<Item = &'a Transaction>,
@ -910,8 +911,8 @@ impl Accounts {
rent_collector: &RentCollector, rent_collector: &RentCollector,
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
fix_recent_blockhashes_sysvar_delay: bool, fix_recent_blockhashes_sysvar_delay: bool,
demote_sysvar_write_locks: bool,
merge_nonce_error_into_system_error: bool, merge_nonce_error_into_system_error: bool,
demote_program_write_locks: bool,
) -> Vec<(&'a Pubkey, &'a AccountSharedData)> { ) -> Vec<(&'a Pubkey, &'a AccountSharedData)> {
let mut accounts = Vec::with_capacity(loaded.len()); let mut accounts = Vec::with_capacity(loaded.len());
for (i, ((raccs, _nonce_rollback), tx)) in loaded.iter_mut().zip(txs).enumerate() { for (i, ((raccs, _nonce_rollback), tx)) in loaded.iter_mut().zip(txs).enumerate() {
@ -964,7 +965,7 @@ impl Accounts {
fee_payer_index = Some(i); fee_payer_index = Some(i);
} }
let is_fee_payer = Some(i) == fee_payer_index; let is_fee_payer = Some(i) == fee_payer_index;
if message.is_writable(i, demote_sysvar_write_locks) if message.is_writable(i, demote_program_write_locks)
&& (res.is_ok() && (res.is_ok()
|| (maybe_nonce_rollback.is_some() && (is_nonce_account || is_fee_payer))) || (maybe_nonce_rollback.is_some() && (is_nonce_account || is_fee_payer)))
{ {
@ -1724,6 +1725,8 @@ mod tests {
accounts.store_slow_uncached(0, &keypair2.pubkey(), &account2); accounts.store_slow_uncached(0, &keypair2.pubkey(), &account2);
accounts.store_slow_uncached(0, &keypair3.pubkey(), &account3); accounts.store_slow_uncached(0, &keypair3.pubkey(), &account3);
let demote_program_write_locks = true;
let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])];
let message = Message::new_with_compiled_instructions( let message = Message::new_with_compiled_instructions(
1, 1,
@ -1734,10 +1737,7 @@ mod tests {
instructions, instructions,
); );
let tx = Transaction::new(&[&keypair0], message, Hash::default()); let tx = Transaction::new(&[&keypair0], message, Hash::default());
let results0 = accounts.lock_accounts( let results0 = accounts.lock_accounts([tx.clone()].iter(), demote_program_write_locks);
[tx.clone()].iter(),
true, // demote_sysvar_write_locks
);
assert!(results0[0].is_ok()); assert!(results0[0].is_ok());
assert_eq!( assert_eq!(
@ -1772,10 +1772,7 @@ mod tests {
); );
let tx1 = Transaction::new(&[&keypair1], message, Hash::default()); let tx1 = Transaction::new(&[&keypair1], message, Hash::default());
let txs = vec![tx0, tx1]; let txs = vec![tx0, tx1];
let results1 = accounts.lock_accounts( let results1 = accounts.lock_accounts(txs.iter(), demote_program_write_locks);
txs.iter(),
true, // demote_sysvar_write_locks
);
assert!(results1[0].is_ok()); // Read-only account (keypair1) can be referenced multiple times assert!(results1[0].is_ok()); // Read-only account (keypair1) can be referenced multiple times
assert!(results1[1].is_err()); // Read-only account (keypair1) cannot also be locked as writable assert!(results1[1].is_err()); // Read-only account (keypair1) cannot also be locked as writable
@ -1790,16 +1787,8 @@ mod tests {
2 2
); );
accounts.unlock_accounts( accounts.unlock_accounts([tx].iter(), &results0, demote_program_write_locks);
[tx].iter(), accounts.unlock_accounts(txs.iter(), &results1, demote_program_write_locks);
&results0,
true, // demote_sysvar_write_locks
);
accounts.unlock_accounts(
txs.iter(),
&results1,
true, // demote_sysvar_write_locks
);
let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])];
let message = Message::new_with_compiled_instructions( let message = Message::new_with_compiled_instructions(
1, 1,
@ -1810,10 +1799,7 @@ mod tests {
instructions, instructions,
); );
let tx = Transaction::new(&[&keypair1], message, Hash::default()); let tx = Transaction::new(&[&keypair1], message, Hash::default());
let results2 = accounts.lock_accounts( let results2 = accounts.lock_accounts([tx].iter(), demote_program_write_locks);
[tx].iter(),
true, // demote_sysvar_write_locks
);
assert!(results2[0].is_ok()); // Now keypair1 account can be locked as writable assert!(results2[0].is_ok()); // Now keypair1 account can be locked as writable
// Check that read-only lock with zero references is deleted // Check that read-only lock with zero references is deleted
@ -1849,6 +1835,8 @@ mod tests {
accounts.store_slow_uncached(0, &keypair1.pubkey(), &account1); accounts.store_slow_uncached(0, &keypair1.pubkey(), &account1);
accounts.store_slow_uncached(0, &keypair2.pubkey(), &account2); accounts.store_slow_uncached(0, &keypair2.pubkey(), &account2);
let demote_program_write_locks = true;
let accounts_arc = Arc::new(accounts); let accounts_arc = Arc::new(accounts);
let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])];
@ -1881,20 +1869,15 @@ mod tests {
let exit_clone = exit_clone.clone(); let exit_clone = exit_clone.clone();
loop { loop {
let txs = vec![writable_tx.clone()]; let txs = vec![writable_tx.clone()];
let results = accounts_clone.clone().lock_accounts( let results = accounts_clone
txs.iter(), .clone()
true, // demote_sysvar_write_locks .lock_accounts(txs.iter(), demote_program_write_locks);
);
for result in results.iter() { for result in results.iter() {
if result.is_ok() { if result.is_ok() {
counter_clone.clone().fetch_add(1, Ordering::SeqCst); counter_clone.clone().fetch_add(1, Ordering::SeqCst);
} }
} }
accounts_clone.unlock_accounts( accounts_clone.unlock_accounts(txs.iter(), &results, demote_program_write_locks);
txs.iter(),
&results,
true, // demote_sysvar_write_locks
);
if exit_clone.clone().load(Ordering::Relaxed) { if exit_clone.clone().load(Ordering::Relaxed) {
break; break;
} }
@ -1903,25 +1886,84 @@ mod tests {
let counter_clone = counter; let counter_clone = counter;
for _ in 0..5 { for _ in 0..5 {
let txs = vec![readonly_tx.clone()]; let txs = vec![readonly_tx.clone()];
let results = accounts_arc.clone().lock_accounts( let results = accounts_arc
txs.iter(), .clone()
true, // demote_sysvar_write_locks .lock_accounts(txs.iter(), demote_program_write_locks);
);
if results[0].is_ok() { if results[0].is_ok() {
let counter_value = counter_clone.clone().load(Ordering::SeqCst); let counter_value = counter_clone.clone().load(Ordering::SeqCst);
thread::sleep(time::Duration::from_millis(50)); thread::sleep(time::Duration::from_millis(50));
assert_eq!(counter_value, counter_clone.clone().load(Ordering::SeqCst)); assert_eq!(counter_value, counter_clone.clone().load(Ordering::SeqCst));
} }
accounts_arc.unlock_accounts( accounts_arc.unlock_accounts(txs.iter(), &results, demote_program_write_locks);
txs.iter(),
&results,
true, // demote_sysvar_write_locks
);
thread::sleep(time::Duration::from_millis(50)); thread::sleep(time::Duration::from_millis(50));
} }
exit.store(true, Ordering::Relaxed); exit.store(true, Ordering::Relaxed);
} }
#[test]
fn test_demote_program_write_locks() {
let keypair0 = Keypair::new();
let keypair1 = Keypair::new();
let keypair2 = Keypair::new();
let keypair3 = Keypair::new();
let account0 = AccountSharedData::new(1, 0, &Pubkey::default());
let account1 = AccountSharedData::new(2, 0, &Pubkey::default());
let account2 = AccountSharedData::new(3, 0, &Pubkey::default());
let account3 = AccountSharedData::new(4, 0, &Pubkey::default());
let accounts = Accounts::new_with_config(
Vec::new(),
&ClusterType::Development,
AccountSecondaryIndexes::default(),
false,
);
accounts.store_slow_uncached(0, &keypair0.pubkey(), &account0);
accounts.store_slow_uncached(0, &keypair1.pubkey(), &account1);
accounts.store_slow_uncached(0, &keypair2.pubkey(), &account2);
accounts.store_slow_uncached(0, &keypair3.pubkey(), &account3);
let demote_program_write_locks = true;
let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])];
let message = Message::new_with_compiled_instructions(
1,
0,
0, // All accounts marked as writable
vec![keypair0.pubkey(), keypair1.pubkey(), native_loader::id()],
Hash::default(),
instructions,
);
let tx = Transaction::new(&[&keypair0], message, Hash::default());
let results0 = accounts.lock_accounts([tx].iter(), demote_program_write_locks);
assert!(results0[0].is_ok());
// Instruction program-id account demoted to readonly
assert_eq!(
*accounts
.account_locks
.lock()
.unwrap()
.readonly_locks
.get(&native_loader::id())
.unwrap(),
1
);
// Non-program accounts remain writable
assert!(accounts
.account_locks
.lock()
.unwrap()
.write_locks
.contains(&keypair0.pubkey()));
assert!(accounts
.account_locks
.lock()
.unwrap()
.write_locks
.contains(&keypair1.pubkey()));
}
#[test] #[test]
fn test_collect_accounts_to_store() { fn test_collect_accounts_to_store() {
let keypair0 = Keypair::new(); let keypair0 = Keypair::new();
@ -2009,8 +2051,8 @@ mod tests {
&rent_collector, &rent_collector,
&(Hash::default(), FeeCalculator::default()), &(Hash::default(), FeeCalculator::default()),
true, true,
true, // demote_sysvar_write_locks
true, // merge_nonce_error_into_system_error true, // merge_nonce_error_into_system_error
true, // demote_program_write_locks
); );
assert_eq!(collected_accounts.len(), 2); assert_eq!(collected_accounts.len(), 2);
assert!(collected_accounts assert!(collected_accounts
@ -2395,8 +2437,8 @@ mod tests {
&rent_collector, &rent_collector,
&(next_blockhash, FeeCalculator::default()), &(next_blockhash, FeeCalculator::default()),
true, true,
true, // demote_sysvar_write_locks
true, // merge_nonce_error_into_system_error true, // merge_nonce_error_into_system_error
true, // demote_program_write_locks
); );
assert_eq!(collected_accounts.len(), 2); assert_eq!(collected_accounts.len(), 2);
assert_eq!( assert_eq!(
@ -2512,8 +2554,8 @@ mod tests {
&rent_collector, &rent_collector,
&(next_blockhash, FeeCalculator::default()), &(next_blockhash, FeeCalculator::default()),
true, true,
true, // demote_sysvar_write_locks
true, // merge_nonce_error_into_system_error true, // merge_nonce_error_into_system_error
true, // demote_program_write_locks
); );
assert_eq!(collected_accounts.len(), 1); assert_eq!(collected_accounts.len(), 1);
let collected_nonce_account = collected_accounts let collected_nonce_account = collected_accounts

29
runtime/src/bank.rs
View File

@ -2468,11 +2468,6 @@ impl Bank {
tick_height % self.ticks_per_slot == 0 tick_height % self.ticks_per_slot == 0
} }
pub fn demote_sysvar_write_locks(&self) -> bool {
self.feature_set
.is_active(&feature_set::demote_sysvar_write_locks::id())
}
pub fn prepare_batch<'a, 'b>( pub fn prepare_batch<'a, 'b>(
&'a self, &'a self,
txs: impl Iterator<Item = &'b Transaction>, txs: impl Iterator<Item = &'b Transaction>,
@ -2480,9 +2475,9 @@ impl Bank {
let hashed_txs: Vec<HashedTransaction> = txs.map(HashedTransaction::from).collect(); let hashed_txs: Vec<HashedTransaction> = txs.map(HashedTransaction::from).collect();
let lock_results = self.rc.accounts.lock_accounts( let lock_results = self.rc.accounts.lock_accounts(
hashed_txs.as_transactions_iter(), hashed_txs.as_transactions_iter(),
self.demote_sysvar_write_locks(), self.demote_program_write_locks(),
); );
TransactionBatch::new(lock_results, &self, Cow::Owned(hashed_txs)) TransactionBatch::new(lock_results, self, Cow::Owned(hashed_txs))
} }
pub fn prepare_hashed_batch<'a, 'b>( pub fn prepare_hashed_batch<'a, 'b>(
@ -2491,9 +2486,9 @@ impl Bank {
) -> TransactionBatch<'a, 'b> { ) -> TransactionBatch<'a, 'b> {
let lock_results = self.rc.accounts.lock_accounts( let lock_results = self.rc.accounts.lock_accounts(
hashed_txs.as_transactions_iter(), hashed_txs.as_transactions_iter(),
self.demote_sysvar_write_locks(), self.demote_program_write_locks(),
); );
TransactionBatch::new(lock_results, &self, Cow::Borrowed(hashed_txs)) TransactionBatch::new(lock_results, self, Cow::Borrowed(hashed_txs))
} }
pub(crate) fn prepare_simulation_batch<'a, 'b>( pub(crate) fn prepare_simulation_batch<'a, 'b>(
@ -2563,7 +2558,7 @@ impl Bank {
self.rc.accounts.unlock_accounts( self.rc.accounts.unlock_accounts(
batch.transactions_iter(), batch.transactions_iter(),
batch.lock_results(), batch.lock_results(),
self.demote_sysvar_write_locks(), self.demote_program_write_locks(),
) )
} }
} }
@ -3303,8 +3298,8 @@ impl Bank {
&self.rent_collector, &self.rent_collector,
&self.last_blockhash_with_fee_calculator(), &self.last_blockhash_with_fee_calculator(),
self.fix_recent_blockhashes_sysvar_delay(), self.fix_recent_blockhashes_sysvar_delay(),
self.demote_sysvar_write_locks(),
self.merge_nonce_error_into_system_error(), self.merge_nonce_error_into_system_error(),
self.demote_program_write_locks(),
); );
let rent_debits = self.collect_rent(executed, loaded_accounts); let rent_debits = self.collect_rent(executed, loaded_accounts);
@ -4853,6 +4848,11 @@ impl Bank {
.is_active(&feature_set::merge_nonce_error_into_system_error::id()) .is_active(&feature_set::merge_nonce_error_into_system_error::id())
} }
pub fn demote_program_write_locks(&self) -> bool {
self.feature_set
.is_active(&feature_set::demote_program_write_locks::id())
}
// Check if the wallclock time from bank creation to now has exceeded the allotted // Check if the wallclock time from bank creation to now has exceeded the allotted
// time for transaction processing // time for transaction processing
pub fn should_bank_still_be_processing_txs( pub fn should_bank_still_be_processing_txs(
@ -7502,13 +7502,14 @@ pub(crate) mod tests {
assert_eq!(bank.get_balance(&sysvar_pubkey), 1); assert_eq!(bank.get_balance(&sysvar_pubkey), 1);
bank.transfer(500, &mint_keypair, &normal_pubkey).unwrap(); bank.transfer(500, &mint_keypair, &normal_pubkey).unwrap();
bank.transfer(500, &mint_keypair, &sysvar_pubkey).unwrap(); bank.transfer(500, &mint_keypair, &sysvar_pubkey)
.unwrap_err();
assert_eq!(bank.get_balance(&normal_pubkey), 500); assert_eq!(bank.get_balance(&normal_pubkey), 500);
assert_eq!(bank.get_balance(&sysvar_pubkey), 501); assert_eq!(bank.get_balance(&sysvar_pubkey), 1);
let bank = Arc::new(new_from_parent(&bank)); let bank = Arc::new(new_from_parent(&bank));
assert_eq!(bank.get_balance(&normal_pubkey), 500); assert_eq!(bank.get_balance(&normal_pubkey), 500);
assert_eq!(bank.get_balance(&sysvar_pubkey), 501); assert_eq!(bank.get_balance(&sysvar_pubkey), 1);
} }
#[test] #[test]

49
runtime/src/message_processor.rs
View File

@ -9,7 +9,7 @@ use solana_sdk::{
account_utils::StateMut, account_utils::StateMut,
bpf_loader_upgradeable::{self, UpgradeableLoaderState}, bpf_loader_upgradeable::{self, UpgradeableLoaderState},
feature_set::{ feature_set::{
cpi_share_ro_and_exec_accounts, demote_sysvar_write_locks, instructions_sysvar_enabled, cpi_share_ro_and_exec_accounts, demote_program_write_locks, instructions_sysvar_enabled,
updated_verify_policy, FeatureSet, updated_verify_policy, FeatureSet,
}, },
ic_msg, ic_msg,
@ -359,7 +359,8 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
&self.rent, &self.rent,
caller_write_privileges, caller_write_privileges,
&mut self.timings, &mut self.timings,
self.feature_set.is_active(&demote_sysvar_write_locks::id()), self.feature_set
.is_active(&demote_program_write_locks::id()),
self.feature_set.is_active(&updated_verify_policy::id()), self.feature_set.is_active(&updated_verify_policy::id()),
), ),
None => Err(InstructionError::GenericError), // Should never happen None => Err(InstructionError::GenericError), // Should never happen
@ -573,7 +574,7 @@ impl MessageProcessor {
instruction: &'a CompiledInstruction, instruction: &'a CompiledInstruction,
executable_accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)], executable_accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)],
accounts: &'a [Rc<RefCell<AccountSharedData>>], accounts: &'a [Rc<RefCell<AccountSharedData>>],
demote_sysvar_write_locks: bool, demote_program_write_locks: bool,
) -> Vec<KeyedAccount<'a>> { ) -> Vec<KeyedAccount<'a>> {
let mut keyed_accounts = create_keyed_readonly_accounts(&executable_accounts); let mut keyed_accounts = create_keyed_readonly_accounts(&executable_accounts);
let mut keyed_accounts2: Vec<_> = instruction let mut keyed_accounts2: Vec<_> = instruction
@ -584,7 +585,7 @@ impl MessageProcessor {
let index = index as usize; let index = index as usize;
let key = &message.account_keys[index]; let key = &message.account_keys[index];
let account = &accounts[index]; let account = &accounts[index];
if message.is_writable(index, demote_sysvar_write_locks) { if message.is_writable(index, demote_program_write_locks) {
KeyedAccount::new(key, is_signer, account) KeyedAccount::new(key, is_signer, account)
} else { } else {
KeyedAccount::new_readonly(key, is_signer, account) KeyedAccount::new_readonly(key, is_signer, account)
@ -735,7 +736,7 @@ impl MessageProcessor {
accounts, accounts,
account_refs, account_refs,
caller_write_privileges, caller_write_privileges,
demote_sysvar_write_locks, demote_program_write_locks,
) = { ) = {
let invoke_context = invoke_context.borrow(); let invoke_context = invoke_context.borrow();
@ -828,7 +829,7 @@ impl MessageProcessor {
accounts, accounts,
account_refs, account_refs,
caller_write_privileges, caller_write_privileges,
invoke_context.is_feature_active(&demote_sysvar_write_locks::id()), invoke_context.is_feature_active(&demote_program_write_locks::id()),
) )
}; };
@ -847,7 +848,7 @@ impl MessageProcessor {
let invoke_context = invoke_context.borrow(); let invoke_context = invoke_context.borrow();
for (i, (account, account_ref)) in accounts.iter().zip(account_refs).enumerate() { for (i, (account, account_ref)) in accounts.iter().zip(account_refs).enumerate() {
let account = account.borrow(); let account = account.borrow();
if message.is_writable(i, demote_sysvar_write_locks) && !account.executable { if message.is_writable(i, demote_program_write_locks) && !account.executable {
account_ref.try_account_ref_mut()?.lamports = account.lamports; account_ref.try_account_ref_mut()?.lamports = account.lamports;
account_ref.try_account_ref_mut()?.owner = account.owner; account_ref.try_account_ref_mut()?.owner = account.owner;
if account_ref.data_len()? != account.data().len() if account_ref.data_len()? != account.data().len()
@ -890,15 +891,15 @@ impl MessageProcessor {
accounts, accounts,
Some(caller_write_privileges), Some(caller_write_privileges),
)?; )?;
let demote_sysvar_write_locks =
invoke_context.is_feature_active(&demote_sysvar_write_locks::id());
// Construct keyed accounts // Construct keyed accounts
let demote_program_write_locks =
invoke_context.is_feature_active(&demote_program_write_locks::id());
let keyed_accounts = Self::create_keyed_accounts( let keyed_accounts = Self::create_keyed_accounts(
message, message,
instruction, instruction,
executable_accounts, executable_accounts,
accounts, accounts,
demote_sysvar_write_locks, demote_program_write_locks,
); );
// Invoke callee // Invoke callee
@ -961,6 +962,7 @@ impl MessageProcessor {
} }
/// Verify the results of an instruction /// Verify the results of an instruction
#[allow(clippy::too_many_arguments)]
pub fn verify( pub fn verify(
message: &Message, message: &Message,
instruction: &CompiledInstruction, instruction: &CompiledInstruction,
@ -969,8 +971,8 @@ impl MessageProcessor {
accounts: &[Rc<RefCell<AccountSharedData>>], accounts: &[Rc<RefCell<AccountSharedData>>],
rent: &Rent, rent: &Rent,
timings: &mut ExecuteDetailsTimings, timings: &mut ExecuteDetailsTimings,
demote_sysvar_write_locks: bool,
updated_verify_policy: bool, updated_verify_policy: bool,
demote_program_write_locks: bool,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
// Verify all executable accounts have zero outstanding refs // Verify all executable accounts have zero outstanding refs
Self::verify_account_references(executable_accounts)?; Self::verify_account_references(executable_accounts)?;
@ -989,7 +991,7 @@ impl MessageProcessor {
let account = accounts[account_index].borrow(); let account = accounts[account_index].borrow();
pre_accounts[unique_index].verify( pre_accounts[unique_index].verify(
&program_id, &program_id,
message.is_writable(account_index, demote_sysvar_write_locks), message.is_writable(account_index, demote_program_write_locks),
rent, rent,
&account, &account,
timings, timings,
@ -1020,7 +1022,7 @@ impl MessageProcessor {
rent: &Rent, rent: &Rent,
caller_write_privileges: Option<&[bool]>, caller_write_privileges: Option<&[bool]>,
timings: &mut ExecuteDetailsTimings, timings: &mut ExecuteDetailsTimings,
demote_sysvar_write_locks: bool, demote_program_write_locks: bool,
updated_verify_policy: bool, updated_verify_policy: bool,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
// Verify the per-account instruction results // Verify the per-account instruction results
@ -1032,7 +1034,7 @@ impl MessageProcessor {
let is_writable = if let Some(caller_write_privileges) = caller_write_privileges { let is_writable = if let Some(caller_write_privileges) = caller_write_privileges {
caller_write_privileges[account_index] caller_write_privileges[account_index]
} else { } else {
message.is_writable(account_index, demote_sysvar_write_locks) message.is_writable(account_index, demote_program_write_locks)
}; };
// Find the matching PreAccount // Find the matching PreAccount
for pre_account in pre_accounts.iter_mut() { for pre_account in pre_accounts.iter_mut() {
@ -1093,7 +1095,6 @@ impl MessageProcessor {
feature_set: Arc<FeatureSet>, feature_set: Arc<FeatureSet>,
bpf_compute_budget: BpfComputeBudget, bpf_compute_budget: BpfComputeBudget,
timings: &mut ExecuteDetailsTimings, timings: &mut ExecuteDetailsTimings,
demote_sysvar_write_locks: bool,
account_db: Arc<Accounts>, account_db: Arc<Accounts>,
ancestors: &Ancestors, ancestors: &Ancestors,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
@ -1129,12 +1130,14 @@ impl MessageProcessor {
account_db, account_db,
ancestors, ancestors,
); );
let demote_program_write_locks =
invoke_context.is_feature_active(&demote_program_write_locks::id());
let keyed_accounts = Self::create_keyed_accounts( let keyed_accounts = Self::create_keyed_accounts(
message, message,
instruction, instruction,
executable_accounts, executable_accounts,
accounts, accounts,
demote_sysvar_write_locks, demote_program_write_locks,
); );
self.process_instruction( self.process_instruction(
program_id, program_id,
@ -1150,8 +1153,8 @@ impl MessageProcessor {
accounts, accounts,
&rent_collector.rent, &rent_collector.rent,
timings, timings,
demote_sysvar_write_locks,
invoke_context.is_feature_active(&updated_verify_policy::id()), invoke_context.is_feature_active(&updated_verify_policy::id()),
demote_program_write_locks,
)?; )?;
timings.accumulate(&invoke_context.timings); timings.accumulate(&invoke_context.timings);
@ -1180,7 +1183,6 @@ impl MessageProcessor {
account_db: Arc<Accounts>, account_db: Arc<Accounts>,
ancestors: &Ancestors, ancestors: &Ancestors,
) -> Result<(), TransactionError> { ) -> Result<(), TransactionError> {
let demote_sysvar_write_locks = feature_set.is_active(&demote_sysvar_write_locks::id());
for (instruction_index, instruction) in message.instructions.iter().enumerate() { for (instruction_index, instruction) in message.instructions.iter().enumerate() {
let instruction_recorder = instruction_recorders let instruction_recorder = instruction_recorders
.as_ref() .as_ref()
@ -1199,7 +1201,6 @@ impl MessageProcessor {
feature_set.clone(), feature_set.clone(),
bpf_compute_budget, bpf_compute_budget,
timings, timings,
demote_sysvar_write_locks,
account_db.clone(), account_db.clone(),
ancestors, ancestors,
) )
@ -2168,6 +2169,9 @@ mod tests {
]; ];
let programs: Vec<(_, ProcessInstructionWithContext)> = let programs: Vec<(_, ProcessInstructionWithContext)> =
vec![(callee_program_id, mock_process_instruction)]; vec![(callee_program_id, mock_process_instruction)];
let feature_set = FeatureSet::all_enabled();
let demote_program_write_locks = feature_set.is_active(&demote_program_write_locks::id());
let ancestors = Ancestors::default(); let ancestors = Ancestors::default();
let mut invoke_context = ThisInvokeContext::new( let mut invoke_context = ThisInvokeContext::new(
&caller_program_id, &caller_program_id,
@ -2184,7 +2188,7 @@ mod tests {
BpfComputeBudget::default(), BpfComputeBudget::default(),
Rc::new(RefCell::new(Executors::default())), Rc::new(RefCell::new(Executors::default())),
None, None,
Arc::new(FeatureSet::all_enabled()), Arc::new(feature_set),
Arc::new(Accounts::default()), Arc::new(Accounts::default()),
&ancestors, &ancestors,
); );
@ -2200,13 +2204,12 @@ mod tests {
&MockInstruction::NoopSuccess, &MockInstruction::NoopSuccess,
metas.clone(), metas.clone(),
); );
let demote_sysvar_write_locks = true;
let message = Message::new(&[instruction], None); let message = Message::new(&[instruction], None);
let caller_write_privileges = message let caller_write_privileges = message
.account_keys .account_keys
.iter() .iter()
.enumerate() .enumerate()
.map(|(i, _)| message.is_writable(i, demote_sysvar_write_locks)) .map(|(i, _)| message.is_writable(i, demote_program_write_locks))
.collect::<Vec<bool>>(); .collect::<Vec<bool>>();
assert_eq!( assert_eq!(
MessageProcessor::process_cross_program_instruction( MessageProcessor::process_cross_program_instruction(
@ -2241,7 +2244,7 @@ mod tests {
.account_keys .account_keys
.iter() .iter()
.enumerate() .enumerate()
.map(|(i, _)| message.is_writable(i, demote_sysvar_write_locks)) .map(|(i, _)| message.is_writable(i, demote_program_write_locks))
.collect::<Vec<bool>>(); .collect::<Vec<bool>>();
assert_eq!( assert_eq!(
MessageProcessor::process_cross_program_instruction( MessageProcessor::process_cross_program_instruction(

14
sdk/benches/serialize_instructions.rs
View File

@ -14,6 +14,8 @@ fn make_instructions() -> Vec<Instruction> {
vec![inst; 4] vec![inst; 4]
} }
const DEMOTE_PROGRAM_WRITE_LOCKS: bool = true;
#[bench] #[bench]
fn bench_bincode_instruction_serialize(b: &mut Bencher) { fn bench_bincode_instruction_serialize(b: &mut Bencher) {
let instructions = make_instructions(); let instructions = make_instructions();
@ -27,9 +29,7 @@ fn bench_manual_instruction_serialize(b: &mut Bencher) {
let instructions = make_instructions(); let instructions = make_instructions();
let message = Message::new(&instructions, None); let message = Message::new(&instructions, None);
b.iter(|| { b.iter(|| {
test::black_box(message.serialize_instructions( test::black_box(message.serialize_instructions(DEMOTE_PROGRAM_WRITE_LOCKS));
true, // demote_sysvar_write_locks
));
}); });
} }
@ -46,9 +46,7 @@ fn bench_bincode_instruction_deserialize(b: &mut Bencher) {
fn bench_manual_instruction_deserialize(b: &mut Bencher) { fn bench_manual_instruction_deserialize(b: &mut Bencher) {
let instructions = make_instructions(); let instructions = make_instructions();
let message = Message::new(&instructions, None); let message = Message::new(&instructions, None);
let serialized = message.serialize_instructions( let serialized = message.serialize_instructions(DEMOTE_PROGRAM_WRITE_LOCKS);
true, // demote_sysvar_write_locks
);
b.iter(|| { b.iter(|| {
for i in 0..instructions.len() { for i in 0..instructions.len() {
test::black_box(instructions::load_instruction_at(i, &serialized).unwrap()); test::black_box(instructions::load_instruction_at(i, &serialized).unwrap());
@ -60,9 +58,7 @@ fn bench_manual_instruction_deserialize(b: &mut Bencher) {
fn bench_manual_instruction_deserialize_single(b: &mut Bencher) { fn bench_manual_instruction_deserialize_single(b: &mut Bencher) {
let instructions = make_instructions(); let instructions = make_instructions();
let message = Message::new(&instructions, None); let message = Message::new(&instructions, None);
let serialized = message.serialize_instructions( let serialized = message.serialize_instructions(DEMOTE_PROGRAM_WRITE_LOCKS);
true, // demote_sysvar_write_locks
);
b.iter(|| { b.iter(|| {
test::black_box(instructions::load_instruction_at(3, &serialized).unwrap()); test::black_box(instructions::load_instruction_at(3, &serialized).unwrap());
}); });

50
sdk/program/src/message.rs
View File

@ -337,6 +337,16 @@ impl Message {
.collect() .collect()
} }
pub fn is_key_called_as_program(&self, key_index: usize) -> bool {
if let Ok(key_index) = u8::try_from(key_index) {
self.instructions
.iter()
.any(|ix| ix.program_id_index == key_index)
} else {
false
}
}
pub fn is_key_passed_to_program(&self, index: usize) -> bool { pub fn is_key_passed_to_program(&self, index: usize) -> bool {
if let Ok(index) = u8::try_from(index) { if let Ok(index) = u8::try_from(index) {
for ix in self.instructions.iter() { for ix in self.instructions.iter() {
@ -363,7 +373,7 @@ impl Message {
self.program_position(i).is_some() self.program_position(i).is_some()
} }
pub fn is_writable(&self, i: usize, demote_sysvar_write_locks: bool) -> bool { pub fn is_writable(&self, i: usize, demote_program_write_locks: bool) -> bool {
(i < (self.header.num_required_signatures - self.header.num_readonly_signed_accounts) (i < (self.header.num_required_signatures - self.header.num_readonly_signed_accounts)
as usize as usize
|| (i >= self.header.num_required_signatures as usize || (i >= self.header.num_required_signatures as usize
@ -371,9 +381,9 @@ impl Message {
- self.header.num_readonly_unsigned_accounts as usize)) - self.header.num_readonly_unsigned_accounts as usize))
&& !{ && !{
let key = self.account_keys[i]; let key = self.account_keys[i];
demote_sysvar_write_locks sysvar::is_sysvar_id(&key) || BUILTIN_PROGRAMS_KEYS.contains(&key)
&& (sysvar::is_sysvar_id(&key) || BUILTIN_PROGRAMS_KEYS.contains(&key))
} }
&& !(demote_program_write_locks && self.is_key_called_as_program(i))
} }
pub fn is_signer(&self, i: usize) -> bool { pub fn is_signer(&self, i: usize) -> bool {
@ -382,12 +392,12 @@ impl Message {
pub fn get_account_keys_by_lock_type( pub fn get_account_keys_by_lock_type(
&self, &self,
demote_sysvar_write_locks: bool, demote_program_write_locks: bool,
) -> (Vec<&Pubkey>, Vec<&Pubkey>) { ) -> (Vec<&Pubkey>, Vec<&Pubkey>) {
let mut writable_keys = vec![]; let mut writable_keys = vec![];
let mut readonly_keys = vec![]; let mut readonly_keys = vec![];
for (i, key) in self.account_keys.iter().enumerate() { for (i, key) in self.account_keys.iter().enumerate() {
if self.is_writable(i, demote_sysvar_write_locks) { if self.is_writable(i, demote_program_write_locks) {
writable_keys.push(key); writable_keys.push(key);
} else { } else {
readonly_keys.push(key); readonly_keys.push(key);
@ -409,7 +419,7 @@ impl Message {
// 35..67 - program_id // 35..67 - program_id
// 67..69 - data len - u16 // 67..69 - data len - u16
// 69..data_len - data // 69..data_len - data
pub fn serialize_instructions(&self, demote_sysvar_write_locks: bool) -> Vec<u8> { pub fn serialize_instructions(&self, demote_program_write_locks: bool) -> Vec<u8> {
// 64 bytes is a reasonable guess, calculating exactly is slower in benchmarks // 64 bytes is a reasonable guess, calculating exactly is slower in benchmarks
let mut data = Vec::with_capacity(self.instructions.len() * (32 * 2)); let mut data = Vec::with_capacity(self.instructions.len() * (32 * 2));
append_u16(&mut data, self.instructions.len() as u16); append_u16(&mut data, self.instructions.len() as u16);
@ -424,7 +434,7 @@ impl Message {
for account_index in &instruction.accounts { for account_index in &instruction.accounts {
let account_index = *account_index as usize; let account_index = *account_index as usize;
let is_signer = self.is_signer(account_index); let is_signer = self.is_signer(account_index);
let is_writable = self.is_writable(account_index, demote_sysvar_write_locks); let is_writable = self.is_writable(account_index, demote_program_write_locks);
let mut meta_byte = 0; let mut meta_byte = 0;
if is_signer { if is_signer {
meta_byte |= 1 << Self::IS_SIGNER_BIT; meta_byte |= 1 << Self::IS_SIGNER_BIT;
@ -863,13 +873,13 @@ mod tests {
recent_blockhash: Hash::default(), recent_blockhash: Hash::default(),
instructions: vec![], instructions: vec![],
}; };
let demote_sysvar_write_locks = true; let demote_program_write_locks = true;
assert_eq!(message.is_writable(0, demote_sysvar_write_locks), true); assert!(message.is_writable(0, demote_program_write_locks));
assert_eq!(message.is_writable(1, demote_sysvar_write_locks), false); assert!(!message.is_writable(1, demote_program_write_locks));
assert_eq!(message.is_writable(2, demote_sysvar_write_locks), false); assert!(!message.is_writable(2, demote_program_write_locks));
assert_eq!(message.is_writable(3, demote_sysvar_write_locks), true); assert!(message.is_writable(3, demote_program_write_locks));
assert_eq!(message.is_writable(4, demote_sysvar_write_locks), true); assert!(message.is_writable(4, demote_program_write_locks));
assert_eq!(message.is_writable(5, demote_sysvar_write_locks), false); assert!(!message.is_writable(5, demote_program_write_locks));
} }
#[test] #[test]
@ -897,9 +907,7 @@ mod tests {
Some(&id1), Some(&id1),
); );
assert_eq!( assert_eq!(
message.get_account_keys_by_lock_type( message.get_account_keys_by_lock_type(/*demote_program_write_locks=*/ true),
true, // demote_sysvar_write_locks
),
(vec![&id1, &id0], vec![&id3, &id2, &program_id]) (vec![&id1, &id0], vec![&id3, &id2, &program_id])
); );
} }
@ -929,9 +937,7 @@ mod tests {
]; ];
let message = Message::new(&instructions, Some(&id1)); let message = Message::new(&instructions, Some(&id1));
let serialized = message.serialize_instructions( let serialized = message.serialize_instructions(/*demote_program_write_locks=*/ true);
true, // demote_sysvar_write_locks
);
for (i, instruction) in instructions.iter().enumerate() { for (i, instruction) in instructions.iter().enumerate() {
assert_eq!( assert_eq!(
Message::deserialize_instruction(i, &serialized).unwrap(), Message::deserialize_instruction(i, &serialized).unwrap(),
@ -952,9 +958,7 @@ mod tests {
]; ];
let message = Message::new(&instructions, Some(&id1)); let message = Message::new(&instructions, Some(&id1));
let serialized = message.serialize_instructions( let serialized = message.serialize_instructions(/*demote_program_write_locks=*/ true);
true, // demote_sysvar_write_locks
);
assert_eq!( assert_eq!(
Message::deserialize_instruction(instructions.len(), &serialized).unwrap_err(), Message::deserialize_instruction(instructions.len(), &serialized).unwrap_err(),
SanitizeError::IndexOutOfBounds, SanitizeError::IndexOutOfBounds,

10
sdk/src/feature_set.rs
View File

@ -107,10 +107,6 @@ pub mod upgradeable_close_instruction {
solana_sdk::declare_id!("FsPaByos3gA9bUEhp3EimQpQPCoSvCEigHod496NmABQ"); solana_sdk::declare_id!("FsPaByos3gA9bUEhp3EimQpQPCoSvCEigHod496NmABQ");
} }
pub mod demote_sysvar_write_locks {
solana_sdk::declare_id!("86LJYRuq2zgtHuL3FccR6hqFJQMQkFoun4knAxcPiF1P");
}
pub mod sysvar_via_syscall { pub mod sysvar_via_syscall {
solana_sdk::declare_id!("7411E6gFQLDhQkdRjmpXwM1hzHMMoYQUjHicmvGPC1Nf"); solana_sdk::declare_id!("7411E6gFQLDhQkdRjmpXwM1hzHMMoYQUjHicmvGPC1Nf");
} }
@ -170,6 +166,10 @@ pub mod spl_token_v2_set_authority_fix {
solana_sdk::declare_id!("FToKNBYyiF4ky9s8WsmLBXHCht17Ek7RXaLZGHzzQhJ1"); solana_sdk::declare_id!("FToKNBYyiF4ky9s8WsmLBXHCht17Ek7RXaLZGHzzQhJ1");
} }
pub mod demote_program_write_locks {
solana_sdk::declare_id!("3E3jV7v9VcdJL8iYZUMax9DiDno8j7EWUVbhm9RtShj2");
}
lazy_static! { lazy_static! {
/// Map of feature identifiers to user-visible description /// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [ pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -195,7 +195,6 @@ lazy_static! {
(require_stake_for_gossip::id(), "require stakes for propagating crds values through gossip #15561"), (require_stake_for_gossip::id(), "require stakes for propagating crds values through gossip #15561"),
(cpi_data_cost::id(), "charge the compute budget for data passed via CPI"), (cpi_data_cost::id(), "charge the compute budget for data passed via CPI"),
(upgradeable_close_instruction::id(), "close upgradeable buffer accounts"), (upgradeable_close_instruction::id(), "close upgradeable buffer accounts"),
(demote_sysvar_write_locks::id(), "demote builtins and sysvar write locks to readonly #15497"),
(sysvar_via_syscall::id(), "provide sysvars via syscalls"), (sysvar_via_syscall::id(), "provide sysvars via syscalls"),
(check_duplicates_by_hash::id(), "use transaction message hash for duplicate check"), (check_duplicates_by_hash::id(), "use transaction message hash for duplicate check"),
(enforce_aligned_host_addrs::id(), "enforce aligned host addresses"), (enforce_aligned_host_addrs::id(), "enforce aligned host addresses"),
@ -211,6 +210,7 @@ lazy_static! {
(libsecp256k1_0_5_upgrade_enabled::id(), "upgrade libsecp256k1 to v0.5.0"), (libsecp256k1_0_5_upgrade_enabled::id(), "upgrade libsecp256k1 to v0.5.0"),
(merge_nonce_error_into_system_error::id(), "merge NonceError into SystemError"), (merge_nonce_error_into_system_error::id(), "merge NonceError into SystemError"),
(spl_token_v2_set_authority_fix::id(), "spl-token set_authority fix"), (spl_token_v2_set_authority_fix::id(), "spl-token set_authority fix"),
(demote_program_write_locks::id(), "demote program write locks to readonly #19593"),
/*************** ADD NEW FEATURES HERE ***************/ /*************** ADD NEW FEATURES HERE ***************/
] ]
.iter() .iter()

2
transaction-status/src/parse_accounts.rs
View File

@ -13,7 +13,7 @@ pub fn parse_accounts(message: &Message) -> Vec<ParsedAccount> {
for (i, account_key) in message.account_keys.iter().enumerate() { for (i, account_key) in message.account_keys.iter().enumerate() {
accounts.push(ParsedAccount { accounts.push(ParsedAccount {
pubkey: account_key.to_string(), pubkey: account_key.to_string(),
writable: message.is_writable(i, /*demote_sysvar_write_locks=*/ true), writable: message.is_writable(i, /*demote_program_write_locks=*/ true),
signer: message.is_signer(i), signer: message.is_signer(i),
}); });
} }