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

Check struct compatibility in diamond storage pattern #459

Open
frangio opened this issue Nov 18, 2021 · 3 comments
Open

Check struct compatibility in diamond storage pattern #459

frangio opened this issue Nov 18, 2021 · 3 comments

Comments

@frangio
Copy link
Contributor

frangio commented Nov 18, 2021

The diamond storage pattern described in EIP-2535 is gaining popularity. It consists of an internal getter that returns a storage pointer to a struct that is placed in some random location in storage. This is an alternative to storage gaps that is more powerful because it also tolerates different inheritance linearization.

A struct defined in a contract used only in a diamond storage getter should also be checked for layout incompatibilities, but currently isn't

@mudgen
Copy link

mudgen commented May 14, 2022

This is a great idea. Any activity or work on this yet?

@frangio
Copy link
Contributor Author

frangio commented Dec 15, 2022

The current idea is to add an annotation that can state the intent to use a struct at a specific location.

/// @custom:storage-location L
struct S {
    ...
}

Here L may be a slot number or some other way to identify the location. To give an idea of the latter, it might be eip1967:admin to specify the admin slot defined by EIP-1967, or similarly for some other EIP.

When comparing the layouts of two contracts, these annotations would be taken into consideration causing structs declared to be in the same location to be checked for compatibility.

Additionally, ideally we would check to see that any function that returns a storage pointer to this struct type is indeed using the specified location (details TBD, this is a level of static analysis that we don't have in the plugins yet).

A possible obstacle to this approach is that Solidity currently does not expose docstrings for structs (ethereum/solidity#12295), but this can be worked around using a custom lexer like in solidity-comments (could be precisely that library, it currently extracts normal non-doc comments but adding docstrings as well is trivial).

@mudgen
Copy link

mudgen commented Dec 15, 2022

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants