Drop write lock on sysvars (#15497)

* Drop write lock on sysvars

* adds env var for demoting sysvar write lock demotion

* moves demote logic to is_writable

* feature gates sysvar write lock demotion

* adds builtins to write lock demotion

* adds system program id to builtins

* adds Feature111...

* adds an abi-freeze test

* mvines set of builtin program keys

Co-authored-by: Michael Vines <mvines@gmail.com>

* update tests

* adds bpf loader keys

* Add test sysvar

* Plumb demote_sysvar to is_writable

* more plumbing of demote_sysvar_write_locks to is_writable

* patches test_program_bpf_instruction_introspection

* hard codes demote_sysvar_write_locks to false for serialization/encoding methods

* Revert "hard codes demote_sysvar_write_locks to false for serialization/encoding methods"

This reverts commit ae3e2d2e777437bddd753933097a210dcbc1b1fc.

* change the hardcoded ones to demote_sysvar_write_locks=true

* Use data_as_mut_slice

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
Co-authored-by: Michael Vines <mvines@gmail.com>

cli-output/src/display.rs

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

core/src/transaction_status_service.rs

@@ -106,8 +106,9 @@ impl TransactionStatusService {
                             })
                             .expect("FeeCalculator must exist");
                         let fee = fee_calculator.calculate_fee(transaction.message());
