Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

asset-swapper: DFB support + refactors #2536

Merged

Conversation

dorothy-zbornak
Copy link
Contributor

@dorothy-zbornak dorothy-zbornak commented Apr 1, 2020

Description

This adds DexForwarderBridge support AS. When creating the orders, we batch contiguous bridge paths into a single DFB order, so it only pays a single protocol fee. This functionality can be disabled with the shouldBatchBridgeOrders SwapQuoter option.

I've also massively refactored swap_quote_calculator.ts. I created some new functions in quote_simulation.ts whose responsibility is to simulate filling the quote. These first transform the quote orders into a side-agnostic format to remove the need for all the nasty branching logic. So instead of filling a straight maker -> taker asset order, it ends up filling input -> output orders.

We also now have best-case gas estimation and best-case source composition (i.e., what sources were actually used). So now bestCaseQuoteInfo.gas <= worstCaseQuoteInfo.gas, where they were previously equivalent. Also sourceBreakdown now only includes the sources consumed in the best case, so no fallback orders.

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 dorothy-zbornak force-pushed the feat/asset-swapper/DexForwarderBridge-support branch 3 times, most recently from 4b98f93 to e0e8546 Compare April 1, 2020 20:20
@@ -127,7 +127,7 @@ export interface OptimizedMarketOrder extends SignedOrderWithFillableAmounts {
/**
* The optimized fills that generated this order.
*/
fill: CollapsedFill;
fills: CollapsedFill[];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So now orders have an array of collapsed fills instead of a single one. Fun.

@dorothy-zbornak dorothy-zbornak force-pushed the feat/asset-swapper/DexForwarderBridge-support branch 2 times, most recently from df69b02 to c4dd4d8 Compare April 1, 2020 21:24
@dorothy-zbornak dorothy-zbornak changed the base branch from development to feat/rip-instant April 1, 2020 21:24
@dorothy-zbornak dorothy-zbornak force-pushed the feat/asset-swapper/DexForwarderBridge-support branch 2 times, most recently from 2905768 to ff75918 Compare April 3, 2020 05:27
@dorothy-zbornak dorothy-zbornak marked this pull request as ready for review April 3, 2020 06:23
@dorothy-zbornak dorothy-zbornak changed the base branch from feat/rip-instant to development April 3, 2020 06:24
Comment on lines +233 to +235
makerAssetAmount: fr.totalMakerAssetAmount,
takerAssetAmount: fr.takerAssetAmount,
totalTakerAssetAmount: fr.totalTakerAssetAmount,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A consequence of this new approach is that both the maker (in buys) and taker (in sells) asset amounts can theoretically deviate from the input amount slightly when fees are involved. In practice they're probably almost always the same though. So if we want to reference a fixed buy or sell amount in the API, we should be using the quote.makerAssetAmount or quote.takerAssetFillAmount, not the best or worst case values. Just something to note.

@dorothy-zbornak dorothy-zbornak requested a review from dekz April 3, 2020 06:35
@coveralls
Copy link

coveralls commented Apr 3, 2020

Coverage Status

Coverage decreased (-0.1%) to 79.462% when pulling c541340 on feat/asset-swapper/DexForwarderBridge-support into d9be78c on development.

@dorothy-zbornak dorothy-zbornak force-pushed the feat/asset-swapper/DexForwarderBridge-support branch from e38f768 to c6379ca Compare April 7, 2020 16:36
`@0x/contract-addresses`: Redeploy `DexForwarderBridge` on Mainnet with Gas Token freeing
`@0x/contract-addresses`: Revert to older Curve Bridge (without Gas Tokens)
@dorothy-zbornak dorothy-zbornak merged commit a80d1f8 into development Apr 7, 2020
@dorothy-zbornak dorothy-zbornak deleted the feat/asset-swapper/DexForwarderBridge-support branch April 7, 2020 18:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants