asset-swapper: Quicker path-finding #2640

dorothy-zbornak · 2020-07-21T18:04:50Z

Description

We can reduce the number of iterations we go through (runLimit) to find a decent path if we:

Pre-sort all the individual (unmixed) paths by descending adjusted rate
When mixing two paths, sort the fill nodes such that:
- Fill nodes are always contiguous (form a valid subpath)
- Subpaths (segments of the same source) are sorted by descending rate.
Compute the "rate" of a path based off targetInput (fill amount) instead of min(targetInput, pathSize).
- This favors paths that cover more of the total fill (not sure about this).
Increase the runLimit decay rate to 0.5 (prev 0.8). So we halve the number of iterations for each subsequent call to mixPaths().
Start the bestPath in mixPaths() off with the left path (the previous mixed path), instead of starting with none.

This makes it more likely that we walk the viable paths first, so we can cut runLimit down to something like 2**8 (previously 2**13).

This results in a much faster (2-3x) response time with small change in quotes:

Notes:

WETH/DAI/USDC sims buy AND sell
orange is this version w/ runLimit = 2**8
blue is original version w/ runLimit = 2**13

We can probably tweak runLimit to improve results. This should buy us a little breathing room until we can entirely replace the algorithm with something better.

Testing instructions

Types of changes

Checklist:

Prefix PR title with [WIP] if necessary.
Add tests to cover changes as needed.
Update documentation as needed.
Add new entries to the relevant CHANGELOG.jsons.

dorothy-zbornak · 2020-07-21T19:48:00Z

packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts

-    let bestPath: Fill[] = [];
-    let bestPathInput = ZERO_AMOUNT;
-    let bestPathRate = ZERO_AMOUNT;
+    const _maxSteps = Math.max(maxSteps, 16);


Also cap the minimum so everyone gets some action.

dorothy-zbornak · 2020-07-21T21:24:28Z

packages/asset-swapper/src/utils/market_operation_utils/types.ts

+    // Unique ID of the original source path this fill belongs to.
+    // This is generated when the path is generated and is useful to distinguish
+    // paths that have the same `source` IDs but are distinct (e.g., Curves).
+    sourcePathId: string;


New field to tell consolidated sources apart.

moodlezoup

overall approach look good to me

moodlezoup · 2020-07-21T21:07:53Z

packages/asset-swapper/src/utils/market_operation_utils/fills.ts

        return ZERO_AMOUNT;
    }
-    return side === MarketOperation.Sell ? output.div(input) : input.div(output);
+    return side === MarketOperation.Sell ? output.div(targetInput) : targetInput.div(output);


hm yeah idk about this –– have you AB tested this change alone?

7001: Using targetInput as input (this PR)
7002: Using input as input (original recipe)

🤷 whatever wins I guess

moodlezoup · 2020-07-21T22:02:30Z

packages/asset-swapper/src/utils/market_operation_utils/fills.ts

