diff --git a/src/StdStorage.sol b/src/StdStorage.sol index 8228d251..9d496ca1 100644 --- a/src/StdStorage.sol +++ b/src/StdStorage.sol @@ -123,7 +123,8 @@ library stdStorageSafe { if (reads.length == 0) { revert("stdStorage find(StdStorage): No storage use detected for target."); } else { - for (uint256 i = reads.length; --i >= 0;) { + for (uint256 i = reads.length; i > 0;) { + --i; bytes32 prev = vm.load(who, reads[i]); if (prev == bytes32(0)) { emit WARNING_UninitedSlot(who, uint256(reads[i])); diff --git a/test/StdStorage.t.sol b/test/StdStorage.t.sol index 6a97ae46..ab87da38 100644 --- a/test/StdStorage.t.sol +++ b/test/StdStorage.t.sol @@ -350,6 +350,16 @@ contract StdStorageTest is Test { stdstore.target(address(test)).sig("edgeCaseArray(uint256)").with_key(uint256(0)).checked_write(1); assertEq(test.edgeCaseArray(0), 1); } + + // Regression test for https://github.com/foundry-rs/forge-std/issues/740 + // `find()` used to infinite-loop on tokens whose `balanceOf` reads multiple + // storage slots and returns a derived value (reflection tokens). + function test_RevertFindOnReflectionToken() public { + MockReflectionToken token = new MockReflectionToken(); + ReflectionTokenTarget target = new ReflectionTokenTarget(token); + vm.expectRevert("stdStorage find(StdStorage): Slot(s) not found."); + target.findBalanceOf(address(this)); + } } contract StorageTestTarget { @@ -367,6 +377,21 @@ contract StorageTestTarget { } } +contract ReflectionTokenTarget { + using stdStorage for StdStorage; + + StdStorage internal stdstore; + MockReflectionToken internal token; + + constructor(MockReflectionToken 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; @@ -483,3 +508,25 @@ contract StorageTest { return (randomPacking << leftBits) >> (leftBits + rightBits); } } + +// Minimal mock of a reflection token: `balanceOf` reads many storage slots +// and always returns a constant, so no single slot mutation can change its +// return value and stdStorage can never find a matching slot. +contract MockReflectionToken { + uint256 internal _a = 1; + uint256 internal _b = 2; + uint256 internal _c = 3; + mapping(address => uint256) internal _balances; + + constructor() { + _balances[msg.sender] = 1000 ether; + } + + // Reads _a, _b, _c, and _balances[account] but always returns a constant. + // This means mutating any single slot won't change the return value. + function balanceOf(address account) public view returns (uint256) { + uint256 x = _a + _b + _c + _balances[account]; + x; // suppress unused warning + return 42; + } +}