From 6990830a5e0d5c436166ade22b8d12e07b38fbd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Mart=C3=ADn=20Alconada=20Verzini?= Date: Tue, 8 Nov 2022 19:01:27 +0000 Subject: [PATCH] chore: add more tests --- README.md | 2 +- codecov.yml | 5 + src/AutoRoller.sol | 10 +- src/test/AutoRoller.t.sol | 187 +++++++++++++++++++++++++++++++++++++- 4 files changed, 197 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 8cb4cde..95d21d2 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ Here are some important concepts to understand so you know how the auto-roller w * `AutoRollerFactory` - on-chain factory for easy deployment of new auto-roller instances. * `RollerPeriphery` - the slippage protected LP management interface to all AutoRollers. -* `OwnableAdapter` - Sense adapter owned by a unique auto-roller. Through the auto-roller, series rollers can sponsor new series via `AutoRoller.roll` and settle series via `AutoRoller.settle`. +* `OwnableAdapter` - Sense adapter owned by a unique auto-roller. Through the auto-roller, series rollers can sponsor new series via `AutoRoller.roll` and settle series via `AutoRoller.settle`. This adapter must **override** `getMaturityBounds` so only the roller can sponsor preventing a potential DoS attack. ### Phases The auto-roller has an **active** and a **cooldown** phase. During the active phase, there is a specific Space pool that the auto-roller is managing, and the liquidity it holds is in the form of Space LP shares and YTs (e.g. sY-wstETH). In contrast, the cooldown phase is in-between active phases when the auto-roller has settled one series but not yet sponsored a new one and deposited liquidity into the new pool (also at the very beginning of the contract’s lifecycle before it has entered the first pool). diff --git a/codecov.yml b/codecov.yml index e69de29..27f94db 100644 --- a/codecov.yml +++ b/codecov.yml @@ -0,0 +1,5 @@ +# interfaces are automatically ignored by forge coverage +ignore: + - "**/tests/**/*" # ignore all files inside tests/ folder + - "**/**.t*.sol" # ignore all files ending in t*.sol + - "**/scripts/**/*" # ignore all files inside scripts/ folder \ No newline at end of file diff --git a/src/AutoRoller.sol b/src/AutoRoller.sol index 83338ad..a7bec71 100644 --- a/src/AutoRoller.sol +++ b/src/AutoRoller.sol @@ -76,7 +76,7 @@ contract AutoRoller is ERC4626 { DividerLike internal immutable divider; BalancerVault internal immutable balancerVault; - OwnedAdapterLike internal immutable adapter; + OwnedAdapterLike internal immutable adapter; // This adapter must **override** `getMaturityBounds` so only the roller can sponsor preventing a potential DoS attack. uint256 internal immutable ifee; // Cached issuance fee. uint256 internal immutable minSwapAmount; // Min number of PTs that can be swapped out when exiting. @@ -200,9 +200,9 @@ contract AutoRoller is ERC4626 { uint256 targetBal = _asset.balanceOf(address(this)); // Get the reserve balances that would imply the given targetedRate in the Space pool, - // assuming that we we're going to deposit the amount of Target currently in this contract. + // assuming that we're going to deposit the amount of Target currently in this contract. // In other words, this function simulating the reserve balances that would result from the actions: - // 1) Use the some Target to issue PTs/YTs + // 1) Use some Target to issue PTs/YTs // 2) Deposit some amount of Target // 3) Swap PTs into the pool to initialize the targeted rate // 4) Deposit the rest of the PTs and Target in this contract (which remain in the exact ratio the pool expects) @@ -272,7 +272,7 @@ contract AutoRoller is ERC4626 { } /// @notice Settle the active Series, transfer stake and ifees to the settler, and enter a cooldown phase. - /// @dev Because the auto-roller is the series sponsor from the Divider's perspective, this.settle is the only entrypoint for athe lastRoller to settle during the series' sponsor window. + /// @dev Because the auto-roller is the series sponsor from the Divider's perspective, this.settle is the only entrypoint for the lastRoller to settle during the series' sponsor window. /// More info on the series lifecylce: https://docs.sense.finance/docs/series-lifecycle-detail/#phase-3-settling. function settle() public { if(msg.sender != lastRoller) revert InvalidSettler(); @@ -369,7 +369,7 @@ contract AutoRoller is ERC4626 { balances[1 - _pti] = targetToJoin; - if (assets - targetToJoin > 0) { // Assumption: this is false if Space has only Target liquidity. + if (assets - targetToJoin > 0) { // Assumption: this is false if Space has only Target liquidity. TODO: can this ever happen? balances[_pti] = divider.issue(address(adapter), maturity, assets - targetToJoin); } diff --git a/src/test/AutoRoller.t.sol b/src/test/AutoRoller.t.sol index 6c6c148..f3c9884 100644 --- a/src/test/AutoRoller.t.sol +++ b/src/test/AutoRoller.t.sol @@ -167,6 +167,9 @@ contract AutoRollerTest is DSTestPlus { vm.expectRevert(); autoRoller.setParam("OWNER", address(0xbabe)); + vm.expectRevert(); + autoRoller.setParam("UNKNOWN", address(0xbabe)); + vm.expectRevert(); autoRoller.setParam("MAX_RATE", 1337); @@ -176,6 +179,9 @@ contract AutoRollerTest is DSTestPlus { vm.expectRevert(); autoRoller.setParam("COOLDOWN", 1337); + vm.expectRevert(); + autoRoller.setParam("UNKNOWN", 1337); + (, bytes32[] memory writes) = vm.accesses(address(autoRoller)); // Check that no storage slots were written to assertEq(writes.length, 0); @@ -936,12 +942,31 @@ contract AutoRollerTest is DSTestPlus { utils.getNewTargetedRate(0, address(mockAdapter), maturity, space); vm.warp(maturity); + + // Scale goes down + mockAdapter.setScale(0.9e18); autoRoller.settle(); // Targeted rate is 0 if scale has gone down. - mockAdapter.setScale(0.9e18); uint256 targetedRate = utils.getNewTargetedRate(0, address(mockAdapter), maturity, space); assertEq(targetedRate, 0); + + vm.warp(maturity + autoRoller.cooldown()); + + // Roll into new series + autoRoller.roll(); + + // Scale goes up + mockAdapter.setScale(1.9e18); + + maturity = autoRoller.maturity(); + vm.warp(maturity); + + autoRoller.settle(); + + // Targeted rate is not 0 TODO: maybe assert against the rate that should be + targetedRate = utils.getNewTargetedRate(0, address(mockAdapter), maturity, space); + assertGt(targetedRate, 0); } // function testRedeemPreviewReversion() public { @@ -952,6 +977,166 @@ contract AutoRollerTest is DSTestPlus { // redeem doesn't revert // decimals + // OTHER TESTS (LINES NOT COVERED ACCORDING CODECOV) TODO: re-order these tests + function testAfterDepositWithOnlyTargetLiquidity() public { + // The idea is making a test that covers line case on the `afterDeposit` function + // where `if (assets - targetToJoin > 0)` is false. + // I think can never happen since `onSponsorWindowOpened`, when calling `_space.getEQReserves` + // we are doing `targetedRate < 0.01e18 ? 0.01e18 : targetedRate` which means that we are always + // getting a `eqPTReserves` > 0 so we will be issuing PTs. + } + + function testDepositWithdrawWithYTsExcess() public { + // 1. Deposit during the initial cooldown phase. + autoRoller.deposit(0.2e18, address(this)); + assertEq(autoRoller.balanceOf(address(this)), 0.2e18); + + // // 2. Deposit again, this time minting the Vault shares to alice. + // autoRoller.deposit(0.2e18, alice); + // assertEq(autoRoller.balanceOf(alice), 0.2e18); + + // vm.prank(alice); + // // 3. Withdraw all of Alice's Target. + // autoRoller.withdraw(0.2e18, alice, alice); + // assertEq(autoRoller.balanceOf(alice), 0); + // assertEq(target.balanceOf(alice), 0.2e18); + + // 4. Roll the Target into the first Series. + autoRoller.roll(); + + // 5. Deposit during the first active phase. + autoRoller.deposit(0.3e18, address(this)); + assertRelApproxEq(autoRoller.balanceOf(address(this)), 0.5e18, 0.0001e18 /* 0.01% */); + + // 6. Redeem all shares while still in the active phase. + uint256 targetBalPre = target.balanceOf(address(this)); + vm.expectCall(address(periphery), abi.encodeWithSelector(periphery.swapYTsForTarget.selector)); + autoRoller.redeem(autoRoller.balanceOf(address(this)), address(this), address(this)); + uint256 targetBalPost = target.balanceOf(address(this)); + // assertRelApproxEq(targetBalPost - targetBalPre, 0.5e18, 0.0001e18 /* 0.01% */); // TODO: what should t be the expected value? + + // Check that the Target dust leftover is small + assertLt(target.balanceOf(address(autoRoller)), 1e9); + } + + function testPreviewWithdrawInsufficentLiquidity(uint256 amount) public { + // 1. Roll into the first Series. + autoRoller.roll(); + + // 2. Make a deposit. + autoRoller.deposit(1e18, address(this)); + + // 3. Expect revert because of InsufficientLiquidity. + vm.expectRevert(abi.encodeWithSelector(AutoRoller.InsufficientLiquidity.selector)); + autoRoller.previewWithdraw(100e18); + } + + function testMaxWithdrawDuringCooldown(uint256 amount) public { + // 1. Make a deposit durring cooldown. + autoRoller.deposit(1e18, address(this)); + + // 2. Assert max withdraw is same as deposit. + uint256 maxWithdraw = autoRoller.maxWithdraw(address(this)); + assertEq(maxWithdraw, 1e18); + } + + function testMaxRedeemDuringCooldown(uint256 amount) public { + // 1. Make a deposit durring cooldown. + autoRoller.deposit(1e18, address(this)); + + // 2. Assert max withdraw is same as deposit. + uint256 maxRedeem = autoRoller.maxRedeem(address(this)); + assertEq(maxRedeem, 1e18); + } + + function testMaxRedeemWithPTsExcess(uint256 amount) public { + // 1. Make a deposit durring cooldown. + autoRoller.deposit(1e18, address(this)); + + // 2. Roll into the first Series. + autoRoller.roll(); + + // 3. Swap PTs in to generate excess of PTs. + target.mint(address(this), 1e18); + target.approve(address(divider), 1e18); + divider.issue(address(mockAdapter), autoRoller.maturity(), 1e18); + + ERC20 pt = ERC20(divider.pt(address(mockAdapter), autoRoller.maturity())); + Space space = Space(spaceFactory.pools(address(mockAdapter), autoRoller.maturity())); + + pt.approve(address(balancerVault), 1e18); + _swap( + BalancerVault.SingleSwap({ + poolId: space.getPoolId(), + kind: BalancerVault.SwapKind.GIVEN_IN, + assetIn: address(pt), + assetOut: address(autoRoller.asset()), + amount: 0.001e18, + userData: hex"" + }) + ); + + // 4. Assert max redeem is same as deposit. + uint256 maxRedeem = autoRoller.maxRedeem(address(this)); + assertEq(maxRedeem, 1e18); + + // 5. Swap PTs in to generate excess of PTs and that the number of PTs we can sell + // is more than the diff between YTs and PTs. + _swap( + BalancerVault.SingleSwap({ + poolId: space.getPoolId(), + kind: BalancerVault.SwapKind.GIVEN_IN, + assetIn: address(pt), + assetOut: address(autoRoller.asset()), + amount: 0.1e18, // TODO: how do I calculate a number such that it would make the maxPTSale >= diff? This number works though + userData: hex"" + }) + ); + + // 6. Assert max redeem is TODO. + maxRedeem = autoRoller.maxRedeem(address(this)); + // TODO: assertEq(maxRedeem, 123); + + // 7. TODO: add missing scenario: `if (ptReserves >= diff)`. + } + + function testEjectWithPTsExcess() public { + // 1. Deposit during the initial cooldown phase. + autoRoller.deposit(1e18, address(this)); + + // 2. Roll into the first Series. + autoRoller.roll(); + + // 3. Swap PTs in to generate excess of PTs. + target.mint(address(this), 1e18); + target.approve(address(divider), 1e18); + divider.issue(address(mockAdapter), autoRoller.maturity(), 1e18); + + ERC20 pt = ERC20(divider.pt(address(mockAdapter), autoRoller.maturity())); + Space space = Space(spaceFactory.pools(address(mockAdapter), autoRoller.maturity())); + + pt.approve(address(balancerVault), 1e18); + _swap( + BalancerVault.SingleSwap({ + poolId: space.getPoolId(), + kind: BalancerVault.SwapKind.GIVEN_IN, + assetIn: address(pt), + assetOut: address(autoRoller.asset()), + amount: 0.001e18, + userData: hex"" + }) + ); + + // 4. Eject everything. + uint256 ptBalBefore = pt.balanceOf(address(this)); + ( , uint256 excessBal, bool isExcessPTs) = autoRoller.eject(autoRoller.balanceOf(address(this)), address(this), address(this)); + + // Expect just a little PT excess. + assertBoolEq(isExcessPTs, true); + assertLt(excessBal, 1e16); + assertEq(excessBal, pt.balanceOf(address(this)) - ptBalBefore); + } + function _swap(BalancerVault.SingleSwap memory request) internal { BalancerVault.FundManagement memory funds = BalancerVault.FundManagement({ sender: address(this),