diff --git a/contracts/BaseStrategy.sol b/contracts/BaseStrategy.sol index 0505aeb1..b6ecdabb 100644 --- a/contracts/BaseStrategy.sol +++ b/contracts/BaseStrategy.sol @@ -23,32 +23,32 @@ interface VaultAPI is IERC20 { function strategies(address _strategy) external view returns (StrategyParams memory); - /* - * View how much the Vault would increase this strategy's borrow limit, - * based on it's present performance (since its last report). Can be used to - * determine expectedReturn in your strategy. + /** + * View how much the Vault would increase this Strategy's borrow limit, + * based on its present performance (since its last report). Can be used to + * determine expectedReturn in your Strategy. */ function creditAvailable() external view returns (uint256); - /* + /** * View how much the Vault would like to pull back from the Strategy, - * based on it's present performance (since its last report). Can be used to - * determine expectedReturn in your strategy. + * based on its present performance (since its last report). Can be used to + * determine expectedReturn in your Strategy. */ function debtOutstanding() external view returns (uint256); - /* - * View how much the Vault expect this strategy to return at the current block, - * based on it's present performance (since its last report). Can be used to - * determine expectedReturn in your strategy. + /** + * View how much the Vault expect this Strategy to return at the current + * block, based on its present performance (since its last report). Can be + * used to determine expectedReturn in your Strategy. */ function expectedReturn() external view returns (uint256); - /* - * This is the main contact point where the strategy interacts with the Vault. - * It is critical that this call is handled as intended by the Strategy. - * Therefore, this function will be called by BaseStrategy to make sure the - * integration is correct. + /** + * This is the main contact point where the Strategy interacts with the + * Vault. It is critical that this call is handled as intended by the + * Strategy. Therefore, this function will be called by BaseStrategy to + * make sure the integration is correct. */ function report( uint256 _gain, @@ -56,39 +56,38 @@ interface VaultAPI is IERC20 { uint256 _debtPayment ) external returns (uint256); - /* - * This function is used in the scenario where there is a newer strategy that - * would hold the same positions as this one, and those positions are easily - * transferrable to the newer strategy. These positions must be able to be - * transferred at the moment this call is made, if any prep is required to - * execute a full transfer in one transaction, that must be accounted for - * separately from this call. + /** + * This function is used in the scenario where there is a newer Strategy + * that would hold the same positions as this one, and those positions are + * easily transferrable to the newer Strategy. These positions must be able + * to be transferred at the moment this call is made, if any prep is + * required to execute a full transfer in one transaction, that must be + * accounted for separately from this call. */ function migrateStrategy(address _newStrategy) external; - /* - * This function should only be used in the scenario where the strategy is + /** + * This function should only be used in the scenario where the Strategy is * being retired but no migration of the positions are possible, or in the * extreme scenario that the Strategy needs to be put into "Emergency Exit" * mode in order for it to exit as quickly as possible. The latter scenario * could be for any reason that is considered "critical" that the Strategy - * exits it's position as fast as possible, such as a sudden change in market - * conditions leading to losses, or an imminent failure in an external - * dependency. + * exits its position as fast as possible, such as a sudden change in + * market conditions leading to losses, or an imminent failure in an + * external dependency. */ function revokeStrategy() external; - /* + /** * View the governance address of the Vault to assert privileged functions * can only be called by governance. The Strategy serves the Vault, so it * is subject to governance defined by the Vault. - * */ function governance() external view returns (address); } -/* - * This interface is here for the keeper bot to use +/** + * This interface is here for the keeper bot to use. */ interface StrategyAPI { function apiVersion() external pure returns (string memory); @@ -110,23 +109,44 @@ interface StrategyAPI { event Harvested(uint256 profit, uint256 loss, uint256 debtPayment, uint256 debtOutstanding); } -/* - * BaseStrategy implements all of the required functionality to interoperate closely - * with the core protocol. This contract should be inherited and the abstract methods - * implemented to adapt the strategy to the particular needs it has to create a return. +/** + * @title Yearn Base Strategy + * @author yearn.finance + * @notice + * BaseStrategy implements all of the required functionality to interoperate + * closely with the Vault contract. This contract should be inherited and the + * abstract methods implemented to adapt the Strategy to the particular needs + * it has to create a return. + * + * Of special interest is the relationship between `harvest()` and + * `vault.report()'. `harvest()` may be called simply because enough time has + * elapsed since the last report, and not because any funds need to be moved + * or positions adjusted. This is critical so that the Vault may maintain an + * accurate picture of the Strategy's performance. See `vault.report()`, + * `harvest()`, and `harvestTrigger()` for further details. */ - abstract contract BaseStrategy { using SafeMath for uint256; - // Version of this contract's StrategyAPI (must match Vault) + /** + * @notice + * Used to track which version of `StrategyAPI` this Strategy + * implements. + * @dev The Strategy's version must match the Vault's `API_VERSION`. + * @return A string which holds the current API version of this contract. + */ function apiVersion() public pure returns (string memory) { return "0.2.1"; } - // Name of this contract's Strategy (Must override!) - // NOTE: You can use this field to manage the "version" of this strategy - // e.g. `StrategySomethingOrOtherV1`. It's up to you! + /** + * @notice This Strategy's name. + * @dev + * You can use this field to manage the "version" of this Strategy, e.g. + * `StrategySomethingOrOtherV1`. However, "API Version" is managed by + * `apiVersion()` function above. + * @return This Strategy's name. + */ function name() external virtual pure returns (string memory); VaultAPI public vault; @@ -151,17 +171,19 @@ abstract contract BaseStrategy { event UpdatedDebtThreshold(uint256 debtThreshold); - // The minimum number of seconds between harvest calls - // NOTE: Override this value with your own, or set dynamically below + // The minimum number of seconds between harvest calls. See + // `setMinReportDelay()` for more details. uint256 public minReportDelay = 86400; // ~ once a day - // The minimum multiple that `callCost` must be above the credit/profit to be "justifiable" - // NOTE: Override this value with your own, or set dynamically below + // The minimum multiple that `callCost` must be above the credit/profit to + // be "justifiable". See `setProfitFactor()` for more details. uint256 public profitFactor = 100; - // Use this to adjust the threshold at which running a debt causes a harvest trigger + // Use this to adjust the threshold at which running a debt causes a + // harvest trigger. See `setDebtThreshold()` for more details. uint256 public debtThreshold = 0; + // See note on `setEmergencyExit()`. bool public emergencyExit; // modifiers @@ -185,6 +207,13 @@ abstract contract BaseStrategy { _; } + /** + * @notice + * Initializes the Strategy, this is called only once, when the + * contract is deployed. + * @dev `_vault` should implement `VaultAPI`. + * @param _vault The address of the Vault responsible for this Strategy. + */ constructor(address _vault) public { vault = VaultAPI(_vault); want = IERC20(vault.token()); @@ -194,77 +223,160 @@ abstract contract BaseStrategy { keeper = msg.sender; } + /** + * @notice + * Used to change `strategist`. + * + * This may only be called by governance or the existing strategist. + * @param _strategist The new address to assign as `strategist`. + */ function setStrategist(address _strategist) external onlyAuthorized { strategist = _strategist; emit UpdatedStrategist(_strategist); } + /** + * @notice + * Used to change `keeper`. + * + * `keeper` is the only address that may call `tend()` or `harvest()`, + * other than `governance()` or `strategist`. However, unlike + * `governance()` or `strategist`, `keeper` may *only* call `tend()` + * and `harvest()`, and no other authorized functions, following the + * principle of least privilege. + * + * This may only be called by governance or the strategist. + * @param _keeper The new address to assign as `keeper`. + */ function setKeeper(address _keeper) external onlyAuthorized { keeper = _keeper; emit UpdatedKeeper(_keeper); } + /** + * @notice + * Used to change `rewards`. Any distributed rewards will cease flowing + * to the old address and begin flowing to this address once the change + * is in effect. + * + * This will not change any Strategy reports in progress, only + * new reports made after this change goes into effect. + * + * This may only be called by the strategist. + * @param _rewards The address to use for collecting rewards. + */ function setRewards(address _rewards) external onlyStrategist { rewards = _rewards; emit UpdatedRewards(_rewards); } + /** + * @notice + * Used to change `minReportDelay`. `minReportDelay` is the minimum number + * of blocks that should pass before `harvest()` is called. + * + * For external keepers (such as the Keep3r network), this is the minimum + * time between jobs, to prevent excessive costs. (see `harvestTrigger()` + * for more details.) + * + * This may only be called by governance or the strategist. + * @param _delay The minimum number of blocks to wait between harvests. + */ function setMinReportDelay(uint256 _delay) external onlyAuthorized { minReportDelay = _delay; emit UpdatedReportDelay(_delay); } + /** + * @notice + * Used to change `profitFactor`. `profitFactor` is used to determine + * if it's worthwhile to harvest, given gas costs. (See `harvestTrigger()` + * for more details.) + * + * This may only be called by governance or the strategist. + * @param _profitFactor A ratio to multiply anticipated + * `harvest()` gas cost against. + */ function setProfitFactor(uint256 _profitFactor) external onlyAuthorized { profitFactor = _profitFactor; emit UpdatedProfitFactor(_profitFactor); } + /** + * @notice + * Sets how far the Strategy can go into loss without a harvest and report + * being required. + * + * By default this is 0, meaning any losses would cause a harvest which + * will subsequently report the loss to the Vault for tracking. (See + * `harvestTrigger()` for more details.) + * + * This may only be called by governance or the strategist. + * @param _debtThreshold How big of a loss this Strategy may carry without + * being required to report to the Vault. + */ function setDebtThreshold(uint256 _debtThreshold) external onlyAuthorized { debtThreshold = _debtThreshold; emit UpdatedDebtThreshold(_debtThreshold); } - /* - * Resolve governance address from Vault contract, used to make - * assertions on protected functions in the Strategy + /** + * Resolve governance address from Vault contract, used to make assertions + * on protected functions in the Strategy. */ function governance() internal view returns (address) { return vault.governance(); } - /* - * Provide an accurate estimate for the total amount of assets (principle + return) - * that this strategy is currently managing, denominated in terms of `want` tokens. - * This total should be "realizable" e.g. the total value that could *actually* be - * obtained from this strategy if it were to divest it's entire position based on - * current on-chain conditions. + /** + * @notice + * Provide an accurate estimate for the total amount of assets + * (principle + return) that this Strategy is currently managing, + * denominated in terms of `want` tokens. * - * NOTE: care must be taken in using this function, since it relies on external - * systems, which could be manipulated by the attacker to give an inflated - * (or reduced) value produced by this function, based on current on-chain - * conditions (e.g. this function is possible to influence through flashloan - * attacks, oracle manipulations, or other DeFi attack mechanisms). + * This total should be "realizable" e.g. the total value that could + * *actually* be obtained from this Strategy if it were to divest its + * entire position based on current on-chain conditions. + * @dev + * Care must be taken in using this function, since it relies on external + * systems, which could be manipulated by the attacker to give an inflated + * (or reduced) value produced by this function, based on current on-chain + * conditions (e.g. this function is possible to influence through + * flashloan attacks, oracle manipulations, or other DeFi attack + * mechanisms). * - * NOTE: It is up to governance to use this function to correctly order this strategy - * relative to its peers in the withdrawal queue to minimize losses for the Vault - * based on sudden withdrawals. This value should be higher than the total debt of - * the strategy and higher than it's expected value to be "safe". + * It is up to governance to use this function to correctly order this + * Strategy relative to its peers in the withdrawal queue to minimize + * losses for the Vault based on sudden withdrawals. This value should be + * higher than the total debt of the Strategy and higher than its expected + * value to be "safe". + * @return The estimated total assets in this Strategy. */ function estimatedTotalAssets() public virtual view returns (uint256); - /* - * Perform any strategy unwinding or other calls necessary to capture the "free return" - * this strategy has generated since the last time it's core position(s) were adjusted. - * Examples include unwrapping extra rewards. This call is only used during "normal operation" - * of a Strategy, and should be optimized to minimize losses as much as possible. This method - * returns any realized profits and/or realized losses incurred, and should return the total - * amounts of profits/losses/debt payments (in `want` tokens) for the Vault's accounting - * (e.g. `want.balanceOf(this) >= _debtPayment + _profit - _loss`). - * - * NOTE: `_debtPayment` should be less than or equal to `_debtOutstanding`. It is okay for it - * to be less than `_debtOutstanding`, as that should only used as a guide for how much - * is left to pay back. Payments should be made to minimize loss from slippage, debt, + /** + * Perform any Strategy unwinding or other calls necessary to capture the + * "free return" this Strategy has generated since the last time its core + * position(s) were adjusted. Examples include unwrapping extra rewards. + * This call is only used during "normal operation" of a Strategy, and + * should be optimized to minimize losses as much as possible. + * + * This method returns any realized profits and/or realized losses + * incurred, and should return the total amounts of profits/losses/debt + * payments (in `want` tokens) for the Vault's accounting (e.g. + * `want.balanceOf(this) >= _debtPayment + _profit - _loss`). + * + * `_debtOutstanding` will be 0 if the Strategy is not past the configured + * debt limit, otherwise its value will be how far past the debt limit + * the Strategy is. The Strategy's debt limit is configured in the Vault. + * + * NOTE: `_debtPayment` should be less than or equal to `_debtOutstanding`. + * It is okay for it to be less than `_debtOutstanding`, as that + * should only used as a guide for how much is left to pay back. + * Payments should be made to minimize loss from slippage, debt, * withdrawal fees, etc. + * + * See `vault.debtOutstanding()`. */ function prepareReturn(uint256 _debtOutstanding) internal @@ -275,23 +387,26 @@ abstract contract BaseStrategy { uint256 _debtPayment ); - /* - * Perform any adjustments to the core position(s) of this strategy given + /** + * Perform any adjustments to the core position(s) of this Strategy given * what change the Vault made in the "investable capital" available to the - * strategy. Note that all "free capital" in the strategy after the report - * was made is available for reinvestment. Also note that this number could - * be 0, and you should handle that scenario accordingly. + * Strategy. Note that all "free capital" in the Strategy after the report + * was made is available for reinvestment. Also note that this number + * could be 0, and you should handle that scenario accordingly. + * + * See comments regarding `_debtOutstanding` on `prepareReturn()`. */ function adjustPosition(uint256 _debtOutstanding) internal virtual; - /* - * Make as much capital as possible "free" for the Vault to take. Some slippage - * is allowed, since when this method is called the strategist is no longer receiving - * their performance fee. The goal is for the strategy to divest as quickly as possible - * while not suffering exorbitant losses. This function is used during emergency exit - * instead of `prepareReturn()`. This method returns any realized losses incurred, and - * should also return the amount of `want` tokens available to repay outstanding debt - * to the Vault. + /** + * Make as much capital as possible "free" for the Vault to take. Some + * slippage is allowed, since when this method is called the strategist is + * no longer receiving their performance fee. The goal is for the Strategy + * to divest as quickly as possible while not suffering exorbitant losses. + * This function is used during emergency exit instead of + * `prepareReturn()`. This method returns any realized losses incurred, + * and should also return the amount of `want` tokens available to repay + * outstanding debt to the Vault. */ function exitPosition(uint256 _debtOutstanding) internal @@ -302,9 +417,12 @@ abstract contract BaseStrategy { uint256 _debtPayment ); - /* - * Vault calls this function after shares are created during `Vault.report()`. - * You can customize this function to any share distribution mechanism you want. + /** + * `Harvest()` calls this function after shares are created during + * `vault.report()`. You can customize this function to any share + * distribution mechanism you want. + * + * See `vault.report()` for further details. */ function distributeRewards() internal virtual { // Transfer 100% of newly-minted shares awarded to this contract to the rewards address. @@ -314,53 +432,88 @@ abstract contract BaseStrategy { } } - /* - * Provide a signal to the keeper that `tend()` should be called. The keeper will provide - * the estimated gas cost that they would pay to call `tend()`, and this function should - * use that estimate to make a determination if calling it is "worth it" for the keeper. - * This is not the only consideration into issuing this trigger, for example if the position - * would be negatively affected if `tend()` is not called shortly, then this can return `true` - * even if the keeper might be "at a loss" (keepers are always reimbursed by Yearn) - * - * NOTE: `callCost` must be priced in terms of `want` + /** + * @notice + * Provide a signal to the keeper that `tend()` should be called. The + * keeper will provide the estimated gas cost that they would pay to call + * `tend()`, and this function should use that estimate to make a + * determination if calling it is "worth it" for the keeper. This is not + * the only consideration into issuing this trigger, for example if the + * position would be negatively affected if `tend()` is not called + * shortly, then this can return `true` even if the keeper might be + * "at a loss" (keepers are always reimbursed by Yearn). + * @dev + * `callCost` must be priced in terms of `want`. * - * NOTE: this call and `harvestTrigger` should never return `true` at the same time. + * This call and `harvestTrigger()` should never return `true` at the same + * time. + * @param callCost The keeper's estimated cast cost to call `tend()`. + * @return `true` if `tend()` should be called, `false` otherwise. */ function tendTrigger(uint256 callCost) public virtual view returns (bool) { - // We usually don't need tend, but if there are positions that need active maintainence, - // overriding this function is how you would signal for that + // We usually don't need tend, but if there are positions that need + // active maintainence, overriding this function is how you would + // signal for that. return false; } + /** + * @notice + * Adjust the Strategy's position. The purpose of tending isn't to + * realize gains, but to maximize yield by reinvesting any returns. + * + * See comments on `adjustPosition()`. + * + * This may only be called by governance, the strategist, or the keeper. + */ function tend() external onlyKeepers { // Don't take profits with this call, but adjust for better gains adjustPosition(vault.debtOutstanding()); } - /* - * Provide a signal to the keeper that `harvest()` should be called. The keeper will provide - * the estimated gas cost that they would pay to call `harvest()`, and this function should - * use that estimate to make a determination if calling it is "worth it" for the keeper. - * This is not the only consideration into issuing this trigger, for example if the position - * would be negatively affected if `harvest()` is not called shortly, then this can return `true` - * even if the keeper might be "at a loss" (keepers are always reimbursed by Yearn) + /** + * @notice + * Provide a signal to the keeper that `harvest()` should be called. The + * keeper will provide the estimated gas cost that they would pay to call + * `harvest()`, and this function should use that estimate to make a + * determination if calling it is "worth it" for the keeper. This is not + * the only consideration into issuing this trigger, for example if the + * position would be negatively affected if `harvest()` is not called + * shortly, then this can return `true` even if the keeper might be "at a + * loss" (keepers are always reimbursed by Yearn). + * @dev + * `callCost` must be priced in terms of `want`. * - * NOTE: `callCost` must be priced in terms of `want` + * This call and `tendTrigger` should never return `true` at the + * same time. * - * NOTE: this call and `tendTrigger` should never return `true` at the same time. + * See `minReportDelay`, `profitFactor`, `debtThreshold` to adjust the + * strategist-controlled parameters that will influence whether this call + * returns `true` or not. These parameters will be used in conjunction + * with the parameters reported to the Vault (see `params`) to determine + * if calling `harvest()` is merited. + * + * It is expected that an external system will check `harvestTrigger()`. + * This could be a script run off a desktop or cloud bot (e.g. + * https://github.com/iearn-finance/yearn-vaults/blob/master/scripts/keep.py), + * or via an integration with the Keep3r network (e.g. + * https://github.com/Macarse/GenericKeep3rV2/blob/master/contracts/keep3r/GenericKeep3rV2.sol). + * @param callCost The keeper's estimated cast cost to call `harvest()`. + * @return `true` if `harvest()` should be called, `false` otherwise. */ function harvestTrigger(uint256 callCost) public virtual view returns (bool) { StrategyParams memory params = vault.strategies(address(this)); - // Should not trigger if strategy is not activated + // Should not trigger if Strategy is not activated if (params.activation == 0) return false; - // Should trigger if hadn't been called in a while + // Should trigger if hasn't been called in a while if (block.timestamp.sub(params.lastReport) >= minReportDelay) return true; // If some amount is owed, pay it back - // NOTE: Since debt is adjusted in step-wise fashion, it is appropiate to always trigger here, - // because the resulting change should be large (might not always be the case) + // NOTE: Since debt is adjusted in step-wise fashion, it is appropriate + // to always trigger here, because the resulting change should be + // large (might not always be the case). uint256 outstanding = vault.debtOutstanding(); if (outstanding > 0) return true; @@ -372,11 +525,29 @@ abstract contract BaseStrategy { uint256 profit = 0; if (total > params.totalDebt) profit = total.sub(params.totalDebt); // We've earned a profit! - // Otherwise, only trigger if it "makes sense" economically (gas cost is String[28]: Used to track the deployed version of this contract. In practice you can use this version number to compare with Yearn's GitHub and determine which version of the source matches this deployed contract. + @dev + All strategies must have an `apiVersion()` that matches the Vault's + `API_VERSION`. @return API_VERSION which holds the current version of this contract. """ return API_VERSION @@ -598,7 +601,7 @@ def _issueSharesForAmount(_to: address, _amount: uint256) -> uint256: # Issues `_amount` Vault shares to `_to`. # Shares must be issued prior to taking on new collateral, or # calculation will be wrong. This means that only *trusted* tokens - # (with no capability for exploitive behavior) can be used. + # (with no capability for exploitative behavior) can be used. shares: uint256 = 0 # HACK: Saves 2 SLOADs (~4000 gas) totalSupply: uint256 = self.totalSupply @@ -712,7 +715,11 @@ def maxAvailableShares() -> uint256: Determines the total quantity of shares this Vault can provide, factoring in assets currently residing in the Vault, as well as those deployed to strategies. - @dev Regarding how shares are calculated, see dev note on `deposit`. + @dev + Regarding how shares are calculated, see dev note on `deposit`. + + If you want to calculated the maximum a user could withdraw up to, + you want to use this function. @return The total quantity of shares this Vault can provide. """ shares: uint256 = self._sharesForAmount(self.token.balanceOf(self)) @@ -1247,6 +1254,12 @@ def report(_gain: uint256, _loss: uint256, _debtPayment: uint256) -> uint256: Reports the amount of assets the calling Strategy has free (usually in terms of ROI). + The performance fee is determined here, off of the strategy's profits + (if any), and sent to governance. + + The strategist's fee is also determined here (off of profits), to be + handled according to the strategist on the next harvest. + This may only be called by a Strategy managed by this Vault. @dev For approved strategies, this is the most efficient behavior.