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

[Bug] Un-stick funds in FeeVault via mediated access through the controller #171

Merged
merged 2 commits into from
Feb 6, 2023
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/Pool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,18 @@ contract Pool is IPool, ERC20Upgradeable, BeaconImplementation {
IERC20Upgradeable(_liquidityAsset).safeTransfer(recipient, fixedFee);
}

/**
* @inheritdoc IPool
*/
function withdrawFeeVault(uint256 amount, address receiver)
external
onlyNotPaused
onlyPoolController
onlySnapshottedPool
{
_feeVault.withdrawERC20(address(_liquidityAsset), amount, receiver);
}

/*//////////////////////////////////////////////////////////////
Withdraw Controller Proxy Methods
//////////////////////////////////////////////////////////////*/
Expand Down
12 changes: 12 additions & 0 deletions contracts/controllers/PoolController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,18 @@ contract PoolController is IPoolController, BeaconImplementation {
);
}

/**
* @inheritdoc IPoolController
*/
function withdrawFeeVault(uint256 amount, address receiver)
external
onlyNotPaused
onlyPermittedAdmin
onlyAdmin
{
pool.withdrawFeeVault(amount, receiver);
}

/*//////////////////////////////////////////////////////////////
Snapshot
//////////////////////////////////////////////////////////////*/
Expand Down
6 changes: 6 additions & 0 deletions contracts/controllers/interfaces/IPoolController.sol
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,12 @@ interface IPoolController {
*/
function claimFixedFee() external;

/**
* @dev Called by the pool admin, this claims fees that have accumulated
* in the Pool's FeeVault from ongoing borrower payments.
*/
function withdrawFeeVault(uint256 amount, address receiver) external;

/*//////////////////////////////////////////////////////////////
Snapshot
//////////////////////////////////////////////////////////////*/
Expand Down
5 changes: 5 additions & 0 deletions contracts/interfaces/IPool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,11 @@ interface IPool is IERC4626, IRequestWithdrawable {
uint256
) external;

/**
* @dev Called by the Pool Controller, it withdraws from the FeeVault.
*/
function withdrawFeeVault(uint256 amount, address receiver) external;

/**
* @dev Calculate the total amount of underlying assets held by the vault,
* excluding any assets due for withdrawal.
Expand Down
104 changes: 104 additions & 0 deletions test/controllers/PoolController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,36 @@ describe("PoolController", () => {
};
}

async function loadPoolFixtureWithServiceFee() {
const { poolAdmin, pauser, borrower, otherAccount } =
await getCommonSigners();
const { pool, poolController, liquidityAsset, serviceConfiguration } =
await deployPool({
poolAdmin,
settings: { serviceFeeBps: 500 },
pauser
});

const { loan } = await deployLoan(
pool.address,
borrower.address,
liquidityAsset.address,
serviceConfiguration
);

return {
pool,
poolController,
liquidityAsset,
poolAdmin,
borrower,
otherAccount,
loan,
serviceConfiguration,
pauser
};
}

describe("setRequestFee()", () => {
it("sets the request fee in Bps", async () => {
const { poolController, poolAdmin } = await loadFixture(loadPoolFixture);
Expand Down Expand Up @@ -1382,6 +1412,80 @@ describe("PoolController", () => {
});
});

describe("withdraw accumulated fees", () => {
it("claiming fees is only available to the pool admin", async () => {
const { poolController, otherAccount } = await loadFixture(
loadPoolFixtureWithServiceFee
);

const tx = poolController
.connect(otherAccount)
.withdrawFeeVault(1, otherAccount.address);
await expect(tx).to.be.revertedWith("Pool: caller is not admin");
});

it("reverts if the protocol is paused", async () => {
const { poolController, poolAdmin, serviceConfiguration, pauser } =
await loadFixture(loadPoolFixtureWithServiceFee);

await serviceConfiguration.connect(pauser).setPaused(true);

const tx = poolController
.connect(poolAdmin)
.withdrawFeeVault(1, poolAdmin.address);
await expect(tx).to.be.revertedWith("Pool: Protocol paused");
});

it("can withdraw accumulated fees", async () => {
const {
pool,
loan,
borrower,
poolController,
otherAccount,
poolAdmin,
liquidityAsset
} = await loadFixture(loadPoolFixtureWithServiceFee);

// Make payments on a loan to accumulate fees in the feevault

// Fund loan + drawdown
await activatePool(pool, poolAdmin, liquidityAsset);
await depositToPool(pool, otherAccount, liquidityAsset, 1_000_000);
await fundLoan(loan, poolController, poolAdmin);
await loan.connect(borrower).drawdown(await loan.principal());

// Make first payment
await time.increase(await loan.paymentPeriod());

// 4166 interest payment
// with 5% service fee sliced off for the FeeVault == 208
await liquidityAsset.connect(borrower).approve(loan.address, 4166);
await loan.connect(borrower).completeNextPayment();

// Check that fees have accumulated...
const feeVaultAddr = await pool.feeVault();
const balance = await liquidityAsset.balanceOf(feeVaultAddr);
expect(balance).to.equal(208);

// Check that the PA can claim the fee
const txn = await poolController
.connect(poolAdmin)
.withdrawFeeVault(208, poolAdmin.address);
await expect(txn).to.not.be.reverted;

// Check that the fee moved from the vault to the PA
await expect(txn).to.changeTokenBalances(
liquidityAsset,
[poolAdmin.address, feeVaultAddr],
[+208, -208]
);

// Vault should now be empty
expect(await liquidityAsset.balanceOf(feeVaultAddr)).to.equal(0);
});
});

describe("Upgrades", () => {
it("Can be upgraded", async () => {
const { poolController, poolLib, poolControllerFactory, deployer } =
Expand Down