Asset-swapper: Fallback orders + refactors #2513

dorothy-zbornak · 2020-03-06T07:51:48Z

Description

Generate fallback orders

If the allowFallback option is passed into SwapQuoter, fallback orders can be generated. How this works is we first generate an optimized quote. Then if this quote includes native orders, we will generate a second quote for the same size as the native orders and append it to the original quote. This fallback quote will not reuse any sources in the original quote.

We do, however, allow Kyber conflicts in the fallback quote because, even though in theory this should reduce reverts, in reality it increases reverts because with conflicting sources omitted we very often cannot find an fallback path. Even with conflicts, early simulations (w/ bridgeSliipage = 1%) never seem to fail on these quotes, so I think we good?

Gas estimates

Because eth_estimateGas will always fail to correctly anticipate the gas cost of fallback orders, AS now attempts to provide a gas estimate in the bestCaseQuoteInfo and worstCaseQuoteInfo fields. I'm lazy so they're both the same right now but in the future we can get more clever with the best case.

New(ish) optimizer algorithm

I tweaked the algorithm and cleaned it up a lot. We still try to explore as many fill combinations as we can (we have to because fees make price curves non-convex). The algo should also run a bit faster now thanks to Remco's suggestion of iteratively solving two paths at a time, rather than all at once. This is important because we now essentially run it twice in order to generate the fallback orders.

Massive refactors to `market_operation_utils`

Yeah, that was turning into a hot mess. All the code in that directory are much nicer and simpler now.

Changes to swap quote options

Removed:

slippagePercentage: The fallback orders essentially double the possible fill amount already.
dustFractionThreshold: No longer necessary because the optimizer can handle a larger # of paths.
noConflicts: We never set this to false so it's now always on.

Added:

allowFallback: If true (default), allows the generation of fallback orders
gasSchedule: Estimated gas usage of filling a liquidity source-- similar to feeSchedule but expressed in gas instead of ETH.
maxFallbackSlippage: The maximum rate % change allowed in the fallback (compared to the optimal quote). If the fallback's slippage exceeds this %, no fallback will be provided.

Renamed:

fees -> feeSchedule

Ordering of orders in quote

Because of how the new optimizer works, there is no longer a strong guarantee that orders will be sorted by decreasing rate. If you relied on this, sorry boo.

Bugs

I found a small bug related to how DEX paths were generated, which may have led to slightly worse pricing.
Because the code is so different, I went ahead and addressed [Asset-swapper] Fix bug with batch market buy #2517 in here as well.

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.

dekz

Taken a quick peek but wanted to discuss some things to get the clear in my mind.

I feel like there are 2 approaches to the goal of lowering reverts with a backup path.

Find the best path using on-chain sources, use 0x to improve price if available. So we have a backup to 0x failing.
Find the best path, create another backup path incase the best path fails. This feels a bit difficult to reason about. It could be 100% Uniswap, then 100% 0x as a backup (previously this would be just Uniswap, so little to be gained in this scenario). It could also be 100% Uniswap then 100% Kyber as a backup, which could be a conflict. It can also be the inverse of 100% Kyber with 100% Uniswap backup, also a potential conflict.

I am biased to the 1) approach as that seems simple, but keen to hear your thoughts. In my head 0x should never be the backup as I find it difficult to reason the case where Uniswap would fail and 0x orders would still be fillable.

When a path includes Curve, and a backup path includes another Curve, this is now 2million+ faux gas estimate as a "just in case".

packages/asset-swapper/src/constants.ts

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

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

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

dorothy-zbornak · 2020-03-09T18:29:57Z

Taken a quick peek but wanted to discuss some things to get the clear in my mind.

I feel like there are 2 approaches to the goal of lowering reverts with a backup path.

Find the best path using on-chain sources, use 0x to improve price if available. So we have a backup to 0x failing.

Find the best path, create another backup path incase the best path fails. This feels a bit difficult to reason about. It could be 100% Uniswap, then 100% 0x as a backup (previously this would be just Uniswap, so little to be gained in this scenario). It could also be 100% Uniswap then 100% Kyber as a backup, which could be a conflict. It can also be the inverse of 100% Kyber with 100% Uniswap backup, also a potential conflict.

