diff options
-rw-r--r-- | src/0x.js.ts | 2 | ||||
-rw-r--r-- | src/contract_wrappers/exchange_wrapper.ts | 129 | ||||
-rw-r--r-- | src/types.ts | 45 | ||||
-rw-r--r-- | src/utils/schema_validator.ts | 2 | ||||
-rw-r--r-- | test/exchange_wrapper_test.ts | 216 | ||||
-rw-r--r-- | test/utils/fill_scenarios.ts | 93 | ||||
-rw-r--r-- | test/utils/order_factory.ts | 17 | ||||
-rw-r--r-- | test/utils/token_utils.ts | 24 |
8 files changed, 378 insertions, 150 deletions
diff --git a/src/0x.js.ts b/src/0x.js.ts index 0cda7732d..850827fee 100644 --- a/src/0x.js.ts +++ b/src/0x.js.ts @@ -135,7 +135,7 @@ export class ZeroEx { return senderAccountIfExists; } /** - * Computes the orderHash given the order parameters and returns it as a hex encoded string. + * Computes the orderHash for a given order and returns it as a hex encoded string. */ public async getOrderHashHexAsync(order: Order|SignedOrder): Promise<string> { const exchangeContractAddr = await this.getExchangeAddressAsync(); diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index de614f471..b67fd33ac 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -7,7 +7,6 @@ import { ExchangeContract, ExchangeContractErrCodes, ExchangeContractErrs, - FillOrderValidationErrs, OrderValues, OrderAddresses, SignedOrder, @@ -19,6 +18,7 @@ import { CreateContractEvent, ContractEventObj, EventCallback, + ContractResponse, } from '../types'; import {assert} from '../utils/assert'; import {utils} from '../utils/utils'; @@ -27,18 +27,17 @@ import * as ExchangeArtifacts from '../artifacts/Exchange.json'; import {ecSignatureSchema} from '../schemas/ec_signature_schema'; import {signedOrderSchema} from '../schemas/order_schemas'; import {SchemaValidator} from '../utils/schema_validator'; -import {ContractResponse} from '../types'; import {constants} from '../utils/constants'; import {TokenWrapper} from './token_wrapper'; export class ExchangeWrapper extends ContractWrapper { private exchangeContractErrCodesToMsg = { - [ExchangeContractErrCodes.ERROR_FILL_EXPIRED]: ExchangeContractErrs.ORDER_EXPIRED, - [ExchangeContractErrCodes.ERROR_CANCEL_EXPIRED]: ExchangeContractErrs.ORDER_EXPIRED, + [ExchangeContractErrCodes.ERROR_FILL_EXPIRED]: ExchangeContractErrs.ORDER_FILL_EXPIRED, + [ExchangeContractErrCodes.ERROR_CANCEL_EXPIRED]: ExchangeContractErrs.ORDER_FILL_EXPIRED, [ExchangeContractErrCodes.ERROR_FILL_NO_VALUE]: ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO, [ExchangeContractErrCodes.ERROR_CANCEL_NO_VALUE]: ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO, - [ExchangeContractErrCodes.ERROR_FILL_TRUNCATION]: ExchangeContractErrs.ORDER_ROUNDING_ERROR, - [ExchangeContractErrCodes.ERROR_FILL_BALANCE_ALLOWANCE]: ExchangeContractErrs.ORDER_BALANCE_ALLOWANCE_ERROR, + [ExchangeContractErrCodes.ERROR_FILL_TRUNCATION]: ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR, + [ExchangeContractErrCodes.ERROR_FILL_BALANCE_ALLOWANCE]: ExchangeContractErrs.FILL_BALANCE_ALLOWANCE_ERROR, }; private exchangeContractIfExists?: ExchangeContract; private exchangeLogEventObjs: ContractEventObj[]; @@ -112,22 +111,24 @@ export class ExchangeWrapper extends ContractWrapper { return cancelledAmountInBaseUnits; } /** - * Fills a signed order with a fillAmount denominated in baseUnits of the taker token. The caller can - * decide whether they want the call to throw if the balance/allowance checks fail by setting - * shouldCheckTransfer to false. If set to true, the call will fail without throwing, preserving gas costs. + * Fills a signed order with a fillAmount denominated in baseUnits of the taker token. + * Since the order in which transactions are included in the next block is indeterminate, race-conditions + * could arise where a users balance or allowance changes before the fillOrder executes. Because of this, + * we allow you to specify `shouldCheckTransfer`. If true, the smart contract will not throw if while + * executing, the parties do not have sufficient balances/allowances, preserving gas costs. Setting it to + * false forgoes this check and causes the smart contract to throw instead. */ - public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmountInBaseUnits: BigNumber.BigNumber, + public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean): Promise<void> { assert.doesConformToSchema('signedOrder', SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), signedOrderSchema); - assert.isBigNumber('fillTakerAmountInBaseUnits', fillTakerAmountInBaseUnits); + assert.isBigNumber('fillTakerAmount', fillTakerAmount); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); - await this.validateFillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, senderAddress); - const exchangeInstance = await this.getExchangeContractAsync(); + await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -147,7 +148,7 @@ export class ExchangeWrapper extends ContractWrapper { const gas = await exchangeInstance.fill.estimateGas( orderAddresses, orderValues, - fillTakerAmountInBaseUnits, + fillTakerAmount, shouldCheckTransfer, signedOrder.ecSignature.v, signedOrder.ecSignature.r, @@ -159,7 +160,7 @@ export class ExchangeWrapper extends ContractWrapper { const response: ContractResponse = await exchangeInstance.fill( orderAddresses, orderValues, - fillTakerAmountInBaseUnits, + fillTakerAmount, shouldCheckTransfer, signedOrder.ecSignature.v, signedOrder.ecSignature.r, @@ -203,39 +204,89 @@ export class ExchangeWrapper extends ContractWrapper { } this.exchangeLogEventObjs = []; } - private async validateFillOrderAsync(signedOrder: SignedOrder, fillTakerAmountInBaseUnits: BigNumber.BigNumber, - senderAddress: string) { - if (fillTakerAmountInBaseUnits.eq(0)) { - throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); + private async validateFillOrderAndThrowIfInvalidAsync(signedOrder: SignedOrder, + fillTakerAmount: BigNumber.BigNumber, + senderAddress: string): Promise<void> { + if (fillTakerAmount.eq(0)) { + throw new Error(ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO); } if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { - throw new Error(FillOrderValidationErrs.NOT_A_TAKER); + throw new Error(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); + } + const currentUnixTimestampSec = Date.now() / 1000; + if (signedOrder.expirationUnixTimestampSec.lessThan(currentUnixTimestampSec)) { + throw new Error(ExchangeContractErrs.ORDER_FILL_EXPIRED); } - if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { - throw new Error(FillOrderValidationErrs.EXPIRED); + const zrxTokenAddress = await this.getZRXTokenAddressAsync(); + await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, + senderAddress, zrxTokenAddress); + + const wouldRoundingErrorOccur = await this.isRoundingErrorAsync( + signedOrder.takerTokenAmount, fillTakerAmount, signedOrder.makerTokenAmount, + ); + if (wouldRoundingErrorOccur) { + throw new Error(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); } + } + + /** + * This method does not currently validate the edge-case where the makerToken or takerToken is also the token used + * to pay fees (ZRX). It is possible for them to have enough for fees and the transfer but not both. + * Handling the edge-cases that arise when this happens would require making sure that the user has sufficient + * funds to pay both the fees and the transfer amount. We decided to punt on this for now as the contracts + * will throw for these edge-cases. + * TODO: Throw errors before calling the smart contract for these edge-cases + * TODO: in order to minimize the callers gas costs. + */ + private async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder: SignedOrder, + fillTakerAmount: BigNumber.BigNumber, + senderAddress: string, + zrxTokenAddress: string): Promise<void> { + const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, - signedOrder.maker); + signedOrder.maker); const takerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); const makerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); const takerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, senderAddress); - // How many taker tokens would you get for 1 maker token; + + // exchangeRate is the price of one maker token denominated in taker tokens const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); - const fillMakerAmountInBaseUnits = fillTakerAmountInBaseUnits.div(exchangeRate); + const fillMakerAmountInBaseUnits = fillTakerAmount.div(exchangeRate); - if (fillTakerAmountInBaseUnits.greaterThan(takerBalance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); + if (fillTakerAmount.greaterThan(takerBalance)) { + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_BALANCE); } - if (fillTakerAmountInBaseUnits.greaterThan(takerAllowance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_ALLOWANCE); + if (fillTakerAmount.greaterThan(takerAllowance)) { + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_ALLOWANCE); } if (fillMakerAmountInBaseUnits.greaterThan(makerBalance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_BALANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_BALANCE); } if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_ALLOWANCE); + } + + const makerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, + signedOrder.maker); + const takerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); + const makerFeeAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, + signedOrder.maker); + const takerFeeAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, + senderAddress); + + if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_BALANCE); + } + if (signedOrder.takerFee.greaterThan(takerFeeAllowance)) { + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_ALLOWANCE); + } + if (signedOrder.makerFee.greaterThan(makerFeeBalance)) { + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_BALANCE); + } + if (signedOrder.makerFee.greaterThan(makerFeeAllowance)) { + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_ALLOWANCE); } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { @@ -246,6 +297,18 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(errMessage); } } + private async isRoundingErrorAsync(takerTokenAmount: BigNumber.BigNumber, + fillTakerAmount: BigNumber.BigNumber, + makerTokenAmount: BigNumber.BigNumber): Promise<boolean> { + const exchangeInstance = await this.getExchangeContractAsync(); + const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); + const isRoundingError = await exchangeInstance.isRoundingError.call( + takerTokenAmount, fillTakerAmount, makerTokenAmount, { + from: senderAddress, + }, + ); + return isRoundingError; + } private async getExchangeContractAsync(): Promise<ExchangeContract> { if (!_.isUndefined(this.exchangeContractIfExists)) { return this.exchangeContractIfExists; @@ -254,4 +317,8 @@ export class ExchangeWrapper extends ContractWrapper { this.exchangeContractIfExists = contractInstance as ExchangeContract; return this.exchangeContractIfExists; } + private async getZRXTokenAddressAsync(): Promise<string> { + const exchangeInstance = await this.getExchangeContractAsync(); + return exchangeInstance.ZRX.call(); + } } diff --git a/src/types.ts b/src/types.ts index c6ad68d3f..b5430a783 100644 --- a/src/types.ts +++ b/src/types.ts @@ -10,11 +10,12 @@ function strEnum(values: string[]): {[key: string]: string} { } export const ZeroExError = strEnum([ - 'CONTRACT_DOES_NOT_EXIST', - 'UNHANDLED_ERROR', - 'USER_HAS_NO_ASSOCIATED_ADDRESSES', - 'INVALID_SIGNATURE', - 'CONTRACT_NOT_DEPLOYED_ON_NETWORK', + 'CONTRACT_DOES_NOT_EXIST', + 'UNHANDLED_ERROR', + 'USER_HAS_NO_ASSOCIATED_ADDRESSES', + 'INVALID_SIGNATURE', + 'CONTRACT_NOT_DEPLOYED_ON_NETWORK', + 'ZRX_NOT_IN_TOKEN_REGISTRY', ]); export type ZeroExError = keyof typeof ZeroExError; @@ -37,6 +38,10 @@ export interface ExchangeContract { getUnavailableValueT: { call: (orderHash: string) => BigNumber.BigNumber; }; + isRoundingError: { + call: (takerTokenAmount: BigNumber.BigNumber, fillTakerAmount: BigNumber.BigNumber, + makerTokenAmount: BigNumber.BigNumber, txOpts: TxOpts) => Promise<boolean>; + }; fill: { (orderAddresses: OrderAddresses, orderValues: OrderValues, fillAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean, v: number, r: string, s: string, txOpts: TxOpts): ContractResponse; @@ -65,6 +70,9 @@ export interface ExchangeContract { LogFill: CreateContractEvent; LogCancel: CreateContractEvent; LogError: CreateContractEvent; + ZRX: { + call: () => Promise<string>; + }; } export interface TokenContract { @@ -103,23 +111,22 @@ export enum ExchangeContractErrCodes { } export const ExchangeContractErrs = strEnum([ - 'ORDER_EXPIRED', + 'ORDER_FILL_EXPIRED', 'ORDER_REMAINING_FILL_AMOUNT_ZERO', - 'ORDER_ROUNDING_ERROR', - 'ORDER_BALANCE_ALLOWANCE_ERROR', -]); -export type ExchangeContractErrs = keyof typeof ExchangeContractErrs; + 'ORDER_FILL_ROUNDING_ERROR', + 'FILL_BALANCE_ALLOWANCE_ERROR', + 'INSUFFICIENT_TAKER_BALANCE', + 'INSUFFICIENT_TAKER_ALLOWANCE', + 'INSUFFICIENT_MAKER_BALANCE', + 'INSUFFICIENT_MAKER_ALLOWANCE', + 'INSUFFICIENT_TAKER_FEE_BALANCE', + 'INSUFFICIENT_TAKER_FEE_ALLOWANCE', + 'INSUFFICIENT_MAKER_FEE_BALANCE', + 'INSUFFICIENT_MAKER_FEE_ALLOWANCE', + 'TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER', -export const FillOrderValidationErrs = strEnum([ - 'FILL_AMOUNT_IS_ZERO', - 'NOT_A_TAKER', - 'EXPIRED', - 'NOT_ENOUGH_TAKER_BALANCE', - 'NOT_ENOUGH_TAKER_ALLOWANCE', - 'NOT_ENOUGH_MAKER_BALANCE', - 'NOT_ENOUGH_MAKER_ALLOWANCE', ]); -export type FillOrderValidationErrs = keyof typeof FillOrderValidationErrs; +export type ExchangeContractErrs = keyof typeof ExchangeContractErrs; export interface ContractResponse { logs: ContractEvent[]; diff --git a/src/utils/schema_validator.ts b/src/utils/schema_validator.ts index db8a960ba..932ddf62a 100644 --- a/src/utils/schema_validator.ts +++ b/src/utils/schema_validator.ts @@ -6,7 +6,7 @@ import {tokenSchema} from '../schemas/token_schema'; export class SchemaValidator { private validator: Validator; // In order to validate a complex JS object using jsonschema, we must replace any complex - // sub-types (e.g BigNumber) with a simpler string represenation. Since BigNumber and other + // sub-types (e.g BigNumber) with a simpler string representation. Since BigNumber and other // complex types implement the `toString` method, we can stringify the object and // then parse it. The resultant object can then be checked using jsonschema. public static convertToJSONSchemaCompatibleObject(obj: object): object { diff --git a/test/exchange_wrapper_test.ts b/test/exchange_wrapper_test.ts index cfb75789c..32b47f4f9 100644 --- a/test/exchange_wrapper_test.ts +++ b/test/exchange_wrapper_test.ts @@ -9,17 +9,17 @@ import promisify = require('es6-promisify'); import {web3Factory} from './utils/web3_factory'; import {ZeroEx} from '../src/0x.js'; import {BlockchainLifecycle} from './utils/blockchain_lifecycle'; -import {orderFactory} from './utils/order_factory'; import { - FillOrderValidationErrs, Token, SignedOrder, SubscriptionOpts, ExchangeEvents, ContractEvent, DoneCallback, + ExchangeContractErrs, } from '../src/types'; import {FillScenarios} from './utils/fill_scenarios'; +import {TokenUtils} from './utils/token_utils'; chai.use(dirtyChai); chai.use(ChaiBigNumber()); @@ -29,17 +29,21 @@ const blockchainLifecycle = new BlockchainLifecycle(); const NON_EXISTENT_ORDER_HASH = '0x79370342234e7acd6bbeac335bd3bb1d368383294b64b8160a00f4060e4d3777'; describe('ExchangeWrapper', () => { + let web3: Web3; let zeroEx: ZeroEx; let userAddresses: string[]; - let web3: Web3; + let tokenUtils: TokenUtils; let tokens: Token[]; let fillScenarios: FillScenarios; + let zrxTokenAddress: string; before(async () => { web3 = web3Factory.create(); zeroEx = new ZeroEx(web3); userAddresses = await promisify(web3.eth.getAccounts)(); tokens = await zeroEx.tokenRegistry.getTokensAsync(); - fillScenarios = new FillScenarios(zeroEx, userAddresses, tokens); + tokenUtils = new TokenUtils(tokens); + zrxTokenAddress = tokenUtils.getProtocolTokenOrThrow().address; + fillScenarios = new FillScenarios(zeroEx, userAddresses, tokens, zrxTokenAddress); }); beforeEach(async () => { await blockchainLifecycle.startAsync(); @@ -120,14 +124,16 @@ describe('ExchangeWrapper', () => { describe('#fillOrderAsync', () => { let makerTokenAddress: string; let takerTokenAddress: string; - let coinBase: string; + let coinbase: string; let makerAddress: string; let takerAddress: string; - const fillTakerAmountInBaseUnits = new BigNumber(5); + let feeRecipient: string; + const fillTakerAmount = new BigNumber(5); const shouldCheckTransfer = false; - before('fetch tokens', async () => { - [coinBase, makerAddress, takerAddress] = userAddresses; - const [makerToken, takerToken] = tokens; + before(async () => { + [coinbase, makerAddress, takerAddress, feeRecipient] = userAddresses; + tokens = await zeroEx.tokenRegistry.getTokensAsync(); + const [makerToken, takerToken] = tokenUtils.getNonProtocolTokens(); makerTokenAddress = makerToken.address; takerTokenAddress = takerToken.address; }); @@ -137,92 +143,149 @@ describe('ExchangeWrapper', () => { describe('failed fills', () => { it('should throw when the fill amount is zero', async () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); const zeroFillAmount = new BigNumber(0); zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( signedOrder, zeroFillAmount, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); + )).to.be.rejectedWith(ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO); }); it('should throw when sender is not a taker', async () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_A_TAKER); + signedOrder, fillTakerAmount, shouldCheckTransfer, + )).to.be.rejectedWith(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); }); it('should throw when order is expired', async () => { const expirationInPast = new BigNumber(42); const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, expirationInPast, ); zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.EXPIRED); + signedOrder, fillTakerAmount, shouldCheckTransfer, + )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_EXPIRED); }); - it('should throw when taker balance is less than fill amount', async () => { + describe('should throw when not enough balance or allowance to fulfill the order', () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, - ); - zeroEx.setTransactionSenderAccount(takerAddress); - const moreThanTheBalance = new BigNumber(6); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, moreThanTheBalance, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); + const balanceToSubtractFromMaker = new BigNumber(3); + const lackingAllowance = new BigNumber(3); + let signedOrder: SignedOrder; + beforeEach('create fillable signed order', async () => { + signedOrder = await fillScenarios.createFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + ); + }); + it('should throw when taker balance is less than fill amount', async () => { + await zeroEx.token.transferAsync( + takerTokenAddress, takerAddress, coinbase, balanceToSubtractFromMaker, + ); + zeroEx.setTransactionSenderAccount(takerAddress); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_BALANCE); + }); + it('should throw when taker allowance is less than fill amount', async () => { + const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); + await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, + newAllowanceWhichIsLessThanFillAmount); + zeroEx.setTransactionSenderAccount(takerAddress); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_ALLOWANCE); + }); + it('should throw when maker balance is less than maker fill amount', async () => { + await zeroEx.token.transferAsync( + makerTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, + ); + zeroEx.setTransactionSenderAccount(takerAddress); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_BALANCE); + }); + it('should throw when maker allowance is less than maker fill amount', async () => { + const newAllowanceWhichIsLessThanFillAmount = fillTakerAmount.minus(lackingAllowance); + await zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, + newAllowanceWhichIsLessThanFillAmount); + zeroEx.setTransactionSenderAccount(takerAddress); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_ALLOWANCE); + }); }); - it('should throw when taker allowance is less than fill amount', async () => { - const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, + it('should throw when there a rounding error would have occurred', async () => { + const makerAmount = new BigNumber(3); + const takerAmount = new BigNumber(5); + const signedOrder = await fillScenarios.createAsymmetricFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, + makerAmount, takerAmount, ); - const newAllowanceWhichIsLessThanFillAmount = fillTakerAmountInBaseUnits.minus(1); - await zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, - newAllowanceWhichIsLessThanFillAmount); + const fillTakerAmountThatCausesRoundingError = new BigNumber(3); zeroEx.setTransactionSenderAccount(takerAddress); return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_TAKER_ALLOWANCE); + signedOrder, fillTakerAmountThatCausesRoundingError, shouldCheckTransfer, + )).to.be.rejectedWith(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); }); - it('should throw when maker balance is less than maker fill amount', async () => { + describe('should throw when not enough balance or allowance to pay fees', () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, - ); - const lackingMakerBalance = new BigNumber(3); - await zeroEx.token.transferAsync(makerTokenAddress, makerAddress, coinBase, lackingMakerBalance); - zeroEx.setTransactionSenderAccount(takerAddress); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_MAKER_BALANCE); - }); - it('should throw when maker allowance is less than maker fill amount', async () => { - const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( - makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, - ); - const newAllowanceWhichIsLessThanFillAmount = fillTakerAmountInBaseUnits.minus(1); - await zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, - newAllowanceWhichIsLessThanFillAmount); - zeroEx.setTransactionSenderAccount(takerAddress); - return expect(zeroEx.exchange.fillOrderAsync( - signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer, - )).to.be.rejectedWith(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE); + const makerFee = new BigNumber(2); + const takerFee = new BigNumber(2); + let signedOrder: SignedOrder; + beforeEach('setup', async () => { + signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerTokenAddress, takerTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + zeroEx.setTransactionSenderAccount(takerAddress); + }); + it('should throw when maker doesn\'t have enough balance to pay fees', async () => { + const balanceToSubtractFromMaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, makerAddress, coinbase, balanceToSubtractFromMaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_BALANCE); + }); + it('should throw when maker doesn\'t have enough allowance to pay fees', async () => { + const newAllowanceWhichIsLessThanFees = makerFee.minus(1); + await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, makerAddress, + newAllowanceWhichIsLessThanFees); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_ALLOWANCE); + }); + it('should throw when taker doesn\'t have enough balance to pay fees', async () => { + const balanceToSubtractFromTaker = new BigNumber(1); + await zeroEx.token.transferAsync( + zrxTokenAddress, takerAddress, coinbase, balanceToSubtractFromTaker, + ); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_BALANCE); + }); + it('should throw when taker doesn\'t have enough allowance to pay fees', async () => { + const newAllowanceWhichIsLessThanFees = makerFee.minus(1); + await zeroEx.token.setProxyAllowanceAsync(zrxTokenAddress, takerAddress, + newAllowanceWhichIsLessThanFees); + return expect(zeroEx.exchange.fillOrderAsync( + signedOrder, fillTakerAmount, shouldCheckTransfer, + )).to.be.rejectedWith(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_ALLOWANCE); + }); }); }); describe('successful fills', () => { - it('should fill the valid order', async () => { + it('should fill a valid order', async () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); - expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress)) .to.be.bignumber.equal(fillableAmount); expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress)) @@ -232,19 +295,19 @@ describe('ExchangeWrapper', () => { expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) .to.be.bignumber.equal(fillableAmount); zeroEx.setTransactionSenderAccount(takerAddress); - await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, shouldCheckTransfer); + await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmount, shouldCheckTransfer); expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress)) - .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmountInBaseUnits)); + .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmount)); expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress)) - .to.be.bignumber.equal(fillTakerAmountInBaseUnits); + .to.be.bignumber.equal(fillTakerAmount); expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, takerAddress)) - .to.be.bignumber.equal(fillTakerAmountInBaseUnits); + .to.be.bignumber.equal(fillTakerAmount); expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) - .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmountInBaseUnits)); + .to.be.bignumber.equal(fillableAmount.minus(fillTakerAmount)); }); it('should partially fill the valid order', async () => { const fillableAmount = new BigNumber(5); - const signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + const signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); const partialFillAmount = new BigNumber(3); @@ -259,6 +322,19 @@ describe('ExchangeWrapper', () => { expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, takerAddress)) .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount)); }); + it('should fill the valid orders with fees', async () => { + const fillableAmount = new BigNumber(5); + const makerFee = new BigNumber(1); + const takerFee = new BigNumber(2); + const signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerTokenAddress, takerTokenAddress, makerFee, takerFee, + makerAddress, takerAddress, fillableAmount, feeRecipient, + ); + zeroEx.setTransactionSenderAccount(takerAddress); + await zeroEx.exchange.fillOrderAsync(signedOrder, fillTakerAmount, shouldCheckTransfer); + expect(await zeroEx.token.getBalanceAsync(zrxTokenAddress, feeRecipient)) + .to.be.bignumber.equal(makerFee.plus(takerFee)); + }); }); }); describe('tests that require partially filled order', () => { @@ -332,20 +408,20 @@ describe('ExchangeWrapper', () => { const shouldCheckTransfer = false; let makerTokenAddress: string; let takerTokenAddress: string; - let coinBase: string; + let coinbase: string; let takerAddress: string; let makerAddress: string; let fillableAmount: BigNumber.BigNumber; let signedOrder: SignedOrder; before(() => { - [coinBase, makerAddress, takerAddress] = userAddresses; + [coinbase, makerAddress, takerAddress] = userAddresses; const [makerToken, takerToken] = tokens; makerTokenAddress = makerToken.address; takerTokenAddress = takerToken.address; }); beforeEach(async () => { fillableAmount = new BigNumber(5); - signedOrder = await fillScenarios.createAFillableSignedOrderAsync( + signedOrder = await fillScenarios.createFillableSignedOrderAsync( makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, fillableAmount, ); }); diff --git a/test/utils/fill_scenarios.ts b/test/utils/fill_scenarios.ts index 17c2dbeba..ea5eb77eb 100644 --- a/test/utils/fill_scenarios.ts +++ b/test/utils/fill_scenarios.ts @@ -2,35 +2,54 @@ import * as BigNumber from 'bignumber.js'; import {ZeroEx} from '../../src/0x.js'; import {Token, SignedOrder} from '../../src/types'; import {orderFactory} from '../utils/order_factory'; +import {constants} from './constants'; export class FillScenarios { private zeroEx: ZeroEx; private userAddresses: string[]; private tokens: Token[]; - private coinBase: string; - constructor(zeroEx: ZeroEx, userAddresses: string[], tokens: Token[]) { + private coinbase: string; + private zrxTokenAddress: string; + constructor(zeroEx: ZeroEx, userAddresses: string[], tokens: Token[], zrxTokenAddress: string) { this.zeroEx = zeroEx; this.userAddresses = userAddresses; this.tokens = tokens; - this.coinBase = userAddresses[0]; + this.coinbase = userAddresses[0]; + this.zrxTokenAddress = zrxTokenAddress; } - public async createAFillableSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string, - makerAddress: string, takerAddress: string, - fillableAmount: BigNumber.BigNumber, - expirationUnixTimestampSec?: BigNumber.BigNumber): + public async createFillableSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string, + makerAddress: string, takerAddress: string, + fillableAmount: BigNumber.BigNumber, + expirationUnixTimestampSec?: BigNumber.BigNumber): Promise<SignedOrder> { - await this.zeroEx.token.transferAsync(makerTokenAddress, this.coinBase, makerAddress, fillableAmount); - await this.zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, fillableAmount); - await this.zeroEx.token.transferAsync(takerTokenAddress, this.coinBase, takerAddress, fillableAmount); - await this.zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, fillableAmount); - - const transactionSenderAccount = await this.zeroEx.getTransactionSenderAccountIfExistsAsync(); - this.zeroEx.setTransactionSenderAccount(makerAddress); - const signedOrder = await orderFactory.createSignedOrderAsync(this.zeroEx, makerAddress, - takerAddress, fillableAmount, makerTokenAddress, fillableAmount, takerTokenAddress, - expirationUnixTimestampSec); - this.zeroEx.setTransactionSenderAccount(transactionSenderAccount as string); - return signedOrder; + return this.createAsymmetricFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, + fillableAmount, fillableAmount, expirationUnixTimestampSec, + ); + } + public async createFillableSignedOrderWithFeesAsync( + makerTokenAddress: string, takerTokenAddress: string, + makerFee: BigNumber.BigNumber, takerFee: BigNumber.BigNumber, + makerAddress: string, takerAddress: string, + fillableAmount: BigNumber.BigNumber, + feeRecepient: string, expirationUnixTimestampSec?: BigNumber.BigNumber, + ): Promise<SignedOrder> { + return this.createAsymmetricFillableSignedOrderWithFeesAsync( + makerTokenAddress, takerTokenAddress, makerFee, takerFee, makerAddress, takerAddress, + fillableAmount, fillableAmount, feeRecepient, expirationUnixTimestampSec, + ); + } + public async createAsymmetricFillableSignedOrderAsync( + makerTokenAddress: string, takerTokenAddress: string, makerAddress: string, takerAddress: string, + makerFillableAmount: BigNumber.BigNumber, takerFillableAmount: BigNumber.BigNumber, + expirationUnixTimestampSec?: BigNumber.BigNumber): Promise<SignedOrder> { + const makerFee = new BigNumber(0); + const takerFee = new BigNumber(0); + const feeRecepient = constants.NULL_ADDRESS; + return this.createAsymmetricFillableSignedOrderWithFeesAsync( + makerTokenAddress, takerTokenAddress, makerFee, takerFee, makerAddress, takerAddress, + makerFillableAmount, takerFillableAmount, feeRecepient, expirationUnixTimestampSec, + ); } public async createPartiallyFilledSignedOrderAsync(makerTokenAddress: string, takerTokenAddress: string, takerAddress: string, fillableAmount: BigNumber.BigNumber, @@ -41,8 +60,10 @@ export class FillScenarios { await this.zeroEx.token.transferAsync(takerTokenAddress, makerAddress, takerAddress, fillableAmount); await this.zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, fillableAmount); - const signedOrder = await orderFactory.createSignedOrderAsync(this.zeroEx, makerAddress, - takerAddress, fillableAmount, makerTokenAddress, fillableAmount, takerTokenAddress); + const signedOrder = await this.createAsymmetricFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, + fillableAmount, fillableAmount, + ); this.zeroEx.setTransactionSenderAccount(takerAddress); const shouldCheckTransfer = false; @@ -52,4 +73,34 @@ export class FillScenarios { this.zeroEx.setTransactionSenderAccount(prevSenderAccount as string); return signedOrder; } + private async createAsymmetricFillableSignedOrderWithFeesAsync( + makerTokenAddress: string, takerTokenAddress: string, + makerFee: BigNumber.BigNumber, takerFee: BigNumber.BigNumber, + makerAddress: string, takerAddress: string, + makerFillableAmount: BigNumber.BigNumber, takerFillableAmount: BigNumber.BigNumber, + feeRecepient: string, expirationUnixTimestampSec?: BigNumber.BigNumber): Promise<SignedOrder> { + await this.zeroEx.token.transferAsync(makerTokenAddress, this.coinbase, makerAddress, makerFillableAmount); + await this.zeroEx.token.setProxyAllowanceAsync(makerTokenAddress, makerAddress, makerFillableAmount); + await this.zeroEx.token.transferAsync(takerTokenAddress, this.coinbase, takerAddress, takerFillableAmount); + await this.zeroEx.token.setProxyAllowanceAsync(takerTokenAddress, takerAddress, takerFillableAmount); + + if (!makerFee.isZero()) { + await this.zeroEx.token.transferAsync(this.zrxTokenAddress, this.coinbase, makerAddress, makerFee); + await this.zeroEx.token.setProxyAllowanceAsync(this.zrxTokenAddress, makerAddress, makerFee); + } + if (!takerFee.isZero()) { + await this.zeroEx.token.transferAsync(this.zrxTokenAddress, this.coinbase, takerAddress, takerFee); + await this.zeroEx.token.setProxyAllowanceAsync(this.zrxTokenAddress, takerAddress, takerFee); + } + + const prevTransactionSenderAccount = await this.zeroEx.getTransactionSenderAccountIfExistsAsync(); + this.zeroEx.setTransactionSenderAccount(makerAddress); + const signedOrder = await orderFactory.createSignedOrderAsync(this.zeroEx, + makerAddress, takerAddress, makerFee, takerFee, + makerFillableAmount, makerTokenAddress, takerFillableAmount, takerTokenAddress, + feeRecepient, expirationUnixTimestampSec); + // We re-set the transactionSender to avoid introducing side-effects + this.zeroEx.setTransactionSenderAccount(prevTransactionSenderAccount as string); + return signedOrder; + } } diff --git a/test/utils/order_factory.ts b/test/utils/order_factory.ts index 0f370ed34..373dbddc6 100644 --- a/test/utils/order_factory.ts +++ b/test/utils/order_factory.ts @@ -10,10 +10,13 @@ export const orderFactory = { zeroEx: ZeroEx, maker: string, taker: string, - makerTokenAmount: BigNumber.BigNumber|number, + makerFee: BigNumber.BigNumber, + takerFee: BigNumber.BigNumber, + makerTokenAmount: BigNumber.BigNumber, makerTokenAddress: string, - takerTokenAmount: BigNumber.BigNumber|number, + takerTokenAmount: BigNumber.BigNumber, takerTokenAddress: string, + feeRecipient: string, expirationUnixTimestampSec?: BigNumber.BigNumber): Promise<SignedOrder> { const defaultExpirationUnixTimestampSec = new BigNumber(2524604400); // Close to infinite expirationUnixTimestampSec = _.isUndefined(expirationUnixTimestampSec) ? @@ -22,14 +25,14 @@ export const orderFactory = { const order = { maker, taker, - makerFee: new BigNumber(0), - takerFee: new BigNumber(0), - makerTokenAmount: _.isNumber(makerTokenAmount) ? new BigNumber(makerTokenAmount) : makerTokenAmount, - takerTokenAmount: _.isNumber(takerTokenAmount) ? new BigNumber(takerTokenAmount) : takerTokenAmount, + makerFee, + takerFee, + makerTokenAmount, + takerTokenAmount, makerTokenAddress, takerTokenAddress, salt: ZeroEx.generatePseudoRandomSalt(), - feeRecipient: constants.NULL_ADDRESS, + feeRecipient, expirationUnixTimestampSec, }; const orderHash = await zeroEx.getOrderHashHexAsync(order); diff --git a/test/utils/token_utils.ts b/test/utils/token_utils.ts new file mode 100644 index 000000000..14788b299 --- /dev/null +++ b/test/utils/token_utils.ts @@ -0,0 +1,24 @@ +import * as _ from 'lodash'; +import {Token, ZeroExError} from '../../src/types'; + +const PROTOCOL_TOKEN_SYMBOL = 'ZRX'; + +export class TokenUtils { + private tokens: Token[]; + constructor(tokens: Token[]) { + this.tokens = tokens; + } + public getProtocolTokenOrThrow(): Token { + const zrxToken = _.find(this.tokens, {symbol: PROTOCOL_TOKEN_SYMBOL}); + if (_.isUndefined(zrxToken)) { + throw new Error(ZeroExError.ZRX_NOT_IN_TOKEN_REGISTRY); + } + return zrxToken; + } + public getNonProtocolTokens(): Token[] { + const nonProtocolTokens = _.filter(this.tokens, token => { + return token.symbol !== PROTOCOL_TOKEN_SYMBOL; + }); + return nonProtocolTokens; + } +} |