curation: add external function to set token master copy and avoid re-deploy the clone on minting reset

Author: abarmat

contracts/curation/Curation.sol

@@ -163,7 +163,14 @@ contract Curation is CurationV1Storage, GraphUpgradeable {
         emit ParameterUpdated("curationTaxPercentage");
     }
 
-    // TODO: add public version of this
+    /**
+     * @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
@@ -226,11 +233,9 @@ contract Curation is CurationV1Storage, GraphUpgradeable {
 
         // 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
-            // TODO: review if we can avoid re-deploying if was previously created
             if (address(curationPool.gcs) == address(0)) {
                 // Use a minimal proxy to reduce gas cost
                 IGraphCurationToken gcs = IGraphCurationToken(Clones.clone(curationTokenMaster));
@@ -295,9 +300,11 @@ contract Curation is CurationV1Storage, GraphUpgradeable {
         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

contracts/curation/GraphCurationToken.sol

@@ -7,13 +7,16 @@ import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
 import "../governance/Governed.sol";
 
 /**
- * TODO: update to reflect that it is now a cloneable
  * @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 ERC20Upgradeable, Governed {
     /**

contracts/curation/ICuration.sol

@@ -13,6 +13,8 @@ interface ICuration {
 
     function setCurationTaxPercentage(uint32 _percentage) external;
 
+    function setCurationTokenMaster(address _curationTokenMaster) external;
+
     // -- Curation --
 
     function mint(

contracts/governance/Managed.sol

@@ -56,12 +56,12 @@ contract Managed {
         require(msg.sender == address(controller), "Caller must be Controller");
     }
 
-    modifier notPartialPaused {
+    modifier notPartialPaused() {
         _notPartialPaused();
         _;
     }
 
-    modifier notPaused {
+    modifier notPaused() {
         _notPaused();
         _;
     }

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')
+    })
+  })
 })

test/curation/curation.test.ts

@@ -456,6 +456,22 @@ describe('Curation', () => {
         .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 () {