Merge pull request #2 from pyth-network/chore-add-code-comments

chore Add Code comments

Author: aditya520

src/morpho-pyth/MorphoPythOracle.sol

@@ -12,7 +12,23 @@ import "@pythnetwork/pyth-sdk-solidity/IPyth.sol";
 
 /// @title MorphoPythOracle
 /// @author Pyth Data Association
-/// @notice Morpho Blue oracle using Pyth Price Feeds.
+/// @notice Morpho oracle implementation that combines Pyth price feeds with ERC-4626 vault pricing
+/// @dev This oracle calculates prices by combining multiple data sources:
+///      - Up to 2 Pyth price feeds each for base and quote assets
+///      - Optional ERC-4626 vault share-to-asset conversion for base and quote
+///      - Configurable staleness checks for price feed age validation
+///
+/// Price Calculation Formula:
+/// price = SCALE_FACTOR * (baseVaultAssets * baseFeed1 * baseFeed2) / (quoteVaultAssets * quoteFeed1 * quoteFeed2)
+///
+/// Security Considerations:
+/// - Single priceFeedMaxAge used for all feeds may not suit different asset volatilities
+/// - ERC-4626 vaults can be manipulated through donations, flash loans, or fee changes
+/// - Pyth confidence intervals are not validated, potentially accepting uncertain prices
+/// - Conversion samples must be large enough to avoid rounding to zero
+///
+/// @dev This contract follows Morpho's design philosophy prioritizing flexibility over safety.
+///      Users must validate all configuration parameters and monitor oracle behavior.
 contract MorphoPythOracle is IMorphoPythOracle {
     using Math for uint256;
 
@@ -50,8 +66,40 @@ contract MorphoPythOracle is IMorphoPythOracle {
     uint256 public immutable SCALE_FACTOR;
 
     /// @inheritdoc IMorphoPythOracle
+    /// @dev WARNING: Single staleness threshold applied to all feeds regardless of asset characteristics.
+    ///      Fast-moving assets may need shorter max age (e.g., 15s) while stable assets could tolerate longer (e.g.,
+    /// 60s).
+    ///      Using a universal value may reject valid stable prices or accept stale volatile prices.
+    ///      Consider asset-specific staleness checks for improved accuracy and reliability.
     uint256 public PRICE_FEED_MAX_AGE;
 
+    /// @notice Initializes a new MorphoPythOracle instance
+    /// @dev Constructor performs parameter validation but cannot prevent all misconfigurations.
+    ///      Users must ensure parameters are appropriate for their use case.
+    ///
+    /// @param pyth_ Address of the Pyth contract - must be the official Pyth contract for the chain
+    /// @param baseVault ERC-4626 vault for base asset, or address(0) to skip vault conversion
+    /// @param baseVaultConversionSample Sample shares amount for base vault conversion (must provide adequate
+    /// precision)
+    /// @param baseFeed1 First Pyth price feed ID for base asset, or bytes32(0) for price=1
+    /// @param baseFeed2 Second Pyth price feed ID for base asset, or bytes32(0) for price=1
+    /// @param baseTokenDecimals Decimal places for base token
+    /// @param quoteVault ERC-4626 vault for quote asset, or address(0) to skip vault conversion
+    /// @param quoteVaultConversionSample Sample shares amount for quote vault conversion (must provide adequate
+    /// precision)
+    /// @param quoteFeed1 First Pyth price feed ID for quote asset, or bytes32(0) for price=1
+    /// @param quoteFeed2 Second Pyth price feed ID for quote asset, or bytes32(0) for price=1
+    /// @param quoteTokenDecimals Decimal places for quote token
+    /// @param priceFeedMaxAge Maximum acceptable age in seconds for price feeds (applies to all feeds)
+    ///
+    /// @dev CRITICAL: Conversion samples must be large enough that convertToAssets() returns non-zero values.
+    ///      Small samples may round to zero, breaking price calculations. Test with actual vault implementations!
+    ///
+    /// @dev VAULT SECURITY: If using vaults, ensure they are trusted implementations resistant to:
+    ///      - Share price manipulation via direct token transfers
+    ///      - Flash loan attacks that temporarily affect asset/share ratios
+    ///      - Dynamic fee changes that alter convertToAssets() results
+    ///      - First depositor attacks setting malicious initial exchange rates
     constructor(
         address pyth_,
         IERC4626 baseVault,
@@ -105,6 +153,21 @@ contract MorphoPythOracle is IMorphoPythOracle {
     /* PRICE */
 
     /// @inheritdoc IOracle
+    /// @notice Calculates the current price by combining vault asset values and Pyth feed prices
+    /// @return The calculated price with 18 decimal precision
+    /// @dev Price calculation: SCALE_FACTOR * (baseAssets * baseFeeds) / (quoteAssets * quoteFeeds)
+    ///
+    /// SECURITY WARNINGS:
+    /// - Vault prices can be manipulated if vaults are not manipulation-resistant
+    /// - Single PRICE_FEED_MAX_AGE applied to all feeds regardless of asset volatility
+    /// - Pyth confidence intervals are ignored - uncertain prices may be accepted
+    /// - No per-block deviation caps - prices can change drastically within one block
+    ///
+    /// @dev This function will revert if:
+    ///      - Any Pyth feed returns a negative price
+    ///      - Any feed is older than PRICE_FEED_MAX_AGE
+    ///      - Vault convertToAssets calls fail
+    ///      - Arithmetic overflow in multiplication/division
     function price() external view returns (uint256) {
         return SCALE_FACTOR.mulDiv(
             BASE_VAULT.getAssets(BASE_VAULT_CONVERSION_SAMPLE)

src/morpho-pyth/MorphoPythOracleFactory.sol

@@ -9,16 +9,42 @@ import {MorphoPythOracle} from "./MorphoPythOracle.sol";
 
 /// @title MorphoPythOracleFactory
 /// @author Pyth Data Association
-/// @notice This contract allows to create MorphoPythOracle oracles, and to index them easily.
+/// @notice Factory contract for creating MorphoPythOracle instances with permissionless deployment
+/// @dev This factory provides a permissionless way to deploy MorphoPythOracle contracts. Users should carefully
+///      validate all parameters and resulting oracle configurations before use in production environments.
+///
+/// Security Considerations:
+/// - This factory accepts arbitrary Pyth contract addresses and feed IDs without validation
+/// - Market creators and users must verify oracle addresses and configurations independently
+/// - The factory only tracks that an oracle was deployed via this factory, not that it's safe to use
+/// - Malicious actors can deploy oracles with fake Pyth contracts or manipulated vault addresses
+///
+/// @dev Following Morpho's design philosophy, this factory prioritizes flexibility over built-in safety checks.
+///      See Morpho documentation on oracle risks and validation requirements.
 contract MorphoPythOracleFactory is IMorphoPythOracleFactory {
     /* STORAGE */
 
     /// @inheritdoc IMorphoPythOracleFactory
+    /// @dev This mapping only indicates that an oracle was deployed via this factory.
+    ///      It does NOT guarantee the oracle configuration is safe or uses trusted parameters.
+    ///      Users must independently verify oracle parameters including Pyth address and vault addresses.
     mapping(address => bool) public isMorphoPythOracle;
 
     /* EXTERNAL */
 
     /// @inheritdoc IMorphoPythOracleFactory
+    /// @dev SECURITY WARNING: This function accepts arbitrary addresses and parameters without validation.
+    ///      Callers can provide malicious Pyth contracts, manipulable vaults, or invalid feed IDs.
+    ///
+    /// Critical Validation Required by Users:
+    /// - Verify `pyth` address matches the official Pyth contract for your chain
+    /// - Validate all feed IDs exist and correspond to intended price feeds
+    /// - Ensure vault addresses are trusted ERC-4626 implementations if used
+    /// - Check that conversion samples provide adequate precision without overflow
+    /// - Verify `priceFeedMaxAge` is appropriate for all asset types involved
+    ///
+    /// @dev Following Morpho's trust model: "Market creators and users need to carefully validate
+    ///      the oracle address and its configuration." This includes all parameters passed to this function.
     function createMorphoPythOracle(
         address pyth,
         IERC4626 baseVault,

src/morpho-pyth/interfaces/IMorphoPythOracle.sol

@@ -7,9 +7,15 @@ import {IPyth} from "@pythnetwork/pyth-sdk-solidity/IPyth.sol";
 
 /// @title IMorphoPythOracle
 /// @author Pyth Data Association
-/// @notice Interface of MorphoPythOracle.
-/// @dev This interface is used to interact with the MorphoPythOracle contract.
-/// @dev Fetch price feed ids from https://www.pyth.network/developers/price-feed-ids
+/// @notice Interface for MorphoPythOracle - a Morpho Blue compatible oracle using Pyth price feeds
+/// @dev This interface extends IOracle to provide access to oracle configuration parameters.
+///      All configuration is immutable after deployment and should be carefully validated.
+///
+/// @dev Price feed IDs can be found at: https://www.pyth.network/developers/price-feed-ids
+///      Ensure feed IDs correspond to the intended assets before deployment.
+///
+/// @dev This oracle combines Pyth price feeds with optional ERC-4626 vault share pricing.
+///      Users must validate that all components (Pyth contract, feeds, vaults) are trustworthy.
 interface IMorphoPythOracle is IOracle {
     /// @notice Returns the address of the Pyth contract deployed on the chain.
     function pyth() external view returns (IPyth);

src/morpho-pyth/libraries/PythFeedLib.sol

@@ -6,21 +6,43 @@ import {PythStructs} from "@pythnetwork/pyth-sdk-solidity/PythStructs.sol";
 import {PythErrorsLib} from "./PythErrorsLib.sol";
 /// @title PythFeedLib
 /// @author Pyth Data Association
-/// @notice Library exposing functions to interact with a Pyth feed.
+/// @notice Library exposing functions to interact with a Pyth feed
+/// @dev This library provides basic price fetching from Pyth feeds with staleness protection.
+///
+/// SECURITY LIMITATION: This implementation ignores Pyth's confidence intervals (conf field).
+/// Pyth aggregates prices from multiple providers and returns both the price and a confidence
+/// interval indicating price uncertainty. Large confidence intervals suggest unreliable prices
+/// that should potentially be rejected.
 
 library PythFeedLib {
-    /// @dev Returns the price of a `priceId`.
-    /// @dev When `priceId` is the address zero, returns 1.
-    /// @dev If the price is older than `maxAge`, throws `0x19abf40e` StalePrice Error.
+    /// @notice Returns the price of a Pyth price feed
+    /// @param pyth The Pyth contract instance
+    /// @param priceId The Pyth price feed identifier, or bytes32(0) to return 1
+    /// @param maxAge Maximum acceptable price age in seconds
+    /// @return The price value (always positive)
+    /// @dev When `priceId` is bytes32(0), returns 1 (useful for omitting feeds in calculations)
+    /// @dev Reverts with `0x19abf40e` StalePrice error if price is older than `maxAge`
+    /// @dev Reverts if price is negative (should not occur with valid Pyth feeds)
+    ///
+    /// SECURITY WARNING: This function ignores the confidence interval (price.conf) returned by Pyth.
     function getPrice(IPyth pyth, bytes32 priceId, uint256 maxAge) internal view returns (uint256) {
         if (priceId == bytes32(0)) return 1;
 
         PythStructs.Price memory price = pyth.getPriceNoOlderThan(priceId, maxAge);
         require(int256(price.price) >= 0, PythErrorsLib.NEGATIVE_ANSWER);
+
+        // NOTE: price.conf (confidence interval) is not validated here
+        // Large conf values indicate uncertain prices that may need rejection
+
         return uint256(int256(price.price));
     }
-    /// @dev Returns the number of decimals of a `priceId`.
-    /// @dev When `priceId` is the address zero, returns 0.
+    /// @notice Returns the number of decimal places for a Pyth price feed
+    /// @param pyth The Pyth contract instance
+    /// @param priceId The Pyth price feed identifier, or bytes32(0) to return 0
+    /// @return The number of decimal places for the price feed
+    /// @dev When `priceId` is bytes32(0), returns 0 (useful for omitting feeds in calculations)
+    /// @dev Uses getPriceUnsafe() which does not validate price age - only for decimal retrieval
+    /// @dev Converts negative exponent to positive decimal count (e.g., expo=-8 returns 8)
 
     function getDecimals(IPyth pyth, bytes32 priceId) internal view returns (uint256) {
         if (priceId == bytes32(0)) return 0;

src/morpho-pyth/libraries/VaultLib.sol

@@ -5,10 +5,16 @@ import {IERC4626} from "../../interfaces/IERC4626.sol";
 
 /// @title VaultLib
 /// @author Pyth Data Association
-/// @notice Library exposing functions to price shares of an ERC4626 vault.
+/// @notice Library exposing functions to price shares of an ERC4626 vault
+/// @dev This library provides share-to-asset conversion for ERC-4626 vaults used in price calculations.
+/// Users should only use vaults with manipulation-resistant designs and trusted governance.
+
 library VaultLib {
-    /// @dev Converts `shares` into the corresponding assets on the `vault`.
-    /// @dev When `vault` is the address zero, returns 1.
+    /// @notice Converts vault shares to underlying asset amount
+    /// @param vault The ERC-4626 vault contract, or address(0) to return 1
+    /// @param shares The amount of vault shares to convert
+    /// @return The equivalent amount of underlying assets
+    /// @dev When `vault` is address(0), returns 1 (useful for skipping vault conversion)
     function getAssets(IERC4626 vault, uint256 shares) internal view returns (uint256) {
         if (address(vault) == address(0)) return 1;