Merge pull request #107 from The-Poolz/issue-104

use errors instead of requires

Author: YouStillAlive

contracts/Array.sol

@@ -4,14 +4,17 @@ pragma solidity ^0.8.0;
 
 /// @title contains array utility functions
 library Array {
+    error InvalidArrayLength(uint256 _arrLength, uint256 _n);
+    error ZeroArrayLength();
+    
     /// @dev returns a new slice of the array
     function KeepNElementsInArray(uint256[] memory _arr, uint256 _n)
         internal
         pure
         returns (uint256[] memory newArray)
     {
         if (_arr.length == _n) return _arr;
-        require(_arr.length > _n, "can't cut more then got");
+        if (_arr.length <= _n) revert InvalidArrayLength(_arr.length, _n);
         newArray = new uint256[](_n);
         for (uint256 i = 0; i < _n; ++i) {
             newArray[i] = _arr[i];
@@ -25,7 +28,7 @@ library Array {
         returns (address[] memory newArray)
     {
         if (_arr.length == _n) return _arr;
-        require(_arr.length > _n, "can't cut more then got");
+        if (_arr.length <= _n) revert InvalidArrayLength(_arr.length, _n);
         newArray = new address[](_n);
         for (uint256 i = 0; i < _n; ++i) {
             newArray[i] = _arr[i];
@@ -39,7 +42,7 @@ library Array {
         pure
         returns (bool)
     {
-        require(_arr.length > 0, "array should be greater than zero");
+        if (_arr.length == 0) revert ZeroArrayLength();
         uint256 temp = _arr[0];
         for (uint256 i = 1; i < _arr.length; ++i) {
             if (temp > _arr[i]) {

contracts/ERC20Helper.sol

@@ -8,15 +8,20 @@ import "@ironblocks/firewall-consumer/contracts/FirewallConsumer.sol";
 contract ERC20Helper is FirewallConsumer {
     event TransferOut(uint256 Amount, address To, address Token);
     event TransferIn(uint256 Amount, address From, address Token);
+
+    error NoAllowance();
+    error SentIncorrectAmount();
+    error ReceivedIncorrectAmount();
+    error ZeroAmount();
+
     modifier TestAllowance(
         address _token,
         address _owner,
         uint256 _amount
     ) {
-        require(
-            ERC20(_token).allowance(_owner, address(this)) >= _amount,
-            "ERC20Helper: no allowance"
-        );
+        if (ERC20(_token).allowance(_owner, address(this)) < _amount) {
+            revert NoAllowance();
+        }
         _;
     }
 
@@ -28,33 +33,27 @@ contract ERC20Helper is FirewallConsumer {
         uint256 OldBalance = ERC20(_Token).balanceOf(address(this));
         emit TransferOut(_Amount, _Reciver, _Token);
         ERC20(_Token).transfer(_Reciver, _Amount);
-        require(
-            (ERC20(_Token).balanceOf(address(this)) + _Amount) == OldBalance,
-            "ERC20Helper: sent incorrect amount"
-        );
+        if (ERC20(_Token).balanceOf(address(this)) == (OldBalance + _Amount)) revert SentIncorrectAmount();
     }
 
     function TransferInToken(
         address _Token,
         address _Subject,
         uint256 _Amount
     ) internal TestAllowance(_Token, _Subject, _Amount) {
-        require(_Amount > 0);
+        if (_Amount == 0) revert ZeroAmount();
         uint256 OldBalance = ERC20(_Token).balanceOf(address(this));
         ERC20(_Token).transferFrom(_Subject, address(this), _Amount);
         emit TransferIn(_Amount, _Subject, _Token);
-        require(
-            (OldBalance + _Amount) == ERC20(_Token).balanceOf(address(this)),
-            "ERC20Helper: Received Incorrect Amount"
-        );
+        if (ERC20(_Token).balanceOf(address(this)) != (OldBalance + _Amount)) revert ReceivedIncorrectAmount();
     }
 
     function ApproveAllowanceERC20(
         address _Token,
         address _Subject,
         uint256 _Amount
     ) internal firewallProtectedSig(0x91251680) {
-        require(_Amount > 0);
+        if (_Amount == 0) revert ZeroAmount();
         ERC20(_Token).approve(_Subject, _Amount);
     }
 }

contracts/ERC721Helper.sol

@@ -9,24 +9,23 @@ contract ERC721Helper is FirewallConsumer {
     event TransferOut(address Token, uint256 TokenId, address To);
     event TransferIn(address Token, uint256 TokenId, address From);
 
+    error NoAllowance();
+
     modifier TestNFTAllowance(
         address _token,
         uint256 _tokenId,
         address _owner
     ) {
-        require(
-            IERC721(_token).isApprovedForAll(_owner, address(this)) ||
-                IERC721(_token).getApproved(_tokenId) == address(this),
-            "No Allowance"
-        );
+        if (
+            !IERC721(_token).isApprovedForAll(_owner, address(this)) &&
+            IERC721(_token).getApproved(_tokenId) != address(this)
+        ) {
+            revert NoAllowance();
+        }
         _;
     }
 
-    function TransferNFTOut(
-        address _Token,
-        uint256 _TokenId,
-        address _To
-    ) internal firewallProtectedSig(0x53905fab) {
+    function TransferNFTOut(address _Token, uint256 _TokenId, address _To) internal firewallProtectedSig(0x53905fab) {
         IERC721(_Token).transferFrom(address(this), _To, _TokenId);
         emit TransferOut(_Token, _TokenId, _To);
         assert(IERC721(_Token).ownerOf(_TokenId) == _To);
@@ -42,11 +41,7 @@ contract ERC721Helper is FirewallConsumer {
         assert(IERC721(_Token).ownerOf(_TokenId) == address(this));
     }
 
-    function SetApproveForAllNFT(
-        address _Token,
-        address _To,
-        bool _Approve
-    ) internal firewallProtectedSig(0xd5ebe78c) {
+    function SetApproveForAllNFT(address _Token, address _To, bool _Approve) internal firewallProtectedSig(0xd5ebe78c) {
         IERC721(_Token).setApprovalForAll(_To, _Approve);
     }
 }

contracts/ETHHelper.sol

@@ -5,16 +5,17 @@ import "@openzeppelin/contracts/access/Ownable.sol";
 import "@ironblocks/firewall-consumer/contracts/FirewallConsumer.sol";
 
 contract ETHHelper is Ownable, FirewallConsumer {
-    constructor() Ownable(_msgSender()) {
-        IsPayble = false;
-    }
+    error InvalidAmount();
+    error SentIncorrectAmount();
+
+    constructor() Ownable(_msgSender()) {}
 
     modifier ReceivETH(
         uint256 msgValue,
         address msgSender,
         uint256 _MinETHInvest
     ) {
-        require(msgValue >= _MinETHInvest, "Send ETH to invest");
+        if (msgValue < _MinETHInvest) revert InvalidAmount();
         emit TransferInETH(msgValue, msgSender);
         _;
     }
@@ -37,6 +38,6 @@ contract ETHHelper is Ownable, FirewallConsumer {
         emit TransferOutETH(_amount, _Reciver);
         uint256 beforeBalance = address(_Reciver).balance;
         _Reciver.transfer(_amount);
-        require((beforeBalance + _amount) == address(_Reciver).balance, "The transfer did not complite");
+        if ((beforeBalance + _amount) != address(_Reciver).balance) revert SentIncorrectAmount();
     }
 }

contracts/GovManager.sol

@@ -6,18 +6,16 @@ import "@openzeppelin/contracts/access/Ownable.sol";
 import "@ironblocks/firewall-consumer/contracts/FirewallConsumer.sol";
 
 contract GovManager is Ownable, FirewallConsumer {
-    event GovernorUpdated (
-        address indexed oldGovernor,
-        address indexed newGovernor
-    );
+    event GovernorUpdated(address indexed oldGovernor, address indexed newGovernor);
+
+    error AuthorizationError();
 
     address public GovernorContract;
 
     modifier onlyOwnerOrGov() {
-        require(
-            msg.sender == owner() || msg.sender == GovernorContract,
-            "Authorization Error"
-        );
+        if (msg.sender != owner() && msg.sender != GovernorContract) {
+            revert AuthorizationError();
+        }
         _;
     }
 

contracts/mocks/ETHHelperMock.sol

@@ -4,9 +4,9 @@ pragma solidity ^0.8.0;
 import "../ETHHelper.sol";
 
 contract ETHHelperMock is ETHHelper {
-    function receiveETH(
-        uint256 minETHInvest
-    ) external payable ReceivETH(msg.value, msg.sender, minETHInvest) {}
+    constructor() ETHHelper() {}
+
+    function receiveETH(uint256 minETHInvest) external payable ReceivETH(msg.value, msg.sender, minETHInvest) {}
 
     function transferETH(address payable reciver, uint256 amount) external {
         TransferETH(reciver, amount);

test/1_Admin.ts

@@ -61,6 +61,6 @@ describe('Admin Tests', function () {
   it('should fail to set POZ timer when called without invalid address', async () => {
     await pozInstance.setGovernorContract(ZERO_ADDRESS);
     const timer = 1000;
-    await expect(pozInstance.connect(addr).SetPozTimer(timer)).to.be.revertedWith('Authorization Error');
+    await expect(pozInstance.connect(addr).SetPozTimer(timer)).to.be.revertedWithCustomError(pozInstance, "AuthorizationError");
   });
 });

test/4_Pausable.ts

@@ -12,8 +12,8 @@ describe('PausableHelper', function () {
 
   it('should revert invalid user access', async () => {
     const [, addr1] = await ethers.getSigners();
-    await expect(pausableHelper.connect(addr1).pause()).to.be.revertedWith('Authorization Error');
-    await expect(pausableHelper.connect(addr1).unpause()).to.be.revertedWith('Authorization Error');
+    await expect(pausableHelper.connect(addr1).pause()).to.be.revertedWithCustomError(pausableHelper, "AuthorizationError")
+    await expect(pausableHelper.connect(addr1).unpause()).to.be.revertedWithCustomError(pausableHelper, "AuthorizationError");
   });
 
   it('check pause event', async () => {

test/5_Array.ts

@@ -23,7 +23,7 @@ describe('Array library', function () {
     const arr = [1, 2, 46, 6, 9];
     const newArr = await arrayLibrary.callStatic.keepNElementsInArray(arr, 2);
     expect(newArr.toString()).to.equal(arr.slice(0, 2).toString());
-    await expect(arrayLibrary.keepNElementsInArray(arr, 20)).to.be.revertedWith("can't cut more then got");
+    await expect(arrayLibrary.keepNElementsInArray(arr, 20)).to.be.revertedWithCustomError(arrayLibrary, "InvalidArrayLength")
     // return the same array if the length is the same
     const sameArr = await arrayLibrary.callStatic.keepNElementsInArray(arr, arr.length);
     expect(sameArr.toString()).to.equal(arr.toString());
@@ -34,14 +34,14 @@ describe('Array library', function () {
     const arr = [addr0.address, addr1.address, addr2.address, addr4.address];
     const newArr = await arrayLibrary.callStatic.KeepNElementsInArray(arr, 2);
     expect(newArr.toString()).to.equal(arr.slice(0, 2).toString());
-    await expect(arrayLibrary.KeepNElementsInArray(arr, 20)).to.be.revertedWith("can't cut more then got");
+    await expect(arrayLibrary.KeepNElementsInArray(arr, 20)).to.be.revertedWithCustomError(arrayLibrary, "InvalidArrayLength");
     // return the same array if the length is the same
     const sameArr = await arrayLibrary.callStatic.KeepNElementsInArray(arr, arr.length);
     expect(sameArr.toString()).to.equal(arr.toString());
   });
 
   it('is Array Ordered', async () => {
-    await expect(arrayLibrary.isArrayOrdered([])).to.be.revertedWith('array should be greater than zero');
+    await expect(arrayLibrary.isArrayOrdered([])).to.be.revertedWithCustomError(arrayLibrary, "ZeroArrayLength");
     const orderedArr = [1, 2, 3, 4, 5];
     const nonOrderedArr = [5, 8, 3, 19];
     let status = await arrayLibrary.callStatic.isArrayOrdered(orderedArr);

test/6_ERC721Helper.ts

@@ -57,8 +57,9 @@ describe('ERC721 Helper tests', function () {
   });
 
   it('should return no allowance message', async () => {
-    await expect(erc721Helper.transferNFTIn(erc721Token.address, itemId, user.address)).to.be.revertedWith(
-      'No Allowance',
+    await expect(erc721Helper.transferNFTIn(erc721Token.address, itemId, user.address)).to.be.revertedWithCustomError(
+      erc721Helper,
+      'NoAllowance',
     );
   });
 });

test/7_ETHHelper.ts

@@ -28,8 +28,9 @@ describe('ETH Helper tests', function () {
   });
 
   it('should revert invalid amount eth', async () => {
-    await expect(ethHelper.receiveETH(minETHInvest, { value: minETHInvest / 2 })).to.be.revertedWith(
-      'Send ETH to invest',
+    await expect(ethHelper.receiveETH(minETHInvest, { value: minETHInvest / 2 })).to.be.revertedWithCustomError(
+      ethHelper,
+      'InvalidAmount',
     );
   });
 

test/8_ERC20Helper.ts

@@ -44,8 +44,9 @@ describe('ERC20 Helper tests', function () {
   });
 
   it('should revert invalid approved amount', async () => {
-    await expect(erc20Helper.transferInToken(token.address, owner.address, MAX_UINT256)).to.be.revertedWith(
-      'ERC20Helper: no allowance',
+    await expect(erc20Helper.transferInToken(token.address, owner.address, MAX_UINT256)).to.be.revertedWithCustomError(
+      erc20Helper,
+      'NoAllowance',
     );
   });