From 16b57263f47b1628d8bf4a4ca08ae9d674be9375 Mon Sep 17 00:00:00 2001 From: devtooligan Date: Tue, 2 May 2023 09:54:51 -0700 Subject: [PATCH 1/5] feat: martin printer --- slither/printers/all_printers.py | 1 + slither/printers/summary/martin.py | 114 +++++++++++++++++++++++++++++ slither/utils/myprettytable.py | 39 ++++++++++ 3 files changed, 154 insertions(+) create mode 100644 slither/printers/summary/martin.py diff --git a/slither/printers/all_printers.py b/slither/printers/all_printers.py index 6dc8dddbd..c836b98d2 100644 --- a/slither/printers/all_printers.py +++ b/slither/printers/all_printers.py @@ -20,3 +20,4 @@ from .summary.evm import PrinterEVM from .summary.when_not_paused import PrinterWhenNotPaused from .summary.declaration import Declaration from .functions.dominator import Dominator +from .summary.martin import Martin diff --git a/slither/printers/summary/martin.py b/slither/printers/summary/martin.py new file mode 100644 index 000000000..1bb59c4ff --- /dev/null +++ b/slither/printers/summary/martin.py @@ -0,0 +1,114 @@ +""" + Robert "Uncle Bob" Martin - Agile software metrics + https://en.wikipedia.org/wiki/Software_package_metrics + + Efferent Coupling (Ce): Number of contracts that the contract depends on + Afferent Coupling (Ca): Number of contracts that depend on a contract + Instability (I): Ratio of efferent coupling to total coupling (Ce / (Ce + Ca)) + Abstractness (A): Number of abstract contracts / total number of contracts + Distance from the Main Sequence (D): abs(A + I - 1) + +""" +from slither.printers.abstract_printer import AbstractPrinter +from slither.slithir.operations.high_level_call import HighLevelCall +from slither.utils.myprettytable import make_pretty_table + + +def count_abstracts(contracts): + """ + Count the number of abstract contracts + Args: + contracts(list): list of contracts + Returns: + a tuple of (abstract_contract_count, total_contract_count) + """ + abstract_contract_count = 0 + for c in contracts: + if not c.is_fully_implemented: + abstract_contract_count += 1 + return (abstract_contract_count, len(contracts)) + + +def compute_coupling(contracts: list, abstractness: float) -> dict: + """ + Used to compute the coupling between contracts external calls made to internal contracts + Args: + contracts: list of contracts + Returns: + dict of contract names with dicts of the coupling metrics: + { + "contract_name1": { + "Dependents": 0, + "Dependencies": 3 + "Instability": 1.0, + "Abstractness": 0.0, + "Distance from main sequence": 1.0, + }, + "contract_name2": { + "Dependents": 1, + "Dependencies": 0 + "Instability": 0.0, + "Abstractness": 1.0, + "Distance from main sequence": 0.0, + } + """ + dependencies = {} + for contract in contracts: + for func in contract.functions: + high_level_calls = [ + ir for node in func.nodes for ir in node.irs_ssa if isinstance(ir, HighLevelCall) + ] + # convert irs to string with target function and contract name + external_calls = [h.destination.type.type.name for h in high_level_calls] + dependencies[contract.name] = set(external_calls) + dependents = {} + for contract, deps in dependencies.items(): + for dep in deps: + if dep not in dependents: + dependents[dep] = set() + dependents[dep].add(contract) + + coupling_dict = {} + for contract in contracts: + ce = len(dependencies.get(contract.name, [])) + ca = len(dependents.get(contract.name, [])) + i = 0.0 + d = 0.0 + if ce + ca > 0: + i = float(ce / (ce + ca)) + d = float(abs(i - abstractness)) + coupling_dict[contract.name] = { + "Dependents": ca, + "Dependencies": ce, + "Instability": f"{i:.2f}", + "Distance from main sequence": f"{d:.2f}", + } + return coupling_dict + + +class Martin(AbstractPrinter): + ARGUMENT = "martin" + HELP = "Martin agile software metrics (Ca, Ce, I, A, D)" + + WIKI = "https://github.com/trailofbits/slither/wiki/Printer-documentation#martin" + + def output(self, _filename): + (abstract_contract_count, total_contract_count) = count_abstracts(self.contracts) + abstractness = float(abstract_contract_count / total_contract_count) + coupling_dict = compute_coupling(self.contracts, abstractness) + + table = make_pretty_table( + ["Contract", *list(coupling_dict[self.contracts[0].name].keys())], coupling_dict + ) + txt = "Martin agile software metrics\n" + txt += "Efferent Coupling (Ce) - Number of contracts that a contract depends on\n" + txt += "Afferent Coupling (Ca) - Number of contracts that depend on the contract\n" + txt += "Instability (I) - Ratio of efferent coupling to total coupling (Ce / (Ce + Ca))\n" + txt += "Abstractness (A) - Number of abstract contracts / total number of contracts\n" + txt += "Distance from the Main Sequence (D) - abs(A + I - 1)\n" + txt += "\n" + txt += f"Abstractness (overall): {round(abstractness, 2)}\n" + str(table) + self.info(txt) + res = self.generate_output(txt) + res.add_pretty_table(table, "Code Lines") + return res diff --git a/slither/utils/myprettytable.py b/slither/utils/myprettytable.py index af10a6ff2..efdb96504 100644 --- a/slither/utils/myprettytable.py +++ b/slither/utils/myprettytable.py @@ -22,3 +22,42 @@ class MyPrettyTable: def __str__(self) -> str: return str(self.to_pretty_table()) + + +# **Dict to MyPrettyTable utility functions** + + +# Converts a dict to a MyPrettyTable. Dict keys are the row headers. +# @param headers str[] of column names +# @param body dict of row headers with a dict of the values +# @param totals bool optional add Totals row +def make_pretty_table(headers: list, body: dict, totals: bool = False) -> MyPrettyTable: + table = MyPrettyTable(headers) + for row in body: + table_row = [row] + [body[row][key] for key in headers[1:]] + table.add_row(table_row) + if totals: + table.add_row(["Total"] + [sum([body[row][key] for row in body]) for key in headers[1:]]) + return table + + +# takes a dict of dicts and returns a dict of dicts with the keys transposed +# example: +# in: +# { +# "dep": {"loc": 0, "sloc": 0, "cloc": 0}, +# "test": {"loc": 0, "sloc": 0, "cloc": 0}, +# "src": {"loc": 0, "sloc": 0, "cloc": 0}, +# } +# out: +# { +# 'loc': {'dep': 0, 'test': 0, 'src': 0}, +# 'sloc': {'dep': 0, 'test': 0, 'src': 0}, +# 'cloc': {'dep': 0, 'test': 0, 'src': 0}, +# } +def transpose(table): + any_key = list(table.keys())[0] + return { + inner_key: {outer_key: table[outer_key][inner_key] for outer_key in table} + for inner_key in table[any_key] + } From 975da91d333182b7a10d1be29993368073c1edb3 Mon Sep 17 00:00:00 2001 From: devtooligan Date: Thu, 15 Jun 2023 14:20:18 -0700 Subject: [PATCH 2/5] chore: add type --- slither/printers/summary/martin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/slither/printers/summary/martin.py b/slither/printers/summary/martin.py index 1bb59c4ff..b289a21f9 100644 --- a/slither/printers/summary/martin.py +++ b/slither/printers/summary/martin.py @@ -12,9 +12,9 @@ from slither.printers.abstract_printer import AbstractPrinter from slither.slithir.operations.high_level_call import HighLevelCall from slither.utils.myprettytable import make_pretty_table +from typing import Tuple - -def count_abstracts(contracts): +def count_abstracts(contracts) -> Tuple[int, int]: """ Count the number of abstract contracts Args: From 6f280c18b98c6ccee39f09a3f0b9381e0816ce5b Mon Sep 17 00:00:00 2001 From: devtooligan Date: Thu, 15 Jun 2023 14:36:19 -0700 Subject: [PATCH 3/5] chore: pylint --- slither/printers/summary/martin.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/slither/printers/summary/martin.py b/slither/printers/summary/martin.py index b289a21f9..693ec1575 100644 --- a/slither/printers/summary/martin.py +++ b/slither/printers/summary/martin.py @@ -9,10 +9,11 @@ Distance from the Main Sequence (D): abs(A + I - 1) """ -from slither.printers.abstract_printer import AbstractPrinter +from typing import Tuple from slither.slithir.operations.high_level_call import HighLevelCall from slither.utils.myprettytable import make_pretty_table -from typing import Tuple +from slither.printers.abstract_printer import AbstractPrinter + def count_abstracts(contracts) -> Tuple[int, int]: """ From 7cb7cb94ad00df6fe78775b656189643d896a993 Mon Sep 17 00:00:00 2001 From: alpharush <0xalpharush@protonmail.com> Date: Thu, 15 Jun 2023 17:21:44 -0500 Subject: [PATCH 4/5] inform user if inheritance cannot be resolved (#1956) * inform user if inheritance cannot be resolved * lint * update explanation --- .../slither_compilation_unit_solc.py | 11 ++++++++++- .../contract_with_duplicate_names.sol | 2 ++ .../inheritance_resolution_error/import.sol | 1 + tests/unit/core/test_error_messages.py | 18 ++++++++++++++++++ 4 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 tests/unit/core/test_data/inheritance_resolution_error/contract_with_duplicate_names.sol create mode 100644 tests/unit/core/test_data/inheritance_resolution_error/import.sol create mode 100644 tests/unit/core/test_error_messages.py diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index f4258cd41..a739c6f97 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -33,6 +33,10 @@ logger = logging.getLogger("SlitherSolcParsing") logger.setLevel(logging.INFO) +class InheritanceResolutionError(SlitherException): + pass + + def _handle_import_aliases( symbol_aliases: Dict, import_directive: Import, scope: FileScope ) -> None: @@ -432,7 +436,12 @@ Please rename it, this name is reserved for Slither's internals""" target = contract_parser.underlying_contract.file_scope.get_contract_from_name( contract_name ) - assert target + if target == contract_parser.underlying_contract: + raise InheritanceResolutionError( + "Could not resolve contract inheritance. This is likely caused by an import renaming that collides with existing names (see https://github.com/crytic/slither/issues/1758)." + f"\n Try changing `contract {target}` ({target.source_mapping}) to a unique name." + ) + assert target, f"Contract {contract_name} not found" ancestors.append(target) elif i in self._contracts_by_id: ancestors.append(self._contracts_by_id[i]) diff --git a/tests/unit/core/test_data/inheritance_resolution_error/contract_with_duplicate_names.sol b/tests/unit/core/test_data/inheritance_resolution_error/contract_with_duplicate_names.sol new file mode 100644 index 000000000..d5c6816e2 --- /dev/null +++ b/tests/unit/core/test_data/inheritance_resolution_error/contract_with_duplicate_names.sol @@ -0,0 +1,2 @@ +import {ERC20 as ERC20_1} from "./import.sol"; +contract ERC20 is ERC20_1 {} \ No newline at end of file diff --git a/tests/unit/core/test_data/inheritance_resolution_error/import.sol b/tests/unit/core/test_data/inheritance_resolution_error/import.sol new file mode 100644 index 000000000..e3221165a --- /dev/null +++ b/tests/unit/core/test_data/inheritance_resolution_error/import.sol @@ -0,0 +1 @@ +contract ERC20 {} \ No newline at end of file diff --git a/tests/unit/core/test_error_messages.py b/tests/unit/core/test_error_messages.py new file mode 100644 index 000000000..d0d915d56 --- /dev/null +++ b/tests/unit/core/test_error_messages.py @@ -0,0 +1,18 @@ +from pathlib import Path +import pytest + + +from slither import Slither +from slither.solc_parsing.slither_compilation_unit_solc import InheritanceResolutionError + +TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" +INHERITANCE_ERROR_ROOT = Path(TEST_DATA_DIR, "inheritance_resolution_error") + + +def test_inheritance_resolution_error(solc_binary_path) -> None: + with pytest.raises(InheritanceResolutionError): + solc_path = solc_binary_path("0.8.0") + Slither( + Path(INHERITANCE_ERROR_ROOT, "contract_with_duplicate_names.sol").as_posix(), + solc=solc_path, + ) From ccad54263de5802c9893bd6eb151aa68d1a2cb95 Mon Sep 17 00:00:00 2001 From: Simone <79767264+smonicas@users.noreply.github.com> Date: Fri, 16 Jun 2023 00:21:59 +0200 Subject: [PATCH 5/5] Handle if crytic-compile returns an empty ast (#1961) --- slither/solc_parsing/slither_compilation_unit_solc.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py index a739c6f97..00ac3a519 100644 --- a/slither/solc_parsing/slither_compilation_unit_solc.py +++ b/slither/solc_parsing/slither_compilation_unit_solc.py @@ -198,6 +198,13 @@ class SlitherCompilationUnitSolc(CallerContextExpression): # pylint: disable=too-many-branches,too-many-statements,too-many-locals def parse_top_level_from_loaded_json(self, data_loaded: Dict, filename: str) -> None: + if not data_loaded or data_loaded is None: + logger.error( + "crytic-compile returned an empty AST. " + "If you are trying to analyze a contract from etherscan or similar make sure it has source code available." + ) + return + if "nodeType" in data_loaded: self._is_compact_ast = True