Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Added a storage gap at the end of Initializable's storage layout. #215

Merged
merged 2 commits into from
Oct 10, 2018
Merged

Added a storage gap at the end of Initializable's storage layout. #215

merged 2 commits into from
Oct 10, 2018

Conversation

nventuro
Copy link
Contributor

Created from the discussion in OpenZeppelin/openzeppelin-contracts-upgradeable#29

This is the same gap all openzeppelin-zos contracts have. My editor also removed some trailing whitespace, but I can undo that if you have strong feelings about it 😛

@spalladino
Copy link
Contributor

@nventuro it seems there was a test that was dependent on storage positions. Could you fix them?

@nventuro
Copy link
Contributor Author

Done! I also improved the wording of the comment, since it was very outdated.

// - 0: Initializable storage
// - 1-50: Initailizable reserved storage (50 slots)
// - 51: initializerRan
// - 52: x
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be...?

//  - 0: Initializable#initialized
//  - 1: Initializable#initializing
//  - 2-51: Initailizable#gap
//  - 52: x

Copy link
Contributor Author

@nventuro nventuro Oct 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, both initialized and initializing are bool, so they are packed together into the same storage slot. The test uses InitiazableMock, which has bool initializerRan and uint256 x (in that order), those are not packed due to x using the full slot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If only we had a storage layout report... 😛

@spalladino spalladino added the status:ready-to-merge Order mergify to merge label Oct 10, 2018
@mergify mergify bot merged commit 12d4292 into OpenZeppelin:master Oct 10, 2018
@nventuro nventuro deleted the initializable-storage-gap branch October 10, 2018 14:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:ready-to-merge Order mergify to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants