Merge Develop into Master (#934)

* Logging for RewardsManager and PositionManager invariants (#925)

* added logging for positionManager

* added rewards and position logging

* clean up

* updated rewards mapping back to public so tests pass

* how modifier was being called in rewardsPoolHandler

* revised so logging pools is not required when logging positions

* cleanup

* readme cleanup

---------

Co-authored-by: Ian Harvey <iharvey@comcast.net>

* Invariants Improvement: Add multiple pool support in position and rewards manager invariant testing (#927)

* Add multiple pools in position and rewards manager invariant testing

* Fix RW6 regression test

* Fix rewardsClaimed and updateRewardsClaimed in Rewards manager

* Fix compile error

* PR feedback

* Add configurable number of pools for position and rewards manager invariant testing

* Positions Invariants: Multiple positions, transfer positions (#926)

* added randomness

* added the ability to transfer positions

* increased chance of rewards being claimed in handlers

* cleanup

* responded to comments

---------

Co-authored-by: Ian Harvey <iharvey@comcast.net>

* Fuzz test additions (#924)

* Add fuzz test for borrower borrows fuzzed amount and getting kick after some time

* Add fuzz test for take fuzzed amount of collateral from auction

* Add fuzz test for settle with fuzzed pool deposit

* Add fuzz test for add and remove collateral in ERC721Pool

* Fuzzed buckets used in borrow and kick fuzz test

* PR feedback

* Invariant Improvements: Position rewards logging for multiple pools (#931)

* Update position and rewards manager invariant logging for multiple pools

* Fix regression test to run for any token precision and Quote token limits

* PR feedback

* Invariants Improvements: Add Multiple position in single handler in Position (#928)

* Update Position invariants handler to memorialize and redeem multiple positions

* PR feedback

* Add partial random positions redeem in redeem position handler

* Add random time skips between epochs in rewards manager

* Add bucket bankruptcy scenario for rewards manager (#930)

* Add bucket bankruptcy scenario for rewards manager

* Fix evm reverts

* PR feedback

* Update prepare test methods to add position in NFT if there is no position in it

---------

Co-authored-by: Ian Harvey <iharvey@comcast.net>
Co-authored-by: Prateek Gupta <prateek105@users.noreply.github.com>

Author: 3 people

src/RewardsManager.sol

@@ -15,7 +15,11 @@ import {
     IRewardsManagerState,
     IRewardsManagerDerivedState
 } from './interfaces/rewards/IRewardsManager.sol';
-import { StakeInfo, BucketState } from './interfaces/rewards/IRewardsManagerState.sol';
+import { 
+    StakeInfo,
+    BucketState,
+    PoolRewardsInfo
+} from './interfaces/rewards/IRewardsManagerState.sol';
 
 import { PositionManager } from './PositionManager.sol';
 
@@ -67,13 +71,9 @@ contract RewardsManager is IRewardsManager {
 
     /// @dev `tokenID => epoch => bool has claimed` mapping.
     mapping(uint256 => mapping(uint256 => bool)) public override isEpochClaimed;
-    /// @dev `epoch => rewards claimed` mapping.
-    mapping(uint256 => uint256) public override rewardsClaimed;
-    /// @dev `epoch => update bucket rate rewards claimed` mapping. Tracks the total amount of update rewards claimed.
-    mapping(uint256 => uint256) public override updateRewardsClaimed;
 
-    /// @dev Mapping of per pool bucket exchange rates at a given burn event `poolAddress => bucketIndex => epoch => bucket exchange rate`.
-    mapping(address => mapping(uint256 => mapping(uint256 => uint256))) internal bucketExchangeRates;
+    /// @dev Mapping `pool address => Pool rewards info`.
+    mapping(address => PoolRewardsInfo) internal poolRewardsInfo;
 
     /// @dev Mapping `tokenID => Stake info`.
     mapping(uint256 => StakeInfo) internal stakes;
@@ -318,7 +318,23 @@ contract RewardsManager is IRewardsManager {
         uint256 bucketIndex_,
         uint256 epoch_
     ) external view override returns (bool) {
-        return bucketExchangeRates[pool_][bucketIndex_][epoch_] != 0;
+        return poolRewardsInfo[pool_].bucketExchangeRates[bucketIndex_][epoch_] != 0;
+    }
+
+    /// @inheritdoc IRewardsManagerState
+    function getRewardsClaimed(
+        address pool_,
+        uint256 epoch_
+    ) external view override returns (uint256) {
+        return poolRewardsInfo[pool_].rewardsClaimed[epoch_];
+    }
+
+    /// @inheritdoc IRewardsManagerState
+    function getUpdateRewardsClaimed(
+        address pool_,
+        uint256 epoch_
+    ) external view override returns (uint256) {
+        return poolRewardsInfo[pool_].updateBucketRewardsClaimed[epoch_];
     }
 
     /**************************/
@@ -340,6 +356,8 @@ contract RewardsManager is IRewardsManager {
         uint256 lastClaimedEpoch = stakes[tokenId_].lastClaimedEpoch;
         uint256 stakingEpoch     = stakes[tokenId_].stakingEpoch;
 
+        mapping(uint256 => uint256) storage rewardsClaimed = poolRewardsInfo[ajnaPool].rewardsClaimed;
+
         uint256[] memory positionIndexes = positionManager.getPositionIndexesFiltered(tokenId_);
 
         // iterate through all burn periods to calculate and claim rewards
@@ -358,7 +376,7 @@ contract RewardsManager is IRewardsManager {
             unchecked { ++epoch; }
 
             // update epoch token claim trackers
-            rewardsClaimed[epoch]           += nextEpochRewards;
+            rewardsClaimed[epoch] += nextEpochRewards;
             isEpochClaimed[tokenId_][epoch] = true;
         }
     }
@@ -382,7 +400,7 @@ contract RewardsManager is IRewardsManager {
     ) internal view returns (uint256 epochRewards_) {
 
         uint256 nextEpoch = epoch_ + 1;
-        uint256 claimedRewardsInNextEpoch = rewardsClaimed[nextEpoch];
+        uint256 claimedRewardsInNextEpoch = poolRewardsInfo[ajnaPool_].rewardsClaimed[nextEpoch];
         uint256 bucketIndex;
         uint256 interestEarned;
 
@@ -397,7 +415,7 @@ contract RewardsManager is IRewardsManager {
             if (epoch_ != stakingEpoch_) {
 
                 // if staked in a previous epoch then use the initial exchange rate of epoch
-                bucketRate = bucketExchangeRates[ajnaPool_][bucketIndex][epoch_];
+                bucketRate = poolRewardsInfo[ajnaPool_].bucketExchangeRates[bucketIndex][epoch_];
             } else {
 
                 // if staked during the epoch then use the bucket rate at the time of staking
@@ -445,7 +463,7 @@ contract RewardsManager is IRewardsManager {
 
         if (exchangeRate_ != 0) {
 
-            uint256 nextExchangeRate = bucketExchangeRates[pool_][bucketIndex_][nextEventEpoch_];
+            uint256 nextExchangeRate = poolRewardsInfo[pool_].bucketExchangeRates[bucketIndex_][nextEventEpoch_];
 
             // calculate interest earned only if next exchange rate is higher than current exchange rate
             if (nextExchangeRate > exchangeRate_) {
@@ -609,6 +627,7 @@ contract RewardsManager is IRewardsManager {
         }
         else {
             if (block.timestamp <= curBurnTime + UPDATE_PERIOD) {
+                mapping(uint256 => uint256) storage updateRewardsClaimed = poolRewardsInfo[pool_].updateBucketRewardsClaimed;
 
                 // update exchange rates and calculate rewards if tokens were burned and within allowed time period
                 uint256 noOfIndexes = indexes_.length;
@@ -657,7 +676,7 @@ contract RewardsManager is IRewardsManager {
         uint256 burnEpoch_
     ) internal {
         // cache storage pointer for reduced gas
-        mapping(uint256 => uint256) storage _bucketExchangeRates = bucketExchangeRates[pool_][bucketIndex_];
+        mapping(uint256 => uint256) storage _bucketExchangeRates = poolRewardsInfo[pool_].bucketExchangeRates[bucketIndex_];
         uint256 burnExchangeRate = _bucketExchangeRates[burnEpoch_];
 
         // update bucket exchange rate at epoch only if it wasn't previously updated
@@ -686,7 +705,7 @@ contract RewardsManager is IRewardsManager {
         uint256 interestEarned_
     ) internal returns (uint256 rewards_) {
         // cache storage pointer for reduced gas
-        mapping(uint256 => uint256) storage _bucketExchangeRates = bucketExchangeRates[pool_][bucketIndex_];
+        mapping(uint256 => uint256) storage _bucketExchangeRates = poolRewardsInfo[pool_].bucketExchangeRates[bucketIndex_];
         uint256 burnExchangeRate = _bucketExchangeRates[burnEpoch_];
 
         // update bucket exchange rate at epoch only if it wasn't previously updated

src/interfaces/rewards/IRewardsManagerState.sol

@@ -33,19 +33,23 @@ interface IRewardsManagerState {
 
     /**
      *  @notice Track the total amount of rewards that have been claimed for a given epoch.
+     *  @param  pool_    The pool to get the rewards claimed for.
      *  @param  epoch_   The burn epoch to track if rewards were claimed.
      *  @return The amount of rewards claimed in given epoch.
      */
-    function rewardsClaimed(
+    function getRewardsClaimed(
+        address pool_,
         uint256 epoch_
     ) external view returns (uint256);
 
     /**
      *  @notice Track the total amount of rewards that have been claimed for a given burn event's bucket updates.
+     *  @param  pool_    The pool to get the update rewards claimed for.
      *  @param  epoch_   The burn epoch to track if rewards were claimed.
      *  @return The amount of update rewards claimed in given epoch.
      */
-    function updateRewardsClaimed(
+    function getUpdateRewardsClaimed(
+        address pool_,
         uint256 epoch_
     ) external view returns (uint256);
 
@@ -92,3 +96,10 @@ struct BucketState {
     uint256 lpsAtStakeTime;  // [WAD] LP amount the NFT owner is entitled in current bucket at the time of staking
     uint256 rateAtStakeTime; // [WAD] current bucket exchange rate at the time of staking
 }
+
+/// @dev Struct holding Pool rewards info
+struct PoolRewardsInfo {
+    mapping(uint256 => uint256) rewardsClaimed;                           // staking rewards claimed in a epoch
+    mapping(uint256 => uint256) updateBucketRewardsClaimed;               // update exchange rate rewards claimed in a epoch
+    mapping(uint256 => mapping(uint256 => uint256)) bucketExchangeRates;  // bucket exchange rates at a given burn event for each bucket
+}

tests/README.md

@@ -54,7 +54,7 @@ python regression_generator.py
 ### Invariant tests
 #### Configuration
 Invariant test scenarios can be externally configured by customizing following environment variables:
-| Variable | Pool Type | Default | Description |
+| Variable | Contract Type | Default | Description |
 | ------------- | ------------- | ------------- | ------------- |
 | NO_OF_ACTORS  | ERC20 ERC721 | 10 | Max number of actors to interact with the pool |
 | QUOTE_PRECISION  | ERC20 ERC721 | 18 | Precision of token used as quote token |
@@ -77,10 +77,12 @@ Invariant test scenarios can be externally configured by customizing following e
 | SKIP_TIME_TO_KICK | ERC20 ERC721 | 200 days | The time to be skipped and drive a new loan undercollateralized. Use a big value to ensure a successful kick |
 | MAX_EPOCH_ADVANCE | ERC20 ERC721 | 5 | The maximum number of epochs that will be created before an unstake or claimRewards call |
 | MAX_AJNA_AMOUNT | ERC20 ERC721 | 100_000_000 | The maximum amount of ajna provided to the rewards contract |
+| NO_OF_POOLS | Position Rewards | 10 | Number of pools to be used in position and rewards manager invariant testing |
 | FOUNDRY_INVARIANT_RUNS | ERC20 ERC721 | 10 | The number of runs for each scenario |
 | FOUNDRY_INVARIANT_DEPTH | ERC20 ERC721 | 200 | The number of actions performed in each scenario |
-| LOGS_VERBOSITY | ERC20 ERC721 | 0 | <p> Details to log <p> 0 = No Logs <p> 1 = pool State  <p> 2 = pool State, Auctions details <p> 3 = pool State, Auctions details , Buckets details <p> 4 = pool State, Auctions details , Buckets details, Lender details <p> 5 = pool State, Auctions details , Buckets details, Lender details, Borrower details <p> Note - Log File with name `logFile.txt` will be generated in project directory|
-
+| LOGS_VERBOSITY_POOL | ERC20 ERC721 | 0 | <p> Details to log <p> 0 = No Logs <p> 1 = pool State  <p> 2 = pool State, Auctions details <p> 3 = pool State, Auctions details , Buckets details <p> 4 = pool State, Auctions details , Buckets details, Lender details <p> 5 = pool State, Auctions details , Buckets details, Lender details, Borrower details <p> Note - Log File with name `logFile.txt` will be generated in project directory|
+| LOGS_VERBOSITY_POSITION | ERC20 ERC721 | 0 | <p> Details to log <p> 0 = No Logs <p> 1 = positionManager details <p> Note - Log File with name `logFile.txt` will be generated in project directory|
+| LOGS_VERBOSITY_REWARDS | ERC20 ERC721 | 0 | <p> Details to log <p> 0 = No Logs <p> 1 = rewardsManager details <p> Note - Log File with name `logFile.txt` will be generated in project directory|
 #### Invariant names
 
 The `<invariant_name>` placeholder in commands below could take following values:

tests/forge/invariants/ERC20Pool/BasicERC20PoolInvariants.t.sol

@@ -37,8 +37,6 @@ contract BasicERC20PoolInvariants is BasicInvariants {
         _basicERC20PoolHandler = new BasicERC20PoolHandler(
             address(_erc20pool),
             address(_ajna),
-            address(_quote),
-            address(_collateral),
             address(_poolInfo),
             _numOfActors,
             address(this)

tests/forge/invariants/ERC20Pool/LiquidationERC20PoolInvariants.t.sol

@@ -21,8 +21,6 @@ contract LiquidationERC20PoolInvariants is BasicERC20PoolInvariants, Liquidation
         _liquidationERC20PoolHandler = new LiquidationERC20PoolHandler(
             address(_erc20pool),
             address(_ajna),
-            address(_quote),
-            address(_collateral),
             address(_poolInfo),
             _numOfActors,
             address(this)

tests/forge/invariants/ERC20Pool/PanicExitERC20PoolInvariants.t.sol

@@ -29,8 +29,6 @@ contract PanicExitERC20PoolInvariants is BasicERC20PoolInvariants, LiquidationIn
         _panicExitERC20PoolHandler = new PanicExitERC20PoolHandler(
             address(_erc20pool),
             address(_ajna),
-            address(_quote),
-            address(_collateral),
             address(_poolInfo),
             address(this)
         );

tests/forge/invariants/ERC20Pool/RealWorldScenarioInvariants.t.sol

@@ -22,8 +22,6 @@ contract RealWorldScenarioInvariants is ReserveInvariants, LiquidationERC20PoolI
         _reserveERC20PoolHandler = new ReserveERC20PoolHandler(
             address(_erc20pool),
             address(_ajna),
-            address(_quote),
-            address(_collateral),
             address(_poolInfo),
             _numOfActors,
             address(this)

tests/forge/invariants/ERC20Pool/ReserveERC20PoolInvariants.t.sol

@@ -21,8 +21,6 @@ contract ReserveERC20PoolInvariants is ReserveInvariants, LiquidationERC20PoolIn
         _reserveERC20PoolHandler = new ReserveERC20PoolHandler(
             address(_erc20pool),
             address(_ajna),
-            address(_quote),
-            address(_collateral),
             address(_poolInfo),
             _numOfActors,
             address(this)

tests/forge/invariants/ERC20Pool/TradingERC20PoolInvariants.t.sol

@@ -29,8 +29,6 @@ contract TradingERC20PoolInvariants is BasicERC20PoolInvariants, LiquidationInva
         _tradingERC20PoolHandler = new TradingERC20PoolHandler(
             address(_erc20pool),
             address(_ajna),
-            address(_quote),
-            address(_collateral),
             address(_poolInfo),
             address(this)
         );

tests/forge/invariants/ERC20Pool/handlers/BasicERC20PoolHandler.sol

@@ -21,12 +21,10 @@ contract BasicERC20PoolHandler is UnboundedBasicERC20PoolHandler, BasicPoolHandl
     constructor(
         address pool_,
         address ajna_,
-        address quote_,
-        address collateral_,
         address poolInfo_,
         uint256 numOfActors_,
         address testContract_
-    ) BaseERC20PoolHandler(pool_, ajna_, quote_, collateral_, poolInfo_, numOfActors_, testContract_) {
+    ) BaseERC20PoolHandler(pool_, ajna_, poolInfo_, numOfActors_, testContract_) {
 
     }
 

tests/forge/invariants/ERC20Pool/handlers/LiquidationERC20PoolHandler.sol

@@ -10,12 +10,10 @@ contract LiquidationERC20PoolHandler is LiquidationPoolHandler, BasicERC20PoolHa
     constructor(
         address pool_,
         address ajna_,
-        address quote_,
-        address collateral_,
         address poolInfo_,
         uint256 numOfActors_,
         address testContract_
-    ) BasicERC20PoolHandler(pool_, ajna_, quote_, collateral_, poolInfo_, numOfActors_, testContract_) {
+    ) BasicERC20PoolHandler(pool_, ajna_, poolInfo_, numOfActors_, testContract_) {
 
     }
 

tests/forge/invariants/ERC20Pool/handlers/PanicExitERC20PoolHandler.sol

@@ -29,11 +29,9 @@ contract PanicExitERC20PoolHandler is UnboundedLiquidationPoolHandler, Unbounded
     constructor(
         address pool_,
         address ajna_,
-        address quote_,
-        address collateral_,
         address poolInfo_,
         address testContract_
-    ) BaseERC20PoolHandler(pool_, ajna_, quote_, collateral_, poolInfo_, 0, testContract_) {
+    ) BaseERC20PoolHandler(pool_, ajna_, poolInfo_, 0, testContract_) {
         numberOfBuckets = buckets.length();
         setUp();
     }

tests/forge/invariants/ERC20Pool/handlers/ReserveERC20PoolHandler.sol

@@ -11,11 +11,9 @@ contract ReserveERC20PoolHandler is ReservePoolHandler, LiquidationERC20PoolHand
     constructor(
         address pool_,
         address ajna_,
-        address quote_,
-        address collateral_,
         address poolInfo_,
         uint256 numOfActors_,
         address testContract_
-    ) LiquidationERC20PoolHandler(pool_, ajna_, quote_, collateral_, poolInfo_, numOfActors_, testContract_) {}
+    ) LiquidationERC20PoolHandler(pool_, ajna_, poolInfo_, numOfActors_, testContract_) {}
 
 }

tests/forge/invariants/ERC20Pool/handlers/TradingERC20PoolHandler.sol

@@ -30,11 +30,9 @@ contract TradingERC20PoolHandler is UnboundedLiquidationPoolHandler, UnboundedBa
     constructor(
         address pool_,
         address ajna_,
-        address quote_,
-        address collateral_,
         address poolInfo_,
         address testContract_
-    ) BaseERC20PoolHandler(pool_, ajna_, quote_, collateral_, poolInfo_, 0, testContract_) {
+    ) BaseERC20PoolHandler(pool_, ajna_, poolInfo_, 0, testContract_) {
         numberOfBuckets = buckets.length();
         setUp();
     }

tests/forge/invariants/ERC20Pool/handlers/unbounded/BaseERC20PoolHandler.sol

@@ -24,12 +24,10 @@ abstract contract BaseERC20PoolHandler is BaseHandler {
     constructor(
         address pool_,
         address ajna_,
-        address quote_,
-        address collateral_,
         address poolInfo_,
         uint256 numOfActors_,
         address testContract_
-    ) BaseHandler(pool_, ajna_, quote_, poolInfo_, testContract_) {
+    ) BaseHandler(pool_, ajna_, poolInfo_, testContract_) {
 
         LENDER_MIN_BUCKET_INDEX = vm.envOr("BUCKET_INDEX_ERC20", uint256(2570));
         LENDER_MAX_BUCKET_INDEX = LENDER_MIN_BUCKET_INDEX + vm.envOr("NO_OF_BUCKETS", uint256(3)) - 1;
@@ -47,12 +45,12 @@ abstract contract BaseERC20PoolHandler is BaseHandler {
             buckets.add(bucket);
         }
 
-        // Tokens
-        _collateral = TokenWithNDecimals(collateral_);
-
         // Pool
         _erc20Pool  = ERC20Pool(pool_);
 
+        // Tokens
+        _collateral = TokenWithNDecimals(_erc20Pool.collateralAddress());
+
         // Actors
         actors = _buildActors(numOfActors_);
     }

tests/forge/invariants/ERC721Pool/BasicERC721PoolInvariants.t.sol

@@ -38,8 +38,6 @@ contract BasicERC721PoolInvariants is BasicInvariants {
         _basicERC721PoolHandler = new BasicERC721PoolHandler(
             address(_erc721pool),
             address(_ajna),
-            address(_quote),
-            address(_collateral),
             address(_poolInfo),
             _numOfActors,
             address(this)

tests/forge/invariants/ERC721Pool/LiquidationERC721PoolInvariants.t.sol

@@ -21,8 +21,6 @@ contract LiquidationERC721PoolInvariants is BasicERC721PoolInvariants, Liquidati
         _liquidationERC721PoolHandler = new LiquidationERC721PoolHandler(
             address(_erc721pool),
             address(_ajna),
-            address(_quote),
-            address(_collateral),
             address(_poolInfo),
             _numOfActors,
             address(this)

tests/forge/invariants/ERC721Pool/PanicExitERC721PoolInvariants.t.sol

@@ -29,8 +29,6 @@ contract PanicExitERC721PoolInvariants is BasicERC721PoolInvariants, Liquidation
         _panicExitERC721PoolHandler = new PanicExitERC721PoolHandler(
             address(_erc721pool),
             address(_ajna),
-            address(_quote),
-            address(_collateral),
             address(_poolInfo),
             address(this)
         );

tests/forge/invariants/ERC721Pool/RealWorldScenarioInvariants.t.sol

@@ -22,8 +22,6 @@ contract RealWorldScenarioInvariants is ReserveInvariants, LiquidationERC721Pool
         _reserveERC721PoolHandler = new ReserveERC721PoolHandler(
             address(_erc721pool),
             address(_ajna),
-            address(_quote),
-            address(_collateral),
             address(_poolInfo),
             _numOfActors,
             address(this)

tests/forge/invariants/ERC721Pool/ReserveERC721PoolInvariants.t.sol

@@ -23,8 +23,6 @@ contract ReserveERC721PoolInvariants is ReserveInvariants, LiquidationERC721Pool
         _reserveERC721PoolHandler = new ReserveERC721PoolHandler(
             address(_erc721pool),
             address(_ajna),
-            address(_quote),
-            address(_collateral),
             address(_poolInfo),
             _numOfActors,
             address(this)

tests/forge/invariants/ERC721Pool/handlers/BasicERC721PoolHandler.sol

@@ -22,12 +22,10 @@ contract BasicERC721PoolHandler is UnboundedBasicERC721PoolHandler, BasicPoolHan
     constructor(
         address pool_,
         address ajna_,
-        address quote_,
-        address collateral_,
         address poolInfo_,
         uint256 numOfActors_,
         address testContract_
-    ) BaseERC721PoolHandler(pool_, ajna_, quote_, collateral_, poolInfo_, numOfActors_, testContract_) {
+    ) BaseERC721PoolHandler(pool_, ajna_, poolInfo_, numOfActors_, testContract_) {
 
     }