Demote write locks on transaction program ids

cli-output/src/display.rs

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

core/src/banking_stage.rs

@@ -1049,6 +1049,7 @@ impl BankingStage {
         libsecp256k1_fail_on_bad_count: bool,
         cost_tracker: &Arc<RwLock<CostTracker>>,
         banking_stage_stats: &BankingStageStats,
+        demote_program_write_locks: bool,
     ) -> (Vec<SanitizedTransaction>, Vec<usize>, Vec<usize>) {
         let mut retryable_transaction_packet_indexes: Vec<usize> = vec![];
 
@@ -1082,7 +1083,8 @@ impl BankingStage {
             verified_transactions_with_packet_indexes
                 .into_iter()
                 .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() {
                         debug!("transaction {:?} would exceed limit: {:?}", tx, result);
                         retryable_transaction_packet_indexes.push(tx_index);
@@ -1165,6 +1167,7 @@ impl BankingStage {
                 bank.libsecp256k1_fail_on_bad_count(),
                 cost_tracker,
                 banking_stage_stats,
+                bank.demote_program_write_locks(),
             );
         packet_conversion_time.stop();
         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");
         transactions.iter().enumerate().for_each(|(index, tx)| {
             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();
@@ -1268,6 +1274,7 @@ impl BankingStage {
                 bank.libsecp256k1_fail_on_bad_count(),
                 cost_tracker,
                 banking_stage_stats,
+                bank.demote_program_write_locks(),
             );
         unprocessed_packet_conversion_time.stop();
 

core/src/cost_model.rs

@@ -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();
 
         // calculate transaction exeution cost
@@ -122,7 +126,7 @@ impl CostModel {
         // calculate account access cost
         let message = transaction.message();
         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 {
                 self.transaction_cost.writable_accounts.push(*k);
@@ -353,7 +357,7 @@ mod tests {
         .unwrap();
 
         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!(signer1.pubkey(), tx_cost.writable_accounts[0]);
         assert_eq!(signer2.pubkey(), tx_cost.writable_accounts[1]);
@@ -395,7 +399,7 @@ mod tests {
         cost_model
             .upsert_instruction_cost(&system_program::id(), expected_execution_cost)
             .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_execution_cost, tx_cost.execution_cost);
         assert_eq!(2, tx_cost.writable_accounts.len());
@@ -465,7 +469,8 @@ mod tests {
                 } else {
                     thread::spawn(move || {
                         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!(expected_account_cost, tx_cost.account_access_cost);
                     })

core/src/cost_tracker.rs

@@ -46,18 +46,23 @@ impl CostTracker {
     pub fn would_transaction_fit(
         &self,
         transaction: &SanitizedTransaction,
+        demote_program_write_locks: bool,
     ) -> Result<(), CostModelError> {
         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(
             &tx_cost.writable_accounts,
             &(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 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;
         for account_key in tx_cost.writable_accounts.iter() {
             *self

ledger-tool/src/main.rs

@@ -792,7 +792,10 @@ 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);
+                let tx_cost = cost_model.calculate_cost(
+                    &transaction,
+                    true, // demote_program_write_locks
+                );
                 if cost_tracker.try_add(tx_cost).is_err() {
                     println!(
                         "Slot: {}, CostModel rejected transaction {:?}, stats {:?}!",

program-runtime/src/instruction_processor.rs

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

program-test/src/lib.rs

@@ -24,6 +24,7 @@ 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,
@@ -261,12 +262,14 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
             }
             panic!("Program id {} wasn't found in account_infos", program_id);
         };
+        let demote_program_write_locks =
+            invoke_context.is_feature_active(&demote_program_write_locks::id());
         // TODO don't have the caller's keyed_accounts so can't validate writer or signer escalation or deescalation yet
         let caller_privileges = message
             .account_keys
             .iter()
             .enumerate()
-            .map(|(i, _)| message.is_writable(i))
+            .map(|(i, _)| message.is_writable(i, demote_program_write_locks))
             .collect::<Vec<bool>>();
 
         stable_log::program_invoke(&logger, &program_id, invoke_context.invoke_depth());
@@ -337,7 +340,7 @@ impl solana_sdk::program_stubs::SyscallStubs for SyscallStubs {
 
         // Copy writeable account modifications back into the caller's AccountInfos
         for (i, (pubkey, account)) in accounts.iter().enumerate().take(message.account_keys.len()) {
-            if !message.is_writable(i) {
+            if !message.is_writable(i, demote_program_write_locks) {
                 continue;
             }
             for account_info in account_infos {

programs/bpf/tests/programs.rs

@@ -1796,7 +1796,7 @@ fn test_program_bpf_upgrade_and_invoke_in_same_tx() {
         "solana_bpf_rust_panic",
     );
 
-    // Invoke, then upgrade the program, and then invoke again in same tx
+    // Attempt to invoke, then upgrade the program in same tx
     let message = Message::new(
         &[
             invoke_instruction.clone(),
@@ -1815,10 +1815,12 @@ fn test_program_bpf_upgrade_and_invoke_in_same_tx() {
         message.clone(),
         bank.last_blockhash(),
     );
+    // program_id is automatically demoted to readonly, preventing the upgrade, which requires
+    // writeability
     let (result, _) = process_transaction_and_record_inner(&bank, tx);
     assert_eq!(
         result.unwrap_err(),
-        TransactionError::InstructionError(2, InstructionError::ProgramFailedToComplete)
+        TransactionError::InstructionError(1, InstructionError::InvalidArgument)
     );
 }
 
@@ -2114,96 +2116,6 @@ fn test_program_bpf_upgrade_via_cpi() {
     assert_ne!(programdata, original_programdata);
 }
 
-#[cfg(feature = "bpf_rust")]
-#[test]
-fn test_program_bpf_upgrade_self_via_cpi() {
-    solana_logger::setup();
-
-    let GenesisConfigInfo {
-        genesis_config,
-        mint_keypair,
-        ..
-    } = create_genesis_config(50);
-    let mut bank = Bank::new_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")]
 #[test]
 fn test_program_bpf_set_upgrade_authority_via_cpi() {