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

doc: readme parameter restriction #82

Merged
merged 3 commits into from
Oct 7, 2024
Merged

doc: readme parameter restriction #82

merged 3 commits into from
Oct 7, 2024

Conversation

peyha
Copy link
Collaborator

@peyha peyha commented Oct 7, 2024

Wrote a short paragraph on the bounds of pre-liquidation parameters
Still struggling to find a clean phrasing for "allowing additionnal pre-liquidation close factor configuration" (the idea is that with preLCF2 > 1 you can handle more pre-liquidation close factor curves)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
- preLCF1 <= preLCF2;
- preLFC1 <= 1;
- 1 <= preLIF1 <= preLIF2 <= 1 / LLTV.
Note: Using `preLCF2 > 1`, you can select at which LTV between preLltv and LLTV the entire position will be pre-liquidated.
Copy link
Contributor

@QGarchery QGarchery Oct 7, 2024

Choose a reason for hiding this comment

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

Suggested change
Note: Using `preLCF2 > 1`, you can select at which LTV between preLltv and LLTV the entire position will be pre-liquidated.
Note: Using `preLCF2 > 1`, one can select a LTV smaller than LLTV after which the position can be fully pre-liquidated.

maxPreLltv: marketParams.lltv - 1,
minPreLCF: WAD + 1,
maxPreLCF: type(uint256).max,
minPreLIF: WAD + 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
minPreLIF: WAD + 1,
minPreLIF: WAD,

@adhusson adhusson self-requested a review October 7, 2024 09:06
Copy link
Contributor

@colin-morpho colin-morpho left a comment

Choose a reason for hiding this comment

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

Minor comments.

README.md Outdated Show resolved Hide resolved
README.md Outdated
- preLFC1 <= 1;
- 1 <= preLIF1 <= preLIF2 <= 1 / LLTV.
Note: it is not mandatory that `preLCF1 <= 1` and `preLCF1 <= 1` hold.
Indeed without this, the pre-liquidation close factor can reach 100% before the position is liquidatable allowing additionnal pre-liquidation close factor configurations.
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively you can write.

Suggested change
Indeed without this, the pre-liquidation close factor can reach 100% before the position is liquidatable allowing additionnal pre-liquidation close factor configurations.
Indeed, in some cases the pre-liquidation close factor reaches 100% with LTV lower than LLTV.

Copy link
Collaborator Author

@peyha peyha Oct 7, 2024

Choose a reason for hiding this comment

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

Used @adhusson's suggestion for this
"Note: Using preLCF2 > 1, you can select at which LTV between preLltv and LLTV the entire position will be pre-liquidated."
wdyt ?


### Pre-liquidation parameters restrictions

The PreLiquidation smart-contract enforces the following properties:
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove "following" as the properties are listed in the same sentence. Should you list them separately (e.g. by labeling with them names or ids out of a sentence) then using following would apply as the sentence is not self contained.

Suggested change
The PreLiquidation smart-contract enforces the following properties:
The PreLiquidation smart-contract enforces the properties:

README.md Outdated Show resolved Hide resolved
@peyha peyha merged commit 7ee05f3 into main Oct 7, 2024
4 checks passed
@peyha peyha deleted the doc/readme branch October 7, 2024 09:19
@MathisGD MathisGD mentioned this pull request Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants