-
Notifications
You must be signed in to change notification settings - Fork 134
Halmos Test Properties migrated to Certora #747
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
base: main
Are you sure you want to change the base?
Conversation
…lastUpdateCannotDecrease, irmCannotBeDisabled, lltvCannotBeDisabled, idToMarketParamsForCreatedMarketCannotChange
MathisGD
left a comment
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.
looking good!
| MorphoHarness.Id id; | ||
|
|
||
| require e.block.timestamp <= max_uint128; | ||
| // Assume the property before the interaction. |
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.
Let's add this everywhere since the file had it before. Other option is to refactor the file to use reason strings on require statements, as you prefer
| { | ||
| MorphoHarness.Id id; | ||
|
|
||
| require e.block.timestamp <= max_uint128; |
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.
We can document why this requirement is ok (extremely far in the future), same for occurrences below
| function check_nonceCannotDecrease(bytes4 selector, address caller, address user) public { | ||
| uint256 nonceBefore = morpho.nonce(user); | ||
|
|
||
| _callMorpho(selector, caller); | ||
|
|
||
| uint256 nonceAfter = morpho.nonce(user); | ||
| assert(nonceAfter == nonceBefore || nonceAfter == nonceBefore + 1); | ||
| } |
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.
We can recover this one in Certora as well
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.
Now we can also move tests one folder down: test/forge -> test 🎉
We intend to remove the halmos tests in Morpho Blue. This PR introduces certora rules for Halmos tested properties that were already not proved with Certora.
Interestingly, the properties
lastUpdateNonZeroandlastUpdateCannotDecreaseneeded an assumption that timestamp is <= max_uint128. Else, the explicit cast here can makelastUpdatedzero. Surprisingly, this was not required in Halmos. Time permitting, we can create a halmos bug report.