Skip to content

fix: naming-convention false positive on ERC-7201 $ storage pointer - #3080

Open
innbuld wants to merge 1 commit into
crytic:masterfrom
innbuld:fix/naming-convention-erc7201-dollar
Open

fix: naming-convention false positive on ERC-7201 $ storage pointer#3080
innbuld wants to merge 1 commit into
crytic:masterfrom
innbuld:fix/naming-convention-erc7201-dollar

Conversation

@innbuld

@innbuld innbuld commented Aug 13, 2026

Copy link
Copy Markdown

Summary

The naming-convention detector reports the identifier $ as is not in mixedCase. $ is a valid Solidity identifier and the de-facto convention for the ERC-7201 namespaced-storage struct pointer (OpenZeppelin upgradeable, Solady, and most modern contracts). It has no letters to case, so it should be treated as conformant, consistent with the detector's other pragmatic exceptions (ERC20 lowercase names, leading _, i_/s_).

Impact

Any project using ERC-7201 namespaced storage gets one false positive per function f(SomeStorage storage $). Solady alone emits 24+ Parameter ... .$ is not in mixedCase findings, drowning out real results.

Minimal repro:

library LibExample {
    struct BytesStorage { bytes32 _spacer; }
    function length(BytesStorage storage $) internal view returns (uint256 r) {
        assembly { r := shr(224, sload($.slot)) }
    }
}
$ slither LibExample.sol --detect naming-convention
Parameter LibExample.length(LibExample.BytesStorage).$ is not in mixedCase

Fix

Exempt the exact identifier $ in is_mixed_case and is_mixed_case_with_underscore. The exemption is intentionally limited to the lone $ (the demonstrated convention) and does not broaden to arbitrary $-containing names, so no genuine violations are silenced.

Testing

  • Adds naming_convention_dollar_storage.sol, covering $ as a read parameter, $ as an unused parameter, and a snake_cased tripwire parameter that must still be flagged. The snapshot contains exactly the tripwire finding.
  • Full detector e2e suite passes (385 tests); ruff check/ruff format clean.

@innbuld
innbuld requested a review from smonicas as a code owner August 13, 2026 09:06
@CLAassistant

CLAassistant commented Aug 13, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

`$` 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.
@innbuld
innbuld force-pushed the fix/naming-convention-erc7201-dollar branch from 7d85e1c to 5f52586 Compare August 13, 2026 09:17
@innbuld innbuld changed the title Fix naming-convention false positive on ERC-7201 $ storage pointer fix: naming-convention false positive on ERC-7201 $ storage pointer Aug 13, 2026
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.

2 participants