token: erc20 transfers (#195)

* refactor: context notation
* token: remove the non-standard callback and make it behave like standard ERC20
* token: add minting tests

Author: abarmat

.soliumignore

@@ -1,7 +1,6 @@
 node_modules
 contracts/Staking.sol
 contracts/Migrations.sol
-contracts/DisputeManager.sol
 contracts/bancor
 contracts/openzeppelin
 contracts/MultiSigWallet.sol

contracts/Curation.sol

@@ -154,33 +154,37 @@ contract Curation is Governed, BancorFormula {
     }
 
     /**
-     * @dev Accept tokens
-     * @notice Receive Graph tokens
-     * @param _from Token holder's address
-     * @param _value Amount of Graph Tokens
-     * @param _data Extra data payload
-     * @return true if token transfer is processed
+     * @dev Assign Graph Tokens received from staking to the subgraph reserve
+     * @param _subgraphID Subgraph where funds should be allocated as reserves
+     * @param _tokens Amount of Graph Tokens to add to reserves
      */
-    function tokensReceived(
-        address _from,
-        uint256 _value,
-        bytes calldata _data
-    ) external returns (bool) {
-        // Make sure the token is the caller of this function
-        require(msg.sender == address(token), "Caller is not the GRT token contract");
-
-        // Decode subgraphID
-        bytes32 subgraphID = _data.slice(0, 32).toBytes32(0);
-
-        // Transfers from staking means we are assigning fees to reserves
-        if (_from == staking) {
-            _collect(subgraphID, _value);
-            return true;
-        }
+    function collect(bytes32 _subgraphID, uint256 _tokens) external {
+        require(msg.sender == staking, "Caller must be the staking contract");
+
+        // Transfer tokens to collect from staking to this contract
+        require(
+            token.transferFrom(staking, address(this), _tokens),
+            "Cannot transfer tokens to collect"
+        );
+        // Collect tranferred tokens and assign to subgraph reserves
+        _collect(_subgraphID, _tokens);
+    }
+
+    /**
+     * @dev Called by a curator to deposit Graph Tokens in exchange for shares of a subgraph
+     * @param _subgraphID Subgraph ID where the curator is staking Graph Tokens
+     * @param _tokens Amount of Graph Tokens to stake
+     */
+    function stake(bytes32 _subgraphID, uint256 _tokens) external {
+        address curator = msg.sender;
 
-        // Any other source address means they are staking tokens for shares
-        _stake(_from, subgraphID, _value);
-        return true;
+        // Transfer tokens from the curator to this contract
+        require(
+            token.transferFrom(curator, address(this), _tokens),
+            "Cannot transfer tokens to stake"
+        );
+        // Stake transferred tokens to subgraph
+        _stake(curator, _subgraphID, _tokens);
     }
 
     /**
@@ -351,8 +355,8 @@ contract Curation is Governed, BancorFormula {
 
     /**
      * @dev Deposit Graph Tokens in exchange for shares of a subgraph
-     * @param _subgraphID Subgraph ID where the curator is staking Graph Tokens
      * @param _curator Address of staking party
+     * @param _subgraphID Subgraph ID where the curator is staking Graph Tokens
      * @param _tokens Amount of Graph Tokens to stake
      */
     function _stake(

contracts/DisputeManager.sol

@@ -280,23 +280,23 @@ contract DisputeManager is Governed {
     }
 
     /**
-     * @dev Accept tokens
-     * @notice Receive Graph tokens
-     * @param _from Token sender address
-     * @param _value Amount of Graph Tokens
-     * @param _data Extra data payload
+     * @dev Create a dispute for the arbitrator to resolve.
+     * This function is called by a fisherman and will need to `_deposit` at
+     * least `minimumDeposit` GRT tokens.
+     * @param _attestationData Attestation bytes submitted by the fisherman
+     * @param _deposit Amount of tokens staked as deposit
      */
-    function tokensReceived(address _from, uint256 _value, bytes calldata _data)
-        external
-        returns (bool)
+    function createDispute(bytes calldata _attestationData, uint256 _deposit) external
     {
-        // Make sure the token is the caller of this function
-        require(msg.sender == address(token), "Caller is not the GRT token contract");
-
-        // Create a dispute using the received attestation
-        _createDispute(_data, _from, _value);
+        address fisherman = msg.sender;
 
-        return true;
+        // Transfer tokens to deposit from fisherman to this contract
+        require(
+            token.transferFrom(fisherman, address(this), _deposit),
+            "Cannot transfer tokens to deposit"
+        );
+        // Create a dispute using the received attestation and deposit
+        _createDispute(fisherman, _deposit, _attestationData);
     }
 
     /**
@@ -392,7 +392,7 @@ contract DisputeManager is Governed {
      * @param _fisherman Creator of dispute
      * @param _deposit Amount of tokens staked as deposit
      */
-    function _createDispute(bytes memory _attestationData, address _fisherman, uint256 _deposit) private {
+    function _createDispute(address _fisherman, uint256 _deposit, bytes memory _attestationData) private {
         // Check attestation data length
         require(_attestationData.length == ATTESTATION_SIZE_BYTES, "Attestation must be 161 bytes long");
 

contracts/GraphToken.sol

@@ -1,94 +1,100 @@
 pragma solidity ^0.6.4;
 pragma experimental ABIEncoderV2;
 
-/*
- * @title GraphToken contract
- *
- */
-
 import "./Governed.sol";
+import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/ERC20Burnable.sol";
-import "@openzeppelin/contracts/token/ERC20/ERC20Detailed.sol";
 
 
-// NOTE: This is based off of ERC777TokensRecipient interface, but does not fully implement it
-interface TokenReceiver {
-    function tokensReceived(address _from, uint256 _amount, bytes calldata _data)
-        external
-        returns (bool);
-}
-
+/**
+ * @title GraphToken contract
+ * @dev This is the implementation of the ERC20 Graph Token.
+ */
+contract GraphToken is Governed, ERC20, ERC20Burnable {
+    // -- State --
 
-contract GraphToken is Governed, ERC20Detailed, ERC20Burnable {
     mapping(address => bool) private _minters;
 
     // -- Events --
+
     event MinterAdded(address indexed account);
     event MinterRemoved(address indexed account);
 
     modifier onlyMinter() {
-        require(isMinter(msg.sender) || msg.sender == governor, "Only minter can call");
+        require(isMinter(msg.sender), "Only minter can call");
         _;
     }
 
-    /*
-     * @dev Init Graph Token contract
-     * @param _governor <address> Address of the multisig contract as Governor of this contract
-     * @param _initialSupply <uint256> Initial supply of Graph Tokens
+    /**
+     * @dev Graph Token Contract Constructor
+     * @param _governor Owner address of this contract
+     * @param _initialSupply Initial supply of GRT
      */
     constructor(address _governor, uint256 _initialSupply)
         public
-        ERC20Detailed("Graph Token", "GRT", 18)
+        ERC20("Graph Token", "GRT")
         Governed(_governor)
     {
-        // Governor is initially the sole treasurer
-        _addMinter(_governor);
-
         // The Governor has the initial supply of tokens
         _mint(_governor, _initialSupply);
+        // The Governor is the default minter
+        _addMinter(_governor);
     }
 
+    /**
+     * @dev Add a new minter
+     * @param _account Address of the minter
+     */
     function addMinter(address _account) external onlyGovernor {
         _addMinter(_account);
     }
 
+    /**
+     * @dev Remove a minter
+     * @param _account Address of the minter
+     */
     function removeMinter(address _account) external onlyGovernor {
         _removeMinter(_account);
     }
 
-    function isMinter(address _account) public view returns (bool) {
-        return _minters[_account];
-    }
-
+    /**
+     * @dev Renounce to be a minter
+     */
     function renounceMinter() external {
         _removeMinter(msg.sender);
     }
 
-    function mint(address _account, uint256 _amount) external onlyMinter returns (bool) {
-        _mint(_account, _amount);
-        return true;
+    /**
+     * @dev Mint new tokens
+     * @param _to Address to send the newly minted tokens
+     * @param _amount Amount of tokens to mint
+     */
+    function mint(address _to, uint256 _amount) external onlyMinter {
+        _mint(_to, _amount);
     }
 
-    /*
-     * @dev Transfer Graph tokens to the Staking interface
-     * @notice Interacts with Staking contract
-     * @notice Overriding `transfer` was not working with web3.js so we renamed to `transferToTokenReceiver`
+    /**
+     * @dev Return if the `_account` is a minter or not
+     * @param _account Address to check
+     * @return True if the `_account` is minter
      */
-    function transferToTokenReceiver(address _to, uint256 _amount, bytes memory _data)
-        public
-        returns (bool success)
-    {
-        assert(super.transfer(_to, _amount)); // Handle basic transfer functionality
-        // @imp 08 Have staking contract receive the token and handle the data
-        assert(TokenReceiver(_to).tokensReceived(msg.sender, _amount, _data));
-        success = true;
+    function isMinter(address _account) public view returns (bool) {
+        return _minters[_account];
     }
 
+    /**
+     * @dev Add a new minter
+     * @param _account Address of the minter
+     */
     function _addMinter(address _account) internal {
         _minters[_account] = true;
         emit MinterAdded(_account);
     }
 
+    /**
+     * @dev Remove a minter
+     * @param _account Address of the minter
+     */
     function _removeMinter(address _account) internal {
         _minters[_account] = false;
         emit MinterRemoved(_account);