Skip to content

Conversation

IzaiahSun
Copy link

What is happening in the previous version

function onERC721Received(
      address,
      address from,
      uint256,
      bytes calldata
  ) external returns (bytes4) {
      require(_opened, "Already yet open.");
      require(msg.sender == address(erc721Contract), "Unauthorized token");
      _safeMint(from, ERC721_RATIO); // Adjust minting based on the ERC721_RATIO
      return this.onERC721Received.selector;
}

If I'm using Slither to analyze the above code, the SSA variables will be generated for the parameter from in slither/slithir/utils/ssa.py line 127-135 and 162-172:

init_definition = {}
    for v in function.parameters:
        if v.name:
            init_definition[v.name] = (v, function.entry_point)
            function.entry_point.add_ssa_ir(Phi(LocalIRVariable(v), set()))

    for v in function.returns:
        if v.name:
            init_definition[v.name] = (v, function.entry_point)
for v in function.parameters:
    if v.name:
        new_var = LocalIRVariable(v)
        function.add_parameter_ssa(new_var)
        if new_var.is_storage:
            fake_variable = LocalIRVariable(v)
            fake_variable.name = "STORAGE_" + fake_variable.name
            fake_variable.set_location("reference_to_storage")
            new_var.refers_to = {fake_variable}
            init_local_variables_instances[fake_variable.name] = fake_variable
        init_local_variables_instances[v.name] = new_var

In this case, even a parameter is never written in a function, it will have two SSA variable, one is for the parameter (in the given case is from_0), and another for the usage (from_1).

When building the data dependency of SSA variables in slither/analyses/data_dependency/data_dependency.py, function compute_dependency_function, it will not consider such dependency. So from_1 will not have dependency with from_0, which is wrong.

How I solved this

I added some code into slither/analyses/data_dependency/data_dependency.py, function compute_dependency_function. When a function is analyzed, I will record all the SSA variables that are never written (used as lvalue) but used (as rvalue). If these variables' non-ssa version is the same with parameters, I will add a dependency edge between them.

The new test case is provided a proof-of-concept.

@IzaiahSun IzaiahSun requested a review from smonicas as a code owner May 12, 2025 13:35
@CLAassistant
Copy link

CLAassistant commented May 12, 2025

CLA assistant check
All committers have signed the CLA.

@IzaiahSun
Copy link
Author

Dear maintainers, this linter error seems to be the issue of GitHub's authentication, not the code. Could you help check this?
image

@smonicas
Copy link
Collaborator

Hi, it should be due to the warnings you see in the screenshot. If you run pylint slither --rcfile pyproject.toml locally you should get them.

@IzaiahSun
Copy link
Author

Hi, it should be due to the warnings you see in the screenshot. If you run pylint slither --rcfile pyproject.toml locally you should get them.

Hi, I have run this locally, I have the following output, and no such warnings are shown.

