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

remove reference to increaseAllowance #9

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

urikirsh
Copy link
Collaborator

@urikirsh urikirsh commented Feb 2, 2024

Removed reference to the function increaseAllowance in the ERC20.spec.
The function increaseAllowance is not a standard ERC20 function (see list here https://ethereum.org/developers/docs/standards/tokens/erc-20). It created a bad error message on m0's code when Jared tried to run the rule on their code, which had an ERC20 that did not contain this function:
[main] ERROR ALWAYS - Error in spec file (ERC20.spec:102:80): could not type expression "sig:MToken.increaseAllowance(address, uint256)", message: could not find method
https://prover.certora.com/output/7053/8a687fb3ff0e4b06b5c357738a5e8340/?anonymousKey=5cb589114f221eaf6c38bde67e7824d55686a0f5

As we want this spec to be generic, I am removing the reference to this function.

@urikirsh urikirsh added the bug Something isn't working label Feb 2, 2024
@urikirsh urikirsh marked this pull request as ready for review February 2, 2024 20:15
@jflatow
Copy link

jflatow commented Feb 5, 2024

LGTM - also having a commented out check for it seems not such a bad idea if it comes up a lot, but probably usage will be waning: OpenZeppelin/openzeppelin-contracts#4583

Hopefully other common situations will be handled by some EIP and we can just put the vanilla checks in a EIPXXX.spec for other cases

@urikirsh urikirsh merged commit 01cd04d into master Feb 5, 2024
4 checks passed
@urikirsh urikirsh deleted the uri/remove_increaseAllowance branch February 5, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants