Consolidates duplicate functions within a common library for token transfers (#464)

Use a TokenUtils for sending, pulling and burning tokens

Author: abarmat

contracts/disputes/DisputeManager.sol

@@ -8,6 +8,7 @@ import "@openzeppelin/contracts/cryptography/ECDSA.sol";
 
 import "../governance/Managed.sol";
 import "../upgrades/GraphUpgradeable.sol";
+import "../utils/TokenUtils.sol";
 
 import "./DisputeManagerStorage.sol";
 import "./IDisputeManager.sol";
@@ -562,7 +563,7 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa
             _slashIndexer(dispute.indexer, dispute.fisherman, dispute.disputeType);
 
         // Give the fisherman their deposit back
-        _pushTokens(dispute.fisherman, dispute.deposit);
+        TokenUtils.pushTokens(graphToken(), dispute.fisherman, dispute.deposit);
 
         // Resolve the conflicting dispute if any
         _resolveDisputeInConflict(dispute);
@@ -590,9 +591,7 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa
         );
 
         // Burn the fisherman's deposit
-        if (dispute.deposit > 0) {
-            graphToken().burn(dispute.deposit);
-        }
+        TokenUtils.burnTokens(graphToken(), dispute.deposit);
 
         emit DisputeRejected(_disputeID, dispute.indexer, dispute.fisherman, dispute.deposit);
     }
@@ -606,7 +605,7 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa
         Dispute memory dispute = _resolveDispute(_disputeID);
 
         // Return deposit to the fisherman
-        _pushTokens(dispute.fisherman, dispute.deposit);
+        TokenUtils.pushTokens(graphToken(), dispute.fisherman, dispute.deposit);
 
         // Resolve the conflicting dispute if any
         _resolveDisputeInConflict(dispute);
@@ -662,33 +661,7 @@ contract DisputeManager is DisputeManagerV1Storage, GraphUpgradeable, IDisputeMa
         require(_deposit >= minimumDeposit, "Dispute deposit is under minimum required");
 
         // Transfer tokens to deposit from fisherman to this contract
