Repository navigation
fix(core): keep account indexes in step with every write - #832
Merged
MicaiahReid merged 3 commits intoOct 5, 2026
Merged
Conversation
Contributor
|
L0STE
force-pushed
the
fix/account-index-stale-entries
branch
from
September 28, 2026 12:05
2a62c3c to
1f3f34d
Compare
MicaiahReid
self-requested a review
September 30, 2026 00:29
MicaiahReid
previously approved these changes
Sep 30, 2026
L0STE
force-pushed
the
fix/account-index-stale-entries
branch
from
October 4, 2026 20:07
1500d03 to
8cf9a02
Compare
MicaiahReid
approved these changes
Oct 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
update_account_registriesdrops an account's index entries (owner, token owner, mint, delegate)for the state the account had before the update, then indexes the new state. It reads that prior
state back with
self.get_account(pubkey). But, as its own comment says, the inner SVM is alreadyup to date by the time it runs, so every caller (
set_accountand both transaction paths in thelocker) gets back the new state, or nothing, instead of the old one:
token account stays in
token_accounts.accounts_by_ownerbucket keeps the account.For example, when a transaction closes a token account:
postTokenBalancesstill lists the account, at amount 0, because they are built fromtoken_accounts.A validator builds
postTokenBalancesfrom the accounts after execution, where a closed account nolonger exists, so it is not listed (
svm/src/transaction_balances.rs:78-108in Agave). An indexerreading Surfpool's balances sees a token account that no longer exists.
The same stale entry lists a reassigned account under its old owner in
getProgramAccounts. Readsthat resolve the index against the SVM, such as
getTokenAccountsByOwner, happen to drop missingaccounts, which hid the closed-account case there.
Fix
Callers pass the account as it was before the update, and
update_account_registriesdrops theentries for that state:
set_accountreads the account before overwriting it.accounts_before, and pass it along.The two branches that each read the prior state back are now one removal at the top, for both the
deleted and the updated case.
The same gap exists where scenario overrides write accounts.
materialize_overrides_for_slotwrotethrough
self.inner.set_accountin four places (the fetched target, its coupled account, and thetoken and IDL overrides), skipping everything
SurfnetSvm::set_accountdoes after the write. Atoken amount set by a scenario never reached the token index, so
getTokenLargestAccountskeptreporting the old amount. Accounts a
fetch_before_useoverride forked in were never indexed, sogetProgramAccountsdid not list them. Account subscribers were not notified, and on an on-disksurfnet the override was not written to the database. The second commit routes those four writes
through
set_account, the path every other write already takes, includingapply_account_updatefor accounts fetched on demand.
The third commit stops one failing override from losing the rest of its slot.
materialize_overrides_for_slottakes the whole slot off the schedule, and two paths in the tokenbranch returned early without putting the remaining overrides back:
amountthat isn't au64, nowlogs a warning and is skipped, as an IDL override that fails to forge already is.
failed account read in the same loop already does.
Test
a_closed_token_account_has_no_post_token_balance: a realCloseAccounttransaction succeeds, andits
postTokenBalancesis empty.an_account_is_listed_only_under_its_current_owner: afterset_accountchanges an account's owner,get_account_owned_bylists it under the new owner only.update_account_registriesdirectly, and now passthe prior account.
a_token_amount_override_reaches_the_token_index: after a scenario sets a token account's amount,the token index holds that amount.
test_fetch_before_use_materializes_a_coupled_accountnow also checks that the target and itscoupled mint are listed under the Token program. Its canned token account had an all-zero mint,
which is the System Program's address. That account always exists, so the coupled write never ran,
and the old "the coupled mint must fill the gap" check passed even with that write removed. The
fixture now carries a real mint, as does
test_fetch_before_use_keeps_a_locally_modified_coupled_account.an_idl_override_notifies_account_subscribers: an IDL override notifies the account'ssubscribers with the new data.
a_bad_token_override_does_not_drop_the_overrides_after_it: a token override with an invalidamount is skipped, and the next override in the same slot still sets the amount. A failed token
write can't be forced in a test without a hook production doesn't have, so that path has no test
of its own. It uses the same
restore_unprocessedas the read above it.The five new tests, and the strengthened fetch test, fail on
main. The existing tests passed there because they callupdate_account_registrieswithout updating the inner SVM first, so the read-back happened to findthe old account. Each test was also checked by breaking the code one change at a time:
set_accountor the transaction path not passing the prior state, the removal skipped, and theprior state read back from the inner SVM as before, and each of the four override writes going
back to
self.inner.set_account, the coupled write dropped, and a coupled account overwriting localstate, and a bad override aborting the slot again. Every break turns at least one of them red.
set_accountnow reads the account before writing it. That read replaces the oneupdate_account_registriesused to do, so each write still reads the account once. Measured inrelease mode, 20,000 new-account writes and 20,000 overwrites of a 1 KiB account take the same time
before and after (7.4–7.7 s and 14.3–15.6 ms, in three alternating runs each).
Validation
cargo +nightly fmt --all -- --checkcargo clippy -p surfpool-core --all-targetsadds no warningscargo test -p surfpool-core --lib -- a_closed_token_account_has_no_post_token_balance an_account_is_listed_only_under_its_current_owner garbage_collected override fetch_before_usecargo test -p surfpool-core --lib: 799 pass. The only failure istest_simulate_add_alt_entries_fetching, which fails the same way onmain(fix(rpc): count loaded accounts data size as SIMD-0186 does #831 fixes it).test_simnet_ready/test_simnet_ticksfail intermittently onmaintoo. Separately,test_send_transaction_skip_sig_verify_processes_and_updates_state::with_on_disk_sqlite_dbsometimeshangs when the whole suite runs, on
mainas well as here.