-
Notifications
You must be signed in to change notification settings - Fork 9
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
[Certora] Handle overflows #105
base: main
Are you sure you want to change the base?
Changes from 3 commits
6621340
2ebbdff
6ab5bad
2279357
c394113
f55c098
273104e
a73bda1
f6f5b89
f121755
fdbcc97
389f418
376c4fd
65567c2
06b6407
3fb5d42
01aca82
0874bf3
619435e
4de56d8
4961928
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,7 +67,6 @@ rule mint(env e) { | |
|
||
// cache state | ||
uint256 toBalanceBefore = balanceOf(to); | ||
uint256 toVotingPowerBefore = delegatedVotingPower(delegatee(to)); | ||
uint256 otherBalanceBefore = balanceOf(other); | ||
uint256 totalSupplyBefore = totalSupply(); | ||
|
||
|
@@ -76,8 +75,7 @@ rule mint(env e) { | |
|
||
// check outcome | ||
if (lastReverted) { | ||
assert e.msg.sender != owner() || to == 0 || totalSupplyBefore + amount > max_uint256 || | ||
toVotingPowerBefore + amount > max_uint256; | ||
assert e.msg.sender != owner() || to == 0 || totalSupplyBefore + amount > max_uint256 ; | ||
} else { | ||
Comment on lines
77
to
79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could we avoid this pattern? (and have a rule for checking reverts, and one rule for checking the non revert case's effects) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we are still following OZ's specs (and actually getting closer to it in this PR) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we did mention the idea of a more generic framework on ERC20 at some point but we didn't find a clever way to make this happen to decided to continue this route, if I'm not mistaken. |
||
// updates balance and totalSupply | ||
assert e.msg.sender == owner(); | ||
|
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.
what is this spec useful for now that we have ERC20Reverts and ERC20Invariants?
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.
This is the spec from OZ, with some liveness and safety properties that are specific to functions and that are not invariants. For example there is a rule on transfer specifically