diff --git a/contracts/zero-ex/contracts/src/features/IMigrate.sol b/contracts/zero-ex/contracts/src/features/IMigrate.sol index 76c69a9501..99e46515e5 100644 --- a/contracts/zero-ex/contracts/src/features/IMigrate.sol +++ b/contracts/zero-ex/contracts/src/features/IMigrate.sol @@ -30,7 +30,7 @@ interface IMigrate { /// @dev Execute a migration function in the context of the ZeroEx contract. /// The result of the function being called should be the magic bytes - /// 0x2c64c5ef. Only callable by the owner. + /// 0x2c64c5ef (`keccack('MIGRATE_SUCCESS')`). Only callable by the owner. /// The owner will be temporarily set to `address(this)` inside the call. /// The original owner can be retrieved through `getMigrationOwner()`.` /// @param target The migrator contract address. diff --git a/contracts/zero-ex/contracts/src/features/Migrate.sol b/contracts/zero-ex/contracts/src/features/Migrate.sol index f5ff916c19..8bbc20cd6c 100644 --- a/contracts/zero-ex/contracts/src/features/Migrate.sol +++ b/contracts/zero-ex/contracts/src/features/Migrate.sol @@ -55,7 +55,7 @@ contract Migrate is /// @dev Execute a migration function in the context of the ZeroEx contract. /// The result of the function being called should be the magic bytes - /// 0x2c64c5ef. Only callable by the owner. + /// 0x2c64c5ef (`keccack('MIGRATE_SUCCESS')`). Only callable by the owner. /// The owner will be temporarily set to `address(this)` inside the call. /// The original owner can be retrieved through `getMigrationOwner()`.` /// @param target The migrator contract address. diff --git a/contracts/zero-ex/contracts/src/migrations/LibBootstrap.sol b/contracts/zero-ex/contracts/src/migrations/LibBootstrap.sol index e2a1744517..b5304441fc 100644 --- a/contracts/zero-ex/contracts/src/migrations/LibBootstrap.sol +++ b/contracts/zero-ex/contracts/src/migrations/LibBootstrap.sol @@ -26,6 +26,7 @@ import "../errors/LibProxyRichErrors.sol"; library LibBootstrap { /// @dev Magic bytes returned by the bootstrapper to indicate success. + /// This is `keccack('BOOTSTRAP_SUCCESS')`. bytes4 internal constant BOOTSTRAP_SUCCESS = 0xd150751b; /// @dev Perform a delegatecall and ensure it returns the magic bytes. diff --git a/contracts/zero-ex/contracts/src/migrations/LibMigrate.sol b/contracts/zero-ex/contracts/src/migrations/LibMigrate.sol index ebc71edc98..c994dfbdd7 100644 --- a/contracts/zero-ex/contracts/src/migrations/LibMigrate.sol +++ b/contracts/zero-ex/contracts/src/migrations/LibMigrate.sol @@ -26,6 +26,7 @@ import "../errors/LibMigrateRichErrors.sol"; library LibMigrate { /// @dev Magic bytes returned by a migrator to indicate success. + /// This is `keccack('MIGRATE_SUCCESS')`. bytes4 internal constant MIGRATE_SUCCESS = 0x2c64c5ef; /// @dev Perform a delegatecall and ensure it returns the magic bytes. diff --git a/contracts/zero-ex/contracts/src/storage/LibStorage.sol b/contracts/zero-ex/contracts/src/storage/LibStorage.sol index e494c652ff..403a1440c6 100644 --- a/contracts/zero-ex/contracts/src/storage/LibStorage.sol +++ b/contracts/zero-ex/contracts/src/storage/LibStorage.sol @@ -44,6 +44,9 @@ library LibStorage { pure returns (uint256 offset) { + // We don't use safeMul here to save gas. + // This should never overflow with a reasonable `STORAGE_OFFSET_MULTIPLIER` + // because Solidity will do a range check on `storageId` during the cast. return uint256(storageId) * STORAGE_OFFSET_MULTIPLIER; } } diff --git a/contracts/zero-ex/test/storage_uniqueness_test.ts b/contracts/zero-ex/test/storage_uniqueness_test.ts index f0e51d0c18..180433fa12 100644 --- a/contracts/zero-ex/test/storage_uniqueness_test.ts +++ b/contracts/zero-ex/test/storage_uniqueness_test.ts @@ -14,7 +14,7 @@ describe('Storage ID uniqueness test', () => { } } - it('all STORAGE_IDs are unique in storage libraries', async () => { + it('all StorageId references are unique in storage libraries', async () => { const sourcePaths = (await promisify(readdir)(STORAGE_SOURCES_DIR)) .filter(p => p.endsWith('.sol')) .map(p => resolve(STORAGE_SOURCES_DIR, p)); @@ -26,7 +26,7 @@ describe('Storage ID uniqueness test', () => { for (let j = 0; j < storageIds.length; ++j) { if (i !== j && storageId === storageIds[j]) { throw new Error( - `Found duplicate STORAGE_ID ${storageId} ` + + `Found duplicate StorageId ${storageId} ` + `in files ${basename(sourcePaths[i])}, ${basename(sourcePaths[j])}`, ); }