From be5fd42ea5d2db6b9f4f815376fab79bb79bbd52 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Mon, 27 Jul 2020 18:03:38 +1000 Subject: [PATCH] refactor and accumulate sources over buckets --- src/services/swap_service.ts | 8 +-- src/utils/market_depth_utils.ts | 43 ++++++++++----- test/market_depth_test.ts | 98 ++++++++++++++++++++++++++++----- 3 files changed, 118 insertions(+), 31 deletions(-) diff --git a/src/services/swap_service.ts b/src/services/swap_service.ts index 4f3710eb7..2b65fbafe 100644 --- a/src/services/swap_service.ts +++ b/src/services/swap_service.ts @@ -276,8 +276,8 @@ export class SwapService { public async calculateMarketDepthAsync( params: CalaculateMarketDepthParams, ): Promise<{ - asks: { dataByBucketPrice: BucketedPriceDepth[] }; - bids: { dataByBucketPrice: BucketedPriceDepth[] }; + asks: { depth: BucketedPriceDepth[] }; + bids: { depth: BucketedPriceDepth[] }; }> { const { buyToken, sellToken, sellAmount, numSamples, sampleDistributionBase, excludedSources } = params; const marketDepth = await this._swapQuoter.getBidAskLiquidityForMakerTakerAssetPairAsync( @@ -318,10 +318,10 @@ export class SwapService { return { // We're buying buyToken and SELLING sellToken (DAI) (50k) // Price goes from HIGH to LOW - asks: { dataByBucketPrice: askDepth }, + asks: { depth: askDepth }, // We're BUYING sellToken (DAI) (50k) and selling buyToken // Price goes from LOW to HIGH - bids: { dataByBucketPrice: bidDepth }, + bids: { depth: bidDepth }, }; } diff --git a/src/utils/market_depth_utils.ts b/src/utils/market_depth_utils.ts index 9e4d274d1..f639df448 100644 --- a/src/utils/market_depth_utils.ts +++ b/src/utils/market_depth_utils.ts @@ -141,7 +141,7 @@ export const marketDepthUtils = { }, distributeSamplesToBuckets: (depthSide: MarketDepthSide, buckets: BigNumber[], side: MarketOperation) => { - const allocatedBuckets = buckets.map((b, i) => ({ price: b, bucket: i, bucketTotal: ZERO })); + const allocatedBuckets = buckets.map((b, i) => ({ price: b, bucket: i, bucketTotal: ZERO, sources: {} })); const getBucketId = (price: BigNumber): number => { return buckets.findIndex(b => side === MarketOperation.Sell ? price.isGreaterThanOrEqualTo(b) : price.isLessThanOrEqualTo(b), @@ -185,29 +185,46 @@ export const marketDepthUtils = { continue; } const bucket = allocatedBuckets[bucketId]; - if (bucket[source]) { - bucket.bucketTotal = bucket.bucketTotal.minus(bucket[source]).plus(sample.output); - bucket[source] = sample.output; - } else { - bucket.bucketTotal = bucket.bucketTotal.plus(sample.output); - bucket[source] = sample.output; - } + // If two samples from the same source have landed in the same bucket, take the latter + bucket.bucketTotal = + bucket.sources[source] && !bucket.sources[source].isZero() + ? bucket.bucketTotal.minus(bucket.sources[source]).plus(sample.output) + : bucket.bucketTotal.plus(sample.output); + bucket.sources[source] = sample.output; } } let totalCumulative = ZERO; // Normalize the source names back and create a cumulative total - const normalizedCumulativeBuckets = allocatedBuckets.map(b => { + const normalizedCumulativeBuckets = allocatedBuckets.map((b, bucketIndex) => { totalCumulative = totalCumulative.plus(b.bucketTotal); - for (const key of Object.keys(b)) { + // Sum all of the various pools which fall under once source (Balancer, Uniswap, Curve...) + const findLastNonEmptyBucketResult = (source: ERC20BridgeSource) => { + let lastValue = ZERO; + for (let i = bucketIndex - 1; i >= 0; i--) { + const value = allocatedBuckets[i].sources[source]; + if (value && !value.isZero()) { + lastValue = value; + break; + } + } + return lastValue; + }; + for (const key of Object.keys(b.sources)) { const source = key.split(':')[0]; if (source !== key && Object.values(ERC20BridgeSource).includes(source as ERC20BridgeSource)) { // Curve:0xabcd,100 -> Curve,100 // Add Curve:0abcd to Curve - b[source] = b[source] ? b[source].plus(b[key]) : b[key]; - delete b[key]; + b.sources[source] = b.sources[source] ? b.sources[source].plus(b.sources[key]) : b.sources[key]; + delete b.sources[key]; } } - return { ...b, cumulative: totalCumulative }; + // Accumulate the sources from the previous bucket + for (const source of Object.values(ERC20BridgeSource)) { + // Add the previous bucket + const previousValue = findLastNonEmptyBucketResult(source); + b.sources[source] = previousValue.plus(b.sources[source] || ZERO); + } + return { ...b, cumulative: totalCumulative, sources: b.sources }; }); return normalizedCumulativeBuckets; }, diff --git a/test/market_depth_test.ts b/test/market_depth_test.ts index 28fdcb882..bc468cdac 100644 --- a/test/market_depth_test.ts +++ b/test/market_depth_test.ts @@ -160,27 +160,31 @@ describe(SUITE_NAME, () => { MarketOperation.Sell, ); const [first, second, third, fourth] = allocated; - expect(first[ERC20BridgeSource.Uniswap]).to.be.bignumber.eq(20); expect(first.cumulative).to.be.bignumber.eq(20); expect(first.bucketTotal).to.be.bignumber.eq(20); expect(first.bucket).to.be.eq(0); expect(first.price).to.be.bignumber.eq(10); + expect(first.sources[ERC20BridgeSource.Uniswap]).to.be.bignumber.eq(20); expect(second.cumulative).to.be.bignumber.eq(20); expect(second.bucketTotal).to.be.bignumber.eq(0); expect(second.bucket).to.be.eq(1); expect(second.price).to.be.bignumber.eq(8); + expect(second.sources[ERC20BridgeSource.Uniswap]).to.be.bignumber.eq(20); expect(third.cumulative).to.be.bignumber.eq(28); expect(third.bucketTotal).to.be.bignumber.eq(8); - expect(third[ERC20BridgeSource.Native]).to.be.bignumber.eq(8); expect(third.bucket).to.be.eq(2); expect(third.price).to.be.bignumber.eq(4); + expect(third.sources[ERC20BridgeSource.Native]).to.be.bignumber.eq(8); + expect(third.sources[ERC20BridgeSource.Uniswap]).to.be.bignumber.eq(20); expect(fourth.cumulative).to.be.bignumber.eq(28); expect(fourth.bucketTotal).to.be.bignumber.eq(0); expect(fourth.bucket).to.be.eq(3); expect(fourth.price).to.be.bignumber.eq(1); + expect(fourth.sources[ERC20BridgeSource.Native]).to.be.bignumber.eq(8); + expect(fourth.sources[ERC20BridgeSource.Uniswap]).to.be.bignumber.eq(20); }); it('does not allocate to a bucket if there is none available', async () => { const buckets = [B(10)]; @@ -195,6 +199,7 @@ describe(SUITE_NAME, () => { expect(first.bucketTotal).to.be.bignumber.eq(0); expect(first.bucket).to.be.eq(0); expect(first.price).to.be.bignumber.eq(10); + expect(first.sources[ERC20BridgeSource.Uniswap]).to.be.bignumber.eq(0); }); }); describe('buy', () => { @@ -211,17 +216,18 @@ describe(SUITE_NAME, () => { expect(first.bucket).to.be.eq(0); expect(first.price).to.be.bignumber.eq(1); - expect(second[ERC20BridgeSource.Native]).to.be.bignumber.eq(8); expect(second.cumulative).to.be.bignumber.eq(8); expect(second.bucketTotal).to.be.bignumber.eq(8); expect(second.bucket).to.be.eq(1); expect(second.price).to.be.bignumber.eq(4); + expect(second.sources[ERC20BridgeSource.Native]).to.be.bignumber.eq(8); - expect(third[ERC20BridgeSource.Uniswap]).to.be.bignumber.eq(20); expect(third.cumulative).to.be.bignumber.eq(28); expect(third.bucketTotal).to.be.bignumber.eq(20); expect(third.bucket).to.be.eq(2); expect(third.price).to.be.bignumber.eq(10); + expect(third.sources[ERC20BridgeSource.Uniswap]).to.be.bignumber.eq(20); + expect(third.sources[ERC20BridgeSource.Native]).to.be.bignumber.eq(8); }); }); }); @@ -246,14 +252,46 @@ describe(SUITE_NAME, () => { 1, // distribution 20, // max end perc ); + const emptySources = {}; + Object.values(ERC20BridgeSource).forEach(s => (emptySources[s] = ZERO)); expect(result).to.be.deep.eq([ - { price: B(10), bucket: 0, bucketTotal: B(10), cumulative: B(10), [ERC20BridgeSource.Uniswap]: B(10) }, - { price: B(9.5), bucket: 1, bucketTotal: ZERO, cumulative: B(10) }, - { price: B(9), bucket: 2, bucketTotal: ZERO, cumulative: B(10) }, - { price: B(8.5), bucket: 3, bucketTotal: ZERO, cumulative: B(10) }, + { + price: B(10), + bucket: 0, + bucketTotal: B(10), + cumulative: B(10), + sources: { ...emptySources, [ERC20BridgeSource.Uniswap]: B(10) }, + }, + { + price: B(9.5), + bucket: 1, + bucketTotal: ZERO, + cumulative: B(10), + sources: { ...emptySources, [ERC20BridgeSource.Uniswap]: B(10) }, + }, + { + price: B(9), + bucket: 2, + bucketTotal: ZERO, + cumulative: B(10), + sources: { ...emptySources, [ERC20BridgeSource.Uniswap]: B(10) }, + }, + { + price: B(8.5), + bucket: 3, + bucketTotal: ZERO, + cumulative: B(10), + sources: { ...emptySources, [ERC20BridgeSource.Uniswap]: B(10) }, + }, // Native is the sample for the sample for 2 (16) (overriding thhe 1 sample), since we didn't sample for 10 it does // not contain the entire order - { price: B(8), bucket: 4, bucketTotal: B(16), cumulative: B(26), [ERC20BridgeSource.Native]: B(16) }, + { + price: B(8), + bucket: 4, + bucketTotal: B(16), + cumulative: B(26), + sources: { ...emptySources, [ERC20BridgeSource.Uniswap]: B(10), [ERC20BridgeSource.Native]: B(16) }, + }, ]); }); @@ -277,12 +315,44 @@ describe(SUITE_NAME, () => { 1, // distribution 20, // max end perc ); + const emptySources = {}; + Object.values(ERC20BridgeSource).forEach(s => (emptySources[s] = ZERO)); expect(result).to.be.deep.eq([ - { price: B(10), bucket: 0, bucketTotal: B(10), cumulative: B(10), [ERC20BridgeSource.Uniswap]: B(10) }, - { price: B(9.5), bucket: 1, bucketTotal: ZERO, cumulative: B(10) }, - { price: B(9), bucket: 2, bucketTotal: ZERO, cumulative: B(10) }, - { price: B(8.5), bucket: 3, bucketTotal: ZERO, cumulative: B(10) }, - { price: B(8), bucket: 4, bucketTotal: B(80), cumulative: B(90), [ERC20BridgeSource.Native]: B(80) }, + { + price: B(10), + bucket: 0, + bucketTotal: B(10), + cumulative: B(10), + sources: { ...emptySources, [ERC20BridgeSource.Uniswap]: B(10) }, + }, + { + price: B(9.5), + bucket: 1, + bucketTotal: ZERO, + cumulative: B(10), + sources: { ...emptySources, [ERC20BridgeSource.Uniswap]: B(10) }, + }, + { + price: B(9), + bucket: 2, + bucketTotal: ZERO, + cumulative: B(10), + sources: { ...emptySources, [ERC20BridgeSource.Uniswap]: B(10) }, + }, + { + price: B(8.5), + bucket: 3, + bucketTotal: ZERO, + cumulative: B(10), + sources: { ...emptySources, [ERC20BridgeSource.Uniswap]: B(10) }, + }, + { + price: B(8), + bucket: 4, + bucketTotal: B(80), + cumulative: B(90), + sources: { ...emptySources, [ERC20BridgeSource.Uniswap]: B(10), [ERC20BridgeSource.Native]: B(80) }, + }, ]); }); });