diff --git a/contracts/zero-ex/contracts/src/errors/LibMigrateRichErrors.sol b/contracts/zero-ex/contracts/src/errors/LibMigrateRichErrors.sol deleted file mode 100644 index 0400c7c371..0000000000 --- a/contracts/zero-ex/contracts/src/errors/LibMigrateRichErrors.sol +++ /dev/null @@ -1,47 +0,0 @@ -/* - - 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; - - -library LibMigrateRichErrors { - - // solhint-disable func-name-mixedcase - - function AlreadyMigratingError() - internal - pure - returns (bytes memory) - { - return abi.encodeWithSelector( - bytes4(keccak256("AlreadyMigratingError()")) - ); - } - - function MigrateCallFailedError(address target, bytes memory resultData) - internal - pure - returns (bytes memory) - { - return abi.encodeWithSelector( - bytes4(keccak256("MigrateCallFailedError(address,bytes)")), - target, - resultData - ); - } -} diff --git a/contracts/zero-ex/contracts/src/errors/LibOwnableRichErrors.sol b/contracts/zero-ex/contracts/src/errors/LibOwnableRichErrors.sol index 6cb47072e6..313d8822f9 100644 --- a/contracts/zero-ex/contracts/src/errors/LibOwnableRichErrors.sol +++ b/contracts/zero-ex/contracts/src/errors/LibOwnableRichErrors.sol @@ -47,4 +47,26 @@ library LibOwnableRichErrors { bytes4(keccak256("TransferOwnerToZeroError()")) ); } + + function AlreadyMigratingError() + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + bytes4(keccak256("AlreadyMigratingError()")) + ); + } + + function MigrateCallFailedError(address target, bytes memory resultData) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + bytes4(keccak256("MigrateCallFailedError(address,bytes)")), + target, + resultData + ); + } } diff --git a/contracts/zero-ex/contracts/src/features/IMigrate.sol b/contracts/zero-ex/contracts/src/features/IMigrate.sol deleted file mode 100644 index 99e46515e5..0000000000 --- a/contracts/zero-ex/contracts/src/features/IMigrate.sol +++ /dev/null @@ -1,43 +0,0 @@ -/* - - 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 Migration features. -interface IMigrate { - - /// @dev Emitted when `migrate()` is called. - /// @param caller The caller of `migrate()`. - /// @param migrator The migration contract. - event Migrated(address caller, address migrator); - - /// @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 (`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. - /// @param data The call data. - function migrate(address target, bytes calldata data) external; - - /// @dev Get the true owner of this contract during a migration. - /// @return owner The true owner of this contract. - function getMigrationOwner() external view returns (address owner); -} diff --git a/contracts/zero-ex/contracts/src/features/IOwnable.sol b/contracts/zero-ex/contracts/src/features/IOwnable.sol index 5f8ab8cb38..a15dcf3625 100644 --- a/contracts/zero-ex/contracts/src/features/IOwnable.sol +++ b/contracts/zero-ex/contracts/src/features/IOwnable.sol @@ -23,7 +23,23 @@ import "@0x/contracts-utils/contracts/src/v06/interfaces/IOwnableV06.sol"; // solhint-disable no-empty-blocks -/// @dev Owner management features. +/// @dev Owner management and migration features. interface IOwnable is IOwnableV06 -{} +{ + /// @dev Emitted when `migrate()` is called. + /// @param caller The caller of `migrate()`. + /// @param migrator The migration contract. + /// @param newOwner The address of the new owner. + event Migrated(address caller, address migrator, address newOwner); + + /// @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 (`keccack('MIGRATE_SUCCESS')`). Only callable by the owner. + /// The owner will be temporarily set to `address(this)` inside the call. + /// Before returning, the owner will be set to `newOwner`. + /// @param target The migrator contract address. + /// @param newOwner The address of the new owner. + /// @param data The call data. + function migrate(address target, bytes calldata data, address newOwner) external; +} diff --git a/contracts/zero-ex/contracts/src/features/Migrate.sol b/contracts/zero-ex/contracts/src/features/Migrate.sol deleted file mode 100644 index cc78fa1263..0000000000 --- a/contracts/zero-ex/contracts/src/features/Migrate.sol +++ /dev/null @@ -1,94 +0,0 @@ -/* - - 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; - -import "../fixins/FixinOwnable.sol"; -import "../errors/LibOwnableRichErrors.sol"; -import "../storage/LibOwnableStorage.sol"; -import "../storage/LibMigrateStorage.sol"; -import "../migrations/LibMigrate.sol"; -import "../migrations/LibBootstrap.sol"; -import "./IFeature.sol"; -import "./IMigrate.sol"; -import "./ISimpleFunctionRegistry.sol"; - - -/// @dev Migration features. -contract Migrate is - IFeature, - IMigrate, - FixinOwnable -{ - // solhint-disable const-name-snakecase - - /// @dev Name of this feature. - string constant public override FEATURE_NAME = "Migrate"; - /// @dev Version of this feature. - uint256 constant public override FEATURE_VERSION = (1 << 64) | (0 << 32) | (0); - - /// @dev Initializes this feature. - /// @param impl The actual address of this feature contract. - /// @return success Magic bytes if successful. - function bootstrap(address impl) external returns (bytes4 success) { - // Register feature functions. - ISimpleFunctionRegistry(address(this)).extend(this.migrate.selector, impl); - ISimpleFunctionRegistry(address(this)).extend(this.getMigrationOwner.selector, impl); - return LibBootstrap.BOOTSTRAP_SUCCESS; - } - - /// @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 (`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. - /// @param data The call data. - function migrate(address target, bytes calldata data) - external - override - onlyOwner - { - LibOwnableStorage.Storage storage ownableStor = LibOwnableStorage.getStorage(); - LibMigrateStorage.Storage storage stor = LibMigrateStorage.getStorage(); - address prevOwner = ownableStor.owner; - if (prevOwner == address(this)) { - // If the owner is already set to ourselves then we've reentered. - _rrevert(LibMigrateRichErrors.AlreadyMigratingError()); - } - // Temporarily set the owner to ourselves to enable admin functions. - ownableStor.owner = address(this); - stor.migrationOwner = prevOwner; - - // Perform the migration. - LibMigrate.delegatecallMigrateFunction(target, data); - - // Restore the owner. - ownableStor.owner = prevOwner; - stor.migrationOwner = address(0); - - emit Migrated(msg.sender, target); - } - - /// @dev Get the true owner of this contract during a migration. - /// @return owner The true owner of this contract. - function getMigrationOwner() external override view returns (address owner) { - return LibMigrateStorage.getStorage().migrationOwner; - } -} diff --git a/contracts/zero-ex/contracts/src/features/Ownable.sol b/contracts/zero-ex/contracts/src/features/Ownable.sol index 1b8429434c..2946664ffc 100644 --- a/contracts/zero-ex/contracts/src/features/Ownable.sol +++ b/contracts/zero-ex/contracts/src/features/Ownable.sol @@ -23,6 +23,7 @@ import "../fixins/FixinOwnable.sol"; import "../errors/LibOwnableRichErrors.sol"; import "../storage/LibOwnableStorage.sol"; import "../migrations/LibBootstrap.sol"; +import "../migrations/LibMigrate.sol"; import "./IFeature.sol"; import "./IOwnable.sol"; import "./ISimpleFunctionRegistry.sol"; @@ -34,26 +35,36 @@ contract Ownable is IOwnable, FixinOwnable { - // solhint-disable const-name-snakecase + // solhint-disable const-name-snakecase /// @dev Name of this feature. string constant public override FEATURE_NAME = "Ownable"; /// @dev Version of this feature. uint256 constant public override FEATURE_VERSION = (1 << 64) | (0 << 32) | (0); + // solhint-enable const-name-snakecase + + // solhint-disable + /// @dev The deployed address of this contract. + address immutable private _implementation; + // solhint-enable + + constructor() public { + _implementation = address(this); + } /// @dev Initializes this feature. The intial owner will be set to this (ZeroEx) /// to allow the bootstrappers to call `extend()`. Ownership should be /// transferred to the real owner by the bootstrapper after /// bootstrapping is complete. - /// @param impl the actual address of this feature contract. /// @return success Magic bytes if successful. - function bootstrap(address impl) external returns (bytes4 success) { + function bootstrap() external returns (bytes4 success) { // Set the owner to ourselves to allow bootstrappers to call `extend()`. LibOwnableStorage.getStorage().owner = address(this); // Register feature functions. - ISimpleFunctionRegistry(address(this)).extend(this.transferOwnership.selector, impl); - ISimpleFunctionRegistry(address(this)).extend(this.getOwner.selector, impl); + ISimpleFunctionRegistry(address(this)).extend(this.transferOwnership.selector, _implementation); + ISimpleFunctionRegistry(address(this)).extend(this.getOwner.selector, _implementation); + ISimpleFunctionRegistry(address(this)).extend(this.migrate.selector, _implementation); return LibBootstrap.BOOTSTRAP_SUCCESS; } @@ -75,6 +86,37 @@ contract Ownable 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 (`keccack('MIGRATE_SUCCESS')`). Only callable by the owner. + /// The owner will be temporarily set to `address(this)` inside the call. + /// Before returning, the owner will be set to `newOwner`. + /// @param target The migrator contract address. + /// @param data The call data. + /// @param newOwner The address of the new owner. + function migrate(address target, bytes calldata data, address newOwner) + external + override + onlyOwner + { + LibOwnableStorage.Storage storage stor = LibOwnableStorage.getStorage(); + address prevOwner = stor.owner; + if (prevOwner == address(this)) { + // If the owner is already set to ourselves then we've reentered. + _rrevert(LibOwnableRichErrors.AlreadyMigratingError()); + } + // Temporarily set the owner to ourselves so we can perform admin functions. + stor.owner = address(this); + + // Perform the migration. + LibMigrate.delegatecallMigrateFunction(target, data); + + // Update the owner. + stor.owner = newOwner; + + emit Migrated(msg.sender, target, newOwner); + } + /// @dev Get the owner of this contract. /// @return owner_ The owner of this contract. function getOwner() external override view returns (address owner_) { diff --git a/contracts/zero-ex/contracts/src/features/SimpleFunctionRegistry.sol b/contracts/zero-ex/contracts/src/features/SimpleFunctionRegistry.sol index 0ce160a134..8e0c1cade9 100644 --- a/contracts/zero-ex/contracts/src/features/SimpleFunctionRegistry.sol +++ b/contracts/zero-ex/contracts/src/features/SimpleFunctionRegistry.sol @@ -34,12 +34,22 @@ contract SimpleFunctionRegistry is ISimpleFunctionRegistry, FixinOwnable { - // solhint-disable const-name-snakecase + // solhint-disable const-name-snakecase /// @dev Name of this feature. string constant public override FEATURE_NAME = "SimpleFunctionRegistry"; /// @dev Version of this feature. uint256 constant public override FEATURE_VERSION = (1 << 64) | (0 << 32) | (0); + // solhint-enable const-name-snakecase + + // solhint-disable + /// @dev The deployed address of this contract. + address immutable private _implementation; + // solhint-enable + + constructor() public { + _implementation = address(this); + } /// @dev Initializes this feature. /// @param impl The actual address of this feature contract. diff --git a/contracts/zero-ex/contracts/src/migrations/InitialMigration.sol b/contracts/zero-ex/contracts/src/migrations/InitialMigration.sol index 02c0e04130..1ac217b252 100644 --- a/contracts/zero-ex/contracts/src/migrations/InitialMigration.sol +++ b/contracts/zero-ex/contracts/src/migrations/InitialMigration.sol @@ -23,7 +23,6 @@ import "../ZeroEx.sol"; import "../features/IBootstrap.sol"; import "../features/SimpleFunctionRegistry.sol"; import "../features/Ownable.sol"; -import "../features/Migrate.sol"; import "./LibBootstrap.sol"; @@ -72,13 +71,6 @@ contract InitialMigration { abi.encodeWithSelector(ownable.bootstrap.selector, address(ownable)) ); - // Initialize Migrate. - Migrate migrate = new Migrate(); - LibBootstrap.delegatecallBootstrapFunction( - address(migrate), - abi.encodeWithSelector(migrate.bootstrap.selector, address(migrate)) - ); - // Transfer ownership to the real owner. Ownable(address(this)).transferOwnership(owner); diff --git a/contracts/zero-ex/contracts/src/migrations/LibMigrate.sol b/contracts/zero-ex/contracts/src/migrations/LibMigrate.sol index c994dfbdd7..6581e539a1 100644 --- a/contracts/zero-ex/contracts/src/migrations/LibMigrate.sol +++ b/contracts/zero-ex/contracts/src/migrations/LibMigrate.sol @@ -20,7 +20,7 @@ pragma solidity ^0.6.5; pragma experimental ABIEncoderV2; import "@0x/contracts-utils/contracts/src/v06/errors/LibRichErrorsV06.sol"; -import "../errors/LibMigrateRichErrors.sol"; +import "../errors/LibOwnableRichErrors.sol"; library LibMigrate { @@ -44,7 +44,7 @@ library LibMigrate { abi.decode(resultData, (bytes4)) != MIGRATE_SUCCESS) { LibRichErrorsV06.rrevert( - LibMigrateRichErrors.MigrateCallFailedError(target, resultData) + LibOwnableRichErrors.MigrateCallFailedError(target, resultData) ); } } diff --git a/contracts/zero-ex/contracts/src/storage/LibMigrateStorage.sol b/contracts/zero-ex/contracts/src/storage/LibMigrateStorage.sol deleted file mode 100644 index 2aab390f5d..0000000000 --- a/contracts/zero-ex/contracts/src/storage/LibMigrateStorage.sol +++ /dev/null @@ -1,41 +0,0 @@ -/* - - 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; - -import "./LibStorage.sol"; - - -/// @dev Storage helpers for the `Migrate` feature. -library LibMigrateStorage { - - /// @dev Storage bucket for this feature. - struct Storage { - // The owner of this contract prior to the `migrate()` call. - address migrationOwner; - } - - /// @dev Get the storage bucket for this contract. - function getStorage() internal pure returns (Storage storage stor) { - uint256 storageOffset = LibStorage.getStorageOffset( - LibStorage.StorageId.Migrate - ); - assembly { stor_slot := storageOffset } - } -} diff --git a/contracts/zero-ex/contracts/src/storage/LibStorage.sol b/contracts/zero-ex/contracts/src/storage/LibStorage.sol index 083f933a4f..edcd92e1f1 100644 --- a/contracts/zero-ex/contracts/src/storage/LibStorage.sol +++ b/contracts/zero-ex/contracts/src/storage/LibStorage.sol @@ -33,8 +33,7 @@ library LibStorage { Unused, // Unused buffer for state accidents. Proxy, SimpleFunctionRegistry, - Ownable, - Migrate + Ownable } /// @dev Get the storage offset given a storage ID. diff --git a/contracts/zero-ex/contracts/test/TestMigrator.sol b/contracts/zero-ex/contracts/test/TestMigrator.sol index 8aeb6ad6e9..71393a4018 100644 --- a/contracts/zero-ex/contracts/test/TestMigrator.sol +++ b/contracts/zero-ex/contracts/test/TestMigrator.sol @@ -21,21 +21,18 @@ pragma experimental ABIEncoderV2; import "../src/migrations/LibMigrate.sol"; import "../src/features/IOwnable.sol"; -import "../src/features/IMigrate.sol"; contract TestMigrator { event TestMigrateCalled( bytes callData, - address owner, - address actualOwner + address owner ); function succeedingMigrate() external returns (bytes4 success) { emit TestMigrateCalled( msg.data, - IOwnable(address(this)).getOwner(), - IMigrate(address(this)).getMigrationOwner() + IOwnable(address(this)).getOwner() ); return LibMigrate.MIGRATE_SUCCESS; } @@ -43,8 +40,7 @@ contract TestMigrator { function failingMigrate() external returns (bytes4 success) { emit TestMigrateCalled( msg.data, - IOwnable(address(this)).getOwner(), - IMigrate(address(this)).getMigrationOwner() + IOwnable(address(this)).getOwner() ); return 0xdeadbeef; } diff --git a/contracts/zero-ex/package.json b/contracts/zero-ex/package.json index 63b1ba1f76..1397061b7e 100644 --- a/contracts/zero-ex/package.json +++ b/contracts/zero-ex/package.json @@ -38,9 +38,9 @@ "docs:json": "typedoc --excludePrivate --excludeExternals --excludeProtected --ignoreCompilerErrors --target ES5 --tsconfig typedoc-tsconfig.json --json $JSON_FILE_PATH $PROJECT_FILES" }, "config": { - "publicInterfaceContracts": "ZeroEx,IMigrate,IOwnable,ISimpleFunctionRegistry", + "publicInterfaceContracts": "ZeroEx,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|LibStorage|Migrate|Ownable|SimpleFunctionRegistry|TestInitialMigration|TestMigrator|TestSimpleFunctionRegistryFeatureImpl1|TestSimpleFunctionRegistryFeatureImpl2|TestZeroExFeature|ZeroEx).json" + "abis": "./test/generated-artifacts/@(Bootstrap|FixinCommon|FixinOwnable|IBootstrap|IFeature|IOwnable|ISimpleFunctionRegistry|ITestSimpleFunctionRegistryFeature|InitialMigration|LibBootstrap|LibCommonRichErrors|LibMigrate|LibOwnableRichErrors|LibOwnableStorage|LibProxyRichErrors|LibProxyStorage|LibSimpleFunctionRegistryRichErrors|LibSimpleFunctionRegistryStorage|LibStorage|Ownable|SimpleFunctionRegistry|TestInitialMigration|TestMigrator|TestSimpleFunctionRegistryFeatureImpl1|TestSimpleFunctionRegistryFeatureImpl2|TestZeroExFeature|ZeroEx).json" }, "repository": { "type": "git", diff --git a/contracts/zero-ex/src/artifacts.ts b/contracts/zero-ex/src/artifacts.ts index 47b300d3af..aa3ab07a57 100644 --- a/contracts/zero-ex/src/artifacts.ts +++ b/contracts/zero-ex/src/artifacts.ts @@ -5,13 +5,11 @@ */ import { ContractArtifact } from 'ethereum-types'; -import * as IMigrate from '../generated-artifacts/IMigrate.json'; import * as IOwnable from '../generated-artifacts/IOwnable.json'; import * as ISimpleFunctionRegistry from '../generated-artifacts/ISimpleFunctionRegistry.json'; import * as ZeroEx from '../generated-artifacts/ZeroEx.json'; export const artifacts = { ZeroEx: ZeroEx as ContractArtifact, - IMigrate: IMigrate as ContractArtifact, IOwnable: IOwnable as ContractArtifact, ISimpleFunctionRegistry: ISimpleFunctionRegistry as ContractArtifact, }; diff --git a/contracts/zero-ex/src/wrappers.ts b/contracts/zero-ex/src/wrappers.ts index 32e226aa5c..b7b12cc29d 100644 --- a/contracts/zero-ex/src/wrappers.ts +++ b/contracts/zero-ex/src/wrappers.ts @@ -3,7 +3,6 @@ * Warning: This file is auto-generated by contracts-gen. Don't edit manually. * ----------------------------------------------------------------------------- */ -export * from '../generated-wrappers/i_migrate'; export * from '../generated-wrappers/i_ownable'; export * from '../generated-wrappers/i_simple_function_registry'; export * from '../generated-wrappers/zero_ex'; diff --git a/contracts/zero-ex/test/artifacts.ts b/contracts/zero-ex/test/artifacts.ts index 37920eaa34..3d8de03402 100644 --- a/contracts/zero-ex/test/artifacts.ts +++ b/contracts/zero-ex/test/artifacts.ts @@ -10,7 +10,6 @@ import * as FixinCommon from '../test/generated-artifacts/FixinCommon.json'; import * as FixinOwnable from '../test/generated-artifacts/FixinOwnable.json'; import * as IBootstrap from '../test/generated-artifacts/IBootstrap.json'; import * as IFeature from '../test/generated-artifacts/IFeature.json'; -import * as IMigrate from '../test/generated-artifacts/IMigrate.json'; import * as InitialMigration from '../test/generated-artifacts/InitialMigration.json'; import * as IOwnable from '../test/generated-artifacts/IOwnable.json'; import * as ISimpleFunctionRegistry from '../test/generated-artifacts/ISimpleFunctionRegistry.json'; @@ -18,8 +17,6 @@ import * as ITestSimpleFunctionRegistryFeature from '../test/generated-artifacts import * as LibBootstrap from '../test/generated-artifacts/LibBootstrap.json'; import * as LibCommonRichErrors from '../test/generated-artifacts/LibCommonRichErrors.json'; import * as LibMigrate from '../test/generated-artifacts/LibMigrate.json'; -import * as LibMigrateRichErrors from '../test/generated-artifacts/LibMigrateRichErrors.json'; -import * as LibMigrateStorage from '../test/generated-artifacts/LibMigrateStorage.json'; import * as LibOwnableRichErrors from '../test/generated-artifacts/LibOwnableRichErrors.json'; import * as LibOwnableStorage from '../test/generated-artifacts/LibOwnableStorage.json'; import * as LibProxyRichErrors from '../test/generated-artifacts/LibProxyRichErrors.json'; @@ -27,7 +24,6 @@ import * as LibProxyStorage from '../test/generated-artifacts/LibProxyStorage.js 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'; import * as TestInitialMigration from '../test/generated-artifacts/TestInitialMigration.json'; @@ -39,17 +35,14 @@ import * as ZeroEx from '../test/generated-artifacts/ZeroEx.json'; export const artifacts = { ZeroEx: ZeroEx as ContractArtifact, LibCommonRichErrors: LibCommonRichErrors as ContractArtifact, - LibMigrateRichErrors: LibMigrateRichErrors as ContractArtifact, LibOwnableRichErrors: LibOwnableRichErrors as ContractArtifact, LibProxyRichErrors: LibProxyRichErrors as ContractArtifact, LibSimpleFunctionRegistryRichErrors: LibSimpleFunctionRegistryRichErrors as ContractArtifact, Bootstrap: Bootstrap as ContractArtifact, IBootstrap: IBootstrap as ContractArtifact, IFeature: IFeature as ContractArtifact, - IMigrate: IMigrate as ContractArtifact, IOwnable: IOwnable as ContractArtifact, ISimpleFunctionRegistry: ISimpleFunctionRegistry as ContractArtifact, - Migrate: Migrate as ContractArtifact, Ownable: Ownable as ContractArtifact, SimpleFunctionRegistry: SimpleFunctionRegistry as ContractArtifact, FixinCommon: FixinCommon as ContractArtifact, @@ -57,7 +50,6 @@ export const artifacts = { InitialMigration: InitialMigration as ContractArtifact, LibBootstrap: LibBootstrap as ContractArtifact, LibMigrate: LibMigrate as ContractArtifact, - LibMigrateStorage: LibMigrateStorage as ContractArtifact, LibOwnableStorage: LibOwnableStorage as ContractArtifact, LibProxyStorage: LibProxyStorage as ContractArtifact, LibSimpleFunctionRegistryStorage: LibSimpleFunctionRegistryStorage as ContractArtifact, diff --git a/contracts/zero-ex/test/features/migrate_test.ts b/contracts/zero-ex/test/features/migrate_test.ts deleted file mode 100644 index 51bfb242fd..0000000000 --- a/contracts/zero-ex/test/features/migrate_test.ts +++ /dev/null @@ -1,94 +0,0 @@ -import { blockchainTests, expect, LogDecoder, randomAddress, verifyEventsFromLogs } from '@0x/contracts-test-utils'; -import { hexUtils, OwnableRevertErrors, StringRevertError, ZeroExRevertErrors } from '@0x/utils'; - -import { artifacts } from '../artifacts'; -import { initialMigrateAsync } from '../utils/migration'; -import { IMigrateContract, IOwnableContract, TestMigratorContract, TestMigratorEvents } from '../wrappers'; - -blockchainTests.resets('Migrate feature', env => { - let owner: string; - let ownable: IOwnableContract; - let migrate: IMigrateContract; - let testMigrator: TestMigratorContract; - let succeedingMigrateFnCallData: string; - let failingMigrateFnCallData: string; - let revertingMigrateFnCallData: string; - let logDecoder: LogDecoder; - - before(async () => { - logDecoder = new LogDecoder(env.web3Wrapper, artifacts); - [owner] = await env.getAccountAddressesAsync(); - const zeroEx = await initialMigrateAsync(owner, env.provider, env.txDefaults); - ownable = new IOwnableContract(zeroEx.address, env.provider, env.txDefaults); - migrate = new IMigrateContract(zeroEx.address, env.provider, env.txDefaults); - testMigrator = await TestMigratorContract.deployFrom0xArtifactAsync( - artifacts.TestMigrator, - env.provider, - env.txDefaults, - artifacts, - ); - succeedingMigrateFnCallData = testMigrator.succeedingMigrate().getABIEncodedTransactionData(); - failingMigrateFnCallData = testMigrator.failingMigrate().getABIEncodedTransactionData(); - revertingMigrateFnCallData = testMigrator.revertingMigrate().getABIEncodedTransactionData(); - }); - - describe('migrate()', () => { - it('non-owner cannot call migrate()', async () => { - const notOwner = randomAddress(); - const tx = migrate - .migrate(testMigrator.address, succeedingMigrateFnCallData) - .awaitTransactionSuccessAsync({ from: notOwner }); - return expect(tx).to.revertWith(new OwnableRevertErrors.OnlyOwnerError(notOwner, owner)); - }); - - it('can successfully execute a migration', async () => { - const receipt = await migrate - .migrate(testMigrator.address, succeedingMigrateFnCallData) - .awaitTransactionSuccessAsync({ from: owner }); - const { logs } = logDecoder.decodeReceiptLogs(receipt); - verifyEventsFromLogs( - logs, - [ - { - callData: succeedingMigrateFnCallData, - owner: migrate.address, - actualOwner: owner, - }, - ], - TestMigratorEvents.TestMigrateCalled, - ); - }); - - it('owner is restored after a migration', async () => { - await migrate - .migrate(testMigrator.address, succeedingMigrateFnCallData) - .awaitTransactionSuccessAsync({ from: owner }); - const currentOwner = await ownable.getOwner().callAsync(); - expect(currentOwner).to.eq(owner); - }); - - it('failing migration reverts', async () => { - const tx = migrate - .migrate(testMigrator.address, failingMigrateFnCallData) - .awaitTransactionSuccessAsync({ from: owner }); - return expect(tx).to.revertWith( - new ZeroExRevertErrors.Migrate.MigrateCallFailedError( - testMigrator.address, - hexUtils.rightPad('0xdeadbeef'), - ), - ); - }); - - it('reverting migration reverts', async () => { - const tx = migrate - .migrate(testMigrator.address, revertingMigrateFnCallData) - .awaitTransactionSuccessAsync({ from: owner }); - return expect(tx).to.revertWith( - new ZeroExRevertErrors.Migrate.MigrateCallFailedError( - testMigrator.address, - new StringRevertError('OOPSIE').encode(), - ), - ); - }); - }); -}); diff --git a/contracts/zero-ex/test/features/ownable_test.ts b/contracts/zero-ex/test/features/ownable_test.ts index 1609d90a21..88bf408143 100644 --- a/contracts/zero-ex/test/features/ownable_test.ts +++ b/contracts/zero-ex/test/features/ownable_test.ts @@ -1,18 +1,34 @@ -import { blockchainTests, expect, randomAddress, verifyEventsFromLogs } from '@0x/contracts-test-utils'; -import { OwnableRevertErrors } from '@0x/utils'; +import { blockchainTests, expect, LogDecoder, randomAddress, verifyEventsFromLogs } from '@0x/contracts-test-utils'; +import { hexUtils, OwnableRevertErrors, StringRevertError, ZeroExRevertErrors } from '@0x/utils'; +import { artifacts } from '../artifacts'; import { initialMigrateAsync } from '../utils/migration'; -import { IOwnableContract, IOwnableEvents } from '../wrappers'; +import { IOwnableContract, IOwnableEvents, TestMigratorContract, TestMigratorEvents } from '../wrappers'; blockchainTests.resets('Ownable feature', env => { const notOwner = randomAddress(); let owner: string; let ownable: IOwnableContract; + let testMigrator: TestMigratorContract; + let succeedingMigrateFnCallData: string; + let failingMigrateFnCallData: string; + let revertingMigrateFnCallData: string; + let logDecoder: LogDecoder; before(async () => { [owner] = await env.getAccountAddressesAsync(); + logDecoder = new LogDecoder(env.web3Wrapper, artifacts); const zeroEx = await initialMigrateAsync(owner, env.provider, env.txDefaults); ownable = new IOwnableContract(zeroEx.address, env.provider, env.txDefaults); + testMigrator = await TestMigratorContract.deployFrom0xArtifactAsync( + artifacts.TestMigrator, + env.provider, + env.txDefaults, + artifacts, + ); + succeedingMigrateFnCallData = testMigrator.succeedingMigrate().getABIEncodedTransactionData(); + failingMigrateFnCallData = testMigrator.failingMigrate().getABIEncodedTransactionData(); + revertingMigrateFnCallData = testMigrator.revertingMigrate().getABIEncodedTransactionData(); }); describe('transferOwnership()', () => { @@ -38,4 +54,57 @@ blockchainTests.resets('Ownable feature', env => { expect(await ownable.getOwner().callAsync()).to.eq(newOwner); }); }); + + describe('migrate()', () => { + const newOwner = randomAddress(); + + it('non-owner cannot call migrate()', async () => { + const tx = ownable + .migrate(testMigrator.address, succeedingMigrateFnCallData, newOwner) + .awaitTransactionSuccessAsync({ from: notOwner }); + return expect(tx).to.revertWith(new OwnableRevertErrors.OnlyOwnerError(notOwner, owner)); + }); + + it('can successfully execute a migration', async () => { + const receipt = await ownable + .migrate(testMigrator.address, succeedingMigrateFnCallData, newOwner) + .awaitTransactionSuccessAsync({ from: owner }); + const { logs } = logDecoder.decodeReceiptLogs(receipt); + verifyEventsFromLogs( + logs, + [ + { + callData: succeedingMigrateFnCallData, + owner: ownable.address, + }, + ], + TestMigratorEvents.TestMigrateCalled, + ); + expect(await ownable.getOwner().callAsync()).to.eq(newOwner); + }); + + it('failing migration reverts', async () => { + const tx = ownable + .migrate(testMigrator.address, failingMigrateFnCallData, newOwner) + .awaitTransactionSuccessAsync({ from: owner }); + return expect(tx).to.revertWith( + new ZeroExRevertErrors.Ownable.MigrateCallFailedError( + testMigrator.address, + hexUtils.rightPad('0xdeadbeef'), + ), + ); + }); + + it('reverting migration reverts', async () => { + const tx = ownable + .migrate(testMigrator.address, revertingMigrateFnCallData, newOwner) + .awaitTransactionSuccessAsync({ from: owner }); + return expect(tx).to.revertWith( + new ZeroExRevertErrors.Ownable.MigrateCallFailedError( + testMigrator.address, + new StringRevertError('OOPSIE').encode(), + ), + ); + }); + }); }); diff --git a/contracts/zero-ex/test/wrappers.ts b/contracts/zero-ex/test/wrappers.ts index 5b229bc0ac..60d1125898 100644 --- a/contracts/zero-ex/test/wrappers.ts +++ b/contracts/zero-ex/test/wrappers.ts @@ -8,7 +8,6 @@ export * from '../test/generated-wrappers/fixin_common'; export * from '../test/generated-wrappers/fixin_ownable'; export * from '../test/generated-wrappers/i_bootstrap'; export * from '../test/generated-wrappers/i_feature'; -export * from '../test/generated-wrappers/i_migrate'; export * from '../test/generated-wrappers/i_ownable'; export * from '../test/generated-wrappers/i_simple_function_registry'; export * from '../test/generated-wrappers/i_test_simple_function_registry_feature'; @@ -16,8 +15,6 @@ export * from '../test/generated-wrappers/initial_migration'; export * from '../test/generated-wrappers/lib_bootstrap'; export * from '../test/generated-wrappers/lib_common_rich_errors'; export * from '../test/generated-wrappers/lib_migrate'; -export * from '../test/generated-wrappers/lib_migrate_rich_errors'; -export * from '../test/generated-wrappers/lib_migrate_storage'; export * from '../test/generated-wrappers/lib_ownable_rich_errors'; export * from '../test/generated-wrappers/lib_ownable_storage'; export * from '../test/generated-wrappers/lib_proxy_rich_errors'; @@ -25,7 +22,6 @@ 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'; export * from '../test/generated-wrappers/test_initial_migration'; diff --git a/contracts/zero-ex/tsconfig.json b/contracts/zero-ex/tsconfig.json index 09bd4baabc..628c352f79 100644 --- a/contracts/zero-ex/tsconfig.json +++ b/contracts/zero-ex/tsconfig.json @@ -3,7 +3,6 @@ "compilerOptions": { "outDir": "lib", "rootDir": ".", "resolveJsonModule": true }, "include": ["./src/**/*", "./test/**/*", "./generated-wrappers/**/*"], "files": [ - "generated-artifacts/IMigrate.json", "generated-artifacts/IOwnable.json", "generated-artifacts/ISimpleFunctionRegistry.json", "generated-artifacts/ZeroEx.json", @@ -12,7 +11,6 @@ "test/generated-artifacts/FixinOwnable.json", "test/generated-artifacts/IBootstrap.json", "test/generated-artifacts/IFeature.json", - "test/generated-artifacts/IMigrate.json", "test/generated-artifacts/IOwnable.json", "test/generated-artifacts/ISimpleFunctionRegistry.json", "test/generated-artifacts/ITestSimpleFunctionRegistryFeature.json", @@ -20,8 +18,6 @@ "test/generated-artifacts/LibBootstrap.json", "test/generated-artifacts/LibCommonRichErrors.json", "test/generated-artifacts/LibMigrate.json", - "test/generated-artifacts/LibMigrateRichErrors.json", - "test/generated-artifacts/LibMigrateStorage.json", "test/generated-artifacts/LibOwnableRichErrors.json", "test/generated-artifacts/LibOwnableStorage.json", "test/generated-artifacts/LibProxyRichErrors.json", @@ -29,7 +25,6 @@ "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", "test/generated-artifacts/TestInitialMigration.json",