fix: asset-swapper yield to the event loop every mix path (#2637)

* fix: allow a empty overrides to signal no default

* fix: asset-swapper yield to the event loop every mix path

* fix: optimizations skip order find if Native excluded

* changelogs

* chore: update protocol fee multiplier

* fix: tests async
This commit is contained in:
Jacob Evans 2020-07-21 17:06:05 +10:00 committed by GitHub
parent b5eb1c9ee8
commit aae93bb6a7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 100 additions and 69 deletions

View File

@ -1,4 +1,17 @@
[
{
"version": "4.6.1",
"changes": [
{
"note": "Allow an empty override for sampler overrides",
"pr": 2637
},
{
"note": "Potentially heavy CPU functions inside the optimizer now yield to the event loop. As such they are now async.",
"pr": 2637
}
]
},
{
"version": "4.6.0",
"changes": [

View File

@ -31,7 +31,7 @@ const DEFAULT_ORDER_PRUNER_OPTS: OrderPrunerOpts = {
// 6 seconds polling interval
const PROTOCOL_FEE_UTILS_POLLING_INTERVAL_IN_MS = 6000;
const PROTOCOL_FEE_MULTIPLIER = new BigNumber(150000);
const PROTOCOL_FEE_MULTIPLIER = new BigNumber(70000);
// default 50% buffer for selecting native orders to be aggregated with other sources
const MARKET_UTILS_AMOUNT_BUFFER_PERCENTAGE = 0.5;

View File

@ -26,6 +26,7 @@ import { calculateLiquidity } from './utils/calculate_liquidity';
import { MarketOperationUtils } from './utils/market_operation_utils';
import { createDummyOrderForSampler } from './utils/market_operation_utils/orders';
import { DexOrderSampler } from './utils/market_operation_utils/sampler';
import { ERC20BridgeSource } from './utils/market_operation_utils/types';
import { orderPrunerUtils } from './utils/order_prune_utils';
import { OrderStateUtils } from './utils/order_state_utils';
import { ProtocolFeeUtils } from './utils/protocol_fee_utils';
@ -193,8 +194,7 @@ export class SwapQuoter {
[this._contractAddresses.erc20BridgeSampler]: { code: samplerBytecode },
}
: {};
const samplerOverrides = _.merge(
{},
const samplerOverrides = _.assign(
{ block: BlockParamLiteral.Latest, overrides: defaultCodeOverrides },
options.samplerOverrides,
);
@ -563,7 +563,12 @@ export class SwapQuoter {
}
// get batches of orders from different sources, awaiting sources in parallel
const orderBatchPromises: Array<Promise<SignedOrder[]>> = [];
orderBatchPromises.push(this._getSignedOrdersAsync(makerAssetData, takerAssetData)); // order book
orderBatchPromises.push(
// Don't fetch from the DB if Native has been excluded
opts.excludedSources.includes(ERC20BridgeSource.Native)
? Promise.resolve([])
: this._getSignedOrdersAsync(makerAssetData, takerAssetData),
);
if (
opts.rfqt &&
opts.rfqt.intentOnFilling &&

View File

@ -1,7 +1,6 @@
import { BigNumber } from '@0x/utils';
import { bmath, getPoolsWithTokens, parsePoolData } from '@balancer-labs/sor';
import { Decimal } from 'decimal.js';
import * as _ from 'lodash';
export interface BalancerPool {
id: string;
@ -23,7 +22,7 @@ interface CacheValue {
// tslint:disable:custom-no-magic-numbers
const FIVE_SECONDS_MS = 5 * 1000;
const DEFAULT_TIMEOUT_MS = 1000;
const MAX_POOLS_FETCHED = 3;
const MAX_POOLS_FETCHED = 2;
const Decimal20 = Decimal.clone({ precision: 20 });
// tslint:enable:custom-no-magic-numbers
@ -31,6 +30,7 @@ export class BalancerPoolsCache {
constructor(
private readonly _cache: { [key: string]: CacheValue } = {},
public cacheExpiryMs: number = FIVE_SECONDS_MS,
private readonly maxPoolsFetched: number = MAX_POOLS_FETCHED,
) {}
public async getPoolsForPairAsync(
@ -65,7 +65,7 @@ export class BalancerPoolsCache {
const pools = parsePoolData(poolData, takerToken, makerToken).sort((a, b) =>
b.balanceOut.minus(a.balanceOut).toNumber(),
);
return pools.length > MAX_POOLS_FETCHED ? pools.slice(0, MAX_POOLS_FETCHED) : pools;
return pools.length > this.maxPoolsFetched ? pools.slice(0, this.maxPoolsFetched) : pools;
} catch (err) {
return [];
}

View File

@ -14,7 +14,7 @@ import {
createSignedOrdersWithFillableAmounts,
getNativeOrderTokens,
} from './orders';
import { findOptimalPath } from './path_optimizer';
import { findOptimalPathAsync } from './path_optimizer';
import { DexOrderSampler, getSampleAmounts } from './sampler';
import {
AggregationError,
@ -125,8 +125,8 @@ export class MarketOperationUtils {
_opts,
);
const balancerPromise = this._sampler.executeAsync(
await DexOrderSampler.ops.getSellQuotesAsync(
const balancerPromise = DexOrderSampler.ops
.getSellQuotesAsync(
difference([ERC20BridgeSource.Balancer], _opts.excludedSources),
makerToken,
takerToken,
@ -135,15 +135,15 @@ export class MarketOperationUtils {
this._sampler.balancerPoolsCache,
this._liquidityProviderRegistry,
this._multiBridge,
),
);
)
.then(async r => this._sampler.executeAsync(r));
const [
[orderFillableAmounts, liquidityProviderAddress, ethToMakerAssetRate, dexQuotes],
rfqtIndicativeQuotes,
[balancerQuotes],
] = await Promise.all([samplerPromise, rfqtPromise, balancerPromise]);
return this._generateOptimizedOrders({
return this._generateOptimizedOrdersAsync({
orderFillableAmounts,
nativeOrders,
dexQuotes: dexQuotes.concat(balancerQuotes),
@ -247,7 +247,7 @@ export class MarketOperationUtils {
[balancerQuotes],
] = await Promise.all([samplerPromise, rfqtPromise, balancerPromise]);
return this._generateOptimizedOrders({
return this._generateOptimizedOrdersAsync({
orderFillableAmounts,
nativeOrders,
dexQuotes: dexQuotes.concat(balancerQuotes),
@ -325,7 +325,8 @@ export class MarketOperationUtils {
const batchEthToTakerAssetRate = executeResults.splice(0, batchNativeOrders.length) as BigNumber[];
const batchDexQuotes = executeResults.splice(0, batchNativeOrders.length) as DexSample[][][];
return batchNativeOrders.map((nativeOrders, i) => {
return Promise.all(
batchNativeOrders.map(async (nativeOrders, i) => {
if (nativeOrders.length === 0) {
throw new Error(AggregationError.EmptyOrders);
}
@ -335,7 +336,7 @@ export class MarketOperationUtils {
const dexQuotes = batchDexQuotes[i];
const makerAmount = makerAmounts[i];
try {
return this._generateOptimizedOrders({
return await this._generateOptimizedOrdersAsync({
orderFillableAmounts,
nativeOrders,
dexQuotes,
@ -357,10 +358,11 @@ export class MarketOperationUtils {
// rather than throw NO_OPTIMAL_PATH we return undefined
return undefined;
}
});
}),
);
}
private _generateOptimizedOrders(opts: {
private async _generateOptimizedOrdersAsync(opts: {
side: MarketOperation;
inputToken: string;
outputToken: string;
@ -379,7 +381,7 @@ export class MarketOperationUtils {
shouldBatchBridgeOrders?: boolean;
liquidityProviderAddress?: string;
multiBridgeAddress?: string;
}): OptimizedMarketOrder[] {
}): Promise<OptimizedMarketOrder[]> {
const { inputToken, outputToken, side, inputAmount } = opts;
const maxFallbackSlippage = opts.maxFallbackSlippage || 0;
// Convert native orders and dex quotes into fill paths.
@ -397,7 +399,7 @@ export class MarketOperationUtils {
feeSchedule: opts.feeSchedule,
});
// Find the optimal path.
let optimalPath = findOptimalPath(side, paths, inputAmount, opts.runLimit) || [];
let optimalPath = (await findOptimalPathAsync(side, paths, inputAmount, opts.runLimit)) || [];
if (optimalPath.length === 0) {
throw new Error(AggregationError.NoOptimalPath);
}
@ -407,7 +409,8 @@ export class MarketOperationUtils {
// We create a fallback path that is exclusive of Native liquidity
// This is the optimal on-chain path for the entire input amount
const nonNativePaths = paths.filter(p => p.length > 0 && p[0].source !== ERC20BridgeSource.Native);
const nonNativeOptimalPath = findOptimalPath(side, nonNativePaths, inputAmount, opts.runLimit) || [];
const nonNativeOptimalPath =
(await findOptimalPathAsync(side, nonNativePaths, inputAmount, opts.runLimit)) || [];
// Calculate the slippage of on-chain sources compared to the most optimal path
const fallbackSlippage = getPathAdjustedSlippage(
side,

View File

@ -9,17 +9,21 @@ import { Fill } from './types';
// tslint:disable: prefer-for-of custom-no-magic-numbers completed-docs
const RUN_LIMIT_DECAY_FACTOR = 0.8;
// Used to yield the event loop when performing CPU intensive tasks
// tislint:disable-next-line:no-inferred-empty-object-type
const setImmediateAsync = async (delay: number = 0) =>
new Promise<void>(resolve => setImmediate(() => resolve(), delay));
/**
* Find the optimal mixture of paths that maximizes (for sells) or minimizes
* (for buys) output, while meeting the input requirement.
*/
export function findOptimalPath(
export async function findOptimalPathAsync(
side: MarketOperation,
paths: Fill[][],
targetInput: BigNumber,
runLimit: number = 2 ** 15,
): Fill[] | undefined {
): Promise<Fill[] | undefined> {
// Sort paths in descending order by adjusted output amount.
const sortedPaths = paths
.slice(0)
@ -27,6 +31,7 @@ export function findOptimalPath(
let optimalPath = sortedPaths[0] || [];
for (const [i, path] of sortedPaths.slice(1).entries()) {
optimalPath = mixPaths(side, optimalPath, path, targetInput, runLimit * RUN_LIMIT_DECAY_FACTOR ** i);
await setImmediateAsync();
}
return isPathComplete(optimalPath, targetInput) ? optimalPath : undefined;
}

View File

@ -71,7 +71,6 @@ describe('ExchangeSwapQuoteConsumer', () => {
let takerTokenAddress: string;
let makerAssetData: string;
let takerAssetData: string;
let wethAssetData: string;
let contractAddresses: ContractAddresses;
let exchangeContract: ExchangeContract;
@ -96,10 +95,9 @@ describe('ExchangeSwapQuoteConsumer', () => {
userAddresses = await web3Wrapper.getAvailableAddressesAsync();
[coinbaseAddress, takerAddress, makerAddress, feeRecipient] = userAddresses;
[makerTokenAddress, takerTokenAddress] = tokenUtils.getDummyERC20TokenAddresses();
[makerAssetData, takerAssetData, wethAssetData] = [
[makerAssetData, takerAssetData] = [
assetDataUtils.encodeERC20AssetData(makerTokenAddress),
assetDataUtils.encodeERC20AssetData(takerTokenAddress),
assetDataUtils.encodeERC20AssetData(contractAddresses.etherToken),
];
erc20MakerTokenContract = new ERC20TokenContract(makerTokenAddress, provider);
erc20TakerTokenContract = new ERC20TokenContract(takerTokenAddress, provider);

View File

@ -21,7 +21,7 @@ import { createFillPaths } from '../src/utils/market_operation_utils/fills';
import { DexOrderSampler } from '../src/utils/market_operation_utils/sampler';
import { DexSample, ERC20BridgeSource, FillData, NativeFillData } from '../src/utils/market_operation_utils/types';
// tslint:disable: custom-no-magic-numbers
// tslint:disable: custom-no-magic-numbers promise-function-async
describe('MarketOperationUtils tests', () => {
const CHAIN_ID = 1;
const contractAddresses = { ...getContractAddressesForChainOrThrow(CHAIN_ID), multiBridge: NULL_ADDRESS };
@ -153,7 +153,7 @@ describe('MarketOperationUtils tests', () => {
fillAmounts: BigNumber[],
wethAddress: string,
liquidityProviderAddress?: string,
) => DexSample[][];
) => Promise<DexSample[][]>;
function createGetMultipleSellQuotesOperationFromRates(rates: RatesBySource): GetMultipleQuotesOperation {
return (
@ -163,7 +163,7 @@ describe('MarketOperationUtils tests', () => {
fillAmounts: BigNumber[],
_wethAddress: string,
) => {
return sources.map(s => createSamplesFromRates(s, fillAmounts, rates[s]));
return Promise.resolve(sources.map(s => createSamplesFromRates(s, fillAmounts, rates[s])));
};
}
@ -206,7 +206,9 @@ describe('MarketOperationUtils tests', () => {
fillAmounts: BigNumber[],
_wethAddress: string,
) => {
return sources.map(s => createSamplesFromRates(s, fillAmounts, rates[s].map(r => new BigNumber(1).div(r))));
return Promise.resolve(
sources.map(s => createSamplesFromRates(s, fillAmounts, rates[s].map(r => new BigNumber(1).div(r)))),
);
};
}

View File

@ -23,7 +23,7 @@ const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
const ONE_ETH_IN_WEI = new BigNumber(1000000000000000000);
const TESTRPC_CHAIN_ID = devConstants.TESTRPC_CHAIN_ID;
const GAS_PRICE = new BigNumber(devConstants.DEFAULT_GAS_PRICE);
const PROTOCOL_FEE_MULTIPLIER = 150000;
const PROTOCOL_FEE_MULTIPLIER = 70000;
const PROTOCOL_FEE_PER_FILL = GAS_PRICE.times(PROTOCOL_FEE_MULTIPLIER);
const UNLIMITED_ALLOWANCE_IN_BASE_UNITS = new BigNumber(2).pow(256).minus(1); // tslint:disable-line:custom-no-magic-numbers
const EXPIRY_BUFFER_MS = 120000;
@ -44,7 +44,6 @@ describe('orderPrunerUtils', () => {
let makerAssetData: string;
let takerAssetData: string;
let orderFactory: OrderFactory;
let wethAssetData: string;
let contractAddresses: ContractAddresses;
let nonOpenSignedOrder: SignedOrder;
@ -68,10 +67,9 @@ describe('orderPrunerUtils', () => {
erc20TakerTokenContract = new ERC20TokenContract(takerTokenAddress, provider);
exchangeContract = new ExchangeContract(contractAddresses.exchange, provider);
[makerAssetData, takerAssetData, wethAssetData] = [
[makerAssetData, takerAssetData] = [
assetDataUtils.encodeERC20AssetData(makerTokenAddress),
assetDataUtils.encodeERC20AssetData(takerTokenAddress),
assetDataUtils.encodeERC20AssetData(contractAddresses.etherToken),
];
// Configure order defaults

View File

@ -23,7 +23,7 @@ const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
const ONE_ETH_IN_WEI = new BigNumber(1000000000000000000);
const TESTRPC_CHAIN_ID = devConstants.TESTRPC_CHAIN_ID;
const GAS_PRICE = new BigNumber(devConstants.DEFAULT_GAS_PRICE);
const PROTOCOL_FEE_MULTIPLIER = 150000;
const PROTOCOL_FEE_MULTIPLIER = 70000;
const PROTOCOL_FEE_PER_FILL = GAS_PRICE.times(PROTOCOL_FEE_MULTIPLIER);
const UNLIMITED_ALLOWANCE_IN_BASE_UNITS = new BigNumber(2).pow(256).minus(1); // tslint:disable-line:custom-no-magic-numbers
@ -51,7 +51,6 @@ describe('OrderStateUtils', () => {
let makerAssetData: string;
let takerAssetData: string;
let orderFactory: OrderFactory;
let wethAssetData: string;
let contractAddresses: ContractAddresses;
let orderStateUtils: OrderStateUtils;
@ -78,10 +77,9 @@ describe('OrderStateUtils', () => {
erc20TakerTokenContract = new ERC20TokenContract(takerTokenAddress, provider);
exchangeContract = new ExchangeContract(contractAddresses.exchange, provider);
[makerAssetData, takerAssetData, wethAssetData] = [
[makerAssetData, takerAssetData] = [
assetDataUtils.encodeERC20AssetData(makerTokenAddress),
assetDataUtils.encodeERC20AssetData(takerTokenAddress),
assetDataUtils.encodeERC20AssetData(contractAddresses.etherToken),
];
// Configure order defaults

View File

@ -1,4 +1,13 @@
[
{
"version": "6.4.1",
"changes": [
{
"note": "Change test protocol fee to 70000.",
"pr": 2637
}
]
},
{
"version": "6.4.0",
"changes": [

View File

@ -274,7 +274,7 @@ export async function runMigrationsAsync(
await stakingProxy.addAuthorizedAddress(txDefaults.from).awaitTransactionSuccessAsync(txDefaults);
await stakingDel.addExchangeAddress(exchange.address).awaitTransactionSuccessAsync(txDefaults);
await exchange.setProtocolFeeCollectorAddress(stakingProxy.address).awaitTransactionSuccessAsync(txDefaults);
await exchange.setProtocolFeeMultiplier(new BigNumber(150000)).awaitTransactionSuccessAsync(txDefaults);
await exchange.setProtocolFeeMultiplier(new BigNumber(70000)).awaitTransactionSuccessAsync(txDefaults);
await zrxVault.addAuthorizedAddress(txDefaults.from).awaitTransactionSuccessAsync(txDefaults);
await zrxVault.setStakingProxy(stakingProxy.address).awaitTransactionSuccessAsync(txDefaults);