Skip to content

Commit

Permalink
Add Message::is_maybe_writable
Browse files Browse the repository at this point in the history
  • Loading branch information
jstarry committed Feb 27, 2024
1 parent 9ea7043 commit edbdf6f
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 34 deletions.
4 changes: 2 additions & 2 deletions rpc-client/src/nonblocking/rpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ impl RpcClient {
'sending: for _ in 0..SEND_RETRIES {
let signature = self.send_transaction(transaction).await?;

let recent_blockhash = if transaction.uses_durable_nonce() {
let recent_blockhash = if transaction.maybe_uses_durable_nonce() {
let (recent_blockhash, ..) = self
.get_latest_blockhash_with_commitment(CommitmentConfig::processed())
.await?;
Expand Down Expand Up @@ -753,7 +753,7 @@ impl RpcClient {
commitment: CommitmentConfig,
config: RpcSendTransactionConfig,
) -> ClientResult<Signature> {
let recent_blockhash = if transaction.uses_durable_nonce() {
let recent_blockhash = if transaction.maybe_uses_durable_nonce() {
self.get_latest_blockhash_with_commitment(CommitmentConfig::processed())
.await?
.0
Expand Down
12 changes: 6 additions & 6 deletions rpc-client/src/rpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use {
message::{v0, Message as LegacyMessage},
pubkey::Pubkey,
signature::Signature,
transaction::{self, uses_durable_nonce, Transaction, VersionedTransaction},
transaction::{self, maybe_uses_durable_nonce, Transaction, VersionedTransaction},
},
solana_transaction_status::{
EncodedConfirmedBlock, EncodedConfirmedTransactionWithStatusMeta, TransactionStatus,
Expand Down Expand Up @@ -80,7 +80,7 @@ impl SerializableMessage for v0::Message {}
pub trait SerializableTransaction: Serialize {
fn get_signature(&self) -> &Signature;
fn get_recent_blockhash(&self) -> &Hash;
fn uses_durable_nonce(&self) -> bool;
fn maybe_uses_durable_nonce(&self) -> bool;
}
impl SerializableTransaction for Transaction {
fn get_signature(&self) -> &Signature {
Expand All @@ -89,8 +89,8 @@ impl SerializableTransaction for Transaction {
fn get_recent_blockhash(&self) -> &Hash {
&self.message.recent_blockhash
}
fn uses_durable_nonce(&self) -> bool {
uses_durable_nonce(self).is_some()
fn maybe_uses_durable_nonce(&self) -> bool {
maybe_uses_durable_nonce(self).is_some()
}
}
impl SerializableTransaction for VersionedTransaction {
Expand All @@ -100,8 +100,8 @@ impl SerializableTransaction for VersionedTransaction {
fn get_recent_blockhash(&self) -> &Hash {
self.message.recent_blockhash()
}
fn uses_durable_nonce(&self) -> bool {
self.uses_durable_nonce()
fn maybe_uses_durable_nonce(&self) -> bool {
self.maybe_uses_durable_nonce()
}
}

Expand Down
22 changes: 19 additions & 3 deletions sdk/program/src/message/legacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,12 +548,28 @@ 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)
}

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)
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/program/src/message/versions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
Expand Down
7 changes: 4 additions & 3 deletions sdk/program/src/message/versions/v0/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
&& !{
Expand Down
22 changes: 11 additions & 11 deletions sdk/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,7 @@ impl Transaction {
}
}

pub fn uses_durable_nonce(tx: &Transaction) -> Option<&CompiledInstruction> {
pub fn maybe_uses_durable_nonce(tx: &Transaction) -> Option<&CompiledInstruction> {
let message = tx.message();
message
.instructions
Expand All @@ -1093,7 +1093,7 @@ pub fn uses_durable_nonce(tx: &Transaction) -> Option<&CompiledInstruction> {
// Nonce account is writable
&& matches!(
instruction.accounts.first(),
Some(index) if message.is_writable(*index as usize)
Some(index) if message.is_maybe_writable(*index as usize)
)
})
}
Expand Down Expand Up @@ -1553,19 +1553,19 @@ mod tests {
#[test]
fn tx_uses_nonce_ok() {
let (_, _, tx) = nonced_transfer_tx();
assert!(uses_durable_nonce(&tx).is_some());
assert!(maybe_uses_durable_nonce(&tx).is_some());
}

#[test]
fn tx_uses_nonce_empty_ix_fail() {
assert!(uses_durable_nonce(&Transaction::default()).is_none());
assert!(maybe_uses_durable_nonce(&Transaction::default()).is_none());
}

#[test]
fn tx_uses_nonce_bad_prog_id_idx_fail() {
let (_, _, mut tx) = nonced_transfer_tx();
tx.message.instructions.get_mut(0).unwrap().program_id_index = 255u8;
assert!(uses_durable_nonce(&tx).is_none());
assert!(maybe_uses_durable_nonce(&tx).is_none());
}

#[test]
Expand All @@ -1580,7 +1580,7 @@ mod tests {
];
let message = Message::new(&instructions, Some(&from_pubkey));
let tx = Transaction::new(&[&from_keypair, &nonce_keypair], message, Hash::default());
assert!(uses_durable_nonce(&tx).is_none());
assert!(maybe_uses_durable_nonce(&tx).is_none());
}

#[test]
Expand All @@ -1606,7 +1606,7 @@ mod tests {
&[&from_keypair, &nonce_keypair],
Hash::default(),
);
assert!(uses_durable_nonce(&tx).is_none());
assert!(maybe_uses_durable_nonce(&tx).is_none());
}

#[test]
Expand All @@ -1626,13 +1626,13 @@ mod tests {
];
let message = Message::new(&instructions, Some(&nonce_pubkey));
let tx = Transaction::new(&[&from_keypair, &nonce_keypair], message, Hash::default());
assert!(uses_durable_nonce(&tx).is_none());
assert!(maybe_uses_durable_nonce(&tx).is_none());
}

#[test]
fn get_nonce_pub_from_ix_ok() {
let (_, nonce_pubkey, tx) = nonced_transfer_tx();
let nonce_ix = uses_durable_nonce(&tx).unwrap();
let nonce_ix = maybe_uses_durable_nonce(&tx).unwrap();
assert_eq!(
get_nonce_pubkey_from_instruction(nonce_ix, &tx),
Some(&nonce_pubkey),
Expand All @@ -1642,7 +1642,7 @@ mod tests {
#[test]
fn get_nonce_pub_from_ix_no_accounts_fail() {
let (_, _, tx) = nonced_transfer_tx();
let nonce_ix = uses_durable_nonce(&tx).unwrap();
let nonce_ix = maybe_uses_durable_nonce(&tx).unwrap();
let mut nonce_ix = nonce_ix.clone();
nonce_ix.accounts.clear();
assert_eq!(get_nonce_pubkey_from_instruction(&nonce_ix, &tx), None,);
Expand All @@ -1651,7 +1651,7 @@ mod tests {
#[test]
fn get_nonce_pub_from_ix_bad_acc_idx_fail() {
let (_, _, tx) = nonced_transfer_tx();
let nonce_ix = uses_durable_nonce(&tx).unwrap();
let nonce_ix = maybe_uses_durable_nonce(&tx).unwrap();
let mut nonce_ix = nonce_ix.clone();
nonce_ix.accounts[0] = 255u8;
assert_eq!(get_nonce_pubkey_from_instruction(&nonce_ix, &tx), None,);
Expand Down
14 changes: 7 additions & 7 deletions sdk/src/transaction/versioned/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ impl VersionedTransaction {
/// instruction. Since dynamically loaded addresses can't have write locks
/// demoted without loading addresses, this shouldn't be used in the
/// runtime.
pub fn uses_durable_nonce(&self) -> bool {
pub fn maybe_uses_durable_nonce(&self) -> bool {
let message = &self.message;
message
.instructions()
Expand Down Expand Up @@ -291,12 +291,12 @@ mod tests {
#[test]
fn tx_uses_nonce_ok() {
let (_, _, tx) = nonced_transfer_tx();
assert!(tx.uses_durable_nonce());
assert!(tx.maybe_uses_durable_nonce());
}

#[test]
fn tx_uses_nonce_empty_ix_fail() {
assert!(!VersionedTransaction::default().uses_durable_nonce());
assert!(!VersionedTransaction::default().maybe_uses_durable_nonce());
}

#[test]
Expand All @@ -308,7 +308,7 @@ mod tests {
}
VersionedMessage::V0(_) => unreachable!(),
};
assert!(!tx.uses_durable_nonce());
assert!(!tx.maybe_uses_durable_nonce());
}

#[test]
Expand All @@ -324,7 +324,7 @@ mod tests {
let message = LegacyMessage::new(&instructions, Some(&from_pubkey));
let tx = Transaction::new(&[&from_keypair, &nonce_keypair], message, Hash::default());
let tx = VersionedTransaction::from(tx);
assert!(!tx.uses_durable_nonce());
assert!(!tx.maybe_uses_durable_nonce());
}

#[test]
Expand All @@ -351,7 +351,7 @@ mod tests {
Hash::default(),
);
let tx = VersionedTransaction::from(tx);
assert!(!tx.uses_durable_nonce());
assert!(!tx.maybe_uses_durable_nonce());
}

#[test]
Expand All @@ -372,6 +372,6 @@ mod tests {
let message = LegacyMessage::new(&instructions, Some(&nonce_pubkey));
let tx = Transaction::new(&[&from_keypair, &nonce_keypair], message, Hash::default());
let tx = VersionedTransaction::from(tx);
assert!(!tx.uses_durable_nonce());
assert!(!tx.maybe_uses_durable_nonce());
}
}
2 changes: 1 addition & 1 deletion transaction-status/src/parse_accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});
Expand Down

0 comments on commit edbdf6f

Please sign in to comment.