search

cowprotocol / cow-sdk

CoW protocol SDK
https://docs.cow.fi/cow-protocol/reference/sdks/cow-sdk
Other
31 stars 9 forks source link

feat: update totalFee calculation #197

Closed alfetopito closed 8 months ago

alfetopito commented 8 months ago

Summary

As per this internal discussion, the backend updated how the fees are returned by the api.

In this PR the calculation is updated to return the sum of executedSurplusFee and executedFeeAmount.

Context

Take as example this order: https://explorer.cow.fi/orders/0xdedb65b51a6f81493f8e6e1d811b56a57297dbb6fb11716d85e646a66a5d041f85cc3c8da85612013acabe9b5d954d578860b3c165a22c43?tab=overview

Currently it shows as 0 fee, even though executedFeeAmount is set.

The issue is that this order has both fields executedSurplusFee and executedFeeAmount returned, but executedSurplusFee is set to 0. In the current implementation, it takes precedence over executedFeeAmount.

https://github.com/cowprotocol/cow-sdk/blob/3abfdcfd1ab223394e43d647de07a88d06f0b30d/src/order-book/transformOrder.ts#L28

This logic was introduced over 1y ago and according to this comment from Nick:

tl;dr: the “real” fee is executedSurplusFee ?? executedFeeAmount, and not (executedSurplusFee ?? 0) + executedFeeAmount.

According to Dusan:

... probably caused by this change: https://github.com/cowprotocol/services/pull/2103 We extracted the executed_surplus_fee out of the parent struct which caused it to be always serialized (even for market orders). Considering we will probably deliver protocol fees (which are expressed through executed_surplus_fee ) before we completely remove market orders (orders with signed fee_amount), and in that case we will have orders that have both executed_fee_amount and executed_surplus_fee different from zero => I would go with summarizing the values for UI.

Test

Unit tests

coveralls commented 8 months ago

Coverage Status

coverage: 78.995% (+0.1%) from 78.899% when pulling cac5a4dc0a6e6639f92b3f72df59f8b60d636d8b on update-totalFee-calculation into 755a01b1a96778c080d3eda8832d756d27d3110f on main.