diff --git a/contracts/Pool.sol b/contracts/Pool.sol index 35de2c07..71254113 100644 --- a/contracts/Pool.sol +++ b/contracts/Pool.sol @@ -96,6 +96,14 @@ contract Pool is IPool, ERC20 { _; } + /** + * @dev Modifier that can be overriden by derived classes to enforce + * access control. + */ + modifier onlyPermittedLender() virtual { + _; + } + /** * @dev Constructor for Pool * @param liquidityAsset asset held by the poo @@ -648,6 +656,7 @@ contract Pool is IPool, ERC20 { virtual override atState(IPoolLifeCycleState.Active) + onlyPermittedLender onlyCrankedPool returns (uint256 shares) { @@ -705,6 +714,7 @@ contract Pool is IPool, ERC20 { virtual override atState(IPoolLifeCycleState.Active) + onlyPermittedLender onlyCrankedPool returns (uint256 assets) { @@ -755,7 +765,13 @@ contract Pool is IPool, ERC20 { uint256 assets, address receiver, address owner - ) external virtual onlyCrankedPool returns (uint256 shares) { + ) + public + virtual + onlyPermittedLender + onlyCrankedPool + returns (uint256 shares) + { require(receiver == owner, "Pool: Withdrawal to unrelated address"); require(receiver == msg.sender, "Pool: Must transfer to msg.sender"); require(assets > 0, "Pool: 0 withdraw not allowed"); @@ -803,7 +819,13 @@ contract Pool is IPool, ERC20 { uint256 shares, address receiver, address owner - ) external virtual onlyCrankedPool returns (uint256 assets) { + ) + public + virtual + onlyPermittedLender + onlyCrankedPool + returns (uint256 assets) + { require(receiver == owner, "Pool: Withdrawal to unrelated address"); require(receiver == msg.sender, "Pool: Must transfer to msg.sender"); require(shares > 0, "Pool: 0 redeem not allowed"); diff --git a/contracts/permissioned/PermissionedPool.sol b/contracts/permissioned/PermissionedPool.sol index 3f3a1d42..abb6bdb0 100644 --- a/contracts/permissioned/PermissionedPool.sol +++ b/contracts/permissioned/PermissionedPool.sol @@ -18,7 +18,7 @@ contract PermissionedPool is Pool { /** * @dev a modifier to only allow valid lenders to perform an action */ - modifier onlyValidLender() { + modifier onlyPermittedLender() override { require( poolAccessControl.isValidParticipant(msg.sender), "caller is not a valid lender" @@ -26,17 +26,6 @@ contract PermissionedPool is Pool { _; } - /** - * @dev a modifier to only allow valid lenders to perform an action - */ - modifier onlyValidReceiver(address receiver) { - require( - poolAccessControl.isValidParticipant(receiver), - "receiver is not a valid lender" - ); - _; - } - /** * @dev The constructor for the PermissionedPool contract. It calls the * constructor of the Pool contract and then creates a new instance of the @@ -85,6 +74,8 @@ contract PermissionedPool is Pool { /** * @inheritdoc Pool + * @dev Since Pool does not enforce that msg.sender == receiver, we only + * check the receiver here. */ function maxDeposit(address receiver) public @@ -92,10 +83,7 @@ contract PermissionedPool is Pool { override returns (uint256) { - if ( - !poolAccessControl.isValidParticipant(msg.sender) || - !poolAccessControl.isValidParticipant(receiver) - ) { + if (!poolAccessControl.isValidParticipant(receiver)) { return 0; } @@ -104,63 +92,14 @@ contract PermissionedPool is Pool { /** * @inheritdoc Pool - */ - function deposit(uint256 assets, address receiver) - public - override - onlyValidLender - onlyValidReceiver(receiver) - returns (uint256 shares) - { - return super.deposit(assets, receiver); - } - - /** - * @inheritdoc Pool + * @dev Since Pool does not enforce that msg.sender == receiver, we only + * check the receiver here. */ function maxMint(address receiver) public view override returns (uint256) { - if ( - !poolAccessControl.isValidParticipant(msg.sender) || - !poolAccessControl.isValidParticipant(receiver) - ) { + if (!poolAccessControl.isValidParticipant(receiver)) { return 0; } return super.maxMint(receiver); } - - /** - * @inheritdoc Pool - */ - function mint(uint256 shares, address receiver) - public - override - onlyValidLender - onlyValidReceiver(receiver) - returns (uint256) - { - return super.mint(shares, receiver); - } - - /** - * @inheritdoc Pool - */ - function withdraw( - uint256, /* assets */ - address, /* receiver */ - address /* owner */ - ) external override onlyValidLender returns (uint256 shares) { - return 0; - } - - /** - * @inheritdoc Pool - */ - function redeem( - uint256, /* shares */ - address, /* receiver */ - address /* owner */ - ) external override onlyValidLender returns (uint256 assets) { - return 0; - } } diff --git a/test/Pool.test.ts b/test/Pool.test.ts index a8e6e442..b994ad57 100644 --- a/test/Pool.test.ts +++ b/test/Pool.test.ts @@ -1,7 +1,12 @@ import { time, loadFixture } from "@nomicfoundation/hardhat-network-helpers"; import { expect } from "chai"; import { ethers } from "hardhat"; -import { deployPool, depositToPool, activatePool } from "./support/pool"; +import { + deployPool, + depositToPool, + activatePool, + DEFAULT_POOL_SETTINGS +} from "./support/pool"; import { deployLoan, collateralizeLoan, fundLoan } from "./support/loan"; describe("Pool", () => { @@ -67,6 +72,47 @@ describe("Pool", () => { return { pool, otherAccount, loan, borrower, liquidityAsset }; } + describe("maxDeposit()", async () => { + it("returns 0 when pool is still initialized", async () => { + const { pool, otherAccount } = await loadFixture(loadPoolFixture); + expect(await pool.maxDeposit(otherAccount.address)).to.equal(0); + }); + + it("returns the full pool capacity when the pool is activated", async () => { + const { pool, poolAdmin, otherAccount, liquidityAsset } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + + expect(await pool.maxDeposit(otherAccount.address)).to.equal( + DEFAULT_POOL_SETTINGS.maxCapacity + ); + }); + + it("returns the full pool capacity less totalAvailableAssets", async () => { + const { pool, poolAdmin, otherAccount, liquidityAsset } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + const depositAmt = 10; + await depositToPool(pool, otherAccount, liquidityAsset, depositAmt); + + expect(await pool.maxDeposit(otherAccount.address)).to.equal( + DEFAULT_POOL_SETTINGS.maxCapacity - depositAmt + ); + }); + + it("returns 0 when pool is closed ", async () => { + const { pool, poolAdmin, otherAccount, liquidityAsset } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + const { endDate } = await pool.settings(); + await time.increaseTo(endDate); + expect(await pool.maxDeposit(otherAccount.address)).to.equal(0); + }); + }); + describe("deposit()", async () => { it("deposit cannot be called if pool is initialized", async () => { const { pool, otherAccount } = await loadFixture(loadPoolFixture); @@ -163,6 +209,47 @@ describe("Pool", () => { }); }); + describe("maxMint()", async () => { + it("returns 0 when pool is still initialized", async () => { + const { pool, otherAccount } = await loadFixture(loadPoolFixture); + expect(await pool.maxMint(otherAccount.address)).to.equal(0); + }); + + it("returns the full pool capacity when the pool is activated", async () => { + const { pool, poolAdmin, otherAccount, liquidityAsset } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + + expect(await pool.maxMint(otherAccount.address)).to.equal( + DEFAULT_POOL_SETTINGS.maxCapacity + ); + }); + + it("returns the full pool capacity less totalAvailableAssets", async () => { + const { pool, poolAdmin, otherAccount, liquidityAsset } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + const depositAmt = 10; + await depositToPool(pool, otherAccount, liquidityAsset, depositAmt); + + expect(await pool.maxMint(otherAccount.address)).to.equal( + DEFAULT_POOL_SETTINGS.maxCapacity - depositAmt + ); + }); + + it("returns 0 when pool is closed ", async () => { + const { pool, poolAdmin, otherAccount, liquidityAsset } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + const { endDate } = await pool.settings(); + await time.increaseTo(endDate); + expect(await pool.maxMint(otherAccount.address)).to.equal(0); + }); + }); + describe("mint()", async () => { it("mint cannot be called if pool is initialized", async () => { const { pool, otherAccount } = await loadFixture(loadPoolFixture); diff --git a/test/permissioned/PermissionedPool.test.ts b/test/permissioned/PermissionedPool.test.ts index a5a7bce5..7e36fc6e 100644 --- a/test/permissioned/PermissionedPool.test.ts +++ b/test/permissioned/PermissionedPool.test.ts @@ -1,7 +1,13 @@ import { time, loadFixture } from "@nomicfoundation/hardhat-network-helpers"; import { expect } from "chai"; import { ethers } from "hardhat"; -import { activatePool, deployPermissionedPool } from "../support/pool"; +import { + activatePool, + DEFAULT_POOL_SETTINGS, + deployPermissionedPool, + depositToPool, + progressWithdrawWindow +} from "../support/pool"; describe("PermissionedPool", () => { async function loadPoolFixture() { @@ -27,6 +33,7 @@ describe("PermissionedPool", () => { return { pool, poolController, + poolAccessControl, liquidityAsset, poolAdmin, otherAccount, @@ -36,92 +43,208 @@ describe("PermissionedPool", () => { } describe("maxMint()", async () => { - it("returns 0 if lender or receiver not in access control list", async () => { - const { pool, otherAccount, thirdAccount } = await loadFixture( - loadPoolFixture - ); + it("returns 0 if lender not in access control list", async () => { + const { pool, poolAdmin, liquidityAsset, otherAccount } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); expect( - await pool.connect(otherAccount).maxMint(thirdAccount.address) + await pool.connect(otherAccount).maxMint(otherAccount.address) ).to.equal(0); }); + + it("returns a real value if lender is on access control list", async () => { + const { pool, poolAdmin, allowedLender, liquidityAsset } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + + expect( + await pool.connect(allowedLender).maxMint(allowedLender.address) + ).to.equal(DEFAULT_POOL_SETTINGS.maxCapacity); + }); }); describe("maxDeposit()", async () => { it("returns 0 if lender or receiver not in access control list", async () => { - const { pool, otherAccount, thirdAccount } = await loadFixture( - loadPoolFixture - ); + const { pool, poolAdmin, otherAccount, liquidityAsset } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); expect( - await pool.connect(otherAccount).maxMint(thirdAccount.address) + await pool.connect(otherAccount).maxMint(otherAccount.address) ).to.equal(0); }); + + it("returns a real value if lender is on access control list", async () => { + const { pool, poolAdmin, allowedLender, liquidityAsset } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + + expect( + await pool.connect(allowedLender).maxMint(allowedLender.address) + ).to.equal(DEFAULT_POOL_SETTINGS.maxCapacity); + }); }); - describe("Permissions", () => { - describe("deposit()", () => { - it("reverts if not allowed lender", async () => { - const { pool, otherAccount, thirdAccount } = await loadFixture( - loadPoolFixture - ); + describe("deposit()", () => { + it("reverts if not allowed lender", async () => { + const { pool, poolAdmin, liquidityAsset, otherAccount } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + + await expect( + pool.connect(otherAccount).deposit(10, otherAccount.address) + ).to.be.revertedWith("caller is not a valid lender"); + }); + + it("allows deposits if allowed lender", async () => { + const { pool, poolAdmin, liquidityAsset, allowedLender } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); - await expect( - pool.connect(otherAccount).deposit(10, thirdAccount.address) - ).to.be.revertedWith("caller is not a valid lender"); - }); + await liquidityAsset.mint(allowedLender.address, 10); + await liquidityAsset.connect(allowedLender).approve(pool.address, 10); + + await expect( + pool.connect(allowedLender).deposit(10, allowedLender.address) + ).to.emit(pool, "Deposit"); }); + }); - describe("mint()", () => { - it("reverts if not allowed lender", async () => { - const { pool, otherAccount, thirdAccount } = await loadFixture( - loadPoolFixture - ); + describe("mint()", () => { + it("reverts if not allowed lender", async () => { + const { pool, poolAdmin, liquidityAsset, otherAccount } = + await loadFixture(loadPoolFixture); - await expect( - pool.connect(otherAccount).mint(10, thirdAccount.address) - ).to.be.revertedWith("caller is not a valid lender"); - }); + await activatePool(pool, poolAdmin, liquidityAsset); + + await expect( + pool.connect(otherAccount).mint(10, otherAccount.address) + ).to.be.revertedWith("caller is not a valid lender"); }); - describe("crank()", () => { - it("reverts if not allowed lender or admin", async () => { - const { pool, otherAccount } = await loadFixture(loadPoolFixture); + it("allows minting if allowed lender", async () => { + const { pool, poolAdmin, liquidityAsset, allowedLender } = + await loadFixture(loadPoolFixture); - await expect(pool.connect(otherAccount).crank()).to.be.revertedWith( - "Pool: not allowed" - ); - }); + await activatePool(pool, poolAdmin, liquidityAsset); - it("cranks the pool if allowed lender", async () => { - const { pool, poolAdmin, allowedLender, liquidityAsset } = - await loadFixture(loadPoolFixture); + await liquidityAsset.mint(allowedLender.address, 10); + await liquidityAsset.connect(allowedLender).approve(pool.address, 10); - await activatePool(pool, poolAdmin, liquidityAsset); + await expect( + pool.connect(allowedLender).mint(10, allowedLender.address) + ).to.emit(pool, "Deposit"); + }); + }); + + describe("redeem()", () => { + it("reverts if not allowed lender", async () => { + const { pool, poolAdmin, liquidityAsset, otherAccount } = + await loadFixture(loadPoolFixture); - const { withdrawRequestPeriodDuration } = await pool.settings(); - await time.increase(withdrawRequestPeriodDuration); + await activatePool(pool, poolAdmin, liquidityAsset); - await expect(pool.connect(allowedLender).crank()).to.emit( - pool, - "PoolCranked" - ); - }); + await expect( + pool + .connect(otherAccount) + .redeem(10, otherAccount.address, otherAccount.address) + ).to.be.revertedWith("caller is not a valid lender"); + }); - it("cranks the pool if PA via poolController", async () => { - const { pool, poolAdmin, poolController, liquidityAsset } = - await loadFixture(loadPoolFixture); + it("allows redeeming if allowed lender", async () => { + const { pool, poolAdmin, liquidityAsset, allowedLender } = + await loadFixture(loadPoolFixture); - await activatePool(pool, poolAdmin, liquidityAsset); + await activatePool(pool, poolAdmin, liquidityAsset); + await depositToPool(pool, allowedLender, liquidityAsset, 10); - const { withdrawRequestPeriodDuration } = await pool.settings(); - await time.increase(withdrawRequestPeriodDuration); + await pool.connect(allowedLender).requestRedeem(5); + await progressWithdrawWindow(pool); - await expect(poolController.connect(poolAdmin).crank()).to.emit( - pool, - "PoolCranked" - ); - }); + await expect( + pool + .connect(allowedLender) + .redeem(1, allowedLender.address, allowedLender.address) + ).to.emit(pool, "Withdraw"); + }); + }); + + describe("withdraw()", () => { + it("reverts if not allowed lender", async () => { + const { pool, poolAdmin, liquidityAsset, otherAccount } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + + await expect( + pool + .connect(otherAccount) + .withdraw(10, otherAccount.address, otherAccount.address) + ).to.be.revertedWith("caller is not a valid lender"); + }); + + it("allows withdrawing if allowed lender", async () => { + const { pool, poolAdmin, liquidityAsset, allowedLender } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + await depositToPool(pool, allowedLender, liquidityAsset, 10); + + await pool.connect(allowedLender).requestRedeem(5); + await progressWithdrawWindow(pool); + + await expect( + pool + .connect(allowedLender) + .withdraw(1, allowedLender.address, allowedLender.address) + ).to.emit(pool, "Withdraw"); + }); + }); + + describe("crank()", () => { + it("reverts if not allowed lender or admin", async () => { + const { pool, otherAccount } = await loadFixture(loadPoolFixture); + + await expect(pool.connect(otherAccount).crank()).to.be.revertedWith( + "Pool: not allowed" + ); + }); + + it("cranks the pool if allowed lender", async () => { + const { pool, poolAdmin, allowedLender, liquidityAsset } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + + const { withdrawRequestPeriodDuration } = await pool.settings(); + await time.increase(withdrawRequestPeriodDuration); + + await expect(pool.connect(allowedLender).crank()).to.emit( + pool, + "PoolCranked" + ); + }); + + it("cranks the pool if PA via poolController", async () => { + const { pool, poolAdmin, poolController, liquidityAsset } = + await loadFixture(loadPoolFixture); + + await activatePool(pool, poolAdmin, liquidityAsset); + + const { withdrawRequestPeriodDuration } = await pool.settings(); + await time.increase(withdrawRequestPeriodDuration); + + await expect(poolController.connect(poolAdmin).crank()).to.emit( + pool, + "PoolCranked" + ); }); }); }); diff --git a/test/support/pool.ts b/test/support/pool.ts index 546a0519..693c8ee8 100644 --- a/test/support/pool.ts +++ b/test/support/pool.ts @@ -1,4 +1,5 @@ import { ethers } from "hardhat"; +import { time } from "@nomicfoundation/hardhat-network-helpers"; import { MockERC20, Pool } from "../../typechain-types"; import { deployMockERC20 } from "./erc20"; import { @@ -312,3 +313,8 @@ export async function deployPoolControllerFactory( const factory = await Factory.deploy(serviceConfigAddress); return factory.deployed(); } + +export async function progressWithdrawWindow(pool: any) { + const { withdrawRequestPeriodDuration } = await pool.settings(); + await time.increase(withdrawRequestPeriodDuration); +}