Test coordinator protocol fees

contracts/coordinator/contracts/src/MixinCoordinatorCore.sol

@@ -26,12 +26,21 @@ import "./interfaces/ICoordinatorCore.sol";
 import "./interfaces/ICoordinatorApprovalVerifier.sol";
 
 
+// solhint-disable no-empty-blocks
 contract MixinCoordinatorCore is
     Refundable,
     LibConstants,
     ICoordinatorApprovalVerifier,
     ICoordinatorCore
 {
+
+    /// @dev A payable fallback function that makes this contract "payable". This is necessary to allow
+    ///      this contract to gracefully handle refunds from the Exchange.
+    function ()
+        external
+        payable
+    {}
+
     /// @dev Executes a 0x transaction that has been signed by the feeRecipients that correspond to
     ///      each order in the transaction's Exchange calldata.
     /// @param transaction 0x transaction containing salt, signerAddress, and data.

contracts/coordinator/test/coordinator.ts

@@ -1,5 +1,5 @@
 import { ERC20ProxyContract, ERC20Wrapper } from '@0x/contracts-asset-proxy';
-import { DummyERC20TokenContract } from '@0x/contracts-erc20';
+import { artifacts as erc20Artifacts, DummyERC20TokenContract, WETH9Contract } from '@0x/contracts-erc20';
 import {
     artifacts as exchangeArtifacts,
     constants as exchangeConstants,
@@ -10,6 +10,7 @@ import {
     ExchangeEvents,
     ExchangeFillEventArgs,
     ExchangeFunctionName,
+    TestProtocolFeeCollectorContract,
 } from '@0x/contracts-exchange';
 import {
     blockchainTests,
@@ -48,25 +49,16 @@ blockchainTests.resets('Coordinator tests', env => {
     let makerTransactionFactory: TransactionFactory;
     let approvalFactory: ApprovalFactory;
 
-    // const GAS_PRICE = new BigNumber(env.txDefaults.gasPrice || constants.DEFAULT_GAS_PRICE);
+    const GAS_PRICE = new BigNumber(env.txDefaults.gasPrice || constants.DEFAULT_GAS_PRICE);
-    // const PROTOCOL_FEE_MULTIPLIER = new BigNumber(150);
+    const PROTOCOL_FEE_MULTIPLIER = new BigNumber(150);
-    // const PROTOCOL_FEE = GAS_PRICE.times(PROTOCOL_FEE_MULTIPLIER);
+    const PROTOCOL_FEE = GAS_PRICE.times(PROTOCOL_FEE_MULTIPLIER);
-    const PROTOCOL_FEE = new BigNumber(0);
 
     before(async () => {
         chainId = await env.getChainIdAsync();
         const accounts = await env.getAccountAddressesAsync();
         const usedAddresses = ([owner, makerAddress, takerAddress, feeRecipientAddress] = accounts);
 
-        erc20Wrapper = new ERC20Wrapper(env.provider, usedAddresses, owner);
+        // Deploy Exchange
-        erc20Proxy = await erc20Wrapper.deployProxyAsync();
-        const numDummyErc20ToDeploy = 3;
-        [erc20TokenA, erc20TokenB, makerFeeToken] = await erc20Wrapper.deployDummyTokensAsync(
-            numDummyErc20ToDeploy,
-            constants.DUMMY_TOKEN_DECIMALS,
-        );
-        await erc20Wrapper.setBalancesAndAllowancesAsync();
-
         exchange = await ExchangeContract.deployFrom0xArtifactAsync(
             exchangeArtifacts.Exchange,
             env.provider,
@@ -75,17 +67,53 @@ blockchainTests.resets('Coordinator tests', env => {
             new BigNumber(chainId),
         );
 
-        await erc20Proxy.addAuthorizedAddress.awaitTransactionSuccessAsync(
+        // Set up ERC20
-            exchange.address,
+        erc20Wrapper = new ERC20Wrapper(env.provider, usedAddresses, owner);
-            { from: owner },
+        erc20Proxy = await erc20Wrapper.deployProxyAsync();
-            constants.AWAIT_TRANSACTION_MINED_MS,
+        const numDummyErc20ToDeploy = 3;
-        );
+        [erc20TokenA, erc20TokenB, makerFeeToken] = await erc20Wrapper.deployDummyTokensAsync(
-        await exchange.registerAssetProxy.awaitTransactionSuccessAsync(
+            numDummyErc20ToDeploy,
-            erc20Proxy.address,
+            constants.DUMMY_TOKEN_DECIMALS,
-            { from: owner },
-            constants.AWAIT_TRANSACTION_MINED_MS,
         );
+        await erc20Proxy.addAuthorizedAddress.awaitTransactionSuccessAsync(exchange.address, { from: owner });
+        await exchange.registerAssetProxy.awaitTransactionSuccessAsync(erc20Proxy.address, { from: owner });
 
+        // Set up WETH
+        const wethContract = await WETH9Contract.deployFrom0xArtifactAsync(
+            erc20Artifacts.WETH9,
+            env.provider,
+            env.txDefaults,
+            {},
+        );
+        const weth = new DummyERC20TokenContract(wethContract.address, env.provider);
+        erc20Wrapper.addDummyTokenContract(weth);
+        await erc20Wrapper.setBalancesAndAllowancesAsync();
+
+        // Set up Protocol Fee Collector
+        const protocolFeeCollector = await TestProtocolFeeCollectorContract.deployFrom0xArtifactAsync(
+            exchangeArtifacts.TestProtocolFeeCollector,
+            env.provider,
+            env.txDefaults,
+            {},
+            weth.address,
+        );
+        await exchange.setProtocolFeeMultiplier.awaitTransactionSuccessAsync(PROTOCOL_FEE_MULTIPLIER);
+        await exchange.setProtocolFeeCollectorAddress.awaitTransactionSuccessAsync(protocolFeeCollector.address);
+        for (const account of usedAddresses) {
+            await wethContract.deposit.awaitTransactionSuccessAsync({
+                from: account,
+                value: constants.ONE_ETHER,
+            });
+            await wethContract.approve.awaitTransactionSuccessAsync(
+                protocolFeeCollector.address,
+                constants.UNLIMITED_ALLOWANCE_IN_BASE_UNITS,
+                {
+                    from: account,
+                },
+            );
+        }
+
+        // Deploy Coordinator
         coordinatorContract = await CoordinatorContract.deployFrom0xArtifactAsync(
             artifacts.Coordinator,
             env.provider,
@@ -100,7 +128,6 @@ blockchainTests.resets('Coordinator tests', env => {
             ...constants.STATIC_ORDER_PARAMS,
             senderAddress: coordinatorContract.address,
             makerAddress,
-            takerAddress,
             feeRecipientAddress,
             makerAssetData: assetDataUtils.encodeERC20AssetData(erc20TokenA.address),
             takerAssetData: assetDataUtils.encodeERC20AssetData(erc20TokenB.address),
@@ -133,13 +160,13 @@ blockchainTests.resets('Coordinator tests', env => {
     function expectedFillEvent(order: SignedOrder): ExchangeFillEventArgs {
         return {
             makerAddress: order.makerAddress,
+            takerAddress,
+            senderAddress: order.senderAddress,
             feeRecipientAddress: order.feeRecipientAddress,
             makerAssetData: order.makerAssetData,
             takerAssetData: order.takerAssetData,
             makerFeeAssetData: order.makerFeeAssetData,
             takerFeeAssetData: order.takerFeeAssetData,
-            takerAddress: order.takerAddress,
-            senderAddress: order.senderAddress,
             makerAssetFilledAmount: order.makerAssetAmount,
             takerAssetFilledAmount: order.takerAssetAmount,
             makerFeePaid: order.makerFee,
@@ -152,10 +179,10 @@ blockchainTests.resets('Coordinator tests', env => {
     function expectedCancelEvent(order: SignedOrder): ExchangeCancelEventArgs {
         return {
             makerAddress: order.makerAddress,
+            senderAddress: order.senderAddress,
             feeRecipientAddress: order.feeRecipientAddress,
             makerAssetData: order.makerAssetData,
             takerAssetData: order.takerAssetData,
-            senderAddress: order.senderAddress,
             orderHash: orderHashUtils.getOrderHashHex(order),
         };
     }
@@ -179,10 +206,8 @@ blockchainTests.resets('Coordinator tests', env => {
                     transaction.signature,
                     [approvalExpirationTimeSeconds],
                     [approval.signature],
-                    { from: takerAddress },
+                    { from: takerAddress, value: PROTOCOL_FEE },
-                    constants.AWAIT_TRANSACTION_MINED_MS,
                 );
-
                 verifyEvents(transactionReceipt, [expectedFillEvent(orders[0])], ExchangeEvents.Fill);
             });
             it(`${fnName} should fill the order if called by approver`, async () => {
@@ -195,8 +220,7 @@ blockchainTests.resets('Coordinator tests', env => {
                     transaction.signature,
                     [],
                     [],
-                    { from: feeRecipientAddress },
+                    { from: feeRecipientAddress, value: PROTOCOL_FEE },
-                    constants.AWAIT_TRANSACTION_MINED_MS,
                 );
                 verifyEvents(transactionReceipt, [expectedFillEvent(orders[0])], ExchangeEvents.Fill);
             });
@@ -213,6 +237,7 @@ blockchainTests.resets('Coordinator tests', env => {
                     {
                         from: takerAddress,
                         gas: constants.MAX_EXECUTE_TRANSACTION_GAS,
+                        value: PROTOCOL_FEE,
                     },
                 );
 
@@ -239,7 +264,7 @@ blockchainTests.resets('Coordinator tests', env => {
                     transaction.signature,
                     [approvalExpirationTimeSeconds],
                     [signature],
-                    { from: takerAddress },
+                    { from: takerAddress, value: PROTOCOL_FEE },
                 );
 
                 const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
@@ -265,7 +290,7 @@ blockchainTests.resets('Coordinator tests', env => {
                     transaction.signature,
                     [approvalExpirationTimeSeconds],
                     [approval.signature],
-                    { from: takerAddress },
+                    { from: takerAddress, value: PROTOCOL_FEE },
                 );
 
                 const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
@@ -291,7 +316,7 @@ blockchainTests.resets('Coordinator tests', env => {
                     transaction.signature,
                     [approvalExpirationTimeSeconds],
                     [approval.signature],
-                    { from: owner },
+                    { from: owner, value: PROTOCOL_FEE },
                 );
                 expect(tx).to.revertWith(new CoordinatorRevertErrors.InvalidOriginError(takerAddress));
             });
@@ -316,8 +341,11 @@ blockchainTests.resets('Coordinator tests', env => {
                     transaction.signature,
                     [approvalExpirationTimeSeconds],
                     [approval.signature],
-                    { from: takerAddress, gas: constants.MAX_EXECUTE_TRANSACTION_GAS },
+                    {
-                    constants.AWAIT_TRANSACTION_MINED_MS,
+                        from: takerAddress,
+                        gas: constants.MAX_EXECUTE_TRANSACTION_GAS,
+                        value: PROTOCOL_FEE.times(orders.length),
+                    },
                 );
 
                 const expectedEvents = orders.map(order => expectedFillEvent(order));
@@ -333,8 +361,11 @@ blockchainTests.resets('Coordinator tests', env => {
                     transaction.signature,
                     [],
                     [],
-                    { from: feeRecipientAddress, gas: constants.MAX_EXECUTE_TRANSACTION_GAS },
+                    {
-                    constants.AWAIT_TRANSACTION_MINED_MS,
+                        from: feeRecipientAddress,
+                        gas: constants.MAX_EXECUTE_TRANSACTION_GAS,
+                        value: PROTOCOL_FEE.times(orders.length),
+                    },
                 );
 
                 const expectedEvents = orders.map(order => expectedFillEvent(order));
@@ -358,7 +389,7 @@ blockchainTests.resets('Coordinator tests', env => {
                     transaction.signature,
                     [approvalExpirationTimeSeconds],
                     [signature],
-                    { from: takerAddress },
+                    { from: takerAddress, value: PROTOCOL_FEE.times(orders.length) },
                 );
 
                 const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
@@ -383,7 +414,7 @@ blockchainTests.resets('Coordinator tests', env => {
                     transaction.signature,
                     [approvalExpirationTimeSeconds],
                     [approval.signature],
-                    { from: takerAddress },
+                    { from: takerAddress, value: PROTOCOL_FEE.times(orders.length) },
                 );
 
                 const transactionHash = transactionHashUtils.getTransactionHashHex(transaction);
@@ -409,7 +440,7 @@ blockchainTests.resets('Coordinator tests', env => {
                     transaction.signature,
                     [approvalExpirationTimeSeconds],
                     [approval.signature],
-                    { from: owner },
+                    { from: owner, value: PROTOCOL_FEE.times(orders.length) },
                 );
                 expect(tx).to.revertWith(new CoordinatorRevertErrors.InvalidOriginError(takerAddress));
             });

contracts/exchange-forwarder/package.json

@@ -35,7 +35,7 @@
         "compile:truffle": "truffle compile"
     },
     "config": {
-        "abis": "./generated-artifacts/@(Forwarder|IAssets|IForwarder|IForwarderCore|LibConstants|LibForwarderRichErrors|MixinAssets|MixinExchangeWrapper|MixinForwarderCore|MixinWeth|TestProtocolFeeCollector).json",
+        "abis": "./generated-artifacts/@(Forwarder|IAssets|IForwarder|IForwarderCore|LibConstants|LibForwarderRichErrors|MixinAssets|MixinExchangeWrapper|MixinForwarderCore|MixinWeth).json",
         "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually."
     },
     "repository": {

contracts/exchange-forwarder/src/artifacts.ts

@@ -15,7 +15,6 @@ import * as MixinAssets from '../generated-artifacts/MixinAssets.json';
 import * as MixinExchangeWrapper from '../generated-artifacts/MixinExchangeWrapper.json';
 import * as MixinForwarderCore from '../generated-artifacts/MixinForwarderCore.json';
 import * as MixinWeth from '../generated-artifacts/MixinWeth.json';
-import * as TestProtocolFeeCollector from '../generated-artifacts/TestProtocolFeeCollector.json';
 export const artifacts = {
     Forwarder: Forwarder as ContractArtifact,
     MixinAssets: MixinAssets as ContractArtifact,
@@ -27,5 +26,4 @@ export const artifacts = {
     IForwarderCore: IForwarderCore as ContractArtifact,
     LibConstants: LibConstants as ContractArtifact,
     LibForwarderRichErrors: LibForwarderRichErrors as ContractArtifact,
-    TestProtocolFeeCollector: TestProtocolFeeCollector as ContractArtifact,
 };

contracts/exchange-forwarder/src/wrappers.ts

@@ -13,4 +13,3 @@ export * from '../generated-wrappers/mixin_assets';
 export * from '../generated-wrappers/mixin_exchange_wrapper';
 export * from '../generated-wrappers/mixin_forwarder_core';
 export * from '../generated-wrappers/mixin_weth';
-export * from '../generated-wrappers/test_protocol_fee_collector';

contracts/exchange-forwarder/test/forwarder.ts

@@ -1,7 +1,12 @@
 import { ERC20Wrapper, ERC721Wrapper } from '@0x/contracts-asset-proxy';
 import { artifacts as erc20Artifacts, DummyERC20TokenContract, WETH9Contract } from '@0x/contracts-erc20';
 import { DummyERC721TokenContract } from '@0x/contracts-erc721';
-import { artifacts as exchangeArtifacts, ExchangeContract, ExchangeWrapper } from '@0x/contracts-exchange';
+import {
+    artifacts as exchangeArtifacts,
+    ExchangeContract,
+    ExchangeWrapper,
+    TestProtocolFeeCollectorContract,
+} from '@0x/contracts-exchange';
 import {
     blockchainTests,
     constants,
@@ -15,13 +20,7 @@ import { assetDataUtils, ForwarderRevertErrors } from '@0x/order-utils';
 import { BigNumber } from '@0x/utils';
 import { Web3Wrapper } from '@0x/web3-wrapper';
 
-import {
+import { artifacts, ForwarderContract, ForwarderTestFactory, ForwarderWrapper } from '../src';
-    artifacts,
-    ForwarderContract,
-    ForwarderTestFactory,
-    ForwarderWrapper,
-    TestProtocolFeeCollectorContract,
-} from '../src';
 
 const DECIMALS_DEFAULT = 18;
 
@@ -114,7 +113,7 @@ blockchainTests(ContractName.Forwarder, env => {
 
         // Set up Protocol Fee Collector
         protocolFeeCollector = await TestProtocolFeeCollectorContract.deployFrom0xArtifactAsync(
-            artifacts.TestProtocolFeeCollector,
+            exchangeArtifacts.TestProtocolFeeCollector,
             env.provider,
             env.txDefaults,
             {},

contracts/exchange-forwarder/tsconfig.json

@@ -12,8 +12,7 @@
         "generated-artifacts/MixinAssets.json",
         "generated-artifacts/MixinExchangeWrapper.json",
         "generated-artifacts/MixinForwarderCore.json",
-        "generated-artifacts/MixinWeth.json",
+        "generated-artifacts/MixinWeth.json"
-        "generated-artifacts/TestProtocolFeeCollector.json"
     ],
     "exclude": ["./deploy/solc/solc_bin"]
 }

contracts/exchange/contracts/test/TestProtocolFeeCollector.sol

@@ -22,7 +22,7 @@ pragma experimental ABIEncoderV2;
 import "@0x/contracts-erc20/contracts/src/interfaces/IEtherToken.sol";
 
 
- // solhint-disable no-unused-vars
+ // solhint-disable no-unused-vars, no-empty-blocks
 contract TestProtocolFeeCollector {
 
     address private _wethAddress;
@@ -35,6 +35,11 @@ contract TestProtocolFeeCollector {
         _wethAddress = wethAddress;
     }
 
+    function ()
+        external
+        payable
+    {}
+
     /// @dev Pays a protocol fee in WETH (Forwarder orders will always pay protocol fees in WETH).
     /// @param makerAddress The address of the order's maker.
     /// @param payerAddress The address of the protocol fee payer.
@@ -47,13 +52,15 @@ contract TestProtocolFeeCollector {
         external
         payable
     {
-        assert(msg.value == 0);
+        if (msg.value != protocolFeePaid) {
+            assert(msg.value == 0);
 
-        // Transfer the protocol fee to this address in WETH.
+            // Transfer the protocol fee to this address in WETH.
-        IEtherToken(_wethAddress).transferFrom(
+            IEtherToken(_wethAddress).transferFrom(
-            payerAddress,
+                payerAddress,
-            address(this),
+                address(this),
-            protocolFeePaid
+                protocolFeePaid
-        );
+            );
+        }
     }
 }

contracts/exchange/package.json

@@ -35,7 +35,7 @@
         "compile:truffle": "truffle compile"
     },
     "config": {
-        "abis": "./generated-artifacts/@(Exchange|IAssetProxy|IAssetProxyDispatcher|IEIP1271Data|IEIP1271Wallet|IExchange|IExchangeCore|IMatchOrders|IProtocolFees|ISignatureValidator|ITransactions|ITransferSimulator|IWallet|IWrapperFunctions|IsolatedExchange|LibExchangeRichErrorDecoder|MixinAssetProxyDispatcher|MixinExchangeCore|MixinMatchOrders|MixinProtocolFees|MixinSignatureValidator|MixinTransactions|MixinTransferSimulator|MixinWrapperFunctions|ReentrancyTester|TestAssetProxyDispatcher|TestExchangeInternals|TestLibExchangeRichErrorDecoder|TestProtocolFees|TestProtocolFeesReceiver|TestSignatureValidator|TestTransactions|TestValidatorWallet|TestWrapperFunctions).json",
+        "abis": "./generated-artifacts/@(Exchange|IAssetProxy|IAssetProxyDispatcher|IEIP1271Data|IEIP1271Wallet|IExchange|IExchangeCore|IMatchOrders|IProtocolFees|ISignatureValidator|ITransactions|ITransferSimulator|IWallet|IWrapperFunctions|IsolatedExchange|LibExchangeRichErrorDecoder|MixinAssetProxyDispatcher|MixinExchangeCore|MixinMatchOrders|MixinProtocolFees|MixinSignatureValidator|MixinTransactions|MixinTransferSimulator|MixinWrapperFunctions|ReentrancyTester|TestAssetProxyDispatcher|TestExchangeInternals|TestLibExchangeRichErrorDecoder|TestProtocolFeeCollector|TestProtocolFees|TestProtocolFeesReceiver|TestSignatureValidator|TestTransactions|TestValidatorWallet|TestWrapperFunctions).json",
         "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually."
     },
     "repository": {

contracts/exchange/src/artifacts.ts

@@ -33,6 +33,7 @@ import * as ReentrancyTester from '../generated-artifacts/ReentrancyTester.json'
 import * as TestAssetProxyDispatcher from '../generated-artifacts/TestAssetProxyDispatcher.json';
 import * as TestExchangeInternals from '../generated-artifacts/TestExchangeInternals.json';
 import * as TestLibExchangeRichErrorDecoder from '../generated-artifacts/TestLibExchangeRichErrorDecoder.json';
+import * as TestProtocolFeeCollector from '../generated-artifacts/TestProtocolFeeCollector.json';
 import * as TestProtocolFees from '../generated-artifacts/TestProtocolFees.json';
 import * as TestProtocolFeesReceiver from '../generated-artifacts/TestProtocolFeesReceiver.json';
 import * as TestSignatureValidator from '../generated-artifacts/TestSignatureValidator.json';
@@ -68,6 +69,7 @@ export const artifacts = {
     TestAssetProxyDispatcher: TestAssetProxyDispatcher as ContractArtifact,
     TestExchangeInternals: TestExchangeInternals as ContractArtifact,
     TestLibExchangeRichErrorDecoder: TestLibExchangeRichErrorDecoder as ContractArtifact,
+    TestProtocolFeeCollector: TestProtocolFeeCollector as ContractArtifact,
     TestProtocolFees: TestProtocolFees as ContractArtifact,
     TestProtocolFeesReceiver: TestProtocolFeesReceiver as ContractArtifact,
     TestSignatureValidator: TestSignatureValidator as ContractArtifact,

contracts/exchange/src/wrappers.ts

@@ -31,6 +31,7 @@ export * from '../generated-wrappers/reentrancy_tester';
 export * from '../generated-wrappers/test_asset_proxy_dispatcher';
 export * from '../generated-wrappers/test_exchange_internals';
 export * from '../generated-wrappers/test_lib_exchange_rich_error_decoder';
+export * from '../generated-wrappers/test_protocol_fee_collector';
 export * from '../generated-wrappers/test_protocol_fees';
 export * from '../generated-wrappers/test_protocol_fees_receiver';
 export * from '../generated-wrappers/test_signature_validator';

contracts/exchange/tsconfig.json

@@ -31,6 +31,7 @@
         "generated-artifacts/TestAssetProxyDispatcher.json",
         "generated-artifacts/TestExchangeInternals.json",
         "generated-artifacts/TestLibExchangeRichErrorDecoder.json",
+        "generated-artifacts/TestProtocolFeeCollector.json",
         "generated-artifacts/TestProtocolFees.json",
         "generated-artifacts/TestProtocolFeesReceiver.json",
         "generated-artifacts/TestSignatureValidator.json",