Skip to content

fix(core): keep account indexes in step with every write - #832

Merged
MicaiahReid merged 3 commits into
solana-foundation:mainfrom
L0STE:fix/account-index-stale-entries
Oct 5, 2026
Merged

MicaiahReid merged 3 commits into
solana-foundation:mainfrom
L0STE:fix/account-index-stale-entries

Conversation

@L0STE

@L0STE L0STE commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Problem

update_account_registries drops 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 already
up to date by the time it runs, so every caller (set_account and both transaction paths in the
locker) gets back the new state, or nothing, instead of the old one:

  • When a transaction closes an account, the read returns nothing, so nothing is removed. A closed
    token account stays in token_accounts.
  • When an account changes owner, the read returns the new owner, so the old owner's
    accounts_by_owner bucket keeps the account.

For example, when a transaction closes a token account:

  1. The transaction succeeds, and the account no longer exists.
  2. Its postTokenBalances still lists the account, at amount 0, because they are built from
    token_accounts.

A validator builds postTokenBalances from the accounts after execution, where a closed account no
longer exists, so it is not listed (svm/src/transaction_balances.rs:78-108 in Agave). An indexer
reading 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. Reads
that resolve the index against the SVM, such as getTokenAccountsByOwner, happen to drop missing
accounts, which hid the closed-account case there.

Fix

Callers pass the account as it was before the update, and update_account_registries drops the
entries for that state:

  • set_account reads the account before overwriting it.
  • Both transaction paths in the locker already hold 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_slot wrote
through self.inner.set_account in four places (the fetched target, its coupled account, and the
token and IDL overrides), skipping everything SurfnetSvm::set_account does after the write. A
token amount set by a scenario never reached the token index, so getTokenLargestAccounts kept
reporting the old amount. Accounts a fetch_before_use override forked in were never indexed, so
getProgramAccounts did not list them. Account subscribers were not notified, and on an on-disk
surfnet 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, including apply_account_update
for accounts fetched on demand.

The third commit stops one failing override from losing the rest of its slot.
materialize_overrides_for_slot takes the whole slot off the schedule, and two paths in the token
branch returned early without putting the remaining overrides back:

  • A token override whose values can't be applied, such as an amount that isn't a u64, now
    logs a warning and is skipped, as an IDL override that fails to forge already is.
  • A failed token write now puts the unprocessed overrides back before returning the error, as a
    failed account read in the same loop already does.

Test

  • a_closed_token_account_has_no_post_token_balance: a real CloseAccount transaction succeeds, and
    its postTokenBalances is empty.
  • an_account_is_listed_only_under_its_current_owner: after set_account changes an account's owner,
    get_account_owned_by lists it under the new owner only.
  • The two existing garbage-collection tests call update_account_registries directly, and now pass
    the 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_account now also checks that the target and its
    coupled 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's
    subscribers with the new data.
  • a_bad_token_override_does_not_drop_the_overrides_after_it: a token override with an invalid
    amount 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_unprocessed as the read above it.

The five new tests, and the strengthened fetch test, fail on main. The existing tests passed there because they call
update_account_registries without updating the inner SVM first, so the read-back happened to find
the old account. Each test was also checked by breaking the code one change at a time:
set_account or the transaction path not passing the prior state, the removal skipped, and the
prior 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 local
state, and a bad override aborting the slot again. Every break turns at least one of them red.

set_account now reads the account before writing it. That read replaces the one
update_account_registries used to do, so each write still reads the account once. Measured in
release 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 -- --check
  • cargo clippy -p surfpool-core --all-targets adds no warnings
  • cargo 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_use
  • cargo test -p surfpool-core --lib: 799 pass. The only failure is
    test_simulate_add_alt_entries_fetching, which fails the same way on main (fix(rpc): count loaded accounts data size as SIMD-0186 does #831 fixes it).
    test_simnet_ready/test_simnet_ticks fail intermittently on main too. Separately,
    test_send_transaction_skip_sig_verify_processes_and_updates_state::with_on_disk_sqlite_db sometimes
    hangs when the whole suite runs, on main as well as here.

@greptile-apps

greptile-apps Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[High risk] Changes how account state changes are tracked in the database layer.

The PR appears safe to merge; no new actionable issue was identified.

Summary

The PR removes stale account-index entries using the account’s pre-write state and routes scenario override writes through the normal account-update path.

  • Adds regression coverage for closed token accounts, owner changes, fetched coupled accounts, and scenario overrides.
  • Preserves later scheduled overrides when a token write fails and skips malformed token values.

Reviews (5) · Last reviewed commit: "fix(core): keep a slot's other overrides..."

Comment thread crates/core/src/surfnet/svm.rs Outdated
@L0STE
L0STE force-pushed the fix/account-index-stale-entries branch from 2a62c3c to 1f3f34d Compare September 28, 2026 12:05
@L0STE L0STE changed the title fix(core): drop an account's index entries for the state it had before fix(core): keep account indexes in step with every write Sep 28, 2026
@MicaiahReid MicaiahReid self-assigned this Sep 28, 2026
@MicaiahReid
MicaiahReid self-requested a review September 30, 2026 00:29
MicaiahReid
MicaiahReid previously approved these changes Sep 30, 2026
@L0STE
L0STE force-pushed the fix/account-index-stale-entries branch from 1500d03 to 8cf9a02 Compare October 4, 2026 20:07
@MicaiahReid
MicaiahReid merged commit 1c39492 into solana-foundation:main Oct 5, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants