Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error: Storage Slot not found in dealERC721 #318

Open
SonOfMosiah opened this issue Mar 2, 2023 · 13 comments
Open

Error: Storage Slot not found in dealERC721 #318

SonOfMosiah opened this issue Mar 2, 2023 · 13 comments

Comments

@SonOfMosiah
Copy link

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

@wirew0lf
Copy link
Contributor

wirew0lf commented Mar 2, 2023

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?

@mds1
Copy link
Collaborator

mds1 commented Mar 2, 2023

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

@wirew0lf
Copy link
Contributor

wirew0lf commented Mar 3, 2023

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.

@mds1
Copy link
Collaborator

mds1 commented Mar 5, 2023

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

@wirew0lf
Copy link
Contributor

wirew0lf commented Mar 6, 2023

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.

@nine-december
Copy link

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

@PutraLaksmana
Copy link

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.

@hyperspacebunny
Copy link

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: fac7bd8 but unsure about the why still.

@mds1
Copy link
Collaborator

mds1 commented Feb 21, 2024

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

@kamuik16
Copy link

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
Copy link

kamuik16 commented Mar 27, 2024

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!

@mds1
Copy link
Collaborator

mds1 commented Mar 27, 2024

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

@rpedroni
Copy link

Can confirm updating forge-std works when dealing proxy USDC

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

No branches or pull requests

8 participants