Refactor ManagedPool constructor arguments (#2078)

* Rename NewPoolParams * Introduce ConfigParams * Apply changes to test and helpers * Fix final tests * Fix benchmarks. (#2086) Co-authored-by: Juan Ignacio Ubeira <[email protected]>
balancer · Dec 1, 2022 · 38a8381 · 38a8381
1 parent f58068b
commit 38a8381
Show file tree

Hide file tree

Showing 15 changed files with 161 additions and 108 deletions.
diff --git a/pkg/pool-weighted/contracts/managed/ControlledManagedPoolFactory.sol b/pkg/pool-weighted/contracts/managed/ControlledManagedPoolFactory.sol
@@ -39,7 +39,8 @@ contract ControlledManagedPoolFactory {
      * @dev Deploys a new `ManagedPool`.
      */
     function create(
-        ManagedPoolSettings.NewPoolParams memory poolParams,
+        ManagedPool.ManagedPoolParams memory params,
+        ManagedPoolSettings.ManagedPoolSettingsParams memory settingsParams,
         BasePoolController.BasePoolRights calldata basePoolRights,
         ManagedPoolController.ManagedPoolRights calldata managedPoolRights,
         uint256 minWeightChangeDuration,
@@ -53,7 +54,7 @@ contract ControlledManagedPoolFactory {
         );
 
         // Let the base factory deploy the pool (owner is the controller)
-        pool = ManagedPoolFactory(managedPoolFactory).create(poolParams, address(poolController));
+        pool = ManagedPoolFactory(managedPoolFactory).create(params, settingsParams, address(poolController));
 
         // Finally, initialize the controller
         poolController.initialize(pool);

diff --git a/pkg/pool-weighted/contracts/managed/ManagedPool.sol b/pkg/pool-weighted/contracts/managed/ManagedPool.sol
@@ -58,32 +58,43 @@ contract ManagedPool is ManagedPoolSettings {
     uint256 private constant _PREMINTED_TOKEN_BALANCE = 2**(111);
     IExternalWeightedMath private immutable _weightedMath;
 
+    struct ManagedPoolParams {
+        string name;
+        string symbol;
+        address[] assetManagers;
+    }
+
+    struct ManagedPoolConfigParams {
+        IVault vault;
+        IProtocolFeePercentagesProvider protocolFeeProvider;
+        IExternalWeightedMath weightedMath;
+        uint256 pauseWindowDuration;
+        uint256 bufferPeriodDuration;
+    }
+
     constructor(
-        NewPoolParams memory params,
-        IVault vault,
-        IProtocolFeePercentagesProvider protocolFeeProvider,
-        IExternalWeightedMath weightedMath,
-        address owner,
-        uint256 pauseWindowDuration,
-        uint256 bufferPeriodDuration
+        ManagedPoolParams memory params,
+        ManagedPoolConfigParams memory configParams,
+        ManagedPoolSettingsParams memory settingsParams,
+        address owner
     )
         NewBasePool(
-            vault,
+            configParams.vault,
             PoolRegistrationLib.registerComposablePool(
-                vault,
+                configParams.vault,
                 IVault.PoolSpecialization.MINIMAL_SWAP_INFO,
-                params.tokens,
+                settingsParams.tokens,
                 params.assetManagers
             ),
             params.name,
             params.symbol,
-            pauseWindowDuration,
-            bufferPeriodDuration,
+            configParams.pauseWindowDuration,
+            configParams.bufferPeriodDuration,
             owner
         )
-        ManagedPoolSettings(params, protocolFeeProvider)
+        ManagedPoolSettings(settingsParams, configParams.protocolFeeProvider)
     {
-        _weightedMath = weightedMath;
+        _weightedMath = configParams.weightedMath;
     }
 
     function _getWeightedMath() internal view returns (IExternalWeightedMath) {

diff --git a/pkg/pool-weighted/contracts/managed/ManagedPoolFactory.sol b/pkg/pool-weighted/contracts/managed/ManagedPoolFactory.sol
@@ -50,23 +50,21 @@ contract ManagedPoolFactory is BasePoolFactory, FactoryWidePauseWindow {
     /**
      * @dev Deploys a new `ManagedPool`. The owner should be a contract, deployed by another factory.
      */
-    function create(ManagedPoolSettings.NewPoolParams memory poolParams, address owner)
-        external
-        returns (address pool)
-    {
+    function create(
+        ManagedPool.ManagedPoolParams memory params,
+        ManagedPoolSettings.ManagedPoolSettingsParams memory settingsParams,
+        address owner
+    ) external returns (address pool) {
         (uint256 pauseWindowDuration, uint256 bufferPeriodDuration) = getPauseConfiguration();
 
-        return
-            _create(
-                abi.encode(
-                    poolParams,
-                    getVault(),
-                    getProtocolFeePercentagesProvider(),
-                    _weightedMath,
-                    owner,
-                    pauseWindowDuration,
-                    bufferPeriodDuration
-                )
-            );
+        ManagedPool.ManagedPoolConfigParams memory configParams = ManagedPool.ManagedPoolConfigParams({
+            vault: getVault(),
+            protocolFeeProvider: getProtocolFeePercentagesProvider(),
+            weightedMath: _weightedMath,
+            pauseWindowDuration: pauseWindowDuration,
+            bufferPeriodDuration: bufferPeriodDuration
+        });
+
+        return _create(abi.encode(params, configParams, settingsParams, owner));
     }
 }
diff --git a/pkg/pool-weighted/contracts/managed/ManagedPoolSettings.sol b/pkg/pool-weighted/contracts/managed/ManagedPoolSettings.sol
@@ -91,20 +91,17 @@ abstract contract ManagedPoolSettings is NewBasePool, ProtocolFeeCache, IManaged
     // If mustAllowlistLPs is enabled, this is the list of addresses allowed to join the pool
     mapping(address => bool) private _allowedAddresses;
 
-    struct NewPoolParams {
-        string name;
-        string symbol;
+    struct ManagedPoolSettingsParams {
         IERC20[] tokens;
         uint256[] normalizedWeights;
-        address[] assetManagers;
         uint256 swapFeePercentage;
         bool swapEnabledOnStart;
         bool mustAllowlistLPs;
         uint256 managementAumFeePercentage;
         uint256 aumFeeId;
     }
 
-    constructor(NewPoolParams memory params, IProtocolFeePercentagesProvider protocolFeeProvider)
+    constructor(ManagedPoolSettingsParams memory params, IProtocolFeePercentagesProvider protocolFeeProvider)
         ProtocolFeeCache(
             protocolFeeProvider,
             ProviderFeeIDs({ swap: ProtocolFeeType.SWAP, yield: ProtocolFeeType.YIELD, aum: params.aumFeeId })
@@ -114,7 +111,7 @@ abstract contract ManagedPoolSettings is NewBasePool, ProtocolFeeCache, IManaged
         _require(totalTokens >= _MIN_TOKENS, Errors.MIN_TOKENS);
         _require(totalTokens <= _MAX_TOKENS, Errors.MAX_TOKENS);
 
-        InputHelpers.ensureInputLengthMatch(totalTokens, params.normalizedWeights.length, params.assetManagers.length);
+        InputHelpers.ensureInputLengthMatch(totalTokens, params.normalizedWeights.length);
 
         // Validate and set initial fees
         _setManagementAumFeePercentage(params.managementAumFeePercentage);

diff --git a/pkg/pool-weighted/contracts/test/MockManagedPool.sol b/pkg/pool-weighted/contracts/test/MockManagedPool.sol
@@ -20,14 +20,11 @@ import "../ExternalWeightedMath.sol";
 
 contract MockManagedPool is ManagedPool {
     constructor(
-        NewPoolParams memory params,
-        IVault vault,
-        IProtocolFeePercentagesProvider protocolFeeProvider,
-        ExternalWeightedMath weightedMath,
-        address owner,
-        uint256 pauseWindowDuration,
-        uint256 bufferPeriodDuration
-    ) ManagedPool(params, vault, protocolFeeProvider, weightedMath, owner, pauseWindowDuration, bufferPeriodDuration) {
+        ManagedPoolParams memory params,
+        ManagedPoolConfigParams memory configParams,
+        ManagedPoolSettingsParams memory settingsParams,
+        address owner
+    ) ManagedPool(params, configParams, settingsParams, owner) {
         // solhint-disable-previous-line no-empty-blocks
     }
 

diff --git a/pkg/pool-weighted/contracts/test/MockManagedPoolSettings.sol b/pkg/pool-weighted/contracts/test/MockManagedPoolSettings.sol
@@ -24,29 +24,28 @@ contract MockManagedPoolSettings is ManagedPoolSettings {
     ExternalWeightedMath private immutable _weightedMath;
 
     constructor(
-        NewPoolParams memory params,
+        ManagedPoolSettingsParams memory settingsParams,
         IVault vault,
         IProtocolFeePercentagesProvider protocolFeeProvider,
         ExternalWeightedMath weightedMath,
-        address owner,
-        uint256 pauseWindowDuration,
-        uint256 bufferPeriodDuration
+        address[] memory asssetManagers,
+        address owner
     )
         NewBasePool(
             vault,
             PoolRegistrationLib.registerPoolWithAssetManagers(
                 vault,
                 IVault.PoolSpecialization.MINIMAL_SWAP_INFO,
-                params.tokens,
-                params.assetManagers
+                settingsParams.tokens,
+                asssetManagers
             ),
-            params.name,
-            params.symbol,
-            pauseWindowDuration,
-            bufferPeriodDuration,
+            "MockManagedPoolName",
+            "MockManagedPoolSymbol",
+            90 days,
+            30 days,
             owner
         )
-        ManagedPoolSettings(params, protocolFeeProvider)
+        ManagedPoolSettings(settingsParams, protocolFeeProvider)
     {
         _weightedMath = weightedMath;
     }
@@ -55,7 +54,11 @@ contract MockManagedPoolSettings is ManagedPoolSettings {
         return _getVirtualSupply();
     }
 
-    function _onInitializePool(address, address, bytes memory userData) internal override returns (uint256, uint256[] memory) {
+    function _onInitializePool(
+        address,
+        address,
+        bytes memory userData
+    ) internal override returns (uint256, uint256[] memory) {
         WeightedPoolUserData.JoinKind kind = userData.joinKind();
         _require(kind == WeightedPoolUserData.JoinKind.INIT, Errors.UNINITIALIZED);
 

diff --git a/pkg/pool-weighted/test/ControlledManagedPoolFactory.test.ts b/pkg/pool-weighted/test/ControlledManagedPoolFactory.test.ts
@@ -73,12 +73,15 @@ describe('ControlledManagedPoolFactory', function () {
     const assetManagers: string[] = Array(tokens.length).fill(ZERO_ADDRESS);
     assetManagers[tokens.indexOf(tokens.DAI)] = assetManager.address;
 
-    const newPoolParams: ManagedPoolParams = {
+    const poolParams = {
       name: NAME,
       symbol: SYMBOL,
+      assetManagers,
+    };
+
+    const settingsParams: ManagedPoolParams = {
       tokens: tokens.addresses,
       normalizedWeights: WEIGHTS,
-      assetManagers: assetManagers,
       swapFeePercentage: POOL_SWAP_FEE_PERCENTAGE,
       swapEnabledOnStart: swapsEnabled,
       mustAllowlistLPs: mustAllowlistLPs,
@@ -104,7 +107,14 @@ describe('ControlledManagedPoolFactory', function () {
     const receipt = await (
       await controlledFactory
         .connect(manager)
-        .create(newPoolParams, basePoolRights, managedPoolRights, MIN_WEIGHT_CHANGE_DURATION, manager.address)
+        .create(
+          poolParams,
+          settingsParams,
+          basePoolRights,
+          managedPoolRights,
+          MIN_WEIGHT_CHANGE_DURATION,
+          manager.address
+        )
     ).wait();
 
     const event = expectEvent.inReceipt(receipt, 'ManagedPoolCreated');

diff --git a/pkg/pool-weighted/test/ManagedPool.test.ts b/pkg/pool-weighted/test/ManagedPool.test.ts
@@ -70,7 +70,6 @@ describe('ManagedPool', function () {
       owner: owner.address,
       aumFeeId: ProtocolFee.AUM,
       poolType: WeightedPoolType.MOCK_MANAGED_POOL,
-      mockContractName: 'MockManagedPool',
       ...overrides,
     };
     return WeightedPool.create(params);

diff --git a/pkg/pool-weighted/test/ManagedPoolFactory.test.ts b/pkg/pool-weighted/test/ManagedPoolFactory.test.ts
@@ -59,20 +59,23 @@ describe('ManagedPoolFactory', function () {
     const assetManagers: string[] = Array(tokens.length).fill(ZERO_ADDRESS);
     assetManagers[tokens.indexOf(tokens.DAI)] = assetManager.address;
 
-    const newPoolParams: ManagedPoolParams = {
+    const poolParams = {
       name: NAME,
       symbol: SYMBOL,
+      assetManagers: assetManagers,
+    };
+
+    const settingsParams: ManagedPoolParams = {
       tokens: tokens.addresses,
       normalizedWeights: WEIGHTS,
-      assetManagers: assetManagers,
       swapFeePercentage: POOL_SWAP_FEE_PERCENTAGE,
       swapEnabledOnStart: swapsEnabled,
       mustAllowlistLPs: mustAllowlistLPs,
       managementAumFeePercentage: POOL_MANAGEMENT_AUM_FEE_PERCENTAGE,
       aumFeeId: ProtocolFee.AUM,
     };
 
-    const receipt = await (await factory.connect(manager).create(newPoolParams, manager.address)).wait();
+    const receipt = await (await factory.connect(manager).create(poolParams, settingsParams, manager.address)).wait();
 
     const event = expectEvent.inReceipt(receipt, 'PoolCreated');
     return deployedAt('ManagedPool', event.args.pool);

diff --git a/pkg/pool-weighted/test/ManagedPoolSettings.test.ts b/pkg/pool-weighted/test/ManagedPoolSettings.test.ts
@@ -83,8 +83,7 @@ describe('ManagedPoolSettings', function () {
     const fullParams = {
       ...params,
       swapFeePercentage: INITIAL_SWAP_FEE,
-      poolType: WeightedPoolType.MOCK_MANAGED_POOL,
-      mockContractName: 'MockManagedPoolSettings',
+      poolType: WeightedPoolType.MOCK_MANAGED_POOL_SETTINGS,
     };
     return WeightedPool.create(fullParams);
   }
@@ -162,13 +161,6 @@ describe('ManagedPoolSettings', function () {
 
               expect(poolScalingFactors).to.deep.equal(tokenScalingFactors);
             });
-
-            it('sets asset managers', async () => {
-              await tokens.asyncEach(async (token, i) => {
-                const info = await pool.getTokenInfo(token);
-                expect(info.assetManager).to.eq(assetManagers[i]);
-              });
-            });
           });
         });
       }
@@ -524,7 +516,6 @@ describe('ManagedPoolSettings', function () {
       context('when the sender is not the owner', () => {
         it('non-owners cannot update weights', async () => {
           const now = await currentTimestamp();
-
           await expect(pool.updateWeightsGradually(other, now, now, poolWeights)).to.be.revertedWith(
             'SENDER_NOT_ALLOWED'
           );

diff --git a/pkg/pool-weighted/test/managed/ownerOnlyActions.test.ts b/pkg/pool-weighted/test/managed/ownerOnlyActions.test.ts
@@ -21,24 +21,24 @@ describe('ManagedPool owner only actions', () => {
     const circuitBreakerLib = await deploy('CircuitBreakerLib');
     pool = await deploy('MockManagedPool', {
       args: [
+        { name: '', symbol: '', assetManagers: new Array(2).fill(ZERO_ADDRESS) },
+        {
+          vault: vault.address,
+          protocolFeeProvider: vault.getFeesProvider().address,
+          weightedMath: math.address,
+          pauseWindowDuration: 0,
+          bufferPeriodDuration: 0,
+        },
         {
-          name: '',
-          symbol: '',
           tokens: tokens.addresses,
           normalizedWeights: [fp(0.5), fp(0.5)],
-          assetManagers: new Array(2).fill(ZERO_ADDRESS),
           swapFeePercentage: fp(0.05),
           swapEnabledOnStart: true,
           mustAllowlistLPs: false,
           managementAumFeePercentage: fp(0),
           aumFeeId: ProtocolFee.AUM,
         },
-        vault.address,
-        vault.getFeesProvider().address,
-        math.address,
         ZERO_ADDRESS,
-        0,
-        0,
       ],
       libraries: {
         CircuitBreakerLib: circuitBreakerLib.address,

diff --git a/pvt/benchmarks/misc.ts b/pvt/benchmarks/misc.ts
@@ -81,12 +81,15 @@ export async function deployPool(vault: Vault, tokens: TokenList, poolName: Pool
 
     switch (poolName) {
       case 'ManagedPool': {
-        const newPoolParams: ManagedPoolParams = {
+        const userInputs = {
           name: name,
           symbol: symbol,
+          assetManagers: Array(tokens.length).fill(ZERO_ADDRESS),
+        };
+
+        const managedPoolSettings: ManagedPoolParams = {
           tokens: tokens.addresses,
           normalizedWeights: weights,
-          assetManagers: Array(tokens.length).fill(ZERO_ADDRESS),
           swapFeePercentage: swapFeePercentage,
           swapEnabledOnStart: true,
           mustAllowlistLPs: false,
@@ -109,7 +112,7 @@ export async function deployPool(vault: Vault, tokens: TokenList, poolName: Pool
           canChangeMgmtFees: true,
         };
 
-        params = [newPoolParams, basePoolRights, managedPoolRights, DAY, creator.address];
+        params = [userInputs, managedPoolSettings, basePoolRights, managedPoolRights, DAY, creator.address];
         break;
       }
       default: {

diff --git a/pvt/helpers/src/models/pools/weighted/WeightedPool.ts b/pvt/helpers/src/models/pools/weighted/WeightedPool.ts
@@ -562,7 +562,11 @@ export default class WeightedPool extends BasePool {
   }
 
   private _isManagedPool() {
-    return this.poolType == WeightedPoolType.MANAGED_POOL || this.poolType == WeightedPoolType.MOCK_MANAGED_POOL;
+    return (
+      this.poolType == WeightedPoolType.MANAGED_POOL ||
+      this.poolType == WeightedPoolType.MOCK_MANAGED_POOL ||
+      this.poolType == WeightedPoolType.MOCK_MANAGED_POOL_SETTINGS
+    );
   }
 
   async setJoinExitEnabled(from: SignerWithAddress, joinExitEnabled: boolean): Promise<ContractTransaction> {