Merge pull request #15 from api3dao/quanstamp-initial-report

Quanstamp initial report

Author: acenolaza

contracts/InverseApi3ReaderProxyV1.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.27;
+pragma solidity 0.8.27;
 
 import "@api3/contracts/interfaces/IApi3ReaderProxy.sol";
 import "./interfaces/IInverseApi3ReaderProxyV1.sol";
@@ -24,10 +24,9 @@ contract InverseApi3ReaderProxyV1 is IInverseApi3ReaderProxyV1 {
     }
 
     /// @notice Returns the inverted value of the underlying IApi3ReaderProxy
-    /// @dev This inverts the 18-decimal fixed-point value using 1e36 / value.
-    /// The operation will revert if `baseValue` is zero (division by zero) or if
-    /// `baseValue` is so small (yet non-zero) that the resulting inverted value
-    /// would overflow the `int224` type.
+    /// @dev Calculates `int224(1e36) / baseValue`. The operation will revert if
+    /// `baseValue` is zero. If `baseValue` is non-zero but its absolute value is
+    /// greater than `1e36`, the result of the integer division will be `0`.
     /// @return value Inverted value of the underlying proxy
     /// @return timestamp Timestamp of the underlying proxy
     function read()
@@ -39,7 +38,11 @@ contract InverseApi3ReaderProxyV1 is IInverseApi3ReaderProxyV1 {
         (int224 baseValue, uint32 baseTimestamp) = IApi3ReaderProxy(proxy)
             .read();
 
-        value = int224((1e36) / int256(baseValue));
+        if (baseValue == 0) {
+            revert DivisionByZero();
+        }
+
+        value = int224(1e36) / baseValue;
         timestamp = baseTimestamp;
     }
 

contracts/NormalizedApi3ReaderProxyV1.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.27;
+pragma solidity 0.8.27;
 
 import "./interfaces/INormalizedApi3ReaderProxyV1.sol";
 
