Compound Mainnet USDT Migration Review
June 24-26, 2024
Summary
Medium Severity Issues: 1 (1 Partially Resolved)
Notes & Additional Information: 1 (1 Unresolved)
Total Issues: 2 (1 Partially Resolved, 1 Unresolved)
Scope
OpenZeppelin reviewed Pull Request 862 of the compound-finance/comet repository at commit 3ade273 which will submit a governance proposal to create a USDT Comet market on Mainnet using the official Comet Migration process.
In scope was the governance proposal created by the simulated Enact Workflow using the migration files:
.github/workflows/enact-migration.yaml
contracts/Comet.sol
contracts/CometCore.sol
contracts/CometMainInterface.sol
contracts/IERC20NonStandard.sol
deployments/mainnet/usdt/migrations/1717664323_configurate_and_ens.ts
Reviewing base and collateral assets of the USDT Comet market on Mainnet was out of scope.
Overview
As described in the Compound Community Forum Proposal, Woof Software and the Compound Growth Program propose launching the USDT market on Mainnet with wstETH, ETH, WBTC, COMP, UNI and LINK as collateral.
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 862 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 correct oracle price feeds were used.
- 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
Before proposal enactment to incorporate the USDT market on Mainnet, 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.
Medium Severity
Misconfigured and Wrong Oracles Used
All the collaterals for the USDT market should be priced in USD. However, the wstETH asset is priced in ETH instead of USD. Due to a misconfigured address in the WBTC oracle, it is also pricing the collateral in ETH instead of USD. Both of these will cause problems when attempting to return the appropriate price such that borrowers won’t be able to borrow using these as collateral.
Consider using the MultiplicativePriceFeed contract to get the wstETH and the WBTC price in USD by utilizing the wstETH/ETH and ETH/USD price feeds for wstETH and the WBTC/BTC and BTC/USD price feeds for WBTC. cbETH collateral is priced in USD similarly on Base.
Update: Partially resolved in commit ea783f7.
The wstETH asset is priced using the WstETHPriceFeed with the stETHtoETHPriceFeed parameter set to the address of a stETh/USD price feed because there are not any wstETH price feeds provided by ChainLink on Mainnet to use with the MultiplicativePriceFeed. Although, this solution will function as expected, the description and some identifiers describe its intended use with a stETH/ETH price feed instead. Consider creating a duplicate contract with updated description and identifiers to avoid possible confusion.
The misconfigured BTC/USD price feed address was resolved.
Notes & Additional Information
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. This record value is determined during development and constructed in the migration script before proposal. The setText method of the ENS Resolver used in this migration only allows replacing the current value with a new value. There is a chance that this proposal can unintentionally remove recently added market details or include the anticipated effects of defeated proposals.
To prevent such a scenario from occurring, consider these options:
Use setText One Proposal At a Time
Wait until all proposals that use setText to update the ENS record have either executed or failed before constructing the record text set by setText. If another proposal is submitted after record construction and before proposal, wait and reconstruct. If any another proposal using setText to update the ENS record is proposed, cancel all but one of the active proposals that have not been defeated.
Call a Wrapper of setText
Create and deploy an audited contract and method that constructs the ENS record text at the time of proposal execution so that the value is set correctly even if the ENS record changes after proposal. Use the value of the ENS record at the time of proposal execution with the new chainId, baseSymbol, and market address mapping to determine the new text value to set with setText. Calling this new wrapper method, instead of calling setText directly as a proposal step, will preserve the current record value and only insert the new market details without the risk of overwriting the changes of another proposal. This option is compatible with any number of other concurrent proposals using the same wrapper method.
Update: Unresolved by commit ea783f7. The script reads the curent ENS record at the time of proposal and inserts market mappings of proposals pending execution as of the date of the commit so that this Mainnet USDT Market can be proposed before the pending proposals are executed. This implmentation assumes all proposals are executed in order will overwrite any unanticipated changes to the ENS record that occur between proposal and execution.
Conclusion
Upon reviewing the calldata generated by the migration script, we found one medium severity issue and one informational severity issue. Apart from this, the proposal has been correctly created and follows all current recommendations.
. 
