Merge pull request #72 from 1inch/refactor/SC-929-Improve-farming-cod…

…e-readability-Distributor [SC-929] Created abstract `Distributor` contract
1inch · Oct 27, 2023 · f84ce4b · f84ce4b
2 parents 47134f2 + 37388cc
commit f84ce4b
Show file tree

Hide file tree

Showing 12 changed files with 73 additions and 90 deletions.
diff --git a/contracts/Distributor.sol b/contracts/Distributor.sol
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
+
+import { IDistributor } from "./interfaces/IDistributor.sol";
+
+abstract contract Distributor is IDistributor, Ownable {
+    address internal _distributor;
+
+    modifier onlyDistributor {
+        if (msg.sender != _distributor) revert NotDistributor();
+        _;
+    }
+
+    /**
+     * @notice See {IDistributor-setDistributor}
+     */
+    function setDistributor(address distributor_) public virtual onlyOwner {
+        if (distributor_ == address(0)) revert ZeroDistributorAddress();
+        emit DistributorChanged(distributor_);
+        _distributor = distributor_;
+    }
+
+    /**
+     * @notice See {IDistributor-distributor}
+     */
+    function distributor() public view virtual returns (address) {
+        return _distributor;
+    }
+}
diff --git a/contracts/FarmingPlugin.sol b/contracts/FarmingPlugin.sol
@@ -10,31 +10,24 @@ import { Plugin } from "@1inch/token-plugins/contracts/Plugin.sol";
 import { IERC20Plugins } from "@1inch/token-plugins/contracts/interfaces/IERC20Plugins.sol";
 
 import { IFarmingPlugin } from "./interfaces/IFarmingPlugin.sol";
+import { Distributor } from "./Distributor.sol";
 import { FarmingLib, FarmAccounting } from "./FarmingLib.sol";
 
