modular lif#992
Conversation
Rebased PR #992 on current main and resolved conflicts with the continuousFeeCap changes. Conflict resolution: combined liquidationCursor with continuousFeeCap in EIP-712 hashes and regenerated the frontend signature fixture.
d1b23b6 to
a5c54ea
Compare
Co-authored-by: prd-carapulse[bot] <264278285+prd-carapulse[bot]@users.noreply.github.com>
Co-authored-by: MathisGD <74971347+MathisGD@users.noreply.github.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
| /// Check that maxLif <= 2*WAD for valid cursor values | ||
| rule maxLifIsAtMostTwoWad(uint256 lltv, uint256 cursor) { | ||
| require lltv <= WAD(), "see rule createdMarketsHaveLltvLessThanOrEqualToOne"; | ||
| require cursor <= WAD() / 2, "see LIQUIDATION_CURSOR_HIGH in ConstantsLib"; | ||
| assert maxLif(lltv, cursor) <= 2 * WAD(); | ||
| } |
There was a problem hiding this comment.
this isn't true anymore, but we don't need it
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e55adcd989
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // maxLif is recomputed on the fly from (lltv, liquidationCursor) during liquidate. Summarize it by a deterministic ghost: its | ||
| // value bounds are assumed below (see ExactMath.spec), and its own well-definedness (no division by zero for valid | ||
| // liquidationCursors) is proven there too. | ||
| function maxLif(uint256 lltv, uint256 liquidationCursor) internal returns (uint256) => maxLifGhost(lltv, liquidationCursor); |
There was a problem hiding this comment.
Do not hide maxLif divisions in no-div proof
This summary removes the body of maxLif from the rule that is supposed to catch zero denominators in mulDivDown/mulDivUp, but maxLif itself now computes a denominator from (lltv, liquidationCursor); for example lltv == 0 and liquidationCursor == WAD reaches a zero denominator. Because ExactMath.spec calls maxLif plainly rather than with @withrevert, that reverting boundary is pruned rather than proving well-definedness, so this rule can pass while missing the new division site. Either keep maxLif unsummarized here or add a dedicated @withrevert well-definedness rule/precondition for the boundary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This only happens with those exact values: lltv == 0 and liquidationCursor == WAD.
I think it's ok because it would revert at market creation, and never after.
Still, it may be simpler to require that liquidationCursor < WAD, instead of liquidationCursor <= WAD
Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Co-authored-by: Quentin Garchery <garchery.quentin@gmail.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Co-authored-by: Adrien Husson <adhusson@gmail.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Co-authored-by: Adrien Husson <adhusson@gmail.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Co-authored-by: Adrien Husson <adhusson@gmail.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Co-authored-by: MathisGD <74971347+MathisGD@users.noreply.github.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Co-authored-by: Adrien Husson <adhusson@gmail.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
Co-authored-by: Adrien Husson <adhusson@gmail.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d673ac0d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function summaryToId(Midnight.Market market) returns (bytes32) { | ||
| require forall uint256 i. i < market.collateralParams.length => market.collateralParams[i].lltv <= WAD(), "proven in CreatedMarkets.spec"; | ||
| require forall uint256 i. i < market.collateralParams.length => market.collateralParams[i].maxLif >= WAD() && market.collateralParams[i].maxLif <= 2 * WAD(), "proven in ExactMath.spec"; | ||
| require forall uint256 i. i < market.collateralParams.length => maxLifGhost(market.collateralParams[i].lltv, market.collateralParams[i].liquidationCursor) >= WAD() && maxLifGhost(market.collateralParams[i].lltv, market.collateralParams[i].liquidationCursor) <= 2 * WAD(), "proven in ExactMath.spec"; |
There was a problem hiding this comment.
Restore the missing maxLif upper-bound proof
This precondition now assumes maxLifGhost <= 2 * WAD() as "proven in ExactMath.spec", but this commit removed maxLifIsAtMostTwoWad and a repo-wide search only finds that rule name in these comments. Because maxLif is summarized by an unconstrained ghost here, the overflow proof can silently rely on an unproved bound; restore a maxLif <= 2 * WAD rule for the valid (lltv, liquidationCursor) domain or cite/use the CreatedMarkets invariant for created markets instead.
Useful? React with 👍 / 👎.
| uint256 liquidationCursor = market.collateralParams[i].liquidationCursor; | ||
| require(isLltvEnabled[lltv], LltvNotEnabled()); | ||
| require(isLiquidationCursorEnabled[liquidationCursor], LiquidationCursorNotEnabled()); | ||
| require(maxLif(lltv, liquidationCursor) <= 2 * WAD, InvalidMaxLif()); |
There was a problem hiding this comment.
| require(maxLif(lltv, liquidationCursor) <= 2 * WAD, InvalidMaxLif()); | |
| require(maxLif(lltv, liquidationCursor) <= 2 * WAD, InvalidMaxLif()); | |
| require(lltv == WAD || lltv * maxLif(lltv, liquidationCursor) <= 0.999 ether * WAD); |
modularize liquidation cursors.