Merge pull request #104 from NFTX-project/fee-distributor-add-guardians

FeeDistributor: inherit Pausable to add guardians

Author: apoorvlathey

src/NFTXFeeDistributorV3.sol

@@ -2,7 +2,7 @@
 pragma solidity =0.8.15;
 
 // inheriting
-import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
+import {Pausable} from "@src/custom/Pausable.sol";
 import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
 
 // libs
@@ -27,7 +27,7 @@ import {INFTXFeeDistributorV3} from "@src/interfaces/INFTXFeeDistributorV3.sol";
  */
 contract NFTXFeeDistributorV3 is
     INFTXFeeDistributorV3,
-    Ownable,
+    Pausable,
     ReentrancyGuard
 {
     using SafeERC20 for IERC20;
@@ -44,6 +44,9 @@ contract NFTXFeeDistributorV3 is
     uint256 constant POOL_DEFAULT_ALLOC = 0.8 ether; // 80%
     uint256 constant INVENTORY_DEFAULT_ALLOC = 0.2 ether; // 20%
 
+    uint256 constant DISTRIBUTE_LOCK_ID = 0;
+    uint256 constant DISTRIBUTE_VTOKENS_LOCK_ID = 1;
+
     // =============================================================
     //                           VARIABLES
     // =============================================================
@@ -56,8 +59,6 @@ contract NFTXFeeDistributorV3 is
     uint256 public override allocTotal;
     FeeReceiver[] public override feeReceivers;
 
-    bool public override distributionPaused;
-
     // =============================================================
     //                          CONSTRUCTOR
     // =============================================================
@@ -108,11 +109,14 @@ contract NFTXFeeDistributorV3 is
      * @inheritdoc INFTXFeeDistributorV3
      */
     function distribute(uint256 vaultId) external override nonReentrant {
+        onlyOwnerIfPaused(DISTRIBUTE_LOCK_ID);
+
         INFTXVaultV3 vault = INFTXVaultV3(nftxVaultFactory.vault(vaultId));
 
         uint256 wethBalance = WETH.balanceOf(address(this));
 
-        if (distributionPaused || allocTotal == 0) {
+        uint256 _allocTotal = allocTotal;
+        if (_allocTotal == 0) {
             WETH.transfer(treasury, wethBalance);
             return;
         }
@@ -124,7 +128,7 @@ contract NFTXFeeDistributorV3 is
 
             uint256 wethAmountToSend = leftover +
                 (wethBalance * feeReceiver.allocPoint) /
-                allocTotal;
+                _allocTotal;
 
             bool tokenSent = _sendForReceiver(
                 feeReceiver,
@@ -152,6 +156,8 @@ contract NFTXFeeDistributorV3 is
         address vToken,
         uint256 vTokenAmount
     ) external {
+        onlyOwnerIfPaused(DISTRIBUTE_VTOKENS_LOCK_ID);
+
         if (msg.sender != address(nftxRouter)) revert SenderNotNFTXRouter();
 
         uint256 liquidity = IUniswapV3Pool(pool).liquidity();
@@ -229,14 +235,6 @@ contract NFTXFeeDistributorV3 is
         emit NewNFTXRouter(address(nftxRouter_));
     }
 
-    /**
-     * @inheritdoc INFTXFeeDistributorV3
-     */
-    function pauseFeeDistribution(bool pause) external override onlyOwner {
-        distributionPaused = pause;
-        emit PauseDistribution(pause);
-    }
-
     /**
      * @inheritdoc INFTXFeeDistributorV3
      */

src/interfaces/INFTXFeeDistributorV3.sol

@@ -55,8 +55,6 @@ interface INFTXFeeDistributorV3 {
             ReceiverType receiverType
         );
 
-    function distributionPaused() external view returns (bool);
-
     // =============================================================
     //                            EVENTS
     // =============================================================
@@ -115,7 +113,5 @@ interface INFTXFeeDistributorV3 {
 
     function setNFTXRouter(INFTXRouter nftxRouter) external;
 
-    function pauseFeeDistribution(bool pause) external;
-
     function rescueTokens(IERC20 token) external;
 }

test/NFTXFeeDistributorV3.t.sol

@@ -7,17 +7,27 @@ import {TickHelpers} from "@src/lib/TickHelpers.sol";
 import {UniswapV3PoolUpgradeable, IUniswapV3Pool} from "@uni-core/UniswapV3PoolUpgradeable.sol";
 import {INFTXRouter} from "@src/NFTXRouter.sol";
 import {INFTXFeeDistributorV3} from "@src/interfaces/INFTXFeeDistributorV3.sol";
+import {Pausable} from "@src/custom/Pausable.sol";
 
 import {TestBase} from "@test/TestBase.sol";
 
 contract NFTXFeeDistributorV3Tests is TestBase {
+    uint256 constant DISTRIBUTE_LOCK_ID = 0;
+    uint256 constant DISTRIBUTE_VTOKENS_LOCK_ID = 1;
+
     event AddFeeReceiver(address receiver, uint256 allocPoint);
     event UpdateFeeReceiverAlloc(address receiver, uint256 allocPoint);
     event UpdateFeeReceiverAddress(address oldReceiver, address newReceiver);
     event RemoveFeeReceiver(address receiver);
     event UpdateTreasuryAddress(address oldTreasury, address newTreasury);
     event PauseDistribution(bool paused);
 
+    // constructor
+
+    function test_feeDistributor_constructor() external {
+        assertEq(feeDistributor.owner(), address(this));
+    }
+
     // UniswapV3FactoryUpgradeable#setFeeDistributor
 
     function test_setFeeDistributor_RevertsForNonOwner() external {
@@ -52,27 +62,28 @@ contract NFTXFeeDistributorV3Tests is TestBase {
 
     // FeeDistributor#distribute
 
-    function test_feeDistribuion_whenDistributionPaused() external {
+    function test_feeDistribution_whenDistributionPaused() external {
         _mintPosition(1);
 
-        // Pause distribution
-        feeDistributor.pauseFeeDistribution(true);
-        assertEq(feeDistributor.distributionPaused(), true);
+        // add to guardians
+        feeDistributor.setIsGuardian(address(this), true);
 
-        uint256 preTreasuryWethBalance = weth.balanceOf(TREASURY);
+        // Pause distribution
+        feeDistributor.pause(DISTRIBUTE_LOCK_ID);
+        assertEq(feeDistributor.isPaused(DISTRIBUTE_LOCK_ID), true);
 
         uint256 wethFees = 2 ether;
 
         // distribute fees
         weth.deposit{value: wethFees}();
         weth.transfer(address(feeDistributor), wethFees);
-        feeDistributor.distribute(0);
 
-        uint256 postTreasuryWethBalance = weth.balanceOf(TREASURY);
-        assertEq(postTreasuryWethBalance - preTreasuryWethBalance, wethFees);
+        hoax(makeAddr("nonOwner"));
+        vm.expectRevert(Pausable.Paused.selector);
+        feeDistributor.distribute(0);
     }
 
-    function test_feeDistribuion_whenZeroAllocTotal() external {
+    function test_feeDistribution_whenZeroAllocTotal() external {
         _mintPosition(1);
 
         // Remove all receivers
@@ -271,6 +282,36 @@ contract NFTXFeeDistributorV3Tests is TestBase {
         console.log("ETH received", ethReceived);
     }
 
+    // FeeDistributor#distributeVTokensToPool
+
+    function test_distributeVTokensToPool_whenPaused() external {
+        // minting so that Pool is deployed
+        _mintPosition(1);
+
+        // add to guardians
+        feeDistributor.setIsGuardian(address(this), true);
+
+        // Pause distribution
+        feeDistributor.pause(DISTRIBUTE_VTOKENS_LOCK_ID);
+        assertEq(feeDistributor.isPaused(DISTRIBUTE_VTOKENS_LOCK_ID), true);
+
+        (uint256 mintedVTokens, ) = _mintVToken({
+            qty: 1,
+            depositor: address(this),
+            receiver: address(feeDistributor)
+        });
+
+        address pool = nftxRouter.getPool(address(vtoken), DEFAULT_FEE_TIER);
+
+        hoax(makeAddr("nonOwner"));
+        vm.expectRevert(Pausable.Paused.selector);
+        feeDistributor.distributeVTokensToPool({
+            pool: pool,
+            vToken: address(vtoken),
+            vTokenAmount: mintedVTokens
+        });
+    }
+
     // FeeDistributor#setTreasuryAddress
 
     function test_setTreasuryAddress_RevertsForNonOwner() external {
@@ -324,30 +365,6 @@ contract NFTXFeeDistributorV3Tests is TestBase {
         assertEq(postNFTXRouter, newNFTXRouter);
     }
 
-    // FeeDistributor#pauseFeeDistribution
-
-    function test_pauseFeeDistribution_RevertsForNonOwner() external {
-        bool newPause = true;
-
-        hoax(makeAddr("nonOwner"));
-        vm.expectRevert("Ownable: caller is not the owner");
-        feeDistributor.pauseFeeDistribution(newPause);
-    }
-
-    function test_pauseFeeDistribution_Success() external {
-        bool preDistributionPaused = feeDistributor.distributionPaused();
-
-        bool newPause = !preDistributionPaused;
-
-        vm.expectEmit(false, false, false, true);
-        emit PauseDistribution(newPause);
-        feeDistributor.pauseFeeDistribution(newPause);
-
-        bool postDistributionPaused = feeDistributor.distributionPaused();
-
-        assertEq(postDistributionPaused, newPause);
-    }
-
     // FeeDistributor#rescueTokens
 
     function test_rescueTokens_RevertsForNonOwner() external {