From 8aa673aabe292e7d4684f7cc98ec6ef2ea55ca09 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 15:36:07 +0200 Subject: Add initial implementation with success test --- src/contract_wrappers/exchange_wrapper.ts | 53 ++++++++++++++++++++++++++++++- 1 file changed, 52 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 d3a53a9f7..b8e34b9e8 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -9,9 +9,9 @@ import { ExchangeContractErrs, OrderValues, OrderAddresses, + Order, SignedOrder, ContractEvent, - ZeroExError, ExchangeEvents, SubscriptionOpts, IndexFilterValues, @@ -168,6 +168,53 @@ export class ExchangeWrapper extends ContractWrapper { ); this.throwErrorLogsAsErrors(response.logs); } + /** + * Cancels the order. + */ + public async cancelOrderAsync(order: Order, cancelAmount: BigNumber.BigNumber): Promise { + assert.doesConformToSchema('order', + SchemaValidator.convertToJSONSchemaCompatibleObject(order as object), + signedOrderSchema); + assert.isBigNumber('cancelAmount', cancelAmount); + await assert.isSenderAddressAvailableAsync(this.web3Wrapper, order.maker); + + const exchangeInstance = await this.getExchangeContractAsync(); + await this.validateCancelOrderAndThrowIfInvalidAsync(order, cancelAmount); + + const orderAddresses: OrderAddresses = [ + order.maker, + order.taker, + order.makerTokenAddress, + order.takerTokenAddress, + order.feeRecipient, + ]; + const orderValues: OrderValues = [ + order.makerTokenAmount, + order.takerTokenAmount, + order.makerFee, + order.takerFee, + order.expirationUnixTimestampSec, + order.salt, + ]; + const gas = await exchangeInstance.cancel.estimateGas( + orderAddresses, + orderValues, + cancelAmount, + { + from: order.maker, + }, + ); + const response: ContractResponse = await exchangeInstance.cancel( + orderAddresses, + orderValues, + cancelAmount, + { + from: order.maker, + gas, + }, + ); + this.throwErrorLogsAsErrors(response.logs); + } /** * Subscribe to an event type emitted by the Exchange smart contract */ @@ -225,6 +272,10 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); } } + private async validateCancelOrderAndThrowIfInvalidAsync(order: Order, + cancelAmount: BigNumber.BigNumber): Promise { + // TODO + } /** * This method does not currently validate the edge-case where the makerToken or takerToken is also the token used -- cgit From 08b7c24bbf23685814c5bba5148577022e40e8b0 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 15:48:47 +0200 Subject: Fix tests --- 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 b8e34b9e8..982a8c0c1 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -176,7 +176,7 @@ export class ExchangeWrapper extends ContractWrapper { SchemaValidator.convertToJSONSchemaCompatibleObject(order as object), signedOrderSchema); assert.isBigNumber('cancelAmount', cancelAmount); - await assert.isSenderAddressAvailableAsync(this.web3Wrapper, order.maker); + await assert.isSenderAddressAvailableAsync(this.web3Wrapper, 'order.maker', order.maker); const exchangeInstance = await this.getExchangeContractAsync(); await this.validateCancelOrderAndThrowIfInvalidAsync(order, cancelAmount); -- cgit From fa910bca0ebcc71e5fd7aa656972d77bec08c9e5 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 16:57:13 +0200 Subject: Add checks and tests for expired order and zero fill amount --- src/contract_wrappers/exchange_wrapper.ts | 8 +++++++- 1 file changed, 7 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 982a8c0c1..b5d5152dd 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -274,7 +274,13 @@ export class ExchangeWrapper extends ContractWrapper { } private async validateCancelOrderAndThrowIfInvalidAsync(order: Order, cancelAmount: BigNumber.BigNumber): Promise { - // TODO + if (cancelAmount.eq(0)) { + throw new Error(ExchangeContractErrs.ORDER_CANCEL_AMOUNT_ZERO); + } + const currentUnixTimestampSec = Date.now() / 1000; + if (order.expirationUnixTimestampSec.lessThan(currentUnixTimestampSec)) { + throw new Error(ExchangeContractErrs.ORDER_CANCEL_EXPIRED); + } } /** -- cgit From 42b4952693b66d722fa541af4a2ae9034c1cd3e7 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 6 Jun 2017 17:26:16 +0200 Subject: Add test when the order was already cancelled or filled --- src/contract_wrappers/exchange_wrapper.ts | 66 ++++++++++++++++--------------- 1 file changed, 35 insertions(+), 31 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index b5d5152dd..4b7bd172f 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -42,6 +42,24 @@ export class ExchangeWrapper extends ContractWrapper { private exchangeContractIfExists?: ExchangeContract; private exchangeLogEventObjs: ContractEventObj[]; private tokenWrapper: TokenWrapper; + private static getOrderAddressesAndValues(order: Order): [OrderAddresses, OrderValues] { + const orderAddresses: OrderAddresses = [ + order.maker, + order.taker, + order.makerTokenAddress, + order.takerTokenAddress, + order.feeRecipient, + ]; + const orderValues: OrderValues = [ + order.makerTokenAmount, + order.takerTokenAmount, + order.makerFee, + order.takerFee, + order.expirationUnixTimestampSec, + order.salt, + ]; + return [orderAddresses, orderValues]; + } constructor(web3Wrapper: Web3Wrapper, tokenWrapper: TokenWrapper) { super(web3Wrapper); this.tokenWrapper = tokenWrapper; @@ -126,21 +144,7 @@ export class ExchangeWrapper extends ContractWrapper { const exchangeInstance = await this.getExchangeContractAsync(); await this.validateFillOrderAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, takerAddress); - const orderAddresses: OrderAddresses = [ - signedOrder.maker, - signedOrder.taker, - signedOrder.makerTokenAddress, - signedOrder.takerTokenAddress, - signedOrder.feeRecipient, - ]; - const orderValues: OrderValues = [ - signedOrder.makerTokenAmount, - signedOrder.takerTokenAmount, - signedOrder.makerFee, - signedOrder.takerFee, - signedOrder.expirationUnixTimestampSec, - signedOrder.salt, - ]; + const [orderAddresses, orderValues] = ExchangeWrapper.getOrderAddressesAndValues(signedOrder); const gas = await exchangeInstance.fill.estimateGas( orderAddresses, orderValues, @@ -181,21 +185,7 @@ export class ExchangeWrapper extends ContractWrapper { const exchangeInstance = await this.getExchangeContractAsync(); await this.validateCancelOrderAndThrowIfInvalidAsync(order, cancelAmount); - const orderAddresses: OrderAddresses = [ - order.maker, - order.taker, - order.makerTokenAddress, - order.takerTokenAddress, - order.feeRecipient, - ]; - const orderValues: OrderValues = [ - order.makerTokenAmount, - order.takerTokenAmount, - order.makerFee, - order.takerFee, - order.expirationUnixTimestampSec, - order.salt, - ]; + const [orderAddresses, orderValues] = ExchangeWrapper.getOrderAddressesAndValues(order); const gas = await exchangeInstance.cancel.estimateGas( orderAddresses, orderValues, @@ -241,6 +231,16 @@ export class ExchangeWrapper extends ContractWrapper { logEventObj.watch(callback); this.exchangeLogEventObjs.push(logEventObj); } + + /** + * Get order hash + */ + public async getOrderHashAsync(order: Order): Promise { + const [orderAddresses, orderValues] = ExchangeWrapper.getOrderAddressesAndValues(order); + const exchangeInstance = await this.getExchangeContractAsync(); + const orderHash = await exchangeInstance.getOrderHash.call(orderAddresses, orderValues); + return orderHash; + } private async stopWatchingExchangeLogEventsAsync() { const stopWatchingPromises = _.map(this.exchangeLogEventObjs, logEventObj => { return promisify(logEventObj.stopWatching, logEventObj)(); @@ -277,12 +277,16 @@ export class ExchangeWrapper extends ContractWrapper { if (cancelAmount.eq(0)) { throw new Error(ExchangeContractErrs.ORDER_CANCEL_AMOUNT_ZERO); } + const orderHash = await this.getOrderHashAsync(order); + const unavailableAmount = await this.getUnavailableTakerAmountAsync(orderHash); + if (order.takerTokenAmount.minus(unavailableAmount).eq(0)) { + throw new Error(ExchangeContractErrs.ORDER_ALREADY_CANCELLED_OR_FILLED); + } const currentUnixTimestampSec = Date.now() / 1000; if (order.expirationUnixTimestampSec.lessThan(currentUnixTimestampSec)) { throw new Error(ExchangeContractErrs.ORDER_CANCEL_EXPIRED); } } - /** * 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. -- cgit From 9daca6a4be95a87a63e293300d0768e3e63162d2 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 11:04:42 +0200 Subject: Address feedback --- src/contract_wrappers/exchange_wrapper.ts | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 4b7bd172f..537eb08a4 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -25,7 +25,7 @@ import {utils} from '../utils/utils'; import {ContractWrapper} from './contract_wrapper'; import * as ExchangeArtifacts from '../artifacts/Exchange.json'; import {ecSignatureSchema} from '../schemas/ec_signature_schema'; -import {signedOrderSchema} from '../schemas/order_schemas'; +import {signedOrderSchema, orderSchema} from '../schemas/order_schemas'; import {SchemaValidator} from '../utils/schema_validator'; import {constants} from '../utils/constants'; import {TokenWrapper} from './token_wrapper'; @@ -173,23 +173,23 @@ export class ExchangeWrapper extends ContractWrapper { this.throwErrorLogsAsErrors(response.logs); } /** - * Cancels the order. + * Cancel a given fill amount of an order. Cancellations are cumulative. */ - public async cancelOrderAsync(order: Order, cancelAmount: BigNumber.BigNumber): Promise { + public async cancelOrderAsync(order: Order, takerTokenCancelAmount: BigNumber.BigNumber): Promise { assert.doesConformToSchema('order', SchemaValidator.convertToJSONSchemaCompatibleObject(order as object), - signedOrderSchema); - assert.isBigNumber('cancelAmount', cancelAmount); + orderSchema); + assert.isBigNumber('takerTokenCancelAmount', takerTokenCancelAmount); await assert.isSenderAddressAvailableAsync(this.web3Wrapper, 'order.maker', order.maker); const exchangeInstance = await this.getExchangeContractAsync(); - await this.validateCancelOrderAndThrowIfInvalidAsync(order, cancelAmount); + await this.validateCancelOrderAndThrowIfInvalidAsync(order, takerTokenCancelAmount); const [orderAddresses, orderValues] = ExchangeWrapper.getOrderAddressesAndValues(order); const gas = await exchangeInstance.cancel.estimateGas( orderAddresses, orderValues, - cancelAmount, + takerTokenCancelAmount, { from: order.maker, }, @@ -197,7 +197,7 @@ export class ExchangeWrapper extends ContractWrapper { const response: ContractResponse = await exchangeInstance.cancel( orderAddresses, orderValues, - cancelAmount, + takerTokenCancelAmount, { from: order.maker, gas, @@ -231,14 +231,13 @@ export class ExchangeWrapper extends ContractWrapper { logEventObj.watch(callback); this.exchangeLogEventObjs.push(logEventObj); } - /** - * Get order hash + * Computes the orderHash for a given order and returns it as a hex encoded string. */ public async getOrderHashAsync(order: Order): Promise { const [orderAddresses, orderValues] = ExchangeWrapper.getOrderAddressesAndValues(order); const exchangeInstance = await this.getExchangeContractAsync(); - const orderHash = await exchangeInstance.getOrderHash.call(orderAddresses, orderValues); + const orderHash = utils.getOrderHashHex(order, exchangeInstance.address); return orderHash; } private async stopWatchingExchangeLogEventsAsync() { @@ -257,7 +256,7 @@ export class ExchangeWrapper extends ContractWrapper { if (signedOrder.taker !== constants.NULL_ADDRESS && signedOrder.taker !== senderAddress) { throw new Error(ExchangeContractErrs.TRANSACTION_SENDER_IS_NOT_FILL_ORDER_TAKER); } - const currentUnixTimestampSec = Date.now() / 1000; + const currentUnixTimestampSec = utils.getCurrentUnixTimestamp(); if (signedOrder.expirationUnixTimestampSec.lessThan(currentUnixTimestampSec)) { throw new Error(ExchangeContractErrs.ORDER_FILL_EXPIRED); } @@ -272,9 +271,9 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(ExchangeContractErrs.ORDER_FILL_ROUNDING_ERROR); } } - private async validateCancelOrderAndThrowIfInvalidAsync(order: Order, - cancelAmount: BigNumber.BigNumber): Promise { - if (cancelAmount.eq(0)) { + private async validateCancelOrderAndThrowIfInvalidAsync( + order: Order, takerTokenCancelAmount: BigNumber.BigNumber): Promise { + if (takerTokenCancelAmount.eq(0)) { throw new Error(ExchangeContractErrs.ORDER_CANCEL_AMOUNT_ZERO); } const orderHash = await this.getOrderHashAsync(order); @@ -282,7 +281,7 @@ export class ExchangeWrapper extends ContractWrapper { if (order.takerTokenAmount.minus(unavailableAmount).eq(0)) { throw new Error(ExchangeContractErrs.ORDER_ALREADY_CANCELLED_OR_FILLED); } - const currentUnixTimestampSec = Date.now() / 1000; + const currentUnixTimestampSec = utils.getCurrentUnixTimestamp(); if (order.expirationUnixTimestampSec.lessThan(currentUnixTimestampSec)) { throw new Error(ExchangeContractErrs.ORDER_CANCEL_EXPIRED); } -- cgit From 498cc6cea12d9c7d2bb1af6b50f5609efb074734 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 11:44:07 +0200 Subject: Use union type Order|SignedOrder --- 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 537eb08a4..fe03dbd16 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -175,7 +175,7 @@ export class ExchangeWrapper extends ContractWrapper { /** * Cancel a given fill amount of an order. Cancellations are cumulative. */ - public async cancelOrderAsync(order: Order, takerTokenCancelAmount: BigNumber.BigNumber): Promise { + public async cancelOrderAsync(order: Order|SignedOrder, takerTokenCancelAmount: BigNumber.BigNumber): Promise { assert.doesConformToSchema('order', SchemaValidator.convertToJSONSchemaCompatibleObject(order as object), orderSchema); @@ -234,7 +234,7 @@ export class ExchangeWrapper extends ContractWrapper { /** * Computes the orderHash for a given order and returns it as a hex encoded string. */ - public async getOrderHashAsync(order: Order): Promise { + public async getOrderHashAsync(order: Order|SignedOrder): Promise { const [orderAddresses, orderValues] = ExchangeWrapper.getOrderAddressesAndValues(order); const exchangeInstance = await this.getExchangeContractAsync(); const orderHash = utils.getOrderHashHex(order, exchangeInstance.address); -- cgit From 09f67322f1548b77ea3835b8a885e8c538835af8 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 7 Jun 2017 12:20:28 +0200 Subject: Remove assertions from utils methods --- src/contract_wrappers/exchange_wrapper.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'src/contract_wrappers') diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index fe03dbd16..6f62934dc 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -231,10 +231,7 @@ export class ExchangeWrapper extends ContractWrapper { logEventObj.watch(callback); this.exchangeLogEventObjs.push(logEventObj); } - /** - * Computes the orderHash for a given order and returns it as a hex encoded string. - */ - public async getOrderHashAsync(order: Order|SignedOrder): Promise { + private async getOrderHashAsync(order: Order|SignedOrder): Promise { const [orderAddresses, orderValues] = ExchangeWrapper.getOrderAddressesAndValues(order); const exchangeInstance = await this.getExchangeContractAsync(); const orderHash = utils.getOrderHashHex(order, exchangeInstance.address); -- cgit