slither icon indicating copy to clipboard operation
slither copied to clipboard

[False-Positive]: `constable-states` false positive introduced in `0.11.0`

Open arr00 opened this issue 11 months ago • 4 comments

Describe the false alarm that Slither raise and how you know it's inaccurate:

See this action run. There is a new false positive introduced in 0.11.0 where the strings mentioned below are now tagged for being constable.

Frequency

Occasionally

Code example to reproduce the issue:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/19c2f2f5a5ea43e18dfff2b92b54b76815783d93/contracts/utils/cryptography/EIP712.sol#L51-L52

Version:

0.11.0

Relevant log output:

EIP712._nameFallback (contracts/utils/cryptography/EIP712.sol#51) should be constant 
EIP712._versionFallback (contracts/utils/cryptography/EIP712.sol#52) should be constant

arr00 avatar Feb 03 '25 21:02 arr00

I'm not sure to understand the issue. If i run slither contracts/utils/cryptography/EIP712.sol --detect constable-states they are detected also in 0.10.4

smonicas avatar Feb 03 '25 22:02 smonicas

When running locally I do see that--but you can see that setting it back to that version in CI silences the error. Regardless, it is still a false positive but it is relatively hard to detect why. We conditionality set the value in the constructor if necessary (uses this lib https://github.com/OpenZeppelin/openzeppelin-contracts/blob/19c2f2f5a5ea43e18dfff2b92b54b76815783d93/contracts/utils/ShortStrings.sol#L88-L95).

arr00 avatar Feb 03 '25 22:02 arr00

I see the pattern used and it's quite complicated. I will try to see if it is possible to remove this false positive. You can also disable the detector with a comment above each variable instead of changing the Slither version.

    // slither-disable-next-line constable-states
    string private _nameFallback;
    // slither-disable-next-line constable-states
    string private _versionFallback;

smonicas avatar Feb 03 '25 22:02 smonicas

Ok, we can do that if necessary. Do you know this is only being surfaced now with the new release?

arr00 avatar Feb 03 '25 23:02 arr00