diff options
author | Leonid <logvinov.leon@gmail.com> | 2017-07-26 04:26:14 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-07-26 04:26:14 +0800 |
commit | 89aa19f6e4042ffcda906e4fa6a7de3d6d2e57c1 (patch) | |
tree | 357d487f6f4442a04457d85b55853a78bb4b4e85 /src | |
parent | 97e680aba1a68c509e903e929a848db53182722f (diff) | |
parent | 660aa224ca990af27867cdd1523553f50ba50b77 (diff) | |
download | dexon-sol-tools-89aa19f6e4042ffcda906e4fa6a7de3d6d2e57c1.tar dexon-sol-tools-89aa19f6e4042ffcda906e4fa6a7de3d6d2e57c1.tar.gz dexon-sol-tools-89aa19f6e4042ffcda906e4fa6a7de3d6d2e57c1.tar.bz2 dexon-sol-tools-89aa19f6e4042ffcda906e4fa6a7de3d6d2e57c1.tar.lz dexon-sol-tools-89aa19f6e4042ffcda906e4fa6a7de3d6d2e57c1.tar.xz dexon-sol-tools-89aa19f6e4042ffcda906e4fa6a7de3d6d2e57c1.tar.zst dexon-sol-tools-89aa19f6e4042ffcda906e4fa6a7de3d6d2e57c1.zip |
Merge pull request #109 from 0xProject/fees-balance-allowance-validation
Fees balance allowance validation
Diffstat (limited to 'src')
-rw-r--r-- | src/0x.ts | 1 | ||||
-rw-r--r-- | src/contract_wrappers/exchange_wrapper.ts | 75 | ||||
-rw-r--r-- | src/contract_wrappers/proxy_wrapper.ts | 1 | ||||
-rw-r--r-- | src/contract_wrappers/token_registry_wrapper.ts | 1 | ||||
-rw-r--r-- | src/types.ts | 1 | ||||
-rw-r--r-- | src/utils/assert.ts | 1 | ||||
-rw-r--r-- | src/utils/order_validation_utils.ts | 81 |
7 files changed, 87 insertions, 74 deletions
@@ -3,7 +3,6 @@ import * as BigNumber from 'bignumber.js'; import {bigNumberConfigs} from './bignumber_config'; import * as ethUtil from 'ethereumjs-util'; import contract = require('truffle-contract'); -import * as Web3 from 'web3'; import findVersions = require('find-versions'); import compareVersions = require('compare-versions'); import {Web3Wrapper} from './web3_wrapper'; diff --git a/src/contract_wrappers/exchange_wrapper.ts b/src/contract_wrappers/exchange_wrapper.ts index 9cc29e286..2ddd63422 100644 --- a/src/contract_wrappers/exchange_wrapper.ts +++ b/src/contract_wrappers/exchange_wrapper.ts @@ -1,7 +1,6 @@ import * as _ from 'lodash'; import * as BigNumber from 'bignumber.js'; import promisify = require('es6-promisify'); -import * as Web3 from 'web3'; import {Web3Wrapper} from '../web3_wrapper'; import { ECSignature, @@ -27,16 +26,12 @@ import { LogErrorContractEventArgs, LogFillContractEventArgs, LogCancelContractEventArgs, - EventCallback, - ContractEventArg, - ExchangeContractByAddress, - ContractArtifact, } from '../types'; import {assert} from '../utils/assert'; import {utils} from '../utils/utils'; import {eventUtils} from '../utils/event_utils'; +import {OrderValidationUtils} from '../utils/order_validation_utils'; import {ContractWrapper} from './contract_wrapper'; -import {ProxyWrapper} from './proxy_wrapper'; import {ecSignatureSchema} from '../schemas/ec_signature_schema'; import {signedOrdersSchema} from '../schemas/signed_orders_schema'; import {subscriptionOptsSchema} from '../schemas/subscription_opts_schema'; @@ -66,6 +61,7 @@ export class ExchangeWrapper extends ContractWrapper { }; private _exchangeContractIfExists?: ExchangeContract; private _exchangeLogEventEmitters: ContractEventEmitter[]; + private _orderValidationUtils: OrderValidationUtils; private _tokenWrapper: TokenWrapper; private static _getOrderAddressesAndValues(order: Order): [OrderAddresses, OrderValues] { const orderAddresses: OrderAddresses = [ @@ -88,6 +84,7 @@ export class ExchangeWrapper extends ContractWrapper { constructor(web3Wrapper: Web3Wrapper, tokenWrapper: TokenWrapper) { super(web3Wrapper); this._tokenWrapper = tokenWrapper; + this._orderValidationUtils = new OrderValidationUtils(tokenWrapper); this._exchangeLogEventEmitters = []; } /** @@ -667,8 +664,9 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(ExchangeContractErrs.OrderFillExpired); } const zrxTokenAddress = await this._getZRXTokenAddressAsync(signedOrder.exchangeContractAddress); - await this._validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder, fillTakerAmount, - senderAddress, zrxTokenAddress); + await this._orderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, + ); const wouldRoundingErrorOccur = await this._isRoundingErrorAsync( signedOrder.takerTokenAmount, fillTakerAmount, signedOrder.makerTokenAmount, @@ -702,67 +700,6 @@ export class ExchangeWrapper extends ContractWrapper { throw new Error(ExchangeContractErrs.InsufficientRemainingFillAmount); } } - /** - * 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 in order to minimize - * the callers gas costs. - */ - private async _validateFillOrderBalancesAndAllowancesAndThrowIfInvalidAsync(signedOrder: SignedOrder, - fillTakerAmount: BigNumber.BigNumber, - senderAddress: string, - zrxTokenAddress: string, - ): Promise<void> { - - const makerBalance = await this._tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, - signedOrder.maker); - 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); - - // 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.InsufficientTakerBalance); - } - if (fillTakerAmount.greaterThan(takerAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); - } - if (fillMakerAmountInBaseUnits.greaterThan(makerBalance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerBalance); - } - if (fillMakerAmountInBaseUnits.greaterThan(makerAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); - } - - const makerFeeBalance = await this._tokenWrapper.getBalanceAsync(zrxTokenAddress, - signedOrder.maker); - const takerFeeBalance = await this._tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); - const makerFeeAllowance = await this._tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - signedOrder.maker); - const takerFeeAllowance = await this._tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, - senderAddress); - - if (signedOrder.takerFee.greaterThan(takerFeeBalance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerFeeBalance); - } - if (signedOrder.takerFee.greaterThan(takerFeeAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientTakerFeeAllowance); - } - if (signedOrder.makerFee.greaterThan(makerFeeBalance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerFeeBalance); - } - if (signedOrder.makerFee.greaterThan(makerFeeAllowance)) { - throw new Error(ExchangeContractErrs.InsufficientMakerFeeAllowance); - } - } private _throwErrorLogsAsErrors(logs: ContractEvent[]): void { const errEvent = _.find(logs, {event: 'LogError'}); if (!_.isUndefined(errEvent)) { diff --git a/src/contract_wrappers/proxy_wrapper.ts b/src/contract_wrappers/proxy_wrapper.ts index c247754f8..9deed5231 100644 --- a/src/contract_wrappers/proxy_wrapper.ts +++ b/src/contract_wrappers/proxy_wrapper.ts @@ -1,5 +1,4 @@ import * as _ from 'lodash'; -import {Web3Wrapper} from '../web3_wrapper'; import {ContractWrapper} from './contract_wrapper'; import * as ProxyArtifacts from '../artifacts/Proxy.json'; import {ProxyContract} from '../types'; diff --git a/src/contract_wrappers/token_registry_wrapper.ts b/src/contract_wrappers/token_registry_wrapper.ts index c9f21e46f..d15e5c4f7 100644 --- a/src/contract_wrappers/token_registry_wrapper.ts +++ b/src/contract_wrappers/token_registry_wrapper.ts @@ -1,7 +1,6 @@ import * as _ from 'lodash'; import {Web3Wrapper} from '../web3_wrapper'; import {Token, TokenRegistryContract, TokenMetadata} from '../types'; -import {assert} from '../utils/assert'; import {ContractWrapper} from './contract_wrapper'; import * as TokenRegistryArtifacts from '../artifacts/TokenRegistry.json'; diff --git a/src/types.ts b/src/types.ts index 974057fed..ee45acf11 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,4 +1,3 @@ -import * as _ from 'lodash'; import * as Web3 from 'web3'; export enum ZeroExError { diff --git a/src/utils/assert.ts b/src/utils/assert.ts index dab796c4e..969209208 100644 --- a/src/utils/assert.ts +++ b/src/utils/assert.ts @@ -3,7 +3,6 @@ import * as BigNumber from 'bignumber.js'; import * as Web3 from 'web3'; import {Web3Wrapper} from '../web3_wrapper'; import {SchemaValidator} from './schema_validator'; -import {utils} from './utils'; const HEX_REGEX = /^0x[0-9A-F]*$/i; diff --git a/src/utils/order_validation_utils.ts b/src/utils/order_validation_utils.ts new file mode 100644 index 000000000..8a737040c --- /dev/null +++ b/src/utils/order_validation_utils.ts @@ -0,0 +1,81 @@ +import {ExchangeContractErrs, SignedOrder} from '../types'; +import {TokenWrapper} from '../contract_wrappers/token_wrapper'; + +export class OrderValidationUtils { + private tokenWrapper: TokenWrapper; + constructor(tokenWrapper: TokenWrapper) { + this.tokenWrapper = tokenWrapper; + } + public async validateFillOrderBalancesAllowancesThrowIfInvalidAsync( + signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string, + ): Promise<void> { + await this.validateFillOrderMakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, zrxTokenAddress, + ); + await this.validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder, fillTakerAmount, senderAddress, zrxTokenAddress, + ); + } + private async validateFillOrderMakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, zrxTokenAddress: string, + ): Promise<void> { + const makerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.makerTokenAddress, signedOrder.maker); + const makerAllowance = await this.tokenWrapper.getProxyAllowanceAsync( + signedOrder.makerTokenAddress, signedOrder.maker); + + const isMakerTokenZRX = signedOrder.makerTokenAddress === zrxTokenAddress; + // exchangeRate is the price of one maker token denominated in taker tokens + const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); + const fillMakerAmount = fillTakerAmount.div(exchangeRate); + + const requiredMakerAmount = isMakerTokenZRX ? fillMakerAmount.plus(signedOrder.makerFee) : fillMakerAmount; + if (requiredMakerAmount.greaterThan(makerBalance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerBalance); + } + if (requiredMakerAmount.greaterThan(makerAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerAllowance); + } + + if (!isMakerTokenZRX) { + const makerZRXBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, signedOrder.maker); + const makerZRXAllowance = await this.tokenWrapper.getProxyAllowanceAsync( + zrxTokenAddress, signedOrder.maker); + + if (signedOrder.makerFee.greaterThan(makerZRXBalance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerFeeBalance); + } + if (signedOrder.makerFee.greaterThan(makerZRXAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientMakerFeeAllowance); + } + } + } + private async validateFillOrderTakerBalancesAllowancesThrowIfInvalidAsync( + signedOrder: SignedOrder, fillTakerAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string, + ): Promise<void> { + const takerBalance = await this.tokenWrapper.getBalanceAsync(signedOrder.takerTokenAddress, senderAddress); + const takerAllowance = await this.tokenWrapper.getProxyAllowanceAsync( + signedOrder.takerTokenAddress, senderAddress); + + const isTakerTokenZRX = signedOrder.takerTokenAddress === zrxTokenAddress; + + const requiredTakerAmount = isTakerTokenZRX ? fillTakerAmount.plus(signedOrder.takerFee) : fillTakerAmount; + if (requiredTakerAmount.greaterThan(takerBalance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerBalance); + } + if (requiredTakerAmount.greaterThan(takerAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerAllowance); + } + + if (!isTakerTokenZRX) { + const takerZRXBalance = await this.tokenWrapper.getBalanceAsync(zrxTokenAddress, senderAddress); + const takerZRXAllowance = await this.tokenWrapper.getProxyAllowanceAsync(zrxTokenAddress, senderAddress); + + if (signedOrder.takerFee.greaterThan(takerZRXBalance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerFeeBalance); + } + if (signedOrder.takerFee.greaterThan(takerZRXAllowance)) { + throw new Error(ExchangeContractErrs.InsufficientTakerFeeAllowance); + } + } + } +} |