diff --git a/src/constants.ts b/src/constants.ts index dcce8d7ba..a8ff9a31f 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -17,6 +17,7 @@ export const DEFAULT_LOGGER_INCLUDE_TIMESTAMP = true; export const ONE_SECOND_MS = 1000; export const ONE_MINUTE_MS = ONE_SECOND_MS * 60; export const TEN_MINUTES_MS = ONE_MINUTE_MS * 10; +export const DEFAULT_VALIDATION_GAS_LIMIT = 10e6; // The number of orders to post to Mesh at one time export const MESH_ORDERS_BATCH_SIZE = 200; diff --git a/src/services/meta_transaction_service.ts b/src/services/meta_transaction_service.ts index 5cf68e546..1d4372edb 100644 --- a/src/services/meta_transaction_service.ts +++ b/src/services/meta_transaction_service.ts @@ -21,6 +21,7 @@ import { RFQT_SKIP_BUY_REQUESTS, } from '../config'; import { + DEFAULT_VALIDATION_GAS_LIMIT, ONE_GWEI, ONE_MINUTE_MS, ONE_SECOND_MS, @@ -265,6 +266,7 @@ export class MetaTransactionService { from: PUBLIC_ADDRESS_FOR_ETH_CALLS, gasPrice, value: protocolFee, + gas: DEFAULT_VALIDATION_GAS_LIMIT, }); } catch (err) { // we reach into the underlying revert and throw it instead of @@ -296,7 +298,6 @@ export class MetaTransactionService { protocolFee: BigNumber, ): Promise { const gasPrice = zeroExTransaction.gasPrice; - // TODO(dekz): our pattern is to eth_call and estimateGas in parallel and return the result of eth_call validations const gas = await this._contractWrappers.exchange .executeTransaction(zeroExTransaction, signature) .estimateGasAsync({ diff --git a/src/services/swap_service.ts b/src/services/swap_service.ts index a5da963ff..07d9f5972 100644 --- a/src/services/swap_service.ts +++ b/src/services/swap_service.ts @@ -24,6 +24,7 @@ import { RFQT_SKIP_BUY_REQUESTS, } from '../config'; import { + DEFAULT_VALIDATION_GAS_LIMIT, GAS_LIMIT_BUFFER_MULTIPLIER, GST2_WALLET_ADDRESSES, ONE, @@ -86,7 +87,16 @@ export class SwapService { } public async calculateSwapQuoteAsync(params: CalculateSwapQuoteParams): Promise { - const { buyAmount, buyTokenAddress, sellTokenAddress, isETHSell, from, affiliateAddress } = params; + const { + buyAmount, + buyTokenAddress, + sellTokenAddress, + isETHSell, + from, + affiliateAddress, + // tslint:disable-next-line:boolean-naming + skipValidation, + } = params; const swapQuote = await this._getMarketBuyOrSellQuoteAsync(params); const attributedSwapQuote = serviceUtils.attributeSwapQuoteOrders(swapQuote); @@ -115,9 +125,7 @@ export class SwapService { ); let conservativeBestCaseGasEstimate = new BigNumber(worstCaseGas).plus(gasTokenGasCost); - // Temporarily disable validation - // if (!skipValidation && from) { - if (false) { + if (!skipValidation && from) { // Force a revert error if the takerAddress does not have enough ETH. const txDataValue = isETHSell ? BigNumber.min(value, await this._web3Wrapper.getBalanceInWeiAsync(from)) @@ -135,7 +143,10 @@ export class SwapService { // Add a buffer to get the worst case gas estimate const worstCaseGasEstimate = conservativeBestCaseGasEstimate.times(GAS_LIMIT_BUFFER_MULTIPLIER).integerValue(); // Cap the refund at 50% our best estimate - const estimatedGasTokenRefund = BigNumber.min(conservativeBestCaseGasEstimate.div(2), gasTokenRefund); + const estimatedGasTokenRefund = BigNumber.min( + conservativeBestCaseGasEstimate.div(2), + gasTokenRefund, + ).decimalPlaces(0); const { price, guaranteedPrice } = await this._getSwapQuotePriceAsync( buyAmount, buyTokenAddress, @@ -275,12 +286,8 @@ export class SwapService { } private async _estimateGasOrThrowRevertErrorAsync(txData: Partial): Promise { - // Perform this concurrently - // if the call fails the gas estimation will also fail, we can throw a more helpful - // error message than gas estimation failure - const estimateGasPromise = this._web3Wrapper.estimateGasAsync(txData).catch(_e => 0); - await this._throwIfCallIsRevertErrorAsync(txData); - const gas = await estimateGasPromise; + const gas = await this._web3Wrapper.estimateGasAsync(txData).catch(_e => DEFAULT_VALIDATION_GAS_LIMIT); + await this._throwIfCallIsRevertErrorAsync({ ...txData, gas }); return new BigNumber(gas); } @@ -291,9 +298,20 @@ export class SwapService { callResult = await this._web3Wrapper.callAsync(txData); } catch (e) { // RPCSubprovider can throw if .error exists on the response payload - // This `error` response occurs from Parity nodes (incl Alchemy) but not on INFURA (geth) - revertError = decodeThrownErrorAsRevertError(e); - throw revertError; + // This `error` response occurs from Parity nodes (incl Alchemy) and Geth nodes >= 1.9.14 + // Geth 1.9.15 + if (e.message && /execution reverted/.test(e.message) && e.data) { + try { + revertError = RevertError.decode(e.data, false); + } catch (e) { + // No revert error + } + } else { + revertError = decodeThrownErrorAsRevertError(e); + } + if (revertError) { + throw revertError; + } } try { revertError = RevertError.decode(callResult, false); diff --git a/test/rfqt_test.ts b/test/rfqt_test.ts index 0ae1761bd..9be32a776 100644 --- a/test/rfqt_test.ts +++ b/test/rfqt_test.ts @@ -285,7 +285,7 @@ describe(SUITE_NAME, () => { }, ); }); - it.skip('should fail validation when taker can not actually fill', async () => { + it('should fail validation when taker can not actually fill', async () => { const wethContract = new WETH9Contract(contractAddresses.etherToken, provider); await wethContract .approve(contractAddresses.erc20Proxy, new BigNumber(0)) diff --git a/test/swap_test.ts b/test/swap_test.ts index 8c20afc8e..788fd57c8 100644 --- a/test/swap_test.ts +++ b/test/swap_test.ts @@ -217,7 +217,7 @@ describe(SUITE_NAME, () => { }, ); }); - it.skip('should throw a validation error if takerAddress cannot complete the quote', async () => { + it('should throw a validation error if takerAddress cannot complete the quote', async () => { // The taker does not have an allowance await quoteAndExpectAsync( {