Optional claim/stash SP ETH rewards

contracts/src/Interfaces/IStabilityPool.sol

@@ -52,16 +52,18 @@ interface IStabilityPool is ILiquityBase {
     * - Increases deposit, and takes new snapshots of accumulators P and S
     * - Sends depositor's accumulated ETH gains to depositor
     */
-    function provideToSP(uint256 _amount) external;
+    function provideToSP(uint256 _amount, bool _doClaim) external;
 
     /*  withdrawFromSP():
     * - Calculates depositor's ETH gain
     * - Calculates the compounded deposit
-    * - Sends the requested BOLD withdrawal to depositor 
+    * - Sends the requested BOLD withdrawal to depositor
     * - (If _amount > userDeposit, the user withdraws all of their compounded deposit)
     * - Decreases deposit by withdrawn amount and takes new snapshots of accumulators P and S
     */
-    function withdrawFromSP(uint256 _amount) external;
+    function withdrawFromSP(uint256 _amount, bool doClaim) external;
+
+    function claimAllETHGains() external;
 
     /*
      * Initial checks:
@@ -73,6 +75,8 @@ interface IStabilityPool is ILiquityBase {
      */
     function offset(uint256 _debt, uint256 _coll) external;
 
+    function stashedETH(address _depositor) external view returns (uint256);
+
     /*
      * Returns the total amount of ETH held by the pool, accounted in an internal variable instead of `balance`,
      * to exclude edge cases like ETH received from a self-destruct.

contracts/src/StabilityPool.sol

@@ -69,9 +69,9 @@ import "./Dependencies/CheckContract.sol";
  * So, to track P accurately, we use a scale factor: if a liquidation would cause P to decrease to <1e-9 (and be rounded to 0 by Solidity),
  * we first multiply P by 1e9, and increment a currentScale factor by 1.
  *
- * The added benefit of using 1e9 for the scale factor (rather than 1e18) is that it ensures negligible precision loss close to the 
- * scale boundary: when P is at its minimum value of 1e9, the relative precision loss in P due to floor division is only on the 
- * order of 1e-9. 
+ * The added benefit of using 1e9 for the scale factor (rather than 1e18) is that it ensures negligible precision loss close to the
+ * scale boundary: when P is at its minimum value of 1e9, the relative precision loss in P due to floor division is only on the
+ * order of 1e-9.
  *
  * --- EPOCHS ---
  *
@@ -163,6 +163,7 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
 
     mapping(address => Deposit) public deposits; // depositor address -> Deposit struct
     mapping(address => Snapshots) public depositSnapshots; // depositor address -> snapshots struct
+    mapping(address => uint256) public stashedETH;
 
     /*  Product 'P': Running product by which to multiply an initial deposit, in order to find the current compounded deposit,
     * after a series of liquidations have occurred, each of which cancel some Bold debt with the deposit.
@@ -280,14 +281,14 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
     * - Increases deposit, and takes new snapshots of accumulators P and S
     * - Sends depositor's accumulated ETH gains to depositor
     */
-    function provideToSP(uint256 _amount) external override {
+    function provideToSP(uint256 _amount, bool _doClaim) external override {
         _requireNonZeroAmount(_amount);
 
         activePool.mintAggInterest();
 
         uint256 initialDeposit = deposits[msg.sender].initialValue;
 
-        uint256 depositorETHGain = getDepositorETHGain(msg.sender);
+        uint256 currentETHGain = getDepositorETHGain(msg.sender);
         uint256 compoundedBoldDeposit = getCompoundedBoldDeposit(msg.sender);
         uint256 boldLoss = initialDeposit - compoundedBoldDeposit; // Needed only for event log
 
@@ -297,26 +298,25 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
         _updateDepositAndSnapshots(msg.sender, newDeposit);
         emit UserDepositChanged(msg.sender, newDeposit);
 
-        emit ETHGainWithdrawn(msg.sender, depositorETHGain, boldLoss); // Bold Loss required for event log
-
-        _sendETHGainToDepositor(depositorETHGain);
+        _stashOrSendETHGains(msg.sender, currentETHGain, boldLoss, _doClaim);
+        assert(getDepositorETHGain(msg.sender) == 0);
     }
 
     /*  withdrawFromSP():
     * - Calculates depositor's ETH gain
     * - Calculates the compounded deposit
-    * - Sends the requested BOLD withdrawal to depositor 
+    * - Sends the requested BOLD withdrawal to depositor
     * - (If _amount > userDeposit, the user withdraws all of their compounded deposit)
     * - Decreases deposit by withdrawn amount and takes new snapshots of accumulators P and S
     */
-    function withdrawFromSP(uint256 _amount) external override {
+    function withdrawFromSP(uint256 _amount, bool _doClaim) external override {
         // TODO: if (_amount !=0) {_requireNoUnderCollateralizedTroves();}
         uint256 initialDeposit = deposits[msg.sender].initialValue;
         _requireUserHasDeposit(initialDeposit);
 
         activePool.mintAggInterest();
 
-        uint256 depositorETHGain = getDepositorETHGain(msg.sender);
+        uint256 currentETHGain = getDepositorETHGain(msg.sender);
 
         uint256 compoundedBoldDeposit = getCompoundedBoldDeposit(msg.sender);
         uint256 BoldtoWithdraw = LiquityMath._min(_amount, compoundedBoldDeposit);
@@ -329,9 +329,57 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
         _updateDepositAndSnapshots(msg.sender, newDeposit);
         emit UserDepositChanged(msg.sender, newDeposit);
 
-        emit ETHGainWithdrawn(msg.sender, depositorETHGain, boldLoss); // Bold Loss required for event log
+        _stashOrSendETHGains(msg.sender, currentETHGain, boldLoss, _doClaim);
+        assert(getDepositorETHGain(msg.sender) == 0);
+    }
+
+    function _stashOrSendETHGains(address _depositor, uint256 _currentETHGain, uint256 _boldLoss, bool _doClaim) internal {
+        if (_doClaim) {
+            // Get the total gain (stashed + current), zero the stashed balance, send total gain to depositor
+            uint ETHToSend = _getTotalETHGainAndZeroStash(_depositor, _currentETHGain);
+
+            emit ETHGainWithdrawn(msg.sender, ETHToSend, _boldLoss); // Bold Loss required for event log
+            _sendETHGainToDepositor(ETHToSend);
+
+        } else {
+            // Just stash the current gain
+            stashedETH[_depositor] += _currentETHGain;
+        }
+    }
+
+    function _getTotalETHGainAndZeroStash(address _depositor, uint256 _currentETHGain) internal returns (uint256) {
+         uint256 stashedETHGain = stashedETH[_depositor];
+        uint256 totalETHGain = stashedETHGain + _currentETHGain;
+
+        // TODO: Gas - saves gas when stashedETHGain == 0?
+        if (stashedETHGain > 0) {stashedETH[_depositor] = 0;}
+
+        return totalETHGain;
+    }
+
+    // TODO: Make this also claim BOlD gains when they are implemented
+    function claimAllETHGains() external {
+        // We don't require they have a deposit: they may have stashed gains and no deposit
+        uint256 initialDeposit = deposits[msg.sender].initialValue;
+        uint256 boldLoss;
+        uint256 currentETHGain;
+
+        activePool.mintAggInterest();
+
+        // If they have a deposit, update it and update its snapshots
+        if (initialDeposit > 0) {
+            currentETHGain = getDepositorETHGain(msg.sender);  // Only active deposits can have a current ETH gain
+
+            uint256 compoundedBoldDeposit = getCompoundedBoldDeposit(msg.sender);
+            boldLoss = initialDeposit - compoundedBoldDeposit; // Needed only for event log
+
+            _updateDepositAndSnapshots(msg.sender, compoundedBoldDeposit);
+        }
+
+        uint256 ETHToSend = _getTotalETHGainAndZeroStash(msg.sender,  currentETHGain);
 
-        _sendETHGainToDepositor(depositorETHGain);
+        _sendETHGainToDepositor(ETHToSend);
+        assert(getDepositorETHGain(msg.sender) == 0);
     }
 
     // --- Liquidation functions ---
@@ -364,8 +412,8 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
         * Compute the Bold and ETH rewards. Uses a "feedback" error correction, to keep
         * the cumulative error in the P and S state variables low:
         *
-        * 1) Form numerators which compensate for the floor division errors that occurred the last time this 
-        * function was called.  
+        * 1) Form numerators which compensate for the floor division errors that occurred the last time this
+        * function was called.
         * 2) Calculate "per-unit-staked" ratios.
         * 3) Multiply each ratio back by its denominator, to reveal the current floor division error.
         * 4) Store these errors for use in the next correction when this function is called.

