Skip to content
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

fix: overflow bug in payout curve #162

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

bonomat
Copy link
Collaborator

@bonomat bonomat commented Oct 31, 2023

It is possible that a payout curve decreases with the amount of payout as the events increase. This lead to an overflow panic in the evaluate function

bonomat added a commit to get10101/10101 that referenced this pull request Oct 31, 2023
Fix commit: p2pderivatives/rust-dlc@f233718
PR into latest: p2pderivatives/rust-dlc#162

Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
Copy link
Contributor

@Tibo-lg Tibo-lg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, surprised that it was not caught before.

let slope = (right_point.outcome_payout - left_point.outcome_payout) as f64
/ (right_point.event_outcome - left_point.event_outcome) as f64;
let slope = (right_point.outcome_payout as f64 - left_point.outcome_payout as f64)
/ (right_point.event_outcome as f64 - left_point.event_outcome as f64);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: as we expect event outcomes to be increasing only you don't really need to change this line

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out. I've removed these changes.

@luckysori luckysori assigned luckysori and bonomat and unassigned luckysori Nov 1, 2023
@luckysori luckysori added the ln label Nov 1, 2023
It is possible that a payout curve decreases with the amount of payout as the events increase. This lead to an overflow panic in the evaluate function

Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
@bonomat bonomat force-pushed the fix/overflow-bug-in-payoutcurve branch from f233718 to e42cfcf Compare November 1, 2023 18:32
bonomat added a commit to get10101/10101 that referenced this pull request Nov 1, 2023
Fix commit: p2pderivatives/rust-dlc@f233718
PR into latest: p2pderivatives/rust-dlc#162

Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
@bonomat bonomat merged commit 6801d17 into feature/ln-dlc-channels Nov 1, 2023
192 checks passed
@bonomat bonomat deleted the fix/overflow-bug-in-payoutcurve branch November 1, 2023 19:32
bonomat added a commit to get10101/10101 that referenced this pull request Nov 2, 2023
Fix commit: p2pderivatives/rust-dlc@f233718
PR into latest: p2pderivatives/rust-dlc#162

Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants