@0x/contracts-zero-ex: Use (scaled) enums for storage IDs

contracts/zero-ex/contracts/src/features/Bootstrap.sol

@@ -53,7 +53,7 @@ contract Bootstrap is
     }
 
     /// @dev Bootstrap the initial feature set of this contract by delegatecalling
-    ///      into `_bootstrapper`. Before exiting the `bootstrap()` function will
+    ///      into `target`. Before exiting the `bootstrap()` function will
     ///      deregister itself from the proxy to prevent being called again.
     /// @param target The bootstrapper contract address.
     /// @param callData The call data to execute on `_bootstrapper`.
@@ -65,11 +65,12 @@ contract Bootstrap is
                 _bootstrapCaller
             ));
         }
-        LibBootstrap.delegatecallBootstrapFunction(target, callData);
         // Deregister.
         LibProxyStorage.getStorage().impls[this.bootstrap.selector] = address(0);
         // Self-destruct.
         Bootstrap(_implementation).die();
+        // Call the bootstrapper.
+        LibBootstrap.delegatecallBootstrapFunction(target, callData);
     }
 
     /// @dev Self-destructs this contract.

contracts/zero-ex/contracts/src/storage/LibMigrateStorage.sol

@@ -19,14 +19,12 @@
 pragma solidity ^0.6.5;
 pragma experimental ABIEncoderV2;
 
