[Certora] lib/bundler assets/debts reachability#916
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb5264a55a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eabec97f3b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
bhargavbh
left a comment
bhargavbh
left a comment
There was a problem hiding this comment.
very nice roundtrip properties to have for bundler's liveness guarantees.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ed13dd96a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
bhargavbh
left a comment
bhargavbh
left a comment
There was a problem hiding this comment.
LGTM. Please update the README with the rules description.
Co-authored-by: Bhargav Bhatt <40268131+bhargavbh@users.noreply.github.com> Signed-off-by: lilCertora <lilian@certora.com>
commit some suggestions Co-authored-by: Quentin Garchery <garchery.quentin@gmail.com> Signed-off-by: lilCertora <lilian@certora.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7bd9544bf7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| // a*b <= max_uint256 and d >= 1 => a*b/d <= a*b <= max_uint256, so the cast never truncates. | ||
| return require_uint256(a * b / d); |
There was a problem hiding this comment.
Can we use an assert_uint256 here?
| return require_uint256(a * b / d); | |
| return assert_uint256(a * b / d); |
There was a problem hiding this comment.
i had problems doing it : https://prover.certora.com/output/7508195/f0ed1006a3ad4a19a80c44279b3737b2/
There was a problem hiding this comment.
can't access this link, but I also had problems in this run. Any idea what that might be ?
There was a problem hiding this comment.
looks like an internal bug before reaching the assert_uint256 :
ERROR COMMON - Had an exception while getting object to verify for rule
repayAndWithdrawCollateralRepaysTargetUnits,
java.lang.IllegalArgumentException: Failed requirement.
at verifier.PatchingProgramWrapper.updateAssertSnippetsOf(PatchingProgramWrapper.kt:149)
Co-authored-by: Jochen Hoenicke <hoenicke@gmail.com> Signed-off-by: lilCertora <lilian@certora.com>
| rule buyerAssetsReachable(env e, Midnight.Offer offer, uint256 targetBuyerAssets, address taker, address takerCallback, bytes takerCallbackData, address receiverIfTakerIsSeller, bytes ratifierData) { | ||
| bytes32 id = summaryToId(offer.market); | ||
|
|
||
| uint256 units = TakeAmountsLibHarness.buyerAssetsToUnits(e, currentContract, id, offer, targetBuyerAssets); |
There was a problem hiding this comment.
the fact that revert cases are ignored hides the fact that actually not all buyerAssets are reachable all the time. maybe at least:
| uint256 units = TakeAmountsLibHarness.buyerAssetsToUnits(e, currentContract, id, offer, targetBuyerAssets); | |
| // reverts if the offer's price is above one (in that case not all buyerAssets are reachable. | |
| uint256 units = TakeAmountsLibHarness.buyerAssetsToUnits(e, currentContract, id, offer, targetBuyerAssets); |
or a more advanced verification with @withrevert
5 participants
corresponding thread