From 1fb3863bfc94c031e65a80a5cc4a5c52ec7b9df7 Mon Sep 17 00:00:00 2001 From: Kevin Cheng Date: Thu, 5 Oct 2023 13:31:20 -0700 Subject: [PATCH] Clean up code and documentation (#12) This PR removes some unneeded internal functions and improves the documentation in the contract. There are no functional changes in this PR. --- src/CometHelpers.sol | 46 +++--- src/CometWrapper.sol | 312 +++++++++++++++++++++++++--------------- test/CometWrapper.t.sol | 28 ++-- test/Rewards.t.sol | 7 +- 4 files changed, 236 insertions(+), 157 deletions(-) diff --git a/src/CometHelpers.sol b/src/CometHelpers.sol index a64d2f1..5a84ead 100644 --- a/src/CometHelpers.sol +++ b/src/CometHelpers.sol @@ -3,8 +3,11 @@ pragma solidity 0.8.17; import {CometMath} from "./vendor/CometMath.sol"; -/// @notice Includes helper functions ripped from different contracts in Comet instead -/// of copying whole contracts. Also includes error definitions, events, and constants. +/** + * @title Comet Helper Functions + * @notice Pure helper functions taken from different contracts in Comet + * @author Compound & gjaldon + */ contract CometHelpers is CometMath { uint64 internal constant FACTOR_SCALE = 1e18; uint64 internal constant BASE_INDEX_SCALE = 1e15; @@ -15,23 +18,19 @@ contract CometHelpers is CometMath { DOWN } - error InsufficientAllowance(); - error TimestampTooLarge(); - error UninitializedReward(); - error ZeroShares(); - error ZeroAddress(); - - event RewardClaimed(address indexed src, address indexed recipient, address indexed token, uint256 amount); - - /// @dev Multiply a number by a factor - /// https://github.com/compound-finance/comet/blob/main/contracts/Comet.sol#L681-L683 + /** + * @dev Multiply a number by a factor + * https://github.com/compound-finance/comet/blob/main/contracts/Comet.sol#L681-L683 + */ function mulFactor(uint256 n, uint256 factor) internal pure returns (uint256) { return n * factor / FACTOR_SCALE; } - /// @dev The principal amount projected forward by the supply index - /// Note: The returned value can be rounded up or down - /// From https://github.com/compound-finance/comet/blob/main/contracts/CometCore.sol#L83-L85 + /** + * @dev The principal amount projected forward by the supply index + * Note: The returned value can be rounded up or down + * From https://github.com/compound-finance/comet/blob/main/contracts/CometCore.sol#L83-L85 + */ function presentValueSupply(uint64 baseSupplyIndex_, uint256 principalValue_, Rounding rounding) internal pure returns (uint256) { if (rounding == Rounding.DOWN) { return principalValue_ * baseSupplyIndex_ / BASE_INDEX_SCALE; @@ -40,10 +39,12 @@ contract CometHelpers is CometMath { } } - /// @dev The present value projected backward by the supply index (rounded down) - /// Note: The returned value can be rounded up or down - /// Note: This will overflow (revert) at 2^104/1e18=~20 trillion principal for assets with 18 decimals. - /// From https://github.com/compound-finance/comet/blob/main/contracts/CometCore.sol#L109-L111 + /** + * @dev The present value projected backward by the supply index (rounded down) + * Note: The returned value can be rounded up or down + * Note: This will overflow (revert) at 2^104/1e18=~20 trillion principal for assets with 18 decimals. + * From https://github.com/compound-finance/comet/blob/main/contracts/CometCore.sol#L109-L111 + */ function principalValueSupply(uint64 baseSupplyIndex_, uint256 presentValue_, Rounding rounding) internal pure returns (uint104) { if (rounding == Rounding.DOWN) { return safe104((presentValue_ * BASE_INDEX_SCALE) / baseSupplyIndex_); @@ -51,11 +52,4 @@ contract CometHelpers is CometMath { return safe104((presentValue_ * BASE_INDEX_SCALE + baseSupplyIndex_ - 1) / baseSupplyIndex_); } } - - /// @dev The current timestamp - /// From https://github.com/compound-finance/comet/blob/main/contracts/Comet.sol#L375-L378 - function getNowInternal() internal view virtual returns (uint40) { - if (block.timestamp >= 2**40) revert TimestampTooLarge(); - return uint40(block.timestamp); - } } diff --git a/src/CometWrapper.sol b/src/CometWrapper.sol index ac279e0..5100cb9 100644 --- a/src/CometWrapper.sol +++ b/src/CometWrapper.sol @@ -8,8 +8,11 @@ import { CometInterface, TotalsBasic } from "./vendor/CometInterface.sol"; import { CometHelpers } from "./CometHelpers.sol"; import { ICometRewards } from "./vendor/ICometRewards.sol"; -/// @notice A vault contract that accepts deposits of a Comet token like cUSDCv3 as an asset -/// and mints shares which are the Wrapped Comet token. +/** + * @title Comet Wrapper + * @notice Wrapper contract that adds ERC4626 and ERC7246 functionality to the rebasing Comet token (e.g. cUSDCv3) + * @author Compound & gjaldon + */ contract CometWrapper is ERC4626, CometHelpers { using SafeTransferLib for ERC20; @@ -18,39 +21,73 @@ contract CometWrapper is ERC4626, CometHelpers { uint64 baseTrackingIndex; } + /// @notice Mapping of users to basic data mapping(address => UserBasic) public userBasic; + + /// @notice Mapping of users to their rewards claimed mapping(address => uint256) public rewardsClaimed; + /// @notice The Comet address that this contract wraps CometInterface public immutable comet; + + /// @notice The CometRewards address that this contract can claim rewards from ICometRewards public immutable cometRewards; + + /// @notice The scale for reward tracking uint256 public immutable trackingIndexScale; + + /// @notice Factor to divide by when accruing rewards in order to preserve 6 decimals (i.e. baseScale / 1e6) uint256 internal immutable accrualDescaleFactor; - constructor(ERC20 _asset, ICometRewards _cometRewards, string memory _name, string memory _symbol) - ERC4626(_asset, _name, _symbol) + /** Custom errors **/ + + error InsufficientAllowance(); + error TimestampTooLarge(); + error UninitializedReward(); + error ZeroShares(); + error ZeroAddress(); + + /** Custom events **/ + + /// @notice Event emitted when a reward is claimed for a user + event RewardClaimed(address indexed src, address indexed recipient, address indexed token, uint256 amount); + + /** + * @notice Construct a new Comet Wrapper instance + * @param comet_ The Comet token to wrap + * @param cometRewards_ The rewards contract for the Comet market + * @param name_ The wrapper token name + * @param symbol_ The wrapper token symbol + */ + constructor(ERC20 comet_, ICometRewards cometRewards_, string memory name_, string memory symbol_) + ERC4626(comet_, name_, symbol_) { - if (address(_cometRewards) == address(0)) revert ZeroAddress(); + if (address(cometRewards_) == address(0)) revert ZeroAddress(); // minimal validation that contract is CometRewards - _cometRewards.rewardConfig(address(_asset)); + cometRewards_.rewardConfig(address(comet_)); - comet = CometInterface(address(_asset)); - cometRewards = _cometRewards; + comet = CometInterface(address(comet_)); + cometRewards = cometRewards_; trackingIndexScale = comet.trackingIndexScale(); accrualDescaleFactor = uint64(10 ** asset.decimals()) / BASE_ACCRUAL_SCALE; } - /// @notice Returns total assets managed by the vault - /// @return total assets + /** + * @notice Returns total assets managed by the vault + * @return total assets + */ function totalAssets() public view override returns (uint256) { uint64 baseSupplyIndex_ = accruedSupplyIndex(); uint256 supply = totalSupply; return supply > 0 ? presentValueSupply(baseSupplyIndex_, supply, Rounding.DOWN) : 0; } - /// @notice Deposits assets into the vault and gets shares (Wrapped Comet token) in return - /// @param assets The amount of assets to be deposited by the caller - /// @param receiver The recipient address of the minted shares - /// @return The amount of shares that are minted to the receiver + /** + * @notice Deposits assets into the vault and gets shares (Wrapped Comet token) in return + * @param assets The amount of assets to be deposited by the caller + * @param receiver The recipient address of the minted shares + * @return The amount of shares that are minted to the receiver + */ function deposit(uint256 assets, address receiver) public override returns (uint256) { asset.safeTransferFrom(msg.sender, address(this), assets); @@ -65,10 +102,12 @@ contract CometWrapper is ERC4626, CometHelpers { return shares; } - /// @notice Mints shares (Wrapped Comet) in exchange for Comet tokens - /// @param shares The amount of shares to be minted for the receive - /// @param receiver The recipient address of the minted shares - /// @return The amount of assets that are deposited by the caller + /** + * @notice Mints shares (Wrapped Comet) in exchange for Comet tokens + * @param shares The amount of shares to be minted for the receive + * @param receiver The recipient address of the minted shares + * @return The amount of assets that are deposited by the caller + */ function mint(uint256 shares, address receiver) public override returns (uint256) { if (shares == 0) revert ZeroShares(); @@ -83,12 +122,14 @@ contract CometWrapper is ERC4626, CometHelpers { return assets; } - /// @notice Withdraws assets (Comet) from the vault and burns corresponding shares (Wrapped Comet). - /// Caller can only withdraw assets from owner if they have been given allowance to. - /// @param assets The amount of assets to be withdrawn by the caller - /// @param receiver The recipient address of the withdrawn assets - /// @param owner The owner of the assets to be withdrawn - /// @return The amount of shares of the owner that are burned + /** + * @notice Withdraws assets (Comet) from the vault and burns corresponding shares (Wrapped Comet). + * Caller can only withdraw assets from owner if they have been given allowance to. + * @param assets The amount of assets to be withdrawn by the caller + * @param receiver The recipient address of the withdrawn assets + * @param owner The owner of the assets to be withdrawn + * @return The amount of shares of the owner that are burned + */ function withdraw(uint256 assets, address receiver, address owner) public override returns (uint256) { accrueInternal(owner); uint256 shares = previewWithdraw(assets); @@ -110,12 +151,14 @@ contract CometWrapper is ERC4626, CometHelpers { return shares; } - /// @notice Redeems shares (Wrapped Comet) in exchange for assets (Wrapped Comet). - /// Caller can only redeem shares from owner if they have been given allowance to. - /// @param shares The amount of shares to be redeemed - /// @param receiver The recipient address of the withdrawn assets - /// @param owner The owner of the shares to be redeemed - /// @return The amount of assets that is withdrawn and sent to the receiver + /** + * @notice Redeems shares (Wrapped Comet) in exchange for assets (Wrapped Comet). + * Caller can only redeem shares from owner if they have been given allowance to. + * @param shares The amount of shares to be redeemed + * @param receiver The recipient address of the withdrawn assets + * @param owner The owner of the shares to be redeemed + * @return The amount of assets that is withdrawn and sent to the receiver + */ function redeem(uint256 shares, address receiver, address owner) public override returns (uint256) { if (shares == 0) revert ZeroShares(); if (msg.sender != owner) { @@ -137,20 +180,24 @@ contract CometWrapper is ERC4626, CometHelpers { return assets; } - /// @notice Transfer shares from caller to the recipient - /// @param to The receiver of the shares (Wrapped Comet) to be transferred - /// @param amount The amount of shares to be transferred - /// @return bool Indicates success of the transfer + /** + * @notice Transfer shares from caller to the recipient + * @param to The receiver of the shares (Wrapped Comet) to be transferred + * @param amount The amount of shares to be transferred + * @return bool Indicates success of the transfer + */ function transfer(address to, uint256 amount) public override returns (bool) { transferInternal(msg.sender, to, amount); return true; } - /// @notice Transfer shares from a specified source to a recipient - /// @param from The source of the shares to be transferred - /// @param to The receiver of the shares (Wrapped Comet) to be transferred - /// @param amount The amount of shares to be transferred - /// @return bool Indicates success of the transfer + /** + * @notice Transfer shares from a specified source to a recipient + * @param from The source of the shares to be transferred + * @param to The receiver of the shares (Wrapped Comet) to be transferred + * @param amount The amount of shares to be transferred + * @return bool Indicates success of the transfer + */ function transferFrom(address from, address to, uint256 amount) public override returns (bool) { uint256 allowed = msg.sender == from ? type(uint256).max : allowance[from][msg.sender]; if (allowed < amount) revert InsufficientAllowance(); @@ -162,6 +209,10 @@ contract CometWrapper is ERC4626, CometHelpers { return true; } + /** + * @dev Update the balances of the addresses involved in a token transfer. Before the balances are updated, + * interest is first accrued and tracking indices are updated. + */ function transferInternal(address from, address to, uint256 amount) internal { // Accrue rewards before transferring assets comet.accrueAccount(address(this)); @@ -174,21 +225,25 @@ contract CometWrapper is ERC4626, CometHelpers { emit Transfer(from, to, amount); } - /// @notice Total assets of an account that are managed by this vault - /// @dev The asset balance is computed from an account's shares balance which mirrors how Comet - /// computes token balances. This is done this way since balances are ever-increasing due to - /// interest accrual. - /// @param account The address to be queried - /// @return The total amount of assets held by an account + /** + * @notice Total assets of an account that are managed by this vault + * @dev The asset balance is computed from an account's shares balance which mirrors how Comet + * computes token balances. This is done this way since balances are ever-increasing due to + * interest accrual. + * @param account The address to be queried + * @return The total amount of assets held by an account + */ function underlyingBalance(address account) public view returns (uint256) { uint64 baseSupplyIndex_ = accruedSupplyIndex(); uint256 principal = balanceOf[account]; return principal > 0 ? presentValueSupply(baseSupplyIndex_, principal, Rounding.DOWN) : 0; } - /// @dev Updates an account's `baseTrackingAccrued` which keeps track of rewards accrued by the account. - /// This uses the latest `trackingSupplyIndex` from Comet to compute for rewards accrual for accounts - /// that supply the base asset to Comet. + /** + * @dev Updates an account's `baseTrackingAccrued` which keeps track of rewards accrued by the account. + * This uses the latest `trackingSupplyIndex` from Comet to compute for rewards accrual for accounts + * that supply the base asset to Comet. + */ function updateTrackingIndex(address account) internal { UserBasic memory basic = userBasic[account]; uint256 principal = balanceOf[account]; @@ -203,22 +258,30 @@ contract CometWrapper is ERC4626, CometHelpers { userBasic[account] = basic; } + /** + * @dev Update the interest accrued to the wrapper and the tracking index for an account + */ function accrueInternal(address account) internal { comet.accrueAccount(address(this)); updateTrackingIndex(account); } - /// @notice Get the reward owed to an account - /// @dev This is designed to exactly match computation of rewards in Comet - /// and uses the same configuration as CometRewards. It is a combination of both - /// [`getRewardOwed`](https://github.com/compound-finance/comet/blob/63e98e5d231ef50c755a9489eb346a561fc7663c/contracts/CometRewards.sol#L110) and [`getRewardAccrued`](https://github.com/compound-finance/comet/blob/63e98e5d231ef50c755a9489eb346a561fc7663c/contracts/CometRewards.sol#L171). - /// @param account The address to be queried - /// @return The total amount of rewards owed to an account + /** + * @notice Get the reward owed to an account + * @dev This is designed to exactly match computation of rewards in Comet + * and uses the same configuration as CometRewards. It is a combination of both + * [`getRewardOwed`](https://github.com/compound-finance/comet/blob/63e98e5d231ef50c755a9489eb346a561fc7663c/contracts/CometRewards.sol#L110) and [`getRewardAccrued`](https://github.com/compound-finance/comet/blob/63e98e5d231ef50c755a9489eb346a561fc7663c/contracts/CometRewards.sol#L171). + * @param account The address to be queried + * @return The total amount of rewards owed to an account + */ function getRewardOwed(address account) external returns (uint256) { ICometRewards.RewardConfig memory config = cometRewards.rewardConfig(address(comet)); return getRewardOwedInternal(config, account); } + /** + * @dev Mimics the reward owed calculation in CometRewards to arrive at the reward owed to a user of the wrapper + */ function getRewardOwedInternal(ICometRewards.RewardConfig memory config, address account) internal returns (uint256) { if (config.token == address(0)) revert UninitializedReward(); @@ -226,6 +289,9 @@ contract CometWrapper is ERC4626, CometHelpers { uint256 claimed = rewardsClaimed[account]; uint256 accrued = basic.baseTrackingAccrued; + // Note: Newer CometRewards contracts (those deployed on L2s) store a multiplier and use it during the reward calculation. + // As of 10/05/2023, all the multipliers are currently set to 1e18, so the following code is still compatible. This contract + // will need to properly handle the multiplier if there is ever a rewards contract that sets it to some other value. if (config.shouldUpscale) { accrued *= config.rescaleFactor; } else { @@ -237,9 +303,11 @@ contract CometWrapper is ERC4626, CometHelpers { return owed; } - /// @notice Claims caller's rewards and sends them to recipient - /// @dev Always calls CometRewards for updated configs - /// @param to The address that will receive the rewards + /** + * @notice Claims caller's rewards and sends them to recipient + * @dev Always calls CometRewards for updated configs + * @param to The address that will receive the rewards + */ function claimTo(address to) external { address from = msg.sender; ICometRewards.RewardConfig memory config = cometRewards.rewardConfig(address(comet)); @@ -253,23 +321,26 @@ contract CometWrapper is ERC4626, CometHelpers { } } - /// @notice Accrues rewards for the account - /// @dev Latest trackingSupplyIndex is fetched from Comet so we can compute accurate rewards. - /// This mirrors the logic for rewards accrual in CometRewards so we properly account for users' - /// rewards as if they had used Comet directly. - /// @param account The address to whose rewards we want to accrue - /// @return The UserBasic struct with updated baseTrackingIndex and/or baseTrackingAccrued fields + /** + * @notice Accrues rewards for the account + * @dev Latest trackingSupplyIndex is fetched from Comet so we can compute accurate rewards. + * This mirrors the logic for rewards accrual in CometRewards so we properly account for users' + * rewards as if they had used Comet directly. + * @param account The address to whose rewards we want to accrue + * @return The UserBasic struct with updated baseTrackingIndex and/or baseTrackingAccrued fields + */ function accrueRewards(address account) public returns (UserBasic memory) { comet.accrueAccount(address(this)); updateTrackingIndex(account); - // TODO: can optimize by having updateTrackingIndex return it return userBasic[account]; } - /// @dev This returns latest baseSupplyIndex regardless of whether comet.accrueAccount has been called for the - /// current block. This works like `Comet.accruedInterestedIndices` at but not including computation of - /// `baseBorrowIndex` since we do not need that index in CometWrapper: - /// https://github.com/compound-finance/comet/blob/63e98e5d231ef50c755a9489eb346a561fc7663c/contracts/Comet.sol#L383-L394 + /** + * @dev This returns latest baseSupplyIndex regardless of whether comet.accrueAccount has been called for the + * current block. This works like `Comet.accruedInterestedIndices` at but not including computation of + * `baseBorrowIndex` since we do not need that index in CometWrapper: + * https://github.com/compound-finance/comet/blob/63e98e5d231ef50c755a9489eb346a561fc7663c/contracts/Comet.sol#L383-L394 + */ function accruedSupplyIndex() internal view returns (uint64) { (uint64 baseSupplyIndex_,,uint40 lastAccrualTime) = getSupplyIndices(); uint256 timeElapsed = uint256(getNowInternal() - lastAccrualTime); @@ -281,9 +352,11 @@ contract CometWrapper is ERC4626, CometHelpers { return baseSupplyIndex_; } - /// @dev To maintain accuracy, we fetch `baseSupplyIndex` and `trackingSupplyIndex` directly from Comet. - /// baseSupplyIndex is used on the principal to get the user's latest balance including interest accruals. - /// trackingSupplyIndex is used to compute for rewards accruals. + /** + * @dev To maintain accuracy, we fetch `baseSupplyIndex` and `trackingSupplyIndex` directly from Comet. + * baseSupplyIndex is used on the principal to get the user's latest balance including interest accruals. + * trackingSupplyIndex is used to compute for rewards accruals. + */ function getSupplyIndices() internal view returns (uint64 baseSupplyIndex_, uint64 trackingSupplyIndex_, uint40 lastAccrualTime_) { TotalsBasic memory totals = comet.totalsBasic(); baseSupplyIndex_ = totals.baseSupplyIndex; @@ -291,29 +364,37 @@ contract CometWrapper is ERC4626, CometHelpers { lastAccrualTime_ = totals.lastAccrualTime; } - /// @notice Returns the amount of assets that the Vault would exchange for the amount of shares provided, in an ideal - /// scenario where all the conditions are met. - /// @dev Treats shares as principal and computes for assets by taking into account interest accrual. Relies on latest - /// `baseSupplyIndex` from Comet which is the global index used for interest accrual the from supply rate. - /// @param shares The amount of shares to be converted to assets - /// @return The total amount of assets computed from the given shares + /** + * @notice Returns the amount of assets that the Vault would exchange for the amount of shares provided, in an ideal + * scenario where all the conditions are met. + * @dev Treats shares as principal and computes for assets by taking into account interest accrual. Relies on latest + * `baseSupplyIndex` from Comet which is the global index used for interest accrual the from supply rate. + * @param shares The amount of shares to be converted to assets + * @return The total amount of assets computed from the given shares + */ function convertToAssets(uint256 shares) public view override returns (uint256) { - return convertToAssetsInternal(shares, Rounding.DOWN); + uint64 baseSupplyIndex_ = accruedSupplyIndex(); + return shares > 0 ? presentValueSupply(baseSupplyIndex_, shares, Rounding.DOWN) : 0; } - /// @notice Returns the amount of shares that the Vault would exchange for the amount of assets provided, in an ideal - /// scenario where all the conditions are met. - /// @dev Assets are converted to shares by computing for the principal using the latest `baseSupplyIndex` from Comet. - /// @param assets The amount of assets to be converted to shares - /// @return The total amount of shares computed from the given assets + /** + * @notice Returns the amount of shares that the Vault would exchange for the amount of assets provided, in an ideal + * scenario where all the conditions are met. + * @dev Assets are converted to shares by computing for the principal using the latest `baseSupplyIndex` from Comet. + * @param assets The amount of assets to be converted to shares + * @return The total amount of shares computed from the given assets + */ function convertToShares(uint256 assets) public view override returns (uint256) { - return convertToSharesInternal(assets, Rounding.DOWN); + uint64 baseSupplyIndex_ = accruedSupplyIndex(); + return assets > 0 ? principalValueSupply(baseSupplyIndex_, assets, Rounding.DOWN) : 0; } - /// @notice Allows an on-chain or off-chain user to simulate the effects of their deposit at the current block, given - /// current on-chain conditions. - /// @param assets The amount of assets to deposit - /// @return The total amount of shares that would be minted by the deposit + /** + * @notice Allows an on-chain or off-chain user to simulate the effects of their deposit at the current block, given + * current on-chain conditions. + * @param assets The amount of assets to deposit + * @return The total amount of shares that would be minted by the deposit + */ function previewDeposit(uint256 assets) public view override returns (uint256) { // Calculate shares to mint by calculating the new principal amount uint64 baseSupplyIndex_ = accruedSupplyIndex(); @@ -325,10 +406,12 @@ contract CometWrapper is ERC4626, CometHelpers { return shares; } - /// @notice Allows an on-chain or off-chain user to simulate the effects of their mint at the current block, given - /// current on-chain conditions. - /// @param shares The amount of shares to mint - /// @return The total amount of assets required to mint the given shares + /** + * @notice Allows an on-chain or off-chain user to simulate the effects of their mint at the current block, given + * current on-chain conditions. + * @param shares The amount of shares to mint + * @return The total amount of assets required to mint the given shares + */ function previewMint(uint256 shares) public view override returns (uint256) { // Back out the quantity of assets to deposit in order to increment principal by `shares` uint64 baseSupplyIndex_ = accruedSupplyIndex(); @@ -340,10 +423,12 @@ contract CometWrapper is ERC4626, CometHelpers { return assets; } - /// @notice Allows an on-chain or off-chain user to simulate the effects of their withdrawal at the current block, - /// given current on-chain conditions. - /// @param assets The amount of assets to withdraw - /// @return The total amount of shares required to withdraw the given assets + /** + * @notice Allows an on-chain or off-chain user to simulate the effects of their withdrawal at the current block, + * given current on-chain conditions. + * @param assets The amount of assets to withdraw + * @return The total amount of shares required to withdraw the given assets + */ function previewWithdraw(uint256 assets) public view override returns (uint256) { // Calculate the quantity of shares to burn by calculating the new principal amount uint64 baseSupplyIndex_ = accruedSupplyIndex(); @@ -354,10 +439,12 @@ contract CometWrapper is ERC4626, CometHelpers { return currentPrincipal - newPrincipal; } - /// @notice Allows an on-chain or off-chain user to simulate the effects of their redemption at the current block, - /// given current on-chain conditions. - /// @param shares The amount of shares to redeem - /// @return The total amount of assets that would be withdrawn by the redemption + /** + * @notice Allows an on-chain or off-chain user to simulate the effects of their redemption at the current block, + * given current on-chain conditions. + * @param shares The amount of shares to redeem + * @return The total amount of assets that would be withdrawn by the redemption + */ function previewRedeem(uint256 shares) public view override returns (uint256) { // Back out the quantity of assets to withdraw in order to decrement principal by `shares` uint64 baseSupplyIndex_ = accruedSupplyIndex(); @@ -368,21 +455,22 @@ contract CometWrapper is ERC4626, CometHelpers { return totalAssets() - newBalance; } - /// @notice Maximum amount of the underlying asset that can be withdrawn from the owner balance in the Vault, - /// through a withdraw call. - /// @param owner The owner of the assets to be withdrawn - /// @return The total amount of assets that could be withdrawn + /** + * @notice Maximum amount of the underlying asset that can be withdrawn from the owner balance in the Vault, + * through a withdraw call. + * @param owner The owner of the assets to be withdrawn + * @return The total amount of assets that could be withdrawn + */ function maxWithdraw(address owner) public view override returns (uint256) { return previewRedeem(balanceOf[owner]); } - function convertToAssetsInternal(uint256 shares, Rounding rounding) internal view returns (uint256) { - uint64 baseSupplyIndex_ = accruedSupplyIndex(); - return shares > 0 ? presentValueSupply(baseSupplyIndex_, shares, rounding) : 0; - } - - function convertToSharesInternal(uint256 assets, Rounding rounding) internal view returns (uint256) { - uint64 baseSupplyIndex_ = accruedSupplyIndex(); - return assets > 0 ? principalValueSupply(baseSupplyIndex_, assets, rounding) : 0; + /** + * @dev The current timestamp + * From https://github.com/compound-finance/comet/blob/main/contracts/Comet.sol#L375-L378 + */ + function getNowInternal() internal view virtual returns (uint40) { + if (block.timestamp >= 2**40) revert TimestampTooLarge(); + return uint40(block.timestamp); } } diff --git a/test/CometWrapper.t.sol b/test/CometWrapper.t.sol index d4b546e..04e2ed8 100644 --- a/test/CometWrapper.t.sol +++ b/test/CometWrapper.t.sol @@ -48,7 +48,7 @@ abstract contract CometWrapperTest is CoreTest, CometMath { function test_constructor_revertsOnInvalidCometRewards() public { // reverts on zero address - vm.expectRevert(CometHelpers.ZeroAddress.selector); + vm.expectRevert(CometWrapper.ZeroAddress.selector); new CometWrapper(ERC20(address(comet)), ICometRewards(address(0)), "Name", "Symbol"); // reverts on non-zero address that isn't CometRewards @@ -533,7 +533,7 @@ abstract contract CometWrapperTest is CoreTest, CometMath { assertEq(cometWrapper.balanceOf(alice), 5_000e6 - sharesToWithdraw); // Reverts if trying to withdraw again now that allowance is used up - vm.expectRevert(CometHelpers.InsufficientAllowance.selector); + vm.expectRevert(CometWrapper.InsufficientAllowance.selector); cometWrapper.withdraw(assetsToWithdraw, bob, alice); vm.stopPrank(); assertEq(cometWrapper.allowance(alice, bob), sharesToApprove - sharesToWithdraw); @@ -558,7 +558,7 @@ abstract contract CometWrapperTest is CoreTest, CometMath { vm.stopPrank(); vm.prank(bob); - vm.expectRevert(CometHelpers.InsufficientAllowance.selector); + vm.expectRevert(CometWrapper.InsufficientAllowance.selector); cometWrapper.withdraw(900e6, bob, alice); } @@ -786,7 +786,7 @@ abstract contract CometWrapperTest is CoreTest, CometMath { assertApproxEqAbs(cometWrapper.balanceOf(alice), 5_000e6 - sharesToWithdraw, 1); // Reverts if trying to redeem again now that allowance is used up - vm.expectRevert(CometHelpers.InsufficientAllowance.selector); + vm.expectRevert(CometWrapper.InsufficientAllowance.selector); cometWrapper.redeem(sharesToWithdraw, bob, alice); vm.stopPrank(); assertEq(cometWrapper.allowance(alice, bob), sharesToApprove - sharesToWithdraw); @@ -811,20 +811,20 @@ abstract contract CometWrapperTest is CoreTest, CometMath { vm.stopPrank(); vm.prank(bob); - vm.expectRevert(CometHelpers.InsufficientAllowance.selector); + vm.expectRevert(CometWrapper.InsufficientAllowance.selector); cometWrapper.redeem(900e6, bob, alice); } function test_revertsOnZeroShares() public { vm.startPrank(alice); comet.allow(wrapperAddress, true); - vm.expectRevert(CometHelpers.ZeroShares.selector); + vm.expectRevert(CometWrapper.ZeroShares.selector); cometWrapper.mint(0, alice); - vm.expectRevert(CometHelpers.ZeroShares.selector); + vm.expectRevert(CometWrapper.ZeroShares.selector); cometWrapper.redeem(0, alice, alice); - vm.expectRevert(CometHelpers.ZeroShares.selector); + vm.expectRevert(CometWrapper.ZeroShares.selector); cometWrapper.withdraw(0, alice, alice); - vm.expectRevert(CometHelpers.ZeroShares.selector); + vm.expectRevert(CometWrapper.ZeroShares.selector); cometWrapper.deposit(0, alice); vm.stopPrank(); } @@ -892,7 +892,7 @@ abstract contract CometWrapperTest is CoreTest, CometMath { // Need approvals to transferFrom alice to bob vm.prank(bob); - vm.expectRevert(CometHelpers.InsufficientAllowance.selector); + vm.expectRevert(CometWrapper.InsufficientAllowance.selector); cometWrapper.transferFrom(alice, bob, 5_000e6); vm.prank(alice); @@ -906,7 +906,7 @@ abstract contract CometWrapperTest is CoreTest, CometMath { assertEq(cometWrapper.balanceOf(bob), 2_500e6); // Reverts if trying to transferFrom again now that allowance is used up - vm.expectRevert(CometHelpers.InsufficientAllowance.selector); + vm.expectRevert(CometWrapper.InsufficientAllowance.selector); cometWrapper.transferFrom(alice, bob, 2_500e6); vm.stopPrank(); assertEq(cometWrapper.allowance(alice, bob), 200e6); @@ -931,19 +931,19 @@ abstract contract CometWrapperTest is CoreTest, CometMath { vm.stopPrank(); vm.prank(bob); - vm.expectRevert(CometHelpers.InsufficientAllowance.selector); + vm.expectRevert(CometWrapper.InsufficientAllowance.selector); cometWrapper.transferFrom(alice, bob, 900e6); vm.prank(alice); cometWrapper.approve(bob, 500e6); vm.startPrank(bob); - vm.expectRevert(CometHelpers.InsufficientAllowance.selector); + vm.expectRevert(CometWrapper.InsufficientAllowance.selector); cometWrapper.transferFrom(alice, bob, 800e6); // larger than allowance cometWrapper.transferFrom(alice, bob, 400e6); // less than allowance - vm.expectRevert(CometHelpers.InsufficientAllowance.selector); + vm.expectRevert(CometWrapper.InsufficientAllowance.selector); cometWrapper.transferFrom(alice, bob, 200e6); // larger than remaining allowance assertEq(cometWrapper.balanceOf(bob), 400e6); diff --git a/test/Rewards.t.sol b/test/Rewards.t.sol index dbbfec0..a946439 100644 --- a/test/Rewards.t.sol +++ b/test/Rewards.t.sol @@ -4,7 +4,6 @@ pragma solidity 0.8.17; import { CoreTest } from "./CoreTest.sol"; import { CometWrapper, ICometRewards, CometHelpers, ERC20 } from "../src/CometWrapper.sol"; import { Deployable, ICometConfigurator, ICometProxyAdmin } from "../src/vendor/ICometConfigurator.sol"; -import "forge-std/console.sol"; abstract contract RewardsTest is CoreTest { function test_getRewardOwed(uint256 aliceAmount, uint256 bobAmount) public { @@ -83,7 +82,7 @@ abstract contract RewardsTest is CoreTest { CometWrapper newCometWrapper = new CometWrapper(ERC20(cometAddress), ICometRewards(newRewardsAddr), "Net Comet Wrapper", "NewWcUSDCv3"); - vm.expectRevert(CometHelpers.UninitializedReward.selector); + vm.expectRevert(CometWrapper.UninitializedReward.selector); newCometWrapper.getRewardOwed(alice); } @@ -169,7 +168,7 @@ abstract contract RewardsTest is CoreTest { new CometWrapper(ERC20(cometAddress), ICometRewards(newRewardsAddr), "Net Comet Wrapper", "NewWcUSDCv3"); vm.prank(alice); - vm.expectRevert(CometHelpers.UninitializedReward.selector); + vm.expectRevert(CometWrapper.UninitializedReward.selector); newCometWrapper.claimTo(alice); } @@ -296,5 +295,3 @@ abstract contract RewardsTest is CoreTest { } // TODO: test cWETHv3 -// TODO: test L2 reward contracts that use multipliers -// TODO: multiple reward contracts? \ No newline at end of file