-
Notifications
You must be signed in to change notification settings - Fork 23
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
Generate own step payout function #2352
Conversation
cfce164
to
cd8a54f
Compare
95117e7
to
2665f22
Compare
a09789b
to
d73e17f
Compare
d73e17f
to
5e6dd19
Compare
`NumericalDescriptor::get_range_payouts` should be called with the _total_ collateral, including position margin and the collateral reserves.
We were allowing the short liquidation payout to go under the short party's collateral reserve, which was obviously wrong. The payout curve is still a bit rough around the edges (literally). This is because we have to make relatively big payout jumps to connect the liquidation intervals with the adjacent intervals. I'm not sure there is a good solution for this other than going back to the ItchySats payout curve.
This is the (probably) last thing that prevents us from being able to rely on the payout curve liquidation intervals for things like the available balance and, relatedly, position resizing.
Before this patch, we generated a polynomial payout function which would then be transformed by `rust-dlc` into a step function. This approach was problematic because we could not trust `rust-dlc` to not mess with the intervals excessively. Specifically, any changes to the liquidation intervals could cause problems because they represent the collateral reserves, which are used to indicate available balances and are circumstantially needed to complete features such as position resizing. Now we take full control of the payout function generation, directly generating a step function that can be consumed by `rust-dlc` _without_ rounding. The payout is calculated by taking the PNL at the half-way point of the interval. It is important to emphasise that we must opt out of rounding by using a `RoundingIntervals` with a single element of the form: ```rust RoundingInterval { begin_interval: 0, rounding_mod: 1, } ``` Overall, I think the code is considerably simpler and there are fewer edge cases to handle. One important thing to consider is that we can now directly control the number of payouts we generate, which has an effect on the number of CETs that will be created. This is currently set by the constant `PAYOUT_CURVE_DISCRETIZATION_INTERVALS`, but in the future we could make it dynamic. Additionally, at the moment the majority of the intervals are of equal length in terms of price. Eventually we might want to generate more, smaller intervals around the starting price, and make the less likely intervals bigger as we approach the liquidation zones. Other notable changes --------------------- Since we had to replace a snapshot, we've introduced a dev-dependency to `insta`[^1], a library that makes it easy to write and maintain snapshot tests. There is a companion tool, `cargo-insta`[^2], which can help with this process. [^1]: https://github.com/mitsuhiko/insta. [^2]: https://github.com/mitsuhiko/insta/tree/master/cargo-insta.
5e6dd19
to
10e0cee
Compare
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.
LGTM
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.
Please excuse the late review.
It's been a while that I've been working on this, so please be kind if this is a stupid question for a Sunday night , and I also know that this was built on my initial version. But help me out please:
In this example from payout_curve_csv
, the offerer goes long with a leverage of 2 and the and the acceptor short with leverage 2.
If the price goes up, he wins everything from ~60k (short liquidation price).
but shouldn't he be losing everything from ~20k?
@@ -298,6 +237,140 @@ mod tests { | |||
} | |||
} | |||
|
|||
#[test] | |||
fn payout_function_respects_collateral_reserve() { |
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.
Would you mind adding some comments in here please? It's hard to follow what is happening.
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.
@luckysori reminder to do this tomorrow.
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.
Done here: #2368.
let initial_price = dec!(28_251); | ||
let quantity = 500.0; | ||
let leverage_coordinator = 2.0; | ||
let coordinator_margin = calculate_margin(initial_price, quantity, leverage_coordinator); |
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.
It might be worth using hardcoded values all the way
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.
Hmmm, I think these should not be hard-coded because they are derived from other parameters.
|
||
let coordinator_direction = Direction::Short; | ||
|
||
let coordinator_collateral_reserve = 2_120_386; |
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.
Do you have a formula of how to get to this value?
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.
This one is just completely arbitrary based on an example that failed when running tests. It's not computed, the fee reserve can be arbitrary.
Fair question! The example configures the offer party with a fee reserve of 300k sats. https://github.com/get10101/10101/blob/main/crates/payout_curve/examples/payout_curve_csv.rs#L54-L56 So the offer party's minimum payout is set by that amount. |
aahhh... that makes sense. Thanks for the explanation :D |
Awesome work btw. :) |
Note to reviewers: the second patch was an attempt at fixing #2288 which was later replaced by rewriting the payout curve generation logic. I would recommend to review this PR in one go (not patch by patch).
Fixes #2288.
Fixes #1834.
Before this PR, we generated a polynomial payout function which would then be transformed by
rust-dlc
into a step function. This approach was problematic because we could not trustrust-dlc
to not mess with the intervals excessively.Specifically, any changes to the liquidation intervals could cause problems because they represent the collateral reserves, which are used to indicate available balances and are circumstantially needed to complete features such as position resizing.
Now we take full control of the payout function generation, directly generating a step function that can be consumed by
rust-dlc
without rounding. The payout is calculated by taking the PNL at the half-way point of the interval.It is important to emphasise that we must opt out of rounding by using a
RoundingIntervals
with a single element of the form:Overall, I think the code is considerably simpler and there are fewer edge cases to handle.
One important thing to consider is that we can now directly control the number of payouts we generate, which has an effect on the number of CETs that will be created. This is currently set by the constant
PAYOUT_CURVE_DISCRETIZATION_INTERVALS
, but in the future we could make it dynamic.Additionally, at the moment the majority of the intervals are of equal length in terms of price. Eventually we might want to generate more, smaller intervals around the starting price, and make the less likely intervals bigger as we approach the liquidation zones.
Other notable changes
Since we had to replace a snapshot, we've introduced a dev-dependency to
insta
1, a library that makes it easy to write and maintain snapshot tests. There is a companion tool,cargo-insta
2, which can help with this process.Footnotes
https://github.com/mitsuhiko/insta. ↩
https://github.com/mitsuhiko/insta/tree/master/cargo-insta. ↩