diff --git a/src/StdStorage.sol b/src/StdStorage.sol index 3b56eb8e..472ed4cd 100644 --- a/src/StdStorage.sol +++ b/src/StdStorage.sol @@ -117,6 +117,19 @@ library stdStorageSafe { return (success && (prevReturnValue != newReturnValue)); } + /// @notice Returns whether any slot in `reads` changes the return value of the configured target call. + /// @dev Only used after a search fails, to tell "none of the slots we read matter" apart from + /// "a slot matters, but its stored value is transformed before being returned" (e.g. rebasing tokens). + function _anySlotAffectsCall(StdStorage storage self, bytes32[] memory reads) private returns (bool) { + for (uint256 i = reads.length; i > 0;) { + --i; + if (checkSlotMutatesCall(self, reads[i])) { + return true; + } + } + return false; + } + /// @notice Searches for the bit offset of the packed variable within `slot` from the left or right side. function findOffset(StdStorage storage self, bytes32 slot, bool left) internal returns (bool, uint256) { for (uint256 offset = 0; offset < 256; offset++) { @@ -221,10 +234,19 @@ library stdStorageSafe { } } - require( - self.finds[who][fsig][keccak256(abi.encodePacked(params, field_depth))].found, - "stdStorage find(StdStorage): Slot(s) not found." - ); + if (!self.finds[who][fsig][keccak256(abi.encodePacked(params, field_depth))].found) { + // Distinguish "none of the slots we read matter" from "a slot matters but never + // holds the returned value", which means the target transforms the stored value + // before returning it. Skipped for short bytes/string returns, where a word-wise + // comparison does not apply and a mismatch says nothing about how the value is + // produced. Only reached on the failing path, so this costs nothing otherwise. + if (callData.shortBytesStorageValue == bytes32(0) && _anySlotAffectsCall(self, callData.reads)) { + revert( + "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())." + ); + } + revert("stdStorage find(StdStorage): Slot(s) not found."); + } if (_clear) { clear(self); diff --git a/test/StdStorage.t.sol b/test/StdStorage.t.sol index 34782ac1..63488c79 100644 --- a/test/StdStorage.t.sol +++ b/test/StdStorage.t.sol @@ -393,6 +393,21 @@ contract StdStorageTest is Test { vm.expectRevert("stdStorage find(StdStorage): Slot(s) not found."); target.findBalanceOf(address(this)); } + + // Regression test for https://github.com/foundry-rs/forge-std/issues/140 + // `deal()` on a scaled-balance token (e.g. an Aave aToken) fails because + // `balanceOf` returns the stored balance scaled by an index, so no slot ever + // holds the returned value verbatim. Unlike a reflection token, the balance + // slot *is* detected as affecting the call, so report that specifically + // instead of claiming no slot was found. + function test_RevertFindOnScaledBalanceToken() public { + MockScaledBalanceToken token = new MockScaledBalanceToken(); + ScaledBalanceTokenTarget target = new ScaledBalanceTokenTarget(token); + vm.expectRevert( + "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())." + ); + target.findBalanceOf(address(this)); + } } contract StorageTestTarget { @@ -449,6 +464,21 @@ contract ReflectionTokenTarget { } } +contract ScaledBalanceTokenTarget { + using stdStorage for StdStorage; + + StdStorage internal stdstore; + MockScaledBalanceToken internal token; + + constructor(MockScaledBalanceToken token_) { + token = token_; + } + + function findBalanceOf(address who) public { + stdstore.target(address(token)).sig("balanceOf(address)").with_key(who).find(); + } +} + contract StorageTest { uint256 public exists = 1; mapping(address => uint256) public map_addr; @@ -587,3 +617,21 @@ contract MockReflectionToken { return 42; } } + +// Minimal mock of a scaled-balance token (e.g. an Aave aToken): `balanceOf` +// returns the stored scaled balance multiplied by a liquidity index. Mutating +// the balance slot does change the return value, but the slot never holds that +// value, so stdStorage detects a relevant slot yet still cannot match it. +contract MockScaledBalanceToken { + uint256 internal constant RAY = 1e27; + uint256 internal _index = 1.5e27; + mapping(address => uint256) internal _scaledBalances; + + constructor() { + _scaledBalances[msg.sender] = 1000 ether; + } + + function balanceOf(address account) public view returns (uint256) { + return (_scaledBalances[account] * _index) / RAY; + } +}