Skip to content

feat(lint): add incorrect ERC20 interface lint#14428

Merged
mablr merged 7 commits intofoundry-rs:masterfrom
figtracer:lint/incorrect-erc20-interface
Apr 24, 2026
Merged

feat(lint): add incorrect ERC20 interface lint#14428
mablr merged 7 commits intofoundry-rs:masterfrom
figtracer:lint/incorrect-erc20-interface

Conversation

@figtracer
Copy link
Copy Markdown
Collaborator

@figtracer figtracer commented Apr 23, 2026

Summary

  • add a medium-severity lint for ERC20 interfaces and implementations with incorrect return signatures
  • register the lint and add focused testdata coverage for incorrect and correct ERC20 shapes
  • add the lint to the forge-lint overview so the help link slug is present in this repo

Testing

  • cargo test -p forge-lint IncorrectERC20Interface -- --nocapture

Context

  • companion docs PR will be opened in foundry-rs/book for the linting overview page

#14381

Copy link
Copy Markdown
Collaborator

@mablr mablr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just a smol nit on tests

Comment thread crates/lint/testdata/IncorrectERC20Interface.sol
@mablr
Copy link
Copy Markdown
Collaborator

mablr commented Apr 23, 2026

ensure_lint_rule_docs fails

@figtracer
Copy link
Copy Markdown
Collaborator Author

figtracer commented Apr 23, 2026

ensure_lint_rule_docs fails

yea we need foundry-rs/book#1795 1st

@figtracer figtracer enabled auto-merge (squash) April 23, 2026 15:20
zerosnacks and others added 3 commits April 23, 2026 18:21
Expanding IERC20 with full function signatures caused solc compilation
failures (Error 4822: Overriding function return types differ) because
IERC20Incorrect inherits from IERC20 and overrides with incompatible
return types. Revert to the empty-base pattern used by IncorrectERC721Interface.

Amp-Thread-ID: https://ampcode.com/threads/T-019dbb72-89c7-740d-b6cf-b87a1509d3e3
Co-authored-by: Amp <amp@ampcode.com>
@figtracer figtracer force-pushed the lint/incorrect-erc20-interface branch from 7c5ed2b to 8035434 Compare April 23, 2026 17:55
@figtracer figtracer requested a review from mablr April 23, 2026 20:48
Comment on lines +34 to +44
// SHOULD PASS: Interface named IERC20 with correct function signatures
interface IERC20NamedCorrect {
function transfer(address to, uint256 value) external returns (bool);
function balanceOf(address account) external view returns (uint256);
}

// SHOULD PASS: Contract that is NOT named ERC20 and does not inherit from one
interface INotERC20 {
function transfer(address to, uint256 value) external returns (uint256);
function balanceOf(address account) external view returns (bool);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two take the same path: not ERC20/IERC20

IERC20NamedCorrect should be renamed IERC20 ?

@figtracer figtracer disabled auto-merge April 24, 2026 07:42
mablr
mablr previously approved these changes Apr 24, 2026
Copy link
Copy Markdown
Collaborator

@mablr mablr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@figtracer
Copy link
Copy Markdown
Collaborator Author

figtracer commented Apr 24, 2026

lgtm

i changed this back for consistency with the ERC721 one, but we can follow-up with this if really needed (?)

the goal was to not need 2 files, i believe this approach is correct as well @mablr

Copy link
Copy Markdown
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

@mablr mablr merged commit 38072ea into foundry-rs:master Apr 24, 2026
16 checks passed
@github-project-automation github-project-automation Bot moved this to Done in Foundry Apr 24, 2026
@figtracer figtracer deleted the lint/incorrect-erc20-interface branch April 24, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants