feat(core): revert get_account_local and get_multiple_accounts_local#368
Conversation
|
Nice start @vict0rcarvalh0! You did a great job finding this patch... Here some considerations |
crates/core/src/surfnet/locker.rs
Outdated
| false, | ||
| ), | ||
| Some(account) => { | ||
| if account.eq(&Account::default()) || account.lamports == 0 { |
There was a problem hiding this comment.
A default instance would look like:
lamports: 0
data: Vec::new()
owner: Pubkey::default() (all zeros)
executable: false
rent_epoch: 0
So checking it's redundant looking for both since looking for a Account::default(), already haves account.lamports == 0, and in the case where a account have lamports == 0 it means that the account does not exist. And after the fix of LiteSVM where accounts with 0 lamports are excluded from the accounts_db
You should be able to just
Some(account) => GetAccountResult::FoundAccount(
*pubkey, account,
false,
),Because LiteSVM now handles the deleting of accounts with 0 lamports we should just try to find it and it will returns the account or false since it does not exists.
There was a problem hiding this comment.
Makes sense Arthur! Just made a new commit refactoring that
crates/core/src/surfnet/locker.rs
Outdated
| false, | ||
| ), | ||
| Some(account) => { | ||
| if account.eq(&Account::default()) || account.lamports == 0 { |
There was a problem hiding this comment.
Same as above here :)
|
What do you think @MicaiahReid ? |
…hecks for zero-lamports accounts
MicaiahReid
left a comment
There was a problem hiding this comment.
Perfect, thanks @vict0rcarvalh0!
|
LGTM |
LiteSVM got merged to update set_account to remove 0 lamports accounts. So get_account_local and get_multiple_accounts_local were reverted to the previous implementation. Re-ran surfpool-core integration tests that validate reset behavior:
Both passed successfully.
LiteSVM reference: LiteSVM/litesvm#218
Fixes #332