Deprecate is_builtin_key_or_sysvar function (#788)

cli-output/src/display.rs

@@ -13,6 +13,7 @@ use {
         native_token::lamports_to_sol,
         program_utils::limited_deserialize,
         pubkey::Pubkey,
+        reserved_account_keys::ReservedAccountKeys,
         signature::Signature,
         stake,
         transaction::{TransactionError, TransactionVersion, VersionedTransaction},
@@ -217,6 +218,7 @@ fn write_transaction<W: io::Write>(
     write_recent_blockhash(w, message.recent_blockhash(), prefix)?;
     write_signatures(w, &transaction.signatures, sigverify_status, prefix)?;
 
+    let reserved_account_keys = ReservedAccountKeys::new_all_activated().active;
     let mut fee_payer_index = None;
     for (account_index, account) in account_keys.iter().enumerate() {
         if fee_payer_index.is_none() && message.is_non_loader_key(account_index) {
@@ -225,7 +227,7 @@ fn write_transaction<W: io::Write>(
 
         let account_meta = CliAccountMeta {
             is_signer: message.is_signer(account_index),
-            is_writable: message.is_maybe_writable(account_index),
+            is_writable: message.is_maybe_writable(account_index, Some(&reserved_account_keys)),
             is_invoked: message.is_invoked(account_index),
         };
 

sdk/program/src/message/legacy.rs

@@ -22,7 +22,7 @@ use {
         short_vec, system_instruction, system_program, sysvar, wasm_bindgen,
     },
     lazy_static::lazy_static,
-    std::{convert::TryFrom, str::FromStr},
+    std::{collections::HashSet, convert::TryFrom, str::FromStr},
 };
 
 lazy_static! {
@@ -58,6 +58,10 @@ lazy_static! {
     };
 }
 
+#[deprecated(
+    since = "2.0.0",
+    note = "please use `solana_sdk::reserved_account_keys::ReservedAccountKeys::is_reserved` instead"
+)]
 #[allow(deprecated)]
 pub fn is_builtin_key_or_sysvar(key: &Pubkey) -> bool {
     if MAYBE_BUILTIN_KEY_OR_SYSVAR[key.0[0] as usize] {
@@ -564,22 +568,45 @@ impl Message {
     /// isn't used here to demote write locks, this shouldn't be used in the
     /// runtime.
     #[deprecated(since = "2.0.0", note = "Please use `is_maybe_writable` instead")]
+    #[allow(deprecated)]
     pub fn is_writable(&self, i: usize) -> bool {
         (self.is_writable_index(i))
             && !is_builtin_key_or_sysvar(&self.account_keys[i])
             && !self.demote_program_id(i)
     }
 
     /// Returns true if the account at the specified index is writable by the
-    /// instructions in this message. Since the dynamic set of reserved accounts
-    /// isn't used here to demote write locks, this shouldn't be used in the
-    /// runtime.
-    pub fn is_maybe_writable(&self, i: usize) -> bool {
+    /// instructions in this message. The `reserved_account_keys` param has been
+    /// optional to allow clients to approximate writability without requiring
+    /// fetching the latest set of reserved account keys. If this method is
+    /// called by the runtime, the latest set of reserved account keys must be
+    /// passed.
+    pub fn is_maybe_writable(
+        &self,
+        i: usize,
+        reserved_account_keys: Option<&HashSet<Pubkey>>,
+    ) -> bool {
         (self.is_writable_index(i))
-            && !is_builtin_key_or_sysvar(&self.account_keys[i])
+            && !self.is_account_maybe_reserved(i, reserved_account_keys)
             && !self.demote_program_id(i)
     }
 
+    /// Returns true if the account at the specified index is in the optional
+    /// reserved account keys set.
+    fn is_account_maybe_reserved(
+        &self,
+        key_index: usize,
+        reserved_account_keys: Option<&HashSet<Pubkey>>,
+    ) -> bool {
+        let mut is_maybe_reserved = false;
+        if let Some(reserved_account_keys) = reserved_account_keys {
+            if let Some(key) = self.account_keys.get(key_index) {
+                is_maybe_reserved = reserved_account_keys.contains(key);
+            }
+        }
+        is_maybe_reserved
+    }
+
     pub fn is_signer(&self, i: usize) -> bool {
         i < self.header.num_required_signatures as usize
     }
@@ -589,7 +616,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_maybe_writable(i) {
+            if self.is_maybe_writable(i, None) {
                 writable_keys.push(key);
             } else {
                 readonly_keys.push(key);
@@ -777,6 +804,58 @@ mod tests {
         assert!(!message.is_writable(5));
     }
 
+    #[test]
+    fn test_is_maybe_writable() {
+        let key0 = Pubkey::new_unique();
+        let key1 = Pubkey::new_unique();
+        let key2 = Pubkey::new_unique();
+        let key3 = Pubkey::new_unique();
+        let key4 = Pubkey::new_unique();
+        let key5 = Pubkey::new_unique();
+
+        let message = Message {
+            header: MessageHeader {
+                num_required_signatures: 3,
+                num_readonly_signed_accounts: 2,
+                num_readonly_unsigned_accounts: 1,
+            },
+            account_keys: vec![key0, key1, key2, key3, key4, key5],
+            recent_blockhash: Hash::default(),
+            instructions: vec![],
+        };
+
+        let reserved_account_keys = HashSet::from([key3]);
+
+        assert!(message.is_maybe_writable(0, Some(&reserved_account_keys)));
+        assert!(!message.is_maybe_writable(1, Some(&reserved_account_keys)));
+        assert!(!message.is_maybe_writable(2, Some(&reserved_account_keys)));
+        assert!(!message.is_maybe_writable(3, Some(&reserved_account_keys)));
+        assert!(message.is_maybe_writable(3, None));
+        assert!(message.is_maybe_writable(4, Some(&reserved_account_keys)));
+        assert!(!message.is_maybe_writable(5, Some(&reserved_account_keys)));
+        assert!(!message.is_maybe_writable(6, Some(&reserved_account_keys)));
+    }
+
+    #[test]
+    fn test_is_account_maybe_reserved() {
+        let key0 = Pubkey::new_unique();
+        let key1 = Pubkey::new_unique();
+
+        let message = Message {
+            account_keys: vec![key0, key1],
+            ..Message::default()
+        };
+
+        let reserved_account_keys = HashSet::from([key1]);
+
+        assert!(!message.is_account_maybe_reserved(0, Some(&reserved_account_keys)));
+        assert!(message.is_account_maybe_reserved(1, Some(&reserved_account_keys)));
+        assert!(!message.is_account_maybe_reserved(2, Some(&reserved_account_keys)));
+        assert!(!message.is_account_maybe_reserved(0, None));
+        assert!(!message.is_account_maybe_reserved(1, None));
+        assert!(!message.is_account_maybe_reserved(2, None));
+    }
+
     #[test]
     fn test_get_account_keys_by_lock_type() {
         let program_id = Pubkey::default();

sdk/program/src/message/versions/mod.rs

@@ -12,7 +12,7 @@ use {
         ser::{SerializeTuple, Serializer},
         Deserialize, Serialize,
     },
-    std::fmt,
+    std::{collections::HashSet, fmt},
 };
 
 mod sanitized;
@@ -77,10 +77,14 @@ impl VersionedMessage {
     /// instructions in this message. Since dynamically loaded addresses can't
     /// have write locks demoted without loading addresses, this shouldn't be
     /// used in the runtime.
-    pub fn is_maybe_writable(&self, index: usize) -> bool {
+    pub fn is_maybe_writable(
+        &self,
+        index: usize,
+        reserved_account_keys: Option<&HashSet<Pubkey>>,
+    ) -> bool {
         match self {
-            Self::Legacy(message) => message.is_maybe_writable(index),
-            Self::V0(message) => message.is_maybe_writable(index),
+            Self::Legacy(message) => message.is_maybe_writable(index, reserved_account_keys),
+            Self::V0(message) => message.is_maybe_writable(index, reserved_account_keys),
         }
     }
 

sdk/program/src/message/versions/v0/mod.rs

@@ -9,21 +9,23 @@
 //! [`v0`]: crate::message::v0
 //! [future message format]: https://docs.solanalabs.com/proposals/versioned-transactions
 
-use crate::{
-    address_lookup_table_account::AddressLookupTableAccount,
-    bpf_loader_upgradeable,
-    hash::Hash,
-    instruction::{CompiledInstruction, Instruction},
-    message::{
-        compiled_keys::{CompileError, CompiledKeys},
-        legacy::is_builtin_key_or_sysvar,
-        AccountKeys, MessageHeader, MESSAGE_VERSION_PREFIX,
+pub use loaded::*;
+use {
+    crate::{
+        address_lookup_table_account::AddressLookupTableAccount,
+        bpf_loader_upgradeable,
+        hash::Hash,
+        instruction::{CompiledInstruction, Instruction},
+        message::{
+            compiled_keys::{CompileError, CompiledKeys},
+            AccountKeys, MessageHeader, MESSAGE_VERSION_PREFIX,
+        },
+        pubkey::Pubkey,
+        sanitize::SanitizeError,
+        short_vec,
     },
-    pubkey::Pubkey,
-    sanitize::SanitizeError,
-    short_vec,
+    std::collections::HashSet,
 };
-pub use loaded::*;
 
 mod loaded;
 
@@ -335,24 +337,40 @@ impl Message {
     }
 
     /// Returns true if the account at the specified index was requested as
-    /// writable. Before loading addresses and without the reserved account keys
-    /// set, we can't demote write locks properly so this should not be used by
-    /// the runtime.
-    pub fn is_maybe_writable(&self, key_index: usize) -> bool {
+    /// writable. Before loading addresses, we can't demote write locks properly
+    /// so this should not be used by the runtime. The `reserved_account_keys`
+    /// param is optional to allow clients to approximate writability without
+    /// requiring fetching the latest set of reserved account keys.
+    pub fn is_maybe_writable(
+        &self,
+        key_index: usize,
+        reserved_account_keys: Option<&HashSet<Pubkey>>,
+    ) -> bool {
         self.is_writable_index(key_index)
-            && !{
-                // demote reserved ids
-                self.account_keys
-                    .get(key_index)
-                    .map(is_builtin_key_or_sysvar)
-                    .unwrap_or_default()
-            }
+            && !self.is_account_maybe_reserved(key_index, reserved_account_keys)
             && !{
                 // demote program ids
                 self.is_key_called_as_program(key_index)
                     && !self.is_upgradeable_loader_in_static_keys()
             }
     }
+
+    /// Returns true if the account at the specified index is in the reserved
+    /// account keys set. Before loading addresses, we can't detect reserved
+    /// account keys properly so this shouldn't be used by the runtime.
+    fn is_account_maybe_reserved(
+        &self,
+        key_index: usize,
+        reserved_account_keys: Option<&HashSet<Pubkey>>,
+    ) -> bool {
+        let mut is_maybe_reserved = false;
+        if let Some(reserved_account_keys) = reserved_account_keys {
+            if let Some(key) = self.account_keys.get(key_index) {
+                is_maybe_reserved = reserved_account_keys.contains(key);
+            }
+        }
+        is_maybe_reserved
+    }
 }
 
 #[cfg(test)]
@@ -686,4 +704,71 @@ mod tests {
             })
         );
     }
+
+    #[test]
+    fn test_is_maybe_writable() {
+        let key0 = Pubkey::new_unique();
+        let key1 = Pubkey::new_unique();
+        let key2 = Pubkey::new_unique();
+        let key3 = Pubkey::new_unique();
+        let key4 = Pubkey::new_unique();
+        let key5 = Pubkey::new_unique();
+
+        let message = Message {
+            header: MessageHeader {
+                num_required_signatures: 3,
+                num_readonly_signed_accounts: 2,
+                num_readonly_unsigned_accounts: 1,
+            },
+            account_keys: vec![key0, key1, key2, key3, key4, key5],
+            address_table_lookups: vec![MessageAddressTableLookup {
+                account_key: Pubkey::new_unique(),
+                writable_indexes: vec![0],
+                readonly_indexes: vec![1],
+            }],
+            ..Message::default()
+        };
+
+        let reserved_account_keys = HashSet::from([key3]);
+
+        assert!(message.is_maybe_writable(0, Some(&reserved_account_keys)));
+        assert!(!message.is_maybe_writable(1, Some(&reserved_account_keys)));
+        assert!(!message.is_maybe_writable(2, Some(&reserved_account_keys)));
+        assert!(!message.is_maybe_writable(3, Some(&reserved_account_keys)));
+        assert!(message.is_maybe_writable(3, None));
+        assert!(message.is_maybe_writable(4, Some(&reserved_account_keys)));
+        assert!(!message.is_maybe_writable(5, Some(&reserved_account_keys)));
+        assert!(message.is_maybe_writable(6, Some(&reserved_account_keys)));
+        assert!(!message.is_maybe_writable(7, Some(&reserved_account_keys)));
+        assert!(!message.is_maybe_writable(8, Some(&reserved_account_keys)));
+    }
+
+    #[test]
+    fn test_is_account_maybe_reserved() {
+        let key0 = Pubkey::new_unique();
+        let key1 = Pubkey::new_unique();
+
+        let message = Message {
+            account_keys: vec![key0, key1],
+            address_table_lookups: vec![MessageAddressTableLookup {
+                account_key: Pubkey::new_unique(),
+                writable_indexes: vec![0],
+                readonly_indexes: vec![1],
+            }],
+            ..Message::default()
+        };
+
+        let reserved_account_keys = HashSet::from([key1]);
+
+        assert!(!message.is_account_maybe_reserved(0, Some(&reserved_account_keys)));
+        assert!(message.is_account_maybe_reserved(1, Some(&reserved_account_keys)));
+        assert!(!message.is_account_maybe_reserved(2, Some(&reserved_account_keys)));
+        assert!(!message.is_account_maybe_reserved(3, Some(&reserved_account_keys)));
+        assert!(!message.is_account_maybe_reserved(4, Some(&reserved_account_keys)));
+        assert!(!message.is_account_maybe_reserved(0, None));
+        assert!(!message.is_account_maybe_reserved(1, None));
+        assert!(!message.is_account_maybe_reserved(2, None));
+        assert!(!message.is_account_maybe_reserved(3, None));
+        assert!(!message.is_account_maybe_reserved(4, None));
+    }
 }

transaction-status/src/parse_accounts.rs

@@ -1,4 +1,7 @@
-use solana_sdk::message::{v0::LoadedMessage, Message};
+use solana_sdk::{
+    message::{v0::LoadedMessage, Message},
+    reserved_account_keys::ReservedAccountKeys,
+};
 
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
 #[serde(rename_all = "camelCase")]
@@ -17,11 +20,12 @@ pub enum ParsedAccountSource {
 }
 
 pub fn parse_legacy_message_accounts(message: &Message) -> Vec<ParsedAccount> {
+    let reserved_account_keys = ReservedAccountKeys::new_all_activated().active;
     let mut accounts: Vec<ParsedAccount> = vec![];
     for (i, account_key) in message.account_keys.iter().enumerate() {
         accounts.push(ParsedAccount {
             pubkey: account_key.to_string(),
-            writable: message.is_maybe_writable(i),
+            writable: message.is_maybe_writable(i, Some(&reserved_account_keys)),
             signer: message.is_signer(i),
             source: Some(ParsedAccountSource::Transaction),
         });