From f7847f9235e8215204e8e221268f1d9f47552ad7 Mon Sep 17 00:00:00 2001 From: Tom Linton Date: Tue, 15 Sep 2020 12:45:52 +1200 Subject: [PATCH 1/2] Fix decimals on ratio outputs --- contracts/contracts/vault/Vault.sol | 31 ++++++++++++++++++----------- contracts/test/vault/redeem.js | 31 +++++++++++++++++++++-------- 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/contracts/contracts/vault/Vault.sol b/contracts/contracts/vault/Vault.sol index e7e1a660ca..8b3677d487 100644 --- a/contracts/contracts/vault/Vault.sol +++ b/contracts/contracts/vault/Vault.sol @@ -1,5 +1,4 @@ pragma solidity 0.5.11; - /* The Vault contract stores assets. On a deposit, OUSD will be minted and sent to the depositor. On a withdrawal, OUSD will be burned and assets will be sent to @@ -594,6 +593,7 @@ contract Vault is Initializable, InitializableGovernable { internal returns (uint256[] memory outputs) { + uint256 totalBalance = _checkBalance(); uint256 totalOutputValue = 0; // Running total of USD value of assets uint256 combinedAssetValue = 0; uint256 assetCount = getAssetCount(); @@ -602,20 +602,22 @@ contract Vault is Initializable, InitializableGovernable { // Initialise arrays // Price of each asset in USD in 1e18 uint256[] memory assetPrices = new uint256[](assetCount); + uint256[] memory assetDecimals = new uint256[](assetCount); outputs = new uint256[](assetCount); for (uint256 i = 0; i < allAssets.length; i++) { - uint256 assetDecimals = Helpers.getDecimals(allAssets[i]); + assetDecimals[i] = Helpers.getDecimals(allAssets[i]); // Get all the USD prices of the asset in 1e18 assetPrices[i] = _priceUSDMax( allAssets[i], - uint256(1).scaleBy(int8(assetDecimals)) + uint256(1).scaleBy(int8(assetDecimals[i])) ); + // Get the proportional amount of this token for the redeem in 1e18 uint256 proportionalAmount = _checkBalance(allAssets[i]) - .scaleBy(int8(18 - assetDecimals)) + .scaleBy(int8(18 - assetDecimals[i])) .mul(_amount) - .div(_checkBalance()); + .div(totalBalance); if (proportionalAmount > 0) { // Non zero output means this asset is contributing to the @@ -629,7 +631,7 @@ contract Vault is Initializable, InitializableGovernable { ); // Save the output amount in the decimals of the asset outputs[i] = proportionalAmount.scaleBy( - int8(assetDecimals - 18) + int8(assetDecimals[i] - 18) ); } } @@ -642,14 +644,19 @@ contract Vault is Initializable, InitializableGovernable { for (uint256 i = 0; i < outputs.length; i++) { if (outputs[i] == 0) continue; + uint256 adjustment = 0; if (outputValueDiff < 0) { - outputs[i] -= uint256(-outputValueDiff) - .divPrecisely(combinedAssetValue) - .div(redeemAssetCount); + adjustment = uint256(-outputValueDiff) + .div(combinedAssetValue) + .divPrecisely(redeemAssetCount) + .scaleBy(int8(assetDecimals[i] - 18)); + outputs[i] -= adjustment; } else if (outputValueDiff > 0) { - outputs[i] += uint256(outputValueDiff) - .divPrecisely(combinedAssetValue) - .div(redeemAssetCount); + adjustment = uint256(outputValueDiff) + .div(combinedAssetValue) + .divPrecisely(redeemAssetCount) + .scaleBy(int8(assetDecimals[i] - 18)); + outputs[i] += adjustment; } } } diff --git a/contracts/test/vault/redeem.js b/contracts/test/vault/redeem.js index 8049920dc0..f1fe4326b2 100644 --- a/contracts/test/vault/redeem.js +++ b/contracts/test/vault/redeem.js @@ -143,10 +143,10 @@ describe("Vault Redeem", function () { await vault.connect(anna).redeemAll(); // 100 USDC and 350 DAI in contract - // 100/450 * 250 USDC + existing 1000 - // 350/450 * 250 DAI + existing 1000 - await expect(anna).has.an.approxBalanceOf("1044.44", dai); + // (1000-100) + 100/450 * 250 USDC + // (1000-150) + 350/450 * 250 DAI await expect(anna).has.an.approxBalanceOf("955.55", usdc); + await expect(anna).has.an.approxBalanceOf("1044.44", dai); }); it("Should redeem entire OUSD balance, with a higher oracle price", async () => { @@ -166,18 +166,33 @@ describe("Vault Redeem", function () { await vault.connect(anna).mint(dai.address, daiUnits("150.0")); await expect(anna).has.a.balanceOf("250.00", ousd); - await setOracleTokenPriceUsd("DAI", "1.20"); await setOracleTokenPriceUsd("USDC", "1.30"); + await setOracleTokenPriceUsd("DAI", "1.20"); await vault.connect(governor).rebase(); + // Anna's share of OUSD is 200/450 + // Vault has 100 USDC and 350 DAI total + // 200/450 * 100 * 1.3 + 350 * 1.2 + await expect(anna).has.an.approxBalanceOf("305.55", ousd); + // Withdraw all await ousd.connect(anna).approve(vault.address, ousdUnits("250.0")); await vault.connect(anna).redeemAll(); // 100 USDC and 350 DAI in contract - // 100/450 * 250 USDC + existing 1000 - // 350/450 * 250 DAI + existing 1000 - await expect(anna).has.an.approxBalanceOf("1", dai); // To be mathed out - await expect(anna).has.an.approxBalanceOf("1", usdc); // To be mathed out + // 100 * 1.30 = 130 USD value for USDC + // 350 * 1.20 = 420 USD value for DAI + // 100/450 * 305.5555555 = 67.9012345556 USDC + // 350/450 * 305.5555555 = 237.654320944 DAI + // Difference between output value and redeem value is: + // 305.5555555 - (67.9012345556 * 1.3 + 237.654320944 * 1.20) = -67.9012345551 + // Remove 67.9012345551/2.5/2 from USDC + // Remove 67.9012345551/2.5/2 from DAI + // (1000-100) + 100/450 * 305.5555555 - 67.9012345551/2.5/2 USDC + // (1000-150) + 350/450 * 305.5555555 - 67.9012345551/2.5/2 DAI + await expect(anna).has.an.approxBalanceOf("954.32", usdc); // To be mathed out + await expect(anna).has.an.approxBalanceOf("1074.07", dai); // To be mathed out }); + + it("Should redeem entire OUSD balance, with a lower oracle price"); }); From 45905f5afa12362e7c17054440fb63cd634149b3 Mon Sep 17 00:00:00 2001 From: Tom Linton Date: Tue, 15 Sep 2020 16:22:02 +1200 Subject: [PATCH 2/2] Fix divisor --- contracts/contracts/vault/Vault.sol | 8 ++++---- contracts/test/vault/redeem.js | 10 ++++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/contracts/contracts/vault/Vault.sol b/contracts/contracts/vault/Vault.sol index 8b3677d487..744d10c339 100644 --- a/contracts/contracts/vault/Vault.sol +++ b/contracts/contracts/vault/Vault.sol @@ -647,14 +647,14 @@ contract Vault is Initializable, InitializableGovernable { uint256 adjustment = 0; if (outputValueDiff < 0) { adjustment = uint256(-outputValueDiff) - .div(combinedAssetValue) - .divPrecisely(redeemAssetCount) + .divPrecisely(combinedAssetValue) + .div(redeemAssetCount) .scaleBy(int8(assetDecimals[i] - 18)); outputs[i] -= adjustment; } else if (outputValueDiff > 0) { adjustment = uint256(outputValueDiff) - .div(combinedAssetValue) - .divPrecisely(redeemAssetCount) + .divPrecisely(combinedAssetValue) + .div(redeemAssetCount) .scaleBy(int8(assetDecimals[i] - 18)); outputs[i] += adjustment; } diff --git a/contracts/test/vault/redeem.js b/contracts/test/vault/redeem.js index f1fe4326b2..d974973237 100644 --- a/contracts/test/vault/redeem.js +++ b/contracts/test/vault/redeem.js @@ -34,12 +34,18 @@ describe("Vault Redeem", function () { it("Should allow a redeem at different asset prices", async () => { const { ousd, vault, dai, matt } = await loadFixture(defaultFixture); - await expect(matt).has.a.balanceOf("100.00", ousd, "starting balance"); + await expect(matt).has.a.balanceOf( + "100.00", + ousd, + "Matt has incorrect starting balance" + ); await expect(matt).has.a.balanceOf("900.00", dai); expect(await ousd.totalSupply()).to.eq(ousdUnits("200.0")); - // Intentionaly skipping the rebase after the price change, + // Intentionally skipping the rebase after the price change, // to watch it happen automatically await setOracleTokenPriceUsd("DAI", "1.50"); + + // 200 DAI in Vault, change in price means vault value is $300 await vault.connect(matt).redeem(ousdUnits("2.0")); // with DAI now worth $1.5, we should only get 1.33 DAI for our two OUSD. await expect(matt).has.a.approxBalanceOf("901.33", dai);