feat(lint): add incorrect ERC20 interface lint

crates/lint/README.md

@@ -13,6 +13,7 @@ It helps enforce best practices and improve code quality within Foundry projects
   - `erc20-unchecked-transfer`: ERC20 `transfer` and `transferFrom` calls should check the return value.
 - **Medium Severity:**
   - `divide-before-multiply`: Warns against performing division before multiplication in the same expression, which can cause precision loss.
+  - `incorrect-erc20-interface`: Flags ERC20 interfaces and implementations with non-compliant function signatures.
   - `incorrect-erc721-interface`: Flags ERC721 interfaces and implementations with non-compliant function signatures.
   - `unsafe-typecast`: Typecasts that can truncate values should be checked.
 - **Informational / Style Guide:**

crates/lint/src/sol/med/incorrect_erc20_interface.rs

@@ -0,0 +1,112 @@
+use super::IncorrectERC20Interface;
+use crate::{
+    linter::{LateLintPass, LintContext},
+    sol::{Severity, SolLint},
+};
+use solar::sema::hir;
+
+declare_forge_lint!(
+    INCORRECT_ERC20_INTERFACE,
+    Severity::Med,
+    "incorrect-erc20-interface",
+    "incorrect ERC20 function interface"
+);
+
+impl<'hir> LateLintPass<'hir> for IncorrectERC20Interface {
+    fn check_contract(
+        &mut self,
+        ctx: &LintContext,
+        hir: &'hir hir::Hir<'hir>,
+        contract: &'hir hir::Contract<'hir>,
+    ) {
+        // Check if the contract is a possible ERC20 by name or inheritance.
+        let is_erc20 = contract.linearized_bases.iter().any(|base_id| {
+            let name = hir.contract(*base_id).name.as_str();
+            name == "ERC20" || name == "IERC20"
+        });
+
+        if !is_erc20 {
+            return;
+        }
+
+        // If this contract implements a function from ERC721, we can assume it is an ERC721 token.
+        // These tokens offer functions which are similar to ERC20, but are not compatible.
+        let is_erc721 = contract.linearized_bases.iter().any(|base_id| {
+            let name = hir.contract(*base_id).name.as_str();
+            name == "ERC721" || name == "IERC721"
+        });
+
+        if is_erc721 {
+            return;
+        }
+
+        // Check each function in the contract for incorrect ERC20 signatures.
+        for item_id in contract.items {
+            let Some(fid) = item_id.as_function() else { continue };
+            let func = hir.function(fid);
+
+            if !func.kind.is_function() {
+                continue;
+            }
+
+            let Some(name) = func.name else { continue };
+
+            if has_incorrect_erc20_signature(hir, name.as_str(), func.parameters, func.returns) {
+                ctx.emit(&INCORRECT_ERC20_INTERFACE, func.span);
+            }
+        }
+    }
+}
+
+/// Checks if a function signature does not match the expected ERC20 specification.
+///
+/// Returns `true` if the function name and parameter types match an ERC20 function but the return
+/// types are incorrect.
+fn has_incorrect_erc20_signature(
+    hir: &hir::Hir<'_>,
+    name: &str,
+    parameters: &[hir::VariableId],
+    returns: &[hir::VariableId],
+) -> bool {
+    let is_type = |var_id: hir::VariableId, type_str: &str| {
+        matches!(
+            &hir.variable(var_id).ty.kind,
+            hir::TypeKind::Elementary(ty) if ty.to_abi_str() == type_str
+        )
+    };
+
+    let params_match = |params: &[hir::VariableId], expected: &[&str]| -> bool {
+        params.len() == expected.len()
+            && params.iter().zip(expected).all(|(&id, &ty)| is_type(id, ty))
+    };
+
+    let returns_match = |rets: &[hir::VariableId], expected: &[&str]| -> bool {
+        rets.len() == expected.len() && rets.iter().zip(expected).all(|(&id, &ty)| is_type(id, ty))
+    };
+
+    match name {
+        // function transfer(address,uint256) external returns (bool)
+        "transfer" if params_match(parameters, &["address", "uint256"]) => {
+            !returns_match(returns, &["bool"])
+        }
+        // function transferFrom(address,address,uint256) external returns (bool)
+        "transferFrom" if params_match(parameters, &["address", "address", "uint256"]) => {
+            !returns_match(returns, &["bool"])
+        }
+        // function approve(address,uint256) external returns (bool)
+        "approve" if params_match(parameters, &["address", "uint256"]) => {
+            !returns_match(returns, &["bool"])
+        }
+        // function allowance(address,address) external view returns (uint256)
+        "allowance" if params_match(parameters, &["address", "address"]) => {
+            !returns_match(returns, &["uint256"])
+        }
+        // function balanceOf(address) external view returns (uint256)
+        "balanceOf" if params_match(parameters, &["address"]) => {
+            !returns_match(returns, &["uint256"])
+        }
+        // function totalSupply() external view returns (uint256)
+        "totalSupply" if params_match(parameters, &[]) => !returns_match(returns, &["uint256"]),
+        _ => false,
+    }
+}

crates/lint/src/sol/med/mod.rs

@@ -3,6 +3,9 @@ use crate::sol::{EarlyLintPass, LateLintPass, SolLint};
 mod div_mul;
 use div_mul::DIVIDE_BEFORE_MULTIPLY;
 