I am biased to the 1) approach as that seems simple, but keen to hear your thoughts. In my head 0x should never be the backup as I find it difficult to reason the case where Uniswap would fail and 0x orders would still be fillable.

When a path includes Curve, and a backup path includes another Curve, this is now 2million+ faux gas estimate as a "just in case".

What do you think of:

Find the optimal quote.
If the optimal quote contains a 0x order:
1. Generate a fallback quote with the unused sources (+ factoring in Kyber conflicts).

The separate quotes thing is to ensure the optimizer is solving for as close to the actual fill amount as possible, which gives us better pricing.

`@0x/asset-swapper`: add fallback orders. `@0x/asset-swapper`: Remove `noConflicts` and `dustThreshold` options. `@0x/asset-swapper`: Add `allowFallback` option.

`@0x/asset-swapper`: Make native fills one single path. `@0x/asset-swapper`: Redo the optimizer algo again to be more thorough. `@0x/asset-swapper`: Make `getMedianSellRate()` return `1` if maker token == taker token.

…imal path. `@0x/asset-swapper`: Exclude conflicting sources across both optimal and fallback paths.

`@0x/asset-swapper`: Rename `fees` `SwapQuoter` option to `feeSchedule`.

`@0x/asset-swapper`: Allow Kyber conflicts in fallback path.

dekz

Want to spend some more time tomorrow just going through the market operations utils deeper. But this is looking good.

dekz · 2020-03-10T09:39:55Z

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

+    /**
+     * Estimated gas consumed by each liquidity source.
+     */
+    gasSchedule: { [source: string]: number };


Thoughts on removing the feeSchedule and just adding 150e3 to the gasSchedule where feeSchedule is used?

I could go either way. It's probably not good design for AS to assume the protocol fee, even though it already does in a few places (we should remove these).

It's also not particularly hard for the caller to do the operation you described theirself.

packages/instant/src/util/swap_quote_updater.ts

packages/instant/CHANGELOG.json

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

…bug. `@0x/asset-swapper`: Add `maxFallbackSlippage` option.

dekz

Couple of little things but this looks good. Going to simbot now and think more on what this looks like with gasLimit and if we'd need any adjustment. But worse case we wouldn't be introducing more reverts, just transferring the reported revert as OOG.

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

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

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

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

dorothy-zbornak force-pushed the feat/asset-swapper/death-to-reverts branch from 53af414 to 4b54365 Compare March 6, 2020 07:54

dorothy-zbornak changed the title ~~Asset-swapper: Fallback orders~~ Asset-swapper: Fallback orders + refactors Mar 6, 2020

dorothy-zbornak force-pushed the feat/asset-swapper/death-to-reverts branch from 4b54365 to 1f04198 Compare March 6, 2020 23:40

dorothy-zbornak marked this pull request as ready for review March 6, 2020 23:56

dorothy-zbornak requested review from BMillman19, dave4506 and fragosti as code owners March 6, 2020 23:56

dorothy-zbornak force-pushed the feat/asset-swapper/death-to-reverts branch from 66304b5 to 3d4cd22 Compare March 9, 2020 05:53

dekz reviewed Mar 9, 2020

View reviewed changes

dekz mentioned this pull request Mar 9, 2020

[Asset-swapper] Fix bug with batch market buy #2517

Closed

4 tasks

merklejerk added 7 commits March 9, 2020 21:44

@0x/asset-swapper: Refactor market op utils.

6ccadcb

`@0x/asset-swapper`: add fallback orders. `@0x/asset-swapper`: Remove `noConflicts` and `dustThreshold` options. `@0x/asset-swapper`: Add `allowFallback` option.

rebase

f901c40

@0x/asset-swapper: Rebase and fix some minor bugs.

d0805d4

@0x/asset-swapper: Reintroduce runLimit option.

8179a96

`@0x/asset-swapper`: Make native fills one single path. `@0x/asset-swapper`: Redo the optimizer algo again to be more thorough. `@0x/asset-swapper`: Make `getMedianSellRate()` return `1` if maker token == taker token.

