From c7c84e2087b263a12a521ef88739132696ec747a Mon Sep 17 00:00:00 2001 From: Ian Harvey Date: Wed, 7 Jun 2023 08:11:50 -0400 Subject: [PATCH] Quote to lps inc on move (#877) * cleaned up requires, formally added more PositionManager invar * cleaned up requires, formally added more PositionManager invar * added logs * works with diff * cleanup --------- Co-authored-by: Ian Harvey --- tests/INVARIANTS.md | 7 ++ .../PositionsInvariants.t.sol | 17 ++--- .../RewardsInvariants.t.sol | 29 +++----- .../handlers/PositionHandlerAbstract.sol | 8 +-- .../handlers/RewardsHandler.sol | 4 -- .../unbounded/BasePositionsHandler.sol | 9 ++- .../unbounded/UnboundedPositionsHandler.sol | 72 +++++++++++-------- .../unbounded/UnboundedRewardsHandler.sol | 5 +- .../forge/invariants/base/BaseInvariants.sol | 16 ----- .../invariants/base/BasicInvariants.t.sol | 1 + .../base/LiquidationInvariants.t.sol | 1 + .../invariants/base/ReserveInvariants.t.sol | 1 + .../invariants/interfaces/IBaseHandler.sol | 7 -- .../IPositionsAndRewardsHandler.sol | 13 ++++ .../RegressionRewardsManager.t.sol | 9 ++- tests/forge/utils/DSTestPlus.sol | 15 +++- 16 files changed, 115 insertions(+), 99 deletions(-) create mode 100644 tests/forge/invariants/interfaces/IPositionsAndRewardsHandler.sol diff --git a/tests/INVARIANTS.md b/tests/INVARIANTS.md index b4a1ab304..1832ed1ab 100644 --- a/tests/INVARIANTS.md +++ b/tests/INVARIANTS.md @@ -93,3 +93,10 @@ - **PM1**: LP balance of PositionManager in a Pool for a Bucket should be the sum of the positions[...][index].lps for all tokens/users - **PM2** Sum of the LP balance of the PositionManager in a Pool across all buckets should be the sum of the positions[...].[...].lps across all indexes and tokens/users - **PM3**: Position deposit time (`depositTime`) tracked by tokenId in PositionManager (`positions[tokenId][index]`) should always be of equal or lesser value than the PositionManager's LP at that index in the pool contract. +- **PM4**: mint should populate `poolKey[tokenId]`, `positionIndexes` should be empty and the owner of the NFT should be the caller. +- **PM5**: burn should reset `poolKey[tokenId]`, `positionIndexes` should be empty and the owner of the NFT should be the zero address. +- **PM6**: moveLiquidity should remove all LP from the from index and set deposit time to zero. To index should receive increased LP and deposit time should match positionManager's deposit time. PositionManager should have less then or equal to the same quoteToken after move. +- **PM7**: memorializePosition should transfer all pool contract LP in the provided index to the positionManager. PositionManager should take on the greater of the lender of positionManager's deposit time. PositionManager's LP in the provided index should match what the lender had in the pool contract before memorializePosition was called. +- **PM8**: reedemPositions should maintain the preivous owner of the NFT position and pool associated with position. Remove the passed in indexes from the positionIndexes associated token. LP of redeemer should increase with same amount that LP of PositionManager decrease (by checking the pool). tokenId associated with the indexes redeemed should be zero. + + diff --git a/tests/forge/invariants/PositionsAndRewards/PositionsInvariants.t.sol b/tests/forge/invariants/PositionsAndRewards/PositionsInvariants.t.sol index 314ed7eb7..903ce97fb 100644 --- a/tests/forge/invariants/PositionsAndRewards/PositionsInvariants.t.sol +++ b/tests/forge/invariants/PositionsAndRewards/PositionsInvariants.t.sol @@ -12,11 +12,12 @@ import { ERC721PoolFactory } from 'src/ERC721PoolFactory.sol'; import { PositionManager } from 'src/PositionManager.sol'; import { Maths } from 'src/libraries/internal/Maths.sol'; -import { IBaseHandler } from '../interfaces/IBaseHandler.sol'; -import { BaseInvariants } from '../base/BaseInvariants.sol'; -import { ReserveERC20PoolInvariants } from '../ERC20Pool/ReserveERC20PoolInvariants.t.sol'; -import { ReserveERC20PoolHandler } from '../ERC20Pool/handlers/ReserveERC20PoolHandler.sol'; -import { TokenWithNDecimals } from '../../utils/Tokens.sol'; +import { IBaseHandler } from '../interfaces/IBaseHandler.sol'; +import { IPositionsAndRewardsHandler } from '../interfaces/IPositionsAndRewardsHandler.sol'; +import { BaseInvariants } from '../base/BaseInvariants.sol'; +import { ReserveERC20PoolInvariants } from '../ERC20Pool/ReserveERC20PoolInvariants.t.sol'; +import { ReserveERC20PoolHandler } from '../ERC20Pool/handlers/ReserveERC20PoolHandler.sol'; +import { TokenWithNDecimals } from '../../utils/Tokens.sol'; import { PositionHandler } from './handlers/PositionHandler.sol'; @@ -43,7 +44,7 @@ contract PositionsInvariants is BaseInvariants { _erc721impl = _erc721poolFactory.implementation(); _erc20pool = ERC20Pool(_erc20poolFactory.deployPool(address(_collateral), address(_quote), 0.05 * 10**18)); _pool = Pool(address(_erc20pool)); - _position = new PositionManager(_erc20poolFactory, _erc721poolFactory); + _position = new PositionManager(_erc20poolFactory, _erc721poolFactory); excludeContract(address(_ajna)); excludeContract(address(_collateral)); @@ -71,7 +72,7 @@ contract PositionsInvariants is BaseInvariants { } function invariant_positions_PM1_PM2_PM3() public useCurrentTimestamp { - uint256[] memory bucketIndexes = IBaseHandler(_handler).getBucketIndexesWithPosition(); + uint256[] memory bucketIndexes = IPositionsAndRewardsHandler(_handler).getBucketIndexesWithPosition(); // loop over bucket indexes with positions for (uint256 i = 0; i < bucketIndexes.length; i++) { @@ -84,7 +85,7 @@ contract PositionsInvariants is BaseInvariants { poolLpAccum += poolLp; // loop over tokenIds in bucket indexes - uint256[] memory tokenIds = IBaseHandler(_handler).getTokenIdsByBucketIndex(bucketIndex); + uint256[] memory tokenIds = IPositionsAndRewardsHandler(_handler).getTokenIdsByBucketIndex(bucketIndex); for (uint256 k = 0; k < tokenIds.length; k++) { uint256 tokenId = tokenIds[k]; diff --git a/tests/forge/invariants/PositionsAndRewards/RewardsInvariants.t.sol b/tests/forge/invariants/PositionsAndRewards/RewardsInvariants.t.sol index d68d3a645..4488121e0 100644 --- a/tests/forge/invariants/PositionsAndRewards/RewardsInvariants.t.sol +++ b/tests/forge/invariants/PositionsAndRewards/RewardsInvariants.t.sol @@ -7,9 +7,10 @@ import { Maths } from 'src/libraries/internal/Maths.sol'; import { RewardsManager } from 'src/RewardsManager.sol'; import { _getEpochInfo } from 'src/RewardsManager.sol'; -import { IBaseHandler } from '../interfaces/IBaseHandler.sol'; -import { RewardsHandler } from './handlers/RewardsHandler.sol'; -import { PositionsInvariants } from './PositionsInvariants.t.sol'; +import { IBaseHandler } from '../interfaces/IBaseHandler.sol'; +import { IPositionsAndRewardsHandler } from '../interfaces/IPositionsAndRewardsHandler.sol'; +import { RewardsHandler } from './handlers/RewardsHandler.sol'; +import { PositionsInvariants } from './PositionsInvariants.t.sol'; contract RewardsInvariants is PositionsInvariants { @@ -42,39 +43,25 @@ contract RewardsInvariants is PositionsInvariants { _handler = address(_rewardsHandler); } - - function invariant_rewards_RW1() public useCurrentTimestamp { - + + function invariant_rewards_RW1_RW2() public useCurrentTimestamp { + // get current epoch (is incremented every kickReserve() call) uint256 curEpoch = _pool.currentBurnEpoch(); // get rewards that have been claimed - uint256 claimedRewards = IBaseHandler(_handler).totalRewardPerEpoch(curEpoch); + uint256 claimedRewards = IPositionsAndRewardsHandler(_handler).totalRewardPerEpoch(curEpoch); // total ajna burned by the pool over the epoch (, uint256 totalBurnedInPeriod,) = _getEpochInfo(address(_pool), curEpoch); // stake rewards cap is 80% of total burned uint256 stakeRewardsCap = Maths.wmul(totalBurnedInPeriod, 0.8 * 1e18); - // check claimed rewards < rewards cap if (stakeRewardsCap != 0) require(claimedRewards < stakeRewardsCap, "Rewards invariant RW1"); - } - - function invariant_rewards_RW2() public useCurrentTimestamp { - - // get current epoch (is incremented every kickReserve() call) - uint256 curEpoch = _pool.currentBurnEpoch(); - - // get rewards that have been claimed - uint256 claimedRewards = IBaseHandler(_handler).totalRewardPerEpoch(curEpoch); - - // total ajna burned by the pool over the epoch - (, uint256 totalBurnedInPeriod,) = _getEpochInfo(address(_pool), curEpoch); // update rewards cap is 10% of total burned uint256 updateRewardsCap = Maths.wmul(totalBurnedInPeriod, 0.1 * 1e18); - // check claimed rewards < rewards cap if (updateRewardsCap != 0) require(claimedRewards < updateRewardsCap, "Rewards invariant RW2"); } diff --git a/tests/forge/invariants/PositionsAndRewards/handlers/PositionHandlerAbstract.sol b/tests/forge/invariants/PositionsAndRewards/handlers/PositionHandlerAbstract.sol index 8e6846107..14e61f1e9 100644 --- a/tests/forge/invariants/PositionsAndRewards/handlers/PositionHandlerAbstract.sol +++ b/tests/forge/invariants/PositionsAndRewards/handlers/PositionHandlerAbstract.sol @@ -14,7 +14,7 @@ import { PositionManager } from 'src/PositionManager.sol'; import { ERC20Pool } from 'src/ERC20Pool.sol'; import { UnboundedPositionsHandler } from './unbounded/UnboundedPositionsHandler.sol'; -import { BaseERC20PoolHandler } from '../../ERC20Pool/handlers/unbounded/BaseERC20PoolHandler.sol'; +import { BaseERC20PoolHandler } from '../../ERC20Pool/handlers/unbounded/BaseERC20PoolHandler.sol'; abstract contract PositionHandlerAbstract is UnboundedPositionsHandler { @@ -169,12 +169,12 @@ abstract contract PositionHandlerAbstract is UnboundedPositionsHandler { uint256 fromIndex_, uint256 toIndex_ ) internal returns (uint256 tokenId_, uint256 boundedFromIndex_, uint256 boundedToIndex_) { - boundedFromIndex_ = constrictToRange(fromIndex_, LENDER_MIN_BUCKET_INDEX, LENDER_MAX_BUCKET_INDEX); - boundedToIndex_ = constrictToRange(toIndex_, LENDER_MIN_BUCKET_INDEX, LENDER_MAX_BUCKET_INDEX); + boundedFromIndex_ = constrictToRange(fromIndex_, LENDER_MIN_BUCKET_INDEX, LENDER_MAX_BUCKET_INDEX); + boundedToIndex_ = constrictToRange(toIndex_, LENDER_MIN_BUCKET_INDEX, LENDER_MAX_BUCKET_INDEX); uint256[] memory indexes; (tokenId_, indexes) = _getNFTPosition(boundedFromIndex_, amountToMove_); - boundedFromIndex_ = indexes[0]; + boundedFromIndex_ = indexes[0]; } diff --git a/tests/forge/invariants/PositionsAndRewards/handlers/RewardsHandler.sol b/tests/forge/invariants/PositionsAndRewards/handlers/RewardsHandler.sol index df98c6488..b81f5634b 100644 --- a/tests/forge/invariants/PositionsAndRewards/handlers/RewardsHandler.sol +++ b/tests/forge/invariants/PositionsAndRewards/handlers/RewardsHandler.sol @@ -67,10 +67,6 @@ contract RewardsHandler is UnboundedRewardsHandler, PositionHandlerAbstract, Res // Action phase _unstake(tokenId); - - // Post action - // check token was transferred from rewards contract to actor - assertEq(_position.ownerOf(tokenId), _actor); } function updateExchangeRate( diff --git a/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/BasePositionsHandler.sol b/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/BasePositionsHandler.sol index 575c63ff9..bdc1d1647 100644 --- a/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/BasePositionsHandler.sol +++ b/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/BasePositionsHandler.sol @@ -10,7 +10,7 @@ import { IPositionManagerOwnerActions } from 'src/interfaces/position/IPositionM import { _depositFeeRate } from 'src/libraries/helpers/PoolHelper.sol'; import { Maths } from "src/libraries/internal/Maths.sol"; -import { BaseERC20PoolHandler } from '../../../ERC20Pool/handlers/unbounded/BaseERC20PoolHandler.sol'; +import { BaseERC20PoolHandler } from '../../../ERC20Pool/handlers/unbounded/BaseERC20PoolHandler.sol'; import { PositionManager } from 'src/PositionManager.sol'; import { RewardsManager } from 'src/RewardsManager.sol'; @@ -34,19 +34,18 @@ abstract contract BasePositionsHandler is BaseERC20PoolHandler { mapping(uint256 => EnumerableSet.UintSet) internal tokenIdsByBucketIndex; EnumerableSet.UintSet internal bucketIndexesWithPosition; - // used for removing all CT and QT to reset exchange rate - mapping(address => EnumerableSet.UintSet) internal tokenIdsByActor; + // used for removing all CT and QT to reset bucket exchange rate mapping(uint256 => address) internal actorByTokenId; + mapping(address => EnumerableSet.UintSet) internal tokenIdsByActor; mapping(uint256 => EnumerableSet.UintSet) internal bucketIndexesByTokenId; - // used to track LP changes in `` + // used to track LP changes in `_redeemPositions()` and `_memorializePositions()` mapping(uint256 => uint256) internal bucketIndexToActorPositionManLps; mapping(uint256 => uint256) internal bucketIndexToPositionManPoolLps; mapping(uint256 => uint256) internal bucketIndexToActorPoolLps; mapping(uint256 => uint256) internal bucketIndexToDepositTime; // Rewards invariant test state // - EnumerableSet.UintSet internal stakedTokenIds; mapping(uint256 => uint256) public totalRewardPerEpoch; // total rewards per epoch uint256 public totalStakerRewPerEpoch; // amount of reserve decrease uint256 public totalUpdaterRewPerEpoch; // amount of reserve increase diff --git a/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/UnboundedPositionsHandler.sol b/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/UnboundedPositionsHandler.sol index 694b9a9f2..56f7aa75a 100644 --- a/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/UnboundedPositionsHandler.sol +++ b/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/UnboundedPositionsHandler.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.18; +import '../../../../utils/DSTestPlus.sol'; import '@openzeppelin/contracts/utils/structs/EnumerableSet.sol'; import { IPositionManagerOwnerActions } from 'src/interfaces/position/IPositionManagerOwnerActions.sol'; @@ -12,8 +13,6 @@ import { } from 'src/libraries/helpers/PoolHelper.sol'; import { Maths } from "src/libraries/internal/Maths.sol"; -import '@std/console.sol'; - import { BaseERC20PoolHandler } from '../../../ERC20Pool/handlers/unbounded/BaseERC20PoolHandler.sol'; import { BasePositionsHandler } from './BasePositionsHandler.sol'; @@ -46,7 +45,7 @@ abstract contract UnboundedPositionsHandler is BasePositionsHandler { // assert that the underlying LP balance in PositionManager is 0 (uint256 posPreActionLps,) = _position.getPositionInfo(tokenId_, indexes_[i]); - require(posPreActionLps == 0); + require(posPreActionLps == 0, "tokenID already has lps associated on memorialize"); } @@ -72,12 +71,16 @@ abstract contract UnboundedPositionsHandler is BasePositionsHandler { // assert that the LP that now exists in the pool contract matches the amount added by the actor (uint256 poolLps, uint256 poolDepositTime) = _pool.lenderInfo(bucketIndex, address(_position)); - require(poolLps == bucketIndexToActorPoolLps[bucketIndex] + bucketIndexToPositionManPoolLps[bucketIndex]); - require(poolDepositTime == bucketIndexToDepositTime[bucketIndex]); + require(poolLps == bucketIndexToActorPoolLps[bucketIndex] + bucketIndexToPositionManPoolLps[bucketIndex], + "PM7: pool contract lps do not match amount added by actor"); + + require(poolDepositTime == bucketIndexToDepositTime[bucketIndex], + "PM7: positionManager depositTime does not match most recent depositTime"); - // assert that the underlying LP balance in PositionManager has increased + // assert that the positionManager LP balance of the actor has increased (uint256 posLps,) = _position.getPositionInfo(tokenId_, bucketIndex); - require(posLps == bucketIndexToActorPoolLps[bucketIndex]); + require(posLps == bucketIndexToActorPoolLps[bucketIndex], + "PM7: positionManager lps do not match amount added by actor"); delete bucketIndexToActorPoolLps[bucketIndex]; delete bucketIndexToPositionManPoolLps[bucketIndex]; @@ -97,15 +100,15 @@ abstract contract UnboundedPositionsHandler is BasePositionsHandler { // Post Action Checks // // assert that the minter is the owner - require(_position.ownerOf(tokenId) == _actor); + require(_position.ownerOf(tokenId) == _actor, "PM4: minter is not owner"); // assert that poolKey is returns correct pool address address poolAddress = _position.poolKey(tokenId); - require(poolAddress == address(_pool)); + require(poolAddress == address(_pool), "PM4: poolKey does not match pool address"); // assert that no positions are associated with this tokenId uint256[] memory posIndexes = _position.getPositionIndexes(tokenId); - require(posIndexes.length == 0); + require(posIndexes.length == 0, "PM4: positions are associated with tokenId"); } catch (bytes memory err) { _ensurePoolError(err); @@ -131,7 +134,7 @@ abstract contract UnboundedPositionsHandler is BasePositionsHandler { // assert that the underlying LP balance in PositionManager is greater than 0 (uint256 posPreActionLps,) = _position.getPositionInfo(tokenId_, indexes_[i]); - require(posPreActionLps > 0); + require(posPreActionLps > 0, "tokenID does not have lps associated on redemption"); } try _position.redeemPositions(address(_pool), tokenId_, indexes_) { @@ -148,15 +151,16 @@ abstract contract UnboundedPositionsHandler is BasePositionsHandler { // Post action Checks // // assert that the minter is still the owner - require(_position.ownerOf(tokenId_) == preActionOwner, 'owner is no longer minter on redemption'); + require(_position.ownerOf(tokenId_) == preActionOwner, + 'PM8: previous owner is no longer owner on redemption'); // assert that poolKey is still same address poolAddress = _position.poolKey(tokenId_); - require(poolAddress == address(_pool), 'poolKey has changed on redemption'); + require(poolAddress == address(_pool), 'PM8: poolKey has changed on redemption'); // assert that no positions are associated with this tokenId uint256[] memory posIndexes = _position.getPositionIndexes(tokenId_); - require(posIndexes.length == 0, 'positions still exist after redemption'); + require(posIndexes.length == 0, 'PM8: positions still exist after redemption'); for(uint256 i=0; i < indexes_.length; i++) { uint256 bucketIndex = indexes_[i]; @@ -165,20 +169,22 @@ abstract contract UnboundedPositionsHandler is BasePositionsHandler { uint256 positionManPoolLps = bucketIndexToPositionManPoolLps[bucketIndex]; (uint256 poolActorLps,) = _pool.lenderInfo(bucketIndex, preActionOwner); - (uint256 poolPosLps,) = _pool.lenderInfo(bucketIndex, address(_position)); + (uint256 poolPosLps,) = _pool.lenderInfo(bucketIndex, address(_position)); // assert PositionsMan LP in pool matches the amount redeemed by actor // positionMan has now == positionMan pre - actor's LP change - require(poolPosLps == positionManPoolLps - (poolActorLps - actorPoolLps)); + require(poolPosLps == positionManPoolLps - (poolActorLps - actorPoolLps), + "PM8: positionManager's pool contract lps do not match amount redeemed by actor"); // assert actor LP in pool matches amount removed from the posMan's position // assert actor LP in pool = what actor LP had pre + what LP positionManager redeemed to actor - require(poolActorLps == actorPoolLps + (positionManPoolLps - poolPosLps)); + require(poolActorLps == actorPoolLps + (positionManPoolLps - poolPosLps), + "PM8: actor's pool contract lps do not match amount redeemed by actor"); // assert that the underlying LP balance in PositionManager is zero (uint256 posLps, uint256 posDepositTime) = _position.getPositionInfo(tokenId_, bucketIndex); - require(posLps == 0); - require(posDepositTime == 0); + require(posLps == 0, "PM8: tokenId has lps after redemption"); + require(posDepositTime == 0, "PM8: tokenId has depositTime after redemption"); // delete mappings for reuse delete bucketIndexToActorPoolLps[bucketIndex]; @@ -250,24 +256,30 @@ abstract contract UnboundedPositionsHandler is BasePositionsHandler { bucketIndexesWithPosition.add(toIndex_); tokenIdsByBucketIndex[toIndex_].add(tokenId_); - // assert that underlying LP balance in PositionManager of fromIndex is less than or equal to preAction and deposit time in PositionManager is 0 + // assert that fromIndex LP and deposit time are both zero (uint256 fromLps, uint256 fromDepositTime) = _position.getPositionInfo(tokenId_, fromIndex_); - require(fromLps == 0); // difficult to estimate LPS, assert that it is less than - require(fromDepositTime == 0); + require(fromLps == 0, "PM6: from bucket still has LPs after move"); + require(fromDepositTime == 0, "PM6: from bucket still has deposit time after move"); - // assert that underlying LP balance in PositionManager of toIndex is increased and deposit time in PositionManager is updated + // assert that toIndex LP is increased and deposit time matches positionManagers depositTime pre action (uint256 toLps, uint256 toDepositTime) = _position.getPositionInfo(tokenId_, toIndex_); - require(toLps >= preActionToLps); // difficult to estimate LPS, assert that it is greater than (,uint256 postActionDepositTime)= _pool.lenderInfo(toIndex_, address(_position)); - require(toDepositTime == postActionDepositTime); + require(toLps >= preActionToLps, "PM6: to bucket lps have not increased"); // difficult to estimate LPS, assert that it is greater than + require(toDepositTime == postActionDepositTime, "PM6: to bucket deposit time does not match positionManager"); // get post action QT represented in positionManager for tokenID uint256 postActionFromIndexQuote = _getQuoteAtIndex(fromLps, fromIndex_); uint256 postActionToIndexQuote = _getQuoteAtIndex(toLps, toIndex_); - // assert total QT represented in positionManager for tokenID postAction is the same as preAction + // positionManager's total QT postAction is less than or equal to preAction // can be less than or equal due to fee on movements above -> below LUP - assert (preActionFromIndexQuote + preActionToIndexQuote >= postActionFromIndexQuote + postActionToIndexQuote); + greaterThanWithinDiff( + preActionFromIndexQuote + preActionToIndexQuote, + postActionFromIndexQuote + postActionToIndexQuote, + 1, + "PM6: positiionManager QT balance has increased by `1` margin" + ); + } catch (bytes memory err) { _ensurePoolError(err); @@ -283,15 +295,15 @@ abstract contract UnboundedPositionsHandler is BasePositionsHandler { // Post Action Checks // // should revert if token id is burned vm.expectRevert("ERC721: invalid token ID"); - _position.ownerOf(tokenId_); + require(_position.ownerOf(tokenId_) == address(0), "PM5: ownership is not zero address"); // assert that poolKey is returns zero address address poolAddress = _position.poolKey(tokenId_); - require(poolAddress == address(0)); + require(poolAddress == address(0), "PM5: poolKey has not been reset on burn"); // assert that no positions are associated with this tokenId uint256[] memory posIndexes = _position.getPositionIndexes(tokenId_); - require(posIndexes.length == 0); + require(posIndexes.length == 0, "PM5: positions still exist after burn"); } catch (bytes memory err) { _ensurePoolError(err); } diff --git a/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/UnboundedRewardsHandler.sol b/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/UnboundedRewardsHandler.sol index c86a6a7e2..bcea24b1b 100644 --- a/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/UnboundedRewardsHandler.sol +++ b/tests/forge/invariants/PositionsAndRewards/handlers/unbounded/UnboundedRewardsHandler.sol @@ -47,6 +47,9 @@ abstract contract UnboundedRewardsHandler is BasePositionsHandler { try _rewards.unstake(tokenId_) { + // check token was transferred from rewards contract to actor + require(_position.ownerOf(tokenId_) == _actor, "actor should receive ownership after unstaking"); + // actor should receive tokenId, positionManager loses ownership tokenIdsByActor[address(_actor)].add(tokenId_); tokenIdsByActor[address(_rewards)].remove(tokenId_); @@ -57,8 +60,6 @@ abstract contract UnboundedRewardsHandler is BasePositionsHandler { totalRewardPerEpoch[lastClaimedEpoch] += _quote.balanceOf(_actor) - actorBalanceBeforeClaim; } - stakedTokenIds.remove(tokenId_); - } catch (bytes memory err) { _ensurePoolError(err); } diff --git a/tests/forge/invariants/base/BaseInvariants.sol b/tests/forge/invariants/base/BaseInvariants.sol index 598453581..e5c0069df 100644 --- a/tests/forge/invariants/base/BaseInvariants.sol +++ b/tests/forge/invariants/base/BaseInvariants.sol @@ -63,20 +63,4 @@ abstract contract BaseInvariants is Test { function setCurrentTimestamp(uint256 currentTimestamp_) external { currentTimestamp = currentTimestamp_; } - - /************************/ - /*** Helper Functions ***/ - /************************/ - - function getDiff(uint256 x, uint256 y) internal pure returns (uint256 diff) { - diff = x > y ? x - y : y - x; - } - - function requireWithinDiff(uint256 x, uint256 y, uint256 expectedDiff, string memory err) internal pure { - require(getDiff(x, y) <= expectedDiff, err); - } - - function greaterThanWithinDiff(uint256 x, uint256 y, uint256 expectedDiff, string memory err) internal pure { - require(x > y || getDiff(x, y) <= expectedDiff, err); - } } \ No newline at end of file diff --git a/tests/forge/invariants/base/BasicInvariants.t.sol b/tests/forge/invariants/base/BasicInvariants.t.sol index 364972e1e..956d679e5 100644 --- a/tests/forge/invariants/base/BasicInvariants.t.sol +++ b/tests/forge/invariants/base/BasicInvariants.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.18; import "@std/console.sol"; +import '../../utils/DSTestPlus.sol'; import { IERC20Pool } from 'src/interfaces/pool/erc20/IERC20Pool.sol'; import { Maths } from 'src/libraries/internal/Maths.sol'; diff --git a/tests/forge/invariants/base/LiquidationInvariants.t.sol b/tests/forge/invariants/base/LiquidationInvariants.t.sol index 7fe6dff79..9ce277173 100644 --- a/tests/forge/invariants/base/LiquidationInvariants.t.sol +++ b/tests/forge/invariants/base/LiquidationInvariants.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.18; import "@std/console.sol"; +import '../../utils/DSTestPlus.sol'; import { IBaseHandler } from '../interfaces/IBaseHandler.sol'; import { BasicInvariants } from './BasicInvariants.t.sol'; diff --git a/tests/forge/invariants/base/ReserveInvariants.t.sol b/tests/forge/invariants/base/ReserveInvariants.t.sol index 23de328dd..e5c349566 100644 --- a/tests/forge/invariants/base/ReserveInvariants.t.sol +++ b/tests/forge/invariants/base/ReserveInvariants.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.18; import "@std/console.sol"; +import '../../utils/DSTestPlus.sol'; import { Maths } from "src/libraries/internal/Maths.sol"; diff --git a/tests/forge/invariants/interfaces/IBaseHandler.sol b/tests/forge/invariants/interfaces/IBaseHandler.sol index 09b15dfe1..4cd2f6d4e 100644 --- a/tests/forge/invariants/interfaces/IBaseHandler.sol +++ b/tests/forge/invariants/interfaces/IBaseHandler.sol @@ -27,13 +27,6 @@ interface IBaseHandler { function previousReserves() external view returns(uint256); function increaseInReserves() external view returns(uint256); function decreaseInReserves() external view returns(uint256); - - function totalRewardPerEpoch(uint256) external view returns(uint256); - - function getBucketIndexesWithPosition() external view returns(uint256[] memory); - function getTokenIdsByBucketIndex(uint256) external view returns(uint256[] memory); - function getBucketIndexesByTokenId(uint256) external view returns(uint256[] memory); - function getTokenIdsByActor() external view returns(uint256[] memory); function previousTotalBonds() external view returns(uint256); function increaseInBonds() external view returns(uint256); diff --git a/tests/forge/invariants/interfaces/IPositionsAndRewardsHandler.sol b/tests/forge/invariants/interfaces/IPositionsAndRewardsHandler.sol new file mode 100644 index 000000000..44980f0ed --- /dev/null +++ b/tests/forge/invariants/interfaces/IPositionsAndRewardsHandler.sol @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: UNLICENSED + +pragma solidity 0.8.18; + +interface IPositionsAndRewardsHandler { + + function totalRewardPerEpoch(uint256) external view returns(uint256); + + function getBucketIndexesWithPosition() external view returns(uint256[] memory); + function getTokenIdsByBucketIndex(uint256) external view returns(uint256[] memory); + function getBucketIndexesByTokenId(uint256) external view returns(uint256[] memory); + function getTokenIdsByActor() external view returns(uint256[] memory); +} \ No newline at end of file diff --git a/tests/forge/regression/PositionAndRewards/RegressionRewardsManager.t.sol b/tests/forge/regression/PositionAndRewards/RegressionRewardsManager.t.sol index ac3d4ffd6..7f3f0a241 100644 --- a/tests/forge/regression/PositionAndRewards/RegressionRewardsManager.t.sol +++ b/tests/forge/regression/PositionAndRewards/RegressionRewardsManager.t.sol @@ -53,7 +53,7 @@ contract RegressionRewardsManager is RewardsInvariants { // Invariant was failing when rewards cap is equal to zero // Fixed by updating invariants to run only when rewards cap is non zero function test_regression_rewards_RW1() public { - invariant_rewards_RW1(); + invariant_rewards_RW1_RW2(); } // Test was failing due to unbounded debt drawn in `_preUnstake` @@ -279,4 +279,11 @@ contract RegressionRewardsManager is RewardsInvariants { _rewardsHandler.bucketTake(2, 15470539950385543111949808932971047871463497008525518386, false, 115792089237316195423570985008687907853269984665640564039457584007913129639932, 1); _rewardsHandler.redeemPositions(115792089237316195423570985008687907853269984665640564039457584007913129639933, 2652132885321220255, 2, 1557926034); } + + function test_regression_PM6_failure() external { + _rewardsHandler.bucketTake(29456557203126366201854827466482433206831494327361303, 19307664601998129837361, false, 3492651658979151995106448, 0); + _rewardsHandler.kickWithDeposit(4429580015302257459201655018526, 2770867242698718418626, 9388); + _rewardsHandler.repayDebt(1000000034925771973, 6930625368245303852701363167, 695149882294170920069268290133705109872933519679164510383901578196897792); + _rewardsHandler.moveLiquidity(41132919728951221583605488, 1102564553356549573347, 3410238307441803358653636, 52534, 4545656474572434187813557497); + } } diff --git a/tests/forge/utils/DSTestPlus.sol b/tests/forge/utils/DSTestPlus.sol index 564f5de82..d97415854 100644 --- a/tests/forge/utils/DSTestPlus.sol +++ b/tests/forge/utils/DSTestPlus.sol @@ -1488,5 +1488,18 @@ abstract contract DSTestPlus is Test, IPoolEvents { // calculate the fee rate based upon the interest rate feeRate_ = _borrowFeeRate(newInterestRate) + Maths.WAD; } - } + + function getDiff(uint256 x, uint256 y) pure returns (uint256 diff) { + diff = x > y ? x - y : y - x; + } + + function requireWithinDiff(uint256 x, uint256 y, uint256 expectedDiff, string memory err) pure { + require(getDiff(x, y) <= expectedDiff, err); + } + + function greaterThanWithinDiff(uint256 x, uint256 y, uint256 expectedDiff, string memory err) pure { + require(x > y || getDiff(x, y) <= expectedDiff, err); + } + +