Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jstarry committed Jul 31, 2024
1 parent d524247 commit ae57c00
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 38 deletions.
27 changes: 15 additions & 12 deletions svm/src/account_saver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,20 @@ fn collect_accounts_for_successful_tx<'a>(
transaction_accounts: &'a [TransactionAccount],
) {
let message = transaction.message();
for (i, (address, account)) in (0..message.account_keys().len()).zip(transaction_accounts) {
if message.is_writable(i) {
// Accounts that are invoked and also not passed as an instruction
// account to a program don't need to be stored because it's assumed
// to be impossible for a committable transaction to modify an
// invoked account if said account isn't passed to some program.
if !message.is_invoked(i) || message.is_instruction_account(i) {
collected_accounts.push((address, account));
collected_account_transactions.push(Some(transaction));
for (_, (address, account)) in (0..message.account_keys().len())
.zip(transaction_accounts)
.filter(|(i, _)| {
message.is_writable(*i) && {
// Accounts that are invoked and also not passed as an instruction
// account to a program don't need to be stored because it's assumed
// to be impossible for a committable transaction to modify an
// invoked account if said account isn't passed to some program.
!message.is_invoked(*i) || message.is_instruction_account(*i)
}
}
})
{
collected_accounts.push((address, account));
collected_account_transactions.push(Some(transaction));
}
}

Expand All @@ -112,7 +115,7 @@ fn collect_accounts_for_failed_tx<'a>(
// Since we know we are dealing with a valid nonce account,
// unwrap is safe here
nonce
.try_advance_account(*durable_nonce, lamports_per_signature)
.try_advance_nonce(*durable_nonce, lamports_per_signature)
.unwrap();
collected_accounts.push((nonce.address(), nonce.account()));
collected_account_transactions.push(Some(transaction));
Expand All @@ -127,7 +130,7 @@ fn collect_accounts_for_failed_tx<'a>(
// Since we know we are dealing with a valid nonce account,
// unwrap is safe here
nonce
.try_advance_account(*durable_nonce, lamports_per_signature)
.try_advance_nonce(*durable_nonce, lamports_per_signature)
.unwrap();
collected_accounts.push((nonce.address(), nonce.account()));
collected_account_transactions.push(Some(transaction));
Expand Down
14 changes: 7 additions & 7 deletions svm/src/nonce_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl NoncePartial {
// Advance the stored blockhash to prevent fee theft by someone
// replaying nonce transactions that have failed with an
// `InstructionError`.
pub fn try_advance_account(
pub fn try_advance_nonce(
&mut self,
durable_nonce: DurableNonce,
lamports_per_signature: u64,
Expand Down Expand Up @@ -114,7 +114,7 @@ mod tests {
}

#[test]
fn test_try_advance_account_success() {
fn test_try_advance_nonce_success() {
let authority = Pubkey::new_unique();
let mut nonce_partial = NoncePartial::new(
Pubkey::new_unique(),
Expand All @@ -127,7 +127,7 @@ mod tests {

let new_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
let new_lamports_per_signature = 100;
let result = nonce_partial.try_advance_account(new_nonce, new_lamports_per_signature);
let result = nonce_partial.try_advance_nonce(new_nonce, new_lamports_per_signature);
assert_eq!(result, Ok(()));

let nonce_versions = StateMut::<NonceVersions>::state(&nonce_partial.account).unwrap();
Expand All @@ -142,26 +142,26 @@ mod tests {
}

#[test]
fn test_try_advance_account_invalid() {
fn test_try_advance_nonce_invalid() {
let mut nonce_partial = NoncePartial::new(
Pubkey::new_unique(),
AccountSharedData::new(1_000_000, 0, &Pubkey::default()),
);

let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
let result = nonce_partial.try_advance_account(durable_nonce, 5000);
let result = nonce_partial.try_advance_nonce(durable_nonce, 5000);
assert_eq!(result, Err(AdvanceNonceError::Invalid));
}

#[test]
fn test_try_advance_account_uninitialized() {
fn test_try_advance_nonce_uninitialized() {
let mut nonce_partial = NoncePartial::new(
Pubkey::new_unique(),
create_nonce_account(NonceState::Uninitialized),
);

let durable_nonce = DurableNonce::from_blockhash(&Hash::new_unique());
let result = nonce_partial.try_advance_account(durable_nonce, 5000);
let result = nonce_partial.try_advance_nonce(durable_nonce, 5000);
assert_eq!(result, Err(AdvanceNonceError::Uninitialized));
}
}
19 changes: 0 additions & 19 deletions svm/src/rollback_accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,25 +72,6 @@ impl RollbackAccounts {
}
}

pub fn nonce(&self) -> Option<&NoncePartial> {
match self {
Self::FeePayerOnly { .. } => None,
Self::SameNonceAndFeePayer { nonce } | Self::SeparateNonceAndFeePayer { nonce, .. } => {
Some(nonce)
}
}
}

pub fn fee_payer_account(&self) -> &AccountSharedData {
match self {
Self::FeePayerOnly { fee_payer_account }
| Self::SeparateNonceAndFeePayer {
fee_payer_account, ..
} => fee_payer_account,
Self::SameNonceAndFeePayer { nonce } => nonce.account(),
}
}

/// Number of accounts tracked for rollback
pub fn count(&self) -> usize {
match self {
Expand Down

0 comments on commit ae57c00

Please sign in to comment.