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

Fix __eq__() in ArrayType and UserDefinedType #1506

Closed
wants to merge 7 commits into from

Conversation

webthethird
Copy link
Contributor

Resolves false negatives in the DifferentVariableContractProxy and DifferentVariableContractNewContract upgradeability checks in variables_order.py when comparing two arrays of contract types.
E.g. when comparing CToken[] public allMarkets against itself in two versions of an inherited storage contract, as well as mapping(address => CToken[]) public accountAssets, the order-vars-contracts upgradeability check was telling me these two variables didn't match when they clearly did.

Note: to completely resolve other related false negatives, i.e., resulting from direct comparison of two contracts or structures, at least some of the __eq__() method implementations which I suggested in PR #895 should also be merged in.

Resolves false negatives in the `DifferentVariableContractProxy` and `DifferentVariableContractNewContract` upgradeability checks in [variables_order.py](https://github.com/crytic/slither/blob/master/slither/tools/upgradeability/checks/variables_order.py) when comparing two arrays of contract types.
E.g. when comparing `CToken[] public allMarkets` against itself in two versions of an inherited storage contract, as well as `mapping(address => CToken[]) public accountAssets`, the `order-vars-contracts` upgradeability check was telling me these two variables didn't match when they clearly did.
Workaround due to `Contract.__eq__()` not being implemented in master branch, except when comparing Contract object to string.
Would be better if `Contract.__eq__()` were fully implemented (see PR crytic#895).
@webthethird webthethird changed the title Update __eq__() in ArrayType Fix __eq__() in ArrayType and UserDefinedType Dec 15, 2022
@montyly montyly changed the base branch from master to dev December 20, 2022 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant