From 916b05d54f782342f97db6d9fddefe84380227e9 Mon Sep 17 00:00:00 2001 From: Andrew Zhou Date: Mon, 9 Sep 2024 18:27:56 -0700 Subject: [PATCH] miscellaneous audit fixes --- src/RepoTokenList.sol | 2 +- src/RepoTokenUtils.sol | 34 ---------------- src/Strategy.sol | 52 +++++------------------- src/TermAuctionList.sol | 7 ++-- src/TermVaultEventEmitter.sol | 1 + src/interfaces/term/ITermVaultEvents.sol | 2 + src/periphery/StrategyAprOracle.sol | 2 +- 7 files changed, 19 insertions(+), 81 deletions(-) diff --git a/src/RepoTokenList.sol b/src/RepoTokenList.sol index 1767a323..4339d388 100644 --- a/src/RepoTokenList.sol +++ b/src/RepoTokenList.sol @@ -324,7 +324,7 @@ library RepoTokenList { (redemptionTimestamp, purchaseToken, , collateralManager) = repoToken.config(); // Validate purchase token - if (purchaseToken != address(asset)) { + if (purchaseToken != asset) { revert InvalidRepoToken(address(repoToken)); } diff --git a/src/RepoTokenUtils.sol b/src/RepoTokenUtils.sol index 1376f403..6ed920d4 100644 --- a/src/RepoTokenUtils.sol +++ b/src/RepoTokenUtils.sol @@ -12,40 +12,6 @@ library RepoTokenUtils { uint256 public constant THREESIXTY_DAYCOUNT_SECONDS = 360 days; uint256 public constant RATE_PRECISION = 1e18; - /*////////////////////////////////////////////////////////////// - PURE FUNCTIONS - //////////////////////////////////////////////////////////////*/ - - /** - * @notice Convert repoToken amount to purchase token precision - * @param repoTokenPrecision The precision of the repoToken - * @param purchaseTokenPrecision The precision of the purchase token - * @param purchaseTokenAmountInRepoPrecision The amount of purchase token in repoToken precision - * @return The amount in purchase token precision - */ - function repoToPurchasePrecision( - uint256 repoTokenPrecision, - uint256 purchaseTokenPrecision, - uint256 purchaseTokenAmountInRepoPrecision - ) internal pure returns (uint256) { - return (purchaseTokenAmountInRepoPrecision * purchaseTokenPrecision) / repoTokenPrecision; - } - - /** - * @notice Convert purchase token amount to repoToken precision - * @param repoTokenPrecision The precision of the repoToken - * @param purchaseTokenPrecision The precision of the purchase token - * @param repoTokenAmount The amount of repoToken - * @return The amount in repoToken precision - */ - function purchaseToRepoPrecision( - uint256 repoTokenPrecision, - uint256 purchaseTokenPrecision, - uint256 repoTokenAmount - ) internal pure returns (uint256) { - return (repoTokenAmount * repoTokenPrecision) / purchaseTokenPrecision; - } - /*////////////////////////////////////////////////////////////// VIEW FUNCTIONS //////////////////////////////////////////////////////////////*/ diff --git a/src/Strategy.sol b/src/Strategy.sol index 57e41094..8741c3d4 100644 --- a/src/Strategy.sol +++ b/src/Strategy.sol @@ -53,6 +53,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard { IERC4626 public immutable YEARN_VAULT; /// @notice State variables + bool public depositLock; /// @dev Previous term controller ITermController public prevTermController; /// @dev Current term controller @@ -62,10 +63,9 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard { TermAuctionListData internal termAuctionListData; uint256 public timeToMaturityThreshold; // seconds uint256 public requiredReserveRatio; // 1e18 - uint256 public discountRateMarkup; // 1e18 (TODO: check this) + uint256 public discountRateMarkup; // 1e18 uint256 public repoTokenConcentrationLimit; // 1e18 mapping(address => bool) public repoTokenBlacklist; - bool public depositLock; modifier notBlacklisted(address repoToken) { if (repoTokenBlacklist[repoToken]) { @@ -344,7 +344,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard { uint256 proceeds; if (repoToken != address(0)) { if (!_isTermDeployed(repoToken)) { - revert RepoTokenList.InvalidRepoToken(address(repoToken)); + revert RepoTokenList.InvalidRepoToken(repoToken); } uint256 redemptionTimestamp = repoTokenListData.validateRepoToken( @@ -743,7 +743,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard { revert InvalidTermAuction(address(termAuction)); } if (!_isTermDeployed(repoToken)) { - revert RepoTokenList.InvalidRepoToken(address(repoToken)); + revert RepoTokenList.InvalidRepoToken(repoToken); } require(termAuction.termRepoId() == ITermRepoToken(repoToken).termRepoId(), "repoToken does not match term repo ID"); @@ -976,7 +976,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard { } /** - * @notice Close the auction + * @notice Required for post-processing after auction clos */ function auctionClosed() external { _sweepAsset(); @@ -1001,7 +1001,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard { // Make sure repo token is valid and deployed by Term if (!_isTermDeployed(repoToken)) { - revert RepoTokenList.InvalidRepoToken(address(repoToken)); + revert RepoTokenList.InvalidRepoToken(repoToken); } // Validate and insert the repoToken into the list, retrieve auction rate and redemption timestamp @@ -1101,6 +1101,11 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard { discountRateAdapter = ITermDiscountRateAdapter(_discountRateAdapter); IERC20(_asset).safeApprove(_yearnVault, type(uint256).max); + + timeToMaturityThreshold = 45 days; + requiredReserveRatio = 0.2e18; + discountRateMarkup = 0.005e18; + repoTokenConcentrationLimit = 0.1e18; } /*////////////////////////////////////////////////////////////// @@ -1213,38 +1218,6 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard { return _totalLiquidBalance(); } - /** - * @notice Gets the max amount of `asset` that an address can deposit. - * @dev Defaults to an unlimited amount for any address. But can - * be overridden by strategists. - * - * This function will be called before any deposit or mints to enforce - * any limits desired by the strategist. This can be used for either a - * traditional deposit limit or for implementing a whitelist etc. - * - * EX: - * if(isAllowed[_owner]) return super.availableDepositLimit(_owner); - * - * This does not need to take into account any conversion rates - * from shares to assets. But should know that any non max uint256 - * amounts may be converted to shares. So it is recommended to keep - * custom amounts low enough as not to cause overflow when multiplied - * by `totalSupply`. - * - * @param . The address that is depositing into the strategy. - * @return . The available amount the `_owner` can deposit in terms of `asset` - * - function availableDepositLimit( - address _owner - ) public view override returns (uint256) { - TODO: If desired Implement deposit limit logic and any needed state variables . - - EX: - uint256 totalAssets = TokenizedStrategy.totalAssets(); - return totalAssets >= depositLimit ? 0 : depositLimit - totalAssets; - } - */ - /** * @dev Optional function for strategist to override that can * be called in between reports. @@ -1300,12 +1273,9 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard { * @param _amount The amount of asset to attempt to free. * function _emergencyWithdraw(uint256 _amount) internal override { - TODO: If desired implement simple logic to free deployed funds. - EX: _amount = min(_amount, aToken.balanceOf(address(this))); _freeFunds(_amount); } - */ } diff --git a/src/TermAuctionList.sol b/src/TermAuctionList.sol index 672b3b75..17564a41 100644 --- a/src/TermAuctionList.sol +++ b/src/TermAuctionList.sol @@ -210,7 +210,7 @@ library TermAuctionList { removeNode = true; bytes32[] memory offerIds = new bytes32[](1); offerIds[0] = current; - offer.offerLocker.unlockOffers(offerIds); // unlocking offer in this scenario withdraws offer ammount + offer.offerLocker.unlockOffers(offerIds); // unlocking offer in this scenario withdraws offer amount } } @@ -226,7 +226,6 @@ library TermAuctionList { } if (insertRepoToken) { - // TODO: do we need to validate termDeployed(repoToken) here? // Auction still open => include offerAmount in totalValue // (otherwise locked purchaseToken will be missing from TV) @@ -286,7 +285,7 @@ library TermAuctionList { // Handle new or unseen repo tokens /// @dev offer processed, but auctionClosed not yet called and auction is new so repoToken not on List and wont be picked up /// checking repoTokendiscountRates to make sure we are not double counting on re-openings - if (offer.termAuction.auctionCompleted() && repoTokenListData.discountRates[offer.repoToken] == 0) { + if (repoTokenListData.discountRates[offer.repoToken] == 0 && offer.termAuction.auctionCompleted()) { if (!offer.isRepoTokenSeen) { uint256 repoTokenAmountInBaseAssetPrecision = RepoTokenUtils.getNormalizedRepoTokenAmount( offer.repoToken, @@ -355,7 +354,7 @@ library TermAuctionList { // Handle new repo tokens or reopening auctions /// @dev offer processed, but auctionClosed not yet called and auction is new so repoToken not on List and wont be picked up /// checking repoTokendiscountRates to make sure we are not double counting on re-openings - if (offer.termAuction.auctionCompleted() && repoTokenListData.discountRates[offer.repoToken] == 0) { + if (repoTokenListData.discountRates[offer.repoToken] == 0 && offer.termAuction.auctionCompleted()) { // use normalized repoToken amount if repoToken is not in the list if (!offer.isRepoTokenSeen) { offerAmount = RepoTokenUtils.getNormalizedRepoTokenAmount( diff --git a/src/TermVaultEventEmitter.sol b/src/TermVaultEventEmitter.sol index 84240332..d5716c36 100644 --- a/src/TermVaultEventEmitter.sol +++ b/src/TermVaultEventEmitter.sol @@ -33,6 +33,7 @@ contract TermVaultEventEmitter is Initializable, UUPSUpgradeable, AccessControlU function pairVaultContract(address vaultContract) external onlyRole(ADMIN_ROLE){ _grantRole(VAULT_CONTRACT, vaultContract); + emit VaultContractPaired(vaultContract); } function emitTermControllerUpdated(address oldController, address newController) external onlyRole(VAULT_CONTRACT) { diff --git a/src/interfaces/term/ITermVaultEvents.sol b/src/interfaces/term/ITermVaultEvents.sol index 5ebb1076..c7ec9e79 100644 --- a/src/interfaces/term/ITermVaultEvents.sol +++ b/src/interfaces/term/ITermVaultEvents.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.18; interface ITermVaultEvents { + event VaultContractPaired(address vault); + event TermControllerUpdated(address oldController, address newController); event TimeToMaturityThresholdUpdated(uint256 oldThreshold, uint256 newThreshold); diff --git a/src/periphery/StrategyAprOracle.sol b/src/periphery/StrategyAprOracle.sol index 1c0bfb8f..96d0f8ab 100644 --- a/src/periphery/StrategyAprOracle.sol +++ b/src/periphery/StrategyAprOracle.sol @@ -28,7 +28,7 @@ contract StrategyAprOracle is AprOracleBase { function aprAfterDebtChange( address _strategy, int256 _delta - ) external view override returns (uint256) { + ) external pure override returns (uint256) { // TODO: Implement any necessary logic to return the most accurate // APR estimation for the strategy. return 1e17;