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

VAL-113 Validate all "bps" variables #114

Merged
merged 6 commits into from
Nov 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions contracts/PoolFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,18 @@ contract PoolFactory is IPoolFactory {
),
"PoolFactory: Invalid first loss minimum"
);
require(
settings.withdrawGateBps <= 10_000,
"PoolFactory: Invalid withdraw gate"
);
require(
settings.requestFeeBps <= 10_000,
"PoolFactory: Invalid request fee"
);
require(
settings.requestCancellationFeeBps <= 10_000,
"PoolFactory: Invalid request cancellation fee"
);

// Create the pool
Pool pool = new Pool(
Expand Down
3 changes: 3 additions & 0 deletions contracts/controllers/PoolController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ contract PoolController is IPoolController {
onlyAdmin
atState(IPoolLifeCycleState.Initialized)
{
require(feeBps <= 10_000, "Pool: fee too large");
_settings.requestFeeBps = feeBps;
}

Expand All @@ -159,6 +160,7 @@ contract PoolController is IPoolController {
onlyAdmin
atState(IPoolLifeCycleState.Initialized)
{
require(feeBps <= 10_000, "Pool: fee too large");
_settings.requestCancellationFeeBps = feeBps;
}

Expand All @@ -184,6 +186,7 @@ contract PoolController is IPoolController {
onlyAdmin
atInitializedOrActiveState
{
require(_withdrawGateBps <= 10_000, "Pool: invalid bps");
_settings.withdrawGateBps = _withdrawGateBps;
}

Expand Down
48 changes: 48 additions & 0 deletions test/PoolFactory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,54 @@ describe("PoolFactory", () => {
).to.be.revertedWith("PoolFactory: Invalid first loss minimum");
});

it("reverts if withdraw gate is too large", async () => {
const { serviceConfiguration, poolFactory, liquidityAsset } =
await loadFixture(deployFixture);

// Set a first loss minimum
await serviceConfiguration.setFirstLossMinimum(liquidityAsset.address, 1);

// Attempt to create a pool with > 100% withdraw gate
const poolSettings = Object.assign({}, DEFAULT_POOL_SETTINGS, {
withdrawGateBps: 10_001
});
await expect(
poolFactory.createPool(liquidityAsset.address, poolSettings)
).to.be.revertedWith("PoolFactory: Invalid withdraw gate");
});

it("reverts if withdrawal request fee is too large", async () => {
const { serviceConfiguration, poolFactory, liquidityAsset } =
await loadFixture(deployFixture);

// Set a first loss minimum
await serviceConfiguration.setFirstLossMinimum(liquidityAsset.address, 1);

// Attempt to create a pool with > 100% withdraw gate
const poolSettings = Object.assign({}, DEFAULT_POOL_SETTINGS, {
requestFeeBps: 10_001
});
await expect(
poolFactory.createPool(liquidityAsset.address, poolSettings)
).to.be.revertedWith("PoolFactory: Invalid request fee");
});

it("reverts if withdrawal request cancellation fee is too large", async () => {
const { serviceConfiguration, poolFactory, liquidityAsset } =
await loadFixture(deployFixture);

// Set a first loss minimum
await serviceConfiguration.setFirstLossMinimum(liquidityAsset.address, 1);

// Attempt to create a pool with > 100% withdraw gate
const poolSettings = Object.assign({}, DEFAULT_POOL_SETTINGS, {
requestCancellationFeeBps: 10_001
});
await expect(
poolFactory.createPool(liquidityAsset.address, poolSettings)
).to.be.revertedWith("PoolFactory: Invalid request cancellation fee");
});

it("emits PoolCreated", async () => {
const { poolFactory, liquidityAsset } = await loadFixture(deployFixture);

Expand Down
26 changes: 26 additions & 0 deletions test/controllers/PoolController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ describe("PoolController", () => {
expect(settings.requestFeeBps).to.equal(1000);
});

it("prevents setting a value that's too large", async () => {
const { poolController, poolAdmin } = await loadFixture(loadPoolFixture);

await expect(
poolController.connect(poolAdmin).setRequestFee(10_001)
).to.be.revertedWith("Pool: fee too large");
});

it("does not let anyone except the admin to set the fee", async () => {
const { poolController, otherAccount } = await loadFixture(
loadPoolFixture
Expand Down Expand Up @@ -174,6 +182,14 @@ describe("PoolController", () => {
poolController.connect(poolAdmin).setRequestCancellationFee(10)
).to.be.revertedWith("Pool: FunctionInvalidAtThisLifeCycleState");
});

it("does not allow setting a request cancellation fee that's too large", async () => {
const { poolController, poolAdmin } = await loadFixture(loadPoolFixture);

await expect(
poolController.connect(poolAdmin).setRequestCancellationFee(10_001)
).to.be.revertedWith("Pool: fee too large");
});
});

describe("requestCancellationFee()", () => {
Expand Down Expand Up @@ -217,6 +233,16 @@ describe("PoolController", () => {
it("does not allow setting the request fee if the pool is paused", async () => {
// TODO: Pause pool
});

it("prevents setting a value too large ", async () => {
const { pool, poolController, poolAdmin, liquidityAsset } =
await loadFixture(loadPoolFixture);
await activatePool(pool, poolAdmin, liquidityAsset);

await expect(
poolController.connect(poolAdmin).setWithdrawGate(10_001)
).to.be.revertedWith("Pool: invalid bps");
});
});

describe("withdrawGate()", () => {
Expand Down