-                        let (writable_keys, readonly_keys) =
+                        let (writable_keys, readonly_keys) = transaction
-                            transaction.message.get_account_keys_by_lock_type();
+                            .message
+                            .get_account_keys_by_lock_type(bank.demote_sysvar_write_locks());
 
                         let inner_instructions = inner_instructions.map(|inner_instructions| {
                             inner_instructions

program-test/src/lib.rs

@@ -30,6 +30,7 @@ use {
     solana_sdk::{
         account::{Account, AccountSharedData, ReadableAccount},
         clock::Slot,
+        feature_set::demote_sysvar_write_locks,
         genesis_config::{ClusterType, GenesisConfig},
         keyed_account::KeyedAccount,
         process_instruction::{
@@ -244,12 +245,14 @@ impl program_stubs::SyscallStubs for SyscallStubs {
             }
             panic!("Program id {} wasn't found in account_infos", program_id);
         };
+        let demote_sysvar_write_locks =
+            invoke_context.is_feature_active(&demote_sysvar_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_sysvar_write_locks))
             .collect::<Vec<bool>>();
 
         stable_log::program_invoke(&logger, &program_id, invoke_context.invoke_depth());

programs/bpf/tests/programs.rs

@@ -1323,7 +1323,9 @@ fn test_program_bpf_instruction_introspection() {
     let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction);
     assert_eq!(
         result.unwrap_err().unwrap(),
-        TransactionError::InvalidAccountIndex
+        // sysvar write locks are demoted to read only. So this will no longer
+        // cause InvalidAccountIndex error.
+        TransactionError::InstructionError(0, InstructionError::ProgramFailedToComplete),
     );
 
     // No accounts, should error

programs/bpf_loader/src/syscalls.rs

@@ -16,7 +16,10 @@ use solana_sdk::{
     bpf_loader, bpf_loader_deprecated,
     bpf_loader_upgradeable::{self, UpgradeableLoaderState},
     entrypoint::{MAX_PERMITTED_DATA_INCREASE, SUCCESS},
-    feature_set::{cpi_data_cost, cpi_share_ro_and_exec_accounts, ristretto_mul_syscall_enabled},
+    feature_set::{
+        cpi_data_cost, cpi_share_ro_and_exec_accounts, demote_sysvar_write_locks,
+        ristretto_mul_syscall_enabled,
+    },
     hash::{Hasher, HASH_BYTES},
     ic_msg,
     instruction::{AccountMeta, Instruction, InstructionError},
@@ -1571,7 +1574,14 @@ fn call<'a>(
     signers_seeds_len: u64,
     memory_mapping: &MemoryMapping,
 ) -> Result<u64, EbpfError<BpfError>> {
-    let (message, executables, accounts, account_refs, caller_write_privileges) = {
+    let (
+        message,
+        executables,
+        accounts,
+        account_refs,
+        caller_write_privileges,
+        demote_sysvar_write_locks,
+    ) = {
         let invoke_context = syscall.get_context()?;
 
         invoke_context
@@ -1653,6 +1663,7 @@ fn call<'a>(
             accounts,
             account_refs,
             caller_write_privileges,
+            invoke_context.is_feature_active(&demote_sysvar_write_locks::id()),
         )
     };
 
@@ -1678,7 +1689,7 @@ fn call<'a>(
         for (i, (account, account_ref)) in accounts.iter().zip(account_refs).enumerate() {
             let account = account.borrow();
             if let Some(account_ref) = account_ref {
-                if message.is_writable(i) && !account.executable {
+                if message.is_writable(i, demote_sysvar_write_locks) && !account.executable {
                     *account_ref.lamports = account.lamports;
                     *account_ref.owner = account.owner;
                     if account_ref.data.len() != account.data().len() {

runtime/src/accounts.rs

@@ -163,8 +163,11 @@ impl Accounts {
         false
     }
 
-    fn construct_instructions_account(message: &Message) -> AccountSharedData {
+    fn construct_instructions_account(
-        let mut data = message.serialize_instructions();
+        message: &Message,
+        demote_sysvar_write_locks: bool,
+    ) -> AccountSharedData {
+        let mut data = message.serialize_instructions(demote_sysvar_write_locks);
         // add room for current instruction index.
         data.resize(data.len() + 2, 0);
         AccountSharedData::from(Account {
@@ -193,6 +196,8 @@ impl Accounts {
             let mut tx_rent: TransactionRent = 0;
             let mut accounts = Vec::with_capacity(message.account_keys.len());
             let mut account_deps = Vec::with_capacity(message.account_keys.len());
+            let demote_sysvar_write_locks =
+                feature_set.is_active(&feature_set::demote_sysvar_write_locks::id());
 
             for (i, key) in message.account_keys.iter().enumerate() {
                 let account = if message.is_non_loader_key(key, i) {
@@ -203,16 +208,16 @@ impl Accounts {
                     if solana_sdk::sysvar::instructions::check_id(key)
                         && feature_set.is_active(&feature_set::instructions_sysvar_enabled::id())
                     {
-                        if message.is_writable(i) {
+                        if message.is_writable(i, demote_sysvar_write_locks) {
                             return Err(TransactionError::InvalidAccountIndex);
                         }
-                        Self::construct_instructions_account(message)
+                        Self::construct_instructions_account(message, demote_sysvar_write_locks)
                     } else {
                         let (account, rent) = self
                             .accounts_db
                             .load(ancestors, key)
                             .map(|(mut account, _)| {
-                                if message.is_writable(i) {
+                                if message.is_writable(i, demote_sysvar_write_locks) {
                                     let rent_due = rent_collector
                                         .collect_from_existing_account(&key, &mut account);
                                     (account, rent_due)
@@ -753,13 +758,21 @@ impl Accounts {
         Ok(())
     }
 
-    fn unlock_account(&self, tx: &Transaction, result: &Result<()>, locks: &mut AccountLocks) {
+    fn unlock_account(
+        &self,
+        tx: &Transaction,
+        result: &Result<()>,
+        locks: &mut AccountLocks,
+        demote_sysvar_write_locks: bool,
+    ) {
         match result {
             Err(TransactionError::AccountInUse) => (),
             Err(TransactionError::SanitizeFailure) => (),
             Err(TransactionError::AccountLoadedTwice) => (),
             _ => {
-                let (writable_keys, readonly_keys) = &tx.message().get_account_keys_by_lock_type();
+                let (writable_keys, readonly_keys) = &tx
+                    .message()
+                    .get_account_keys_by_lock_type(demote_sysvar_write_locks);
                 for k in writable_keys {
                     locks.unlock_write(k);
                 }
@@ -792,6 +805,7 @@ impl Accounts {
         &self,
         txs: &[Transaction],
         txs_iteration_order: Option<&[usize]>,
+        demote_sysvar_write_locks: bool,
     ) -> Vec<Result<()>> {
         use solana_sdk::sanitize::Sanitize;
         let keys: Vec<Result<_>> = OrderedIterator::new(txs, txs_iteration_order)
@@ -802,7 +816,9 @@ impl Accounts {
                     return Err(TransactionError::AccountLoadedTwice);
                 }
 
-                Ok(tx.message().get_account_keys_by_lock_type())
+                Ok(tx
+                    .message()
+                    .get_account_keys_by_lock_type(demote_sysvar_write_locks))
             })
             .collect();
         let mut account_locks = &mut self.account_locks.lock().unwrap();
@@ -822,13 +838,16 @@ impl Accounts {
         txs: &[Transaction],
         txs_iteration_order: Option<&[usize]>,
         results: &[Result<()>],
+        demote_sysvar_write_locks: bool,
     ) {
         let mut account_locks = self.account_locks.lock().unwrap();
         debug!("bank unlock accounts");
 
         OrderedIterator::new(txs, txs_iteration_order)
             .zip(results.iter())
-            .for_each(|((_, tx), result)| self.unlock_account(tx, result, &mut account_locks));
+            .for_each(|((_, tx), result)| {
+                self.unlock_account(tx, result, &mut account_locks, demote_sysvar_write_locks)
+            });
     }
 
     /// Store the accounts into the DB
@@ -844,6 +863,7 @@ impl Accounts {
         rent_collector: &RentCollector,
         last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
         fix_recent_blockhashes_sysvar_delay: bool,
+        demote_sysvar_write_locks: bool,
     ) {
         let accounts_to_store = self.collect_accounts_to_store(
             txs,
@@ -853,6 +873,7 @@ impl Accounts {
             rent_collector,
             last_blockhash_with_fee_calculator,
             fix_recent_blockhashes_sysvar_delay,
+            demote_sysvar_write_locks,
         );
         self.accounts_db.store_cached(slot, &accounts_to_store);
     }
@@ -877,6 +898,7 @@ impl Accounts {
         rent_collector: &RentCollector,
         last_blockhash_with_fee_calculator: &(Hash, FeeCalculator),
         fix_recent_blockhashes_sysvar_delay: bool,
+        demote_sysvar_write_locks: bool,
     ) -> Vec<(&'a Pubkey, &'a AccountSharedData)> {
         let mut accounts = Vec::with_capacity(loaded.len());
         for (i, ((raccs, _nonce_rollback), (_, tx))) in loaded
@@ -927,7 +949,7 @@ impl Accounts {
                     fee_payer_index = Some(i);
                 }
                 let is_fee_payer = Some(i) == fee_payer_index;
-                if message.is_writable(i)
+                if message.is_writable(i, demote_sysvar_write_locks)
                     && (res.is_ok()
                         || (maybe_nonce_rollback.is_some() && (is_nonce_account || is_fee_payer)))
                 {
@@ -1665,7 +1687,11 @@ mod tests {
             instructions,
         );
         let tx = Transaction::new(&[&keypair0], message, Hash::default());
-        let results0 = accounts.lock_accounts(&[tx.clone()], None);
+        let results0 = accounts.lock_accounts(
+            &[tx.clone()],
+            None, // txs_iteration_order
+            true, // demote_sysvar_write_locks
+        );
 
         assert!(results0[0].is_ok());
         assert_eq!(
@@ -1700,7 +1726,10 @@ mod tests {
         );
         let tx1 = Transaction::new(&[&keypair1], message, Hash::default());
         let txs = vec![tx0, tx1];
-        let results1 = accounts.lock_accounts(&txs, None);
+        let results1 = accounts.lock_accounts(
+            &txs, None, // txs_iteration_order
+            true, // demote_sysvar_write_locks
+        );
 
         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
@@ -1715,9 +1744,16 @@ mod tests {
             2
         );
 
-        accounts.unlock_accounts(&[tx], None, &results0);
+        accounts.unlock_accounts(
-        accounts.unlock_accounts(&txs, None, &results1);
+            &[tx],
-
+            None, // txs_iteration_order
+            &results0,
+            true, // demote_sysvar_write_locks
+        );
+        accounts.unlock_accounts(
+            &txs, None, // txs_iteration_order
+            &results1, true, // demote_sysvar_write_locks
+        );
         let instructions = vec![CompiledInstruction::new(2, &(), vec![0, 1])];
         let message = Message::new_with_compiled_instructions(
             1,
@@ -1728,8 +1764,11 @@ mod tests {
             instructions,
         );
         let tx = Transaction::new(&[&keypair1], message, Hash::default());
-        let results2 = accounts.lock_accounts(&[tx], None);
+        let results2 = accounts.lock_accounts(
-
+            &[tx],
+            None, // txs_iteration_order
+            true, // demote_sysvar_write_locks
+        );
         assert!(results2[0].is_ok()); // Now keypair1 account can be locked as writable
 
         // Check that read-only lock with zero references is deleted
@@ -1793,13 +1832,19 @@ mod tests {
             let exit_clone = exit_clone.clone();
             loop {
                 let txs = vec![writable_tx.clone()];
-                let results = accounts_clone.clone().lock_accounts(&txs, None);
+                let results = accounts_clone.clone().lock_accounts(
+                    &txs, None, // txs_iteration_order
+                    true, // demote_sysvar_write_locks
+                );
                 for result in results.iter() {
                     if result.is_ok() {
                         counter_clone.clone().fetch_add(1, Ordering::SeqCst);
                     }
                 }
-                accounts_clone.unlock_accounts(&txs, None, &results);
+                accounts_clone.unlock_accounts(
+                    &txs, None, // txs_iteration_order
+                    &results, true, // demote_sysvar_write_locks
+                );
                 if exit_clone.clone().load(Ordering::Relaxed) {
                     break;
                 }
@@ -1808,13 +1853,19 @@ mod tests {
         let counter_clone = counter;
         for _ in 0..5 {
             let txs = vec![readonly_tx.clone()];
-            let results = accounts_arc.clone().lock_accounts(&txs, None);
+            let results = accounts_arc.clone().lock_accounts(
+                &txs, None, // txs_iteration_order
+                true, // demote_sysvar_write_locks
+            );
             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, None, &results);
+            accounts_arc.unlock_accounts(
+                &txs, None, // txs_iteration_order
+                &results, true, // demote_sysvar_write_locks
+            );
             thread::sleep(time::Duration::from_millis(50));
         }
         exit.store(true, Ordering::Relaxed);
@@ -1902,6 +1953,7 @@ mod tests {
             &rent_collector,
             &(Hash::default(), FeeCalculator::default()),
             true,
+            true, // demote_sysvar_write_locks
         );
         assert_eq!(collected_accounts.len(), 2);
         assert!(collected_accounts
@@ -2267,6 +2319,7 @@ mod tests {
             &rent_collector,
             &(next_blockhash, FeeCalculator::default()),
             true,
+            true, // demote_sysvar_write_locks
         );
         assert_eq!(collected_accounts.len(), 2);
         assert_eq!(
@@ -2378,6 +2431,7 @@ mod tests {
             &rent_collector,
             &(next_blockhash, FeeCalculator::default()),
             true,
+            true, // demote_sysvar_write_locks
         );
         assert_eq!(collected_accounts.len(), 1);
         let collected_nonce_account = collected_accounts

runtime/src/bank.rs

@@ -2411,15 +2411,21 @@ impl Bank {
             .map_or(Ok(()), |sig| self.get_signature_status(sig).unwrap())
     }
 
+    pub fn demote_sysvar_write_locks(&self) -> bool {
+        self.feature_set
+            .is_active(&feature_set::demote_sysvar_write_locks::id())
+    }
+
     pub fn prepare_batch<'a, 'b>(
         &'a self,
         txs: &'b [Transaction],
         iteration_order: Option<Vec<usize>>,
     ) -> TransactionBatch<'a, 'b> {
-        let results = self
+        let results = self.rc.accounts.lock_accounts(
-            .rc
+            txs,
-            .accounts
+            iteration_order.as_deref(),
-            .lock_accounts(txs, iteration_order.as_deref());
+            self.demote_sysvar_write_locks(),
+        );
         TransactionBatch::new(results, &self, txs, iteration_order)
     }
 
@@ -2484,6 +2490,7 @@ impl Bank {
                 batch.transactions(),
                 batch.iteration_order(),
                 batch.lock_results(),
+                self.demote_sysvar_write_locks(),
             )
         }
     }
@@ -3210,6 +3217,7 @@ impl Bank {
             &self.rent_collector,
             &self.last_blockhash_with_fee_calculator(),
             self.fix_recent_blockhashes_sysvar_delay(),
+            self.demote_sysvar_write_locks(),
         );
         self.collect_rent(executed, loaded_accounts);
 
@@ -12262,4 +12270,72 @@ pub(crate) mod tests {
             vec![pubkeys_balances[3], pubkeys_balances[1]]
         );
     }
+
+    #[test]
+    fn test_transfer_sysvar() {
+        solana_logger::setup();
+        let GenesisConfigInfo {
+            genesis_config,
+            mint_keypair,
+            ..
+        } = create_genesis_config_with_leader(
+            1_000_000_000_000_000,
+            &Pubkey::new_unique(),
+            bootstrap_validator_stake_lamports(),
+        );
+        let mut bank = Bank::new(&genesis_config);
+
+        fn mock_ix_processor(
+            _pubkey: &Pubkey,
+            ka: &[KeyedAccount],
+            _data: &[u8],
+            _invoke_context: &mut dyn InvokeContext,
+        ) -> std::result::Result<(), InstructionError> {
+            use solana_sdk::account::WritableAccount;
+            let mut data = ka[1].try_account_ref_mut()?;
+            data.data_as_mut_slice()[0] = 5;
+            Ok(())
+        }
+
+        let program_id = solana_sdk::pubkey::new_rand();
+        bank.add_builtin("mock_program1", program_id, mock_ix_processor);
+
+        let blockhash = bank.last_blockhash();
+        let blockhash_sysvar = sysvar::recent_blockhashes::id();
+        let orig_lamports = bank
+            .get_account(&sysvar::recent_blockhashes::id())
+            .unwrap()
+            .lamports;
+        info!("{:?}", bank.get_account(&sysvar::recent_blockhashes::id()));
+        let tx = system_transaction::transfer(&mint_keypair, &blockhash_sysvar, 10, blockhash);
+        assert_eq!(
+            bank.process_transaction(&tx),
+            Err(TransactionError::InstructionError(
+                0,
+                InstructionError::ReadonlyLamportChange
+            ))
+        );
+        assert_eq!(
+            bank.get_account(&sysvar::recent_blockhashes::id())
+                .unwrap()
+                .lamports,
+            orig_lamports
+        );
+        info!("{:?}", bank.get_account(&sysvar::recent_blockhashes::id()));
+
+        let accounts = vec![
+            AccountMeta::new(mint_keypair.pubkey(), true),
+            AccountMeta::new(blockhash_sysvar, false),
+        ];
+        let ix = Instruction::new_with_bincode(program_id, &0, accounts);
+        let message = Message::new(&[ix], Some(&mint_keypair.pubkey()));
+        let tx = Transaction::new(&[&mint_keypair], message, blockhash);
+        assert_eq!(
+            bank.process_transaction(&tx),
+            Err(TransactionError::InstructionError(
+                0,
+                InstructionError::ReadonlyDataModified
+            ))
+        );
+    }
 }

runtime/src/message_processor.rs

@@ -8,7 +8,10 @@ use solana_sdk::{
     account::{AccountSharedData, ReadableAccount, WritableAccount},
     account_utils::StateMut,
     bpf_loader_upgradeable::{self, UpgradeableLoaderState},
-    feature_set::{cpi_share_ro_and_exec_accounts, instructions_sysvar_enabled, FeatureSet},
+    feature_set::{
+        cpi_share_ro_and_exec_accounts, demote_sysvar_write_locks, instructions_sysvar_enabled,
+        FeatureSet,
+    },
     ic_msg,
     instruction::{CompiledInstruction, Instruction, InstructionError},
     keyed_account::{create_keyed_readonly_accounts, KeyedAccount},
@@ -336,6 +339,7 @@ impl<'a> InvokeContext for ThisInvokeContext<'a> {
                 &self.rent,
                 caller_write_privileges,
                 &mut self.timings,
+                self.feature_set.is_active(&demote_sysvar_write_locks::id()),
             ),
             None => Err(InstructionError::GenericError), // Should never happen
         }
@@ -527,6 +531,7 @@ impl MessageProcessor {
         instruction: &'a CompiledInstruction,
         executable_accounts: &'a [(Pubkey, Rc<RefCell<AccountSharedData>>)],
         accounts: &'a [Rc<RefCell<AccountSharedData>>],
+        demote_sysvar_write_locks: bool,
     ) -> Vec<KeyedAccount<'a>> {
         let mut keyed_accounts = create_keyed_readonly_accounts(&executable_accounts);
         let mut keyed_accounts2: Vec<_> = instruction
@@ -537,7 +542,7 @@ impl MessageProcessor {
                 let index = index as usize;
                 let key = &message.account_keys[index];
                 let account = &accounts[index];
-                if message.is_writable(index) {
+                if message.is_writable(index, demote_sysvar_write_locks) {
                     KeyedAccount::new(key, is_signer, account)
                 } else {
                     KeyedAccount::new_readonly(key, is_signer, account)
@@ -682,7 +687,14 @@ impl MessageProcessor {
     ) -> Result<(), InstructionError> {
         let invoke_context = RefCell::new(invoke_context);
 
-        let (message, executables, accounts, account_refs, caller_write_privileges) = {
+        let (
+            message,
+            executables,
+            accounts,
+            account_refs,
+            caller_write_privileges,
+            demote_sysvar_write_locks,
+        ) = {
             let invoke_context = invoke_context.borrow();
 
             let caller_program_id = invoke_context.get_caller()?;
@@ -774,6 +786,7 @@ impl MessageProcessor {
                 accounts,
                 account_refs,
                 caller_write_privileges,
+                invoke_context.is_feature_active(&demote_sysvar_write_locks::id()),
             )
         };
 
@@ -792,7 +805,7 @@ impl MessageProcessor {
             let invoke_context = invoke_context.borrow();
             for (i, (account, account_ref)) in accounts.iter().zip(account_refs).enumerate() {
                 let account = account.borrow();
-                if message.is_writable(i) && !account.executable {
+                if message.is_writable(i, demote_sysvar_write_locks) && !account.executable {
                     account_ref.try_account_ref_mut()?.lamports = account.lamports;
                     account_ref.try_account_ref_mut()?.owner = account.owner;
                     if account_ref.data_len()? != account.data().len()
@@ -835,10 +848,16 @@ impl MessageProcessor {
                 accounts,
                 Some(caller_write_privileges),
             )?;
-
+            let demote_sysvar_write_locks =
+                invoke_context.is_feature_active(&demote_sysvar_write_locks::id());
             // Construct keyed accounts
-            let keyed_accounts =
+            let keyed_accounts = Self::create_keyed_accounts(
-                Self::create_keyed_accounts(message, instruction, executable_accounts, accounts);
+                message,
+                instruction,
+                executable_accounts,
+                accounts,
+                demote_sysvar_write_locks,
+            );
 
             // Invoke callee
             invoke_context.push(program_id)?;
@@ -908,6 +927,7 @@ impl MessageProcessor {
         accounts: &[Rc<RefCell<AccountSharedData>>],
         rent: &Rent,
         timings: &mut ExecuteDetailsTimings,
+        demote_sysvar_write_locks: bool,
     ) -> Result<(), InstructionError> {
         // Verify all executable accounts have zero outstanding refs
         Self::verify_account_references(executable_accounts)?;
@@ -926,7 +946,7 @@ impl MessageProcessor {
                 let account = accounts[account_index].borrow();
                 pre_accounts[unique_index].verify(
                     &program_id,
-                    message.is_writable(account_index),
+                    message.is_writable(account_index, demote_sysvar_write_locks),
                     rent,
                     &account,
                     timings,
@@ -955,6 +975,7 @@ impl MessageProcessor {
         rent: &Rent,
         caller_write_privileges: Option<&[bool]>,
         timings: &mut ExecuteDetailsTimings,
+        demote_sysvar_write_locks: bool,
     ) -> Result<(), InstructionError> {
         // Verify the per-account instruction results
         let (mut pre_sum, mut post_sum) = (0_u128, 0_u128);
@@ -965,7 +986,7 @@ impl MessageProcessor {
                 let is_writable = if let Some(caller_write_privileges) = caller_write_privileges {
                     caller_write_privileges[account_index]
                 } else {
-                    message.is_writable(account_index)
+                    message.is_writable(account_index, demote_sysvar_write_locks)
                 };
                 // Find the matching PreAccount
                 for pre_account in pre_accounts.iter_mut() {
@@ -1019,6 +1040,7 @@ impl MessageProcessor {
         feature_set: Arc<FeatureSet>,
         bpf_compute_budget: BpfComputeBudget,
         timings: &mut ExecuteDetailsTimings,
+        demote_sysvar_write_locks: bool,
     ) -> Result<(), InstructionError> {
         // Fixup the special instructions key if present
         // before the account pre-values are taken care of
@@ -1050,8 +1072,13 @@ impl MessageProcessor {
             instruction_recorder,
             feature_set,
         );
-        let keyed_accounts =
+        let keyed_accounts = Self::create_keyed_accounts(
-            Self::create_keyed_accounts(message, instruction, executable_accounts, accounts);
+            message,
+            instruction,
+            executable_accounts,
+            accounts,
+            demote_sysvar_write_locks,
+        );
         self.process_instruction(
             program_id,
             &keyed_accounts,
@@ -1066,6 +1093,7 @@ impl MessageProcessor {
             accounts,
             &rent_collector.rent,
             timings,
+            demote_sysvar_write_locks,
         )?;
 
         timings.accumulate(&invoke_context.timings);
@@ -1092,6 +1120,7 @@ impl MessageProcessor {
         bpf_compute_budget: BpfComputeBudget,
         timings: &mut ExecuteDetailsTimings,
     ) -> Result<(), TransactionError> {
+        let demote_sysvar_write_locks = feature_set.is_active(&demote_sysvar_write_locks::id());
         for (instruction_index, instruction) in message.instructions.iter().enumerate() {
             let instruction_recorder = instruction_recorders
                 .as_ref()
@@ -1110,6 +1139,7 @@ impl MessageProcessor {
                 feature_set.clone(),
                 bpf_compute_budget,
                 timings,
+                demote_sysvar_write_locks,
             )
             .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?;
         }
@@ -2071,12 +2101,13 @@ mod tests {
             &MockInstruction::NoopSuccess,
             metas.clone(),
         );
+        let demote_sysvar_write_locks = true;
         let message = Message::new(&[instruction], None);
         let caller_write_privileges = message
             .account_keys
             .iter()
             .enumerate()
-            .map(|(i, _)| message.is_writable(i))
+            .map(|(i, _)| message.is_writable(i, demote_sysvar_write_locks))
             .collect::<Vec<bool>>();
         assert_eq!(
             MessageProcessor::process_cross_program_instruction(
@@ -2111,7 +2142,7 @@ mod tests {
                 .account_keys
                 .iter()
                 .enumerate()
-                .map(|(i, _)| message.is_writable(i))
+                .map(|(i, _)| message.is_writable(i, demote_sysvar_write_locks))
                 .collect::<Vec<bool>>();
             assert_eq!(
                 MessageProcessor::process_cross_program_instruction(

sdk/benches/serialize_instructions.rs

@@ -27,7 +27,9 @@ fn bench_manual_instruction_serialize(b: &mut Bencher) {
     let instructions = make_instructions();
     let message = Message::new(&instructions, None);
     b.iter(|| {
-        test::black_box(message.serialize_instructions());
+        test::black_box(message.serialize_instructions(
+            true, // demote_sysvar_write_locks
+        ));
     });
 }
 
@@ -44,7 +46,9 @@ fn bench_bincode_instruction_deserialize(b: &mut Bencher) {
 fn bench_manual_instruction_deserialize(b: &mut Bencher) {
     let instructions = make_instructions();
     let message = Message::new(&instructions, None);
-    let serialized = message.serialize_instructions();
+    let serialized = message.serialize_instructions(
+        true, // demote_sysvar_write_locks
+    );
     b.iter(|| {
         for i in 0..instructions.len() {
             test::black_box(instructions::load_instruction_at(i, &serialized).unwrap());
@@ -56,7 +60,9 @@ fn bench_manual_instruction_deserialize(b: &mut Bencher) {
 fn bench_manual_instruction_deserialize_single(b: &mut Bencher) {
     let instructions = make_instructions();
     let message = Message::new(&instructions, None);
-    let serialized = message.serialize_instructions();
+    let serialized = message.serialize_instructions(
+        true, // demote_sysvar_write_locks
+    );
     b.iter(|| {
         test::black_box(instructions::load_instruction_at(3, &serialized).unwrap());
     });

sdk/program/src/message.rs

@@ -6,13 +6,34 @@ use crate::serialize_utils::{
     append_slice, append_u16, append_u8, read_pubkey, read_slice, read_u16, read_u8,
 };
 use crate::{
+    bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable,
     hash::Hash,
     instruction::{AccountMeta, CompiledInstruction, Instruction},
     pubkey::Pubkey,
-    short_vec, system_instruction,
+    short_vec, system_instruction, system_program, sysvar,
 };
 use itertools::Itertools;
-use std::convert::TryFrom;
+use lazy_static::lazy_static;
+use std::{convert::TryFrom, str::FromStr};
+
+lazy_static! {
+    // Copied keys over since direct references create cyclical dependency.
+    static ref BUILTIN_PROGRAMS_KEYS: [Pubkey; 10] = {
+        let parse = |s| Pubkey::from_str(s).unwrap();
+        [
+            parse("Config1111111111111111111111111111111111111"),
+            parse("Feature111111111111111111111111111111111111"),
+            parse("NativeLoader1111111111111111111111111111111"),
+            parse("Stake11111111111111111111111111111111111111"),
+            parse("StakeConfig11111111111111111111111111111111"),
+            parse("Vote111111111111111111111111111111111111111"),
+            system_program::id(),
+            bpf_loader::id(),
+            bpf_loader_deprecated::id(),
+            bpf_loader_upgradeable::id(),
+        ]
+    };
+}
 
 fn position(keys: &[Pubkey], key: &Pubkey) -> u8 {
     keys.iter().position(|k| k == key).unwrap() as u8
@@ -326,23 +347,31 @@ impl Message {
         self.program_position(i).is_some()
     }
 
-    pub fn is_writable(&self, i: usize) -> bool {
+    pub fn is_writable(&self, i: usize, demote_sysvar_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
             || (i >= self.header.num_required_signatures as usize
                 && i < self.account_keys.len()
-                    - self.header.num_readonly_unsigned_accounts as usize)
+                    - self.header.num_readonly_unsigned_accounts as usize))
+            && !{
+                let key = self.account_keys[i];
+                demote_sysvar_write_locks
+                    && (sysvar::is_sysvar_id(&key) || BUILTIN_PROGRAMS_KEYS.contains(&key))
+            }
     }
 
     pub fn is_signer(&self, i: usize) -> bool {
         i < self.header.num_required_signatures as usize
     }
 
-    pub fn get_account_keys_by_lock_type(&self) -> (Vec<&Pubkey>, Vec<&Pubkey>) {
+    pub fn get_account_keys_by_lock_type(
+        &self,
+        demote_sysvar_write_locks: bool,
+    ) -> (Vec<&Pubkey>, Vec<&Pubkey>) {
         let mut writable_keys = vec![];
         let mut readonly_keys = vec![];
         for (i, key) in self.account_keys.iter().enumerate() {
-            if self.is_writable(i) {
+            if self.is_writable(i, demote_sysvar_write_locks) {
                 writable_keys.push(key);
             } else {
                 readonly_keys.push(key);
@@ -364,7 +393,7 @@ impl Message {
     //   36..64 - program_id
     //     33..34 - data len - u16
     //     35..data_len - data
-    pub fn serialize_instructions(&self) -> Vec<u8> {
+    pub fn serialize_instructions(&self, demote_sysvar_write_locks: bool) -> Vec<u8> {
         // 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);
@@ -379,7 +408,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);
+                let is_writable = self.is_writable(account_index, demote_sysvar_write_locks);
                 let mut meta_byte = 0;
                 if is_signer {
                     meta_byte |= 1 << Self::IS_SIGNER_BIT;
@@ -459,7 +488,8 @@ impl Message {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::instruction::AccountMeta;
+    use crate::{hash, instruction::AccountMeta};
+    use std::collections::HashSet;
 
     #[test]
     fn test_message_unique_program_ids() {
@@ -471,6 +501,27 @@ mod tests {
         assert_eq!(program_ids, vec![program_id0]);
     }
 
+    #[test]
+    fn test_builtin_program_keys() {
+        let keys: HashSet<Pubkey> = BUILTIN_PROGRAMS_KEYS.iter().copied().collect();
+        assert_eq!(keys.len(), 10);
+        for k in keys {
+            let k = format!("{}", k);
+            assert!(k.ends_with("11111111111111111111111"));
+        }
+    }
+
+    #[test]
+    fn test_builtin_program_keys_abi_freeze() {
+        // Once the feature is flipped on, we can't further modify
+        // BUILTIN_PROGRAMS_KEYS without the risk of breaking consensus.
+        let builtins = format!("{:?}", *BUILTIN_PROGRAMS_KEYS);
+        assert_eq!(
+            format!("{}", hash::hash(builtins.as_bytes())),
+            "ACqmMkYbo9eqK6QrRSrB3HLyR6uHhLf31SCfGUAJjiWj"
+        );
+    }
+
     #[test]
     fn test_message_unique_program_ids_not_adjacent() {
         let program_id0 = Pubkey::default();
@@ -796,12 +847,13 @@ mod tests {
             recent_blockhash: Hash::default(),
             instructions: vec![],
         };
-        assert_eq!(message.is_writable(0), true);
+        let demote_sysvar_write_locks = true;
-        assert_eq!(message.is_writable(1), false);
+        assert_eq!(message.is_writable(0, demote_sysvar_write_locks), true);
-        assert_eq!(message.is_writable(2), false);
+        assert_eq!(message.is_writable(1, demote_sysvar_write_locks), false);
-        assert_eq!(message.is_writable(3), true);
+        assert_eq!(message.is_writable(2, demote_sysvar_write_locks), false);
-        assert_eq!(message.is_writable(4), true);
+        assert_eq!(message.is_writable(3, demote_sysvar_write_locks), true);
-        assert_eq!(message.is_writable(5), false);
+        assert_eq!(message.is_writable(4, demote_sysvar_write_locks), true);
+        assert_eq!(message.is_writable(5, demote_sysvar_write_locks), false);
     }
 
     #[test]
@@ -829,7 +881,9 @@ mod tests {
             Some(&id1),
         );
         assert_eq!(
-            message.get_account_keys_by_lock_type(),
+            message.get_account_keys_by_lock_type(
+                true, // demote_sysvar_write_locks
+            ),
             (vec![&id1, &id0], vec![&id3, &id2, &program_id])
         );
     }
@@ -859,7 +913,9 @@ mod tests {
         ];
 
         let message = Message::new(&instructions, Some(&id1));
-        let serialized = message.serialize_instructions();
+        let serialized = message.serialize_instructions(
+            true, // demote_sysvar_write_locks
+        );
         for (i, instruction) in instructions.iter().enumerate() {
             assert_eq!(
                 Message::deserialize_instruction(i, &serialized).unwrap(),
@@ -880,7 +936,9 @@ mod tests {
         ];
 
         let message = Message::new(&instructions, Some(&id1));
-        let serialized = message.serialize_instructions();
+        let serialized = message.serialize_instructions(
+            true, // demote_sysvar_write_locks
+        );
         assert_eq!(
             Message::deserialize_instruction(instructions.len(), &serialized).unwrap_err(),
             SanitizeError::IndexOutOfBounds,

sdk/src/feature_set.rs

@@ -123,6 +123,10 @@ pub mod upgradeable_close_instruction {
     solana_sdk::declare_id!("FsPaByos3gA9bUEhp3EimQpQPCoSvCEigHod496NmABQ");
 }
 
+pub mod demote_sysvar_write_locks {
+    solana_sdk::declare_id!("86LJYRuq2zgtHuL3FccR6hqFJQMQkFoun4knAxcPiF1P");
+}
+
 lazy_static! {
     /// Map of feature identifiers to user-visible description
     pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@@ -154,6 +158,7 @@ lazy_static! {
         (require_stake_for_gossip::id(), "require stakes for propagating crds values through gossip #15561"),
         (cpi_data_cost::id(), "charge the compute budget for data passed via CPI"),
         (upgradeable_close_instruction::id(), "close upgradeable buffer accounts"),
+        (demote_sysvar_write_locks::id(), "demote builtins and sysvar write locks to readonly #15497"),
         /*************** ADD NEW FEATURES HERE ***************/
     ]
     .iter()

transaction-status/src/parse_accounts.rs

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