Skip to content

Commit

Permalink
svm: only inspect loaded accounts
Browse files Browse the repository at this point in the history
  • Loading branch information
2501babe committed Oct 10, 2024
1 parent 3fc84a5 commit 0a4cbbe
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 34 deletions.
27 changes: 7 additions & 20 deletions svm/src/account_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,6 @@ fn load_transaction_account<CB: TransactionProcessingCallback>(
loaded_programs: &ProgramCacheForTxBatch,
) -> Result<(LoadedTransactionAccount, bool)> {
let mut account_found = true;
let mut was_inspected = false;
let is_instruction_account = u8::try_from(account_index)
.map(|i| instruction_accounts.contains(&&i))
.unwrap_or(false);
Expand Down Expand Up @@ -454,17 +453,11 @@ fn load_transaction_account<CB: TransactionProcessingCallback>(
callbacks
.get_account_shared_data(account_key)
.map(|mut account| {
let rent_collected = if is_writable {
// Inspect the account prior to collecting rent, since
// rent collection can modify the account.
debug_assert!(!was_inspected);
callbacks.inspect_account(
account_key,
AccountState::Alive(&account),
is_writable,
);
was_inspected = true;
// Inspect the account prior to collecting rent, since
// rent collection can modify the account.
callbacks.inspect_account(account_key, AccountState::Alive(&account), is_writable);

let rent_collected = if is_writable {
collect_rent_from_account(
feature_set,
rent_collector,
Expand All @@ -483,8 +476,11 @@ fn load_transaction_account<CB: TransactionProcessingCallback>(
}
})
.unwrap_or_else(|| {
callbacks.inspect_account(account_key, AccountState::Dead, is_writable);

account_found = false;
let mut default_account = AccountSharedData::default();

// All new accounts must be rent-exempt (enforced in Bank::execute_loaded_transaction).
// Currently, rent collection sets rent_epoch to u64::MAX, but initializing the account
// with this field already set would allow us to skip rent collection for these accounts.
Expand All @@ -497,15 +493,6 @@ fn load_transaction_account<CB: TransactionProcessingCallback>(
})
};

if !was_inspected {
let account_state = if account_found {
AccountState::Alive(&loaded_account.account)
} else {
AccountState::Dead
};
callbacks.inspect_account(account_key, account_state, is_writable);
}

Ok((loaded_account, account_found))
}

Expand Down
20 changes: 6 additions & 14 deletions svm/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1192,26 +1192,18 @@ fn svm_inspect_account() {
);
}

// The transfer program account is also loaded during transaction processing, however the
// account state passed to `inspect_account()` is *not* the same as what is held by
// MockBankCallback::account_shared_data. So we check the transfer program differently.
//
// First ensure we have the correct number of inspected accounts, correctly counting the
// transfer program.
// The transfer program account is retreived from the program cache, which does not
// inspect accounts, because they are necessarily read-only. Verify it has not made
// its way into the inspected accounts list.
let num_expected_inspected_accounts: usize =
expected_inspected_accounts.values().map(Vec::len).sum();
let num_actual_inspected_accounts: usize =
actual_inspected_accounts.values().map(Vec::len).sum();

assert_eq!(
num_expected_inspected_accounts + 2,
num_expected_inspected_accounts,
num_actual_inspected_accounts,
);

// And second, ensure the inspected transfer program accounts are alive and not writable.
let actual_transfer_program_accounts =
actual_inspected_accounts.get(&transfer_program).unwrap();
for actual_transfer_program_account in actual_transfer_program_accounts {
assert!(actual_transfer_program_account.0.is_some());
assert!(!actual_transfer_program_account.1);
}
assert!(!actual_inspected_accounts.contains_key(&transfer_program));
}

0 comments on commit 0a4cbbe

Please sign in to comment.