From 49eb02705b639c55b24146195cbdbc5d3ec06607 Mon Sep 17 00:00:00 2001 From: Adrian Soghoian Date: Wed, 30 Nov 2022 11:16:52 -0500 Subject: [PATCH 1/5] Add withdraw / redeem methods and tests to PermissionedPool --- contracts/Pool.sol | 4 +- contracts/permissioned/PermissionedPool.sol | 44 ++-- test/Pool.test.ts | 89 +++++++- test/permissioned/PermissionedPool.test.ts | 239 +++++++++++++++----- test/support/pool.ts | 6 + 5 files changed, 301 insertions(+), 81 deletions(-) diff --git a/contracts/Pool.sol b/contracts/Pool.sol index 35de2c07..0e4fc6f4 100644 --- a/contracts/Pool.sol +++ b/contracts/Pool.sol @@ -755,7 +755,7 @@ contract Pool is IPool, ERC20 { uint256 assets, address receiver, address owner - ) external virtual onlyCrankedPool returns (uint256 shares) { + ) public virtual 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 +803,7 @@ contract Pool is IPool, ERC20 { uint256 shares, address receiver, address owner - ) external virtual onlyCrankedPool returns (uint256 assets) { + ) public virtual 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..e5bd9d9c 100644 --- a/contracts/permissioned/PermissionedPool.sol +++ b/contracts/permissioned/PermissionedPool.sol @@ -85,6 +85,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 +94,7 @@ contract PermissionedPool is Pool { override returns (uint256) { - if ( - !poolAccessControl.isValidParticipant(msg.sender) || - !poolAccessControl.isValidParticipant(receiver) - ) { + if (!poolAccessControl.isValidParticipant(receiver)) { return 0; } @@ -104,12 +103,13 @@ contract PermissionedPool is Pool { /** * @inheritdoc Pool + * @dev Since Pool enforces that msg.sender must be receiver, we only check + * the sender here. */ function deposit(uint256 assets, address receiver) public override onlyValidLender - onlyValidReceiver(receiver) returns (uint256 shares) { return super.deposit(assets, receiver); @@ -117,12 +117,11 @@ contract PermissionedPool is Pool { /** * @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; } @@ -131,12 +130,13 @@ contract PermissionedPool is Pool { /** * @inheritdoc Pool + * @dev Since Pool enforces that msg.sender must be receiver, we only check + * the sender here. */ function mint(uint256 shares, address receiver) public override onlyValidLender - onlyValidReceiver(receiver) returns (uint256) { return super.mint(shares, receiver); @@ -144,23 +144,27 @@ contract PermissionedPool is Pool { /** * @inheritdoc Pool + * @dev Since Pool enforces that msg.sender == receiver == owner, we + * only check the sender here. */ function withdraw( - uint256, /* assets */ - address, /* receiver */ - address /* owner */ - ) external override onlyValidLender returns (uint256 shares) { - return 0; + uint256 assets, + address receiver, + address owner + ) public override onlyValidLender returns (uint256 shares) { + return super.withdraw(assets, receiver, owner); } /** * @inheritdoc Pool + * @dev Since Pool enforces that msg.sender == receiver == owner, we + * only check the sender here. */ function redeem( - uint256, /* shares */ - address, /* receiver */ - address /* owner */ - ) external override onlyValidLender returns (uint256 assets) { - return 0; + uint256 shares, + address receiver, + address owner + ) public override onlyValidLender returns (uint256 assets) { + return super.redeem(shares, receiver, owner); } } 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..651f1e09 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.only("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); +} From a3a7575c550348893e55364982cffe8191049340 Mon Sep 17 00:00:00 2001 From: Adrian Soghoian Date: Wed, 30 Nov 2022 11:19:37 -0500 Subject: [PATCH 2/5] Remove only --- test/permissioned/PermissionedPool.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/permissioned/PermissionedPool.test.ts b/test/permissioned/PermissionedPool.test.ts index 651f1e09..7e36fc6e 100644 --- a/test/permissioned/PermissionedPool.test.ts +++ b/test/permissioned/PermissionedPool.test.ts @@ -144,7 +144,7 @@ describe("PermissionedPool", () => { }); }); - describe.only("redeem()", () => { + describe("redeem()", () => { it("reverts if not allowed lender", async () => { const { pool, poolAdmin, liquidityAsset, otherAccount } = await loadFixture(loadPoolFixture); From 575db6cf5ceeef6d869b3115792ec9f8ff68dd65 Mon Sep 17 00:00:00 2001 From: Adrian Soghoian Date: Wed, 30 Nov 2022 11:20:15 -0500 Subject: [PATCH 3/5] Remove unused modifier --- contracts/permissioned/PermissionedPool.sol | 40 ++++++--------------- 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/contracts/permissioned/PermissionedPool.sol b/contracts/permissioned/PermissionedPool.sol index e5bd9d9c..58a8722e 100644 --- a/contracts/permissioned/PermissionedPool.sol +++ b/contracts/permissioned/PermissionedPool.sol @@ -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 @@ -88,12 +77,9 @@ contract PermissionedPool is Pool { * @dev Since Pool does not enforce that msg.sender == receiver, we only * check the receiver here. */ - function maxDeposit(address receiver) - public - view - override - returns (uint256) - { + function maxDeposit( + address receiver + ) public view override returns (uint256) { if (!poolAccessControl.isValidParticipant(receiver)) { return 0; } @@ -106,12 +92,10 @@ contract PermissionedPool is Pool { * @dev Since Pool enforces that msg.sender must be receiver, we only check * the sender here. */ - function deposit(uint256 assets, address receiver) - public - override - onlyValidLender - returns (uint256 shares) - { + function deposit( + uint256 assets, + address receiver + ) public override onlyValidLender returns (uint256 shares) { return super.deposit(assets, receiver); } @@ -133,12 +117,10 @@ contract PermissionedPool is Pool { * @dev Since Pool enforces that msg.sender must be receiver, we only check * the sender here. */ - function mint(uint256 shares, address receiver) - public - override - onlyValidLender - returns (uint256) - { + function mint( + uint256 shares, + address receiver + ) public override onlyValidLender returns (uint256) { return super.mint(shares, receiver); } From de8abc44eba345203460282e867906504687996d Mon Sep 17 00:00:00 2001 From: Adrian Soghoian Date: Wed, 30 Nov 2022 11:22:28 -0500 Subject: [PATCH 4/5] lint --- contracts/permissioned/PermissionedPool.sol | 29 +++++++++++++-------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/contracts/permissioned/PermissionedPool.sol b/contracts/permissioned/PermissionedPool.sol index 58a8722e..5184a151 100644 --- a/contracts/permissioned/PermissionedPool.sol +++ b/contracts/permissioned/PermissionedPool.sol @@ -77,9 +77,12 @@ contract PermissionedPool is Pool { * @dev Since Pool does not enforce that msg.sender == receiver, we only * check the receiver here. */ - function maxDeposit( - address receiver - ) public view override returns (uint256) { + function maxDeposit(address receiver) + public + view + override + returns (uint256) + { if (!poolAccessControl.isValidParticipant(receiver)) { return 0; } @@ -92,10 +95,12 @@ contract PermissionedPool is Pool { * @dev Since Pool enforces that msg.sender must be receiver, we only check * the sender here. */ - function deposit( - uint256 assets, - address receiver - ) public override onlyValidLender returns (uint256 shares) { + function deposit(uint256 assets, address receiver) + public + override + onlyValidLender + returns (uint256 shares) + { return super.deposit(assets, receiver); } @@ -117,10 +122,12 @@ contract PermissionedPool is Pool { * @dev Since Pool enforces that msg.sender must be receiver, we only check * the sender here. */ - function mint( - uint256 shares, - address receiver - ) public override onlyValidLender returns (uint256) { + function mint(uint256 shares, address receiver) + public + override + onlyValidLender + returns (uint256) + { return super.mint(shares, receiver); } From d8fc44ebbcd53f4f5f25248aeecef5dcb1be1eb9 Mon Sep 17 00:00:00 2001 From: Adrian Soghoian Date: Thu, 1 Dec 2022 09:27:59 -0500 Subject: [PATCH 5/5] Use pattern of overriden modifier --- contracts/Pool.sol | 26 +++++++++- contracts/permissioned/PermissionedPool.sol | 56 +-------------------- 2 files changed, 25 insertions(+), 57 deletions(-) diff --git a/contracts/Pool.sol b/contracts/Pool.sol index 0e4fc6f4..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 - ) public 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 - ) public 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 5184a151..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" @@ -90,20 +90,6 @@ contract PermissionedPool is Pool { return super.maxDeposit(receiver); } - /** - * @inheritdoc Pool - * @dev Since Pool enforces that msg.sender must be receiver, we only check - * the sender here. - */ - function deposit(uint256 assets, address receiver) - public - override - onlyValidLender - returns (uint256 shares) - { - return super.deposit(assets, receiver); - } - /** * @inheritdoc Pool * @dev Since Pool does not enforce that msg.sender == receiver, we only @@ -116,44 +102,4 @@ contract PermissionedPool is Pool { return super.maxMint(receiver); } - - /** - * @inheritdoc Pool - * @dev Since Pool enforces that msg.sender must be receiver, we only check - * the sender here. - */ - function mint(uint256 shares, address receiver) - public - override - onlyValidLender - returns (uint256) - { - return super.mint(shares, receiver); - } - - /** - * @inheritdoc Pool - * @dev Since Pool enforces that msg.sender == receiver == owner, we - * only check the sender here. - */ - function withdraw( - uint256 assets, - address receiver, - address owner - ) public override onlyValidLender returns (uint256 shares) { - return super.withdraw(assets, receiver, owner); - } - - /** - * @inheritdoc Pool - * @dev Since Pool enforces that msg.sender == receiver == owner, we - * only check the sender here. - */ - function redeem( - uint256 shares, - address receiver, - address owner - ) public override onlyValidLender returns (uint256 assets) { - return super.redeem(shares, receiver, owner); - } }