Fix price volatility on unstaking (#18)

src/BaseDelegation.sol

@@ -31,7 +31,7 @@ library WithdrawalQueue {
         return abi.decode(data, (uint256));
     }
 
-    function queue(Fifo storage fifo, uint256 amount) internal {
+    function enqueue(Fifo storage fifo, uint256 amount) internal {
         fifo.items[fifo.last] = Item(block.number + unbondingPeriod(), amount);
         fifo.last++;
     }
@@ -198,7 +198,7 @@ abstract contract BaseDelegation is Delegation, PausableUpgradeable, Ownable2Ste
 
     function _enqueueWithdrawal(uint256 amount) internal virtual {
         BaseDelegationStorage storage $ = _getBaseDelegationStorage();
-        $.withdrawals[_msgSender()].queue(amount);
+        $.withdrawals[_msgSender()].enqueue(amount);
         $.totalWithdrawals += amount;
     }
 

src/LiquidDelegationV2.sol

@@ -105,31 +105,6 @@ contract LiquidDelegationV2 is BaseDelegation, ILiquidDelegation {
         emit Staked(_msgSender(), msg.value, abi.encode(shares));
     }
 
-    //TODO: remove the whole function, was added temporarily for testing if the
-    //      validator can unstake its entire deposit to remove itself from the committee,
-    //      returns the amount to be paid, the contract's balance,
-    //      the gap to be withdrawn from the deposit and the contract's deposit
-    function unstake2(uint256 shares) public view returns(uint256, uint256, int256, uint256) {
-        uint256 amount;
-        LiquidDelegationStorage storage $ = _getLiquidDelegationStorage();
-        // before calculating the amount deduct the commission from the yet untaxed rewards
-        //taxRewards();
-        uint256 rewards = getRewards();
-        uint256 commission = (rewards - $.taxedRewards) * getCommissionNumerator() / DENOMINATOR;
-        uint256 taxedRewards = rewards - commission;
-        if (NonRebasingLST($.lst).totalSupply() == 0)
-            amount = shares;
-        else
-            amount = (getStake() + taxedRewards) * shares / NonRebasingLST($.lst).totalSupply();
-        //_enqueueWithdrawal(amount);
-        // maintain a balance that is always sufficient to cover the claims
-        //if (address(this).balance < getTotalWithdrawals())
-        //    _decreaseDeposit(getTotalWithdrawals() - address(this).balance);
-        return (amount, address(this).balance - commission, int256(getTotalWithdrawals()) + int256(amount) - int256(address(this).balance - commission), getStake());
-        //NonRebasingLST($.lst).burn(_msgSender(), shares);
-        //emit Unstaked(_msgSender(), amount, abi.encode(shares));
-    }
-
     function unstake(uint256 shares) public override whenNotPaused {
         uint256 amount;
         LiquidDelegationStorage storage $ = _getLiquidDelegationStorage();
@@ -139,10 +114,12 @@ contract LiquidDelegationV2 is BaseDelegation, ILiquidDelegation {
             amount = shares;
         else
             amount = (getStake() + $.taxedRewards) * shares / NonRebasingLST($.lst).totalSupply();
+        // stake the surplus of taxed rewards not needed for covering the pending withdrawals
+        // before we increase the pending withdrawals by enqueueing the current amount
+        _stakeRewards();
         _enqueueWithdrawal(amount);
         // maintain a balance that is always sufficient to cover the claims
-        if (address(this).balance < getTotalWithdrawals())
-            _decreaseDeposit(getTotalWithdrawals() - address(this).balance);
+        _decreaseDeposit(amount);
         NonRebasingLST($.lst).burn(_msgSender(), shares);
         emit Unstaked(_msgSender(), amount, abi.encode(shares));
     }
@@ -191,17 +168,17 @@ contract LiquidDelegationV2 is BaseDelegation, ILiquidDelegation {
     }
 
     function stakeRewards() public override onlyOwner {
+        _stakeRewards();
+    }
+
+    function _stakeRewards() internal {
         LiquidDelegationStorage storage $ = _getLiquidDelegationStorage();
         // rewards must be taxed before deposited since
         // they will not be taxed when they are unstaked
         taxRewards();
         // we must not deposit the funds we need to pay out the claims
         if (address(this).balance > getTotalWithdrawals()) {
-            // TODO: moving funds between rewards and deposit should
-            //       be okay but it's not, because the price calculation
-            //       assumes the rewards are higher than the taxed rewards
-            //       but after moving the rewards to the deposit they are not
-            // not only the rewards (balance) will be reduced
+            // not only the rewards (balance) must be reduced
             // by the deposit topup but also the taxed rewards
             $.taxedRewards -= address(this).balance - getTotalWithdrawals();
             _increaseDeposit(address(this).balance - getTotalWithdrawals());

src/NonLiquidDelegationV2.sol

@@ -166,16 +166,14 @@ contract NonLiquidDelegationV2 is BaseDelegation, INonLiquidDelegation {
     }
 
     function stake() public override payable whenNotPaused {
-        if (_isActivated())
-            _increaseDeposit(msg.value);
+        _increaseDeposit(msg.value);
         _append(int256(msg.value));
         emit Staked(_msgSender(), msg.value, "");
     }
 
     function unstake(uint256 value) public override whenNotPaused {
         _append(-int256(value));
-        if (_isActivated())
-            _decreaseDeposit(uint256(value));
+        _decreaseDeposit(uint256(value));
         _enqueueWithdrawal(value);
         emit Unstaked(_msgSender(), value, "");
     }
@@ -248,8 +246,7 @@ contract NonLiquidDelegationV2 is BaseDelegation, INonLiquidDelegation {
 
     function stakeRewards() public override {
         (uint256 amount, ) = _useRewards(type(uint256).max, type(uint64).max);
-        if (_isActivated())
-            _increaseDeposit(amount);
+        _increaseDeposit(amount);
         _append(int256(amount));
         emit Staked(_msgSender(), amount, "");
     }

test/LiquidDelegation.t.sol

@@ -533,7 +533,7 @@ contract LiquidDelegationTest is Test {
         console.log("validator withdrew");
         console.log("validator balance: %s", owner.balance);
         //vm.roll(block.number + Deposit(delegation.DEPOSIT_CONTRACT()).withdrawalPeriod());
-        //TODO: once https://github.com/Zilliqa/zq2/issues/1761 is fixed, uncomment the previous line and comment out the next one
+        //TODO: remove the next line and uncomment the previous once https://github.com/Zilliqa/zq2/issues/1761 is fixed
         vm.warp(block.timestamp + Deposit(delegation.DEPOSIT_CONTRACT()).withdrawalPeriod()); // skip(WithdrawalQueue.unbondingPeriod());
         Deposit(delegation.DEPOSIT_CONTRACT()).withdraw();
         console.log("validator withdrew again");