Namespace storage #420
Namespace storage #420
Conversation
minghinmatthewlam
requested review from
iansuvak,
feuGeneA,
geoff-vball,
bernard-avalabs,
michaelkaplan13 and
cam-schultz
| */ | ||
| mapping(address teleporterAddress => bool paused) private _pausedTeleporterAddresses; | ||
| // solhint-disable private-vars-leading-underscore | ||
| /// @custom:storage-location erc7201:teleporter.storage.TeleporterUpgradeable |
There was a problem hiding this comment.
For my own understanding, do you know why the custom storage location has three forward slashes? This seems to be the convention in OZ's docs as well as the EIP 7201 spec, but I couldn't find any justification for it.
There was a problem hiding this comment.
Not understanding the rationale behind this was bothering me also...dug into it and my mind was blown 😅
There was a problem hiding this comment.
So "///" is for single line natspec comments, and "/**" is for multi-line natspec comments.
(Where all natspec comments start with a @ tag, and can be used to auto-generate docs)
| @@ -22,7 +22,15 @@ abstract contract TeleporterOwnerUpgradeable is TeleporterUpgradeable, OwnableUp | |||
| address initialOwner | |||
| ) internal onlyInitializing { | |||
| __TeleporterUpgradeable_init(teleporterRegistryAddress); | |||
| __Ownable_init(); | |||
| __Ownable_init_unchained(); | |||
There was a problem hiding this comment.
IIUC, __Ownable_init vs __Ownable_init_unchained doesn't have any functional difference here.
For my understanding, is there a reason why __Ownable_init_unchained should be preferred? Seems like the normal _init would be recommended unless there is a specific need to ensure none of the Ownable parent initializers are called
Upgrading to OpenZeppelin v5 and Solidity 0.8.20
|
merging into #397 |
Why this should be merged
Fixes #415
How this works
Adds namespace storage to all contracts according to ERC7201. Namespace storage helps prevent collision between storage of different logic contracts when used in the context of upgradeable contracts and proxies.
Changes:
@custom:storage-locationaccording to erc7201$structHow this was tested
CI
TODO: should ideally have unit tests to make sure that the storage location are computed correctly.
How is this documented
TBD