Move executed_surplus_fee

[sunce86]

Description

OrderClass::Limit variant contained executed_surplus_fee field just for the needs of the API so the fee could be displayed to the users.

I moved the executed_surplus_fee to parent struct since there is no good reason to keep it as part of the OrderClass, on the contrary, we have a lot of places where we use the order class but omit the value of executed_surplus_fee as irrelevant.

[fleupold]

How does this change affect existing orders in the database?

In order to make coordination with the frontend changes easier, I wonder if a migration plan could be:

1. Populate executed_fee_amount for all order types (while keeping executed_surplus_fee for limit orders)
2. Change frontend to read executed_fee_amount for all orders
3. Drop executed_surplus_fee from the model

[sunce86]

How does this change affect existing orders in the database?

In order to make coordination with the frontend changes easier, I wonder if a migration plan could be:

1. Populate executed_fee_amount for all order types (while keeping executed_surplus_fee for limit orders)
2. Change frontend to read executed_fee_amount for all orders
3. Drop executed_surplus_fee from the model

Yes, that would be the way to go. Will prepare the PR for (1).

[MartinquaXD]

The change makes sense to me when executed in coordination with the frontend.

[sunce86]

Updated the PR description and the code. Now the change is just to move the executed_surplus_fee to the parent struct.

[fleupold]

Looks alright to me. However, if we introduce a new field this change needs to be tightly coordinated with the frontend team, doesn't it? Otherwise displaying the fee in the fronted will break. My understanding was that we would start exposing surplus fee as part of executed fee amount (the two values are already summed up iirc)

Can you test that change locally and show how a limit order fee would look like in the explorer?

[fleupold]

If this is only used for the UI, can we not add it to the existing executed_fee_amount ?

[sunce86]

if we introduce a new field this change needs to be tightly coordinated with the frontend team

This PR does not include new field, it just moves the filed from one struct to it's parent struct, but the serialization for the API is the same (as the unit test deserialization_and_back proves).

If this is only used for the UI, can we not add it to the existing executed_fee_amount

I don't know. I've asked the frontend team this several times, but never got a clear yes/no answer, so I dropped the idea for now and decided to just refactor the backend side without changing the API.