diff --git a/aderyn_core/src/detect/detector.rs b/aderyn_core/src/detect/detector.rs index 98535c98..e8cfa3cd 100644 --- a/aderyn_core/src/detect/detector.rs +++ b/aderyn_core/src/detect/detector.rs @@ -95,6 +95,7 @@ pub fn get_all_issue_detectors() -> Vec> { Box::::default(), Box::::default(), Box::::default(), + Box::::default(), ] } @@ -186,6 +187,7 @@ pub(crate) enum IssueDetectorNamePool { UninitializedLocalVariable, ReturnBomb, OutOfOrderRetryable, + StateVariableCouldBeDeclaredConstant, // NOTE: `Undecided` will be the default name (for new bots). // If it's accepted, a new variant will be added to this enum before normalizing it in aderyn Undecided, @@ -195,6 +197,9 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option { + Some(Box::::default()) + } IssueDetectorNamePool::FunctionPointerInConstructor => { Some(Box::::default()) } diff --git a/aderyn_core/src/detect/low/mod.rs b/aderyn_core/src/detect/low/mod.rs index e398255c..1a43d140 100644 --- a/aderyn_core/src/detect/low/mod.rs +++ b/aderyn_core/src/detect/low/mod.rs @@ -24,6 +24,7 @@ pub(crate) mod require_with_string; pub(crate) mod return_bomb; pub(crate) mod reverts_and_requries_in_loops; pub(crate) mod solmate_safe_transfer_lib; +pub(crate) mod state_variable_could_be_constant; pub(crate) mod unindexed_events; pub(crate) mod uninitialized_local_variables; pub(crate) mod unsafe_erc20_functions; @@ -62,6 +63,7 @@ pub use require_with_string::RequireWithStringDetector; pub use return_bomb::ReturnBombDetector; pub use reverts_and_requries_in_loops::RevertsAndRequiresInLoopsDetector; pub use solmate_safe_transfer_lib::SolmateSafeTransferLibDetector; +pub use state_variable_could_be_constant::StateVariableCouldBeConstantDetector; pub use unindexed_events::UnindexedEventsDetector; pub use uninitialized_local_variables::UninitializedLocalVariableDetector; pub use unsafe_erc20_functions::UnsafeERC20FunctionsDetector; diff --git a/aderyn_core/src/detect/low/state_variable_could_be_constant.rs b/aderyn_core/src/detect/low/state_variable_could_be_constant.rs new file mode 100644 index 00000000..1ae04508 --- /dev/null +++ b/aderyn_core/src/detect/low/state_variable_could_be_constant.rs @@ -0,0 +1,201 @@ +use std::collections::{BTreeMap, HashSet}; +use std::error::Error; + +use crate::ast::{FunctionCallKind, Mutability, NodeID}; + +use crate::capture; +use crate::context::browser::ExtractFunctionCalls; +use crate::detect::detector::IssueDetectorNamePool; +use crate::{ + context::workspace_context::WorkspaceContext, + detect::detector::{IssueDetector, IssueSeverity}, +}; + +#[derive(Default)] +pub struct StateVariableCouldBeConstantDetector { + // Keys are: [0] source file name, [1] line number, [2] character location of node. + // Do not add items manually, use `capture!` to add nodes to this BTreeMap. + found_instances: BTreeMap<(String, usize, String), NodeID>, +} + +impl IssueDetector for StateVariableCouldBeConstantDetector { + fn detect(&mut self, context: &WorkspaceContext) -> Result> { + // PLAN + // 1. Collect all state variables that are not marked constant or immutable and are also not structs/mappings/contracts (collection A) + // 2. Investigate every function and collect all the state variables that could change (collection B) + // 3. Result = collection A - collection B + + let mut collection_a = Vec::new(); + + for variable in context.variable_declarations() { + // If we're not able to set the value upfront, then it cannot be constant + if variable.value.is_none() { + continue; + } + + if let Some(rhs_value) = variable.value.as_ref() { + let function_calls = ExtractFunctionCalls::from(rhs_value).extracted; + if function_calls + .iter() + .any(|f| f.kind == FunctionCallKind::FunctionCall) + { + continue; + } + } + + if variable.mutability() == Some(&Mutability::Immutable) { + continue; + } + + // Do not report it if it's a struct / mapping + if variable + .type_descriptions + .type_string + .as_ref() + .is_some_and(|type_string| { + type_string.starts_with("mapping") || type_string.starts_with("struct") + }) + { + continue; + } + + if variable.overrides.is_some() { + continue; + } + + if variable.state_variable && !variable.constant { + collection_a.push(variable); + } + } + + let mut all_state_changes = None; + for func in context.function_definitions() { + if let Some(changes) = func.state_variable_changes(context) { + if all_state_changes.is_none() { + all_state_changes = Some(changes); + } else if let Some(existing_changes) = all_state_changes { + let new_changes = existing_changes + changes; + all_state_changes = Some(new_changes); + } + } + } + + if let Some(all_state_changes) = all_state_changes { + let collection_b = all_state_changes.fetch_non_exhaustive_manipulated_state_variables(); + let collection_b_ids: HashSet<_> = collection_b.into_iter().map(|v| v.id).collect(); + + // RESULT = collection A - collection B + for variable in collection_a { + if !collection_b_ids.contains(&variable.id) { + capture!(self, context, variable); + } + } + } + + Ok(!self.found_instances.is_empty()) + } + + fn severity(&self) -> IssueSeverity { + IssueSeverity::Low + } + + fn title(&self) -> String { + String::from("State variable could be declared constant") + } + + fn description(&self) -> String { + String::from("State variables that are not updated following deployment should be declared constant to save gas. Add the `constant` attribute to state variables that never change.") + } + + fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> { + self.found_instances.clone() + } + + fn name(&self) -> String { + format!( + "{}", + IssueDetectorNamePool::StateVariableCouldBeDeclaredConstant + ) + } +} + +mod function_state_changes_finder_helper { + use crate::{ + ast::{ASTNode, FunctionDefinition}, + context::{ + browser::ApproximateStorageChangeFinder, + graph::{CallGraph, CallGraphDirection, CallGraphVisitor}, + workspace_context::WorkspaceContext, + }, + }; + + impl FunctionDefinition { + /// Investigates the function with the help callgraph and accumulates all the state variables + /// that have been changed. + pub fn state_variable_changes<'a>( + &self, + context: &'a WorkspaceContext, + ) -> Option> { + let mut tracker = StateVariableChangeTracker { + changes: None, + context, + }; + + let investigator = + CallGraph::new(context, &[&(self.into())], CallGraphDirection::Inward).ok()?; + investigator.accept(context, &mut tracker).ok()?; + + tracker.changes.take() + } + } + + struct StateVariableChangeTracker<'a> { + context: &'a WorkspaceContext, + changes: Option>, + } + + impl<'a> CallGraphVisitor for StateVariableChangeTracker<'a> { + fn visit_any(&mut self, node: &ASTNode) -> eyre::Result<()> { + let changes = ApproximateStorageChangeFinder::from(self.context, node); + if self.changes.is_none() { + self.changes = Some(changes); + } else if let Some(existing_changes) = self.changes.take() { + let new_changes = existing_changes + changes; + self.changes = Some(new_changes); + } + Ok(()) + } + } +} + +#[cfg(test)] +mod state_variable_could_be_constant_tests { + use serial_test::serial; + + use crate::detect::{ + detector::IssueDetector, + low::state_variable_could_be_constant::StateVariableCouldBeConstantDetector, + }; + + #[test] + #[serial] + fn test_state_variable_could_be_declared_constant() { + let context = crate::detect::test_utils::load_solidity_source_unit( + "../tests/contract-playground/src/StateVariableCouldBeDeclaredConstant.sol", + ); + + let mut detector = StateVariableCouldBeConstantDetector::default(); + let found = detector.detect(&context).unwrap(); + // assert that the detector found an issue + assert!(found); + // assert that the detector found the correct number of instances + assert_eq!(detector.instances().len(), 2); + + println!("{:?}", detector.instances()); + // assert the severity is low + assert_eq!( + detector.severity(), + crate::detect::detector::IssueSeverity::Low + ); + } +} diff --git a/reports/adhoc-sol-files-report.md b/reports/adhoc-sol-files-report.md index d7bf3ca7..ecd0c934 100644 --- a/reports/adhoc-sol-files-report.md +++ b/reports/adhoc-sol-files-report.md @@ -31,6 +31,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-16: Unused Custom Error](#l-16-unused-custom-error) - [L-17: Potentially unused `private` / `internal` state variables found.](#l-17-potentially-unused-private--internal-state-variables-found) - [L-18: Dead Code](#l-18-dead-code) + - [L-19: State variable could be declared constant](#l-19-state-variable-could-be-declared-constant) # Summary @@ -75,7 +76,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 4 | -| Low | 18 | +| Low | 19 | # High Issues @@ -786,3 +787,56 @@ Functions that are not used. Consider removing them. +## L-19: State variable could be declared constant + +State variables that are not updated following deployment should be declared constant to save gas. Add the `constant` attribute to state variables that never change. + +
7 Found Instances + + +- Found in StateVariables.sol [Line: 13](../tests/adhoc-sol-files/StateVariables.sol#L13) + + ```solidity + uint256 private staticNonEmptyPrivateNumber = 1; + ``` + +- Found in StateVariables.sol [Line: 14](../tests/adhoc-sol-files/StateVariables.sol#L14) + + ```solidity + uint256 internal staticNonEmptyInternalNumber = 2; + ``` + +- Found in StateVariables.sol [Line: 15](../tests/adhoc-sol-files/StateVariables.sol#L15) + + ```solidity + uint256 public staticNonEmptyPublicNumber = 3; + ``` + +- Found in multiple-versions/0.5/B.sol [Line: 5](../tests/adhoc-sol-files/multiple-versions/0.5/B.sol#L5) + + ```solidity + address public MY_ADDRESS = address(0); + ``` + +- Found in multiple-versions/0.5/B.sol [Line: 6](../tests/adhoc-sol-files/multiple-versions/0.5/B.sol#L6) + + ```solidity + uint256 public MY_UINT = 134131; + ``` + +- Found in multiple-versions/0.8/B.sol [Line: 5](../tests/adhoc-sol-files/multiple-versions/0.8/B.sol#L5) + + ```solidity + address public MY_ADDRESS = address(0); + ``` + +- Found in multiple-versions/0.8/B.sol [Line: 6](../tests/adhoc-sol-files/multiple-versions/0.8/B.sol#L6) + + ```solidity + uint256 public MY_UINT = 134131; + ``` + +
+ + + diff --git a/reports/hardhat-playground-report.md b/reports/hardhat-playground-report.md index 7ae8b72e..45c78266 100644 --- a/reports/hardhat-playground-report.md +++ b/reports/hardhat-playground-report.md @@ -23,6 +23,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-7: PUSH0 is not supported by all chains](#l-7-push0-is-not-supported-by-all-chains) - [L-8: Contract still has TODOs](#l-8-contract-still-has-todos) - [L-9: Potentially unused `private` / `internal` state variables found.](#l-9-potentially-unused-private--internal-state-variables-found) + - [L-10: State variable could be declared constant](#l-10-state-variable-could-be-declared-constant) # Summary @@ -54,7 +55,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 5 | -| Low | 9 | +| Low | 10 | # High Issues @@ -456,3 +457,32 @@ State variable appears to be unused. No analysis has been performed to see if an +## L-10: State variable could be declared constant + +State variables that are not updated following deployment should be declared constant to save gas. Add the `constant` attribute to state variables that never change. + +
3 Found Instances + + +- Found in contracts/StateVariables.sol [Line: 13](../tests/hardhat-js-playground/contracts/StateVariables.sol#L13) + + ```solidity + uint256 private staticNonEmptyPrivateNumber = 1; + ``` + +- Found in contracts/StateVariables.sol [Line: 14](../tests/hardhat-js-playground/contracts/StateVariables.sol#L14) + + ```solidity + uint256 internal staticNonEmptyInternalNumber = 2; + ``` + +- Found in contracts/StateVariables.sol [Line: 15](../tests/hardhat-js-playground/contracts/StateVariables.sol#L15) + + ```solidity + uint256 public staticNonEmptyPublicNumber = 3; + ``` + +
+ + + diff --git a/reports/nft-report.md b/reports/nft-report.md index 676c4052..ea1da2ce 100644 --- a/reports/nft-report.md +++ b/reports/nft-report.md @@ -11,6 +11,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-1: Solidity pragma should be specific, not wide](#l-1-solidity-pragma-should-be-specific-not-wide) - [L-2: `public` functions not used internally could be marked `external`](#l-2-public-functions-not-used-internally-could-be-marked-external) - [L-3: PUSH0 is not supported by all chains](#l-3-push0-is-not-supported-by-all-chains) + - [L-4: State variable could be declared constant](#l-4-state-variable-could-be-declared-constant) # Summary @@ -40,7 +41,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 0 | -| Low | 3 | +| Low | 4 | # Low Issues @@ -102,3 +103,26 @@ Solc compiler version 0.8.20 switches the default target EVM version to Shanghai +## L-4: State variable could be declared constant + +State variables that are not updated following deployment should be declared constant to save gas. Add the `constant` attribute to state variables that never change. + +
2 Found Instances + + +- Found in src/inner-core-modules/ICM.sol [Line: 6](../tests/foundry-nft-f23/src/inner-core-modules/ICM.sol#L6) + + ```solidity + string public PROJECT_NAME = "Dogie"; + ``` + +- Found in src/inner-core-modules/ICM.sol [Line: 7](../tests/foundry-nft-f23/src/inner-core-modules/ICM.sol#L7) + + ```solidity + string public PROJECT_SYMBOL = "DOG"; + ``` + +
+ + + diff --git a/reports/report.json b/reports/report.json index 3b48a476..51b58e37 100644 --- a/reports/report.json +++ b/reports/report.json @@ -1,7 +1,7 @@ { "files_summary": { - "total_source_units": 97, - "total_sloc": 3370 + "total_source_units": 98, + "total_sloc": 3397 }, "files_details": { "files_details": [ @@ -229,6 +229,10 @@ "file_path": "src/StateShadowing.sol", "n_sloc": 17 }, + { + "file_path": "src/StateVariableCouldBeDeclaredConstant.sol", + "n_sloc": 27 + }, { "file_path": "src/StateVariables.sol", "n_sloc": 58 @@ -397,7 +401,7 @@ }, "issue_count": { "high": 42, - "low": 37 + "low": 38 }, "high_issues": { "issues": [ @@ -5765,6 +5769,217 @@ "src_char": "330:50" } ] + }, + { + "title": "State variable could be declared constant", + "description": "State variables that are not updated following deployment should be declared constant to save gas. Add the `constant` attribute to state variables that never change.", + "detector_name": "state-variable-could-be-declared-constant", + "instances": [ + { + "contract_path": "src/CostlyOperationsInsideLoops.sol", + "line_no": 6, + "src": "123:21", + "src_char": "123:21" + }, + { + "contract_path": "src/FunctionInitializingState.sol", + "line_no": 7, + "src": "176:17", + "src_char": "176:17" + }, + { + "contract_path": "src/FunctionInitializingState.sol", + "line_no": 58, + "src": "1603:17", + "src_char": "1603:17" + }, + { + "contract_path": "src/IncorrectERC20.sol", + "line_no": 5, + "src": "101:4", + "src_char": "101:4" + }, + { + "contract_path": "src/IncorrectERC20.sol", + "line_no": 6, + "src": "144:6", + "src_char": "144:6" + }, + { + "contract_path": "src/IncorrectERC20.sol", + "line_no": 7, + "src": "177:8", + "src_char": "177:8" + }, + { + "contract_path": "src/IncorrectERC20.sol", + "line_no": 8, + "src": "211:11", + "src_char": "211:11" + }, + { + "contract_path": "src/IncorrectERC20.sol", + "line_no": 48, + "src": "1426:4", + "src_char": "1426:4" + }, + { + "contract_path": "src/IncorrectERC20.sol", + "line_no": 49, + "src": "1467:6", + "src_char": "1467:6" + }, + { + "contract_path": "src/IncorrectERC20.sol", + "line_no": 50, + "src": "1500:8", + "src_char": "1500:8" + }, + { + "contract_path": "src/IncorrectERC20.sol", + "line_no": 51, + "src": "1535:12", + "src_char": "1535:12" + }, + { + "contract_path": "src/IncorrectERC721.sol", + "line_no": 5, + "src": "102:4", + "src_char": "102:4" + }, + { + "contract_path": "src/IncorrectERC721.sol", + "line_no": 6, + "src": "143:6", + "src_char": "143:6" + }, + { + "contract_path": "src/IncorrectERC721.sol", + "line_no": 145, + "src": "4001:4", + "src_char": "4001:4" + }, + { + "contract_path": "src/IncorrectERC721.sol", + "line_no": 146, + "src": "4040:6", + "src_char": "4040:6" + }, + { + "contract_path": "src/StateVariableCouldBeDeclaredConstant.sol", + "line_no": 8, + "src": "211:13", + "src_char": "211:13" + }, + { + "contract_path": "src/StateVariableCouldBeDeclaredConstant.sol", + "line_no": 13, + "src": "385:1", + "src_char": "385:1" + }, + { + "contract_path": "src/StateVariables.sol", + "line_no": 13, + "src": "383:27", + "src_char": "383:27" + }, + { + "contract_path": "src/StateVariables.sol", + "line_no": 14, + "src": "437:28", + "src_char": "437:28" + }, + { + "contract_path": "src/StateVariables.sol", + "line_no": 15, + "src": "490:26", + "src_char": "490:26" + }, + { + "contract_path": "src/StateVariablesManipulation.sol", + "line_no": 40, + "src": "1188:9", + "src_char": "1188:9" + }, + { + "contract_path": "src/StateVariablesManipulation.sol", + "line_no": 41, + "src": "1221:10", + "src_char": "1221:10" + }, + { + "contract_path": "src/StateVariablesManipulation.sol", + "line_no": 72, + "src": "2068:13", + "src_char": "2068:13" + }, + { + "contract_path": "src/UninitializedStateVariable.sol", + "line_no": 8, + "src": "198:11", + "src_char": "198:11" + }, + { + "contract_path": "src/cloc/AnotherHeavilyCommentedContract.sol", + "line_no": 14, + "src": "151:3", + "src_char": "151:3" + }, + { + "contract_path": "src/cloc/AnotherHeavilyCommentedContract.sol", + "line_no": 16, + "src": "190:3", + "src_char": "190:3" + }, + { + "contract_path": "src/cloc/AnotherHeavilyCommentedContract.sol", + "line_no": 19, + "src": "261:3", + "src_char": "261:3" + }, + { + "contract_path": "src/cloc/AnotherHeavilyCommentedContract.sol", + "line_no": 22, + "src": "367:3", + "src_char": "367:3" + }, + { + "contract_path": "src/cloc/AnotherHeavilyCommentedContract.sol", + "line_no": 29, + "src": "477:3", + "src_char": "477:3" + }, + { + "contract_path": "src/cloc/HeavilyCommentedContract.sol", + "line_no": 14, + "src": "160:3", + "src_char": "160:3" + }, + { + "contract_path": "src/cloc/HeavilyCommentedContract.sol", + "line_no": 16, + "src": "199:3", + "src_char": "199:3" + }, + { + "contract_path": "src/cloc/HeavilyCommentedContract.sol", + "line_no": 19, + "src": "270:3", + "src_char": "270:3" + }, + { + "contract_path": "src/cloc/HeavilyCommentedContract.sol", + "line_no": 22, + "src": "376:3", + "src_char": "376:3" + }, + { + "contract_path": "src/cloc/HeavilyCommentedContract.sol", + "line_no": 29, + "src": "486:3", + "src_char": "486:3" + } + ] } ] }, @@ -5847,6 +6062,7 @@ "builtin-symbol-shadow", "function-selector-collision", "unchecked-low-level-call", - "function-pointer-in-constructor" + "function-pointer-in-constructor", + "state-variable-could-be-declared-constant" ] } \ No newline at end of file diff --git a/reports/report.md b/reports/report.md index d79b2e55..2c440c53 100644 --- a/reports/report.md +++ b/reports/report.md @@ -88,6 +88,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-35: Costly operations inside loops.](#l-35-costly-operations-inside-loops) - [L-36: Builtin Symbol Shadowing](#l-36-builtin-symbol-shadowing) - [L-37: Function pointers used in constructors.](#l-37-function-pointers-used-in-constructors) + - [L-38: State variable could be declared constant](#l-38-state-variable-could-be-declared-constant) # Summary @@ -96,8 +97,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | -| .sol Files | 97 | -| Total nSLOC | 3370 | +| .sol Files | 98 | +| Total nSLOC | 3397 | ## Files Details @@ -160,6 +161,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/RevertsAndRequriesInLoops.sol | 27 | | src/SendEtherNoChecks.sol | 58 | | src/StateShadowing.sol | 17 | +| src/StateVariableCouldBeDeclaredConstant.sol | 27 | | src/StateVariables.sol | 58 | | src/StateVariablesManipulation.sol | 250 | | src/StorageConditionals.sol | 59 | @@ -201,7 +203,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/reused_contract_name/ContractB.sol | 7 | | src/uniswap/UniswapV2Swapper.sol | 50 | | src/uniswap/UniswapV3Swapper.sol | 150 | -| **Total** | **3370** | +| **Total** | **3397** | ## Issue Summary @@ -209,7 +211,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 42 | -| Low | 37 | +| Low | 38 | # High Issues @@ -5877,3 +5879,218 @@ solc versions below 0.5.9 contain a compiler bug leading to unexpected behavior +## L-38: State variable could be declared constant + +State variables that are not updated following deployment should be declared constant to save gas. Add the `constant` attribute to state variables that never change. + +
34 Found Instances + + +- Found in src/CostlyOperationsInsideLoops.sol [Line: 6](../tests/contract-playground/src/CostlyOperationsInsideLoops.sol#L6) + + ```solidity + uint loop_count = 100; + ``` + +- Found in src/FunctionInitializingState.sol [Line: 7](../tests/contract-playground/src/FunctionInitializingState.sol#L7) + + ```solidity + uint public w = 5; + ``` + +- Found in src/FunctionInitializingState.sol [Line: 58](../tests/contract-playground/src/FunctionInitializingState.sol#L58) + + ```solidity + uint public w = 5; + ``` + +- Found in src/IncorrectERC20.sol [Line: 5](../tests/contract-playground/src/IncorrectERC20.sol#L5) + + ```solidity + string public name = "IncorrectToken"; + ``` + +- Found in src/IncorrectERC20.sol [Line: 6](../tests/contract-playground/src/IncorrectERC20.sol#L6) + + ```solidity + string public symbol = "ICT"; + ``` + +- Found in src/IncorrectERC20.sol [Line: 7](../tests/contract-playground/src/IncorrectERC20.sol#L7) + + ```solidity + uint8 public decimals = 18; + ``` + +- Found in src/IncorrectERC20.sol [Line: 8](../tests/contract-playground/src/IncorrectERC20.sol#L8) + + ```solidity + uint256 public totalSupply = 1000000; + ``` + +- Found in src/IncorrectERC20.sol [Line: 48](../tests/contract-playground/src/IncorrectERC20.sol#L48) + + ```solidity + string public name = "CorrectToken"; + ``` + +- Found in src/IncorrectERC20.sol [Line: 49](../tests/contract-playground/src/IncorrectERC20.sol#L49) + + ```solidity + string public symbol = "CRT"; + ``` + +- Found in src/IncorrectERC20.sol [Line: 50](../tests/contract-playground/src/IncorrectERC20.sol#L50) + + ```solidity + uint8 public decimals = 18; + ``` + +- Found in src/IncorrectERC20.sol [Line: 51](../tests/contract-playground/src/IncorrectERC20.sol#L51) + + ```solidity + uint256 private _totalSupply = 1000000 * 10 ** uint(decimals); + ``` + +- Found in src/IncorrectERC721.sol [Line: 5](../tests/contract-playground/src/IncorrectERC721.sol#L5) + + ```solidity + string public name = "IncorrectNFT"; + ``` + +- Found in src/IncorrectERC721.sol [Line: 6](../tests/contract-playground/src/IncorrectERC721.sol#L6) + + ```solidity + string public symbol = "INFT"; + ``` + +- Found in src/IncorrectERC721.sol [Line: 145](../tests/contract-playground/src/IncorrectERC721.sol#L145) + + ```solidity + string public name = "CorrectNFT"; + ``` + +- Found in src/IncorrectERC721.sol [Line: 146](../tests/contract-playground/src/IncorrectERC721.sol#L146) + + ```solidity + string public symbol = "CNFT"; + ``` + +- Found in src/StateVariableCouldBeDeclaredConstant.sol [Line: 8](../tests/contract-playground/src/StateVariableCouldBeDeclaredConstant.sol#L8) + + ```solidity + uint256 public constantValue = 100; + ``` + +- Found in src/StateVariableCouldBeDeclaredConstant.sol [Line: 13](../tests/contract-playground/src/StateVariableCouldBeDeclaredConstant.sol#L13) + + ```solidity + SVIERC20 public h = SVIERC20(address(3)); // This could be declared constant + ``` + +- Found in src/StateVariables.sol [Line: 13](../tests/contract-playground/src/StateVariables.sol#L13) + + ```solidity + uint256 private staticNonEmptyPrivateNumber = 1; + ``` + +- Found in src/StateVariables.sol [Line: 14](../tests/contract-playground/src/StateVariables.sol#L14) + + ```solidity + uint256 internal staticNonEmptyInternalNumber = 2; + ``` + +- Found in src/StateVariables.sol [Line: 15](../tests/contract-playground/src/StateVariables.sol#L15) + + ```solidity + uint256 public staticNonEmptyPublicNumber = 3; + ``` + +- Found in src/StateVariablesManipulation.sol [Line: 40](../tests/contract-playground/src/StateVariablesManipulation.sol#L40) + + ```solidity + int256 public simpleInt = 100; + ``` + +- Found in src/StateVariablesManipulation.sol [Line: 41](../tests/contract-playground/src/StateVariablesManipulation.sol#L41) + + ```solidity + bool public simpleBool = true; + ``` + +- Found in src/StateVariablesManipulation.sol [Line: 72](../tests/contract-playground/src/StateVariablesManipulation.sol#L72) + + ```solidity + uint256[5] public assignToMeNow = [1, 4, 5, 8, 9]; // 1 state var assigned here + ``` + +- Found in src/UninitializedStateVariable.sol [Line: 8](../tests/contract-playground/src/UninitializedStateVariable.sol#L8) + + ```solidity + string public s_publisher = "Blockchain Ltd."; // GOOD (because it's initialized here.) + ``` + +- Found in src/cloc/AnotherHeavilyCommentedContract.sol [Line: 14](../tests/contract-playground/src/cloc/AnotherHeavilyCommentedContract.sol#L14) + + ```solidity + uint256 s_1 = 0; + ``` + +- Found in src/cloc/AnotherHeavilyCommentedContract.sol [Line: 16](../tests/contract-playground/src/cloc/AnotherHeavilyCommentedContract.sol#L16) + + ```solidity + uint256 s_2 = 0; + ``` + +- Found in src/cloc/AnotherHeavilyCommentedContract.sol [Line: 19](../tests/contract-playground/src/cloc/AnotherHeavilyCommentedContract.sol#L19) + + ```solidity + uint256 s_3 = 0; // this is a side comment + ``` + +- Found in src/cloc/AnotherHeavilyCommentedContract.sol [Line: 22](../tests/contract-playground/src/cloc/AnotherHeavilyCommentedContract.sol#L22) + + ```solidity + uint256 s_4 = 0; // scc-dblah + ``` + +- Found in src/cloc/AnotherHeavilyCommentedContract.sol [Line: 29](../tests/contract-playground/src/cloc/AnotherHeavilyCommentedContract.sol#L29) + + ```solidity + this is longer comment */ uint256 s_5 = 0; + ``` + +- Found in src/cloc/HeavilyCommentedContract.sol [Line: 14](../tests/contract-playground/src/cloc/HeavilyCommentedContract.sol#L14) + + ```solidity + uint256 s_1 = 0; + ``` + +- Found in src/cloc/HeavilyCommentedContract.sol [Line: 16](../tests/contract-playground/src/cloc/HeavilyCommentedContract.sol#L16) + + ```solidity + uint256 s_2 = 0; + ``` + +- Found in src/cloc/HeavilyCommentedContract.sol [Line: 19](../tests/contract-playground/src/cloc/HeavilyCommentedContract.sol#L19) + + ```solidity + uint256 s_3 = 0; // this is a side comment + ``` + +- Found in src/cloc/HeavilyCommentedContract.sol [Line: 22](../tests/contract-playground/src/cloc/HeavilyCommentedContract.sol#L22) + + ```solidity + uint256 s_4 = 0; // scc-dblah + ``` + +- Found in src/cloc/HeavilyCommentedContract.sol [Line: 29](../tests/contract-playground/src/cloc/HeavilyCommentedContract.sol#L29) + + ```solidity + this is longer comment */ uint256 s_5 = 0; + ``` + +
+ + + diff --git a/reports/report.sarif b/reports/report.sarif index 33e6d3dc..69225a93 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -9531,6 +9531,389 @@ "text": "solc versions below 0.5.9 contain a compiler bug leading to unexpected behavior when calling uninitialized function pointers in constructors. It is recommended to not use function pointers in constructors." }, "ruleId": "function-pointer-in-constructor" + }, + { + "level": "note", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/CostlyOperationsInsideLoops.sol" + }, + "region": { + "byteLength": 21, + "byteOffset": 123 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/FunctionInitializingState.sol" + }, + "region": { + "byteLength": 17, + "byteOffset": 176 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/FunctionInitializingState.sol" + }, + "region": { + "byteLength": 17, + "byteOffset": 1603 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC20.sol" + }, + "region": { + "byteLength": 4, + "byteOffset": 101 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC20.sol" + }, + "region": { + "byteLength": 6, + "byteOffset": 144 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC20.sol" + }, + "region": { + "byteLength": 8, + "byteOffset": 177 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC20.sol" + }, + "region": { + "byteLength": 11, + "byteOffset": 211 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC20.sol" + }, + "region": { + "byteLength": 4, + "byteOffset": 1426 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC20.sol" + }, + "region": { + "byteLength": 6, + "byteOffset": 1467 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC20.sol" + }, + "region": { + "byteLength": 8, + "byteOffset": 1500 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC20.sol" + }, + "region": { + "byteLength": 12, + "byteOffset": 1535 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC721.sol" + }, + "region": { + "byteLength": 4, + "byteOffset": 102 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC721.sol" + }, + "region": { + "byteLength": 6, + "byteOffset": 143 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC721.sol" + }, + "region": { + "byteLength": 4, + "byteOffset": 4001 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/IncorrectERC721.sol" + }, + "region": { + "byteLength": 6, + "byteOffset": 4040 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariableCouldBeDeclaredConstant.sol" + }, + "region": { + "byteLength": 13, + "byteOffset": 211 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariableCouldBeDeclaredConstant.sol" + }, + "region": { + "byteLength": 1, + "byteOffset": 385 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariables.sol" + }, + "region": { + "byteLength": 27, + "byteOffset": 383 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariables.sol" + }, + "region": { + "byteLength": 28, + "byteOffset": 437 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariables.sol" + }, + "region": { + "byteLength": 26, + "byteOffset": 490 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariablesManipulation.sol" + }, + "region": { + "byteLength": 9, + "byteOffset": 1188 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariablesManipulation.sol" + }, + "region": { + "byteLength": 10, + "byteOffset": 1221 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateVariablesManipulation.sol" + }, + "region": { + "byteLength": 13, + "byteOffset": 2068 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UninitializedStateVariable.sol" + }, + "region": { + "byteLength": 11, + "byteOffset": 198 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/AnotherHeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 151 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/AnotherHeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 190 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/AnotherHeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 261 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/AnotherHeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 367 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/AnotherHeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 477 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/HeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 160 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/HeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 199 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/HeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 270 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/HeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 376 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/cloc/HeavilyCommentedContract.sol" + }, + "region": { + "byteLength": 3, + "byteOffset": 486 + } + } + } + ], + "message": { + "text": "State variables that are not updated following deployment should be declared constant to save gas. Add the `constant` attribute to state variables that never change." + }, + "ruleId": "state-variable-could-be-declared-constant" } ], "tool": { diff --git a/tests/contract-playground/src/StateVariableCouldBeDeclaredConstant.sol b/tests/contract-playground/src/StateVariableCouldBeDeclaredConstant.sol new file mode 100644 index 00000000..d869d3d4 --- /dev/null +++ b/tests/contract-playground/src/StateVariableCouldBeDeclaredConstant.sol @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.19; + +interface SVIERC20 {} + +contract StateVariableCouldBeDeclaredConstant { + // This state variable could be declared constant but isn't + uint256 public constantValue = 100; + + // Other state variables + uint256 public variableValue; // This one cannot be marked constant. (It can be marked immutable) + + SVIERC20 public h = SVIERC20(address(3)); // This could be declared constant + + constructor() { + variableValue = 50; + } + + function getConstantValue() external view returns (uint256) { + return constantValue; + } +} + +contract StateVariableCouldBeDeclaredConstant2 { + // This state variable could NOT be declared constant + uint256 public cannotBeconstantValue = 100; + + // Other state variables + uint256 public variableValue; + + constructor() { + variableValue = 50; + } +} + +contract StateVariableCouldBeDeclaredConstant2Child is + StateVariableCouldBeDeclaredConstant2 +{ + function changeIt() external { + cannotBeconstantValue = 130; + } +}