Comet balance constraint + Constraint and Transfer scenarios + Comet …

…Solidity bug fix (#247) This PR includes the following changes: -Constraint for setting the Comet balance of an actor -More transfer scenarios and new set of scenarios for testing constraints -Fixing a Solidity underflow bug that was caught by the transfer scenarios -Librarifying some code between the BalanceConstraint and -CometBalanceConstraint Example of CometBalanceConstraint usage: ``` scenario( 'Comet#comet balance constraint', { cometBalances: { albert: { $base: 100, $asset0: 10 }, // in units of asset, not wei }, }, ```
compound-finance · Mar 21, 2022 · 71aec02 · 71aec02
1 parent 7bd91fb
commit 71aec02
Show file tree

Hide file tree

Showing 11 changed files with 487 additions and 103 deletions.
diff --git a/contracts/Comet.sol b/contracts/Comet.sol
@@ -990,8 +990,9 @@ contract Comet is CometCore {
         (uint104 withdrawAmount, uint104 borrowAmount) = withdrawAndBorrowAmount(srcBalance, amount);
         (uint104 repayAmount, uint104 supplyAmount) = repayAndSupplyAmount(dstBalance, amount);
 
-        totalSupplyBalance += supplyAmount - withdrawAmount;
-        totalBorrowBalance += borrowAmount - repayAmount;
+        // Note: Instead of `totalSupplyBalance += supplyAmount - withdrawAmount` to avoid underflow errors.
+        totalSupplyBalance = totalSupplyBalance + supplyAmount - withdrawAmount;
+        totalBorrowBalance = totalBorrowBalance + borrowAmount - repayAmount;
 
         srcBalance -= signed104(amount);
         dstBalance += signed104(amount);

diff --git a/scenario/ConstraintScenario.ts b/scenario/ConstraintScenario.ts
@@ -0,0 +1,68 @@
+import { scenario } from './context/CometContext';
+import { expect } from 'chai';
+import { expectApproximately } from './utils';
+
+scenario(
+  'Comet#constraint > collateral CometBalanceConstraint + BalanceConstraint both satisfied',
+  {
+    upgrade: true,
+    balances: {
+      albert: { $asset0: 100 }, // in units of asset, not wei
+    },
+    cometBalances: {
+      albert: { $asset0: 100 }, // in units of asset, not wei
+    },
+  },
+  async ({ comet, actors }, world, context) => {
+    const { albert } = actors;
+    const { asset: asset0Address, scale } = await comet.getAssetInfo(0);
+    const collateralAsset = context.getAssetByAddress(asset0Address);
+
+    expect(await collateralAsset.balanceOf(albert.address)).to.be.equal(scale.toBigInt() * 100n);
+    expect(await comet.collateralBalanceOf(albert.address, collateralAsset.address)).to.be.equal(scale.mul(100));
+  }
+);
+
+scenario(
+  'Comet#constraint > base CometBalanceConstraint + BalanceConstraint both satisfied',
+  {
+    upgrade: true,
+    balances: {
+      albert: { $base: 100 }, // in units of asset, not wei
+    },
+    cometBalances: {
+      albert: { $base: 100 }, // in units of asset, not wei
+    },
+  },
+  async ({ comet, actors }, world, context) => {
+    const { albert } = actors;
+    const baseAssetAddress = await comet.baseToken();
+    const baseAsset = context.getAssetByAddress(baseAssetAddress);
+    const scale = (await comet.baseScale()).toBigInt();
+
+    expect(await baseAsset.balanceOf(albert.address)).to.be.equal(100n * scale);
+    expect(await albert.getCometBaseBalance()).to.be.equal(100n * scale);
+  }
+);
+
+scenario(
+  'Comet#constraint > negative comet base balance (borrow position)',
+  {
+    upgrade: true,    
+    balances: {
+      albert: { $base: 100 }, // in units of asset, not wei
+    },
+    cometBalances: {
+      albert: { $base: -100 }, // in units of asset, not wei
+    },
+  },
+  async ({ comet, actors }, world, context) => {
+    const { albert } = actors;
+    const baseAssetAddress = await comet.baseToken();
+    const baseAsset = context.getAssetByAddress(baseAssetAddress);
+    const scale = (await comet.baseScale()).toBigInt();
+
+    expect(await baseAsset.balanceOf(albert.address)).to.be.equal(100n * scale);
+    expectApproximately(await albert.getCometBaseBalance(), -100n * scale, 1n);
+  }
+);
diff --git a/scenario/TransferScenario.ts b/scenario/TransferScenario.ts
@@ -1,5 +1,6 @@
 import { scenario } from './context/CometContext';
 import { expect } from 'chai';
+import { expectApproximately } from './utils';
 
 // XXX consider creating these tests for assets0-15
 scenario(
@@ -15,15 +16,15 @@ scenario(
     const { asset: asset0Address, scale } = await comet.getAssetInfo(0);
     const collateralAsset = context.getAssetByAddress(asset0Address);
 
-    expect(await collateralAsset.balanceOf(albert.address)).to.be.equal(scale.toBigInt() * 100n);
+    expect(await collateralAsset.balanceOf(albert.address)).to.be.equal(100n * scale.toBigInt());
 
     // Albert supplies 100 units of collateral to Comet
     await collateralAsset.approve(albert, comet.address);
-    await albert.supplyAsset({asset: collateralAsset.address, amount: scale.toBigInt() * 100n})
+    await albert.supplyAsset({ asset: collateralAsset.address, amount: 100n * scale.toBigInt() })
 
     // Albert transfers 50 units of collateral to Betty
     const toTransfer = scale.toBigInt() * 50n;
-    const txn = await albert.transferAsset({dst: betty.address, asset: collateralAsset.address, amount: toTransfer});
+    const txn = await albert.transferAsset({ dst: betty.address, asset: collateralAsset.address, amount: toTransfer });
 
     expect(await comet.collateralBalanceOf(albert.address, collateralAsset.address)).to.be.equal(scale.mul(50));
     expect(await comet.collateralBalanceOf(betty.address, collateralAsset.address)).to.be.equal(scale.mul(50));
@@ -44,22 +45,20 @@ scenario(
     const { albert, betty } = actors;
     const baseAssetAddress = await comet.baseToken();
     const baseAsset = context.getAssetByAddress(baseAssetAddress);
-    const scale = await comet.baseScale();
+    const scale = (await comet.baseScale()).toBigInt();
 
-    console.log('base asset is ', await baseAsset.token.symbol())
-
-    expect(await baseAsset.balanceOf(albert.address)).to.be.equal(scale.toBigInt() * 100n);
+    expect(await baseAsset.balanceOf(albert.address)).to.be.equal(100n * scale);
 
     // Albert supplies 100 units of collateral to Comet
     await baseAsset.approve(albert, comet.address);
-    await albert.supplyAsset({asset: baseAsset.address, amount: scale.toBigInt() * 100n})
+    await albert.supplyAsset({ asset: baseAsset.address, amount: 100n * scale })
 
     // Albert transfers 50 units of collateral to Betty
-    const toTransfer = scale.toBigInt() * 50n;
-    const txn = await albert.transferAsset({dst: betty.address, asset: baseAsset.address, amount: toTransfer});
+    const toTransfer = scale * 50n;
+    const txn = await albert.transferAsset({ dst: betty.address, asset: baseAsset.address, amount: toTransfer });
 
-    expect(await comet.balanceOf(albert.address)).to.be.equal(scale.mul(50));
-    expect(await comet.balanceOf(betty.address)).to.be.equal(scale.mul(50));
+    expect(await comet.balanceOf(albert.address)).to.be.equal(50n * scale);
+    expect(await comet.balanceOf(betty.address)).to.be.equal(50n * scale);
 
     return txn; // return txn to measure gas
   }
@@ -69,32 +68,131 @@ scenario(
   'Comet#transfer > partial withdraw / borrow base to partial repay / supply',
   {
     upgrade: true,
-    balances: {
-      // albert: { USDC: exp(50, 6) },
-      // betty: { USDC: exp(-50, 6) }
+    cometBalances: {
+      albert: { $base: 10, $asset0: 50 }, // in units of asset, not wei
+      betty: { $base: -10 },
+      charles: { $base: 1000 }, // to give the protocol enough base for others to borrow from
     },
   },
-  async ({ comet, actors }) => {
+  async ({ comet, actors }, world, context) => {
     const { albert, betty } = actors;
-    // XXX need comet balances constraint
-    //await albert.transferAsset(betty, USDC, exp(100, 6));
+    const baseAssetAddress = await comet.baseToken();
+    const baseAsset = context.getAssetByAddress(baseAssetAddress);
+    const scale = (await comet.baseScale()).toBigInt();
+    const precision = scale / 1_000_000n; // 1e-6 asset units of precision
+
+    expectApproximately(await albert.getCometBaseBalance(), 10n * scale, precision);
+    expectApproximately(await betty.getCometBaseBalance(), -10n * scale, precision);
+
+    // Albert with positive balance transfers to Betty with negative balance
+    const toTransfer = 15n * scale;
+    const txn = await albert.transferAsset({ dst: betty.address, asset: baseAsset.address, amount: toTransfer });
+
+    // Albert ends with negative balance and Betty with positive balance
+    expectApproximately(await albert.getCometBaseBalance(), -5n * scale, precision);
+    expectApproximately(await betty.getCometBaseBalance(), 5n * scale, precision);
+
+    return txn; // return txn to measure gas
   }
 );
 
 scenario(
   'Comet#transferFrom > withdraw to repay',
   {
     upgrade: true,
-    balances: {
-      // albert: { USDC: exp(100, 6) },
-      // betty: { USDC: exp(-100, 6) },
+    cometBalances: {
+      albert: { $base: 10, $asset0: 50 }, // in units of asset, not wei
+      betty: { $base: -10 },
+      charles: { $base: 1000 }, // to give the protocol enough base for others to borrow from
     },
   },
-  async ({ comet, actors }) => {
-    const { albert, betty, charles } = actors;
-    // XXX
-    //await albert.allow(charles, true);
-    //await charles.transferAssetFrom(albert, better, USDC, exp(100, 6));
+  async ({ comet, actors }, world, context) => {
+    const { albert, betty } = actors;
+    const baseAssetAddress = await comet.baseToken();
+    const baseAsset = context.getAssetByAddress(baseAssetAddress);
+    const scale = (await comet.baseScale()).toBigInt();
+    const precision = scale / 100_000n; // 1e-5 asset units of precision
+
+    expectApproximately(await albert.getCometBaseBalance(), 10n * scale, precision);
+    expectApproximately(await betty.getCometBaseBalance(), -10n * scale, precision);
+
+    await albert.allow(betty, true);
+
+    // Betty withdraws from Albert to repay her own borrows
+    const toTransfer = 5n * scale;
+    const txn = await betty.transferAssetFrom({ src: albert.address, dst: betty.address, asset: baseAsset.address, amount: toTransfer });
+
+    expectApproximately(await albert.getCometBaseBalance(), 5n * scale, precision);
+    expectApproximately(await betty.getCometBaseBalance(), -5n * scale, precision);
+
+    return txn; // return txn to measure gas
+  }
+);
+
+scenario(
+  'Comet#transfer reverts if undercollateralized',
+  {
+    upgrade: true,
+    cometBalances: {
+      albert: { $base: 100, $asset0: 0.000001 }, // in units of asset, not wei
+      betty: { $base: -100 },
+      charles: { $base: 1000 }, // to give the protocol enough base for others to borrow from
+    },
+  },
+  async ({ comet, actors }, world, context) => {
+    const { albert, betty } = actors;
+    const baseAssetAddress = await comet.baseToken();
+    const baseAsset = context.getAssetByAddress(baseAssetAddress);
+    const scale = (await comet.baseScale()).toBigInt();
+    const precision = scale / 1_000_000n; // 1e-6 asset units of precision
+
+    expectApproximately(await albert.getCometBaseBalance(), 100n * scale, precision);
+    expectApproximately(await betty.getCometBaseBalance(), -100n * scale, precision);
+
+    // Albert with positive balance transfers to Betty with negative balance
+    const toTransfer = 150n * scale;
+    await expect(
+      albert.transferAsset({
+        dst: betty.address,
+        asset: baseAsset.address,
+        amount: toTransfer,
+      })
+    ).to.be.revertedWith("custom error 'NotCollateralized()'");
+  }
+);
+
+scenario(
+  'Comet#transferFrom reverts if undercollateralized',
+  {
+    upgrade: true,
+    cometBalances: {
+      albert: { $base: 100, $asset0: 0.000001 }, // in units of asset, not wei
+      betty: { $base: -100 },
+      charles: { $base: 1000 }, // to give the protocol enough base for others to borrow from
+    },
+  },
+  async ({ comet, actors }, world, context) => {
+    const { albert, betty } = actors;
+    const baseAssetAddress = await comet.baseToken();
+    const baseAsset = context.getAssetByAddress(baseAssetAddress);
+    const scale = (await comet.baseScale()).toBigInt();
+    const precision = scale / 1_000_000n; // 1e-6 asset units of precision
+
+    expectApproximately(await albert.getCometBaseBalance(), 100n * scale, precision);
+    expectApproximately(await betty.getCometBaseBalance(), -100n * scale, precision);
+
+    await albert.allow(betty, true);
+
+    // Albert with positive balance transfers to Betty with negative balance
+    const toTransfer = 150n * scale;
+    await expect(
+      betty.transferAssetFrom({
+        src: albert.address,
+        dst: betty.address,
+        asset: baseAsset.address,
+        amount: toTransfer,
+      })
+    ).to.be.revertedWith("custom error 'NotCollateralized()'");
   }
 );