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
This commit is contained in:
Tyera Eulberg 2021-09-03 21:05:30 -06:00 committed by GitHub
parent 7578db7ee3
commit decec3cd8b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 297 additions and 177 deletions

View File

@ -140,7 +140,7 @@ fn format_account_mode(message: &Message, index: usize) -> String {
} else { } 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) "w" // comment for consistent rust fmt (no joking; lol)
} else { } else {
"-" "-"

View File

@ -1049,6 +1049,7 @@ impl BankingStage {
libsecp256k1_fail_on_bad_count: bool, libsecp256k1_fail_on_bad_count: bool,
cost_tracker: &Arc<RwLock<CostTracker>>, cost_tracker: &Arc<RwLock<CostTracker>>,
banking_stage_stats: &BankingStageStats, banking_stage_stats: &BankingStageStats,
demote_program_write_locks: bool,
) -> (Vec<SanitizedTransaction>, Vec<usize>, Vec<usize>) { ) -> (Vec<SanitizedTransaction>, Vec<usize>, Vec<usize>) {
let mut retryable_transaction_packet_indexes: Vec<usize> = vec![]; let mut retryable_transaction_packet_indexes: Vec<usize> = vec![];
@ -1082,7 +1083,8 @@ impl BankingStage {
verified_transactions_with_packet_indexes verified_transactions_with_packet_indexes
.into_iter() .into_iter()
.filter_map(|(tx, tx_index)| { .filter_map(|(tx, tx_index)| {
let result = cost_tracker_readonly.would_transaction_fit(&tx); let result = cost_tracker_readonly
.would_transaction_fit(&tx, demote_program_write_locks);
if result.is_err() { if result.is_err() {
debug!("transaction {:?} would exceed limit: {:?}", tx, result); debug!("transaction {:?} would exceed limit: {:?}", tx, result);
retryable_transaction_packet_indexes.push(tx_index); retryable_transaction_packet_indexes.push(tx_index);
@ -1165,6 +1167,7 @@ impl BankingStage {
bank.libsecp256k1_fail_on_bad_count(), bank.libsecp256k1_fail_on_bad_count(),
cost_tracker, cost_tracker,
banking_stage_stats, banking_stage_stats,
bank.demote_program_write_locks(),
); );
packet_conversion_time.stop(); packet_conversion_time.stop();
inc_new_counter_info!("banking_stage-packet_conversion", 1); inc_new_counter_info!("banking_stage-packet_conversion", 1);
@ -1201,7 +1204,10 @@ impl BankingStage {
let mut cost_tracking_time = Measure::start("cost_tracking_time"); let mut cost_tracking_time = Measure::start("cost_tracking_time");
transactions.iter().enumerate().for_each(|(index, tx)| { transactions.iter().enumerate().for_each(|(index, tx)| {
if unprocessed_tx_indexes.iter().all(|&i| i != index) { if unprocessed_tx_indexes.iter().all(|&i| i != index) {
cost_tracker.write().unwrap().add_transaction_cost(tx); cost_tracker
.write()
.unwrap()
.add_transaction_cost(tx, bank.demote_program_write_locks());
} }
}); });
cost_tracking_time.stop(); cost_tracking_time.stop();
@ -1268,6 +1274,7 @@ impl BankingStage {
bank.libsecp256k1_fail_on_bad_count(), bank.libsecp256k1_fail_on_bad_count(),
cost_tracker, cost_tracker,
banking_stage_stats, banking_stage_stats,
bank.demote_program_write_locks(),
); );
unprocessed_packet_conversion_time.stop(); unprocessed_packet_conversion_time.stop();

View File

@ -113,7 +113,11 @@ impl CostModel {
); );
} }
pub fn calculate_cost(&mut self, transaction: &SanitizedTransaction) -> &TransactionCost { pub fn calculate_cost(
&mut self,
transaction: &SanitizedTransaction,
demote_program_write_locks: bool,
) -> &TransactionCost {
self.transaction_cost.reset(); self.transaction_cost.reset();
// calculate transaction exeution cost // calculate transaction exeution cost
@ -122,7 +126,7 @@ impl CostModel {
// calculate account access cost // calculate account access cost
let message = transaction.message(); let message = transaction.message();
message.account_keys_iter().enumerate().for_each(|(i, k)| { message.account_keys_iter().enumerate().for_each(|(i, k)| {
let is_writable = message.is_writable(i); let is_writable = message.is_writable(i, demote_program_write_locks);
if is_writable { if is_writable {
self.transaction_cost.writable_accounts.push(*k); self.transaction_cost.writable_accounts.push(*k);
@ -353,7 +357,7 @@ mod tests {
.unwrap(); .unwrap();
let mut cost_model = CostModel::default(); let mut cost_model = CostModel::default();
let tx_cost = cost_model.calculate_cost(&tx); let tx_cost = cost_model.calculate_cost(&tx, /*demote_program_write_locks=*/ true);
assert_eq!(2 + 2, tx_cost.writable_accounts.len()); assert_eq!(2 + 2, tx_cost.writable_accounts.len());
assert_eq!(signer1.pubkey(), tx_cost.writable_accounts[0]); assert_eq!(signer1.pubkey(), tx_cost.writable_accounts[0]);
assert_eq!(signer2.pubkey(), tx_cost.writable_accounts[1]); assert_eq!(signer2.pubkey(), tx_cost.writable_accounts[1]);
@ -395,7 +399,7 @@ mod tests {
cost_model cost_model
.upsert_instruction_cost(&system_program::id(), expected_execution_cost) .upsert_instruction_cost(&system_program::id(), expected_execution_cost)
.unwrap(); .unwrap();
let tx_cost = cost_model.calculate_cost(&tx); let tx_cost = cost_model.calculate_cost(&tx, /*demote_program_write_locks=*/ true);
assert_eq!(expected_account_cost, tx_cost.account_access_cost); assert_eq!(expected_account_cost, tx_cost.account_access_cost);
assert_eq!(expected_execution_cost, tx_cost.execution_cost); assert_eq!(expected_execution_cost, tx_cost.execution_cost);
assert_eq!(2, tx_cost.writable_accounts.len()); assert_eq!(2, tx_cost.writable_accounts.len());
@ -465,7 +469,8 @@ mod tests {
} else { } else {
thread::spawn(move || { thread::spawn(move || {
let mut cost_model = cost_model.write().unwrap(); let mut cost_model = cost_model.write().unwrap();
let tx_cost = cost_model.calculate_cost(&tx); let tx_cost = cost_model
.calculate_cost(&tx, /*demote_program_write_locks=*/ true);
assert_eq!(3, tx_cost.writable_accounts.len()); assert_eq!(3, tx_cost.writable_accounts.len());
assert_eq!(expected_account_cost, tx_cost.account_access_cost); assert_eq!(expected_account_cost, tx_cost.account_access_cost);
}) })

View File

@ -46,18 +46,23 @@ impl CostTracker {
pub fn would_transaction_fit( pub fn would_transaction_fit(
&self, &self,
transaction: &SanitizedTransaction, transaction: &SanitizedTransaction,
demote_program_write_locks: bool,
) -> Result<(), CostModelError> { ) -> Result<(), CostModelError> {
let mut cost_model = self.cost_model.write().unwrap(); let mut cost_model = self.cost_model.write().unwrap();
let tx_cost = cost_model.calculate_cost(transaction); let tx_cost = cost_model.calculate_cost(transaction, demote_program_write_locks);
self.would_fit( self.would_fit(
&tx_cost.writable_accounts, &tx_cost.writable_accounts,
&(tx_cost.account_access_cost + tx_cost.execution_cost), &(tx_cost.account_access_cost + tx_cost.execution_cost),
) )
} }
pub fn add_transaction_cost(&mut self, transaction: &SanitizedTransaction) { pub fn add_transaction_cost(
&mut self,
transaction: &SanitizedTransaction,
demote_program_write_locks: bool,
) {
let mut cost_model = self.cost_model.write().unwrap(); let mut cost_model = self.cost_model.write().unwrap();
let tx_cost = cost_model.calculate_cost(transaction); let tx_cost = cost_model.calculate_cost(transaction, demote_program_write_locks);
let cost = tx_cost.account_access_cost + tx_cost.execution_cost; let cost = tx_cost.account_access_cost + tx_cost.execution_cost;
for account_key in tx_cost.writable_accounts.iter() { for account_key in tx_cost.writable_accounts.iter() {
*self *self

View File

@ -792,7 +792,10 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String>
.for_each(|transaction| { .for_each(|transaction| {
num_programs += transaction.message().instructions().len(); num_programs += transaction.message().instructions().len();
let tx_cost = cost_model.calculate_cost(&transaction); let tx_cost = cost_model.calculate_cost(
&transaction,
true, // demote_program_write_locks
);
if cost_tracker.try_add(tx_cost).is_err() { if cost_tracker.try_add(tx_cost).is_err() {
println!( println!(
"Slot: {}, CostModel rejected transaction {:?}, stats {:?}!", "Slot: {}, CostModel rejected transaction {:?}, stats {:?}!",

View File

@ -4,6 +4,7 @@ use solana_sdk::{
account::{AccountSharedData, ReadableAccount, WritableAccount}, account::{AccountSharedData, ReadableAccount, WritableAccount},
account_utils::StateMut, account_utils::StateMut,
bpf_loader_upgradeable::{self, UpgradeableLoaderState}, bpf_loader_upgradeable::{self, UpgradeableLoaderState},
feature_set::demote_program_write_locks,
ic_logger_msg, ic_msg, ic_logger_msg, ic_msg,
instruction::{CompiledInstruction, Instruction, InstructionError}, instruction::{CompiledInstruction, Instruction, InstructionError},
keyed_account::{keyed_account_at_index, KeyedAccount}, keyed_account::{keyed_account_at_index, KeyedAccount},
@ -347,6 +348,7 @@ impl InstructionProcessor {
instruction: &'a CompiledInstruction, instruction: &'a CompiledInstruction,
executable_accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)], executable_accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)],
accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)], accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)],
demote_program_write_locks: bool,
) -> Vec<(bool, bool, &'a Pubkey, &'a RefCell<AccountSharedData>)> { ) -> Vec<(bool, bool, &'a Pubkey, &'a RefCell<AccountSharedData>)> {
executable_accounts executable_accounts
.iter() .iter()
@ -355,7 +357,7 @@ impl InstructionProcessor {
let index = *index as usize; let index = *index as usize;
( (
message.is_signer(index), message.is_signer(index),
message.is_writable(index), message.is_writable(index, demote_program_write_locks),
&accounts[index].0, &accounts[index].0,
&accounts[index].1 as &RefCell<AccountSharedData>, &accounts[index].1 as &RefCell<AccountSharedData>,
) )
@ -600,6 +602,8 @@ impl InstructionProcessor {
{ {
let invoke_context = invoke_context.borrow(); let invoke_context = invoke_context.borrow();
let demote_program_write_locks =
invoke_context.is_feature_active(&demote_program_write_locks::id());
let keyed_accounts = invoke_context.get_keyed_accounts()?; let keyed_accounts = invoke_context.get_keyed_accounts()?;
for (src_keyed_account_index, ((_key, account), dst_keyed_account_index)) in accounts for (src_keyed_account_index, ((_key, account), dst_keyed_account_index)) in accounts
.iter() .iter()
@ -608,7 +612,9 @@ impl InstructionProcessor {
{ {
let dst_keyed_account = &keyed_accounts[dst_keyed_account_index]; let dst_keyed_account = &keyed_accounts[dst_keyed_account_index];
let src_keyed_account = account.borrow(); let src_keyed_account = account.borrow();
if message.is_writable(src_keyed_account_index) && !src_keyed_account.executable() { if message.is_writable(src_keyed_account_index, demote_program_write_locks)
&& !src_keyed_account.executable()
{
if dst_keyed_account.data_len()? != src_keyed_account.data().len() if dst_keyed_account.data_len()? != src_keyed_account.data().len()
&& dst_keyed_account.data_len()? != 0 && dst_keyed_account.data_len()? != 0
{ {
@ -652,8 +658,15 @@ impl InstructionProcessor {
invoke_context.verify_and_update(instruction, accounts, caller_write_privileges)?; invoke_context.verify_and_update(instruction, accounts, caller_write_privileges)?;
// Construct keyed accounts // Construct keyed accounts
let keyed_accounts = let demote_program_write_locks =
Self::create_keyed_accounts(message, instruction, executable_accounts, accounts); invoke_context.is_feature_active(&demote_program_write_locks::id());
let keyed_accounts = Self::create_keyed_accounts(
message,
instruction,
executable_accounts,
accounts,
demote_program_write_locks,
);
// Invoke callee // Invoke callee
invoke_context.push(program_id, &keyed_accounts)?; invoke_context.push(program_id, &keyed_accounts)?;
@ -671,7 +684,7 @@ impl InstructionProcessor {
if result.is_ok() { if result.is_ok() {
// Verify the called program has not misbehaved // Verify the called program has not misbehaved
let write_privileges: Vec<bool> = (0..message.account_keys.len()) let write_privileges: Vec<bool> = (0..message.account_keys.len())
.map(|i| message.is_writable(i)) .map(|i| message.is_writable(i, demote_program_write_locks))
.collect(); .collect();
result = invoke_context.verify_and_update(instruction, accounts, &write_privileges); result = invoke_context.verify_and_update(instruction, accounts, &write_privileges);
} }

View File

@ -24,6 +24,7 @@ use {
compute_budget::ComputeBudget, compute_budget::ComputeBudget,
entrypoint::{ProgramResult, SUCCESS}, entrypoint::{ProgramResult, SUCCESS},
epoch_schedule::EpochSchedule, epoch_schedule::EpochSchedule,
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,
@ -261,12 +262,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_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 // 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)) .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());
@ -337,7 +340,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, (pubkey, account)) in accounts.iter().enumerate().take(message.account_keys.len()) { 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; continue;
} }
for account_info in account_infos { for account_info in account_infos {

View File

@ -1796,7 +1796,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(),
@ -1815,10 +1815,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)
); );
} }
@ -2114,96 +2116,6 @@ fn test_program_bpf_upgrade_via_cpi() {
assert_ne!(programdata, original_programdata); 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_for_tests(&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),
],
);
// 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() {

View File

@ -21,9 +21,9 @@ 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::{
blake3_syscall_enabled, close_upgradeable_program_accounts, disable_fees_sysvar, blake3_syscall_enabled, close_upgradeable_program_accounts, demote_program_write_locks,
enforce_aligned_host_addrs, libsecp256k1_0_5_upgrade_enabled, mem_overlap_fix, disable_fees_sysvar, enforce_aligned_host_addrs, libsecp256k1_0_5_upgrade_enabled,
secp256k1_recover_syscall_enabled, mem_overlap_fix, secp256k1_recover_syscall_enabled,
}, },
hash::{Hasher, HASH_BYTES}, hash::{Hasher, HASH_BYTES},
ic_msg, ic_msg,
@ -2293,7 +2293,14 @@ fn call<'a>(
signers_seeds_len: u64, signers_seeds_len: u64,
memory_mapping: &MemoryMapping, memory_mapping: &MemoryMapping,
) -> Result<u64, EbpfError<BpfError>> { ) -> 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()?; let invoke_context = syscall.get_context()?;
invoke_context invoke_context
@ -2388,6 +2395,7 @@ fn call<'a>(
accounts, accounts,
account_refs, account_refs,
caller_write_privileges, caller_write_privileges,
invoke_context.is_feature_active(&demote_program_write_locks::id()),
) )
}; };
@ -2413,7 +2421,7 @@ fn call<'a>(
for (i, ((_key, account), account_ref)) in accounts.iter().zip(account_refs).enumerate() { for (i, ((_key, 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) && !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() {

View File

@ -113,7 +113,8 @@ impl TransactionStatusService {
}) })
.expect("FeeCalculator must exist"); .expect("FeeCalculator must exist");
let fee = transaction.message().calculate_fee(&fee_calculator); let fee = transaction.message().calculate_fee(&fee_calculator);
let tx_account_locks = transaction.get_account_locks(); let tx_account_locks =
transaction.get_account_locks(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

View File

@ -201,8 +201,9 @@ impl Accounts {
fn construct_instructions_account( fn construct_instructions_account(
message: &SanitizedMessage, message: &SanitizedMessage,
is_owned_by_sysvar: bool, is_owned_by_sysvar: bool,
demote_program_write_locks: bool,
) -> AccountSharedData { ) -> AccountSharedData {
let mut data = message.serialize_instructions(); 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);
let owner = if is_owned_by_sysvar { let owner = if is_owned_by_sysvar {
@ -240,6 +241,8 @@ impl Accounts {
let mut account_deps = Vec::with_capacity(message.account_keys_len()); let mut account_deps = Vec::with_capacity(message.account_keys_len());
let mut rent_debits = RentDebits::default(); let mut rent_debits = RentDebits::default();
let rent_for_sysvars = feature_set.is_active(&feature_set::rent_for_sysvars::id()); let rent_for_sysvars = feature_set.is_active(&feature_set::rent_for_sysvars::id());
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(i) { let account = if message.is_non_loader_key(i) {
@ -248,20 +251,21 @@ impl Accounts {
} }
if solana_sdk::sysvar::instructions::check_id(key) { if solana_sdk::sysvar::instructions::check_id(key) {
if message.is_writable(i) { if message.is_writable(i, demote_program_write_locks) {
return Err(TransactionError::InvalidAccountIndex); return Err(TransactionError::InvalidAccountIndex);
} }
Self::construct_instructions_account( Self::construct_instructions_account(
message, message,
feature_set feature_set
.is_active(&feature_set::instructions_sysvar_owned_by_sysvar::id()), .is_active(&feature_set::instructions_sysvar_owned_by_sysvar::id()),
demote_program_write_locks,
) )
} else { } else {
let (account, rent) = self let (account, rent) = self
.accounts_db .accounts_db
.load_with_fixed_root(ancestors, key) .load_with_fixed_root(ancestors, key)
.map(|(mut account, _)| { .map(|(mut account, _)| {
if message.is_writable(i) { if message.is_writable(i, demote_program_write_locks) {
let rent_due = rent_collector.collect_from_existing_account( let rent_due = rent_collector.collect_from_existing_account(
key, key,
&mut account, &mut account,
@ -886,8 +890,11 @@ impl Accounts {
pub fn lock_accounts<'a>( pub fn lock_accounts<'a>(
&self, &self,
txs: impl Iterator<Item = &'a SanitizedTransaction>, txs: impl Iterator<Item = &'a SanitizedTransaction>,
demote_program_write_locks: bool,
) -> Vec<Result<()>> { ) -> Vec<Result<()>> {
let keys: Vec<_> = txs.map(|tx| tx.get_account_locks()).collect(); let keys: Vec<_> = txs
.map(|tx| tx.get_account_locks(demote_program_write_locks))
.collect();
let mut account_locks = &mut self.account_locks.lock().unwrap(); let mut account_locks = &mut self.account_locks.lock().unwrap();
keys.into_iter() keys.into_iter()
.map(|keys| self.lock_account(&mut account_locks, keys.writable, keys.readonly)) .map(|keys| self.lock_account(&mut account_locks, keys.writable, keys.readonly))
@ -900,6 +907,7 @@ impl Accounts {
&self, &self,
txs: impl Iterator<Item = &'a SanitizedTransaction>, txs: impl Iterator<Item = &'a SanitizedTransaction>,
results: &[Result<()>], results: &[Result<()>],
demote_program_write_locks: bool,
) { ) {
let keys: Vec<_> = txs let keys: Vec<_> = txs
.zip(results) .zip(results)
@ -907,7 +915,7 @@ impl Accounts {
Err(TransactionError::AccountInUse) => None, Err(TransactionError::AccountInUse) => None,
Err(TransactionError::SanitizeFailure) => None, Err(TransactionError::SanitizeFailure) => None,
Err(TransactionError::AccountLoadedTwice) => None, Err(TransactionError::AccountLoadedTwice) => None,
_ => Some(tx.get_account_locks()), _ => Some(tx.get_account_locks(demote_program_write_locks)),
}) })
.collect(); .collect();
let mut account_locks = self.account_locks.lock().unwrap(); let mut account_locks = self.account_locks.lock().unwrap();
@ -930,6 +938,7 @@ impl Accounts {
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
rent_for_sysvars: bool, rent_for_sysvars: 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,
@ -939,6 +948,7 @@ impl Accounts {
last_blockhash_with_fee_calculator, last_blockhash_with_fee_calculator,
rent_for_sysvars, rent_for_sysvars,
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);
} }
@ -964,6 +974,7 @@ impl Accounts {
last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
rent_for_sysvars: bool, rent_for_sysvars: 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() {
@ -1012,7 +1023,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) 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)))
{ {
@ -1731,6 +1742,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,
@ -1741,7 +1754,7 @@ mod tests {
instructions, instructions,
); );
let tx = new_sanitized_tx(&[&keypair0], message, Hash::default()); let tx = new_sanitized_tx(&[&keypair0], message, Hash::default());
let results0 = accounts.lock_accounts([tx.clone()].iter()); let results0 = accounts.lock_accounts([tx.clone()].iter(), demote_program_write_locks);
assert!(results0[0].is_ok()); assert!(results0[0].is_ok());
assert_eq!( assert_eq!(
@ -1776,7 +1789,7 @@ mod tests {
); );
let tx1 = new_sanitized_tx(&[&keypair1], message, Hash::default()); let tx1 = new_sanitized_tx(&[&keypair1], message, Hash::default());
let txs = vec![tx0, tx1]; let txs = vec![tx0, tx1];
let results1 = accounts.lock_accounts(txs.iter()); let results1 = accounts.lock_accounts(txs.iter(), demote_program_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
@ -1791,8 +1804,8 @@ mod tests {
2 2
); );
accounts.unlock_accounts([tx].iter(), &results0); accounts.unlock_accounts([tx].iter(), &results0, demote_program_write_locks);
accounts.unlock_accounts(txs.iter(), &results1); accounts.unlock_accounts(txs.iter(), &results1, demote_program_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,
@ -1803,7 +1816,7 @@ mod tests {
instructions, instructions,
); );
let tx = new_sanitized_tx(&[&keypair1], message, Hash::default()); let tx = new_sanitized_tx(&[&keypair1], message, Hash::default());
let results2 = accounts.lock_accounts([tx].iter()); let results2 = accounts.lock_accounts([tx].iter(), demote_program_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
@ -1840,6 +1853,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])];
@ -1872,13 +1887,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(txs.iter()); let results = accounts_clone
.clone()
.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(txs.iter(), &results); accounts_clone.unlock_accounts(txs.iter(), &results, demote_program_write_locks);
if exit_clone.clone().load(Ordering::Relaxed) { if exit_clone.clone().load(Ordering::Relaxed) {
break; break;
} }
@ -1887,18 +1904,85 @@ 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(txs.iter()); let results = accounts_arc
.clone()
.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(txs.iter(), &results); accounts_arc.unlock_accounts(txs.iter(), &results, demote_program_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_for_tests(
Vec::new(),
&ClusterType::Development,
AccountSecondaryIndexes::default(),
false,
AccountShrinkThreshold::default(),
);
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 = new_sanitized_tx(&[&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();
@ -1991,6 +2075,7 @@ mod tests {
&(Hash::default(), FeeCalculator::default()), &(Hash::default(), FeeCalculator::default()),
true, true,
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
@ -2370,6 +2455,7 @@ mod tests {
&(next_blockhash, FeeCalculator::default()), &(next_blockhash, FeeCalculator::default()),
true, true,
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!(
@ -2487,6 +2573,7 @@ mod tests {
&(next_blockhash, FeeCalculator::default()), &(next_blockhash, FeeCalculator::default()),
true, true,
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

View File

@ -2755,7 +2755,10 @@ impl Bank {
.into_iter() .into_iter()
.map(SanitizedTransaction::try_from) .map(SanitizedTransaction::try_from)
.collect::<Result<Vec<_>>>()?; .collect::<Result<Vec<_>>>()?;
let lock_results = self.rc.accounts.lock_accounts(sanitized_txs.iter()); let lock_results = self
.rc
.accounts
.lock_accounts(sanitized_txs.iter(), self.demote_program_write_locks());
Ok(TransactionBatch::new( Ok(TransactionBatch::new(
lock_results, lock_results,
self, self,
@ -2775,7 +2778,10 @@ impl Bank {
}) })
}) })
.collect::<Result<Vec<_>>>()?; .collect::<Result<Vec<_>>>()?;
let lock_results = self.rc.accounts.lock_accounts(sanitized_txs.iter()); let lock_results = self
.rc
.accounts
.lock_accounts(sanitized_txs.iter(), self.demote_program_write_locks());
Ok(TransactionBatch::new( Ok(TransactionBatch::new(
lock_results, lock_results,
self, self,
@ -2788,7 +2794,10 @@ impl Bank {
&'a self, &'a self,
txs: &'b [SanitizedTransaction], txs: &'b [SanitizedTransaction],
) -> TransactionBatch<'a, 'b> { ) -> TransactionBatch<'a, 'b> {
let lock_results = self.rc.accounts.lock_accounts(txs.iter()); let lock_results = self
.rc
.accounts
.lock_accounts(txs.iter(), self.demote_program_write_locks());
TransactionBatch::new(lock_results, self, Cow::Borrowed(txs)) TransactionBatch::new(lock_results, self, Cow::Borrowed(txs))
} }
@ -2863,9 +2872,11 @@ impl Bank {
pub fn unlock_accounts(&self, batch: &mut TransactionBatch) { pub fn unlock_accounts(&self, batch: &mut TransactionBatch) {
if batch.needs_unlock { if batch.needs_unlock {
batch.needs_unlock = false; batch.needs_unlock = false;
self.rc self.rc.accounts.unlock_accounts(
.accounts batch.sanitized_transactions().iter(),
.unlock_accounts(batch.sanitized_transactions().iter(), batch.lock_results()) batch.lock_results(),
self.demote_program_write_locks(),
)
} }
} }
@ -3624,6 +3635,7 @@ impl Bank {
&self.last_blockhash_with_fee_calculator(), &self.last_blockhash_with_fee_calculator(),
self.rent_for_sysvars(), self.rent_for_sysvars(),
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_txs); let rent_debits = self.collect_rent(executed, loaded_txs);
@ -5376,6 +5388,11 @@ impl Bank {
.is_active(&feature_set::stake_program_advance_activating_credits_observed::id()) .is_active(&feature_set::stake_program_advance_activating_credits_observed::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(

View File

@ -10,7 +10,8 @@ use solana_sdk::{
account::{AccountSharedData, ReadableAccount, WritableAccount}, account::{AccountSharedData, ReadableAccount, WritableAccount},
compute_budget::ComputeBudget, compute_budget::ComputeBudget,
feature_set::{ feature_set::{
neon_evm_compute_budget, tx_wide_compute_cap, updated_verify_policy, FeatureSet, demote_program_write_locks, neon_evm_compute_budget, tx_wide_compute_cap,
updated_verify_policy, FeatureSet,
}, },
fee_calculator::FeeCalculator, fee_calculator::FeeCalculator,
hash::Hash, hash::Hash,
@ -94,6 +95,7 @@ impl<'a> ThisInvokeContext<'a> {
instruction, instruction,
executable_accounts, executable_accounts,
accounts, accounts,
feature_set.is_active(&demote_program_write_locks::id()),
); );
let compute_meter = if feature_set.is_active(&tx_wide_compute_cap::id()) { let compute_meter = if feature_set.is_active(&tx_wide_compute_cap::id()) {
compute_meter compute_meter
@ -389,6 +391,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,
@ -399,6 +402,7 @@ impl MessageProcessor {
timings: &mut ExecuteDetailsTimings, timings: &mut ExecuteDetailsTimings,
logger: Rc<RefCell<dyn Logger>>, logger: Rc<RefCell<dyn Logger>>,
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)?;
@ -419,7 +423,7 @@ impl MessageProcessor {
pre_accounts[unique_index] pre_accounts[unique_index]
.verify( .verify(
program_id, program_id,
message.is_writable(account_index), message.is_writable(account_index, demote_program_write_locks),
rent, rent,
&account, &account,
timings, timings,
@ -534,6 +538,7 @@ impl MessageProcessor {
timings, timings,
invoke_context.get_logger(), invoke_context.get_logger(),
invoke_context.is_feature_active(&updated_verify_policy::id()), invoke_context.is_feature_active(&updated_verify_policy::id()),
invoke_context.is_feature_active(&demote_program_write_locks::id()),
)?; )?;
timings.accumulate(&invoke_context.timings); timings.accumulate(&invoke_context.timings);
@ -713,7 +718,7 @@ mod tests {
))), ))),
)); ));
let write_privileges: Vec<bool> = (0..message.account_keys.len()) let write_privileges: Vec<bool> = (0..message.account_keys.len())
.map(|i| message.is_writable(i)) .map(|i| message.is_writable(i, /*demote_program_write_locks=*/ true))
.collect(); .collect();
invoke_context invoke_context
.verify_and_update(&message.instructions[0], &these_accounts, &write_privileges) .verify_and_update(&message.instructions[0], &these_accounts, &write_privileges)
@ -1193,6 +1198,8 @@ mod tests {
metas.clone(), metas.clone(),
); );
let message = Message::new(&[instruction], None); let message = Message::new(&[instruction], None);
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 blockhash = Hash::default(); let blockhash = Hash::default();
@ -1210,7 +1217,7 @@ mod tests {
Rc::new(RefCell::new(MockComputeMeter::default())), Rc::new(RefCell::new(MockComputeMeter::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_for_tests()), Arc::new(Accounts::default_for_tests()),
&ancestors, &ancestors,
&blockhash, &blockhash,
@ -1222,7 +1229,7 @@ mod tests {
.account_keys .account_keys
.iter() .iter()
.enumerate() .enumerate()
.map(|(i, _)| message.is_writable(i)) .map(|(i, _)| message.is_writable(i, demote_program_write_locks))
.collect::<Vec<bool>>(); .collect::<Vec<bool>>();
accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1; accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1;
assert_eq!( assert_eq!(
@ -1282,7 +1289,7 @@ mod tests {
.account_keys .account_keys
.iter() .iter()
.enumerate() .enumerate()
.map(|(i, _)| message.is_writable(i)) .map(|(i, _)| message.is_writable(i, demote_program_write_locks))
.collect::<Vec<bool>>(); .collect::<Vec<bool>>();
assert_eq!( assert_eq!(
InstructionProcessor::process_cross_program_instruction( InstructionProcessor::process_cross_program_instruction(

View File

@ -15,6 +15,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();
@ -30,7 +32,7 @@ fn bench_manual_instruction_serialize(b: &mut Bencher) {
SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique()))) SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique())))
.unwrap(); .unwrap();
b.iter(|| { b.iter(|| {
test::black_box(message.serialize_instructions()); test::black_box(message.serialize_instructions(DEMOTE_PROGRAM_WRITE_LOCKS));
}); });
} }
@ -49,7 +51,7 @@ fn bench_manual_instruction_deserialize(b: &mut Bencher) {
let message = let message =
SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique()))) SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique())))
.unwrap(); .unwrap();
let serialized = message.serialize_instructions(); let serialized = message.serialize_instructions(DEMOTE_PROGRAM_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());
@ -63,7 +65,7 @@ fn bench_manual_instruction_deserialize_single(b: &mut Bencher) {
let message = let message =
SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique()))) SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique())))
.unwrap(); .unwrap();
let serialized = message.serialize_instructions(); let serialized = message.serialize_instructions(DEMOTE_PROGRAM_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());
}); });

View File

@ -353,7 +353,7 @@ impl Message {
self.program_position(i).is_some() self.program_position(i).is_some()
} }
pub fn is_writable(&self, i: usize) -> 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
@ -363,6 +363,7 @@ impl Message {
let key = self.account_keys[i]; let key = self.account_keys[i];
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 {
@ -374,7 +375,7 @@ impl Message {
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) { if self.is_writable(i, /*demote_program_write_locks=*/ true) {
writable_keys.push(key); writable_keys.push(key);
} else { } else {
readonly_keys.push(key); readonly_keys.push(key);
@ -412,7 +413,8 @@ 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); let is_writable =
self.is_writable(account_index, /*demote_program_write_locks=*/ true);
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;
@ -866,12 +868,13 @@ mod tests {
recent_blockhash: Hash::default(), recent_blockhash: Hash::default(),
instructions: vec![], instructions: vec![],
}; };
assert!(message.is_writable(0)); let demote_program_write_locks = true;
assert!(!message.is_writable(1)); assert!(message.is_writable(0, demote_program_write_locks));
assert!(!message.is_writable(2)); assert!(!message.is_writable(1, demote_program_write_locks));
assert!(message.is_writable(3)); assert!(!message.is_writable(2, demote_program_write_locks));
assert!(message.is_writable(4)); assert!(message.is_writable(3, demote_program_write_locks));
assert!(!message.is_writable(5)); assert!(message.is_writable(4, demote_program_write_locks));
assert!(!message.is_writable(5, demote_program_write_locks));
} }
#[test] #[test]

View File

@ -4,7 +4,7 @@ use {
pubkey::Pubkey, pubkey::Pubkey,
sysvar, sysvar,
}, },
std::collections::HashSet, std::{collections::HashSet, convert::TryFrom},
}; };
/// Combination of a version #0 message and its mapped addresses /// Combination of a version #0 message and its mapped addresses
@ -96,20 +96,32 @@ impl MappedMessage {
} }
/// Returns true if the account at the specified index was loaded as writable /// Returns true if the account at the specified index was loaded as writable
pub fn is_writable(&self, key_index: usize) -> bool { pub fn is_writable(&self, key_index: usize, demote_program_write_locks: bool) -> bool {
if self.is_writable_index(key_index) { if self.is_writable_index(key_index) {
if let Some(key) = self.get_account_key(key_index) { if let Some(key) = self.get_account_key(key_index) {
return !(sysvar::is_sysvar_id(key) || BUILTIN_PROGRAMS_KEYS.contains(key)); return !(sysvar::is_sysvar_id(key) || BUILTIN_PROGRAMS_KEYS.contains(key)
|| (demote_program_write_locks && self.is_key_called_as_program(key_index)));
} }
} }
false false
} }
/// Returns true if the account at the specified index is called as a program by an instruction
pub fn is_key_called_as_program(&self, key_index: usize) -> bool {
if let Ok(key_index) = u8::try_from(key_index) {
self.message.instructions
.iter()
.any(|ix| ix.program_id_index == key_index)
} else {
false
}
}
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use crate::{message::MessageHeader, system_program, sysvar}; use crate::{instruction::CompiledInstruction, message::MessageHeader, system_program, sysvar};
use itertools::Itertools; use itertools::Itertools;
fn create_test_mapped_message() -> (MappedMessage, [Pubkey; 6]) { fn create_test_mapped_message() -> (MappedMessage, [Pubkey; 6]) {
@ -236,10 +248,42 @@ mod tests {
mapped_msg.message.account_keys[0] = sysvar::clock::id(); mapped_msg.message.account_keys[0] = sysvar::clock::id();
assert!(mapped_msg.is_writable_index(0)); assert!(mapped_msg.is_writable_index(0));
assert!(!mapped_msg.is_writable(0)); assert!(!mapped_msg.is_writable(0, /*demote_program_write_locks=*/ true));
mapped_msg.message.account_keys[0] = system_program::id(); mapped_msg.message.account_keys[0] = system_program::id();
assert!(mapped_msg.is_writable_index(0)); assert!(mapped_msg.is_writable_index(0));
assert!(!mapped_msg.is_writable(0)); assert!(!mapped_msg.is_writable(0, /*demote_program_write_locks=*/ true));
}
#[test]
fn test_demote_writable_program() {
let key0 = Pubkey::new_unique();
let key1 = Pubkey::new_unique();
let key2 = Pubkey::new_unique();
let mapped_msg = MappedMessage {
message: v0::Message {
header: MessageHeader {
num_required_signatures: 1,
num_readonly_signed_accounts: 0,
num_readonly_unsigned_accounts: 0,
},
account_keys: vec![key0],
instructions: vec![
CompiledInstruction {
program_id_index: 2,
accounts: vec![1],
data: vec![],
}
],
..v0::Message::default()
},
mapped_addresses: MappedAddresses {
writable: vec![key1, key2],
readonly: vec![],
},
};
assert!(mapped_msg.is_writable_index(2));
assert!(!mapped_msg.is_writable(2, /*demote_program_write_locks=*/ true));
} }
} }

View File

@ -175,12 +175,9 @@ impl SanitizedMessage {
/// Returns true if the account at the specified index is invoked as a /// Returns true if the account at the specified index is invoked as a
/// program in this message. /// program in this message.
pub fn is_invoked(&self, key_index: usize) -> bool { pub fn is_invoked(&self, key_index: usize) -> bool {
if let Ok(key_index) = u8::try_from(key_index) { match self {
self.instructions() Self::Legacy(message) => message.is_key_called_as_program(key_index),
.iter() Self::V0(message) => message.is_key_called_as_program(key_index),
.any(|ix| ix.program_id_index == key_index)
} else {
false
} }
} }
@ -192,10 +189,10 @@ impl SanitizedMessage {
/// Returns true if the account at the specified index is writable by the /// Returns true if the account at the specified index is writable by the
/// instructions in this message. /// instructions in this message.
pub fn is_writable(&self, index: usize) -> bool { pub fn is_writable(&self, index: usize, demote_program_write_locks: bool) -> bool {
match self { match self {
Self::Legacy(message) => message.is_writable(index), Self::Legacy(message) => message.is_writable(index, demote_program_write_locks),
Self::V0(message) => message.is_writable(index), Self::V0(message) => message.is_writable(index, demote_program_write_locks),
} }
} }
@ -219,7 +216,7 @@ impl SanitizedMessage {
// 67..69 - data len - u16 // 67..69 - data len - u16
// 69..data_len - data // 69..data_len - data
#[allow(clippy::integer_arithmetic)] #[allow(clippy::integer_arithmetic)]
pub fn serialize_instructions(&self) -> 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);
@ -234,7 +231,7 @@ impl SanitizedMessage {
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); let is_writable = self.is_writable(account_index, demote_program_write_locks);
let mut account_meta = InstructionsSysvarAccountMeta::NONE; let mut account_meta = InstructionsSysvarAccountMeta::NONE;
if is_signer { if is_signer {
account_meta |= InstructionsSysvarAccountMeta::IS_SIGNER; account_meta |= InstructionsSysvarAccountMeta::IS_SIGNER;
@ -440,9 +437,10 @@ mod tests {
), ),
]; ];
let demote_program_write_locks = true;
let message = Message::new(&instructions, Some(&id1)); let message = Message::new(&instructions, Some(&id1));
let sanitized_message = SanitizedMessage::try_from(message.clone()).unwrap(); let sanitized_message = SanitizedMessage::try_from(message.clone()).unwrap();
let serialized = sanitized_message.serialize_instructions(); let serialized = sanitized_message.serialize_instructions(demote_program_write_locks);
// assert that SanitizedMessage::serialize_instructions has the same behavior as the // assert that SanitizedMessage::serialize_instructions has the same behavior as the
// deprecated Message::serialize_instructions method // deprecated Message::serialize_instructions method

View File

@ -191,6 +191,10 @@ pub mod stake_program_advance_activating_credits_observed {
solana_sdk::declare_id!("SAdVFw3RZvzbo6DvySbSdBnHN4gkzSTH9dSxesyKKPj"); solana_sdk::declare_id!("SAdVFw3RZvzbo6DvySbSdBnHN4gkzSTH9dSxesyKKPj");
} }
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> = [
@ -234,6 +238,7 @@ lazy_static! {
(instructions_sysvar_owned_by_sysvar::id(), "fix owner for instructions sysvar"), (instructions_sysvar_owned_by_sysvar::id(), "fix owner for instructions sysvar"),
(close_upgradeable_program_accounts::id(), "enable closing upgradeable program accounts"), (close_upgradeable_program_accounts::id(), "enable closing upgradeable program accounts"),
(stake_program_advance_activating_credits_observed::id(), "Enable advancing credits observed for activation epoch #19309"), (stake_program_advance_activating_credits_observed::id(), "Enable advancing credits observed for activation epoch #19309"),
(demote_program_write_locks::id(), "demote program write locks to readonly #19593"),
/*************** ADD NEW FEATURES HERE ***************/ /*************** ADD NEW FEATURES HERE ***************/
] ]
.iter() .iter()

View File

@ -125,7 +125,7 @@ impl SanitizedTransaction {
} }
/// Return the list of accounts that must be locked during processing this transaction. /// Return the list of accounts that must be locked during processing this transaction.
pub fn get_account_locks(&self) -> TransactionAccountLocks { pub fn get_account_locks(&self, demote_program_write_locks: bool) -> TransactionAccountLocks {
let message = &self.message; let message = &self.message;
let num_readonly_accounts = message.num_readonly_accounts(); let num_readonly_accounts = message.num_readonly_accounts();
let num_writable_accounts = message let num_writable_accounts = message
@ -138,7 +138,7 @@ impl SanitizedTransaction {
}; };
for (i, key) in message.account_keys_iter().enumerate() { for (i, key) in message.account_keys_iter().enumerate() {
if message.is_writable(i) { if message.is_writable(i, demote_program_write_locks) {
account_locks.writable.push(key); account_locks.writable.push(key);
} else { } else {
account_locks.readonly.push(key); account_locks.readonly.push(key);

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), writable: message.is_writable(i, /*demote_program_write_locks=*/ true),
signer: message.is_signer(i), signer: message.is_signer(i),
}); });
} }