Fix quote fee handling in market price #2572
Conversation
|
Driver tests should probably tell whether the logic is legit. |
| // 294580438010728) GNO for 9871580343970612988 DAI, then scaled to sell amount | ||
| // gives 10000000000000000000 GNO for 9871289555090525964 DAI | ||
| // 294580438010728 GNO fee. Equivalent to: 10000000000000000000 GNO for | ||
| // 9871580343970612988 * ((10000000000000000000 - 294580438010728) / |
There was a problem hiding this comment.
Note how we no longer add fee on top of sell amount, but instead, we now reduce the buy amount by converted amount.
| ) -> bool { | ||
| match kind { | ||
| OrderKind::Buy => { | ||
| order.sell.full_mul(quote.buy) < (quote.sell + quote.fee).full_mul(order.buy) |
There was a problem hiding this comment.
Buy order stayed the same as before, as we do need to add fee to sell amount in this case
| order | ||
| .sell | ||
| .full_mul(quote.buy - quote.fee * quote.buy / quote.sell) | ||
| < quote.sell.full_mul(order.buy) |
There was a problem hiding this comment.
For buy sell orders, we need to reduce the quote buy amount by converted fee amount
There was a problem hiding this comment.
I guess you mean "sell" orders here.
There was a problem hiding this comment.
@harisang Exactly, fixed the comment
sunce86
changed the title
| 205315427345679u128.into(), | ||
| 202678773689953u128.into(), |
There was a problem hiding this comment.
Yes, very clear that this is now working as intended ;-)
There was a problem hiding this comment.
That will be tackled in the #2561 🙂
Description
Fixes the way we scale quote sell and quote buy amounts to consider quote fee.
Quoting @fhenneke and his example:
Note that this is affecting only sell orders.
How to test
Fixed existing tests to be aligned with new logic.