Skip to content

Commit

Permalink
delete baseBalanceOf (#311)
Browse files Browse the repository at this point in the history
* delete baseBalanceOf

* add baseBalanceOf helper

* update comet-ext-test

* update erc20-test

* update transfer-test

* update withdraw-test

* delete baseBalanceOf test

* use CometInterface as type for baseBalanceOf

* update scenarios

* restore withdraw-test; clean-up

* BigNumber instead of Number

* add BigNumber comment; reduce scale of negative comet base balance test
  • Loading branch information
scott-silver committed Apr 22, 2022
1 parent 4624e4b commit 7802bd0
Show file tree
Hide file tree
Showing 12 changed files with 63 additions and 68 deletions.
10 changes: 0 additions & 10 deletions contracts/CometExt.sol
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,6 @@ contract CometExt is CometCore {
return string(symbol_);
}

/**
* @notice Query the current base balance of an account
* @dev Note: does not update interest indices before calculating
* @param account The account whose balance to query
* @return The present day base balance of the account
*/
function baseBalanceOf(address account) external view returns (int104) {
return presentValue(userBasic[account].principal);
}

/**
* @notice Query the current collateral balance of an account
* @param account The account whose balance to query
Expand Down
1 change: 0 additions & 1 deletion contracts/CometInterface.sol
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ abstract contract CometInterface is CometCore, ERC20 {
function getBorrowRate() virtual external view returns (uint64);
function getUtilization() virtual external view returns (uint);

function baseBalanceOf(address account) virtual external view returns (int104);
function collateralBalanceOf(address account, address asset) virtual external view returns (uint128);
function borrowBalanceOf(address account) virtual external view returns (uint256);
function baseTrackingAccrued(address account) virtual external view returns (uint64);
Expand Down
8 changes: 6 additions & 2 deletions scenario/ConstraintScenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,18 @@ scenario(
albert: { $base: -100 }, // in units of asset, not wei
},
},
async ({ comet, actors }, world, context) => {
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);
expectApproximately(
await albert.getCometBaseBalance(),
-100n * scale,
(scale / 1000000n) // .000001 units of asset
);
}
);

Expand Down
12 changes: 6 additions & 6 deletions scenario/LiquidationScenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ timeElapsed = -liquidationMargin / (baseBalanceOf * price / baseScale) / (borrow
*/
async function timeUntilUnderwater({comet, actor, fudgeFactor = 0n}: {comet: CometInterface, actor: CometActor, fudgeFactor?: bigint}): Promise<number> {
const liquidationMargin = (await comet.getLiquidationMargin(actor.address)).toBigInt();
const baseBalanceOf = (await comet.baseBalanceOf(actor.address)).toBigInt();
const baseBalance = await actor.getCometBaseBalance();
const basePrice = (await comet.getPrice(await comet.baseTokenPriceFeed())).toBigInt();
const borrowRate = (await comet.getBorrowRate()).toBigInt();
const baseScale = (await comet.baseScale()).toBigInt();
Expand All @@ -24,7 +24,7 @@ async function timeUntilUnderwater({comet, actor, fudgeFactor = 0n}: {comet: Com

// XXX throw error if baseBalanceOf is positive and liquidationMargin is positive
return Number((-liquidationMargin * factorScale * baseScale /
(baseBalanceOf * basePrice) /
(baseBalance * basePrice) /
borrowRate) + fudgeFactor);
}

Expand Down Expand Up @@ -132,8 +132,8 @@ scenario(
expect(lp1.numAbsorbed.toNumber()).to.eq(lp0.numAbsorbed.toNumber() + 1);
// XXX test approxSpend?

const baseBalance = (await comet.baseBalanceOf(albert.address)).toNumber();
expect(baseBalance).to.be.greaterThanOrEqual(0);
const baseBalance = await albert.getCometBaseBalance();
expect(Number(baseBalance)).to.be.greaterThanOrEqual(0);

// clears out all of liquidated user's collateral
const numAssets = await comet.numAssets();
Expand All @@ -150,8 +150,8 @@ scenario(
// XXX Skipping temporarily because testnet is in a weird state where an EOA ('admin') still
// has permission to withdraw Comet's collateral, while Timelock does not. This is because the
// permission was set up in the initialize() function. There is currently no way to update this
// permission in Comet, so a new function (e.g. `approveCometPermission`) needs to be created
// to allow governance to modify which addresses can withdraw assets from Comet's Comet balance.
// permission in Comet, so a new function (e.g. `approveCometPermission`) needs to be created
// to allow governance to modify which addresses can withdraw assets from Comet's Comet balance.
scenario.skip(
'Comet#liquidation > governor can withdraw collateral after successful liquidation',
{
Expand Down
6 changes: 3 additions & 3 deletions scenario/WithdrawScenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ scenario(

expect(await baseAsset.balanceOf(albert.address)).to.be.equal(0n);
expect(await comet.balanceOf(albert.address)).to.be.equal(0n);

// Albert borrows 1 unit of base from Comet
const txn = await albert.withdrawAsset({ asset: baseAsset.address, amount: 1n * scale });

Expand Down Expand Up @@ -166,7 +166,7 @@ scenario(

expect(await baseAsset.balanceOf(betty.address)).to.be.equal(0n);
expect(await comet.balanceOf(albert.address)).to.be.equal(0n);

await albert.allow(betty, true);

// Betty borrows 1 unit of base using Albert's account
Expand Down Expand Up @@ -195,7 +195,7 @@ scenario(
const baseAssetAddress = await comet.baseToken();
const baseAsset = context.getAssetByAddress(baseAssetAddress);
const scale = (await comet.baseScale()).toBigInt();

// Betty borrows 1 unit of base using Albert's account
await expect(
betty.withdrawAssetFrom({
Expand Down
23 changes: 13 additions & 10 deletions scenario/constraints/CometBalanceConstraint.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { Constraint, Scenario, Solution, World } from '../../plugins/scenario';
import { Constraint, World } from '../../plugins/scenario';
import { CometContext } from '../context/CometContext';
import CometActor from '../context/CometActor';
import { expect } from 'chai';
import { Requirements } from './Requirements';
import { exp, factorScale } from '../../test/helpers';
import { baseBalanceOf, exp, factorScale } from '../../test/helpers';
import { ComparativeAmount, ComparisonOp, getAssetFromName, parseAmount, max, min } from '../utils';
import { BigNumber } from 'ethers';

async function borrowBase(borrowActor: CometActor, toBorrowBase: bigint, world: World, context: CometContext) {
const comet = await context.getComet();
Expand Down Expand Up @@ -77,7 +78,7 @@ export class CometBalanceConstraint<T extends CometContext, R extends Requiremen
toTransfer = max(exp(amount.val, decimals) - cometBalance, 0);
break;
case ComparisonOp.LTE:
// `toTransfer` should not be positive
// `toTransfer` should not be positive
toTransfer = min(exp(amount.val, decimals) - cometBalance, 0);
break;
case ComparisonOp.GT:
Expand Down Expand Up @@ -136,21 +137,23 @@ export class CometBalanceConstraint<T extends CometContext, R extends Requiremen
const amount = parseAmount(rawAmount);
const decimals = await asset.token.decimals();
const baseToken = await comet.baseToken();
let actualBalance;
let expectedBalance;
// Chai matchers (like `.to.be.at.most()`) only work for numbers and
// BigNumbers, so we convert from BigInt to BigNumber
let actualBalance: BigNumber;
let expectedBalance: BigNumber;
if (asset.address === baseToken) {
actualBalance = await comet.baseBalanceOf(actor.address);
actualBalance = BigNumber.from(await baseBalanceOf(comet, actor.address));
const baseIndexScale = (await comet.baseIndexScale()).toBigInt();
let baseIndex;
if (amount.val >= 0) {
baseIndex = (await comet.totalsBasic()).baseSupplyIndex.toBigInt();
} else {
baseIndex = (await comet.totalsBasic()).baseBorrowIndex.toBigInt();
}
expectedBalance = getExpectedBaseBalance(exp(amount.val, decimals), baseIndexScale, baseIndex);
expectedBalance = BigNumber.from(getExpectedBaseBalance(exp(amount.val, decimals), baseIndexScale, baseIndex));
} else {
actualBalance = await comet.collateralBalanceOf(actor.address, asset.address);
expectedBalance = exp(amount.val, decimals);
actualBalance = BigNumber.from(await comet.collateralBalanceOf(actor.address, asset.address));
expectedBalance = BigNumber.from(exp(amount.val, decimals));
}
switch (amount.op) {
case ComparisonOp.EQ:
Expand All @@ -159,7 +162,7 @@ export class CometBalanceConstraint<T extends CometContext, R extends Requiremen
case ComparisonOp.GTE:
expect(actualBalance).to.be.at.least(expectedBalance);
break;
case ComparisonOp.LTE:
case ComparisonOp.LTE:
expect(actualBalance).to.be.at.most(expectedBalance);
break;
case ComparisonOp.GT:
Expand Down
3 changes: 2 additions & 1 deletion scenario/context/CometActor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { BigNumberish, Signature, ethers, ContractReceipt } from 'ethers';
import { CometContext } from './CometContext';
import { AddressLike, resolveAddress } from './Address';
import { ERC20__factory } from '../../build/types';
import { baseBalanceOf } from '../../test/helpers';

const types = {
Authorization: [
Expand Down Expand Up @@ -54,7 +55,7 @@ export default class CometActor {

async getCometBaseBalance(): Promise<bigint> {
let comet = await this.context.getComet();
return (await comet.baseBalanceOf(this.signer.address)).toBigInt();
return baseBalanceOf(comet, this.signer.address);
}

async sendEth(recipient: AddressLike, amount: number) {
Expand Down
7 changes: 0 additions & 7 deletions test/comet-ext-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,6 @@ describe('CometExt', function () {
});
});

it('returns principal as baseBalanceOf', async () => {
await comet.setBasePrincipal(user.address, 100e6);

const baseBalanceOf = await comet.baseBalanceOf(user.address);
expect(baseBalanceOf).to.eq(200e6); // baseSupplyIndex = 2e15
});

it('returns collateralBalance (in units of the collateral asset)', async () => {
const { WETH } = tokens;

Expand Down
16 changes: 8 additions & 8 deletions test/erc20-test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ethers, event, expect, makeProtocol, setTotalsBasic, wait } from './helpers';
import { baseBalanceOf, ethers, event, expect, makeProtocol, setTotalsBasic, wait } from './helpers';

describe('erc20', function () {
it('has correct name', async () => {
Expand Down Expand Up @@ -69,7 +69,7 @@ describe('erc20', function () {
users: [alice, bob],
} = await makeProtocol();

expect(await comet.baseBalanceOf(bob.address)).to.eq(0);
expect(await baseBalanceOf(comet, bob.address)).to.eq(0n);

await comet.setBasePrincipal(alice.address, 50e6);
await setTotalsBasic(comet, {
Expand All @@ -78,8 +78,8 @@ describe('erc20', function () {

const tx = await wait(comet.connect(alice).transfer(bob.address, 100e6));

expect(await comet.baseBalanceOf(alice.address)).to.eq(0);
expect(await comet.baseBalanceOf(bob.address)).to.eq(100e6);
expect(await baseBalanceOf(comet, alice.address)).to.eq(0n);
expect(await baseBalanceOf(comet, bob.address)).to.eq(BigInt(100e6));
expect(event(tx, 0)).to.be.deep.equal({
Transfer: {
from: alice.address,
Expand All @@ -103,8 +103,8 @@ describe('erc20', function () {

await comet.connect(alice).transferFrom(alice.address, bob.address, 100e6)

expect(await comet.baseBalanceOf(alice.address)).to.eq(0);
expect(await comet.baseBalanceOf(bob.address)).to.eq(100e6);
expect(await baseBalanceOf(comet, alice.address)).to.eq(0n);
expect(await baseBalanceOf(comet, bob.address)).to.eq(BigInt(100e6));
});

it('reverts ERC20 transferFrom without approval', async () => {
Expand Down Expand Up @@ -139,8 +139,8 @@ describe('erc20', function () {
// bob can now transfer funds from alice
await comet.connect(bob).transferFrom(alice.address, bob.address, 100e6);

expect(await comet.baseBalanceOf(alice.address)).to.eq(0);
expect(await comet.baseBalanceOf(bob.address)).to.eq(100e6);
expect(await baseBalanceOf(comet, alice.address)).to.eq(0n);
expect(await baseBalanceOf(comet, bob.address)).to.eq(BigInt(100e6));
});

it('reverts ERC20 transferFrom with revoked approval', async () => {
Expand Down
27 changes: 17 additions & 10 deletions test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
Configurator,
Configurator__factory,
CometHarnessInterface,
CometInterface,
} from '../build/types';
import { BigNumber } from 'ethers';
import { TransactionReceipt, TransactionResponse } from '@ethersproject/abstract-provider';
Expand Down Expand Up @@ -427,15 +428,6 @@ export async function makeConfigurator(opts: ProtocolOpts = {}): Promise<Configu
};
}

type Portfolio = {
internal: {
[symbol: string]: BigInt,
},
external: {
[symbol: string]: BigInt,
}
}

export async function makeRewards(opts: RewardsOpts = {}): Promise<Rewards> {
const signers = await ethers.getSigners();

Expand Down Expand Up @@ -479,8 +471,23 @@ export function objectify(arrayObject) {
return obj;
}

export async function baseBalanceOf(comet: CometInterface, account: string): Promise<bigint> {
const balanceOf = await comet.balanceOf(account);
const borrowBalanceOf = await comet.borrowBalanceOf(account);
return balanceOf.sub(borrowBalanceOf).toBigInt();
}

type Portfolio = {
internal: {
[symbol: string]: BigInt,
},
external: {
[symbol: string]: BigInt,
}
}

export async function portfolio({ comet, base, tokens }, account): Promise<Portfolio> {
const internal = { [base]: BigInt(await comet.baseBalanceOf(account)) };
const internal = { [base]: await baseBalanceOf(comet, account) };
const external = { [base]: BigInt(await tokens[base].balanceOf(account)) };
for (const symbol in tokens) {
if (symbol != base) {
Expand Down
5 changes: 2 additions & 3 deletions test/transfer-test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { event, expect, exp, makeProtocol, portfolio, setTotalsBasic, wait } from './helpers';
import { BigNumber } from "ethers";
import { baseBalanceOf, event, expect, exp, makeProtocol, portfolio, setTotalsBasic, wait } from './helpers';

describe('transfer', function () {
it('transfers base from sender if the asset is base', async () => {
Expand Down Expand Up @@ -148,7 +147,7 @@ describe('transfer', function () {

const t1 = await comet.totalsBasic();

expect(await comet.baseBalanceOf(alice.address)).to.eq(-100e6);
expect(await baseBalanceOf(comet, alice.address)).to.eq(BigInt(-100e6));
});

it('cant borrow less than the minimum', async () => {
Expand Down
13 changes: 6 additions & 7 deletions test/withdraw-test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { BigNumber } from "ethers";
import { EvilToken, EvilToken__factory, FaucetToken } from '../build/types';
import { ethers, event, expect, exp, makeProtocol, portfolio, ReentryAttack, setTotalsBasic, wait } from './helpers';
import { baseBalanceOf, ethers, event, expect, exp, makeProtocol, portfolio, ReentryAttack, setTotalsBasic, wait } from './helpers';

describe('withdrawTo', function () {
it('withdraws base from sender if the asset is base', async () => {
Expand Down Expand Up @@ -207,7 +206,7 @@ describe('withdrawTo', function () {
const t1 = await comet.totalsBasic();
const baseIndexScale = await comet.baseIndexScale();

expect(await comet.baseBalanceOf(alice.address)).to.eq(-1e6);
expect(await baseBalanceOf(comet, alice.address)).to.eq(BigInt(-1e6));
expect(await USDC.balanceOf(bob.address)).to.eq(1e6);
});
});
Expand Down Expand Up @@ -332,9 +331,9 @@ describe('withdraw', function () {

// no USDC transferred
expect(await USDC.balanceOf(comet.address)).to.eq(100e6);
expect(await comet.baseBalanceOf(alice.address)).to.eq(0);
expect(await baseBalanceOf(comet, alice.address)).to.eq(0n);
expect(await USDC.balanceOf(alice.address)).to.eq(0);
expect(await comet.baseBalanceOf(bob.address)).to.eq(0);
expect(await baseBalanceOf(comet, bob.address)).to.eq(0n);
expect(await USDC.balanceOf(bob.address)).to.eq(0);
});

Expand Down Expand Up @@ -379,9 +378,9 @@ describe('withdraw', function () {

// no USDC transferred
expect(await USDC.balanceOf(comet.address)).to.eq(100e6);
expect(await comet.baseBalanceOf(alice.address)).to.eq(0);
expect(await baseBalanceOf(comet, alice.address)).to.eq(0n);
expect(await USDC.balanceOf(alice.address)).to.eq(0);
expect(await comet.baseBalanceOf(bob.address)).to.eq(0);
expect(await baseBalanceOf(comet, bob.address)).to.eq(0n);
expect(await USDC.balanceOf(bob.address)).to.eq(0);
});
});
Expand Down

0 comments on commit 7802bd0

Please sign in to comment.