Remove mint from creating a name signal

Author: davekay100

contracts/GNS.sol

@@ -199,9 +199,8 @@ contract GNS is Governed, BancorFormula {
     }
 
     /**
-     * @dev Allows a graph account to publish a new subgraph, which means a new subgraph number
-     * will be used.
-     * @param _graphAccount Account that is publishing the subgraph
+     * @dev Allows a graph account to set a default name
+     * @param _graphAccount Account that is setting their name
      * @param _nameSystem Name system account already has ownership of a name in
      * @param _nameIdentifier The unique identifier that is used to identify the name in the system
      * @param _name The name being set as default
@@ -216,10 +215,9 @@ contract GNS is Governed, BancorFormula {
     }
 
     /**
-     * @dev Allows a graph account to publish a new subgraph, which means a new subgraph number
-     * will be used.
-     * @param _graphAccount Account that is publishing the subgraph
-     * @param _subgraphNumber Subgraph deployment ID of the version, linked to the name
+     * @dev Allows a graph account update the metadata of a subgraph they have published
+     * @param _graphAccount Account that owns the subgraph
+     * @param _subgraphNumber Subgraph number
      * @param _subgraphMetadata IPFS hash for the subgraph metadata
      */
     function updateSubgraphMetadata(
@@ -237,20 +235,18 @@ contract GNS is Governed, BancorFormula {
      * @param _subgraphDeploymentID Subgraph deployment ID of the version, linked to the name
      * @param _versionMetadata IPFS hash for the subgraph version metadata
      * @param _subgraphMetadata IPFS hash for the subgraph metadata
-     * @param _graphTokens Graph tokens deposited to initialze the curve
      */
     function publishNewSubgraph(
         address _graphAccount,
         bytes32 _subgraphDeploymentID,
         bytes32 _versionMetadata,
-        bytes32 _subgraphMetadata,
-        uint256 _graphTokens
+        bytes32 _subgraphMetadata
     ) external onlyGraphAccountOwner(_graphAccount) {
         uint256 subgraphNumber = graphAccountSubgraphNumbers[_graphAccount];
-        publishVersion(_graphAccount, subgraphNumber, _subgraphDeploymentID, _versionMetadata);
+        _publishVersion(_graphAccount, subgraphNumber, _subgraphDeploymentID, _versionMetadata);
         graphAccountSubgraphNumbers[_graphAccount]++;
         updateSubgraphMetadata(_graphAccount, subgraphNumber, _subgraphMetadata);
-        enableNameSignal(_graphAccount, subgraphNumber, _graphTokens);
+        _enableNameSignal(_graphAccount, subgraphNumber);
     }
 
     /**
@@ -277,8 +273,8 @@ contract GNS is Governed, BancorFormula {
             _subgraphDeploymentID != oldSubgraphDeploymentID,
             "GNS: Cannot publish a new version with the same subgraph deployment ID"
         );
-        publishVersion(_graphAccount, _subgraphNumber, _subgraphDeploymentID, _versionMetadata);
-        upgradeNameSignal(_graphAccount, _subgraphNumber, _subgraphDeploymentID);
+        _publishVersion(_graphAccount, _subgraphNumber, _subgraphDeploymentID, _versionMetadata);
+        _upgradeNameSignal(_graphAccount, _subgraphNumber, _subgraphDeploymentID);
     }
 
     /**
@@ -288,7 +284,7 @@ contract GNS is Governed, BancorFormula {
      * @param _subgraphDeploymentID Subgraph deployment ID of the version, linked to the name
      * @param _versionMetadata IPFS hash for the subgraph version metadata
      */
-    function publishVersion(
+    function _publishVersion(
         address _graphAccount,
         uint256 _subgraphNumber,
         bytes32 _subgraphDeploymentID,
@@ -324,21 +320,16 @@ contract GNS is Governed, BancorFormula {
         delete subgraphs[_graphAccount][_subgraphNumber];
         emit SubgraphDeprecated(_graphAccount, _subgraphNumber);
 
-        disableNameSignal(_graphAccount, _subgraphNumber, subgraphDeploymentID);
+        _disableNameSignal(_graphAccount, _subgraphNumber, subgraphDeploymentID);
     }
 
     /**
      * @dev Enable name signal on a graph accounts numbered subgraph, which points to a subgraph
      * deployment
      * @param _graphAccount Graph account enabling name signal
      * @param _subgraphNumber Subgraph number being used
-     * @param _graphTokens Graph tokens deposited to initialze the curve
      */
-    function enableNameSignal(
-        address _graphAccount,
-        uint256 _subgraphNumber,
-        uint256 _graphTokens
-    ) private {
+    function _enableNameSignal(address _graphAccount, uint256 _subgraphNumber) private {
         NameCurationPool storage namePool = nameSignals[_graphAccount][_subgraphNumber];
         bytes32 subgraphDeploymentID = subgraphs[_graphAccount][_subgraphNumber];
         namePool.reserveRatio = defaultReserveRatio;
@@ -349,8 +340,6 @@ contract GNS is Governed, BancorFormula {
             subgraphDeploymentID,
             defaultReserveRatio
         );
-
-        _mintNSignal(_graphAccount, _subgraphNumber, _graphTokens);
     }
 
     /**
@@ -359,7 +348,7 @@ contract GNS is Governed, BancorFormula {
      * @param _subgraphNumber Subgraph number being used
      * @param _newSubgraphDeploymentID Deployment ID being upgraded to
      */
-    function upgradeNameSignal(
+    function _upgradeNameSignal(
         address _graphAccount,
         uint256 _subgraphNumber,
         bytes32 _newSubgraphDeploymentID
@@ -453,25 +442,30 @@ contract GNS is Governed, BancorFormula {
      * @param _subgraphNumber Subgraph number
      * @param _subgraphDeploymentID Subgraph deployment ID of the deprecating subgraph
      */
-    function disableNameSignal(
+    function _disableNameSignal(
         address _graphAccount,
         uint256 _subgraphNumber,
         bytes32 _subgraphDeploymentID
     ) private {
         NameCurationPool storage namePool = nameSignals[_graphAccount][_subgraphNumber];
         uint256 vSignal = namePool.vSignal;
         namePool.vSignal = 0;
-        (uint256 tokens, uint256 withdrawalFees) = curation.burn(_subgraphDeploymentID, vSignal);
-
-        // Get the owner of the Name to reimburse the withdrawal fee
-        require(
-            token.transferFrom(_graphAccount, address(this), withdrawalFees),
-            "GNS: Error reimbursing withdrawal fees"
-        );
+        // If no nSignal, then no need to burn vSignal
+        if (namePool.nSignal != 0) {
+            (uint256 tokens, uint256 withdrawalFees) = curation.burn(
+                _subgraphDeploymentID,
+                vSignal
+            );
 
+            // Get the owner of the Name to reimburse the withdrawal fee
+            require(
+                token.transferFrom(_graphAccount, address(this), withdrawalFees),
+                "GNS: Error reimbursing withdrawal fees"
+            );
+            namePool.withdrawableGRT = tokens + withdrawalFees;
+        }
         // Set the NameCurationPool fields to make it disabled
         namePool.disabled = true;
-        namePool.withdrawableGRT = tokens + withdrawalFees;
         emit NameSignalDisabled(_graphAccount, _subgraphNumber, namePool.withdrawableGRT);
     }
 
@@ -630,10 +624,6 @@ contract GNS is Governed, BancorFormula {
         uint256 _vSignal
     ) public view returns (uint256) {
         NameCurationPool memory namePool = nameSignals[_graphAccount][_subgraphNumber];
-        require(
-            namePool.disabled == false,
-            "GNS: Name cannot be disabled to call vSignalToNSignal()"
-        );
         uint256 vSignal = _vSignal;
         uint256 nSignalInit = 0;
         uint32 reserveRatio = namePool.reserveRatio;

contracts/bancor/BancorFormula.sol

@@ -233,7 +233,8 @@ contract BancorFormula {
                 _reserveBalance > 0 &&
                 _reserveRatio > 0 &&
                 _reserveRatio <= MAX_RATIO &&
-                _sellAmount <= _supply
+                _sellAmount <= _supply,
+            "invalid parameters"
         );
 
         // special case for 0 sell amount

test/gns.test.ts

@@ -35,6 +35,7 @@ describe('GNS', () => {
 
   const tokens1000 = toGRT('1000')
   const tokens10000 = toGRT('10000')
+  const tokens100000 = toGRT('100000')
   const withdrawalPercentage = 50000
   let subgraph0: Subgraph
   let subgraph1: Subgraph
@@ -69,21 +70,15 @@ describe('GNS', () => {
     account: Account,
     graphAccount: string,
     subgraphNumber: number,
-    graphTokens: BigNumber,
     subgraphToPublish = subgraph0, // Defaults to subgraph created in before()
   ): Promise<ContractTransaction> => {
-    const signals = await gns.tokensToNSignal(graphAccount, subgraphNumber, graphTokens)
-    const vSigEstimate = signals[0]
-    const nSigEstimate = signals[1]
-
     const tx = gns
       .connect(account.signer)
       .publishNewSubgraph(
         graphAccount,
         subgraphToPublish.subgraphDeploymentID,
         subgraphToPublish.versionMetadata,
         subgraphToPublish.subgraphMetadata,
-        graphTokens,
       )
     await expect(tx)
       .emit(gns, 'SubgraphPublished')
@@ -102,17 +97,9 @@ describe('GNS', () => {
         subgraphToPublish.subgraphMetadata,
       )
 
-    const tokensVSig = await getTokensAndVSig(subgraph0.subgraphDeploymentID)
-    const tokensAfter = tokensVSig[0]
-    const vSigCuration = tokensVSig[1]
-    expect(graphTokens).eq(tokensAfter)
-
     const pool = await gns.nameSignals(graphAccount, subgraphNumber)
-    const vSigPool = pool[0]
-    const nSig = pool[1]
-    expect(vSigCuration).eq(vSigPool).eq(vSigEstimate)
-    expect(nSigEstimate).eq(nSig)
-
+    const reserveRatio = pool[3]
+    expect(reserveRatio).eq(1000000)
     return tx
   }
   const publishNewVersion = async (
@@ -332,7 +319,7 @@ describe('GNS', () => {
     const tx = gns.connect(account.signer).mintNSignal(graphAccount, subgraphNumber0, graphTokens)
     await expect(tx)
       .emit(gns, 'NSignalMinted')
-      .withArgs(graphAccount, subgraphNumber0, other.address, nSigEstimate, vSigEstimate)
+      .withArgs(graphAccount, subgraphNumber0, account.address, nSigEstimate, vSigEstimate)
 
     const tokensVSig = await getTokensAndVSig(subgraph0.subgraphDeploymentID)
     const tokensAfter = tokensVSig[0]
@@ -462,12 +449,12 @@ describe('GNS', () => {
     subgraph1 = createSubgraph(me, '1')
     defaultName = createDefaultName('graph')
     // Give some funds to the signers and approve gns contract to use funds on signers behalf
-    await grt.connect(governor.signer).mint(me.address, tokens10000)
-    await grt.connect(governor.signer).mint(other.address, tokens10000)
-    await grt.connect(me.signer).approve(gns.address, tokens10000)
-    await grt.connect(me.signer).approve(curation.address, tokens10000)
-    await grt.connect(other.signer).approve(gns.address, tokens10000)
-    await grt.connect(other.signer).approve(curation.address, tokens10000)
+    await grt.connect(governor.signer).mint(me.address, tokens100000)
+    await grt.connect(governor.signer).mint(other.address, tokens100000)
+    await grt.connect(me.signer).approve(gns.address, tokens100000)
+    await grt.connect(me.signer).approve(curation.address, tokens100000)
+    await grt.connect(other.signer).approve(gns.address, tokens100000)
+    await grt.connect(other.signer).approve(curation.address, tokens100000)
 
     // Update withdrawal fee to test the functionality of it in disableNameSignal()
     await curation.connect(governor.signer).setWithdrawalFeePercentage(withdrawalPercentage)
@@ -517,22 +504,22 @@ describe('GNS', () => {
     describe('isPublished', function () {
       it('should return if the subgraph is published', async function () {
         expect(await gns.isPublished(subgraph0.graphAccount.address, 0)).eq(false)
-        await publishNewSubgraph(me, me.address, 0, tokens1000)
+        await publishNewSubgraph(me, me.address, 0)
         expect(await gns.isPublished(subgraph0.graphAccount.address, 0)).eq(true)
       })
     })
 
     describe('publishNewSubgraph', async function () {
       it('should publish a new subgraph and first version with it', async function () {
-        await publishNewSubgraph(me, me.address, 0, tokens1000)
+        await publishNewSubgraph(me, me.address, 0)
         // State updated
         const deploymentID = await gns.subgraphs(subgraph0.graphAccount.address, 0)
         expect(subgraph0.subgraphDeploymentID).eq(deploymentID)
       })
 
       it('should publish a new subgraph with an incremented value', async function () {
-        await publishNewSubgraph(me, me.address, 0, tokens1000)
-        await publishNewSubgraph(me, me.address, 1, tokens1000, subgraph1)
+        await publishNewSubgraph(me, me.address, 0)
+        await publishNewSubgraph(me, me.address, 1, subgraph1)
         const deploymentID = await gns.subgraphs(subgraph1.graphAccount.address, 1)
         expect(subgraph1.subgraphDeploymentID).eq(deploymentID)
       })
@@ -545,7 +532,6 @@ describe('GNS', () => {
             ethers.constants.HashZero,
             subgraph0.versionMetadata,
             subgraph0.subgraphMetadata,
-            tokens1000,
           )
         await expect(tx).revertedWith('GNS: Only graph account owner can call')
       })
@@ -558,15 +544,15 @@ describe('GNS', () => {
             ethers.constants.HashZero,
             subgraph0.versionMetadata,
             subgraph0.subgraphMetadata,
-            tokens1000,
           )
         await expect(tx).revertedWith('GNS: Cannot set deploymentID to 0 in publish')
       })
     })
 
     describe('publishNewVersion', async function () {
       beforeEach(async () => {
-        await publishNewSubgraph(me, me.address, 0, tokens1000)
+        await publishNewSubgraph(me, me.address, 0)
+        await mintNSignal(me, me.address, 0, tokens10000)
       })
       it('should publish a new version on an existing subgraph', async function () {
         await publishNewVersion(me, me.address, 0, subgraph1)
@@ -655,7 +641,8 @@ describe('GNS', () => {
 
     describe('deprecateSubgraph', async function () {
       beforeEach(async () => {
-        await publishNewSubgraph(me, me.address, 0, tokens1000)
+        await publishNewSubgraph(me, me.address, 0)
+        await mintNSignal(me, me.address, 0, tokens10000)
       })
       it('should deprecate a subgraph', async function () {
         await deprecateSubgraph(me, me.address, 0)
@@ -693,11 +680,11 @@ describe('GNS', () => {
     const subgraphNumber0 = 0
     describe('mintNSignal()', async function () {
       it('should deposit into the name signal curve', async function () {
-        await publishNewSubgraph(me, me.address, subgraphNumber0, tokens1000)
+        await publishNewSubgraph(me, me.address, subgraphNumber0)
         await mintNSignal(other, me.address, subgraphNumber0, tokens10000)
       })
       it('should fail when name signal is disabled', async function () {
-        await publishNewSubgraph(me, me.address, subgraphNumber0, tokens1000)
+        await publishNewSubgraph(me, me.address, subgraphNumber0)
         await deprecateSubgraph(me, me.address, 0)
         const tx = gns.connect(me.signer).mintNSignal(me.address, subgraphNumber0, tokens1000)
         await expect(tx).revertedWith('GNS: Cannot be disabled')
@@ -709,7 +696,7 @@ describe('GNS', () => {
     })
     describe('burnNSignal()', async function () {
       beforeEach(async () => {
-        await publishNewSubgraph(me, me.address, subgraphNumber0, tokens1000)
+        await publishNewSubgraph(me, me.address, subgraphNumber0)
         await mintNSignal(other, me.address, subgraphNumber0, tokens10000)
       })
       it('should withdraw from the name signal curve', async function () {
@@ -733,7 +720,7 @@ describe('GNS', () => {
     })
     describe('withdraw()', async function () {
       beforeEach(async () => {
-        await publishNewSubgraph(me, me.address, subgraphNumber0, tokens1000)
+        await publishNewSubgraph(me, me.address, subgraphNumber0)
         await mintNSignal(other, me.address, subgraphNumber0, tokens10000)
       })
       it('should withdraw GRT from a disabled name signal', async function () {
@@ -747,13 +734,11 @@ describe('GNS', () => {
       it('should fail when there is no more GRT to withdraw', async function () {
         await deprecateSubgraph(me, me.address, 0)
         await withdraw(other, me.address, subgraphNumber0)
-        await withdraw(me, me.address, subgraphNumber0)
         const tx = gns.connect(other.signer).withdraw(me.address, subgraphNumber0)
         await expect(tx).revertedWith('GNS: No more GRT to withdraw')
       })
       it('should fail if the curator has no nSignal', async function () {
         await deprecateSubgraph(me, me.address, 0)
-        await withdraw(me, me.address, subgraphNumber0)
         const tx = gns.connect(me.signer).withdraw(me.address, subgraphNumber0)
         await expect(tx).revertedWith('GNS: Curator must have some nSignal to withdraw GRT')
       })