-        _pullTokens(msg.sender, _deposit);
-    }
-
-    /**
-     * @dev Pull tokens from an address to this contract.
-     * @param _from Address sending the tokens
-     * @param _amount Amount of tokens to transfer
-     */
-    function _pullTokens(address _from, uint256 _amount) private {
-        if (_amount > 0) {
-            // Transfer tokens to deposit from fisherman to this contract
-            require(
-                graphToken().transferFrom(_from, address(this), _amount),
-                "Cannot transfer tokens"
-            );
-        }
-    }
-
-    /**
-     * @dev Push tokens from this contract to a receiving address.
-     * @param _to Address receiving the tokens
-     * @param _amount Amount of tokens to transfer
-     */
-    function _pushTokens(address _to, uint256 _amount) private {
-        if (_amount > 0) {
-            require(graphToken().transfer(_to, _amount), "Cannot transfer tokens");
-        }
+        TokenUtils.pullTokens(graphToken(), msg.sender, _deposit);
     }
 
     /**

contracts/staking/Staking.sol

@@ -6,6 +6,7 @@ pragma experimental ABIEncoderV2;
 import "@openzeppelin/contracts/cryptography/ECDSA.sol";
 
 import "../upgrades/GraphUpgradeable.sol";
+import "../utils/TokenUtils.sol";
 
 import "./IStaking.sol";
 import "./StakingStorage.sol";
@@ -695,7 +696,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
         );
 
         // Transfer tokens to stake from caller to this contract
-        _pullTokens(graphToken(), msg.sender, _tokens);
+        TokenUtils.pullTokens(graphToken(), msg.sender, _tokens);
 
         // Stake the transferred tokens
         _stake(_indexer, _tokens);
@@ -789,10 +790,10 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
         IGraphToken graphToken = graphToken();
 
         // Set apart the reward for the beneficiary and burn remaining slashed stake
-        _burnTokens(graphToken, _tokens.sub(_reward));
+        TokenUtils.burnTokens(graphToken, _tokens.sub(_reward));
 
         // Give the beneficiary a reward for slashing
-        _pushTokens(graphToken, _beneficiary, _reward);
+        TokenUtils.pushTokens(graphToken, _beneficiary, _reward);
 
         emit StakeSlashed(_indexer, _tokens, _reward, _beneficiary);
     }
@@ -812,7 +813,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
         address delegator = msg.sender;
 
         // Transfer tokens to delegate to this contract
-        _pullTokens(graphToken(), delegator, _tokens);
+        TokenUtils.pullTokens(graphToken(), delegator, _tokens);
 
         // Update state
         return _delegate(delegator, _indexer, _tokens);
@@ -971,7 +972,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
         if (queryFees > 0) {
             // Pull tokens to collect from the authorized sender
             IGraphToken graphToken = graphToken();
-            _pullTokens(graphToken, msg.sender, _tokens);
+            TokenUtils.pullTokens(graphToken, msg.sender, _tokens);
 
             // -- Collect protocol tax --
             // If the Allocation is not active or closed we are going to charge a 100% protocol tax
@@ -1070,7 +1071,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
         require(tokensToWithdraw > 0, "!tokens");
 
         // Return tokens to the indexer
-        _pushTokens(graphToken(), _indexer, tokensToWithdraw);
+        TokenUtils.pushTokens(graphToken(), _indexer, tokensToWithdraw);
 
         emit StakeWithdrawn(_indexer, tokensToWithdraw);
     }
@@ -1262,7 +1263,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
 
         // When all allocations processed then burn unclaimed fees and prune rebate pool
         if (rebatePool.unclaimedAllocationsCount == 0) {
-            _burnTokens(graphToken, rebatePool.unclaimedFees());
+            TokenUtils.burnTokens(graphToken, rebatePool.unclaimedFees());
             delete rebates[alloc.closedAtEpoch];
         }
 
@@ -1408,7 +1409,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
             _delegate(_delegator, _delegateToIndexer, tokensToWithdraw);
         } else {
             // Return tokens to the delegator
-            _pushTokens(graphToken(), _delegator, tokensToWithdraw);
+            TokenUtils.pushTokens(graphToken(), _delegator, tokensToWithdraw);
         }
 
         return tokensToWithdraw;
@@ -1484,7 +1485,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
                 // Transfer and call collect()
                 // This function transfer tokens to a trusted protocol contracts
                 // Then we call collect() to do the transfer bookeeping
-                _pushTokens(_graphToken, address(curation), curationFees);
+                TokenUtils.pushTokens(_graphToken, address(curation), curationFees);
                 curation.collect(_subgraphDeploymentID, curationFees);
             }
             return curationFees;
@@ -1505,7 +1506,7 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
         uint256 _percentage
     ) private returns (uint256) {
         uint256 tax = uint256(_percentage).mul(_tokens).div(MAX_PPM);
-        _burnTokens(_graphToken, tax); // Burn tax if any
+        TokenUtils.burnTokens(_graphToken, tax); // Burn tax if any
         return tax;
     }
 
@@ -1616,54 +1617,11 @@ contract Staking is StakingV2Storage, GraphUpgradeable, IStaking {
         } else {
             // Transfer funds to the beneficiary's designated rewards destination if set
             address destination = rewardsDestination[_beneficiary];
-            _pushTokens(
+            TokenUtils.pushTokens(
                 _graphToken,
                 destination == address(0) ? _beneficiary : destination,
                 _amount
             );
         }
     }
-
-    /**
-     * @dev Burn tokens held by this contract.
-     * @param _graphToken Token to burn
-     * @param _amount Amount of tokens to burn
-     */
-    function _burnTokens(IGraphToken _graphToken, uint256 _amount) private {
-        if (_amount > 0) {
-            _graphToken.burn(_amount);
-        }
-    }
-
-    /**
-     * @dev Pull tokens from an address to this contract.
-     * @param _graphToken Token to transfer
-     * @param _from Address sending the tokens
-     * @param _amount Amount of tokens to transfer
-     */
-    function _pullTokens(
-        IGraphToken _graphToken,
-        address _from,
-        uint256 _amount
-    ) private {
-        if (_amount > 0) {
-            require(_graphToken.transferFrom(_from, address(this), _amount), "!transfer");
-        }
-    }
-
-    /**
-     * @dev Push tokens from this contract to a receiving address.
-     * @param _graphToken Token to transfer
-     * @param _to Address receiving the tokens
-     * @param _amount Amount of tokens to transfer
-     */
-    function _pushTokens(
-        IGraphToken _graphToken,
-        address _to,
-        uint256 _amount
-    ) private {
-        if (_amount > 0) {
-            require(_graphToken.transfer(_to, _amount), "!transfer");
-        }
-    }
 }

