Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

delete baseBalanceOf #311

Merged
merged 12 commits into from
Apr 22, 2022
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);
kevincheng96 marked this conversation as resolved.
Show resolved Hide resolved
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);
kevincheng96 marked this conversation as resolved.
Show resolved Hide resolved

// 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;
kevincheng96 marked this conversation as resolved.
Show resolved Hide resolved
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> {
scott-silver marked this conversation as resolved.
Show resolved Hide resolved
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