Fix fees shown for send chain swaps #641
Conversation
| // overpayments, we use the actual claim value as the final received amount | ||
| // for fee calculation. | ||
| PaymentType::Receive => s.payer_amount_sat - tx.amount_sat, | ||
| PaymentType::Send => s.payer_amount_sat - s.receiver_amount_sat, |
There was a problem hiding this comment.
Should we take the (tx.amount_sat + tx.feez_sat) instead of payer_amount_sat in the case of Send? Or you think the current approach is cleaner and more clear? I am tending to think the current code is clearer, just looking for another opinion.
There was a problem hiding this comment.
The tx fees_sat would only cover the lockup fee, not the boltz fees also
There was a problem hiding this comment.
I think the amount_sat is also incorrect in the send cases. It should reflect the amount received and the fees_sat the total fees to send that amount
There was a problem hiding this comment.
@dangeross In the case of send isn't the tx is the lockup tx? In that case isn't the tx.amount_sat + tx.fee_sat is the total amount sent out of the wallet (e.g payer amount)? The boltz fees should be part of the amount_sat IIUC.
There was a problem hiding this comment.
Should we take the
(tx.amount_sat + tx.feez_sat)instead ofpayer_amount_satin the case ofSend? Or you think the current approach is cleaner and more clear? I am tending to think the current code is clearer, just looking for another opinion.
@roeierez The values of tx.amount_sat and s.payer_amount_sat are the same (the tx.amount_sat includes the fees IIUC), so adding the tx fees on top would be incorrect. But yes, we could use tx.amount_sat instead of s.payer_amount_sat. I don't have a strong preference. We can keep it like this.
I think the amount_sat is also incorrect in the send cases. It should reflect the amount received and the fees_sat the total fees to send that amount
@dangeross I was thinking about this, and it depends on how we define the amount_sat field. Is it the amount a user decided to receive/send? Or the amount that a user actually receives/spends? Because of fees, for sending, these options are different. Our current approach is the latter. We can discuss if we want to change to the former. Regardless, as long as the meaning of the field is clear to the SDK integrators, it doesn't make any difference, as they can easily calculate the alternative value.
There was a problem hiding this comment.
@danielgranhao I am not sure tx.amount_sat (in the case of sending) contains the onchain fees for the lockup transaction which actually is expressed in the fee_sat.
So IIUC the total amount sent out of the user wallet (e.g payer_amount_sat) should be tx.amount_sat (which includes the tx output amount and the boltz service fees) plus the fee_sat which is the tx lockup onchain fees.
Nevertheless I think the current approach is clean and IMO we can leave it as is.
No description provided.