contracts/utils/TokenUtils.sol

@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.7.3;
+
+import "../token/IGraphToken.sol";
+
+library TokenUtils {
+    /**
+     * @dev Pull tokens from an address to this contract.
+     * @param _graphToken Token to transfer
+     * @param _from Address sending the tokens
+     * @param _amount Amount of tokens to transfer
+     */
+    function pullTokens(
+        IGraphToken _graphToken,
+        address _from,
+        uint256 _amount
+    ) internal {
+        if (_amount > 0) {
+            require(_graphToken.transferFrom(_from, address(this), _amount), "!transfer");
+        }
+    }
+
+    /**
+     * @dev Push tokens from this contract to a receiving address.
+     * @param _graphToken Token to transfer
+     * @param _to Address receiving the tokens
+     * @param _amount Amount of tokens to transfer
+     */
+    function pushTokens(
+        IGraphToken _graphToken,
+        address _to,
+        uint256 _amount
+    ) internal {
+        if (_amount > 0) {
+            require(_graphToken.transfer(_to, _amount), "!transfer");
+        }
+    }
+
+    /**
+     * @dev Burn tokens held by this contract.
+     * @param _graphToken Token to burn
+     * @param _amount Amount of tokens to burn
+     */
+    function burnTokens(IGraphToken _graphToken, uint256 _amount) internal {
+        if (_amount > 0) {
+            _graphToken.burn(_amount);
+        }
+    }
+}

package.json

@@ -30,24 +30,25 @@
     "@types/inquirer": "^7.3.1",
     "@types/minimist": "^1.2.1",
     "@types/mocha": "^8.2.2",
-    "@types/node": "^14.14.41",
-    "@types/yargs": "^15.0.13",
-    "@typescript-eslint/eslint-plugin": "^3.10.1",
-    "@typescript-eslint/parser": "^3.10.1",
+    "@types/node": "^15.0.1",
+    "@types/yargs": "^16.0.0",
+    "@typescript-eslint/eslint-plugin": "^4.0.0",
+    "@typescript-eslint/parser": "^4.0.0",
+    "@typescript-eslint/typescript-estree": "^4.0.0",
     "axios": "^0.21.1",
     "bignumber.js": "^9.0.0",
     "chai": "^4.3.4",
     "cli-table": "^0.3.6",
     "consola": "^2.15.3",
     "dotenv": "^8.2.0",
     "eslint": "^7.24.0",
-    "eslint-config-prettier": "^6.11.0",
-    "eslint-config-standard": "^14.1.1",
+    "eslint-config-prettier": "^8.3.0",
+    "eslint-config-standard": "^16.0.0",
     "eslint-plugin-import": "^2.22.0",
     "eslint-plugin-mocha-no-only": "^1.1.1",
     "eslint-plugin-node": "^11.1.0",
     "eslint-plugin-prettier": "^3.4.0",
-    "eslint-plugin-promise": "^4.3.1",
+    "eslint-plugin-promise": "^5.0.0",
     "eslint-plugin-standard": "^4.1.0",
     "ethereum-waffle": "^3.3.0",
     "ethlint": "^1.2.5",

test/disputes/query.test.ts

@@ -26,7 +26,7 @@ const { AddressZero, HashZero } = constants
 
 const NON_EXISTING_DISPUTE_ID = randomHexBytes()
 
-describe.only('DisputeManager:Query', async () => {
+describe('DisputeManager:Query', async () => {
   let me: Account
   let other: Account
   let governor: Account

test/lib/deployment.ts

@@ -195,9 +195,10 @@ export async function deployGNS(
 }
 
 export async function deployEthereumDIDRegistry(deployer: Signer): Promise<EthereumDIDRegistry> {
-  return (deployContract('EthereumDIDRegistry', deployer) as unknown) as Promise<
-    EthereumDIDRegistry
-  >
+  return (deployContract(
+    'EthereumDIDRegistry',
+    deployer,
+  ) as unknown) as Promise<EthereumDIDRegistry>
 }
 
 export async function deployServiceRegistry(