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

Cannot find the storage slot for a public string variable #3869

Open
1 of 2 tasks
simontianx opened this issue Dec 11, 2022 · 10 comments
Open
1 of 2 tasks

Cannot find the storage slot for a public string variable #3869

simontianx opened this issue Dec 11, 2022 · 10 comments
Labels
A-cheatcodes Area: cheatcodes C-forge Command: forge T-bug Type: bug T-to-reproduce Type: requires reproduction

Comments

@simontianx
Copy link

simontianx commented Dec 11, 2022

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (b28119b 2022-11-23T03:06:50.152811Z)

What command(s) is the bug in?

forge test

Operating System

Windows

Describe the bug

For a simple storage layout given below, it's possible to find the storage slot for owner and number by running stdstore.target(address(test)).sig("owner()").find(); or stdstore.target(address(test)).sig("number()").find();, but it's failing when running stdstore.target(address(test)).sig("sentence()").find();.

contract SimpleStorage {
    address public owner;
    string public sentence;
    uint256 public number;
}
@simontianx simontianx added the T-bug Type: bug label Dec 11, 2022
@simontianx simontianx changed the title Cannot find storage slot for a public string variable Cannot find the storage slot for a public string variable Dec 11, 2022
@rkrasiuk
Copy link
Collaborator

@mds1 @ZeroEkkusu ptal

@rkrasiuk rkrasiuk added C-forge Command: forge A-cheatcodes Area: cheatcodes labels Dec 22, 2022
@mds1
Copy link
Collaborator

mds1 commented Dec 22, 2022

This makes sense to me: address and uint256 always use a single slot in storage, whereas string is a dynamic type so it's length / number of slots varies, and I don't think stdstore supports dynamic types currently. It probably can be extended to do so, though I'm not sure how difficult it would be (@brockelmore might know better)

@brockelmore
Copy link
Member

multi-slot stuff would be a big unlock but quite painful to accomplish safely. If the string fits in 32 bytes, it works but anything longer won't. I would recommend manually using vm.load if you need to handle multi slot strings

@ZeroEkkusu
Copy link
Contributor

ZeroEkkusu commented Dec 22, 2022

@brockelmore Is it possible to have a vm cheatcode that returns storage layout - e.g. when you take a look at the storage layout in the artifacts, you can see the names of variables and it is obvious what is packed where and of what type it is. Types do get more complicated with structs, etc. forge-std could leverage it to create helpers - it'd be possible to access internal and private variables as well. How doable is that?

Edit: I thought about it a few months ago but saw that parsing storage layout gets complicated with structs/nesting, as I mentioned.

@mds1
Copy link
Collaborator

mds1 commented Dec 22, 2022

We've talked about that a bit in the past, a few considerations that I recall:

  • pretty sure solc does not output storage layout by default, so the cheatcode would need to throw or recompile if storage layout isn't present. Also need to decide if you compile with storage layout by default and slow down compilation for all users, even ones who don't need it, or require users to add that flag when using this cheatcode.
  • the current stdstorage approach (not needing storage layout) is still better in some cases: (1) you don't have source code but you have an ABI, (2) you're working against a fork for a contract already deployed (avoids needing to compile it to get storage layout), and (3) you're running forge against local contracts compiled with another language so don't have storage layout info in the expected solidity format
  • i.e. the optimal solution is to have both approaches behind the scenes and figure out the best UX to make this work

@ZeroEkkusu
Copy link
Contributor

I agree with all three points.

@brockelmore
Copy link
Member

brockelmore commented Dec 22, 2022

Now that i think about it there is a nonzero chance that if we know we are finding a string or bytes we can check for sequentialities in the read slots and match against those actually

@mds1
Copy link
Collaborator

mds1 commented Dec 22, 2022

Also need to consider that the data might be in the slot itself, or the slot might just have the encoded length with the data in keccak(slot): https://docs.soliditylang.org/en/v0.8.17/internals/layout_in_storage.html#bytes-and-string

But yea, for the latter case you can just keep reading adjacent slots until it's empty which should pretty much always be a safe assumption

@simontianx
Copy link
Author

@mds1 @rkrasiuk @brockelmore @ZeroEkkusu Thanks for the answers and comments. In the given example, I was thinking that the information at the slot 1 can be read directly to know about the string sentence as in ethers.getStorageAt, but stdstore.target(address(test)).sig("sentence()").find(); doesn't seem to work that way. I am wondering if it's possible to read directly from a storage slot no matter what type of variables it represents. Thanks.

@brockelmore
Copy link
Member

@simontianx can you try the branch of forge-std in the PR i linked above? Should work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cheatcodes Area: cheatcodes C-forge Command: forge T-bug Type: bug T-to-reproduce Type: requires reproduction
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

7 participants