Fix owner tax percentage so it adds slightly more. Previous fix did not work

Author: davekay100

contracts/discovery/GNS.sol

@@ -371,7 +371,6 @@ contract GNS is GNSV1Storage, GraphUpgradeable, IGNS {
 
         // Take the owner cut of the curation tax, add it to the total
         uint32 curationTaxPercentage = curation.getCurationTaxPercentage();
-
         uint256 tokensWithTax = _chargeOwnerTax(tokens, _graphAccount, curationTaxPercentage);
 
         // Update pool: constant nSignal, vSignal can change
@@ -547,21 +546,32 @@ contract GNS is GNSV1Storage, GraphUpgradeable, IGNS {
             return 0;
         }
 
-        // calculate tax on current tokens
-        uint256 topUpTax = _tokens.mul(_curationTaxPercentage).div(MAX_PPM);
-        // take full tax, and multiply it by owner percentage to
-        // find the amount owner needs to add
-        uint256 ownerTax = topUpTax.mul(ownerTaxPercentage).div(MAX_PPM);
+        // Tax on the total bonding curve funds
+        uint256 taxOnOriginal = _tokens.mul(_curationTaxPercentage).div(MAX_PPM);
+        // Total after the tax
+        uint256 totalWithoutOwnerTax = _tokens.sub(taxOnOriginal);
+        // The portion of tax that the owner will pay
+        uint256 ownerTax = taxOnOriginal.mul(ownerTaxPercentage).div(MAX_PPM);
+
+        uint256 totalWithOwnerTax = totalWithoutOwnerTax.add(ownerTax);
+
+        // The total after tax, plus owner partial repay, divided by
+        // the tax, to adjust it slightly upwards. ex:
+            // 100 GRT, 5 GRT Tax, owner pays 100% --> 5 GRT
+            // To get 100 in the protocol after tax, Owner deposits
+            // ~5.26, as ~105.26 * .95 = 100
+        uint256 totalAdjustedUp = totalWithOwnerTax.mul(MAX_PPM).div(
+            uint256(MAX_PPM).sub(uint256(_curationTaxPercentage))
+        );
+
+        uint256 ownerTaxAdjustedUp = totalAdjustedUp.sub(_tokens);
 
         // Get the owner of the subgraph to reimburse the curation tax
         require(
-            graphToken().transferFrom(_owner, address(this), ownerTax),
+            graphToken().transferFrom(_owner, address(this), ownerTaxAdjustedUp),
             "GNS: Error reimbursing curation tax"
         );
-
-        // new amount is the total tokens before plus the owner tax
-        uint256 totalSentToCuration = _tokens.add(ownerTax);
-        return totalSentToCuration;
+        return totalAdjustedUp;
     }
 
     /**

test/gns.test.ts

@@ -173,6 +173,7 @@ describe('GNS', () => {
     const namePoolBefore = await gns.nameSignals(graphAccount, subgraphNumber)
 
     // Check what selling all nSignal, which == selling all vSignal, should return for tokens
+    // NOTE - no tax on burning on nSignal
     const { 1: tokensReceivedEstimate } = await gns.nSignalToTokens(
       graphAccount,
       subgraphNumber,
@@ -182,24 +183,34 @@ describe('GNS', () => {
     // Deposit 100, 5 is taxed, 95 GRT in curve
     // Upgrade - calculate 5% tax on 95 --> 4.75 GRT
     // Multiple by ownerPercentage --> 50% * 4.75 = 2.375 GRT
-    // Owner adds 2.375 to the amount, we deposit 97.375 GRT into the curve
-    // In the end there will be 92.5 GRT left after the taxation of
-    // the upgrade
+    // Owner adds 2.375 to 90.25, we deposit 92.625 GRT into the curve
+    // Divide this by 0.95 to get exactly 97.5 total tokens to be deposited
 
     // nSignalToTokens returns the amount of tokens with tax removed
     // already. So we must add in the tokens removed
     const MAX_PPM = 1000000
-    const ownerFullTax = tokensReceivedEstimate.mul(curationTaxPercentage).div(MAX_PPM)
+    const taxOnOriginal = tokensReceivedEstimate.mul(curationTaxPercentage).div(MAX_PPM)
+    const totalWithoutOwnerTax = tokensReceivedEstimate.sub(taxOnOriginal)
+    const ownerTax = taxOnOriginal.mul(ownerTaxPercentage).div(MAX_PPM)
 
-    const ownerTax = ownerFullTax.mul(ownerTaxPercentage).div(MAX_PPM)
-    const upgradeTokenReturn = tokensReceivedEstimate.add(ownerTax)
+    const totalWithOwnerTax = totalWithoutOwnerTax.add(ownerTax)
+
+    const totalAdjustedUp = totalWithOwnerTax.mul(MAX_PPM).div(MAX_PPM - curationTaxPercentage)
 
     // Re-estimate amount of signal to get considering the owner tax paid by the owner
     const { 0: newVSignalEstimate, 1: newCurationTaxEstimate } = await curation.tokensToSignal(
       subgraphToPublish.subgraphDeploymentID,
-      upgradeTokenReturn,
+      totalAdjustedUp,
     )
 
+    console.log('taxOnOriginal: ', taxOnOriginal.toString())
+    console.log('totalWithoutOwnerTax: ', totalWithoutOwnerTax.toString())
+    console.log('ownerTax: ', ownerTax.toString())
+    console.log('totalWithOwnerTax: ', totalWithOwnerTax.toString())
+    console.log('total adj up: ', totalAdjustedUp.toString())
+
+    console.log('total adj up: ', totalAdjustedUp.toString())
+
     const tx = gns
       .connect(account.signer)
       .publishNewVersion(
@@ -221,7 +232,7 @@ describe('GNS', () => {
         graphAccount,
         subgraphNumber,
         newVSignalEstimate,
-        upgradeTokenReturn,
+        totalAdjustedUp,
         subgraphToPublish.subgraphDeploymentID,
       )
 
@@ -236,7 +247,7 @@ describe('GNS', () => {
     const [tokensAfterNewCurve, vSignalAfterNewCurve] = await getTokensAndVSignal(
       subgraphToPublish.subgraphDeploymentID,
     )
-    expect(tokensAfterNewCurve).eq(upgradeTokenReturn.sub(newCurationTaxEstimate))
+    expect(tokensAfterNewCurve).eq(totalAdjustedUp.sub(newCurationTaxEstimate))
     expect(vSignalAfterNewCurve).eq(newVSignalEstimate)
 
     // Check the nSignal pool