Support EIP-7201 in `forge inspect storage`

[radeksvarz]

Component

Forge

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

• [X] Foundry
• [X] Foundryup

What version of Foundry are you on?

forge 0.2.0 (89f0fb9 2024-04-14T00:20:50.727844499Z)

What command(s) is the bug in?

forge inspect

Operating System

Linux

Describe the bug

https://eips.ethereum.org/EIPS/eip-7201 introduces a new (namespace) approach for storage layout esp. for upgradeable contracts. This is already used in OZ v5.

forge inspect storage however does not list such namespaced storage.

To replicate

forge inspect storage of ERC20 based on https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/master/contracts/token/ERC20/ERC20Upgradeable.sol

ideal fix

forge inspect storage --pretty appends the ERC7201 based storage to the list.

[DaniPopes]

inspect storage looks in the artifacts produced by the compiler, so this is mostly out of scope.

[radeksvarz]

Fully agree, this is a new feature. Apologies that I marked that as a bug.

As a workaround one can use the struct placeholder - see below.

forge inspect storage lists such placeholder.

It is not ideal as the content of the struct is not listed, though. I am not sure whether forge inspect could analyze such ERC7201 structs?

    /// @custom:storage-location erc7201:MyContract.storage
    struct MyContractStorage {
        uint256 counter;
        uint256 other;       
    }

    /**
     * @dev Namespace label in the storage layout report (empty space).
     */
    MyContractStorage private ERC7201_MyContract_storage;

    /// custom storage location
    function $MyContractStorage() internal pure returns (MyContractStorage storage $) {
        assembly {
            // keccak256(abi.encode(uint256(keccak256("MyContract.storage")) - 1)) & ~bytes32(uint256(0xff))
            $.slot := 0xfc5b10ad1725e952e459d9135f3c2e559ba1ada7f29b8d58e8d2d858cbc65300
        }
    }

[sakulstra]

Is there any rough estimation on if/when this might be supported?

We consider using oz v5 for an upcoming project, but having no support on forge inspect is a quite huge downside. From reading the related issue it's not exactly clear to me if it's even possible to do right now, so would appreciate some clarification.

[zerosnacks]

Is there any rough estimation on if/when this might be supported?

We consider using oz v5 for an upcoming project, but having no support on forge inspect is a quite huge downside. From reading the related issue it's not exactly clear to me if it's even possible to do right now, so would appreciate some clarification.

Hi @sakulstra,

See Kamil's recent comment here: https://github.com/ethereum/solidity/issues/597#issuecomment-2220808904 outlining the latest updates. It is still in a design phase.

Regarding OpenZeppelin V5, the namespace storage layout exists so you do not have to worry about your local storage layout interfering with theirs. On those grounds it should not be a blocker. It appears https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades does have some level of support for validating namespaced storage layout upgrades.

There might be a possibility to use the /// @custom:storage-location <FORMULA_ID>:<NAMESPACE_ID> identifier defined in the spec when the AST is enabled (--build-info, --ast?). Given that this is critical information users rely on I'm a bit weary of shipping a solution that relies on a temporary workaround like this. It would also involve quite significant work on forge inspect.

[sakulstra]

@zerosnacks thanks for the heads up!

Regarding OpenZeppelin V5, the namespace storage layout exists so you do not have to worry about your local storage layout interfering with theirs. On those grounds it should not be a blocker.

I understand that this is the rational, but the reality is that ppl end up doing fancy stuff (if it's circle rewriting _balances to contain a boolean or aave rewriting it to a struct) - and in these cases it's just super important to have the ability to properly diff storage layouts.

---

By my understand of what is proposed oz will be able do do sth like this in a future solidity version without breaking existing storage(this would be possible in all proposed methods i think, so probably safe to assume it will be possible in the final one as well):

- abstract contract ERC20Upgradeable is Initializable, ContextUpgradeable, IERC20, IERC20Metadata, IERC20Errors {
+ abstract contract ERC20Upgradeable at 0x52c63247e1f47db19d5ce0460030c497f067ca4cebf71ba98eeadabe20bace00 is Initializable, ContextUpgradeable, IERC20, IERC20Metadata, IERC20Errors {
-    /// @custom:storage-location erc7201:openzeppelin.storage.ERC20
-    struct ERC20Storage {
        mapping(address account => uint256) _balances;

        mapping(address account => mapping(address spender => uint256)) _allowances;

        uint256 _totalSupply;

        string _name;
        string _symbol;
-    }

-    // keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.ERC20")) - 1)) & ~bytes32(uint256(0xff))
-    bytes32 private constant ERC20StorageLocation = 0x52c63247e1f47db19d5ce0460030c497f067ca4cebf71ba98eeadabe20bace00;

And once it's out of design phase and part of the language, it's more reasonable to support in foundry I guess. Perhaps that's a compromise we can live with 😅