Warn when the storage layout base is near the end of storage (2^64 slots or less) #15912
Conversation
matheusaaguiar
changed the base branch from
develop
to
storageLocationStorageLayout
matheusaaguiar
force-pushed
the
storageLocationStorageLayout
branch
from
a13d53e to
f27af62
Compare
matheusaaguiar
force-pushed
the
warnStorageLayoutNearTheEnd
branch
2 times, most recently
from
3683a32 to
9d77c0f
Compare
matheusaaguiar
force-pushed
the
warnStorageLayoutNearTheEnd
branch
from
9d77c0f to
acb5733
Compare
| if ( | ||
| u256 slotsLeft = std::numeric_limits<u256>::max() - *storageLayoutSpecifier->annotation().baseSlot; | ||
| slotsLeft <= u256(1) << 64 | ||
| ) |
There was a problem hiding this comment.
Now that I think of it, this check is not really limited to contracts with a layout. The dangerous thing is leaving too few slots between the last storage variable and the storage end, regardless of the base slot. It's more likely to happen if you have a non-zero base slot, but technically, you could end up in that situation also by just having large enough variables. And you should get this warning even if you're already getting the one for oversized variables because they warn about different issues.
Let's do this:
- Check base slot + size rather than just base slot.
- Perform the check for both storage and transient storage.
- Do this check even if the layout specifier is not present.
- If the contract has no layout specifier, point at the contract itself in the error.
- If contract has storage/transient variables, point at the last one, saying how close that variable is to the edge.
- Do it only if there are no previous errors reported. This way we won't be unnecessarily reporting it when the base slot + size goes past the edge and the contract won't compile anyway.
There was a problem hiding this comment.
Actually, not sure about transient storage. There are no upgradability issues there so I wonder if we should not warn there or if the fact that this is so close to the edge still warrants a warning...
| 3495_error, | ||
| storageLayoutSpecifier->baseSlotExpression().location(), | ||
| fmt::format( | ||
| "There are {} slots before the end of the contract storage when this specified base layout is used.", |
There was a problem hiding this comment.
How about this?
| "There are {} slots before the end of the contract storage when this specified base layout is used.", | |
| "This contract is very close to the end of storage. This limits its future upgradability.", |
Like I said above, I'd put the number of slots left in the secondary message, pointing at a specific variable.
matheusaaguiar
force-pushed
the
storageLocationStorageLayout
branch
4 times, most recently
from
0c0c671 to
1423298
Compare
cameel
added
the
has dependencies
part of #15727.