From 7dddf2010e16167260f7ffbb7c7d2fd83609d8a0 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 29 May 2017 22:14:18 +0200 Subject: Add getExchangeInstanceOrThrowAsync && getSenderAddressOrThrowAsync --- src/contract_wrappers/exchange_wrapper.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index f0f153c2b..e18b6d35e 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -16,11 +16,8 @@ export class ExchangeWrapper extends ContractWrapper { assert.doesConformToSchema('ecSignature', ecSignature, ecSignatureSchema); assert.isETHAddressHex('signerAddressHex', signerAddressHex); - const senderAddressIfExists = await this.web3Wrapper.getSenderAddressIfExistsAsync(); - assert.assert(!_.isUndefined(senderAddressIfExists), ZeroExError.USER_HAS_NO_ASSOCIATED_ADDRESSES); - - const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); - const exchangeInstance = contractInstance as ExchangeContract; + const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); + const exchangeInstance = await this.getExchangeInstanceOrThrowAsync(); const isValidSignature = await exchangeInstance.isValidSignature.call( signerAddressHex, @@ -29,9 +26,13 @@ export class ExchangeWrapper extends ContractWrapper { ecSignature.r, ecSignature.s, { - from: senderAddressIfExists, + from: senderAddress, }, ); return isValidSignature; } + private async getExchangeInstanceOrThrowAsync(): Promise { + const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); + return contractInstance as ExchangeContract; + } } -- cgit v1.2.3 From 96092de5cb97430af35a1c03f22fdc34d7100107 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 29 May 2017 22:30:18 +0200 Subject: Add initial implementation of fillOrderAsync --- src/contract_wrappers/exchange_wrapper.ts | 64 ++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index e18b6d35e..dfa639954 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -1,10 +1,18 @@ import * as _ from 'lodash'; import {Web3Wrapper} from '../web3_wrapper'; -import {ECSignature, ZeroExError, ExchangeContract} from '../types'; +import { + ECSignature, + ExchangeContract, + ExchangeContractErrs, + OrderValues, + OrderAddresses, +} from '../types'; import {assert} from '../utils/assert'; import {ContractWrapper} from './contract_wrapper'; import * as ExchangeArtifacts from '../artifacts/Exchange.json'; import {ecSignatureSchema} from '../schemas/ec_signature_schema'; +import {ContractResponse} from '../types'; +import {constants} from '../utils/constants'; export class ExchangeWrapper extends ContractWrapper { constructor(web3Wrapper: Web3Wrapper) { @@ -31,6 +39,60 @@ export class ExchangeWrapper extends ContractWrapper { ); return isValidSignature; } + public async fillOrderAsync(maker: string, taker: string, makerTokenAddress: string, + takerTokenAddress: string, makerTokenAmount: BigNumber.BigNumber, + takerTokenAmount: BigNumber.BigNumber, makerFee: BigNumber.BigNumber, + takerFee: BigNumber.BigNumber, expirationUnixTimestampSec: BigNumber.BigNumber, + feeRecipient: string, fillAmount: BigNumber.BigNumber, + signatureData: ECSignature, salt: BigNumber.BigNumber) { + const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); + const exchangeInstance = await this.getExchangeInstanceOrThrowAsync(); + + taker = taker === '' ? constants.NULL_ADDRESS : taker; + const shouldCheckTransfer = true; + const orderAddresses: OrderAddresses = [ + maker, + taker, + makerTokenAddress, + takerTokenAddress, + feeRecipient, + ]; + const orderValues: OrderValues = [ + makerTokenAmount, + takerTokenAmount, + makerFee, + takerFee, + expirationUnixTimestampSec, + salt, + ]; + const response: ContractResponse = await exchangeInstance.fill( + orderAddresses, + orderValues, + fillAmount, + shouldCheckTransfer, + signatureData.v, + signatureData.r, + signatureData.s, + { + from: senderAddress, + }, + ); + const errEvent = _.find(response.logs, {event: 'LogError'}); + if (!_.isUndefined(errEvent)) { + const errCode = errEvent.args.errorId.toNumber(); + const humanReadableErrMessage = this.exchangeContractErrToMsg[errCode]; + throw new Error(humanReadableErrMessage); + } + return response; + } + private exchangeContractErrToMsg = { + [ExchangeContractErrs.ERROR_FILL_EXPIRED]: 'The order you attempted to fill is expired', + [ExchangeContractErrs.ERROR_CANCEL_EXPIRED]: 'The order you attempted to cancel is expired', + [ExchangeContractErrs.ERROR_FILL_NO_VALUE]: 'This order has already been filled or cancelled', + [ExchangeContractErrs.ERROR_CANCEL_NO_VALUE]: 'This order has already been filled or cancelled', + [ExchangeContractErrs.ERROR_FILL_TRUNCATION]: 'The rounding error was too large when filling this order', + [ExchangeContractErrs.ERROR_FILL_BALANCE_ALLOWANCE]: 'Maker or taker has insufficient balance or allowance', + }; private async getExchangeInstanceOrThrowAsync(): Promise { const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); return contractInstance as ExchangeContract; -- cgit v1.2.3 From b46ad44856fd3744d90562426356283c90db4fc1 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 29 May 2017 22:34:06 +0200 Subject: Move field declaration to the begining of the class --- src/contract_wrappers/exchange_wrapper.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index dfa639954..ce5a7b8f2 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -15,6 +15,14 @@ import {ContractResponse} from '../types'; import {constants} from '../utils/constants'; export class ExchangeWrapper extends ContractWrapper { + private exchangeContractErrToMsg = { + [ExchangeContractErrs.ERROR_FILL_EXPIRED]: 'The order you attempted to fill is expired', + [ExchangeContractErrs.ERROR_CANCEL_EXPIRED]: 'The order you attempted to cancel is expired', + [ExchangeContractErrs.ERROR_FILL_NO_VALUE]: 'This order has already been filled or cancelled', + [ExchangeContractErrs.ERROR_CANCEL_NO_VALUE]: 'This order has already been filled or cancelled', + [ExchangeContractErrs.ERROR_FILL_TRUNCATION]: 'The rounding error was too large when filling this order', + [ExchangeContractErrs.ERROR_FILL_BALANCE_ALLOWANCE]: 'Maker or taker has insufficient balance or allowance', + }; constructor(web3Wrapper: Web3Wrapper) { super(web3Wrapper); } @@ -85,14 +93,6 @@ export class ExchangeWrapper extends ContractWrapper { } return response; } - private exchangeContractErrToMsg = { - [ExchangeContractErrs.ERROR_FILL_EXPIRED]: 'The order you attempted to fill is expired', - [ExchangeContractErrs.ERROR_CANCEL_EXPIRED]: 'The order you attempted to cancel is expired', - [ExchangeContractErrs.ERROR_FILL_NO_VALUE]: 'This order has already been filled or cancelled', - [ExchangeContractErrs.ERROR_CANCEL_NO_VALUE]: 'This order has already been filled or cancelled', - [ExchangeContractErrs.ERROR_FILL_TRUNCATION]: 'The rounding error was too large when filling this order', - [ExchangeContractErrs.ERROR_FILL_BALANCE_ALLOWANCE]: 'Maker or taker has insufficient balance or allowance', - }; private async getExchangeInstanceOrThrowAsync(): Promise { const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); return contractInstance as ExchangeContract; -- cgit v1.2.3 From 7fea7119272a1dd7033719504433357e219847c2 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 29 May 2017 22:42:32 +0200 Subject: Add assertions for parameters --- src/contract_wrappers/exchange_wrapper.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index ce5a7b8f2..691d465cc 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -52,7 +52,21 @@ export class ExchangeWrapper extends ContractWrapper { takerTokenAmount: BigNumber.BigNumber, makerFee: BigNumber.BigNumber, takerFee: BigNumber.BigNumber, expirationUnixTimestampSec: BigNumber.BigNumber, feeRecipient: string, fillAmount: BigNumber.BigNumber, - signatureData: ECSignature, salt: BigNumber.BigNumber) { + ecSignature: ECSignature, salt: BigNumber.BigNumber) { + assert.isBigNumber('salt', salt); + assert.isBigNumber('makerFee', makerFee); + assert.isBigNumber('takerFee', takerFee); + assert.isBigNumber('fillAmount', fillAmount); + assert.isBigNumber('makerTokenAmount', makerTokenAmount); + assert.isBigNumber('takerTokenAmount', takerTokenAmount); + assert.isBigNumber('expirationUnixTimestampSec', expirationUnixTimestampSec); + assert.isETHAddressHex('maker', maker); + assert.isETHAddressHex('taker', taker); + assert.isETHAddressHex('feeRecipient', feeRecipient); + assert.isETHAddressHex('makerTokenAddress', makerTokenAddress); + assert.isETHAddressHex('takerTokenAddress', takerTokenAddress); + assert.doesConformToSchema('ecSignature', ecSignature, ecSignatureSchema); + const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeInstanceOrThrowAsync(); -- cgit v1.2.3 From 9bb738bb2f2b26b61049315ca26a96511460062e Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Mon, 29 May 2017 22:45:05 +0200 Subject: Finish ecSignature renaming --- src/contract_wrappers/exchange_wrapper.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 691d465cc..c129eb367 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -92,9 +92,9 @@ export class ExchangeWrapper extends ContractWrapper { orderValues, fillAmount, shouldCheckTransfer, - signatureData.v, - signatureData.r, - signatureData.s, + ecSignature.v, + ecSignature.r, + ecSignature.s, { from: senderAddress, }, -- cgit v1.2.3 From 380c1e1ca521ffad6f4e3832d896bd737f67eab2 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 30 May 2017 12:17:43 +0200 Subject: Make fillOrderAsync accept signedOrder instead of a shit-ton of params --- src/contract_wrappers/exchange_wrapper.ts | 56 ++++++++++++------------------- 1 file changed, 21 insertions(+), 35 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index c129eb367..a4db63b66 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -6,11 +6,13 @@ import { ExchangeContractErrs, OrderValues, OrderAddresses, + SignedOrder, } from '../types'; import {assert} from '../utils/assert'; import {ContractWrapper} from './contract_wrapper'; import * as ExchangeArtifacts from '../artifacts/Exchange.json'; import {ecSignatureSchema} from '../schemas/ec_signature_schema'; +import {signedOrderSchema} from '../schemas/signed_order_schema'; import {ContractResponse} from '../types'; import {constants} from '../utils/constants'; @@ -47,54 +49,38 @@ export class ExchangeWrapper extends ContractWrapper { ); return isValidSignature; } - public async fillOrderAsync(maker: string, taker: string, makerTokenAddress: string, - takerTokenAddress: string, makerTokenAmount: BigNumber.BigNumber, - takerTokenAmount: BigNumber.BigNumber, makerFee: BigNumber.BigNumber, - takerFee: BigNumber.BigNumber, expirationUnixTimestampSec: BigNumber.BigNumber, - feeRecipient: string, fillAmount: BigNumber.BigNumber, - ecSignature: ECSignature, salt: BigNumber.BigNumber) { - assert.isBigNumber('salt', salt); - assert.isBigNumber('makerFee', makerFee); - assert.isBigNumber('takerFee', takerFee); - assert.isBigNumber('fillAmount', fillAmount); - assert.isBigNumber('makerTokenAmount', makerTokenAmount); - assert.isBigNumber('takerTokenAmount', takerTokenAmount); - assert.isBigNumber('expirationUnixTimestampSec', expirationUnixTimestampSec); - assert.isETHAddressHex('maker', maker); - assert.isETHAddressHex('taker', taker); - assert.isETHAddressHex('feeRecipient', feeRecipient); - assert.isETHAddressHex('makerTokenAddress', makerTokenAddress); - assert.isETHAddressHex('takerTokenAddress', takerTokenAddress); - assert.doesConformToSchema('ecSignature', ecSignature, ecSignatureSchema); + public async fillOrderAsync(signedOrder: SignedOrder, shouldCheckTransfer: boolean = true) { + assert.doesConformToSchema('signedOrder', JSON.parse(JSON.stringify(signedOrder)), signedOrderSchema); + assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeInstanceOrThrowAsync(); - taker = taker === '' ? constants.NULL_ADDRESS : taker; - const shouldCheckTransfer = true; + const taker = _.isUndefined(signedOrder.taker) ? constants.NULL_ADDRESS : signedOrder.taker; + const orderAddresses: OrderAddresses = [ - maker, + signedOrder.maker, taker, - makerTokenAddress, - takerTokenAddress, - feeRecipient, + signedOrder.makerTokenAddress, + signedOrder.takerTokenAddress, + signedOrder.feeRecipient, ]; const orderValues: OrderValues = [ - makerTokenAmount, - takerTokenAmount, - makerFee, - takerFee, - expirationUnixTimestampSec, - salt, + signedOrder.makerTokenAmount, + signedOrder.takerTokenAmount, + signedOrder.makerFee, + signedOrder.takerFee, + signedOrder.expirationUnixTimestampSec, + signedOrder.salt, ]; const response: ContractResponse = await exchangeInstance.fill( orderAddresses, orderValues, - fillAmount, + signedOrder.fillAmount, shouldCheckTransfer, - ecSignature.v, - ecSignature.r, - ecSignature.s, + signedOrder.ecSignature.v, + signedOrder.ecSignature.r, + signedOrder.ecSignature.s, { from: senderAddress, }, -- cgit v1.2.3 From 766d46041ec3958ebd625eb5adbc5b46b329b6d8 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 30 May 2017 12:55:42 +0200 Subject: Add fillAmount parameter --- src/contract_wrappers/exchange_wrapper.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index a4db63b66..141e9502a 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -49,7 +49,8 @@ export class ExchangeWrapper extends ContractWrapper { ); return isValidSignature; } - public async fillOrderAsync(signedOrder: SignedOrder, shouldCheckTransfer: boolean = true) { + public async fillOrderAsync(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, + shouldCheckTransfer: boolean = true): Promise { assert.doesConformToSchema('signedOrder', JSON.parse(JSON.stringify(signedOrder)), signedOrderSchema); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); @@ -76,7 +77,7 @@ export class ExchangeWrapper extends ContractWrapper { const response: ContractResponse = await exchangeInstance.fill( orderAddresses, orderValues, - signedOrder.fillAmount, + fillAmount, shouldCheckTransfer, signedOrder.ecSignature.v, signedOrder.ecSignature.r, -- cgit v1.2.3 From 8c0d783ccc40d5430a5fdc8253968aa457f717bd Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 30 May 2017 14:48:45 +0200 Subject: Add throwErrorLogsAsErrors --- src/contract_wrappers/exchange_wrapper.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 141e9502a..7a80c5800 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -7,6 +7,7 @@ import { OrderValues, OrderAddresses, SignedOrder, + ContractEvent, } from '../types'; import {assert} from '../utils/assert'; import {ContractWrapper} from './contract_wrapper'; @@ -86,16 +87,19 @@ export class ExchangeWrapper extends ContractWrapper { from: senderAddress, }, ); - const errEvent = _.find(response.logs, {event: 'LogError'}); - if (!_.isUndefined(errEvent)) { - const errCode = errEvent.args.errorId.toNumber(); - const humanReadableErrMessage = this.exchangeContractErrToMsg[errCode]; - throw new Error(humanReadableErrMessage); - } + this.throwErrorLogsAsErrors(response.logs); return response; } private async getExchangeInstanceOrThrowAsync(): Promise { const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); return contractInstance as ExchangeContract; } + private throwErrorLogsAsErrors(logs: ContractEvent[]): void { + const errEvent = _.find(logs, {event: 'LogError'}); + if (!_.isUndefined(errEvent)) { + const errCode = errEvent.args.errorId.toNumber(); + const humanReadableErrMessage = this.exchangeContractErrToMsg[errCode]; + throw new Error(humanReadableErrMessage); + } + } } -- cgit v1.2.3 From a4e5ec6c08596d3d59f07aa237887bf27b198e67 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 30 May 2017 15:58:51 +0200 Subject: Add isBigNumber argument assertion --- src/contract_wrappers/exchange_wrapper.ts | 1 + 1 file changed, 1 insertion(+) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 66e53a7d4..0749a7e80 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -57,6 +57,7 @@ export class ExchangeWrapper extends ContractWrapper { public async fillOrderAsync(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean = true): Promise { assert.doesConformToSchema('signedOrder', JSON.parse(JSON.stringify(signedOrder)), signedOrderSchema); + assert.isBigNumber('fillAmount', fillAmount); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); -- cgit v1.2.3 From 722a6ad5dce484ef5875132fd57970390c69e85d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 31 May 2017 15:56:52 +0200 Subject: Don't return contract response --- src/contract_wrappers/exchange_wrapper.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 0749a7e80..3f6210894 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -55,7 +55,7 @@ export class ExchangeWrapper extends ContractWrapper { return isValidSignature; } public async fillOrderAsync(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, - shouldCheckTransfer: boolean = true): Promise { + shouldCheckTransfer: boolean = true): Promise { assert.doesConformToSchema('signedOrder', JSON.parse(JSON.stringify(signedOrder)), signedOrderSchema); assert.isBigNumber('fillAmount', fillAmount); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); @@ -93,7 +93,6 @@ export class ExchangeWrapper extends ContractWrapper { }, ); this.throwErrorLogsAsErrors(response.logs); - return response; } private async getExchangeInstanceOrThrowAsync(): Promise { const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); -- cgit v1.2.3 From b05e96699f781398c2aac97082c565a531b6e4eb Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 1 Jun 2017 11:08:14 +0200 Subject: Temporarily fix gas extimation --- src/contract_wrappers/exchange_wrapper.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 3f6210894..754210778 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -80,6 +80,18 @@ export class ExchangeWrapper extends ContractWrapper { signedOrder.expirationUnixTimestampSec, signedOrder.salt, ]; + const gas = await exchangeInstance.fill.estimateGas( + orderAddresses, + orderValues, + fillAmount, + shouldCheckTransfer, + signedOrder.ecSignature.v, + signedOrder.ecSignature.r, + signedOrder.ecSignature.s, + { + from: senderAddress, + }, + ); const response: ContractResponse = await exchangeInstance.fill( orderAddresses, orderValues, @@ -90,6 +102,7 @@ export class ExchangeWrapper extends ContractWrapper { signedOrder.ecSignature.s, { from: senderAddress, + gas, }, ); this.throwErrorLogsAsErrors(response.logs); -- cgit v1.2.3 From c74b16eda9465104e1969b1635ac826835d203eb Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 14:46:09 +0200 Subject: Add convertToJSONSchemaCompatibleObject method --- src/contract_wrappers/exchange_wrapper.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 754210778..59e6c755e 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -14,6 +14,7 @@ import {ContractWrapper} from './contract_wrapper'; import * as ExchangeArtifacts from '../artifacts/Exchange.json'; import {ecSignatureSchema} from '../schemas/ec_signature_schema'; import {signedOrderSchema} from '../schemas/signed_order_schema'; +import {SchemaValidator} from '../utils/schema_validator'; import {ContractResponse} from '../types'; import {constants} from '../utils/constants'; @@ -56,7 +57,9 @@ export class ExchangeWrapper extends ContractWrapper { } public async fillOrderAsync(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean = true): Promise { - assert.doesConformToSchema('signedOrder', JSON.parse(JSON.stringify(signedOrder)), signedOrderSchema); + assert.doesConformToSchema('signedOrder', + SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), + signedOrderSchema); assert.isBigNumber('fillAmount', fillAmount); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); -- cgit v1.2.3 From 1668a085049d3b065c47709ef0018a66f637b992 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 14:59:03 +0200 Subject: rename signed_order_schema to order_schemas --- src/contract_wrappers/exchange_wrapper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 59e6c755e..ad2573c6b 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -13,7 +13,7 @@ import {assert} from '../utils/assert'; import {ContractWrapper} from './contract_wrapper'; import * as ExchangeArtifacts from '../artifacts/Exchange.json'; import {ecSignatureSchema} from '../schemas/ec_signature_schema'; -import {signedOrderSchema} from '../schemas/signed_order_schema'; +import {signedOrderSchema} from '../schemas/order_schemas'; import {SchemaValidator} from '../utils/schema_validator'; import {ContractResponse} from '../types'; import {constants} from '../utils/constants'; -- cgit v1.2.3 From 88316455b74d2ecde1da55bf2bb36b6c9b46bcb1 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 15:10:22 +0200 Subject: Make taker required on Order object and expect NULL_ADDRESS from 0x.js --- src/contract_wrappers/exchange_wrapper.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index ad2573c6b..d1763e307 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -66,11 +66,9 @@ export class ExchangeWrapper extends ContractWrapper { const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeInstanceOrThrowAsync(); - const taker = _.isUndefined(signedOrder.taker) ? constants.NULL_ADDRESS : signedOrder.taker; - const orderAddresses: OrderAddresses = [ signedOrder.maker, - taker, + signedOrder.taker, signedOrder.makerTokenAddress, signedOrder.takerTokenAddress, signedOrder.feeRecipient, -- cgit v1.2.3 From 07cdfa655be81862f7ff40fe5ac6fdb38d780370 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 1 Jun 2017 15:12:27 +0200 Subject: Add FILL_AMOUNT_IS_ZERO check --- src/contract_wrappers/exchange_wrapper.ts | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index d1763e307..7a5b6ac8c 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -4,6 +4,7 @@ import { ECSignature, ExchangeContract, ExchangeContractErrs, + FillOrderValidationErrs, OrderValues, OrderAddresses, SignedOrder, @@ -66,6 +67,8 @@ export class ExchangeWrapper extends ContractWrapper { const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeInstanceOrThrowAsync(); + this.validateFillOrder(signedOrder, fillAmount, senderAddress, shouldCheckTransfer); + const orderAddresses: OrderAddresses = [ signedOrder.maker, signedOrder.taker, @@ -108,6 +111,12 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } + private validateFillOrder(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, senderAddress: string, + shouldCheckTransfer: boolean = true) { + if (fillAmount.eq(0)) { + throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); + } + } private async getExchangeInstanceOrThrowAsync(): Promise { const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); return contractInstance as ExchangeContract; -- cgit v1.2.3 From d5d20db439a4a6fe913462bd216a776cc4300450 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 1 Jun 2017 15:18:13 +0200 Subject: Add NOT_A_TAKER check --- src/contract_wrappers/exchange_wrapper.ts | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 7a5b6ac8c..e53754e07 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -116,6 +116,9 @@ export class ExchangeWrapper extends ContractWrapper { if (fillAmount.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } + if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { + throw new Error(FillOrderValidationErrs.NOT_A_TAKER); + } } private async getExchangeInstanceOrThrowAsync(): Promise { const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); -- cgit v1.2.3 From d8e35c364ea94b606810b340fb02d8706e257c3c Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 1 Jun 2017 15:46:27 +0200 Subject: Add EXPIRED test --- src/contract_wrappers/exchange_wrapper.ts | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index e53754e07..88b0fa913 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -119,6 +119,9 @@ export class ExchangeWrapper extends ContractWrapper { if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { throw new Error(FillOrderValidationErrs.NOT_A_TAKER); } + if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { + throw new Error(FillOrderValidationErrs.EXPIRED); + } } private async getExchangeInstanceOrThrowAsync(): Promise { const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); -- cgit v1.2.3 From 6a57c42e253a52a449cae8a5c6565276cb01a86c Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 15:51:45 +0200 Subject: Rename ExchangeContractErrs to ExchangeContractErrCodes --- src/contract_wrappers/exchange_wrapper.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index d1763e307..1311730a0 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -3,7 +3,7 @@ import {Web3Wrapper} from '../web3_wrapper'; import { ECSignature, ExchangeContract, - ExchangeContractErrs, + ExchangeContractErrCodes, OrderValues, OrderAddresses, SignedOrder, @@ -19,13 +19,13 @@ import {ContractResponse} from '../types'; import {constants} from '../utils/constants'; export class ExchangeWrapper extends ContractWrapper { - private exchangeContractErrToMsg = { - [ExchangeContractErrs.ERROR_FILL_EXPIRED]: 'The order you attempted to fill is expired', - [ExchangeContractErrs.ERROR_CANCEL_EXPIRED]: 'The order you attempted to cancel is expired', - [ExchangeContractErrs.ERROR_FILL_NO_VALUE]: 'This order has already been filled or cancelled', - [ExchangeContractErrs.ERROR_CANCEL_NO_VALUE]: 'This order has already been filled or cancelled', - [ExchangeContractErrs.ERROR_FILL_TRUNCATION]: 'The rounding error was too large when filling this order', - [ExchangeContractErrs.ERROR_FILL_BALANCE_ALLOWANCE]: 'Maker or taker has insufficient balance or allowance', + private exchangeContractErrCodesToMsg = { + [ExchangeContractErrCodes.ERROR_FILL_EXPIRED]: 'The order you attempted to fill is expired', + [ExchangeContractErrCodes.ERROR_CANCEL_EXPIRED]: 'The order you attempted to cancel is expired', + [ExchangeContractErrCodes.ERROR_FILL_NO_VALUE]: 'This order has already been filled or cancelled', + [ExchangeContractErrCodes.ERROR_CANCEL_NO_VALUE]: 'This order has already been filled or cancelled', + [ExchangeContractErrCodes.ERROR_FILL_TRUNCATION]: 'The rounding error was too large when filling this order', + [ExchangeContractErrCodes.ERROR_FILL_BALANCE_ALLOWANCE]: 'Maker or taker has insufficient balance or allowance', }; private exchangeContractIfExists?: ExchangeContract; constructor(web3Wrapper: Web3Wrapper) { @@ -116,7 +116,7 @@ export class ExchangeWrapper extends ContractWrapper { const errEvent = _.find(logs, {event: 'LogError'}); if (!_.isUndefined(errEvent)) { const errCode = errEvent.args.errorId.toNumber(); - const humanReadableErrMessage = this.exchangeContractErrToMsg[errCode]; + const humanReadableErrMessage = this.exchangeContractErrCodesToMsg[errCode]; throw new Error(humanReadableErrMessage); } } -- cgit v1.2.3 From 88a70afa70f84efc866ff53121d05a6d8d77bff8 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 16:03:09 +0200 Subject: Add ExchangeContractErrs string enum --- src/contract_wrappers/exchange_wrapper.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index bf3f10e28..1232b969b 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -4,6 +4,7 @@ import { ECSignature, ExchangeContract, ExchangeContractErrCodes, + ExchangeContractErrs, FillOrderValidationErrs, OrderValues, OrderAddresses, @@ -21,12 +22,12 @@ import {constants} from '../utils/constants'; export class ExchangeWrapper extends ContractWrapper { private exchangeContractErrCodesToMsg = { - [ExchangeContractErrCodes.ERROR_FILL_EXPIRED]: 'The order you attempted to fill is expired', - [ExchangeContractErrCodes.ERROR_CANCEL_EXPIRED]: 'The order you attempted to cancel is expired', - [ExchangeContractErrCodes.ERROR_FILL_NO_VALUE]: 'This order has already been filled or cancelled', - [ExchangeContractErrCodes.ERROR_CANCEL_NO_VALUE]: 'This order has already been filled or cancelled', - [ExchangeContractErrCodes.ERROR_FILL_TRUNCATION]: 'The rounding error was too large when filling this order', - [ExchangeContractErrCodes.ERROR_FILL_BALANCE_ALLOWANCE]: 'Maker or taker has insufficient balance or allowance', + [ExchangeContractErrCodes.ERROR_FILL_EXPIRED]: ExchangeContractErrs.ORDER_EXPIRED, + [ExchangeContractErrCodes.ERROR_CANCEL_EXPIRED]: ExchangeContractErrs.ORDER_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, }; private exchangeContractIfExists?: ExchangeContract; constructor(web3Wrapper: Web3Wrapper) { @@ -131,8 +132,8 @@ export class ExchangeWrapper extends ContractWrapper { const errEvent = _.find(logs, {event: 'LogError'}); if (!_.isUndefined(errEvent)) { const errCode = errEvent.args.errorId.toNumber(); - const humanReadableErrMessage = this.exchangeContractErrCodesToMsg[errCode]; - throw new Error(humanReadableErrMessage); + const errMessage = this.exchangeContractErrCodesToMsg[errCode]; + throw new Error(errMessage); } } private async getExchangeContractAsync(): Promise { -- cgit v1.2.3 From 771c88ec791cb6bedf2ddd3418e89dec1581e326 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 1 Jun 2017 16:11:25 +0200 Subject: Revert "Add EXPIRED test" This reverts commit d8e35c364ea94b606810b340fb02d8706e257c3c. --- src/contract_wrappers/exchange_wrapper.ts | 3 --- 1 file changed, 3 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 1232b969b..dbb427d2c 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -120,9 +120,6 @@ export class ExchangeWrapper extends ContractWrapper { if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { throw new Error(FillOrderValidationErrs.NOT_A_TAKER); } - if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { - throw new Error(FillOrderValidationErrs.EXPIRED); - } } private async getExchangeInstanceOrThrowAsync(): Promise { const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); -- cgit v1.2.3 From 78259dc2af07a7c56b89669cd0b6a1c7b997092b Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 1 Jun 2017 16:15:58 +0200 Subject: Add expired check --- src/contract_wrappers/exchange_wrapper.ts | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index dbb427d2c..1232b969b 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -120,6 +120,9 @@ export class ExchangeWrapper extends ContractWrapper { if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { throw new Error(FillOrderValidationErrs.NOT_A_TAKER); } + if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { + throw new Error(FillOrderValidationErrs.EXPIRED); + } } private async getExchangeInstanceOrThrowAsync(): Promise { const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); -- cgit v1.2.3 From 44f11442424c88d1130ee398d0714636c5ade045 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 16:21:01 +0200 Subject: Add comment for fillOrderAsync method, rename fillAmount to fillTakerAmountInBaseUnits and remove default value for shouldCheckTransfer --- src/contract_wrappers/exchange_wrapper.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index dbb427d2c..40f22bd28 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -57,18 +57,23 @@ export class ExchangeWrapper extends ContractWrapper { ); return isValidSignature; } - public async fillOrderAsync(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, - shouldCheckTransfer: boolean = true): Promise { + /** + * Fills a signed order with a specified amount 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 true, the call will fail without throwing, preserving gas costs. + */ + public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmountInBaseUnits: BigNumber.BigNumber, + shouldCheckTransfer: boolean): Promise { assert.doesConformToSchema('signedOrder', SchemaValidator.convertToJSONSchemaCompatibleObject(signedOrder as object), signedOrderSchema); - assert.isBigNumber('fillAmount', fillAmount); + assert.isBigNumber('fillTakerAmountInBaseUnits', fillTakerAmountInBaseUnits); assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeInstanceOrThrowAsync(); - this.validateFillOrder(signedOrder, fillAmount, senderAddress, shouldCheckTransfer); + this.validateFillOrder(signedOrder, fillTakerAmountInBaseUnits, senderAddress, shouldCheckTransfer); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -88,7 +93,7 @@ export class ExchangeWrapper extends ContractWrapper { const gas = await exchangeInstance.fill.estimateGas( orderAddresses, orderValues, - fillAmount, + fillTakerAmountInBaseUnits, shouldCheckTransfer, signedOrder.ecSignature.v, signedOrder.ecSignature.r, @@ -100,7 +105,7 @@ export class ExchangeWrapper extends ContractWrapper { const response: ContractResponse = await exchangeInstance.fill( orderAddresses, orderValues, - fillAmount, + fillTakerAmountInBaseUnits, shouldCheckTransfer, signedOrder.ecSignature.v, signedOrder.ecSignature.r, -- cgit v1.2.3 From 6c590354c7536ae3ed941fb757be9d0240739e14 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 16:25:28 +0200 Subject: improve comment --- src/contract_wrappers/exchange_wrapper.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index ded0d3519..96bb4c0a0 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -58,9 +58,9 @@ export class ExchangeWrapper extends ContractWrapper { return isValidSignature; } /** - * Fills a signed order with a specified amount in baseUnits of the taker token. The caller can + * 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 true, the call will fail without throwing, preserving gas costs. + * shouldCheckTransfer to false. If set to true, the call will fail without throwing, preserving gas costs. */ public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmountInBaseUnits: BigNumber.BigNumber, shouldCheckTransfer: boolean): Promise { -- cgit v1.2.3 From f756e1d24e4a27b4e0ce8078884408a321fe2d28 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 16:25:37 +0200 Subject: remove unneeded method --- src/contract_wrappers/exchange_wrapper.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 96bb4c0a0..39b6d27b4 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -71,7 +71,7 @@ export class ExchangeWrapper extends ContractWrapper { assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); - const exchangeInstance = await this.getExchangeInstanceOrThrowAsync(); + const exchangeInstance = await this.getExchangeContractAsync(); this.validateFillOrder(signedOrder, fillTakerAmountInBaseUnits, senderAddress, shouldCheckTransfer); @@ -129,10 +129,6 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(FillOrderValidationErrs.EXPIRED); } } - private async getExchangeInstanceOrThrowAsync(): Promise { - const contractInstance = await this.instantiateContractIfExistsAsync((ExchangeArtifacts as any)); - return contractInstance as ExchangeContract; - } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { const errEvent = _.find(logs, {event: 'LogError'}); if (!_.isUndefined(errEvent)) { -- cgit v1.2.3 From fe025506c97da7d1d32fae0973624d49470340b9 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 16:30:06 +0200 Subject: remove unused arg --- src/contract_wrappers/exchange_wrapper.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 39b6d27b4..0712dd34a 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -117,8 +117,7 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } - private validateFillOrder(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, senderAddress: string, - shouldCheckTransfer: boolean = true) { + private validateFillOrder(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, senderAddress: string) { if (fillAmount.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } -- cgit v1.2.3 From ea42715b9ade871f54d01741ce45f36bce2a6ca2 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 16:54:20 +0200 Subject: remove superfluous param --- src/contract_wrappers/exchange_wrapper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 0712dd34a..b2d201516 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -73,7 +73,7 @@ export class ExchangeWrapper extends ContractWrapper { const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeContractAsync(); - this.validateFillOrder(signedOrder, fillTakerAmountInBaseUnits, senderAddress, shouldCheckTransfer); + this.validateFillOrder(signedOrder, fillTakerAmountInBaseUnits, senderAddress); const orderAddresses: OrderAddresses = [ signedOrder.maker, -- cgit v1.2.3 From 0554f947b12f36a1cd5f7be469123e794b467f0b Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 1 Jun 2017 18:21:12 +0200 Subject: Add not enough taker balance tests --- src/contract_wrappers/exchange_wrapper.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index b2d201516..dd3c3c933 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -19,6 +19,7 @@ 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 = { @@ -30,8 +31,10 @@ export class ExchangeWrapper extends ContractWrapper { [ExchangeContractErrCodes.ERROR_FILL_BALANCE_ALLOWANCE]: ExchangeContractErrs.ORDER_BALANCE_ALLOWANCE_ERROR, }; private exchangeContractIfExists?: ExchangeContract; - constructor(web3Wrapper: Web3Wrapper) { + private tokenWrapper: TokenWrapper; + constructor(web3Wrapper: Web3Wrapper, tokenWrapper: TokenWrapper) { super(web3Wrapper); + this.tokenWrapper = tokenWrapper; } public invalidateContractInstance(): void { delete this.exchangeContractIfExists; @@ -117,7 +120,7 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } - private validateFillOrder(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, senderAddress: string) { + private async validateFillOrder(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, senderAddress: string) { if (fillAmount.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } @@ -127,6 +130,15 @@ export class ExchangeWrapper extends ContractWrapper { if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { throw new Error(FillOrderValidationErrs.EXPIRED); } + const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, 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); + if (fillAmount.greaterThan(takerBalance)) { + throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); + } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { const errEvent = _.find(logs, {event: 'LogError'}); -- cgit v1.2.3 From 520248e6787a04b2fcf45d7f185d85214a84bfd5 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Thu, 1 Jun 2017 18:31:08 +0200 Subject: Make validateFillOrder asyncronous --- src/contract_wrappers/exchange_wrapper.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index dd3c3c933..dc2c95d4e 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -76,7 +76,7 @@ export class ExchangeWrapper extends ContractWrapper { const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeContractAsync(); - this.validateFillOrder(signedOrder, fillTakerAmountInBaseUnits, senderAddress); + await this.validateFillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, senderAddress); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -120,7 +120,7 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } - private async validateFillOrder(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, senderAddress: string) { + private async validateFillOrderAsync(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, senderAddress: string) { if (fillAmount.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } -- cgit v1.2.3 From a1be87058536f5ab10c68c570c1b9994defe52b1 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 1 Jun 2017 19:47:22 +0200 Subject: Create a FillsScenario utils module and use it in the fillOrder tests --- src/contract_wrappers/exchange_wrapper.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index dc2c95d4e..eace55f4d 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -74,10 +74,10 @@ export class ExchangeWrapper extends ContractWrapper { assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); - const exchangeInstance = await this.getExchangeContractAsync(); - await this.validateFillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, senderAddress); + const exchangeInstance = await this.getExchangeContractAsync(); + const orderAddresses: OrderAddresses = [ signedOrder.maker, signedOrder.taker, @@ -120,7 +120,8 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } - private async validateFillOrderAsync(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, senderAddress: string) { + private async validateFillOrderAsync(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, + senderAddress: string) { if (fillAmount.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } @@ -130,7 +131,8 @@ export class ExchangeWrapper extends ContractWrapper { if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { throw new Error(FillOrderValidationErrs.EXPIRED); } - const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); + const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, + signedOrder.maker); const takerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); const makerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); -- cgit v1.2.3 From 5155f4980440f1c91006b124406c8b4a7a90e300 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 12:03:52 +0200 Subject: Add test for insufficient balance and make all async tests async --- src/contract_wrappers/exchange_wrapper.ts | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index eace55f4d..e2ac07b55 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -141,6 +141,9 @@ export class ExchangeWrapper extends ContractWrapper { if (fillAmount.greaterThan(takerBalance)) { throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); } + if (fillAmount.greaterThan(takerAllowance)) { + throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_ALLOWANCE); + } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { const errEvent = _.find(logs, {event: 'LogError'}); -- cgit v1.2.3 From 2a0c6abbe7f9abeacc4fea05bc468413ec1f4732 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 12:20:18 +0200 Subject: Validate maker balance and allowance & tests --- src/contract_wrappers/exchange_wrapper.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index e2ac07b55..3fb187de2 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -120,9 +120,9 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } - private async validateFillOrderAsync(signedOrder: SignedOrder, fillAmount: BigNumber.BigNumber, + private async validateFillOrderAsync(signedOrder: SignedOrder, fillTakerAmountInBaseUnits: BigNumber.BigNumber, senderAddress: string) { - if (fillAmount.eq(0)) { + if (fillTakerAmountInBaseUnits.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { @@ -138,12 +138,22 @@ export class ExchangeWrapper extends ContractWrapper { signedOrder.maker); const takerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, senderAddress); - if (fillAmount.greaterThan(takerBalance)) { + // How many taker tokens would you get for 1 maker token; + const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); + const fillMakerAmountInBaseUnits = fillTakerAmountInBaseUnits.div(exchangeRate); + + if (fillTakerAmountInBaseUnits.greaterThan(takerBalance)) { throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); } - if (fillAmount.greaterThan(takerAllowance)) { + if (fillTakerAmountInBaseUnits.greaterThan(takerAllowance)) { throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_ALLOWANCE); } + if (fillMakerAmountInBaseUnits.greaterThan(makerBalance)) { + throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_BALANCE); + } + if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { + throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE); + } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { const errEvent = _.find(logs, {event: 'LogError'}); -- cgit v1.2.3 From e1ee6b84945e729d894f6535be02f3541e43dbf0 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 12:57:21 +0200 Subject: Add check for ROUNDING_ERROR and test for it --- src/contract_wrappers/exchange_wrapper.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 3fb187de2..fe5fc3d78 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -154,6 +154,10 @@ export class ExchangeWrapper extends ContractWrapper { if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE); } + if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmountInBaseUnits, + signedOrder.makerTokenAmount)) { + throw new Error(FillOrderValidationErrs.ROUNDING_ERROR); + } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { const errEvent = _.find(logs, {event: 'LogError'}); @@ -163,6 +167,18 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(errMessage); } } + private async isRoundingErrorAsync(takerTokenAmount: BigNumber.BigNumber, + fillTakerAmountInBaseUnits: BigNumber.BigNumber, + makerTokenAmount: BigNumber.BigNumber): Promise { + const exchangeInstance = await this.getExchangeContractAsync(); + const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); + const isRoundingError = await exchangeInstance.isRoundingError.call( + takerTokenAmount, fillTakerAmountInBaseUnits, makerTokenAmount, { + from: senderAddress, + }, + ); + return isRoundingError; + } private async getExchangeContractAsync(): Promise { if (!_.isUndefined(this.exchangeContractIfExists)) { return this.exchangeContractIfExists; -- cgit v1.2.3 From c650d1ba204a862de81b225118d9bcd1a38ad25d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 16:18:54 +0200 Subject: Add tests and checks for fees balances and allowances --- src/contract_wrappers/exchange_wrapper.ts | 54 +++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 9 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index fe5fc3d78..fa5c16485 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -74,9 +74,9 @@ export class ExchangeWrapper extends ContractWrapper { assert.isBoolean('shouldCheckTransfer', shouldCheckTransfer); const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); - await this.validateFillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, senderAddress); - const exchangeInstance = await this.getExchangeContractAsync(); + const zrxTokenAddress = await exchangeInstance.ZRX.call(); + await this.validateFillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, senderAddress, zrxTokenAddress); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -121,7 +121,7 @@ export class ExchangeWrapper extends ContractWrapper { this.throwErrorLogsAsErrors(response.logs); } private async validateFillOrderAsync(signedOrder: SignedOrder, fillTakerAmountInBaseUnits: BigNumber.BigNumber, - senderAddress: string) { + senderAddress: string, zrxTokenAddress: string): Promise { if (fillTakerAmountInBaseUnits.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } @@ -131,13 +131,32 @@ export class ExchangeWrapper extends ContractWrapper { if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { throw new Error(FillOrderValidationErrs.EXPIRED); } + + await this.validateFillOrderBalancesAndAllowancesAsync(signedOrder, fillTakerAmountInBaseUnits, + senderAddress, zrxTokenAddress); + + if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmountInBaseUnits, + signedOrder.makerTokenAmount)) { + throw new Error(FillOrderValidationErrs.ROUNDING_ERROR); + } + } + private async validateFillOrderBalancesAndAllowancesAsync(signedOrder: SignedOrder, + fillTakerAmountInBaseUnits: BigNumber.BigNumber, + senderAddress: string, + zrxTokenAddress: string): Promise { + // TODO: There is a possibility that the user might have enough funds + // to fulfill the order or pay fees but not both. This will happen if + // makerToken === zrxToken || makerToken === zrxToken + // We don't check it for now. The contract checks it and throws. + 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); + signedOrder.maker); const takerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, - senderAddress); + senderAddress); + // How many taker tokens would you get for 1 maker token; const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); const fillMakerAmountInBaseUnits = fillTakerAmountInBaseUnits.div(exchangeRate); @@ -154,9 +173,26 @@ export class ExchangeWrapper extends ContractWrapper { if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE); } - if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmountInBaseUnits, - signedOrder.makerTokenAmount)) { - throw new Error(FillOrderValidationErrs.ROUNDING_ERROR); + + 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(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_BALANCE); + } + if (signedOrder.takerFee.greaterThan(takerFeeAllowance)) { + throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE); + } + if (signedOrder.makerFee.greaterThan(makerFeeBalance)) { + throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_BALANCE); + } + if (signedOrder.makerFee.greaterThan(makerFeeAllowance)) { + throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE); } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { -- cgit v1.2.3 From ade42047436f8f81bcc158fdfdc297dc6a2b1f66 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 2 Jun 2017 16:54:21 +0200 Subject: Improve error names and remove duplicate import --- src/contract_wrappers/exchange_wrapper.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index fa5c16485..93e49cf33 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -10,6 +10,7 @@ import { OrderAddresses, SignedOrder, ContractEvent, + ContractResponse, } from '../types'; import {assert} from '../utils/assert'; import {ContractWrapper} from './contract_wrapper'; @@ -17,18 +18,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 tokenWrapper: TokenWrapper; -- cgit v1.2.3 From 38c48eb2266632a10fee75ecea7bbce4c343c1cb Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 2 Jun 2017 17:02:16 +0200 Subject: Improve fillOrderAsync comment --- src/contract_wrappers/exchange_wrapper.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 93e49cf33..131211771 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -61,9 +61,12 @@ export class ExchangeWrapper extends ContractWrapper { return isValidSignature; } /** - * 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 foregoes this check and causes the smart contract to throw instead. */ public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmountInBaseUnits: BigNumber.BigNumber, shouldCheckTransfer: boolean): Promise { -- cgit v1.2.3 From 827a0d4e91169e338cbdc8042d158aaf7fdcf96c Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 2 Jun 2017 17:03:36 +0200 Subject: rename fillTakerAmountInBaseUnits to fillTakerAmount --- src/contract_wrappers/exchange_wrapper.ts | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 131211771..f0f6e79f7 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -68,18 +68,18 @@ export class ExchangeWrapper extends ContractWrapper { * executing, the parties do not have sufficient balances/allowances, preserving gas costs. Setting it to * false foregoes 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 { 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(); const exchangeInstance = await this.getExchangeContractAsync(); const zrxTokenAddress = await exchangeInstance.ZRX.call(); - await this.validateFillOrderAsync(signedOrder, fillTakerAmountInBaseUnits, senderAddress, zrxTokenAddress); + await this.validateFillOrderAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -99,7 +99,7 @@ export class ExchangeWrapper extends ContractWrapper { const gas = await exchangeInstance.fill.estimateGas( orderAddresses, orderValues, - fillTakerAmountInBaseUnits, + fillTakerAmount, shouldCheckTransfer, signedOrder.ecSignature.v, signedOrder.ecSignature.r, @@ -111,7 +111,7 @@ export class ExchangeWrapper extends ContractWrapper { const response: ContractResponse = await exchangeInstance.fill( orderAddresses, orderValues, - fillTakerAmountInBaseUnits, + fillTakerAmount, shouldCheckTransfer, signedOrder.ecSignature.v, signedOrder.ecSignature.r, @@ -123,9 +123,9 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } - private async validateFillOrderAsync(signedOrder: SignedOrder, fillTakerAmountInBaseUnits: BigNumber.BigNumber, + private async validateFillOrderAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string): Promise { - if (fillTakerAmountInBaseUnits.eq(0)) { + if (fillTakerAmount.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { @@ -135,16 +135,16 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(FillOrderValidationErrs.EXPIRED); } - await this.validateFillOrderBalancesAndAllowancesAsync(signedOrder, fillTakerAmountInBaseUnits, + await this.validateFillOrderBalancesAndAllowancesAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); - if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmountInBaseUnits, + if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmount, signedOrder.makerTokenAmount)) { throw new Error(FillOrderValidationErrs.ROUNDING_ERROR); } } private async validateFillOrderBalancesAndAllowancesAsync(signedOrder: SignedOrder, - fillTakerAmountInBaseUnits: BigNumber.BigNumber, + fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string): Promise { // TODO: There is a possibility that the user might have enough funds @@ -162,12 +162,12 @@ export class ExchangeWrapper extends ContractWrapper { // How many taker tokens would you get for 1 maker token; const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); - const fillMakerAmountInBaseUnits = fillTakerAmountInBaseUnits.div(exchangeRate); + const fillMakerAmountInBaseUnits = fillTakerAmount.div(exchangeRate); - if (fillTakerAmountInBaseUnits.greaterThan(takerBalance)) { + if (fillTakerAmount.greaterThan(takerBalance)) { throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); } - if (fillTakerAmountInBaseUnits.greaterThan(takerAllowance)) { + if (fillTakerAmount.greaterThan(takerAllowance)) { throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_ALLOWANCE); } if (fillMakerAmountInBaseUnits.greaterThan(makerBalance)) { @@ -207,12 +207,12 @@ export class ExchangeWrapper extends ContractWrapper { } } private async isRoundingErrorAsync(takerTokenAmount: BigNumber.BigNumber, - fillTakerAmountInBaseUnits: BigNumber.BigNumber, + fillTakerAmount: BigNumber.BigNumber, makerTokenAmount: BigNumber.BigNumber): Promise { const exchangeInstance = await this.getExchangeContractAsync(); const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const isRoundingError = await exchangeInstance.isRoundingError.call( - takerTokenAmount, fillTakerAmountInBaseUnits, makerTokenAmount, { + takerTokenAmount, fillTakerAmount, makerTokenAmount, { from: senderAddress, }, ); -- cgit v1.2.3 From 9756aa86b051940b88f87fbecb313bf07590eca3 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:34:01 +0200 Subject: Add getZRXTokenAddressAsync --- src/contract_wrappers/exchange_wrapper.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index f0f6e79f7..0c7f27507 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -78,7 +78,7 @@ export class ExchangeWrapper extends ContractWrapper { const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeContractAsync(); - const zrxTokenAddress = await exchangeInstance.ZRX.call(); + const zrxTokenAddress = await this.getZRXTokenAddressAsync(exchangeInstance); await this.validateFillOrderAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); const orderAddresses: OrderAddresses = [ @@ -226,4 +226,7 @@ export class ExchangeWrapper extends ContractWrapper { this.exchangeContractIfExists = contractInstance as ExchangeContract; return this.exchangeContractIfExists; } + private async getZRXTokenAddressAsync(exchangeInstance: ExchangeContract): Promise { + return exchangeInstance.ZRX.call(); + } } -- cgit v1.2.3 From ef3a27ed0545e2e30e965b928ae13c8a53e16e8d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:35:05 +0200 Subject: Rename validation functions --- src/contract_wrappers/exchange_wrapper.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 0c7f27507..965b31710 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -79,7 +79,7 @@ export class ExchangeWrapper extends ContractWrapper { const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeContractAsync(); const zrxTokenAddress = await this.getZRXTokenAddressAsync(exchangeInstance); - await this.validateFillOrderAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); + await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -123,8 +123,8 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } - private async validateFillOrderAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - senderAddress: string, zrxTokenAddress: string): Promise { + private async validateFillOrderAndThrowIfInvalidAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, + senderAddress: string, zrxTokenAddress: string): Promise { if (fillTakerAmount.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } @@ -135,7 +135,7 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(FillOrderValidationErrs.EXPIRED); } - await this.validateFillOrderBalancesAndAllowancesAsync(signedOrder, fillTakerAmount, + await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmount, @@ -143,10 +143,10 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(FillOrderValidationErrs.ROUNDING_ERROR); } } - private async validateFillOrderBalancesAndAllowancesAsync(signedOrder: SignedOrder, - fillTakerAmount: BigNumber.BigNumber, - senderAddress: string, - zrxTokenAddress: string): Promise { + private async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder: SignedOrder, + fillTakerAmount: BigNumber.BigNumber, + senderAddress: string, + zrxTokenAddress: string): Promise { // TODO: There is a possibility that the user might have enough funds // to fulfill the order or pay fees but not both. This will happen if // makerToken === zrxToken || makerToken === zrxToken -- cgit v1.2.3 From 0f7bd0597234dbeafcee31e6f715f3c2004696df Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:37:28 +0200 Subject: Don't pass zrxTokenAsign --- src/contract_wrappers/exchange_wrapper.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 965b31710..d7d56c276 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -78,8 +78,7 @@ export class ExchangeWrapper extends ContractWrapper { const senderAddress = await this.web3Wrapper.getSenderAddressOrThrowAsync(); const exchangeInstance = await this.getExchangeContractAsync(); - const zrxTokenAddress = await this.getZRXTokenAddressAsync(exchangeInstance); - await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); + await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress); const orderAddresses: OrderAddresses = [ signedOrder.maker, @@ -123,8 +122,9 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } - private async validateFillOrderAndThrowIfInvalidAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, - senderAddress: string, zrxTokenAddress: string): Promise { + private async validateFillOrderAndThrowIfInvalidAsync(signedOrder: SignedOrder, + fillTakerAmount: BigNumber.BigNumber, + senderAddress: string): Promise { if (fillTakerAmount.eq(0)) { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } @@ -134,7 +134,7 @@ export class ExchangeWrapper extends ContractWrapper { if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { throw new Error(FillOrderValidationErrs.EXPIRED); } - + const zrxTokenAddress = await this.getZRXTokenAddressAsync(); await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); @@ -226,7 +226,8 @@ export class ExchangeWrapper extends ContractWrapper { this.exchangeContractIfExists = contractInstance as ExchangeContract; return this.exchangeContractIfExists; } - private async getZRXTokenAddressAsync(exchangeInstance: ExchangeContract): Promise { + private async getZRXTokenAddressAsync(): Promise { + const exchangeInstance = await this.getExchangeContractAsync(); return exchangeInstance.ZRX.call(); } } -- cgit v1.2.3 From e5cc0e0562eccf67eff9cf0521c4884ffc3b0f3b Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:38:47 +0200 Subject: Rename NOT_A_TAKER to TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER --- src/contract_wrappers/exchange_wrapper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index d7d56c276..ac15faff4 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -129,7 +129,7 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); } if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { - throw new Error(FillOrderValidationErrs.NOT_A_TAKER); + throw new Error(FillOrderValidationErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); } if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { throw new Error(FillOrderValidationErrs.EXPIRED); -- cgit v1.2.3 From f5158eebf335c9fbcfb7cfb6f32a0ecd1161391d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:39:21 +0200 Subject: Assign timestamp to a variable --- src/contract_wrappers/exchange_wrapper.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index ac15faff4..4c46404bb 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -131,7 +131,8 @@ export class ExchangeWrapper extends ContractWrapper { if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { throw new Error(FillOrderValidationErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); } - if (signedOrder.expirationUnixTimestampSec.lessThan(Date.now() / 1000)) { + const currentUnixTimestampSec = Date.now() / 1000; + if (signedOrder.expirationUnixTimestampSec.lessThan(currentUnixTimestampSec)) { throw new Error(FillOrderValidationErrs.EXPIRED); } const zrxTokenAddress = await this.getZRXTokenAddressAsync(); -- cgit v1.2.3 From 7fd84e29ab8c27cb872cf8cd0f631d4b78abba0e Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:40:06 +0200 Subject: Rename EXPIRED to FILL_ORDER_EXPIRED --- src/contract_wrappers/exchange_wrapper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 4c46404bb..e957530ae 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -133,7 +133,7 @@ export class ExchangeWrapper extends ContractWrapper { } const currentUnixTimestampSec = Date.now() / 1000; if (signedOrder.expirationUnixTimestampSec.lessThan(currentUnixTimestampSec)) { - throw new Error(FillOrderValidationErrs.EXPIRED); + throw new Error(FillOrderValidationErrs.FILL_ORDER_EXPIRED); } const zrxTokenAddress = await this.getZRXTokenAddressAsync(); await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, -- cgit v1.2.3 From 54e8bf773091eb2c8587c72e50892afec42abb9d Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 17:41:23 +0200 Subject: Assign wouldRoundingErrorOccur to a variable --- src/contract_wrappers/exchange_wrapper.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index e957530ae..828994829 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -139,8 +139,10 @@ export class ExchangeWrapper extends ContractWrapper { await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress); - if (await this.isRoundingErrorAsync(signedOrder.takerTokenAmount, fillTakerAmount, - signedOrder.makerTokenAmount)) { + const wouldRoundingErrorOccur = await this.isRoundingErrorAsync( + signedOrder.takerTokenAmount, fillTakerAmount, signedOrder.makerTokenAmount, + ); + if (wouldRoundingErrorOccur) { throw new Error(FillOrderValidationErrs.ROUNDING_ERROR); } } -- cgit v1.2.3 From abf2cf4c5e2ea6d09d862871adfa16ca108907e2 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:00:37 +0200 Subject: Move FillOrderValidatinErrs to ExchangeContractErrs --- src/contract_wrappers/exchange_wrapper.ts | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 828994829..716f9c77a 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -5,7 +5,6 @@ import { ExchangeContract, ExchangeContractErrCodes, ExchangeContractErrs, - FillOrderValidationErrs, OrderValues, OrderAddresses, SignedOrder, @@ -126,14 +125,14 @@ export class ExchangeWrapper extends ContractWrapper { fillTakerAmount: BigNumber.BigNumber, senderAddress: string): Promise { if (fillTakerAmount.eq(0)) { - throw new Error(FillOrderValidationErrs.FILL_AMOUNT_IS_ZERO); + throw new Error(ExchangeContractErrs.ORDER_REMAINING_FILL_AMOUNT_ZERO); } if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { - throw new Error(FillOrderValidationErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_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(FillOrderValidationErrs.FILL_ORDER_EXPIRED); + throw new Error(ExchangeContractErrs.ORDER_FILL_EXPIRED); } const zrxTokenAddress = await this.getZRXTokenAddressAsync(); await this.validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, @@ -143,7 +142,7 @@ export class ExchangeWrapper extends ContractWrapper { signedOrder.takerTokenAmount, fillTakerAmount, signedOrder.makerTokenAmount, ); if (wouldRoundingErrorOccur) { - throw new Error(FillOrderValidationErrs.ROUNDING_ERROR); + throw new Error(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); } } private async validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder: SignedOrder, @@ -168,16 +167,16 @@ export class ExchangeWrapper extends ContractWrapper { const fillMakerAmountInBaseUnits = fillTakerAmount.div(exchangeRate); if (fillTakerAmount.greaterThan(takerBalance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_BALANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_BALANCE); } if (fillTakerAmount.greaterThan(takerAllowance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_ALLOWANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_ALLOWANCE); } if (fillMakerAmountInBaseUnits.greaterThan(makerBalance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_BALANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_BALANCE); } if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_ALLOWANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_ALLOWANCE); } const makerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, @@ -189,16 +188,16 @@ export class ExchangeWrapper extends ContractWrapper { senderAddress); if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_BALANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_BALANCE); } if (signedOrder.takerFee.greaterThan(takerFeeAllowance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE); } if (signedOrder.makerFee.greaterThan(makerFeeBalance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_BALANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_BALANCE); } if (signedOrder.makerFee.greaterThan(makerFeeAllowance)) { - throw new Error(FillOrderValidationErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE); + throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE); } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { -- cgit v1.2.3 From 89d93494788f06d227628dfcfbd712e26ef02b77 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:05:27 +0200 Subject: Rewrite comment --- src/contract_wrappers/exchange_wrapper.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 716f9c77a..ba3b05758 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -65,7 +65,7 @@ export class ExchangeWrapper extends ContractWrapper { * 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 foregoes this check and causes the smart contract to throw instead. + * false forgoes this check and causes the smart contract to throw instead. */ public async fillOrderAsync(signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, shouldCheckTransfer: boolean): Promise { @@ -145,14 +145,20 @@ export class ExchangeWrapper extends ContractWrapper { 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 { - // TODO: There is a possibility that the user might have enough funds - // to fulfill the order or pay fees but not both. This will happen if - // makerToken === zrxToken || makerToken === zrxToken - // We don't check it for now. The contract checks it and throws. const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); -- cgit v1.2.3 From 45fadeb690e27acccc776515ada8d39e3633194a Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:06:06 +0200 Subject: Allign arguments --- src/contract_wrappers/exchange_wrapper.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index ba3b05758..55ddcc46a 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -161,12 +161,12 @@ export class ExchangeWrapper extends ContractWrapper { zrxTokenAddress: string): Promise { 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); + signedOrder.maker); const takerAllowance = await this.tokenWrapper.getProxyAllowanceAsync(signedOrder.takerTokenAddress, - senderAddress); + senderAddress); // How many taker tokens would you get for 1 maker token; const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); -- cgit v1.2.3 From 76280dd5dbc5a5a2fb5f7230fa73d594f822c7e6 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 2 Jun 2017 18:10:06 +0200 Subject: Address feedback --- src/contract_wrappers/exchange_wrapper.ts | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 55ddcc46a..4aa532bdd 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -168,42 +168,42 @@ export class ExchangeWrapper extends ContractWrapper { 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 = fillTakerAmount.div(exchangeRate); if (fillTakerAmount.greaterThan(takerBalance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_BALANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_BALANCE); } if (fillTakerAmount.greaterThan(takerAllowance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_ALLOWANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_ALLOWANCE); } if (fillMakerAmountInBaseUnits.greaterThan(makerBalance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_BALANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_BALANCE); } if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_ALLOWANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_ALLOWANCE); } const makerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, - signedOrder.maker); + signedOrder.maker); const takerFeeBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); const makerFeeAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - signedOrder.maker); + signedOrder.maker); const takerFeeAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - senderAddress); + senderAddress); if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_BALANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_BALANCE); } if (signedOrder.takerFee.greaterThan(takerFeeAllowance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_TAKER_FEE_ALLOWANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_TAKER_FEE_ALLOWANCE); } if (signedOrder.makerFee.greaterThan(makerFeeBalance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_BALANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_BALANCE); } if (signedOrder.makerFee.greaterThan(makerFeeAllowance)) { - throw new Error(ExchangeContractErrs.NOT_ENOUGH_MAKER_FEE_ALLOWANCE); + throw new Error(ExchangeContractErrs.INSUFFICIENT_MAKER_FEE_ALLOWANCE); } } private throwErrorLogsAsErrors(logs: ContractEvent[]): void { -- cgit v1.2.3