Merge pull request #257 from liquity/tob_code_quality_2-7

Fix ToB code quality recommendations 2 & 4-7

packages/contracts/contracts/BorrowerOperations.sol

@@ -383,7 +383,7 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe
     // Send ETH to Active Pool and increase its recorded ETH balance
     function _activePoolAddColl(uint _amount) internal {
         (bool success, ) = address(activePool).call{value: _amount}("");
-        assert(success == true);
+        require(success, "BorrowerOps: Sending ETH to ActivePool failed");
     }
 
     // Issue the specified amount of LUSD to _account and increases the total active debt (_rawDebtIncrease potentially includes a LUSDFee)
@@ -411,7 +411,7 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe
     }
 
     function _requireNotInRecoveryMode() internal view {
-        require(_checkRecoveryMode() == false, "BorrowerOps: Operation not permitted during Recovery Mode");
+        require(!_checkRecoveryMode(), "BorrowerOps: Operation not permitted during Recovery Mode");
     }
 
     function _requireICRisAboveMCR(uint _newICR)  internal pure {

packages/contracts/contracts/SortedTroves.sol

@@ -46,7 +46,7 @@ import "./Dependencies/console.sol";
 contract SortedTroves is Ownable, CheckContract, ISortedTroves {
     using SafeMath for uint256;
 
-    event TroveManagerAddressChanged(address _newTrovelManagerAddress);
+    event TroveManagerAddressChanged(address _troveManagerAddress);
     event BorrowerOperationsAddressChanged(address _borrowerOperationsAddress);
 
     address public borrowerOperationsAddress;

packages/contracts/contracts/StabilityPool.sol

@@ -425,6 +425,17 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
     }
 
     function _computeLQTYPerUnitStaked(uint _LQTYIssuance, uint _totalLUSDDeposits) internal returns (uint) {
+        /*  
+        * Calculate the LQTY-per-unit staked.  Division uses a "feedback" error correction, to keep the 
+        * cumulative error low in the running total G:
+        *
+        * 1) Form a numerator which compensates for the floor division error that occurred the last time this 
+        * function was called.  
+        * 2) Calculate "per-unit-staked" ratio.
+        * 3) Multiply the ratio back by its denominator, to reveal the current floor division error.
+        * 4) Store this error for use in the next correction when this function is called.
+        * 5) Note: static analysis tools complain about this "division before multiplication", however, it is intended.
+        */
         uint LQTYNumerator = _LQTYIssuance.mul(DECIMAL_PRECISION).add(lastLQTYError);
 
         uint LQTYPerUnitStaked = LQTYNumerator.div(_totalLUSDDeposits);
@@ -465,13 +476,21 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
         internal
         returns (uint ETHGainPerUnitStaked, uint LUSDLossPerUnitStaked)
     {
-        uint LUSDLossNumerator = _debtToOffset.mul(DECIMAL_PRECISION).sub(lastLUSDLossError_Offset);
-        uint ETHNumerator = _collToAdd.mul(DECIMAL_PRECISION).add(lastETHError_Offset);
-
         /*
         * Compute the LUSD and ETH rewards. Uses a "feedback" error correction, to keep
-        * the cumulative error in the P and S state variables low.
+        * 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.  
+        * 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.
+        * 5) Note: static analysis tools complain about this "division before multiplication", however, it is intended.
         */
+        uint LUSDLossNumerator = _debtToOffset.mul(DECIMAL_PRECISION).sub(lastLUSDLossError_Offset);
+        uint ETHNumerator = _collToAdd.mul(DECIMAL_PRECISION).add(lastETHError_Offset);
+
+
         if (_debtToOffset >= _totalLUSDDeposits) {
             LUSDLossPerUnitStaked = DECIMAL_PRECISION;
             lastLUSDLossError_Offset = 0;
@@ -888,7 +907,7 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
     }
 
     function _requireFrontEndNotRegistered(address _address) internal view {
-        require(frontEnds[_address].registered == false, "StabilityPool: must not already be a registered front end");
+        require(!frontEnds[_address].registered, "StabilityPool: must not already be a registered front end");
     }
 
      function _requireFrontEndIsRegisteredOrZero(address _address) internal view {

packages/contracts/contracts/TroveManager.sol

@@ -439,9 +439,9 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager {
         L.recoveryModeAtStart = _checkRecoveryMode();
 
         // Perform the appropriate liquidation sequence - tally the values, and obtain their totals
-        if (L.recoveryModeAtStart == true) {
+        if (L.recoveryModeAtStart) {
             T = _getTotalsFromLiquidateTrovesSequence_RecoveryMode(L.price, L.LUSDInStabPool, _n);
-        } else { // if L.recoveryModeAtStart == false
+        } else { // if !L.recoveryModeAtStart
             T = _getTotalsFromLiquidateTrovesSequence_NormalMode(L.price, L.LUSDInStabPool, _n);
         }
 
@@ -492,7 +492,7 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager {
 
             L.ICR = getCurrentICR(L.user, _price);
 
-            if (L.backToNormalMode == false) {
+            if (!L.backToNormalMode) {
                 // Break the loop if ICR is greater than MCR and Stability Pool is empty
                 if (L.ICR >= MCR && L.remainingLUSDInStabPool == 0) { break; }
 
@@ -510,7 +510,7 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager {
 
                 L.backToNormalMode = !_checkPotentialRecoveryMode(L.entireSystemColl, L.entireSystemDebt, _price);
             }
-            else if (L.backToNormalMode == true && L.ICR < MCR) {
+            else if (L.backToNormalMode && L.ICR < MCR) {
                 V = _liquidateNormalMode(L.user, L.remainingLUSDInStabPool);
 
                 L.remainingLUSDInStabPool = L.remainingLUSDInStabPool.sub(V.debtToOffset);
@@ -568,9 +568,9 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager {
         L.recoveryModeAtStart = _checkRecoveryMode();
 
         // Perform the appropriate liquidation sequence - tally values and obtain their totals.
-        if (L.recoveryModeAtStart == true) {
+        if (L.recoveryModeAtStart) {
            T = _getTotalFromBatchLiquidate_RecoveryMode(L.price, L.LUSDInStabPool, _troveArray);
-        } else {  //  if L.recoveryModeAtStart == false
+        } else {  //  if !L.recoveryModeAtStart
             T = _getTotalsFromBatchLiquidate_NormalMode(L.price, L.LUSDInStabPool, _troveArray);
         }
 
@@ -616,7 +616,7 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager {
             L.user = _troveArray[L.i];
             L.ICR = getCurrentICR(L.user, _price);
 
-            if (L.backToNormalMode == false) {
+            if (!L.backToNormalMode) {
 
                 // Skip this trove if ICR is greater than MCR and Stability Pool is empty
                 if (L.ICR >= MCR && L.remainingLUSDInStabPool == 0) { continue; }
@@ -636,7 +636,7 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager {
                 L.backToNormalMode = !_checkPotentialRecoveryMode(L.entireSystemColl, L.entireSystemDebt, _price);
             }
 
-            else if (L.backToNormalMode == true && L.ICR < MCR) {
+            else if (L.backToNormalMode && L.ICR < MCR) {
                 V = _liquidateNormalMode(L.user, L.remainingLUSDInStabPool);
                 L.remainingLUSDInStabPool = L.remainingLUSDInStabPool.sub(V.debtToOffset);
 
@@ -839,10 +839,9 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager {
         _requireAmountGreaterThanZero(_LUSDamount);
         _requireLUSDBalanceCoversRedemption(msg.sender, _LUSDamount);
 
-        uint activeDebt = activePool.getLUSDDebt();
-        uint defaultedDebt = defaultPool.getLUSDDebt();        
+        uint entireSystemDebt = getEntireSystemDebt();       
         // Confirm redeemer's balance is less than total systemic debt
-        assert(lusdToken.balanceOf(msg.sender) <= (activeDebt.add(defaultedDebt)));
+        assert(lusdToken.balanceOf(msg.sender) <= entireSystemDebt);
 
         uint remainingLUSD = _LUSDamount;
         uint price = priceFeed.getPrice();
@@ -1092,19 +1091,27 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager {
         if (_debt == 0) { return; }
 
         /*
-        * Add distributed coll and debt rewards-per-unit-staked to the running totals.
-        * Division uses a "feedback" error correction, to keep the cumulative error in
-        * the  L_ETH and L_LUSDDebt state variables low.
+        * Add distributed coll and debt rewards-per-unit-staked to the running totals. Division uses a "feedback" 
+        * error correction, to keep the cumulative error low in the running totals L_ETH and L_LUSDDebt:
+        *
+        * 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.
+        * 5) Note: static analysis tools complain about this "division before multiplication", however, it is intended.
         */
         uint ETHNumerator = _coll.mul(DECIMAL_PRECISION).add(lastETHError_Redistribution);
         uint LUSDDebtNumerator = _debt.mul(DECIMAL_PRECISION).add(lastLUSDDebtError_Redistribution);
 
+        // Get the per-unit-staked terms
         uint ETHRewardPerUnitStaked = ETHNumerator.div(totalStakes);
         uint LUSDDebtRewardPerUnitStaked = LUSDDebtNumerator.div(totalStakes);
 
         lastETHError_Redistribution = ETHNumerator.sub(ETHRewardPerUnitStaked.mul(totalStakes));
         lastLUSDDebtError_Redistribution = LUSDDebtNumerator.sub(LUSDDebtRewardPerUnitStaked.mul(totalStakes));
 
+        // Add per-unit-staked terms to the running totals
         L_ETH = L_ETH.add(ETHRewardPerUnitStaked);
         L_LUSDDebt = L_LUSDDebt.add(LUSDDebtRewardPerUnitStaked);