diff --git a/cli-output/src/display.rs b/cli-output/src/display.rs index 88c99f50f..6e2e5b13d 100644 --- a/cli-output/src/display.rs +++ b/cli-output/src/display.rs @@ -139,7 +139,7 @@ fn format_account_mode(message: &Message, index: usize) -> String { } else { "-" }, - if message.is_writable(index, /*demote_program_write_locks=*/ true) { + if message.is_writable(index) { "w" // comment for consistent rust fmt (no joking; lol) } else { "-" diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 15898d4ff..1bb5c7bb7 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -935,8 +935,7 @@ impl BankingStage { gossip_vote_sender: &ReplayVoteSender, qos_service: &Arc, ) -> (Result, Vec) { - let tx_costs = - qos_service.compute_transaction_costs(txs.iter(), bank.demote_program_write_locks()); + let tx_costs = qos_service.compute_transaction_costs(txs.iter()); let transactions_qos_results = qos_service.select_transactions_per_cost(txs.iter(), tx_costs.iter(), bank); diff --git a/core/src/qos_service.rs b/core/src/qos_service.rs index de50031ce..9725ee271 100644 --- a/core/src/qos_service.rs +++ b/core/src/qos_service.rs @@ -78,13 +78,12 @@ impl QosService { pub fn compute_transaction_costs<'a>( &self, transactions: impl Iterator, - demote_program_write_locks: bool, ) -> Vec { let mut compute_cost_time = Measure::start("compute_cost_time"); let cost_model = self.cost_model.read().unwrap(); let txs_costs: Vec<_> = transactions .map(|tx| { - let cost = cost_model.calculate_cost(tx, demote_program_write_locks); + let cost = cost_model.calculate_cost(tx); debug!( "transaction {:?}, cost {:?}, cost sum {}", tx, @@ -261,7 +260,7 @@ mod tests { let cost_model = Arc::new(RwLock::new(CostModel::default())); let qos_service = QosService::new(cost_model.clone()); - let txs_costs = qos_service.compute_transaction_costs(txs.iter(), false); + let txs_costs = qos_service.compute_transaction_costs(txs.iter()); // verify the size of txs_costs and its contents assert_eq!(txs_costs.len(), txs.len()); @@ -271,11 +270,7 @@ mod tests { .map(|(index, cost)| { assert_eq!( cost.sum(), - cost_model - .read() - .unwrap() - .calculate_cost(&txs[index], false) - .sum() + cost_model.read().unwrap().calculate_cost(&txs[index]).sum() ); }) .collect_vec(); @@ -306,14 +301,14 @@ mod tests { let transfer_tx_cost = cost_model .read() .unwrap() - .calculate_cost(&transfer_tx, false) + .calculate_cost(&transfer_tx) .sum(); // make a vec of txs let txs = vec![transfer_tx.clone(), vote_tx.clone(), transfer_tx, vote_tx]; let qos_service = QosService::new(cost_model); - let txs_costs = qos_service.compute_transaction_costs(txs.iter(), false); + let txs_costs = qos_service.compute_transaction_costs(txs.iter()); // set cost tracker limit to fit 1 transfer tx, vote tx bypasses limit check let cost_limit = transfer_tx_cost; @@ -359,7 +354,7 @@ mod tests { .name("test-producer-1".to_string()) .spawn(move || { debug!("thread 1 starts with {} txs", txs_1.len()); - let tx_costs = qos_service_1.compute_transaction_costs(txs_1.iter(), false); + let tx_costs = qos_service_1.compute_transaction_costs(txs_1.iter()); assert_eq!(txs_count, tx_costs.len()); debug!( "thread 1 done, generated {} count, see service count as {}", @@ -376,7 +371,7 @@ mod tests { .name("test-producer-2".to_string()) .spawn(move || { debug!("thread 2 starts with {} txs", txs_2.len()); - let tx_costs = qos_service_2.compute_transaction_costs(txs_2.iter(), false); + let tx_costs = qos_service_2.compute_transaction_costs(txs_2.iter()); assert_eq!(txs_count, tx_costs.len()); debug!( "thread 2 done, generated {} count, see service count as {}", diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 1b678c030..e9f01b4f8 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -800,10 +800,7 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String> .for_each(|transaction| { num_programs += transaction.message().instructions().len(); - let tx_cost = cost_model.calculate_cost( - &transaction, - true, // demote_program_write_locks - ); + let tx_cost = cost_model.calculate_cost(&transaction); let result = cost_tracker.try_add(&transaction, &tx_cost); if result.is_err() { println!( diff --git a/program-runtime/src/invoke_context.rs b/program-runtime/src/invoke_context.rs index 5474087fd..32674da18 100644 --- a/program-runtime/src/invoke_context.rs +++ b/program-runtime/src/invoke_context.rs @@ -10,9 +10,8 @@ use { bpf_loader_upgradeable::{self, UpgradeableLoaderState}, compute_budget::ComputeBudget, feature_set::{ - demote_program_write_locks, do_support_realloc, neon_evm_compute_budget, - reject_empty_instruction_without_program, remove_native_loader, requestable_heap_size, - tx_wide_compute_cap, FeatureSet, + do_support_realloc, neon_evm_compute_budget, reject_empty_instruction_without_program, + remove_native_loader, requestable_heap_size, tx_wide_compute_cap, FeatureSet, }, hash::Hash, instruction::{AccountMeta, CompiledInstruction, Instruction, InstructionError}, @@ -282,9 +281,6 @@ impl<'a> InvokeContext<'a> { } // Create the KeyedAccounts that will be passed to the program - let demote_program_write_locks = self - .feature_set - .is_active(&demote_program_write_locks::id()); let keyed_accounts = program_indices .iter() .map(|account_index| { @@ -304,7 +300,7 @@ impl<'a> InvokeContext<'a> { }; ( message.is_signer(index_in_instruction), - message.is_writable(index_in_instruction, demote_program_write_locks), + message.is_writable(index_in_instruction), &self.accounts[account_index].0, &self.accounts[account_index].1 as &RefCell, ) @@ -336,9 +332,6 @@ impl<'a> InvokeContext<'a> { program_indices: &[usize], ) -> Result<(), InstructionError> { let program_id = instruction.program_id(&message.account_keys); - let demote_program_write_locks = self - .feature_set - .is_active(&demote_program_write_locks::id()); let do_support_realloc = self.feature_set.is_active(&do_support_realloc::id()); // Verify all executable accounts have zero outstanding refs @@ -364,7 +357,7 @@ impl<'a> InvokeContext<'a> { pre_account .verify( program_id, - message.is_writable(account_index, demote_program_write_locks), + message.is_writable(account_index), &self.rent, &account, &mut self.timings, @@ -669,11 +662,8 @@ impl<'a> InvokeContext<'a> { if is_lowest_invocation_level { self.verify(message, instruction, program_indices)?; } else { - let demote_program_write_locks = self - .feature_set - .is_active(&demote_program_write_locks::id()); let write_privileges: Vec = (0..message.account_keys.len()) - .map(|i| message.is_writable(i, demote_program_write_locks)) + .map(|i| message.is_writable(i)) .collect(); self.verify_and_update(instruction, account_indices, &write_privileges)?; } @@ -1147,7 +1137,7 @@ mod tests { None, ); let write_privileges: Vec = (0..message.account_keys.len()) - .map(|i| message.is_writable(i, /*demote_program_write_locks=*/ true)) + .map(|i| message.is_writable(i)) .collect(); // modify account owned by the program @@ -1269,14 +1259,11 @@ mod tests { .unwrap(); // not owned account modified by the caller (before the invoke) - let demote_program_write_locks = invoke_context - .feature_set - .is_active(&demote_program_write_locks::id()); let caller_write_privileges = message .account_keys .iter() .enumerate() - .map(|(i, _)| message.is_writable(i, demote_program_write_locks)) + .map(|(i, _)| message.is_writable(i)) .collect::>(); accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1; assert_eq!( @@ -1330,7 +1317,7 @@ mod tests { .account_keys .iter() .enumerate() - .map(|(i, _)| message.is_writable(i, demote_program_write_locks)) + .map(|(i, _)| message.is_writable(i)) .collect::>(); assert_eq!( invoke_context.process_instruction( diff --git a/program-test/src/lib.rs b/program-test/src/lib.rs index bbd1bd5f4..a335c39fe 100644 --- a/program-test/src/lib.rs +++ b/program-test/src/lib.rs @@ -26,7 +26,6 @@ use { compute_budget::ComputeBudget, entrypoint::{ProgramResult, SUCCESS}, epoch_schedule::EpochSchedule, - feature_set::demote_program_write_locks, fee_calculator::{FeeCalculator, FeeRateGovernor}, genesis_config::{ClusterType, GenesisConfig}, hash::Hash, @@ -244,15 +243,12 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { let message = Message::new(&[instruction.clone()], None); let program_id_index = message.instructions[0].program_id_index as usize; let program_id = message.account_keys[program_id_index]; - let demote_program_write_locks = invoke_context - .feature_set - .is_active(&demote_program_write_locks::id()); // TODO don't have the caller's keyed_accounts so can't validate writer or signer escalation or deescalation yet let caller_privileges = message .account_keys .iter() .enumerate() - .map(|(i, _)| message.is_writable(i, demote_program_write_locks)) + .map(|(i, _)| message.is_writable(i)) .collect::>(); stable_log::program_invoke(&log_collector, &program_id, invoke_context.invoke_depth()); @@ -280,7 +276,7 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs { account.set_executable(account_info.executable); account.set_rent_epoch(account_info.rent_epoch); } - let account_info = if message.is_writable(i, demote_program_write_locks) { + let account_info = if message.is_writable(i) { Some(account_info) } else { None diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 2f2c72409..b9f3f3504 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -24,11 +24,10 @@ use { entrypoint::{BPF_ALIGN_OF_U128, MAX_PERMITTED_DATA_INCREASE, SUCCESS}, epoch_schedule::EpochSchedule, feature_set::{ - blake3_syscall_enabled, demote_program_write_locks, disable_fees_sysvar, - do_support_realloc, fixed_memcpy_nonoverlapping_check, - libsecp256k1_0_5_upgrade_enabled, prevent_calling_precompiles_as_programs, - return_data_syscall_enabled, secp256k1_recover_syscall_enabled, - sol_log_data_syscall_enabled, + blake3_syscall_enabled, disable_fees_sysvar, do_support_realloc, + fixed_memcpy_nonoverlapping_check, libsecp256k1_0_5_upgrade_enabled, + prevent_calling_precompiles_as_programs, return_data_syscall_enabled, + secp256k1_recover_syscall_enabled, sol_log_data_syscall_enabled, }, hash::{Hasher, HASH_BYTES}, instruction::{AccountMeta, Instruction, InstructionError}, @@ -2198,9 +2197,6 @@ fn get_translated_accounts<'a, T, F>( where F: Fn(&T, &InvokeContext) -> Result, EbpfError>, { - let demote_program_write_locks = invoke_context - .feature_set - .is_active(&demote_program_write_locks::id()); let keyed_accounts = invoke_context .get_instruction_keyed_accounts() .map_err(SyscallError::InstructionError)?; @@ -2229,7 +2225,7 @@ where account.set_executable(caller_account.executable); account.set_rent_epoch(caller_account.rent_epoch); } - let caller_account = if message.is_writable(i, demote_program_write_locks) { + let caller_account = if message.is_writable(i) { if let Some(orig_data_len_index) = keyed_accounts .iter() .position(|keyed_account| keyed_account.unsigned_key() == account_key) diff --git a/rpc/src/transaction_status_service.rs b/rpc/src/transaction_status_service.rs index 50d7ecb09..9523df641 100644 --- a/rpc/src/transaction_status_service.rs +++ b/rpc/src/transaction_status_service.rs @@ -123,8 +123,7 @@ impl TransactionStatusService { transaction.message(), lamports_per_signature, ); - let tx_account_locks = - transaction.get_account_locks(bank.demote_program_write_locks()); + let tx_account_locks = transaction.get_account_locks(); let inner_instructions = inner_instructions.map(|inner_instructions| { inner_instructions diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 1370d415e..507aef238 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -209,9 +209,8 @@ impl Accounts { fn construct_instructions_account( message: &SanitizedMessage, is_owned_by_sysvar: bool, - demote_program_write_locks: bool, ) -> AccountSharedData { - let data = construct_instructions_data(message, demote_program_write_locks); + let data = construct_instructions_data(message); let owner = if is_owned_by_sysvar { sysvar::id() } else { @@ -247,9 +246,6 @@ impl Accounts { let mut account_deps = Vec::with_capacity(message.account_keys_len()); let mut rent_debits = RentDebits::default(); 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() { let account = if !message.is_non_loader_key(i) { // Fill in an empty account for the program slots. @@ -264,14 +260,13 @@ impl Accounts { message, feature_set .is_active(&feature_set::instructions_sysvar_owned_by_sysvar::id()), - demote_program_write_locks, ) } else { let (account, rent) = self .accounts_db .load_with_fixed_root(ancestors, key) .map(|(mut account, _)| { - if message.is_writable(i, demote_program_write_locks) { + if message.is_writable(i) { let rent_due = rent_collector.collect_from_existing_account( key, &mut account, @@ -286,10 +281,7 @@ impl Accounts { .unwrap_or_default(); if bpf_loader_upgradeable::check_id(account.owner()) { - if demote_program_write_locks - && message.is_writable(i, demote_program_write_locks) - && !message.is_upgradeable_loader_present() - { + if message.is_writable(i) && !message.is_upgradeable_loader_present() { error_counters.invalid_writable_account += 1; return Err(TransactionError::InvalidWritableAccount); } @@ -315,10 +307,7 @@ impl Accounts { return Err(TransactionError::InvalidProgramForExecution); } } - } else if account.executable() - && demote_program_write_locks - && message.is_writable(i, demote_program_write_locks) - { + } else if account.executable() && message.is_writable(i) { error_counters.invalid_writable_account += 1; return Err(TransactionError::InvalidWritableAccount); } @@ -985,11 +974,8 @@ impl Accounts { pub fn lock_accounts<'a>( &self, txs: impl Iterator, - demote_program_write_locks: bool, ) -> Vec> { - let keys: Vec<_> = txs - .map(|tx| tx.get_account_locks(demote_program_write_locks)) - .collect(); + let keys: Vec<_> = txs.map(|tx| tx.get_account_locks()).collect(); let account_locks = &mut self.account_locks.lock().unwrap(); keys.into_iter() .map(|keys| self.lock_account(account_locks, keys.writable, keys.readonly)) @@ -1002,12 +988,11 @@ impl Accounts { &self, txs: impl Iterator, results: impl Iterator>, - demote_program_write_locks: bool, ) -> Vec> { let key_results: Vec<_> = txs .zip(results) .map(|(tx, result)| match result { - Ok(()) => Ok(tx.get_account_locks(demote_program_write_locks)), + Ok(()) => Ok(tx.get_account_locks()), Err(e) => Err(e), }) .collect(); @@ -1027,7 +1012,6 @@ impl Accounts { &self, txs: impl Iterator, results: &[Result<()>], - demote_program_write_locks: bool, ) { let keys: Vec<_> = txs .zip(results) @@ -1038,7 +1022,7 @@ impl Accounts { | Err(TransactionError::WouldExceedMaxBlockCostLimit) | Err(TransactionError::WouldExceedMaxAccountCostLimit) | Err(TransactionError::WouldExceedMaxAccountDataCostLimit) => None, - _ => Some(tx.get_account_locks(demote_program_write_locks)), + _ => Some(tx.get_account_locks()), }) .collect(); let mut account_locks = self.account_locks.lock().unwrap(); @@ -1061,7 +1045,6 @@ impl Accounts { blockhash: &Hash, lamports_per_signature: u64, rent_for_sysvars: bool, - demote_program_write_locks: bool, leave_nonce_on_success: bool, ) { let accounts_to_store = self.collect_accounts_to_store( @@ -1072,7 +1055,6 @@ impl Accounts { blockhash, lamports_per_signature, rent_for_sysvars, - demote_program_write_locks, leave_nonce_on_success, ); self.accounts_db.store_cached(slot, &accounts_to_store); @@ -1100,7 +1082,6 @@ impl Accounts { blockhash: &Hash, lamports_per_signature: u64, rent_for_sysvars: bool, - demote_program_write_locks: bool, leave_nonce_on_success: bool, ) -> Vec<(&'a Pubkey, &'a AccountSharedData)> { let mut accounts = Vec::with_capacity(load_results.len()); @@ -1137,7 +1118,7 @@ impl Accounts { fee_payer_index = Some(i); } let is_fee_payer = Some(i) == fee_payer_index; - if message.is_writable(i, demote_program_write_locks) { + if message.is_writable(i) { let is_nonce_account = prepare_if_nonce_account( address, account, @@ -2171,8 +2152,6 @@ mod tests { 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, @@ -2183,7 +2162,7 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair0], message, Hash::default()); - let results0 = accounts.lock_accounts([tx.clone()].iter(), demote_program_write_locks); + let results0 = accounts.lock_accounts([tx.clone()].iter()); assert!(results0[0].is_ok()); assert_eq!( @@ -2218,7 +2197,7 @@ mod tests { ); let tx1 = new_sanitized_tx(&[&keypair1], message, Hash::default()); let txs = vec![tx0, tx1]; - let results1 = accounts.lock_accounts(txs.iter(), demote_program_write_locks); + let results1 = accounts.lock_accounts(txs.iter()); 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 @@ -2233,8 +2212,8 @@ mod tests { 2 ); - accounts.unlock_accounts([tx].iter(), &results0, demote_program_write_locks); - accounts.unlock_accounts(txs.iter(), &results1, demote_program_write_locks); + accounts.unlock_accounts([tx].iter(), &results0); + accounts.unlock_accounts(txs.iter(), &results1); let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; let message = Message::new_with_compiled_instructions( 1, @@ -2245,7 +2224,7 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair1], message, Hash::default()); - let results2 = accounts.lock_accounts([tx].iter(), demote_program_write_locks); + let results2 = accounts.lock_accounts([tx].iter()); assert!(results2[0].is_ok()); // Now keypair1 account can be locked as writable // Check that read-only lock with zero references is deleted @@ -2282,8 +2261,6 @@ mod tests { accounts.store_slow_uncached(0, &keypair1.pubkey(), &account1); accounts.store_slow_uncached(0, &keypair2.pubkey(), &account2); - let demote_program_write_locks = true; - let accounts_arc = Arc::new(accounts); let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])]; @@ -2316,15 +2293,13 @@ mod tests { let exit_clone = exit_clone.clone(); loop { let txs = vec![writable_tx.clone()]; - let results = accounts_clone - .clone() - .lock_accounts(txs.iter(), demote_program_write_locks); + let results = accounts_clone.clone().lock_accounts(txs.iter()); for result in results.iter() { if result.is_ok() { counter_clone.clone().fetch_add(1, Ordering::SeqCst); } } - accounts_clone.unlock_accounts(txs.iter(), &results, demote_program_write_locks); + accounts_clone.unlock_accounts(txs.iter(), &results); if exit_clone.clone().load(Ordering::Relaxed) { break; } @@ -2333,15 +2308,13 @@ mod tests { let counter_clone = counter; for _ in 0..5 { let txs = vec![readonly_tx.clone()]; - let results = accounts_arc - .clone() - .lock_accounts(txs.iter(), demote_program_write_locks); + let results = accounts_arc.clone().lock_accounts(txs.iter()); if results[0].is_ok() { let counter_value = counter_clone.clone().load(Ordering::SeqCst); thread::sleep(time::Duration::from_millis(50)); assert_eq!(counter_value, counter_clone.clone().load(Ordering::SeqCst)); } - accounts_arc.unlock_accounts(txs.iter(), &results, demote_program_write_locks); + accounts_arc.unlock_accounts(txs.iter(), &results); thread::sleep(time::Duration::from_millis(50)); } exit.store(true, Ordering::Relaxed); @@ -2371,8 +2344,6 @@ mod tests { 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, @@ -2383,7 +2354,7 @@ mod tests { instructions, ); let tx = new_sanitized_tx(&[&keypair0], message, Hash::default()); - let results0 = accounts.lock_accounts([tx].iter(), demote_program_write_locks); + let results0 = accounts.lock_accounts([tx].iter()); assert!(results0[0].is_ok()); // Instruction program-id account demoted to readonly @@ -2436,8 +2407,6 @@ mod tests { 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, @@ -2476,11 +2445,7 @@ mod tests { Ok(()), ]; - let results = accounts.lock_accounts_with_results( - txs.iter(), - qos_results.into_iter(), - demote_program_write_locks, - ); + let results = accounts.lock_accounts_with_results(txs.iter(), qos_results.into_iter()); assert!(results[0].is_ok()); // Read-only account (keypair0) can be referenced multiple times assert!(results[1].is_err()); // is not locked due to !qos_results[1].is_ok() @@ -2506,7 +2471,7 @@ mod tests { .get(&keypair2.pubkey()) .is_none()); - accounts.unlock_accounts(txs.iter(), &results, demote_program_write_locks); + accounts.unlock_accounts(txs.iter(), &results); // check all locks to be removed assert!(accounts @@ -2610,7 +2575,6 @@ mod tests { &Hash::default(), 0, true, - true, // demote_program_write_locks true, // leave_nonce_on_success ); assert_eq!(collected_accounts.len(), 2); @@ -3040,7 +3004,6 @@ mod tests { &next_blockhash, 0, true, - true, // demote_program_write_locks true, // leave_nonce_on_success ); assert_eq!(collected_accounts.len(), 2); @@ -3151,7 +3114,6 @@ mod tests { &next_blockhash, 0, true, - true, // demote_program_write_locks true, // leave_nonce_on_success ); assert_eq!(collected_accounts.len(), 1); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 5781e5da6..4b783ce99 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3068,10 +3068,7 @@ impl Bank { .into_iter() .map(SanitizedTransaction::from_transaction_for_tests) .collect::>(); - let lock_results = self - .rc - .accounts - .lock_accounts(sanitized_txs.iter(), self.demote_program_write_locks()); + let lock_results = self.rc.accounts.lock_accounts(sanitized_txs.iter()); TransactionBatch::new(lock_results, self, Cow::Owned(sanitized_txs)) } @@ -3087,10 +3084,7 @@ impl Bank { }) }) .collect::>>()?; - let lock_results = self - .rc - .accounts - .lock_accounts(sanitized_txs.iter(), self.demote_program_write_locks()); + let lock_results = self.rc.accounts.lock_accounts(sanitized_txs.iter()); Ok(TransactionBatch::new( lock_results, self, @@ -3103,10 +3097,7 @@ impl Bank { &'a self, txs: &'b [SanitizedTransaction], ) -> TransactionBatch<'a, 'b> { - let lock_results = self - .rc - .accounts - .lock_accounts(txs.iter(), self.demote_program_write_locks()); + let lock_results = self.rc.accounts.lock_accounts(txs.iter()); TransactionBatch::new(lock_results, self, Cow::Borrowed(txs)) } @@ -3118,11 +3109,10 @@ impl Bank { transaction_results: impl Iterator>, ) -> TransactionBatch<'a, 'b> { // this lock_results could be: Ok, AccountInUse, WouldExceedBlockMaxLimit or WouldExceedAccountMaxLimit - let lock_results = self.rc.accounts.lock_accounts_with_results( - transactions.iter(), - transaction_results, - self.demote_program_write_locks(), - ); + let lock_results = self + .rc + .accounts + .lock_accounts_with_results(transactions.iter(), transaction_results); TransactionBatch::new(lock_results, self, Cow::Borrowed(transactions)) } @@ -3204,11 +3194,9 @@ impl Bank { pub fn unlock_accounts(&self, batch: &mut TransactionBatch) { if batch.needs_unlock { batch.needs_unlock = false; - self.rc.accounts.unlock_accounts( - batch.sanitized_transactions().iter(), - batch.lock_results(), - self.demote_program_write_locks(), - ) + self.rc + .accounts + .unlock_accounts(batch.sanitized_transactions().iter(), batch.lock_results()) } } @@ -3921,7 +3909,6 @@ impl Bank { &blockhash, lamports_per_signature, self.rent_for_sysvars(), - self.demote_program_write_locks(), self.leave_nonce_on_success(), ); let rent_debits = self.collect_rent(executed_results, loaded_txs); @@ -5768,11 +5755,6 @@ impl Bank { .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()) - } - pub fn leave_nonce_on_success(&self) -> bool { self.feature_set .is_active(&feature_set::leave_nonce_on_success::id()) @@ -7271,10 +7253,7 @@ pub(crate) mod tests { assert_eq!( bank.process_transaction(&tx), - Err(TransactionError::InstructionError( - 0, - InstructionError::ExecutableLamportChange - )) + Err(TransactionError::InvalidWritableAccount) ); assert_eq!(bank.get_balance(&account_pubkey), account_balance); } diff --git a/runtime/src/cost_model.rs b/runtime/src/cost_model.rs index ab74f9de5..9c29da595 100644 --- a/runtime/src/cost_model.rs +++ b/runtime/src/cost_model.rs @@ -111,15 +111,11 @@ impl CostModel { ); } - pub fn calculate_cost( - &self, - transaction: &SanitizedTransaction, - demote_program_write_locks: bool, - ) -> TransactionCost { + pub fn calculate_cost(&self, transaction: &SanitizedTransaction) -> TransactionCost { let mut tx_cost = TransactionCost::new_with_capacity(MAX_WRITABLE_ACCOUNTS); tx_cost.signature_cost = self.get_signature_cost(transaction); - self.get_write_lock_cost(&mut tx_cost, transaction, demote_program_write_locks); + self.get_write_lock_cost(&mut tx_cost, transaction); tx_cost.data_bytes_cost = self.get_data_bytes_cost(transaction); tx_cost.execution_cost = self.get_transaction_cost(transaction); tx_cost.cost_weight = self.calculate_cost_weight(transaction); @@ -154,11 +150,10 @@ impl CostModel { &self, tx_cost: &mut TransactionCost, transaction: &SanitizedTransaction, - demote_program_write_locks: bool, ) { let message = transaction.message(); message.account_keys_iter().enumerate().for_each(|(i, k)| { - let is_writable = message.is_writable(i, demote_program_write_locks); + let is_writable = message.is_writable(i); if is_writable { tx_cost.writable_accounts.push(*k); @@ -487,7 +482,7 @@ mod tests { ); let cost_model = CostModel::default(); - let tx_cost = cost_model.calculate_cost(&tx, /*demote_program_write_locks=*/ true); + let tx_cost = cost_model.calculate_cost(&tx); assert_eq!(2 + 2, tx_cost.writable_accounts.len()); assert_eq!(signer1.pubkey(), tx_cost.writable_accounts[0]); assert_eq!(signer2.pubkey(), tx_cost.writable_accounts[1]); @@ -531,7 +526,7 @@ mod tests { cost_model .upsert_instruction_cost(&system_program::id(), expected_execution_cost) .unwrap(); - let tx_cost = cost_model.calculate_cost(&tx, /*demote_program_write_locks=*/ true); + let tx_cost = cost_model.calculate_cost(&tx); assert_eq!(expected_account_cost, tx_cost.write_lock_cost); assert_eq!(expected_execution_cost, tx_cost.execution_cost); assert_eq!(2, tx_cost.writable_accounts.len()); @@ -600,8 +595,7 @@ mod tests { } else { thread::spawn(move || { let cost_model = cost_model.write().unwrap(); - let tx_cost = cost_model - .calculate_cost(&tx, /*demote_program_write_locks=*/ true); + let tx_cost = cost_model.calculate_cost(&tx); assert_eq!(3, tx_cost.writable_accounts.len()); assert_eq!(expected_account_cost, tx_cost.write_lock_cost); }) diff --git a/sdk/benches/serialize_instructions.rs b/sdk/benches/serialize_instructions.rs index 0788bdc5a..d780d36f8 100644 --- a/sdk/benches/serialize_instructions.rs +++ b/sdk/benches/serialize_instructions.rs @@ -19,8 +19,6 @@ fn make_instructions() -> Vec { vec![inst; 4] } -const DEMOTE_PROGRAM_WRITE_LOCKS: bool = true; - #[bench] fn bench_bincode_instruction_serialize(b: &mut Bencher) { let instructions = make_instructions(); @@ -36,7 +34,7 @@ fn bench_manual_instruction_serialize(b: &mut Bencher) { SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique()))) .unwrap(); b.iter(|| { - test::black_box(message.serialize_instructions(DEMOTE_PROGRAM_WRITE_LOCKS)); + test::black_box(message.serialize_instructions()); }); } @@ -55,7 +53,7 @@ fn bench_manual_instruction_deserialize(b: &mut Bencher) { let message = SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique()))) .unwrap(); - let serialized = message.serialize_instructions(DEMOTE_PROGRAM_WRITE_LOCKS); + let serialized = message.serialize_instructions(); b.iter(|| { for i in 0..instructions.len() { #[allow(deprecated)] @@ -70,7 +68,7 @@ fn bench_manual_instruction_deserialize_single(b: &mut Bencher) { let message = SanitizedMessage::try_from(Message::new(&instructions, Some(&Pubkey::new_unique()))) .unwrap(); - let serialized = message.serialize_instructions(DEMOTE_PROGRAM_WRITE_LOCKS); + let serialized = message.serialize_instructions(); b.iter(|| { #[allow(deprecated)] test::black_box(instructions::load_instruction_at(3, &serialized).unwrap()); diff --git a/sdk/program/src/message/legacy.rs b/sdk/program/src/message/legacy.rs index a02ff8d6a..cd9d65e71 100644 --- a/sdk/program/src/message/legacy.rs +++ b/sdk/program/src/message/legacy.rs @@ -367,10 +367,9 @@ impl Message { self.program_position(i).is_some() } - pub fn is_writable(&self, i: usize, demote_program_write_locks: bool) -> bool { - let demote_program_id = demote_program_write_locks - && self.is_key_called_as_program(i) - && !self.is_upgradeable_loader_present(); + pub fn is_writable(&self, i: usize) -> bool { + let demote_program_id = + self.is_key_called_as_program(i) && !self.is_upgradeable_loader_present(); (i < (self.header.num_required_signatures - self.header.num_readonly_signed_accounts) as usize || (i >= self.header.num_required_signatures as usize @@ -392,7 +391,7 @@ impl Message { let mut writable_keys = vec![]; let mut readonly_keys = vec![]; for (i, key) in self.account_keys.iter().enumerate() { - if self.is_writable(i, /*demote_program_write_locks=*/ true) { + if self.is_writable(i) { writable_keys.push(key); } else { readonly_keys.push(key); @@ -430,8 +429,7 @@ impl Message { for account_index in &instruction.accounts { let account_index = *account_index as usize; let is_signer = self.is_signer(account_index); - let is_writable = - self.is_writable(account_index, /*demote_program_write_locks=*/ true); + let is_writable = self.is_writable(account_index); let mut meta_byte = 0; if is_signer { meta_byte |= 1 << Self::IS_SIGNER_BIT; @@ -894,13 +892,12 @@ mod tests { recent_blockhash: Hash::default(), instructions: vec![], }; - let demote_program_write_locks = true; - assert!(message.is_writable(0, demote_program_write_locks)); - assert!(!message.is_writable(1, demote_program_write_locks)); - assert!(!message.is_writable(2, demote_program_write_locks)); - assert!(message.is_writable(3, demote_program_write_locks)); - assert!(message.is_writable(4, demote_program_write_locks)); - assert!(!message.is_writable(5, demote_program_write_locks)); + assert!(message.is_writable(0)); + assert!(!message.is_writable(1)); + assert!(!message.is_writable(2)); + assert!(message.is_writable(3)); + assert!(message.is_writable(4)); + assert!(!message.is_writable(5)); } #[test] diff --git a/sdk/program/src/message/sanitized.rs b/sdk/program/src/message/sanitized.rs index 25ef25c0a..b09902774 100644 --- a/sdk/program/src/message/sanitized.rs +++ b/sdk/program/src/message/sanitized.rs @@ -187,10 +187,10 @@ impl SanitizedMessage { /// Returns true if the account at the specified index is writable by the /// instructions in this message. - pub fn is_writable(&self, index: usize, demote_program_write_locks: bool) -> bool { + pub fn is_writable(&self, index: usize) -> bool { match self { - Self::Legacy(message) => message.is_writable(index, demote_program_write_locks), - Self::V0(message) => message.is_writable(index, demote_program_write_locks), + Self::Legacy(message) => message.is_writable(index), + Self::V0(message) => message.is_writable(index), } } @@ -214,7 +214,7 @@ impl SanitizedMessage { // 67..69 - data len - u16 // 69..data_len - data #[allow(clippy::integer_arithmetic)] - pub fn serialize_instructions(&self, demote_program_write_locks: bool) -> Vec { + pub fn serialize_instructions(&self) -> Vec { // 64 bytes is a reasonable guess, calculating exactly is slower in benchmarks let mut data = Vec::with_capacity(self.instructions().len() * (32 * 2)); append_u16(&mut data, self.instructions().len() as u16); @@ -229,7 +229,7 @@ impl SanitizedMessage { for account_index in &instruction.accounts { let account_index = *account_index as usize; let is_signer = self.is_signer(account_index); - let is_writable = self.is_writable(account_index, demote_program_write_locks); + let is_writable = self.is_writable(account_index); let mut account_meta = InstructionsSysvarAccountMeta::NONE; if is_signer { account_meta |= InstructionsSysvarAccountMeta::IS_SIGNER; @@ -426,10 +426,9 @@ mod tests { ), ]; - let demote_program_write_locks = true; let message = LegacyMessage::new(&instructions, Some(&id1)); let sanitized_message = SanitizedMessage::try_from(message.clone()).unwrap(); - let serialized = sanitized_message.serialize_instructions(demote_program_write_locks); + let serialized = sanitized_message.serialize_instructions(); // assert that SanitizedMessage::serialize_instructions has the same behavior as the // deprecated Message::serialize_instructions method diff --git a/sdk/program/src/message/versions/v0/loaded.rs b/sdk/program/src/message/versions/v0/loaded.rs index d0b39fcbd..7bb62b8b0 100644 --- a/sdk/program/src/message/versions/v0/loaded.rs +++ b/sdk/program/src/message/versions/v0/loaded.rs @@ -104,11 +104,10 @@ impl LoadedMessage { } /// Returns true if the account at the specified index was loaded as writable - pub fn is_writable(&self, key_index: usize, demote_program_write_locks: bool) -> bool { + pub fn is_writable(&self, key_index: usize) -> bool { if self.is_writable_index(key_index) { if let Some(key) = self.get_account_key(key_index) { - let demote_program_id = demote_program_write_locks - && self.is_key_called_as_program(key_index) + let demote_program_id = self.is_key_called_as_program(key_index) && !self.is_upgradeable_loader_present(); return !(sysvar::is_sysvar_id(key) || BUILTIN_PROGRAMS_KEYS.contains(key) @@ -269,11 +268,11 @@ mod tests { message.message.account_keys[0] = sysvar::clock::id(); assert!(message.is_writable_index(0)); - assert!(!message.is_writable(0, /*demote_program_write_locks=*/ true)); + assert!(!message.is_writable(0)); message.message.account_keys[0] = system_program::id(); assert!(message.is_writable_index(0)); - assert!(!message.is_writable(0, /*demote_program_write_locks=*/ true)); + assert!(!message.is_writable(0)); } #[test] @@ -303,6 +302,6 @@ mod tests { }; assert!(message.is_writable_index(2)); - assert!(!message.is_writable(2, /*demote_program_write_locks=*/ true)); + assert!(!message.is_writable(2)); } } diff --git a/sdk/program/src/sysvar/instructions.rs b/sdk/program/src/sysvar/instructions.rs index bd50d87fb..db2ba248f 100644 --- a/sdk/program/src/sysvar/instructions.rs +++ b/sdk/program/src/sysvar/instructions.rs @@ -13,11 +13,8 @@ crate::declare_sysvar_id!("Sysvar1nstructions1111111111111111111111111", Instruc // Construct the account data for the Instruction sSysvar #[cfg(not(target_arch = "bpf"))] -pub fn construct_instructions_data( - message: &crate::message::SanitizedMessage, - demote_program_write_locks: bool, -) -> Vec { - let mut data = message.serialize_instructions(demote_program_write_locks); +pub fn construct_instructions_data(message: &crate::message::SanitizedMessage) -> Vec { + let mut data = message.serialize_instructions(); // add room for current instruction index. data.resize(data.len() + 2, 0); @@ -154,7 +151,7 @@ mod tests { let key = id(); let mut lamports = 0; - let mut data = construct_instructions_data(&sanitized_message, true); + let mut data = construct_instructions_data(&sanitized_message); let owner = crate::sysvar::id(); let mut account_info = AccountInfo::new( &key, @@ -208,7 +205,7 @@ mod tests { let key = id(); let mut lamports = 0; - let mut data = construct_instructions_data(&sanitized_message, true); + let mut data = construct_instructions_data(&sanitized_message); store_current_index(&mut data, 1); let owner = crate::sysvar::id(); let mut account_info = AccountInfo::new( @@ -266,7 +263,7 @@ mod tests { let key = id(); let mut lamports = 0; - let mut data = construct_instructions_data(&sanitized_message, true); + let mut data = construct_instructions_data(&sanitized_message); store_current_index(&mut data, 1); let owner = crate::sysvar::id(); let mut account_info = AccountInfo::new( diff --git a/sdk/src/transaction/mod.rs b/sdk/src/transaction/mod.rs index 8fdb7306e..eba6a2c0d 100644 --- a/sdk/src/transaction/mod.rs +++ b/sdk/src/transaction/mod.rs @@ -410,7 +410,7 @@ pub fn uses_durable_nonce(tx: &Transaction) -> Option<&CompiledInstruction> { // Nonce account is writable && matches!( instruction.accounts.get(0), - Some(index) if message.is_writable(*index as usize, true) + Some(index) if message.is_writable(*index as usize) ) }) } diff --git a/sdk/src/transaction/sanitized.rs b/sdk/src/transaction/sanitized.rs index 0adf97b4d..f904aac25 100644 --- a/sdk/src/transaction/sanitized.rs +++ b/sdk/src/transaction/sanitized.rs @@ -140,7 +140,7 @@ impl SanitizedTransaction { } /// Return the list of accounts that must be locked during processing this transaction. - pub fn get_account_locks(&self, demote_program_write_locks: bool) -> TransactionAccountLocks { + pub fn get_account_locks(&self) -> TransactionAccountLocks { let message = &self.message; let num_readonly_accounts = message.num_readonly_accounts(); let num_writable_accounts = message @@ -153,7 +153,7 @@ impl SanitizedTransaction { }; for (i, key) in message.account_keys_iter().enumerate() { - if message.is_writable(i, demote_program_write_locks) { + if message.is_writable(i) { account_locks.writable.push(key); } else { account_locks.readonly.push(key); @@ -183,7 +183,7 @@ impl SanitizedTransaction { .and_then(|ix| { ix.accounts.get(0).and_then(|idx| { let idx = *idx as usize; - if nonce_must_be_writable && !self.message.is_writable(idx, true) { + if nonce_must_be_writable && !self.message.is_writable(idx) { None } else { self.message.get_account_key(idx) diff --git a/transaction-status/src/parse_accounts.rs b/transaction-status/src/parse_accounts.rs index b6c315db4..35d8d4c9e 100644 --- a/transaction-status/src/parse_accounts.rs +++ b/transaction-status/src/parse_accounts.rs @@ -13,7 +13,7 @@ pub fn parse_accounts(message: &Message) -> Vec { for (i, account_key) in message.account_keys.iter().enumerate() { accounts.push(ParsedAccount { pubkey: account_key.to_string(), - writable: message.is_writable(i, /*demote_program_write_locks=*/ true), + writable: message.is_writable(i), signer: message.is_signer(i), }); }