+import "./LibStorage.sol";
+
 
 /// @dev Storage helpers for the `Migrate` feature.
 library LibMigrateStorage {
 
-    /// @dev Globally unique offset for the storage bucket.
-    bytes32 constant internal STORAGE_ID =
-        0x0ed67b719caa0e9bebb7147a4de9fdb6f1c82a984b2297d741a9888432214d5c;
-
     /// @dev Storage bucket for this feature.
     struct Storage {
         // The owner of this contract prior to the `migrate()` call.
@@ -35,7 +33,9 @@ library LibMigrateStorage {
 
     /// @dev Get the storage bucket for this contract.
     function getStorage() internal pure returns (Storage storage stor) {
-        bytes32 storageId = STORAGE_ID;
+        uint256 storageOffset = LibStorage.getStorageOffset(
-        assembly { stor_slot := storageId }
+            LibStorage.StorageId.Migrate
+        );
+        assembly { stor_slot := storageOffset }
     }
 }

contracts/zero-ex/contracts/src/storage/LibOwnableStorage.sol

@@ -19,14 +19,12 @@
 pragma solidity ^0.6.5;
 pragma experimental ABIEncoderV2;
 
+import "./LibStorage.sol";
+
 
 /// @dev Storage helpers for the `Ownable` feature.
 library LibOwnableStorage {
 
-    /// @dev Globally unique offset for the storage bucket.
-    bytes32 constant internal STORAGE_ID =
-        0xeef73acb590dd70cb88ccc8e9832ea7f198de2f3c87ff92d610497d647795b3c;
-
     /// @dev Storage bucket for this feature.
     struct Storage {
         // The owner of this contract.
@@ -35,7 +33,9 @@ library LibOwnableStorage {
 
     /// @dev Get the storage bucket for this contract.
     function getStorage() internal pure returns (Storage storage stor) {
-        bytes32 storageId = STORAGE_ID;
+        uint256 storageOffset = LibStorage.getStorageOffset(
-        assembly { stor_slot := storageId }
+            LibStorage.StorageId.Ownable
+        );
+        assembly { stor_slot := storageOffset }
     }
 }

contracts/zero-ex/contracts/src/storage/LibProxyStorage.sol

@@ -19,14 +19,12 @@
 pragma solidity ^0.6.5;
 pragma experimental ABIEncoderV2;
 
+import "./LibStorage.sol";
+
 
 /// @dev Storage helpers for the proxy contract.
 library LibProxyStorage {
 
-    /// @dev Globally unique offset for the storage bucket.
-    bytes32 constant internal STORAGE_ID =
-        0xd7434ab6df6fb6431870c66fc3fc3d21ddd20b029595942d9e0ffc950ce32f66;
-
     /// @dev Storage bucket for proxy contract.
     struct Storage {
         // Mapping of function selector -> function implementation
@@ -35,7 +33,9 @@ library LibProxyStorage {
 
     /// @dev Get the storage bucket for this contract.
     function getStorage() internal pure returns (Storage storage stor) {
-        bytes32 storageId = STORAGE_ID;
+        uint256 storageOffset = LibStorage.getStorageOffset(
-        assembly { stor_slot := storageId }
+            LibStorage.StorageId.Proxy
+        );
+        assembly { stor_slot := storageOffset }
     }
 }

contracts/zero-ex/contracts/src/storage/LibSimpleFunctionRegistryStorage.sol

@@ -19,14 +19,12 @@
 pragma solidity ^0.6.5;
 pragma experimental ABIEncoderV2;
 
+import "./LibStorage.sol";
+
 
 /// @dev Storage helpers for the `SimpleFunctionRegistry` feature.
 library LibSimpleFunctionRegistryStorage {
 
-    /// @dev Globally unique offset for the storage bucket.
-    bytes32 constant internal STORAGE_ID =
-        0x9817e79514d088041d4834d6eb535f4fa107927bddb0f3c55c3e6b2bfe43daa5;
-
     /// @dev Storage bucket for this feature.
     struct Storage {
         // Mapping of function selector -> implementation history.
@@ -35,7 +33,9 @@ library LibSimpleFunctionRegistryStorage {
 
     /// @dev Get the storage bucket for this contract.
     function getStorage() internal pure returns (Storage storage stor) {
-        bytes32 storageId = STORAGE_ID;
+        uint256 storageOffset = LibStorage.getStorageOffset(
-        assembly { stor_slot := storageId }
+            LibStorage.StorageId.SimpleFunctionRegistry
+        );
+        assembly { stor_slot := storageOffset }
     }
 }

contracts/zero-ex/contracts/src/storage/LibStorage.sol

@@ -0,0 +1,49 @@
+/*
+
+  Copyright 2020 ZeroEx Intl.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+
+*/
+
+pragma solidity ^0.6.5;
+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 Storage IDs for feature storage buckets.
+    enum StorageId {
+        Proxy,
+        SimpleFunctionRegistry,
+        Ownable,
+        Migrate
+    }
+
+    /// @dev Get the storage offset given a storage ID.
+    /// @param storageId An entry in `StorageId`
+    /// @return offset The storage offset.
+    function getStorageOffset(StorageId storageId)
+        internal
+        pure
+        returns (uint256 offset)
+    {
+        return uint256(storageId) * STORAGE_OFFSET_MULTIPLIER;
+    }
+}

contracts/zero-ex/package.json

@@ -40,7 +40,7 @@
     "config": {
         "publicInterfaceContracts": "ZeroEx,IMigrate,IOwnable,ISimpleFunctionRegistry",
         "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.",
-        "abis": "./test/generated-artifacts/@(Bootstrap|FixinCommon|FixinOwnable|IBootstrap|IFeature|IMigrate|IOwnable|ISimpleFunctionRegistry|ITestSimpleFunctionRegistryFeature|InitialMigration|LibBootstrap|LibCommonRichErrors|LibMigrate|LibMigrateRichErrors|LibMigrateStorage|LibOwnableRichErrors|LibOwnableStorage|LibProxyRichErrors|LibProxyStorage|LibSimpleFunctionRegistryRichErrors|LibSimpleFunctionRegistryStorage|Migrate|Ownable|SimpleFunctionRegistry|TestInitialMigration|TestMigrator|TestSimpleFunctionRegistryFeatureImpl1|TestSimpleFunctionRegistryFeatureImpl2|TestZeroExFeature|ZeroEx).json"
+        "abis": "./test/generated-artifacts/@(Bootstrap|FixinCommon|FixinOwnable|IBootstrap|IFeature|IMigrate|IOwnable|ISimpleFunctionRegistry|ITestSimpleFunctionRegistryFeature|InitialMigration|LibBootstrap|LibCommonRichErrors|LibMigrate|LibMigrateRichErrors|LibMigrateStorage|LibOwnableRichErrors|LibOwnableStorage|LibProxyRichErrors|LibProxyStorage|LibSimpleFunctionRegistryRichErrors|LibSimpleFunctionRegistryStorage|LibStorage|Migrate|Ownable|SimpleFunctionRegistry|TestInitialMigration|TestMigrator|TestSimpleFunctionRegistryFeatureImpl1|TestSimpleFunctionRegistryFeatureImpl2|TestZeroExFeature|ZeroEx).json"
     },
     "repository": {
         "type": "git",

contracts/zero-ex/test/artifacts.ts

@@ -26,6 +26,7 @@ import * as LibProxyRichErrors from '../test/generated-artifacts/LibProxyRichErr
 import * as LibProxyStorage from '../test/generated-artifacts/LibProxyStorage.json';
 import * as LibSimpleFunctionRegistryRichErrors from '../test/generated-artifacts/LibSimpleFunctionRegistryRichErrors.json';
 import * as LibSimpleFunctionRegistryStorage from '../test/generated-artifacts/LibSimpleFunctionRegistryStorage.json';
+import * as LibStorage from '../test/generated-artifacts/LibStorage.json';
 import * as Migrate from '../test/generated-artifacts/Migrate.json';
 import * as Ownable from '../test/generated-artifacts/Ownable.json';
 import * as SimpleFunctionRegistry from '../test/generated-artifacts/SimpleFunctionRegistry.json';
@@ -60,6 +61,7 @@ export const artifacts = {
     LibOwnableStorage: LibOwnableStorage as ContractArtifact,
     LibProxyStorage: LibProxyStorage as ContractArtifact,
     LibSimpleFunctionRegistryStorage: LibSimpleFunctionRegistryStorage as ContractArtifact,
+    LibStorage: LibStorage as ContractArtifact,
     ITestSimpleFunctionRegistryFeature: ITestSimpleFunctionRegistryFeature as ContractArtifact,
     TestInitialMigration: TestInitialMigration as ContractArtifact,
     TestMigrator: TestMigrator as ContractArtifact,

contracts/zero-ex/test/storage_uniqueness_test.ts

@@ -5,9 +5,10 @@ import { promisify } from 'util';
 
 describe('Storage ID uniqueness test', () => {
     const STORAGE_SOURCES_DIR = resolve(__dirname, '../../contracts/src/storage');
+
     async function findStorageIdFromSourceFileAsync(path: string): Promise<string | void> {
         const contents = await promisify(readFile)(path, { encoding: 'utf-8' });
-        const m = /STORAGE_ID\s*=\s*(0x[a-fA-F0-9]{64})/m.exec(contents);
+        const m = /LibStorage\.\s*getStorageOffset\(\s*LibStorage\.\s*StorageId\.\s*(\w+)\s*\)/m.exec(contents);
         if (m) {
             return m[1];
         }
@@ -17,7 +18,9 @@ describe('Storage ID uniqueness test', () => {
         const sourcePaths = (await promisify(readdir)(STORAGE_SOURCES_DIR))
             .filter(p => p.endsWith('.sol'))
             .map(p => resolve(STORAGE_SOURCES_DIR, p));
-        const storageIds = await Promise.all(sourcePaths.map(async p => findStorageIdFromSourceFileAsync(p)));
+        const storageIds = (await Promise.all(sourcePaths.map(async p => findStorageIdFromSourceFileAsync(p)))).filter(
+            id => !!id,
+        );
         for (let i = 0; i < storageIds.length; ++i) {
             const storageId = storageIds[i];
             for (let j = 0; j < storageIds.length; ++j) {

contracts/zero-ex/test/wrappers.ts

@@ -24,6 +24,7 @@ export * from '../test/generated-wrappers/lib_proxy_rich_errors';
 export * from '../test/generated-wrappers/lib_proxy_storage';
 export * from '../test/generated-wrappers/lib_simple_function_registry_rich_errors';
 export * from '../test/generated-wrappers/lib_simple_function_registry_storage';
+export * from '../test/generated-wrappers/lib_storage';
 export * from '../test/generated-wrappers/migrate';
 export * from '../test/generated-wrappers/ownable';
 export * from '../test/generated-wrappers/simple_function_registry';

contracts/zero-ex/tsconfig.json

@@ -28,6 +28,7 @@
         "test/generated-artifacts/LibProxyStorage.json",
         "test/generated-artifacts/LibSimpleFunctionRegistryRichErrors.json",
         "test/generated-artifacts/LibSimpleFunctionRegistryStorage.json",
+        "test/generated-artifacts/LibStorage.json",
         "test/generated-artifacts/Migrate.json",
         "test/generated-artifacts/Ownable.json",
         "test/generated-artifacts/SimpleFunctionRegistry.json",