fix: dead code, improvements from coderabit

slither/detectors/oracles/deprecated_chainlink_calls.py

@@ -5,7 +5,7 @@
 
 class DeprecatedChainlinkCall(AbstractDetector):
     """
-    Documentation
+    Documentation: This detector scans for deprecated Chainlink API calls in Solidity contracts. For example, it flags the use of `getAnswer` which is no longer recommended.
     """
 
     ARGUMENT = "deprecated-chainlink-call"

slither/detectors/oracles/supported_oracles/chainlink_oracle.py

@@ -28,25 +28,6 @@ class ChainlinkOracle(Oracle):
     def __init__(self):
         super().__init__(CHAINLINK_ORACLE_CALLS, INTERFACES)
 
-    # This function checks if the RoundId value is validated in connection with answeredInRound value
-    # But this last variable was deprecated. We left this function for possible future use.
-    @staticmethod
-    def check_RoundId(var: VarInCondition, var2: VarInCondition) -> bool:
-        if var is None or var2 is None:
-            return False
-        for node in var.nodes_with_var:
-            for ir in node.irs:
-                if isinstance(ir, Binary):
-                    if ir.type in (BinaryType.GREATER, BinaryType.GREATER_EQUAL):
-                        if ir.variable_right == var.var and ir.variable_left == var2.var:
-                            return True
-                    elif ir.type in (BinaryType.LESS, BinaryType.LESS_EQUAL):
-                        if ir.variable_right == var2.var and ir.variable_left == var.var:
-                            return True
-            return check_revert(node) or return_boolean(node)
-
-        return False
-
     @staticmethod
     def generate_naive_order():
         vars_order = {}
@@ -97,23 +78,25 @@ def is_sequencer_check(self, answer, startedAt):
     def naive_data_validation(self):
         vars_order = self.find_which_vars_are_used()
         problems = []
+        # Iterating through all oracle variables which were returned by the oracle call
         for (index, var) in vars_order.items():
             if not self.is_needed_to_check_conditions(var):
                 continue
-            # if index == ChainlinkVars.ROUNDID.value: # Commented due to deprecation of AnsweredInRound
-            #     if not self.check_RoundId(var, vars_order[ChainlinkVars.ANSWEREDINROUND.value]):
-            #         problems.append("RoundID value is not checked correctly. It was returned by the oracle call in the function {} of contract {}.\n".format( oracle.function, oracle.node.source_mapping))
+            # Second variable is the price value
             if index == ChainlinkVars.ANSWER.value:
                 if not self.check_price(var):
                     problems.append(
                         f"The price value is validated incorrectly. This value is returned by Chainlink oracle call {self.contract}.{self.interface}.{self.oracle_api} ({self.node.source_mapping}).\n"
                     )
+            # Third variable is the updatedAt value, indicating when the price was updated
             elif index == ChainlinkVars.UPDATEDAT.value:
                 if not self.check_staleness(var):
                     problems.append(
                         f"The price can be stale due to incorrect validation of updatedAt value. This value is returned by Chainlink oracle call {self.contract}.{self.interface}.{self.oracle_api} ({self.node.source_mapping}).\n"
                     )
 
+            # Fourth variable is the startedAt value, indicating when the round was started.
+            # Used in connection with sequencer
             elif (
                 index == ChainlinkVars.STARTEDAT.value
                 and vars_order[ChainlinkVars.STARTEDAT.value] is not None
@@ -122,6 +105,7 @@ def naive_data_validation(self):
                 if self.is_sequencer_check(vars_order[ChainlinkVars.ANSWER.value], var):
                     problems = []
                     break
+        # Iterate through checks performed out of the function where the oracle call is performed
         for tup in self.out_of_function_checks:
             problems.append(
                 f"The variation of {tup[0]} is checked on the lines {[str(node.source_mapping) for node in tup[1][::5]]}. Not in the original function where the Oracle call is performed.\n"

slither/detectors/oracles/supported_oracles/oracle.py

@@ -109,7 +109,7 @@ def check_staleness(self, var: VarInCondition) -> bool:
         different_behavior = False
         if hasattr(var, "nodes_with_var"):
             for node in var.nodes_with_var:
-                # This is temporarily check which will be improved in the future. Mostly we are looking for block.timestamp and trust the developer that he is using it correctly
+                # This check look for conditions where the timestamp is used
                 if self.timestamp_in_node(node):
                     return True
                 if not different_behavior:

tests/e2e/detectors/test_data/oracle-data-validation/0.8.20/oracle_data_check_price_in_double_internal_fc.sol

@@ -53,6 +53,7 @@ contract StableOracleDAI {
         return false;
     }
 
+    //  Returns the latest price after validating it to be greater than zero and checking for data staleness and round completion.
     function getPriceUSD() external view returns (uint256) {
         uint256 wethPriceUSD = 1;
         uint256 DAIWethPrice = 1;

tests/e2e/detectors/test_data/oracle-data-validation/0.8.20/oracle_timestamp_in_var.sol

@@ -45,10 +45,10 @@ contract ChainlinkETHUSDPriceConsumer {
         );
     }
     /**
-     * Returns the latest price
+     * Returns the latest price. Ensures the price is not outdated by checking the `updatedAt` timestamp.
      */
     function getLatestPrice() public view returns (int) {
-        (, int price, ,uint256 updateAt , ) = priceFeed.latestRoundData();
+        (, int price, , uint256 updateAt, ) = priceFeed.latestRoundData();
         uint current_timestamp = block.timestamp;
         require(current_timestamp - updateAt < 1 minutes, "Price is outdated");
         return price;