From 5f52586d427e9cee55838c0c2f3e6e9dd7a4d06d Mon Sep 17 00:00:00 2001 From: innbuld Date: Thu, 13 Aug 2026 09:27:38 +0100 Subject: [PATCH] fix: naming-convention false positive on ERC-7201 `$` storage pointer `$` is a valid Solidity identifier and the de-facto convention for the ERC-7201 namespaced-storage struct pointer (OpenZeppelin upgradeable, Solady, and others). It has no letters to case, so the mixedCase checks should treat it as conformant. Before this change, every `function f(SomeStorage storage $)` reported a `Parameter ... .$ is not in mixedCase` false positive (24+ on Solady alone). This exempts the exact identifier `$` in `is_mixed_case` and `is_mixed_case_with_underscore`, matching the existing pragmatic exceptions (ERC20 lowercase names, leading `_`, `i_`/`s_`). Adds a regression test with a tripwire confirming that genuine Solidity-level naming violations are still reported. --- .../naming_convention/naming_convention.py | 13 ++++++- ...aming_convention_dollar_storage_sol__0.txt | 2 + .../naming_convention_dollar_storage.sol | 36 ++++++++++++++++++ ...g_convention_dollar_storage.sol-0.8.20.zip | Bin 0 -> 2819 bytes tests/e2e/detectors/test_detectors.py | 5 +++ 5 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_8_20_naming_convention_dollar_storage_sol__0.txt create mode 100644 tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol create mode 100644 tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol-0.8.20.zip diff --git a/slither/detectors/naming_convention/naming_convention.py b/slither/detectors/naming_convention/naming_convention.py index de577b08e..34966df42 100644 --- a/slither/detectors/naming_convention/naming_convention.py +++ b/slither/detectors/naming_convention/naming_convention.py @@ -16,6 +16,7 @@ class NamingConvention(AbstractDetector): Exceptions: - Allow constant variables name/symbol/decimals to be lowercase (ERC20) - Allow '_' at the beginning of the mixed_case match for private variables and unused parameters + - Allow '$' (the ERC-7201 namespaced-storage pointer name) for the mixed_case match - Ignore echidna properties (functions with names starting 'echidna_' or 'crytic_' """ @@ -33,7 +34,8 @@ class NamingConvention(AbstractDetector): Solidity defines a [naming convention](https://solidity.readthedocs.io/en/v0.4.25/style-guide.html#naming-conventions) that should be followed. #### Rule exceptions - Allow constant variable name/symbol/decimals to be lowercase (`ERC20`). -- Allow `_` at the beginning of the `mixed_case` match for private variables and unused parameters.""" +- Allow `_` at the beginning of the `mixed_case` match for private variables and unused parameters. +- Allow `$` (the [ERC-7201](https://eips.ethereum.org/EIPS/eip-7201) namespaced-storage pointer name) for the `mixed_case` match.""" # endregion wiki_description WIKI_RECOMMENDATION = "Follow the Solidity [naming convention](https://solidity.readthedocs.io/en/v0.4.25/style-guide.html#naming-conventions)." @@ -54,12 +56,21 @@ def is_state_naming(name: str) -> bool: @staticmethod def is_mixed_case(name: str) -> bool: + if name == "$": + # `$` is a valid Solidity identifier and the conventional name for + # the ERC-7201 namespaced-storage struct pointer (used by + # OpenZeppelin, Solady, ...). It has no letters to case, so it + # trivially conforms to mixedCase. + return True return re.search("^[a-z]([A-Za-z0-9]+)?_?$", name) is not None @staticmethod def is_mixed_case_with_underscore(name: str) -> bool: # Allow _ at the beginning to represent private variable # or unused parameters + if name == "$": + # ERC-7201 namespaced-storage pointer, see `is_mixed_case`. + return True return re.search("^[_]?[a-z]([A-Za-z0-9]+)?_?$", name) is not None @staticmethod diff --git a/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_8_20_naming_convention_dollar_storage_sol__0.txt b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_8_20_naming_convention_dollar_storage_sol__0.txt new file mode 100644 index 000000000..a7c99851c --- /dev/null +++ b/tests/e2e/detectors/snapshots/detectors__detector_NamingConvention_0_8_20_naming_convention_dollar_storage_sol__0.txt @@ -0,0 +1,2 @@ +Parameter LibExample.tripwire(uint256).bad_param (tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol#33) is not in mixedCase + diff --git a/tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol b/tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol new file mode 100644 index 000000000..37e313170 --- /dev/null +++ b/tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.20; + +/// Regression coverage for the `naming-convention` fix on the ERC-7201 +/// namespaced-storage pointer name `$`. +/// +/// `$` is a valid Solidity identifier and the de-facto convention for the +/// storage-struct pointer (OpenZeppelin upgradeable, Solady, ...). It carries +/// no letters to case, so the mixedCase checks must not flag it. Before the +/// fix, every `function f(SomeStorage storage $)` reported a `Parameter ... .$ +/// is not in mixedCase` false positive (24+ on Solady alone). +/// +/// Expected output after fix: exactly one finding, for the deliberately +/// snake_cased parameter `bad_param` (the tripwire proving the detector still +/// flags genuine Solidity-level violations). + +library LibExample { + struct BytesStorage { + bytes32 _spacer; + } + + // `$` is read here -> exercises `is_mixed_case` on the storage pointer. + function length(BytesStorage storage $) internal view returns (uint256 result) { + assembly { + result := shr(224, sload($.slot)) + } + } + + // `$` is unused here -> exercises `is_mixed_case_with_underscore`. + function clear(BytesStorage storage $) internal {} + + // Tripwire: a snake_cased parameter must still be reported. + function tripwire(uint256 bad_param) internal pure returns (uint256) { + return bad_param; + } +} diff --git a/tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol-0.8.20.zip b/tests/e2e/detectors/test_data/naming-convention/0.8.20/naming_convention_dollar_storage.sol-0.8.20.zip new file mode 100644 index 0000000000000000000000000000000000000000..0c989d9074e698c5d3234234099bcef2989a5c3c GIT binary patch literal 2819 zcmbW(XCM>~0|xNR-VsjOJ0YWsID6ze#1WD0WSnt_v-d9JaAkBZ>r~DtA!nSCJ)=+| z8Hwx_e`j59|M&a*<$azH&!^}64`xD3rvsn?Z~z!g*d6I-^Z1on0Dv(i06+}@03coc z5J*pFHxx3&9T|i`A)Ozhe0^O5ozX$4Kvz$91vJXn**(l36&U2~8tCSQ2ys{NMx&6- zbo2m603Z$k(0csX6Lba&C>njh-&MzDel{)UU=5vCE zbiD~gdIpsG5tRH=?Ka^mtdjPNAm0}Pe&e}2y_U)tA>(CGt1?*nttz2@?6y{)78xLI zR1id;*q1q@cjWe9*Nq}@B|zbH1%#MPHL~Jn&`qNW+phDZBZ?c5e#EfktmOG`Fe%5~ zuAomdOl8~e1H2P=*S5_=sAn!yf|u;zPrCwD;@- zCwOyg6Gk0W?@;oQGDV$Em~hEv9^n~Bjf{re_;l4er%zi8Wqgw5zM?y?x(%^b6*54G z(8r--rI|wJ#q*!B)^b?n|W&09x^IQB0FgPfBPcjX7c7Qm;=cbeglfJcUTbM56PY^x|N`Gidut=9ed zLE-f8U6H2yk|6bx6lQ62=X}5$7kfB$C^!gQ<@L32)hk|mXvpJw$pbHBrs?VsS7oWs@!`rQ+|Z+46uVNR zs$|2lL4f*xf!+()woPDd+rFAy=sGZe^lYvP^WplusJ;(kE`i(VX8~xCM1PPV1e?Ud zeEje@MyIPU5l_7G7TWHPNmxCV`WnFWe1iLu$a50>ShJK=czt?*OVC;OI{U`GmaRcOh|@*FPW(XmrO&Yx8-pDx zRARiBv#T(Zt|Llvy3u+zb40$mb^;hmqEXOJG2wRUv&{Dx+mjaglb>HVACr|PYz++o z#vLx7_P|a@fh-QOP!%4xKIxKDW4hUz5f0)gdbo*NKwjjvvom0Cp7;9QaGxPV;4>?) z;vUXX>LgnDp_#guL?#05qEc1NmNSP%uy{d|y}-N^D<+6{@!j`#xavvFY`^q2X#I$a zrp}k^N?&1E=6j4)$`#*NY|H%$lCS6bvbN6OF7L62J3YJjOsneTZ3NdHmU{-N4AJsk zoW^@FkoRmMu(lBRD$%;V=?LbY_>z{}iKb^Owb1fJYxsEH0dUkGAs;zRwgNkC&AsbC?i$~;-H zl)B6%NQ23tL!Cb?(R-?3(tB|#lXE4bRjlwtA=fO>z#rTJ%}`sM&<9JIr|s(0tg)O^W$jx zai;;pY84KxHV4ruKtp3GQzj9-f0I(kcWrod=|q2cM|Rb@R$vPlqUWU@&+GVNKM8&? z+2p7nsN(015)1p4rEsjo%&AOzyDPbu&ptDSuvwxnL)pgZc7~*fx9Ps1L_yv`7R+s^ z`y9fq9Gz0S+EMT%JAVg`htAn3g&XGP+ZWT%$J&BJd1J~np}%xSigEiJw+07e;5VkH zzAx<W0{beD?)TRh$c9lal1amhn}!ng ztnL_>bw8ISGoT9kn*^_!?#N?PnkWxe2-G0t^tPY#4Rq^2#q0+M(y606_Ez*Etv&qr zC1@b>`qu3~cWwAJyFKVqlnfu!6-R?JYr3+}L|>QR=O5mrjH@!@sN!Q@B~OSJ@0*{` zyHSSkW3%aQh?k&awQ&MkEOMLY2l8v^Qz=Rfn;-etYrpSI8vglp{OY62qtvJ8MiAwt z-)Zo;iUUebct2Frscc~{V-?vU<!CFU8R$Am=H!AB_6PhT zoAoFg5m9*d?h=qGy!La^=Pv(_Jpe_{C^YwYb@xq*4`WZ2oc@5lw-G!)^OFw#2^Irb zv?wg=pIKy_l@Gvu#@1e0f{Qj{EGO>0|568e{yE&V~xxCU1?Kdy-vmA zU4{U=)eT8J+y&%|t&hx1+hyKY7BKgv4-Z*>!}#$}s~Qv;h?{L&$$J4kGx9=(V`py| zjke*i)|HftiPw&@$q$5YlV@@V1R3?E}sE=G1GV+0!BH#M^SC&&ApM7sTx!+tE5omu|;Wc6ycy$J;X1463lB2Q= z9X&lBajw$A^bcYzqd#~RxfxsyZ;Fhw`qR4@#7)-O>t(34*Ty=?p_pFq-L0_F?>>Lr z%xu zYUkK{!T@|c{pzpuIjER@2t$*f&#BZ0N z_6!Zrtbh-#0t6qXH7q`3VmNenZ0Ffwv!=$1rfR!mTJ>3sNJ~d-WHNX0lPef$B3BzT zb>F{b^=ja5DyOaava88lVy5x=0UKn|x+`Bg)oFrGyL(flI1QPA@+i%ZidRc#UiU1x z&RfL!=x7ISeDQ4EYh|d(NG`MTp