From ea7ccb5e505a0027195f5b790117f2ed5ba3b6bd Mon Sep 17 00:00:00 2001 From: Schlag <89420541+Schlagonia@users.noreply.github.com> Date: Fri, 19 Jan 2024 14:57:54 -0700 Subject: [PATCH] feat: just track total assets (#73) * feat: just track total assets * fix: comments --- src/BaseStrategy.sol | 13 +- src/TokenizedStrategy.sol | 168 +++++------------------- src/interfaces/ITokenizedStrategy.sol | 4 - src/test/Accounting.t.sol | 129 ++++++++++-------- src/test/CustomImplementation.t.sol | 5 +- src/test/FaultyStrategy.t.sol | 70 +++------- src/test/InvariantsSingleStrategy.t.sol | 4 - src/test/mocks/MockFaultyStrategy.sol | 2 +- src/test/mocks/MockIlliquidStrategy.sol | 2 +- src/test/utils/BaseInvariant.sol | 9 +- src/test/utils/Setup.sol | 26 +++- 11 files changed, 155 insertions(+), 277 deletions(-) diff --git a/src/BaseStrategy.sol b/src/BaseStrategy.sol index 88a1acf..14e2e0a 100644 --- a/src/BaseStrategy.sol +++ b/src/BaseStrategy.sol @@ -172,7 +172,7 @@ abstract contract BaseStrategy { * be entirely permissionless and thus can be sandwiched or otherwise * manipulated. * - * @param _amount The amount of 'asset' that the strategy should attempt + * @param _amount The amount of 'asset' that the strategy can attempt * to deposit in the yield source. */ function _deployFunds(uint256 _amount) internal virtual; @@ -248,9 +248,7 @@ abstract contract BaseStrategy { * sandwiched can use the tend when a certain threshold * of idle to totalAssets has been reached. * - * The TokenizedStrategy contract will do all needed debt and idle updates - * after this has finished and will have no effect on PPS of the strategy - * till report() is called. + * This will have no effect on PPS of the strategy till report() is called. * * @param _totalIdle The current amount of idle funds that are available to deploy. */ @@ -370,7 +368,7 @@ abstract contract BaseStrategy { * Unless a whitelist is implemented this will be entirely permissionless * and thus can be sandwiched or otherwise manipulated. * - * @param _amount The amount of 'asset' that the strategy should + * @param _amount The amount of 'asset' that the strategy can * attempt to deposit in the yield source. */ function deployFunds(uint256 _amount) external virtual onlySelf { @@ -415,7 +413,7 @@ abstract contract BaseStrategy { * so msg.sender == address(this). * * We name the function `tendThis` so that `tend` calls are forwarded to - * the TokenizedStrategy so it can do the necessary accounting. + * the TokenizedStrategy. * @param _totalIdle The amount of current idle funds that can be * deployed during the tend @@ -432,8 +430,7 @@ abstract contract BaseStrategy { * the TokenizedStrategy so msg.sender == address(this). * * We name the function `shutdownWithdraw` so that `emergencyWithdraw` - * calls are forwarded to the TokenizedStrategy so it can do the necessary - * accounting after the withdraw. + * calls are forwarded to the TokenizedStrategy. * * @param _amount The amount of asset to attempt to free. */ diff --git a/src/TokenizedStrategy.sol b/src/TokenizedStrategy.sol index e5affd3..9b9121b 100644 --- a/src/TokenizedStrategy.sol +++ b/src/TokenizedStrategy.sol @@ -222,11 +222,9 @@ contract TokenizedStrategy { mapping(address => mapping(address => uint256)) allowances; // Mapping to track the allowances for the strategies shares. - // Assets data to track totals the strategy holds. - // We manually track idle instead of relying on asset.balanceOf(address(this)) - // to prevent PPS manipulation through airdrops. - uint256 totalIdle; // The total amount of loose `asset` the strategy holds. - uint256 totalDebt; // The total amount `asset` that is currently deployed by the strategy. + // Assets data to track total the strategy holds. + // We manually track `totalAssets` to prevent PPS manipulation through airdrops. + uint256 totalAssets; // Variables for profit reporting and locking. @@ -801,16 +799,12 @@ contract TokenizedStrategy { * @notice Get the total amount of assets this strategy holds * as of the last report. * - * We manually track debt and idle to avoid any PPS manipulation - * from donations, touch values of debt etc. + * We manually track `totalAssets` to avoid any PPS manipulation. * * @return . Total assets the strategy holds. */ function totalAssets() public view returns (uint256) { - StrategyData storage S = _strategyStorage(); - unchecked { - return S.totalIdle + S.totalDebt; - } + return _strategyStorage().totalAssets; } /** @@ -852,28 +846,13 @@ contract TokenizedStrategy { // Need to transfer before minting or ERC777s could reenter. _asset.safeTransferFrom(msg.sender, address(this), assets); - // We will deposit up to current idle plus the new amount added - uint256 toDeploy = S.totalIdle + assets; - - // Cache for post {deployFunds} checks. - uint256 beforeBalance = _asset.balanceOf(address(this)); - - // Deploy up to all loose funds. - IBaseStrategy(address(this)).deployFunds(toDeploy); - - // Always get the actual amount deployed. We double check the - // diff against toDeploy for complete accuracy. - uint256 deployed = Math.min( - beforeBalance - _asset.balanceOf(address(this)), - toDeploy + // We can deploy the full loose balance currently held. + IBaseStrategy(address(this)).deployFunds( + _asset.balanceOf(address(this)) ); // Adjust total Assets. - S.totalDebt += deployed; - unchecked { - // Cant't underflow due to previous min check. - S.totalIdle = toDeploy - deployed; - } + S.totalAssets += assets; // mint shares _mint(receiver, shares); @@ -909,29 +888,18 @@ contract TokenizedStrategy { // Expected behavior is to need to free funds so we cache `_asset`. ERC20 _asset = S.asset; - uint256 idle = S.totalIdle; - + uint256 idle = _asset.balanceOf(address(this)); + uint256 loss; // Check if we need to withdraw funds. if (idle < assets) { - // Cache before balance for diff checks. - uint256 before = _asset.balanceOf(address(this)); - // Tell Strategy to free what we need. unchecked { IBaseStrategy(address(this)).freeFunds(assets - idle); } - // Return the actual amount withdrawn. Adjust for potential over withdraws. - uint256 withdrawn = Math.min( - _asset.balanceOf(address(this)) - before, - S.totalDebt - ); + // Return the actual amount withdrawn. Adjust for potential under withdraws. + idle = _asset.balanceOf(address(this)); - unchecked { - idle += withdrawn; - } - - uint256 loss; // If we didn't get enough out then we have a loss. if (idle < assets) { unchecked { @@ -948,16 +916,15 @@ contract TokenizedStrategy { // Lower the amount to be withdrawn. assets = idle; } - - // Update debt storage. - S.totalDebt -= (withdrawn + loss); } - // Update idle based on how much we took. - S.totalIdle = idle - assets; + // Update assets based on how much we took. + S.totalAssets -= (assets + loss); + // Burn the owners shares. _burn(owner, shares); + // Transfer the amount of underlying to the receiver. _asset.safeTransfer(receiver, assets); emit Withdraw(msg.sender, receiver, owner, assets, shares); @@ -1007,18 +974,13 @@ contract TokenizedStrategy { // Cache storage pointer since its used repeatedly. StrategyData storage S = _strategyStorage(); - uint256 oldTotalAssets; - unchecked { - // Manually calculate totalAssets to save a SLOAD. - oldTotalAssets = S.totalIdle + S.totalDebt; - } + uint256 oldTotalAssets = S.totalAssets; // Tell the strategy to report the real total assets it has. // It should do all reward selling and redepositing now and // account for deployed and loose `asset` so we can accurately // account for all funds including those potentially airdropped - // by a trade factory. It is safe here to use asset.balanceOf() - // instead of totalIdle because any profits are immediately locked. + // and then have any profits immediately locked. uint256 newTotalAssets = IBaseStrategy(address(this)) .harvestAndReport(); @@ -1144,13 +1106,8 @@ contract TokenizedStrategy { S.profitUnlockingRate = 0; } - // Update storage we use the actual loose here since it should have - // been accounted for in `harvestAndReport` and any airdropped amounts - // would have been locked to prevent PPS manipulation. - uint256 newIdle = S.asset.balanceOf(address(this)); - S.totalIdle = newIdle; - S.totalDebt = newTotalAssets - newIdle; - + // Update the new total assets value. + S.totalAssets = newTotalAssets; S.lastReport = uint128(block.timestamp); // Emit event with info @@ -1230,62 +1187,20 @@ contract TokenizedStrategy { * This will callback the internal '_tend' call in the BaseStrategy * with the total current amount available to the strategy to deploy. * - * Keepers are expected to use protected relays in tend calls so this - * can be used for illiquid or manipulatable strategies to compound + * This is a permissioned function so if desired it could + * be used for illiquid or manipulatable strategies to compound * rewards, perform maintenance or deposit/withdraw funds. * - * All accounting for totalDebt and totalIdle updates will be done - * here post '_tend'. + * This will not cause any change in PPS. Total assets will + * be the same before and after. * - * This should never cause an increase in PPS. Total assets should - * be the same before and after - * - * A report() call will be needed to record the profit. + * A report() call will be needed to record any profits or losses. */ function tend() external nonReentrant onlyKeepers { - // Tend the strategy with the current totalIdle. - IBaseStrategy(address(this)).tendThis(_strategyStorage().totalIdle); - - // Update balances based on ending state. - _updateBalances(); - } - - /** - * @notice Update the internal balances that make up `totalAssets`. - * @dev This will update the ratio of debt and idle that make up - * totalAssets based on the actual current loose amount of `asset` - * in a safe way. But will keep `totalAssets` the same, thus having - * no effect on Price Per Share. - */ - function _updateBalances() internal { - StrategyData storage S = _strategyStorage(); - - // Get the current loose balance. - uint256 assetBalance = S.asset.balanceOf(address(this)); - - // If its already accurate do nothing. - if (S.totalIdle == assetBalance) return; - - // Get the total assets the strategy should have. - uint256 _totalAssets = totalAssets(); - - // If we have enough loose to cover all assets. - if (assetBalance >= _totalAssets) { - // Set idle to totalAssets. - S.totalIdle = _totalAssets; - // Set debt to 0. - S.totalDebt = 0; - } else { - // Otherwise idle is the actual loose balance. - S.totalIdle = assetBalance; - unchecked { - // And debt is the difference. - S.totalDebt = _totalAssets - assetBalance; - } - } - - // Enforce the invariant. - require(_totalAssets == totalAssets(), "!totalAssets"); + // Tend the strategy with the current loose balance. + IBaseStrategy(address(this)).tendThis( + _strategyStorage().asset.balanceOf(address(this)) + ); } /*////////////////////////////////////////////////////////////// @@ -1315,8 +1230,8 @@ contract TokenizedStrategy { * strategy has been shutdown. * @dev This can only be called post {shutdownStrategy}. * - * This will update totalDebt and totalIdle based on the amount of - * loose `asset` after the withdraw leaving `totalAssets` unchanged. + * This will never cause a change in PPS. Total assets will + * be the same before and after. * * A strategist will need to override the {_emergencyWithdraw} function * in their strategy for this to work. @@ -1331,9 +1246,6 @@ contract TokenizedStrategy { // Withdraw from the yield source. IBaseStrategy(address(this)).shutdownWithdraw(amount); - - // Record the updated balances based on the new amounts. - _updateBalances(); } /*////////////////////////////////////////////////////////////// @@ -1356,22 +1268,6 @@ contract TokenizedStrategy { return API_VERSION; } - /** - * @notice Get the current total idle for a strategy. - * @return . The current amount of idle funds. - */ - function totalIdle() external view returns (uint256) { - return _strategyStorage().totalIdle; - } - - /** - * @notice Get the current total debt for a strategy. - * @return . The current amount of debt. - */ - function totalDebt() external view returns (uint256) { - return _strategyStorage().totalDebt; - } - /** * @notice Get the current address that controls the strategy. * @return . Address of management diff --git a/src/interfaces/ITokenizedStrategy.sol b/src/interfaces/ITokenizedStrategy.sol index 15e588c..c610350 100644 --- a/src/interfaces/ITokenizedStrategy.sol +++ b/src/interfaces/ITokenizedStrategy.sol @@ -104,10 +104,6 @@ interface ITokenizedStrategy is IERC4626, IERC20Permit { function pricePerShare() external view returns (uint256); - function totalIdle() external view returns (uint256); - - function totalDebt() external view returns (uint256); - function management() external view returns (address); function pendingManagement() external view returns (address); diff --git a/src/test/Accounting.t.sol b/src/test/Accounting.t.sol index 191199a..a71a68a 100644 --- a/src/test/Accounting.t.sol +++ b/src/test/Accounting.t.sol @@ -4,8 +4,6 @@ pragma solidity 0.8.18; import "forge-std/console.sol"; import {Setup, IMockStrategy} from "./utils/Setup.sol"; -// TODO: add checkStrategyTotals to all of these tests - contract AccountingTest is Setup { function setUp() public override { super.setUp(); @@ -41,20 +39,25 @@ contract AccountingTest is Setup { uint256 toAirdrop = (_amount * _profitFactor) / MAX_BPS; asset.mint(address(strategy), toAirdrop); - // nothing should change + // PPS shouldn't change but the balance does. assertEq(strategy.pricePerShare(), pricePerShare); - assertEq(strategy.totalDebt(), _amount); - assertEq(strategy.totalIdle(), 0); + checkStrategyTotals( + strategy, + _amount, + _amount - toAirdrop, + toAirdrop, + _amount + ); uint256 beforeBalance = asset.balanceOf(_address); vm.prank(_address); strategy.redeem(_amount, _address, _address); - // should have pulled out just the deposit amount + // should have pulled out just the deposited amount leaving the rest deployed. assertEq(asset.balanceOf(_address), beforeBalance + _amount); - assertEq(strategy.totalDebt(), 0); - assertEq(strategy.totalIdle(), 0); - assertEq(asset.balanceOf(address(strategy)), toAirdrop); + assertEq(asset.balanceOf(address(strategy)), 0); + assertEq(asset.balanceOf(address(yieldSource)), toAirdrop); + checkStrategyTotals(strategy, 0, 0, 0, 0); } function test_airdropDoesNotIncreasePPS_reportRecordsIt( @@ -87,10 +90,15 @@ contract AccountingTest is Setup { uint256 toAirdrop = (_amount * _profitFactor) / MAX_BPS; asset.mint(address(strategy), toAirdrop); - // nothing should change + // PPS shouldn't change but the balance does. assertEq(strategy.pricePerShare(), pricePerShare); - assertEq(strategy.totalDebt(), _amount); - assertEq(strategy.totalIdle(), 0); + checkStrategyTotals( + strategy, + _amount, + _amount - toAirdrop, + toAirdrop, + _amount + ); // process a report to realize the gain from the airdrop uint256 profit; @@ -99,8 +107,13 @@ contract AccountingTest is Setup { assertEq(strategy.pricePerShare(), pricePerShare); assertEq(profit, toAirdrop); - assertEq(strategy.totalDebt(), _amount + toAirdrop); - assertEq(strategy.totalIdle(), 0); + checkStrategyTotals( + strategy, + _amount + toAirdrop, + _amount + toAirdrop, + 0, + _amount + toAirdrop + ); // allow some profit to come unlocked skip(profitMaxUnlockTime / 2); @@ -121,8 +134,9 @@ contract AccountingTest is Setup { wad + ((wad * _profitFactor) / MAX_BPS), MAX_BPS ); - assertEq(strategy.totalDebt(), _amount + toAirdrop); - assertEq(strategy.totalIdle(), 0); + + // Total is the same but balance has adjusted again + checkStrategyTotals(strategy, _amount + toAirdrop, _amount, toAirdrop); uint256 beforeBalance = asset.balanceOf(_address); vm.prank(_address); @@ -133,9 +147,9 @@ contract AccountingTest is Setup { asset.balanceOf(_address), beforeBalance + _amount + toAirdrop ); - assertEq(strategy.totalDebt(), 0); - assertEq(strategy.totalIdle(), 0); - assertEq(asset.balanceOf(address(strategy)), toAirdrop); + assertEq(asset.balanceOf(address(strategy)), 0); + assertEq(asset.balanceOf(address(yieldSource)), toAirdrop); + checkStrategyTotals(strategy, 0, 0, 0, 0); } function test_earningYieldDoesNotIncreasePPS( @@ -170,8 +184,7 @@ contract AccountingTest is Setup { // nothing should change assertEq(strategy.pricePerShare(), pricePerShare); - assertEq(strategy.totalDebt(), _amount); - assertEq(strategy.totalIdle(), 0); + checkStrategyTotals(strategy, _amount, _amount, 0, _amount); uint256 beforeBalance = asset.balanceOf(_address); vm.prank(_address); @@ -179,9 +192,8 @@ contract AccountingTest is Setup { // should have pulled out just the deposit amount assertEq(asset.balanceOf(_address), beforeBalance + _amount); - assertEq(strategy.totalDebt(), 0); - assertEq(strategy.totalIdle(), 0); assertEq(asset.balanceOf(address(yieldSource)), toAirdrop); + checkStrategyTotals(strategy, 0, 0, 0, 0); } function test_earningYieldDoesNotIncreasePPS_reportRecordsIt( @@ -214,10 +226,10 @@ contract AccountingTest is Setup { uint256 toAirdrop = (_amount * _profitFactor) / MAX_BPS; asset.mint(address(yieldSource), toAirdrop); assertEq(asset.balanceOf(address(yieldSource)), _amount + toAirdrop); + // nothing should change assertEq(strategy.pricePerShare(), pricePerShare); - assertEq(strategy.totalDebt(), _amount); - assertEq(strategy.totalIdle(), 0); + checkStrategyTotals(strategy, _amount, _amount, 0, _amount); // process a report to realize the gain from the airdrop uint256 profit; @@ -226,8 +238,14 @@ contract AccountingTest is Setup { assertEq(strategy.pricePerShare(), pricePerShare); assertEq(profit, toAirdrop); - assertEq(strategy.totalDebt(), _amount + toAirdrop); - assertEq(strategy.totalIdle(), 0); + + checkStrategyTotals( + strategy, + _amount + toAirdrop, + _amount + toAirdrop, + 0, + _amount + toAirdrop + ); // allow some profit to come unlocked skip(profitMaxUnlockTime / 2); @@ -248,8 +266,14 @@ contract AccountingTest is Setup { wad + ((wad * _profitFactor) / MAX_BPS), MAX_BPS ); - assertEq(strategy.totalDebt(), _amount + toAirdrop); - assertEq(strategy.totalIdle(), 0); + + // Total is the same. + checkStrategyTotals( + strategy, + _amount + toAirdrop, + _amount + toAirdrop, + 0 + ); uint256 beforeBalance = asset.balanceOf(_address); vm.prank(_address); @@ -260,9 +284,9 @@ contract AccountingTest is Setup { asset.balanceOf(_address), beforeBalance + _amount + toAirdrop ); - assertEq(strategy.totalDebt(), 0); - assertEq(strategy.totalIdle(), 0); + assertEq(asset.balanceOf(address(yieldSource)), toAirdrop); + checkStrategyTotals(strategy, 0, 0, 0, 0); } function test_tend_noIdle_harvestProfit( @@ -287,14 +311,13 @@ contract AccountingTest is Setup { uint256 toAirdrop = (_amount * _profitFactor) / MAX_BPS; asset.mint(address(strategy), toAirdrop); assertEq(asset.balanceOf(address(strategy)), toAirdrop); + checkStrategyTotals(strategy, _amount, _amount - toAirdrop, toAirdrop); vm.prank(keeper); strategy.tend(); // Should have deposited the toAirdrop amount but no other changes - assertEq(strategy.totalAssets(), _amount, "!assets"); - assertEq(strategy.totalDebt(), _amount, "1debt"); - assertEq(strategy.totalIdle(), 0, "!idle"); + checkStrategyTotals(strategy, _amount, _amount, 0); assertEq( asset.balanceOf(address(yieldSource)), _amount + toAirdrop, @@ -320,9 +343,8 @@ contract AccountingTest is Setup { // should have pulled out the deposit plus profit that was reported but not the second airdrop assertEq(asset.balanceOf(user), beforeBalance + _amount + toAirdrop); - assertEq(strategy.totalDebt(), 0); - assertEq(strategy.totalIdle(), 0); assertEq(asset.balanceOf(address(yieldSource)), 0); + checkStrategyTotals(strategy, 0, 0, 0, 0); } function test_tend_idleFunds_harvestProfit( @@ -344,9 +366,14 @@ contract AccountingTest is Setup { mintAndDepositIntoStrategy(strategy, user, _amount); uint256 expectedDeposit = _amount / 2; - assertEq(strategy.totalAssets(), _amount, "!assets"); - assertEq(strategy.totalDebt(), expectedDeposit, "1debt"); - assertEq(strategy.totalIdle(), _amount - expectedDeposit, "!idle"); + checkStrategyTotals( + strategy, + _amount, + expectedDeposit, + _amount - expectedDeposit, + _amount + ); + assertEq( asset.balanceOf(address(yieldSource)), expectedDeposit, @@ -367,9 +394,7 @@ contract AccountingTest is Setup { strategy.tend(); // Should have withdrawn all the funds from the yield source - assertEq(strategy.totalAssets(), _amount, "!assets"); - assertEq(strategy.totalDebt(), 0, "1debt"); - assertEq(strategy.totalIdle(), _amount, "!idle"); + checkStrategyTotals(strategy, _amount, 0, _amount, _amount); assertEq(asset.balanceOf(address(yieldSource)), 0, "!yieldSource"); assertEq(asset.balanceOf(address(strategy)), _amount + toAirdrop); assertEq(strategy.pricePerShare(), wad, "!pps"); @@ -378,10 +403,10 @@ contract AccountingTest is Setup { vm.prank(keeper); strategy.report(); - assertEq(strategy.totalAssets(), _amount + toAirdrop); - assertEq(strategy.totalDebt(), (_amount + toAirdrop) / 2); - assertEq( - strategy.totalIdle(), + checkStrategyTotals( + strategy, + _amount + toAirdrop, + (_amount + toAirdrop) / 2, (_amount + toAirdrop) - ((_amount + toAirdrop) / 2) ); assertEq( @@ -454,10 +479,8 @@ contract AccountingTest is Setup { uint256 afterBalance = asset.balanceOf(_address); assertEq(afterBalance - beforeBalance, expectedOut); - assertEq(strategy.totalDebt(), 0); - assertEq(strategy.totalIdle(), 0); - assertEq(strategy.totalSupply(), 0); assertEq(strategy.pricePerShare(), wad); + checkStrategyTotals(strategy, 0, 0, 0, 0); } function test_redeemWithUnrealizedLoss( @@ -490,10 +513,8 @@ contract AccountingTest is Setup { uint256 afterBalance = asset.balanceOf(_address); assertEq(afterBalance - beforeBalance, expectedOut); - assertEq(strategy.totalDebt(), 0); - assertEq(strategy.totalIdle(), 0); - assertEq(strategy.totalSupply(), 0); assertEq(strategy.pricePerShare(), wad); + checkStrategyTotals(strategy, 0, 0, 0, 0); } function test_redeemWithUnrealizedLoss_allowNoLoss_reverts( @@ -558,9 +579,7 @@ contract AccountingTest is Setup { uint256 afterBalance = asset.balanceOf(_address); assertEq(afterBalance - beforeBalance, expectedOut); - assertEq(strategy.totalDebt(), 0); - assertEq(strategy.totalIdle(), 0); - assertEq(strategy.totalSupply(), 0); assertEq(strategy.pricePerShare(), wad); + checkStrategyTotals(strategy, 0, 0, 0, 0); } } diff --git a/src/test/CustomImplementation.t.sol b/src/test/CustomImplementation.t.sol index 74683d8..bfc1b1e 100644 --- a/src/test/CustomImplementation.t.sol +++ b/src/test/CustomImplementation.t.sol @@ -31,7 +31,7 @@ contract CustomImplementationsTest is Setup { mintAndDepositIntoStrategy(strategy, _address, _amount); - uint256 idle = strategy.totalIdle(); + uint256 idle = asset.balanceOf(address(strategy)); assertGt(idle, 0); // Assure we have a withdraw limit @@ -58,7 +58,7 @@ contract CustomImplementationsTest is Setup { increaseTimeAndCheckBuffer(strategy, 5 days, profit / 2); - idle = strategy.totalIdle(); + idle = asset.balanceOf(address(strategy)); assertGt(idle, 0); // Assure we have a withdraw limit @@ -89,7 +89,6 @@ contract CustomImplementationsTest is Setup { // We need to give a i wei rounding buffer assertApproxEq(asset.balanceOf(_address) - before, idle, 1); - assertApproxEq(strategy.totalIdle(), 0, 1); assertApproxEq(strategy.availableWithdrawLimit(_address), 0, 1); assertApproxEq(strategy.maxWithdraw(_address), 0, 1); assertApproxEq(strategy.maxRedeem(_address), 0, 1); diff --git a/src/test/FaultyStrategy.t.sol b/src/test/FaultyStrategy.t.sol index 11d6106..1e36e11 100644 --- a/src/test/FaultyStrategy.t.sol +++ b/src/test/FaultyStrategy.t.sol @@ -98,51 +98,6 @@ contract FaultyStrategy is Setup { assertEq(asset.balanceOf(address(strategy)), _faultAmount); } - // Test that tend will update balances correctly even if they - // were not moved during the tend call. - function test_fundsMoved_tendUpdates( - address _address, - uint256 _amount - ) public { - _amount = bound(_amount, minFuzzAmount, maxFuzzAmount); - strategy = IMockStrategy(setUpFaultyStrategy()); - vm.assume( - _address != address(0) && - _address != address(strategy) && - _address != address(yieldSource) - ); - - setFees(0, 0); - mintAndDepositIntoStrategy(strategy, _address, _amount); - - checkStrategyTotals(strategy, _amount, _amount, 0, _amount); - - // For rounding - uint256 toWithdraw = _amount / 2; - uint256 difference = _amount - toWithdraw; - - // Simulate funds moving around by an non-core function. - vm.prank(address(strategy)); - yieldSource.withdraw(toWithdraw); - - // Make sure the balances were updated but the strategy doesn't know yet. - assertEq(asset.balanceOf(address(strategy)), toWithdraw); - assertEq(asset.balanceOf(address(yieldSource)), difference); - checkStrategyTotals(strategy, _amount, _amount, 0, _amount); - - // Set the tend to do nothing. - strategy.setDontTend(true); - assertTrue(strategy.dontTend()); - - vm.prank(keeper); - strategy.tend(); - - // Make sure the balances were updated correctly on both ends - assertEq(asset.balanceOf(address(strategy)), toWithdraw); - assertEq(asset.balanceOf(address(yieldSource)), difference); - checkStrategyTotals(strategy, _amount, difference, toWithdraw, _amount); - } - function test_deployFundsViewReentrancy( address _address, uint256 _amount, @@ -269,8 +224,15 @@ contract FaultyStrategy is Setup { createAndCheckLoss(strategy, profit, 0, true); } - function test_tendViewReentrancy(address _address, uint256 _amount) public { + function test_tendViewReentrancy( + address _address, + uint256 _amount, + uint16 _profitFactor + ) public { _amount = bound(_amount, minFuzzAmount, maxFuzzAmount); + _profitFactor = uint16(bound(uint256(_profitFactor), 10, MAX_BPS)); + uint256 toAirdrop = (_amount * _profitFactor) / MAX_BPS; + strategy = IMockStrategy(setUpFaultyStrategy()); vm.assume( @@ -284,8 +246,10 @@ contract FaultyStrategy is Setup { mintAndDepositIntoStrategy(strategy, _address, _amount); configureFaultyStrategy(0, true); - // Tend with some idle - storeCallBackVariables(strategy.totalIdle()); + + // Tend with some loose balance + asset.mint(address(strategy), toAirdrop); + storeCallBackVariables(toAirdrop); vm.prank(keeper); strategy.tend(); @@ -368,7 +332,7 @@ contract FaultyStrategy is Setup { checkStrategyTotals(strategy, _amount, _amount, 0, _amount); } - // Reentrancy cant be allowed during a report call/ + // Reentrancy cant be allowed during a report call. function test_reportReentrancy_reverts( address _address, uint256 _amount, @@ -403,7 +367,13 @@ contract FaultyStrategy is Setup { vm.prank(keeper); strategy.report(); - checkStrategyTotals(strategy, _amount, _amount, 0, _amount); + checkStrategyTotals( + strategy, + _amount, + _amount - profit, + profit, + _amount + ); } function test_tendReentrancy_reverts( diff --git a/src/test/InvariantsSingleStrategy.t.sol b/src/test/InvariantsSingleStrategy.t.sol index b1accfa..fd7df87 100644 --- a/src/test/InvariantsSingleStrategy.t.sol +++ b/src/test/InvariantsSingleStrategy.t.sol @@ -34,10 +34,6 @@ contract SingleStrategyInvariantTest is BaseInvariant { assert_totalAssets(); } - function invariant_idle() public { - assert_idle(); - } - function invariant_maxWithdraw() public { assert_maxWithdraw(); } diff --git a/src/test/mocks/MockFaultyStrategy.sol b/src/test/mocks/MockFaultyStrategy.sol index 851ceda..e43e9ab 100644 --- a/src/test/mocks/MockFaultyStrategy.sol +++ b/src/test/mocks/MockFaultyStrategy.sol @@ -32,7 +32,7 @@ contract MockFaultyStrategy is BaseStrategy { if (doCallBack) { callBack(_amount); } - MockYieldSource(yieldSource).deposit(_amount + fault); + MockYieldSource(yieldSource).deposit(_amount); } function _freeFunds(uint256 _amount) internal override { diff --git a/src/test/mocks/MockIlliquidStrategy.sol b/src/test/mocks/MockIlliquidStrategy.sol index 0f6a3e3..cc6d123 100644 --- a/src/test/mocks/MockIlliquidStrategy.sol +++ b/src/test/mocks/MockIlliquidStrategy.sol @@ -55,7 +55,7 @@ contract MockIlliquidStrategy is BaseStrategy { function availableWithdrawLimit( address /*_owner*/ ) public view override returns (uint256) { - return TokenizedStrategy.totalIdle(); + return asset.balanceOf(address(this)); } function setWhitelist(bool _bool) external { diff --git a/src/test/utils/BaseInvariant.sol b/src/test/utils/BaseInvariant.sol index 2f934af..392b890 100644 --- a/src/test/utils/BaseInvariant.sol +++ b/src/test/utils/BaseInvariant.sol @@ -21,14 +21,7 @@ abstract contract BaseInvariant is Setup { } function assert_totalAssets() public { - assertEq( - strategy.totalAssets(), - strategy.totalIdle() + strategy.totalDebt() - ); - } - - function assert_idle() public { - assertLe(strategy.totalIdle(), asset.balanceOf(address(strategy))); + // check totalDeposits + profit - withdraws - losses } function assert_maxWithdraw() public { diff --git a/src/test/utils/Setup.sol b/src/test/utils/Setup.sol index 232907c..ed1432d 100644 --- a/src/test/utils/Setup.sol +++ b/src/test/utils/Setup.sol @@ -158,9 +158,15 @@ contract Setup is ExtendedTest, IEvents { uint256 _totalIdle, uint256 _totalSupply ) public { - assertEq(_strategy.totalAssets(), _totalAssets, "!totalAssets"); - assertEq(_strategy.totalDebt(), _totalDebt, "!totalDebt"); - assertEq(_strategy.totalIdle(), _totalIdle, "!totalIdle"); + uint256 _assets = _strategy.totalAssets(); + uint256 _balance = ERC20Mock(_strategy.asset()).balanceOf( + address(_strategy) + ); + uint256 _idle = _balance > _assets ? _assets : _balance; + uint256 _debt = _assets - _idle; + assertEq(_assets, _totalAssets, "!totalAssets"); + assertEq(_debt, _totalDebt, "!totalDebt"); + assertEq(_idle, _totalIdle, "!totalIdle"); assertEq(_totalAssets, _totalDebt + _totalIdle, "!Added"); // We give supply a buffer or 1 wei for rounding assertApproxEq(_strategy.totalSupply(), _totalSupply, 1, "!supply"); @@ -173,9 +179,15 @@ contract Setup is ExtendedTest, IEvents { uint256 _totalDebt, uint256 _totalIdle ) public { - assertEq(_strategy.totalAssets(), _totalAssets, "!totalAssets"); - assertEq(_strategy.totalDebt(), _totalDebt, "!totalDebt"); - assertEq(_strategy.totalIdle(), _totalIdle, "!totalIdle"); + uint256 _assets = _strategy.totalAssets(); + uint256 _balance = ERC20Mock(_strategy.asset()).balanceOf( + address(_strategy) + ); + uint256 _idle = _balance > _assets ? _assets : _balance; + uint256 _debt = _assets - _idle; + assertEq(_assets, _totalAssets, "!totalAssets"); + assertEq(_debt, _totalDebt, "!totalDebt"); + assertEq(_idle, _totalIdle, "!totalIdle"); assertEq(_totalAssets, _totalDebt + _totalIdle, "!Added"); } @@ -265,7 +277,7 @@ contract Setup is ExtendedTest, IEvents { assembly { // Perf fee is stored in the 12th slot of the Struct. - slot := add(S.slot, 12) + slot := add(S.slot, 11) } // Performance fee is packed in a slot with other variables so we need