Skip to content
This repository has been archived by the owner on Oct 6, 2023. It is now read-only.

Add some missing test cases to AccountsDepositWithdrawEndowments.. #401

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add some missing test cases to AccountsDepositWithdrawEndowments.. #401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
647e872
Add slither analysis step
0xNeshi Oct 4, 2023
c3a4a9a
Extract mature endow. set to 'before' hook in AccountsDepositWithdraw…
0xNeshi Oct 4, 2023
9de34a2
Add 'upon withdraw from non-mature endowments reverts if sender is no…
0xNeshi Oct 4, 2023
94f1ccd
Remove slither from ci.yml
0xNeshi Oct 4, 2023
21e9483
Add 'passes: Charity to a Charity Endowment transfer' test
0xNeshi Oct 4, 2023
a583be6
Extract temp charity in 'from Mature endowments'
0xNeshi Oct 4, 2023
c1ff29f
target es2020
0xNeshi Oct 4, 2023
20dfb7b
Use Allowlist enum in tests
0xNeshi Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions test/core/accounts/AccountsAllowance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
} from "typechain-types";
import {genWallet, getProxyAdminOwner, getSigners} from "utils";
import {deployFacetAsProxy} from "./utils";
import {AllowlistType} from "types";

describe("AccountsAllowance", function () {
const {ethers} = hre;
Expand Down Expand Up @@ -66,8 +67,14 @@ describe("AccountsAllowance", function () {
);

// Beneficiaries & Maturity Allowlists set for endowment user
await wait(state.setAllowlist(ACCOUNT_ID, 0, [await user.getAddress()])); // beneficiaries
await wait(state.setAllowlist(ACCOUNT_ID, 2, [await user.getAddress()])); // maturity
await wait(
state.setAllowlist(ACCOUNT_ID, AllowlistType.AllowlistedBeneficiaries, [
await user.getAddress(),
])
);
await wait(
state.setAllowlist(ACCOUNT_ID, AllowlistType.MaturityAllowlist, [await user.getAddress()])
);
});

let snapshot: SnapshotRestorer;
Expand Down
194 changes: 156 additions & 38 deletions test/core/accounts/AccountsDepositWithdrawEndowments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
import {AccountMessages} from "typechain-types/contracts/core/accounts/facets/AccountsDepositWithdrawEndowments";
import {RegistrarStorage} from "typechain-types/contracts/core/registrar/Registrar";
import {AccountStorage} from "typechain-types/contracts/test/accounts/TestFacetProxyContract";
import {VaultType} from "types";
import {AllowlistType, VaultType} from "types";
import {genWallet, getProxyAdminOwner, getSigners} from "utils";
import {deployFacetAsProxy} from "./utils";

Expand Down Expand Up @@ -512,10 +512,14 @@ describe("AccountsDepositWithdrawEndowments", function () {
// setup an allowed contributor wallet to sign/send
const allowedContributor = await genWallet().address;
await impersonateAccount(allowedContributor);
await setBalance(allowedContributor, 1000000000000000000); // give it some gas money, as impersonateAccount does not
await setBalance(allowedContributor, 1000000000000000000n); // give it some gas money, as impersonateAccount does not
Copy link
Contributor Author

@0xNeshi 0xNeshi Oct 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were warnings that such big numbers can't be represented as integers:

Numeric literals with absolute values equal to 2^53 or greater are too large to be represented accurately as integers
image

Switched them to BigInt just to be safe

const acctSigner = await ethers.getSigner(allowedContributor);
// set a different wallet in allowlist
await wait(state.setAllowlist(normalEndowId, 1, [genWallet().address]));
await wait(
state.setAllowlist(normalEndowId, AllowlistType.AllowlistedContributors, [
genWallet().address,
])
);

await expect(
facet
Expand All @@ -528,10 +532,14 @@ describe("AccountsDepositWithdrawEndowments", function () {
// setup an allowed contributor wallet to sign/send and setup in allowlist
const allowedContributor = await genWallet().address;
await impersonateAccount(allowedContributor);
await setBalance(allowedContributor, 1000000000000000000); // give it some gas money, as impersonateAccount does not
await setBalance(allowedContributor, 1000000000000000000n); // give it some gas money, as impersonateAccount does not
const acctSigner = await ethers.getSigner(allowedContributor);
// set new signer's wallet in allowlist
await wait(state.setAllowlist(normalEndowId, 1, [allowedContributor]));
await wait(
state.setAllowlist(normalEndowId, AllowlistType.AllowlistedContributors, [
allowedContributor,
])
);

const expectedLockedAmt = BigNumber.from(4000);
const expectedLiquidAmt = BigNumber.from(6000);
Expand Down Expand Up @@ -768,8 +776,22 @@ describe("AccountsDepositWithdrawEndowments", function () {
});

describe("from Non-Mature endowments", () => {
it("reverts if sender is not owner", async () => {
Copy link
Contributor Author

@0xNeshi 0xNeshi Oct 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added missing test cases (more below)...

We still have many test cases missing, so many of our code paths are not covered.

Tried adding solidity-coverage to our repo, but stumbled upon the the most common issue with it when using viaIR (we use it), which is sc-forks/solidity-coverage#715
If we remove viaIR to make coverage work, we get the "Stack Too Deep" error, which is just too big to tackle in this PR, so just removed the coverage tool for now (although it would be very useful to have it).

await expect(
facet
.connect(accOwner)
.withdraw(charityId, VaultType.LIQUID, genWallet().address, 0, [
{addr: tokenFake.address, amnt: 1},
])
).to.be.revertedWith("Unauthorized");
});

it("reverts if beneficiary address is not listed in allowlistedBeneficiaries nor is it the Endowment Owner", async () => {
await wait(state.setAllowlist(normalEndowId, 0, [genWallet().address]));
await wait(
state.setAllowlist(normalEndowId, AllowlistType.AllowlistedBeneficiaries, [
genWallet().address,
])
);

await expect(
facet.withdraw(normalEndowId, VaultType.LIQUID, genWallet().address, 0, [
Expand Down Expand Up @@ -804,7 +826,7 @@ describe("AccountsDepositWithdrawEndowments", function () {
it("reverts for a closed endowment (endow beneficiary) when a wallet that is not the beneficiary endowment owner tries to withdraw", async () => {
const beneficiaryAddress = genWallet().address;
await impersonateAccount(beneficiaryAddress);
await setBalance(beneficiaryAddress, 1000000000000000000); // give it some gas money, as impersonateAccount does not
await setBalance(beneficiaryAddress, 1000000000000000000n); // give it some gas money, as impersonateAccount does not
const acctSigner = await ethers.getSigner(beneficiaryAddress);

// setup new charity-type endow
Expand Down Expand Up @@ -834,7 +856,7 @@ describe("AccountsDepositWithdrawEndowments", function () {
it("passes: closed endowment with beneficiary address set, sender is beneficiary wallet, no fees applied ", async () => {
const beneficiaryAddress = genWallet().address;
await impersonateAccount(beneficiaryAddress);
await setBalance(beneficiaryAddress, 1000000000000000000); // give it some gas money, as impersonateAccount does not
await setBalance(beneficiaryAddress, 1000000000000000000n); // give it some gas money, as impersonateAccount does not
const acctSigner = await ethers.getSigner(beneficiaryAddress);
const beneficiaryId = 0;
const acctType = VaultType.LIQUID;
Expand Down Expand Up @@ -1144,7 +1166,11 @@ describe("AccountsDepositWithdrawEndowments", function () {
withdrawFee: {payoutAddress: await endowOwner.getAddress(), bps: 10}, // 0.1%
};
await wait(state.setEndowmentDetails(normalEndowId, normalWithAllowlist));
await wait(state.setAllowlist(normalEndowId, 0, [beneficiaryAddress]));
await wait(
state.setAllowlist(normalEndowId, AllowlistType.AllowlistedBeneficiaries, [
beneficiaryAddress,
])
);

await expect(
facet
Expand Down Expand Up @@ -1309,15 +1335,42 @@ describe("AccountsDepositWithdrawEndowments", function () {
});

describe("from Mature endowments", () => {
it("reverts if beneficiary address is not listed in maturityAllowlist", async () => {
const tempCharityId = 11;

let rootSnapshot: SnapshotRestorer;

before(async () => {
rootSnapshot = await takeSnapshot();

const currTime = await time.latest();

const endowment = await state.getEndowmentDetails(normalEndowId);
const charity = await state.getEndowmentDetails(charityId);
const matureEndowment: AccountStorage.EndowmentStruct = {
...normalEndow,
...endowment,
maturityTime: currTime,
};
const charityEndowment: AccountStorage.EndowmentStruct = {
...charity,
maturityTime: currTime,
};
await wait(state.setEndowmentDetails(normalEndowId, matureEndowment));
await wait(state.setAllowlist(normalEndowId, 2, [genWallet().address]));
await wait(state.setEndowmentDetails(charityId, charityEndowment));

// temp charity
await wait(state.setEndowmentDetails(tempCharityId, charity));
});

after(async () => {
await rootSnapshot.restore();
});

it("reverts if beneficiary address is not listed in maturityAllowlist", async () => {
await wait(
state.setAllowlist(normalEndowId, AllowlistType.MaturityAllowlist, [
await endowOwner.getAddress(),
])
);

const acctType = VaultType.LIQUID;
const beneficiaryId = 0;
Expand All @@ -1327,27 +1380,56 @@ describe("AccountsDepositWithdrawEndowments", function () {

await expect(
facet.withdraw(normalEndowId, acctType, beneficiaryAddress, beneficiaryId, tokens)
).to.be.revertedWith("Beneficiary address is not listed in maturityAllowlist");
});

it("reverts if maturity allowlist is empty", async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More missing test cases

await expect(
facet.withdraw(normalEndowId, VaultType.LIQUID, genWallet().address, 0, [
{addr: tokenFake.address, amnt: 1},
])
).to.be.revertedWith("Unauthorized");
});

it("reverts if owner is not in maturity allowlist", async () => {
await wait(
state.setAllowlist(normalEndowId, AllowlistType.MaturityAllowlist, [genWallet().address])
);
await expect(
facet.withdraw(normalEndowId, VaultType.LIQUID, genWallet().address, 0, [
{addr: tokenFake.address, amnt: 1},
])
).to.be.revertedWith("Unauthorized");
});

it("reverts if sender is not in maturity allowlist", async () => {
await wait(
state.setAllowlist(charityId, AllowlistType.MaturityAllowlist, [genWallet().address])
);
await expect(
facet
.connect(accOwner)
.withdraw(charityId, VaultType.LIQUID, genWallet().address, 0, [
{addr: tokenFake.address, amnt: 1},
])
).to.be.revertedWith("Unauthorized");
});

describe("LOCKED withdrawals", () => {
it("passes: Normal to Address (protocol-level normal fee only)", async () => {
const currTime = await time.latest();

registrarFake.getFeeSettingsByFeeType.returns({bps: 200, payoutAddress: treasury});

const matureEndowment: AccountStorage.EndowmentStruct = {
...normalEndow,
maturityTime: currTime,
};
await wait(state.setEndowmentDetails(normalEndowId, matureEndowment));
const beneficiarySigner = new ethers.Wallet(
genWallet().privateKey,
hre.ethers.provider
);
await accOwner.sendTransaction({value: ethers.utils.parseEther("1"), to: beneficiarySigner.address})
const beneficiarySigner = new ethers.Wallet(genWallet().privateKey, hre.ethers.provider);
await accOwner.sendTransaction({
value: ethers.utils.parseEther("1"),
to: beneficiarySigner.address,
});

await wait(state.setAllowlist(normalEndowId, 2, [beneficiarySigner.address]));
await wait(
state.setAllowlist(normalEndowId, AllowlistType.MaturityAllowlist, [
beneficiarySigner.address,
])
);

const acctType = VaultType.LOCKED;
const beneficiaryId = 0;
Expand All @@ -1359,7 +1441,9 @@ describe("AccountsDepositWithdrawEndowments", function () {
const finalAmountLeftover = amount.sub(expectedFeeAp);

await expect(
facet.connect(beneficiarySigner).withdraw(normalEndowId, acctType, beneficiarySigner.address, beneficiaryId, tokens)
facet
.connect(beneficiarySigner)
.withdraw(normalEndowId, acctType, beneficiarySigner.address, beneficiaryId, tokens)
)
.to.emit(facet, "EndowmentWithdraw")
.withArgs(
Expand All @@ -1385,28 +1469,22 @@ describe("AccountsDepositWithdrawEndowments", function () {
});

it("passes: Normal to a Charity Endowment transfer", async () => {
const currTime = await time.latest();

const matureEndowment: AccountStorage.EndowmentStruct = {
...normalEndow,
maturityTime: currTime,
};
await wait(state.setEndowmentDetails(normalEndowId, matureEndowment));
await wait(state.setAllowlist(normalEndowId, 2, [await indexFund.getAddress()]));
await wait(
state.setAllowlist(normalEndowId, AllowlistType.MaturityAllowlist, [
await endowOwner.getAddress(),
])
);

const acctType = VaultType.LOCKED;
const beneficiaryAddress = ethers.constants.AddressZero;
const beneficiaryId = charityId;
const tokens: IAccountsDepositWithdrawEndowments.TokenInfoStruct[] = [
{addr: tokenFake.address, amnt: 5000},
];
const regConfig = await registrarFake.queryConfig();
const amount = BigNumber.from(tokens[0].amnt);

await expect(
facet
.connect(indexFund)
.withdraw(normalEndowId, acctType, beneficiaryAddress, beneficiaryId, tokens)
facet.withdraw(normalEndowId, acctType, beneficiaryAddress, beneficiaryId, tokens)
)
.to.emit(facet, "EndowmentWithdraw")
.withArgs(
Expand All @@ -1431,6 +1509,46 @@ describe("AccountsDepositWithdrawEndowments", function () {
expect(lockBalBen).to.equal(lockBal.add(amount));
expect(liqBalBen).to.equal(liqBal);
});

it("passes: Charity to a Charity Endowment transfer", async () => {
const acctType = VaultType.LOCKED;
const beneficiaryAddress = ethers.constants.AddressZero;
const beneficiaryId = tempCharityId;
const tokens: IAccountsDepositWithdrawEndowments.TokenInfoStruct[] = [
{addr: tokenFake.address, amnt: 5000},
];
const amount = BigNumber.from(tokens[0].amnt);

// create charity endowment to act as beneficiary
await wait(
state.setAllowlist(charityId, AllowlistType.MaturityAllowlist, [
await endowOwner.getAddress(),
])
);

await expect(
facet.withdraw(charityId, acctType, beneficiaryAddress, beneficiaryId, tokens)
)
.to.emit(facet, "EndowmentWithdraw")
.withArgs(
charityId,
tokens[0].addr,
tokens[0].amnt,
acctType,
beneficiaryAddress,
beneficiaryId
);

const [lockedBalance] = await state.getEndowmentTokenBalance(charityId, tokens[0].addr);
expect(lockedBalance).to.equal(lockBal.sub(amount));

const [lockBalBen, liqBalBen] = await state.getEndowmentTokenBalance(
beneficiaryId,
tokens[0].addr
);
expect(lockBalBen).to.equal(amount);
expect(liqBalBen).to.equal(0);
});
});
});

Expand Down
9 changes: 7 additions & 2 deletions test/core/accounts/AccountsUpdateEndowmentSettingsController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
} from "typechain-types/contracts/test/accounts/TestFacetProxyContract";
import {genWallet, getProxyAdminOwner, getSigners} from "utils";
import {deployFacetAsProxy, updateAllSettings} from "./utils";
import {AllowlistType} from "types";

use(smock.matchers);

Expand Down Expand Up @@ -227,7 +228,9 @@ describe("AccountsUpdateEndowmentSettingsController", function () {
it("returns same members in starting allowlist if only pre-existing addresses are passed in add", async () => {
// set starting allowlist
const wallet = await genWallet().address;
await wait(state.setAllowlist(normalEndowId, 0, [wallet]));
await wait(
state.setAllowlist(normalEndowId, AllowlistType.AllowlistedBeneficiaries, [wallet])
);

const remove: any[] = [];
const add: any[] = [wallet];
Expand All @@ -245,7 +248,9 @@ describe("AccountsUpdateEndowmentSettingsController", function () {
const wallet3 = await genWallet().address;

// set starting allowlist
await wait(state.setAllowlist(normalEndowId, 0, [wallet]));
await wait(
state.setAllowlist(normalEndowId, AllowlistType.AllowlistedBeneficiaries, [wallet])
);

const remove: any[] = [wallet2];
const add: any[] = [wallet, wallet2, wallet3];
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"compilerOptions": {
"baseUrl": ".",
"target": "es2018",
"target": "ES2020",
Copy link
Contributor Author

@0xNeshi 0xNeshi Oct 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed to be able to use BigInts (see #401 (comment))

"module": "commonjs",
"strict": true,
"esModuleInterop": true,
Expand Down
6 changes: 6 additions & 0 deletions types/enums.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ export enum StrategyApprovalState {
DEPRECATED,
}

export enum AllowlistType {
AllowlistedBeneficiaries,
AllowlistedContributors,
MaturityAllowlist,
}

export enum VaultType {
LOCKED,
LIQUID,
Expand Down