curation: refactor of signalToTokens to return withdrawal fee (#254)

* curation: update signalToToken to return withdrawal fee

* Fix tests failing from not considering fees

* Fix failing tests on node 10

Co-authored-by: David Kajpust <[email protected]>

Author: abarmat

contracts/GNS.sol

@@ -555,7 +555,7 @@ contract GNS is Governed, BancorFormula {
     }
 
     /**
-     * @dev Calculations for buying name signal
+     * @dev Calculations for burning name signal
      * @param _graphAccount Subgraph owner
      * @param _subgraphNumber Subgraph owners subgraph number which was curated on by nameCurators
      * @param _nSignal nSignal being burnt to receive vSignal to be burnt into GRT
@@ -576,8 +576,8 @@ contract GNS is Governed, BancorFormula {
     }
 
     /**
-     * @dev Calculations burning vSignal from disabled or upgrade, and takes the withdrawal fee
-     * from the name curator owner so they cannot grief all the name curators stake
+     * @dev Calculations burning vSignal from disabled or upgrade, while keeping n signal constant.
+     * Takes the withdrawal fee from the name owner so they cannot grief all the name curators
      * @param _graphAccount Subgraph owner
      * @param _subgraphDeploymentID Subgraph deployment to burn all vSignal from
      * @param _vSignal vSignal being burnt
@@ -625,11 +625,22 @@ contract GNS is Governed, BancorFormula {
         address _graphAccount,
         uint256 _subgraphNumber,
         uint256 _nSignal
-    ) public view returns (uint256, uint256) {
+    )
+        public
+        view
+        returns (
+            uint256,
+            uint256,
+            uint256
+        )
+    {
         NameCurationPool memory namePool = nameSignals[_graphAccount][_subgraphNumber];
         uint256 vSignal = nSignalToVSignal(_graphAccount, _subgraphNumber, _nSignal);
-        uint256 tokens = curation.signalToTokens(namePool.subgraphDeploymentID, vSignal);
-        return (vSignal, tokens);
+        (uint256 tokens, uint256 withdrawalFees) = curation.signalToTokens(
+            namePool.subgraphDeploymentID,
+            vSignal
+        );
+        return (vSignal, tokens, withdrawalFees);
     }
 
     /**

contracts/curation/Curation.sol

@@ -206,8 +206,7 @@ contract Curation is CurationV1Storage, ICuration, Governed {
         );
 
         // Stake tokens to a curation pool reserve
-        uint256 signal = _mint(curator, _subgraphDeploymentID, _tokens);
-        return signal;
+        return _mint(curator, _subgraphDeploymentID, _tokens);
     }
 
     /**
@@ -231,17 +230,19 @@ contract Curation is CurationV1Storage, ICuration, Governed {
         );
 
         // Update balance and get the amount of tokens to refund based on returned signal
-        uint256 tokens = _burnSignal(curator, _subgraphDeploymentID, _signal);
+        (uint256 tokens, uint256 withdrawalFees) = _burnSignal(
+            curator,
+            _subgraphDeploymentID,
+            _signal
+        );
 
         // If all signal burnt delete the curation pool
         if (getCurationPoolSignal(_subgraphDeploymentID) == 0) {
             delete pools[_subgraphDeploymentID];
         }
 
-        // Calculate withdrawal fees and burn the tokens
-        uint256 withdrawalFees = uint256(withdrawalFeePercentage).mul(tokens).div(MAX_PPM);
+        // Burn withdrawal fees
         if (withdrawalFees > 0) {
-            tokens = tokens.sub(withdrawalFees);
             token.burn(withdrawalFees);
         }
 
@@ -332,13 +333,13 @@ contract Curation is CurationV1Storage, ICuration, Governed {
      * @dev Calculate number of tokens to get when burning signal from a curation pool.
      * @param _subgraphDeploymentID Subgraph deployment to burn signal
      * @param _signal Amount of signal to burn
-     * @return Amount of tokens to get after burning signal
+     * @return Amount of tokens to get after burning signal and withdrawal fees
      */
     function signalToTokens(bytes32 _subgraphDeploymentID, uint256 _signal)
         public
         override
         view
-        returns (uint256)
+        returns (uint256, uint256)
     {
         CurationPool memory curationPool = pools[_subgraphDeploymentID];
         uint256 curationPoolSignal = getCurationPoolSignal(_subgraphDeploymentID);
@@ -350,13 +351,16 @@ contract Curation is CurationV1Storage, ICuration, Governed {
             curationPoolSignal >= _signal,
             "Signal must be above or equal to signal issued in the curation pool"
         );
-        return
-            calculateSaleReturn(
-                curationPoolSignal,
-                curationPool.tokens,
-                uint32(curationPool.reserveRatio),
-                _signal
-            );
+
+        uint256 tokens = calculateSaleReturn(
+            curationPoolSignal,
+            curationPool.tokens,
+            curationPool.reserveRatio,
+            _signal
+        );
+        uint256 withdrawalFees = tokens.mul(uint256(withdrawalFeePercentage)).div(MAX_PPM);
+
+        return (tokens.sub(withdrawalFees), withdrawalFees);
     }
 
     /**
@@ -388,23 +392,23 @@ contract Curation is CurationV1Storage, ICuration, Governed {
      * @param _curator Curator address
      * @param _subgraphDeploymentID Subgraph deployment pool to burn signal
      * @param _signal Amount of signal to burn
-     * @return Number of tokens received
+     * @return Number of tokens received and withdrawal fees
      */
     function _burnSignal(
         address _curator,
         bytes32 _subgraphDeploymentID,
         uint256 _signal
-    ) private returns (uint256) {
+    ) private returns (uint256, uint256) {
         CurationPool storage curationPool = pools[_subgraphDeploymentID];
-        uint256 tokens = signalToTokens(_subgraphDeploymentID, _signal);
+        (uint256 tokens, uint256 withdrawalFees) = signalToTokens(_subgraphDeploymentID, _signal);
 
         // Update GRT tokens held as reserves
-        curationPool.tokens = curationPool.tokens.sub(tokens);
+        curationPool.tokens = curationPool.tokens.sub(tokens.add(withdrawalFees));
 
         // Burn signal from curator
         curationPool.gst.burnFrom(_curator, _signal);
 
-        return tokens;
+        return (tokens, withdrawalFees);
     }
 
     /**

contracts/curation/ICuration.sol

@@ -40,5 +40,5 @@ interface ICuration {
     function signalToTokens(bytes32 _subgraphDeploymentID, uint256 _signal)
         external
         view
-        returns (uint256);
+        returns (uint256, uint256);
 }

scripts/flatten

@@ -1,12 +1,14 @@
 #!/bin/bash
 
 OUT_DIR="build/full"
-IFS='/'
 
+echo ${OUT_DIR}/contracts
 mkdir -p ${OUT_DIR}/contracts
 
 echo "Flattening contracts..."
 
+IFS='/'
+
 for path in contracts/**/*.sol; do
     parts=( $path )
     name=${parts[${#parts[@]}-1]}

test/curation/curation.test.ts

@@ -99,7 +99,7 @@ describe('Curation', () => {
 
     // Calculations
     const withdrawalFeePercentage = await curation.withdrawalFeePercentage()
-    const withdrawalFees = toBN(withdrawalFeePercentage).mul(expectedTokens).div(toBN(MAX_PPM))
+    const withdrawalFees = expectedTokens.mul(toBN(withdrawalFeePercentage)).div(toBN(MAX_PPM))
 
     // Redeem
     const tx = curation.connect(curator.signer).burn(subgraphDeploymentID, signalToRedeem)
@@ -194,8 +194,29 @@ describe('Curation', () => {
 
       // Conversion
       const signal = await curation.getCurationPoolSignal(subgraphDeploymentID)
-      const tokens = await curation.signalToTokens(subgraphDeploymentID, signal)
-      expect(tokens).eq(tokensToStake)
+      const { 0: expectedTokens } = await curation.signalToTokens(subgraphDeploymentID, signal)
+      expect(expectedTokens).eq(tokensToStake)
+    })
+
+    it('convert signal to tokens (with withdrawal fees)', async function () {
+      // Set fees for withdrawal
+      const withdrawalFeePercentage = 50000 // 5%
+      await curation.connect(governor.signer).setWithdrawalFeePercentage(withdrawalFeePercentage)
+
+      // Curate
+      await curation.connect(curator.signer).mint(subgraphDeploymentID, tokensToStake)
+
+      // Expected
+      const expectedWithdrawalFees = tokensToStake.mul(withdrawalFeePercentage).div(MAX_PPM)
+
+      // Conversion
+      const signal = await curation.getCurationPoolSignal(subgraphDeploymentID)
+      const { 0: tokens, 1: withdrawalFees } = await curation.signalToTokens(
+        subgraphDeploymentID,
+        signal,
+      )
+      expect(tokens).eq(tokensToStake.sub(expectedWithdrawalFees))
+      expect(withdrawalFees).eq(expectedWithdrawalFees)
     })
 
     it('convert tokens to signal', async function () {
@@ -290,7 +311,10 @@ describe('Curation', () => {
       // Redeem "almost" all signal
       const signal = await curation.getCuratorSignal(curator.address, subgraphDeploymentID)
       const signalToRedeem = signal.sub(toGRT('0.000001'))
-      const expectedTokens = await curation.signalToTokens(subgraphDeploymentID, signalToRedeem)
+      const { 0: expectedTokens } = await curation.signalToTokens(
+        subgraphDeploymentID,
+        signalToRedeem,
+      )
       await shouldRedeem(signalToRedeem, expectedTokens)
 
       // The pool should have less tokens that required by minimumCurationStake
@@ -304,7 +328,8 @@ describe('Curation', () => {
     })
 
     it('should allow to redeem and account for withdrawal fees', async function () {
-      await curation.connect(governor.signer).setWithdrawalFeePercentage(50000)
+      // Set fees for withdrawal
+      await curation.connect(governor.signer).setWithdrawalFeePercentage(50000) // 5%
 
       // Get all signal of the curator
       const signalToRedeem = await curation.getCuratorSignal(curator.address, subgraphDeploymentID)