SIP-150: fix issuance debt bug

sips/sip-150.md

@@ -0,0 +1,99 @@
+---
+sip: 150
+title: Issuance Debt Adjustment Truncation Fix
+status: Draft
+author: Anton Jurisevic (@zyzek)
+discussions-to: https://research.synthetix.io
+created: 2021-06-16
+---
+
+<!--You can leave these HTML comments in your merged SIP and delete the visible duplicate text guides, they will not appear and may be helpful to refer to if you edit it again. This is the suggested template for new SIPs. Note that an SIP number will be assigned by an editor. When opening a pull request to submit your SIP, please use an abbreviated title in the filename, `sip-draft_title_abbrev.md`. The title should be 44 characters or less.-->
+
+## Simple Summary
+
+This corrects a potentially-exploitable problem whereby
+issuer's debt balances immediately after issuing synths are less than the sUSD they have minted.
+
+
+## Abstract
+<!--A short (~200 word) description of the proposed change, the abstract should clearly describe the proposed change. This is what *will* be done if the SIP is implemented, not *why* it should be done or *how* it will be done. If the SIP proposes deploying a new contract, write, "we propose to deploy a new contract that will do x".-->
+
+Since the EtherWrapper cap was raised, calculation of the excluded debt component of partial debt updates has been
+interfering with incrementing the cached debt number upon issuance of synths.
+This SIP resolves the issue by not computing the excluded debt component during sUSD issuance.
+
+
+## Motivation
+<!--This is the problem statement. This is the *why* of the SIP. It should clearly explain *why* the current state of the protocol is inadequate.  It is critical that you explain *why* the change is needed, if the SIP proposes changing how something is calculated, you must address *why* the current calculation is innaccurate or wrong. This is not the place to describe how the SIP will address the issue!-->
+
+The attack surface is limited by the size of a user's wallet, since the available profit is at most the user's issued
+fraction of the debt pool. The burn lock ([SIP 40](https://sips.synthetix.io/sips/sip-40)) also makes this
+difficult to take advantage of in practice.
+However, if a debt snapshot is not performed quickly enough, an issuer could make free profit
+by burning their newly-issued sUSD, freeing up all their staked SNX while leaving some sUSD left over.
+
+
+## Specification
+<!--The specification should describe the syntax and semantics of any new feature, there are five sections
+1. Overview
+2. Rationale
+3. Technical Specification
+4. Test Cases
+5. Configurable Values
+-->
+
+### Overview
+
+The `DebtCache._updateCachedSynthDebtsWithRates` function is called during issuance to update the cached debt with the
+new synth supply. Due to the way non-SNX-backed debt was being excluded from the cached debt in this function,
+once the value of non-SNX-backed debt exceeded the circulating sUSD supply the delta being applied truncated to zero.
+Consequently, the newly-issued synths were effectively invisible to the debt cache, and everyone's debt balance
+will be underreported until a fresh snapshot is taken.
+
+### Rationale
+
+The existing logic is incorrect, and this SIP proposes to correct it. It should be noted that
+as it stands, the offending code will now be completely unused. This is intentional until a more robust pass is made
+on the debt pool when the L1 and L2 debt pools are unified. This means that the excluded debt component
+will only be recomputed on full snapshots.
+
+### Technical Specification
+
+No changes will be made to the external API, the fix is limited to one internal function only.
+
+Upon issuance, the `DebtCache._updateCachedSynthDebtsWithRates` function is called to update the contribution of
+sUSD to the debt pool. During this procedure, code approximately equivalent to the following is executed:
+
+```solidity
+// Compute the change to apply to the cached debt
+uint excludedDebt = _cachedSynthDebt[EXCLUDED_DEBT_KEY];
+uint cachedSum = _cachedSynthDebt[sUSD].floorsub(excludedDebt);
+uint currentSum = sUSD.totalSupply().floorsub(excludedDebt);
+
+// Apply that change
+_cachedDebt = _cachedDebt.sub(cachedSum).add(currentSum);
+```
+
+Here, the `floorsub` function is the same as ordinary subtraction except that it clamps the result to
+zero if it would be negative.
+At the time of writing, the circulating sUSD supply is worth around $200 million, while the excluded debt component
+is worth more than $500 million. Hence the values of `cachedSum` and `currentSum` truncate to zero, and the
+result is that `_cachedDebt` is not updated.
+As a result, debt balances underreported until the value of `_cachedDebt` is updated by a full snapshot.
+If the newly issued amount of synths is `x`, then debt balances will be off by a factor about equal to
+`_cachedDebt / (_cachedDebt + x)`.
+
+The solution proposed in [the implementation](https://github.com/Synthetixio/synthetix/pull/1340)
+is to execute no logic related to `excludedDebt` at all during the issuance process. Not only
+does this resolve the issue, but it is actually unnecessary to be touching this value at all during issuance.
+
+### Test Cases
+
+See the accompanying [pull request](https://github.com/Synthetixio/synthetix/pull/1340).
+
+### Configurable Values (Via SCCP)
+
+N/A
+
+## Copyright
+Copyright and related rights waived via [CC0](https://creativecommons.org/publicdomain/zero/1.0/).