Inline assembly has now made fully up upgradable contracts possible. One of the main hangups with this is that the storage locations have to stay the same across upgrades. Would it be possible to introduce support for specifying the storage locations for storage variables?
not so!
See Nick Johnson's Library on upgradeability :)
https://gist.github.com/Arachnid/4ca9da48d51e23e5cfe0f0e14dd6318f#file-upgradeable-sol
Especially with contract upgrades in mind, wouldn't it be better to copy the storage layout and "disable" unused state variables by e.g. prefixing them? Otherwise I don't see how you would practically verify that the storage layout is consistent between upgrades.
Is there documentation on how storage layout is determined?
On Wed, May 25, 2016, 1:54 AM chriseth [email protected] wrote:
Especially with contract upgrades in mind, wouldn't it be better to copy
the storage layout and "disable" unused state variables by e.g. prefixing
them? Otherwise I don't see how you would practically verify that the
storage layout is consistent between upgrades.—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/ethereum/solidity/issues/597#issuecomment-221499635
http://solidity.readthedocs.io/en/latest/miscellaneous.html#layout-of-state-variables-in-storage
Ok, so after reading up on storage layouts...
contract MyContractV1 {
uint a;
bytse32 b;
}
In this example, a should be stored in slot 0 and b in slot 1.
Now, consider I _upgrade_ it to the following.
contract MyContractV2 {
int c;
uint a;
bytes32 b;
}
This would end up with c stored in slot 0, a in 1, and b in 2 which would break things.
So, instead, I propose being able to do the following.
contract MyContractV2 {
int c;
uint a @ 0x0;
bytes32 b @ 0x1;
}
The solidity compiler would see that a and b are designated for storage slots 0 and 1 respectively, and would then place c at the next available location, slot 2.
Does that make sense? Is this possible?
I was looking for a complementary/similar feature: the ability to disable packing. (i.e. currently if two storage parameters are each < 256 bits and together they fit into one slot, they are packed together.)
Ultimately the compiler could optimise the packing based on the frequency of changes to one ore more variables within.
With your suggestion this is a given, each marked variable gets its own slot. I would use a different markup though:
storage(0) int a;
storage(1) bytes32 b;
I would use a different markup though
the @ was just the first thing that came to mind. I like storage(...) better.
I think the tradeoff between introducing errors and decreasing readability is much better when just adding int c at the end. If you want, you can also use inheritance (let the upgraded contract inherit from the old contract).
^ 👍 for the inheritance structure...it overall is cheaper and more cost effective to do it that way. I envision a lot of modularity around dapps in the future in regards to storage to better handle updates and save gas.
This came up again as a discussion with @federicobond and I think a good middle ground could be to have an annotation (as proposed in https://github.com/ethereum/solidity/issues/597#issuecomment-221611370), but instead of marking a storage slot, it would rather have a string literal as a key, which is hashed to produce a 256-bit key for storage.
This would be more expensive (due to the fact of using 32-byte long constants and one couldn't combine multiple variables into a single slot), but might be justified for some.
When this annotation is missing, it would default to the current behaviour.
For syntax I propose:
int256 a storage_key("this_is_my_variable");
bytes32 b storage_key("and_this_too");
@axic
I really don't think solidity should have such low-level impact on the storage location. If you want to dislocate storage variables, why not use structs or a mapping to structs?
One more possible other solution:
contract MyContract {
storage("some-collection") {
uint foo;
uint bar;
}
storage("other-collection") {
mapping (uint => bool) qux;
MyStruct baz;
}
}
The advantage of this is that contracts could define blocks of variables that are colocated in storage, but providing gaps, to extend structs later, etc.
Just throwing this as an idea: given that this need arises from avoiding clashes when working with upgradeability, wouldn't it make sense to just avoid clashing by storing all variables in a hashed location, similar to how a mapping works? We could either store all variables from the same contract/struct together (the hash being a contract identifier, and variables are stored at offsets of that hash), or all individual variables in sparse hashed locations.
The issue remains on how to generate an identifier for a contact, to ensure there are no clashes between different contracts, but that identifier is more robust than a simple name. Maybe requiring a special constant with a random value for every contract that will use this approach, similar to old Java's serialVersionUID?
There was also a lengthy related discussion in #4017.
This came up again with #7891 .
If we want to expose really general control we need three components:
Natural restrictions would apply (violating would result in compile time errors):
offset + sizeOfType <= 32numberOfBytesReserved >= sizeOfType(offset + numberOfBytesReserved) % 32 == 0 and only later decide whether to lift thatI would suggest to make all such specifiers optional. Variables without specifiers before any variables with specifiers will be assigned slots as before.
For variables without specifiers after any variables with specifiers there are two options:
For the purpose of inheritance: locations are assigned just as if it was one flat contract containing all variables in the order of C3 linearization.
Example (we can always decide on a different syntax):
contract A {
uint256 a; // will occuply full slot 0
// slots 1 and 2 will remain unused
storage{slot: 3, offset: 0, reserved: 32} bool b; // will occupy full slot 3
storage{slot: 4, offset: 1} bool c; // will occupy the second byte in slot 4
storage{slot: 4, offset: 0} bool d; // will occupy the first byte in slot 4
storage{slot: 4, offset: 16} uint128 d; // will occupy the second half of slot 4
uint128 e; // will occupy the first half of slot 5
storage{slot: 5, offset: 16} uint128 f; // will occupy the second half of slot 5
storage{slot: 6, offset: 0} bool g; // will occupy first byte in slot 6
bool h; // will occupy second byte in slot 6
storage{slot: 6, offset: 2, reserved: 2} bool i; // will occupy third byte in slot 6
bool j; // will occupy fifth byte in slot 6
storage{slot: 6, offset: 16, reserved: 48} uint128 k; // will occupy second half of slot 6
// slot 7 will remain unused
uint128 l; // will use the first half of slot 8
}
An alternative notation-wise would be to merge slot and offset into a single byte offset that is then split into slot = byteOffset/32 and offset = byteOffset%32 (to which the same restrictions would apply). A copy of the example above using this notation:
contract A {
uint256 a; // will occuply full slot 0
// slots 1 and 2 will remain unused
storage{offset: 96, reserved: 32} bool b; // will occupy full slot 3
storage{offset: 129} bool c; // will occupy the second byte in slot 4
storage{offset: 128} bool d; // will occupy the first byte in slot 4
storage{offset: 144} uint128 d; // will occupy the second half of slot 4
uint128 e; // will occupy the first half of slot 5
storage{offset: 160} uint128 f; // will occupy the second half of slot 5
storage{offset: 192} bool g; // will occupy first byte in slot 6
bool h; // will occupy second byte in slot 6
storage{offset: 194, reserved: 2} bool i; // will occupy third byte in slot 6
bool j; // will occupy fifth byte in slot 6
storage{offset: 208, reserved: 48} uint128 k; // will occupy second half of slot 6
// slot 7 will remain unused
uint128 l; // will use the first half of slot 8
}
Another alternative would be to require specifying the location for all variables, if the location is specified for any variable.
Also we could at a later point allow compile time evaluated expressions in the specifier, i.e.:
storage{slot: keccak256("some_key")} uint256 some_key;
Although we'd need to consider that one could construct those to specifically collide with some mapping key, so this would be dangerous.
Although that's also true for choosing some specific value for slot: that happens to be the location of some mapping element.
Maybe we should gather some data about how this feature would be used. One use is avoiding clashes during upgrades, another is having more efficient use of storage by combining small variables in a certain way. I think just providing full flexibility all the time might not be the way to go as it is too easy to get wrong. So it could already be enough to only allow hashed locations and another way to specify which variables to combine (without specifying the offset exactly) or when to insert "start a new slot here".
What can "go wrong"? Or in particular, what can go wrong that we can't easily detect at compile time?
I'd argue that it makes more sense to provide a general solution and, if deemed necessary, restrict it to simple cases (as in restrict to some particular kinds of values for slot, etc. - e.g. restricting to only supporting "start a new slot here" would be to require slot to be the "current slot" plus one and require offset to be zero).
That way we can always extend the very same solution to support more cases, instead of needing breaking changes and new language features...
One use is avoiding clashes during upgrades
For the sake of upgrades, it'd seem that the only requirement is to be able to assign an immutable id to a variable, which should be deterministically mapped to a slot (like the storage{slot: keccak256("some_key")} proposed above). It's not really important _where_ in the storage the variable is kept.
As for EIP2330 linked above, the requirements are pretty much the same. As long as there is a deterministic process for calculating the storage slot, the actual slot can then be just exposed in the ABI for any consumers.
After reading this issue, and thinking about the issue, this is my proposed solution.
deterministicuint256 deterministic myNumber; is keccak256('myNumber')immutable which doesn't use storage, deterministic variables will be removed when calculating the sequential slots of each variable, the keccak256 is calculated at compile time and used for all instances.variable name in multiple inherited contracts with mixed deterministic states it should throw an error at compile time saying "can't use deterministically declared variable non-deterministically"It seems simple enough to remove complexity but accomplish some of the major goals of this thread.
However, it seems like this thread has grown with a list of reasons and use cases which can only be satisfied by increasingly complex low level access which is difficult to implement without adding a foot cannon.
Edit: Thinking about it more, it might be sufficient to commit only to the variable name, as someone using this feature would probably make their variable names more descriptive address deterministic openZepplinProxyImplementation; which should probably throw an error if an inherited contract tries to use uint256 deterministic openZepplinProxyImplementation; which could be another foot cannon.
Edit2: I have a low-deploy-gas-cost proxy contract that I optimized the bytecode for, and it would be great to use single byte storage slots (ie. 0xff) without needing to create 255 dummies... I just tried upping my storage slot for the proxy to PUSH32 with a random hash instead of the 0 I'm using right now, it bumped my deploy cost from 80k to 100k (since I have 1 PUSH in the deploy code and 2 PUSHes of it in the contract code.)... so I would definitely also enjoy the ability to specify an arbitrary value for the slot as well... that said, my use case is extremely niche so I understand not accommodating it.
Copying the suggestion from @dominicletz from #7593:
A new keyword
fixed(@N)is proposed that can be used to define fixed slot position in interfaces.interface ContractAddressMap { public fixed(@5) mapping(bytes32=>address) addr; }
Most helpful comment
This came up again as a discussion with @federicobond and I think a good middle ground could be to have an annotation (as proposed in https://github.com/ethereum/solidity/issues/597#issuecomment-221611370), but instead of marking a storage slot, it would rather have a string literal as a key, which is hashed to produce a 256-bit key for storage.
This would be more expensive (due to the fact of using 32-byte long constants and one couldn't combine multiple variables into a single slot), but might be justified for some.
When this annotation is missing, it would default to the current behaviour.
For syntax I propose: