Skip to content
This repository has been archived by the owner on Jan 10, 2025. It is now read-only.

lending: Add overflow check in liquidation proptest #1199

Merged
merged 1 commit into from
Feb 8, 2021

Conversation

joncinque
Copy link
Contributor

This issue was discovered during CI:
https://github.com/solana-labs/solana-program-library/runs/1833196857

The approach is to do the same calc as _liquidate_obligation and make
sure that the MockConverter doesn't overflow Decimal. This seemed
like a simpler approach than doing a local proptest constraint.

The actual failure is on line 150 in _liquidate_obligation: obligation.max_liquidation_amount() is always the amount we try to convert, and in this case:

conversion rate = 681
collateral = u64::MAX (roughly)
max liquidation amount = collateral / 2
collateral exchange rate = 0.01

receive amount = conversion_rate * (collateral / 2) * collateral_exchange_rate
receive_amount = 681 * u64::MAX / 2 * 0.01 > u64::MAX

On top of that, this PR includes the proptest regression to run the failing case
on every cargo test.

This issue was discovered during CI:
https://github.com/solana-labs/solana-program-library/runs/1833196857

The approach is to do the same calc as `_liquidate_obligation` and make
sure that the `MockConverter` doesn't overflow `Decimal`.  This seemed
like a simpler approach than doing a complex local proptest constraint.

On top of that, it includes the proptest regression to run it on every
`cargo test`.
@joncinque joncinque requested a review from jstarry February 8, 2021 12:15
Copy link
Contributor

@jstarry jstarry left a comment

Choose a reason for hiding this comment

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

Thanks!

@joncinque joncinque merged commit 65c3af3 into solana-labs:master Feb 8, 2021
@joncinque joncinque deleted the tl-proptest-fix branch February 8, 2021 13:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants