Skip to content

Add swaps savings calculation#9611

Merged
rekmarks merged 8 commits intodevelopfrom
swaps-average-costs-calculation
Oct 19, 2020
Merged

Add swaps savings calculation#9611
rekmarks merged 8 commits intodevelopfrom
swaps-average-costs-calculation

Conversation

@rekmarks
Copy link
Copy Markdown
Member

@rekmarks rekmarks commented Oct 15, 2020

This PR adds savings calculations for swaps, in the swaps controller.

The function previously named _findTopQuoteAggId has 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:

total_eth_cost = swap_tx_gas_cost + approval_tx_gas_cost + swap_tx_eth_sent

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_sent includes third-party fees and, if the source token is ETH, the amount of ETH swapped.

trade_fee = total_eth_cost - (source_token_is_eth ? amount_of_eth_swapped : 0)

trade_value = destination_token_eth_value - total_eth_cost

For each quote, we store trade_fee and trade_value for use in subsequent calculations.

We want the trade value to be as high as possible, so we subtract the median from the best value.

performance_savings = best_quote_trade_value - median(all_trade_values)

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.

fee_savings = median(all_trade_fees) - best_quote_trade_fee

Note that either component of total_savings can be negative, and we often observe one negative and one positive in practice.

total_savings = performance_savings + fee_savings

@rekmarks rekmarks added the DO-NOT-MERGE Pull requests that should not be merged label Oct 15, 2020
@rekmarks rekmarks requested a review from danjm October 15, 2020 07:47
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [3c07786]
Page Load Metrics (423 ± 55 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint30523552
domContentLoaded30164142211455
load30364342311455
domInteractive30164142111455

@rekmarks rekmarks force-pushed the swaps-average-costs-calculation branch from 3c07786 to 6efd36f Compare October 15, 2020 17:22
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [6efd36f]
Page Load Metrics (399 ± 52 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint287636105
domContentLoaded29062739810952
load29162939910952
domInteractive28962739810952

@rekmarks rekmarks force-pushed the swaps-average-costs-calculation branch from 6efd36f to bb610df Compare October 18, 2020 04:22
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [bb610df]
Page Load Metrics (480 ± 63 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint349651199
domContentLoaded31679747913263
load31879948013263
domInteractive31679747813263

@rekmarks rekmarks force-pushed the swaps-average-costs-calculation branch from bb610df to fa3f6c6 Compare October 18, 2020 19:39
@rekmarks rekmarks changed the title Swaps average savings calculation Swaps savings calculation Oct 19, 2020
@rekmarks rekmarks removed the DO-NOT-MERGE Pull requests that should not be merged label Oct 19, 2020
@rekmarks rekmarks marked this pull request as ready for review October 19, 2020 04:54
@rekmarks rekmarks requested a review from a team as a code owner October 19, 2020 04:54
@rekmarks rekmarks requested a review from danjm October 19, 2020 05:21
@rekmarks rekmarks changed the title Swaps savings calculation Adds swaps savings calculation Oct 19, 2020
@rekmarks rekmarks changed the title Adds swaps savings calculation Add swaps savings calculation Oct 19, 2020
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [a028130]
Page Load Metrics (493 ± 49 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint35122582412
domContentLoaded34780249110350
load34880249310349
domInteractive34680149110350

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [2d93cd5]
Page Load Metrics (383 ± 52 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint27423352
domContentLoaded28458738110852
load28558838310852
domInteractive28458738110852

10,
)
.minus(tokenConversionRate ? ethFee.toString(10) : 0, 10)
.minus(tokenConversionRate ? totalEthCost : 0, 10)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)

const tokenConversionRate = tokenConversionRates[destinationToken]
const ethValueOfTrade =
destinationTokenInfo.symbol === 'ETH'
? calcTokenAmount(destinationAmount, 18).minus(ethFee, 10)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [37d1964]
Page Load Metrics (575 ± 81 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaintNaNNaNNaNNaNNaN
domContentLoaded39191157216881
load39291257516881
domInteractive39191157216881

danjm
danjm previously approved these changes Oct 19, 2020
Copy link
Copy Markdown
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

LGTM. I think we might want to hold off on merging until at least until we cut v8.1.2

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [bdc96fb]
Page Load Metrics (443 ± 60 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint2910440178
domContentLoaded30372644112660
load30572744312560
domInteractive30372644112660

Copy link
Copy Markdown
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

Reviewed live with @rekmarks looks good to me!

@rekmarks rekmarks merged commit 7de7e7d into develop Oct 19, 2020
@rekmarks rekmarks deleted the swaps-average-costs-calculation branch October 19, 2020 21:52
@github-actions github-actions bot locked and limited conversation to collaborators Oct 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants