Rethink changeSupply

contracts/contracts/token/OUSD.sol

@@ -449,31 +449,43 @@ contract OUSD is Initializable, InitializableERC20Detailed, Governable {
         onlyVault
         nonReentrant
     {
+        // Prevents division by zero
         require(_totalSupply > 0, "Cannot increase 0 supply");
-
-        if (_totalSupply == _newTotalSupply) {
-            emit TotalSupplyUpdated(
-                _totalSupply,
-                rebasingCredits,
-                rebasingCreditsPerToken
-            );
-            return;
-        }
-
-        _totalSupply = _newTotalSupply > MAX_SUPPLY
-            ? MAX_SUPPLY
-            : _newTotalSupply;
-
-        rebasingCreditsPerToken = rebasingCredits.divPrecisely(
-            _totalSupply.sub(nonRebasingSupply)
-        );
-
+
+        // We currently require the OUSD balance to only go up. If in the
+        // future OUSD is changed to also rebase down, then all math in this
+        // contract needs to be rechecked.
+        require(_newTotalSupply >= _totalSupply);
+
+        // Ensures headroom for mathematical operations
+        require(_newTotalSupply <= MAX_SUPPLY);
+
+        // Calculates the inverse value of each credit. The add(1) at the end
+        // ensures that any rounding errors round rebasing accounts down.
+        // If we rounded up, we would exceed the total supply.
+        rebasingCreditsPerToken = rebasingCredits
+            .divPrecisely(_newTotalSupply.sub(nonRebasingSupply))
+            .add(1);
+
+        // If rebasingCreditsPerToken is ever 0, then all the accounting
+        // between rebasing and non-rebasing accounts will be wrong. The above
+        // add(1) should prevent this from ever reaching 0, but it is a
+        // critical invariant and so we make this explicit.
         require(rebasingCreditsPerToken > 0, "Invalid change in supply");
 
-        _totalSupply = rebasingCredits
+        // actualSupply is the sum of all OUSD accounts
+        uint256 actualSupply = rebasingCredits
             .divPrecisely(rebasingCreditsPerToken)
             .add(nonRebasingSupply);
 
+        // Verify that the sum of all OUSD accounts is equal to or less than
+        // the value of backing assets. This ensures that all rounding has been
+        // done correctly, and all OUSD tokens are fully backed.
+        require(_newTotalSupply >= actualSupply);
+
+        // Store supply.
+        _totalSupply = _newTotalSupply;
+
         emit TotalSupplyUpdated(
             _totalSupply,
             rebasingCredits,

contracts/test/vault/rebase.js

@@ -1,5 +1,5 @@
 const { defaultFixture } = require("../_fixture");
-const { expect } = require("chai");
+const { expect, assert } = require("chai");
 
 const {
   ousdUnits,
@@ -252,3 +252,42 @@ describe("Vault yield accrual to OGN", async () => {
     });
   });
 });
+
+describe("OUSD rebasing", async () => {
+  let fixture;
+
+  async function expectSolvantVault() {
+    const vaultValue = await fixture.vault.totalValue();
+    const ousdSupply = await fixture.ousd.totalSupply();
+    const rebaseRatio = await fixture.ousd.rebasingCreditsPer;
+    assert(ousdSupply <= vaultValue, "OUSD supply greater than vault value");
+  }
+
+  it("should correctly round rebases", async () => {
+    fixture = await loadFixture(defaultFixture);
+    const { matt, josh, dai, ousd, vault } = fixture;
+    await expectSolvantVault();
+
+    await dai.connect(matt).mint(daiUnits("1000000000000000000"));
+    await dai
+      .connect(matt)
+      .approve(vault.address, daiUnits("1000000000000000000"));
+    await vault
+      .connect(matt)
+      .mint(dai.address, daiUnits("1000000000000000000"), 0);
+    await expectSolvantVault();
+
+    await dai.mint(1);
+    await dai.transfer(vault.address, 1);
+    await vault.rebase();
+    await expectSolvantVault();
+
+    const whaleBalance = await ousd.balanceOf(matt.address);
+    await vault.connect(matt).redeem(whaleBalance, 0);
+    await expectSolvantVault();
+
+    const joshBalance = await ousd.balanceOf(josh.address);
+    await vault.connect(josh).redeem(joshBalance, 0);
+    await expectSolvantVault();
+  });
+});