@@ -45,15 +45,9 @@ contract NormalizedApi3ReaderProxyV1 is INormalizedApi3ReaderProxyV1 {
 
     /// @notice Returns the price of the underlying Chainlink feed normalized to
     /// 18 decimals.
-    /// @dev Fetches an `int256` answer from the Chainlink feed and scales it
-    /// to 18 decimals using pre-calculated factors. The result is cast to
-    /// `int224` to conform to the `IApi3ReaderProxy` interface.
-    /// IMPORTANT: If the normalized `int256` value is outside the `int224`
-    /// range, this cast causes silent truncation and data loss. Deployers
-    /// must verify that the source feed's characteristics (value magnitude
-    /// and original decimals) ensure the 18-decimal normalized value fits
-    /// `int224`. Scaling arithmetic (prior to cast) reverts on `int256`
-    /// overflow.
+    /// @dev Fetches and scales the Chainlink feed's `int256` answer.
+    /// The scaled `int256` result is then cast to `int224`. This cast is
+    /// unchecked for gas optimization and may silently truncate.
     /// @return value The normalized signed fixed-point value with 18 decimals
     /// @return timestamp The updatedAt timestamp of the feed
     function read()

contracts/PriceCappedApi3ReaderProxyV1.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.27;
+pragma solidity 0.8.27;
 
 import "@api3/contracts/interfaces/IApi3ReaderProxy.sol";
 import "./interfaces/IPriceCappedApi3ReaderProxyV1.sol";

contracts/ProductApi3ReaderProxyV1.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.27;
+pragma solidity 0.8.27;
 
 import "@api3/contracts/interfaces/IApi3ReaderProxy.sol";
 import "./interfaces/IProductApi3ReaderProxyV1.sol";
@@ -33,13 +33,14 @@ contract ProductApi3ReaderProxyV1 is IProductApi3ReaderProxyV1 {
 
     /// @notice Returns the current value and timestamp of the rate composition
     /// between two IApi3ReaderProxy proxies by multiplying their values
-    /// @dev There is a risk of multiplication overflowing if the result exceeds
-    /// `int256` bounds. The returned timestamp is `block.timestamp`, marking
-    /// when this newly derived product value was computed on-chain.
-    /// Timestamps from underlying `IApi3ReaderProxy` feeds are not aggregated.
-    /// Their diverse nature (see `IApi3ReaderProxy` interface for details like
-    /// off-chain origins or varying update cadences) makes aggregation complex
-    /// and potentially misleading for this product's timestamp.
+    /// @dev Calculates product as `(int256(value1) * int256(value2)) / 1e18`.
+    /// The initial multiplication `int256(value1) * int256(value2)` may revert
+    /// on `int256` overflow. The final `int256` result of the full expression
+    /// is then cast to `int224`. This cast is unchecked for gas optimization
+    /// and may silently truncate if the result exceeds `int224` limits.
+    /// The returned timestamp is `block.timestamp`, reflecting the on-chain
+    /// computation time of the product. Underlying feed timestamps are not
+    /// aggregated due to complexity and potential for misinterpretation.
     /// @return value Value of the product of the two proxies
     /// @return timestamp Timestamp of the current block
     function read()

contracts/adapters/ScaledApi3FeedProxyV1.sol

@@ -1,14 +1,20 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.27;
+pragma solidity 0.8.27;
 
 import "@api3/contracts/interfaces/IApi3ReaderProxy.sol";
 import "./interfaces/IScaledApi3FeedProxyV1.sol";
 
 /// @title An immutable Chainlink AggregatorV2V3Interface feed contract that
 /// scales the value of an IApi3ReaderProxy data feed to a target number of
 /// decimals
-/// @dev This contract assumes the source proxy always returns values with
-/// 18 decimals (as all IApi3ReaderProxy-compatible proxies do)
+/// @dev This contract reads an `int224` value (assumed to be 18 decimals)
+/// from the underlying `IApi3ReaderProxy` and scales it to `targetDecimals`.
+/// The scaling arithmetic uses `int256` for intermediate results, allowing the
+/// scaled value to exceed `int224` limits if upscaling significantly; it will
+/// revert on `int256` overflow.
+/// When downscaling, integer division (`proxyValue / scalingFactor`) is used,
+/// which truncates and may lead to precision loss. Integrators must carefully
+/// consider this potential precision loss for their specific use case.
 contract ScaledApi3FeedProxyV1 is IScaledApi3FeedProxyV1 {
     /// @notice IApi3ReaderProxy contract address
     address public immutable override proxy;
@@ -133,15 +139,10 @@ contract ScaledApi3FeedProxyV1 is IScaledApi3FeedProxyV1 {
 
     /// @notice Reads a value from the underlying `IApi3ReaderProxy` and
     /// scales it to `targetDecimals`.
-    /// @dev Reads an `int224` value (assumed to be 18 decimals) from the
-    /// underlying `IApi3ReaderProxy`. This value is then scaled to
-    /// `targetDecimals` using pre-calculated factors. The scaling arithmetic
-    /// (e.g., `proxyValue * scalingFactor`) involves an `int224` (`proxyValue`)
-    /// and an `int256` (`scalingFactor`). `proxyValue` is implicitly promoted
-    /// to `int256` for this operation, resulting in an `int256` value.
-    /// This allows the scaled result to exceed the `int224` range, provided
-    /// it fits within `int256`. Arithmetic operations will revert on `int256`
-    /// overflow. The function returns the scaled value as an `int256`.
+    /// @dev Reads from the underlying proxy and applies scaling to
+    /// `targetDecimals`. Upscaling uses multiplication; downscaling uses integer
+    /// division (which truncates). All scaling arithmetic is performed using
+    /// `int256`.
     /// @return value The scaled signed fixed-point value with `targetDecimals`.
     /// @return timestamp The timestamp from the underlying proxy.
     function _read() internal view returns (int256 value, uint32 timestamp) {

contracts/test/MockAggregatorV2V3.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.27;
+pragma solidity 0.8.27;
 
 import "../vendor/@chainlink/[email protected]/src/v0.8/shared/interfaces/AggregatorV2V3Interface.sol";