From 6ca8edbf19225bc34b2207a729a48cd3c8f91e81 Mon Sep 17 00:00:00 2001 From: Alex Towle Date: Mon, 30 Sep 2019 16:59:59 -0700 Subject: [PATCH] `@0x:contracts-exchange` Moved the deployment test to the exchange and addressed other review comments --- contracts/exchange/package.json | 2 + contracts/exchange/src/index.ts | 1 + contracts/exchange/src/wrapper_interfaces.ts | 53 ++++++ .../test/end-to-end}/deployment.ts | 179 +++++++++--------- contracts/staking/package.json | 1 - contracts/test-utils/src/constants.ts | 5 - 6 files changed, 149 insertions(+), 92 deletions(-) create mode 100644 contracts/exchange/src/wrapper_interfaces.ts rename contracts/{staking/test/end_to_end_tests => exchange/test/end-to-end}/deployment.ts (74%) diff --git a/contracts/exchange/package.json b/contracts/exchange/package.json index e14b5b896b..ca5e7ce055 100644 --- a/contracts/exchange/package.json +++ b/contracts/exchange/package.json @@ -50,6 +50,8 @@ "devDependencies": { "@0x/abi-gen": "^4.2.1", "@0x/contracts-gen": "^1.0.15", + "@0x/contracts-multisig": "^3.1.14", + "@0x/contracts-staking": "^1.0.0", "@0x/contracts-test-utils": "^3.1.16", "@0x/dev-utils": "^2.3.3", "@0x/sol-compiler": "^3.1.15", diff --git a/contracts/exchange/src/index.ts b/contracts/exchange/src/index.ts index ba813e7caf..ea1294353f 100644 --- a/contracts/exchange/src/index.ts +++ b/contracts/exchange/src/index.ts @@ -1,3 +1,4 @@ export * from './artifacts'; export * from './wrappers'; export * from '../test/utils'; +export * from './wrapper_interfaces'; diff --git a/contracts/exchange/src/wrapper_interfaces.ts b/contracts/exchange/src/wrapper_interfaces.ts new file mode 100644 index 0000000000..11acfb6624 --- /dev/null +++ b/contracts/exchange/src/wrapper_interfaces.ts @@ -0,0 +1,53 @@ +import { PromiseWithTransactionHash } from '@0x/base-contract'; +import { BlockParam, CallData, TransactionReceiptWithDecodedLogs, TxData } from 'ethereum-types'; + +// Generated Wrapper Interfaces +export interface AssetProxyDispatcher { + registerAssetProxy: { + awaitTransactionSuccessAsync: ( + assetProxy: string, + txData?: Partial, + pollingIntervalMs?: number, + timeoutMs?: number, + ) => PromiseWithTransactionHash; + }; + getAssetProxy: { + callAsync(assetProxyId: string, callData?: Partial, defaultBlock?: BlockParam): Promise; + }; +} + +export interface Authorizable extends Ownable { + addAuthorizedAddress: { + awaitTransactionSuccessAsync: ( + target: string, + txData?: Partial, + pollingIntervalMs?: number, + timeoutMs?: number, + ) => PromiseWithTransactionHash; + }; + removeAuthorizedAddress: { + awaitTransactionSuccessAsync: ( + target: string, + txData?: Partial, + pollingIntervalMs?: number, + timeoutMs?: number, + ) => PromiseWithTransactionHash; + }; + authorized: { + callAsync(authority: string, callData?: Partial, defaultBlock?: BlockParam): Promise; + }; +} + +export interface Ownable { + transferOwnership: { + awaitTransactionSuccessAsync: ( + newOwner: string, + txData?: Partial, + pollingIntervalMs?: number, + timeoutMs?: number, + ) => PromiseWithTransactionHash; + }; + owner: { + callAsync(callData?: Partial, defaultBlock?: BlockParam): Promise; + }; +} diff --git a/contracts/staking/test/end_to_end_tests/deployment.ts b/contracts/exchange/test/end-to-end/deployment.ts similarity index 74% rename from contracts/staking/test/end_to_end_tests/deployment.ts rename to contracts/exchange/test/end-to-end/deployment.ts index b471b26d79..e557adae3f 100644 --- a/contracts/staking/test/end_to_end_tests/deployment.ts +++ b/contracts/exchange/test/end-to-end/deployment.ts @@ -6,28 +6,35 @@ import { MultiAssetProxyContract, StaticCallProxyContract, } from '@0x/contracts-asset-proxy'; -import { - artifacts as exchangeArtifacts, - ExchangeContract, - MixinAssetProxyDispatcherAssetProxyRegisteredEventArgs, - MixinProtocolFeesProtocolFeeCollectorAddressEventArgs, - MixinProtocolFeesProtocolFeeMultiplierEventArgs, -} from '@0x/contracts-exchange'; import { artifacts as multisigArtifacts, AssetProxyOwnerContract } from '@0x/contracts-multisig'; -import { blockchainTests, constants, expect } from '@0x/contracts-test-utils'; +import { + artifacts as stakingArtifacts, + ReadOnlyProxyContract, + StakingContract, + StakingEvents, + StakingExchangeAddedEventArgs, + StakingProxyContract, +} from '@0x/contracts-staking'; +import { blockchainTests, constants, expect, filterLogsToArguments } from '@0x/contracts-test-utils'; import { AuthorizableAuthorizedAddressAddedEventArgs, AuthorizableAuthorizedAddressRemovedEventArgs, + AuthorizableEvents, } from '@0x/contracts-utils'; +import { AssetProxyId } from '@0x/types'; import { BigNumber } from '@0x/utils'; -import { LogWithDecodedArgs, TxData } from 'ethereum-types'; +import { TxData } from 'ethereum-types'; import { - artifacts as stakingArtifacts, - MixinExchangeManagerExchangeAddedEventArgs, - ReadOnlyProxyContract, - StakingContract, - StakingProxyContract, + artifacts as exchangeArtifacts, + AssetProxyDispatcher, + Authorizable, + ExchangeAssetProxyRegisteredEventArgs, + ExchangeContract, + ExchangeEvents, + ExchangeProtocolFeeCollectorAddressEventArgs, + ExchangeProtocolFeeMultiplierEventArgs, + Ownable, } from '../../src'; // tslint:disable:no-unnecessary-type-assertion @@ -160,7 +167,7 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { describe('exchange specific', () => { // Registers an asset proxy in the exchange contract and ensure that the correct state changes occurred. async function registerAssetProxyAndAssertSuccessAsync( - registrationContract: any, // TODO(jalextowle): Express this in a type-safe way (during a debt-demolition) + registrationContract: AssetProxyDispatcher, assetProxyAddress: string, assetProxyId: string, ): Promise { @@ -173,13 +180,11 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { ); // Ensure that the correct event was logged. - expect(receipt.logs.length).to.be.eq(1); - const log = receipt.logs[0] as LogWithDecodedArgs< - MixinAssetProxyDispatcherAssetProxyRegisteredEventArgs - >; - expect(log.event).to.be.eq('AssetProxyRegistered'); - expect(log.args.id).to.be.eq(assetProxyId); - expect(log.args.assetProxy).to.be.eq(assetProxyAddress); + const logs = filterLogsToArguments( + receipt.logs, + ExchangeEvents.AssetProxyRegistered, + ); + expect(logs).to.be.deep.eq([{ id: assetProxyId, assetProxy: assetProxyAddress }]); // Ensure that the asset proxy was actually registered. const proxyAddress = await registrationContract.getAssetProxy.callAsync(assetProxyId); @@ -188,7 +193,7 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { // Authorizes an address for a given asset proxy using the owner address. async function authorizeAddressAndAssertSuccessAsync( - authorizable: any, // TODO(jalextowle): Express this in a type-safe way (during a debt-demolition) + authorizable: Authorizable, newAuthorityAddress: string, ): Promise { // Authorize the address. @@ -198,11 +203,11 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { ); // Ensure that the correct log was emitted. - expect(receipt.logs.length).to.be.eq(1); - const log = receipt.logs[0] as LogWithDecodedArgs; - expect(log.event).to.be.eq('AuthorizedAddressAdded'); - expect(log.args.target).to.be.eq(newAuthorityAddress); - expect(log.args.caller).to.be.eq(owner); + const logs = filterLogsToArguments( + receipt.logs, + AuthorizableEvents.AuthorizedAddressAdded, + ); + expect(logs).to.be.deep.eq([{ target: newAuthorityAddress, caller: owner }]); // Ensure that the address was actually authorized. const wasAuthorized = await authorizable.authorized.callAsync(newAuthorityAddress); @@ -211,46 +216,38 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { it('should successfully register the asset proxies in the exchange', async () => { // Register the asset proxies in the exchange. - await registerAssetProxyAndAssertSuccessAsync(exchange, erc20Proxy.address, constants.ERC20_PROXY_ID); - await registerAssetProxyAndAssertSuccessAsync(exchange, erc721Proxy.address, constants.ERC721_PROXY_ID); - await registerAssetProxyAndAssertSuccessAsync( - exchange, - erc1155Proxy.address, - constants.ERC1155_PROXY_ID, - ); + await registerAssetProxyAndAssertSuccessAsync(exchange, erc20Proxy.address, AssetProxyId.ERC20); + await registerAssetProxyAndAssertSuccessAsync(exchange, erc721Proxy.address, AssetProxyId.ERC721); + await registerAssetProxyAndAssertSuccessAsync(exchange, erc1155Proxy.address, AssetProxyId.ERC1155); await registerAssetProxyAndAssertSuccessAsync( exchange, multiAssetProxy.address, - constants.MULTI_ASSET_PROXY_ID, + AssetProxyId.MultiAsset, ); await registerAssetProxyAndAssertSuccessAsync( exchange, staticCallProxy.address, - constants.STATIC_CALL_PROXY_ID, + AssetProxyId.StaticCall, ); }); it('should successfully register the asset proxies in the multi-asset proxy', async () => { // Register the asset proxies in the multi-asset proxy. - await registerAssetProxyAndAssertSuccessAsync( - multiAssetProxy, - erc20Proxy.address, - constants.ERC20_PROXY_ID, - ); + await registerAssetProxyAndAssertSuccessAsync(multiAssetProxy, erc20Proxy.address, AssetProxyId.ERC20); await registerAssetProxyAndAssertSuccessAsync( multiAssetProxy, erc721Proxy.address, - constants.ERC721_PROXY_ID, + AssetProxyId.ERC721, ); await registerAssetProxyAndAssertSuccessAsync( multiAssetProxy, erc1155Proxy.address, - constants.ERC1155_PROXY_ID, + AssetProxyId.ERC1155, ); await registerAssetProxyAndAssertSuccessAsync( multiAssetProxy, staticCallProxy.address, - constants.STATIC_CALL_PROXY_ID, + AssetProxyId.StaticCall, ); }); @@ -273,12 +270,12 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { describe('staking specific', () => { it('should have properly configured the staking proxy with the logic contract and read-only proxy', async () => { // Ensure that the registered read-only proxy is correct. - const readOnlyProxyAddres = await stakingProxy.readOnlyProxy.callAsync(); - expect(readOnlyProxyAddres).to.be.eq(readOnlyProxy.address); + const readOnlyProxyAddress = await stakingProxy.readOnlyProxy.callAsync(); + expect(readOnlyProxyAddress).to.be.eq(readOnlyProxy.address); // Ensure that the registered read-only proxy callee is correct. - const readOnlyProxyCalleeAddres = await stakingProxy.readOnlyProxyCallee.callAsync(); - expect(readOnlyProxyCalleeAddres).to.be.eq(staking.address); + const readOnlyProxyCalleeAddress = await stakingProxy.readOnlyProxyCallee.callAsync(); + expect(readOnlyProxyCalleeAddress).to.be.eq(staking.address); // Ensure that the registered staking contract is correct. const stakingAddress = await stakingProxy.stakingContract.callAsync(); @@ -288,13 +285,16 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { it('should have initialized the correct parameters in the staking proxy', async () => { // Ensure that the correct parameters were set. const params = await stakingWrapper.getParams.callAsync(); - expect(params.length).to.be.eq(6); - expect(params[0]).bignumber.to.be.eq(new BigNumber(864000)); // epochDurationInSeconds - expect(params[1]).bignumber.to.be.eq(new BigNumber(900000)); // rewardDelegatedStakeWeight - expect(params[2]).bignumber.to.be.eq(new BigNumber(100000000000000000000)); // minimumPoolStake - expect(params[3]).bignumber.to.be.eq(10); // maximumMakerInPool - expect(params[4]).bignumber.to.be.eq(1); // cobbDouglasAlphaNumerator - expect(params[5]).bignumber.to.be.eq(2); // cobbDouglasAlphaDenominator + expect(params).to.be.deep.eq( + [ + 864000, // epochDurationInSeconds + 900000, // rewardDelegatedStakeWeight + 100000000000000000000, // minimumPoolStake + 10, // maximumMakerInPool + 1, // cobbDouglasAlphaNumerator + 2, // cobbDouglasAlphaDenominator + ].map(value => new BigNumber(value)), + ); }); }); @@ -306,10 +306,11 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { }); // Ensure that the correct events were logged. - expect(receipt.logs.length).to.be.eq(1); - const log = receipt.logs[0] as LogWithDecodedArgs; - expect(log.event).to.be.eq('ExchangeAdded'); - expect(log.args.exchangeAddress).to.be.eq(exchange.address); + const logs = filterLogsToArguments( + receipt.logs, + StakingEvents.ExchangeAdded, + ); + expect(logs).to.be.deep.eq([{ exchangeAddress: exchange.address }]); // Ensure that the exchange was registered. const wasRegistered = await stakingWrapper.validExchanges.callAsync(exchange.address); @@ -326,13 +327,16 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { ); // Ensure that the correct events were logged. - expect(receipt.logs.length).to.be.eq(1); - const log = receipt.logs[0] as LogWithDecodedArgs< - MixinProtocolFeesProtocolFeeCollectorAddressEventArgs - >; - expect(log.event).to.be.eq('ProtocolFeeCollectorAddress'); - expect(log.args.oldProtocolFeeCollector).bignumber.to.be.eq(constants.NULL_ADDRESS); - expect(log.args.updatedProtocolFeeCollector).bignumber.to.be.eq(stakingProxy.address); + const logs = filterLogsToArguments( + receipt.logs, + ExchangeEvents.ProtocolFeeCollectorAddress, + ); + expect(logs).to.be.deep.eq([ + { + oldProtocolFeeCollector: constants.NULL_ADDRESS, + updatedProtocolFeeCollector: stakingProxy.address, + }, + ]); // Ensure that the staking contract was registered. const feeCollector = await exchange.protocolFeeCollector.callAsync(); @@ -346,11 +350,16 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { ); // Ensure that the correct events were logged. - expect(receipt.logs.length).to.be.eq(1); - const log = receipt.logs[0] as LogWithDecodedArgs; - expect(log.event).to.be.eq('ProtocolFeeMultiplier'); - expect(log.args.oldProtocolFeeMultiplier).bignumber.to.be.eq(constants.ZERO_AMOUNT); - expect(log.args.updatedProtocolFeeMultiplier).bignumber.to.be.eq(protocolFeeMultiplier); + const logs = filterLogsToArguments( + receipt.logs, + ExchangeEvents.ProtocolFeeMultiplier, + ); + expect(logs).to.be.deep.eq([ + { + oldProtocolFeeMultiplier: constants.ZERO_AMOUNT, + updatedProtocolFeeMultiplier: protocolFeeMultiplier, + }, + ]); // Ensure that the protocol fee multiplier was set correctly. const multiplier = await exchange.protocolFeeMultiplier.callAsync(); @@ -360,19 +369,18 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { }); describe('transferring ownership', () => { - // TODO(jalextowle): Express this in a type-safe way (during a debt-demolition) // Removes authorization of the "externally owned address" owner and transfers the authorization // to the asset proxy owner. - async function transferAuthorizationAndAssertSuccessAsync(contract: any): Promise { + async function transferAuthorizationAndAssertSuccessAsync(contract: Authorizable): Promise { // Remove authorization from the old owner. let receipt = await contract.removeAuthorizedAddress.awaitTransactionSuccessAsync(owner, { from: owner }); // Ensure that the correct log was recorded. - expect(receipt.logs.length).to.be.eq(1); - let log = receipt.logs[0] as LogWithDecodedArgs; - expect(log.event).to.be.eq('AuthorizedAddressRemoved'); - expect(log.args.target).to.be.eq(owner); - expect(log.args.caller).to.be.eq(owner); + let logs = filterLogsToArguments( + receipt.logs, + AuthorizableEvents.AuthorizedAddressRemoved, + ); + expect(logs).to.be.deep.eq([{ target: owner, caller: owner }]); // Ensure that the owner was actually removed. let isAuthorized = await contract.authorized.callAsync(owner); @@ -384,20 +392,19 @@ blockchainTests('Deployment and Configuration End to End Tests', env => { }); // Ensure that the correct log was recorded. - expect(receipt.logs.length).to.be.eq(1); - log = receipt.logs[0] as LogWithDecodedArgs; - expect(log.event).to.be.eq('AuthorizedAddressAdded'); - expect(log.args.target).to.be.eq(assetProxyOwner.address); - expect(log.args.caller).to.be.eq(owner); + logs = filterLogsToArguments( + receipt.logs, + AuthorizableEvents.AuthorizedAddressAdded, + ); + expect(logs).to.be.deep.eq([{ target: assetProxyOwner.address, caller: owner }]); // Ensure that the asset-proxy owner was actually authorized. isAuthorized = await contract.authorized.callAsync(assetProxyOwner.address); expect(isAuthorized).to.be.true(); } - // TODO(jalextowle): Express this in a type-safe way (during a debt-demolition) // Transfers ownership of a contract to the asset-proxy owner, and ensures that the change was actually made. - async function transferOwnershipAndAssertSuccessAsync(contract: any): Promise { + async function transferOwnershipAndAssertSuccessAsync(contract: Ownable): Promise { // Transfer ownership to the new owner. await contract.transferOwnership.awaitTransactionSuccessAsync(assetProxyOwner.address, { from: owner }); diff --git a/contracts/staking/package.json b/contracts/staking/package.json index ac37bfe6b9..1b049cbf2d 100644 --- a/contracts/staking/package.json +++ b/contracts/staking/package.json @@ -51,7 +51,6 @@ "devDependencies": { "@0x/abi-gen": "^4.1.0", "@0x/contracts-gen": "^1.0.13", - "@0x/contracts-exchange": "^2.1.14", "@0x/contracts-multisig": "^3.1.14", "@0x/contracts-test-utils": "^3.1.2", "@0x/dev-utils": "^2.2.1", diff --git a/contracts/test-utils/src/constants.ts b/contracts/test-utils/src/constants.ts index 41c6174d50..45bcc6ff72 100644 --- a/contracts/test-utils/src/constants.ts +++ b/contracts/test-utils/src/constants.ts @@ -84,9 +84,4 @@ export const constants = { PPM_DENOMINATOR: 1e6, PPM_100_PERCENT: 1e6, MAX_CODE_SIZE: 24576, - ERC20_PROXY_ID: '0xf47261b0', - ERC721_PROXY_ID: '0x02571792', - ERC1155_PROXY_ID: '0xa7cb5fb7', - MULTI_ASSET_PROXY_ID: '0x94cfcdd7', - STATIC_CALL_PROXY_ID: '0xc339d10a', };