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

Allow custom storage slots #162

Open
ItsNickBarry opened this issue Nov 9, 2022 · 4 comments · May be fixed by #236
Open

Allow custom storage slots #162

ItsNickBarry opened this issue Nov 9, 2022 · 4 comments · May be fixed by #236

Comments

@ItsNickBarry
Copy link
Member

A function like this must be added to each storage library:

function layout(bytes32 slot) internal pure returns (Layout storage l) {
    assembly {
        l.slot := slot
    }
}
@ItsNickBarry
Copy link
Member Author

If we do it like this, we can maintain full test coverage without writing any new tests:

function layout() internal pure returns (Layout storage l) {
    l = layout(STORAGE_SLOT);
}

function layout(bytes32 slot) internal pure returns (Layout storage l) {
    assembly {
        l.slot := slot
    }
}

@ItsNickBarry
Copy link
Member Author

After some discussion, this seems less useful than I originally thought. For it to work, each contract would have to store a reference to the STORAGE_SLOT defined in each library, and access storage via the layout(bytes32) function. The storage slot reference would have to be overridable.

This could still be a useful change, but needs more thought.


Use cases for custom slots:

  • End users maintaining legacy code can override the SS storage slot if SS ever changes its storage slot seed format (solidstate.contracts.storage.xxx).
  • Some storage layouts (and contracts) can be reused for different purposes in the same deployment - for example, it may be useful to treat some internal value as an ERC20 and use the ERC20 storage layout for this purpose. This would be most applicable to diamond proxies.

@ItsNickBarry
Copy link
Member Author

Would only work if storage libraries were merged into internal contracts, which is not desirable.

@ItsNickBarry
Copy link
Member Author

I'm once again considering implementing this. If Solidstate introduces a breaking change to support EIP-7201 (#166), we will want to provide a way for existing users to continue using the legacy storage slots.

@ItsNickBarry ItsNickBarry reopened this Jan 29, 2024
@ItsNickBarry ItsNickBarry linked a pull request Jan 30, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

1 participant