ERC-4626 Price Feed Audit

1

Summary

Timeline: 2024-10-22 – 2024-10-28

Total Issues: 4 (4 resolved)

Low Severity Issues: 1 (1 resolved)

Notes & Additional Information: 3 (3 resolved)

Scope

OpenZeppelin reviewed pull request #929 of the compound-finance/comet repository at commit 53bc4b9.

In scope were the following files:

contracts
├── IERC4626.sol
└── pricefeeds
    └── PriceFeedWith4626Support.sol

System Overview

The ERC-4626 standard has been designed to optimize and unify the technical parameters of yield-bearing vaults. ERC-4626 tokens are non-rebasing, standard tokens that are based on ERC-20 and represent the shares of a single underlying yield-bearing ERC-20 token.

The PriceFeedWith4626Support contract enables the pricing of ERC-4626 tokens for use in Compound v3 markets as collateral assets. This contract leverages the convertToAssets function from an ERC-4626-compatible rate provider to convert shares to the corresponding amount of the underlying assets. This converted value is then paired with a Chainlink oracle feed that prices the underlying asset in terms of the market’s chosen denomination, enabling consistent valuation across different assets within the market.

Security Model and Trust Assumptions

The reliability, robustness, and accuracy of the PriceFeedWith4626Support contract rely on the security of the underlying ERC-4626 implementation and, particularly, on the behavior of the convertToAssets function which must comply with the ERC-4626 specification.

For safe integration into the Comet market, any underlying ERC-4626 token used with this price feed must satisfy the following conditions:

  • Specification Compliance: Adheres strictly to the ERC-4626 specification.
  • Audit and Security Review: Has been audited and is free from share manipulation ERC-4626 vulnerabilities, such as direct or stealth donations.
  • Upgradeability: Is either non-upgradeable or has upgrade permissions controlled by a decentralized or trusted entity.
  • Fees: The rate provider must not incorporate fees. Otherwise, the convertToAssets function would return an artificially inflated share price.
  • Custom Vault Logic: The rate provider should avoid implementing custom vault mechanisms, such as delayed withdrawals or other constraints that could affect share value. These mechanisms may lead to shares trading at a discount compared to their face value by impacting the availability or liquidity of the underlying assets, while the convertToAssets function would still return the inflated face value.
  • Flash Loan Prevention: Does not contain flash loan functionality.
  • Liquidity and Slippage: Is highly liquid, ensuring negligible slippage in share-to-asset conversions. Otherwise, the convertToAssets function could return an inflated share price.

L2 Sequencer Availability: For L2 Comets, it is assumed that the Chainlink data feed will provide consistent and accurate prices under normal network conditions. However, in the rare event of an L2 sequencer outage, the AggregatorV3Interface(underlyingPriceFeed).latestRoundData function within PriceFeedWith4626Support.sol may return less reliable data due to potential delays in the underlying network’s processing. While this scenario is unlikely and the impact on price accuracy might not be relevant, it has nonetheless been included here to acknowledge the potential dependence on sequencer availability in L2 networks.

Collateral Token Analysis and Assumptions