+mod incorrect_erc20_interface;
+use incorrect_erc20_interface::INCORRECT_ERC20_INTERFACE;
+
 mod incorrect_erc721_interface;
 use incorrect_erc721_interface::INCORRECT_ERC721_INTERFACE;
 
@@ -11,6 +14,7 @@ use unsafe_typecast::UNSAFE_TYPECAST;
 
 register_lints!(
     (DivideBeforeMultiply, early, (DIVIDE_BEFORE_MULTIPLY)),
+    (IncorrectERC20Interface, late, (INCORRECT_ERC20_INTERFACE)),
     (IncorrectERC721Interface, late, (INCORRECT_ERC721_INTERFACE)),
     (UnsafeTypecast, late, (UNSAFE_TYPECAST))
 );

crates/lint/testdata/IncorrectERC20Interface.sol

@@ -0,0 +1,44 @@
+//@compile-flags: --severity high med low info
+
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.18;
+
+interface IERC20 {}
+
+// SHOULD FAIL: Interface named ERC20 with incorrect function signatures
+interface ERC20 {
+    function transfer(address to, uint256 value) external returns (uint256); //~WARN: incorrect ERC20 function interface
+    function approve(address spender, uint256 value) external returns (uint256); //~WARN: incorrect ERC20 function interface
+}
+
+// SHOULD FAIL: Interface inheriting from IERC20 with incorrect function signatures
+interface IERC20Incorrect is IERC20 {
+    function transfer(address to, uint256 value) external returns (uint256); //~WARN: incorrect ERC20 function interface
+    function transferFrom(address from, address to, uint256 value) external returns (uint256); //~WARN: incorrect ERC20 function interface
+    function approve(address spender, uint256 value) external returns (uint256); //~WARN: incorrect ERC20 function interface
+    function allowance(address owner, address spender) external view returns (bool); //~WARN: incorrect ERC20 function interface
+    function balanceOf(address account) external view returns (bool); //~WARN: incorrect ERC20 function interface
+    function totalSupply() external view returns (bool); //~WARN: incorrect ERC20 function interface
+}
+
+// SHOULD PASS: Correct ERC20 interface inheriting from IERC20
+interface IERC20Correct is IERC20 {
+    function transfer(address to, uint256 value) external returns (bool);
+    function transferFrom(address from, address to, uint256 value) external returns (bool);
+    function approve(address spender, uint256 value) external returns (bool);
+    function allowance(address owner, address spender) external view returns (uint256);
+    function balanceOf(address account) external view returns (uint256);
+    function totalSupply() external view returns (uint256);
+}
+
+// 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);
+}

crates/lint/testdata/IncorrectERC20Interface.stderr

@@ -0,0 +1,120 @@
+note[interface-naming]: interface names should be prefixed with 'I'
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │ interface ERC20 {
+   │           ━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#interface-naming
+
+note[multi-contract-file]: prefer having only one contract, interface or library per file
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │ interface IERC20 {}
+   │           ━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file
+
+note[multi-contract-file]: prefer having only one contract, interface or library per file
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │ interface ERC20 {
+   │           ━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file
+
+note[multi-contract-file]: prefer having only one contract, interface or library per file
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │ interface IERC20Incorrect is IERC20 {
+   │           ━━━━━━━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file
+
+note[multi-contract-file]: prefer having only one contract, interface or library per file
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │ interface IERC20Correct is IERC20 {
+   │           ━━━━━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file
+
+note[multi-contract-file]: prefer having only one contract, interface or library per file
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │ interface IERC20NamedCorrect {
+   │           ━━━━━━━━━━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file
+
+note[multi-contract-file]: prefer having only one contract, interface or library per file
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │ interface INotERC20 {
+   │           ━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file
+
+warning[incorrect-erc20-interface]: incorrect ERC20 function interface
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │     function transfer(address to, uint256 value) external returns (uint256);
+   │     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#incorrect-erc20-interface
+
+warning[incorrect-erc20-interface]: incorrect ERC20 function interface
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │     function approve(address spender, uint256 value) external returns (uint256);
+   │     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#incorrect-erc20-interface
+
+warning[incorrect-erc20-interface]: incorrect ERC20 function interface
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │     function transfer(address to, uint256 value) external returns (uint256);
+   │     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#incorrect-erc20-interface
+
+warning[incorrect-erc20-interface]: incorrect ERC20 function interface
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │     function transferFrom(address from, address to, uint256 value) external returns (uint256);
+   │     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#incorrect-erc20-interface
+
+warning[incorrect-erc20-interface]: incorrect ERC20 function interface
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │     function approve(address spender, uint256 value) external returns (uint256);
+   │     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#incorrect-erc20-interface
+
+warning[incorrect-erc20-interface]: incorrect ERC20 function interface
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │     function allowance(address owner, address spender) external view returns (bool);
+   │     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#incorrect-erc20-interface
+
+warning[incorrect-erc20-interface]: incorrect ERC20 function interface
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │     function balanceOf(address account) external view returns (bool);
+   │     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#incorrect-erc20-interface
+
+warning[incorrect-erc20-interface]: incorrect ERC20 function interface
+   ╭▸ ROOT/testdata/IncorrectERC20Interface.sol:LL:CC
+   │
+LL │     function totalSupply() external view returns (bool);
+   │     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+   │
+   ╰ help: https://book.getfoundry.sh/reference/forge/forge-lint#incorrect-erc20-interface
+