-
Notifications
You must be signed in to change notification settings - Fork 110
feat(tpu): provide premium for tpu #2405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
…nal, which doesn't impl std default
# Conflicts: # mm2src/mm2_main/src/lp_swap/taker_swap_v2.rs # mm2src/mm2_main/tests/docker_tests/swap_proto_v2_tests.rs
borngraced
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! minor notes
| #[serde(default, skip_serializing_if = "SwapVersion::is_legacy")] | ||
| pub swap_version: SwapVersion, | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| premium: Option<MmNumber>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not add a doc comment for this field?
|
As I can see, we don't show premium in the orderbook so when users are viewing orderbook they see only order prices from 'setprice' rpc and are not aware of extra premium pay. But when 'buy' or 'sell' order is created its price should take into account the premium part. How would user know this part amount? |
| } | ||
|
|
||
| let is_legacy = self.swap_version.is_legacy() || taker.swap_version.is_legacy(); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we may get premium as reference here, like:
let premium_default = MmNumber::default();
let premium = self.premium.as_ref().unwrap_or(&premium_default);
and use it within match taker.action {...} instead of cloning.
(but not sure this is much better though)
mm2src/mm2_main/src/lp_ordermatch.rs
Outdated
| OrderMatchResult::NotMatched | ||
| // taker_rel_amount must cover the premium requested by maker | ||
| let required_rel_amount = | ||
| taker_base_amount * &self.price + self.premium.clone().unwrap_or_default(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we require Taker to pay Maker order's premium.
As I can see, premium is a fixed amount in rel coin units, assigned when the order is created.
What if the order is filled only partially, will Taker always pay the same premium amount (that is, shouldn't premium be proportional to the order amount being filled)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the order is filled only partially, will Taker always pay the same premium amount
yes, premium is a fixed rel amount maker requires for a swap. They ask it for the action (swap) not for a trading volume.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not fair:
If an order was filled 3 times partially the maker would receive 3 premiums.
If same order is filled only one time for the full amount the maker receives only 1 premium.
This seems unfair. I think the maker should receive same premium no matter how many times the order was filled.
For this the premium should be calculated proportionally to the amount filled.
(Not sure though, maybe this was discussed already and was decided to pay premium like it is implemented now)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During swap maker pays fees for transactions, which doesn't depend on tx value, its better to treat premium as guaranteed fixed amount which could cover maker expenses on swap
borngraced
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
|
I am okay with the code in this PR, |
This PR introduces the "premium" feature for TPU.
Maker is now able to set the "premium" field in the
setpricerequest, which represents the extra amount ofrelcoins they require from taker.Additionally, this PR introduces specific order matching behavior for TPU.
When a taker request is processed by a maker order, the taker order is treated as a limit order if the taker "buys." This means taker will receive as much as they want to buy, and they will also send the entire amount they offered for the purchase.
If taker action is "sell," then taker order is treated as a market order.
It is worth mentioning that for the taker "sell" action, we allow maker to send fewer coins, instead of asking taker to send extra
relpremium.