diff --git a/slither/core/compilation_unit.py b/slither/core/compilation_unit.py index 6221fd8bd..df652dab0 100644 --- a/slither/core/compilation_unit.py +++ b/slither/core/compilation_unit.py @@ -302,10 +302,7 @@ class SlitherCompilationUnit(Context): slot = 0 offset = 0 - for var in contract.state_variables_ordered: - if var.is_constant or var.is_immutable: - continue - + for var in contract.stored_state_variables_ordered: assert var.type size, new_slot = var.type.storage_size diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py index 5403d227a..7224859c6 100644 --- a/slither/core/declarations/contract.py +++ b/slither/core/declarations/contract.py @@ -436,6 +436,33 @@ class Contract(SourceMapping): # pylint: disable=too-many-public-methods """ return list(self._variables.values()) + @property + def stored_state_variables(self) -> List["StateVariable"]: + """ + Returns state variables with storage locations, excluding private variables from inherited contracts. + Use stored_state_variables_ordered to access variables with storage locations in their declaration order. + + This implementation filters out state variables if they are constant or immutable. It will be + updated to accommodate any new non-storage keywords that might replace 'constant' and 'immutable' in the future. + + Returns: + List[StateVariable]: A list of state variables with storage locations. + """ + return [variable for variable in self.state_variables if variable.is_stored] + + @property + def stored_state_variables_ordered(self) -> List["StateVariable"]: + """ + list(StateVariable): List of the state variables with storage locations by order of declaration. + + This implementation filters out state variables if they are constant or immutable. It will be + updated to accommodate any new non-storage keywords that might replace 'constant' and 'immutable' in the future. + + Returns: + List[StateVariable]: A list of state variables with storage locations ordered by declaration. + """ + return [variable for variable in self.state_variables_ordered if variable.is_stored] + @property def state_variables_entry_points(self) -> List["StateVariable"]: """ diff --git a/slither/core/variables/variable.py b/slither/core/variables/variable.py index f9ef19024..63d1a7a83 100644 --- a/slither/core/variables/variable.py +++ b/slither/core/variables/variable.py @@ -93,6 +93,13 @@ class Variable(SourceMapping): def is_constant(self, is_cst: bool) -> None: self._is_constant = is_cst + @property + def is_stored(self) -> bool: + """ + Checks if a variable is stored, based on it not being constant or immutable. Future updates may adjust for new non-storage keywords. + """ + return not self._is_constant and not self._is_immutable + @property def is_reentrant(self) -> bool: return self._is_reentrant diff --git a/slither/detectors/variables/unchanged_state_variables.py b/slither/detectors/variables/unchanged_state_variables.py index 0e73ab57b..5771d9630 100644 --- a/slither/detectors/variables/unchanged_state_variables.py +++ b/slither/detectors/variables/unchanged_state_variables.py @@ -25,7 +25,7 @@ def _is_valid_type(v: StateVariable) -> bool: def _valid_candidate(v: StateVariable) -> bool: - return _is_valid_type(v) and not (v.is_constant or v.is_immutable) + return _is_valid_type(v) def _is_constant_var(v: Variable) -> bool: @@ -92,7 +92,7 @@ class UnchangedStateVariables: variables = [] functions = [] - variables.append(c.state_variables) + variables.append(c.stored_state_variables) functions.append(c.all_functions_called) valid_candidates: Set[StateVariable] = { diff --git a/slither/printers/summary/variable_order.py b/slither/printers/summary/variable_order.py index 3325b7a01..0d8ce2612 100644 --- a/slither/printers/summary/variable_order.py +++ b/slither/printers/summary/variable_order.py @@ -28,10 +28,9 @@ class VariableOrder(AbstractPrinter): for contract in self.slither.contracts_derived: txt += f"\n{contract.name}:\n" table = MyPrettyTable(["Name", "Type", "Slot", "Offset"]) - for variable in contract.state_variables_ordered: - if not variable.is_constant and not variable.is_immutable: - slot, offset = contract.compilation_unit.storage_layout_of(contract, variable) - table.add_row([variable.canonical_name, str(variable.type), slot, offset]) + for variable in contract.stored_state_variables_ordered: + slot, offset = contract.compilation_unit.storage_layout_of(contract, variable) + table.add_row([variable.canonical_name, str(variable.type), slot, offset]) all_tables.append((contract.name, table)) txt += str(table) + "\n" diff --git a/slither/tools/read_storage/read_storage.py b/slither/tools/read_storage/read_storage.py index 8c0cf515d..728636f2e 100644 --- a/slither/tools/read_storage/read_storage.py +++ b/slither/tools/read_storage/read_storage.py @@ -398,7 +398,7 @@ class SlitherReadStorage: for contract in self.contracts: for var in contract.state_variables_ordered: if func(var): - if not var.is_constant and not var.is_immutable: + if var.is_stored: self._target_variables.append((contract, var)) elif ( self.unstructured diff --git a/slither/tools/upgradeability/checks/variable_initialization.py b/slither/tools/upgradeability/checks/variable_initialization.py index b4535ddfe..b86036c87 100644 --- a/slither/tools/upgradeability/checks/variable_initialization.py +++ b/slither/tools/upgradeability/checks/variable_initialization.py @@ -43,8 +43,8 @@ Using initialize functions to write initial values in state variables. def _check(self) -> List[Output]: results = [] - for s in self.contract.state_variables_ordered: - if s.initialized and not (s.is_constant or s.is_immutable): + for s in self.contract.stored_state_variables_ordered: + if s.initialized: info: CHECK_INFO = [s, " is a state variable with an initial value.\n"] json = self.generate_result(info) results.append(json) diff --git a/slither/tools/upgradeability/checks/variables_order.py b/slither/tools/upgradeability/checks/variables_order.py index 002559b6e..8d525a6dd 100644 --- a/slither/tools/upgradeability/checks/variables_order.py +++ b/slither/tools/upgradeability/checks/variables_order.py @@ -115,16 +115,8 @@ Avoid variables in the proxy. If a variable is in the proxy, ensure it has the s def _check(self) -> List[Output]: contract1 = self._contract1() contract2 = self._contract2() - order1 = [ - variable - for variable in contract1.state_variables_ordered - if not (variable.is_constant or variable.is_immutable) - ] - order2 = [ - variable - for variable in contract2.state_variables_ordered - if not (variable.is_constant or variable.is_immutable) - ] + order1 = contract1.stored_state_variables_ordered + order2 = contract2.stored_state_variables_ordered results: List[Output] = [] for idx, _ in enumerate(order1): @@ -244,16 +236,8 @@ Avoid variables in the proxy. If a variable is in the proxy, ensure it has the s def _check(self) -> List[Output]: contract1 = self._contract1() contract2 = self._contract2() - order1 = [ - variable - for variable in contract1.state_variables_ordered - if not (variable.is_constant or variable.is_immutable) - ] - order2 = [ - variable - for variable in contract2.state_variables_ordered - if not (variable.is_constant or variable.is_immutable) - ] + order1 = contract1.stored_state_variables_ordered + order2 = contract2.stored_state_variables_ordered results = [] diff --git a/slither/utils/encoding.py b/slither/utils/encoding.py index 288b58150..86e26cdbe 100644 --- a/slither/utils/encoding.py +++ b/slither/utils/encoding.py @@ -71,7 +71,7 @@ def encode_var_for_compare(var: Union[variables.Variable, SolidityVariable]) -> if isinstance(var, variables.LocalVariable): return f"local_solc_variable({ntype(var.type)},{var.location})" if isinstance(var, variables.StateVariable): - if not (var.is_constant or var.is_immutable): + if var.is_stored: try: slot, _ = var.contract.compilation_unit.storage_layout_of(var.contract, var) except KeyError: diff --git a/slither/utils/upgradeability.py b/slither/utils/upgradeability.py index bbb253175..dd7bfe465 100644 --- a/slither/utils/upgradeability.py +++ b/slither/utils/upgradeability.py @@ -81,12 +81,8 @@ def compare( tainted-contracts: list[TaintedExternalContract] """ - order_vars1 = [ - v for v in v1.state_variables_ordered if not v.is_constant and not v.is_immutable - ] - order_vars2 = [ - v for v in v2.state_variables_ordered if not v.is_constant and not v.is_immutable - ] + order_vars1 = v1.stored_state_variables_ordered + order_vars2 = v2.stored_state_variables_ordered func_sigs1 = [function.solidity_signature for function in v1.functions] func_sigs2 = [function.solidity_signature for function in v2.functions] @@ -206,7 +202,7 @@ def tainted_external_contracts(funcs: List[Function]) -> List[TaintedExternalCon elif ( isinstance(target, StateVariable) and target not in (v for v in tainted_contracts[contract.name].tainted_variables) - and not (target.is_constant or target.is_immutable) + and target.is_stored ): # Found a new high-level call to a public state variable getter tainted_contracts[contract.name].add_tainted_variable(target) @@ -304,12 +300,8 @@ def get_missing_vars(v1: Contract, v2: Contract) -> List[StateVariable]: List of StateVariables from v1 missing in v2 """ results = [] - order_vars1 = [ - v for v in v1.state_variables_ordered if not v.is_constant and not v.is_immutable - ] - order_vars2 = [ - v for v in v2.state_variables_ordered if not v.is_constant and not v.is_immutable - ] + order_vars1 = v1.stored_state_variables_ordered + order_vars2 = v2.stored_state_variables_ordered if len(order_vars2) < len(order_vars1): for variable in order_vars1: if variable.name not in [v.name for v in order_vars2]: @@ -366,7 +358,7 @@ def get_proxy_implementation_slot(proxy: Contract) -> Optional[SlotInfo]: delegate = get_proxy_implementation_var(proxy) if isinstance(delegate, StateVariable): - if not delegate.is_constant and not delegate.is_immutable: + if delegate.is_stored: srs = SlitherReadStorage([proxy], 20) return srs.get_storage_slot(delegate, proxy) if delegate.is_constant and delegate.type.name == "bytes32":