-contract FarmingPlugin is Plugin, IFarmingPlugin, Ownable {
+contract FarmingPlugin is Plugin, IFarmingPlugin, Distributor {
     using SafeERC20 for IERC20;
     using FarmingLib for FarmingLib.Info;
     using FarmAccounting for FarmAccounting.Info;
     using Address for address payable;
 
     error ZeroFarmableTokenAddress();
     error ZeroRewardsTokenAddress();
-    error ZeroDistributorAddress();
-    error SameDistributor();
     error InsufficientFunds();
 
     IERC20 public immutable rewardsToken;
 
-    address private _distributor;
     uint256 private _totalSupply;
     FarmingLib.Data private _farm;
 
-    modifier onlyDistributor {
-        if (msg.sender != _distributor) revert AccessDenied();
-        _;
-    }
-
     constructor(IERC20Plugins farmableToken_, IERC20 rewardsToken_)
         Plugin(farmableToken_)
     {
@@ -52,18 +45,6 @@ contract FarmingPlugin is Plugin, IFarmingPlugin, Ownable {
         return _totalSupply;
     }
 
-    function distributor() public view returns(address) {
-        return _distributor;
-    }
-
-    function setDistributor(address distributor_) public virtual onlyOwner {
-        if (distributor_ == address(0)) revert ZeroDistributorAddress();
-        address oldDistributor = _distributor;
-        if (distributor_ == oldDistributor) revert SameDistributor();
-        emit DistributorChanged(oldDistributor, distributor_);
-        _distributor = distributor_;
-    }
-
     function startFarming(uint256 amount, uint256 period) public virtual onlyDistributor {
         uint256 reward = _makeInfo().startFarming(amount, period);
         emit RewardUpdated(reward, period);

diff --git a/contracts/FarmingPool.sol b/contracts/FarmingPool.sol
@@ -9,18 +9,17 @@ import { Address } from "@openzeppelin/contracts/utils/Address.sol";
 import { SafeERC20 } from "@1inch/solidity-utils/contracts/libraries/SafeERC20.sol";
 
 import { IFarmingPool } from "./interfaces/IFarmingPool.sol";
+import { Distributor } from "./Distributor.sol";
 import { FarmAccounting, FarmingLib } from "./FarmingLib.sol";
 
-contract FarmingPool is IFarmingPool, Ownable, ERC20 {
+contract FarmingPool is IFarmingPool, Distributor, ERC20 {
     using SafeERC20 for IERC20;
     using Address for address payable;
     using FarmingLib for FarmingLib.Info;
 
     error SameStakingAndRewardsTokens();
     error ZeroStakingTokenAddress();
     error ZeroRewardsTokenAddress();
-    error ZeroDistributorAddress();
-    error SameDistributor();
     error AccessDenied();
     error InsufficientFunds();
     error MaxBalanceExceeded();
@@ -30,14 +29,8 @@ contract FarmingPool is IFarmingPool, Ownable, ERC20 {
     IERC20 public immutable stakingToken;
     IERC20 public immutable rewardsToken;
 
-    address private _distributor;
     FarmingLib.Data private _farm;
 
-    modifier onlyDistributor {
-        if (msg.sender != _distributor) revert AccessDenied();
-        _;
-    }
-
     constructor(IERC20Metadata stakingToken_, IERC20 rewardsToken_)
         ERC20(
             string(abi.encodePacked("Farming of ", stakingToken_.name())),
@@ -59,18 +52,6 @@ contract FarmingPool is IFarmingPool, Ownable, ERC20 {
         return _farm.farmInfo;
     }
 
-    function distributor() public view virtual returns (address) {
-        return _distributor;
-    }
-
-    function setDistributor(address distributor_) public virtual onlyOwner {
-        if (distributor_ == address(0)) revert ZeroDistributorAddress();
-        address oldDistributor = _distributor;
-        if (distributor_ == oldDistributor) revert SameDistributor();
-        emit DistributorChanged(oldDistributor, distributor_);
-        _distributor = distributor_;
-    }
-
     function startFarming(uint256 amount, uint256 period) public virtual onlyDistributor {
         uint256 reward = _makeInfo().startFarming(amount, period);
         emit RewardUpdated(reward, period);

diff --git a/contracts/MultiFarmingPlugin.sol b/contracts/MultiFarmingPlugin.sol
@@ -11,9 +11,10 @@ import { AddressArray, AddressSet } from "@1inch/solidity-utils/contracts/librar
 import { IERC20Plugins } from "@1inch/token-plugins/contracts/interfaces/IERC20Plugins.sol";
 
 import { IMultiFarmingPlugin } from "./interfaces/IMultiFarmingPlugin.sol";
+import { Distributor } from "./Distributor.sol";
 import { FarmAccounting, FarmingLib } from "./FarmingLib.sol";
 
-contract MultiFarmingPlugin is Plugin, IMultiFarmingPlugin, Ownable {
+contract MultiFarmingPlugin is Plugin, IMultiFarmingPlugin, Distributor {
     using SafeERC20 for IERC20;
     using FarmingLib for FarmingLib.Info;
     using Address for address payable;
@@ -22,8 +23,6 @@ contract MultiFarmingPlugin is Plugin, IMultiFarmingPlugin, Ownable {
 
     error ZeroFarmableTokenAddress();
     error ZeroRewardsTokenAddress();
-    error ZeroDistributorAddress();
-    error SameDistributor();
     error RewardsTokenAlreadyAdded();
     error RewardsTokensLimitTooHigh(uint256);
     error RewardsTokensLimitReached();
@@ -32,16 +31,10 @@ contract MultiFarmingPlugin is Plugin, IMultiFarmingPlugin, Ownable {
 
     uint256 public immutable rewardsTokensLimit;
 
-    address private _distributor;
     uint256 private _totalSupply;
     mapping(IERC20 => FarmingLib.Data) private _farms;
     AddressSet.Data private _rewardsTokens;
 
-    modifier onlyDistributor {
-        if (msg.sender != _distributor) revert AccessDenied();
-        _;
-    }
-
     constructor(IERC20Plugins farmableToken_, uint256 rewardsTokensLimit_) Plugin(farmableToken_) {
         if (rewardsTokensLimit_ > 5) revert RewardsTokensLimitTooHigh(rewardsTokensLimit_);
         if (address(farmableToken_) == address(0)) revert ZeroFarmableTokenAddress();
@@ -61,18 +54,6 @@ contract MultiFarmingPlugin is Plugin, IMultiFarmingPlugin, Ownable {
         return _totalSupply;
     }
 
-    function distributor() public view returns(address) {
-        return _distributor;
-    }
-
-    function setDistributor(address distributor_) public virtual onlyOwner {
-        if (distributor_ == address(0)) revert ZeroDistributorAddress();
-        address oldDistributor = _distributor;
-        if (distributor_ == oldDistributor) revert SameDistributor();
-        emit DistributorChanged(oldDistributor, distributor_);
-        _distributor = distributor_;
-    }
-
     function addRewardsToken(address rewardsToken) public virtual onlyOwner {
         if (rewardsToken == address(0)) revert ZeroRewardsTokenAddress();
         if (_rewardsTokens.length() == rewardsTokensLimit) revert RewardsTokensLimitReached();

diff --git a/contracts/interfaces/IDistributor.sol b/contracts/interfaces/IDistributor.sol
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+interface IDistributor {
+    event DistributorChanged(address newDistributor);
+
+    error NotDistributor();
+    error ZeroDistributorAddress();
+
+    /**
+     * @notice Sets the entity that can manage the farming
+     */
+    function setDistributor(address distributor_) external;
+
+    /**
+     * @notice Returns the entity that can manage the farming
+     */
+    function distributor() external view returns(address);
+}
diff --git a/contracts/interfaces/IFarmingPlugin.sol b/contracts/interfaces/IFarmingPlugin.sol
@@ -8,22 +8,18 @@ import { FarmAccounting } from "../accounting/FarmAccounting.sol";
 
 interface IFarmingPlugin is IPlugin {
     event FarmCreated(address token, address reward);
-    event DistributorChanged(address oldDistributor, address newDistributor);
     event RewardUpdated(uint256 reward, uint256 duration);
 
     // View functions
     function totalSupply() external view returns(uint256);
-    function distributor() external view returns(address);
     function farmInfo() external view returns(FarmAccounting.Info memory);
     function farmed(address account) external view returns(uint256);
 
     // User functions
     function claim() external;
 
-    // Owner functions
-    function setDistributor(address distributor_) external;
-
     // Distributor functions
     function startFarming(uint256 amount, uint256 period) external;
+    function stopFarming() external;
     function rescueFunds(IERC20 token, uint256 amount) external;
 }
diff --git a/contracts/interfaces/IFarmingPool.sol b/contracts/interfaces/IFarmingPool.sol
@@ -6,11 +6,9 @@ import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { FarmAccounting } from "../accounting/FarmAccounting.sol";
 
 interface IFarmingPool is IERC20 {
-    event DistributorChanged(address oldDistributor, address newDistributor);
     event RewardUpdated(uint256 reward, uint256 duration);
 
     // View functions
-    function distributor() external view returns(address);
     function farmInfo() external view returns(FarmAccounting.Info memory);
     function farmed(address account) external view returns(uint256);
 
@@ -20,10 +18,8 @@ interface IFarmingPool is IERC20 {
     function claim() external;
     function exit() external;
 
-    // Owner functions
-    function setDistributor(address distributor_) external;
-
     // Distributor functions
     function startFarming(uint256 amount, uint256 period) external;
+    function stopFarming() external;
     function rescueFunds(IERC20 token, uint256 amount) external;
 }
diff --git a/contracts/interfaces/IMultiFarmingPlugin.sol b/contracts/interfaces/IMultiFarmingPlugin.sol
@@ -8,23 +8,19 @@ import { FarmAccounting } from "../accounting/FarmAccounting.sol";
 
 interface IMultiFarmingPlugin is IPlugin {
     event FarmCreated(address token, address reward);
-    event DistributorChanged(address oldDistributor, address newDistributor);
     event RewardUpdated(address token, uint256 reward, uint256 duration);
 
     // View functions
     function totalSupply() external view returns(uint256);
-    function distributor() external view returns(address);
     function farmInfo(IERC20 rewardsToken) external view returns(FarmAccounting.Info memory);
     function farmed(IERC20 rewardsToken, address account) external view returns(uint256);
 
     // User functions
     function claim(IERC20 rewardsToken) external;
     function claim() external;
 
-    // Owner functions
-    function setDistributor(address distributor_) external;
-
     // Distributor functions
     function startFarming(IERC20 rewardsToken, uint256 amount, uint256 period) external;
+    function stopFarming(IERC20 rewardsToken) external;
     function rescueFunds(IERC20 token, uint256 amount) external;
 }
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
     "name": "@1inch/farming",
-    "version": "3.1.0",
+    "version": "3.2.0",
     "description": "Set of contracts for farming incentives",
     "homepage": "https://github.com/1inch/farming#readme",
     "author": "1inch",

diff --git a/test/FarmingPlugin.js b/test/FarmingPlugin.js
@@ -56,13 +56,13 @@ describe('FarmingPlugin', function () {
                 ***Test Steps**
                 Start farming using `wallet2`
                 ***Expected results**
-                Revert with error `'AccessDenied()'`.
+                Revert with error `'NotDistributor()'`.
             */
             it('should thrown with rewards distribution access denied ', async function () {
                 const { farm } = await loadFixture(initContracts);
                 await expect(
                     farm.connect(wallet2).startFarming(1000, 60 * 60 * 24),
-                ).to.be.revertedWithCustomError(farm, 'AccessDenied');
+                ).to.be.revertedWithCustomError(farm, 'NotDistributor');
             });
 
             /*
@@ -235,15 +235,15 @@ describe('FarmingPlugin', function () {
                 - `wallet2` calls `rescueFunds` function
 
                 ***Expected results**
-                - Call is reverted with an error `'AccessDenied()'`
+                - Call is reverted with an error `'NotDistributor()'`
             */
             it('should thrown with access denied', async function () {
                 const { farm } = await loadFixture(initContracts);
                 const distributor = await farm.distributor();
                 expect(wallet2).to.not.equal(distributor);
                 await expect(
                     farm.connect(wallet2).stopFarming(),
-                ).to.be.revertedWithCustomError(farm, 'AccessDenied');
+                ).to.be.revertedWithCustomError(farm, 'NotDistributor');
             });
 
             /*
@@ -363,15 +363,15 @@ describe('FarmingPlugin', function () {
                 - `wallet2` calls `rescueFunds` function
 
                 ***Expected results**
-                - Call is reverted with an error `'AccessDenied()'`
+                - Call is reverted with an error `'NotDistributor()'`
             */
             it('should thrown with access denied', async function () {
                 const { gift, farm } = await loadFixture(initContracts);
                 const distributor = await farm.distributor();
                 expect(wallet2).to.not.equal(distributor);
                 await expect(
                     farm.connect(wallet2).rescueFunds(gift, '1000'),
-                ).to.be.revertedWithCustomError(farm, 'AccessDenied');
+                ).to.be.revertedWithCustomError(farm, 'NotDistributor');
             });
 
             /*

diff --git a/test/FarmingPool.js b/test/FarmingPool.js
@@ -52,7 +52,7 @@ describe('FarmingPool', function () {
             const { farm } = await loadFixture(initContracts);
             await expect(
                 farm.connect(wallet2).startFarming(1000, 60 * 60 * 24),
-            ).to.be.revertedWithCustomError(farm, 'AccessDenied');
+            ).to.be.revertedWithCustomError(farm, 'NotDistributor');
         });
     });
 
@@ -515,7 +515,7 @@ describe('FarmingPool', function () {
             expect(wallet2.address).to.not.equal(distributor);
             await expect(
                 farm.connect(wallet2).stopFarming(),
-            ).to.be.revertedWithCustomError(farm, 'AccessDenied');
+            ).to.be.revertedWithCustomError(farm, 'NotDistributor');
         });
 
         it('should transfer tokens from farm to wallet', async function () {
@@ -582,7 +582,7 @@ describe('FarmingPool', function () {
             expect(wallet2.address).to.not.equal(distributor);
             await expect(
                 farm.connect(wallet2).rescueFunds(gift, '1000'),
-            ).to.be.revertedWithCustomError(farm, 'AccessDenied');
+            ).to.be.revertedWithCustomError(farm, 'NotDistributor');
         });
 
         it('should thrown with not enough balance for staking token', async function () {

diff --git a/test/MultiFarmingPlugin.js b/test/MultiFarmingPlugin.js
@@ -104,7 +104,7 @@ describe('MultiFarmingPlugin', function () {
             expect(wallet2).to.not.equal(distributor);
             await expect(
                 multiFarm.connect(wallet2).stopFarming(gift),
-            ).to.be.revertedWithCustomError(multiFarm, 'AccessDenied');
+            ).to.be.revertedWithCustomError(multiFarm, 'NotDistributor');
         });
 
         it('should be reverted because of an invalid reward token', async function () {
@@ -168,7 +168,7 @@ describe('MultiFarmingPlugin', function () {
             expect(wallet2).to.not.equal(distributor);
             await expect(
                 multiFarm.connect(wallet2).rescueFunds(gift, '1000'),
-            ).to.be.revertedWithCustomError(multiFarm, 'AccessDenied');
+            ).to.be.revertedWithCustomError(multiFarm, 'NotDistributor');
         });
 
         it('should thrown with insufficient funds', async function () {