@@ -112,6 +114,7 @@ function dexQuotesToPaths(
    ethToOutputRate: BigNumber,
    fees: FeeSchedule,
 ): Fill[][] {
+    const sourcePathId = hexUtils.random();


this should be inside the for loop I think

~~I want it to be unique to the whole path. So we can tell which fill belongs to which curve.~~

nvm ur rite

yay that fixed my sims

packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts

coveralls · 2020-07-23T02:41:53Z

Coverage remained the same at 79.598% when pulling ec1006a on exp/asset-swapper/path-finding-vroom-vroom into 2d29014 on feat/btc-curves.

moodlezoup · 2020-07-23T04:17:51Z

packages/asset-swapper/test/market_operation_utils_test.ts

                    [ERC20BridgeSource.Native],
-                    [ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Curve],
+                    [ERC20BridgeSource.Eth2Dai],


wait what's going on here?

Yeah it's a little funky that Curve comes right after Uniswap now, but both ways are technically valid so I'm like w/e 🤷

welcome to asset-swapper lol

moodlezoup · 2020-07-23T04:37:17Z

packages/asset-swapper/test/market_operation_utils_test.ts

                    [ERC20BridgeSource.Native],
-                    [ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Curve],
+                    [ERC20BridgeSource.Eth2Dai],


…vroom

dorothy-zbornak changed the title ~~opt~~ asset-swapper: Quicker path-finding Jul 21, 2020

dorothy-zbornak force-pushed the exp/asset-swapper/path-finding-vroom-vroom branch 2 times, most recently from 149a57a to 3b6b660 Compare July 21, 2020 19:35

dorothy-zbornak commented Jul 21, 2020

View reviewed changes

dorothy-zbornak force-pushed the exp/asset-swapper/path-finding-vroom-vroom branch from 3b6b660 to 51ce376 Compare July 21, 2020 21:18

dorothy-zbornak commented Jul 21, 2020

View reviewed changes

dorothy-zbornak force-pushed the exp/asset-swapper/path-finding-vroom-vroom branch from 51ce376 to e3526e2 Compare July 21, 2020 21:44

moodlezoup reviewed Jul 21, 2020

View reviewed changes

dorothy-zbornak commented Jul 22, 2020

View reviewed changes

packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts Outdated Show resolved Hide resolved

dorothy-zbornak force-pushed the exp/asset-swapper/path-finding-vroom-vroom branch 2 times, most recently from b5b688e to 43a7d59 Compare July 23, 2020 02:14

dorothy-zbornak marked this pull request as ready for review July 23, 2020 03:50

dorothy-zbornak requested review from BMillman19, dave4506 and fragosti as code owners July 23, 2020 03:50

dorothy-zbornak requested review from dekz and moodlezoup July 23, 2020 03:50

moodlezoup reviewed Jul 23, 2020

View reviewed changes

moodlezoup approved these changes Jul 23, 2020

View reviewed changes

dorothy-zbornak force-pushed the feat/btc-curves branch from bb64233 to 2d29014 Compare July 23, 2020 04:38

dorothy-zbornak requested review from abandeali1 and hysz as code owners July 23, 2020 04:38

dorothy-zbornak force-pushed the exp/asset-swapper/path-finding-vroom-vroom branch from ec54580 to ec1006a Compare July 23, 2020 04:39

Base automatically changed from feat/btc-curves to development July 23, 2020 20:43

dorothy-zbornak and others added 3 commits July 23, 2020 16:44

@0x/asset-swapper: Speed up path optimizer.

a40cbf9

@0x/asset-swapper: address my own review comment

9e354d2

@0x/asset-swapper: Update changelog

d071054

dorothy-zbornak force-pushed the exp/asset-swapper/path-finding-vroom-vroom branch from ec1006a to d071054 Compare July 23, 2020 20:46

Merge branch 'development' into exp/asset-swapper/path-finding-vroom-…

7830f94

…vroom

dekz merged commit ae2a6fb into development Jul 27, 2020

dekz deleted the exp/asset-swapper/path-finding-vroom-vroom branch July 27, 2020 05:09

dekz mentioned this pull request Jul 27, 2020

feat: Depth Chart API endpoint 0xProject/0x-api#290

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

asset-swapper: Quicker path-finding #2640

asset-swapper: Quicker path-finding #2640

dorothy-zbornak commented Jul 21, 2020 •

edited

Loading

dorothy-zbornak Jul 21, 2020

dorothy-zbornak Jul 21, 2020

moodlezoup left a comment

moodlezoup Jul 21, 2020

dorothy-zbornak Jul 22, 2020 •

edited

Loading

moodlezoup Jul 22, 2020

moodlezoup Jul 21, 2020

dorothy-zbornak Jul 21, 2020 •

edited

Loading

dorothy-zbornak Jul 21, 2020

dorothy-zbornak Jul 21, 2020

coveralls commented Jul 23, 2020 •

edited

Loading

moodlezoup Jul 23, 2020

dorothy-zbornak Jul 23, 2020

moodlezoup Jul 23, 2020

dorothy-zbornak Jul 23, 2020

moodlezoup Jul 23, 2020

asset-swapper: Quicker path-finding #2640

asset-swapper: Quicker path-finding #2640

Conversation

dorothy-zbornak commented Jul 21, 2020 • edited Loading

Description

Testing instructions

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moodlezoup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dorothy-zbornak Jul 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dorothy-zbornak Jul 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jul 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dorothy-zbornak commented Jul 21, 2020 •

edited

Loading

dorothy-zbornak Jul 22, 2020 •

edited

Loading

dorothy-zbornak Jul 21, 2020 •

edited

Loading

coveralls commented Jul 23, 2020 •

edited

Loading