Add Message::is_maybe_writable (#35340)

sdk/program/src/message/legacy.rs

@@ -548,12 +548,32 @@ impl Message {
         self.is_key_called_as_program(i) && !self.is_upgradeable_loader_present()
     }
 
-    pub fn is_writable(&self, i: usize) -> bool {
-        (i < (self.header.num_required_signatures - self.header.num_readonly_signed_accounts)
+    /// Returns true if the account at the specified index was requested to be
+    /// writable. This method should not be used directly.
+    fn is_writable_index(&self, i: usize) -> bool {
+        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)
+    }
+
+    /// Returns true if the account at the specified index should be write
+    /// locked when loaded for transaction processing in the runtime. This
+    /// method differs from `is_maybe_writable` because it is aware of the
+    /// latest reserved accounts which are not allowed to be write locked.
+    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 {
+        (self.is_writable_index(i))
             && !is_builtin_key_or_sysvar(&self.account_keys[i])
             && !self.demote_program_id(i)
     }

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

@@ -79,7 +79,7 @@ impl VersionedMessage {
     /// used in the runtime.
     pub fn is_maybe_writable(&self, index: usize) -> bool {
         match self {
-            Self::Legacy(message) => message.is_writable(index),
+            Self::Legacy(message) => message.is_maybe_writable(index),
             Self::V0(message) => message.is_maybe_writable(index),
         }
     }

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

@@ -334,9 +334,10 @@ impl Message {
             .any(|&key| key == bpf_loader_upgradeable::id())
     }
 
-    /// Returns true if the account at the specified index was requested as writable.
-    /// Before loading addresses, we can't demote write locks for dynamically loaded
-    /// addresses so this should not be used by the runtime.
+    /// 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 {
         self.is_writable_index(key_index)
             && !{

sdk/src/transaction/mod.rs

@@ -1074,6 +1074,7 @@ impl Transaction {
     }
 }
 
+/// Returns true if transaction begins with an advance nonce instruction.
 pub fn uses_durable_nonce(tx: &Transaction) -> Option<&CompiledInstruction> {
     let message = tx.message();
     message
@@ -1090,11 +1091,6 @@ pub fn uses_durable_nonce(tx: &Transaction) -> Option<&CompiledInstruction> {
                 limited_deserialize(&instruction.data),
                 Ok(SystemInstruction::AdvanceNonceAccount)
             )
-            // Nonce account is writable
-            && matches!(
-                instruction.accounts.first(),
-                Some(index) if message.is_writable(*index as usize)
-            )
         })
 }
 
@@ -1119,7 +1115,7 @@ mod tests {
             hash::hash,
             instruction::AccountMeta,
             signature::{Keypair, Presigner, Signer},
-            system_instruction, sysvar,
+            system_instruction,
         },
         bincode::{deserialize, serialize, serialized_size},
         std::mem::size_of,
@@ -1583,32 +1579,6 @@ mod tests {
         assert!(uses_durable_nonce(&tx).is_none());
     }
 
-    #[test]
-    fn tx_uses_ro_nonce_account() {
-        let from_keypair = Keypair::new();
-        let from_pubkey = from_keypair.pubkey();
-        let nonce_keypair = Keypair::new();
-        let nonce_pubkey = nonce_keypair.pubkey();
-        let account_metas = vec![
-            AccountMeta::new_readonly(nonce_pubkey, false),
-            #[allow(deprecated)]
-            AccountMeta::new_readonly(sysvar::recent_blockhashes::id(), false),
-            AccountMeta::new_readonly(nonce_pubkey, true),
-        ];
-        let nonce_instruction = Instruction::new_with_bincode(
-            system_program::id(),
-            &system_instruction::SystemInstruction::AdvanceNonceAccount,
-            account_metas,
-        );
-        let tx = Transaction::new_signed_with_payer(
-            &[nonce_instruction],
-            Some(&from_pubkey),
-            &[&from_keypair, &nonce_keypair],
-            Hash::default(),
-        );
-        assert!(uses_durable_nonce(&tx).is_none());
-    }
-
     #[test]
     fn tx_uses_nonce_wrong_first_nonce_ix_fail() {
         let from_keypair = Keypair::new();

sdk/src/transaction/versioned/mod.rs

@@ -185,10 +185,7 @@ impl VersionedTransaction {
             .collect()
     }
 
-    /// Returns true if transaction begins with a valid advance nonce
-    /// instruction. Since dynamically loaded addresses can't have write locks
-    /// demoted without loading addresses, this shouldn't be used in the
-    /// runtime.
+    /// Returns true if transaction begins with an advance nonce instruction.
     pub fn uses_durable_nonce(&self) -> bool {
         let message = &self.message;
         message
@@ -205,11 +202,6 @@ impl VersionedTransaction {
                     limited_deserialize(&instruction.data),
                     Ok(SystemInstruction::AdvanceNonceAccount)
                 )
-                // Nonce account is writable
-                && matches!(
-                    instruction.accounts.first(),
-                    Some(index) if message.is_maybe_writable(*index as usize)
-                )
             })
             .is_some()
     }
@@ -222,7 +214,7 @@ mod tests {
         crate::{
             message::Message as LegacyMessage,
             signer::{keypair::Keypair, Signer},
-            system_instruction, sysvar,
+            system_instruction,
         },
         solana_program::{
             instruction::{AccountMeta, Instruction},
@@ -327,33 +319,6 @@ mod tests {
         assert!(!tx.uses_durable_nonce());
     }
 
-    #[test]
-    fn tx_uses_ro_nonce_account() {
-        let from_keypair = Keypair::new();
-        let from_pubkey = from_keypair.pubkey();
-        let nonce_keypair = Keypair::new();
-        let nonce_pubkey = nonce_keypair.pubkey();
-        let account_metas = vec![
-            AccountMeta::new_readonly(nonce_pubkey, false),
-            #[allow(deprecated)]
-            AccountMeta::new_readonly(sysvar::recent_blockhashes::id(), false),
-            AccountMeta::new_readonly(nonce_pubkey, true),
-        ];
-        let nonce_instruction = Instruction::new_with_bincode(
-            system_program::id(),
-            &system_instruction::SystemInstruction::AdvanceNonceAccount,
-            account_metas,
-        );
-        let tx = Transaction::new_signed_with_payer(
-            &[nonce_instruction],
-            Some(&from_pubkey),
-            &[&from_keypair, &nonce_keypair],
-            Hash::default(),
-        );
-        let tx = VersionedTransaction::from(tx);
-        assert!(!tx.uses_durable_nonce());
-    }
-
     #[test]
     fn tx_uses_nonce_wrong_first_nonce_ix_fail() {
         let from_keypair = Keypair::new();

transaction-status/src/parse_accounts.rs

@@ -21,7 +21,7 @@ pub fn parse_legacy_message_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_maybe_writable(i),
             signer: message.is_signer(i),
             source: Some(ParsedAccountSource::Transaction),
         });