Skip to content

fix(StdStorage): distinguish derived return values from missing slots - #898

Open
developer3516 wants to merge 1 commit into
foundry-rs:masterfrom
developer3516:fix/deal-computed-balance-diagnostic
Open

fix(StdStorage): distinguish derived return values from missing slots#898
developer3516 wants to merge 1 commit into
foundry-rs:masterfrom
developer3516:fix/deal-computed-balance-diagnostic

Conversation

@developer3516

Copy link
Copy Markdown

Motivation

Closes #140.

deal() fails on tokens whose balance is computed rather than stored. The original report uses an Aave aToken, whose balanceOf returns the stored scaled balance multiplied by a liquidity index:

balanceOf(user) == scaledBalance(user) * getReserveNormalizedIncome() / RAY

stdStorage.find() locates a slot by requiring that the value held in the slot equals the value returned by the call. For a scaled-balance token that is never true, so the search fails with:

stdStorage find(StdStorage): Slot(s) not found.

That message is misleading. The balance slot is found — checkSlotMutatesCall correctly reports that mutating it changes balanceOf. It is rejected one step later by the value-equality check, and the user is told nothing about why.

Solution

Report the two failures separately. When the search fails, probe whether any recorded read slot actually influences the return value:

  • No slot influences it → unchanged, "Slot(s) not found." This is the reflection-token case from bug(StdCheats): deal() hangs on reflection tokens #740, whose regression test is untouched.
  • A slot influences it but none hold it → a new message naming the two realistic causes:
stdStorage find(StdStorage): Slot(s) affect the return value but none hold it.
Target may derive the value (e.g. rebasing token) or pack it (try enable_packed_slots()).

Implementation notes:

  • The probe lives in a helper rather than a flag threaded through the search loop. find() is already at the EVM stack limit — adding a single local there fails to compile with Stack too deep.
  • The probe only runs on the failing path, which reverts anyway, so successful lookups are unaffected.
  • Short bytes/string returns are excluded. There a word-wise mismatch says nothing about how the value is produced, so test_RevertStorageFindRestoresFailedShortBytesProbe keeps the original message.

Testing

test_RevertFindOnScaledBalanceToken adds a minimal aToken-style mock (stored balance × index) and asserts the new message. It mirrors the structure of the existing test_RevertFindOnReflectionToken.

forge test: 207 passing, 0 failing. forge fmt --check clean.

This does not make deal() work on such tokens — that would require inverting the token's own accounting, which is out of scope. It tells the user what happened so they can reach for vm.store or the underlying scaled-balance accessor.

`find()` reverts with "Slot(s) not found." whenever no storage slot holds
the value returned by the target call. That message is misleading for
tokens whose balance is computed rather than stored: an Aave aToken scales
a stored balance by a liquidity index, so `deal()` fails even though the
balance slot is correctly identified as affecting the call.

Report that case separately. When at least one read slot demonstrably
changes the return value but none hold it, say so and name the two
realistic causes: a derived value, or a packed variable that needs
`enable_packed_slots()`.

The extra probe only runs on the failing path, so the common case is
unaffected. Short bytes/string returns are excluded, since a word-wise
mismatch there says nothing about how the value is produced; those keep
the original message, as does the reflection-token case from foundry-rs#740 where
no slot affects the call at all.

Closes foundry-rs#140
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.

bug(StdCheats): deal fails on tokens with computed balanceOf (aTokens, rebasing)

1 participant