@0x/asset-swapper: Clean up median price calls.

95bebd6

@0x/instant: Fix for changed asset-swapper configs.

847a7f4

@0x/asset-swapper: Only generate fallbacks for native orders in opt…

cc12ad8

…imal path. `@0x/asset-swapper`: Exclude conflicting sources across both optimal and fallback paths.

dorothy-zbornak force-pushed the feat/asset-swapper/death-to-reverts branch from 1c6ca0c to cc12ad8 Compare March 10, 2020 01:44

@0x/asset-swapper: Add gasSchedule option to SwapQuoter.

05bf55d

`@0x/asset-swapper`: Rename `fees` `SwapQuoter` option to `feeSchedule`.

dorothy-zbornak force-pushed the feat/asset-swapper/death-to-reverts branch from 21e1c53 to 05bf55d Compare March 10, 2020 02:33

@0x/asset-swapper: Clean up source breakdown code a bit.

0e7a473

`@0x/asset-swapper`: Allow Kyber conflicts in fallback path.

dekz reviewed Mar 10, 2020

View reviewed changes

@0x/instant: Fix changelog.

5fd767b

dorothy-zbornak force-pushed the feat/asset-swapper/death-to-reverts branch from 004008e to 9f2e6b3 Compare March 10, 2020 18:10

@0x/asset-swapper: Fix getPathSize() and getAdjustedPathSize() …

37597ec

…bug. `@0x/asset-swapper`: Add `maxFallbackSlippage` option.

dorothy-zbornak force-pushed the feat/asset-swapper/death-to-reverts branch from 9f2e6b3 to 37597ec Compare March 10, 2020 18:25

@0x/asset-swapper: Fix double bridgeSlippage on generated orders.

109d466

dekz self-requested a review March 11, 2020 03:55

@0x/asset-swapper: Sort native path fills by ADJUSTED rate.

87999f4

dekz approved these changes Mar 11, 2020

View reviewed changes

@0x/asset-swapper: Address review comments.

1a9063a

dorothy-zbornak force-pushed the feat/asset-swapper/death-to-reverts branch from c82fd94 to 1a9063a Compare March 11, 2020 16:24

dorothy-zbornak merged commit baf6372 into development Mar 12, 2020

dorothy-zbornak deleted the feat/asset-swapper/death-to-reverts branch March 12, 2020 22:45

PirosB3 mentioned this pull request Jul 3, 2020

fix: Changes latency to be capped at 600ms, added some extra typing 0xProject/0x-api#275

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Asset-swapper: Fallback orders + refactors #2513

Asset-swapper: Fallback orders + refactors #2513

dorothy-zbornak commented Mar 6, 2020 •

edited

Loading

dekz left a comment •

edited

Loading

dorothy-zbornak commented Mar 9, 2020 •

edited

Loading

dekz left a comment

dekz Mar 10, 2020

dorothy-zbornak Mar 10, 2020

dorothy-zbornak Mar 10, 2020

dekz left a comment

Asset-swapper: Fallback orders + refactors #2513

Asset-swapper: Fallback orders + refactors #2513

Conversation

dorothy-zbornak commented Mar 6, 2020 • edited Loading

Description

Generate fallback orders

Gas estimates

New(ish) optimizer algorithm

Massive refactors to market_operation_utils

Changes to swap quote options

Ordering of orders in quote

Bugs

Testing instructions

Types of changes

Checklist:

dekz left a comment • edited Loading

Choose a reason for hiding this comment

dorothy-zbornak commented Mar 9, 2020 • edited Loading

dekz left a comment

Choose a reason for hiding this comment

dekz Mar 10, 2020

Choose a reason for hiding this comment

dorothy-zbornak Mar 10, 2020

Choose a reason for hiding this comment

dorothy-zbornak Mar 10, 2020

Choose a reason for hiding this comment

dekz left a comment

Choose a reason for hiding this comment

dorothy-zbornak commented Mar 6, 2020 •

edited

Loading

Massive refactors to `market_operation_utils`

dekz left a comment •

edited

Loading

dorothy-zbornak commented Mar 9, 2020 •

edited

Loading