Use a minimal proxy for the curation shares ERC20 (#505)

abarmat · web-flow · commit 2e84093ed320 · 2021-11-28T21:33:10.000-03:00
* curation: use a minimal proxy to save gas when minting for first time
* curation: use token utils for transfers
* tests: remove unnecessary block advance in test
* curation: add external function to set token master copy and avoid re-deploy the clone on minting reset
* chore: add graph curation master copy token to deployment config
diff --git a/cli/commands/migrate.ts b/cli/commands/migrate.ts
@@ -22,6 +22,7 @@ let allContracts = [
   'Controller',
   'EpochManager',
   'GraphToken',
+  'GraphCurationToken',
   'ServiceRegistry',
   'Curation',
   'GNS',
diff --git a/contracts/curation/Curation.sol b/contracts/curation/Curation.sol
@@ -2,10 +2,13 @@
 
 pragma solidity ^0.7.6;
 
+import "@openzeppelin/contracts/utils/Address.sol";
 import "@openzeppelin/contracts/math/SafeMath.sol";
+import "@openzeppelin/contracts/proxy/Clones.sol";
 
 import "../bancor/BancorFormula.sol";
 import "../upgrades/GraphUpgradeable.sol";
+import "../utils/TokenUtils.sol";
 
 import "./CurationStorage.sol";
 import "./ICuration.sol";
@@ -23,7 +26,7 @@ import "./GraphCurationToken.sol";
  * Holders can burn GCS using this contract to get GRT tokens back according to the
  * bonding curve.
  */
-contract Curation is CurationV1Storage, GraphUpgradeable, ICuration {
+contract Curation is CurationV1Storage, GraphUpgradeable {
     using SafeMath for uint256;
 
     // 100% in parts per million
@@ -70,6 +73,7 @@ contract Curation is CurationV1Storage, GraphUpgradeable, ICuration {
     function initialize(
         address _controller,
         address _bondingCurve,
+        address _curationTokenMaster,
         uint32 _defaultReserveRatio,
         uint32 _curationTaxPercentage,
         uint256 _minimumCurationDeposit
@@ -83,6 +87,7 @@ contract Curation is CurationV1Storage, GraphUpgradeable, ICuration {
         _setDefaultReserveRatio(_defaultReserveRatio);
         _setCurationTaxPercentage(_curationTaxPercentage);
         _setMinimumCurationDeposit(_minimumCurationDeposit);
+        _setCurationTokenMaster(_curationTokenMaster);
     }
 
     /**
@@ -154,10 +159,30 @@ contract Curation is CurationV1Storage, GraphUpgradeable, ICuration {
             "Curation tax percentage must be below or equal to MAX_PPM"
         );
 
-        _curationTaxPercentage = _percentage;
+        curationTaxPercentage = _percentage;
         emit ParameterUpdated("curationTaxPercentage");
     }
 
+    /**
+     * @dev Set the master copy to use as clones for the curation token.
+     * @param _curationTokenMaster Address of implementation contract to use for curation tokens
+     */
+    function setCurationTokenMaster(address _curationTokenMaster) external override onlyGovernor {
+        _setCurationTokenMaster(_curationTokenMaster);
+    }
+
+    /**
+     * @dev Internal: Set the master copy to use as clones for the curation token.
+     * @param _curationTokenMaster Address of implementation contract to use for curation tokens
+     */
+    function _setCurationTokenMaster(address _curationTokenMaster) private {
+        require(_curationTokenMaster != address(0), "Token master must be non-empty");
+        require(Address.isContract(_curationTokenMaster), "Token master must be a contract");
+
+        curationTokenMaster = _curationTokenMaster;
+        emit ParameterUpdated("curationTokenMaster");
+    }
+
     /**
      * @dev Assign Graph Tokens collected as curation fees to the curation pool reserve.
      * This function can only be called by the Staking contract and will do the bookeeping of
@@ -208,36 +233,27 @@ contract Curation is CurationV1Storage, GraphUpgradeable, ICuration {
 
         // If it hasn't been curated before then initialize the curve
         if (!isCurated(_subgraphDeploymentID)) {
-            // Initialize
             curationPool.reserveRatio = defaultReserveRatio;
 
             // If no signal token for the pool - create one
             if (address(curationPool.gcs) == address(0)) {
-                // TODO: Use a minimal proxy to reduce gas cost
-                // https://github.com/graphprotocol/contracts/issues/405
-                // --abarmat-- 20201113
-                curationPool.gcs = IGraphCurationToken(
-                    address(new GraphCurationToken(address(this)))
-                );
+                // Use a minimal proxy to reduce gas cost
+                IGraphCurationToken gcs = IGraphCurationToken(Clones.clone(curationTokenMaster));
+                gcs.initialize(address(this));
+                curationPool.gcs = gcs;
             }
         }
 
         // Trigger update rewards calculation snapshot
         _updateRewards(_subgraphDeploymentID);
 
         // Transfer tokens from the curator to this contract
-        // This needs to happen after _updateRewards snapshot as that function
+        // Burn the curation tax
+        // NOTE: This needs to happen after _updateRewards snapshot as that function
         // is using balanceOf(curation)
-        IGraphToken graphToken = graphToken();
-        require(
-            graphToken.transferFrom(curator, address(this), _tokensIn),
-            "Cannot transfer tokens to deposit"
-        );
-
-        // Burn withdrawal fees
-        if (curationTax > 0) {
-            graphToken.burn(curationTax);
-        }
+        IGraphToken _graphToken = graphToken();
+        TokenUtils.pullTokens(_graphToken, curator, _tokensIn);
+        TokenUtils.burnTokens(_graphToken, curationTax);
 
         // Update curation pool
         curationPool.tokens = curationPool.tokens.add(_tokensIn.sub(curationTax));
@@ -284,13 +300,15 @@ contract Curation is CurationV1Storage, GraphUpgradeable, ICuration {
         curationPool.tokens = curationPool.tokens.sub(tokensOut);
         curationPool.gcs.burnFrom(curator, _signalIn);
 
-        // If all signal burnt delete the curation pool
+        // If all signal burnt delete the curation pool except for the
+        // curation token contract to avoid recreating it on a new mint
         if (getCurationPoolSignal(_subgraphDeploymentID) == 0) {
-            delete pools[_subgraphDeploymentID];
+            curationPool.tokens = 0;
+            curationPool.reserveRatio = 0;
         }
 
         // Return the tokens to the curator
-        require(graphToken().transfer(curator, tokensOut), "Error sending curator tokens");
+        TokenUtils.pushTokens(graphToken(), curator, tokensOut);
 
         emit Burned(curator, _subgraphDeploymentID, tokensOut, _signalIn);
 
@@ -318,10 +336,8 @@ contract Curation is CurationV1Storage, GraphUpgradeable, ICuration {
         override
         returns (uint256)
     {
-        if (address(pools[_subgraphDeploymentID].gcs) == address(0)) {
-            return 0;
-        }
-        return pools[_subgraphDeploymentID].gcs.balanceOf(_curator);
+        IGraphCurationToken gcs = pools[_subgraphDeploymentID].gcs;
+        return (address(gcs) == address(0)) ? 0 : gcs.balanceOf(_curator);
     }
 
     /**
@@ -335,10 +351,8 @@ contract Curation is CurationV1Storage, GraphUpgradeable, ICuration {
         override
         returns (uint256)
     {
-        if (address(pools[_subgraphDeploymentID].gcs) == address(0)) {
-            return 0;
-        }
-        return pools[_subgraphDeploymentID].gcs.totalSupply();
+        IGraphCurationToken gcs = pools[_subgraphDeploymentID].gcs;
+        return (address(gcs) == address(0)) ? 0 : gcs.totalSupply();
     }
 
     /**
@@ -355,14 +369,6 @@ contract Curation is CurationV1Storage, GraphUpgradeable, ICuration {
         return pools[_subgraphDeploymentID].tokens;
     }
 
-    /**
-     * @dev Get curation tax percentage
-     * @return Amount the curation tax percentage in PPM
-     */
-    function curationTaxPercentage() external view override returns (uint32) {
-        return _curationTaxPercentage;
-    }
-
     /**
      * @dev Calculate amount of signal that can be bought with tokens in a curation pool.
      * This function considers and excludes the deposit tax.
@@ -376,7 +382,7 @@ contract Curation is CurationV1Storage, GraphUpgradeable, ICuration {
         override
         returns (uint256, uint256)
     {
-        uint256 curationTax = _tokensIn.mul(uint256(_curationTaxPercentage)).div(MAX_PPM);
+        uint256 curationTax = _tokensIn.mul(uint256(curationTaxPercentage)).div(MAX_PPM);
         uint256 signalOut = _tokensToSignal(_subgraphDeploymentID, _tokensIn.sub(curationTax));
         return (signalOut, curationTax);
     }
diff --git a/contracts/curation/CurationStorage.sol b/contracts/curation/CurationStorage.sol
@@ -2,28 +2,39 @@
 
 pragma solidity ^0.7.6;
 
-import "./ICuration.sol";
 import "../governance/Managed.sol";
 
-contract CurationV1Storage is Managed {
+abstract contract CurationV1Storage is Managed, ICuration {
+    // -- Pool --
+
+    struct CurationPool {
+        uint256 tokens; // GRT Tokens stored as reserves for the subgraph deployment
+        uint32 reserveRatio; // Ratio for the bonding curve
+        IGraphCurationToken gcs; // Curation token contract for this curation pool
+    }
+
     // -- State --
 
     // Tax charged when curator deposit funds
     // Parts per million. (Allows for 4 decimal points, 999,999 = 99.9999%)
-    uint32 internal _curationTaxPercentage;
+    uint32 public override curationTaxPercentage;
 
     // Default reserve ratio to configure curator shares bonding curve
     // Parts per million. (Allows for 4 decimal points, 999,999 = 99.9999%)
     uint32 public defaultReserveRatio;
 
+    // Master copy address that holds implementation of curation token
+    // This is used as the target for GraphCurationToken clones
+    address public curationTokenMaster;
+
     // Minimum amount allowed to be deposited by curators to initialize a pool
     // This is the `startPoolBalance` for the bonding curve
     uint256 public minimumCurationDeposit;
 
-    // Bonding curve formula
+    // Bonding curve library
     address public bondingCurve;
 
     // Mapping of subgraphDeploymentID => CurationPool
     // There is only one CurationPool per SubgraphDeploymentID
-    mapping(bytes32 => ICuration.CurationPool) public pools;
+    mapping(bytes32 => CurationPool) public pools;
 }
diff --git a/contracts/curation/GraphCurationToken.sol b/contracts/curation/GraphCurationToken.sol
@@ -2,25 +2,30 @@
 
 pragma solidity ^0.7.6;
 
-import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
 
 import "../governance/Governed.sol";
 
 /**
  * @title GraphCurationToken contract
  * @dev This is the implementation of the Curation ERC20 token (GCS).
+ *
  * GCS are created for each subgraph deployment curated in the Curation contract.
  * The Curation contract is the owner of GCS tokens and the only one allowed to mint or
  * burn them. GCS tokens are transferrable and their holders can do any action allowed
  * in a standard ERC20 token implementation except for burning them.
+ *
+ * This contract is meant to be used as the implementation for Minimal Proxy clones for
+ * gas-saving purposes.
  */
-contract GraphCurationToken is ERC20, Governed {
+contract GraphCurationToken is ERC20Upgradeable, Governed {
     /**
-     * @dev Graph Curation Token Contract Constructor.
+     * @dev Graph Curation Token Contract initializer.
      * @param _owner Address of the contract issuing this token
      */
-    constructor(address _owner) ERC20("Graph Curation Share", "GCS") {
+    function initialize(address _owner) external initializer {
         Governed._initialize(_owner);
+        ERC20Upgradeable.__ERC20_init("Graph Curation Share", "GCS");
     }
 
     /**
diff --git a/contracts/curation/ICuration.sol b/contracts/curation/ICuration.sol
@@ -5,14 +5,6 @@ pragma solidity ^0.7.6;
 import "./IGraphCurationToken.sol";
 
 interface ICuration {
-    // -- Pool --
-
-    struct CurationPool {
-        uint256 tokens; // GRT Tokens stored as reserves for the subgraph deployment
-        uint32 reserveRatio; // Ratio for the bonding curve
-        IGraphCurationToken gcs; // Curation token contract for this curation pool
-    }
-
     // -- Configuration --
 
     function setDefaultReserveRatio(uint32 _defaultReserveRatio) external;
@@ -21,6 +13,8 @@ interface ICuration {
 
     function setCurationTaxPercentage(uint32 _percentage) external;
 
+    function setCurationTokenMaster(address _curationTokenMaster) external;
+
     // -- Curation --
 
     function mint(
diff --git a/contracts/curation/IGraphCurationToken.sol b/contracts/curation/IGraphCurationToken.sol
@@ -2,9 +2,11 @@
 
 pragma solidity ^0.7.6;
 
-import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
+
+interface IGraphCurationToken is IERC20Upgradeable {
+    function initialize(address _owner) external;
 
-interface IGraphCurationToken is IERC20 {
     function burnFrom(address _account, uint256 _amount) external;
 
     function mint(address _to, uint256 _amount) external;
diff --git a/graph.config.yml b/graph.config.yml
@@ -39,12 +39,13 @@ contracts:
       initialSupply: "10000000000000000000000000000" # 10,000,000,000 GRT
     calls:
       - fn: "addMinter"
-        minter: "${{RewardsManager.address}}"
+        minter: "${{RewardsManager.address}}"    
   Curation:
     proxy: true
     init:
       controller: "${{Controller.address}}"
       bondingCurve: "${{BancorFormula.address}}"
+      curationTokenMaster: "${{GraphCurationToken.address}}"
       reserveRatio: 500000 # 50% (parts per million)
       curationTaxPercentage: 25000 # 2.5% (parts per million)
       minimumCurationDeposit: "1000000000000000000" # 1 GRT
diff --git a/package.json b/package.json
@@ -25,6 +25,7 @@
     "@nomiclabs/hardhat-etherscan": "^2.1.1",
     "@nomiclabs/hardhat-waffle": "^2.0.1",
     "@openzeppelin/contracts": "^3.4.1",
+    "@openzeppelin/contracts-upgradeable": "3.4.2",
     "@openzeppelin/hardhat-upgrades": "^1.6.0",
     "@tenderly/hardhat-tenderly": "^1.0.11",
     "@typechain/ethers-v5": "^7.0.0",
diff --git a/test/curation/configuration.test.ts b/test/curation/configuration.test.ts
@@ -1,10 +1,13 @@
 import { expect } from 'chai'
+import { constants } from 'ethers'
 
 import { Curation } from '../../build/types/Curation'
 
 import { defaults } from '../lib/deployment'
 import { NetworkFixture } from '../lib/fixtures'
-import { getAccounts, toBN, Account } from '../lib/testHelpers'
+import { getAccounts, toBN, Account, randomAddress } from '../lib/testHelpers'
+
+const { AddressZero } = constants
 
 const MAX_PPM = 1000000
 
@@ -99,4 +102,29 @@ describe('Curation:Config', () => {
       await expect(tx).revertedWith('Caller must be Controller governor')
     })
   })
+
+  describe('curationTokenMaster', function () {
+    it('should set `curationTokenMaster`', async function () {
+      const newCurationTokenMaster = curation.address
+      await curation.connect(governor.signer).setCurationTokenMaster(newCurationTokenMaster)
+    })
+
+    it('reject set `curationTokenMaster` to empty value', async function () {
+      const newCurationTokenMaster = AddressZero
+      const tx = curation.connect(governor.signer).setCurationTokenMaster(newCurationTokenMaster)
+      await expect(tx).revertedWith('Token master must be non-empty')
+    })
+
+    it('reject set `curationTokenMaster` to non-contract', async function () {
+      const newCurationTokenMaster = randomAddress()
+      const tx = curation.connect(governor.signer).setCurationTokenMaster(newCurationTokenMaster)
+      await expect(tx).revertedWith('Token master must be a contract')
+    })
+
+    it('reject set `curationTokenMaster` if not allowed', async function () {
+      const newCurationTokenMaster = curation.address
+      const tx = curation.connect(me.signer).setCurationTokenMaster(newCurationTokenMaster)
+      await expect(tx).revertedWith('Caller must be Controller governor')
+    })
+  })
 })
diff --git a/test/curation/curation.test.ts b/test/curation/curation.test.ts
diff --git a/test/lib/deployment.ts b/test/lib/deployment.ts
diff --git a/test/staking/delegation.test.ts b/test/staking/delegation.test.ts
diff --git a/yarn.lock b/yarn.lock

-Original file line number
+Diff line change
         .burn(subgraphDeploymentID, signalToRedeem, expectedTokens.add(1))
       await expect(tx).revertedWith('Slippage protection')
     })
++
 +    it('should not re-deploy the curation token when signal is reset', async function () {
 +      const beforeSubgraphPool = await curation.pools(subgraphDeploymentID)
++
 +      // Burn all the signal
 +      const signalToRedeem = await curation.getCuratorSignal(curator.address, subgraphDeploymentID)
 +      const expectedTokens = tokensToDeposit
 +      await shouldBurn(signalToRedeem, expectedTokens)
++
 +      // Mint again on the same subgraph
 +      await curation.connect(curator.signer).mint(subgraphDeploymentID, tokensToDeposit, 0)
++
 +      // Check state
 +      const afterSubgraphPool = await curation.pools(subgraphDeploymentID)
 +      expect(afterSubgraphPool.gcs).eq(beforeSubgraphPool.gcs)
 +    })
   })
   describe('conservation', async function () {
-Original file line number
+Diff line change
 ): Promise<Curation> {
   // Dependency
   const bondingCurve = (await deployContract('BancorFormula', deployer)) as unknown as BancorFormula
 +  const curationTokenMaster = await deployContract('GraphCurationToken', deployer)
   // Deploy
   return network.deployContractWithProxy(
+    [
       controller,
       bondingCurve.address,
 +      curationTokenMaster.address,
       defaults.curation.reserveRatio,
       defaults.curation.curationTaxPercentage,
       defaults.curation.minimumCurationDeposit,
-Original file line number
+Diff line change
         await staking.setDelegationUnbondingPeriod('2')
         await shouldDelegate(delegator, toGRT('100'))
         await shouldUndelegate(delegator, toGRT('50'))
 -        await advanceBlock()
         await advanceToNextEpoch(epochManager) // epoch 1
         await advanceToNextEpoch(epochManager) // epoch 2
         await shouldUndelegate(delegator, toGRT('10'))
-Original file line number
+Diff line change
     "@types/sinon-chai" "^3.2.3"
     "@types/web3" "1.0.19"
 +"@openzeppelin/contracts-upgradeable@3.4.2":
 +  version "3.4.2"
 +  resolved "https://registry.yarnpkg.com/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-3.4.2.tgz#2c2a1b0fa748235a1f495b6489349776365c51b3"
 +  integrity sha512-mDlBS17ymb2wpaLcrqRYdnBAmP1EwqhOXMvqWk2c5Q1N1pm5TkiCtXM9Xzznh4bYsQBq0aIWEkFFE2+iLSN1Tw==
++
 "@openzeppelin/contracts@^3.4.1":
   version "3.4.2"
   resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-3.4.2.tgz#d81f786fda2871d1eb8a8c5a73e455753ba53527"