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

[Certora] If calls succeed state changes #229

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

colin-morpho
Copy link
Contributor

@colin-morpho colin-morpho commented Jan 15, 2025

This PR replaces #227.
We verify, up to some exceptions, that whenever an adapter function is called the states changes or the execution reverts.
We try to this in most general fashion, using a parametric rule.

Before accepting this PR please check that:

  • the CI is updated;
  • the verification succeed;
  • the documentation has been updated.

@colin-morpho colin-morpho added the verif Fromal verification with Certora label Jan 15, 2025
@colin-morpho colin-morpho self-assigned this Jan 15, 2025
@colin-morpho colin-morpho force-pushed the colin@verif/reverts-alt branch from 1648ac9 to 1a21f46 Compare January 15, 2025 16:40
@colin-morpho
Copy link
Contributor Author

For reference, comments of interest made on #227.
#227 (comment)
#227 (comment)
#227 (comment)

@colin-morpho colin-morpho marked this pull request as ready for review January 17, 2025 14:55
@colin-morpho colin-morpho changed the title [Certora] Alternate of #227 [Certora] If calls succeed state changes Jan 17, 2025
@colin-morpho colin-morpho changed the base branch from colin@verif/reverts to main January 17, 2025 15:04
@colin-morpho colin-morpho mentioned this pull request Jan 17, 2025
6 tasks
certora/specs/GeneralAdapter1Reverts.spec Outdated Show resolved Hide resolved
certora/specs/GeneralAdapter1Reverts.spec Outdated Show resolved Hide resolved
certora/specs/GeneralAdapter1Reverts.spec Outdated Show resolved Hide resolved
certora/specs/GeneralAdapter1Reverts.spec Outdated Show resolved Hide resolved
certora/specs/GeneralAdapter1Reverts.spec Outdated Show resolved Hide resolved
certora/specs/GeneralAdapter1Reverts.spec Outdated Show resolved Hide resolved
certora/confs/GeneralAdapter1Reverts.conf Show resolved Hide resolved
certora/dispatch/WETH9.sol Outdated Show resolved Hide resolved
certora/specs/GeneralAdapter1Reverts.spec Outdated Show resolved Hide resolved
certora/specs/GeneralAdapter1Reverts.spec Outdated Show resolved Hide resolved
certora/specs/GeneralAdapter1Reverts.spec Outdated Show resolved Hide resolved
certora/specs/GeneralAdapter1Reverts.spec Outdated Show resolved Hide resolved
certora/specs/GeneralAdapter1Reverts.spec Outdated Show resolved Hide resolved
"Address": "solc-0.8.28",
"Permit2Lib": "solc-0.8.28",
"AllowanceTransfer": "solc-0.8.17",
"WETH9": "solc-0.5.15"
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it compiled with version 0.4.19 ? See here

"lib/morpho-blue/src/Morpho.sol",
"lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol",
"lib/permit2/src/libraries/Permit2Lib.sol",
"lib/permit2/src/AllowanceTransfer.sol",
Copy link
Contributor

Choose a reason for hiding this comment

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

We could use the Permit2 contract directly (which inherits from AllowanceTransfer). I know it's not necessary, but AllowanceTransfer is not really well-known compared to Permit2

Comment on lines +65 to +66
assert (receiver == Bundler3.initiator() && token.allowance(e, Bundler3.initiator(), currentContract) == max_uint256 => storageBefore == lastStorage) &&
(storageBefore == lastStorage => receiver == Bundler3.initiator() && token.allowance(e, Bundler3.initiator(), currentContract) == max_uint256);
Copy link
Contributor

Choose a reason for hiding this comment

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

(optional) could apply to other places where equivalence is rewritten as well

Suggested change
assert (receiver == Bundler3.initiator() && token.allowance(e, Bundler3.initiator(), currentContract) == max_uint256 => storageBefore == lastStorage) &&
(storageBefore == lastStorage => receiver == Bundler3.initiator() && token.allowance(e, Bundler3.initiator(), currentContract) == max_uint256);
bool noChangeExpectedCondition = receiver == Bundler3.initiator() && token.allowance(e, Bundler3.initiator(), currentContract) == max_uint256 ;
assert (noChangeExpectedCondition => storageBefore == lastStorage) &&
(storageBefore == lastStorage => noChangeExpectedCondition);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
verif Fromal verification with Certora
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants