Summary
Timeline: From 2024-09-09 To 2024-09-18
Total Issues: 5 (4 resolved)
High Severity Issues: 1 (1 resolved)
Low Severity Issues: 2 (1 resolved)
Notes & Additional Information: 2 (2 resolved)
Scope
OpenZeppelin reviewed pull request #939 of the compound-finance/comet repository at commit ca9253a. This pull request will submit a governance proposal to create a USDe Comet market on the Mantle Network using the official Comet Migration process.
In scope was the governance proposal calldata generated by the simulated Deploy Market Enact Workflow, using the migration files:
.github/workflows/enact-migration.yml
deployments/mantle/usde/migrations1727774346_configurate_and_ens.ts
System Overview
As described in the Compound Community Forum Proposal, the Compound Growth Program proposes launching the USDe market on the Mantle Network. The current scope for the USDe Comet market includes mETH and WETH as collateral assets.
To deploy an official market on Mainnet, a series of specific Compound Governance steps must be taken. Dependencies must be accurately and securely deployed after which an official governance proposal is to be proposed on-chain. The focus of OpenZeppelin’s review was Pull Request #939 which includes the deployed dependencies, the official migration governance proposal, and all the security concerns that may arise during execution of the proposal. Specifically we checked:
- That correct contracts were deployed on-chain.
- That the simulation workflow created the same proposal as the deployment workflow.
- That the proposal description was accurate and descriptive of its effects.
- That proposal dependencies were verified as deployed with correct configurations.
- That proposal instructions were decoded and verified as complete.
- That proposal instructions were simulated and executed successfully without errors.
- That the eventual market configuration was verified to match the parameter values provided by Gauntlet and accepted by the community.
Security Model and Trust Assumptions
The market leverages API3 price feeds for the base and collateral assets. While our team at OpenZeppelin has audited the integration with Compound, we have not reviewed the underlying price feeds and the API3 architecture.
Additionally, the data returned by the latestRoundData function of the underlying Api3PartialAggregatorV2V3Interface does not support roundIds and sets both the updatedAt and startedAt timestamps to the same value. This off-chain timestamp reflects the moment when the first-party oracles signed the data for the latest update. It is possible for this value to decrease between updates, particularly if the API3 feed’s data sources are updated to a new set with less recent updates. However, neither the roundId nor these timestamps are used by the Comet contracts.
Before proposal enactment to incorporate the USDe market on Mantle, it is assumed that there will not be any upgrades or changes to the implementations of the proposal. It is also assumed that any changes from the Comet ecosystem at large such as any other governance proposals between this audit and this proposal execution should not have any effects.
Overall, utilizing API3 as an oracle on the Mantle network relies on three key assumptions: (1) Currently, the API3 team controls the OEVBeneficiary address within the oracles, giving them complete authority over the extracted OEV, which represents the cumulative amounts proposed by auction winners to secure update rights. To ensure API3 positively impacts the protocol, these funds should be allocated in ways that directly benefit the protocol. (2) The architecture and implementation of API3 oracles are free from bugs and have been reviewed by others in the Web3 community to ensure proper functionality. (3) There is a sufficient number of reliable nodes providing accurate price updates for each oracle and acting in good faith.
High Severity
USDT-USDe swap can be sandwiched
Whenever a new Comet market is deployed, it is seeded with the base token as reserves. For the USDe market, USDT is bridged from Mainnet and then is swapped for USDe on Mantle using the Merchant Moe’s USDe-USDT pool.
USDT is swapped for USDe by directly calling the swap function on the pool contract. This function has no slippage protection and is vulnerable to frontrunning. As a result, the entire USDT value could be lost during the swap operation.
Consider using a router contract that has built-in slippage protection for the USDT to USDe swap.
Update: Resolved in pull request #85 at commit fe24058. The team stated
We use a router to set up slippage.
Low Severity
Misleading Docstring in ScalingPriceFeed
The underlyingPriceFeed docstring incorrectly refers to a Chainlink price feed. This can lead to confusion, especially in blockchain explorers.
Consider updating the docstring to accurately reflect that the underlying price feeds are provided by API3.
Update: Resolved in pull request #85 at commit fe24058. The team stated
We updated ScalingPriceFeed to make it impersonate to price feed provider like chainlink. Also, we create ScalingPriceFeedWithCustomDescription that allows to provide description. It is the same as ScalingPriceFeed contract.
Prevention of ENS Record Mismatch
As part of the migration process, the address of every deployed market is added to the v3-official-markets record of the v3-additional-grants.compound-community-licenses.eth domain. Since the setText method of the ENS Resolver can only replace the current value with a new value, there is a risk that this this proposal can unintentionally remove recently added values.
For example, the current proposal will remove the AERO market on Base from the ENS record since it is not included in the data that will be sent to the setText method.
To prevent this, consider:
- Copying the latest value from the
v3-official-marketsrecord of thev3-additional-grants.compound-community-licenses.eth. - Waiting until any other live proposal affecting the ENS record has either succeeded or failed.
- Ensuring that any queued changes to the ENS record are reflected in this proposal.
Update: Acknowledged, will resolve. The team stated:
Currently, we have USDS market, that is queue, but not executed. Before the on-chain proposal push, we will verify that ENS records set up correctly.
Notes & Additional Information
FBTC is not Listed As a Collateral Asset
Contrary to the Mantle USDe Market discussion, FBTC is not listed as a collateral asset to the USDe Comet on Mantle.
Consider adding the missing collateral asset or discussing the reasons for its omission publicly to ensure transparency within the community.
Update: Resolved in pull request #85 at commit fe24058. The team stated
When we provided migration, API3 did not support FBTC and we asked to start supporting it. The updated migration already includes FBTC collateral.
Missing Descriptions in ScalingPriceFeed Contracts
The ScalingPriceFeed contracts used for the baseTokenPriceFeed and WETH’s priceFeed are missing a description.
Consider adding a description for each price feed to clarify their respective roles.
Update: Resolved in pull request #85 at commit fe24058. The team stated
We create ScalingPriceFeedWithCustomDescription that allows us to provide the description. It is the same as the ScalingPriceFeed contract.
Conclusion
After reviewing the latest calldata generated by the migration script, our team at OpenZeppelin has verified that the migration should execute successfully. However, as OpenZeppelin has not conducted a detailed review of the underlying API3 oracles, we can only rely on them under specific trust assumptions outlined in this review or they may introduce risks to the USDe Comet Market.