Move PoolControllerFactory, WithdrawFactory definition to PoolFactory…

… constructor (#101)
circlefin · Nov 28, 2022 · da9b708 · da9b708
1 parent 7211b13
commit da9b708
Show file tree

Hide file tree

Showing 6 changed files with 103 additions and 153 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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");
   });