diff --git a/contracts/zero-ex/contracts/src/storage/LibOwnableStorage.sol b/contracts/zero-ex/contracts/src/storage/LibOwnableStorage.sol index 641c9b4ed4..811109acfc 100644 --- a/contracts/zero-ex/contracts/src/storage/LibOwnableStorage.sol +++ b/contracts/zero-ex/contracts/src/storage/LibOwnableStorage.sol @@ -33,9 +33,9 @@ library LibOwnableStorage { /// @dev Get the storage bucket for this contract. function getStorage() internal pure returns (Storage storage stor) { - uint256 storageOffset = LibStorage.getStorageOffset( + uint256 storageSlot = LibStorage.getStorageSlot( LibStorage.StorageId.Ownable ); - assembly { stor_slot := storageOffset } + assembly { stor_slot := storageSlot } } } diff --git a/contracts/zero-ex/contracts/src/storage/LibProxyStorage.sol b/contracts/zero-ex/contracts/src/storage/LibProxyStorage.sol index c13817eb80..2a37a9439d 100644 --- a/contracts/zero-ex/contracts/src/storage/LibProxyStorage.sol +++ b/contracts/zero-ex/contracts/src/storage/LibProxyStorage.sol @@ -33,9 +33,9 @@ library LibProxyStorage { /// @dev Get the storage bucket for this contract. function getStorage() internal pure returns (Storage storage stor) { - uint256 storageOffset = LibStorage.getStorageOffset( + uint256 storageSlot = LibStorage.getStorageSlot( LibStorage.StorageId.Proxy ); - assembly { stor_slot := storageOffset } + assembly { stor_slot := storageSlot } } } diff --git a/contracts/zero-ex/contracts/src/storage/LibSimpleFunctionRegistryStorage.sol b/contracts/zero-ex/contracts/src/storage/LibSimpleFunctionRegistryStorage.sol index 27dd52fb54..9b536a018d 100644 --- a/contracts/zero-ex/contracts/src/storage/LibSimpleFunctionRegistryStorage.sol +++ b/contracts/zero-ex/contracts/src/storage/LibSimpleFunctionRegistryStorage.sol @@ -33,9 +33,9 @@ library LibSimpleFunctionRegistryStorage { /// @dev Get the storage bucket for this contract. function getStorage() internal pure returns (Storage storage stor) { - uint256 storageOffset = LibStorage.getStorageOffset( + uint256 storageSlot = LibStorage.getStorageSlot( LibStorage.StorageId.SimpleFunctionRegistry ); - assembly { stor_slot := storageOffset } + assembly { stor_slot := storageSlot } } } diff --git a/contracts/zero-ex/contracts/src/storage/LibStorage.sol b/contracts/zero-ex/contracts/src/storage/LibStorage.sol index edcd92e1f1..e6ae3bbd6d 100644 --- a/contracts/zero-ex/contracts/src/storage/LibStorage.sol +++ b/contracts/zero-ex/contracts/src/storage/LibStorage.sol @@ -23,29 +23,29 @@ pragma experimental ABIEncoderV2; /// @dev Common storage helpers library LibStorage { - /// @dev What to multiply a storage ID by to get its offset. - /// This is also the maximum number of fields inside a storage - /// bucket. - uint256 internal constant STORAGE_OFFSET_MULTIPLIER = 1e18; + /// @dev What to bit-shift a storage ID by to get its slot. + /// This gives us a maximum of 2**128 inline fields in each bucket. + uint256 private constant STORAGE_SLOT_EXP = 128; /// @dev Storage IDs for feature storage buckets. enum StorageId { - Unused, // Unused buffer for state accidents. Proxy, SimpleFunctionRegistry, Ownable } - /// @dev Get the storage offset given a storage ID. + /// @dev Get the storage slot given a storage ID. We assign unique, well-spaced + /// slots to storage bucket variables to ensure they do not overlap. + /// See: https://solidity.readthedocs.io/en/v0.6.6/assembly.html#access-to-external-variables-functions-and-libraries /// @param storageId An entry in `StorageId` - /// @return offset The storage offset. - function getStorageOffset(StorageId storageId) + /// @return slot The storage slot. + function getStorageSlot(StorageId storageId) internal pure - returns (uint256 offset) + returns (uint256 slot) { - // This should never overflow with a reasonable `STORAGE_OFFSET_MULTIPLIER` + // This should never overflow with a reasonable `STORAGE_SLOT_EXP` // because Solidity will do a range check on `storageId` during the cast. - return uint256(storageId) * STORAGE_OFFSET_MULTIPLIER; + return (uint256(storageId) + 1) << STORAGE_SLOT_EXP; } }