feat: add implementation of owner contract (#172)

* feat: add implementation of owner contract

* feat: remove validation on BinPoolManager and push to BinPoolManagerOwner

* feat: clean up and add transferPoolManagerOwnership

* feat: remove collectProtocolFees from poolManagerOwner

* test: add tests for the pool manager owners

.forge-snapshots/BinPoolManagerBytecodeSize.snap

@@ -1 +1 @@
-24507
+24457

src/Owner.sol

@@ -6,6 +6,7 @@ import {Ownable} from "./base/Ownable.sol";
 import {Pausable} from "./base/Pausable.sol";
 
 /// @notice Allow owner to pause in case of emergency
+/// @dev This contract is inherited by ProtocolFees and part of both CL/BinPoolManager
 abstract contract Owner is Ownable, Pausable {
     constructor() Ownable(msg.sender) {}
 

src/ProtocolFees.sol

@@ -88,6 +88,7 @@ abstract contract ProtocolFees is IProtocolFees, Owner {
         override
         returns (uint256 amountCollected)
     {
+        // todo: remove msg.sender access to collectProtocolFees
         if (msg.sender != owner() && msg.sender != address(protocolFeeController)) revert InvalidCaller();
 
         amountCollected = (amount == 0) ? protocolFeesAccrued[currency] : amount;

src/base/PausableRole.sol

@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (C) 2024 PancakeSwap
+pragma solidity ^0.8.0;
+
+import {Ownable, Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol";
+import {Pausable} from "@openzeppelin/contracts/utils/Pausable.sol";
+
+/// @notice Allow owner and multiple accounts to pause but only owner can unpause
+/// @dev Potentially allow security partners to programatically pause()
+abstract contract PausableRole is Ownable2Step {
+    /// @notice Thrown when the caller does not have the pausable role or is not owner
+    error NoPausableRole();
+
+    event PausableRoleGranted(address indexed account);
+    event PausableRoleRevoked(address indexed account);
+
+    mapping(address account => bool hasPausableRole) public hasPausableRole;
+
+    constructor() Ownable(msg.sender) {}
+
+    modifier onlyPausableRoleOrOwner() {
+        if (msg.sender != owner() && !hasPausableRole[msg.sender]) revert NoPausableRole();
+        _;
+    }
+
+    /// @notice Grant the pausable role to an account
+    /// @dev Role will be granted to PCS security monitoring integration, so pause can happen the moment any suspicious activity is detected.
+    function grantPausableRole(address account) public onlyOwner {
+        hasPausableRole[account] = true;
+        emit PausableRoleGranted(account);
+    }
+
+    function revokePausableRole(address account) public onlyOwner {
+        hasPausableRole[account] = false;
+        emit PausableRoleRevoked(account);
+    }
+}

src/interfaces/IPoolManagerOwner.sol

@@ -0,0 +1,23 @@
+//SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+import {Currency} from "../types/Currency.sol";
+import {IProtocolFeeController} from "./IProtocolFeeController.sol";
+
+interface IPoolManagerOwner {
+    /// @notice pause pool manager, only owner or account with pausable role can call. Once
+    /// paused, no swaps, donate or add liquidity are allowed, only remove liquidity is permitted.
+    /// @dev PCS will have security monitoring integration to pause the pool manager in case of any suspicious activity
+    function pausePoolManager() external;
+
+    /// @notice unpause pool manager, only owner can call
+    function unpausePoolManager() external;
+
+    /// @notice set the protocol fee controller, only owner can call
+    function setProtocolFeeController(IProtocolFeeController protocolFeeController) external;
+
+    /// @notice transfer the ownership of pool manager to the new owner
+    /// @dev used when a new PoolManagerOwner contract is created and we transfer pool manager owner to new contract
+    /// @param newOwner the address of the new owner
+    function transferPoolManagerOwnership(address newOwner) external;
+}

src/pool-bin/BinPoolManager.sol

@@ -303,8 +303,6 @@ contract BinPoolManager is IBinPoolManager, ProtocolFees, Extsload {
 
     /// @inheritdoc IBinPoolManager
     function setMinBinSharesForDonate(uint256 minBinShare) external override onlyOwner {
-        if (minBinShare < 1e3) revert MinShareTooSmall();
-
         MIN_BIN_SHARE_FOR_DONATE = minBinShare;
         emit SetMinBinSharesForDonate(minBinShare);
     }

src/pool-bin/BinPoolManagerOwner.sol

@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (C) 2024 PancakeSwap
+pragma solidity 0.8.26;
+
+import {Currency} from "../types/Currency.sol";
+import {IProtocolFeeController} from "../interfaces/IProtocolFeeController.sol";
+import {PausableRole} from "../base/PausableRole.sol";
+import {IBinPoolManager} from "./interfaces/IBinPoolManager.sol";
+import {IPoolManagerOwner} from "../interfaces/IPoolManagerOwner.sol";
+
+/// @dev added interface in this file to avoid polluting other files in repository
+interface IBinPoolManagerWithPauseOwnable is IBinPoolManager {
+    function pause() external;
+    function unpause() external;
+    function transferOwnership(address newOwner) external;
+}
+
+/**
+ * @dev This contract is the owner of the BinPoolManager contract
+ *
+ * A seperate owner contract is used to handle some functionality so as to reduce the contract size
+ * of PoolManager. This allow a higher optimizer run, reducing the gas cost for other poolManager functions
+ */
+contract BinPoolManagerOwner is IPoolManagerOwner, PausableRole {
+    /// @notice Error thrown when owner set min share too small
+    error MinShareTooSmall(uint256 minShare);
+
+    IBinPoolManagerWithPauseOwnable public immutable poolManager;
+
+    constructor(IBinPoolManagerWithPauseOwnable _poolManager) {
+        poolManager = _poolManager;
+    }
+
+    /// @inheritdoc IPoolManagerOwner
+    function pausePoolManager() external override onlyPausableRoleOrOwner {
+        poolManager.pause();
+    }
+
+    /// @inheritdoc IPoolManagerOwner
+    function unpausePoolManager() external override onlyOwner {
+        poolManager.unpause();
+    }
+
+    /// @inheritdoc IPoolManagerOwner
+    function setProtocolFeeController(IProtocolFeeController protocolFeeController) external override onlyOwner {
+        poolManager.setProtocolFeeController(protocolFeeController);
+    }
+
+    /// @inheritdoc IPoolManagerOwner
+    function transferPoolManagerOwnership(address newOwner) external override onlyOwner {
+        poolManager.transferOwnership(newOwner);
+    }
+
+    /// @notice Set max bin steps for binPoolManager, see IBinPoolManager for more documentation about this function
+    function setMaxBinStep(uint16 maxBinStep) external onlyOwner {
+        poolManager.setMaxBinStep(maxBinStep);
+    }
+
+    /// @notice Set max share steps for binPoolManager, see IBinPoolManager for more documentation about this function
+    /// @dev Theres an extra check of minBinShare over here, minBinShare before donate should never be 0, otherwise share inflation attack can easily happen
+    function setMinBinSharesForDonate(uint256 minBinShare) external onlyOwner {
+        if (minBinShare < 1e3) revert MinShareTooSmall(minBinShare);
+
+        poolManager.setMinBinSharesForDonate(minBinShare);
+    }
+}

src/pool-bin/interfaces/IBinPoolManager.sol

@@ -24,9 +24,6 @@ interface IBinPoolManager is IProtocolFees, IPoolManager, IExtsload {
     /// @notice Error thrown when owner set max bin step too small
     error MaxBinStepTooSmall(uint16 maxBinStep);
 
-    /// @notice Error thrown when owner set min share too small
-    error MinShareTooSmall();
-
     /// @notice Error thrown when bin has insufficient shares to accept donation
     error InsufficientBinShareForDonate(uint256 shares);
 
@@ -213,5 +210,6 @@ interface IBinPoolManager is IProtocolFees, IPoolManager, IExtsload {
     /// @notice Set min shares in bin before donate is allowed in current bin
     /// @dev Bin share is 1:1 liquidity when liquidity is first added. And liquidity: price * x + y << 128, where price is a 128.128 number. A
     ///         min share amount required in the bin for donate prevents share inflation attack.
+    /// Min share should always be greater than 0, there should be a validation on BinPoolManagerOwner to prevent setting min share to 0
     function setMinBinSharesForDonate(uint256 minShare) external;
 }

src/pool-cl/CLPoolManagerOwner.sol

@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (C) 2024 PancakeSwap
+pragma solidity 0.8.26;
+
+import {Currency} from "../types/Currency.sol";
+import {IProtocolFeeController} from "../interfaces/IProtocolFeeController.sol";
+import {PausableRole} from "../base/PausableRole.sol";
+import {ICLPoolManager} from "./interfaces/ICLPoolManager.sol";
+import {IPoolManagerOwner} from "../interfaces/IPoolManagerOwner.sol";
+
+/// @dev added interface in this file to avoid polluting other files in repository
+interface ICLPoolManagerWithPauseOwnable is ICLPoolManager {
+    function pause() external;
+    function unpause() external;
+    function transferOwnership(address newOwner) external;
+}
+
+/**
+ * @dev This contract is the owner of the CLPoolManager contract
+ *
+ * A seperate owner contract is used to handle some functionality so as to reduce the contract size
+ * of PoolManager. This allow a higher optimizer run, reducing the gas cost for other poolManager functions.
+ */
+contract CLPoolManagerOwner is IPoolManagerOwner, PausableRole {
+    ICLPoolManagerWithPauseOwnable public immutable poolManager;
+
+    constructor(ICLPoolManagerWithPauseOwnable _poolManager) {
+        poolManager = _poolManager;
+    }
+
+    /// @inheritdoc IPoolManagerOwner
+    function pausePoolManager() external override onlyPausableRoleOrOwner {
+        poolManager.pause();
+    }
+
+    /// @inheritdoc IPoolManagerOwner
+    function unpausePoolManager() external override onlyOwner {
+        poolManager.unpause();
+    }
+
+    /// @inheritdoc IPoolManagerOwner
+    function setProtocolFeeController(IProtocolFeeController protocolFeeController) external override onlyOwner {
+        poolManager.setProtocolFeeController(protocolFeeController);
+    }
+
+    /// @inheritdoc IPoolManagerOwner
+    function transferPoolManagerOwnership(address newOwner) external override onlyOwner {
+        poolManager.transferOwnership(newOwner);
+    }
+}

test/base/PausableRole.t.sol

@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.24;
+
+import "forge-std/Test.sol";
+import {PausableRole} from "../../src/base/PausableRole.sol";
+import {Ownable} from "@openzeppelin/contracts/access/Ownable2Step.sol";
+
+contract PausableRoleImplemetation is PausableRole {}
+
+contract PausableRoleTest is Test {
+    PausableRoleImplemetation public pausableRole;
+    address alice = makeAddr("alice");
+
+    function setUp() public {
+        pausableRole = new PausableRoleImplemetation();
+        assertEq(pausableRole.owner(), address(this));
+    }
+
+    function test_GrantPausableRole_OnlyOwner() public {
+        // before
+        assertFalse(pausableRole.hasPausableRole(alice));
+
+        // grant and verify
+        vm.expectEmit();
+        emit PausableRole.PausableRoleGranted(alice);
+        pausableRole.grantPausableRole(alice);
+
+        assertTrue(pausableRole.hasPausableRole(alice));
+    }
+
+    function test_GrantPausableRole_NotOwner() public {
+        vm.prank(alice);
+
+        vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, alice));
+        pausableRole.grantPausableRole(alice);
+    }
+
+    function test_RevokePausableRole_OnlyOwner() public {
+        // before
+        pausableRole.grantPausableRole(alice);
+        assertTrue(pausableRole.hasPausableRole(alice));
+
+        // revoke and verify
+        vm.expectEmit();
+        emit PausableRole.PausableRoleRevoked(alice);
+        pausableRole.revokePausableRole(alice);
+        assertFalse(pausableRole.hasPausableRole(alice));
+    }
+
+    function test_RevokePausableRole_NotOwner() public {
+        vm.prank(alice);
+
+        vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, alice));
+        pausableRole.revokePausableRole(alice);
+    }
+}

test/pool-bin/BinPoolManager.t.sol

@@ -944,13 +944,6 @@ contract BinPoolManagerTest is Test, GasSnapshot, BinTestHelper {
         assertEq(poolManager.MIN_BIN_SHARE_FOR_DONATE(), minShare);
     }
 
-    function testMinBinSharesForDonate_TooSmall(uint256 minShare) public {
-        minShare = bound(minShare, 0, 1e3 - 1);
-
-        vm.expectRevert(IBinPoolManager.MinShareTooSmall.selector);
-        poolManager.setMinBinSharesForDonate(minShare);
-    }
-
     function testMinBinSharesForDonate_OnlyOwner() public {
         vm.prank(makeAddr("bob"));
         vm.expectRevert();