diff --git a/contracts/PoolFactory.sol b/contracts/PoolFactory.sol index 18a72915..0ff8c326 100644 --- a/contracts/PoolFactory.sol +++ b/contracts/PoolFactory.sol @@ -14,8 +14,24 @@ contract PoolFactory is IPoolFactory { */ address private _serviceConfiguration; - constructor(address serviceConfiguration) { + /** + * @dev Reference to the WithdrawControllerFactory contract + */ + address private _withdrawControllerFactory; + + /** + * @dev Reference to the PoolControllerFactory contract + */ + address private _poolControllerFactory; + + constructor( + address serviceConfiguration, + address withdrawControllerFactory, + address poolControllerFactory + ) { _serviceConfiguration = serviceConfiguration; + _withdrawControllerFactory = withdrawControllerFactory; + _poolControllerFactory = poolControllerFactory; } /** @@ -24,8 +40,6 @@ contract PoolFactory is IPoolFactory { */ function createPool( address liquidityAsset, - address withdrawControllerFactory, - address poolControllerFactory, IPoolConfigurableSettings calldata settings ) public virtual returns (address poolAddress) { require( @@ -36,11 +50,6 @@ contract PoolFactory is IPoolFactory { settings.withdrawRequestPeriodDuration > 0, "PoolFactory: Invalid duration" ); - require( - withdrawControllerFactory != address(0) && - poolControllerFactory != address(0), - "PoolFactory: Invalid address" - ); if (settings.fixedFee > 0) { require( settings.fixedFeeInterval > 0, @@ -53,8 +62,8 @@ contract PoolFactory is IPoolFactory { liquidityAsset, msg.sender, _serviceConfiguration, - withdrawControllerFactory, - poolControllerFactory, + _withdrawControllerFactory, + _poolControllerFactory, settings, "PerimeterPoolToken", "PPT" diff --git a/contracts/interfaces/IPoolFactory.sol b/contracts/interfaces/IPoolFactory.sol index de65c6bb..eb36f3cb 100644 --- a/contracts/interfaces/IPoolFactory.sol +++ b/contracts/interfaces/IPoolFactory.sol @@ -16,10 +16,7 @@ interface IPoolFactory { * @dev Creates a pool's PoolAdmin controller * @dev Emits `PoolControllerCreated` event. */ - function createPool( - address, - address, - address, - IPoolConfigurableSettings calldata - ) external returns (address); + function createPool(address, IPoolConfigurableSettings calldata) + external + returns (address); } diff --git a/contracts/permissioned/PermissionedPoolFactory.sol b/contracts/permissioned/PermissionedPoolFactory.sol index 300d0f43..1bd33d6c 100644 --- a/contracts/permissioned/PermissionedPoolFactory.sol +++ b/contracts/permissioned/PermissionedPoolFactory.sol @@ -20,9 +20,25 @@ contract PermissionedPoolFactory is IPoolFactory { */ address private _poolAccessControlFactory; - constructor(address serviceConfiguration, address poolAccessControlFactory) - { + /** + * @dev Reference to the WithdrawControllerFactory contract + */ + address private _withdrawControllerFactory; + + /** + * @dev Reference to the PoolControllerFactory contract + */ + address private _poolControllerFactory; + + constructor( + address serviceConfiguration, + address withdrawControllerFactory, + address poolControllerFactory, + address poolAccessControlFactory + ) { _serviceConfiguration = serviceConfiguration; + _withdrawControllerFactory = withdrawControllerFactory; + _poolControllerFactory = poolControllerFactory; _poolAccessControlFactory = poolAccessControlFactory; } @@ -44,8 +60,6 @@ contract PermissionedPoolFactory is IPoolFactory { */ function createPool( address liquidityAsset, - address withdrawControllerFactory, - address poolControllerFactory, IPoolConfigurableSettings calldata settings ) public override onlyVerifiedPoolAdmin returns (address poolAddress) { require( @@ -57,8 +71,8 @@ contract PermissionedPoolFactory is IPoolFactory { liquidityAsset, msg.sender, address(_serviceConfiguration), - withdrawControllerFactory, - poolControllerFactory, + _withdrawControllerFactory, + _poolControllerFactory, _poolAccessControlFactory, settings, "PerimeterPoolToken", diff --git a/test/PoolFactory.test.ts b/test/PoolFactory.test.ts index 11a046ab..3043619b 100644 --- a/test/PoolFactory.test.ts +++ b/test/PoolFactory.test.ts @@ -27,14 +27,6 @@ describe("PoolFactory", () => { const PoolLib = await ethers.getContractFactory("PoolLib"); const poolLib = await PoolLib.deploy(); - const PoolFactory = await ethers.getContractFactory("PoolFactory", { - libraries: { - PoolLib: poolLib.address - } - }); - const poolFactory = await PoolFactory.deploy(serviceConfiguration.address); - await poolFactory.deployed(); - const withdrawControllerFactory = await deployWithdrawControllerFactory( poolLib.address, serviceConfiguration.address @@ -45,84 +37,40 @@ describe("PoolFactory", () => { serviceConfiguration.address ); + const PoolFactory = await ethers.getContractFactory("PoolFactory", { + libraries: { + PoolLib: poolLib.address + } + }); + const poolFactory = await PoolFactory.deploy( + serviceConfiguration.address, + withdrawControllerFactory.address, + poolControllerFactory.address + ); + await poolFactory.deployed(); + return { poolFactory, - liquidityAsset, - withdrawControllerFactory, - poolControllerFactory + liquidityAsset }; } it("reverts if given a zero withdraw window", async () => { - const { - poolFactory, - withdrawControllerFactory, - poolControllerFactory, - liquidityAsset - } = await loadFixture(deployFixture); + const { poolFactory, liquidityAsset } = await loadFixture(deployFixture); const poolSettings = Object.assign({}, DEFAULT_POOL_SETTINGS, { withdrawRequestPeriodDuration: 0 }); await expect( - poolFactory.createPool( - liquidityAsset.address, - withdrawControllerFactory.address, - poolControllerFactory.address, - poolSettings - ) - ).to.be.revertedWith("PoolFactory: Invalid duration"); - }); - - it("reverts if given an invalid pool withdraw controller factory", async () => { - const { poolFactory, poolControllerFactory, liquidityAsset } = - await loadFixture(deployFixture); - - const poolSettings = Object.assign({}, DEFAULT_POOL_SETTINGS, { - withdrawRequestPeriodDuration: 0 - }); - await expect( - poolFactory.createPool( - /* liquidityAsset */ liquidityAsset.address, - "0x0000000000000000000000000000000000000000", - poolControllerFactory.address, - poolSettings - ) - ).to.be.revertedWith("PoolFactory: Invalid duration"); - }); - - it("reverts if given an invalid pool admin controller factory", async () => { - const { poolFactory, withdrawControllerFactory, liquidityAsset } = - await loadFixture(deployFixture); - - const poolSettings = Object.assign({}, DEFAULT_POOL_SETTINGS, { - withdrawRequestPeriodDuration: 0 - }); - await expect( - poolFactory.createPool( - /* liquidityAsset */ liquidityAsset.address, - withdrawControllerFactory.address, - "0x0000000000000000000000000000000000000000", - poolSettings - ) + poolFactory.createPool(liquidityAsset.address, poolSettings) ).to.be.revertedWith("PoolFactory: Invalid duration"); }); it("emits PoolCreated", async () => { - const { - poolFactory, - liquidityAsset, - withdrawControllerFactory, - poolControllerFactory - } = await loadFixture(deployFixture); + const { poolFactory, liquidityAsset } = await loadFixture(deployFixture); await expect( - poolFactory.createPool( - /* liquidityAsset */ liquidityAsset.address, - withdrawControllerFactory.address, - poolControllerFactory.address, - DEFAULT_POOL_SETTINGS - ) + poolFactory.createPool(liquidityAsset.address, DEFAULT_POOL_SETTINGS) ).to.emit(poolFactory, "PoolCreated"); }); }); diff --git a/test/permissioned/PermissionedPoolFactory.test.ts b/test/permissioned/PermissionedPoolFactory.test.ts index afa2e45f..19fd6b58 100644 --- a/test/permissioned/PermissionedPoolFactory.test.ts +++ b/test/permissioned/PermissionedPoolFactory.test.ts @@ -60,6 +60,16 @@ describe("PermissionedPoolFactory", () => { permissionedServiceConfiguration.address ); + const withdrawControllerFactory = await deployWithdrawControllerFactory( + poolLib.address, + permissionedServiceConfiguration.address + ); + + const poolControllerFactory = await deployPoolControllerFactory( + poolLib.address, + permissionedServiceConfiguration.address + ); + // Deploy the PermissionedPoolFactory const PoolFactory = await ethers.getContractFactory( "PermissionedPoolFactory", @@ -71,6 +81,8 @@ describe("PermissionedPoolFactory", () => { ); const poolFactory = await PoolFactory.deploy( permissionedServiceConfiguration.address, + withdrawControllerFactory.address, + poolControllerFactory.address, poolAccessControlFactory.address ); await poolFactory.deployed(); @@ -81,25 +93,13 @@ describe("PermissionedPoolFactory", () => { ); await tx.wait(); - const withdrawControllerFactory = await deployWithdrawControllerFactory( - poolLib.address, - permissionedServiceConfiguration.address - ); - - const poolControllerFactory = await deployPoolControllerFactory( - poolLib.address, - permissionedServiceConfiguration.address - ); - return { poolFactory, poolAdminAccessControl, operator, otherAccount, liquidityAsset, - tosAcceptanceRegistry, - withdrawControllerFactory, - poolControllerFactory + tosAcceptanceRegistry }; } @@ -109,9 +109,7 @@ describe("PermissionedPoolFactory", () => { poolAdminAccessControl, otherAccount, liquidityAsset, - tosAcceptanceRegistry, - withdrawControllerFactory, - poolControllerFactory + tosAcceptanceRegistry } = await loadFixture(deployFixture); await tosAcceptanceRegistry.connect(otherAccount).acceptTermsOfService(); @@ -120,12 +118,7 @@ describe("PermissionedPoolFactory", () => { await expect( poolFactory .connect(otherAccount) - .createPool( - liquidityAsset.address, - withdrawControllerFactory.address, - poolControllerFactory.address, - DEFAULT_POOL_SETTINGS - ) + .createPool(liquidityAsset.address, DEFAULT_POOL_SETTINGS) ).to.emit(poolFactory, "PoolCreated"); }); @@ -135,21 +128,14 @@ describe("PermissionedPoolFactory", () => { poolAdminAccessControl, otherAccount, liquidityAsset, - tosAcceptanceRegistry, - withdrawControllerFactory, - poolControllerFactory + tosAcceptanceRegistry } = await loadFixture(deployFixture); await tosAcceptanceRegistry.connect(otherAccount).acceptTermsOfService(); await poolAdminAccessControl.allow(otherAccount.getAddress()); await expect( - poolFactory.createPool( - liquidityAsset.address, - withdrawControllerFactory.address, - poolControllerFactory.address, - DEFAULT_POOL_SETTINGS - ) + poolFactory.createPool(liquidityAsset.address, DEFAULT_POOL_SETTINGS) ).to.be.revertedWith("caller is not allowed pool admin"); }); diff --git a/test/support/pool.ts b/test/support/pool.ts index 3275791b..5c11c448 100644 --- a/test/support/pool.ts +++ b/test/support/pool.ts @@ -47,15 +47,6 @@ export async function deployPool({ const PoolLib = await ethers.getContractFactory("PoolLib"); const poolLib = await PoolLib.deploy(); - const PoolFactory = await ethers.getContractFactory("PoolFactory", { - signer: poolAdmin, - libraries: { - PoolLib: poolLib.address - } - }); - const poolFactory = await PoolFactory.deploy(serviceConfiguration.address); - await poolFactory.deployed(); - const withdrawControllerFactory = await deployWithdrawControllerFactory( poolLib.address, serviceConfiguration.address @@ -66,14 +57,22 @@ export async function deployPool({ serviceConfiguration.address ); + const PoolFactory = await ethers.getContractFactory("PoolFactory", { + signer: poolAdmin, + libraries: { + PoolLib: poolLib.address + } + }); + const poolFactory = await PoolFactory.deploy( + serviceConfiguration.address, + withdrawControllerFactory.address, + poolControllerFactory.address + ); + await poolFactory.deployed(); + const txn = await poolFactory .connect(poolAdmin) - .createPool( - liquidityAsset.address, - withdrawControllerFactory.address, - poolControllerFactory.address, - poolSettings - ); + .createPool(liquidityAsset.address, poolSettings); const txnReceipt = await txn.wait(); const poolCreatedEvent = txnReceipt.events?.find( @@ -141,6 +140,16 @@ export async function deployPermissionedPool({ serviceConfiguration.address ); + const withdrawControllerFactory = await deployWithdrawControllerFactory( + poolLib.address, + serviceConfiguration.address + ); + + const poolControllerFactory = await deployPoolControllerFactory( + poolLib.address, + serviceConfiguration.address + ); + const PoolFactory = await ethers.getContractFactory( "PermissionedPoolFactory", { @@ -152,28 +161,15 @@ export async function deployPermissionedPool({ ); const poolFactory = await PoolFactory.deploy( serviceConfiguration.address, + withdrawControllerFactory.address, + poolControllerFactory.address, poolAccessControlFactory.address ); await poolFactory.deployed(); - const withdrawControllerFactory = await deployWithdrawControllerFactory( - poolLib.address, - serviceConfiguration.address - ); - - const poolControllerFactory = await deployPoolControllerFactory( - poolLib.address, - serviceConfiguration.address - ); - const txn = await poolFactory .connect(poolAdmin) - .createPool( - liquidityAsset.address, - withdrawControllerFactory.address, - poolControllerFactory.address, - poolSettings - ); + .createPool(liquidityAsset.address, poolSettings); const txnReceipt = await txn.wait(); const poolCreatedEvent = txnReceipt.events?.find(