SIP-187 fix partial synth updates and debt cache updates (#1551)

* fix partial synth updates and debt cache updates

* Remove require check that cachedSum < Debt as excluded Debt can cause this to fail. Update calc of delta in new synths changed.

Author: jacko125

contracts/DebtCache.sol

@@ -48,28 +48,41 @@ contract DebtCache is BaseDebtCache {
 
     function updateCachedSynthDebts(bytes32[] calldata currencyKeys) external requireSystemActiveIfNotOwner {
         (uint[] memory rates, bool anyRateInvalid) = exchangeRates().ratesAndInvalidForCurrencies(currencyKeys);
-        _updateCachedSynthDebtsWithRates(currencyKeys, rates, anyRateInvalid, false);
+        _updateCachedSynthDebtsWithRates(currencyKeys, rates, anyRateInvalid);
     }
 
     function updateCachedSynthDebtWithRate(bytes32 currencyKey, uint currencyRate) external onlyIssuer {
         bytes32[] memory synthKeyArray = new bytes32[](1);
         synthKeyArray[0] = currencyKey;
         uint[] memory synthRateArray = new uint[](1);
         synthRateArray[0] = currencyRate;
-        _updateCachedSynthDebtsWithRates(synthKeyArray, synthRateArray, false, false);
+        _updateCachedSynthDebtsWithRates(synthKeyArray, synthRateArray, false);
     }
 
     function updateCachedSynthDebtsWithRates(bytes32[] calldata currencyKeys, uint[] calldata currencyRates)
         external
         onlyIssuerOrExchanger
     {
-        _updateCachedSynthDebtsWithRates(currencyKeys, currencyRates, false, false);
+        _updateCachedSynthDebtsWithRates(currencyKeys, currencyRates, false);
     }
 
     function updateDebtCacheValidity(bool currentlyInvalid) external onlyIssuer {
         _updateDebtCacheValidity(currentlyInvalid);
     }
 
+    function updateCachedsUSDDebt(int amount) external onlyIssuer {
+        uint delta = SafeDecimalMath.abs(amount);
+        if (amount > 0) {
+            _cachedSynthDebt[sUSD] = _cachedSynthDebt[sUSD].add(delta);
+            _cachedDebt = _cachedDebt.add(delta);
+        } else {
+            _cachedSynthDebt[sUSD] = _cachedSynthDebt[sUSD].sub(delta);
+            _cachedDebt = _cachedDebt.sub(delta);
+        }
+
+        emit DebtCacheUpdated(_cachedDebt);
+    }
+
     /* ========== INTERNAL FUNCTIONS ========== */
 
     function _updateDebtCacheValidity(bool currentlyInvalid) internal {
@@ -83,8 +96,7 @@ contract DebtCache is BaseDebtCache {
     function _updateCachedSynthDebtsWithRates(
         bytes32[] memory currencyKeys,
         uint[] memory currentRates,
-        bool anyRateIsInvalid,
-        bool recomputeExcludedDebt
+        bool anyRateIsInvalid
     ) internal {
         uint numKeys = currencyKeys.length;
         require(numKeys == currentRates.length, "Input array lengths differ");
@@ -104,26 +116,12 @@ contract DebtCache is BaseDebtCache {
             _cachedSynthDebt[key] = currentSynthDebt;
         }
 
-        // Compute the excluded debt (wrappers, etc.) if necessary.
-        uint excludedDebtSum = _cachedSynthDebt[EXCLUDED_DEBT_KEY];
-        if (recomputeExcludedDebt) {
-            (uint excludedDebt, bool anyNonSnxDebtRateIsInvalid) = _totalNonSnxBackedDebt();
-            anyRateIsInvalid = anyRateIsInvalid || anyNonSnxDebtRateIsInvalid;
-            excludedDebtSum = excludedDebt;
-        }
-
-        // Subtract excluded debt.
-        cachedSum = cachedSum.floorsub(_cachedSynthDebt[EXCLUDED_DEBT_KEY]);
-        currentSum = currentSum.floorsub(excludedDebtSum);
-        _cachedSynthDebt[EXCLUDED_DEBT_KEY] = excludedDebtSum;
-
         // Apply the debt update.
         if (cachedSum != currentSum) {
             uint debt = _cachedDebt;
-            // This requirement should never fail, as the total debt snapshot is the sum of the individual synth
-            // debt snapshots.
-            require(cachedSum <= debt, "Cached synth sum exceeds total debt");
-            debt = debt.sub(cachedSum).add(currentSum);
+            // apply the delta between the cachedSum and currentSum
+            // add currentSum before sub cachedSum to prevent overflow as cachedSum > debt for large amount of excluded debt
+            debt = debt.add(currentSum).sub(cachedSum);
             _cachedDebt = debt;
             emit DebtCacheUpdated(debt);
         }

contracts/Issuer.sol

@@ -7,6 +7,7 @@ import "./MixinSystemSettings.sol";
 import "./interfaces/IIssuer.sol";
 
 // Libraries
+import "./SafeCast.sol";
 import "./SafeDecimalMath.sol";
 
 // Internal references
@@ -47,6 +48,8 @@ interface IIssuerInternalDebtCache {
             bool isInvalid,
             bool isStale
         );
+
+    function updateCachedsUSDDebt(int amount) external;
 }
 
 // https://docs.synthetix.io/contracts/source/contracts/issuer
@@ -466,7 +469,7 @@ contract Issuer is Owned, MixinSystemSettings, IIssuer {
             ISynthRedeemer _synthRedeemer = synthRedeemer();
             synths[sUSD].issue(address(_synthRedeemer), amountOfsUSD);
             // ensure the debt cache is aware of the new sUSD issued
-            debtCache().updateCachedSynthDebtWithRate(sUSD, SafeDecimalMath.unit());
+            debtCache().updateCachedsUSDDebt(SafeCast.toInt256(amountOfsUSD));
             _synthRedeemer.deprecate(IERC20(address(Proxyable(address(synthToRemove)).proxy())), rateToRedeem);
         }
 
@@ -666,7 +669,7 @@ contract Issuer is Owned, MixinSystemSettings, IIssuer {
         synths[sUSD].issue(from, amount);
 
         // Account for the issued debt in the cache
-        debtCache().updateCachedSynthDebtWithRate(sUSD, SafeDecimalMath.unit());
+        debtCache().updateCachedsUSDDebt(SafeCast.toInt256(amount));
 
         // Store their locked SNX amount to determine their fee % for the period
         _appendAccountIssuanceRecord(from);
@@ -692,7 +695,7 @@ contract Issuer is Owned, MixinSystemSettings, IIssuer {
         synths[sUSD].burn(burnAccount, amountBurnt);
 
         // Account for the burnt debt in the cache.
-        debtCache().updateCachedSynthDebtWithRate(sUSD, SafeDecimalMath.unit());
+        debtCache().updateCachedsUSDDebt(-SafeCast.toInt256(amountBurnt));
 
         // Store their debtRatio against a fee period to determine their fee/rewards % for the period
         _appendAccountIssuanceRecord(debtAccount);

contracts/SafeCast.sol

@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.5.16;
+
+/**
+ * @dev Wrappers over Solidity's uintXX casting operators with added overflow
+ * checks.
+ *
+ * Downcasting from uint256 in Solidity does not revert on overflow. This can
+ * easily result in undesired exploitation or bugs, since developers usually
+ * assume that overflows raise errors. `SafeCast` restores this intuition by
+ * reverting the transaction when such an operation overflows.
+ *
+ * Using this library instead of the unchecked operations eliminates an entire
+ * class of bugs, so it's recommended to use it always.
+ *
+ * Can be combined with {SafeMath} to extend it to smaller types, by performing
+ * all math on `uint256` and then downcasting.
+ */
+library SafeCast {
+    /**
+     * @dev Returns the downcasted uint128 from uint256, reverting on
+     * overflow (when the input is greater than largest uint128).
+     *
+     * Counterpart to Solidity's `uint128` operator.
+     *
+     * Requirements:
+     *
+     * - input must fit into 128 bits
+     */
+    function toUint128(uint256 value) internal pure returns (uint128) {
+        require(value < 2**128, "SafeCast: value doesn't fit in 128 bits");
+        return uint128(value);
+    }
+
+    /**
+     * @dev Returns the downcasted uint64 from uint256, reverting on
+     * overflow (when the input is greater than largest uint64).
+     *
+     * Counterpart to Solidity's `uint64` operator.
+     *
+     * Requirements:
+     *
+     * - input must fit into 64 bits
+     */
+    function toUint64(uint256 value) internal pure returns (uint64) {
+        require(value < 2**64, "SafeCast: value doesn't fit in 64 bits");
+        return uint64(value);
+    }
+
+    /**
+     * @dev Returns the downcasted uint32 from uint256, reverting on
+     * overflow (when the input is greater than largest uint32).
+     *
+     * Counterpart to Solidity's `uint32` operator.
+     *
+     * Requirements:
+     *
+     * - input must fit into 32 bits
+     */
+    function toUint32(uint256 value) internal pure returns (uint32) {
+        require(value < 2**32, "SafeCast: value doesn't fit in 32 bits");
+        return uint32(value);
+    }
+
+    /**
+     * @dev Returns the downcasted uint16 from uint256, reverting on
+     * overflow (when the input is greater than largest uint16).
+     *
+     * Counterpart to Solidity's `uint16` operator.
+     *
+     * Requirements:
+     *
+     * - input must fit into 16 bits
+     */
+    function toUint16(uint256 value) internal pure returns (uint16) {
+        require(value < 2**16, "SafeCast: value doesn't fit in 16 bits");
+        return uint16(value);
+    }
+
+    /**
+     * @dev Returns the downcasted uint8 from uint256, reverting on
+     * overflow (when the input is greater than largest uint8).
+     *
+     * Counterpart to Solidity's `uint8` operator.
+     *
+     * Requirements:
+     *
+     * - input must fit into 8 bits.
+     */
+    function toUint8(uint256 value) internal pure returns (uint8) {
+        require(value < 2**8, "SafeCast: value doesn't fit in 8 bits");
+        return uint8(value);
+    }
+
+    /**
+     * @dev Converts a signed int256 into an unsigned uint256.
+     *
+     * Requirements:
+     *
+     * - input must be greater than or equal to 0.
+     */
+    function toUint256(int256 value) internal pure returns (uint256) {
+        require(value >= 0, "SafeCast: value must be positive");
+        return uint256(value);
+    }
+
+    /**
+     * @dev Converts an unsigned uint256 into a signed int256.
+     *
+     * Requirements:
+     *
+     * - input must be less than or equal to maxInt256.
+     */
+    function toInt256(uint256 value) internal pure returns (int256) {
+        require(value < 2**255, "SafeCast: value doesn't fit in an int256");
+        return int256(value);
+    }
+}

contracts/SafeDecimalMath.sol

@@ -189,4 +189,19 @@ library SafeDecimalMath {
     function floorsub(uint a, uint b) internal pure returns (uint) {
         return b >= a ? 0 : a - b;
     }
+
+    /* ---------- Utilities ---------- */
+    /*
+     * Absolute value of the input, returned as a signed number.
+     */
+    function signedAbs(int x) internal pure returns (int) {
+        return x < 0 ? -x : x;
+    }
+
+    /*
+     * Absolute value of the input, returned as an unsigned number.
+     */
+    function abs(int x) internal pure returns (uint) {
+        return uint(signedAbs(x));
+    }
 }

contracts/interfaces/IDebtCache.sol

@@ -51,4 +51,6 @@ interface IDebtCache {
     function purgeCachedSynthDebt(bytes32 currencyKey) external;
 
     function takeDebtSnapshot() external;
+
+    function updateCachedsUSDDebt(int amount) external;
 }

publish/releases.json

@@ -373,6 +373,11 @@
 			"layer": "both",
 			"released": "both",
 			"sources": ["Exchanger", "Issuer", "SynthRedeemer"]
+		},
+		{
+			"sip": 187,
+			"layer": "both",
+			"sources": ["DebtCache", "Issuer"]
 		}
 	],
 	"releases": [

publish/src/commands/deploy/configure-loans.js

@@ -225,15 +225,17 @@ module.exports = async ({
 		comment: 'Set the max amount of debt in the CollateralManager',
 	});
 
-	await runStep({
-		contract: 'CollateralManager',
-		target: CollateralManager,
-		read: 'maxSkewRate',
-		expected: input => input !== '0', // only change if zero
-		write: 'setMaxSkewRate',
-		writeArg: [collateralManagerDefaults['MAX_SKEW_RATE']],
-		comment: 'Set the max skew rate in the CollateralManager',
-	});
+	if (CollateralManager.maxSkewRate) {
+		await runStep({
+			contract: 'CollateralManager',
+			target: CollateralManager,
+			read: 'maxSkewRate',
+			expected: input => input !== '0', // only change if zero
+			write: 'setMaxSkewRate',
+			writeArg: [collateralManagerDefaults['MAX_SKEW_RATE']],
+			comment: 'Set the max skew rate in the CollateralManager',
+		});
+	}
 
 	await runStep({
 		contract: 'CollateralManager',