diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index 8d12babd61..1d6f3c6acd 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -153,6 +153,18 @@ { "note": "Added `DODO`", "pr": 2701 + }, + { + "note": "Fix for some edge cases with `includedSources` and `MultiHop`", + "pr": 2730 + }, + { + "note": "Introduced `excludedFeeSources` to disable sources when determining the price of an asset in ETH", + "pr": 2731 + }, + { + "note": "Support DODO Trade Allowed parameter to automatically disable the pool", + "pr": 2732 } ] }, diff --git a/packages/asset-swapper/contracts/src/DODOSampler.sol b/packages/asset-swapper/contracts/src/DODOSampler.sol index 2d95b6df9c..db018c4f0b 100644 --- a/packages/asset-swapper/contracts/src/DODOSampler.sol +++ b/packages/asset-swapper/contracts/src/DODOSampler.sol @@ -34,6 +34,7 @@ interface IDODOHelper { interface IDODO { function querySellBaseToken(uint256 amount) external view returns (uint256); + function _TRADE_ALLOWED_() external view returns (bool); } contract DODOSampler is @@ -80,6 +81,11 @@ contract DODOSampler is sellBase = false; } + // DODO Pool has been disabled + if (!IDODO(pool)._TRADE_ALLOWED_()) { + return (sellBase, pool, makerTokenAmounts); + } + for (uint256 i = 0; i < numSamples; i++) { uint256 buyAmount = _sampleSellForApproximateBuyFromDODO( abi.encode(takerToken, pool, baseToken), // taker token data @@ -132,6 +138,11 @@ contract DODOSampler is sellBase = false; } + // DODO Pool has been disabled + if (!IDODO(pool)._TRADE_ALLOWED_()) { + return (sellBase, pool, takerTokenAmounts); + } + takerTokenAmounts = _sampleApproximateBuys( ApproximateBuyQuoteOpts({ makerTokenData: abi.encode(makerToken, pool, baseToken), diff --git a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts index d3a6bb7c60..1bb895690c 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts @@ -57,6 +57,7 @@ export const DEFAULT_GET_MARKET_ORDERS_OPTS: GetMarketOrdersOpts = { // tslint:disable-next-line: custom-no-magic-numbers runLimit: 2 ** 15, excludedSources: [], + excludedFeeSources: [], includedSources: [], bridgeSlippage: 0.005, maxFallbackSlippage: 0.05, diff --git a/packages/asset-swapper/src/utils/market_operation_utils/index.ts b/packages/asset-swapper/src/utils/market_operation_utils/index.ts index ee0a98ce23..8f406f055f 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -137,10 +137,12 @@ export class MarketOperationUtils { const _opts = { ...DEFAULT_GET_MARKET_ORDERS_OPTS, ...opts }; const [makerToken, takerToken] = getNativeOrderTokens(nativeOrders[0]); const sampleAmounts = getSampleAmounts(takerAmount, _opts.numSamples, _opts.sampleDistributionBase); + const requestFilters = new SourceFilters().exclude(_opts.excludedSources).include(_opts.includedSources); - const feeSourceFilters = this._feeSources.merge(requestFilters); const quoteSourceFilters = this._sellSources.merge(requestFilters); + const feeSourceFilters = this._feeSources.exclude(_opts.excludedFeeSources); + const { onChain: sampleBalancerOnChain, offChain: sampleBalancerOffChain, @@ -278,10 +280,12 @@ export class MarketOperationUtils { const _opts = { ...DEFAULT_GET_MARKET_ORDERS_OPTS, ...opts }; const [makerToken, takerToken] = getNativeOrderTokens(nativeOrders[0]); const sampleAmounts = getSampleAmounts(makerAmount, _opts.numSamples, _opts.sampleDistributionBase); + const requestFilters = new SourceFilters().exclude(_opts.excludedSources).include(_opts.includedSources); - const feeSourceFilters = this._feeSources.merge(requestFilters); const quoteSourceFilters = this._buySources.merge(requestFilters); + const feeSourceFilters = this._feeSources.exclude(_opts.excludedFeeSources); + const { onChain: sampleBalancerOnChain, offChain: sampleBalancerOffChain, @@ -486,9 +490,10 @@ export class MarketOperationUtils { const _opts = { ...DEFAULT_GET_MARKET_ORDERS_OPTS, ...opts }; const requestFilters = new SourceFilters().exclude(_opts.excludedSources).include(_opts.includedSources); - const feeSourceFilters = this._feeSources.merge(requestFilters); const quoteSourceFilters = this._buySources.merge(requestFilters); + const feeSourceFilters = this._feeSources.exclude(_opts.excludedFeeSources); + const ops = [ ...batchNativeOrders.map(orders => this._sampler.getOrderFillableMakerAmounts(orders, this.contractAddresses.exchange), @@ -620,11 +625,9 @@ export class MarketOperationUtils { ethToInputRate, exchangeProxyOverhead: opts.exchangeProxyOverhead || (() => ZERO_AMOUNT), }; + const optimalPath = await findOptimalPathAsync(side, fills, inputAmount, opts.runLimit, optimizerOpts); - if (optimalPath === undefined) { - throw new Error(AggregationError.NoOptimalPath); - } - const optimalPathRate = optimalPath.adjustedRate(); + const optimalPathRate = optimalPath ? optimalPath.adjustedRate() : ZERO_AMOUNT; const { adjustedRate: bestTwoHopRate, quote: bestTwoHopQuote } = getBestTwoHopQuote( marketSideLiquidity, @@ -640,6 +643,11 @@ export class MarketOperationUtils { }; } + // If there is no optimal path AND we didn't return a MultiHop quote, then throw + if (optimalPath === undefined) { + throw new Error(AggregationError.NoOptimalPath); + } + // Generate a fallback path if native orders are in the optimal path. const nativeFills = optimalPath.fills.filter(f => f.source === ERC20BridgeSource.Native); if (opts.allowFallback && nativeFills.length !== 0) { diff --git a/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts b/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts index 9d6832b858..0a70ab1814 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts @@ -46,26 +46,37 @@ export function getBestTwoHopQuote( exchangeProxyOverhead?: ExchangeProxyOverhead, ): { quote: DexSample | undefined; adjustedRate: BigNumber } { const { side, inputAmount, ethToOutputRate, twoHopQuotes } = marketSideLiquidity; - if (twoHopQuotes.length === 0) { - return { adjustedRate: ZERO_AMOUNT, quote: undefined }; + // Ensure the expected data we require exists. In the case where all hops reverted + // or there were no sources included that allowed for multi hop, + // we can end up with empty, but not undefined, fill data + const filteredQuotes = twoHopQuotes.filter( + quote => + quote && + quote.fillData && + quote.fillData.firstHopSource && + quote.fillData.secondHopSource && + quote.output.isGreaterThan(ZERO_AMOUNT), + ); + if (filteredQuotes.length === 0) { + return { quote: undefined, adjustedRate: ZERO_AMOUNT }; } - const best = twoHopQuotes + const best = filteredQuotes .map(quote => getTwoHopAdjustedRate(side, quote, inputAmount, ethToOutputRate, feeSchedule, exchangeProxyOverhead), ) .reduce( (prev, curr, i) => - curr.isGreaterThan(prev.adjustedRate) ? { adjustedRate: curr, quote: twoHopQuotes[i] } : prev, + curr.isGreaterThan(prev.adjustedRate) ? { adjustedRate: curr, quote: filteredQuotes[i] } : prev, { adjustedRate: getTwoHopAdjustedRate( side, - twoHopQuotes[0], + filteredQuotes[0], inputAmount, ethToOutputRate, feeSchedule, exchangeProxyOverhead, ), - quote: twoHopQuotes[0], + quote: filteredQuotes[0], }, ); return best; diff --git a/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts b/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts index b80b281720..d700eed974 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts @@ -636,11 +636,12 @@ export class SamplerOperations { const [firstHop, secondHop, buyAmount] = this._samplerContract.getABIDecodedReturnData< [HopInfo, HopInfo, BigNumber] >('sampleTwoHopSell', callResults); + // Ensure the hop sources are set even when the buy amount is zero + fillData.firstHopSource = firstHopOps[firstHop.sourceIndex.toNumber()]; + fillData.secondHopSource = secondHopOps[secondHop.sourceIndex.toNumber()]; if (buyAmount.isZero()) { return [ZERO_AMOUNT]; } - fillData.firstHopSource = firstHopOps[firstHop.sourceIndex.toNumber()]; - fillData.secondHopSource = secondHopOps[secondHop.sourceIndex.toNumber()]; fillData.firstHopSource.handleCallResults(firstHop.returnData); fillData.secondHopSource.handleCallResults(secondHop.returnData); return [buyAmount]; diff --git a/packages/asset-swapper/src/utils/market_operation_utils/types.ts b/packages/asset-swapper/src/utils/market_operation_utils/types.ts index 43edff370a..5a7ebc9dc4 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -242,6 +242,11 @@ export interface GetMarketOrdersOpts { * Liquidity sources to exclude. Default is none. */ excludedSources: ERC20BridgeSource[]; + /** + * Liquidity sources to exclude when used to calculate the cost of the route. + * Default is none. + */ + excludedFeeSources: ERC20BridgeSource[]; /** * Liquidity sources to include. Default is none, which allows all supported * sources that aren't excluded by `excludedSources`.