From 8f89ec5533c7f8f322d6c7c4dfe31a95a9d93c83 Mon Sep 17 00:00:00 2001 From: Jeffrey Bennett Date: Fri, 2 Dec 2022 11:45:58 -0500 Subject: [PATCH 1/4] remove managed pool controller, factory using it, and tests --- .../controllers/ManagedPoolController.sol | 250 ----------- .../test/ManagedPoolController.test.ts | 395 ------------------ .../managed/ControlledManagedPoolFactory.sol | 72 ---- .../test/ControlledManagedPoolFactory.test.ts | 269 ------------ 4 files changed, 986 deletions(-) delete mode 100644 pkg/pool-utils/contracts/controllers/ManagedPoolController.sol delete mode 100644 pkg/pool-utils/test/ManagedPoolController.test.ts delete mode 100644 pkg/pool-weighted/contracts/managed/ControlledManagedPoolFactory.sol delete mode 100644 pkg/pool-weighted/test/ControlledManagedPoolFactory.test.ts diff --git a/pkg/pool-utils/contracts/controllers/ManagedPoolController.sol b/pkg/pool-utils/contracts/controllers/ManagedPoolController.sol deleted file mode 100644 index 925243e84c..0000000000 --- a/pkg/pool-utils/contracts/controllers/ManagedPoolController.sol +++ /dev/null @@ -1,250 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-or-later -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -pragma solidity ^0.7.0; -pragma experimental ABIEncoderV2; - -import "@balancer-labs/v2-solidity-utils/contracts/openzeppelin/SafeERC20.sol"; -import "@balancer-labs/v2-interfaces/contracts/pool-utils/IManagedPool.sol"; - -import "./BasePoolController.sol"; - -/** - * @dev Pool controller that serves as the "owner" of a Managed pool, and is in turn owned by - * an account empowered to make calls on this contract, which are forwarded to the underlyling pool. - * - * This contract can place limits on whether and how these calls can be made. For instance, - * imposing a minimum gradual weight change duration. - * - * While Balancer pool owners are immutable, ownership of this pool controller can be transferrable, - * if the corresponding permission is set. - */ -contract ManagedPoolController is BasePoolController { - using SafeERC20 for IERC20; - using WordCodec for bytes32; - - // There are seven managed pool rights: all corresponding to permissioned functions of ManagedPool. - struct ManagedPoolRights { - bool canChangeWeights; - bool canDisableSwaps; - bool canSetMustAllowlistLPs; - bool canSetCircuitBreakers; - bool canChangeTokens; - bool canChangeMgmtFees; - bool canDisableJoinExit; - } - - // The minimum weight change duration could be replaced with more sophisticated rate-limiting. - uint256 internal immutable _minWeightChangeDuration; - - /* solhint-disable max-line-length */ - // Immutable controller state - the first 16 bits are reserved as a bitmap for permission flags - // (3 used in the base class; 7 used here), and the remaining 240 bits can be used by derived classes - // to store any other immutable data. - // - // [ Managed Pool Controller Permissions | Base Controller Permissions ] - // [ 240 | 6 bits | 1 bit | 1 bit | 1 bit | 1 bit | 1 bit | 1 bit | 1 bit | 1 bit | 1 bit | 1 bit ] - // [unused|reserved| join-exit | mgmt fee | tokens | breakers | LPs | swaps | weights | metadata | swap fee | transfer ] - // |MSB LSB| - /* solhint-enable max-line-length */ - - uint256 private constant _CHANGE_WEIGHTS_OFFSET = 3; - uint256 private constant _DISABLE_SWAPS_OFFSET = 4; - uint256 private constant _MUST_ALLOWLIST_LPS_OFFSET = 5; - uint256 private constant _CIRCUIT_BREAKERS_OFFSET = 6; - uint256 private constant _CHANGE_TOKENS_OFFSET = 7; - uint256 private constant _CHANGE_MGMT_FEES_OFFSET = 8; - uint256 private constant _DISABLE_JOIN_EXIT_OFFSET = 9; - - /** - * @dev Pass in the `BasePoolRights` and `ManagedPoolRights` structures, to form the complete set of - * immutable rights. Then pass any parameters related to restrictions on those rights. For instance, - * a minimum duration if changing weights is enabled. - */ - constructor( - BasePoolRights memory baseRights, - ManagedPoolRights memory managedRights, - uint256 minWeightChangeDuration, - address manager - ) BasePoolController(encodePermissions(baseRights, managedRights), manager) { - _minWeightChangeDuration = minWeightChangeDuration; - } - - function encodePermissions(BasePoolRights memory baseRights, ManagedPoolRights memory managedRights) - public - pure - returns (bytes32) - { - bytes32 permissions = super - .encodePermissions(baseRights) - .insertBool(managedRights.canChangeWeights, _CHANGE_WEIGHTS_OFFSET) - .insertBool(managedRights.canDisableSwaps, _DISABLE_SWAPS_OFFSET) - .insertBool(managedRights.canSetMustAllowlistLPs, _MUST_ALLOWLIST_LPS_OFFSET); - - // Needed to avoid "stack too deep" - return - permissions - .insertBool(managedRights.canDisableJoinExit, _DISABLE_JOIN_EXIT_OFFSET) - .insertBool(managedRights.canChangeMgmtFees, _CHANGE_MGMT_FEES_OFFSET) - .insertBool(managedRights.canChangeTokens, _CHANGE_TOKENS_OFFSET) - .insertBool(managedRights.canSetCircuitBreakers, _CIRCUIT_BREAKERS_OFFSET); - } - - /** - * @dev Getter for the canChangeWeights permission. - */ - function canChangeWeights() public view returns (bool) { - return _controllerState.decodeBool(_CHANGE_WEIGHTS_OFFSET); - } - - /** - * @dev Getter for the canDisableSwaps permission. - */ - function canDisableSwaps() public view returns (bool) { - return _controllerState.decodeBool(_DISABLE_SWAPS_OFFSET); - } - - /** - * @dev Getter for the mustAllowlistLPs permission. - */ - function canSetMustAllowlistLPs() public view returns (bool) { - return _controllerState.decodeBool(_MUST_ALLOWLIST_LPS_OFFSET); - } - - /** - * @dev Getter for the canSetCircuitBreakers permission. - */ - function canSetCircuitBreakers() public view returns (bool) { - return _controllerState.decodeBool(_CIRCUIT_BREAKERS_OFFSET); - } - - /** - * @dev Getter for the canChangeTokens permission. - */ - function canChangeTokens() public view returns (bool) { - return _controllerState.decodeBool(_CHANGE_TOKENS_OFFSET); - } - - /** - * @dev Getter for the canChangeManagementFees permission. - */ - function canChangeManagementFees() public view returns (bool) { - return _controllerState.decodeBool(_CHANGE_MGMT_FEES_OFFSET); - } - - /** - * @dev Getter for the canDisableJoinExit permission. - */ - function canDisableJoinExit() public view returns (bool) { - return _controllerState.decodeBool(_DISABLE_JOIN_EXIT_OFFSET); - } - - /** - * @dev Getter for the minimum weight change duration. - */ - function getMinWeightChangeDuration() external view returns (uint256) { - return _minWeightChangeDuration; - } - - /** - * @dev Update weights linearly from the current values to the given end weights, between startTime - * and endTime. - */ - function updateWeightsGradually( - uint256 startTime, - uint256 endTime, - IERC20[] calldata tokens, - uint256[] calldata endWeights - ) external virtual onlyManager withBoundPool { - _require(canChangeWeights(), Errors.FEATURE_DISABLED); - _require( - endTime >= startTime && endTime - startTime >= _minWeightChangeDuration, - Errors.WEIGHT_CHANGE_TOO_FAST - ); - - IManagedPool(pool).updateWeightsGradually(startTime, endTime, tokens, endWeights); - } - - /** - * @dev Pass a call to ManagedPool's setSwapEnabled through to the underlying pool. - */ - function setSwapEnabled(bool swapEnabled) external virtual onlyManager withBoundPool { - _require(canDisableSwaps(), Errors.FEATURE_DISABLED); - - IManagedPool(pool).setSwapEnabled(swapEnabled); - } - - /** - * @dev Pass a call to ManagedPool's setMustAllowlistLPs through to the underlying pool. This could - * be restricted in various ways. For instance, we could allow it to change state only once, or only - * in one direction, but there seems to be no compelling reason to do so in the reference controller. - * - * Deploying a Managed Pool with an empty allowlist could function like an LBP, or a smart treasury. - * Adding a set of addresses to the allowlist enables multiple seed funding sources. Disabling the - * allowlist, or re-enabling it after allowing public LPs, can impose or remove a "cap" on the total supply. - */ - function setMustAllowlistLPs(bool mustAllowlistLPs) external virtual onlyManager withBoundPool { - _require(canSetMustAllowlistLPs(), Errors.FEATURE_DISABLED); - - IManagedPool(pool).setMustAllowlistLPs(mustAllowlistLPs); - } - - /** - * @dev Pass a call to ManagedPool's addAllowedAddress through to the underlying pool. - * The underlying pool handles all state/permission checks. It will revert if the LP allowlist is off. - */ - function addAllowedAddress(address member) external virtual onlyManager withBoundPool { - IManagedPool(pool).addAllowedAddress(member); - } - - /** - * @dev Pass a call to ManagedPool's removeAllowedAddress through to the underlying pool. - * The underlying pool handles all state/permission checks. It will revert if the address was not - * previouslly added to the allowlist. - */ - function removeAllowedAddress(address member) external virtual onlyManager withBoundPool { - IManagedPool(pool).removeAllowedAddress(member); - } - - /** - * @dev Transfer any BPT management fees from this contract to the recipient. - */ - function withdrawCollectedManagementFees(address recipient) external virtual onlyManager withBoundPool { - IERC20(pool).safeTransfer(recipient, IERC20(pool).balanceOf(address(this))); - } - - /** - * @dev Pass a call to ManagedPool's setManagementAumFeePercentage through to the underlying pool. - */ - function setManagementAumFeePercentage(uint256 managementAumFeePercentage) - external - virtual - onlyManager - withBoundPool - returns (uint256) - { - _require(canChangeManagementFees(), Errors.FEATURE_DISABLED); - - return IManagedPool(pool).setManagementAumFeePercentage(managementAumFeePercentage); - } - - /** - * @dev Pass a call to ManagedPool's setJoinExitEnabled through to the underlying pool. - */ - function setJoinExitEnabled(bool joinExitEnabled) external virtual onlyManager withBoundPool { - _require(canDisableJoinExit(), Errors.FEATURE_DISABLED); - - IManagedPool(pool).setJoinExitEnabled(joinExitEnabled); - } -} diff --git a/pkg/pool-utils/test/ManagedPoolController.test.ts b/pkg/pool-utils/test/ManagedPoolController.test.ts deleted file mode 100644 index e79e762a07..0000000000 --- a/pkg/pool-utils/test/ManagedPoolController.test.ts +++ /dev/null @@ -1,395 +0,0 @@ -import { ethers } from 'hardhat'; -import { expect } from 'chai'; -import { Contract } from 'ethers'; - -import { fp } from '@balancer-labs/v2-helpers/src/numbers'; -import TokenList from '@balancer-labs/v2-helpers/src/models/tokens/TokenList'; -import WeightedPool from '@balancer-labs/v2-helpers/src/models/pools/weighted/WeightedPool'; -import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; -import { - WeightedPoolType, - ManagedPoolRights, - BasePoolRights, -} from '@balancer-labs/v2-helpers/src/models/pools/weighted/types'; -import { deploy } from '@balancer-labs/v2-helpers/src/contract'; -import { currentTimestamp, MONTH, DAY, HOUR } from '@balancer-labs/v2-helpers/src/time'; -import Vault from '@balancer-labs/v2-helpers/src/models/vault/Vault'; -import { ZERO_ADDRESS, MAX_UINT256, randomAddress } from '@balancer-labs/v2-helpers/src/constants'; -import { SwapKind } from '@balancer-labs/balancer-js'; - -const POOL_SWAP_FEE_PERCENTAGE = fp(0.01); -const WEIGHTS = [fp(30), fp(60), fp(5), fp(5)]; -const PAUSE_WINDOW_DURATION = MONTH * 3; -const BUFFER_PERIOD_DURATION = MONTH; -const MIN_WEIGHT_CHANGE_DURATION = DAY; - -let admin: SignerWithAddress; -let manager: SignerWithAddress; -let other: SignerWithAddress; -let pool: WeightedPool; -let allTokens: TokenList; -let vault: Vault; -let poolController: Contract; - -const LONG_UPDATE = DAY * 3; -const SHORT_UPDATE = HOUR * 8; -const END_WEIGHTS = [fp(0.6), fp(0.3), fp(0.05), fp(0.05)]; - -before('setup signers', async () => { - [, admin, manager, other] = await ethers.getSigners(); -}); - -sharedBeforeEach('deploy Vault, asset manager, and tokens', async () => { - vault = await Vault.create({ - admin, - pauseWindowDuration: PAUSE_WINDOW_DURATION, - bufferPeriodDuration: BUFFER_PERIOD_DURATION, - }); - - allTokens = await TokenList.create(['MKR', 'DAI', 'SNX', 'BAT'], { sorted: true }); - await allTokens.mint({ to: manager, amount: fp(100) }); - await allTokens.mint({ to: other, amount: fp(100) }); -}); - -async function deployControllerAndPool( - canTransfer = true, - canChangeSwapFee = true, - canUpdateMetadata = true, - canChangeWeights = true, - canDisableSwaps = true, - canSetMustAllowlistLPs = true, - canSetCircuitBreakers = true, - canChangeTokens = true, - canChangeMgmtFees = true, - canDisableJoinExit = true, - swapEnabledOnStart = true, - protocolSwapFeePercentage = MAX_UINT256 -) { - const basePoolRights: BasePoolRights = { - canTransferOwnership: canTransfer, - canChangeSwapFee: canChangeSwapFee, - canUpdateMetadata: canUpdateMetadata, - }; - - const managedPoolRights: ManagedPoolRights = { - canChangeWeights: canChangeWeights, - canDisableSwaps: canDisableSwaps, - canSetMustAllowlistLPs: canSetMustAllowlistLPs, - canSetCircuitBreakers: canSetCircuitBreakers, - canChangeTokens: canChangeTokens, - canChangeMgmtFees: canChangeMgmtFees, - canDisableJoinExit: canDisableJoinExit, - }; - - poolController = await deploy('ManagedPoolController', { - from: manager, - args: [basePoolRights, managedPoolRights, MIN_WEIGHT_CHANGE_DURATION, manager.address], - }); - const assetManagers = Array(allTokens.length).fill(ZERO_ADDRESS); - assetManagers[allTokens.indexOf(allTokens.DAI)] = await randomAddress(); - - const params = { - vault, - tokens: allTokens, - weights: WEIGHTS, - owner: poolController.address, - assetManagers, - swapFeePercentage: POOL_SWAP_FEE_PERCENTAGE, - poolType: WeightedPoolType.MANAGED_POOL, - swapEnabledOnStart: swapEnabledOnStart, - protocolSwapFeePercentage: protocolSwapFeePercentage, - }; - pool = await WeightedPool.create(params); -} - -// Some tests repeated; could have a behavesLikeBasePoolController.behavior.ts -describe('ManagedPoolController', function () { - const NEW_MGMT_AUM_FEE = fp(0.015); - - context('pool controller not initialized', () => { - sharedBeforeEach('deploy controller (default permissions)', async () => { - await deployControllerAndPool(); - }); - - it('creates the pool', async () => { - expect(await pool.getOwner()).to.equal(poolController.address); - }); - - it('sets up the pool controller', async () => { - expect(await poolController.getManager()).to.equal(manager.address); - }); - - it('sets base pool permissions', async () => { - expect(await poolController.canTransferOwnership()).to.be.true; - expect(await poolController.canChangeSwapFee()).to.be.true; - expect(await poolController.canUpdateMetadata()).to.be.true; - }); - - it('sets managed pool permissions', async () => { - expect(await poolController.canChangeWeights()).to.be.true; - expect(await poolController.canDisableSwaps()).to.be.true; - expect(await poolController.canSetMustAllowlistLPs()).to.be.true; - expect(await poolController.canSetCircuitBreakers()).to.be.true; - expect(await poolController.canChangeTokens()).to.be.true; - expect(await poolController.canChangeManagementFees()).to.be.true; - expect(await poolController.canDisableJoinExit()).to.be.true; - }); - - it('sets the minimum weight change duration', async () => { - expect(await poolController.getMinWeightChangeDuration()).to.equal(MIN_WEIGHT_CHANGE_DURATION); - }); - }); - - context('pool controller is initialized', () => { - sharedBeforeEach('initialize pool controller', async () => { - await deployControllerAndPool(); - await poolController.initialize(pool.address); - }); - - describe('change management aum fee percentage', () => { - it('lets the manager set the management AUM fee', async () => { - await poolController.connect(manager).setManagementAumFeePercentage(NEW_MGMT_AUM_FEE); - - const [aumFeePercentage] = await pool.getManagementAumFeeParams(); - - expect(aumFeePercentage).to.equal(NEW_MGMT_AUM_FEE); - }); - - it('reverts if non-manager sets the management AUM fee', async () => { - await expect(poolController.connect(other).setManagementAumFeePercentage(NEW_MGMT_AUM_FEE)).to.be.revertedWith( - 'CALLER_IS_NOT_OWNER' - ); - }); - }); - - describe('set join / exit enabled', () => { - it('lets the manager disable joins and exits', async () => { - await poolController.connect(manager).setJoinExitEnabled(false); - - expect(await pool.getJoinExitEnabled(manager)).to.be.false; - }); - - it('reverts if non-manager disables joins and exits', async () => { - await expect(poolController.connect(other).setJoinExitEnabled(false)).to.be.revertedWith('CALLER_IS_NOT_OWNER'); - }); - }); - - describe('set swap enabled', () => { - it('lets the manager disable trading', async () => { - await poolController.connect(manager).setSwapEnabled(false); - - expect(await pool.getSwapEnabled(manager)).to.be.false; - }); - - it('reverts if non-manager disables trading', async () => { - await expect(poolController.connect(other).setSwapEnabled(false)).to.be.revertedWith('CALLER_IS_NOT_OWNER'); - }); - }); - - describe('management fee collection', () => { - it('lets the manager collect management fees', async () => { - await poolController.connect(manager).withdrawCollectedManagementFees(manager.address); - }); - - it('reverts if non-manager collects management fees', async () => { - await expect(poolController.connect(other).withdrawCollectedManagementFees(other.address)).to.be.revertedWith( - 'CALLER_IS_NOT_OWNER' - ); - }); - }); - - describe('update weights gradually', () => { - let tokens: string[]; - - sharedBeforeEach('get tokens', async () => { - const { tokens: registeredTokens } = await pool.getTokens(); - // Remove BPT - tokens = registeredTokens.slice(1); - }); - - it('lets the manager update weights gradually', async () => { - const now = await currentTimestamp(); - - await poolController.connect(manager).updateWeightsGradually(now, now.add(LONG_UPDATE), tokens, END_WEIGHTS); - }); - - it('reverts if non-manager updates weights gradually', async () => { - const now = await currentTimestamp(); - - await expect( - poolController.connect(other).updateWeightsGradually(now, now.add(LONG_UPDATE), tokens, END_WEIGHTS) - ).to.be.revertedWith('CALLER_IS_NOT_OWNER'); - }); - - it('reverts if manager updates weights too fast', async () => { - const now = await currentTimestamp(); - - await expect( - poolController.connect(manager).updateWeightsGradually(now, now.add(SHORT_UPDATE), tokens, END_WEIGHTS) - ).to.be.revertedWith('WEIGHT_CHANGE_TOO_FAST'); - }); - }); - - describe('control LP allowlist', () => { - it('lets the manager toggle mustAllowlistLPs', async () => { - await poolController.connect(manager).setMustAllowlistLPs(true); - - expect(await pool.getMustAllowlistLPs()).to.be.true; - }); - - it('reverts if non-manager toggle mustAllowlistLPs', async () => { - await expect(poolController.connect(other).setMustAllowlistLPs(true)).to.be.revertedWith('CALLER_IS_NOT_OWNER'); - }); - }); - }); - - describe('pool controller permissions', () => { - context('with transferrable set to false', () => { - sharedBeforeEach('deploy controller (transferrable false)', async () => { - await deployControllerAndPool(false); - }); - - it('reverts if the manager transfers ownership', async () => { - await expect(poolController.connect(manager).transferOwnership(other.address)).to.be.revertedWith( - 'FEATURE_DISABLED' - ); - }); - }); - - context('with canUpdateMetadata set to false', () => { - sharedBeforeEach('deploy controller (canUpdateMetadata false)', async () => { - await deployControllerAndPool(true, true, false); - await poolController.initialize(pool.address); - }); - - it('reverts if the manager updates metadata', async () => { - await expect(poolController.connect(manager).updateMetadata('0x')).to.be.revertedWith('FEATURE_DISABLED'); - }); - }); - - context('with canChangeWeights set to false', () => { - sharedBeforeEach('deploy controller (canChangeWeights false)', async () => { - await deployControllerAndPool(true, true, true, false); - await poolController.initialize(pool.address); - }); - - it('reverts if the manager updates weights', async () => { - const now = await currentTimestamp(); - const { tokens } = await pool.getTokens(); - - await expect( - poolController.connect(manager).updateWeightsGradually(now, now.add(LONG_UPDATE), tokens, END_WEIGHTS) - ).to.be.revertedWith('FEATURE_DISABLED'); - }); - }); - - context('with canDisableSwaps set to false', () => { - sharedBeforeEach('deploy controller (canDisableSwaps false)', async () => { - await deployControllerAndPool(true, true, true, true, false); - await poolController.initialize(pool.address); - }); - - it('reverts if the manager disables swaps', async () => { - await expect(poolController.connect(manager).setSwapEnabled(false)).to.be.revertedWith('FEATURE_DISABLED'); - }); - }); - - context('with canSetMustAllowlistLPs set to false', () => { - sharedBeforeEach('deploy controller (canSetMustAllowlistLPs false)', async () => { - await deployControllerAndPool(true, true, true, true, true, false); - await poolController.initialize(pool.address); - }); - - it('reverts if the manager tries to disable the allowlist', async () => { - await expect(poolController.connect(manager).setMustAllowlistLPs(true)).to.be.revertedWith('FEATURE_DISABLED'); - }); - }); - - context('with canChangeMgmtFees set to false', () => { - sharedBeforeEach('deploy controller (canChangeMgmtFees false)', async () => { - await deployControllerAndPool(true, true, true, true, true, true, true, true, false); - await poolController.initialize(pool.address); - }); - - it('reverts if the manager tries to change the management AUM fee', async () => { - await expect( - poolController.connect(manager).setManagementAumFeePercentage(NEW_MGMT_AUM_FEE) - ).to.be.revertedWith('FEATURE_DISABLED'); - }); - }); - - context('with canDisableJoinExit set to false', () => { - sharedBeforeEach('deploy controller (canDisableJoinExit false)', async () => { - await deployControllerAndPool(true, true, true, true, true, true, true, true, true, false); - await poolController.initialize(pool.address); - }); - - it('reverts if the manager disables swaps', async () => { - await expect(poolController.connect(manager).setJoinExitEnabled(false)).to.be.revertedWith('FEATURE_DISABLED'); - }); - }); - - context('with public swaps disabled (on start)', () => { - sharedBeforeEach('deploy controller (swapEnabledOnStart false)', async () => { - await deployControllerAndPool(true, true, true, true, true, true, true, true, true, true, false); - await poolController.initialize(pool.address); - await allTokens.approve({ from: manager, to: await pool.getVault() }); - const initialBalances = Array(allTokens.length).fill(fp(1)); - await pool.init({ from: manager, initialBalances }); - }); - - it('reverts if anyone swaps', async () => { - const singleSwap = { - poolId: await pool.getPoolId(), - kind: SwapKind.GivenIn, - assetIn: allTokens.first.address, - assetOut: allTokens.second.address, - amount: fp(0.01), - userData: '0x', - }; - const funds = { - sender: manager.address, - fromInternalBalance: false, - recipient: other.address, - toInternalBalance: false, - }; - const limit = 0; // Minimum amount out - const deadline = MAX_UINT256; - - await expect(vault.instance.connect(manager).swap(singleSwap, funds, limit, deadline)).to.be.revertedWith( - 'SWAPS_DISABLED' - ); - }); - }); - - context('with canSetCircuitBreakers set to false', () => { - sharedBeforeEach('deploy controller (canSetCircuitBreakers false)', async () => { - await deployControllerAndPool(true, true, true, true, true, true, false); - }); - - it('sets the set circuit breakers permission', async () => { - expect(await poolController.canSetCircuitBreakers()).to.be.false; - }); - }); - - context('with canChangeTokens set to false', () => { - sharedBeforeEach('deploy controller (canChangeTokens false)', async () => { - await deployControllerAndPool(true, true, true, true, true, true, true, false); - }); - - it('sets the change tokens permission', async () => { - expect(await poolController.canChangeTokens()).to.be.false; - }); - }); - - context('with canChangeMgmtFees set to false', () => { - sharedBeforeEach('deploy controller (canChangeMgmtFees false)', async () => { - await deployControllerAndPool(true, true, true, true, true, true, true, true, false); - }); - - it('sets the set management fee permission', async () => { - expect(await poolController.canChangeManagementFees()).to.be.false; - }); - }); - }); -}); diff --git a/pkg/pool-weighted/contracts/managed/ControlledManagedPoolFactory.sol b/pkg/pool-weighted/contracts/managed/ControlledManagedPoolFactory.sol deleted file mode 100644 index 6773cc9070..0000000000 --- a/pkg/pool-weighted/contracts/managed/ControlledManagedPoolFactory.sol +++ /dev/null @@ -1,72 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-or-later -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -pragma solidity ^0.7.0; -pragma experimental ABIEncoderV2; - -import "@balancer-labs/v2-pool-utils/contracts/controllers/ManagedPoolController.sol"; - -import "./ManagedPoolFactory.sol"; - -/** - * @dev Deploys a new `ManagedPool` owned by a ManagedPoolController with the specified rights. - * It uses the ManagedPoolFactory to deploy the pool. - */ -contract ControlledManagedPoolFactory { - // The address of the ManagedPoolFactory used to deploy the ManagedPool - address public immutable managedPoolFactory; - - mapping(address => bool) private _isPoolFromFactory; - - event ManagedPoolCreated(address indexed pool, address indexed poolController); - - constructor(address factory) { - managedPoolFactory = factory; - } - - /** - * @dev Deploys a new `ManagedPool`. - */ - function create( - ManagedPool.ManagedPoolParams memory params, - ManagedPoolSettings.ManagedPoolSettingsParams memory settingsParams, - BasePoolController.BasePoolRights calldata basePoolRights, - ManagedPoolController.ManagedPoolRights calldata managedPoolRights, - uint256 minWeightChangeDuration, - address manager - ) external returns (address pool) { - ManagedPoolController poolController = new ManagedPoolController( - basePoolRights, - managedPoolRights, - minWeightChangeDuration, - manager - ); - - // Let the base factory deploy the pool (owner is the controller) - pool = ManagedPoolFactory(managedPoolFactory).create(params, settingsParams, address(poolController)); - - // Finally, initialize the controller - poolController.initialize(pool); - - _isPoolFromFactory[pool] = true; - emit ManagedPoolCreated(pool, address(poolController)); - } - - /** - * @dev Returns true if `pool` was created by this factory. - */ - function isPoolFromFactory(address pool) external view returns (bool) { - return _isPoolFromFactory[pool]; - } -} diff --git a/pkg/pool-weighted/test/ControlledManagedPoolFactory.test.ts b/pkg/pool-weighted/test/ControlledManagedPoolFactory.test.ts deleted file mode 100644 index b9c4a63614..0000000000 --- a/pkg/pool-weighted/test/ControlledManagedPoolFactory.test.ts +++ /dev/null @@ -1,269 +0,0 @@ -import { ethers } from 'hardhat'; -import { expect } from 'chai'; -import { BigNumber, Contract } from 'ethers'; - -import { fp } from '@balancer-labs/v2-helpers/src/numbers'; -import { advanceTime, currentTimestamp, MONTH, DAY } from '@balancer-labs/v2-helpers/src/time'; -import * as expectEvent from '@balancer-labs/v2-helpers/src/test/expectEvent'; -import { ZERO_ADDRESS } from '@balancer-labs/v2-helpers/src/constants'; -import { deploy, deployedAt } from '@balancer-labs/v2-helpers/src/contract'; -import { expectEqualWithError } from '@balancer-labs/v2-helpers/src/test/relativeError'; - -import Vault from '@balancer-labs/v2-helpers/src/models/vault/Vault'; -import TokenList from '@balancer-labs/v2-helpers/src/models/tokens/TokenList'; -import { toNormalizedWeights } from '@balancer-labs/balancer-js'; -import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; -import { - BasePoolRights, - ManagedPoolParams, - ManagedPoolRights, -} from '@balancer-labs/v2-helpers/src/models/pools/weighted/types'; -import { ProtocolFee } from '@balancer-labs/v2-helpers/src/models/vault/types'; - -describe('ControlledManagedPoolFactory', function () { - let tokens: TokenList; - let factory: Contract; - let controlledFactory: Contract; - let poolController: Contract; - let vault: Vault; - let manager: SignerWithAddress; - let assetManager: SignerWithAddress; - let admin: SignerWithAddress; - let poolControllerAddress: string; - - const NAME = 'Balancer Pool Token'; - const SYMBOL = 'BPT'; - const POOL_SWAP_FEE_PERCENTAGE = fp(0.01); - const POOL_MANAGEMENT_AUM_FEE_PERCENTAGE = fp(0.01); - const MIN_WEIGHT_CHANGE_DURATION = DAY; - const WEIGHTS = toNormalizedWeights([fp(30), fp(70), fp(5), fp(5)]); - - const BASE_PAUSE_WINDOW_DURATION = MONTH * 3; - const BASE_BUFFER_PERIOD_DURATION = MONTH; - - let createTime: BigNumber; - - before('setup signers', async () => { - [, admin, manager, assetManager] = await ethers.getSigners(); - }); - - sharedBeforeEach('deploy factory & tokens', async () => { - vault = await Vault.create({ admin }); - - const circuitBreakerLib = await deploy('CircuitBreakerLib'); - const addRemoveTokenLib = await deploy('ManagedPoolAddRemoveTokenLib'); - factory = await deploy('ManagedPoolFactory', { - args: [vault.address, vault.getFeesProvider().address, 'factoryVersion', 'poolVersion'], - libraries: { - CircuitBreakerLib: circuitBreakerLib.address, - ManagedPoolAddRemoveTokenLib: addRemoveTokenLib.address, - }, - }); - controlledFactory = await deploy('ControlledManagedPoolFactory', { args: [factory.address] }); - - tokens = await TokenList.create(['MKR', 'DAI', 'SNX', 'BAT'], { sorted: true }); - }); - - async function createPool( - canTransfer = true, - canChangeSwapFee = true, - swapsEnabled = true, - mustAllowlistLPs = false - ): Promise { - const assetManagers: string[] = Array(tokens.length).fill(ZERO_ADDRESS); - assetManagers[tokens.indexOf(tokens.DAI)] = assetManager.address; - - const poolParams = { - name: NAME, - symbol: SYMBOL, - assetManagers, - }; - - const settingsParams: ManagedPoolParams = { - tokens: tokens.addresses, - normalizedWeights: WEIGHTS, - swapFeePercentage: POOL_SWAP_FEE_PERCENTAGE, - swapEnabledOnStart: swapsEnabled, - mustAllowlistLPs: mustAllowlistLPs, - managementAumFeePercentage: POOL_MANAGEMENT_AUM_FEE_PERCENTAGE, - aumFeeId: ProtocolFee.AUM, - }; - - const basePoolRights: BasePoolRights = { - canTransferOwnership: canTransfer, - canChangeSwapFee: canChangeSwapFee, - canUpdateMetadata: true, - }; - - const managedPoolRights: ManagedPoolRights = { - canChangeWeights: true, - canDisableSwaps: true, - canSetMustAllowlistLPs: true, - canSetCircuitBreakers: true, - canChangeTokens: true, - canChangeMgmtFees: true, - }; - - const receipt = await ( - await controlledFactory - .connect(manager) - .create( - poolParams, - settingsParams, - basePoolRights, - managedPoolRights, - MIN_WEIGHT_CHANGE_DURATION, - manager.address - ) - ).wait(); - - const event = expectEvent.inReceipt(receipt, 'ManagedPoolCreated'); - poolControllerAddress = event.args.poolController; - - return deployedAt('ManagedPool', event.args.pool); - } - - describe('constructor arguments', () => { - let pool: Contract; - - sharedBeforeEach(async () => { - pool = await createPool(); - poolController = await deployedAt('ManagedPoolController', pool.getOwner()); - }); - - it('sets the vault', async () => { - expect(await pool.getVault()).to.equal(vault.address); - }); - - it('sets the base factory', async () => { - expect(await controlledFactory.managedPoolFactory()).to.equal(factory.address); - }); - - it('registers the pool', async () => { - expect(await controlledFactory.isPoolFromFactory(pool.address)).to.be.true; - expect(await controlledFactory.isPoolFromFactory(assetManager.address)).to.be.false; - }); - - it('sets the pool owner', async () => { - expect(await pool.getOwner()).to.equal(poolControllerAddress); - }); - - it('registers tokens in the vault', async () => { - const poolId = await pool.getPoolId(); - const poolTokens = await vault.getPoolTokens(poolId); - - expect(poolTokens.tokens).to.deep.eq([pool.address, ...tokens.addresses]); - expect(poolTokens.balances).to.be.zeros; - }); - - it('starts with no BPT', async () => { - expect(await pool.totalSupply()).to.be.equal(0); - }); - - it('sets asset managers', async () => { - await tokens.asyncEach(async (token) => { - const poolId = await pool.getPoolId(); - const info = await vault.getPoolTokenInfo(poolId, token); - if (token.address == tokens.DAI.address) { - expect(info.assetManager).to.equal(assetManager.address); - } else { - expect(info.assetManager).to.equal(ZERO_ADDRESS); - } - }); - }); - - it('sets swap fee', async () => { - expect(await pool.getSwapFeePercentage()).to.equal(POOL_SWAP_FEE_PERCENTAGE); - }); - - it('sets management aum fee', async () => { - const [aumFeePercentage, lastCollectionTimestamp] = await pool.getManagementAumFeeParams(); - expect(aumFeePercentage).to.equal(POOL_MANAGEMENT_AUM_FEE_PERCENTAGE); - expect(lastCollectionTimestamp).to.equal(0); - }); - - it('sets the pool manager ', async () => { - expect(await poolController.getManager()).to.equal(manager.address); - }); - - it('sets the name', async () => { - expect(await pool.name()).to.equal('Balancer Pool Token'); - }); - - it('sets the symbol', async () => { - expect(await pool.symbol()).to.equal('BPT'); - }); - - it('sets the decimals', async () => { - expect(await pool.decimals()).to.equal(18); - }); - }); - - describe('temporarily pausable', () => { - sharedBeforeEach(async () => { - createTime = await currentTimestamp(); - }); - - it('pools have the correct window end times', async () => { - const pool = await createPool(); - const { pauseWindowEndTime, bufferPeriodEndTime } = await pool.getPausedState(); - - // Pool creation slow enough to introduce an error here - expectEqualWithError(pauseWindowEndTime, createTime.add(BASE_PAUSE_WINDOW_DURATION), 5); - expectEqualWithError( - bufferPeriodEndTime, - createTime.add(BASE_PAUSE_WINDOW_DURATION + BASE_BUFFER_PERIOD_DURATION), - 5 - ); - }); - - it('multiple pools have the same window end times', async () => { - const firstPool = await createPool(); - await advanceTime(BASE_PAUSE_WINDOW_DURATION / 3); - const secondPool = await createPool(); - - const { firstPauseWindowEndTime, firstBufferPeriodEndTime } = await firstPool.getPausedState(); - const { secondPauseWindowEndTime, secondBufferPeriodEndTime } = await secondPool.getPausedState(); - - expect(firstPauseWindowEndTime).to.equal(secondPauseWindowEndTime); - expect(firstBufferPeriodEndTime).to.equal(secondBufferPeriodEndTime); - }); - - it('pools created after the pause window end date have no buffer period', async () => { - await advanceTime(BASE_PAUSE_WINDOW_DURATION + 1); - - const pool = await createPool(); - const { pauseWindowEndTime, bufferPeriodEndTime } = await pool.getPausedState(); - const now = await currentTimestamp(); - - expect(pauseWindowEndTime).to.equal(now); - expect(bufferPeriodEndTime).to.equal(now); - }); - }); - - describe('initial state', () => { - it('pool created with swaps enabled', async () => { - const pool = await createPool(); - - expect(await pool.getSwapEnabled()).to.be.true; - }); - - it('pool created with swaps disabled', async () => { - const pool = await createPool(true, true, false); - - expect(await pool.getSwapEnabled()).to.be.false; - }); - - it('pool created with allowlist LPs', async () => { - const pool = await createPool(true, true, true, true); - - expect(await pool.getMustAllowlistLPs()).to.be.true; - }); - - it('pool created with allowlist LPs disabled', async () => { - const pool = await createPool(true, true, true, false); - - expect(await pool.getMustAllowlistLPs()).to.be.false; - }); - }); -}); From fc32ab3204def63ff2d1583e8212884b43b499d1 Mon Sep 17 00:00:00 2001 From: Jeffrey Bennett Date: Fri, 2 Dec 2022 11:46:16 -0500 Subject: [PATCH 2/4] remove BaseController and tests --- .../controllers/BasePoolController.sol | 254 ------------------ .../test/BasePoolController.test.ts | 213 --------------- 2 files changed, 467 deletions(-) delete mode 100644 pkg/pool-utils/contracts/controllers/BasePoolController.sol delete mode 100644 pkg/pool-utils/test/BasePoolController.test.ts diff --git a/pkg/pool-utils/contracts/controllers/BasePoolController.sol b/pkg/pool-utils/contracts/controllers/BasePoolController.sol deleted file mode 100644 index 8f364c6b2c..0000000000 --- a/pkg/pool-utils/contracts/controllers/BasePoolController.sol +++ /dev/null @@ -1,254 +0,0 @@ -// SPDX-License-Identifier: GPL-3.0-or-later -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -pragma solidity ^0.7.0; -pragma experimental ABIEncoderV2; - -import "@balancer-labs/v2-interfaces/contracts/pool-utils/IBasePoolController.sol"; - -import "@balancer-labs/v2-solidity-utils/contracts/helpers/WordCodec.sol"; - -import "../BasePoolAuthorization.sol"; - -/** - * @dev Pool controller that serves as the owner of a Balancer pool, and is in turn owned by - * an account empowered to make calls on this contract, which are forwarded to the underlyling pool. - * - * While the owner of the underlying Balancer pool is immutable, ownership of this pool controller - * can be transferred, if the corresponding permission is set to allow this operation. To prevent - * accidentally setting the manager to an invalid address (and irrevocably losing control of the pool), - * transferring ownership is a two-step process. - * - * If the changeSwapFee permission is enabled, the manager can call setSwapFeePercentage, or delegate - * this right to a different address. - */ -contract BasePoolController is IBasePoolController { - using WordCodec for bytes32; - - // There are three basic pool rights: one for transferring ownership, one for changing the swap fee, - // and the last for associating arbitrary metadata with the pool. - struct BasePoolRights { - bool canTransferOwnership; - bool canChangeSwapFee; - bool canUpdateMetadata; - } - - // The address empowered to call permissioned functions. - address private _manager; - - // Target of a proposed transfer of ownership. Will be non-zero if there is a transfer pending. - // This address must call claimOwnership to complete the transfer. - address private _managerCandidate; - - // Address allowed to call `setSwapFeePercentage`. Initially set to the owner (or 0 if fees are fixed) - address private _swapFeeController; - - // Address of the underlying pool. - address public pool; - - // Immutable controller state - the first 16 bits are reserved as a bitmap for permission flags - // (3 used in this base class), and the remaining 240 bits can be used by derived classes to store - // any other immutable data. - // - // [ | 16 bits for permission flags ] - // [ 240 bits | 13 bits | 1 bit | 1 bit | 1 bit ] - // [ unused | reserved | metadata | swap fee | transfer ] - // |MSB LSB| - bytes32 internal immutable _controllerState; - - uint256 private constant _TRANSFER_OWNERSHIP_OFFSET = 0; - uint256 private constant _CHANGE_SWAP_FEE_OFFSET = 1; - uint256 private constant _UPDATE_METADATA_OFFSET = 2; - - // Optional metadata associated with this controller (or the pool bound to it) - bytes private _metadata; - - // Event declarations - - event OwnershipTransferred(address indexed previousManager, address indexed newManager); - event SwapFeeControllerChanged(address indexed oldSwapFeeController, address indexed newSwapFeeController); - event MetadataUpdated(bytes metadata); - - // Modifiers - - // Add this modifier to all functions that call the underlying pool. - modifier withBoundPool { - _ensurePoolIsBound(); - _; - } - - /** - * @dev Reverts if called by any account other than the manager. - */ - modifier onlyManager() { - _require(getManager() == msg.sender, Errors.CALLER_IS_NOT_OWNER); - _; - } - - /** - * @dev Set permissions and the initial manager for the pool controller. We are "trusting" the manager to be - * a valid address on deployment, as does BasePool. Transferring ownership to a new manager after deployment - * employs a safe, two-step process. - */ - constructor(bytes32 controllerState, address manager) { - _controllerState = controllerState; - _manager = manager; - - // If the swap fee is not fixed, it can be delegated (initially, to the manager). - if (controllerState.decodeBool(_CHANGE_SWAP_FEE_OFFSET)) { - _swapFeeController = manager; - } - } - - /** - * @dev Encode the BaseController portion of the controllerState. This is mainly useful for - * derived classes to call during construction. - */ - function encodePermissions(BasePoolRights memory rights) public pure returns (bytes32) { - bytes32 permissions; - - return - permissions - .insertBool(rights.canTransferOwnership, _TRANSFER_OWNERSHIP_OFFSET) - .insertBool(rights.canChangeSwapFee, _CHANGE_SWAP_FEE_OFFSET) - .insertBool(rights.canUpdateMetadata, _UPDATE_METADATA_OFFSET); - } - - /** - * @dev Getter for the current manager. - */ - function getManager() public view returns (address) { - return _manager; - } - - /** - * @dev Returns the manager candidate, which will be non-zero if there is a pending ownership transfer. - */ - function getManagerCandidate() external view returns (address) { - return _managerCandidate; - } - - /** - * @dev Getter for the current swap fee controller (0 if fees are fixed). - */ - function getSwapFeeController() public view returns (address) { - return _swapFeeController; - } - - /** - * @dev Getter for the transferOwnership permission. - */ - function canTransferOwnership() public view returns (bool) { - return _controllerState.decodeBool(_TRANSFER_OWNERSHIP_OFFSET); - } - - /** - * @dev Getter for the canChangeSwapFee permission. - */ - function canChangeSwapFee() public view returns (bool) { - return _controllerState.decodeBool(_CHANGE_SWAP_FEE_OFFSET); - } - - /** - * @dev Getter for the canUpdateMetadata permission. - */ - function canUpdateMetadata() public view returns (bool) { - return _controllerState.decodeBool(_UPDATE_METADATA_OFFSET); - } - - /** - * @dev The underlying pool owner is immutable, so its address must be known when the pool is deployed. - * This means the controller needs to be deployed first. Yet the controller also needs to know the address - * of the pool it is controlling. - * - * We could either pass in a pool factory and have the controller deploy the pool, or have an initialize - * function to set the pool address after deployment. This decoupled mechanism seems cleaner. - * - * It means the pool address must be in storage vs immutable, but this is acceptable for infrequent admin - * operations. - */ - function initialize(address poolAddress) public virtual override { - // This can only be called once - and the owner of the pool must be this contract - _require( - pool == address(0) && BasePoolAuthorization(poolAddress).getOwner() == address(this), - Errors.INVALID_INITIALIZATION - ); - - pool = poolAddress; - } - - /** - * @dev Stores the proposed new manager in `_managerCandidate`. To prevent accidental transfer to an invalid - * address, the candidate address must call `claimOwnership` to complete the transfer. - * - * Can only be called by the current manager. - */ - function transferOwnership(address newManager) external onlyManager { - _require(canTransferOwnership(), Errors.FEATURE_DISABLED); - - _managerCandidate = newManager; - } - - /** - * @dev This must be called by the manager candidate to complete the transferwnership operation. - * This "claimable" mechanism prevents accidental transfer of ownership to an invalid address. - * - * To keep this function simple and focused, transferring ownership does not affect the swapFeeController. - * Sometimes the new owner might want to retain the "old" swap fee controller (e.g., if it was - * delegated to Gauntlet). Other times an owner may want to take control of fees from the previous - * owner. In the latter case, the new owner should call `setSwapFeeController`. - */ - function claimOwnership() external { - address candidate = _managerCandidate; - - _require(candidate == msg.sender, Errors.SENDER_NOT_ALLOWED); - - emit OwnershipTransferred(_manager, candidate); - _manager = candidate; - - // Setting the candidate to zero prevents calling this repeatedly and generating multiple redundant events, - // and also allows checking (perhaps by a UI) whether there is a pending transfer. - _managerCandidate = address(0); - } - - /** - * @dev Change the address allowed to call setSwapFeePercentage. - */ - function setSwapFeeController(address newSwapFeeController) external onlyManager { - emit SwapFeeControllerChanged(getSwapFeeController(), newSwapFeeController); - - _swapFeeController = newSwapFeeController; - } - - /** - * @dev Getter for the optional metadata. - */ - function getMetadata() public view returns (bytes memory) { - return _metadata; - } - - /** - * @dev Setter for the admin to set/update the metadata - */ - function updateMetadata(bytes memory metadata) external onlyManager { - _require(canUpdateMetadata(), Errors.FEATURE_DISABLED); - - _metadata = metadata; - emit MetadataUpdated(metadata); - } - - function _ensurePoolIsBound() private view { - _require(pool != address(0), Errors.UNINITIALIZED_POOL_CONTROLLER); - } -} diff --git a/pkg/pool-utils/test/BasePoolController.test.ts b/pkg/pool-utils/test/BasePoolController.test.ts deleted file mode 100644 index a4f4f67e66..0000000000 --- a/pkg/pool-utils/test/BasePoolController.test.ts +++ /dev/null @@ -1,213 +0,0 @@ -import { ethers } from 'hardhat'; -import { expect } from 'chai'; -import { Contract } from 'ethers'; - -import { fp } from '@balancer-labs/v2-helpers/src/numbers'; -import TokenList from '@balancer-labs/v2-helpers/src/models/tokens/TokenList'; -import WeightedPool from '@balancer-labs/v2-helpers/src/models/pools/weighted/WeightedPool'; -import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; -import { WeightedPoolType, BasePoolRights } from '@balancer-labs/v2-helpers/src/models/pools/weighted/types'; -import { deploy } from '@balancer-labs/v2-helpers/src/contract'; -import { MONTH } from '@balancer-labs/v2-helpers/src/time'; -import Vault from '@balancer-labs/v2-helpers/src/models/vault/Vault'; -import * as expectEvent from '@balancer-labs/v2-helpers/src/test/expectEvent'; -import { randomAddress, ZERO_ADDRESS } from '@balancer-labs/v2-helpers/src/constants'; -import TypesConverter from '@balancer-labs/v2-helpers/src/models/types/TypesConverter'; - -const POOL_SWAP_FEE_PERCENTAGE = fp(0.01); -const WEIGHTS = [fp(30), fp(60), fp(5), fp(5)]; -const PAUSE_WINDOW_DURATION = MONTH * 3; -const BUFFER_PERIOD_DURATION = MONTH; -const METADATA = '0x4b04c67fb743403d339729f8438ecad295a3a015ca144a0945bb6bb9abe3da20'; - -let admin: SignerWithAddress; -let manager: SignerWithAddress; -let other: SignerWithAddress; -let pool: WeightedPool; -let allTokens: TokenList; -let vault: Vault; -let poolController: Contract; - -before('setup signers', async () => { - [, admin, manager, other] = await ethers.getSigners(); -}); - -sharedBeforeEach('deploy Vault, asset manager, and tokens', async () => { - vault = await Vault.create({ - admin, - pauseWindowDuration: PAUSE_WINDOW_DURATION, - bufferPeriodDuration: BUFFER_PERIOD_DURATION, - }); - - allTokens = await TokenList.create(['MKR', 'DAI', 'SNX', 'BAT'], { sorted: true }); - await allTokens.mint({ to: manager, amount: fp(100) }); -}); - -async function deployControllerAndPool(canTransfer = true, canChangeSwapFee = true, canUpdateMetadata = true) { - const basePoolRights: BasePoolRights = { - canTransferOwnership: canTransfer, - canChangeSwapFee: canChangeSwapFee, - canUpdateMetadata: canUpdateMetadata, - }; - - const controllerState = TypesConverter.toEncodedBasePoolRights(basePoolRights); - - poolController = await deploy('BasePoolController', { from: manager, args: [controllerState, manager.address] }); - const assetManagers = Array(allTokens.length).fill(ZERO_ADDRESS); - assetManagers[allTokens.indexOf(allTokens.DAI)] = await randomAddress(); - - const params = { - vault, - tokens: allTokens, - weights: WEIGHTS, - owner: poolController.address, - assetManagers, - swapFeePercentage: POOL_SWAP_FEE_PERCENTAGE, - poolType: WeightedPoolType.WEIGHTED_POOL, - }; - pool = await WeightedPool.create(params); -} - -describe('BasePoolController', function () { - context('pool controller not initialized', () => { - sharedBeforeEach('deploy controller (default permissions)', async () => { - await deployControllerAndPool(); - }); - - it('creates the pool', async () => { - expect(await pool.getOwner()).to.equal(poolController.address); - }); - - it('sets up the pool controller', async () => { - expect(await poolController.getManager()).to.equal(manager.address); - }); - - it('sets all permissions', async () => { - const allRights: BasePoolRights = { - canTransferOwnership: true, - canChangeSwapFee: true, - canUpdateMetadata: true, - }; - - expect(await poolController.canTransferOwnership()).to.be.true; - expect(await poolController.canChangeSwapFee()).to.be.true; - expect(await poolController.canUpdateMetadata()).to.be.true; - - const controllerState = await poolController.encodePermissions(allRights); - const calculatedState = TypesConverter.toEncodedBasePoolRights(allRights); - - expect(controllerState).to.equal(calculatedState); - }); - - describe('metadata', () => { - it('has no initial metadata', async () => { - expect(await poolController.getMetadata()).to.equal('0x'); - }); - - it('lets the manager update metadata', async () => { - const tx = await poolController.connect(manager).updateMetadata(METADATA); - expectEvent.inReceipt(await tx.wait(), 'MetadataUpdated', { - metadata: METADATA, - }); - - expect(await poolController.getMetadata()).to.equal(METADATA); - }); - - it('reverts if a non-manager updates metadata', async () => { - await expect(poolController.connect(other).updateMetadata(METADATA)).to.be.revertedWith('CALLER_IS_NOT_OWNER'); - }); - }); - }); - - context('pool controller is initialized', () => { - sharedBeforeEach('initialize pool controller', async () => { - await deployControllerAndPool(); - await poolController.initialize(pool.address); - }); - }); - - describe('pool controller permissions', () => { - context('with transferrable set to false', () => { - sharedBeforeEach('deploy controller (transferrable false)', async () => { - await deployControllerAndPool(false); - }); - - it('sets the permission to false', async () => { - expect(await poolController.canTransferOwnership()).to.be.false; - }); - - it('reverts if the manager transfers ownership', async () => { - await expect(poolController.connect(manager).transferOwnership(other.address)).to.be.revertedWith( - 'FEATURE_DISABLED' - ); - }); - }); - - context('with transferrable set to true', () => { - sharedBeforeEach('deploy controller (transferrable true)', async () => { - await deployControllerAndPool(true); - }); - - it('reverts if a non-candidate claims ownership', async () => { - await expect(poolController.connect(other).claimOwnership()).to.be.revertedWith('SENDER_NOT_ALLOWED'); - }); - - it('reverts if a non-manager transfers ownership', async () => { - await expect(poolController.connect(other).transferOwnership(other.address)).to.be.revertedWith( - 'CALLER_IS_NOT_OWNER' - ); - }); - - it('lets the manager transfer ownership', async () => { - await poolController.initialize(pool.address); - await poolController.connect(manager).transferOwnership(other.address); - - // Still have the old manager until claimed - expect(await poolController.getManager()).to.equal(manager.address); - expect(await poolController.getManagerCandidate()).to.equal(other.address); - - await expect(poolController.connect(admin).claimOwnership()).to.be.revertedWith('SENDER_NOT_ALLOWED'); - - const tx = await poolController.connect(other).claimOwnership(); - expectEvent.inReceipt(await tx.wait(), 'OwnershipTransferred', { - previousManager: manager.address, - newManager: other.address, - }); - - // Now the manager has changed - expect(await poolController.getManager()).to.equal(other.address); - expect(await poolController.getManagerCandidate()).to.equal(ZERO_ADDRESS); - }); - }); - - context('with set swap fee set to false', () => { - sharedBeforeEach('deploy controller (set swap fee false)', async () => { - await deployControllerAndPool(true, false); - await poolController.initialize(pool.address); - }); - - it('sets the permission to false', async () => { - expect(await poolController.canChangeSwapFee()).to.be.false; - }); - - it('sets the swap fee controller to zero', async () => { - expect(await poolController.getSwapFeeController()).to.equal(ZERO_ADDRESS); - }); - }); - - context('with update metadata set to false', () => { - sharedBeforeEach('deploy controller (update metadata false)', async () => { - await deployControllerAndPool(true, true, false); - await poolController.initialize(pool.address); - }); - - it('sets the permission to false', async () => { - expect(await poolController.canUpdateMetadata()).to.be.false; - }); - - it('reverts if the manager updates metadata', async () => { - await expect(poolController.connect(manager).updateMetadata(METADATA)).to.be.revertedWith('FEATURE_DISABLED'); - }); - }); - }); -}); From 88d676485c900706f5349849e00a351723fd841d Mon Sep 17 00:00:00 2001 From: Jeffrey Bennett Date: Fri, 2 Dec 2022 13:00:32 -0500 Subject: [PATCH 3/4] fix: remove stale references in benchmarks --- pvt/benchmarks/misc.ts | 30 ++++--------------- .../pools/weighted/WeightedPoolDeployer.ts | 24 ++------------- 2 files changed, 7 insertions(+), 47 deletions(-) diff --git a/pvt/benchmarks/misc.ts b/pvt/benchmarks/misc.ts index 33860d4134..2170f4761c 100644 --- a/pvt/benchmarks/misc.ts +++ b/pvt/benchmarks/misc.ts @@ -9,13 +9,9 @@ import Vault from '@balancer-labs/v2-helpers/src/models/vault/Vault'; import { StablePoolEncoder, toNormalizedWeights, WeightedPoolEncoder } from '@balancer-labs/balancer-js'; import { MAX_UINT256, ZERO_ADDRESS, MAX_WEIGHTED_TOKENS } from '@balancer-labs/v2-helpers/src/constants'; import { bn } from '@balancer-labs/v2-helpers/src/numbers'; -import { advanceTime, MONTH, DAY } from '@balancer-labs/v2-helpers/src/time'; +import { advanceTime, MONTH } from '@balancer-labs/v2-helpers/src/time'; import { range } from 'lodash'; -import { - BasePoolRights, - ManagedPoolParams, - ManagedPoolRights, -} from '@balancer-labs/v2-helpers/src/models/pools/weighted/types'; +import { ManagedPoolParams } from '@balancer-labs/v2-helpers/src/models/pools/weighted/types'; import { poolConfigs } from './config'; import { ProtocolFee } from '@balancer-labs/v2-helpers/src/models/vault/types'; @@ -97,22 +93,7 @@ export async function deployPool(vault: Vault, tokens: TokenList, poolName: Pool aumFeeId: ProtocolFee.AUM, }; - const basePoolRights: BasePoolRights = { - canTransferOwnership: true, - canChangeSwapFee: true, - canUpdateMetadata: true, - }; - - const managedPoolRights: ManagedPoolRights = { - canChangeWeights: true, - canDisableSwaps: true, - canSetMustAllowlistLPs: true, - canSetCircuitBreakers: true, - canChangeTokens: true, - canChangeMgmtFees: true, - }; - - params = [userInputs, managedPoolSettings, basePoolRights, managedPoolRights, DAY, creator.address]; + params = [userInputs, managedPoolSettings, creator.address]; break; } default: { @@ -210,14 +191,13 @@ async function deployPoolFromFactory( if (poolName == 'ManagedPool') { const addRemoveTokenLib = await deploy('v2-pool-weighted/ManagedPoolAddRemoveTokenLib'); const circuitBreakerLib = await deploy('v2-pool-weighted/CircuitBreakerLib'); - const baseFactory = await deploy('v2-pool-weighted/ManagedPoolFactory', { + factory = await deploy('v2-pool-weighted/ManagedPoolFactory', { args: [vault.address, vault.getFeesProvider().address, 'factoryVersion', 'poolVersion'], libraries: { CircuitBreakerLib: circuitBreakerLib.address, ManagedPoolAddRemoveTokenLib: addRemoveTokenLib.address, }, }); - factory = await deploy('v2-pool-weighted/ControlledManagedPoolFactory', { args: [baseFactory.address] }); } else if (poolName == 'ComposableStablePool') { factory = await deploy(`${fullName}Factory`, { args: [vault.address, vault.getFeesProvider().address, '', ''] }); } else { @@ -231,7 +211,7 @@ async function deployPoolFromFactory( if (poolName == 'ManagedPool') { receipt = await (await factory.connect(args.from).create(...args.parameters)).wait(); - event = receipt.events?.find((e) => e.event == 'ManagedPoolCreated'); + event = receipt.events?.find((e) => e.event == 'PoolCreated'); } else { receipt = await (await factory.connect(args.from).create(name, symbol, ...args.parameters, ZERO_ADDRESS)).wait(); event = receipt.events?.find((e) => e.event == 'PoolCreated'); diff --git a/pvt/helpers/src/models/pools/weighted/WeightedPoolDeployer.ts b/pvt/helpers/src/models/pools/weighted/WeightedPoolDeployer.ts index d0fecfed9d..df88a6361a 100644 --- a/pvt/helpers/src/models/pools/weighted/WeightedPoolDeployer.ts +++ b/pvt/helpers/src/models/pools/weighted/WeightedPoolDeployer.ts @@ -291,11 +291,6 @@ export default { }, }); - const controlledFactory = await deploy('v2-pool-weighted/ControlledManagedPoolFactory', { - args: [factory.address], - from, - }); - const poolParams = { name: NAME, symbol: SYMBOL, @@ -312,24 +307,9 @@ export default { aumFeeId: aumFeeId ?? ProtocolFee.AUM, }; - const basePoolRights: BasePoolRights = { - canTransferOwnership: true, - canChangeSwapFee: true, - canUpdateMetadata: true, - }; - - const managedPoolRights: ManagedPoolRights = { - canChangeWeights: true, - canDisableSwaps: true, - canSetMustAllowlistLPs: true, - canSetCircuitBreakers: true, - canChangeTokens: true, - canChangeMgmtFees: true, - }; - - const tx = await controlledFactory + const tx = await factory .connect(from || ZERO_ADDRESS) - .create(poolParams, settingsParams, basePoolRights, managedPoolRights, DAY, from?.address || ZERO_ADDRESS); + .create(poolParams, settingsParams, from?.address || ZERO_ADDRESS); const receipt = await tx.wait(); const event = expectEvent.inReceipt(receipt, 'ManagedPoolCreated'); From 17137965ca6dd6887c8b6573aac4f47030402f5f Mon Sep 17 00:00:00 2001 From: Jeffrey Bennett Date: Fri, 2 Dec 2022 13:06:17 -0500 Subject: [PATCH 4/4] lint --- .../src/models/pools/weighted/WeightedPoolDeployer.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/pvt/helpers/src/models/pools/weighted/WeightedPoolDeployer.ts b/pvt/helpers/src/models/pools/weighted/WeightedPoolDeployer.ts index df88a6361a..78b89826f2 100644 --- a/pvt/helpers/src/models/pools/weighted/WeightedPoolDeployer.ts +++ b/pvt/helpers/src/models/pools/weighted/WeightedPoolDeployer.ts @@ -7,16 +7,8 @@ import Vault from '../../vault/Vault'; import WeightedPool from './WeightedPool'; import VaultDeployer from '../../vault/VaultDeployer'; import TypesConverter from '../../types/TypesConverter'; -import { - BasePoolRights, - ManagedPoolParams, - ManagedPoolRights, - RawWeightedPoolDeployment, - WeightedPoolDeployment, - WeightedPoolType, -} from './types'; +import { ManagedPoolParams, RawWeightedPoolDeployment, WeightedPoolDeployment, WeightedPoolType } from './types'; import { ZERO_ADDRESS } from '@balancer-labs/v2-helpers/src/constants'; -import { DAY } from '@balancer-labs/v2-helpers/src/time'; import { ProtocolFee } from '../../vault/types'; const NAME = 'Balancer Pool Token';