(.venv) ➜  slither git:(dev) pylint slither --rcfile pyproject.toml
************* Module slither.tools.upgradeability.__main__
slither/tools/upgradeability/__main__.py:175:16: W0719: Raising too general exception: Exception (broad-exception-raised)
************* Module slither.tools.upgradeability.checks.constant
slither/tools/upgradeability/checks/constant.py:59:12: W0719: Raising too general exception: Exception (broad-exception-raised)
slither/tools/upgradeability/checks/constant.py:148:12: W0719: Raising too general exception: Exception (broad-exception-raised)
************* Module slither.tools.flattening.flattening
slither/tools/flattening/flattening.py:51:4: R0917: Too many positional arguments (8/5) (too-many-positional-arguments)
slither/tools/flattening/flattening.py:272:4: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
slither/tools/flattening/flattening.py:456:4: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.tools.documentation.__main__
slither/tools/documentation/__main__.py:141:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
slither/tools/documentation/__main__.py:213:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.tools.slither_format.slither_format
slither/tools/slither_format/slither_format.py:113:16: W0719: Raising too general exception: Exception (broad-exception-raised)
************* Module slither.tools.properties.platforms.truffle
slither/tools/properties/platforms/truffle.py:71:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.tools.mutator.utils.patch
slither/tools/mutator/utils/patch.py:6:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.tools.mutator.utils.testing_generated_mutant
slither/tools/mutator/utils/testing_generated_mutant.py:85:0: R0917: Too many positional arguments (8/5) (too-many-positional-arguments)
************* Module slither.tools.mutator.mutators.abstract_mutator
slither/tools/mutator/mutators/abstract_mutator.py:24:4: R0917: Too many positional arguments (12/5) (too-many-positional-arguments)
************* Module slither.printers.summary.when_not_paused
slither/printers/summary/when_not_paused.py:15:11: E1121: Too many positional arguments for function call (too-many-function-args)
************* Module slither.printers.call.call_graph
slither/printers/call/call_graph.py:50:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
slither/printers/call/call_graph.py:113:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
slither/printers/call/call_graph.py:144:0: R0917: Too many positional arguments (8/5) (too-many-positional-arguments)
************* Module slither.slithir.operations.low_level_call
slither/slithir/operations/low_level_call.py:23:4: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.slithir.operations.internal_call
slither/slithir/operations/internal_call.py:15:4: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.slithir.operations.high_level_call
slither/slithir/operations/high_level_call.py:24:4: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.slithir.utils.ssa
slither/slithir/utils/ssa.py:216:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
slither/slithir/utils/ssa.py:416:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
slither/slithir/utils/ssa.py:486:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
slither/slithir/utils/ssa.py:589:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.slithir.tmp_operations.tmp_call
slither/slithir/tmp_operations/tmp_call.py:22:4: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.utils.code_generation
slither/utils/code_generation.py:27:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.formatters.naming_convention.naming_convention
slither/formatters/naming_convention/naming_convention.py:348:0: R0917: Too many positional arguments (8/5) (too-many-positional-arguments)
slither/formatters/naming_convention/naming_convention.py:449:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.formatters.utils.patches
slither/formatters/utils/patches.py:9:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.formatters.variables.unchanged_state_variables
slither/formatters/variables/unchanged_state_variables.py:31:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.formatters.attributes.constant_pragma
slither/formatters/attributes/constant_pragma.py:71:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.formatters.attributes.incorrect_solc
slither/formatters/attributes/incorrect_solc.py:61:0: R0917: Too many positional arguments (6/5) (too-many-positional-arguments)
************* Module slither.solc_parsing.solidity_types.type_parsing
slither/solc_parsing/solidity_types/type_parsing.py:44:0: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.solc_parsing.declarations.contract
slither/solc_parsing/declarations/contract.py:489:4: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
slither/solc_parsing/declarations/contract.py:531:4: R0917: Too many positional arguments (7/5) (too-many-positional-arguments)
************* Module slither.analyses.data_dependency.data_dependency
slither/analyses/data_dependency/data_dependency.py:436:0: R0912: Too many branches (19/12) (too-many-branches)
slither/analyses/data_dependency/data_dependency.py:465:4: R1702: Too many nested blocks (8/5) (too-many-nested-blocks)

-----------------------------------
Your code has been rated at 9.99/10

Only the last several lines are related to my changed code and they should not cause the workflow to fail.

@elopez
Copy link
Member

elopez commented May 28, 2025

Hi! These are the lint warnings that cause the workflow to fail. Once these get fixed the CI will be green 👍

Screenshot 2025-05-28 at 09 01 18

@IzaiahSun
Copy link
Author

I have done the refactor on the code, and there should be no more linter issues.

@IzaiahSun
Copy link
Author

Hi @smonicas, can we proceed to the CI/CD workflows? I have fixed the previous problems and it should work now.

@IzaiahSun
Copy link
Author

Sorry for the previous errors. I have made test locally and in my own repo's CI/CD to make sure that they work.

image image image image

@IzaiahSun
Copy link
Author

Hi Maintainers,

It seems that all the checks are passed. Is it possible to merge the PR or I need to make further changes?

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.

4 participants