svm: Add placeholder to AccountLoader for accounts-db miss #7451
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7451 +/- ##
=========================================
Coverage ? 83.3%
=========================================
Files ? 801
Lines ? 362825
Branches ? 0
=========================================
Hits ? 302305
Misses ? 60520
Partials ? 0 🚀 New features to boost your workflow:
|
svm/src/account_loader.rs
Outdated
| // Does not exist in AccountLoader or accounts-db. | ||
| // Insert a placeholder so we don't need to check again. | ||
| (None, false) => { |
There was a problem hiding this comment.
Subsequent calls to load_account for the same non-existent account will always insert a new placeholder. I think the returned bool should indicate "whether an accounts-db load was performed" rather than "whether the account came from accounts-db" and then the impossible branch below becomes the insert branch and this (None, false) branch just returns None as before
There was a problem hiding this comment.
That means we are missing a test that the same inputs should result in aliasing outputs.
2501babe
force-pushed
the
20250811_skiploadmiss
branch
from
b36f440 to
f5fa522
Compare
| account_loader.load_account(&miss_address); | ||
| let actual_miss_account = account_loader.loaded_accounts.get(&hit_address); | ||
|
|
||
| assert_eq!(actual_miss_account, Some(&expected_miss_account)); |
There was a problem hiding this comment.
It should probably use Arc::ptr_eq otherwise you just test if the content is the same, not if they are the same object.
2501babe
force-pushed
the
20250811_skiploadmiss
branch
from
5a1dfac to
a88cf94
Compare
|
will rebase again after #7466 |
2501babe
force-pushed
the
20250811_skiploadmiss
branch
from
a88cf94 to
4d40722
Compare
2501babe
force-pushed
the
20250811_skiploadmiss
branch
from
4d40722 to
28ff720
Compare
Problem
if a transaction loads a non-existant account as read-only, we can store a placeholder in AccountLoader so that future transactions dont have to go to accounts-db again
Summary of Changes
do so. in practice this is not likely to happen much, since in most cases the transaction will want to create it. but it is a small optimization with negligible downside