contracts/src/test/TestContracts/BaseTest.sol

@@ -160,18 +160,37 @@ contract BaseTest is Test {
         assertEq(recoveryMode, _enabled);
     }
 
-    function makeSPDeposit(address _account, uint256 _amount) public {
+    function makeSPDepositAndClaim(address _account, uint256 _amount) public {
         vm.startPrank(_account);
-        stabilityPool.provideToSP(_amount);
+        stabilityPool.provideToSP(_amount, true);
         vm.stopPrank();
     }
 
-    function makeSPWithdrawal(address _account, uint256 _amount) public {
+    function makeSPDepositNoClaim(address _account, uint256 _amount) public {
         vm.startPrank(_account);
-        stabilityPool.withdrawFromSP(_amount);
+        stabilityPool.provideToSP(_amount, false);
         vm.stopPrank();
     }
 
+    function makeSPWithdrawalAndClaim(address _account, uint256 _amount) public {
+        vm.startPrank(_account);
+        stabilityPool.withdrawFromSP(_amount, true);
+        vm.stopPrank();
+    }
+
+    function makeSPWithdrawalNoClaim(address _account, uint256 _amount) public {
+        vm.startPrank(_account);
+        stabilityPool.withdrawFromSP(_amount, false);
+        vm.stopPrank();
+    }
+
+     function claimAllETHGains(address _account) public {
+        vm.startPrank(_account);
+        stabilityPool.claimAllETHGains();
+        vm.stopPrank();
+    }
+
+
     function closeTrove(address _account, uint256 _troveId) public {
         vm.startPrank(_account);
         borrowerOperations.closeTrove(_troveId);

contracts/src/test/TestContracts/DevTestSetup.sol

@@ -91,8 +91,8 @@ contract DevTestSetup is BaseTest {
         uint256 CTroveId = openTroveNoHints100pct(C, 5 ether, troveDebtRequest_C, interestRate);
 
         // A and B deposit to SP
-        makeSPDeposit(A, troveDebtRequest_A);
-        makeSPDeposit(B, troveDebtRequest_B);
+        makeSPDepositAndClaim(A, troveDebtRequest_A);
+        makeSPDepositAndClaim(B, troveDebtRequest_B);
 
         // Price drops, C becomes liquidateable
         price = 1025e18;
@@ -117,27 +117,44 @@ contract DevTestSetup is BaseTest {
         uint256 troveDebtRequest_D = 2250e18;
         uint256 interestRate = 5e16; // 5%
 
+        TroveIDs memory troveIDs;
+
         uint256 price = 2000e18;
         priceFeed.setPrice(price);
 
-        uint256 ATroveId = openTroveNoHints100pct(A, 5 ether, troveDebtRequest_A, interestRate);
-        uint256 BTroveId = openTroveNoHints100pct(B, 5 ether, troveDebtRequest_B, interestRate);
-        uint256 CTroveId = openTroveNoHints100pct(C, 25e17, troveDebtRequest_C, interestRate);
-        uint256 DTroveId = openTroveNoHints100pct(D, 25e17, troveDebtRequest_D, interestRate);
+        troveIDs.A = openTroveNoHints100pct(A, 5 ether, troveDebtRequest_A, interestRate);
+        troveIDs.B = openTroveNoHints100pct(B, 5 ether, troveDebtRequest_B, interestRate);
+        troveIDs.C = openTroveNoHints100pct(C, 25e17, troveDebtRequest_C, interestRate);
+        troveIDs.D = openTroveNoHints100pct(D, 25e17, troveDebtRequest_D, interestRate);
 
         // A and B deposit to SP
-        makeSPDeposit(A, troveDebtRequest_A);
-        makeSPDeposit(B, troveDebtRequest_B);
+        makeSPDepositAndClaim(A, troveDebtRequest_A);
+        makeSPDepositAndClaim(B, troveDebtRequest_B);
 
         // Price drops, C and D become liquidateable
         price = 1050e18;
         priceFeed.setPrice(price);
 
         assertFalse(troveManager.checkRecoveryMode(price));
-        assertLt(troveManager.getCurrentICR(CTroveId, price), troveManager.MCR());
-        assertLt(troveManager.getCurrentICR(DTroveId, price), troveManager.MCR());
+        assertLt(troveManager.getCurrentICR(troveIDs.C, price), troveManager.MCR());
+        assertLt(troveManager.getCurrentICR(troveIDs.D, price), troveManager.MCR());
 
-        return (ATroveId, BTroveId, CTroveId, DTroveId);
+        return (troveIDs.A, troveIDs.B, troveIDs.C, troveIDs.D);
+    }
+
+    function _setupForSPDepositAdjustments() internal returns (TroveIDs memory) {
+        TroveIDs memory troveIDs;
+        (troveIDs.A, troveIDs.B, troveIDs.C, troveIDs.D) =  _setupForBatchLiquidateTrovesPureOffset();
+
+        // A liquidates C
+        liquidate(A, troveIDs.C);
+
+        // D sends BOLD to A and B so they have some to use in tests
+        transferBold(D, A, boldToken.balanceOf(D) / 2);
+        transferBold(D, B, boldToken.balanceOf(D));
+
+        assertEq(troveManager.getTroveStatus(troveIDs.C), 3); // Status 3 - closed by liquidation
+        return troveIDs;
     }
 
     function _setupForBatchLiquidateTrovesPureRedist() internal returns (uint256, uint256, uint256, uint256) {

contracts/src/test/basicOps.t.sol

@@ -140,13 +140,13 @@ contract BasicOps is DevTestSetup {
         borrowerOperations.openTrove(A, 0, 2e18, 2000e18, 0, 0, 0);
 
         // A makes an SP deposit
-        stabilityPool.provideToSP(100e18);
+        makeSPDepositAndClaim(A, 100e18);
 
         // time passes
         vm.warp(block.timestamp + 7 days);
 
         // A tops up their SP deposit
-        stabilityPool.provideToSP(100e18);
+        makeSPDepositAndClaim(A, 100e18);
 
         // Check A's balance decreased and SP deposit increased
         assertEq(boldToken.balanceOf(A), 1800e18);
@@ -159,7 +159,7 @@ contract BasicOps is DevTestSetup {
         borrowerOperations.openTrove(A, 0, 2e18, 2000e18, 0, 0, 0);
 
         // A makes an SP deposit
-        stabilityPool.provideToSP(100e18);
+        makeSPDepositAndClaim(A, 100e18);
 
         // time passes
         vm.warp(block.timestamp + 7 days);
@@ -169,7 +169,7 @@ contract BasicOps is DevTestSetup {
         assertEq(stabilityPool.getCompoundedBoldDeposit(A), 100e18);
 
         // A withdraws their full SP deposit
-        stabilityPool.withdrawFromSP(100e18);
+        makeSPWithdrawalAndClaim(A, 100e18);
 
         // Check A's balance increased and SP deposit decreased to 0
         assertEq(boldToken.balanceOf(A), 2000e18);