forge-std icon indicating copy to clipboard operation
forge-std copied to clipboard

Error: Storage Slot not found in dealERC721

Open SonOfMosiah opened this issue 2 years ago • 13 comments

Getting the following error when using dealERC721 for UniswapV3 LP positions (i.e. NonfungiblePositionManager): [FAIL. Reason: stdStorage find(StdStorage): Slot(s) not found.]

Seems find() is having a hard time getting the storage slot due to the implementation of EnumerableMaps.

Minimal reproducible repo here.

@wirew0lf

SonOfMosiah avatar Mar 02 '23 00:03 SonOfMosiah

Thanks for the issue, from what I can see in the NonfungiblePositionManager code it uses the standard ERC721 for the ownerOf() function so this shouldn't be a problem. It may be an issue with the StdStorage library for some reason, I'm looking more into it, @mds1 any idea of why StdStorage might not be able to find the variable slot?

wirew0lf avatar Mar 02 '23 08:03 wirew0lf

Probably because the NFT is a proxy contract, IIRC StdStorage does not support that currently and requires the slot to be in the _target address

mds1 avatar Mar 02 '23 19:03 mds1

Indeed, the contract in the poc appears to be using a proxy: https://etherscan.io/address/0xC36442b4a4522E871399CD717aBDD847Ab11FE88

I can think of a solution for this type of cases, although not ideal but it would be the only way to fix this without major changes to StdStorage. Basically it'd be pranking the owner and performing a transferFrom. This is not ideal, mainly because:

  • If transfers are somehow disabled in the contract such as some debt or position tokens the deal will fail.
  • If this is triggered inside a startPrank the first prank will be stopped as I think there is no way to perform a prank inside antoher one, or resume it after the deal prank is performed.

So this could be implemented as a backup method. But I'm curious to hear your thoughts and opinions on this.

wirew0lf avatar Mar 03 '23 19:03 wirew0lf

Ideally we could add proxy support to StdStorage, though I'm not sure offhand how much work that would be. Might not be too bad if we only support standard proxy implementations like EIP-1967 as a fallback if a storage slot couldn't be found normally

mds1 avatar Mar 05 '23 16:03 mds1

Ideally we could add proxy support to StdStorage, though I'm not sure offhand how much work that would be. Might not be too bad if we only support standard proxy implementations like EIP-1967 as a fallback if a storage slot couldn't be found normally

This would be great definitely. I'm not too familiar with how StdStorage works internally, but I can take a look and try to work on this when I have some spare time.

wirew0lf avatar Mar 06 '23 13:03 wirew0lf

+1 here. Same issue trying to deal tokens behind a proxy-implementation pattern.

nine-december avatar Mar 10 '23 13:03 nine-december

usually when I deal proxy tokens, I make it like this:

IERC20 tokenproxy = IERC20("address");
address whale = address("address_holder_tokenproxy");

function setUp() public {
        vm.createSelectFork("mainnet", block.number);
        vm.startPrank(whale);
        tokenproxy.transfer(address(this), 10000e18);
        vm.stopPrank();
    }

It's worked for me.

PutraLaksmana avatar Dec 28 '23 18:12 PutraLaksmana

Stumbled on this issue too after a couple of tests started failing when trying to deal mainnet USDC to an address. I've narrowed the root cause down to this commit: https://github.com/foundry-rs/forge-std/commit/fac7bd8d4e27f42067c4713fd231c3240fd8437a but unsure about the why still.

hyperspacebunny avatar Feb 21 '24 13:02 hyperspacebunny

Mainnet USDC recently ugpraded and now stored balance in a packed slot, so deal for it will be fixed once https://github.com/foundry-rs/forge-std/pull/505 is merged

mds1 avatar Feb 21 '24 21:02 mds1

Mainnet USDC recently ugpraded and now stored balance in a packed slot, so deal for it will be fixed once #505 is merged

Hey! I am having the same issue! I am on the latest foundry version. Still getting an error like this while dealing Mainnet USDC to an address.

[FAIL. Reason: revert: stdStorage find(StdStorage): Slot(s) not found.]

Any solution for this?

kamuik16 avatar Mar 27 '24 16:03 kamuik16

Mainnet USDC recently ugpraded and now stored balance in a packed slot, so deal for it will be fixed once #505 is merged

Hey! I ran into the same issue and had to manually copy paste StdStorage.sol into my forge-std lib because I think it still clones the older versions. Please fix it. Thanks!

It now works correctly!

kamuik16 avatar Mar 27 '24 17:03 kamuik16

Correct, you will need to upgrade your forge-std dependency to the latest release, not just your foundry version

mds1 avatar Mar 27 '24 23:03 mds1

Can confirm updating forge-std works when dealing proxy USDC

rpedroni avatar May 21 '24 18:05 rpedroni