feat: update totalFee calculation

[alfetopito]

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.

cow-sdk/src/order-book/transformOrder.ts

Line 28 in 3abfdcf

const totalFee = executedSurplusFee ?? executedFeeAmount

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: cowprotocol/services#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]

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

[alfetopito]

Using native BigInt to avoid adding a new dependency just for this.

Or should we?

[anxolin]

this solution looks good.

why the underscore in the consts?

[alfetopito]

Because those names already exist in the scope.
Could call then name+BigInt but I didn't find it a good choice.
Suggestions?

[anxolin]

okok, not big deal. works for me. Also the BigInt works too

[anxolin]

nit: Why asserting the transformedOrder totalFee matches the rawOrder executedFeeAmount?
I would find more readable that given the above, the result is '1' (setting the actual value, instead of picking the property name)

[anxolin]

Suggested change

	expect(transformedOrder.totalFee).toEqual(rawOrder.executedFeeAmount)
	expect(transformedOrder.totalFee).toEqual('1') // 1 + 0

[alfetopito]

It's meant to be: it's the unaltered value coming from the original order obj.
I feel like that's a better way to ready it than using an actual value.

But sure, will update it.

[anxolin]

same comment

[anxolin]

this solution looks good.

why the underscore in the consts?

[anxolin]

just some nits