mirror of
https://github.com/crytic/slither.git
synced 2026-10-12 02:12:39 +00:00
perf: cache derived_contracts lookup in compilation unit (#2925)
* perf: cache derived_contracts lookup in compilation unit Add derived_contracts_map property to SlitherCompilationUnit that builds a reverse inheritance mapping (parent -> children) once and caches it. Update Contract.derived_contracts to use this map for O(1) lookup instead of O(n) iteration over all contracts. Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: use full inheritance chain in derived_contracts_map The optimization incorrectly used `immediate_inheritance` (direct parents only) instead of `inheritance` (full chain). This changed the semantics of `derived_contracts` from returning all descendants to only direct children, breaking the external_function detector. For inheritance A <- B <- C: - Before: A.derived_contracts returned [B] (bug) - After: A.derived_contracts returns [B, C] (correct) Co-Authored-By: Claude Opus 4.5 <[email protected]> * test: add tests for derived_contracts behavior Verifies that Contract.derived_contracts returns all descendants, not just direct children. Tests cover: - Diamond inheritance (Base <- Child1, Child2 <- Grandchild) - Deep chains (L1 <- L2 <- L3 <- L4) - No duplicates in results - Empty results for leaf contracts - Documents difference between immediate_inheritance and inheritance Co-Authored-By: Claude Opus 4.5 <[email protected]> --------- Co-authored-by: Claude Opus 4.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.5
parent
722e1a6e39
commit
f05f5478e8
@@ -77,6 +77,9 @@ class SlitherCompilationUnit(Context):
|
||||
|
||||
self._contract_with_missing_inheritance: set[Contract] = set()
|
||||
|
||||
# Reverse inheritance map: parent -> list of all derived contracts
|
||||
self._derived_contracts_map: dict[Contract, list[Contract]] | None = None
|
||||
|
||||
self._source_units: dict[int, str] = {}
|
||||
|
||||
self.counter_slithir_tuple = 0
|
||||
@@ -271,6 +274,24 @@ class SlitherCompilationUnit(Context):
|
||||
def contracts_with_missing_inheritance(self) -> set[Contract]:
|
||||
return self._contract_with_missing_inheritance
|
||||
|
||||
def _build_derived_contracts_map(self) -> dict[Contract, list[Contract]]:
|
||||
"""Build reverse inheritance mapping for O(1) derived_contracts lookup."""
|
||||
from collections import defaultdict
|
||||
|
||||
derived_map: dict[Contract, list[Contract]] = defaultdict(list)
|
||||
for contract in self.contracts:
|
||||
for parent in contract.inheritance:
|
||||
if contract not in derived_map[parent]:
|
||||
derived_map[parent].append(contract)
|
||||
return dict(derived_map)
|
||||
|
||||
@property
|
||||
def derived_contracts_map(self) -> dict[Contract, list[Contract]]:
|
||||
"""Cached reverse inheritance map: parent contract -> list of all derived contracts."""
|
||||
if self._derived_contracts_map is None:
|
||||
self._derived_contracts_map = self._build_derived_contracts_map()
|
||||
return self._derived_contracts_map
|
||||
|
||||
# endregion
|
||||
###################################################################################
|
||||
###################################################################################
|
||||
|
||||
@@ -840,8 +840,7 @@ class Contract(SourceMapping):
|
||||
"""
|
||||
list(Contract): Return the list of contracts derived from self
|
||||
"""
|
||||
candidates = self.compilation_unit.contracts
|
||||
return [c for c in candidates if self in c.inheritance] # type: ignore
|
||||
return self.compilation_unit.derived_contracts_map.get(self, [])
|
||||
|
||||
# endregion
|
||||
###################################################################################
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
// SPDX-License-Identifier: MIT
|
||||
pragma solidity ^0.8.0;
|
||||
|
||||
// Test inheritance patterns for derived_contracts property
|
||||
//
|
||||
// Inheritance graph:
|
||||
//
|
||||
// Base
|
||||
// / \
|
||||
// Child1 Child2
|
||||
// \ /
|
||||
// Grandchild
|
||||
//
|
||||
// Additionally:
|
||||
// Standalone (no inheritance)
|
||||
// Deep chain: L1 <- L2 <- L3 <- L4
|
||||
|
||||
contract Base {
|
||||
uint public baseValue;
|
||||
}
|
||||
|
||||
contract Child1 is Base {
|
||||
uint public child1Value;
|
||||
}
|
||||
|
||||
contract Child2 is Base {
|
||||
uint public child2Value;
|
||||
}
|
||||
|
||||
contract Grandchild is Child1, Child2 {
|
||||
uint public grandchildValue;
|
||||
}
|
||||
|
||||
contract Standalone {
|
||||
uint public standaloneValue;
|
||||
}
|
||||
|
||||
// Deep inheritance chain
|
||||
contract L1 {
|
||||
uint public l1Value;
|
||||
}
|
||||
|
||||
contract L2 is L1 {
|
||||
uint public l2Value;
|
||||
}
|
||||
|
||||
contract L3 is L2 {
|
||||
uint public l3Value;
|
||||
}
|
||||
|
||||
contract L4 is L3 {
|
||||
uint public l4Value;
|
||||
}
|
||||
@@ -0,0 +1,118 @@
|
||||
"""
|
||||
Tests for Contract.derived_contracts property.
|
||||
|
||||
Verifies that derived_contracts returns ALL contracts that inherit from self,
|
||||
not just direct children. This is important for detectors like external_function
|
||||
that need to find all overrides across the inheritance hierarchy.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from slither import Slither
|
||||
|
||||
TEST_DATA_DIR = Path(__file__).resolve().parent / "test_data" / "derived_contracts"
|
||||
|
||||
|
||||
def test_derived_contracts_includes_all_descendants(solc_binary_path):
|
||||
"""Test that derived_contracts returns all descendants, not just direct children.
|
||||
|
||||
For inheritance: Base <- Child1 <- Grandchild
|
||||
Base.derived_contracts should return [Child1, Child2, Grandchild]
|
||||
"""
|
||||
solc_path = solc_binary_path("0.8.0")
|
||||
slither = Slither(Path(TEST_DATA_DIR, "derived_contracts.sol").as_posix(), solc=solc_path)
|
||||
|
||||
base = slither.get_contract_from_name("Base")[0]
|
||||
derived = base.derived_contracts
|
||||
derived_names = {c.name for c in derived}
|
||||
|
||||
# Base should have Child1, Child2, AND Grandchild as derived contracts
|
||||
assert "Child1" in derived_names, "Child1 should be in Base.derived_contracts"
|
||||
assert "Child2" in derived_names, "Child2 should be in Base.derived_contracts"
|
||||
assert "Grandchild" in derived_names, (
|
||||
"Grandchild should be in Base.derived_contracts (indirect descendant)"
|
||||
)
|
||||
|
||||
|
||||
def test_derived_contracts_direct_children(solc_binary_path):
|
||||
"""Test that direct children are included in derived_contracts."""
|
||||
solc_path = solc_binary_path("0.8.0")
|
||||
slither = Slither(Path(TEST_DATA_DIR, "derived_contracts.sol").as_posix(), solc=solc_path)
|
||||
|
||||
child1 = slither.get_contract_from_name("Child1")[0]
|
||||
derived = child1.derived_contracts
|
||||
derived_names = {c.name for c in derived}
|
||||
|
||||
# Child1's only derived contract is Grandchild
|
||||
assert "Grandchild" in derived_names
|
||||
|
||||
|
||||
def test_derived_contracts_deep_chain(solc_binary_path):
|
||||
"""Test derived_contracts with a deep inheritance chain: L1 <- L2 <- L3 <- L4.
|
||||
|
||||
L1.derived_contracts should return [L2, L3, L4] - all descendants.
|
||||
"""
|
||||
solc_path = solc_binary_path("0.8.0")
|
||||
slither = Slither(Path(TEST_DATA_DIR, "derived_contracts.sol").as_posix(), solc=solc_path)
|
||||
|
||||
l1 = slither.get_contract_from_name("L1")[0]
|
||||
derived = l1.derived_contracts
|
||||
derived_names = {c.name for c in derived}
|
||||
|
||||
assert "L2" in derived_names, "L2 should be in L1.derived_contracts"
|
||||
assert "L3" in derived_names, "L3 should be in L1.derived_contracts (2 levels deep)"
|
||||
assert "L4" in derived_names, "L4 should be in L1.derived_contracts (3 levels deep)"
|
||||
|
||||
|
||||
def test_derived_contracts_no_descendants(solc_binary_path):
|
||||
"""Test that contracts with no children return empty derived_contracts."""
|
||||
solc_path = solc_binary_path("0.8.0")
|
||||
slither = Slither(Path(TEST_DATA_DIR, "derived_contracts.sol").as_posix(), solc=solc_path)
|
||||
|
||||
standalone = slither.get_contract_from_name("Standalone")[0]
|
||||
assert standalone.derived_contracts == []
|
||||
|
||||
grandchild = slither.get_contract_from_name("Grandchild")[0]
|
||||
assert grandchild.derived_contracts == []
|
||||
|
||||
l4 = slither.get_contract_from_name("L4")[0]
|
||||
assert l4.derived_contracts == []
|
||||
|
||||
|
||||
def test_derived_contracts_no_duplicates(solc_binary_path):
|
||||
"""Test that derived_contracts doesn't contain duplicates.
|
||||
|
||||
In diamond inheritance (Base <- Child1, Child2 <- Grandchild),
|
||||
Grandchild inherits Base through two paths, but should only appear once.
|
||||
"""
|
||||
solc_path = solc_binary_path("0.8.0")
|
||||
slither = Slither(Path(TEST_DATA_DIR, "derived_contracts.sol").as_posix(), solc=solc_path)
|
||||
|
||||
base = slither.get_contract_from_name("Base")[0]
|
||||
derived = base.derived_contracts
|
||||
derived_names = [c.name for c in derived]
|
||||
|
||||
# Check no duplicates
|
||||
assert len(derived_names) == len(set(derived_names)), "derived_contracts has duplicates"
|
||||
|
||||
|
||||
def test_immediate_vs_full_inheritance(solc_binary_path):
|
||||
"""Verify the difference between immediate_inheritance and inheritance.
|
||||
|
||||
This documents the expected behavior that derived_contracts uses the full
|
||||
inheritance chain, not just immediate parents.
|
||||
"""
|
||||
solc_path = solc_binary_path("0.8.0")
|
||||
slither = Slither(Path(TEST_DATA_DIR, "derived_contracts.sol").as_posix(), solc=solc_path)
|
||||
|
||||
grandchild = slither.get_contract_from_name("Grandchild")[0]
|
||||
|
||||
# immediate_inheritance = direct parents only
|
||||
immediate_names = {c.name for c in grandchild.immediate_inheritance}
|
||||
assert immediate_names == {"Child1", "Child2"}
|
||||
|
||||
# inheritance = full chain (C3 linearization)
|
||||
full_names = {c.name for c in grandchild.inheritance}
|
||||
assert "Base" in full_names, "Base should be in full inheritance chain"
|
||||
assert "Child1" in full_names
|
||||
assert "Child2" in full_names
|
||||
Reference in New Issue
Block a user