Clean up demote program write lock feature (backport #21949) (#21969)

* Clean up demote program write lock feature (#21949)

* Clean up demote program write lock feature

* fix test

(cherry picked from commit 6ff0be6)

# Conflicts:
#	programs/bpf_loader/src/syscalls.rs
#	runtime/src/accounts.rs

* resolve conflicts

Co-authored-by: Justin Starry <[email protected]>

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 {
             "-"

core/src/banking_stage.rs

@@ -935,8 +935,7 @@ impl BankingStage {
         gossip_vote_sender: &ReplayVoteSender,
         qos_service: &Arc<QosService>,
     ) -> (Result<usize, PohRecorderError>, Vec<usize>) {
-        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);

core/src/qos_service.rs

@@ -78,13 +78,12 @@ impl QosService {
     pub fn compute_transaction_costs<'a>(
         &self,
         transactions: impl Iterator<Item = &'a SanitizedTransaction>,
-        demote_program_write_locks: bool,
     ) -> Vec<TransactionCost> {
         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,
@@ -250,7 +249,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());
@@ -260,11 +259,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();
@@ -295,14 +290,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;
@@ -348,7 +343,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 {}",
@@ -365,7 +360,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 {}",

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!(

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<AccountSharedData>,
                 )
@@ -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,
@@ -666,11 +659,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<bool> = (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)?;
                 }
@@ -1136,7 +1126,7 @@ mod tests {
                 None,
             );
             let write_privileges: Vec<bool> = (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
@@ -1258,14 +1248,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::<Vec<bool>>();
         accounts[0].1.borrow_mut().data_as_mut_slice()[0] = 1;
         assert_eq!(
@@ -1319,7 +1306,7 @@ mod tests {
                 .account_keys
                 .iter()
                 .enumerate()
-                .map(|(i, _)| message.is_writable(i, demote_program_write_locks))
+                .map(|(i, _)| message.is_writable(i))
                 .collect::<Vec<bool>>();
             assert_eq!(
                 invoke_context.process_instruction(

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::<Vec<bool>>();
 
         stable_log::program_invoke(&log_collector, &program_id, invoke_context.invoke_depth());
@@ -278,7 +274,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

programs/bpf_loader/src/syscalls.rs

@@ -24,10 +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, 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,
+            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},
@@ -2179,9 +2179,6 @@ fn get_translated_accounts<'a, T, F>(
 where
     F: Fn(&T, &InvokeContext) -> Result<CallerAccount<'a>, EbpfError<BpfError>>,
 {
-    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)?;
@@ -2209,7 +2206,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)

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