openzeppelin-upgrades icon indicating copy to clipboard operation
openzeppelin-upgrades copied to clipboard

Correctly process Solidity custom storage layout

Open frangio opened this issue 9 months ago • 3 comments

Solidity 0.8.29 released a few days ago adds "custom storage layout" via layout at annotations in contracts.

This affects the correctness of storage analysis done by OpenZeppelin Upgrades, so it should eventually be supported, but until that's implemented it should be detected and result in an error.

This is now available in solidity-ast 0.4.60.

frangio avatar Mar 16 '25 00:03 frangio

  • If storageLayout is included in Solidity compiler output, which is expected to be the case with both Hardhat and Foundry, using a custom storage layout affects the slots of variables. Changing the storage layout in the implementation during an upgrade causes validations errors due to slot changes.
  • If validations are run directly without storageLayout, detect the presence of ContractDefinition.storageLayout and give an error if a different layout root is being used.
  • When extracting type information for namespaced storage layout, we need to remove the layout annotation from the modified copy of the source code, otherwise the following error occurs:
TypeError: Storage layout cannot be specified for abstract contracts.
  --> contracts/MyTokenV2.sol:10:29:
   |
10 | abstract contract MyTokenV2 layout at 0x1 is Initializable, ERC20Upgradeable, OwnableUpgradeable, UUPSUpgradeable {
   |                             ^^^^^^^^^^^^^

ericglau avatar Mar 17 '25 19:03 ericglau

  • Also check for conflicts between a custom layout and any of the namespaces in use (!)
    • ~~Ideally, use the namespace id to calculate the offset of each slot for namespaced variables, so they can be compared to normal variables that are using custom storage layout.~~ -> Not planned, see https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/1145
    • Check for a clash in the storage root location, similar to the current check for duplicate namespace ids.

ericglau avatar Mar 21 '25 21:03 ericglau

Regarding namespaces note that Solidity plans to support an erc7201 function for use in layout at annotations (https://github.com/ethereum/solidity/issues/15727).

frangio avatar Mar 21 '25 23:03 frangio