Merged
Conversation
Collaborator
Builds ready [3c07786]
Page Load Metrics (423 ± 55 ms)
|
3c07786 to
6efd36f
Compare
Collaborator
Builds ready [6efd36f]
Page Load Metrics (399 ± 52 ms)
|
danjm
reviewed
Oct 16, 2020
6efd36f to
bb610df
Compare
Collaborator
Builds ready [bb610df]
Page Load Metrics (480 ± 63 ms)
|
bb610df to
fa3f6c6
Compare
Collaborator
Builds ready [a028130]
Page Load Metrics (493 ± 49 ms)
|
Collaborator
Builds ready [2d93cd5]
Page Load Metrics (383 ± 52 ms)
|
danjm
reviewed
Oct 19, 2020
danjm
reviewed
Oct 19, 2020
| 10, | ||
| ) | ||
| .minus(tokenConversionRate ? ethFee.toString(10) : 0, 10) | ||
| .minus(tokenConversionRate ? totalEthCost : 0, 10) |
Contributor
There was a problem hiding this comment.
Review note: this is functionally equivalent. What used to be called ethFee is now totalEthCost. (What is now ethFee is now different from totalEthCost when sourceToken === ETH_SWAPS_TOKEN_ADDRESS)
danjm
reviewed
Oct 19, 2020
| const tokenConversionRate = tokenConversionRates[destinationToken] | ||
| const ethValueOfTrade = | ||
| destinationTokenInfo.symbol === 'ETH' | ||
| ? calcTokenAmount(destinationAmount, 18).minus(ethFee, 10) |
Contributor
There was a problem hiding this comment.
Review note: this is functionally equivalent. What used to be called ethFee is now totalEthCost. (What is now ethFee is now different from totalEthCost when sourceToken === ETH_SWAPS_TOKEN_ADDRESS)
Collaborator
Builds ready [37d1964]
Page Load Metrics (575 ± 81 ms)
|
danjm
previously approved these changes
Oct 19, 2020
Contributor
danjm
left a comment
There was a problem hiding this comment.
LGTM. I think we might want to hold off on merging until at least until we cut v8.1.2
Collaborator
Builds ready [bdc96fb]
Page Load Metrics (443 ± 60 ms)
|
brad-decker
approved these changes
Oct 19, 2020
Contributor
brad-decker
left a comment
There was a problem hiding this comment.
Reviewed live with @rekmarks looks good to me!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds savings calculations for swaps, in the swaps controller.
The function previously named
_findTopQuoteAggIdhas been renamed_findTopQuoteAndCalculateSavings. This function already made the calculations to find the best quote by overall ETH value.With very little added work, we now also calculate the overall swap fees in ETH, which include gas costs and all exchange fees except our own, which proportionally identical across all quotes and already subtracted by the time the quotes are received.
Finally, we calculate the savings for the best quote. In the UI, we will display these savings along with our fee. We calculate savings relative to the median fee and the median trade value for all quotes, compared to that of the best quote.
Here are the equations in pseudocode, assuming all units converted to ETH:
We say
total_eth_cost, because it does not include the value of the source token unless the source token is ETH.swap_tx_eth_sentincludes third-party fees and, if the source token is ETH, the amount of ETH swapped.For each quote, we store
trade_feeandtrade_valuefor use in subsequent calculations.We want the trade value to be as high as possible, so we subtract the median from the best value.
We want the trade fees to be as low as possible, so we subtract the fees of the best quote from the median fee of all quotes.
Note that either component of total_savings can be negative, and we often observe one negative and one positive in practice.