The following ERC-4626 tokens have been assessed for their suitability as collateral assets within the Compound v3 protocol, utilizing the PriceFeedWith4626Support contract. Meeting the ERC-4626 standard requirements is essential for each token as the PriceFeedWith4626Support depends heavily on the convertToAssets function to ensure the reliable conversion of shares into the underlying assets. This section provides a summary of the analysis and recommendations for each evaluated token.

  1. sFRAX
    • Audit Details: The audit conducted by Trail of Bits was based on commit 0761cf8, which slightly differs from the deployed version. However, the changes are superficial and do not affect the security of the contract.
    • Conclusion: Approved for use as collateral.
  2. wUSDM
    • Audit Details: Audited by OpenZeppelin, the deployed wUSDM contract mirrors the audited version with the sole difference of an additional __gap storage slot (49 slots instead of 48). The wUSDM contract is upgradeable, so any changes depend on the security and reliability of its upgrade governance. It also includes a PAUSE_ROLE feature that allows a specific address to pause all actions, including share minting and redemption. During a pause, the price feed will continue providing exchange rates based on the current supply of shares and assets within the vault, with value changes only being possible via direct asset donations to the ERC-4626 contract during the paused state.
    • Conclusion: Approved for use as collateral, with the recommendation to monitor upgrade governance and PAUSE_ROLE functionality.
  3. sUSDS
    • Audit Details: Audited by ChainSecurity at commit e1d160a, which matches the deployed version on Mainnet. The sUSDS contract is upgradeable, so any changes depend on the security and reliability of its upgrade governance. Also, the L2 implementation is not ERC-4626 compatible, and should not be used with the PriceFeedWith4626Support contract for any L2 Comet market.
    • Conclusion: Approved for use as collateral in Mainnet Comets.
  4. pufETH
    • Audit Details: The PufferVaultV3 contract has not undergone an audit. Additionally, it includes fees that are not factored into the convertToAssets calculation, resulting in an inflated share price. Since the contract is upgradeable, any adjustments - including changes to the fee, which is currently set at 1% - depend on the security and reliability of its upgrade governance.
    • Conclusion: Not recommended for use as collateral due to the absence of an audit and the incorporation of fees. A thorough audit is required for reconsideration.
  5. sUSDe
    • Audit Details: The GitHub repository is private, limiting verification of the deployed version against an audited version. Also, it includes a cooldown feature that can prevent withdrawals for a set period, creating a potential risk of insolvency for the protocol.
    • Conclusion: The cooldown feature poses a significant risk to the protocol by delaying liquidations. Consequently, this asset is not recommended for use as collateral in any Comet market. Further analysis is advised should the cooldown feature be permanently disabled.

ERC-4626 tokens are intended to be used solely as collateral assets. Using them as base assets could expose users to unfair liquidation risks if a malicious actor directly donates tokens to the vault, artificially inflating the ERC-4626 share price.

Privileged Roles

The PriceFeedWith4626Support contract has been designed without any privileged roles. All configuration parameters are fixed at deployment, with no permissions for modification post-deployment, ensuring immutable settings and enhancing security.

Low Severity

Missing Explicit Visibility for Variables

The rateProviderDecimals and underlyingDecimals variables in the PriceFeedWith4626Support contract do not have explicit visibility modifiers. This means that the visibility for these two functions will default to internal.

To improve code clarity and avoid potential confusion, consider explicitly specifying the visibility of all contract variables.

Update: Resolved in commit c3044ee.

Notes & Additional Information

Missing Docstring

The description_ parameter of the constructor lacks a relevant docstring.

Consider adding a docstring for the description_ parameter to enhance code quality and improve readability.

Update: Resolved in commit c3044ee.

Variables Could Be immutable

If a variable is only ever assigned a value from within the constructor of a contract or during compile time, it could be declared as immutable. Within PriceFeedWith4626Support.sol, multiple instances of variables that could be immutable were identified:

To better convey the intended use of variables and to potentially save gas, consider adding the immutable keyword to variables that are only set in the constructor.

Update: Resolved in commit c3044ee.

Incomplete IERC4626 Interface

ERC-4626 is an extension of the ERC-20 standard. As such, for completeness, the IERC4626 interface should inherit from both the IERC20 and IERC20Metadata interfaces.

Consider using the latest version of OpenZeppelin IERC4626 (v5.1.0) to ensure that the interface is complete.

Update: Resolved in commit c3044ee.

Conclusion

The PriceFeedWithERC4626Support contract is intended for pricing ERC-4626 tokens in Comet markets. It leverages the convertToAssets function of the underlying ERC-4626 contract to price tokenized vault shares in terms of tokens. As such, the contract depends on the ERC-4626 vault’s ability to securely hold assets and prevent manipulation of shares, ensuring accurate share pricing through convertToAssets. Extreme caution must be exercised when incorporating any ERC-4626 token with this price feed for use in Comet markets. This is because ERC-4626 contracts may implement custom logic that may be vulnerable or affect the accuracy of convertToAssets, potentially compromising the security of the Compound v3 markets that utilize them.